All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] v4l: Clarify RGB666 pixel format definition
@ 2014-07-21 20:39 Laurent Pinchart
  2014-07-21 21:43 ` Hans Verkuil
  0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2014-07-21 20:39 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil

The RGB666 pixel format doesn't include an alpha channel. Document it as
such.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../DocBook/media/v4l/pixfmt-packed-rgb.xml          | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
index 32feac9..c47692a 100644
--- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
@@ -330,20 +330,12 @@ colorspace <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
 	    <entry></entry>
 	    <entry>r<subscript>1</subscript></entry>
 	    <entry>r<subscript>0</subscript></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
-	    <entry></entry>
+	    <entry>-</entry>
+	    <entry>-</entry>
+	    <entry>-</entry>
+	    <entry>-</entry>
+	    <entry>-</entry>
+	    <entry>-</entry>
 	  </row>
 	  <row id="V4L2-PIX-FMT-BGR24">
 	    <entry><constant>V4L2_PIX_FMT_BGR24</constant></entry>
-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH] v4l: Clarify RGB666 pixel format definition
  2014-07-21 20:39 [PATCH] v4l: Clarify RGB666 pixel format definition Laurent Pinchart
@ 2014-07-21 21:43 ` Hans Verkuil
  2014-07-21 22:30   ` Laurent Pinchart
  0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2014-07-21 21:43 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media

On 07/21/2014 10:39 PM, Laurent Pinchart wrote:
> The RGB666 pixel format doesn't include an alpha channel. Document it as
> such.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../DocBook/media/v4l/pixfmt-packed-rgb.xml          | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> index 32feac9..c47692a 100644
> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> @@ -330,20 +330,12 @@ colorspace <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
>  	    <entry></entry>
>  	    <entry>r<subscript>1</subscript></entry>
>  	    <entry>r<subscript>0</subscript></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> -	    <entry></entry>
> +	    <entry>-</entry>
> +	    <entry>-</entry>
> +	    <entry>-</entry>
> +	    <entry>-</entry>
> +	    <entry>-</entry>
> +	    <entry>-</entry>

Just to clarify: BGR666 is a three byte format, not a four byte format?

If so, then:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

>  	  </row>
>  	  <row id="V4L2-PIX-FMT-BGR24">
>  	    <entry><constant>V4L2_PIX_FMT_BGR24</constant></entry>
> 


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

* Re: [PATCH] v4l: Clarify RGB666 pixel format definition
  2014-07-21 21:43 ` Hans Verkuil
@ 2014-07-21 22:30   ` Laurent Pinchart
  2014-07-21 22:44     ` Hans Verkuil
  0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2014-07-21 22:30 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media

Hi Hans,

On Monday 21 July 2014 23:43:16 Hans Verkuil wrote:
> On 07/21/2014 10:39 PM, Laurent Pinchart wrote:
> > The RGB666 pixel format doesn't include an alpha channel. Document it as
> > such.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  .../DocBook/media/v4l/pixfmt-packed-rgb.xml          | 20 +++++----------
> > 1 file changed, 6 insertions(+), 14 deletions(-)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
> > 32feac9..c47692a 100644
> > --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > @@ -330,20 +330,12 @@ colorspace
> > <constant>V4L2_COLORSPACE_SRGB</constant>.</para>> 
> >  	    <entry></entry>
> >  	    <entry>r<subscript>1</subscript></entry>
> >  	    <entry>r<subscript>0</subscript></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > -	    <entry></entry>
> > +	    <entry>-</entry>
> > +	    <entry>-</entry>
> > +	    <entry>-</entry>
> > +	    <entry>-</entry>
> > +	    <entry>-</entry>
> > +	    <entry>-</entry>
> 
> Just to clarify: BGR666 is a three byte format, not a four byte format?

Well... :-)

Three drivers seem to support the BGR666 in mainline : sh_veu, s3c-camif and 
exynos4-is. Further investigation shows that the sh_veu driver lists the 
BGR666 format internally but doesn't expose it to userspace and doesn't 
actually support it, so we're down to two drivers.

