Re: [PATCH v9 05/36] media: subdev: Add v4l2_subdev_validate_and_lock_state()

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

On 12/10/2021 17:16, Hans Verkuil wrote:
> On 05/10/2021 10:57, Tomi Valkeinen wrote:
>> All suitable subdev ops are now passed either the TRY or the ACTIVE
>> state by the v4l2 core. However, other subdev drivers can still call the
>> ops passing NULL as the state, implying the active case.
>>
>> For all current upstream drivers this doesn't matter, as they do not
>> expect to get a valid state for ACTIVE case. But future drivers which
>> support multiplexed streaming and routing will depend on getting a state
>> for both active and try cases.
>>
>> For new drivers we can mandate that the pipelines where the drivers are
>> used need to pass the state properly, or preferably, not call such
>> subdev ops at all.
>>
>> However, if an existing subdev driver is changed to support multiplexed
>> streams, the driver has to consider cases where its ops will be called
>> with NULL state. The problem can easily be solved by using the
>> v4l2_subdev_validate_and_lock_state() helper, introduced here.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   include/media/v4l2-subdev.h | 33 +++++++++++++++++++++++++++++++++
>>   1 file changed, 33 insertions(+)
>>
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index a6e8f531a396..b75eeaee492b 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -1313,4 +1313,37 @@ void v4l2_subdev_lock_state(struct v4l2_subdev_state *state);
>>    */
>>   void v4l2_subdev_unlock_state(struct v4l2_subdev_state *state);
>>   
>> +/**
>> + * v4l2_subdev_validate_and_lock_state() - Gets locked TRY or ACTIVE subdev
> 
> I think this name is confusing since it isn't validating anything.
> 
> I'd call it v4l2_subdev_lock_and_return_state().

Yes, I really didn't like the name either.

v4l2_subdev_lock_and_return_state() isn't quite perfect either, as it 
sounds like it just locks and returns the given state. But I think it's 
better than the current name, so I'll change.

  Tomi