Re: [PATCH v8.1 3/6] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Fri, 24 Apr 2020 16:43:31 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> The VIDIOC_ENUM_FMT ioctl enumerates all formats supported by a video
> node. For MC-centric devices, its behaviour has always been ill-defined,
> with drivers implementing one of the following behaviours:
> 

...

The patch itself is OK. However, there's a change you did at the
documentation that it is unrelated. 

See below.

> diff --git a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> index 8ca6ab701e4a..a987debc7654 100644
> --- a/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-enum-fmt.rst
> @@ -48,10 +48,21 @@ one until ``EINVAL`` is returned. If applicable, drivers shall return
>  formats in preference order, where preferred formats are returned before
>  (that is, with lower ``index`` value) less-preferred formats.
>  
> -.. note::
> -   After switching input or output the list of enumerated image
> -   formats may be different.

Why are you dropping this note?

This basically reverts this changeset:

  commit 93828d6438081649e81b8681df9bf6ad5d691650
  Author: Hans Verkuil <hans.verkuil@cisco.com>
  Date:   Mon Sep 3 09:37:18 2012 -0300

    [media] DocBook: make the G/S/TRY_FMT specification more strict
    
    - S/TRY_FMT should always succeed, unless an invalid type field is passed in.
    - TRY_FMT should give the same result as S_FMT, all other things being equal.
    - ENUMFMT may return different formats for different inputs or outputs.
    This was decided during the 2012 Media Workshop.
    
    Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
    Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
    Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
    Acked-by: Sakari Ailus <sakari.ailus@iki.fi>
    Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

As far as I remember, from our 2012 discussions, some drivers may change 
the enumerated image formats when some ioctls like VIDIOC_S_INPUT and
VIDIOC_S_OUTPUT ioctls are used. I also vaguely remember that 90 and 270
degrees rotation would equally affect it.

Perhaps, the removal was just some mistake. If so, please re-submit
another patch without it.

Otherwise, if are there any good reasons for such change, and it won't
cause any API regressions, please place it on a separate patch, clearly
that.

... Or, maybe you felt that it won't make sense for MC-centric devices.
On such case, please improve the note stating it on a way that it would
be understandable on both MC-centric and bridge-centrid drivers. 

Thanks,
Mauro