Re: Documentation issues with some MBUS flags

[Mauro Carvalho Chehab <mchehab@s-opensource.com>]

Em Wed, 20 Jul 2016 23:45:45 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:

> Hi Mauro,
> 
> Mauro Carvalho Chehab wrote:
> > Hi Sylwester/Sakari,
> >
> > While checking the docs for the V4L2 framework, I noticed something weird
> > Related to those two flags:
> >
> > #define V4L2_MBUS_FRAME_DESC_FL_LEN_MAX                (1U << 0)
> > #define V4L2_MBUS_FRAME_DESC_FL_BLOB           (1U << 1)
> >
> > They were originally introduced by this changeset:
> >
> > commit 291031192426bfc6ad4ab2eb9fa986025a926598
> > Author: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > Date:   Thu May 17 14:33:30 2012 -0300
> >
> >      [media] V4L: Add [get/set]_frame_desc subdev callbacks
> >
> >      Add subdev callbacks for setting up parameters of the frame on media bus
> >      that are not exposed to user space directly. This is just an initial,
> >      mostly stub implementation. struct v4l2_mbus_frame_desc is intended
> >      to be extended with sub-structures specific to a particular hardware media
> >      bus. For now these new callbacks are used only to query or specify maximum
> >      size of a compressed or hybrid (container) media bus frame in octets.
> >
> >      Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >      Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >      Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> >
> > And the comments were modified by this one:
> >
> > commit 174d6a39b07f51212c23a8e30a0440598d18392c
> > Author: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Date:   Wed Dec 18 08:40:28 2013 -0300
> >
> >      [media] v4l: V4L2_MBUS_FRAME_DESC_FL_BLOB is about 1D DMA
> >
> >      V4L2_MBUS_FRAME_DESC_FL_BLOB intends to say the receiver must use 1D DMA to
> >      receive the image, as the format does not have line offsets. This typically
> >      includes all compressed formats.
> >
> >      Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >      Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >      Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> >
> > The problem is that the description of V4L2_MBUS_FRAME_DESC_FL_LEN_MAX
> > says that:
> > 	Indicates the @length field specifies maximum data length.
> >
> > But the description of the @length field says otherwise:
> > 	* @length: number of octets per frame, valid if V4L2_MBUS_FRAME_DESC_FL_BLOB
> >
> > So, I decided to take a look on what the heck is that:
> >
> > 	$ git grep V4L2_MBUS_FRAME_DESC_FL_BLOB
> > 	include/media/v4l2-subdev.h:#define V4L2_MBUS_FRAME_DESC_FL_BLOB                (1U << 1)
> > 	include/media/v4l2-subdev.h: *                    %V4L2_MBUS_FRAME_DESC_FL_BLOB.
> > 	include/media/v4l2-subdev.h: * @length: number of octets per frame, valid if V4L2_MBUS_FRAME_DESC_FL_BLOB
> >
> > (nobody is using it)
> >
> > And:
> >
> > 	$ git grep V4L2_MBUS_FRAME_DESC_FL_LEN_MAX
> > 	drivers/media/i2c/m5mols/m5mols_core.c: fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
> > 	drivers/media/i2c/m5mols/m5mols_core.c: fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
> > 	include/media/v4l2-subdev.h:#define V4L2_MBUS_FRAME_DESC_FL_LEN_MAX             (1U << 0)
> >
> > Only one driver is using it.
> >
> > So, I'm thinking if are there any reason why to keep the
> > V4L2_MBUS_FRAME_DESC_FL_BLOB, and what's the expected behavior
> > when V4L2_MBUS_FRAME_DESC_FL_LEN_MAX is found.
> >
> > Could you shed some light?  
> 
> There isn't really a problem that I could see here, except that the 
> m5mols driver should perhaps set the BLOB flag to indicate it's using 
> 1-dimensional DMA.

Hmm... why both flags should be on? I mean, if they both have the
same meaning, we can get rid of one of them.

> 
> What comes to the flags, the LEN_MAX flag indicates that the length 
> specified in the frame descriptor is the maximum length whereas the real 
> amount of data could be less than that.

Not sure if I understood. Are you saying that, instead of:

 * @length: number of octets per frame, valid if V4L2_MBUS_FRAME_DESC_FL_BLOB
 *	    is set


It should be:

 * @length: number of octets per frame, valid if V4L2_MBUS_FRAME_DESC_FL_LEN_MAX
 *	    is set

as the description of V4L2_MBUS_FRAME_DESC_FL_LEN_MAX suggests?

> The BLOB flag indicates 
> 1-dimensional DMA, but no driver (yet) uses two-dimensional DMA with 
> frame descriptors. The patch adding two-dimensional support is here:
> 
> <URL:http://git.retiisi.org.uk/?p=~sailus/linux.git;a=commitdiff;h=b701bd4160410739b165e19327bb64ce25fc509d>
> 
> AFAIR I posted it to linux-media long time ago but the set currently is 
> still unfinished. It will be needed to add support for sensor embedded 
> data, for instance. Certain newer sensors also provide more distinct 
> types of data than the usual (embedded data and image data).

Well, if the feature using it was not merged, then we can just wipe it
out, re-adding when needed (or eventually changing to something else
if the current definition is not ok).

Thanks,
Mauro