Looking at the S3C6410 datasheet, it's unclear how the hardware stores RGB666 
pixels in memory. It could be either

Byte 0   Byte 1   Byte 2   Byte 3

-------- ------RR RRRRGGGG GGBBBBBB

or

GGBBBBBB RRRRGGGG ------RR --------

None of those correspond to the RGB666 format defined in the spec.

The Exynos4 FIMC isn't documented in the public datasheet, so I can't check 
how the format is defined.

Furthermore, various Renesas video-related IP cores support many different 
RGB666 variants, on either 32 or 24 bits per pixel, with and without alpha.

Beside a loud *sigh*, any comment ? :-)

> >  	  </row>
> >  	  <row id="V4L2-PIX-FMT-BGR24">
> >  	    <entry><constant>V4L2_PIX_FMT_BGR24</constant></entry>

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH] v4l: Clarify RGB666 pixel format definition
  2014-07-21 22:30   ` Laurent Pinchart
@ 2014-07-21 22:44     ` Hans Verkuil
  2014-09-09 13:18       ` Laurent Pinchart
  0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2014-07-21 22:44 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media, Sylwester Nawrocki

On 07/22/2014 12:30 AM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Monday 21 July 2014 23:43:16 Hans Verkuil wrote:
>> On 07/21/2014 10:39 PM, Laurent Pinchart wrote:
>>> The RGB666 pixel format doesn't include an alpha channel. Document it as
>>> such.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> ---
>>>
>>>  .../DocBook/media/v4l/pixfmt-packed-rgb.xml          | 20 +++++----------
>>> 1 file changed, 6 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
>>> 32feac9..c47692a 100644
>>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>> @@ -330,20 +330,12 @@ colorspace
>>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>> 
>>>  	    <entry></entry>
>>>  	    <entry>r<subscript>1</subscript></entry>
>>>  	    <entry>r<subscript>0</subscript></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> -	    <entry></entry>
>>> +	    <entry>-</entry>
>>> +	    <entry>-</entry>
>>> +	    <entry>-</entry>
>>> +	    <entry>-</entry>
>>> +	    <entry>-</entry>
>>> +	    <entry>-</entry>
>>
>> Just to clarify: BGR666 is a three byte format, not a four byte format?
> 
> Well... :-)
> 
> Three drivers seem to support the BGR666 in mainline : sh_veu, s3c-camif and 
> exynos4-is. Further investigation shows that the sh_veu driver lists the 
> BGR666 format internally but doesn't expose it to userspace and doesn't 
> actually support it, so we're down to two drivers.
> 
> Looking at the S3C6410 datasheet, it's unclear how the hardware stores RGB666 
> pixels in memory. It could be either
> 
> Byte 0   Byte 1   Byte 2   Byte 3
> 
> -------- ------RR RRRRGGGG GGBBBBBB
> 
> or
> 
> GGBBBBBB RRRRGGGG ------RR --------
> 
> None of those correspond to the RGB666 format defined in the spec.
> 
> The Exynos4 FIMC isn't documented in the public datasheet, so I can't check 
> how the format is defined.
> 
> Furthermore, various Renesas video-related IP cores support many different 
> RGB666 variants, on either 32 or 24 bits per pixel, with and without alpha.
> 
> Beside a loud *sigh*, any comment ? :-)

You'll have to check with Samsung then. Sylwester, can you shed any light on
what this format *really* is?

Regards,

	Hans

> 
>>>  	  </row>
>>>  	  <row id="V4L2-PIX-FMT-BGR24">
>>>  	    <entry><constant>V4L2_PIX_FMT_BGR24</constant></entry>
> 


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

* Re: [PATCH] v4l: Clarify RGB666 pixel format definition
  2014-07-21 22:44     ` Hans Verkuil
@ 2014-09-09 13:18       ` Laurent Pinchart
  2014-09-09 14:45         ` Sylwester Nawrocki
  0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2014-09-09 13:18 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Sylwester Nawrocki

