Hi Niklas,
Thank you for the patch.
On Fri, Apr 10, 2020 at 11:54:12PM +0200, Niklas Söderlund wrote:
> Add support to filter which pixel formats are supported by a mbus_code
> provided from user-space.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 23 +++++++++++++++++++++
> 1 file changed, 23 insertions(+)
> ---
> Hi Laurent,
>
> I intend to squash this patch with the rcar-vin patch in the
> V4L2_CAP_IO_MC series. As I hope we can merged that series soon so
> I wanted to get early feedback if possible on my take on ENUM_FMT.
>
> // Niklas
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 96ddd36619167fd5..8c8866a8ee8dccfd 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -343,6 +343,29 @@ static int rvin_enum_fmt_vid_cap(struct file *file, void *priv,
> unsigned int i;
> int matched;
>
> + /*
> + * If mbus_code is set only enumerate supported pixel formats for that
> + * bus code. Converting from YCbCr to RGB and RGB to YCbCr is possible
> + * with VIN, so all supported YCbCr and RBB media bus codes can produce
s/RBB/RGB/
> + * all of the related pixel formats. If mbus_code is not set enumerate
> + * all possible pixelformats.
> + *
> + * TODO: Once raw capture formats are added to the driver this needs
> + * to be extended so raw media bus codes only result in a raw pixel
s/a raw/raw/
> + * formats.
> + */
> + switch (f->mbus_code) {
> + case 0:
> + case MEDIA_BUS_FMT_YUYV8_1X16:
> + case MEDIA_BUS_FMT_UYVY8_1X16:
> + case MEDIA_BUS_FMT_UYVY8_2X8:
> + case MEDIA_BUS_FMT_UYVY10_2X10:
> + case MEDIA_BUS_FMT_RGB888_1X24:
There are a few locations where you enumerate the supported media bus
formats (rvin_parallel_subdevice_attach(), rvin_mc_validate_format() and
rcar_csi2_formats[]), it could be nice to centralize this, at least for
the first two. Maybe a rvin_mbus_code_supported() function ? This isn't
mandatory and could be addressed later, and may actually get in the way
when we'll support additional media bus codes.
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> matched = -1;
> for (i = 0; i < ARRAY_SIZE(rvin_formats); i++) {
> if (rvin_format_from_pixel(vin, rvin_formats[i].fourcc))
--
Regards,
Laurent Pinchart
