Re: [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices

From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media@vger.kernel.org,
	"Helen Koike" <helen.koike@collabora.com>,
	"Hans Verkuil" <hverkuil@xs4all.nl>,
	linux-renesas-soc@vger.kernel.org,
	"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>
Subject: Re: [PATCH v6 2/5] media: v4l2: Extend VIDIOC_ENUM_FMT to support MC-centric devices
Date: Mon, 23 Mar 2020 12:12:50 +0200	[thread overview]
Message-ID: <20200323101250.GB20664@kekkonen.localdomain> (raw)
In-Reply-To: <20200323100727.GA4768@pendragon.ideasonboard.com>

On Mon, Mar 23, 2020 at 12:07:27PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Mon, Mar 23, 2020 at 12:03:28PM +0200, Sakari Ailus wrote:
> > On Thu, Mar 19, 2020 at 02:46:58AM +0200, Laurent Pinchart wrote:
> > > 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:
> > > 
> > > - No support for VIDIOC_ENUM_FMT at all
> > > - Enumerating all formats supported by the video node, regardless of the
> > >   configuration of the pipeline
> > > - Enumerating formats supported by the video node for the active
> > >   configuration of the connected subdevice
> > > 
> > > The first behaviour is obviously useless for applications. The second
> > > behaviour provides the most information, but doesn't offer a way to find
> > > what formats are compatible with a given pipeline configuration. The
> > > third behaviour fixes that, but with the drawback that applications
> > > can't enumerate all supported formats anymore, and have to modify the
> > > active configuration of the pipeline to enumerate formats.
> > > 
> > > The situation is messy as none of the implemented behaviours are ideal,
> > > and userspace can't predict what will happen as the behaviour is
> > > driver-specific.
> > > 
> > > To fix this, let's extend the VIDIOC_ENUM_FMT with a missing capability:
> > > enumerating pixel formats for a given media bus code. The media bus code
> > > is passed through the v4l2_fmtdesc structure in a new mbus_code field
> > > (repurposed from the reserved fields). With this capability in place,
> > > applications can enumerate pixel formats for a given media bus code
> > > without modifying the active configuration of the device.
> > > 
> > > The current behaviour of the ioctl is preserved when the new mbus_code
> > > field is set to 0, ensuring compatibility with existing userspace. The
> > > API extension is documented as mandatory for MC-centric devices (as
> > > advertised through the V4L2_CAP_IO_MC capability), allowing applications
> > > and compliance tools to easily determine the availability of the
> > > VIDIOC_ENUM_FMT extension.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > I'd address setting the reserved fields explicitly in a separate patch,
> > simply by removing them. As another field in the struct is assigned, the
> > memory is zeroed and explicit assignment is redundant.
> 
> I'm not sure to follow you, what code are you referring to ?

Have you seen the e-mails from the kbuild bot?

-- 
Sakari Ailus