On Tuesday 22 July 2014 00:44:34 Hans Verkuil wrote:
> On 07/22/2014 12:30 AM, Laurent Pinchart wrote:
> > On Monday 21 July 2014 23:43:16 Hans Verkuil wrote:
> >> On 07/21/2014 10:39 PM, Laurent Pinchart wrote:
> >>> The RGB666 pixel format doesn't include an alpha channel. Document it as
> >>> such.
> >>> 
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>> ---
> >>> 
> >>>  .../DocBook/media/v4l/pixfmt-packed-rgb.xml          | 20
> >>>  +++++----------
> >>> 
> >>> 1 file changed, 6 insertions(+), 14 deletions(-)
> >>> 
> >>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
> >>> 32feac9..c47692a 100644
> >>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>> @@ -330,20 +330,12 @@ colorspace
> >>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>>
> >>>  	    <entry></entry>
> >>>  	    <entry>r<subscript>1</subscript></entry>
> >>>  	    <entry>r<subscript>0</subscript></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry></entry>
> >>> +	    <entry>-</entry>
> >>> +	    <entry>-</entry>
> >>> +	    <entry>-</entry>
> >>> +	    <entry>-</entry>
> >>> +	    <entry>-</entry>
> >>> +	    <entry>-</entry>
> >> 
> >> Just to clarify: BGR666 is a three byte format, not a four byte format?
> > 
> > Well... :-)
> > 
> > Three drivers seem to support the BGR666 in mainline : sh_veu, s3c-camif
> > and exynos4-is. Further investigation shows that the sh_veu driver lists
> > the BGR666 format internally but doesn't expose it to userspace and
> > doesn't actually support it, so we're down to two drivers.
> > 
> > Looking at the S3C6410 datasheet, it's unclear how the hardware stores
> > RGB666 pixels in memory. It could be either
> > 
> > Byte 0   Byte 1   Byte 2   Byte 3
> > 
> > -------- ------RR RRRRGGGG GGBBBBBB
> > 
> > or
> > 
> > GGBBBBBB RRRRGGGG ------RR --------
> > 
> > None of those correspond to the RGB666 format defined in the spec.
> > 
> > The Exynos4 FIMC isn't documented in the public datasheet, so I can't
> > check how the format is defined.
> > 
> > Furthermore, various Renesas video-related IP cores support many different
> > RGB666 variants, on either 32 or 24 bits per pixel, with and without
> > alpha.
> > 
> > Beside a loud *sigh*, any comment ? :-)
> 
> You'll have to check with Samsung then. Sylwester, can you shed any light on
> what this format *really* is?

Ping ?


