| Message ID | 20231003120813.77726-4-sakari.ailus@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Generic line based metadata support, internal pads | expand |
Moi, On Mon, Oct 09, 2023 at 01:34:00PM +0300, Tomi Valkeinen wrote: > On 03/10/2023 15:07, Sakari Ailus wrote: > > Add two new functions, v4l2_subdev_lock_states() and > > v4l2_subdev_unclock_states(), to acquire and release the state of two > > sub-devices. They differ from calling v4l2_subdev_{un,}lock_state() so > > that if the two states share the same lock, the lock is acquired only > > once. > > > > Also use the new functions in v4l2_subdev_link_validate(). > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > drivers/media/v4l2-core/v4l2-subdev.c | 12 +++----- > > include/media/v4l2-subdev.h | 40 +++++++++++++++++++++++++++ > > 2 files changed, 44 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > > index 854f9d4db923..df9a1ae65410 100644 > > --- a/drivers/media/v4l2-core/v4l2-subdev.c > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > > @@ -1377,17 +1377,13 @@ int v4l2_subdev_link_validate(struct media_link *link) > > states_locked = sink_state && source_state; > > - if (states_locked) { > > - v4l2_subdev_lock_state(sink_state); > > - v4l2_subdev_lock_state(source_state); > > - } > > + if (states_locked) > > + v4l2_subdev_lock_states(sink_state, source_state); > > ret = v4l2_subdev_link_validate_locked(link, states_locked); > > - if (states_locked) { > > - v4l2_subdev_unlock_state(sink_state); > > - v4l2_subdev_unlock_state(source_state); > > - } > > + if (states_locked) > > + v4l2_subdev_unlock_states(sink_state, source_state); > > return ret; > > } > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > > index 7c34243ffed9..e49e8af2fb52 100644 > > --- a/include/media/v4l2-subdev.h > > +++ b/include/media/v4l2-subdev.h > > @@ -1418,6 +1418,46 @@ static inline void v4l2_subdev_unlock_state(struct v4l2_subdev_state *state) > > mutex_unlock(state->lock); > > } > > +/** > > + * v4l2_subdev_lock_states - Lock two sub-device states > > + * @state1: One subdevice state > > + * @state2: The other subdevice state > > + * > > + * Locks the state of two sub-devices. > > + * > > + * The states must be unlocked with v4l2_subdev_unlock_states() after use. > > + * > > + * This differs from calling v4l2_subdev_lock_state() on both states so that if > > + * the states share the same lock, the lock is acquired only once (so no > > + * deadlock occurs). Note that it must be ensured the locks must always be > > + * acquired in the same order. > > Either s/must/will/ or maybe "note that the locks must always be > acquired...". How about: The caller is responsible for ensuring the locks will always be acquired in the same order. > > Maybe it should be stated that state1 and state2 have to be from subdevices > that are connected via a media link and something like "the sink subdevice > must always be state 1, and the source subdevice must always be state2". That's less generic than the text in the patch. But in practice I presume this would be the case --- this is intended to be used in link validation.
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 854f9d4db923..df9a1ae65410 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -1377,17 +1377,13 @@ int v4l2_subdev_link_validate(struct media_link *link) states_locked = sink_state && source_state; - if (states_locked) { - v4l2_subdev_lock_state(sink_state); - v4l2_subdev_lock_state(source_state); - } + if (states_locked) + v4l2_subdev_lock_states(sink_state, source_state); ret = v4l2_subdev_link_validate_locked(link, states_locked); - if (states_locked) { - v4l2_subdev_unlock_state(sink_state); - v4l2_subdev_unlock_state(source_state); - } + if (states_locked) + v4l2_subdev_unlock_states(sink_state, source_state); return ret; } diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 7c34243ffed9..e49e8af2fb52 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1418,6 +1418,46 @@ static inline void v4l2_subdev_unlock_state(struct v4l2_subdev_state *state) mutex_unlock(state->lock); } +/** + * v4l2_subdev_lock_states - Lock two sub-device states + * @state1: One subdevice state + * @state2: The other subdevice state + * + * Locks the state of two sub-devices. + * + * The states must be unlocked with v4l2_subdev_unlock_states() after use. + * + * This differs from calling v4l2_subdev_lock_state() on both states so that if + * the states share the same lock, the lock is acquired only once (so no + * deadlock occurs). Note that it must be ensured the locks must always be + * acquired in the same order. + */ +static inline void v4l2_subdev_lock_states(struct v4l2_subdev_state *state1, + struct v4l2_subdev_state *state2) +{ + mutex_lock(state1->lock); + if (state1->lock != state2->lock) + mutex_lock(state2->lock); +} + +/** + * v4l2_subdev_unlock_states() - Unlock two sub-device states + * @state1: One subdevice state + * @state2: The other subdevice state + * + * Unlocks the state of two sub-devices. + * + * This differs from calling v4l2_subdev_unlock_state() on both states so that if + * the states share the same lock, the lock is released only once. + */ +static inline void v4l2_subdev_unlock_states(struct v4l2_subdev_state *state1, + struct v4l2_subdev_state *state2) +{ + mutex_unlock(state1->lock); + if (state1->lock != state2->lock) + mutex_unlock(state2->lock); +} + /** * v4l2_subdev_get_unlocked_active_state() - Checks that the active subdev state * is unlocked and returns it
Add two new functions, v4l2_subdev_lock_states() and v4l2_subdev_unclock_states(), to acquire and release the state of two sub-devices. They differ from calling v4l2_subdev_{un,}lock_state() so that if the two states share the same lock, the lock is acquired only once. Also use the new functions in v4l2_subdev_link_validate(). Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/media/v4l2-core/v4l2-subdev.c | 12 +++----- include/media/v4l2-subdev.h | 40 +++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 8 deletions(-)