linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: v4l2-common: Add BGR666 to v4l2_format_info
@ 2020-03-16  7:01 Dafna Hirschfeld
  2020-03-16  7:22 ` Laurent Pinchart
  0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2020-03-16  7:01 UTC (permalink / raw)
  To: linux-media
  Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
	sakari.ailus, linux-rockchip, mchehab, laurent.pinchart

Add V4L2_PIX_FMT_BGR666 to the format table.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
Hi,
BGR66 is needed for the rkisp1 driver.
Currently it crashes since the call to
v4l2_format_info returns NULL.

 drivers/media/v4l2-core/v4l2-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index d0e5ebc736f9..d7f82b2aa22f 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -253,6 +253,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
 		{ .format = V4L2_PIX_FMT_GREY,    .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_RGB565,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_RGB555,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_BGR666,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
 
 		/* YUV packed formats */
 		{ .format = V4L2_PIX_FMT_YUYV,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] media: v4l2-common: Add BGR666 to v4l2_format_info
  2020-03-16  7:01 [PATCH] media: v4l2-common: Add BGR666 to v4l2_format_info Dafna Hirschfeld
@ 2020-03-16  7:22 ` Laurent Pinchart
  2020-03-16  8:07   ` Dafna Hirschfeld
  0 siblings, 1 reply; 7+ messages in thread
From: Laurent Pinchart @ 2020-03-16  7:22 UTC (permalink / raw)
  To: Dafna Hirschfeld
  Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
	sakari.ailus, linux-rockchip, mchehab

Hi Dafna,

Thank you for the patch.

On Mon, Mar 16, 2020 at 08:01:23AM +0100, Dafna Hirschfeld wrote:
> Add V4L2_PIX_FMT_BGR666 to the format table.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> Hi,
> BGR66 is needed for the rkisp1 driver.
> Currently it crashes since the call to
> v4l2_format_info returns NULL.
> 
>  drivers/media/v4l2-core/v4l2-common.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index d0e5ebc736f9..d7f82b2aa22f 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -253,6 +253,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>  		{ .format = V4L2_PIX_FMT_GREY,    .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_RGB565,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_RGB555,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> +		{ .format = V4L2_PIX_FMT_BGR666,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },

Isn't BGR666 stored in 3 bytes per pixel ?

>  
>  		/* YUV packed formats */
>  		{ .format = V4L2_PIX_FMT_YUYV,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] media: v4l2-common: Add BGR666 to v4l2_format_info
  2020-03-16  7:22 ` Laurent Pinchart
@ 2020-03-16  8:07   ` Dafna Hirschfeld
  2020-03-16  8:15     ` Laurent Pinchart
  0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2020-03-16  8:07 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
	sakari.ailus, linux-rockchip, mchehab

Hi,