-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH] v4l: Clarify RGB666 pixel format definition
  2014-09-09 13:18       ` Laurent Pinchart
@ 2014-09-09 14:45         ` Sylwester Nawrocki
  2014-10-29 11:45           ` Laurent Pinchart
  0 siblings, 1 reply; 8+ messages in thread
From: Sylwester Nawrocki @ 2014-09-09 14:45 UTC (permalink / raw)
  To: Laurent Pinchart, Hans Verkuil; +Cc: linux-media

On 09/09/14 15:18, Laurent Pinchart wrote:
> On Tuesday 22 July 2014 00:44:34 Hans Verkuil wrote:
>> On 07/22/2014 12:30 AM, Laurent Pinchart wrote:
>>> On Monday 21 July 2014 23:43:16 Hans Verkuil wrote:
>>>> On 07/21/2014 10:39 PM, Laurent Pinchart wrote:
>>>>> The RGB666 pixel format doesn't include an alpha channel. Document it as
>>>>> such.
>>>>>
>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>> ---
>>>>>
>>>>>  .../DocBook/media/v4l/pixfmt-packed-rgb.xml          | 20
>>>>>  +++++----------
>>>>>
>>>>> 1 file changed, 6 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>>>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
>>>>> 32feac9..c47692a 100644
>>>>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>>>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>>>> @@ -330,20 +330,12 @@ colorspace
>>>>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>>
>>>>>  	    <entry></entry>
>>>>>  	    <entry>r<subscript>1</subscript></entry>
>>>>>  	    <entry>r<subscript>0</subscript></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> -	    <entry></entry>
>>>>> +	    <entry>-</entry>
>>>>> +	    <entry>-</entry>
>>>>> +	    <entry>-</entry>
>>>>> +	    <entry>-</entry>
>>>>> +	    <entry>-</entry>
>>>>> +	    <entry>-</entry>
>>>>
>>>> Just to clarify: BGR666 is a three byte format, not a four byte format?
>>>
>>> Well... :-)
>>>
>>> Three drivers seem to support the BGR666 in mainline : sh_veu, s3c-camif
>>> and exynos4-is. Further investigation shows that the sh_veu driver lists
>>> the BGR666 format internally but doesn't expose it to userspace and
>>> doesn't actually support it, so we're down to two drivers.
>>>
>>> Looking at the S3C6410 datasheet, it's unclear how the hardware stores
>>> RGB666 pixels in memory. It could be either
>>>
>>> Byte 0   Byte 1   Byte 2   Byte 3
>>>
>>> -------- ------RR RRRRGGGG GGBBBBBB
>>>
>>> or
>>>
>>> GGBBBBBB RRRRGGGG ------RR --------
>>>
>>> None of those correspond to the RGB666 format defined in the spec.
>>>
>>> The Exynos4 FIMC isn't documented in the public datasheet, so I can't
>>> check how the format is defined.
>>>
>>> Furthermore, various Renesas video-related IP cores support many different
>>> RGB666 variants, on either 32 or 24 bits per pixel, with and without
>>> alpha.
>>>
>>> Beside a loud *sigh*, any comment ? :-)
>>
>> You'll have to check with Samsung then. Sylwester, can you shed any light on
>> what this format *really* is?
> 
> Ping ?

My apologies, I didn't notice this earlier.

In case of S5P/Exynos FIMC the format is:

Byte 0   Byte 1   Byte 2   Byte 3

BBBBBBGG GGGGRRRR RR------ --------

i.e. 4 byte per pixel, with 14-bit padding (don't care bits).

As far as S3C6410 CAMIF is concerned it's hard to say. I primarily
developed the s3c-camif driver for S3C2440 SoC, which doesn't support
BGR666 format. I merged some patches from others adding s3c6410 support,
before sending upstream.

Nevertheless, looking at the S3C CAMIF datasheet the RGB666 format seems
identical with the FIMC one. See [1], chapter "20.7.4 MEMORY STORING
METHOD". This would make sense, since the S5P/Exynos FIMC is basically
a significantly evolved S3C CAMIF AFAICT.

--
Regards,
Sylwester

[1] http://www.arm9board.net/download/OK6410/docs/S3C6410X.pdf

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

* Re: [PATCH] v4l: Clarify RGB666 pixel format definition
  2014-09-09 14:45         ` Sylwester Nawrocki
@ 2014-10-29 11:45           ` Laurent Pinchart
  2015-11-09 22:21             ` Laurent Pinchart
  0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2014-10-29 11:45 UTC (permalink / raw)
  To: Sylwester Nawrocki; +Cc: Hans Verkuil, linux-media

Hi Sylwester,

