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).
--
Kind regards,
Sakari Ailus