On 16.03.20 08:22, Laurent Pinchart wrote:
> Hi Dafna,
> 
> Thank you for the patch.
> 
> On Mon, Mar 16, 2020 at 08:01:23AM +0100, Dafna Hirschfeld wrote:
>> Add V4L2_PIX_FMT_BGR666 to the format table.
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>> Hi,
>> BGR66 is needed for the rkisp1 driver.
>> Currently it crashes since the call to
>> v4l2_format_info returns NULL.
>>
>>   drivers/media/v4l2-core/v4l2-common.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
>> index d0e5ebc736f9..d7f82b2aa22f 100644
>> --- a/drivers/media/v4l2-core/v4l2-common.c
>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>> @@ -253,6 +253,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>>   		{ .format = V4L2_PIX_FMT_GREY,    .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>   		{ .format = V4L2_PIX_FMT_RGB565,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>   		{ .format = V4L2_PIX_FMT_RGB555,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>> +		{ .format = V4L2_PIX_FMT_BGR666,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> 
> Isn't BGR666 stored in 3 bytes per pixel ?
Hi, I also discussed it with Helen. From the documentation we understood that it uses 4 bytes and the last one is empty.
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-rgb.html

Dafna

> 
>>   
>>   		/* YUV packed formats */
>>   		{ .format = V4L2_PIX_FMT_YUYV,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] media: v4l2-common: Add BGR666 to v4l2_format_info
  2020-03-16  8:07   ` Dafna Hirschfeld
@ 2020-03-16  8:15     ` Laurent Pinchart
  2020-03-16  8:59       ` Dafna Hirschfeld
  0 siblings, 1 reply; 7+ messages in thread
From: Laurent Pinchart @ 2020-03-16  8:15 UTC (permalink / raw)
  To: Dafna Hirschfeld
  Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
	sakari.ailus, linux-rockchip, mchehab

Hi Dafna,

On Mon, Mar 16, 2020 at 09:07:16AM +0100, Dafna Hirschfeld wrote:
> On 16.03.20 08:22, Laurent Pinchart wrote:
> > On Mon, Mar 16, 2020 at 08:01:23AM +0100, Dafna Hirschfeld wrote:
> >> Add V4L2_PIX_FMT_BGR666 to the format table.
> >>
> >> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> >> ---
> >> Hi,
> >> BGR66 is needed for the rkisp1 driver.
> >> Currently it crashes since the call to
> >> v4l2_format_info returns NULL.
> >>
> >>   drivers/media/v4l2-core/v4l2-common.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> >> index d0e5ebc736f9..d7f82b2aa22f 100644
> >> --- a/drivers/media/v4l2-core/v4l2-common.c
> >> +++ b/drivers/media/v4l2-core/v4l2-common.c
> >> @@ -253,6 +253,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
> >>   		{ .format = V4L2_PIX_FMT_GREY,    .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> >>   		{ .format = V4L2_PIX_FMT_RGB565,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> >>   		{ .format = V4L2_PIX_FMT_RGB555,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> >> +		{ .format = V4L2_PIX_FMT_BGR666,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> > 
> > Isn't BGR666 stored in 3 bytes per pixel ?
>
> Hi, I also discussed it with Helen. From the documentation we
> understood that it uses 4 bytes and the last one is empty.
> https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-rgb.html

Would you then also understand V4L2_PIX_FMT_RGB565 to use 4 bytes with
the last 2 bytes empty ? :-)

I agree that the documentation is somehow ambiguous and should be fixed.
Patches are welcome ;-)

> >>   
> >>   		/* YUV packed formats */
> >>   		{ .format = V4L2_PIX_FMT_YUYV,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] media: v4l2-common: Add BGR666 to v4l2_format_info
  2020-03-16  8:15     ` Laurent Pinchart
@ 2020-03-16  8:59       ` Dafna Hirschfeld
  2020-03-16 10:00         ` Laurent Pinchart
  0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2020-03-16  8:59 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
	sakari.ailus, linux-rockchip, mchehab



On 16.03.20 09:15, Laurent Pinchart wrote:
> Hi Dafna,
> 
> On Mon, Mar 16, 2020 at 09:07:16AM +0100, Dafna Hirschfeld wrote:
>> On 16.03.20 08:22, Laurent Pinchart wrote:
>>> On Mon, Mar 16, 2020 at 08:01:23AM +0100, Dafna Hirschfeld wrote:
>>>> Add V4L2_PIX_FMT_BGR666 to the format table.
>>>>
>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>>> ---
>>>> Hi,
>>>> BGR66 is needed for the rkisp1 driver.
>>>> Currently it crashes since the call to
>>>> v4l2_format_info returns NULL.
>>>>
>>>>    drivers/media/v4l2-core/v4l2-common.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
>>>> index d0e5ebc736f9..d7f82b2aa22f 100644
>>>> --- a/drivers/media/v4l2-core/v4l2-common.c
>>>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>>>> @@ -253,6 +253,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>>>>    		{ .format = V4L2_PIX_FMT_GREY,    .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>>>    		{ .format = V4L2_PIX_FMT_RGB565,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>>>    		{ .format = V4L2_PIX_FMT_RGB555,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>>> +		{ .format = V4L2_PIX_FMT_BGR666,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>>
>>> Isn't BGR666 stored in 3 bytes per pixel ?
>>
>> Hi, I also discussed it with Helen. From the documentation we
>> understood that it uses 4 bytes and the last one is empty.
>> https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-rgb.html
> 
> Would you then also understand V4L2_PIX_FMT_RGB565 to use 4 bytes with
> the last 2 bytes empty ? :-)
hmm, the formats between BGR24 and XRGB32 in the docs table have vertical lines crossing all 4 bytes so we understood that they are all 4 bytes. Isn't it?
Format RGB565 doesn't have does vertical lines on the last two bytes which means it uses 2. At least that is what we understood.

Dafna

> 
> I agree that the documentation is somehow ambiguous and should be fixed.
> Patches are welcome ;-)
> 
>>>>    
>>>>    		/* YUV packed formats */
>>>>    		{ .format = V4L2_PIX_FMT_YUYV,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] media: v4l2-common: Add BGR666 to v4l2_format_info
  2020-03-16  8:59       ` Dafna Hirschfeld
@ 2020-03-16 10:00         ` Laurent Pinchart
  2020-03-16 13:45           ` Dafna Hirschfeld
  0 siblings, 1 reply; 7+ messages in thread
From: Laurent Pinchart @ 2020-03-16 10:00 UTC (permalink / raw)
  To: Dafna Hirschfeld
  Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
	sakari.ailus, linux-rockchip, mchehab

Hi Dafna,

On Mon, Mar 16, 2020 at 09:59:56AM +0100, Dafna Hirschfeld wrote:
> On 16.03.20 09:15, Laurent Pinchart wrote:
> > On Mon, Mar 16, 2020 at 09:07:16AM +0100, Dafna Hirschfeld wrote:
> >> On 16.03.20 08:22, Laurent Pinchart wrote:
> >>> On Mon, Mar 16, 2020 at 08:01:23AM +0100, Dafna Hirschfeld wrote:
> >>>> Add V4L2_PIX_FMT_BGR666 to the format table.
> >>>>
> >>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> >>>> ---
> >>>> Hi,
> >>>> BGR66 is needed for the rkisp1 driver.
> >>>> Currently it crashes since the call to
> >>>> v4l2_format_info returns NULL.
> >>>>
> >>>>    drivers/media/v4l2-core/v4l2-common.c | 1 +
> >>>>    1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> >>>> index d0e5ebc736f9..d7f82b2aa22f 100644
> >>>> --- a/drivers/media/v4l2-core/v4l2-common.c
> >>>> +++ b/drivers/media/v4l2-core/v4l2-common.c
> >>>> @@ -253,6 +253,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
> >>>>    		{ .format = V4L2_PIX_FMT_GREY,    .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> >>>>    		{ .format = V4L2_PIX_FMT_RGB565,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> >>>>    		{ .format = V4L2_PIX_FMT_RGB555,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> >>>> +		{ .format = V4L2_PIX_FMT_BGR666,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> >>>
> >>> Isn't BGR666 stored in 3 bytes per pixel ?
> >>
> >> Hi, I also discussed it with Helen. From the documentation we
> >> understood that it uses 4 bytes and the last one is empty.
> >> https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-rgb.html
> > 
> > Would you then also understand V4L2_PIX_FMT_RGB565 to use 4 bytes with
> > the last 2 bytes empty ? :-)
>
> hmm, the formats between BGR24 and XRGB32 in the docs table have vertical lines crossing all 4 bytes so we understood that they are all 4 bytes. Isn't it?
> Format RGB565 doesn't have does vertical lines on the last two bytes which means it uses 2. At least that is what we understood.

I stand corrected, looking at the drivers implementing
V4L2_PIX_FMT_BGR666, they all handle it as a 32bpp format.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > I agree that the documentation is somehow ambiguous and should be fixed.
> > Patches are welcome ;-)

I think adding explicit '-' or 'x' in the cells that contain "don't
care" bits would help avoiding confusion.

> >>>>    
> >>>>    		/* YUV packed formats */
> >>>>    		{ .format = V4L2_PIX_FMT_YUYV,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] media: v4l2-common: Add BGR666 to v4l2_format_info
  2020-03-16 10:00         ` Laurent Pinchart
@ 2020-03-16 13:45           ` Dafna Hirschfeld
  0 siblings, 0 replies; 7+ messages in thread
From: Dafna Hirschfeld @ 2020-03-16 13:45 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, helen.koike, ezequiel, hverkuil, kernel, dafna3,
	sakari.ailus, linux-rockchip, mchehab



On 16.03.20 11:00, Laurent Pinchart wrote:
> Hi Dafna,
> 
> On Mon, Mar 16, 2020 at 09:59:56AM +0100, Dafna Hirschfeld wrote:
>> On 16.03.20 09:15, Laurent Pinchart wrote:
>>> On Mon, Mar 16, 2020 at 09:07:16AM +0100, Dafna Hirschfeld wrote:
>>>> On 16.03.20 08:22, Laurent Pinchart wrote:
>>>>> On Mon, Mar 16, 2020 at 08:01:23AM +0100, Dafna Hirschfeld wrote:
>>>>>> Add V4L2_PIX_FMT_BGR666 to the format table.
>>>>>>
>>>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>>>>> ---
>>>>>> Hi,
>>>>>> BGR66 is needed for the rkisp1 driver.
>>>>>> Currently it crashes since the call to
>>>>>> v4l2_format_info returns NULL.
>>>>>>
>>>>>>     drivers/media/v4l2-core/v4l2-common.c | 1 +
>>>>>>     1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
>>>>>> index d0e5ebc736f9..d7f82b2aa22f 100644
>>>>>> --- a/drivers/media/v4l2-core/v4l2-common.c
>>>>>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>>>>>> @@ -253,6 +253,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>>>>>>     		{ .format = V4L2_PIX_FMT_GREY,    .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>>>>>     		{ .format = V4L2_PIX_FMT_RGB565,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>>>>>     		{ .format = V4L2_PIX_FMT_RGB555,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>>>>> +		{ .format = V4L2_PIX_FMT_BGR666,  .pixel_enc = V4L2_PIXEL_ENC_RGB, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>>>>
>>>>> Isn't BGR666 stored in 3 bytes per pixel ?
>>>>
>>>> Hi, I also discussed it with Helen. From the documentation we
>>>> understood that it uses 4 bytes and the last one is empty.
>>>> https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-rgb.html
>>>
>>> Would you then also understand V4L2_PIX_FMT_RGB565 to use 4 bytes with
>>> the last 2 bytes empty ? :-)
>>
>> hmm, the formats between BGR24 and XRGB32 in the docs table have vertical lines crossing all 4 bytes so we understood that they are all 4 bytes. Isn't it?
>> Format RGB565 doesn't have does vertical lines on the last two bytes which means it uses 2. At least that is what we understood.
> 
> I stand corrected, looking at the drivers implementing
> V4L2_PIX_FMT_BGR666, they all handle it as a 32bpp format.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
>>> I agree that the documentation is somehow ambiguous and should be fixed.
>>> Patches are welcome ;-)
> 
> I think adding explicit '-' or 'x' in the cells that contain "don't
> care" bits would help avoiding confusion.

sure, I'll do that

> 
>>>>>>     
>>>>>>     		/* YUV packed formats */
>>>>>>     		{ .format = V4L2_PIX_FMT_YUYV,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 },
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-03-16 13:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-16  7:01 [PATCH] media: v4l2-common: Add BGR666 to v4l2_format_info Dafna Hirschfeld
2020-03-16  7:22 ` Laurent Pinchart
2020-03-16  8:07   ` Dafna Hirschfeld
2020-03-16  8:15     ` Laurent Pinchart
2020-03-16  8:59       ` Dafna Hirschfeld
2020-03-16 10:00         ` Laurent Pinchart
2020-03-16 13:45           ` Dafna Hirschfeld

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).