Re: [PATCH v2 11/15] media: intel/ipu6: input system video capture nodes

From: Hans de Goede <hans@hansg.org>
To: bingbu.cao@intel.com, linux-media@vger.kernel.org,
	sakari.ailus@linux.intel.com, laurent.pinchart@ideasonboard.com
Cc: andriy.shevchenko@linux.intel.com, hdegoede@redhat.com,
	ilpo.jarvinen@linux.intel.com, andreaskleist@gmail.com,
	claus.stovgaard@gmail.com, tfiga@chromium.org,
	senozhatsky@chromium.org, tomi.valkeinen@ideasonboard.com,
	bingbu.cao@linux.intel.com, tian.shu.qiu@intel.com,
	hongju.wang@intel.com
Subject: Re: [PATCH v2 11/15] media: intel/ipu6: input system video capture nodes
Date: Sun, 5 Nov 2023 17:59:31 +0100	[thread overview]
Message-ID: <855f69b8-dc33-225a-ebd3-134467d55cf1@hansg.org> (raw)
In-Reply-To: <20231024112924.3934228-12-bingbu.cao@intel.com>

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

Hi Bingbu,

On 10/24/23 13:29, bingbu.cao@intel.com wrote:
> From: Bingbu Cao <bingbu.cao@intel.com>
> 
> Register v4l2 video device and setup the vb2 queue to
> support basic video capture. Video streaming callback
> will trigger the input system driver to construct a
> input system stream configuration for firmware based on
> data type and stream ID and then queue buffers to firmware
> to do capture.
> 
> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>

While testing this new version I found a small issue in the enum_fmt implementation caused by the mbus_code filtering support I suggested.

Attached is a single-line patch which fixes this, can you please fold this into the next version ?

Regards,

Hans

[-- Attachment #2: 0001-media-ipu6-isys-video-Do-not-set-mbus_code-from-vidi.patch --]
[-- Type: text/x-patch, Size: 1465 bytes --]

From 4c3b1445f51e8123ac3fb3fbf76b2c429a1c77fa Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Sun, 5 Nov 2023 17:46:52 +0100
Subject: [PATCH] media: ipu6-isys-video: Do not set mbus_code from
 vidioc_enum_fmt_vid_cap

When enumerating userspace will call vidioc_enum_fmt_vid_cap repeatedly
with higher v4l2_fmtdesc.index values until it returns -EINVAL to
indicate all formats have been enumerated.

Some userspace code such as e.g. v4l2-ctl only fills the v4l2_fmtdesc
struct once and then only increases v4l2_fmtdesc.index for subsequent
calls.

Setting v4l2_fmtdesc.mbus_code when it was 0 changes subsequent
vidioc_enum_fmt_vid_cap from unfiltered mode to mbus-code filtered mode
breaking unfiltered enumeration.

Do not set v4l2_fmtdesc.mbus_code in unfiltered mode to fix this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
index f81934a301b4..e73a46eb7fd3 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
@@ -129,7 +129,6 @@ int ipu6_isys_vidioc_enum_fmt(struct file *file, void *fh,
 	if (!f->mbus_code) {
 		f->flags = 0;
 		f->pixelformat = ipu6_isys_pfmts[f->index].pixelformat;
-		f->mbus_code = ipu6_isys_pfmts[f->index].code;
 		return 0;
 	}
 
-- 
2.41.0