On Tuesday 09 September 2014 16:45:17 Sylwester Nawrocki wrote:
> On 09/09/14 15:18, Laurent Pinchart wrote:
> > On Tuesday 22 July 2014 00:44:34 Hans Verkuil wrote:
> >> On 07/22/2014 12:30 AM, Laurent Pinchart wrote:
> >>> On Monday 21 July 2014 23:43:16 Hans Verkuil wrote:
> >>>> On 07/21/2014 10:39 PM, Laurent Pinchart wrote:
> >>>>> The RGB666 pixel format doesn't include an alpha channel. Document it
> >>>>> as such.
> >>>>> 
> >>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>> ---
> >>>>> 
> >>>>>  .../DocBook/media/v4l/pixfmt-packed-rgb.xml          | 20 ++++-------
> >>>>> 
> >>>>> 1 file changed, 6 insertions(+), 14 deletions(-)
> >>>>> 
> >>>>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>>>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
> >>>>> 32feac9..c47692a 100644
> >>>>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>>>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>>>> @@ -330,20 +330,12 @@ colorspace
> >>>>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
> >>>>>  	    <entry></entry>
> >>>>>  	    <entry>r<subscript>1</subscript></entry>
> >>>>>  	    <entry>r<subscript>0</subscript></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> -	    <entry></entry>
> >>>>> +	    <entry>-</entry>
> >>>>> +	    <entry>-</entry>
> >>>>> +	    <entry>-</entry>
> >>>>> +	    <entry>-</entry>
> >>>>> +	    <entry>-</entry>
> >>>>> +	    <entry>-</entry>
> >>>> 
> >>>> Just to clarify: BGR666 is a three byte format, not a four byte format?
> >>> 
> >>> Well... :-)
> >>> 
> >>> Three drivers seem to support the BGR666 in mainline : sh_veu, s3c-camif
> >>> and exynos4-is. Further investigation shows that the sh_veu driver lists
> >>> the BGR666 format internally but doesn't expose it to userspace and
> >>> doesn't actually support it, so we're down to two drivers.
> >>> 
> >>> Looking at the S3C6410 datasheet, it's unclear how the hardware stores
> >>> RGB666 pixels in memory. It could be either
> >>> 
> >>> Byte 0   Byte 1   Byte 2   Byte 3
> >>> 
> >>> -------- ------RR RRRRGGGG GGBBBBBB
> >>> 
> >>> or
> >>> 
> >>> GGBBBBBB RRRRGGGG ------RR --------
> >>> 
> >>> None of those correspond to the RGB666 format defined in the spec.
> >>> 
> >>> The Exynos4 FIMC isn't documented in the public datasheet, so I can't
> >>> check how the format is defined.
> >>> 
> >>> Furthermore, various Renesas video-related IP cores support many
> >>> different RGB666 variants, on either 32 or 24 bits per pixel, with and
> >>> without alpha.
> >>> 
> >>> Beside a loud *sigh*, any comment ? :-)
> >> 
> >> You'll have to check with Samsung then. Sylwester, can you shed any light
> >> on what this format *really* is?
> > 
> > Ping ?
> 
> My apologies, I didn't notice this earlier.
> 
> In case of S5P/Exynos FIMC the format is:
> 
> Byte 0   Byte 1   Byte 2   Byte 3
> 
> BBBBBBGG GGGGRRRR RR------ --------
> 
> i.e. 4 byte per pixel, with 14-bit padding (don't care bits).
> 
> As far as S3C6410 CAMIF is concerned it's hard to say. I primarily
> developed the s3c-camif driver for S3C2440 SoC, which doesn't support
> BGR666 format. I merged some patches from others adding s3c6410 support,
> before sending upstream.
> 
> Nevertheless, looking at the S3C CAMIF datasheet the RGB666 format seems
> identical with the FIMC one. See [1], chapter "20.7.4 MEMORY STORING
> METHOD". This would make sense, since the S5P/Exynos FIMC is basically
> a significantly evolved S3C CAMIF AFAICT.

Looking at the figure, I would understand RGB666 as follows

Bits 31     24|23         16|15                    8|7                     0
     ---- ----|---- -- R5 R4|R3 R2 R1 R0 G5 G4 G3 G2|G1 G0 B5 B4 B3 B2 B1 B0

Assuming little endian memory format, that would thus be
 
Byte 0   Byte 1   Byte 2   Byte 3

GGBBBBBB RRRRGGGG ------RR --------

If the memory format was big endian it would instead be

-------- ------RR RRRRGGGG GGBBBBBB

> [1] http://www.arm9board.net/download/OK6410/docs/S3C6410X.pdf

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH] v4l: Clarify RGB666 pixel format definition
  2014-10-29 11:45           ` Laurent Pinchart
@ 2015-11-09 22:21             ` Laurent Pinchart
  0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2015-11-09 22:21 UTC (permalink / raw)
  To: Sylwester Nawrocki; +Cc: Hans Verkuil, linux-media

Hi Sylwester,

Reviving a "slightly" old discussion.

