Tomi and Sakari,
On 4/19/23 4:21 PM, Tomi Valkeinen wrote:
> On 19/04/2023 11:04, Sakari Ailus wrote:
>> Hi Bingbu,
>>
>> On Wed, Apr 19, 2023 at 03:44:25PM +0800, Bingbu Cao wrote:
>>> Sakari,
>>>
>>> On 4/12/23 8:19 PM, Sakari Ailus wrote:
>>>> Hi Bingbu,
>>>>
>>>> On Wed, Apr 12, 2023 at 07:40:19PM +0800, Bingbu Cao wrote:
>>>>>>> +static int ov01a10_set_format(struct v4l2_subdev *sd,
>>>>>>> + struct v4l2_subdev_state *sd_state,
>>>>>>> + struct v4l2_subdev_format *fmt)
>>>>>>> +{
>>>>>>> + struct ov01a10 *ov01a10 = to_ov01a10(sd);
>>>>>>> + const struct ov01a10_mode *mode;
>>>>>>> + s32 vblank_def, h_blank;
>>>>>>> +
>>>>>>> + mode = v4l2_find_nearest_size(supported_modes,
>>>>>>> + ARRAY_SIZE(supported_modes), width,
>>>>>>> + height, fmt->format.width,
>>>>>>> + fmt->format.height);
>>>>>>> +
>>>>>>> + mutex_lock(&ov01a10->mutex);
>>>>>>> + ov01a10_update_pad_format(mode, &fmt->format);
>>>>>>
>>>>>> Could you switch to the sub-device state? That is now the preferred way to
>>>>>> serialise access to e.g. the format.
>>>>>>
>>>>>> See e.g.
>>>>>> <URL:https://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git/tree/drivers/media/i2c/ov1063x.c?h=streams/work-v16>.
>>>>>>
>>>>>> The control handler's mutex doubles as a sub-device state mutex.
>>>>>
>>>>> Is it fine to use v4l2_subdev_get_fmt()? Or will it be deprecated soon?
>>>>
>>>> Sure, it's fine to use it.
>>>>
>>>
>>> I tried to use the v4l2_subdev_state_get_stream_format() in set_fmt, but I
>>> see that the pad format of camera sensor was missing in media information.
>>>
>>> - entity 105: ov01a10 3-0036 (1 pad, 1 link)
>>> type V4L2 subdev subtype Sensor flags 0
>>> device node name /dev/v4l-subdev8
>>> pad0: Source
>>> -> "Intel IPU6 CSI2 2":0 [ENABLED]
>>>
>>> And the link validation cannot work as expected, is there something
>>> missing? My kernel head is Linux 6.3-rc2.
>>>
>>> static int ov01a10_set_format(struct v4l2_subdev *sd,
>>> struct v4l2_subdev_state *sd_state,
>>> struct v4l2_subdev_format *fmt)
>>> {
>>> ...
>>> format = v4l2_subdev_state_get_stream_format(sd_state, fmt->pad,
>>> fmt->stream);
>>> if (!format) {
>>> dev_err(&client->dev, "Failed to get stream format\n");
>>> return -EINVAL;
>>> }
>>>
>>> ...
>>> ov01a10_update_pad_format(mode, &fmt->format);
>>> *format = fmt->format;
>>>
>>> return 0;
>>> }
>>
>> v4l2_subdev_state_get_stream_format() will access streams and I presume you
>> won't have any in this case (not even streams support right now as it's a
>> sensor driver with a single sub-device).
>>
>
> This perhaps gives a better example on how to migrate to subdev state:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/media/i2c/imx290.c?id=a2514b9a634ac0a2cfbc329822b8fb58ffe23a80
Got it, thank you!
>
> Tomi
>
--
Best regards,
Bingbu Cao
