soc_camera: Add V4L2_MBUS_FMT_YUYV10_2X10 format

  The V4L2_MBUS_FMT_YUYV10_2X10 format has already been added to mediabus, so
this patch just adds SoC camera support.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/media/platform/soc_camera/soc_mediabus.c |   15 +++++++++++++++
 include/media/soc_mediabus.h                     |    3 +++
 2 files changed, 18 insertions(+), 0 deletions(-)
  

Hi Phil

Thanks for the patch.

On Wed, 17 Apr 2013, Phil Edworthy wrote:

> The V4L2_MBUS_FMT_YUYV10_2X10 format has already been added to mediabus, so
> this patch just adds SoC camera support.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>  drivers/media/platform/soc_camera/soc_mediabus.c |   15 +++++++++++++++
>  include/media/soc_mediabus.h                     |    3 +++
>  2 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/drivers/media/platform/soc_camera/soc_mediabus.c
> index 7569e77..be47d41 100644
> --- a/drivers/media/platform/soc_camera/soc_mediabus.c
> +++ b/drivers/media/platform/soc_camera/soc_mediabus.c
> @@ -57,6 +57,15 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
>  		.layout			= SOC_MBUS_LAYOUT_PACKED,
>  	},
>  }, {
> +	.code = V4L2_MBUS_FMT_YUYV10_2X10,
> +	.fmt = {
> +		.fourcc			= V4L2_PIX_FMT_YUYV,
> +		.name			= "YUYV",
> +		.bits_per_sample	= 10,
> +		.packing		= SOC_MBUS_PACKING_2X10_PADHI,

Wow, what kind of host can pack two 10-bit samples into 3 bytes and write 
3-byte pixels to memory?

> +		.order			= SOC_MBUS_ORDER_LE,
> +	},
> +}, {
>  	.code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
>  	.fmt = {
>  		.fourcc			= V4L2_PIX_FMT_RGB555,
> @@ -403,6 +412,10 @@ int soc_mbus_samples_per_pixel(const struct soc_mbus_pixelfmt *mf,
>  		*numerator = 2;
>  		*denominator = 1;
>  		return 0;
> +	case SOC_MBUS_PACKING_2X10_PADHI:
> +		*numerator = 3;
> +		*denominator = 1;

Why 3? it's 2 samples per pixel, right? Should be *numerator = 2 above?

> +		return 0;
>  	case SOC_MBUS_PACKING_1_5X8:
>  		*numerator = 3;
>  		*denominator = 2;
> @@ -428,6 +441,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const struct soc_mbus_pixelfmt *mf)
>  	case SOC_MBUS_PACKING_2X8_PADLO:
>  	case SOC_MBUS_PACKING_EXTEND16:
>  		return width * 2;
> +	case SOC_MBUS_PACKING_2X10_PADHI:
> +		return width * 3;
>  	case SOC_MBUS_PACKING_1_5X8:
>  		return width * 3 / 2;
>  	case SOC_MBUS_PACKING_VARIABLE:
> diff --git a/include/media/soc_mediabus.h b/include/media/soc_mediabus.h
> index d33f6d0..b131a47 100644
> --- a/include/media/soc_mediabus.h
> +++ b/include/media/soc_mediabus.h
> @@ -21,6 +21,8 @@
>   * @SOC_MBUS_PACKING_2X8_PADHI:	16 bits transferred in 2 8-bit samples, in the
>   *				possibly incomplete byte high bits are padding
>   * @SOC_MBUS_PACKING_2X8_PADLO:	as above, but low bits are padding
> + * @SOC_MBUS_PACKING_2X10_PADHI:20 bits transferred in 2 10-bit samples. The

A TAB is missing after ":"?

> + *				high bits are padding
>   * @SOC_MBUS_PACKING_EXTEND16:	sample width (e.g., 10 bits) has to be extended
>   *				to 16 bits
>   * @SOC_MBUS_PACKING_VARIABLE:	compressed formats with variable packing
> @@ -33,6 +35,7 @@ enum soc_mbus_packing {
>  	SOC_MBUS_PACKING_NONE,
>  	SOC_MBUS_PACKING_2X8_PADHI,
>  	SOC_MBUS_PACKING_2X8_PADLO,
> +	SOC_MBUS_PACKING_2X10_PADHI,
>  	SOC_MBUS_PACKING_EXTEND16,
>  	SOC_MBUS_PACKING_VARIABLE,
>  	SOC_MBUS_PACKING_1_5X8,
> -- 
> 1.7.5.4
> 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Guennadi,

Thanks for the review!

> On Wed, 17 Apr 2013, Phil Edworthy wrote:
> 
> > The V4L2_MBUS_FMT_YUYV10_2X10 format has already been added to 
mediabus, so
> > this patch just adds SoC camera support.
> > 
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> >  drivers/media/platform/soc_camera/soc_mediabus.c |   15 
+++++++++++++++
> >  include/media/soc_mediabus.h                     |    3 +++
> >  2 files changed, 18 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/
> drivers/media/platform/soc_camera/soc_mediabus.c
> > index 7569e77..be47d41 100644
> > --- a/drivers/media/platform/soc_camera/soc_mediabus.c
> > +++ b/drivers/media/platform/soc_camera/soc_mediabus.c
> > @@ -57,6 +57,15 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
> >        .layout         = SOC_MBUS_LAYOUT_PACKED,
> >     },
> >  }, {
> > +   .code = V4L2_MBUS_FMT_YUYV10_2X10,
> > +   .fmt = {
> > +      .fourcc         = V4L2_PIX_FMT_YUYV,
> > +      .name         = "YUYV",
> > +      .bits_per_sample   = 10,
> > +      .packing      = SOC_MBUS_PACKING_2X10_PADHI,
> 
> Wow, what kind of host can pack two 10-bit samples into 3 bytes and 
write 
> 3-byte pixels to memory?
I think I might have misunderstood how this is used. From my 
understanding, the MBUS formats are used to describe the hardware 
interfaces to cameras, i.e. 2 samples of 10 bits. I guess that the fourcc 
field also determines what v4l2 format is required to capture this. 
However, I am not sure how the two relate to each other. How does the 
above code imply 3 bytes?

> > +      .order         = SOC_MBUS_ORDER_LE,
> > +   },
> > +}, {
> >     .code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
> >     .fmt = {
> >        .fourcc         = V4L2_PIX_FMT_RGB555,
> > @@ -403,6 +412,10 @@ int soc_mbus_samples_per_pixel(const struct 
> soc_mbus_pixelfmt *mf,
> >        *numerator = 2;
> >        *denominator = 1;
> >        return 0;
> > +   case SOC_MBUS_PACKING_2X10_PADHI:
> > +      *numerator = 3;
> > +      *denominator = 1;
> 
> Why 3? it's 2 samples per pixel, right? Should be *numerator = 2 above?
Not sure why I thought it should be 3, I think I had it in my head that 
this was the number of bytes needed per pixel. Clearly this is not the 
case!

> > +      return 0;
> >     case SOC_MBUS_PACKING_1_5X8:
> >        *numerator = 3;
> >        *denominator = 2;
> > @@ -428,6 +441,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const 
> struct soc_mbus_pixelfmt *mf)
> >     case SOC_MBUS_PACKING_2X8_PADLO:
> >     case SOC_MBUS_PACKING_EXTEND16:
> >        return width * 2;
> > +   case SOC_MBUS_PACKING_2X10_PADHI:
> > +      return width * 3;
> >     case SOC_MBUS_PACKING_1_5X8:
> >        return width * 3 / 2;
> >     case SOC_MBUS_PACKING_VARIABLE:
> > diff --git a/include/media/soc_mediabus.h 
b/include/media/soc_mediabus.h
> > index d33f6d0..b131a47 100644
> > --- a/include/media/soc_mediabus.h
> > +++ b/include/media/soc_mediabus.h
> > @@ -21,6 +21,8 @@
> >   * @SOC_MBUS_PACKING_2X8_PADHI:   16 bits transferred in 2 8-bit 
> samples, in the
> >   *            possibly incomplete byte high bits are padding
> >   * @SOC_MBUS_PACKING_2X8_PADLO:   as above, but low bits are padding
> > + * @SOC_MBUS_PACKING_2X10_PADHI:20 bits transferred in 2 10-bit 
> samples. The
> 
> A TAB is missing after ":"?
Ok.
 
> > + *            high bits are padding
> >   * @SOC_MBUS_PACKING_EXTEND16:   sample width (e.g., 10 bits) has
> to be extended
> >   *            to 16 bits
> >   * @SOC_MBUS_PACKING_VARIABLE:   compressed formats with variable 
packing
> > @@ -33,6 +35,7 @@ enum soc_mbus_packing {
> >     SOC_MBUS_PACKING_NONE,
> >     SOC_MBUS_PACKING_2X8_PADHI,
> >     SOC_MBUS_PACKING_2X8_PADLO,
> > +   SOC_MBUS_PACKING_2X10_PADHI,
> >     SOC_MBUS_PACKING_EXTEND16,
> >     SOC_MBUS_PACKING_VARIABLE,
> >     SOC_MBUS_PACKING_1_5X8,
> > -- 
> > 1.7.5.4

Thanks
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Phil

On Thu, 25 Apr 2013, phil.edworthy@renesas.com wrote:

> Hi Guennadi,
> 
> Thanks for the review!
> 
> > On Wed, 17 Apr 2013, Phil Edworthy wrote:
> > 
> > > The V4L2_MBUS_FMT_YUYV10_2X10 format has already been added to 
> mediabus, so
> > > this patch just adds SoC camera support.
> > > 
> > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > ---
> > >  drivers/media/platform/soc_camera/soc_mediabus.c |   15 
> +++++++++++++++
> > >  include/media/soc_mediabus.h                     |    3 +++
> > >  2 files changed, 18 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/
> > drivers/media/platform/soc_camera/soc_mediabus.c
> > > index 7569e77..be47d41 100644
> > > --- a/drivers/media/platform/soc_camera/soc_mediabus.c
> > > +++ b/drivers/media/platform/soc_camera/soc_mediabus.c
> > > @@ -57,6 +57,15 @@ static const struct soc_mbus_lookup mbus_fmt[] = {
> > >        .layout         = SOC_MBUS_LAYOUT_PACKED,
> > >     },
> > >  }, {
> > > +   .code = V4L2_MBUS_FMT_YUYV10_2X10,
> > > +   .fmt = {
> > > +      .fourcc         = V4L2_PIX_FMT_YUYV,
> > > +      .name         = "YUYV",
> > > +      .bits_per_sample   = 10,
> > > +      .packing      = SOC_MBUS_PACKING_2X10_PADHI,
> > 
> > Wow, what kind of host can pack two 10-bit samples into 3 bytes and 
> write 
> > 3-byte pixels to memory?
> I think I might have misunderstood how this is used. From my 
> understanding, the MBUS formats are used to describe the hardware 
> interfaces to cameras, i.e. 2 samples of 10 bits. I guess that the fourcc 
> field also determines what v4l2 format is required to capture this. 

No, not quite. This table describes default "pass-through" capture of 
video data on a media bus to memory. E.g. the first entry in the table 
means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the bus, you 
sample 8 bits at a time, and store the samples 1-to-1 into RAM, you get 
the V4L2_PIX_FMT_YUYV format in your buffer. It can also describe some 
standard operations with the sampled data, like swapping the order, 
filling missing high bits (e.g. if you sample 10 bits but store 16 bits 
per sample with high 6 bits nullified). The table also specifies which 
bits are used for padding in the original data, e.g. 
V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has SOC_MBUS_PACKING_2X8_PADLO, whereas 
V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has SOC_MBUS_PACKING_2X8_PADHI, which 
means, that out of 16 bits of data, that you get when you sample an 8-bit 
bus twice, either low or high 6 bits are invalid and should be discarded.

> However, I am not sure how the two relate to each other. How does the 
> above code imply 3 bytes?

Not the above code, but your entry in the soc_mbus_bytes_per_line() 
function below, where you multiply width * 3.

> > > +      .order         = SOC_MBUS_ORDER_LE,
> > > +   },
> > > +}, {
> > >     .code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
> > >     .fmt = {
> > >        .fourcc         = V4L2_PIX_FMT_RGB555,
> > > @@ -403,6 +412,10 @@ int soc_mbus_samples_per_pixel(const struct 
> > soc_mbus_pixelfmt *mf,
> > >        *numerator = 2;
> > >        *denominator = 1;
> > >        return 0;
> > > +   case SOC_MBUS_PACKING_2X10_PADHI:
> > > +      *numerator = 3;
> > > +      *denominator = 1;
> > 
> > Why 3? it's 2 samples per pixel, right? Should be *numerator = 2 above?
> Not sure why I thought it should be 3, I think I had it in my head that 
> this was the number of bytes needed per pixel. Clearly this is not the 
> case!
> 
> > > +      return 0;
> > >     case SOC_MBUS_PACKING_1_5X8:
> > >        *numerator = 3;
> > >        *denominator = 2;
> > > @@ -428,6 +441,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const 
> > struct soc_mbus_pixelfmt *mf)
> > >     case SOC_MBUS_PACKING_2X8_PADLO:
> > >     case SOC_MBUS_PACKING_EXTEND16:
> > >        return width * 2;
> > > +   case SOC_MBUS_PACKING_2X10_PADHI:
> > > +      return width * 3;
> > >     case SOC_MBUS_PACKING_1_5X8:
> > >        return width * 3 / 2;
> > >     case SOC_MBUS_PACKING_VARIABLE:
> > > diff --git a/include/media/soc_mediabus.h 
> b/include/media/soc_mediabus.h
> > > index d33f6d0..b131a47 100644
> > > --- a/include/media/soc_mediabus.h
> > > +++ b/include/media/soc_mediabus.h
> > > @@ -21,6 +21,8 @@
> > >   * @SOC_MBUS_PACKING_2X8_PADHI:   16 bits transferred in 2 8-bit 
> > samples, in the
> > >   *            possibly incomplete byte high bits are padding
> > >   * @SOC_MBUS_PACKING_2X8_PADLO:   as above, but low bits are padding
> > > + * @SOC_MBUS_PACKING_2X10_PADHI:20 bits transferred in 2 10-bit 
> > samples. The
> > 
> > A TAB is missing after ":"?
> Ok.
>  
> > > + *            high bits are padding
> > >   * @SOC_MBUS_PACKING_EXTEND16:   sample width (e.g., 10 bits) has
> > to be extended
> > >   *            to 16 bits
> > >   * @SOC_MBUS_PACKING_VARIABLE:   compressed formats with variable 
> packing
> > > @@ -33,6 +35,7 @@ enum soc_mbus_packing {
> > >     SOC_MBUS_PACKING_NONE,
> > >     SOC_MBUS_PACKING_2X8_PADHI,
> > >     SOC_MBUS_PACKING_2X8_PADLO,
> > > +   SOC_MBUS_PACKING_2X10_PADHI,
> > >     SOC_MBUS_PACKING_EXTEND16,
> > >     SOC_MBUS_PACKING_VARIABLE,
> > >     SOC_MBUS_PACKING_1_5X8,
> > > -- 
> > > 1.7.5.4
> 
> Thanks
> Phil
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Guennadi,

> > > On Wed, 17 Apr 2013, Phil Edworthy wrote:
> > > 
> > > > The V4L2_MBUS_FMT_YUYV10_2X10 format has already been added to 
> > mediabus, so
> > > > this patch just adds SoC camera support.
> > > > 
> > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > > ---
> > > >  drivers/media/platform/soc_camera/soc_mediabus.c |   15 
> > +++++++++++++++
> > > >  include/media/soc_mediabus.h                     |    3 +++
> > > >  2 files changed, 18 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/
> > > drivers/media/platform/soc_camera/soc_mediabus.c
> > > > index 7569e77..be47d41 100644
> > > > --- a/drivers/media/platform/soc_camera/soc_mediabus.c
> > > > +++ b/drivers/media/platform/soc_camera/soc_mediabus.c
> > > > @@ -57,6 +57,15 @@ static const struct soc_mbus_lookup mbus_fmt[] 
= {
> > > >        .layout         = SOC_MBUS_LAYOUT_PACKED,
> > > >     },
> > > >  }, {
> > > > +   .code = V4L2_MBUS_FMT_YUYV10_2X10,
> > > > +   .fmt = {
> > > > +      .fourcc         = V4L2_PIX_FMT_YUYV,
> > > > +      .name         = "YUYV",
> > > > +      .bits_per_sample   = 10,
> > > > +      .packing      = SOC_MBUS_PACKING_2X10_PADHI,
> > > 
> > > Wow, what kind of host can pack two 10-bit samples into 3 bytes and 
> > write 
> > > 3-byte pixels to memory?
> > I think I might have misunderstood how this is used. From my 
> > understanding, the MBUS formats are used to describe the hardware 
> > interfaces to cameras, i.e. 2 samples of 10 bits. I guess that the 
fourcc 
> > field also determines what v4l2 format is required to capture this. 
> 
> No, not quite. This table describes default "pass-through" capture of 
> video data on a media bus to memory. E.g. the first entry in the table 
> means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the bus, 
you 
> sample 8 bits at a time, and store the samples 1-to-1 into RAM, you get 
> the V4L2_PIX_FMT_YUYV format in your buffer. It can also describe some 
> standard operations with the sampled data, like swapping the order, 
> filling missing high bits (e.g. if you sample 10 bits but store 16 bits 
> per sample with high 6 bits nullified). The table also specifies which 
> bits are used for padding in the original data, e.g. 
> V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has SOC_MBUS_PACKING_2X8_PADLO, 
whereas 
> V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has SOC_MBUS_PACKING_2X8_PADHI, which 

> means, that out of 16 bits of data, that you get when you sample an 
8-bit 
> bus twice, either low or high 6 bits are invalid and should be 
discarded.

Ok, I see. However, is it necessary to provide a default pass-through v4l2 
format? I can't see a suitable v4l2 format! For the hardware I have been 
working on, there is always the option of converting the data to another 
format, so this is not really needed. I doubt that it makes sense to add 
yet another v4l2 format for userspace, when typical uses would involve the 
host hardware converting the format to something else, e.g. 
V4L2_PIX_FMT_RGB32.

> > However, I am not sure how the two relate to each other. How does the 
> > above code imply 3 bytes?
> 
> Not the above code, but your entry in the soc_mbus_bytes_per_line() 
> function below, where you multiply width * 3.

It looks like hosts use soc_mbus_bytes_per_line() to report the size of 
video buffers needed. Shouldn't the hosts report the buffer metrics for 
the v4l2 format, since that is what will be output? What has this to do 
with the MBUS specifics?

> > > > +      .order         = SOC_MBUS_ORDER_LE,
> > > > +   },
> > > > +}, {
> > > >     .code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
> > > >     .fmt = {
> > > >        .fourcc         = V4L2_PIX_FMT_RGB555,
> > > > @@ -403,6 +412,10 @@ int soc_mbus_samples_per_pixel(const struct 
> > > soc_mbus_pixelfmt *mf,
> > > >        *numerator = 2;
> > > >        *denominator = 1;
> > > >        return 0;
> > > > +   case SOC_MBUS_PACKING_2X10_PADHI:
> > > > +      *numerator = 3;
> > > > +      *denominator = 1;
> > > 
> > > Why 3? it's 2 samples per pixel, right? Should be *numerator = 2 
above?
> > Not sure why I thought it should be 3, I think I had it in my head 
that 
> > this was the number of bytes needed per pixel. Clearly this is not the 

> > case!
> > 
> > > > +      return 0;
> > > >     case SOC_MBUS_PACKING_1_5X8:
> > > >        *numerator = 3;
> > > >        *denominator = 2;
> > > > @@ -428,6 +441,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const 
> > > struct soc_mbus_pixelfmt *mf)
> > > >     case SOC_MBUS_PACKING_2X8_PADLO:
> > > >     case SOC_MBUS_PACKING_EXTEND16:
> > > >        return width * 2;
> > > > +   case SOC_MBUS_PACKING_2X10_PADHI:
> > > > +      return width * 3;
> > > >     case SOC_MBUS_PACKING_1_5X8:
> > > >        return width * 3 / 2;
> > > >     case SOC_MBUS_PACKING_VARIABLE:
> > > > diff --git a/include/media/soc_mediabus.h 
> > b/include/media/soc_mediabus.h
> > > > index d33f6d0..b131a47 100644
> > > > --- a/include/media/soc_mediabus.h
> > > > +++ b/include/media/soc_mediabus.h
> > > > @@ -21,6 +21,8 @@
> > > >   * @SOC_MBUS_PACKING_2X8_PADHI:   16 bits transferred in 2 8-bit 
> > > samples, in the
> > > >   *            possibly incomplete byte high bits are padding
> > > >   * @SOC_MBUS_PACKING_2X8_PADLO:   as above, but low bits are 
padding
> > > > + * @SOC_MBUS_PACKING_2X10_PADHI:20 bits transferred in 2 10-bit 
> > > samples. The
> > > 
> > > A TAB is missing after ":"?
> > Ok.

I just came to make this changes, however the text for 
SOC_MBUS_PACKING_2X10_PADHI is in line with the surrounding text. Would 
you like all of the other comments to be indented with another tab?

> > > > + *            high bits are padding
> > > >   * @SOC_MBUS_PACKING_EXTEND16:   sample width (e.g., 10 bits) has
> > > to be extended
> > > >   *            to 16 bits
> > > >   * @SOC_MBUS_PACKING_VARIABLE:   compressed formats with variable 

> > packing
> > > > @@ -33,6 +35,7 @@ enum soc_mbus_packing {
> > > >     SOC_MBUS_PACKING_NONE,
> > > >     SOC_MBUS_PACKING_2X8_PADHI,
> > > >     SOC_MBUS_PACKING_2X8_PADLO,
> > > > +   SOC_MBUS_PACKING_2X10_PADHI,
> > > >     SOC_MBUS_PACKING_EXTEND16,
> > > >     SOC_MBUS_PACKING_VARIABLE,
> > > >     SOC_MBUS_PACKING_1_5X8,
> > > > -- 
> > > > 1.7.5.4

Thanks
Phil

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

On Thu, 25 Apr 2013, phil.edworthy@renesas.com wrote:

> Hi Guennadi,
> 
> > > > On Wed, 17 Apr 2013, Phil Edworthy wrote:
> > > > 
> > > > > The V4L2_MBUS_FMT_YUYV10_2X10 format has already been added to 
> > > mediabus, so
> > > > > this patch just adds SoC camera support.
> > > > > 
> > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > > > ---
> > > > >  drivers/media/platform/soc_camera/soc_mediabus.c |   15 
> > > +++++++++++++++
> > > > >  include/media/soc_mediabus.h                     |    3 +++
> > > > >  2 files changed, 18 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/media/platform/soc_camera/soc_mediabus.c b/
> > > > drivers/media/platform/soc_camera/soc_mediabus.c
> > > > > index 7569e77..be47d41 100644
> > > > > --- a/drivers/media/platform/soc_camera/soc_mediabus.c
> > > > > +++ b/drivers/media/platform/soc_camera/soc_mediabus.c
> > > > > @@ -57,6 +57,15 @@ static const struct soc_mbus_lookup mbus_fmt[] 
> = {
> > > > >        .layout         = SOC_MBUS_LAYOUT_PACKED,
> > > > >     },
> > > > >  }, {
> > > > > +   .code = V4L2_MBUS_FMT_YUYV10_2X10,
> > > > > +   .fmt = {
> > > > > +      .fourcc         = V4L2_PIX_FMT_YUYV,
> > > > > +      .name         = "YUYV",
> > > > > +      .bits_per_sample   = 10,
> > > > > +      .packing      = SOC_MBUS_PACKING_2X10_PADHI,
> > > > 
> > > > Wow, what kind of host can pack two 10-bit samples into 3 bytes and 
> > > write 
> > > > 3-byte pixels to memory?
> > > I think I might have misunderstood how this is used. From my 
> > > understanding, the MBUS formats are used to describe the hardware 
> > > interfaces to cameras, i.e. 2 samples of 10 bits. I guess that the 
> fourcc 
> > > field also determines what v4l2 format is required to capture this. 
> > 
> > No, not quite. This table describes default "pass-through" capture of 
> > video data on a media bus to memory. E.g. the first entry in the table 
> > means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the bus, 
> you 
> > sample 8 bits at a time, and store the samples 1-to-1 into RAM, you get 
> > the V4L2_PIX_FMT_YUYV format in your buffer. It can also describe some 
> > standard operations with the sampled data, like swapping the order, 
> > filling missing high bits (e.g. if you sample 10 bits but store 16 bits 
> > per sample with high 6 bits nullified). The table also specifies which 
> > bits are used for padding in the original data, e.g. 
> > V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has SOC_MBUS_PACKING_2X8_PADLO, 
> whereas 
> > V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has SOC_MBUS_PACKING_2X8_PADHI, which 
> 
> > means, that out of 16 bits of data, that you get when you sample an 
> 8-bit 
> > bus twice, either low or high 6 bits are invalid and should be 
> discarded.
> 
> Ok, I see. However, is it necessary to provide a default pass-through v4l2 
> format?

No, it's not. If no (soc-camera) host camera driver is willing to use this 
pass-through conversion, then it's not required.

> I can't see a suitable v4l2 format! For the hardware I have been 
> working on, there is always the option of converting the data to another 
> format, so this is not really needed. I doubt that it makes sense to add 
> yet another v4l2 format for userspace, when typical uses would involve the 
> host hardware converting the format to something else, e.g. 
> V4L2_PIX_FMT_RGB32.

Up to you, really. If you don't need this default conversion, don't add 
it.

> > > However, I am not sure how the two relate to each other. How does the 
> > > above code imply 3 bytes?
> > 
> > Not the above code, but your entry in the soc_mbus_bytes_per_line() 
> > function below, where you multiply width * 3.
> 
> It looks like hosts use soc_mbus_bytes_per_line() to report the size of 
> video buffers needed. Shouldn't the hosts report the buffer metrics for 
> the v4l2 format, since that is what will be output? What has this to do 
> with the MBUS specifics?

struct soc_mbus_pixelfmt describes a conversion from an MBUS code to a 
pixel format in memory. Camera host drivers call that function with a 
_suitable_ conversion descriptor (either a standard or a special one) and 
the function calculates the number of bytes.

> > > > > +      .order         = SOC_MBUS_ORDER_LE,
> > > > > +   },
> > > > > +}, {
> > > > >     .code = V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
> > > > >     .fmt = {
> > > > >        .fourcc         = V4L2_PIX_FMT_RGB555,
> > > > > @@ -403,6 +412,10 @@ int soc_mbus_samples_per_pixel(const struct 
> > > > soc_mbus_pixelfmt *mf,
> > > > >        *numerator = 2;
> > > > >        *denominator = 1;
> > > > >        return 0;
> > > > > +   case SOC_MBUS_PACKING_2X10_PADHI:
> > > > > +      *numerator = 3;
> > > > > +      *denominator = 1;
> > > > 
> > > > Why 3? it's 2 samples per pixel, right? Should be *numerator = 2 
> above?
> > > Not sure why I thought it should be 3, I think I had it in my head 
> that 
> > > this was the number of bytes needed per pixel. Clearly this is not the 
> 
> > > case!
> > > 
> > > > > +      return 0;
> > > > >     case SOC_MBUS_PACKING_1_5X8:
> > > > >        *numerator = 3;
> > > > >        *denominator = 2;
> > > > > @@ -428,6 +441,8 @@ s32 soc_mbus_bytes_per_line(u32 width, const 
> > > > struct soc_mbus_pixelfmt *mf)
> > > > >     case SOC_MBUS_PACKING_2X8_PADLO:
> > > > >     case SOC_MBUS_PACKING_EXTEND16:
> > > > >        return width * 2;
> > > > > +   case SOC_MBUS_PACKING_2X10_PADHI:
> > > > > +      return width * 3;
> > > > >     case SOC_MBUS_PACKING_1_5X8:
> > > > >        return width * 3 / 2;
> > > > >     case SOC_MBUS_PACKING_VARIABLE:
> > > > > diff --git a/include/media/soc_mediabus.h 
> > > b/include/media/soc_mediabus.h
> > > > > index d33f6d0..b131a47 100644
> > > > > --- a/include/media/soc_mediabus.h
> > > > > +++ b/include/media/soc_mediabus.h
> > > > > @@ -21,6 +21,8 @@
> > > > >   * @SOC_MBUS_PACKING_2X8_PADHI:   16 bits transferred in 2 8-bit 
> > > > samples, in the
> > > > >   *            possibly incomplete byte high bits are padding
> > > > >   * @SOC_MBUS_PACKING_2X8_PADLO:   as above, but low bits are 
> padding
> > > > > + * @SOC_MBUS_PACKING_2X10_PADHI:20 bits transferred in 2 10-bit 
> > > > samples. The
> > > > 
> > > > A TAB is missing after ":"?
> > > Ok.
> 
> I just came to make this changes, however the text for 
> SOC_MBUS_PACKING_2X10_PADHI is in line with the surrounding text. Would 
> you like all of the other comments to be indented with another tab?

Ah right, sorry, no, then please don't.

Thanks
Guennadi

> > > > > + *            high bits are padding
> > > > >   * @SOC_MBUS_PACKING_EXTEND16:   sample width (e.g., 10 bits) has
> > > > to be extended
> > > > >   *            to 16 bits
> > > > >   * @SOC_MBUS_PACKING_VARIABLE:   compressed formats with variable 
> 
> > > packing
> > > > > @@ -33,6 +35,7 @@ enum soc_mbus_packing {
> > > > >     SOC_MBUS_PACKING_NONE,
> > > > >     SOC_MBUS_PACKING_2X8_PADHI,
> > > > >     SOC_MBUS_PACKING_2X8_PADLO,
> > > > > +   SOC_MBUS_PACKING_2X10_PADHI,
> > > > >     SOC_MBUS_PACKING_EXTEND16,
> > > > >     SOC_MBUS_PACKING_VARIABLE,
> > > > >     SOC_MBUS_PACKING_1_5X8,
> > > > > -- 
> > > > > 1.7.5.4
> 
> Thanks
> Phil
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Guennadi,

<snip>
> > > > > Wow, what kind of host can pack two 10-bit samples into 3 bytes 
and 
> > > > write 
> > > > > 3-byte pixels to memory?
> > > > I think I might have misunderstood how this is used. From my 
> > > > understanding, the MBUS formats are used to describe the hardware 
> > > > interfaces to cameras, i.e. 2 samples of 10 bits. I guess that the 

> > fourcc 
> > > > field also determines what v4l2 format is required to capture 
this. 
> > > 
> > > No, not quite. This table describes default "pass-through" capture 
of 
> > > video data on a media bus to memory. E.g. the first entry in the 
table 
> > > means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the 
bus, 
> > you 
> > > sample 8 bits at a time, and store the samples 1-to-1 into RAM, you 
get 
> > > the V4L2_PIX_FMT_YUYV format in your buffer. It can also describe 
some 
> > > standard operations with the sampled data, like swapping the order, 
> > > filling missing high bits (e.g. if you sample 10 bits but store 16 
bits 
> > > per sample with high 6 bits nullified). The table also specifies 
which 
> > > bits are used for padding in the original data, e.g. 
> > > V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has SOC_MBUS_PACKING_2X8_PADLO, 
> > whereas 
> > > V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has SOC_MBUS_PACKING_2X8_PADHI, 
which 
> > 
> > > means, that out of 16 bits of data, that you get when you sample an 
> > 8-bit 
> > > bus twice, either low or high 6 bits are invalid and should be 
> > discarded.
> > 
> > Ok, I see. However, is it necessary to provide a default pass-through 
v4l2 
> > format?
> 
> No, it's not. If no (soc-camera) host camera driver is willing to use 
this 
> pass-through conversion, then it's not required.
Ok, I'll look at that when I get a moment!

> > I can't see a suitable v4l2 format! For the hardware I have been 
> > working on, there is always the option of converting the data to 
another 
> > format, so this is not really needed. I doubt that it makes sense to 
add 
> > yet another v4l2 format for userspace, when typical uses would involve 
the 
> > host hardware converting the format to something else, e.g. 
> > V4L2_PIX_FMT_RGB32.
> 
> Up to you, really. If you don't need this default conversion, don't add 
> it.
Ok, it seems like it would be a bad idea to provide a default conversion 
that my not be supported by other hosts.

> > > > However, I am not sure how the two relate to each other. How does 
the 
> > > > above code imply 3 bytes?
> > > 
> > > Not the above code, but your entry in the soc_mbus_bytes_per_line() 
> > > function below, where you multiply width * 3.
> > 
> > It looks like hosts use soc_mbus_bytes_per_line() to report the size 
of 
> > video buffers needed. Shouldn't the hosts report the buffer metrics 
for 
> > the v4l2 format, since that is what will be output? What has this to 
do 
> > with the MBUS specifics?
> 
> struct soc_mbus_pixelfmt describes a conversion from an MBUS code to a 
> pixel format in memory. Camera host drivers call that function with a 
> _suitable_ conversion descriptor (either a standard or a special one) 
and 
> the function calculates the number of bytes.
Right, I think I understand!

Thanks
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Phil

On Fri, 26 Apr 2013, phil.edworthy@renesas.com wrote:

> Hi Guennadi,
> 
> <snip>
> > > > > > Wow, what kind of host can pack two 10-bit samples into 3 bytes 
> and 
> > > > > write 
> > > > > > 3-byte pixels to memory?
> > > > > I think I might have misunderstood how this is used. From my 
> > > > > understanding, the MBUS formats are used to describe the hardware 
> > > > > interfaces to cameras, i.e. 2 samples of 10 bits. I guess that the 
> 
> > > fourcc 
> > > > > field also determines what v4l2 format is required to capture 
> this. 
> > > > 
> > > > No, not quite. This table describes default "pass-through" capture 
> of 
> > > > video data on a media bus to memory. E.g. the first entry in the 
> table 
> > > > means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the 
> bus, 
> > > you 
> > > > sample 8 bits at a time, and store the samples 1-to-1 into RAM, you 
> get 
> > > > the V4L2_PIX_FMT_YUYV format in your buffer. It can also describe 
> some 
> > > > standard operations with the sampled data, like swapping the order, 
> > > > filling missing high bits (e.g. if you sample 10 bits but store 16 
> bits 
> > > > per sample with high 6 bits nullified). The table also specifies 
> which 
> > > > bits are used for padding in the original data, e.g. 
> > > > V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has SOC_MBUS_PACKING_2X8_PADLO, 
> > > whereas 
> > > > V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has SOC_MBUS_PACKING_2X8_PADHI, 
> which 
> > > 
> > > > means, that out of 16 bits of data, that you get when you sample an 
> > > 8-bit 
> > > > bus twice, either low or high 6 bits are invalid and should be 
> > > discarded.
> > > 
> > > Ok, I see. However, is it necessary to provide a default pass-through 
> v4l2 
> > > format?
> > 
> > No, it's not. If no (soc-camera) host camera driver is willing to use 
> this 
> > pass-through conversion, then it's not required.
> Ok, I'll look at that when I get a moment!
> 
> > > I can't see a suitable v4l2 format! For the hardware I have been 
> > > working on, there is always the option of converting the data to 
> another 
> > > format, so this is not really needed. I doubt that it makes sense to 
> add 
> > > yet another v4l2 format for userspace, when typical uses would involve 
> the 
> > > host hardware converting the format to something else, e.g. 
> > > V4L2_PIX_FMT_RGB32.
> > 
> > Up to you, really. If you don't need this default conversion, don't add 
> > it.
> Ok, it seems like it would be a bad idea to provide a default conversion 
> that my not be supported by other hosts.

Right, that table collects "natural" conversions, mostly just 
straightforward bus-to-buffer. In your case of 2 10-bit samples such a 
natural transfer would produce one 16-bit word per sample, of which only 
10 bits are useful information. So, your 20 bits of pixel data would be 
located in bits 25:16 and 9:0 of each 32-bit (long)word. I don't think 
there is a fourcc code, describing such a buffer layout... It probably 
would be useless without a special conversion software.

Thanks
Guennadi

> > > > > However, I am not sure how the two relate to each other. How does 
> the 
> > > > > above code imply 3 bytes?
> > > > 
> > > > Not the above code, but your entry in the soc_mbus_bytes_per_line() 
> > > > function below, where you multiply width * 3.
> > > 
> > > It looks like hosts use soc_mbus_bytes_per_line() to report the size 
> of 
> > > video buffers needed. Shouldn't the hosts report the buffer metrics 
> for 
> > > the v4l2 format, since that is what will be output? What has this to 
> do 
> > > with the MBUS specifics?
> > 
> > struct soc_mbus_pixelfmt describes a conversion from an MBUS code to a 
> > pixel format in memory. Camera host drivers call that function with a 
> > _suitable_ conversion descriptor (either a standard or a special one) 
> and 
> > the function calculates the number of bytes.
> Right, I think I understand!
> 
> Thanks
> Phil
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Guennadi,

> From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> To: phil.edworthy@renesas.com, 
> Cc: linux-media@vger.kernel.org, Mauro Carvalho Chehab 
<mchehab@redhat.com>
> Date: 26/04/2013 22:00
> Subject: Re: [PATCH] soc_camera: Add V4L2_MBUS_FMT_YUYV10_2X10 format
> 
> Hi Phil
> 
> On Fri, 26 Apr 2013, phil.edworthy@renesas.com wrote:
> 
> > Hi Guennadi,
> > 
> > <snip>
> > > > > > > Wow, what kind of host can pack two 10-bit samples into 3 
bytes 
> > and 
> > > > > > write 
> > > > > > > 3-byte pixels to memory?
> > > > > > I think I might have misunderstood how this is used. From my 
> > > > > > understanding, the MBUS formats are used to describe the 
hardware 
> > > > > > interfaces to cameras, i.e. 2 samples of 10 bits. I guess that 
the 
> > 
> > > > fourcc 
> > > > > > field also determines what v4l2 format is required to capture 
> > this. 
> > > > > 
> > > > > No, not quite. This table describes default "pass-through" 
capture 
> > of 
> > > > > video data on a media bus to memory. E.g. the first entry in the 

> > table 
> > > > > means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the 

> > bus, 
> > > > you 
> > > > > sample 8 bits at a time, and store the samples 1-to-1 into RAM, 
you 
> > get 
> > > > > the V4L2_PIX_FMT_YUYV format in your buffer. It can also 
describe 
> > some 
> > > > > standard operations with the sampled data, like swapping the 
order, 
> > > > > filling missing high bits (e.g. if you sample 10 bits but store 
16 
> > bits 
> > > > > per sample with high 6 bits nullified). The table also specifies 

> > which 
> > > > > bits are used for padding in the original data, e.g. 
> > > > > V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has 
SOC_MBUS_PACKING_2X8_PADLO, 
> > > > whereas 
> > > > > V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has 
SOC_MBUS_PACKING_2X8_PADHI, 
> > which 
> > > > 
> > > > > means, that out of 16 bits of data, that you get when you sample 
an 
> > > > 8-bit 
> > > > > bus twice, either low or high 6 bits are invalid and should be 
> > > > discarded.
> > > > 
> > > > Ok, I see. However, is it necessary to provide a default 
pass-through 
> > v4l2 
> > > > format?
> > > 
> > > No, it's not. If no (soc-camera) host camera driver is willing to 
use 
> > this 
> > > pass-through conversion, then it's not required.
> > Ok, I'll look at that when I get a moment!
> > 
> > > > I can't see a suitable v4l2 format! For the hardware I have been 
> > > > working on, there is always the option of converting the data to 
> > another 
> > > > format, so this is not really needed. I doubt that it makes sense 
to 
> > add 
> > > > yet another v4l2 format for userspace, when typical uses would 
involve 
> > the 
> > > > host hardware converting the format to something else, e.g. 
> > > > V4L2_PIX_FMT_RGB32.
> > > 
> > > Up to you, really. If you don't need this default conversion, don't 
add 
> > > it.
> > Ok, it seems like it would be a bad idea to provide a default 
conversion 
> > that my not be supported by other hosts.
> 
> Right, that table collects "natural" conversions, mostly just 
> straightforward bus-to-buffer. In your case of 2 10-bit samples such a 
> natural transfer would produce one 16-bit word per sample, of which only 

> 10 bits are useful information. So, your 20 bits of pixel data would be 
> located in bits 25:16 and 9:0 of each 32-bit (long)word. I don't think 
> there is a fourcc code, describing such a buffer layout... It probably 
> would be useless without a special conversion software.

So, if there is not a natural conversion & I do not populate the fourcc 
field, doesn't the other information in the soc_mbus_lookup struct becomes 
moot? Nothing can allocate a buffer for it, so nothing should be using the 
bits_per_sample, packing or order fields. Similarly, nothing should call 
soc_mbus_samples_per_pixel() with this format. By extension, there is no 
need to add SOC_MBUS_PACKING_2X10_PADHI to soc_mbus_bytes_per_line().

Since V4L2_MBUS_FMT_YUYV10_2X10 already exists without the above 
additional info, I guess there is no need for this patch at all.

Thanks
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

On Wed, 1 May 2013, phil.edworthy@renesas.com wrote:

> Hi Guennadi,
> 
> > From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > To: phil.edworthy@renesas.com, 
> > Cc: linux-media@vger.kernel.org, Mauro Carvalho Chehab 
> <mchehab@redhat.com>
> > Date: 26/04/2013 22:00
> > Subject: Re: [PATCH] soc_camera: Add V4L2_MBUS_FMT_YUYV10_2X10 format
> > 
> > Hi Phil
> > 
> > On Fri, 26 Apr 2013, phil.edworthy@renesas.com wrote:
> > 
> > > Hi Guennadi,
> > > 
> > > <snip>
> > > > > > > > Wow, what kind of host can pack two 10-bit samples into 3 
> bytes 
> > > and 
> > > > > > > write 
> > > > > > > > 3-byte pixels to memory?
> > > > > > > I think I might have misunderstood how this is used. From my 
> > > > > > > understanding, the MBUS formats are used to describe the 
> hardware 
> > > > > > > interfaces to cameras, i.e. 2 samples of 10 bits. I guess that 
> the 
> > > 
> > > > > fourcc 
> > > > > > > field also determines what v4l2 format is required to capture 
> > > this. 
> > > > > > 
> > > > > > No, not quite. This table describes default "pass-through" 
> capture 
> > > of 
> > > > > > video data on a media bus to memory. E.g. the first entry in the 
> 
> > > table 
> > > > > > means, that if you get the V4L2_MBUS_FMT_YUYV8_2X8 format on the 
> 
> > > bus, 
> > > > > you 
> > > > > > sample 8 bits at a time, and store the samples 1-to-1 into RAM, 
> you 
> > > get 
> > > > > > the V4L2_PIX_FMT_YUYV format in your buffer. It can also 
> describe 
> > > some 
> > > > > > standard operations with the sampled data, like swapping the 
> order, 
> > > > > > filling missing high bits (e.g. if you sample 10 bits but store 
> 16 
> > > bits 
> > > > > > per sample with high 6 bits nullified). The table also specifies 
> 
> > > which 
> > > > > > bits are used for padding in the original data, e.g. 
> > > > > > V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE has 
> SOC_MBUS_PACKING_2X8_PADLO, 
> > > > > whereas 
> > > > > > V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE has 
> SOC_MBUS_PACKING_2X8_PADHI, 
> > > which 
> > > > > 
> > > > > > means, that out of 16 bits of data, that you get when you sample 
> an 
> > > > > 8-bit 
> > > > > > bus twice, either low or high 6 bits are invalid and should be 
> > > > > discarded.
> > > > > 
> > > > > Ok, I see. However, is it necessary to provide a default 
> pass-through 
> > > v4l2 
> > > > > format?
> > > > 
> > > > No, it's not. If no (soc-camera) host camera driver is willing to 
> use 
> > > this 
> > > > pass-through conversion, then it's not required.
> > > Ok, I'll look at that when I get a moment!
> > > 
> > > > > I can't see a suitable v4l2 format! For the hardware I have been 
> > > > > working on, there is always the option of converting the data to 
> > > another 
> > > > > format, so this is not really needed. I doubt that it makes sense 
> to 
> > > add 
> > > > > yet another v4l2 format for userspace, when typical uses would 
> involve 
> > > the 
> > > > > host hardware converting the format to something else, e.g. 
> > > > > V4L2_PIX_FMT_RGB32.
> > > > 
> > > > Up to you, really. If you don't need this default conversion, don't 
> add 
> > > > it.
> > > Ok, it seems like it would be a bad idea to provide a default 
> conversion 
> > > that my not be supported by other hosts.
> > 
> > Right, that table collects "natural" conversions, mostly just 
> > straightforward bus-to-buffer. In your case of 2 10-bit samples such a 
> > natural transfer would produce one 16-bit word per sample, of which only 
> 
> > 10 bits are useful information. So, your 20 bits of pixel data would be 
> > located in bits 25:16 and 9:0 of each 32-bit (long)word. I don't think 
> > there is a fourcc code, describing such a buffer layout... It probably 
> > would be useless without a special conversion software.
> 
> So, if there is not a natural conversion & I do not populate the fourcc 
> field, doesn't the other information in the soc_mbus_lookup struct becomes 
> moot? Nothing can allocate a buffer for it, so nothing should be using the 
> bits_per_sample, packing or order fields. Similarly, nothing should call 
> soc_mbus_samples_per_pixel() with this format. By extension, there is no 
> need to add SOC_MBUS_PACKING_2X10_PADHI to soc_mbus_bytes_per_line().

Maybe there is no _natural_ conversion, but your hardware can be able to 
handle this in a _special_ way. Then you allocate a specific instance of 
struct soc_mbus_pixelfmt in your driver for that conversion, similar to 
how, e.g. sh_mobile_ceu_camera.c uses its sh_mobile_ceu_formats[] array. 
Then you still can use soc_mbus_bytes_per_line() and soc_mbus_image_size() 
with that your special format descriptor. But for those you do need to 
define SOC_MBUS_PACKING_2X10_PADHI and teach them to use it. And please 
document this new packing well, because it's not a trivial one.

Thanks
Guennadi

> Since V4L2_MBUS_FMT_YUYV10_2X10 already exists without the above 
> additional info, I guess there is no need for this patch at all.
> 
> Thanks
> Phil
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html