On Wednesday 29 October 2014 13:45:46 Laurent Pinchart wrote:
> On Tuesday 09 September 2014 16:45:17 Sylwester Nawrocki wrote:
> > On 09/09/14 15:18, Laurent Pinchart wrote:
> >> On Tuesday 22 July 2014 00:44:34 Hans Verkuil wrote:
> >>> On 07/22/2014 12:30 AM, Laurent Pinchart wrote:
> >>>> On Monday 21 July 2014 23:43:16 Hans Verkuil wrote:
> >>>>> On 07/21/2014 10:39 PM, Laurent Pinchart wrote:
> >>>>>> The RGB666 pixel format doesn't include an alpha channel. Document
> >>>>>> it as such.
> >>>>>> 
> >>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>> ---
> >>>>>> 
> >>>>>>  .../DocBook/media/v4l/pixfmt-packed-rgb.xml          | 20
> >>>>>>  ++++-------
> >>>>>> 
> >>>>>> 1 file changed, 6 insertions(+), 14 deletions(-)
> >>>>>> 
> >>>>>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>>>>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
> >>>>>> 32feac9..c47692a 100644
> >>>>>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>>>>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>>>>> @@ -330,20 +330,12 @@ colorspace
> >>>>>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
> >>>>>> 
> >>>>>>  	    <entry></entry>
> >>>>>>  	    <entry>r<subscript>1</subscript></entry>
> >>>>>>  	    <entry>r<subscript>0</subscript></entry>
> >>>>>> 
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> -	    <entry></entry>
> >>>>>> +	    <entry>-</entry>
> >>>>>> +	    <entry>-</entry>
> >>>>>> +	    <entry>-</entry>
> >>>>>> +	    <entry>-</entry>
> >>>>>> +	    <entry>-</entry>
> >>>>>> +	    <entry>-</entry>
> >>>>> 
> >>>>> Just to clarify: BGR666 is a three byte format, not a four byte
> >>>>> format?

[snip]

> > My apologies, I didn't notice this earlier.
> > 
> > In case of S5P/Exynos FIMC the format is:
> > 
> > Byte 0   Byte 1   Byte 2   Byte 3
> > 
> > BBBBBBGG GGGGRRRR RR------ --------
> > 
> > i.e. 4 byte per pixel, with 14-bit padding (don't care bits).
> > 
> > As far as S3C6410 CAMIF is concerned it's hard to say. I primarily
> > developed the s3c-camif driver for S3C2440 SoC, which doesn't support
> > BGR666 format. I merged some patches from others adding s3c6410 support,
> > before sending upstream.
> > 
> > Nevertheless, looking at the S3C CAMIF datasheet the RGB666 format seems
> > identical with the FIMC one. See [1], chapter "20.7.4 MEMORY STORING
> > METHOD". This would make sense, since the S5P/Exynos FIMC is basically
> > a significantly evolved S3C CAMIF AFAICT.
> 
> Looking at the figure, I would understand RGB666 as follows
> 
> Bits 31     24|23         16|15                    8|7                     0
> ---- ----|---- -- R5 R4|R3 R2 R1 R0 G5 G4 G3 G2|G1 G0 B5 B4 B3 B2 B1 B0
> 
> Assuming little endian memory format, that would thus be
> 
> Byte 0   Byte 1   Byte 2   Byte 3
> 
> GGBBBBBB RRRRGGGG ------RR --------
> 
> If the memory format was big endian it would instead be
> 
> -------- ------RR RRRRGGGG GGBBBBBB
> 
> > [1] http://www.arm9board.net/download/OK6410/docs/S3C6410X.pdf

Did I understand something incorrectly ?

-- 
Regards,

Laurent Pinchart


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

end of thread, other threads:[~2015-11-09 22:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-21 20:39 [PATCH] v4l: Clarify RGB666 pixel format definition Laurent Pinchart
2014-07-21 21:43 ` Hans Verkuil
2014-07-21 22:30   ` Laurent Pinchart
2014-07-21 22:44     ` Hans Verkuil
2014-09-09 13:18       ` Laurent Pinchart
2014-09-09 14:45         ` Sylwester Nawrocki
2014-10-29 11:45           ` Laurent Pinchart
2015-11-09 22:21             ` Laurent Pinchart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.