2024-03-22 14:11:09

by Martin Tůma

Subject: [PATCH v4 1/3] media: mgb4: Add support for YUV image formats

From: Martin Tůma <[email protected]>

Recent mgb4 firmwares support YUV in addition to the RGB image format. Enable
YUV in the driver when the FW supports it.

Signed-off-by: Martin Tůma <[email protected]>
---
drivers/media/pci/mgb4/mgb4_io.h | 7 ++
drivers/media/pci/mgb4/mgb4_vin.c | 116 ++++++++++++++++++++++++-----
drivers/media/pci/mgb4/mgb4_vout.c | 116 ++++++++++++++++++++++++-----
3 files changed, 203 insertions(+), 36 deletions(-)

diff --git a/drivers/media/pci/mgb4/mgb4_io.h b/drivers/media/pci/mgb4/mgb4_io.h
index 8698db1be4a9..204613a6685c 100644
--- a/drivers/media/pci/mgb4/mgb4_io.h
+++ b/drivers/media/pci/mgb4/mgb4_io.h
@@ -30,4 +30,11 @@ static inline struct mgb4_frame_buffer *to_frame_buffer(struct vb2_v4l2_buffer *
return container_of(vbuf, struct mgb4_frame_buffer, vb);
}

+static inline bool has_yuv(struct mgb4_regs *video)
+{
+ u32 status = mgb4_read_reg(video, 0xD0);
+
+ return (status & (1U << 8));
+}
+
#endif
diff --git a/drivers/media/pci/mgb4/mgb4_vin.c b/drivers/media/pci/mgb4/mgb4_vin.c
index 2cd78c539889..19692e4dfb59 100644
--- a/drivers/media/pci/mgb4/mgb4_vin.c
+++ b/drivers/media/pci/mgb4/mgb4_vin.c
@@ -186,8 +186,11 @@ static int queue_setup(struct vb2_queue *q, unsigned int *nbuffers,
struct device *alloc_devs[])
{
struct mgb4_vin_dev *vindev = vb2_get_drv_priv(q);
+ struct mgb4_regs *video = &vindev->mgbdev->video;
+ u32 config = mgb4_read_reg(video, vindev->config->regs.config);
+ u32 pixelsize = (config & (1U << 16)) ? 2 : 4;
unsigned int size = (vindev->timings.bt.width + vindev->padding)
- * vindev->timings.bt.height * 4;
+ * vindev->timings.bt.height * pixelsize;

/*
* If I/O reconfiguration is in process, do not allow to start
@@ -220,9 +223,12 @@ static int buffer_init(struct vb2_buffer *vb)
static int buffer_prepare(struct vb2_buffer *vb)
{
struct mgb4_vin_dev *vindev = vb2_get_drv_priv(vb->vb2_queue);
+ struct mgb4_regs *video = &vindev->mgbdev->video;
struct device *dev = &vindev->mgbdev->pdev->dev;
+ u32 config = mgb4_read_reg(video, vindev->config->regs.config);
+ u32 pixelsize = (config & (1U << 16)) ? 2 : 4;
unsigned int size = (vindev->timings.bt.width + vindev->padding)
- * vindev->timings.bt.height * 4;
+ * vindev->timings.bt.height * pixelsize;

if (vb2_plane_size(vb, 0) < size) {
dev_err(dev, "buffer too small (%lu < %u)\n",
@@ -359,22 +365,33 @@ static int vidioc_querycap(struct file *file, void *priv,
static int vidioc_enum_fmt(struct file *file, void *priv,
struct v4l2_fmtdesc *f)
{
- if (f->index != 0)
- return -EINVAL;
+ struct mgb4_vin_dev *vindev = video_drvdata(file);
+ struct mgb4_regs *video = &vindev->mgbdev->video;

- f->pixelformat = V4L2_PIX_FMT_ABGR32;
+ if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;

- return 0;
+ if (f->index == 0) {
+ f->pixelformat = V4L2_PIX_FMT_ABGR32;
+ return 0;
+ } else if (f->index == 1 && has_yuv(video)) {
+ f->pixelformat = V4L2_PIX_FMT_YUYV;
+ return 0;
+ } else {
+ return -EINVAL;
+ }
}

static int vidioc_enum_frameintervals(struct file *file, void *priv,
struct v4l2_frmivalenum *ival)
{
struct mgb4_vin_dev *vindev = video_drvdata(file);
+ struct mgb4_regs *video = &vindev->mgbdev->video;

if (ival->index != 0)
return -EINVAL;
- if (ival->pixel_format != V4L2_PIX_FMT_ABGR32)
+ if (!(ival->pixel_format == V4L2_PIX_FMT_ABGR32 ||
+ ((has_yuv(video) && ival->pixel_format == V4L2_PIX_FMT_YUYV))))
return -EINVAL;
if (ival->width != vindev->timings.bt.width ||
ival->height != vindev->timings.bt.height)
@@ -393,13 +410,32 @@ static int vidioc_enum_frameintervals(struct file *file, void *priv,
static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
{
struct mgb4_vin_dev *vindev = video_drvdata(file);
+ struct mgb4_regs *video = &vindev->mgbdev->video;
+ u32 config = mgb4_read_reg(video, vindev->config->regs.config);
+
+ if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;

- f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
f->fmt.pix.width = vindev->timings.bt.width;
f->fmt.pix.height = vindev->timings.bt.height;
f->fmt.pix.field = V4L2_FIELD_NONE;
- f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
- f->fmt.pix.bytesperline = (f->fmt.pix.width + vindev->padding) * 4;
+
+ if (config & (1U << 16)) {
+ f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUYV;
+ if (config & (1U << 20)) {
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
+ } else {
+ if (config & (1U << 19))
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
+ else
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
+ }
+ f->fmt.pix.bytesperline = (f->fmt.pix.width + vindev->padding) * 2;
+ } else {
+ f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
+ f->fmt.pix.bytesperline = (f->fmt.pix.width + vindev->padding) * 4;
+ }
f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;

return 0;
@@ -408,14 +444,33 @@ static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
{
struct mgb4_vin_dev *vindev = video_drvdata(file);
+ struct mgb4_regs *video = &vindev->mgbdev->video;
+ u32 pixelsize;
+
+ if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;

- f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
f->fmt.pix.width = vindev->timings.bt.width;
f->fmt.pix.height = vindev->timings.bt.height;
f->fmt.pix.field = V4L2_FIELD_NONE;
- f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
- f->fmt.pix.bytesperline = max(f->fmt.pix.width * 4,
- ALIGN_DOWN(f->fmt.pix.bytesperline, 4));
+
+ if (has_yuv(video) && f->fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) {
+ pixelsize = 2;
+ if (!(f->fmt.pix.colorspace == V4L2_COLORSPACE_REC709 ||
+ f->fmt.pix.colorspace == V4L2_COLORSPACE_SMPTE170M))
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
+ } else {
+ pixelsize = 4;
+ f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
+ }
+
+ if (f->fmt.pix.bytesperline > f->fmt.pix.width * pixelsize
+ && f->fmt.pix.bytesperline < f->fmt.pix.width * pixelsize * 2)
+ f->fmt.pix.bytesperline = ALIGN(f->fmt.pix.bytesperline,
+ pixelsize);
+ else
+ f->fmt.pix.bytesperline = f->fmt.pix.width * pixelsize;
f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;

return 0;
@@ -425,13 +480,39 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
{
struct mgb4_vin_dev *vindev = video_drvdata(file);
struct mgb4_regs *video = &vindev->mgbdev->video;
+ u32 config, pixelsize;
+ int ret;

if (vb2_is_busy(&vindev->queue))
return -EBUSY;

- vidioc_try_fmt(file, priv, f);
+ ret = vidioc_try_fmt(file, priv, f);
+ if (ret < 0)
+ return ret;
+
+ config = mgb4_read_reg(video, vindev->config->regs.config);
+ if (f->fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) {
+ pixelsize = 2;
+ config |= 1U << 16;
+
+ if (f->fmt.pix.colorspace == V4L2_COLORSPACE_REC709) {
+ config |= 1U << 20;
+ config |= 1U << 19;
+ } else if (f->fmt.pix.colorspace == V4L2_COLORSPACE_SMPTE170M) {
+ config &= ~(1U << 20);
+ config |= 1U << 19;
+ } else {
+ config &= ~(1U << 20);
+ config &= ~(1U << 19);
+ }
+ } else {
+ pixelsize = 4;
+ config &= ~(1U << 16);
+ }
+ mgb4_write_reg(video, vindev->config->regs.config, config);

- vindev->padding = (f->fmt.pix.bytesperline - (f->fmt.pix.width * 4)) / 4;
+ vindev->padding = (f->fmt.pix.bytesperline - (f->fmt.pix.width
+ * pixelsize)) / pixelsize;
mgb4_write_reg(video, vindev->config->regs.padding, vindev->padding);
set_loopback_padding(vindev, vindev->padding);

@@ -467,7 +548,8 @@ static int vidioc_enum_framesizes(struct file *file, void *fh,
{
struct mgb4_vin_dev *vindev = video_drvdata(file);

- if (fsize->index != 0 || fsize->pixel_format != V4L2_PIX_FMT_ABGR32)
+ if (fsize->index != 0 || !(fsize->pixel_format == V4L2_PIX_FMT_ABGR32 ||
+ fsize->pixel_format == V4L2_PIX_FMT_YUYV))
return -EINVAL;

fsize->discrete.width = vindev->timings.bt.width;
diff --git a/drivers/media/pci/mgb4/mgb4_vout.c b/drivers/media/pci/mgb4/mgb4_vout.c
index 241353ee77a5..243fbeaaceb9 100644
--- a/drivers/media/pci/mgb4/mgb4_vout.c
+++ b/drivers/media/pci/mgb4/mgb4_vout.c
@@ -59,7 +59,11 @@ static int queue_setup(struct vb2_queue *q, unsigned int *nbuffers,
struct device *alloc_devs[])
{
struct mgb4_vout_dev *voutdev = vb2_get_drv_priv(q);
- unsigned int size;
+ struct mgb4_regs *video = &voutdev->mgbdev->video;
+ u32 config = mgb4_read_reg(video, voutdev->config->regs.config);
+ u32 pixelsize = (config & (1U << 16)) ? 2 : 4;
+ unsigned int size = (voutdev->width + voutdev->padding) * voutdev->height
+ * pixelsize;

/*
* If I/O reconfiguration is in process, do not allow to start
@@ -69,8 +73,6 @@ static int queue_setup(struct vb2_queue *q, unsigned int *nbuffers,
if (test_bit(0, &voutdev->mgbdev->io_reconfig))
return -EBUSY;

- size = (voutdev->width + voutdev->padding) * voutdev->height * 4;
-
if (*nplanes)
return sizes[0] < size ? -EINVAL : 0;
*nplanes = 1;
@@ -93,9 +95,11 @@ static int buffer_prepare(struct vb2_buffer *vb)
{
struct mgb4_vout_dev *voutdev = vb2_get_drv_priv(vb->vb2_queue);
struct device *dev = &voutdev->mgbdev->pdev->dev;
- unsigned int size;
-
- size = (voutdev->width + voutdev->padding) * voutdev->height * 4;
+ struct mgb4_regs *video = &voutdev->mgbdev->video;
+ u32 config = mgb4_read_reg(video, voutdev->config->regs.config);
+ u32 pixelsize = (config & (1U << 16)) ? 2 : 4;
+ unsigned int size = (voutdev->width + voutdev->padding) * voutdev->height
+ * pixelsize;

if (vb2_plane_size(vb, 0) < size) {
dev_err(dev, "buffer too small (%lu < %u)\n",
@@ -194,24 +198,53 @@ static int vidioc_querycap(struct file *file, void *priv,
static int vidioc_enum_fmt(struct file *file, void *priv,
struct v4l2_fmtdesc *f)
{
- if (f->index != 0)
- return -EINVAL;
+ struct mgb4_vin_dev *voutdev = video_drvdata(file);
+ struct mgb4_regs *video = &voutdev->mgbdev->video;

- f->pixelformat = V4L2_PIX_FMT_ABGR32;
+ if (f->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ return -EINVAL;

- return 0;
+ if (f->index == 0) {
+ f->pixelformat = V4L2_PIX_FMT_ABGR32;
+ return 0;
+ } else if (f->index == 1 && has_yuv(video)) {
+ f->pixelformat = V4L2_PIX_FMT_YUYV;
+ return 0;
+ } else {
+ return -EINVAL;
+ }
}

static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
{
struct mgb4_vout_dev *voutdev = video_drvdata(file);
+ struct mgb4_regs *video = &voutdev->mgbdev->video;
+ u32 config = mgb4_read_reg(video, voutdev->config->regs.config);
+
+ if (f->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ return -EINVAL;

- f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
f->fmt.pix.width = voutdev->width;
f->fmt.pix.height = voutdev->height;
f->fmt.pix.field = V4L2_FIELD_NONE;
- f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
- f->fmt.pix.bytesperline = (f->fmt.pix.width + voutdev->padding) * 4;
+
+ if (config & (1U << 16)) {
+ f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUYV;
+ if (config & (1U << 20)) {
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
+ } else {
+ if (config & (1U << 19))
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
+ else
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
+ }
+ f->fmt.pix.bytesperline = (f->fmt.pix.width + voutdev->padding) * 2;
+ } else {
+ f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
+ f->fmt.pix.bytesperline = (f->fmt.pix.width + voutdev->padding) * 4;
+ }
+
f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;

return 0;
@@ -220,14 +253,33 @@ static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
{
struct mgb4_vout_dev *voutdev = video_drvdata(file);
+ struct mgb4_regs *video = &voutdev->mgbdev->video;
+ u32 pixelsize;
+
+ if (f->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ return -EINVAL;

- f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
f->fmt.pix.width = voutdev->width;
f->fmt.pix.height = voutdev->height;
f->fmt.pix.field = V4L2_FIELD_NONE;
- f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
- f->fmt.pix.bytesperline = max(f->fmt.pix.width * 4,
- ALIGN_DOWN(f->fmt.pix.bytesperline, 4));
+
+ if (has_yuv(video) && f->fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) {
+ pixelsize = 2;
+ if (!(f->fmt.pix.colorspace == V4L2_COLORSPACE_REC709 ||
+ f->fmt.pix.colorspace == V4L2_COLORSPACE_SMPTE170M))
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
+ } else {
+ pixelsize = 4;
+ f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
+ f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
+ }
+
+ if (f->fmt.pix.bytesperline > f->fmt.pix.width * pixelsize
+ && f->fmt.pix.bytesperline < f->fmt.pix.width * pixelsize * 2)
+ f->fmt.pix.bytesperline = ALIGN(f->fmt.pix.bytesperline,
+ pixelsize);
+ else
+ f->fmt.pix.bytesperline = f->fmt.pix.width * pixelsize;
f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;

return 0;
@@ -237,13 +289,39 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
{
struct mgb4_vout_dev *voutdev = video_drvdata(file);
struct mgb4_regs *video = &voutdev->mgbdev->video;
+ u32 config, pixelsize;
+ int ret;

if (vb2_is_busy(&voutdev->queue))
return -EBUSY;

- vidioc_try_fmt(file, priv, f);
+ ret = vidioc_try_fmt(file, priv, f);
+ if (ret < 0)
+ return ret;
+
+ config = mgb4_read_reg(video, voutdev->config->regs.config);
+ if (f->fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) {
+ pixelsize = 2;
+ config |= 1U << 16;
+
+ if (f->fmt.pix.colorspace == V4L2_COLORSPACE_REC709) {
+ config |= 1U << 20;
+ config |= 1U << 19;
+ } else if (f->fmt.pix.colorspace == V4L2_COLORSPACE_SMPTE170M) {
+ config &= ~(1U << 20);
+ config |= 1U << 19;
+ } else {
+ config &= ~(1U << 20);
+ config &= ~(1U << 19);
+ }
+ } else {
+ pixelsize = 4;
+ config &= ~(1U << 16);
+ }
+ mgb4_write_reg(video, voutdev->config->regs.config, config);

- voutdev->padding = (f->fmt.pix.bytesperline - (f->fmt.pix.width * 4)) / 4;
+ voutdev->padding = (f->fmt.pix.bytesperline - (f->fmt.pix.width
+ * pixelsize)) / pixelsize;
mgb4_write_reg(video, voutdev->config->regs.padding, voutdev->padding);

return 0;
--
2.44.0

2024-04-08 09:58:27

by Hans Verkuil

Subject: Re: [PATCH v4 1/3] media: mgb4: Add support for YUV image formats

Hi Martin,

Just a few small comments:

On 22/03/2024 16:10, [email protected] wrote:
> From: Martin Tůma <[email protected]>
>
> Recent mgb4 firmwares support YUV in addition to the RGB image format. Enable
> YUV in the driver when the FW supports it.
>
> Signed-off-by: Martin Tůma <[email protected]>
> ---
> drivers/media/pci/mgb4/mgb4_io.h | 7 ++
> drivers/media/pci/mgb4/mgb4_vin.c | 116 ++++++++++++++++++++++++-----
> drivers/media/pci/mgb4/mgb4_vout.c | 116 ++++++++++++++++++++++++-----
> 3 files changed, 203 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/media/pci/mgb4/mgb4_io.h b/drivers/media/pci/mgb4/mgb4_io.h
> index 8698db1be4a9..204613a6685c 100644
> --- a/drivers/media/pci/mgb4/mgb4_io.h
> +++ b/drivers/media/pci/mgb4/mgb4_io.h
> @@ -30,4 +30,11 @@ static inline struct mgb4_frame_buffer *to_frame_buffer(struct vb2_v4l2_buffer *
> return container_of(vbuf, struct mgb4_frame_buffer, vb);
> }
>
> +static inline bool has_yuv(struct mgb4_regs *video)
> +{
> + u32 status = mgb4_read_reg(video, 0xD0);
> +
> + return (status & (1U << 8));
> +}
> +
> #endif
> diff --git a/drivers/media/pci/mgb4/mgb4_vin.c b/drivers/media/pci/mgb4/mgb4_vin.c
> index 2cd78c539889..19692e4dfb59 100644
> --- a/drivers/media/pci/mgb4/mgb4_vin.c
> +++ b/drivers/media/pci/mgb4/mgb4_vin.c
> @@ -186,8 +186,11 @@ static int queue_setup(struct vb2_queue *q, unsigned int *nbuffers,
> struct device *alloc_devs[])
> {
> struct mgb4_vin_dev *vindev = vb2_get_drv_priv(q);
> + struct mgb4_regs *video = &vindev->mgbdev->video;
> + u32 config = mgb4_read_reg(video, vindev->config->regs.config);
> + u32 pixelsize = (config & (1U << 16)) ? 2 : 4;
> unsigned int size = (vindev->timings.bt.width + vindev->padding)
> - * vindev->timings.bt.height * 4;
> + * vindev->timings.bt.height * pixelsize;
>
> /*
> * If I/O reconfiguration is in process, do not allow to start
> @@ -220,9 +223,12 @@ static int buffer_init(struct vb2_buffer *vb)
> static int buffer_prepare(struct vb2_buffer *vb)
> {
> struct mgb4_vin_dev *vindev = vb2_get_drv_priv(vb->vb2_queue);
> + struct mgb4_regs *video = &vindev->mgbdev->video;
> struct device *dev = &vindev->mgbdev->pdev->dev;
> + u32 config = mgb4_read_reg(video, vindev->config->regs.config);
> + u32 pixelsize = (config & (1U << 16)) ? 2 : 4;
> unsigned int size = (vindev->timings.bt.width + vindev->padding)
> - * vindev->timings.bt.height * 4;
> + * vindev->timings.bt.height * pixelsize;
>
> if (vb2_plane_size(vb, 0) < size) {
> dev_err(dev, "buffer too small (%lu < %u)\n",
> @@ -359,22 +365,33 @@ static int vidioc_querycap(struct file *file, void *priv,
> static int vidioc_enum_fmt(struct file *file, void *priv,
> struct v4l2_fmtdesc *f)
> {
> - if (f->index != 0)
> - return -EINVAL;
> + struct mgb4_vin_dev *vindev = video_drvdata(file);
> + struct mgb4_regs *video = &vindev->mgbdev->video;
>
> - f->pixelformat = V4L2_PIX_FMT_ABGR32;
> + if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + return -EINVAL;

No need to test for this, it is guaranteed to be correct (the v4l2 core handles
that).

>
> - return 0;
> + if (f->index == 0) {
> + f->pixelformat = V4L2_PIX_FMT_ABGR32;
> + return 0;
> + } else if (f->index == 1 && has_yuv(video)) {
> + f->pixelformat = V4L2_PIX_FMT_YUYV;
> + return 0;
> + } else {
> + return -EINVAL;
> + }
> }
>
> static int vidioc_enum_frameintervals(struct file *file, void *priv,
> struct v4l2_frmivalenum *ival)
> {
> struct mgb4_vin_dev *vindev = video_drvdata(file);
> + struct mgb4_regs *video = &vindev->mgbdev->video;
>
> if (ival->index != 0)
> return -EINVAL;
> - if (ival->pixel_format != V4L2_PIX_FMT_ABGR32)
> + if (!(ival->pixel_format == V4L2_PIX_FMT_ABGR32 ||
> + ((has_yuv(video) && ival->pixel_format == V4L2_PIX_FMT_YUYV))))
> return -EINVAL;
> if (ival->width != vindev->timings.bt.width ||
> ival->height != vindev->timings.bt.height)
> @@ -393,13 +410,32 @@ static int vidioc_enum_frameintervals(struct file *file, void *priv,
> static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
> {
> struct mgb4_vin_dev *vindev = video_drvdata(file);
> + struct mgb4_regs *video = &vindev->mgbdev->video;
> + u32 config = mgb4_read_reg(video, vindev->config->regs.config);
> +
> + if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + return -EINVAL;

No need for this test.

>
> - f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
> f->fmt.pix.width = vindev->timings.bt.width;
> f->fmt.pix.height = vindev->timings.bt.height;
> f->fmt.pix.field = V4L2_FIELD_NONE;
> - f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
> - f->fmt.pix.bytesperline = (f->fmt.pix.width + vindev->padding) * 4;
> +
> + if (config & (1U << 16)) {
> + f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUYV;
> + if (config & (1U << 20)) {
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
> + } else {
> + if (config & (1U << 19))
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
> + else
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> + }
> + f->fmt.pix.bytesperline = (f->fmt.pix.width + vindev->padding) * 2;
> + } else {
> + f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
> + f->fmt.pix.bytesperline = (f->fmt.pix.width + vindev->padding) * 4;
> + }
> f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;
>
> return 0;
> @@ -408,14 +444,33 @@ static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
> static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
> {
> struct mgb4_vin_dev *vindev = video_drvdata(file);
> + struct mgb4_regs *video = &vindev->mgbdev->video;
> + u32 pixelsize;
> +
> + if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + return -EINVAL;

No need for this test.

>
> - f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
> f->fmt.pix.width = vindev->timings.bt.width;
> f->fmt.pix.height = vindev->timings.bt.height;
> f->fmt.pix.field = V4L2_FIELD_NONE;
> - f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
> - f->fmt.pix.bytesperline = max(f->fmt.pix.width * 4,
> - ALIGN_DOWN(f->fmt.pix.bytesperline, 4));
> +
> + if (has_yuv(video) && f->fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) {
> + pixelsize = 2;
> + if (!(f->fmt.pix.colorspace == V4L2_COLORSPACE_REC709 ||
> + f->fmt.pix.colorspace == V4L2_COLORSPACE_SMPTE170M))
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> + } else {
> + pixelsize = 4;
> + f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
> + }
> +
> + if (f->fmt.pix.bytesperline > f->fmt.pix.width * pixelsize
> + && f->fmt.pix.bytesperline < f->fmt.pix.width * pixelsize * 2)

Put the '&&' at the end of the previous line instead. checkpatch warns on this.

> + f->fmt.pix.bytesperline = ALIGN(f->fmt.pix.bytesperline,
> + pixelsize);
> + else
> + f->fmt.pix.bytesperline = f->fmt.pix.width * pixelsize;
> f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;
>
> return 0;
> @@ -425,13 +480,39 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
> {
> struct mgb4_vin_dev *vindev = video_drvdata(file);
> struct mgb4_regs *video = &vindev->mgbdev->video;
> + u32 config, pixelsize;
> + int ret;
>
> if (vb2_is_busy(&vindev->queue))
> return -EBUSY;
>
> - vidioc_try_fmt(file, priv, f);
> + ret = vidioc_try_fmt(file, priv, f);
> + if (ret < 0)
> + return ret;

It is better to move the 'vb2_is_busy()' call to after the try_fmt.

That way, even if the vb2 is busy, the fmt is still updated by the
try_fmt call.

> +
> + config = mgb4_read_reg(video, vindev->config->regs.config);
> + if (f->fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) {
> + pixelsize = 2;
> + config |= 1U << 16;
> +
> + if (f->fmt.pix.colorspace == V4L2_COLORSPACE_REC709) {
> + config |= 1U << 20;
> + config |= 1U << 19;
> + } else if (f->fmt.pix.colorspace == V4L2_COLORSPACE_SMPTE170M) {
> + config &= ~(1U << 20);
> + config |= 1U << 19;
> + } else {
> + config &= ~(1U << 20);
> + config &= ~(1U << 19);
> + }
> + } else {
> + pixelsize = 4;
> + config &= ~(1U << 16);
> + }
> + mgb4_write_reg(video, vindev->config->regs.config, config);
>
> - vindev->padding = (f->fmt.pix.bytesperline - (f->fmt.pix.width * 4)) / 4;
> + vindev->padding = (f->fmt.pix.bytesperline - (f->fmt.pix.width
> + * pixelsize)) / pixelsize;
> mgb4_write_reg(video, vindev->config->regs.padding, vindev->padding);
> set_loopback_padding(vindev, vindev->padding);
>
> @@ -467,7 +548,8 @@ static int vidioc_enum_framesizes(struct file *file, void *fh,
> {
> struct mgb4_vin_dev *vindev = video_drvdata(file);
>
> - if (fsize->index != 0 || fsize->pixel_format != V4L2_PIX_FMT_ABGR32)
> + if (fsize->index != 0 || !(fsize->pixel_format == V4L2_PIX_FMT_ABGR32 ||
> + fsize->pixel_format == V4L2_PIX_FMT_YUYV))
> return -EINVAL;
>
> fsize->discrete.width = vindev->timings.bt.width;
> diff --git a/drivers/media/pci/mgb4/mgb4_vout.c b/drivers/media/pci/mgb4/mgb4_vout.c
> index 241353ee77a5..243fbeaaceb9 100644
> --- a/drivers/media/pci/mgb4/mgb4_vout.c
> +++ b/drivers/media/pci/mgb4/mgb4_vout.c
> @@ -59,7 +59,11 @@ static int queue_setup(struct vb2_queue *q, unsigned int *nbuffers,
> struct device *alloc_devs[])
> {
> struct mgb4_vout_dev *voutdev = vb2_get_drv_priv(q);
> - unsigned int size;
> + struct mgb4_regs *video = &voutdev->mgbdev->video;
> + u32 config = mgb4_read_reg(video, voutdev->config->regs.config);
> + u32 pixelsize = (config & (1U << 16)) ? 2 : 4;
> + unsigned int size = (voutdev->width + voutdev->padding) * voutdev->height
> + * pixelsize;
>
> /*
> * If I/O reconfiguration is in process, do not allow to start
> @@ -69,8 +73,6 @@ static int queue_setup(struct vb2_queue *q, unsigned int *nbuffers,
> if (test_bit(0, &voutdev->mgbdev->io_reconfig))
> return -EBUSY;
>
> - size = (voutdev->width + voutdev->padding) * voutdev->height * 4;
> -
> if (*nplanes)
> return sizes[0] < size ? -EINVAL : 0;
> *nplanes = 1;
> @@ -93,9 +95,11 @@ static int buffer_prepare(struct vb2_buffer *vb)
> {
> struct mgb4_vout_dev *voutdev = vb2_get_drv_priv(vb->vb2_queue);
> struct device *dev = &voutdev->mgbdev->pdev->dev;
> - unsigned int size;
> -
> - size = (voutdev->width + voutdev->padding) * voutdev->height * 4;
> + struct mgb4_regs *video = &voutdev->mgbdev->video;
> + u32 config = mgb4_read_reg(video, voutdev->config->regs.config);
> + u32 pixelsize = (config & (1U << 16)) ? 2 : 4;
> + unsigned int size = (voutdev->width + voutdev->padding) * voutdev->height
> + * pixelsize;
>
> if (vb2_plane_size(vb, 0) < size) {
> dev_err(dev, "buffer too small (%lu < %u)\n",
> @@ -194,24 +198,53 @@ static int vidioc_querycap(struct file *file, void *priv,
> static int vidioc_enum_fmt(struct file *file, void *priv,
> struct v4l2_fmtdesc *f)
> {
> - if (f->index != 0)
> - return -EINVAL;
> + struct mgb4_vin_dev *voutdev = video_drvdata(file);
> + struct mgb4_regs *video = &voutdev->mgbdev->video;
>
> - f->pixelformat = V4L2_PIX_FMT_ABGR32;
> + if (f->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> + return -EINVAL;

Test is not needed.

>
> - return 0;
> + if (f->index == 0) {
> + f->pixelformat = V4L2_PIX_FMT_ABGR32;
> + return 0;
> + } else if (f->index == 1 && has_yuv(video)) {
> + f->pixelformat = V4L2_PIX_FMT_YUYV;
> + return 0;
> + } else {
> + return -EINVAL;
> + }
> }
>
> static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
> {
> struct mgb4_vout_dev *voutdev = video_drvdata(file);
> + struct mgb4_regs *video = &voutdev->mgbdev->video;
> + u32 config = mgb4_read_reg(video, voutdev->config->regs.config);
> +
> + if (f->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> + return -EINVAL;

Ditto.

>
> - f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
> f->fmt.pix.width = voutdev->width;
> f->fmt.pix.height = voutdev->height;
> f->fmt.pix.field = V4L2_FIELD_NONE;
> - f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
> - f->fmt.pix.bytesperline = (f->fmt.pix.width + voutdev->padding) * 4;
> +
> + if (config & (1U << 16)) {
> + f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUYV;
> + if (config & (1U << 20)) {
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
> + } else {
> + if (config & (1U << 19))
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
> + else
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> + }
> + f->fmt.pix.bytesperline = (f->fmt.pix.width + voutdev->padding) * 2;
> + } else {
> + f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
> + f->fmt.pix.bytesperline = (f->fmt.pix.width + voutdev->padding) * 4;
> + }
> +
> f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;
>
> return 0;
> @@ -220,14 +253,33 @@ static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
> static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
> {
> struct mgb4_vout_dev *voutdev = video_drvdata(file);
> + struct mgb4_regs *video = &voutdev->mgbdev->video;
> + u32 pixelsize;
> +
> + if (f->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> + return -EINVAL;

Ditto.

>
> - f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
> f->fmt.pix.width = voutdev->width;
> f->fmt.pix.height = voutdev->height;
> f->fmt.pix.field = V4L2_FIELD_NONE;
> - f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
> - f->fmt.pix.bytesperline = max(f->fmt.pix.width * 4,
> - ALIGN_DOWN(f->fmt.pix.bytesperline, 4));
> +
> + if (has_yuv(video) && f->fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) {
> + pixelsize = 2;
> + if (!(f->fmt.pix.colorspace == V4L2_COLORSPACE_REC709 ||
> + f->fmt.pix.colorspace == V4L2_COLORSPACE_SMPTE170M))
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> + } else {
> + pixelsize = 4;
> + f->fmt.pix.pixelformat = V4L2_PIX_FMT_ABGR32;
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_RAW;
> + }
> +
> + if (f->fmt.pix.bytesperline > f->fmt.pix.width * pixelsize
> + && f->fmt.pix.bytesperline < f->fmt.pix.width * pixelsize * 2)

Move '&&' to the end of the previous line.

> + f->fmt.pix.bytesperline = ALIGN(f->fmt.pix.bytesperline,
> + pixelsize);
> + else
> + f->fmt.pix.bytesperline = f->fmt.pix.width * pixelsize;
> f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;
>
> return 0;
> @@ -237,13 +289,39 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
> {
> struct mgb4_vout_dev *voutdev = video_drvdata(file);
> struct mgb4_regs *video = &voutdev->mgbdev->video;
> + u32 config, pixelsize;
> + int ret;
>
> if (vb2_is_busy(&voutdev->queue))
> return -EBUSY;
>
> - vidioc_try_fmt(file, priv, f);
> + ret = vidioc_try_fmt(file, priv, f);
> + if (ret < 0)
> + return ret;

Move vb2_is_busy call to here.

> +
> + config = mgb4_read_reg(video, voutdev->config->regs.config);
> + if (f->fmt.pix.pixelformat == V4L2_PIX_FMT_YUYV) {
> + pixelsize = 2;
> + config |= 1U << 16;
> +
> + if (f->fmt.pix.colorspace == V4L2_COLORSPACE_REC709) {
> + config |= 1U << 20;
> + config |= 1U << 19;
> + } else if (f->fmt.pix.colorspace == V4L2_COLORSPACE_SMPTE170M) {
> + config &= ~(1U << 20);
> + config |= 1U << 19;
> + } else {
> + config &= ~(1U << 20);
> + config &= ~(1U << 19);
> + }
> + } else {
> + pixelsize = 4;
> + config &= ~(1U << 16);
> + }
> + mgb4_write_reg(video, voutdev->config->regs.config, config);
>
> - voutdev->padding = (f->fmt.pix.bytesperline - (f->fmt.pix.width * 4)) / 4;
> + voutdev->padding = (f->fmt.pix.bytesperline - (f->fmt.pix.width
> + * pixelsize)) / pixelsize;
> mgb4_write_reg(video, voutdev->config->regs.padding, voutdev->padding);
>
> return 0;

Regards,

Hans