All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] Standardize YUV support in the fbdev API
@ 2011-05-17 22:07 ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-05-17 22:07 UTC (permalink / raw)
  To: linux-fbdev; +Cc: linux-media, dri-devel

Hi everybody,

I need to implement support for a YUV frame buffer in an fbdev driver. As the 
fbdev API doesn't support this out of the box, I've spent a couple of days 
reading fbdev (and KMS) code and thinking about how we could cleanly add YUV 
support to the API. I'd like to share my findings and thoughts, and hopefully 
receive some comments back.

The terms 'format', 'pixel format', 'frame buffer format' and 'data format' 
will be used interchangeably in this e-mail. They all refer to the way pixels 
are stored in memory, including both the representation of a pixel as integer 
values and the layout of those integer values in memory.


History and current situation
-----------------------------

The fbdev API predates YUV frame buffers. In those old days developers only 
cared (and probably thought) about RGB frame buffers, and they developed an 
API that could express all kind of weird RGB layout in memory (such as R-
GGGGGGGGGGG-BBBB for instance, I can't imagine hardware implementing that). 
This resulted in individual bit field description for the red, green, blue and 
alpha components:

struct fb_bitfield {
        __u32 offset;                  /* beginning of bitfield        */
        __u32 length;                  /* length of bitfield           */
        __u32 msb_right;               /* != 0 : Most significant bit is */
                                       /* right */
};

Grayscale formats were pretty common, so a grayscale field tells color formats 
(grayscale == 0) from grayscale formats (grayscale != 0).

People already realized that hardware developers were crazily inventive (the 
word to remember here is crazily), and that non-standard formats would be 
needed at some point. The fb_var_screeninfo ended up containing the following 
format-related fields.

struct fb_var_screeninfo {
        ...
        __u32 bits_per_pixel;          /* guess what                   */
        __u32 grayscale;               /* != 0 Graylevels instead of colors */

        struct fb_bitfield red;        /* bitfield in fb mem if true color, */
        struct fb_bitfield green;      /* else only length is significant */
        struct fb_bitfield blue;
        struct fb_bitfield transp;     /* transparency                 */

        __u32 nonstd;                  /* != 0 Non standard pixel format */
        ...
};

Two bits have been specified for the nonstd field:

#define FB_NONSTD_HAM           1  /* Hold-And-Modify (HAM)        */
#define FB_NONSTD_REV_PIX_IN_B  2  /* order of pixels in each byte is reversed 
*/

The FB_NONSTD_HAM bit is used by the video/amifb.c driver only to enable HAM 
mode[1]. I very much doubt that any new hardware will implement HAM mode (and 
I certainly hope none will).

The FB_NONSTD_REV_PIX_IN_B is used in video/fb_draw.h by the generic bitblit, 
fillrect and copy area implementations, not directly by drivers.

A couple of drivers hardcode the nonstd field to 1 for some reason. Those are 
video/arcfb.c (1bpp gray display), video/hecubafb.c (1bpp gray display) and 
video/metronomefb.c (8bpp gray display).

The following drivers use nonstd for various other (and sometimes weird) 
purposes:

video/arkfb.c
        Used in 4bpp mode only, to control fb_setcolreg operation
video/fsl-diu-fb.c
        Set var->nonstd bits into var->sync (why?)
video/pxafb.c
        Encode frame buffer xpos and ypos in the nonstd field
video/s3fb.c
        Used in 4bpp mode only, to control fb_setcolreg operation
video/vga16fb.c
        When panning in non-8bpp, non-text mode, decrement xoffset
        Do some other weird stuff when not 0
video/i810/i810_main.c
        Select direct color mode when set to 1 (truecolor otherwise)

Fast forward a couple of years, hardware provides support for YUV frame 
buffers. Several drivers had to provide YUV format selection to applications, 
without any standard API to do so. All of them used the nonstd field for that 
purpose:

media/video/ivtv/
        Enable YUV mode when set to 1
video/pxafb.c
        Encode pixel format in the nonstd field
video/sh_mobile_lcdfb.c
        If bpp == 12 and nonstd != 0, enable NV12 mode
        If bpp == 16 or bpp == 24, ?
video/omap/omapfb_main.c
        Select direct color mode when set to 1 (depend on bpp otherwise)
        Used as a pixel format identifier (YUV422, YUV420 or YUY422)
video/omap2/omapfb/omapfb-main.c
        Select direct color mode when set to 1 (depend on bpp otherwise)
        Used as a pixel format identifier (YUV422 or YUY422)

Those drivers use the nonstd field in different, incompatible ways.


Other related APIs
------------------

Beside the fbdev API, two other kernel/userspace APIs deal with video-related 
modes and formats.

- Kernel Mode Setting (KMS)

The KMS API is similar in purpose to XRandR. Its main purpose is to provide a 
kernel API to configure output video modes. As such, it doesn't care about 
frame buffer formats, as they are irrelevant at the CRTC output.

In addition to handling video modes, the KMS API also provides support for 
creating (and managing) frame buffer devices, as well as dumb scan-out 
buffers. In-memory data format is relevant there, but KMS only handles width, 
height, pitch, depth and bit-per-pixel information. The API doesn't care 
whether the frame buffer or the dumb scan-out buffer contains RGB or YUV data.

An RFC[2] has recently been posted to the dri-devel mailing list to "add 
overlays as first class KMS objects". The proposal includes explicit overlay 
format support, and discussions have so far pushed towards reusing V4L format 
codes for the KMS API.

- Video 4 Linux (V4L)

The V4L API version 2 (usually called V4L2) has since the beginning included 
explicit support for data format, referred to as pixel formats.

Pixel formats are identified by a 32-bit number in the form of a four 
characters code (4CC or FCC[3]). The list of FCCs defined by V4L2 is available 
in [4].

A pixel format uniquely defines the layout of pixel data in memory, including 
the format type (RGB, YUV, ...), number of bits per components, components 
order and subsampling. It doesn't define the range of acceptable values for 
pixel components (such as full-range YUV vs. BT.601[5]), which is carried 
through a separate colorspace identifier[6].


YUV support in the fbdev API
----------------------------

Experience with the V4L2 API, both for desktop and embedded devices, and the 
KMS API, suggests that recent hardware tend to standardize on a relatively 
small number of pixel formats that don't require bitfield-level descriptions. 
A pixel format definition based on identifiers should thus fullfill the 
hardware needs for the foreseeable future.

Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share data 
and buffers between those subsystems, and the planned use of V4L2 FCCs in the 
KMS overlay API, I believe using V4L2 FCCs to identify fbdev formats would be 
a wise decision.

To select a frame buffer YUV format, the fb_var_screeninfo structure will need 
to be extended with a format field. The fbdev API and ABI must not be broken, 
which prevents us from changing the current structure layout and replacing the 
existing format selection mechanism (through the red, green, blue and alpha 
bitfields) completely.

Several solutions exist to introduce a format field in the fb_var_screeninfo 
structure.

- Use the nonstd field as a format identifier. Existing drivers that use the 
nonstd field for other purposes wouldn't be able to implement the new API 
while keeping their existing API. This isn't a show stopper for drivers using 
the FB_NONSTD_HAM and FB_NONSTD_REV_PIX_IN_B bits, as they don't need to 
support any non-RGB format.

Existing drivers that support YUV formats through the nonstd field would have 
to select between the current and the new API, without being able to implement 
both.

- Use one of the reserved fields as a format identifier. Applications are 
supposed to zero the fb_var_screeninfo structure before passing it to the 
kernel, but experience showed that many applications forget to do so. Drivers 
would then be unable to find out whether applications request a particular 
format, or just forgot to initialize the field.

- Add a new FB_NONSTD_FORMAT bit to the nonstd field, and pass the format 
through a separate field. If an application sets the FB_NONSTD_FORMAT bit in 
the nonstd field, drivers will ignore the red, green, blue, transp and 
grayscale fields, and use the format identifier to configure the frame buffer 
format. The grayscale field would then be unneeded and could be reused as a 
format identifier. Alternatively, one of the reserved fields could be used for 
that purpose.

Existing drivers that support YUV formats through the nonstd field don't use 
the field's most significant bits. They could implement both their current API 
and the new API if the FB_NONSTD_FORMAT bit is stored in one of the nonstd 
MSBs.

- Other solutions are possible, such as adding new ioctls. Those solutions are 
more intrusive, and require larger changes to both userspace and kernelspace 
code.


The third solution has my preference. Comments and feedback will be 
appreciated. I will then work on a proof of concept and submit patches.


[1] http://en.wikipedia.org/wiki/Hold_And_Modify
[2] http://lwn.net/Articles/440192/
[3] http://www.fourcc.org/
[4] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L271
[5] http://en.wikipedia.org/wiki/Rec._601
[6] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L175

-- 
Regards,

Laurent Pinchart

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

* [RFC] Standardize YUV support in the fbdev API
@ 2011-05-17 22:07 ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-05-17 22:07 UTC (permalink / raw)
  To: linux-fbdev; +Cc: linux-media, dri-devel

Hi everybody,

I need to implement support for a YUV frame buffer in an fbdev driver. As the 
fbdev API doesn't support this out of the box, I've spent a couple of days 
reading fbdev (and KMS) code and thinking about how we could cleanly add YUV 
support to the API. I'd like to share my findings and thoughts, and hopefully 
receive some comments back.

The terms 'format', 'pixel format', 'frame buffer format' and 'data format' 
will be used interchangeably in this e-mail. They all refer to the way pixels 
are stored in memory, including both the representation of a pixel as integer 
values and the layout of those integer values in memory.


History and current situation
-----------------------------

The fbdev API predates YUV frame buffers. In those old days developers only 
cared (and probably thought) about RGB frame buffers, and they developed an 
API that could express all kind of weird RGB layout in memory (such as R-
GGGGGGGGGGG-BBBB for instance, I can't imagine hardware implementing that). 
This resulted in individual bit field description for the red, green, blue and 
alpha components:

struct fb_bitfield {
        __u32 offset;                  /* beginning of bitfield        */
        __u32 length;                  /* length of bitfield           */
        __u32 msb_right;               /* != 0 : Most significant bit is */
                                       /* right */
};

Grayscale formats were pretty common, so a grayscale field tells color formats 
(grayscale = 0) from grayscale formats (grayscale != 0).

People already realized that hardware developers were crazily inventive (the 
word to remember here is crazily), and that non-standard formats would be 
needed at some point. The fb_var_screeninfo ended up containing the following 
format-related fields.

struct fb_var_screeninfo {
        ...
        __u32 bits_per_pixel;          /* guess what                   */
        __u32 grayscale;               /* != 0 Graylevels instead of colors */

        struct fb_bitfield red;        /* bitfield in fb mem if true color, */
        struct fb_bitfield green;      /* else only length is significant */
        struct fb_bitfield blue;
        struct fb_bitfield transp;     /* transparency                 */

        __u32 nonstd;                  /* != 0 Non standard pixel format */
        ...
};

Two bits have been specified for the nonstd field:

#define FB_NONSTD_HAM           1  /* Hold-And-Modify (HAM)        */
#define FB_NONSTD_REV_PIX_IN_B  2  /* order of pixels in each byte is reversed 
*/

The FB_NONSTD_HAM bit is used by the video/amifb.c driver only to enable HAM 
mode[1]. I very much doubt that any new hardware will implement HAM mode (and 
I certainly hope none will).

The FB_NONSTD_REV_PIX_IN_B is used in video/fb_draw.h by the generic bitblit, 
fillrect and copy area implementations, not directly by drivers.

A couple of drivers hardcode the nonstd field to 1 for some reason. Those are 
video/arcfb.c (1bpp gray display), video/hecubafb.c (1bpp gray display) and 
video/metronomefb.c (8bpp gray display).

The following drivers use nonstd for various other (and sometimes weird) 
purposes:

video/arkfb.c
        Used in 4bpp mode only, to control fb_setcolreg operation
video/fsl-diu-fb.c
        Set var->nonstd bits into var->sync (why?)
video/pxafb.c
        Encode frame buffer xpos and ypos in the nonstd field
video/s3fb.c
        Used in 4bpp mode only, to control fb_setcolreg operation
video/vga16fb.c
        When panning in non-8bpp, non-text mode, decrement xoffset
        Do some other weird stuff when not 0
video/i810/i810_main.c
        Select direct color mode when set to 1 (truecolor otherwise)

Fast forward a couple of years, hardware provides support for YUV frame 
buffers. Several drivers had to provide YUV format selection to applications, 
without any standard API to do so. All of them used the nonstd field for that 
purpose:

media/video/ivtv/
        Enable YUV mode when set to 1
video/pxafb.c
        Encode pixel format in the nonstd field
video/sh_mobile_lcdfb.c
        If bpp = 12 and nonstd != 0, enable NV12 mode
        If bpp = 16 or bpp = 24, ?
video/omap/omapfb_main.c
        Select direct color mode when set to 1 (depend on bpp otherwise)
        Used as a pixel format identifier (YUV422, YUV420 or YUY422)
video/omap2/omapfb/omapfb-main.c
        Select direct color mode when set to 1 (depend on bpp otherwise)
        Used as a pixel format identifier (YUV422 or YUY422)

Those drivers use the nonstd field in different, incompatible ways.


Other related APIs
------------------

Beside the fbdev API, two other kernel/userspace APIs deal with video-related 
modes and formats.

- Kernel Mode Setting (KMS)

The KMS API is similar in purpose to XRandR. Its main purpose is to provide a 
kernel API to configure output video modes. As such, it doesn't care about 
frame buffer formats, as they are irrelevant at the CRTC output.

In addition to handling video modes, the KMS API also provides support for 
creating (and managing) frame buffer devices, as well as dumb scan-out 
buffers. In-memory data format is relevant there, but KMS only handles width, 
height, pitch, depth and bit-per-pixel information. The API doesn't care 
whether the frame buffer or the dumb scan-out buffer contains RGB or YUV data.

An RFC[2] has recently been posted to the dri-devel mailing list to "add 
overlays as first class KMS objects". The proposal includes explicit overlay 
format support, and discussions have so far pushed towards reusing V4L format 
codes for the KMS API.

- Video 4 Linux (V4L)

The V4L API version 2 (usually called V4L2) has since the beginning included 
explicit support for data format, referred to as pixel formats.

Pixel formats are identified by a 32-bit number in the form of a four 
characters code (4CC or FCC[3]). The list of FCCs defined by V4L2 is available 
in [4].

A pixel format uniquely defines the layout of pixel data in memory, including 
the format type (RGB, YUV, ...), number of bits per components, components 
order and subsampling. It doesn't define the range of acceptable values for 
pixel components (such as full-range YUV vs. BT.601[5]), which is carried 
through a separate colorspace identifier[6].


YUV support in the fbdev API
----------------------------

Experience with the V4L2 API, both for desktop and embedded devices, and the 
KMS API, suggests that recent hardware tend to standardize on a relatively 
small number of pixel formats that don't require bitfield-level descriptions. 
A pixel format definition based on identifiers should thus fullfill the 
hardware needs for the foreseeable future.

Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share data 
and buffers between those subsystems, and the planned use of V4L2 FCCs in the 
KMS overlay API, I believe using V4L2 FCCs to identify fbdev formats would be 
a wise decision.

To select a frame buffer YUV format, the fb_var_screeninfo structure will need 
to be extended with a format field. The fbdev API and ABI must not be broken, 
which prevents us from changing the current structure layout and replacing the 
existing format selection mechanism (through the red, green, blue and alpha 
bitfields) completely.

Several solutions exist to introduce a format field in the fb_var_screeninfo 
structure.

- Use the nonstd field as a format identifier. Existing drivers that use the 
nonstd field for other purposes wouldn't be able to implement the new API 
while keeping their existing API. This isn't a show stopper for drivers using 
the FB_NONSTD_HAM and FB_NONSTD_REV_PIX_IN_B bits, as they don't need to 
support any non-RGB format.

Existing drivers that support YUV formats through the nonstd field would have 
to select between the current and the new API, without being able to implement 
both.

- Use one of the reserved fields as a format identifier. Applications are 
supposed to zero the fb_var_screeninfo structure before passing it to the 
kernel, but experience showed that many applications forget to do so. Drivers 
would then be unable to find out whether applications request a particular 
format, or just forgot to initialize the field.

- Add a new FB_NONSTD_FORMAT bit to the nonstd field, and pass the format 
through a separate field. If an application sets the FB_NONSTD_FORMAT bit in 
the nonstd field, drivers will ignore the red, green, blue, transp and 
grayscale fields, and use the format identifier to configure the frame buffer 
format. The grayscale field would then be unneeded and could be reused as a 
format identifier. Alternatively, one of the reserved fields could be used for 
that purpose.

Existing drivers that support YUV formats through the nonstd field don't use 
the field's most significant bits. They could implement both their current API 
and the new API if the FB_NONSTD_FORMAT bit is stored in one of the nonstd 
MSBs.

- Other solutions are possible, such as adding new ioctls. Those solutions are 
more intrusive, and require larger changes to both userspace and kernelspace 
code.


The third solution has my preference. Comments and feedback will be 
appreciated. I will then work on a proof of concept and submit patches.


[1] http://en.wikipedia.org/wiki/Hold_And_Modify
[2] http://lwn.net/Articles/440192/
[3] http://www.fourcc.org/
[4] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L271
[5] http://en.wikipedia.org/wiki/Rec._601
[6] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L175

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC] Standardize YUV support in the fbdev API
  2011-05-17 22:07 ` Laurent Pinchart
@ 2011-05-17 22:44   ` Felipe Contreras
  -1 siblings, 0 replies; 63+ messages in thread
From: Felipe Contreras @ 2011-05-17 22:44 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> I need to implement support for a YUV frame buffer in an fbdev driver. As the
> fbdev API doesn't support this out of the box, I've spent a couple of days
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> support to the API. I'd like to share my findings and thoughts, and hopefully
> receive some comments back.
>
> The terms 'format', 'pixel format', 'frame buffer format' and 'data format'
> will be used interchangeably in this e-mail. They all refer to the way pixels
> are stored in memory, including both the representation of a pixel as integer
> values and the layout of those integer values in memory.

This is a great proposal. It was about time!

> The third solution has my preference. Comments and feedback will be
> appreciated. I will then work on a proof of concept and submit patches.

I also would prefer the third solution. I don't think there's much
difference from the user-space point of view, and a new ioctl would be
cleaner. Also the v4l2 fourcc's should do.

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-17 22:44   ` Felipe Contreras
  0 siblings, 0 replies; 63+ messages in thread
From: Felipe Contreras @ 2011-05-17 22:44 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> I need to implement support for a YUV frame buffer in an fbdev driver. As the
> fbdev API doesn't support this out of the box, I've spent a couple of days
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> support to the API. I'd like to share my findings and thoughts, and hopefully
> receive some comments back.
>
> The terms 'format', 'pixel format', 'frame buffer format' and 'data format'
> will be used interchangeably in this e-mail. They all refer to the way pixels
> are stored in memory, including both the representation of a pixel as integer
> values and the layout of those integer values in memory.

This is a great proposal. It was about time!

> The third solution has my preference. Comments and feedback will be
> appreciated. I will then work on a proof of concept and submit patches.

I also would prefer the third solution. I don't think there's much
difference from the user-space point of view, and a new ioctl would be
cleaner. Also the v4l2 fourcc's should do.

Cheers.

-- 
Felipe Contreras

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

* Re: [RFC] Standardize YUV support in the fbdev API
  2011-05-17 22:07 ` Laurent Pinchart
@ 2011-05-18  0:21   ` Andy Walls
  -1 siblings, 0 replies; 63+ messages in thread
From: Andy Walls @ 2011-05-18  0:21 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

On Wed, 2011-05-18 at 00:07 +0200, Laurent Pinchart wrote:
> Hi everybody,
> 
> I need to implement support for a YUV frame buffer in an fbdev driver. As the 
> fbdev API doesn't support this out of the box, I've spent a couple of days 
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV 
> support to the API. I'd like to share my findings and thoughts, and hopefully 
> receive some comments back.

I haven't looked at anything below, but I'll mention that the ivtv
driver presents an fbdev interface for the YUV output stream of the
CX23415.

It may be worth a look and asking Hans what are the short-comings.

-Andy


> The terms 'format', 'pixel format', 'frame buffer format' and 'data format' 
> will be used interchangeably in this e-mail. They all refer to the way pixels 
> are stored in memory, including both the representation of a pixel as integer 
> values and the layout of those integer values in memory.
> 
> 
> History and current situation
> -----------------------------
> 
> The fbdev API predates YUV frame buffers. In those old days developers only 
> cared (and probably thought) about RGB frame buffers, and they developed an 
> API that could express all kind of weird RGB layout in memory (such as R-
> GGGGGGGGGGG-BBBB for instance, I can't imagine hardware implementing that). 
> This resulted in individual bit field description for the red, green, blue and 
> alpha components:
> 
> struct fb_bitfield {
>         __u32 offset;                  /* beginning of bitfield        */
>         __u32 length;                  /* length of bitfield           */
>         __u32 msb_right;               /* != 0 : Most significant bit is */
>                                        /* right */
> };
> 
> Grayscale formats were pretty common, so a grayscale field tells color formats 
> (grayscale == 0) from grayscale formats (grayscale != 0).
> 
> People already realized that hardware developers were crazily inventive (the 
> word to remember here is crazily), and that non-standard formats would be 
> needed at some point. The fb_var_screeninfo ended up containing the following 
> format-related fields.
> 
> struct fb_var_screeninfo {
>         ...
>         __u32 bits_per_pixel;          /* guess what                   */
>         __u32 grayscale;               /* != 0 Graylevels instead of colors */
> 
>         struct fb_bitfield red;        /* bitfield in fb mem if true color, */
>         struct fb_bitfield green;      /* else only length is significant */
>         struct fb_bitfield blue;
>         struct fb_bitfield transp;     /* transparency                 */
> 
>         __u32 nonstd;                  /* != 0 Non standard pixel format */
>         ...
> };
> 
> Two bits have been specified for the nonstd field:
> 
> #define FB_NONSTD_HAM           1  /* Hold-And-Modify (HAM)        */
> #define FB_NONSTD_REV_PIX_IN_B  2  /* order of pixels in each byte is reversed 
> */
> 
> The FB_NONSTD_HAM bit is used by the video/amifb.c driver only to enable HAM 
> mode[1]. I very much doubt that any new hardware will implement HAM mode (and 
> I certainly hope none will).
> 
> The FB_NONSTD_REV_PIX_IN_B is used in video/fb_draw.h by the generic bitblit, 
> fillrect and copy area implementations, not directly by drivers.
> 
> A couple of drivers hardcode the nonstd field to 1 for some reason. Those are 
> video/arcfb.c (1bpp gray display), video/hecubafb.c (1bpp gray display) and 
> video/metronomefb.c (8bpp gray display).
> 
> The following drivers use nonstd for various other (and sometimes weird) 
> purposes:
> 
> video/arkfb.c
>         Used in 4bpp mode only, to control fb_setcolreg operation
> video/fsl-diu-fb.c
>         Set var->nonstd bits into var->sync (why?)
> video/pxafb.c
>         Encode frame buffer xpos and ypos in the nonstd field
> video/s3fb.c
>         Used in 4bpp mode only, to control fb_setcolreg operation
> video/vga16fb.c
>         When panning in non-8bpp, non-text mode, decrement xoffset
>         Do some other weird stuff when not 0
> video/i810/i810_main.c
>         Select direct color mode when set to 1 (truecolor otherwise)
> 
> Fast forward a couple of years, hardware provides support for YUV frame 
> buffers. Several drivers had to provide YUV format selection to applications, 
> without any standard API to do so. All of them used the nonstd field for that 
> purpose:
> 
> media/video/ivtv/
>         Enable YUV mode when set to 1
> video/pxafb.c
>         Encode pixel format in the nonstd field
> video/sh_mobile_lcdfb.c
>         If bpp == 12 and nonstd != 0, enable NV12 mode
>         If bpp == 16 or bpp == 24, ?
> video/omap/omapfb_main.c
>         Select direct color mode when set to 1 (depend on bpp otherwise)
>         Used as a pixel format identifier (YUV422, YUV420 or YUY422)
> video/omap2/omapfb/omapfb-main.c
>         Select direct color mode when set to 1 (depend on bpp otherwise)
>         Used as a pixel format identifier (YUV422 or YUY422)
> 
> Those drivers use the nonstd field in different, incompatible ways.
> 
> 
> Other related APIs
> ------------------
> 
> Beside the fbdev API, two other kernel/userspace APIs deal with video-related 
> modes and formats.
> 
> - Kernel Mode Setting (KMS)
> 
> The KMS API is similar in purpose to XRandR. Its main purpose is to provide a 
> kernel API to configure output video modes. As such, it doesn't care about 
> frame buffer formats, as they are irrelevant at the CRTC output.
> 
> In addition to handling video modes, the KMS API also provides support for 
> creating (and managing) frame buffer devices, as well as dumb scan-out 
> buffers. In-memory data format is relevant there, but KMS only handles width, 
> height, pitch, depth and bit-per-pixel information. The API doesn't care 
> whether the frame buffer or the dumb scan-out buffer contains RGB or YUV data.
> 
> An RFC[2] has recently been posted to the dri-devel mailing list to "add 
> overlays as first class KMS objects". The proposal includes explicit overlay 
> format support, and discussions have so far pushed towards reusing V4L format 
> codes for the KMS API.
> 
> - Video 4 Linux (V4L)
> 
> The V4L API version 2 (usually called V4L2) has since the beginning included 
> explicit support for data format, referred to as pixel formats.
> 
> Pixel formats are identified by a 32-bit number in the form of a four 
> characters code (4CC or FCC[3]). The list of FCCs defined by V4L2 is available 
> in [4].
> 
> A pixel format uniquely defines the layout of pixel data in memory, including 
> the format type (RGB, YUV, ...), number of bits per components, components 
> order and subsampling. It doesn't define the range of acceptable values for 
> pixel components (such as full-range YUV vs. BT.601[5]), which is carried 
> through a separate colorspace identifier[6].
> 
> 
> YUV support in the fbdev API
> ----------------------------
> 
> Experience with the V4L2 API, both for desktop and embedded devices, and the 
> KMS API, suggests that recent hardware tend to standardize on a relatively 
> small number of pixel formats that don't require bitfield-level descriptions. 
> A pixel format definition based on identifiers should thus fullfill the 
> hardware needs for the foreseeable future.
> 
> Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share data 
> and buffers between those subsystems, and the planned use of V4L2 FCCs in the 
> KMS overlay API, I believe using V4L2 FCCs to identify fbdev formats would be 
> a wise decision.
> 
> To select a frame buffer YUV format, the fb_var_screeninfo structure will need 
> to be extended with a format field. The fbdev API and ABI must not be broken, 
> which prevents us from changing the current structure layout and replacing the 
> existing format selection mechanism (through the red, green, blue and alpha 
> bitfields) completely.
> 
> Several solutions exist to introduce a format field in the fb_var_screeninfo 
> structure.
> 
> - Use the nonstd field as a format identifier. Existing drivers that use the 
> nonstd field for other purposes wouldn't be able to implement the new API 
> while keeping their existing API. This isn't a show stopper for drivers using 
> the FB_NONSTD_HAM and FB_NONSTD_REV_PIX_IN_B bits, as they don't need to 
> support any non-RGB format.
> 
> Existing drivers that support YUV formats through the nonstd field would have 
> to select between the current and the new API, without being able to implement 
> both.
> 
> - Use one of the reserved fields as a format identifier. Applications are 
> supposed to zero the fb_var_screeninfo structure before passing it to the 
> kernel, but experience showed that many applications forget to do so. Drivers 
> would then be unable to find out whether applications request a particular 
> format, or just forgot to initialize the field.
> 
> - Add a new FB_NONSTD_FORMAT bit to the nonstd field, and pass the format 
> through a separate field. If an application sets the FB_NONSTD_FORMAT bit in 
> the nonstd field, drivers will ignore the red, green, blue, transp and 
> grayscale fields, and use the format identifier to configure the frame buffer 
> format. The grayscale field would then be unneeded and could be reused as a 
> format identifier. Alternatively, one of the reserved fields could be used for 
> that purpose.
> 
> Existing drivers that support YUV formats through the nonstd field don't use 
> the field's most significant bits. They could implement both their current API 
> and the new API if the FB_NONSTD_FORMAT bit is stored in one of the nonstd 
> MSBs.
> 
> - Other solutions are possible, such as adding new ioctls. Those solutions are 
> more intrusive, and require larger changes to both userspace and kernelspace 
> code.
> 
> 
> The third solution has my preference. Comments and feedback will be 
> appreciated. I will then work on a proof of concept and submit patches.
> 
> 
> [1] http://en.wikipedia.org/wiki/Hold_And_Modify
> [2] http://lwn.net/Articles/440192/
> [3] http://www.fourcc.org/
> [4] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L271
> [5] http://en.wikipedia.org/wiki/Rec._601
> [6] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L175
> 



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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-18  0:21   ` Andy Walls
  0 siblings, 0 replies; 63+ messages in thread
From: Andy Walls @ 2011-05-18  0:21 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

On Wed, 2011-05-18 at 00:07 +0200, Laurent Pinchart wrote:
> Hi everybody,
> 
> I need to implement support for a YUV frame buffer in an fbdev driver. As the 
> fbdev API doesn't support this out of the box, I've spent a couple of days 
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV 
> support to the API. I'd like to share my findings and thoughts, and hopefully 
> receive some comments back.

I haven't looked at anything below, but I'll mention that the ivtv
driver presents an fbdev interface for the YUV output stream of the
CX23415.

It may be worth a look and asking Hans what are the short-comings.

-Andy


> The terms 'format', 'pixel format', 'frame buffer format' and 'data format' 
> will be used interchangeably in this e-mail. They all refer to the way pixels 
> are stored in memory, including both the representation of a pixel as integer 
> values and the layout of those integer values in memory.
> 
> 
> History and current situation
> -----------------------------
> 
> The fbdev API predates YUV frame buffers. In those old days developers only 
> cared (and probably thought) about RGB frame buffers, and they developed an 
> API that could express all kind of weird RGB layout in memory (such as R-
> GGGGGGGGGGG-BBBB for instance, I can't imagine hardware implementing that). 
> This resulted in individual bit field description for the red, green, blue and 
> alpha components:
> 
> struct fb_bitfield {
>         __u32 offset;                  /* beginning of bitfield        */
>         __u32 length;                  /* length of bitfield           */
>         __u32 msb_right;               /* != 0 : Most significant bit is */
>                                        /* right */
> };
> 
> Grayscale formats were pretty common, so a grayscale field tells color formats 
> (grayscale = 0) from grayscale formats (grayscale != 0).
> 
> People already realized that hardware developers were crazily inventive (the 
> word to remember here is crazily), and that non-standard formats would be 
> needed at some point. The fb_var_screeninfo ended up containing the following 
> format-related fields.
> 
> struct fb_var_screeninfo {
>         ...
>         __u32 bits_per_pixel;          /* guess what                   */
>         __u32 grayscale;               /* != 0 Graylevels instead of colors */
> 
>         struct fb_bitfield red;        /* bitfield in fb mem if true color, */
>         struct fb_bitfield green;      /* else only length is significant */
>         struct fb_bitfield blue;
>         struct fb_bitfield transp;     /* transparency                 */
> 
>         __u32 nonstd;                  /* != 0 Non standard pixel format */
>         ...
> };
> 
> Two bits have been specified for the nonstd field:
> 
> #define FB_NONSTD_HAM           1  /* Hold-And-Modify (HAM)        */
> #define FB_NONSTD_REV_PIX_IN_B  2  /* order of pixels in each byte is reversed 
> */
> 
> The FB_NONSTD_HAM bit is used by the video/amifb.c driver only to enable HAM 
> mode[1]. I very much doubt that any new hardware will implement HAM mode (and 
> I certainly hope none will).
> 
> The FB_NONSTD_REV_PIX_IN_B is used in video/fb_draw.h by the generic bitblit, 
> fillrect and copy area implementations, not directly by drivers.
> 
> A couple of drivers hardcode the nonstd field to 1 for some reason. Those are 
> video/arcfb.c (1bpp gray display), video/hecubafb.c (1bpp gray display) and 
> video/metronomefb.c (8bpp gray display).
> 
> The following drivers use nonstd for various other (and sometimes weird) 
> purposes:
> 
> video/arkfb.c
>         Used in 4bpp mode only, to control fb_setcolreg operation
> video/fsl-diu-fb.c
>         Set var->nonstd bits into var->sync (why?)
> video/pxafb.c
>         Encode frame buffer xpos and ypos in the nonstd field
> video/s3fb.c
>         Used in 4bpp mode only, to control fb_setcolreg operation
> video/vga16fb.c
>         When panning in non-8bpp, non-text mode, decrement xoffset
>         Do some other weird stuff when not 0
> video/i810/i810_main.c
>         Select direct color mode when set to 1 (truecolor otherwise)
> 
> Fast forward a couple of years, hardware provides support for YUV frame 
> buffers. Several drivers had to provide YUV format selection to applications, 
> without any standard API to do so. All of them used the nonstd field for that 
> purpose:
> 
> media/video/ivtv/
>         Enable YUV mode when set to 1
> video/pxafb.c
>         Encode pixel format in the nonstd field
> video/sh_mobile_lcdfb.c
>         If bpp = 12 and nonstd != 0, enable NV12 mode
>         If bpp = 16 or bpp = 24, ?
> video/omap/omapfb_main.c
>         Select direct color mode when set to 1 (depend on bpp otherwise)
>         Used as a pixel format identifier (YUV422, YUV420 or YUY422)
> video/omap2/omapfb/omapfb-main.c
>         Select direct color mode when set to 1 (depend on bpp otherwise)
>         Used as a pixel format identifier (YUV422 or YUY422)
> 
> Those drivers use the nonstd field in different, incompatible ways.
> 
> 
> Other related APIs
> ------------------
> 
> Beside the fbdev API, two other kernel/userspace APIs deal with video-related 
> modes and formats.
> 
> - Kernel Mode Setting (KMS)
> 
> The KMS API is similar in purpose to XRandR. Its main purpose is to provide a 
> kernel API to configure output video modes. As such, it doesn't care about 
> frame buffer formats, as they are irrelevant at the CRTC output.
> 
> In addition to handling video modes, the KMS API also provides support for 
> creating (and managing) frame buffer devices, as well as dumb scan-out 
> buffers. In-memory data format is relevant there, but KMS only handles width, 
> height, pitch, depth and bit-per-pixel information. The API doesn't care 
> whether the frame buffer or the dumb scan-out buffer contains RGB or YUV data.
> 
> An RFC[2] has recently been posted to the dri-devel mailing list to "add 
> overlays as first class KMS objects". The proposal includes explicit overlay 
> format support, and discussions have so far pushed towards reusing V4L format 
> codes for the KMS API.
> 
> - Video 4 Linux (V4L)
> 
> The V4L API version 2 (usually called V4L2) has since the beginning included 
> explicit support for data format, referred to as pixel formats.
> 
> Pixel formats are identified by a 32-bit number in the form of a four 
> characters code (4CC or FCC[3]). The list of FCCs defined by V4L2 is available 
> in [4].
> 
> A pixel format uniquely defines the layout of pixel data in memory, including 
> the format type (RGB, YUV, ...), number of bits per components, components 
> order and subsampling. It doesn't define the range of acceptable values for 
> pixel components (such as full-range YUV vs. BT.601[5]), which is carried 
> through a separate colorspace identifier[6].
> 
> 
> YUV support in the fbdev API
> ----------------------------
> 
> Experience with the V4L2 API, both for desktop and embedded devices, and the 
> KMS API, suggests that recent hardware tend to standardize on a relatively 
> small number of pixel formats that don't require bitfield-level descriptions. 
> A pixel format definition based on identifiers should thus fullfill the 
> hardware needs for the foreseeable future.
> 
> Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share data 
> and buffers between those subsystems, and the planned use of V4L2 FCCs in the 
> KMS overlay API, I believe using V4L2 FCCs to identify fbdev formats would be 
> a wise decision.
> 
> To select a frame buffer YUV format, the fb_var_screeninfo structure will need 
> to be extended with a format field. The fbdev API and ABI must not be broken, 
> which prevents us from changing the current structure layout and replacing the 
> existing format selection mechanism (through the red, green, blue and alpha 
> bitfields) completely.
> 
> Several solutions exist to introduce a format field in the fb_var_screeninfo 
> structure.
> 
> - Use the nonstd field as a format identifier. Existing drivers that use the 
> nonstd field for other purposes wouldn't be able to implement the new API 
> while keeping their existing API. This isn't a show stopper for drivers using 
> the FB_NONSTD_HAM and FB_NONSTD_REV_PIX_IN_B bits, as they don't need to 
> support any non-RGB format.
> 
> Existing drivers that support YUV formats through the nonstd field would have 
> to select between the current and the new API, without being able to implement 
> both.
> 
> - Use one of the reserved fields as a format identifier. Applications are 
> supposed to zero the fb_var_screeninfo structure before passing it to the 
> kernel, but experience showed that many applications forget to do so. Drivers 
> would then be unable to find out whether applications request a particular 
> format, or just forgot to initialize the field.
> 
> - Add a new FB_NONSTD_FORMAT bit to the nonstd field, and pass the format 
> through a separate field. If an application sets the FB_NONSTD_FORMAT bit in 
> the nonstd field, drivers will ignore the red, green, blue, transp and 
> grayscale fields, and use the format identifier to configure the frame buffer 
> format. The grayscale field would then be unneeded and could be reused as a 
> format identifier. Alternatively, one of the reserved fields could be used for 
> that purpose.
> 
> Existing drivers that support YUV formats through the nonstd field don't use 
> the field's most significant bits. They could implement both their current API 
> and the new API if the FB_NONSTD_FORMAT bit is stored in one of the nonstd 
> MSBs.
> 
> - Other solutions are possible, such as adding new ioctls. Those solutions are 
> more intrusive, and require larger changes to both userspace and kernelspace 
> code.
> 
> 
> The third solution has my preference. Comments and feedback will be 
> appreciated. I will then work on a proof of concept and submit patches.
> 
> 
> [1] http://en.wikipedia.org/wiki/Hold_And_Modify
> [2] http://lwn.net/Articles/440192/
> [3] http://www.fourcc.org/
> [4] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L271
> [5] http://en.wikipedia.org/wiki/Rec._601
> [6] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L175
> 



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

* Re: [RFC] Standardize YUV support in the fbdev API
  2011-05-18  0:21   ` Andy Walls
  (?)
@ 2011-05-18  1:09     ` Andy Walls
  -1 siblings, 0 replies; 63+ messages in thread
From: Andy Walls @ 2011-05-18  1:09 UTC (permalink / raw)
  To: Laurent Pinchart, Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

Oops.  Nevermind, you already have looked at what ivtvfb does.

-Andy

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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-18  1:09     ` Andy Walls
  0 siblings, 0 replies; 63+ messages in thread
From: Andy Walls @ 2011-05-18  1:09 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

Oops.  Nevermind, you already have looked at what ivtvfb does.

-Andy

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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-18  1:09     ` Andy Walls
  0 siblings, 0 replies; 63+ messages in thread
From: Andy Walls @ 2011-05-18  1:09 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

Oops.  Nevermind, you already have looked at what ivtvfb does.

-Andy

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

* Re: [RFC] Standardize YUV support in the fbdev API
  2011-05-17 22:44   ` Felipe Contreras
@ 2011-05-18  6:53     ` Hans Verkuil
  -1 siblings, 0 replies; 63+ messages in thread
From: Hans Verkuil @ 2011-05-18  6:53 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Laurent Pinchart, linux-fbdev, linux-media, dri-devel

On Wednesday, May 18, 2011 00:44:26 Felipe Contreras wrote:
> On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > I need to implement support for a YUV frame buffer in an fbdev driver. As the
> > fbdev API doesn't support this out of the box, I've spent a couple of days
> > reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> > support to the API. I'd like to share my findings and thoughts, and hopefully
> > receive some comments back.
> >
> > The terms 'format', 'pixel format', 'frame buffer format' and 'data format'
> > will be used interchangeably in this e-mail. They all refer to the way pixels
> > are stored in memory, including both the representation of a pixel as integer
> > values and the layout of those integer values in memory.
> 
> This is a great proposal. It was about time!
> 
> > The third solution has my preference. Comments and feedback will be
> > appreciated. I will then work on a proof of concept and submit patches.
> 
> I also would prefer the third solution. I don't think there's much
> difference from the user-space point of view, and a new ioctl would be
> cleaner. Also the v4l2 fourcc's should do.

I agree with this.

We might want to take the opportunity to fix this section of the V4L2 Spec:

http://www.xs4all.nl/~hverkuil/spec/media.html#pixfmt-rgb

There are two tables, 2.6 and 2.7. But 2.6 is almost certainly wrong and should
be removed. I suspect many if not all V4L2 drivers are badly broken for
big-endian systems and report the wrong pixel formats.

Officially the pixel formats reflect the contents of the memory. But everything
is swapped on a big endian system, so you are supposed to report a different
pix format. I can't remember seeing any driver do that. Some have built-in
swapping, though, and turn that on if needed.

I really need to run some tests, but I've been telling myself this for years
now :-(

Regards,

	Hans

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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-18  6:53     ` Hans Verkuil
  0 siblings, 0 replies; 63+ messages in thread
From: Hans Verkuil @ 2011-05-18  6:53 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Laurent Pinchart, linux-fbdev, linux-media, dri-devel

On Wednesday, May 18, 2011 00:44:26 Felipe Contreras wrote:
> On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > I need to implement support for a YUV frame buffer in an fbdev driver. As the
> > fbdev API doesn't support this out of the box, I've spent a couple of days
> > reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> > support to the API. I'd like to share my findings and thoughts, and hopefully
> > receive some comments back.
> >
> > The terms 'format', 'pixel format', 'frame buffer format' and 'data format'
> > will be used interchangeably in this e-mail. They all refer to the way pixels
> > are stored in memory, including both the representation of a pixel as integer
> > values and the layout of those integer values in memory.
> 
> This is a great proposal. It was about time!
> 
> > The third solution has my preference. Comments and feedback will be
> > appreciated. I will then work on a proof of concept and submit patches.
> 
> I also would prefer the third solution. I don't think there's much
> difference from the user-space point of view, and a new ioctl would be
> cleaner. Also the v4l2 fourcc's should do.

I agree with this.

We might want to take the opportunity to fix this section of the V4L2 Spec:

http://www.xs4all.nl/~hverkuil/spec/media.html#pixfmt-rgb

There are two tables, 2.6 and 2.7. But 2.6 is almost certainly wrong and should
be removed. I suspect many if not all V4L2 drivers are badly broken for
big-endian systems and report the wrong pixel formats.

Officially the pixel formats reflect the contents of the memory. But everything
is swapped on a big endian system, so you are supposed to report a different
pix format. I can't remember seeing any driver do that. Some have built-in
swapping, though, and turn that on if needed.

I really need to run some tests, but I've been telling myself this for years
now :-(

Regards,

	Hans

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

* Re: [RFC] Standardize YUV support in the fbdev API
  2011-05-17 22:07 ` Laurent Pinchart
@ 2011-05-20 22:33   ` Florian Tobias Schandinat
  -1 siblings, 0 replies; 63+ messages in thread
From: Florian Tobias Schandinat @ 2011-05-20 22:33 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

Hi Laurent,

On 05/17/2011 10:07 PM, Laurent Pinchart wrote:
> Hi everybody,
>
> I need to implement support for a YUV frame buffer in an fbdev driver. As the
> fbdev API doesn't support this out of the box, I've spent a couple of days
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> support to the API. I'd like to share my findings and thoughts, and hopefully
> receive some comments back.

Thanks. I think you did already a good job, hope we can get it done this time.

> Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share data
> and buffers between those subsystems, and the planned use of V4L2 FCCs in the
> KMS overlay API, I believe using V4L2 FCCs to identify fbdev formats would be
> a wise decision.

I agree.

> To select a frame buffer YUV format, the fb_var_screeninfo structure will need
> to be extended with a format field. The fbdev API and ABI must not be broken,
> which prevents us from changing the current structure layout and replacing the
> existing format selection mechanism (through the red, green, blue and alpha
> bitfields) completely.

I agree.

> - Other solutions are possible, such as adding new ioctls. Those solutions are
> more intrusive, and require larger changes to both userspace and kernelspace
> code.

I'm against (ab)using the nonstd field (probably the only sane thing we can do 
with it is declare it non-standard which interpretation is completely dependent 
on the specific driver) or requiring previously unused fields to have a special 
value so I'd like to suggest a different method:

I remembered an earlier discussion:
[ http://marc.info/?l=linux-fbdev&m=129896686208130&w=2 ]

On 03/01/2011 08:07 AM, Geert Uytterhoeven wrote:
 > On Tue, Mar 1, 2011 at 04:13, Damian<dhobsong@igel.co.jp>  wrote:
 >> On 2011/02/24 15:05, Geert Uytterhoeven wrote:
 >>> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
 >>> FB_VISUAL_*
 >>> type instead, which indicates the fb_var_screeninfo.{red,green,blue}
 >>> fields are
 >>> YCbCr instead of RGB.
 >>> Depending on the frame buffer organization, you also need new
 >>> FB_TYPE_*/FB_AUX_*
 >>> types.
 >>
 >> I just wanted to clarify here.  Is your comment about these new flags in
 >> specific reference to this patch or to Magnus' "going forward" comment?  It
 >
 > About new flags.
 >
 >> seems like the beginnings of a method to standardize YCbCr support in fbdev
 >> across all platforms.
 >> Also, do I understand correctly that FB_VISUAL_ would specify the colorspace
 >
 > FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped to
 > colors on the screen, so to me it looks like the sensible way to set up YCbCr.
 >
 >> (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
 >> semiplanar, interleaved, etc)?  I'm not really sure what you are referring
 >> to with the FB_AUX_* however.
 >
 > Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame buffer
 > memory.
 >
 > FB_AUX_* is only used if a specific value of FB_TYPE_* needs an additional
 > parameter (e.g. the interleave value for interleaved bitplanes).

Adding new standard values for these fb_fix_screeninfo fields would solve the 
issue for framebuffers which only support a single format. If you have the need 
to switch I guess it would be a good idea to add a new flag to the vmode 
bitfield in fb_var_screeninfo which looks like a general purpose modifier for 
the videomode. You could than reuse any RGB-specific field you like to pass more 
information.
Maybe we should also use this chance to declare one of the fix_screeninfo 
reserved fields to be used for capability flags or an API version as we can 
assume that those are 0 (at least in sane drivers).


Good luck,

Florian Tobias Schandinat

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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-20 22:33   ` Florian Tobias Schandinat
  0 siblings, 0 replies; 63+ messages in thread
From: Florian Tobias Schandinat @ 2011-05-20 22:33 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

Hi Laurent,

On 05/17/2011 10:07 PM, Laurent Pinchart wrote:
> Hi everybody,
>
> I need to implement support for a YUV frame buffer in an fbdev driver. As the
> fbdev API doesn't support this out of the box, I've spent a couple of days
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> support to the API. I'd like to share my findings and thoughts, and hopefully
> receive some comments back.

Thanks. I think you did already a good job, hope we can get it done this time.

> Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share data
> and buffers between those subsystems, and the planned use of V4L2 FCCs in the
> KMS overlay API, I believe using V4L2 FCCs to identify fbdev formats would be
> a wise decision.

I agree.

> To select a frame buffer YUV format, the fb_var_screeninfo structure will need
> to be extended with a format field. The fbdev API and ABI must not be broken,
> which prevents us from changing the current structure layout and replacing the
> existing format selection mechanism (through the red, green, blue and alpha
> bitfields) completely.

I agree.

> - Other solutions are possible, such as adding new ioctls. Those solutions are
> more intrusive, and require larger changes to both userspace and kernelspace
> code.

I'm against (ab)using the nonstd field (probably the only sane thing we can do 
with it is declare it non-standard which interpretation is completely dependent 
on the specific driver) or requiring previously unused fields to have a special 
value so I'd like to suggest a different method:

I remembered an earlier discussion:
[ http://marc.info/?l=linux-fbdev&m\x129896686208130&w=2 ]

On 03/01/2011 08:07 AM, Geert Uytterhoeven wrote:
 > On Tue, Mar 1, 2011 at 04:13, Damian<dhobsong@igel.co.jp>  wrote:
 >> On 2011/02/24 15:05, Geert Uytterhoeven wrote:
 >>> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
 >>> FB_VISUAL_*
 >>> type instead, which indicates the fb_var_screeninfo.{red,green,blue}
 >>> fields are
 >>> YCbCr instead of RGB.
 >>> Depending on the frame buffer organization, you also need new
 >>> FB_TYPE_*/FB_AUX_*
 >>> types.
 >>
 >> I just wanted to clarify here.  Is your comment about these new flags in
 >> specific reference to this patch or to Magnus' "going forward" comment?  It
 >
 > About new flags.
 >
 >> seems like the beginnings of a method to standardize YCbCr support in fbdev
 >> across all platforms.
 >> Also, do I understand correctly that FB_VISUAL_ would specify the colorspace
 >
 > FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped to
 > colors on the screen, so to me it looks like the sensible way to set up YCbCr.
 >
 >> (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
 >> semiplanar, interleaved, etc)?  I'm not really sure what you are referring
 >> to with the FB_AUX_* however.
 >
 > Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame buffer
 > memory.
 >
 > FB_AUX_* is only used if a specific value of FB_TYPE_* needs an additional
 > parameter (e.g. the interleave value for interleaved bitplanes).

Adding new standard values for these fb_fix_screeninfo fields would solve the 
issue for framebuffers which only support a single format. If you have the need 
to switch I guess it would be a good idea to add a new flag to the vmode 
bitfield in fb_var_screeninfo which looks like a general purpose modifier for 
the videomode. You could than reuse any RGB-specific field you like to pass more 
information.
Maybe we should also use this chance to declare one of the fix_screeninfo 
reserved fields to be used for capability flags or an API version as we can 
assume that those are 0 (at least in sane drivers).


Good luck,

Florian Tobias Schandinat

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

* RE: [RFC] Standardize YUV support in the fbdev API
  2011-05-18  6:53     ` Hans Verkuil
@ 2011-05-23 11:55       ` Marek Szyprowski
  -1 siblings, 0 replies; 63+ messages in thread
From: Marek Szyprowski @ 2011-05-23 11:55 UTC (permalink / raw)
  To: 'Hans Verkuil', 'Felipe Contreras'
  Cc: 'Laurent Pinchart', linux-fbdev, linux-media, dri-devel

Hello,

On Wednesday, May 18, 2011 8:54 AM Hans Verkuil wrote:

> On Wednesday, May 18, 2011 00:44:26 Felipe Contreras wrote:
> > On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > > I need to implement support for a YUV frame buffer in an fbdev driver.
> As the
> > > fbdev API doesn't support this out of the box, I've spent a couple of
> days
> > > reading fbdev (and KMS) code and thinking about how we could cleanly
> add YUV
> > > support to the API. I'd like to share my findings and thoughts, and
> hopefully
> > > receive some comments back.
> > >
> > > The terms 'format', 'pixel format', 'frame buffer format' and 'data
> format'
> > > will be used interchangeably in this e-mail. They all refer to the way
> pixels
> > > are stored in memory, including both the representation of a pixel as
> integer
> > > values and the layout of those integer values in memory.
> >
> > This is a great proposal. It was about time!
> >
> > > The third solution has my preference. Comments and feedback will be
> > > appreciated. I will then work on a proof of concept and submit patches.
> >
> > I also would prefer the third solution. I don't think there's much
> > difference from the user-space point of view, and a new ioctl would be
> > cleaner. Also the v4l2 fourcc's should do.
> 
> I agree with this.
> 
> We might want to take the opportunity to fix this section of the V4L2 Spec:
> 
> http://www.xs4all.nl/~hverkuil/spec/media.html#pixfmt-rgb
> 
> There are two tables, 2.6 and 2.7. But 2.6 is almost certainly wrong and
> should be removed.

That's definitely true. I was confused at the beginning when I saw 2
different tables describing the same pixel formats.

 I suspect many if not all V4L2 drivers are badly broken for
> big-endian systems and report the wrong pixel formats.
> 
> Officially the pixel formats reflect the contents of the memory. But
> everything is swapped on a big endian system, so you are supposed to 
> report a different pix format.

I always thought that pix_format describes the layout of video data in
memory on byte level, which is exactly the same on both little- and big-
endian systems. You can notice swapped data only if you access memory
by units larger than byte, like 16bit or 32bit integers. BTW - I would
really like to avoid little- and big- endian flame, but your statement
about 'everything is swapped on a big endian system' is completely
wrong. It is rather the characteristic of little-endian system not big
endian one if you display the content of the same memory first using
byte access and then using word/long access.

> I can't remember seeing any driver do that. Some have built-in swapping,
> though, and turn that on if needed.

The drivers shouldn't do ANY byte swapping at all. Only tools that
extract pixel data with some 'accelerated' methods (like 32bit integer
casting and bit-level shifting) should be aware of endianess.

> I really need to run some tests, but I've been telling myself this for
> years now :-(

I've checked the BTTV board in my PowerMac/G4 and the display was
correct with xawtv. It is just a matter of selecting correct pix format
basing on the information returned by xsever. 

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



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

* RE: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-23 11:55       ` Marek Szyprowski
  0 siblings, 0 replies; 63+ messages in thread
From: Marek Szyprowski @ 2011-05-23 11:55 UTC (permalink / raw)
  To: 'Hans Verkuil', 'Felipe Contreras'
  Cc: 'Laurent Pinchart', linux-fbdev, linux-media, dri-devel

Hello,

On Wednesday, May 18, 2011 8:54 AM Hans Verkuil wrote:

> On Wednesday, May 18, 2011 00:44:26 Felipe Contreras wrote:
> > On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > > I need to implement support for a YUV frame buffer in an fbdev driver.
> As the
> > > fbdev API doesn't support this out of the box, I've spent a couple of
> days
> > > reading fbdev (and KMS) code and thinking about how we could cleanly
> add YUV
> > > support to the API. I'd like to share my findings and thoughts, and
> hopefully
> > > receive some comments back.
> > >
> > > The terms 'format', 'pixel format', 'frame buffer format' and 'data
> format'
> > > will be used interchangeably in this e-mail. They all refer to the way
> pixels
> > > are stored in memory, including both the representation of a pixel as
> integer
> > > values and the layout of those integer values in memory.
> >
> > This is a great proposal. It was about time!
> >
> > > The third solution has my preference. Comments and feedback will be
> > > appreciated. I will then work on a proof of concept and submit patches.
> >
> > I also would prefer the third solution. I don't think there's much
> > difference from the user-space point of view, and a new ioctl would be
> > cleaner. Also the v4l2 fourcc's should do.
> 
> I agree with this.
> 
> We might want to take the opportunity to fix this section of the V4L2 Spec:
> 
> http://www.xs4all.nl/~hverkuil/spec/media.html#pixfmt-rgb
> 
> There are two tables, 2.6 and 2.7. But 2.6 is almost certainly wrong and
> should be removed.

That's definitely true. I was confused at the beginning when I saw 2
different tables describing the same pixel formats.

 I suspect many if not all V4L2 drivers are badly broken for
> big-endian systems and report the wrong pixel formats.
> 
> Officially the pixel formats reflect the contents of the memory. But
> everything is swapped on a big endian system, so you are supposed to 
> report a different pix format.

I always thought that pix_format describes the layout of video data in
memory on byte level, which is exactly the same on both little- and big-
endian systems. You can notice swapped data only if you access memory
by units larger than byte, like 16bit or 32bit integers. BTW - I would
really like to avoid little- and big- endian flame, but your statement
about 'everything is swapped on a big endian system' is completely
wrong. It is rather the characteristic of little-endian system not big
endian one if you display the content of the same memory first using
byte access and then using word/long access.

> I can't remember seeing any driver do that. Some have built-in swapping,
> though, and turn that on if needed.

The drivers shouldn't do ANY byte swapping at all. Only tools that
extract pixel data with some 'accelerated' methods (like 32bit integer
casting and bit-level shifting) should be aware of endianess.

> I really need to run some tests, but I've been telling myself this for
> years now :-(

I've checked the BTTV board in my PowerMac/G4 and the display was
correct with xawtv. It is just a matter of selecting correct pix format
basing on the information returned by xsever. 

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



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

* Re: [RFC] Standardize YUV support in the fbdev API
  2011-05-23 11:55       ` Marek Szyprowski
@ 2011-05-23 12:09         ` Hans Verkuil
  -1 siblings, 0 replies; 63+ messages in thread
From: Hans Verkuil @ 2011-05-23 12:09 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: 'Felipe Contreras', 'Laurent Pinchart',
	linux-fbdev, linux-media, dri-devel

On Monday, May 23, 2011 13:55:21 Marek Szyprowski wrote:
> Hello,
> 
> On Wednesday, May 18, 2011 8:54 AM Hans Verkuil wrote:
> 
> > On Wednesday, May 18, 2011 00:44:26 Felipe Contreras wrote:
> > > On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
> > > <laurent.pinchart@ideasonboard.com> wrote:
> > > > I need to implement support for a YUV frame buffer in an fbdev driver.
> > As the
> > > > fbdev API doesn't support this out of the box, I've spent a couple of
> > days
> > > > reading fbdev (and KMS) code and thinking about how we could cleanly
> > add YUV
> > > > support to the API. I'd like to share my findings and thoughts, and
> > hopefully
> > > > receive some comments back.
> > > >
> > > > The terms 'format', 'pixel format', 'frame buffer format' and 'data
> > format'
> > > > will be used interchangeably in this e-mail. They all refer to the way
> > pixels
> > > > are stored in memory, including both the representation of a pixel as
> > integer
> > > > values and the layout of those integer values in memory.
> > >
> > > This is a great proposal. It was about time!
> > >
> > > > The third solution has my preference. Comments and feedback will be
> > > > appreciated. I will then work on a proof of concept and submit patches.
> > >
> > > I also would prefer the third solution. I don't think there's much
> > > difference from the user-space point of view, and a new ioctl would be
> > > cleaner. Also the v4l2 fourcc's should do.
> > 
> > I agree with this.
> > 
> > We might want to take the opportunity to fix this section of the V4L2 Spec:
> > 
> > http://www.xs4all.nl/~hverkuil/spec/media.html#pixfmt-rgb
> > 
> > There are two tables, 2.6 and 2.7. But 2.6 is almost certainly wrong and
> > should be removed.
> 
> That's definitely true. I was confused at the beginning when I saw 2
> different tables describing the same pixel formats.
> 
>  I suspect many if not all V4L2 drivers are badly broken for
> > big-endian systems and report the wrong pixel formats.
> > 
> > Officially the pixel formats reflect the contents of the memory. But
> > everything is swapped on a big endian system, so you are supposed to 
> > report a different pix format.
> 
> I always thought that pix_format describes the layout of video data in
> memory on byte level, which is exactly the same on both little- and big-
> endian systems.

Correct.

> You can notice swapped data only if you access memory
> by units larger than byte, like 16bit or 32bit integers. BTW - I would
> really like to avoid little- and big- endian flame, but your statement
> about 'everything is swapped on a big endian system' is completely
> wrong. It is rather the characteristic of little-endian system not big
> endian one if you display the content of the same memory first using
> byte access and then using word/long access.

You are correct, I wasn't thinking it through.
 
> > I can't remember seeing any driver do that. Some have built-in swapping,
> > though, and turn that on if needed.
> 
> The drivers shouldn't do ANY byte swapping at all. Only tools that
> extract pixel data with some 'accelerated' methods (like 32bit integer
> casting and bit-level shifting) should be aware of endianess.
> 
> > I really need to run some tests, but I've been telling myself this for
> > years now :-(
> 
> I've checked the BTTV board in my PowerMac/G4 and the display was
> correct with xawtv. It is just a matter of selecting correct pix format
> basing on the information returned by xsever. 
> 
> Best regards
> 

Just forget my post (except for the part of cleaning up the tables :-) ).

Regards,

	Hans

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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-23 12:09         ` Hans Verkuil
  0 siblings, 0 replies; 63+ messages in thread
From: Hans Verkuil @ 2011-05-23 12:09 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: 'Felipe Contreras', 'Laurent Pinchart',
	linux-fbdev, linux-media, dri-devel

On Monday, May 23, 2011 13:55:21 Marek Szyprowski wrote:
> Hello,
> 
> On Wednesday, May 18, 2011 8:54 AM Hans Verkuil wrote:
> 
> > On Wednesday, May 18, 2011 00:44:26 Felipe Contreras wrote:
> > > On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
> > > <laurent.pinchart@ideasonboard.com> wrote:
> > > > I need to implement support for a YUV frame buffer in an fbdev driver.
> > As the
> > > > fbdev API doesn't support this out of the box, I've spent a couple of
> > days
> > > > reading fbdev (and KMS) code and thinking about how we could cleanly
> > add YUV
> > > > support to the API. I'd like to share my findings and thoughts, and
> > hopefully
> > > > receive some comments back.
> > > >
> > > > The terms 'format', 'pixel format', 'frame buffer format' and 'data
> > format'
> > > > will be used interchangeably in this e-mail. They all refer to the way
> > pixels
> > > > are stored in memory, including both the representation of a pixel as
> > integer
> > > > values and the layout of those integer values in memory.
> > >
> > > This is a great proposal. It was about time!
> > >
> > > > The third solution has my preference. Comments and feedback will be
> > > > appreciated. I will then work on a proof of concept and submit patches.
> > >
> > > I also would prefer the third solution. I don't think there's much
> > > difference from the user-space point of view, and a new ioctl would be
> > > cleaner. Also the v4l2 fourcc's should do.
> > 
> > I agree with this.
> > 
> > We might want to take the opportunity to fix this section of the V4L2 Spec:
> > 
> > http://www.xs4all.nl/~hverkuil/spec/media.html#pixfmt-rgb
> > 
> > There are two tables, 2.6 and 2.7. But 2.6 is almost certainly wrong and
> > should be removed.
> 
> That's definitely true. I was confused at the beginning when I saw 2
> different tables describing the same pixel formats.
> 
>  I suspect many if not all V4L2 drivers are badly broken for
> > big-endian systems and report the wrong pixel formats.
> > 
> > Officially the pixel formats reflect the contents of the memory. But
> > everything is swapped on a big endian system, so you are supposed to 
> > report a different pix format.
> 
> I always thought that pix_format describes the layout of video data in
> memory on byte level, which is exactly the same on both little- and big-
> endian systems.

Correct.

> You can notice swapped data only if you access memory
> by units larger than byte, like 16bit or 32bit integers. BTW - I would
> really like to avoid little- and big- endian flame, but your statement
> about 'everything is swapped on a big endian system' is completely
> wrong. It is rather the characteristic of little-endian system not big
> endian one if you display the content of the same memory first using
> byte access and then using word/long access.

You are correct, I wasn't thinking it through.
 
> > I can't remember seeing any driver do that. Some have built-in swapping,
> > though, and turn that on if needed.
> 
> The drivers shouldn't do ANY byte swapping at all. Only tools that
> extract pixel data with some 'accelerated' methods (like 32bit integer
> casting and bit-level shifting) should be aware of endianess.
> 
> > I really need to run some tests, but I've been telling myself this for
> > years now :-(
> 
> I've checked the BTTV board in my PowerMac/G4 and the display was
> correct with xawtv. It is just a matter of selecting correct pix format
> basing on the information returned by xsever. 
> 
> Best regards
> 

Just forget my post (except for the part of cleaning up the tables :-) ).

Regards,

	Hans

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

* Re: [RFC] Standardize YUV support in the fbdev API
  2011-05-20 22:33   ` Florian Tobias Schandinat
  (?)
@ 2011-05-23 21:00     ` Laurent Pinchart
  -1 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-05-23 21:00 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: linux-fbdev, linux-media, dri-devel

Hi Florian,

On Saturday 21 May 2011 00:33:02 Florian Tobias Schandinat wrote:
> On 05/17/2011 10:07 PM, Laurent Pinchart wrote:
> > Hi everybody,
> > 
> > I need to implement support for a YUV frame buffer in an fbdev driver. As
> > the fbdev API doesn't support this out of the box, I've spent a couple
> > of days reading fbdev (and KMS) code and thinking about how we could
> > cleanly add YUV support to the API. I'd like to share my findings and
> > thoughts, and hopefully receive some comments back.
> 
> Thanks. I think you did already a good job, hope we can get it done this
> time.

Thanks. I'll keep pushing then :-)

> > Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share
> > data and buffers between those subsystems, and the planned use of V4L2
> > FCCs in the KMS overlay API, I believe using V4L2 FCCs to identify fbdev
> > formats would be a wise decision.
> 
> I agree.

There seems to be a general agreement on this, so I'll consider this settled.

> > To select a frame buffer YUV format, the fb_var_screeninfo structure will
> > need to be extended with a format field. The fbdev API and ABI must not
> > be broken, which prevents us from changing the current structure layout
> > and replacing the existing format selection mechanism (through the red,
> > green, blue and alpha bitfields) completely.
> 
> I agree.
> 
> > - Other solutions are possible, such as adding new ioctls. Those
> > solutions are more intrusive, and require larger changes to both
> > userspace and kernelspace code.
> 
> I'm against (ab)using the nonstd field (probably the only sane thing we can
> do with it is declare it non-standard which interpretation is completely
> dependent on the specific driver) or requiring previously unused fields to
> have a special value so I'd like to suggest a different method:
> 
> I remembered an earlier discussion:
> [ http://marc.info/?l=linux-fbdev&m=129896686208130&w=2 ]
> 
> On 03/01/2011 08:07 AM, Geert Uytterhoeven wrote:
>  > On Tue, Mar 1, 2011 at 04:13, Damian<dhobsong@igel.co.jp>  wrote:
>  >> On 2011/02/24 15:05, Geert Uytterhoeven wrote:
>  >>> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
>  >>> FB_VISUAL_*
>  >>> type instead, which indicates the fb_var_screeninfo.{red,green,blue}
>  >>> fields are
>  >>> YCbCr instead of RGB.
>  >>> Depending on the frame buffer organization, you also need new
>  >>> FB_TYPE_*/FB_AUX_*
>  >>> types.
>  >> 
>  >> I just wanted to clarify here.  Is your comment about these new flags
>  >> in specific reference to this patch or to Magnus' "going forward"
>  >> comment?  It
>  > 
>  > About new flags.
>  > 
>  >> seems like the beginnings of a method to standardize YCbCr support in
>  >> fbdev across all platforms.
>  >> Also, do I understand correctly that FB_VISUAL_ would specify the
>  >> colorspace
>  > 
>  > FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped
>  > to colors on the screen, so to me it looks like the sensible way to set
>  > up YCbCr.
>  > 
>  >> (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
>  >> semiplanar, interleaved, etc)?  I'm not really sure what you are
>  >> referring to with the FB_AUX_* however.
>  > 
>  > Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame
>  > buffer memory.
>  > 
>  > FB_AUX_* is only used if a specific value of FB_TYPE_* needs an
>  > additional parameter (e.g. the interleave value for interleaved
>  > bitplanes).
> 
> Adding new standard values for these fb_fix_screeninfo fields would solve
> the issue for framebuffers which only support a single format.

I've never liked changing fixed screen information :-) It would be consistent 
with the API though.

> If you have the need to switch

Yes I need that. This requires an API to set the mode through 
fb_var_screeninfo, which is why I skipped modifying fb_fix_screeinfo.

A new FB_TYPE_* could be used to report that we use a 4CC-based mode, with the 
exact mode reported in one of the fb_fix_screeninfo reserved fields (or the 
type_aux field). This would duplicate the information passed through 
fb_var_screeninfo though. Do you think it's worth it ?

> I guess it would be a good idea to add a new flag to the vmode bitfield in
> fb_var_screeninfo which looks like a general purpose modifier for the
> videomode. You could than reuse any RGB-specific field you like to pass more
> information.

That looks good to me. The grayscale field could be reused to pass the 4CC.

> Maybe we should also use this chance to declare one of the fix_screeninfo
> reserved fields to be used for capability flags or an API version as we can
> assume that those are 0 (at least in sane drivers).

That's always good, although it's not a hard requirement for the purpose of 
YUV support.

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-23 21:00     ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-05-23 21:00 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: linux-fbdev, dri-devel, linux-media

Hi Florian,

On Saturday 21 May 2011 00:33:02 Florian Tobias Schandinat wrote:
> On 05/17/2011 10:07 PM, Laurent Pinchart wrote:
> > Hi everybody,
> > 
> > I need to implement support for a YUV frame buffer in an fbdev driver. As
> > the fbdev API doesn't support this out of the box, I've spent a couple
> > of days reading fbdev (and KMS) code and thinking about how we could
> > cleanly add YUV support to the API. I'd like to share my findings and
> > thoughts, and hopefully receive some comments back.
> 
> Thanks. I think you did already a good job, hope we can get it done this
> time.

Thanks. I'll keep pushing then :-)

> > Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share
> > data and buffers between those subsystems, and the planned use of V4L2
> > FCCs in the KMS overlay API, I believe using V4L2 FCCs to identify fbdev
> > formats would be a wise decision.
> 
> I agree.

There seems to be a general agreement on this, so I'll consider this settled.

> > To select a frame buffer YUV format, the fb_var_screeninfo structure will
> > need to be extended with a format field. The fbdev API and ABI must not
> > be broken, which prevents us from changing the current structure layout
> > and replacing the existing format selection mechanism (through the red,
> > green, blue and alpha bitfields) completely.
> 
> I agree.
> 
> > - Other solutions are possible, such as adding new ioctls. Those
> > solutions are more intrusive, and require larger changes to both
> > userspace and kernelspace code.
> 
> I'm against (ab)using the nonstd field (probably the only sane thing we can
> do with it is declare it non-standard which interpretation is completely
> dependent on the specific driver) or requiring previously unused fields to
> have a special value so I'd like to suggest a different method:
> 
> I remembered an earlier discussion:
> [ http://marc.info/?l=linux-fbdev&m\x129896686208130&w=2 ]
> 
> On 03/01/2011 08:07 AM, Geert Uytterhoeven wrote:
>  > On Tue, Mar 1, 2011 at 04:13, Damian<dhobsong@igel.co.jp>  wrote:
>  >> On 2011/02/24 15:05, Geert Uytterhoeven wrote:
>  >>> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
>  >>> FB_VISUAL_*
>  >>> type instead, which indicates the fb_var_screeninfo.{red,green,blue}
>  >>> fields are
>  >>> YCbCr instead of RGB.
>  >>> Depending on the frame buffer organization, you also need new
>  >>> FB_TYPE_*/FB_AUX_*
>  >>> types.
>  >> 
>  >> I just wanted to clarify here.  Is your comment about these new flags
>  >> in specific reference to this patch or to Magnus' "going forward"
>  >> comment?  It
>  > 
>  > About new flags.
>  > 
>  >> seems like the beginnings of a method to standardize YCbCr support in
>  >> fbdev across all platforms.
>  >> Also, do I understand correctly that FB_VISUAL_ would specify the
>  >> colorspace
>  > 
>  > FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped
>  > to colors on the screen, so to me it looks like the sensible way to set
>  > up YCbCr.
>  > 
>  >> (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
>  >> semiplanar, interleaved, etc)?  I'm not really sure what you are
>  >> referring to with the FB_AUX_* however.
>  > 
>  > Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame
>  > buffer memory.
>  > 
>  > FB_AUX_* is only used if a specific value of FB_TYPE_* needs an
>  > additional parameter (e.g. the interleave value for interleaved
>  > bitplanes).
> 
> Adding new standard values for these fb_fix_screeninfo fields would solve
> the issue for framebuffers which only support a single format.

I've never liked changing fixed screen information :-) It would be consistent 
with the API though.

> If you have the need to switch

Yes I need that. This requires an API to set the mode through 
fb_var_screeninfo, which is why I skipped modifying fb_fix_screeinfo.

A new FB_TYPE_* could be used to report that we use a 4CC-based mode, with the 
exact mode reported in one of the fb_fix_screeninfo reserved fields (or the 
type_aux field). This would duplicate the information passed through 
fb_var_screeninfo though. Do you think it's worth it ?

> I guess it would be a good idea to add a new flag to the vmode bitfield in
> fb_var_screeninfo which looks like a general purpose modifier for the
> videomode. You could than reuse any RGB-specific field you like to pass more
> information.

That looks good to me. The grayscale field could be reused to pass the 4CC.

> Maybe we should also use this chance to declare one of the fix_screeninfo
> reserved fields to be used for capability flags or an API version as we can
> assume that those are 0 (at least in sane drivers).

That's always good, although it's not a hard requirement for the purpose of 
YUV support.

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-23 21:00     ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-05-23 21:00 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: linux-fbdev, dri-devel, linux-media

Hi Florian,

On Saturday 21 May 2011 00:33:02 Florian Tobias Schandinat wrote:
> On 05/17/2011 10:07 PM, Laurent Pinchart wrote:
> > Hi everybody,
> > 
> > I need to implement support for a YUV frame buffer in an fbdev driver. As
> > the fbdev API doesn't support this out of the box, I've spent a couple
> > of days reading fbdev (and KMS) code and thinking about how we could
> > cleanly add YUV support to the API. I'd like to share my findings and
> > thoughts, and hopefully receive some comments back.
> 
> Thanks. I think you did already a good job, hope we can get it done this
> time.

Thanks. I'll keep pushing then :-)

> > Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share
> > data and buffers between those subsystems, and the planned use of V4L2
> > FCCs in the KMS overlay API, I believe using V4L2 FCCs to identify fbdev
> > formats would be a wise decision.
> 
> I agree.

There seems to be a general agreement on this, so I'll consider this settled.

> > To select a frame buffer YUV format, the fb_var_screeninfo structure will
> > need to be extended with a format field. The fbdev API and ABI must not
> > be broken, which prevents us from changing the current structure layout
> > and replacing the existing format selection mechanism (through the red,
> > green, blue and alpha bitfields) completely.
> 
> I agree.
> 
> > - Other solutions are possible, such as adding new ioctls. Those
> > solutions are more intrusive, and require larger changes to both
> > userspace and kernelspace code.
> 
> I'm against (ab)using the nonstd field (probably the only sane thing we can
> do with it is declare it non-standard which interpretation is completely
> dependent on the specific driver) or requiring previously unused fields to
> have a special value so I'd like to suggest a different method:
> 
> I remembered an earlier discussion:
> [ http://marc.info/?l=linux-fbdev&m=129896686208130&w=2 ]
> 
> On 03/01/2011 08:07 AM, Geert Uytterhoeven wrote:
>  > On Tue, Mar 1, 2011 at 04:13, Damian<dhobsong@igel.co.jp>  wrote:
>  >> On 2011/02/24 15:05, Geert Uytterhoeven wrote:
>  >>> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
>  >>> FB_VISUAL_*
>  >>> type instead, which indicates the fb_var_screeninfo.{red,green,blue}
>  >>> fields are
>  >>> YCbCr instead of RGB.
>  >>> Depending on the frame buffer organization, you also need new
>  >>> FB_TYPE_*/FB_AUX_*
>  >>> types.
>  >> 
>  >> I just wanted to clarify here.  Is your comment about these new flags
>  >> in specific reference to this patch or to Magnus' "going forward"
>  >> comment?  It
>  > 
>  > About new flags.
>  > 
>  >> seems like the beginnings of a method to standardize YCbCr support in
>  >> fbdev across all platforms.
>  >> Also, do I understand correctly that FB_VISUAL_ would specify the
>  >> colorspace
>  > 
>  > FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped
>  > to colors on the screen, so to me it looks like the sensible way to set
>  > up YCbCr.
>  > 
>  >> (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
>  >> semiplanar, interleaved, etc)?  I'm not really sure what you are
>  >> referring to with the FB_AUX_* however.
>  > 
>  > Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame
>  > buffer memory.
>  > 
>  > FB_AUX_* is only used if a specific value of FB_TYPE_* needs an
>  > additional parameter (e.g. the interleave value for interleaved
>  > bitplanes).
> 
> Adding new standard values for these fb_fix_screeninfo fields would solve
> the issue for framebuffers which only support a single format.

I've never liked changing fixed screen information :-) It would be consistent 
with the API though.

> If you have the need to switch

Yes I need that. This requires an API to set the mode through 
fb_var_screeninfo, which is why I skipped modifying fb_fix_screeinfo.

A new FB_TYPE_* could be used to report that we use a 4CC-based mode, with the 
exact mode reported in one of the fb_fix_screeninfo reserved fields (or the 
type_aux field). This would duplicate the information passed through 
fb_var_screeninfo though. Do you think it's worth it ?

> I guess it would be a good idea to add a new flag to the vmode bitfield in
> fb_var_screeninfo which looks like a general purpose modifier for the
> videomode. You could than reuse any RGB-specific field you like to pass more
> information.

That looks good to me. The grayscale field could be reused to pass the 4CC.

> Maybe we should also use this chance to declare one of the fix_screeninfo
> reserved fields to be used for capability flags or an API version as we can
> assume that those are 0 (at least in sane drivers).

That's always good, although it's not a hard requirement for the purpose of 
YUV support.

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC] Standardize YUV support in the fbdev API
  2011-05-23 21:00     ` Laurent Pinchart
@ 2011-05-23 22:56       ` Florian Tobias Schandinat
  -1 siblings, 0 replies; 63+ messages in thread
From: Florian Tobias Schandinat @ 2011-05-23 22:56 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

Hi Laurent,

On 05/23/2011 09:00 PM, Laurent Pinchart wrote:
> On Saturday 21 May 2011 00:33:02 Florian Tobias Schandinat wrote:
>> On 05/17/2011 10:07 PM, Laurent Pinchart wrote:
>>> - Other solutions are possible, such as adding new ioctls. Those
>>> solutions are more intrusive, and require larger changes to both
>>> userspace and kernelspace code.
>>
>> I'm against (ab)using the nonstd field (probably the only sane thing we can
>> do with it is declare it non-standard which interpretation is completely
>> dependent on the specific driver) or requiring previously unused fields to
>> have a special value so I'd like to suggest a different method:
>>
>> I remembered an earlier discussion:
>> [ http://marc.info/?l=linux-fbdev&m=129896686208130&w=2 ]
>>
>> On 03/01/2011 08:07 AM, Geert Uytterhoeven wrote:
>>   >  On Tue, Mar 1, 2011 at 04:13, Damian<dhobsong@igel.co.jp>   wrote:
>>   >>  On 2011/02/24 15:05, Geert Uytterhoeven wrote:
>>   >>>  For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
>>   >>>  FB_VISUAL_*
>>   >>>  type instead, which indicates the fb_var_screeninfo.{red,green,blue}
>>   >>>  fields are
>>   >>>  YCbCr instead of RGB.
>>   >>>  Depending on the frame buffer organization, you also need new
>>   >>>  FB_TYPE_*/FB_AUX_*
>>   >>>  types.
>>   >>
>>   >>  I just wanted to clarify here.  Is your comment about these new flags
>>   >>  in specific reference to this patch or to Magnus' "going forward"
>>   >>  comment?  It
>>   >
>>   >  About new flags.
>>   >
>>   >>  seems like the beginnings of a method to standardize YCbCr support in
>>   >>  fbdev across all platforms.
>>   >>  Also, do I understand correctly that FB_VISUAL_ would specify the
>>   >>  colorspace
>>   >
>>   >  FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped
>>   >  to colors on the screen, so to me it looks like the sensible way to set
>>   >  up YCbCr.
>>   >
>>   >>  (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
>>   >>  semiplanar, interleaved, etc)?  I'm not really sure what you are
>>   >>  referring to with the FB_AUX_* however.
>>   >
>>   >  Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame
>>   >  buffer memory.
>>   >
>>   >  FB_AUX_* is only used if a specific value of FB_TYPE_* needs an
>>   >  additional parameter (e.g. the interleave value for interleaved
>>   >  bitplanes).
>>
>> Adding new standard values for these fb_fix_screeninfo fields would solve
>> the issue for framebuffers which only support a single format.
>
> I've never liked changing fixed screen information :-) It would be consistent
> with the API though.

Fixed does only mean that it can't be directly manipulated by applications. The 
driver has to modify it anyway on about every mode change (line_length). Yes 
perhaps some of these fields would be in var today and certainly others 
shouldn't exist at all but I do not blame anyone for not being capable to look 
into the future.

>> If you have the need to switch
>
> Yes I need that. This requires an API to set the mode through
> fb_var_screeninfo, which is why I skipped modifying fb_fix_screeinfo.
>
> A new FB_TYPE_* could be used to report that we use a 4CC-based mode, with the
> exact mode reported in one of the fb_fix_screeninfo reserved fields (or the
> type_aux field). This would duplicate the information passed through
> fb_var_screeninfo though. Do you think it's worth it ?

I think it's more like a FB_VISUAL_FOURCC as you want to express how the color 
<-> pixel mapping is. The FB_TYPE_* is more about whether pixel are packed or 
represented as planes (the 2 format groups mentioned on fourcc.org).
That's certainly something I'd introduce as it would (hopefully) work to prevent 
old applications which don't know your extension manipulating a FOURCC format 
thinking that it is RGB.
So I think we should
fix.visual = FB_VISUAL_FOURCC;
fix.type = FB_TYPE_PACKED_PIXELS | FB_TYPE_PLANES; /* (?) */
or maybe add a FB_TYPE_FOURCC and rely only on the information in FOURCC (as 
things like UYVY could become confusing as macropixel!=pixel)

>> I guess it would be a good idea to add a new flag to the vmode bitfield in
>> fb_var_screeninfo which looks like a general purpose modifier for the
>> videomode. You could than reuse any RGB-specific field you like to pass more
>> information.
>
> That looks good to me. The grayscale field could be reused to pass the 4CC.

var.grayscale = <FOURCC_FORMAT>;
var.vmode = FB_VMODE_FOURCC;
and if this vmode flag is not set it means traditional mode (based on RGBA).

>> Maybe we should also use this chance to declare one of the fix_screeninfo
>> reserved fields to be used for capability flags or an API version as we can
>> assume that those are 0 (at least in sane drivers).
>
> That's always good, although it's not a hard requirement for the purpose of
> YUV support.

Sure. But it's good to let the application know whether you support the new 
extension or whether you just ignore the flag. So I'm voting for a
fix.caps = FB_CAP_FOURCC;


Best regards,

Florian Tobias Schandinat

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

* Re: [RFC] Standardize YUV support in the fbdev API
@ 2011-05-23 22:56       ` Florian Tobias Schandinat
  0 siblings, 0 replies; 63+ messages in thread
From: Florian Tobias Schandinat @ 2011-05-23 22:56 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel

Hi Laurent,

On 05/23/2011 09:00 PM, Laurent Pinchart wrote:
> On Saturday 21 May 2011 00:33:02 Florian Tobias Schandinat wrote:
>> On 05/17/2011 10:07 PM, Laurent Pinchart wrote:
>>> - Other solutions are possible, such as adding new ioctls. Those
>>> solutions are more intrusive, and require larger changes to both
>>> userspace and kernelspace code.
>>
>> I'm against (ab)using the nonstd field (probably the only sane thing we can
>> do with it is declare it non-standard which interpretation is completely
>> dependent on the specific driver) or requiring previously unused fields to
>> have a special value so I'd like to suggest a different method:
>>
>> I remembered an earlier discussion:
>> [ http://marc.info/?l=linux-fbdev&m\x129896686208130&w=2 ]
>>
>> On 03/01/2011 08:07 AM, Geert Uytterhoeven wrote:
>>   >  On Tue, Mar 1, 2011 at 04:13, Damian<dhobsong@igel.co.jp>   wrote:
>>   >>  On 2011/02/24 15:05, Geert Uytterhoeven wrote:
>>   >>>  For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
>>   >>>  FB_VISUAL_*
>>   >>>  type instead, which indicates the fb_var_screeninfo.{red,green,blue}
>>   >>>  fields are
>>   >>>  YCbCr instead of RGB.
>>   >>>  Depending on the frame buffer organization, you also need new
>>   >>>  FB_TYPE_*/FB_AUX_*
>>   >>>  types.
>>   >>
>>   >>  I just wanted to clarify here.  Is your comment about these new flags
>>   >>  in specific reference to this patch or to Magnus' "going forward"
>>   >>  comment?  It
>>   >
>>   >  About new flags.
>>   >
>>   >>  seems like the beginnings of a method to standardize YCbCr support in
>>   >>  fbdev across all platforms.
>>   >>  Also, do I understand correctly that FB_VISUAL_ would specify the
>>   >>  colorspace
>>   >
>>   >  FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped
>>   >  to colors on the screen, so to me it looks like the sensible way to set
>>   >  up YCbCr.
>>   >
>>   >>  (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
>>   >>  semiplanar, interleaved, etc)?  I'm not really sure what you are
>>   >>  referring to with the FB_AUX_* however.
>>   >
>>   >  Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame
>>   >  buffer memory.
>>   >
>>   >  FB_AUX_* is only used if a specific value of FB_TYPE_* needs an
>>   >  additional parameter (e.g. the interleave value for interleaved
>>   >  bitplanes).
>>
>> Adding new standard values for these fb_fix_screeninfo fields would solve
>> the issue for framebuffers which only support a single format.
>
> I've never liked changing fixed screen information :-) It would be consistent
> with the API though.

Fixed does only mean that it can't be directly manipulated by applications. The 
driver has to modify it anyway on about every mode change (line_length). Yes 
perhaps some of these fields would be in var today and certainly others 
shouldn't exist at all but I do not blame anyone for not being capable to look 
into the future.

>> If you have the need to switch
>
> Yes I need that. This requires an API to set the mode through
> fb_var_screeninfo, which is why I skipped modifying fb_fix_screeinfo.
>
> A new FB_TYPE_* could be used to report that we use a 4CC-based mode, with the
> exact mode reported in one of the fb_fix_screeninfo reserved fields (or the
> type_aux field). This would duplicate the information passed through
> fb_var_screeninfo though. Do you think it's worth it ?

I think it's more like a FB_VISUAL_FOURCC as you want to express how the color 
<-> pixel mapping is. The FB_TYPE_* is more about whether pixel are packed or 
represented as planes (the 2 format groups mentioned on fourcc.org).
That's certainly something I'd introduce as it would (hopefully) work to prevent 
old applications which don't know your extension manipulating a FOURCC format 
thinking that it is RGB.
So I think we should
fix.visual = FB_VISUAL_FOURCC;
fix.type = FB_TYPE_PACKED_PIXELS | FB_TYPE_PLANES; /* (?) */
or maybe add a FB_TYPE_FOURCC and rely only on the information in FOURCC (as 
things like UYVY could become confusing as macropixel!=pixel)

>> I guess it would be a good idea to add a new flag to the vmode bitfield in
>> fb_var_screeninfo which looks like a general purpose modifier for the
>> videomode. You could than reuse any RGB-specific field you like to pass more
>> information.
>
> That looks good to me. The grayscale field could be reused to pass the 4CC.

var.grayscale = <FOURCC_FORMAT>;
var.vmode = FB_VMODE_FOURCC;
and if this vmode flag is not set it means traditional mode (based on RGBA).

>> Maybe we should also use this chance to declare one of the fix_screeninfo
>> reserved fields to be used for capability flags or an API version as we can
>> assume that those are 0 (at least in sane drivers).
>
> That's always good, although it's not a hard requirement for the purpose of
> YUV support.

Sure. But it's good to let the application know whether you support the new 
extension or whether you just ignore the flag. So I'm voting for a
fix.caps = FB_CAP_FOURCC;


Best regards,

Florian Tobias Schandinat

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

* [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-05-23 22:56       ` Florian Tobias Schandinat
@ 2011-06-21 15:36         ` Laurent Pinchart
  -1 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-06-21 15:36 UTC (permalink / raw)
  To: linux-fbdev; +Cc: linux-media, dri-devel, FlorianSchandinat

This API will be used to support YUV frame buffer formats in a standard
way.

Last but not least, create a much needed fbdev API documentation and
document the format setting APIs.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/fb/api.txt |  284 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h       |   12 ++-
 2 files changed, 294 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/fb/api.txt

A bit late, but here's a patch that implements an fbdev format configuration
API to support YUV formats. My next step is to implement a prototype in an
fbdev driver to make sure the API works well.

Thanks to Florian Tobias Schandinat for providing feedback on the initial RFC.
Comments are as usual more than welcome. If you feel like writing a couple of
lines of documentation, feel free to extend the API doc. That's much needed.

diff --git a/Documentation/fb/api.txt b/Documentation/fb/api.txt
new file mode 100644
index 0000000..d4cd6ec
--- /dev/null
+++ b/Documentation/fb/api.txt
@@ -0,0 +1,284 @@
+			The Frame Buffer Device API
+			---------------------------
+
+Last revised: June 21, 2011
+
+
+0. Introduction
+---------------
+
+This document describes the frame buffer API used by applications to interact
+with frame buffer devices. In-kernel APIs between device drivers and the frame
+buffer core are not described.
+
+Due to a lack of documentation in the original frame buffer API, drivers
+behaviours differ in subtle (and not so subtle) ways. This document describes
+the recommended API implementation, but applications should be prepared to
+deal with different behaviours.
+
+
+1. Capabilities
+---------------
+
+Device and driver capabilities are reported in the fixed screen information
+capabilities field.
+
+struct fb_fix_screeninfo {
+	...
+	__u16 capabilities;		/* see FB_CAP_*			*/
+	...
+};
+
+Application should use those capabilities to find out what features they can
+expect from the device and driver.
+
+- FB_CAP_FOURCC
+
+The driver supports the four character code (FOURCC) based format setting API.
+When supported, formats are configured using a FOURCC instead of manually
+specifying color components layout.
+
+
+2. Types and visuals
+--------------------
+
+Pixels are stored in memory in hardware-dependent formats. Applications need
+to be aware of the pixel storage format in order to write image data to the
+frame buffer memory in the format expected by the hardware.
+
+Formats are described by frame buffer types and visuals. Some visuals require
+additional information, which are stored in the variable screen information
+bits_per_pixel, grayscale, fourcc, red, green, blue and transp fields.
+
+The following types and visuals are supported.
+
+- FB_TYPE_PACKED_PIXELS
+
+Color components (usually RGB or YUV) are packed together into macropixels
+that are stored in a single plane. The exact color components layout is
+described in a visual-dependent way.
+
+Frame buffer visuals that don't use multiple color components per pixel
+(such as monochrome and pseudo-color visuals) are reported as packed frame
+buffer types, even though they don't stricly speaking pack color components
+into macropixels.
+
+- FB_TYPE_PLANES
+
+Color components are stored in separate planes. Planes are located
+contiguously in memory.
+
+- FB_VISUAL_MONO01
+
+Pixels are black or white and stored on one bit. A bit set to 1 represents a
+black pixel and a bit set to 0 a white pixel. Pixels are packed together in
+bytes with 8 pixels per byte.
+
+FB_VISUAL_MONO01 is used with FB_TYPE_PACKED_PIXELS only.
+
+- FB_VISUAL_MONO10
+
+Pixels are black or white and stored on one bit. A bit set to 1 represents a
+white pixel and a bit set to 0 a black pixel. Pixels are packed together in
+bytes with 8 pixels per byte.
+
+FB_VISUAL_MONO01 is used with FB_TYPE_PACKED_PIXELS only.
+
+- FB_VISUAL_TRUECOLOR
+
+Pixels are broken into red, green and blue components, and each component
+indexes a read-only lookup table for the corresponding value. Lookup tables
+are device-dependent, and provide linear or non-linear ramps.
+
+Each component is stored in memory according to the variable screen
+information red, green, blue and transp fields.
+
+- FB_VISUAL_PSEUDOCOLOR and FB_VISUAL_STATIC_PSEUDOCOLOR
+
+Pixel values are encoded as indices into a colormap that stores red, green and
+blue components. The colormap is read-only for FB_VISUAL_STATIC_PSEUDOCOLOR
+and read-write for FB_VISUAL_PSEUDOCOLOR.
+
+Each pixel value is stored in the number of bits reported by the variable
+screen information bits_per_pixel field. Pixels are contiguous in memory.
+
+FB_VISUAL_PSEUDOCOLOR and FB_VISUAL_STATIC_PSEUDOCOLOR are used with
+FB_TYPE_PACKED_PIXELS only.
+
+- FB_VISUAL_DIRECTCOLOR
+
+Pixels are broken into red, green and blue components, and each component
+indexes a programmable lookup table for the corresponding value.
+
+Each component is stored in memory according to the variable screen
+information red, green, blue and transp fields.
+
+- FB_VISUAL_FOURCC
+
+Pixels are stored in memory as described by the format FOURCC identifier
+stored in the variable screen information fourcc field.
+
+
+3. Screen information
+---------------------
+
+Screen information are queried by applications using the FBIOGET_FSCREENINFO
+and FBIOGET_VSCREENINFO ioctls. Those ioctls take a pointer to a
+fb_fix_screeninfo and fb_var_screeninfo structure respectively.
+
+struct fb_fix_screeninfo stores device independent unchangeable information
+about the frame buffer device and the current format. Those information can't
+be directly modified by applications, but can be changed by the driver when an
+application modifies the format.
+
+struct fb_fix_screeninfo {
+	char id[16];			/* identification string eg "TT Builtin" */
+	unsigned long smem_start;	/* Start of frame buffer mem */
+					/* (physical address) */
+	__u32 smem_len;			/* Length of frame buffer mem */
+	__u32 type;			/* see FB_TYPE_*		*/
+	__u32 type_aux;			/* Interleave for interleaved Planes */
+	__u32 visual;			/* see FB_VISUAL_*		*/
+	__u16 xpanstep;			/* zero if no hardware panning  */
+	__u16 ypanstep;			/* zero if no hardware panning  */
+	__u16 ywrapstep;		/* zero if no hardware ywrap    */
+	__u32 line_length;		/* length of a line in bytes    */
+	unsigned long mmio_start;	/* Start of Memory Mapped I/O   */
+					/* (physical address) */
+	__u32 mmio_len;			/* Length of Memory Mapped I/O  */
+	__u32 accel;			/* Indicate to driver which	*/
+					/*  specific chip/card we have	*/
+	__u16 capabilities;		/* see FB_CAP_*			*/
+	__u16 reserved[2];		/* Reserved for future compatibility */
+};
+
+struct fb_var_screeninfo stores device independent changeable information
+about a frame buffer device, its current format and video mode, as well as
+other miscellaneous parameters.
+
+struct fb_var_screeninfo {
+	__u32 xres;			/* visible resolution		*/
+	__u32 yres;
+	__u32 xres_virtual;		/* virtual resolution		*/
+	__u32 yres_virtual;
+	__u32 xoffset;			/* offset from virtual to visible */
+	__u32 yoffset;			/* resolution			*/
+
+	__u32 bits_per_pixel;		/* guess what			*/
+	union {
+		__u32 grayscale;	/* != 0 Graylevels instead of colors */
+		__u32 fourcc;		/* video mode */
+	};
+
+	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
+	struct fb_bitfield green;	/* else only length is significant */
+	struct fb_bitfield blue;
+	struct fb_bitfield transp;	/* transparency			*/
+
+	__u32 nonstd;			/* != 0 Non standard pixel format */
+
+	__u32 activate;			/* see FB_ACTIVATE_*		*/
+
+	__u32 height;			/* height of picture in mm    */
+	__u32 width;			/* width of picture in mm     */
+
+	__u32 accel_flags;		/* (OBSOLETE) see fb_info.flags */
+
+	/* Timing: All values in pixclocks, except pixclock (of course) */
+	__u32 pixclock;			/* pixel clock in ps (pico seconds) */
+	__u32 left_margin;		/* time from sync to picture	*/
+	__u32 right_margin;		/* time from picture to sync	*/
+	__u32 upper_margin;		/* time from sync to picture	*/
+	__u32 lower_margin;
+	__u32 hsync_len;		/* length of horizontal sync	*/
+	__u32 vsync_len;		/* length of vertical sync	*/
+	__u32 sync;			/* see FB_SYNC_*		*/
+	__u32 vmode;			/* see FB_VMODE_*		*/
+	__u32 rotate;			/* angle we rotate counter clockwise */
+	__u32 reserved[5];		/* Reserved for future compatibility */
+};
+
+To modify variable information, applications call the FBIOPUT_VSCREENINFO
+ioctl with a pointer to a fb_var_screeninfo structure. If the call is
+successful, the driver will update the fixed screen information accordingly.
+
+Instead of filling the complete fb_var_screeninfo structure manually,
+applications should call the FBIOGET_VSCREENINFO ioctl and modify only the
+fields they care about.
+
+
+4. Format configuration
+-----------------------
+
+Frame buffer devices offer two ways to configure the frame buffer format: the
+legacy API and the FOURCC-based API.
+
+
+The legacy API has been the only frame buffer format configuration API for a
+long time and is thus widely used by application. It is the recommended API
+for applications when using RGB and grayscale formats, as well as legacy
+non-standard formats.
+
+To select a format, applications set the fb_var_screeninfo bits_per_pixel field
+to the desired frame buffer depth. Values up to 8 will usually map to
+monochrome, grayscale or pseudocolor visuals, although this is not required.
+
+- For grayscale formats, applications set the grayscale field to a non-zero
+  value. The red, blue, green and transp fields must be set to 0 by
+  applications and ignored by drivers. Drivers must fill the red, blue and
+  green offsets to 0 and lengths to the bits_per_pixel value.
+
+- For pseudocolor formats, applications set the grayscale field to a zero
+  value. The red, blue, green and transp fields must be set to 0 by
+  applications and ignored by drivers. Drivers must fill the red, blue and
+  green offsets to 0 and lengths to the bits_per_pixel value.
+
+- For truecolor and directcolor formats, applications set the grayscale field
+  to a zero value, and the red, blue, green and transp fields to describe the
+  layout of color components in memory.
+
+struct fb_bitfield {
+	__u32 offset;			/* beginning of bitfield	*/
+	__u32 length;			/* length of bitfield		*/
+	__u32 msb_right;		/* != 0 : Most significant bit is */
+					/* right */
+};
+
+  Pixel values are bits_per_pixel wide and are split in non-overlapping red,
+  green, blue and alpha (transparency) components. Location and size of each
+  component in the pixel value are described by the fb_bitfield offset and
+  length fields. Offset are computed from the right.
+
+  Pixels are always stored in an integer number of bytes. If the number of
+  bits per pixel is not a multiple of 8, pixel values are padded to the next
+  multiple of 8 bits.
+
+Upon successful format configuration, drivers update the fb_fix_screeninfo
+type, visual and line_length fields depending on the selected format.
+
+
+The FOURCC-based API replaces format descriptions by four character codes
+(FOURCC). FOURCCs are abstract identifiers that uniquely define a format
+without explicitly describing it. This is the only API that supports YUV
+formats. Drivers are also encouraged to implement the FOURCC-based API for RGB
+and grayscale formats.
+
+Drivers that support the FOURCC-based API report this capability by setting
+the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities field.
+
+FOURCC definitions are located in the linux/videodev2.h header. However, and
+despite starting with the V4L2_PIX_FMT_prefix, they are not restricted to V4L2
+and don't require usage of the V4L2 subsystem. FOURCC documentation is
+available in Documentation/DocBook/v4l/pixfmt.xml.
+
+To select a format, applications set the FB_VMODE_FOURCC bit in the
+fb_var_screeninfo vmode field, and set the fourcc field to the desired FOURCC.
+The bits_per_pixel, red, green, blue, transp and nonstd fields must be set to
+0 by applications and ignored by drivers. Note that the grayscale and fourcc
+fields share the same memory location. Application must thus not set the
+grayscale field to 0.
+
+Upon successful format configuration, drivers update the fb_fix_screeninfo
+type, visual and line_length fields depending on the selected format. The
+visual field is set to FB_VISUAL_FOURCC.
+
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 6a82748..359e98e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -69,6 +69,7 @@
 #define FB_VISUAL_PSEUDOCOLOR		3	/* Pseudo color (like atari) */
 #define FB_VISUAL_DIRECTCOLOR		4	/* Direct color */
 #define FB_VISUAL_STATIC_PSEUDOCOLOR	5	/* Pseudo color readonly */
+#define FB_VISUAL_FOURCC		6	/* Visual identified by a V4L2 FOURCC */
 
 #define FB_ACCEL_NONE		0	/* no hardware accelerator	*/
 #define FB_ACCEL_ATARIBLITT	1	/* Atari Blitter		*/
@@ -154,6 +155,8 @@
 
 #define FB_ACCEL_PUV3_UNIGFX	0xa0	/* PKUnity-v3 Unigfx		*/
 
+#define FB_CAP_FOURCC		1	/* Device supports FOURCC-based formats */
+
 struct fb_fix_screeninfo {
 	char id[16];			/* identification string eg "TT Builtin" */
 	unsigned long smem_start;	/* Start of frame buffer mem */
@@ -171,7 +174,8 @@ struct fb_fix_screeninfo {
 	__u32 mmio_len;			/* Length of Memory Mapped I/O  */
 	__u32 accel;			/* Indicate to driver which	*/
 					/*  specific chip/card we have	*/
-	__u16 reserved[3];		/* Reserved for future compatibility */
+	__u16 capabilities;		/* see FB_CAP_*			*/
+	__u16 reserved[2];		/* Reserved for future compatibility */
 };
 
 /* Interpretation of offset for color fields: All offsets are from the right,
@@ -220,6 +224,7 @@ struct fb_bitfield {
 #define FB_VMODE_INTERLACED	1	/* interlaced	*/
 #define FB_VMODE_DOUBLE		2	/* double scan */
 #define FB_VMODE_ODD_FLD_FIRST	4	/* interlaced: top line first */
+#define FB_VMODE_FOURCC		8	/* video mode expressed as a FOURCC */
 #define FB_VMODE_MASK		255
 
 #define FB_VMODE_YWRAP		256	/* ywrap instead of panning     */
@@ -246,7 +251,10 @@ struct fb_var_screeninfo {
 	__u32 yoffset;			/* resolution			*/
 
 	__u32 bits_per_pixel;		/* guess what			*/
-	__u32 grayscale;		/* != 0 Graylevels instead of colors */
+	union {
+		__u32 grayscale;	/* != 0 Graylevels instead of colors */
+		__u32 fourcc;		/* FOURCC format */
+	};
 
 	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
 	struct fb_bitfield green;	/* else only length is significant */
-- 
Regards,

Laurent Pinchart


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

* [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-06-21 15:36         ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-06-21 15:36 UTC (permalink / raw)
  To: linux-fbdev; +Cc: linux-media, dri-devel, FlorianSchandinat

This API will be used to support YUV frame buffer formats in a standard
way.

Last but not least, create a much needed fbdev API documentation and
document the format setting APIs.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/fb/api.txt |  284 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h       |   12 ++-
 2 files changed, 294 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/fb/api.txt

A bit late, but here's a patch that implements an fbdev format configuration
API to support YUV formats. My next step is to implement a prototype in an
fbdev driver to make sure the API works well.

Thanks to Florian Tobias Schandinat for providing feedback on the initial RFC.
Comments are as usual more than welcome. If you feel like writing a couple of
lines of documentation, feel free to extend the API doc. That's much needed.

diff --git a/Documentation/fb/api.txt b/Documentation/fb/api.txt
new file mode 100644
index 0000000..d4cd6ec
--- /dev/null
+++ b/Documentation/fb/api.txt
@@ -0,0 +1,284 @@
+			The Frame Buffer Device API
+			---------------------------
+
+Last revised: June 21, 2011
+
+
+0. Introduction
+---------------
+
+This document describes the frame buffer API used by applications to interact
+with frame buffer devices. In-kernel APIs between device drivers and the frame
+buffer core are not described.
+
+Due to a lack of documentation in the original frame buffer API, drivers
+behaviours differ in subtle (and not so subtle) ways. This document describes
+the recommended API implementation, but applications should be prepared to
+deal with different behaviours.
+
+
+1. Capabilities
+---------------
+
+Device and driver capabilities are reported in the fixed screen information
+capabilities field.
+
+struct fb_fix_screeninfo {
+	...
+	__u16 capabilities;		/* see FB_CAP_*			*/
+	...
+};
+
+Application should use those capabilities to find out what features they can
+expect from the device and driver.
+
+- FB_CAP_FOURCC
+
+The driver supports the four character code (FOURCC) based format setting API.
+When supported, formats are configured using a FOURCC instead of manually
+specifying color components layout.
+
+
+2. Types and visuals
+--------------------
+
+Pixels are stored in memory in hardware-dependent formats. Applications need
+to be aware of the pixel storage format in order to write image data to the
+frame buffer memory in the format expected by the hardware.
+
+Formats are described by frame buffer types and visuals. Some visuals require
+additional information, which are stored in the variable screen information
+bits_per_pixel, grayscale, fourcc, red, green, blue and transp fields.
+
+The following types and visuals are supported.
+
+- FB_TYPE_PACKED_PIXELS
+
+Color components (usually RGB or YUV) are packed together into macropixels
+that are stored in a single plane. The exact color components layout is
+described in a visual-dependent way.
+
+Frame buffer visuals that don't use multiple color components per pixel
+(such as monochrome and pseudo-color visuals) are reported as packed frame
+buffer types, even though they don't stricly speaking pack color components
+into macropixels.
+
+- FB_TYPE_PLANES
+
+Color components are stored in separate planes. Planes are located
+contiguously in memory.
+
+- FB_VISUAL_MONO01
+
+Pixels are black or white and stored on one bit. A bit set to 1 represents a
+black pixel and a bit set to 0 a white pixel. Pixels are packed together in
+bytes with 8 pixels per byte.
+
+FB_VISUAL_MONO01 is used with FB_TYPE_PACKED_PIXELS only.
+
+- FB_VISUAL_MONO10
+
+Pixels are black or white and stored on one bit. A bit set to 1 represents a
+white pixel and a bit set to 0 a black pixel. Pixels are packed together in
+bytes with 8 pixels per byte.
+
+FB_VISUAL_MONO01 is used with FB_TYPE_PACKED_PIXELS only.
+
+- FB_VISUAL_TRUECOLOR
+
+Pixels are broken into red, green and blue components, and each component
+indexes a read-only lookup table for the corresponding value. Lookup tables
+are device-dependent, and provide linear or non-linear ramps.
+
+Each component is stored in memory according to the variable screen
+information red, green, blue and transp fields.
+
+- FB_VISUAL_PSEUDOCOLOR and FB_VISUAL_STATIC_PSEUDOCOLOR
+
+Pixel values are encoded as indices into a colormap that stores red, green and
+blue components. The colormap is read-only for FB_VISUAL_STATIC_PSEUDOCOLOR
+and read-write for FB_VISUAL_PSEUDOCOLOR.
+
+Each pixel value is stored in the number of bits reported by the variable
+screen information bits_per_pixel field. Pixels are contiguous in memory.
+
+FB_VISUAL_PSEUDOCOLOR and FB_VISUAL_STATIC_PSEUDOCOLOR are used with
+FB_TYPE_PACKED_PIXELS only.
+
+- FB_VISUAL_DIRECTCOLOR
+
+Pixels are broken into red, green and blue components, and each component
+indexes a programmable lookup table for the corresponding value.
+
+Each component is stored in memory according to the variable screen
+information red, green, blue and transp fields.
+
+- FB_VISUAL_FOURCC
+
+Pixels are stored in memory as described by the format FOURCC identifier
+stored in the variable screen information fourcc field.
+
+
+3. Screen information
+---------------------
+
+Screen information are queried by applications using the FBIOGET_FSCREENINFO
+and FBIOGET_VSCREENINFO ioctls. Those ioctls take a pointer to a
+fb_fix_screeninfo and fb_var_screeninfo structure respectively.
+
+struct fb_fix_screeninfo stores device independent unchangeable information
+about the frame buffer device and the current format. Those information can't
+be directly modified by applications, but can be changed by the driver when an
+application modifies the format.
+
+struct fb_fix_screeninfo {
+	char id[16];			/* identification string eg "TT Builtin" */
+	unsigned long smem_start;	/* Start of frame buffer mem */
+					/* (physical address) */
+	__u32 smem_len;			/* Length of frame buffer mem */
+	__u32 type;			/* see FB_TYPE_*		*/
+	__u32 type_aux;			/* Interleave for interleaved Planes */
+	__u32 visual;			/* see FB_VISUAL_*		*/
+	__u16 xpanstep;			/* zero if no hardware panning  */
+	__u16 ypanstep;			/* zero if no hardware panning  */
+	__u16 ywrapstep;		/* zero if no hardware ywrap    */
+	__u32 line_length;		/* length of a line in bytes    */
+	unsigned long mmio_start;	/* Start of Memory Mapped I/O   */
+					/* (physical address) */
+	__u32 mmio_len;			/* Length of Memory Mapped I/O  */
+	__u32 accel;			/* Indicate to driver which	*/
+					/*  specific chip/card we have	*/
+	__u16 capabilities;		/* see FB_CAP_*			*/
+	__u16 reserved[2];		/* Reserved for future compatibility */
+};
+
+struct fb_var_screeninfo stores device independent changeable information
+about a frame buffer device, its current format and video mode, as well as
+other miscellaneous parameters.
+
+struct fb_var_screeninfo {
+	__u32 xres;			/* visible resolution		*/
+	__u32 yres;
+	__u32 xres_virtual;		/* virtual resolution		*/
+	__u32 yres_virtual;
+	__u32 xoffset;			/* offset from virtual to visible */
+	__u32 yoffset;			/* resolution			*/
+
+	__u32 bits_per_pixel;		/* guess what			*/
+	union {
+		__u32 grayscale;	/* != 0 Graylevels instead of colors */
+		__u32 fourcc;		/* video mode */
+	};
+
+	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
+	struct fb_bitfield green;	/* else only length is significant */
+	struct fb_bitfield blue;
+	struct fb_bitfield transp;	/* transparency			*/
+
+	__u32 nonstd;			/* != 0 Non standard pixel format */
+
+	__u32 activate;			/* see FB_ACTIVATE_*		*/
+
+	__u32 height;			/* height of picture in mm    */
+	__u32 width;			/* width of picture in mm     */
+
+	__u32 accel_flags;		/* (OBSOLETE) see fb_info.flags */
+
+	/* Timing: All values in pixclocks, except pixclock (of course) */
+	__u32 pixclock;			/* pixel clock in ps (pico seconds) */
+	__u32 left_margin;		/* time from sync to picture	*/
+	__u32 right_margin;		/* time from picture to sync	*/
+	__u32 upper_margin;		/* time from sync to picture	*/
+	__u32 lower_margin;
+	__u32 hsync_len;		/* length of horizontal sync	*/
+	__u32 vsync_len;		/* length of vertical sync	*/
+	__u32 sync;			/* see FB_SYNC_*		*/
+	__u32 vmode;			/* see FB_VMODE_*		*/
+	__u32 rotate;			/* angle we rotate counter clockwise */
+	__u32 reserved[5];		/* Reserved for future compatibility */
+};
+
+To modify variable information, applications call the FBIOPUT_VSCREENINFO
+ioctl with a pointer to a fb_var_screeninfo structure. If the call is
+successful, the driver will update the fixed screen information accordingly.
+
+Instead of filling the complete fb_var_screeninfo structure manually,
+applications should call the FBIOGET_VSCREENINFO ioctl and modify only the
+fields they care about.
+
+
+4. Format configuration
+-----------------------
+
+Frame buffer devices offer two ways to configure the frame buffer format: the
+legacy API and the FOURCC-based API.
+
+
+The legacy API has been the only frame buffer format configuration API for a
+long time and is thus widely used by application. It is the recommended API
+for applications when using RGB and grayscale formats, as well as legacy
+non-standard formats.
+
+To select a format, applications set the fb_var_screeninfo bits_per_pixel field
+to the desired frame buffer depth. Values up to 8 will usually map to
+monochrome, grayscale or pseudocolor visuals, although this is not required.
+
+- For grayscale formats, applications set the grayscale field to a non-zero
+  value. The red, blue, green and transp fields must be set to 0 by
+  applications and ignored by drivers. Drivers must fill the red, blue and
+  green offsets to 0 and lengths to the bits_per_pixel value.
+
+- For pseudocolor formats, applications set the grayscale field to a zero
+  value. The red, blue, green and transp fields must be set to 0 by
+  applications and ignored by drivers. Drivers must fill the red, blue and
+  green offsets to 0 and lengths to the bits_per_pixel value.
+
+- For truecolor and directcolor formats, applications set the grayscale field
+  to a zero value, and the red, blue, green and transp fields to describe the
+  layout of color components in memory.
+
+struct fb_bitfield {
+	__u32 offset;			/* beginning of bitfield	*/
+	__u32 length;			/* length of bitfield		*/
+	__u32 msb_right;		/* != 0 : Most significant bit is */
+					/* right */
+};
+
+  Pixel values are bits_per_pixel wide and are split in non-overlapping red,
+  green, blue and alpha (transparency) components. Location and size of each
+  component in the pixel value are described by the fb_bitfield offset and
+  length fields. Offset are computed from the right.
+
+  Pixels are always stored in an integer number of bytes. If the number of
+  bits per pixel is not a multiple of 8, pixel values are padded to the next
+  multiple of 8 bits.
+
+Upon successful format configuration, drivers update the fb_fix_screeninfo
+type, visual and line_length fields depending on the selected format.
+
+
+The FOURCC-based API replaces format descriptions by four character codes
+(FOURCC). FOURCCs are abstract identifiers that uniquely define a format
+without explicitly describing it. This is the only API that supports YUV
+formats. Drivers are also encouraged to implement the FOURCC-based API for RGB
+and grayscale formats.
+
+Drivers that support the FOURCC-based API report this capability by setting
+the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities field.
+
+FOURCC definitions are located in the linux/videodev2.h header. However, and
+despite starting with the V4L2_PIX_FMT_prefix, they are not restricted to V4L2
+and don't require usage of the V4L2 subsystem. FOURCC documentation is
+available in Documentation/DocBook/v4l/pixfmt.xml.
+
+To select a format, applications set the FB_VMODE_FOURCC bit in the
+fb_var_screeninfo vmode field, and set the fourcc field to the desired FOURCC.
+The bits_per_pixel, red, green, blue, transp and nonstd fields must be set to
+0 by applications and ignored by drivers. Note that the grayscale and fourcc
+fields share the same memory location. Application must thus not set the
+grayscale field to 0.
+
+Upon successful format configuration, drivers update the fb_fix_screeninfo
+type, visual and line_length fields depending on the selected format. The
+visual field is set to FB_VISUAL_FOURCC.
+
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 6a82748..359e98e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -69,6 +69,7 @@
 #define FB_VISUAL_PSEUDOCOLOR		3	/* Pseudo color (like atari) */
 #define FB_VISUAL_DIRECTCOLOR		4	/* Direct color */
 #define FB_VISUAL_STATIC_PSEUDOCOLOR	5	/* Pseudo color readonly */
+#define FB_VISUAL_FOURCC		6	/* Visual identified by a V4L2 FOURCC */
 
 #define FB_ACCEL_NONE		0	/* no hardware accelerator	*/
 #define FB_ACCEL_ATARIBLITT	1	/* Atari Blitter		*/
@@ -154,6 +155,8 @@
 
 #define FB_ACCEL_PUV3_UNIGFX	0xa0	/* PKUnity-v3 Unigfx		*/
 
+#define FB_CAP_FOURCC		1	/* Device supports FOURCC-based formats */
+
 struct fb_fix_screeninfo {
 	char id[16];			/* identification string eg "TT Builtin" */
 	unsigned long smem_start;	/* Start of frame buffer mem */
@@ -171,7 +174,8 @@ struct fb_fix_screeninfo {
 	__u32 mmio_len;			/* Length of Memory Mapped I/O  */
 	__u32 accel;			/* Indicate to driver which	*/
 					/*  specific chip/card we have	*/
-	__u16 reserved[3];		/* Reserved for future compatibility */
+	__u16 capabilities;		/* see FB_CAP_*			*/
+	__u16 reserved[2];		/* Reserved for future compatibility */
 };
 
 /* Interpretation of offset for color fields: All offsets are from the right,
@@ -220,6 +224,7 @@ struct fb_bitfield {
 #define FB_VMODE_INTERLACED	1	/* interlaced	*/
 #define FB_VMODE_DOUBLE		2	/* double scan */
 #define FB_VMODE_ODD_FLD_FIRST	4	/* interlaced: top line first */
+#define FB_VMODE_FOURCC		8	/* video mode expressed as a FOURCC */
 #define FB_VMODE_MASK		255
 
 #define FB_VMODE_YWRAP		256	/* ywrap instead of panning     */
@@ -246,7 +251,10 @@ struct fb_var_screeninfo {
 	__u32 yoffset;			/* resolution			*/
 
 	__u32 bits_per_pixel;		/* guess what			*/
-	__u32 grayscale;		/* != 0 Graylevels instead of colors */
+	union {
+		__u32 grayscale;	/* != 0 Graylevels instead of colors */
+		__u32 fourcc;		/* FOURCC format */
+	};
 
 	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
 	struct fb_bitfield green;	/* else only length is significant */
-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-06-21 15:36         ` Laurent Pinchart
@ 2011-06-21 20:49           ` Geert Uytterhoeven
  -1 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-06-21 20:49 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel, FlorianSchandinat

Hi Laurent,

On Tue, Jun 21, 2011 at 17:36, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> +The following types and visuals are supported.
> +
> +- FB_TYPE_PACKED_PIXELS
> +
> +- FB_TYPE_PLANES

You forgot FB_TYPE_INTERLEAVED_PLANES, FB_TYPE_TEXT, and
FB_TYPE_VGA_PLANES. Ah, that's the "feel free to extend the API doc"  :-)

> +The FOURCC-based API replaces format descriptions by four character codes
> +(FOURCC). FOURCCs are abstract identifiers that uniquely define a format
> +without explicitly describing it. This is the only API that supports YUV
> +formats. Drivers are also encouraged to implement the FOURCC-based API for RGB
> +and grayscale formats.
> +
> +Drivers that support the FOURCC-based API report this capability by setting
> +the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities field.
> +
> +FOURCC definitions are located in the linux/videodev2.h header. However, and
> +despite starting with the V4L2_PIX_FMT_prefix, they are not restricted to V4L2
> +and don't require usage of the V4L2 subsystem. FOURCC documentation is
> +available in Documentation/DocBook/v4l/pixfmt.xml.
> +
> +To select a format, applications set the FB_VMODE_FOURCC bit in the
> +fb_var_screeninfo vmode field, and set the fourcc field to the desired FOURCC.
> +The bits_per_pixel, red, green, blue, transp and nonstd fields must be set to
> +0 by applications and ignored by drivers. Note that the grayscale and fourcc
> +fields share the same memory location. Application must thus not set the
> +grayscale field to 0.

These are the only parts I don't like: (ab)using the vmode field (this
isn't really a
vmode flag), and the union of grayscale and fourcc (avoid unions where
possible).

What about storing the FOURCC value in nonstd instead?
As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
be non-zero),
I don't think there are any conflicts with existing values of nonstd.
To make it even safer and easier to parse, you could set bit 31 of
nonstd as a FOURCC
indicator.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-06-21 20:49           ` Geert Uytterhoeven
  0 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-06-21 20:49 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel, FlorianSchandinat

Hi Laurent,

On Tue, Jun 21, 2011 at 17:36, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> +The following types and visuals are supported.
> +
> +- FB_TYPE_PACKED_PIXELS
> +
> +- FB_TYPE_PLANES

You forgot FB_TYPE_INTERLEAVED_PLANES, FB_TYPE_TEXT, and
FB_TYPE_VGA_PLANES. Ah, that's the "feel free to extend the API doc"  :-)

> +The FOURCC-based API replaces format descriptions by four character codes
> +(FOURCC). FOURCCs are abstract identifiers that uniquely define a format
> +without explicitly describing it. This is the only API that supports YUV
> +formats. Drivers are also encouraged to implement the FOURCC-based API for RGB
> +and grayscale formats.
> +
> +Drivers that support the FOURCC-based API report this capability by setting
> +the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities field.
> +
> +FOURCC definitions are located in the linux/videodev2.h header. However, and
> +despite starting with the V4L2_PIX_FMT_prefix, they are not restricted to V4L2
> +and don't require usage of the V4L2 subsystem. FOURCC documentation is
> +available in Documentation/DocBook/v4l/pixfmt.xml.
> +
> +To select a format, applications set the FB_VMODE_FOURCC bit in the
> +fb_var_screeninfo vmode field, and set the fourcc field to the desired FOURCC.
> +The bits_per_pixel, red, green, blue, transp and nonstd fields must be set to
> +0 by applications and ignored by drivers. Note that the grayscale and fourcc
> +fields share the same memory location. Application must thus not set the
> +grayscale field to 0.

These are the only parts I don't like: (ab)using the vmode field (this
isn't really a
vmode flag), and the union of grayscale and fourcc (avoid unions where
possible).

What about storing the FOURCC value in nonstd instead?
As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
be non-zero),
I don't think there are any conflicts with existing values of nonstd.
To make it even safer and easier to parse, you could set bit 31 of
nonstd as a FOURCC
indicator.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-06-21 20:49           ` Geert Uytterhoeven
@ 2011-06-21 22:31             ` Laurent Pinchart
  -1 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-06-21 22:31 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-fbdev, linux-media, dri-devel, FlorianSchandinat

Hi Geert,

On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> On Tue, Jun 21, 2011 at 17:36, Laurent Pinchart wrote:
> > +The following types and visuals are supported.
> > +
> > +- FB_TYPE_PACKED_PIXELS
> > +
> > +- FB_TYPE_PLANES
> 
> You forgot FB_TYPE_INTERLEAVED_PLANES, FB_TYPE_TEXT, and
> FB_TYPE_VGA_PLANES. Ah, that's the "feel free to extend the API doc"  :-)

To be honest, I don't know how they work. That's why I haven't documented 
them.

> > +The FOURCC-based API replaces format descriptions by four character
> > codes +(FOURCC). FOURCCs are abstract identifiers that uniquely define a
> > format +without explicitly describing it. This is the only API that
> > supports YUV +formats. Drivers are also encouraged to implement the
> > FOURCC-based API for RGB +and grayscale formats.
> > +
> > +Drivers that support the FOURCC-based API report this capability by
> > setting +the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities
> > field. +
> > +FOURCC definitions are located in the linux/videodev2.h header. However,
> > and +despite starting with the V4L2_PIX_FMT_prefix, they are not
> > restricted to V4L2 +and don't require usage of the V4L2 subsystem.
> > FOURCC documentation is +available in
> > Documentation/DocBook/v4l/pixfmt.xml.
> > +
> > +To select a format, applications set the FB_VMODE_FOURCC bit in the
> > +fb_var_screeninfo vmode field, and set the fourcc field to the desired
> > FOURCC. +The bits_per_pixel, red, green, blue, transp and nonstd fields
> > must be set to +0 by applications and ignored by drivers. Note that the
> > grayscale and fourcc +fields share the same memory location. Application
> > must thus not set the +grayscale field to 0.
> 
> These are the only parts I don't like: (ab)using the vmode field (this
> isn't really a vmode flag), and the union of grayscale and fourcc (avoid
> unions where possible).

I've proposed adding a FB_NONSTD_FORMAT bit to the nonstd field as a FOURCC 
mode indicator in my initial RFC. Florian Tobias Schandinat wasn't very happy 
with that, and proposed using the vmode field instead.

Given that there's virtually no fbdev documentation, whether the vmode field 
and/or nonstd field are good fit for a FOURCC mode indicator is subject to 
interpretation.

> What about storing the FOURCC value in nonstd instead?

Wouldn't that be a union of nonstd and fourcc ? :-) FOURCC-based format 
setting will be a standard fbdev API, I'm not very keen on storing it in the 
nonstd field without a union.

> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
> be non-zero), I don't think there are any conflicts with existing values of
> nonstd. To make it even safer and easier to parse, you could set bit 31 of
> nonstd as a FOURCC indicator.

I would then create a union between nonstd and fourcc, and document nonstd as 
being used for the legacy API only. Most existing drivers use a couple of 
nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and uses 
bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd & 
0xff000000 != 0 could be used as a FOURCC mode test.

This assumes that FOURCCs will never have their last character set to '\0'. Is 
that a safe assumption for the future ?

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-06-21 22:31             ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-06-21 22:31 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-fbdev, linux-media, dri-devel, FlorianSchandinat

Hi Geert,

On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> On Tue, Jun 21, 2011 at 17:36, Laurent Pinchart wrote:
> > +The following types and visuals are supported.
> > +
> > +- FB_TYPE_PACKED_PIXELS
> > +
> > +- FB_TYPE_PLANES
> 
> You forgot FB_TYPE_INTERLEAVED_PLANES, FB_TYPE_TEXT, and
> FB_TYPE_VGA_PLANES. Ah, that's the "feel free to extend the API doc"  :-)

To be honest, I don't know how they work. That's why I haven't documented 
them.

> > +The FOURCC-based API replaces format descriptions by four character
> > codes +(FOURCC). FOURCCs are abstract identifiers that uniquely define a
> > format +without explicitly describing it. This is the only API that
> > supports YUV +formats. Drivers are also encouraged to implement the
> > FOURCC-based API for RGB +and grayscale formats.
> > +
> > +Drivers that support the FOURCC-based API report this capability by
> > setting +the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities
> > field. +
> > +FOURCC definitions are located in the linux/videodev2.h header. However,
> > and +despite starting with the V4L2_PIX_FMT_prefix, they are not
> > restricted to V4L2 +and don't require usage of the V4L2 subsystem.
> > FOURCC documentation is +available in
> > Documentation/DocBook/v4l/pixfmt.xml.
> > +
> > +To select a format, applications set the FB_VMODE_FOURCC bit in the
> > +fb_var_screeninfo vmode field, and set the fourcc field to the desired
> > FOURCC. +The bits_per_pixel, red, green, blue, transp and nonstd fields
> > must be set to +0 by applications and ignored by drivers. Note that the
> > grayscale and fourcc +fields share the same memory location. Application
> > must thus not set the +grayscale field to 0.
> 
> These are the only parts I don't like: (ab)using the vmode field (this
> isn't really a vmode flag), and the union of grayscale and fourcc (avoid
> unions where possible).

I've proposed adding a FB_NONSTD_FORMAT bit to the nonstd field as a FOURCC 
mode indicator in my initial RFC. Florian Tobias Schandinat wasn't very happy 
with that, and proposed using the vmode field instead.

Given that there's virtually no fbdev documentation, whether the vmode field 
and/or nonstd field are good fit for a FOURCC mode indicator is subject to 
interpretation.

> What about storing the FOURCC value in nonstd instead?

Wouldn't that be a union of nonstd and fourcc ? :-) FOURCC-based format 
setting will be a standard fbdev API, I'm not very keen on storing it in the 
nonstd field without a union.

> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
> be non-zero), I don't think there are any conflicts with existing values of
> nonstd. To make it even safer and easier to parse, you could set bit 31 of
> nonstd as a FOURCC indicator.

I would then create a union between nonstd and fourcc, and document nonstd as 
being used for the legacy API only. Most existing drivers use a couple of 
nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and uses 
bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd & 
0xff000000 != 0 could be used as a FOURCC mode test.

This assumes that FOURCCs will never have their last character set to '\0'. Is 
that a safe assumption for the future ?

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-06-21 22:31             ` Laurent Pinchart
  (?)
@ 2011-06-22  5:45             ` Florian Tobias Schandinat
  2011-06-22  8:50                 ` Laurent Pinchart
  2011-06-23 16:08                 ` Geert Uytterhoeven
  -1 siblings, 2 replies; 63+ messages in thread
From: Florian Tobias Schandinat @ 2011-06-22  5:45 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Geert Uytterhoeven, linux-fbdev, linux-media, dri-devel

Hi Geert, Laurent,

On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> Hi Geert,
>
> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
>> On Tue, Jun 21, 2011 at 17:36, Laurent Pinchart wrote:
>>> +The FOURCC-based API replaces format descriptions by four character
>>> codes +(FOURCC). FOURCCs are abstract identifiers that uniquely define a
>>> format +without explicitly describing it. This is the only API that
>>> supports YUV +formats. Drivers are also encouraged to implement the
>>> FOURCC-based API for RGB +and grayscale formats.
>>> +
>>> +Drivers that support the FOURCC-based API report this capability by
>>> setting +the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities
>>> field. +
>>> +FOURCC definitions are located in the linux/videodev2.h header. However,
>>> and +despite starting with the V4L2_PIX_FMT_prefix, they are not
>>> restricted to V4L2 +and don't require usage of the V4L2 subsystem.
>>> FOURCC documentation is +available in
>>> Documentation/DocBook/v4l/pixfmt.xml.
>>> +
>>> +To select a format, applications set the FB_VMODE_FOURCC bit in the
>>> +fb_var_screeninfo vmode field, and set the fourcc field to the desired
>>> FOURCC. +The bits_per_pixel, red, green, blue, transp and nonstd fields
>>> must be set to +0 by applications and ignored by drivers. Note that the
>>> grayscale and fourcc +fields share the same memory location. Application
>>> must thus not set the +grayscale field to 0.
>>
>> These are the only parts I don't like: (ab)using the vmode field (this
>> isn't really a vmode flag), and the union of grayscale and fourcc (avoid
>> unions where possible).
>
> I've proposed adding a FB_NONSTD_FORMAT bit to the nonstd field as a FOURCC
> mode indicator in my initial RFC. Florian Tobias Schandinat wasn't very happy
> with that, and proposed using the vmode field instead.
>
> Given that there's virtually no fbdev documentation, whether the vmode field
> and/or nonstd field are good fit for a FOURCC mode indicator is subject to
> interpretation.

The reason for my suggestion is that the vmode field is accepted to contain only 
flags and at least to me there is no hard line what is part of the video mode 
and what is not. In contrast the nonstd field is already used in a lot of 
different (incompatible) ways. I think if we only use the nonstd field for 
handling FOURCC it is likely that some problems will appear.

>> What about storing the FOURCC value in nonstd instead?
>
> Wouldn't that be a union of nonstd and fourcc ? :-) FOURCC-based format
> setting will be a standard fbdev API, I'm not very keen on storing it in the
> nonstd field without a union.
>
>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
>> be non-zero), I don't think there are any conflicts with existing values of
>> nonstd. To make it even safer and easier to parse, you could set bit 31 of
>> nonstd as a FOURCC indicator.
>
> I would then create a union between nonstd and fourcc, and document nonstd as
> being used for the legacy API only. Most existing drivers use a couple of
> nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and uses
> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
> 0xff000000 != 0 could be used as a FOURCC mode test.
>
> This assumes that FOURCCs will never have their last character set to '\0'. Is
> that a safe assumption for the future ?

Yes, I think. The information I found indicates that space should be used for 
padding, so a \0 shouldn't exist.
I think using only the nonstd field and requiring applications to check the 
capabilities would be possible, although not fool proof ;)

Great work, Laurent, do you have plans to modify fbset to allow using this 
format API from the command line?


Regards,

Florian Tobias Schandinat

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-06-22  5:45             ` Florian Tobias Schandinat
@ 2011-06-22  8:50                 ` Laurent Pinchart
  2011-06-23 16:08                 ` Geert Uytterhoeven
  1 sibling, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-06-22  8:50 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, linux-fbdev, linux-media, dri-devel

Hi Florian,

On Wednesday 22 June 2011 07:45:45 Florian Tobias Schandinat wrote:
> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> >> On Tue, Jun 21, 2011 at 17:36, Laurent Pinchart wrote:
> >>> +The FOURCC-based API replaces format descriptions by four character
> >>> codes +(FOURCC). FOURCCs are abstract identifiers that uniquely define
> >>> a format +without explicitly describing it. This is the only API that
> >>> supports YUV +formats. Drivers are also encouraged to implement the
> >>> FOURCC-based API for RGB +and grayscale formats.
> >>> +
> >>> +Drivers that support the FOURCC-based API report this capability by
> >>> setting +the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities
> >>> field. +
> >>> +FOURCC definitions are located in the linux/videodev2.h header.
> >>> However, and +despite starting with the V4L2_PIX_FMT_prefix, they are
> >>> not restricted to V4L2 +and don't require usage of the V4L2 subsystem.
> >>> FOURCC documentation is +available in
> >>> Documentation/DocBook/v4l/pixfmt.xml.
> >>> +
> >>> +To select a format, applications set the FB_VMODE_FOURCC bit in the
> >>> +fb_var_screeninfo vmode field, and set the fourcc field to the desired
> >>> FOURCC. +The bits_per_pixel, red, green, blue, transp and nonstd fields
> >>> must be set to +0 by applications and ignored by drivers. Note that the
> >>> grayscale and fourcc +fields share the same memory location.
> >>> Application must thus not set the +grayscale field to 0.
> >> 
> >> These are the only parts I don't like: (ab)using the vmode field (this
> >> isn't really a vmode flag), and the union of grayscale and fourcc (avoid
> >> unions where possible).
> > 
> > I've proposed adding a FB_NONSTD_FORMAT bit to the nonstd field as a
> > FOURCC mode indicator in my initial RFC. Florian Tobias Schandinat
> > wasn't very happy with that, and proposed using the vmode field instead.
> > 
> > Given that there's virtually no fbdev documentation, whether the vmode
> > field and/or nonstd field are good fit for a FOURCC mode indicator is
> > subject to interpretation.
> 
> The reason for my suggestion is that the vmode field is accepted to contain
> only flags and at least to me there is no hard line what is part of the
> video mode and what is not.

Lacks of documentation indeed makes that line fuzzy. I really hope that 
api.txt will be extended to cover the full fbdev API :-)

> In contrast the nonstd field is already used in a lot of different
> (incompatible) ways. I think if we only use the nonstd field for handling
> FOURCC it is likely that some problems will appear.
> 
> >> What about storing the FOURCC value in nonstd instead?
> > 
> > Wouldn't that be a union of nonstd and fourcc ? :-) FOURCC-based format
> > setting will be a standard fbdev API, I'm not very keen on storing it in
> > the nonstd field without a union.
> > 
> >> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
> >> be non-zero), I don't think there are any conflicts with existing values
> >> of nonstd. To make it even safer and easier to parse, you could set bit
> >> 31 of nonstd as a FOURCC indicator.
> > 
> > I would then create a union between nonstd and fourcc, and document
> > nonstd as being used for the legacy API only. Most existing drivers use
> > a couple of nonstd bits only. The driver that (ab)uses nonstd the most
> > is pxafb and uses bits 22:0. Bits 31:24 are never used as far as I can
> > tell, so nonstd& 0xff000000 != 0 could be used as a FOURCC mode test.
> > 
> > This assumes that FOURCCs will never have their last character set to
> > '\0'. Is that a safe assumption for the future ?
> 
> Yes, I think. The information I found indicates that space should be used
> for padding, so a \0 shouldn't exist.
> I think using only the nonstd field and requiring applications to check the
> capabilities would be possible, although not fool proof ;)
> 
> Great work, Laurent, do you have plans to modify fbset to allow using this
> format API from the command line?

Once we agree on an API, I will implement it in a driver and update fbset.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-06-22  8:50                 ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-06-22  8:50 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, linux-fbdev, linux-media, dri-devel

Hi Florian,

On Wednesday 22 June 2011 07:45:45 Florian Tobias Schandinat wrote:
> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> >> On Tue, Jun 21, 2011 at 17:36, Laurent Pinchart wrote:
> >>> +The FOURCC-based API replaces format descriptions by four character
> >>> codes +(FOURCC). FOURCCs are abstract identifiers that uniquely define
> >>> a format +without explicitly describing it. This is the only API that
> >>> supports YUV +formats. Drivers are also encouraged to implement the
> >>> FOURCC-based API for RGB +and grayscale formats.
> >>> +
> >>> +Drivers that support the FOURCC-based API report this capability by
> >>> setting +the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities
> >>> field. +
> >>> +FOURCC definitions are located in the linux/videodev2.h header.
> >>> However, and +despite starting with the V4L2_PIX_FMT_prefix, they are
> >>> not restricted to V4L2 +and don't require usage of the V4L2 subsystem.
> >>> FOURCC documentation is +available in
> >>> Documentation/DocBook/v4l/pixfmt.xml.
> >>> +
> >>> +To select a format, applications set the FB_VMODE_FOURCC bit in the
> >>> +fb_var_screeninfo vmode field, and set the fourcc field to the desired
> >>> FOURCC. +The bits_per_pixel, red, green, blue, transp and nonstd fields
> >>> must be set to +0 by applications and ignored by drivers. Note that the
> >>> grayscale and fourcc +fields share the same memory location.
> >>> Application must thus not set the +grayscale field to 0.
> >> 
> >> These are the only parts I don't like: (ab)using the vmode field (this
> >> isn't really a vmode flag), and the union of grayscale and fourcc (avoid
> >> unions where possible).
> > 
> > I've proposed adding a FB_NONSTD_FORMAT bit to the nonstd field as a
> > FOURCC mode indicator in my initial RFC. Florian Tobias Schandinat
> > wasn't very happy with that, and proposed using the vmode field instead.
> > 
> > Given that there's virtually no fbdev documentation, whether the vmode
> > field and/or nonstd field are good fit for a FOURCC mode indicator is
> > subject to interpretation.
> 
> The reason for my suggestion is that the vmode field is accepted to contain
> only flags and at least to me there is no hard line what is part of the
> video mode and what is not.

Lacks of documentation indeed makes that line fuzzy. I really hope that 
api.txt will be extended to cover the full fbdev API :-)

> In contrast the nonstd field is already used in a lot of different
> (incompatible) ways. I think if we only use the nonstd field for handling
> FOURCC it is likely that some problems will appear.
> 
> >> What about storing the FOURCC value in nonstd instead?
> > 
> > Wouldn't that be a union of nonstd and fourcc ? :-) FOURCC-based format
> > setting will be a standard fbdev API, I'm not very keen on storing it in
> > the nonstd field without a union.
> > 
> >> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
> >> be non-zero), I don't think there are any conflicts with existing values
> >> of nonstd. To make it even safer and easier to parse, you could set bit
> >> 31 of nonstd as a FOURCC indicator.
> > 
> > I would then create a union between nonstd and fourcc, and document
> > nonstd as being used for the legacy API only. Most existing drivers use
> > a couple of nonstd bits only. The driver that (ab)uses nonstd the most
> > is pxafb and uses bits 22:0. Bits 31:24 are never used as far as I can
> > tell, so nonstd& 0xff000000 != 0 could be used as a FOURCC mode test.
> > 
> > This assumes that FOURCCs will never have their last character set to
> > '\0'. Is that a safe assumption for the future ?
> 
> Yes, I think. The information I found indicates that space should be used
> for padding, so a \0 shouldn't exist.
> I think using only the nonstd field and requiring applications to check the
> capabilities would be possible, although not fool proof ;)
> 
> Great work, Laurent, do you have plans to modify fbset to allow using this
> format API from the command line?

Once we agree on an API, I will implement it in a driver and update fbset.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-06-22  5:45             ` Florian Tobias Schandinat
@ 2011-06-23 16:08                 ` Geert Uytterhoeven
  2011-06-23 16:08                 ` Geert Uytterhoeven
  1 sibling, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-06-23 16:08 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Laurent Pinchart, linux-fbdev, linux-media, dri-devel

On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
>>> On Tue, Jun 21, 2011 at 17:36, Laurent Pinchart wrote:
>>>> +The FOURCC-based API replaces format descriptions by four character
>>>> codes +(FOURCC). FOURCCs are abstract identifiers that uniquely define a
>>>> format +without explicitly describing it. This is the only API that
>>>> supports YUV +formats. Drivers are also encouraged to implement the
>>>> FOURCC-based API for RGB +and grayscale formats.
>>>> +
>>>> +Drivers that support the FOURCC-based API report this capability by
>>>> setting +the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities
>>>> field. +
>>>> +FOURCC definitions are located in the linux/videodev2.h header.
>>>> However,
>>>> and +despite starting with the V4L2_PIX_FMT_prefix, they are not
>>>> restricted to V4L2 +and don't require usage of the V4L2 subsystem.
>>>> FOURCC documentation is +available in
>>>> Documentation/DocBook/v4l/pixfmt.xml.
>>>> +
>>>> +To select a format, applications set the FB_VMODE_FOURCC bit in the
>>>> +fb_var_screeninfo vmode field, and set the fourcc field to the desired
>>>> FOURCC. +The bits_per_pixel, red, green, blue, transp and nonstd fields
>>>> must be set to +0 by applications and ignored by drivers. Note that the
>>>> grayscale and fourcc +fields share the same memory location. Application
>>>> must thus not set the +grayscale field to 0.
>>>
>>> These are the only parts I don't like: (ab)using the vmode field (this
>>> isn't really a vmode flag), and the union of grayscale and fourcc (avoid
>>> unions where possible).
>>
>> I've proposed adding a FB_NONSTD_FORMAT bit to the nonstd field as a
>> FOURCC
>> mode indicator in my initial RFC. Florian Tobias Schandinat wasn't very
>> happy
>> with that, and proposed using the vmode field instead.
>>
>> Given that there's virtually no fbdev documentation, whether the vmode
>> field
>> and/or nonstd field are good fit for a FOURCC mode indicator is subject to
>> interpretation.
>
> The reason for my suggestion is that the vmode field is accepted to contain
> only flags and at least to me there is no hard line what is part of the
> video mode and what is not. In contrast the nonstd field is already used in
> a lot of different (incompatible) ways. I think if we only use the nonstd
> field for handling FOURCC it is likely that some problems will appear.
>
>>> What about storing the FOURCC value in nonstd instead?
>>
>> Wouldn't that be a union of nonstd and fourcc ? :-) FOURCC-based format
>> setting will be a standard fbdev API, I'm not very keen on storing it in
>> the
>> nonstd field without a union.
>>
>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
>>> be non-zero), I don't think there are any conflicts with existing values
>>> of
>>> nonstd. To make it even safer and easier to parse, you could set bit 31
>>> of
>>> nonstd as a FOURCC indicator.
>>
>> I would then create a union between nonstd and fourcc, and document nonstd
>> as
>> being used for the legacy API only. Most existing drivers use a couple of
>> nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and
>> uses
>> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
>> 0xff000000 != 0 could be used as a FOURCC mode test.
>>
>> This assumes that FOURCCs will never have their last character set to
>> '\0'. Is
>> that a safe assumption for the future ?
>
> Yes, I think. The information I found indicates that space should be used
> for padding, so a \0 shouldn't exist.
> I think using only the nonstd field and requiring applications to check the
> capabilities would be possible, although not fool proof ;)

So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC if
any of them is set.

Nicely backwards compatible, as sane drivers should reject nonstd values they
don't support (apps _will_ start filling in FOURCC values ignoring capabilities,
won't they?).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-06-23 16:08                 ` Geert Uytterhoeven
  0 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-06-23 16:08 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Laurent Pinchart, linux-fbdev, linux-media, dri-devel

On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
>>> On Tue, Jun 21, 2011 at 17:36, Laurent Pinchart wrote:
>>>> +The FOURCC-based API replaces format descriptions by four character
>>>> codes +(FOURCC). FOURCCs are abstract identifiers that uniquely define a
>>>> format +without explicitly describing it. This is the only API that
>>>> supports YUV +formats. Drivers are also encouraged to implement the
>>>> FOURCC-based API for RGB +and grayscale formats.
>>>> +
>>>> +Drivers that support the FOURCC-based API report this capability by
>>>> setting +the FB_CAP_FOURCC bit in the fb_fix_screeninfo capabilities
>>>> field. +
>>>> +FOURCC definitions are located in the linux/videodev2.h header.
>>>> However,
>>>> and +despite starting with the V4L2_PIX_FMT_prefix, they are not
>>>> restricted to V4L2 +and don't require usage of the V4L2 subsystem.
>>>> FOURCC documentation is +available in
>>>> Documentation/DocBook/v4l/pixfmt.xml.
>>>> +
>>>> +To select a format, applications set the FB_VMODE_FOURCC bit in the
>>>> +fb_var_screeninfo vmode field, and set the fourcc field to the desired
>>>> FOURCC. +The bits_per_pixel, red, green, blue, transp and nonstd fields
>>>> must be set to +0 by applications and ignored by drivers. Note that the
>>>> grayscale and fourcc +fields share the same memory location. Application
>>>> must thus not set the +grayscale field to 0.
>>>
>>> These are the only parts I don't like: (ab)using the vmode field (this
>>> isn't really a vmode flag), and the union of grayscale and fourcc (avoid
>>> unions where possible).
>>
>> I've proposed adding a FB_NONSTD_FORMAT bit to the nonstd field as a
>> FOURCC
>> mode indicator in my initial RFC. Florian Tobias Schandinat wasn't very
>> happy
>> with that, and proposed using the vmode field instead.
>>
>> Given that there's virtually no fbdev documentation, whether the vmode
>> field
>> and/or nonstd field are good fit for a FOURCC mode indicator is subject to
>> interpretation.
>
> The reason for my suggestion is that the vmode field is accepted to contain
> only flags and at least to me there is no hard line what is part of the
> video mode and what is not. In contrast the nonstd field is already used in
> a lot of different (incompatible) ways. I think if we only use the nonstd
> field for handling FOURCC it is likely that some problems will appear.
>
>>> What about storing the FOURCC value in nonstd instead?
>>
>> Wouldn't that be a union of nonstd and fourcc ? :-) FOURCC-based format
>> setting will be a standard fbdev API, I'm not very keen on storing it in
>> the
>> nonstd field without a union.
>>
>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
>>> be non-zero), I don't think there are any conflicts with existing values
>>> of
>>> nonstd. To make it even safer and easier to parse, you could set bit 31
>>> of
>>> nonstd as a FOURCC indicator.
>>
>> I would then create a union between nonstd and fourcc, and document nonstd
>> as
>> being used for the legacy API only. Most existing drivers use a couple of
>> nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and
>> uses
>> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
>> 0xff000000 != 0 could be used as a FOURCC mode test.
>>
>> This assumes that FOURCCs will never have their last character set to
>> '\0'. Is
>> that a safe assumption for the future ?
>
> Yes, I think. The information I found indicates that space should be used
> for padding, so a \0 shouldn't exist.
> I think using only the nonstd field and requiring applications to check the
> capabilities would be possible, although not fool proof ;)

So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC if
any of them is set.

Nicely backwards compatible, as sane drivers should reject nonstd values they
don't support (apps _will_ start filling in FOURCC values ignoring capabilities,
won't they?).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-06-23 16:08                 ` Geert Uytterhoeven
@ 2011-06-24  6:19                   ` Paul Mundt
  -1 siblings, 0 replies; 63+ messages in thread
From: Paul Mundt @ 2011-06-24  6:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Florian Tobias Schandinat, Laurent Pinchart, linux-fbdev,
	linux-media, dri-devel

On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat
> <FlorianSchandinat@gmx.de> wrote:
> > On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> >> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> >>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
> >>> be non-zero), I don't think there are any conflicts with existing values
> >>> of
> >>> nonstd. To make it even safer and easier to parse, you could set bit 31
> >>> of
> >>> nonstd as a FOURCC indicator.
> >>
> >> I would then create a union between nonstd and fourcc, and document nonstd
> >> as
> >> being used for the legacy API only. Most existing drivers use a couple of
> >> nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and
> >> uses
> >> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
> >> 0xff000000 != 0 could be used as a FOURCC mode test.
> >>
> >> This assumes that FOURCCs will never have their last character set to
> >> '\0'. Is
> >> that a safe assumption for the future ?
> >
> > Yes, I think. The information I found indicates that space should be used
> > for padding, so a \0 shouldn't exist.
> > I think using only the nonstd field and requiring applications to check the
> > capabilities would be possible, although not fool proof ;)
> 
> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC if
> any of them is set.
> 
> Nicely backwards compatible, as sane drivers should reject nonstd values they
> don't support (apps _will_ start filling in FOURCC values ignoring capabilities,
> won't they?).
> 
That seems like a reasonable case, but if we're going to do that then
certainly the nonstd bit encoding needs to be documented and treated as a
hard ABI.

I'm not so sure about the if any bit in the upper byte is set assume
FOURCC case though, there will presumably be other users in the future
that will want bits for themselves, too. What exactly was the issue with
having a FOURCC capability bit in the upper byte?

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-06-24  6:19                   ` Paul Mundt
  0 siblings, 0 replies; 63+ messages in thread
From: Paul Mundt @ 2011-06-24  6:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Florian Tobias Schandinat, Laurent Pinchart, linux-fbdev,
	linux-media, dri-devel

On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat
> <FlorianSchandinat@gmx.de> wrote:
> > On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> >> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> >>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
> >>> be non-zero), I don't think there are any conflicts with existing values
> >>> of
> >>> nonstd. To make it even safer and easier to parse, you could set bit 31
> >>> of
> >>> nonstd as a FOURCC indicator.
> >>
> >> I would then create a union between nonstd and fourcc, and document nonstd
> >> as
> >> being used for the legacy API only. Most existing drivers use a couple of
> >> nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and
> >> uses
> >> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
> >> 0xff000000 != 0 could be used as a FOURCC mode test.
> >>
> >> This assumes that FOURCCs will never have their last character set to
> >> '\0'. Is
> >> that a safe assumption for the future ?
> >
> > Yes, I think. The information I found indicates that space should be used
> > for padding, so a \0 shouldn't exist.
> > I think using only the nonstd field and requiring applications to check the
> > capabilities would be possible, although not fool proof ;)
> 
> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC if
> any of them is set.
> 
> Nicely backwards compatible, as sane drivers should reject nonstd values they
> don't support (apps _will_ start filling in FOURCC values ignoring capabilities,
> won't they?).
> 
That seems like a reasonable case, but if we're going to do that then
certainly the nonstd bit encoding needs to be documented and treated as a
hard ABI.

I'm not so sure about the if any bit in the upper byte is set assume
FOURCC case though, there will presumably be other users in the future
that will want bits for themselves, too. What exactly was the issue with
having a FOURCC capability bit in the upper byte?

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-06-24  6:19                   ` Paul Mundt
@ 2011-06-24 18:55                     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-06-24 18:55 UTC (permalink / raw)
  To: Paul Mundt
  Cc: Florian Tobias Schandinat, Laurent Pinchart, linux-fbdev,
	linux-media, dri-devel

On Fri, Jun 24, 2011 at 08:19, Paul Mundt <lethal@linux-sh.org> wrote:
> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat
>> <FlorianSchandinat@gmx.de> wrote:
>> > On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
>> >> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
>> >>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
>> >>> be non-zero), I don't think there are any conflicts with existing values
>> >>> of
>> >>> nonstd. To make it even safer and easier to parse, you could set bit 31
>> >>> of
>> >>> nonstd as a FOURCC indicator.
>> >>
>> >> I would then create a union between nonstd and fourcc, and document nonstd
>> >> as
>> >> being used for the legacy API only. Most existing drivers use a couple of
>> >> nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and
>> >> uses
>> >> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
>> >> 0xff000000 != 0 could be used as a FOURCC mode test.
>> >>
>> >> This assumes that FOURCCs will never have their last character set to
>> >> '\0'. Is
>> >> that a safe assumption for the future ?
>> >
>> > Yes, I think. The information I found indicates that space should be used
>> > for padding, so a \0 shouldn't exist.
>> > I think using only the nonstd field and requiring applications to check the
>> > capabilities would be possible, although not fool proof ;)
>>
>> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC if
>> any of them is set.
>>
>> Nicely backwards compatible, as sane drivers should reject nonstd values they
>> don't support (apps _will_ start filling in FOURCC values ignoring capabilities,
>> won't they?).
>>
> That seems like a reasonable case, but if we're going to do that then
> certainly the nonstd bit encoding needs to be documented and treated as a
> hard ABI.
>
> I'm not so sure about the if any bit in the upper byte is set assume
> FOURCC case though, there will presumably be other users in the future
> that will want bits for themselves, too. What exactly was the issue with
> having a FOURCC capability bit in the upper byte?

That indeed gives less issues (as long as you don't stuff 8-bit ASCII
in the MSB)
and more bits for tradiditional nonstd users.

BTW, after giving it some more thought: what does the FB_CAP_FOURCC buys
us? It's not like all drivers will support whatever random FOURCC mode
you give them,
so you have to check whether it's supported by doing a set_var first.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-06-24 18:55                     ` Geert Uytterhoeven
  0 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-06-24 18:55 UTC (permalink / raw)
  To: Paul Mundt
  Cc: Florian Tobias Schandinat, Laurent Pinchart, linux-fbdev,
	linux-media, dri-devel

On Fri, Jun 24, 2011 at 08:19, Paul Mundt <lethal@linux-sh.org> wrote:
> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat
>> <FlorianSchandinat@gmx.de> wrote:
>> > On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
>> >> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
>> >>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
>> >>> be non-zero), I don't think there are any conflicts with existing values
>> >>> of
>> >>> nonstd. To make it even safer and easier to parse, you could set bit 31
>> >>> of
>> >>> nonstd as a FOURCC indicator.
>> >>
>> >> I would then create a union between nonstd and fourcc, and document nonstd
>> >> as
>> >> being used for the legacy API only. Most existing drivers use a couple of
>> >> nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and
>> >> uses
>> >> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
>> >> 0xff000000 != 0 could be used as a FOURCC mode test.
>> >>
>> >> This assumes that FOURCCs will never have their last character set to
>> >> '\0'. Is
>> >> that a safe assumption for the future ?
>> >
>> > Yes, I think. The information I found indicates that space should be used
>> > for padding, so a \0 shouldn't exist.
>> > I think using only the nonstd field and requiring applications to check the
>> > capabilities would be possible, although not fool proof ;)
>>
>> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC if
>> any of them is set.
>>
>> Nicely backwards compatible, as sane drivers should reject nonstd values they
>> don't support (apps _will_ start filling in FOURCC values ignoring capabilities,
>> won't they?).
>>
> That seems like a reasonable case, but if we're going to do that then
> certainly the nonstd bit encoding needs to be documented and treated as a
> hard ABI.
>
> I'm not so sure about the if any bit in the upper byte is set assume
> FOURCC case though, there will presumably be other users in the future
> that will want bits for themselves, too. What exactly was the issue with
> having a FOURCC capability bit in the upper byte?

That indeed gives less issues (as long as you don't stuff 8-bit ASCII
in the MSB)
and more bits for tradiditional nonstd users.

BTW, after giving it some more thought: what does the FB_CAP_FOURCC buys
us? It's not like all drivers will support whatever random FOURCC mode
you give them,
so you have to check whether it's supported by doing a set_var first.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-06-24 18:55                     ` Geert Uytterhoeven
  (?)
@ 2011-06-24 19:45                     ` Florian Tobias Schandinat
  2011-07-11 15:32                         ` Laurent Pinchart
  -1 siblings, 1 reply; 63+ messages in thread
From: Florian Tobias Schandinat @ 2011-06-24 19:45 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Paul Mundt, Laurent Pinchart, linux-fbdev, linux-media, dri-devel

On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> On Fri, Jun 24, 2011 at 08:19, Paul Mundt<lethal@linux-sh.org>  wrote:
>> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
>>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat
>>> <FlorianSchandinat@gmx.de>  wrote:
>>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
>>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
>>>>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes must
>>>>>> be non-zero), I don't think there are any conflicts with existing values
>>>>>> of
>>>>>> nonstd. To make it even safer and easier to parse, you could set bit 31
>>>>>> of
>>>>>> nonstd as a FOURCC indicator.
>>>>>
>>>>> I would then create a union between nonstd and fourcc, and document nonstd
>>>>> as
>>>>> being used for the legacy API only. Most existing drivers use a couple of
>>>>> nonstd bits only. The driver that (ab)uses nonstd the most is pxafb and
>>>>> uses
>>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
>>>>> 0xff000000 != 0 could be used as a FOURCC mode test.
>>>>>
>>>>> This assumes that FOURCCs will never have their last character set to
>>>>> '\0'. Is
>>>>> that a safe assumption for the future ?
>>>>
>>>> Yes, I think. The information I found indicates that space should be used
>>>> for padding, so a \0 shouldn't exist.
>>>> I think using only the nonstd field and requiring applications to check the
>>>> capabilities would be possible, although not fool proof ;)
>>>
>>> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC if
>>> any of them is set.
>>>
>>> Nicely backwards compatible, as sane drivers should reject nonstd values they
>>> don't support (apps _will_ start filling in FOURCC values ignoring capabilities,
>>> won't they?).
>>>
>> That seems like a reasonable case, but if we're going to do that then
>> certainly the nonstd bit encoding needs to be documented and treated as a
>> hard ABI.
>>
>> I'm not so sure about the if any bit in the upper byte is set assume
>> FOURCC case though, there will presumably be other users in the future
>> that will want bits for themselves, too. What exactly was the issue with
>> having a FOURCC capability bit in the upper byte?
>
> That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> in the MSB)
> and more bits for tradiditional nonstd users.

The only disadvantage I can see is that it requires adding this bit in the 
application and stripping it when interpreting it. But I think the 24 lower bits 
should be enough for driver specific behavior (as the current values really can 
only be interpreted per driver).

> BTW, after giving it some more thought: what does the FB_CAP_FOURCC buys
> us? It's not like all drivers will support whatever random FOURCC mode
> you give them,
> so you have to check whether it's supported by doing a set_var first.

Because any solution purely based on the nonstd field is insane. The abuse of 
that field is just too widespread. Drivers that use nonstd usually only check 
whether it is zero or nonzero and others will just interpret parts of nonstd and 
ignore the rest. They will happily accept FOURCC values in the nonstd and just 
doing the wrong thing. Even if we would fix those the problems applications will 
run into with older kernels will remain.


Greetings,

Florian Tobias Schandinat

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-06-24 19:45                     ` Florian Tobias Schandinat
@ 2011-07-11 15:32                         ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-11 15:32 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, Paul Mundt, linux-fbdev, linux-media, dri-devel

On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes
> >>>>>> must be non-zero), I don't think there are any conflicts with
> >>>>>> existing values of
> >>>>>> nonstd. To make it even safer and easier to parse, you could set bit
> >>>>>> 31 of
> >>>>>> nonstd as a FOURCC indicator.
> >>>>> 
> >>>>> I would then create a union between nonstd and fourcc, and document
> >>>>> nonstd as
> >>>>> being used for the legacy API only. Most existing drivers use a
> >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the most
> >>>>> is pxafb and uses
> >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
> >>>>> 0xff000000 != 0 could be used as a FOURCC mode test.
> >>>>> 
> >>>>> This assumes that FOURCCs will never have their last character set to
> >>>>> '\0'. Is
> >>>>> that a safe assumption for the future ?
> >>>> 
> >>>> Yes, I think. The information I found indicates that space should be
> >>>> used for padding, so a \0 shouldn't exist.
> >>>> I think using only the nonstd field and requiring applications to
> >>>> check the capabilities would be possible, although not fool proof ;)
> >>> 
> >>> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC
> >>> if any of them is set.
> >>> 
> >>> Nicely backwards compatible, as sane drivers should reject nonstd
> >>> values they don't support (apps _will_ start filling in FOURCC values
> >>> ignoring capabilities, won't they?).
> >> 
> >> That seems like a reasonable case, but if we're going to do that then
> >> certainly the nonstd bit encoding needs to be documented and treated as
> >> a hard ABI.
> >> 
> >> I'm not so sure about the if any bit in the upper byte is set assume
> >> FOURCC case though, there will presumably be other users in the future
> >> that will want bits for themselves, too. What exactly was the issue with
> >> having a FOURCC capability bit in the upper byte?
> > 
> > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > in the MSB) and more bits for tradiditional nonstd users.
> 
> The only disadvantage I can see is that it requires adding this bit in the
> application and stripping it when interpreting it. But I think the 24 lower
> bits should be enough for driver specific behavior (as the current values
> really can only be interpreted per driver).

I'm also not keen on adding/stripping the MSB. It would be easier for 
applications to use FOURCCs directly.

> > BTW, after giving it some more thought: what does the FB_CAP_FOURCC buys
> > us? It's not like all drivers will support whatever random FOURCC mode
> > you give them, so you have to check whether it's supported by doing a
> > set_var first.
> 
> Because any solution purely based on the nonstd field is insane. The abuse
> of that field is just too widespread. Drivers that use nonstd usually only
> check whether it is zero or nonzero and others will just interpret parts
> of nonstd and ignore the rest. They will happily accept FOURCC values in
> the nonstd and just doing the wrong thing. Even if we would fix those the
> problems applications will run into with older kernels will remain.

I agree with Florian here. Many drivers currently check whether nonstd != 0. 
Who knows what kind of values applications feed them ?

I'd like to reach an agreement on the API, and implement it. I'm fine with 
either grayscale or nonstd to store the FOURCC (with a slight preference for 
grayscale), and with either a vmode flag or using the most significant byte of 
the grayscale/nonstd field to detect FOURCC mode. I believe FB_CAP_FOURCC (or 
something similar) is needed.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-07-11 15:32                         ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-11 15:32 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, Paul Mundt, linux-fbdev, linux-media, dri-devel

On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes
> >>>>>> must be non-zero), I don't think there are any conflicts with
> >>>>>> existing values of
> >>>>>> nonstd. To make it even safer and easier to parse, you could set bit
> >>>>>> 31 of
> >>>>>> nonstd as a FOURCC indicator.
> >>>>> 
> >>>>> I would then create a union between nonstd and fourcc, and document
> >>>>> nonstd as
> >>>>> being used for the legacy API only. Most existing drivers use a
> >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the most
> >>>>> is pxafb and uses
> >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
> >>>>> 0xff000000 != 0 could be used as a FOURCC mode test.
> >>>>> 
> >>>>> This assumes that FOURCCs will never have their last character set to
> >>>>> '\0'. Is
> >>>>> that a safe assumption for the future ?
> >>>> 
> >>>> Yes, I think. The information I found indicates that space should be
> >>>> used for padding, so a \0 shouldn't exist.
> >>>> I think using only the nonstd field and requiring applications to
> >>>> check the capabilities would be possible, although not fool proof ;)
> >>> 
> >>> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC
> >>> if any of them is set.
> >>> 
> >>> Nicely backwards compatible, as sane drivers should reject nonstd
> >>> values they don't support (apps _will_ start filling in FOURCC values
> >>> ignoring capabilities, won't they?).
> >> 
> >> That seems like a reasonable case, but if we're going to do that then
> >> certainly the nonstd bit encoding needs to be documented and treated as
> >> a hard ABI.
> >> 
> >> I'm not so sure about the if any bit in the upper byte is set assume
> >> FOURCC case though, there will presumably be other users in the future
> >> that will want bits for themselves, too. What exactly was the issue with
> >> having a FOURCC capability bit in the upper byte?
> > 
> > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > in the MSB) and more bits for tradiditional nonstd users.
> 
> The only disadvantage I can see is that it requires adding this bit in the
> application and stripping it when interpreting it. But I think the 24 lower
> bits should be enough for driver specific behavior (as the current values
> really can only be interpreted per driver).

I'm also not keen on adding/stripping the MSB. It would be easier for 
applications to use FOURCCs directly.

> > BTW, after giving it some more thought: what does the FB_CAP_FOURCC buys
> > us? It's not like all drivers will support whatever random FOURCC mode
> > you give them, so you have to check whether it's supported by doing a
> > set_var first.
> 
> Because any solution purely based on the nonstd field is insane. The abuse
> of that field is just too widespread. Drivers that use nonstd usually only
> check whether it is zero or nonzero and others will just interpret parts
> of nonstd and ignore the rest. They will happily accept FOURCC values in
> the nonstd and just doing the wrong thing. Even if we would fix those the
> problems applications will run into with older kernels will remain.

I agree with Florian here. Many drivers currently check whether nonstd != 0. 
Who knows what kind of values applications feed them ?

I'd like to reach an agreement on the API, and implement it. I'm fine with 
either grayscale or nonstd to store the FOURCC (with a slight preference for 
grayscale), and with either a vmode flag or using the most significant byte of 
the grayscale/nonstd field to detect FOURCC mode. I believe FB_CAP_FOURCC (or 
something similar) is needed.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-11 15:32                         ` Laurent Pinchart
@ 2011-07-25 10:30                           ` Laurent Pinchart
  -1 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-25 10:30 UTC (permalink / raw)
  To: dri-devel
  Cc: Florian Tobias Schandinat, linux-fbdev, Paul Mundt,
	Geert Uytterhoeven, linux-media

Ping ?

As stated in my previous mail, I'd like to reach an agreement on the API, and 
implement it. I'm fine with either grayscale or nonstd to store the FOURCC 
(with a slight preference for grayscale), and with either a vmode flag or 
using the most significant byte of the grayscale/nonstd field to detect FOURCC 
mode. I believe FB_CAP_FOURCC (or something similar) is needed.

Paul, Geert, Florian, what are your opinions ?

On Monday 11 July 2011 17:32:51 Laurent Pinchart wrote:
> On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> > On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> > >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> > >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> > >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> > >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes
> > >>>>>> must be non-zero), I don't think there are any conflicts with
> > >>>>>> existing values of
> > >>>>>> nonstd. To make it even safer and easier to parse, you could set
> > >>>>>> bit 31 of
> > >>>>>> nonstd as a FOURCC indicator.
> > >>>>> 
> > >>>>> I would then create a union between nonstd and fourcc, and document
> > >>>>> nonstd as
> > >>>>> being used for the legacy API only. Most existing drivers use a
> > >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the
> > >>>>> most is pxafb and uses
> > >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so
> > >>>>> nonstd& 0xff000000 != 0 could be used as a FOURCC mode test.
> > >>>>> 
> > >>>>> This assumes that FOURCCs will never have their last character set
> > >>>>> to '\0'. Is
> > >>>>> that a safe assumption for the future ?
> > >>>> 
> > >>>> Yes, I think. The information I found indicates that space should be
> > >>>> used for padding, so a \0 shouldn't exist.
> > >>>> I think using only the nonstd field and requiring applications to
> > >>>> check the capabilities would be possible, although not fool proof ;)
> > >>> 
> > >>> So we can declare the 8 msb bits of nonstd reserved, and assume
> > >>> FOURCC if any of them is set.
> > >>> 
> > >>> Nicely backwards compatible, as sane drivers should reject nonstd
> > >>> values they don't support (apps _will_ start filling in FOURCC values
> > >>> ignoring capabilities, won't they?).
> > >> 
> > >> That seems like a reasonable case, but if we're going to do that then
> > >> certainly the nonstd bit encoding needs to be documented and treated
> > >> as a hard ABI.
> > >> 
> > >> I'm not so sure about the if any bit in the upper byte is set assume
> > >> FOURCC case though, there will presumably be other users in the future
> > >> that will want bits for themselves, too. What exactly was the issue
> > >> with having a FOURCC capability bit in the upper byte?
> > > 
> > > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > > in the MSB) and more bits for tradiditional nonstd users.
> > 
> > The only disadvantage I can see is that it requires adding this bit in
> > the application and stripping it when interpreting it. But I think the
> > 24 lower bits should be enough for driver specific behavior (as the
> > current values really can only be interpreted per driver).
> 
> I'm also not keen on adding/stripping the MSB. It would be easier for
> applications to use FOURCCs directly.
> 
> > > BTW, after giving it some more thought: what does the FB_CAP_FOURCC
> > > buys us? It's not like all drivers will support whatever random FOURCC
> > > mode you give them, so you have to check whether it's supported by
> > > doing a set_var first.
> > 
> > Because any solution purely based on the nonstd field is insane. The
> > abuse of that field is just too widespread. Drivers that use nonstd
> > usually only check whether it is zero or nonzero and others will just
> > interpret parts of nonstd and ignore the rest. They will happily accept
> > FOURCC values in the nonstd and just doing the wrong thing. Even if we
> > would fix those the problems applications will run into with older
> > kernels will remain.
> 
> I agree with Florian here. Many drivers currently check whether nonstd !=
> 0. Who knows what kind of values applications feed them ?
> 
> I'd like to reach an agreement on the API, and implement it. I'm fine with
> either grayscale or nonstd to store the FOURCC (with a slight preference
> for grayscale), and with either a vmode flag or using the most significant
> byte of the grayscale/nonstd field to detect FOURCC mode. I believe
> FB_CAP_FOURCC (or something similar) is needed.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-07-25 10:30                           ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-25 10:30 UTC (permalink / raw)
  To: dri-devel
  Cc: Florian Tobias Schandinat, linux-fbdev, Paul Mundt,
	Geert Uytterhoeven, linux-media

Ping ?

As stated in my previous mail, I'd like to reach an agreement on the API, and 
implement it. I'm fine with either grayscale or nonstd to store the FOURCC 
(with a slight preference for grayscale), and with either a vmode flag or 
using the most significant byte of the grayscale/nonstd field to detect FOURCC 
mode. I believe FB_CAP_FOURCC (or something similar) is needed.

Paul, Geert, Florian, what are your opinions ?

On Monday 11 July 2011 17:32:51 Laurent Pinchart wrote:
> On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> > On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> > >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> > >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> > >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> > >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes
> > >>>>>> must be non-zero), I don't think there are any conflicts with
> > >>>>>> existing values of
> > >>>>>> nonstd. To make it even safer and easier to parse, you could set
> > >>>>>> bit 31 of
> > >>>>>> nonstd as a FOURCC indicator.
> > >>>>> 
> > >>>>> I would then create a union between nonstd and fourcc, and document
> > >>>>> nonstd as
> > >>>>> being used for the legacy API only. Most existing drivers use a
> > >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the
> > >>>>> most is pxafb and uses
> > >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so
> > >>>>> nonstd& 0xff000000 != 0 could be used as a FOURCC mode test.
> > >>>>> 
> > >>>>> This assumes that FOURCCs will never have their last character set
> > >>>>> to '\0'. Is
> > >>>>> that a safe assumption for the future ?
> > >>>> 
> > >>>> Yes, I think. The information I found indicates that space should be
> > >>>> used for padding, so a \0 shouldn't exist.
> > >>>> I think using only the nonstd field and requiring applications to
> > >>>> check the capabilities would be possible, although not fool proof ;)
> > >>> 
> > >>> So we can declare the 8 msb bits of nonstd reserved, and assume
> > >>> FOURCC if any of them is set.
> > >>> 
> > >>> Nicely backwards compatible, as sane drivers should reject nonstd
> > >>> values they don't support (apps _will_ start filling in FOURCC values
> > >>> ignoring capabilities, won't they?).
> > >> 
> > >> That seems like a reasonable case, but if we're going to do that then
> > >> certainly the nonstd bit encoding needs to be documented and treated
> > >> as a hard ABI.
> > >> 
> > >> I'm not so sure about the if any bit in the upper byte is set assume
> > >> FOURCC case though, there will presumably be other users in the future
> > >> that will want bits for themselves, too. What exactly was the issue
> > >> with having a FOURCC capability bit in the upper byte?
> > > 
> > > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > > in the MSB) and more bits for tradiditional nonstd users.
> > 
> > The only disadvantage I can see is that it requires adding this bit in
> > the application and stripping it when interpreting it. But I think the
> > 24 lower bits should be enough for driver specific behavior (as the
> > current values really can only be interpreted per driver).
> 
> I'm also not keen on adding/stripping the MSB. It would be easier for
> applications to use FOURCCs directly.
> 
> > > BTW, after giving it some more thought: what does the FB_CAP_FOURCC
> > > buys us? It's not like all drivers will support whatever random FOURCC
> > > mode you give them, so you have to check whether it's supported by
> > > doing a set_var first.
> > 
> > Because any solution purely based on the nonstd field is insane. The
> > abuse of that field is just too widespread. Drivers that use nonstd
> > usually only check whether it is zero or nonzero and others will just
> > interpret parts of nonstd and ignore the rest. They will happily accept
> > FOURCC values in the nonstd and just doing the wrong thing. Even if we
> > would fix those the problems applications will run into with older
> > kernels will remain.
> 
> I agree with Florian here. Many drivers currently check whether nonstd !> 0. Who knows what kind of values applications feed them ?
> 
> I'd like to reach an agreement on the API, and implement it. I'm fine with
> either grayscale or nonstd to store the FOURCC (with a slight preference
> for grayscale), and with either a vmode flag or using the most significant
> byte of the grayscale/nonstd field to detect FOURCC mode. I believe
> FB_CAP_FOURCC (or something similar) is needed.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-11 15:32                         ` Laurent Pinchart
@ 2011-07-28  8:31                           ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 63+ messages in thread
From: Guennadi Liakhovetski @ 2011-07-28  8:31 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Florian Tobias Schandinat, Geert Uytterhoeven, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

On Mon, 11 Jul 2011, Laurent Pinchart wrote:

> On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> > On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> > >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> > >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> > >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> > >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes
> > >>>>>> must be non-zero), I don't think there are any conflicts with
> > >>>>>> existing values of
> > >>>>>> nonstd. To make it even safer and easier to parse, you could set bit
> > >>>>>> 31 of
> > >>>>>> nonstd as a FOURCC indicator.
> > >>>>> 
> > >>>>> I would then create a union between nonstd and fourcc, and document
> > >>>>> nonstd as
> > >>>>> being used for the legacy API only. Most existing drivers use a
> > >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the most
> > >>>>> is pxafb and uses
> > >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
> > >>>>> 0xff000000 != 0 could be used as a FOURCC mode test.
> > >>>>> 
> > >>>>> This assumes that FOURCCs will never have their last character set to
> > >>>>> '\0'. Is
> > >>>>> that a safe assumption for the future ?
> > >>>> 
> > >>>> Yes, I think. The information I found indicates that space should be
> > >>>> used for padding, so a \0 shouldn't exist.
> > >>>> I think using only the nonstd field and requiring applications to
> > >>>> check the capabilities would be possible, although not fool proof ;)
> > >>> 
> > >>> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC
> > >>> if any of them is set.
> > >>> 
> > >>> Nicely backwards compatible, as sane drivers should reject nonstd
> > >>> values they don't support (apps _will_ start filling in FOURCC values
> > >>> ignoring capabilities, won't they?).
> > >> 
> > >> That seems like a reasonable case, but if we're going to do that then
> > >> certainly the nonstd bit encoding needs to be documented and treated as
> > >> a hard ABI.
> > >> 
> > >> I'm not so sure about the if any bit in the upper byte is set assume
> > >> FOURCC case though, there will presumably be other users in the future
> > >> that will want bits for themselves, too. What exactly was the issue with
> > >> having a FOURCC capability bit in the upper byte?
> > > 
> > > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > > in the MSB) and more bits for tradiditional nonstd users.
> > 
> > The only disadvantage I can see is that it requires adding this bit in the
> > application and stripping it when interpreting it. But I think the 24 lower
> > bits should be enough for driver specific behavior (as the current values
> > really can only be interpreted per driver).
> 
> I'm also not keen on adding/stripping the MSB. It would be easier for 
> applications to use FOURCCs directly.
> 
> > > BTW, after giving it some more thought: what does the FB_CAP_FOURCC buys
> > > us? It's not like all drivers will support whatever random FOURCC mode
> > > you give them, so you have to check whether it's supported by doing a
> > > set_var first.
> > 
> > Because any solution purely based on the nonstd field is insane. The abuse
> > of that field is just too widespread. Drivers that use nonstd usually only
> > check whether it is zero or nonzero and others will just interpret parts
> > of nonstd and ignore the rest. They will happily accept FOURCC values in
> > the nonstd and just doing the wrong thing. Even if we would fix those the
> > problems applications will run into with older kernels will remain.
> 
> I agree with Florian here. Many drivers currently check whether nonstd != 0. 
> Who knows what kind of values applications feed them ?

FWIW, I prefer the original Laurent's proposal, with a slight uncertainty 
about whether we need the .capability field, or whether the try-and-check 
approach is sufficient. As for struct fb_var_screeninfo fields to support 
switching to a FOURCC mode, I also prefer an explicit dedicated flag to 
specify switching to it. Even though using FOURCC doesn't fit under the 
notion of a videomode, using one of .vmode bits is too tempting, so, I 
would actually take the plunge and use FB_VMODE_FOURCC.

As for the actual location of the fourcc code, I would leave .nonstd 
alone: who knows, maybe drivers will need both, whereas using grayscale 
and fourcc certainly doesn't make any sense. And I personally don't see a 
problem with using a union: buggy applications are, well, buggy... 
Actually, since in FOURCC mode we don't need any of

	__u32 bits_per_pixel;		/* guess what			*/
	__u32 grayscale;		/* != 0 Graylevels instead of colors */

	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
	struct fb_bitfield green;	/* else only length is significant */
	struct fb_bitfield blue;
	struct fb_bitfield transp;	/* transparency			*/	

so, we could put them all in the union for the case, if we need anything 
else for the fourcc configuration in the future.

> I'd like to reach an agreement on the API, and implement it. I'm fine with 
> either grayscale or nonstd to store the FOURCC (with a slight preference for 
> grayscale), and with either a vmode flag or using the most significant byte of 
> the grayscale/nonstd field to detect FOURCC mode. I believe FB_CAP_FOURCC (or 
> something similar) is needed.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-07-28  8:31                           ` Guennadi Liakhovetski
  0 siblings, 0 replies; 63+ messages in thread
From: Guennadi Liakhovetski @ 2011-07-28  8:31 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Florian Tobias Schandinat, Geert Uytterhoeven, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

On Mon, 11 Jul 2011, Laurent Pinchart wrote:

> On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> > On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> > >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> > >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> > >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> > >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4 bytes
> > >>>>>> must be non-zero), I don't think there are any conflicts with
> > >>>>>> existing values of
> > >>>>>> nonstd. To make it even safer and easier to parse, you could set bit
> > >>>>>> 31 of
> > >>>>>> nonstd as a FOURCC indicator.
> > >>>>> 
> > >>>>> I would then create a union between nonstd and fourcc, and document
> > >>>>> nonstd as
> > >>>>> being used for the legacy API only. Most existing drivers use a
> > >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the most
> > >>>>> is pxafb and uses
> > >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so nonstd&
> > >>>>> 0xff000000 != 0 could be used as a FOURCC mode test.
> > >>>>> 
> > >>>>> This assumes that FOURCCs will never have their last character set to
> > >>>>> '\0'. Is
> > >>>>> that a safe assumption for the future ?
> > >>>> 
> > >>>> Yes, I think. The information I found indicates that space should be
> > >>>> used for padding, so a \0 shouldn't exist.
> > >>>> I think using only the nonstd field and requiring applications to
> > >>>> check the capabilities would be possible, although not fool proof ;)
> > >>> 
> > >>> So we can declare the 8 msb bits of nonstd reserved, and assume FOURCC
> > >>> if any of them is set.
> > >>> 
> > >>> Nicely backwards compatible, as sane drivers should reject nonstd
> > >>> values they don't support (apps _will_ start filling in FOURCC values
> > >>> ignoring capabilities, won't they?).
> > >> 
> > >> That seems like a reasonable case, but if we're going to do that then
> > >> certainly the nonstd bit encoding needs to be documented and treated as
> > >> a hard ABI.
> > >> 
> > >> I'm not so sure about the if any bit in the upper byte is set assume
> > >> FOURCC case though, there will presumably be other users in the future
> > >> that will want bits for themselves, too. What exactly was the issue with
> > >> having a FOURCC capability bit in the upper byte?
> > > 
> > > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > > in the MSB) and more bits for tradiditional nonstd users.
> > 
> > The only disadvantage I can see is that it requires adding this bit in the
> > application and stripping it when interpreting it. But I think the 24 lower
> > bits should be enough for driver specific behavior (as the current values
> > really can only be interpreted per driver).
> 
> I'm also not keen on adding/stripping the MSB. It would be easier for 
> applications to use FOURCCs directly.
> 
> > > BTW, after giving it some more thought: what does the FB_CAP_FOURCC buys
> > > us? It's not like all drivers will support whatever random FOURCC mode
> > > you give them, so you have to check whether it's supported by doing a
> > > set_var first.
> > 
> > Because any solution purely based on the nonstd field is insane. The abuse
> > of that field is just too widespread. Drivers that use nonstd usually only
> > check whether it is zero or nonzero and others will just interpret parts
> > of nonstd and ignore the rest. They will happily accept FOURCC values in
> > the nonstd and just doing the wrong thing. Even if we would fix those the
> > problems applications will run into with older kernels will remain.
> 
> I agree with Florian here. Many drivers currently check whether nonstd != 0. 
> Who knows what kind of values applications feed them ?

FWIW, I prefer the original Laurent's proposal, with a slight uncertainty 
about whether we need the .capability field, or whether the try-and-check 
approach is sufficient. As for struct fb_var_screeninfo fields to support 
switching to a FOURCC mode, I also prefer an explicit dedicated flag to 
specify switching to it. Even though using FOURCC doesn't fit under the 
notion of a videomode, using one of .vmode bits is too tempting, so, I 
would actually take the plunge and use FB_VMODE_FOURCC.

As for the actual location of the fourcc code, I would leave .nonstd 
alone: who knows, maybe drivers will need both, whereas using grayscale 
and fourcc certainly doesn't make any sense. And I personally don't see a 
problem with using a union: buggy applications are, well, buggy... 
Actually, since in FOURCC mode we don't need any of

	__u32 bits_per_pixel;		/* guess what			*/
	__u32 grayscale;		/* != 0 Graylevels instead of colors */

	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
	struct fb_bitfield green;	/* else only length is significant */
	struct fb_bitfield blue;
	struct fb_bitfield transp;	/* transparency			*/	

so, we could put them all in the union for the case, if we need anything 
else for the fourcc configuration in the future.

> I'd like to reach an agreement on the API, and implement it. I'm fine with 
> either grayscale or nonstd to store the FOURCC (with a slight preference for 
> grayscale), and with either a vmode flag or using the most significant byte of 
> the grayscale/nonstd field to detect FOURCC mode. I believe FB_CAP_FOURCC (or 
> something similar) is needed.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-28  8:31                           ` Guennadi Liakhovetski
@ 2011-07-28 10:51                             ` Laurent Pinchart
  -1 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-28 10:51 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Florian Tobias Schandinat, Geert Uytterhoeven, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Guennadi,

Thanks for the review.

On Thursday 28 July 2011 10:31:24 Guennadi Liakhovetski wrote:
> On Mon, 11 Jul 2011, Laurent Pinchart wrote:
> > On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> > > On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > > > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> > > >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> > > >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> > > >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > > >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> > > >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4
> > > >>>>>> bytes must be non-zero), I don't think there are any conflicts
> > > >>>>>> with existing values of
> > > >>>>>> nonstd. To make it even safer and easier to parse, you could set
> > > >>>>>> bit 31 of
> > > >>>>>> nonstd as a FOURCC indicator.
> > > >>>>> 
> > > >>>>> I would then create a union between nonstd and fourcc, and
> > > >>>>> document nonstd as
> > > >>>>> being used for the legacy API only. Most existing drivers use a
> > > >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the
> > > >>>>> most is pxafb and uses
> > > >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so
> > > >>>>> nonstd& 0xff000000 != 0 could be used as a FOURCC mode test.
> > > >>>>> 
> > > >>>>> This assumes that FOURCCs will never have their last character
> > > >>>>> set to '\0'. Is
> > > >>>>> that a safe assumption for the future ?
> > > >>>> 
> > > >>>> Yes, I think. The information I found indicates that space should
> > > >>>> be used for padding, so a \0 shouldn't exist.
> > > >>>> I think using only the nonstd field and requiring applications to
> > > >>>> check the capabilities would be possible, although not fool proof
> > > >>>> ;)
> > > >>> 
> > > >>> So we can declare the 8 msb bits of nonstd reserved, and assume
> > > >>> FOURCC if any of them is set.
> > > >>> 
> > > >>> Nicely backwards compatible, as sane drivers should reject nonstd
> > > >>> values they don't support (apps _will_ start filling in FOURCC
> > > >>> values ignoring capabilities, won't they?).
> > > >> 
> > > >> That seems like a reasonable case, but if we're going to do that
> > > >> then certainly the nonstd bit encoding needs to be documented and
> > > >> treated as a hard ABI.
> > > >> 
> > > >> I'm not so sure about the if any bit in the upper byte is set assume
> > > >> FOURCC case though, there will presumably be other users in the
> > > >> future that will want bits for themselves, too. What exactly was
> > > >> the issue with having a FOURCC capability bit in the upper byte?
> > > > 
> > > > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > > > in the MSB) and more bits for tradiditional nonstd users.
> > > 
> > > The only disadvantage I can see is that it requires adding this bit in
> > > the application and stripping it when interpreting it. But I think the
> > > 24 lower bits should be enough for driver specific behavior (as the
> > > current values really can only be interpreted per driver).
> > 
> > I'm also not keen on adding/stripping the MSB. It would be easier for
> > applications to use FOURCCs directly.
> > 
> > > > BTW, after giving it some more thought: what does the FB_CAP_FOURCC
> > > > buys us? It's not like all drivers will support whatever random
> > > > FOURCC mode you give them, so you have to check whether it's
> > > > supported by doing a set_var first.
> > > 
> > > Because any solution purely based on the nonstd field is insane. The
> > > abuse of that field is just too widespread. Drivers that use nonstd
> > > usually only check whether it is zero or nonzero and others will just
> > > interpret parts of nonstd and ignore the rest. They will happily
> > > accept FOURCC values in the nonstd and just doing the wrong thing.
> > > Even if we would fix those the problems applications will run into
> > > with older kernels will remain.
> > 
> > I agree with Florian here. Many drivers currently check whether nonstd !=
> > 0. Who knows what kind of values applications feed them ?
> 
> FWIW, I prefer the original Laurent's proposal, with a slight uncertainty
> about whether we need the .capability field, or whether the try-and-check
> approach is sufficient.

Try-and-check would be better, but unfortunately I don't think it would work. 
Existing drivers will either ignore the new FB_VMODE_FOURCC flag or fail. I 
expect many drivers that ignore it to still accept it blindly, and not clear 
it, so applications won't be able to find out whether the flag is supported.

> As for struct fb_var_screeninfo fields to support switching to a FOURCC
> mode, I also prefer an explicit dedicated flag to specify switching to it.
> Even though using FOURCC doesn't fit under the notion of a videomode, using
> one of .vmode bits is too tempting, so, I would actually take the plunge and
> use FB_VMODE_FOURCC.

Another option would be to consider any grayscale > 1 value as a FOURCC. I've 
briefly checked the in-tree drivers: they only assign grayscale with 0 or 1, 
and check whether grayscale is 0 or different than 0. If a userspace 
application only sets grayscale > 1 when talking to a driver that supports the 
FOURCC-based API, we could get rid of the flag.

What can't be easily found out is whether existing applications set grayscale 
to a > 1 value. They would break when used with FOURCC-aware drivers if we 
consider any grayscale > 1 value as a FOURCC. Is that a risk we can take ?

> As for the actual location of the fourcc code, I would leave .nonstd
> alone: who knows, maybe drivers will need both, whereas using grayscale
> and fourcc certainly doesn't make any sense. And I personally don't see a
> problem with using a union: buggy applications are, well, buggy...

I agree with this.

> Actually, since in FOURCC mode we don't need any of
> 
> 	__u32 bits_per_pixel;		/* guess what			*/
> 	__u32 grayscale;		/* != 0 Graylevels instead of colors */
> 
> 	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
> 	struct fb_bitfield green;	/* else only length is significant */
> 	struct fb_bitfield blue;
> 	struct fb_bitfield transp;	/* transparency			*/
> 
> so, we could put them all in the union for the case, if we need anything
> else for the fourcc configuration in the future.

Good point. It might make sense to exclude bits_per_pixel from the union 
though, as that's interesting information for applications. The red, green, 
blue and transp fields are less useful.

> > I'd like to reach an agreement on the API, and implement it. I'm fine
> > with either grayscale or nonstd to store the FOURCC (with a slight
> > preference for grayscale), and with either a vmode flag or using the
> > most significant byte of the grayscale/nonstd field to detect FOURCC
> > mode. I believe FB_CAP_FOURCC (or something similar) is needed.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-07-28 10:51                             ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-28 10:51 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Florian Tobias Schandinat, Geert Uytterhoeven, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Guennadi,

Thanks for the review.

On Thursday 28 July 2011 10:31:24 Guennadi Liakhovetski wrote:
> On Mon, 11 Jul 2011, Laurent Pinchart wrote:
> > On Friday 24 June 2011 21:45:57 Florian Tobias Schandinat wrote:
> > > On 06/24/2011 06:55 PM, Geert Uytterhoeven wrote:
> > > > On Fri, Jun 24, 2011 at 08:19, Paul Mundt wrote:
> > > >> On Thu, Jun 23, 2011 at 06:08:03PM +0200, Geert Uytterhoeven wrote:
> > > >>> On Wed, Jun 22, 2011 at 07:45, Florian Tobias Schandinat wrote:
> > > >>>> On 06/21/2011 10:31 PM, Laurent Pinchart wrote:
> > > >>>>> On Tuesday 21 June 2011 22:49:14 Geert Uytterhoeven wrote:
> > > >>>>>> As FOURCC values are always 4 ASCII characters (hence all 4
> > > >>>>>> bytes must be non-zero), I don't think there are any conflicts
> > > >>>>>> with existing values of
> > > >>>>>> nonstd. To make it even safer and easier to parse, you could set
> > > >>>>>> bit 31 of
> > > >>>>>> nonstd as a FOURCC indicator.
> > > >>>>> 
> > > >>>>> I would then create a union between nonstd and fourcc, and
> > > >>>>> document nonstd as
> > > >>>>> being used for the legacy API only. Most existing drivers use a
> > > >>>>> couple of nonstd bits only. The driver that (ab)uses nonstd the
> > > >>>>> most is pxafb and uses
> > > >>>>> bits 22:0. Bits 31:24 are never used as far as I can tell, so
> > > >>>>> nonstd& 0xff000000 != 0 could be used as a FOURCC mode test.
> > > >>>>> 
> > > >>>>> This assumes that FOURCCs will never have their last character
> > > >>>>> set to '\0'. Is
> > > >>>>> that a safe assumption for the future ?
> > > >>>> 
> > > >>>> Yes, I think. The information I found indicates that space should
> > > >>>> be used for padding, so a \0 shouldn't exist.
> > > >>>> I think using only the nonstd field and requiring applications to
> > > >>>> check the capabilities would be possible, although not fool proof
> > > >>>> ;)
> > > >>> 
> > > >>> So we can declare the 8 msb bits of nonstd reserved, and assume
> > > >>> FOURCC if any of them is set.
> > > >>> 
> > > >>> Nicely backwards compatible, as sane drivers should reject nonstd
> > > >>> values they don't support (apps _will_ start filling in FOURCC
> > > >>> values ignoring capabilities, won't they?).
> > > >> 
> > > >> That seems like a reasonable case, but if we're going to do that
> > > >> then certainly the nonstd bit encoding needs to be documented and
> > > >> treated as a hard ABI.
> > > >> 
> > > >> I'm not so sure about the if any bit in the upper byte is set assume
> > > >> FOURCC case though, there will presumably be other users in the
> > > >> future that will want bits for themselves, too. What exactly was
> > > >> the issue with having a FOURCC capability bit in the upper byte?
> > > > 
> > > > That indeed gives less issues (as long as you don't stuff 8-bit ASCII
> > > > in the MSB) and more bits for tradiditional nonstd users.
> > > 
> > > The only disadvantage I can see is that it requires adding this bit in
> > > the application and stripping it when interpreting it. But I think the
> > > 24 lower bits should be enough for driver specific behavior (as the
> > > current values really can only be interpreted per driver).
> > 
> > I'm also not keen on adding/stripping the MSB. It would be easier for
> > applications to use FOURCCs directly.
> > 
> > > > BTW, after giving it some more thought: what does the FB_CAP_FOURCC
> > > > buys us? It's not like all drivers will support whatever random
> > > > FOURCC mode you give them, so you have to check whether it's
> > > > supported by doing a set_var first.
> > > 
> > > Because any solution purely based on the nonstd field is insane. The
> > > abuse of that field is just too widespread. Drivers that use nonstd
> > > usually only check whether it is zero or nonzero and others will just
> > > interpret parts of nonstd and ignore the rest. They will happily
> > > accept FOURCC values in the nonstd and just doing the wrong thing.
> > > Even if we would fix those the problems applications will run into
> > > with older kernels will remain.
> > 
> > I agree with Florian here. Many drivers currently check whether nonstd !> > 0. Who knows what kind of values applications feed them ?
> 
> FWIW, I prefer the original Laurent's proposal, with a slight uncertainty
> about whether we need the .capability field, or whether the try-and-check
> approach is sufficient.

Try-and-check would be better, but unfortunately I don't think it would work. 
Existing drivers will either ignore the new FB_VMODE_FOURCC flag or fail. I 
expect many drivers that ignore it to still accept it blindly, and not clear 
it, so applications won't be able to find out whether the flag is supported.

> As for struct fb_var_screeninfo fields to support switching to a FOURCC
> mode, I also prefer an explicit dedicated flag to specify switching to it.
> Even though using FOURCC doesn't fit under the notion of a videomode, using
> one of .vmode bits is too tempting, so, I would actually take the plunge and
> use FB_VMODE_FOURCC.

Another option would be to consider any grayscale > 1 value as a FOURCC. I've 
briefly checked the in-tree drivers: they only assign grayscale with 0 or 1, 
and check whether grayscale is 0 or different than 0. If a userspace 
application only sets grayscale > 1 when talking to a driver that supports the 
FOURCC-based API, we could get rid of the flag.

What can't be easily found out is whether existing applications set grayscale 
to a > 1 value. They would break when used with FOURCC-aware drivers if we 
consider any grayscale > 1 value as a FOURCC. Is that a risk we can take ?

> As for the actual location of the fourcc code, I would leave .nonstd
> alone: who knows, maybe drivers will need both, whereas using grayscale
> and fourcc certainly doesn't make any sense. And I personally don't see a
> problem with using a union: buggy applications are, well, buggy...

I agree with this.

> Actually, since in FOURCC mode we don't need any of
> 
> 	__u32 bits_per_pixel;		/* guess what			*/
> 	__u32 grayscale;		/* != 0 Graylevels instead of colors */
> 
> 	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
> 	struct fb_bitfield green;	/* else only length is significant */
> 	struct fb_bitfield blue;
> 	struct fb_bitfield transp;	/* transparency			*/
> 
> so, we could put them all in the union for the case, if we need anything
> else for the fourcc configuration in the future.

Good point. It might make sense to exclude bits_per_pixel from the union 
though, as that's interesting information for applications. The red, green, 
blue and transp fields are less useful.

> > I'd like to reach an agreement on the API, and implement it. I'm fine
> > with either grayscale or nonstd to store the FOURCC (with a slight
> > preference for grayscale), and with either a vmode flag or using the
> > most significant byte of the grayscale/nonstd field to detect FOURCC
> > mode. I believe FB_CAP_FOURCC (or something similar) is needed.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-28 10:51                             ` Laurent Pinchart
@ 2011-07-31 20:32                               ` Geert Uytterhoeven
  -1 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-07-31 20:32 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Guennadi Liakhovetski, Florian Tobias Schandinat, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>> mode, I also prefer an explicit dedicated flag to specify switching to it.
>> Even though using FOURCC doesn't fit under the notion of a videomode, using
>> one of .vmode bits is too tempting, so, I would actually take the plunge and
>> use FB_VMODE_FOURCC.
>
> Another option would be to consider any grayscale > 1 value as a FOURCC. I've
> briefly checked the in-tree drivers: they only assign grayscale with 0 or 1,
> and check whether grayscale is 0 or different than 0. If a userspace
> application only sets grayscale > 1 when talking to a driver that supports the
> FOURCC-based API, we could get rid of the flag.
>
> What can't be easily found out is whether existing applications set grayscale
> to a > 1 value. They would break when used with FOURCC-aware drivers if we
> consider any grayscale > 1 value as a FOURCC. Is that a risk we can take ?

I think we can. I'd expect applications to use either 1 or -1 (i.e.
all ones), both are
invalid FOURCC values.

Still, I prefer the nonstd way.
And limiting traditional nonstd values to the lowest 24 bits (there
are no in-tree
drivers using the highest 8 bits, right?).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-07-31 20:32                               ` Geert Uytterhoeven
  0 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-07-31 20:32 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Guennadi Liakhovetski, Florian Tobias Schandinat, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>> mode, I also prefer an explicit dedicated flag to specify switching to it.
>> Even though using FOURCC doesn't fit under the notion of a videomode, using
>> one of .vmode bits is too tempting, so, I would actually take the plunge and
>> use FB_VMODE_FOURCC.
>
> Another option would be to consider any grayscale > 1 value as a FOURCC. I've
> briefly checked the in-tree drivers: they only assign grayscale with 0 or 1,
> and check whether grayscale is 0 or different than 0. If a userspace
> application only sets grayscale > 1 when talking to a driver that supports the
> FOURCC-based API, we could get rid of the flag.
>
> What can't be easily found out is whether existing applications set grayscale
> to a > 1 value. They would break when used with FOURCC-aware drivers if we
> consider any grayscale > 1 value as a FOURCC. Is that a risk we can take ?

I think we can. I'd expect applications to use either 1 or -1 (i.e.
all ones), both are
invalid FOURCC values.

Still, I prefer the nonstd way.
And limiting traditional nonstd values to the lowest 24 bits (there
are no in-tree
drivers using the highest 8 bits, right?).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-31 20:32                               ` Geert Uytterhoeven
  (?)
@ 2011-07-31 22:54                               ` Florian Tobias Schandinat
  2011-07-31 23:28                                   ` Laurent Pinchart
  2011-08-01  9:49                                   ` Geert Uytterhoeven
  -1 siblings, 2 replies; 63+ messages in thread
From: Florian Tobias Schandinat @ 2011-07-31 22:54 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Paul Mundt, linux-fbdev,
	linux-media, dri-devel

On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>  wrote:
>>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>>> mode, I also prefer an explicit dedicated flag to specify switching to it.
>>> Even though using FOURCC doesn't fit under the notion of a videomode, using
>>> one of .vmode bits is too tempting, so, I would actually take the plunge and
>>> use FB_VMODE_FOURCC.
>>
>> Another option would be to consider any grayscale>  1 value as a FOURCC. I've
>> briefly checked the in-tree drivers: they only assign grayscale with 0 or 1,
>> and check whether grayscale is 0 or different than 0. If a userspace
>> application only sets grayscale>  1 when talking to a driver that supports the
>> FOURCC-based API, we could get rid of the flag.
>>
>> What can't be easily found out is whether existing applications set grayscale
>> to a>  1 value. They would break when used with FOURCC-aware drivers if we
>> consider any grayscale>  1 value as a FOURCC. Is that a risk we can take ?
>
> I think we can. I'd expect applications to use either 1 or -1 (i.e.
> all ones), both are
> invalid FOURCC values.
>
> Still, I prefer the nonstd way.
> And limiting traditional nonstd values to the lowest 24 bits (there
> are no in-tree
> drivers using the highest 8 bits, right?).

Okay, it would be okay for me to
- write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
- not having an explicit flag to enable FOURCC
- in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
- making support of FOURCC visible to userspace by capabilites |= FB_CAP_FOURCC

The capabilities is not strictly necessary but I think it's very useful as
- it allows applications to make sure the extension is supported (for example to 
adjust the UI)
- it allows applications to distinguish whether a particular format is not 
supported or FOURCC at all
- it allows signaling further extensions of the API
- it does not hurt, one line per driver and still some bytes in fixinfo free


So using it would look like this:
- the driver must have capabilities |= FB_CAP_FOURCC
- the application may check capabilities to know whether FOURCC is supported
- the application may write a raw FOURCC value in nonstd to request changing to 
FOURCC mode with this format
- when the driver switches to a FOURCC mode it must have visual = 
FB_VISUAL_FOURCC and the current FOURCC format in nonstd
- the application should check visual and nonstd to make sure it gets what it wanted


So if there are no strong objections against this I think we should implement it.
I do not really care whether we use a union or not but I think if we decide to 
have one it should cover all fields that are undefined/unused in FOURCC mode.


Hope we can find anything that everyone considers acceptable,

Florian Tobias Schandinat

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-31 22:54                               ` Florian Tobias Schandinat
@ 2011-07-31 23:28                                   ` Laurent Pinchart
  2011-08-01  9:49                                   ` Geert Uytterhoeven
  1 sibling, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-31 23:28 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Florian,

Thanks for the feedback.

On Monday 01 August 2011 00:54:48 Florian Tobias Schandinat wrote:
> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> > On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
> >>> mode, I also prefer an explicit dedicated flag to specify switching to
> >>> it. Even though using FOURCC doesn't fit under the notion of a
> >>> videomode, using one of .vmode bits is too tempting, so, I would
> >>> actually take the plunge and use FB_VMODE_FOURCC.
> >> 
> >> Another option would be to consider any grayscale>  1 value as a FOURCC.
> >> I've briefly checked the in-tree drivers: they only assign grayscale
> >> with 0 or 1, and check whether grayscale is 0 or different than 0. If a
> >> userspace application only sets grayscale>  1 when talking to a driver
> >> that supports the FOURCC-based API, we could get rid of the flag.
> >> 
> >> What can't be easily found out is whether existing applications set
> >> grayscale to a>  1 value. They would break when used with FOURCC-aware
> >> drivers if we consider any grayscale>  1 value as a FOURCC. Is that a
> >> risk we can take ?
> > 
> > I think we can. I'd expect applications to use either 1 or -1 (i.e.
> > all ones), both are
> > invalid FOURCC values.
> > 
> > Still, I prefer the nonstd way.
> > And limiting traditional nonstd values to the lowest 24 bits (there
> > are no in-tree
> > drivers using the highest 8 bits, right?).
> 
> Okay, it would be okay for me to
> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
> - not having an explicit flag to enable FOURCC
> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> - making support of FOURCC visible to userspace by capabilites |=
> FB_CAP_FOURCC
> 
> The capabilities is not strictly necessary but I think it's very useful as
> - it allows applications to make sure the extension is supported (for
> example to adjust the UI)
> - it allows applications to distinguish whether a particular format is not
> supported or FOURCC at all
> - it allows signaling further extensions of the API
> - it does not hurt, one line per driver and still some bytes in fixinfo
> free

Without a FOURCC capability applications will need to try FOURCCs blindly. 
Drivers that are not FOURCC aware would then risk interpreting the FOURCC as 
something else. As you mention below applications will need that check that 
visual == FB_VISUAL_FOURCC, so it's less of an issue than I initially thought, 
but it doesn't become a non-issue. The display might still show glitches.

> So using it would look like this:
> - the driver must have capabilities |= FB_CAP_FOURCC
> - the application may check capabilities to know whether FOURCC is
> supported - the application may write a raw FOURCC value in nonstd to
> request changing to FOURCC mode with this format
> - when the driver switches to a FOURCC mode it must have visual =
> FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> - the application should check visual and nonstd to make sure it gets what
> it wanted
> 
> 
> So if there are no strong objections against this I think we should
> implement it. I do not really care whether we use a union or not but I
> think if we decide to have one it should cover all fields that are
> undefined/unused in FOURCC mode.
> 
> 
> Hope we can find anything that everyone considers acceptable,

This sounds good to me, except that I would use the grayscale field instead of 
the nonstd field. nonstd has pretty weird usecases, while grayscale is better 
defined. nonstd might also make sense combined with FOURCC-based modes, while 
grayscale would be completely redundant.

What's your opinion on that ?

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-07-31 23:28                                   ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-31 23:28 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Florian,

Thanks for the feedback.

On Monday 01 August 2011 00:54:48 Florian Tobias Schandinat wrote:
> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> > On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
> >>> mode, I also prefer an explicit dedicated flag to specify switching to
> >>> it. Even though using FOURCC doesn't fit under the notion of a
> >>> videomode, using one of .vmode bits is too tempting, so, I would
> >>> actually take the plunge and use FB_VMODE_FOURCC.
> >> 
> >> Another option would be to consider any grayscale>  1 value as a FOURCC.
> >> I've briefly checked the in-tree drivers: they only assign grayscale
> >> with 0 or 1, and check whether grayscale is 0 or different than 0. If a
> >> userspace application only sets grayscale>  1 when talking to a driver
> >> that supports the FOURCC-based API, we could get rid of the flag.
> >> 
> >> What can't be easily found out is whether existing applications set
> >> grayscale to a>  1 value. They would break when used with FOURCC-aware
> >> drivers if we consider any grayscale>  1 value as a FOURCC. Is that a
> >> risk we can take ?
> > 
> > I think we can. I'd expect applications to use either 1 or -1 (i.e.
> > all ones), both are
> > invalid FOURCC values.
> > 
> > Still, I prefer the nonstd way.
> > And limiting traditional nonstd values to the lowest 24 bits (there
> > are no in-tree
> > drivers using the highest 8 bits, right?).
> 
> Okay, it would be okay for me to
> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
> - not having an explicit flag to enable FOURCC
> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> - making support of FOURCC visible to userspace by capabilites |> FB_CAP_FOURCC
> 
> The capabilities is not strictly necessary but I think it's very useful as
> - it allows applications to make sure the extension is supported (for
> example to adjust the UI)
> - it allows applications to distinguish whether a particular format is not
> supported or FOURCC at all
> - it allows signaling further extensions of the API
> - it does not hurt, one line per driver and still some bytes in fixinfo
> free

Without a FOURCC capability applications will need to try FOURCCs blindly. 
Drivers that are not FOURCC aware would then risk interpreting the FOURCC as 
something else. As you mention below applications will need that check that 
visual = FB_VISUAL_FOURCC, so it's less of an issue than I initially thought, 
but it doesn't become a non-issue. The display might still show glitches.

> So using it would look like this:
> - the driver must have capabilities |= FB_CAP_FOURCC
> - the application may check capabilities to know whether FOURCC is
> supported - the application may write a raw FOURCC value in nonstd to
> request changing to FOURCC mode with this format
> - when the driver switches to a FOURCC mode it must have visual > FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> - the application should check visual and nonstd to make sure it gets what
> it wanted
> 
> 
> So if there are no strong objections against this I think we should
> implement it. I do not really care whether we use a union or not but I
> think if we decide to have one it should cover all fields that are
> undefined/unused in FOURCC mode.
> 
> 
> Hope we can find anything that everyone considers acceptable,

This sounds good to me, except that I would use the grayscale field instead of 
the nonstd field. nonstd has pretty weird usecases, while grayscale is better 
defined. nonstd might also make sense combined with FOURCC-based modes, while 
grayscale would be completely redundant.

What's your opinion on that ?

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-31 20:32                               ` Geert Uytterhoeven
@ 2011-07-31 23:30                                 ` Laurent Pinchart
  -1 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-31 23:30 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Guennadi Liakhovetski, Florian Tobias Schandinat, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Geert,

Thanks for the feedback.

On Sunday 31 July 2011 22:32:42 Geert Uytterhoeven wrote:
> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >> As for struct fb_var_screeninfo fields to support switching to a FOURCC
> >> mode, I also prefer an explicit dedicated flag to specify switching to
> >> it. Even though using FOURCC doesn't fit under the notion of a
> >> videomode, using one of .vmode bits is too tempting, so, I would
> >> actually take the plunge and use FB_VMODE_FOURCC.
> > 
> > Another option would be to consider any grayscale > 1 value as a FOURCC.
> > I've briefly checked the in-tree drivers: they only assign grayscale
> > with 0 or 1, and check whether grayscale is 0 or different than 0. If a
> > userspace application only sets grayscale > 1 when talking to a driver
> > that supports the FOURCC-based API, we could get rid of the flag.
> > 
> > What can't be easily found out is whether existing applications set
> > grayscale to a > 1 value. They would break when used with FOURCC-aware
> > drivers if we consider any grayscale > 1 value as a FOURCC. Is that a
> > risk we can take ?
> 
> I think we can. I'd expect applications to use either 1 or -1 (i.e. all
> ones), both are invalid FOURCC values.

OK.

> Still, I prefer the nonstd way.
> And limiting traditional nonstd values to the lowest 24 bits (there
> are no in-tree drivers using the highest 8 bits, right?).

None that I've found. I still have a preference for the grayscale field 
though. As mentioned by Guennadi, the grayscale field would become redundant 
for FOURCC-based formats. It's then a good candidate, and would let drivers 
(and applications) do any crazy stuff they want with the nonstd field.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-07-31 23:30                                 ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-07-31 23:30 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Guennadi Liakhovetski, Florian Tobias Schandinat, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Geert,

Thanks for the feedback.

On Sunday 31 July 2011 22:32:42 Geert Uytterhoeven wrote:
> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >> As for struct fb_var_screeninfo fields to support switching to a FOURCC
> >> mode, I also prefer an explicit dedicated flag to specify switching to
> >> it. Even though using FOURCC doesn't fit under the notion of a
> >> videomode, using one of .vmode bits is too tempting, so, I would
> >> actually take the plunge and use FB_VMODE_FOURCC.
> > 
> > Another option would be to consider any grayscale > 1 value as a FOURCC.
> > I've briefly checked the in-tree drivers: they only assign grayscale
> > with 0 or 1, and check whether grayscale is 0 or different than 0. If a
> > userspace application only sets grayscale > 1 when talking to a driver
> > that supports the FOURCC-based API, we could get rid of the flag.
> > 
> > What can't be easily found out is whether existing applications set
> > grayscale to a > 1 value. They would break when used with FOURCC-aware
> > drivers if we consider any grayscale > 1 value as a FOURCC. Is that a
> > risk we can take ?
> 
> I think we can. I'd expect applications to use either 1 or -1 (i.e. all
> ones), both are invalid FOURCC values.

OK.

> Still, I prefer the nonstd way.
> And limiting traditional nonstd values to the lowest 24 bits (there
> are no in-tree drivers using the highest 8 bits, right?).

None that I've found. I still have a preference for the grayscale field 
though. As mentioned by Guennadi, the grayscale field would become redundant 
for FOURCC-based formats. It's then a good candidate, and would let drivers 
(and applications) do any crazy stuff they want with the nonstd field.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-31 23:28                                   ` Laurent Pinchart
@ 2011-07-31 23:58                                     ` Florian Tobias Schandinat
  -1 siblings, 0 replies; 63+ messages in thread
From: Florian Tobias Schandinat @ 2011-07-31 23:58 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Laurent,

On 07/31/2011 11:28 PM, Laurent Pinchart wrote:
> Hi Florian,
>
> Thanks for the feedback.
>
> On Monday 01 August 2011 00:54:48 Florian Tobias Schandinat wrote:
>> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
>>> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
>>>>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>>>>> mode, I also prefer an explicit dedicated flag to specify switching to
>>>>> it. Even though using FOURCC doesn't fit under the notion of a
>>>>> videomode, using one of .vmode bits is too tempting, so, I would
>>>>> actually take the plunge and use FB_VMODE_FOURCC.
>>>>
>>>> Another option would be to consider any grayscale>   1 value as a FOURCC.
>>>> I've briefly checked the in-tree drivers: they only assign grayscale
>>>> with 0 or 1, and check whether grayscale is 0 or different than 0. If a
>>>> userspace application only sets grayscale>   1 when talking to a driver
>>>> that supports the FOURCC-based API, we could get rid of the flag.
>>>>
>>>> What can't be easily found out is whether existing applications set
>>>> grayscale to a>   1 value. They would break when used with FOURCC-aware
>>>> drivers if we consider any grayscale>   1 value as a FOURCC. Is that a
>>>> risk we can take ?
>>>
>>> I think we can. I'd expect applications to use either 1 or -1 (i.e.
>>> all ones), both are
>>> invalid FOURCC values.
>>>
>>> Still, I prefer the nonstd way.
>>> And limiting traditional nonstd values to the lowest 24 bits (there
>>> are no in-tree
>>> drivers using the highest 8 bits, right?).
>>
>> Okay, it would be okay for me to
>> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
>> - not having an explicit flag to enable FOURCC
>> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
>> - making support of FOURCC visible to userspace by capabilites |=
>> FB_CAP_FOURCC
>>
>> The capabilities is not strictly necessary but I think it's very useful as
>> - it allows applications to make sure the extension is supported (for
>> example to adjust the UI)
>> - it allows applications to distinguish whether a particular format is not
>> supported or FOURCC at all
>> - it allows signaling further extensions of the API
>> - it does not hurt, one line per driver and still some bytes in fixinfo
>> free
>
> Without a FOURCC capability applications will need to try FOURCCs blindly.
> Drivers that are not FOURCC aware would then risk interpreting the FOURCC as
> something else. As you mention below applications will need that check that
> visual == FB_VISUAL_FOURCC, so it's less of an issue than I initially thought,
> but it doesn't become a non-issue. The display might still show glitches.

True.

>> So using it would look like this:
>> - the driver must have capabilities |= FB_CAP_FOURCC
>> - the application may check capabilities to know whether FOURCC is
>> supported - the application may write a raw FOURCC value in nonstd to
>> request changing to FOURCC mode with this format
>> - when the driver switches to a FOURCC mode it must have visual =
>> FB_VISUAL_FOURCC and the current FOURCC format in nonstd
>> - the application should check visual and nonstd to make sure it gets what
>> it wanted
>>
>>
>> So if there are no strong objections against this I think we should
>> implement it. I do not really care whether we use a union or not but I
>> think if we decide to have one it should cover all fields that are
>> undefined/unused in FOURCC mode.
>>
>>
>> Hope we can find anything that everyone considers acceptable,
>
> This sounds good to me, except that I would use the grayscale field instead of
> the nonstd field. nonstd has pretty weird usecases, while grayscale is better
> defined. nonstd might also make sense combined with FOURCC-based modes, while
> grayscale would be completely redundant.
>
> What's your opinion on that ?

I do not really care, either one would be okay for me.
You're right that nonstd is used for a lot of things and perhaps some of those 
should still be possible in FOURCC mode. On the other hand I think applications 
are more likely to pass random values to grayscale as its meaning seems globally 
accepted (in contrast to nonstd where the application needs to know the driver 
to get any use of it).
Perhaps you should also say that in FOURCC mode all unused pixel format fields 
should be set to 0 by the application and other values of those may get a 
meaning in later extensions or individual drivers.


Regards,

Florian Tobias Schandinat

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-07-31 23:58                                     ` Florian Tobias Schandinat
  0 siblings, 0 replies; 63+ messages in thread
From: Florian Tobias Schandinat @ 2011-07-31 23:58 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Laurent,

On 07/31/2011 11:28 PM, Laurent Pinchart wrote:
> Hi Florian,
>
> Thanks for the feedback.
>
> On Monday 01 August 2011 00:54:48 Florian Tobias Schandinat wrote:
>> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
>>> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
>>>>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>>>>> mode, I also prefer an explicit dedicated flag to specify switching to
>>>>> it. Even though using FOURCC doesn't fit under the notion of a
>>>>> videomode, using one of .vmode bits is too tempting, so, I would
>>>>> actually take the plunge and use FB_VMODE_FOURCC.
>>>>
>>>> Another option would be to consider any grayscale>   1 value as a FOURCC.
>>>> I've briefly checked the in-tree drivers: they only assign grayscale
>>>> with 0 or 1, and check whether grayscale is 0 or different than 0. If a
>>>> userspace application only sets grayscale>   1 when talking to a driver
>>>> that supports the FOURCC-based API, we could get rid of the flag.
>>>>
>>>> What can't be easily found out is whether existing applications set
>>>> grayscale to a>   1 value. They would break when used with FOURCC-aware
>>>> drivers if we consider any grayscale>   1 value as a FOURCC. Is that a
>>>> risk we can take ?
>>>
>>> I think we can. I'd expect applications to use either 1 or -1 (i.e.
>>> all ones), both are
>>> invalid FOURCC values.
>>>
>>> Still, I prefer the nonstd way.
>>> And limiting traditional nonstd values to the lowest 24 bits (there
>>> are no in-tree
>>> drivers using the highest 8 bits, right?).
>>
>> Okay, it would be okay for me to
>> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
>> - not having an explicit flag to enable FOURCC
>> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
>> - making support of FOURCC visible to userspace by capabilites |>> FB_CAP_FOURCC
>>
>> The capabilities is not strictly necessary but I think it's very useful as
>> - it allows applications to make sure the extension is supported (for
>> example to adjust the UI)
>> - it allows applications to distinguish whether a particular format is not
>> supported or FOURCC at all
>> - it allows signaling further extensions of the API
>> - it does not hurt, one line per driver and still some bytes in fixinfo
>> free
>
> Without a FOURCC capability applications will need to try FOURCCs blindly.
> Drivers that are not FOURCC aware would then risk interpreting the FOURCC as
> something else. As you mention below applications will need that check that
> visual = FB_VISUAL_FOURCC, so it's less of an issue than I initially thought,
> but it doesn't become a non-issue. The display might still show glitches.

True.

>> So using it would look like this:
>> - the driver must have capabilities |= FB_CAP_FOURCC
>> - the application may check capabilities to know whether FOURCC is
>> supported - the application may write a raw FOURCC value in nonstd to
>> request changing to FOURCC mode with this format
>> - when the driver switches to a FOURCC mode it must have visual >> FB_VISUAL_FOURCC and the current FOURCC format in nonstd
>> - the application should check visual and nonstd to make sure it gets what
>> it wanted
>>
>>
>> So if there are no strong objections against this I think we should
>> implement it. I do not really care whether we use a union or not but I
>> think if we decide to have one it should cover all fields that are
>> undefined/unused in FOURCC mode.
>>
>>
>> Hope we can find anything that everyone considers acceptable,
>
> This sounds good to me, except that I would use the grayscale field instead of
> the nonstd field. nonstd has pretty weird usecases, while grayscale is better
> defined. nonstd might also make sense combined with FOURCC-based modes, while
> grayscale would be completely redundant.
>
> What's your opinion on that ?

I do not really care, either one would be okay for me.
You're right that nonstd is used for a lot of things and perhaps some of those 
should still be possible in FOURCC mode. On the other hand I think applications 
are more likely to pass random values to grayscale as its meaning seems globally 
accepted (in contrast to nonstd where the application needs to know the driver 
to get any use of it).
Perhaps you should also say that in FOURCC mode all unused pixel format fields 
should be set to 0 by the application and other values of those may get a 
meaning in later extensions or individual drivers.


Regards,

Florian Tobias Schandinat

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-31 22:54                               ` Florian Tobias Schandinat
@ 2011-08-01  9:49                                   ` Geert Uytterhoeven
  2011-08-01  9:49                                   ` Geert Uytterhoeven
  1 sibling, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-08-01  9:49 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Paul Mundt, linux-fbdev,
	linux-media, dri-devel

On Mon, Aug 1, 2011 at 00:54, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
>> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart
>> <laurent.pinchart@ideasonboard.com>  wrote:
>>>>
>>>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>>>> mode, I also prefer an explicit dedicated flag to specify switching to
>>>> it.
>>>> Even though using FOURCC doesn't fit under the notion of a videomode,
>>>> using
>>>> one of .vmode bits is too tempting, so, I would actually take the plunge
>>>> and
>>>> use FB_VMODE_FOURCC.
>>>
>>> Another option would be to consider any grayscale>  1 value as a FOURCC.
>>> I've
>>> briefly checked the in-tree drivers: they only assign grayscale with 0 or
>>> 1,
>>> and check whether grayscale is 0 or different than 0. If a userspace
>>> application only sets grayscale>  1 when talking to a driver that
>>> supports the
>>> FOURCC-based API, we could get rid of the flag.
>>>
>>> What can't be easily found out is whether existing applications set
>>> grayscale
>>> to a>  1 value. They would break when used with FOURCC-aware drivers if
>>> we
>>> consider any grayscale>  1 value as a FOURCC. Is that a risk we can take
>>> ?
>>
>> I think we can. I'd expect applications to use either 1 or -1 (i.e.
>> all ones), both are
>> invalid FOURCC values.
>>
>> Still, I prefer the nonstd way.
>> And limiting traditional nonstd values to the lowest 24 bits (there
>> are no in-tree
>> drivers using the highest 8 bits, right?).
>
> Okay, it would be okay for me to
> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
> - not having an explicit flag to enable FOURCC
> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> - making support of FOURCC visible to userspace by capabilites |=
> FB_CAP_FOURCC
>
> The capabilities is not strictly necessary but I think it's very useful as
> - it allows applications to make sure the extension is supported (for
> example to adjust the UI)
> - it allows applications to distinguish whether a particular format is not
> supported or FOURCC at all
> - it allows signaling further extensions of the API
> - it does not hurt, one line per driver and still some bytes in fixinfo free
>
>
> So using it would look like this:
> - the driver must have capabilities |= FB_CAP_FOURCC
> - the application may check capabilities to know whether FOURCC is supported
> - the application may write a raw FOURCC value in nonstd to request changing
> to FOURCC mode with this format
> - when the driver switches to a FOURCC mode it must have visual =
> FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> - the application should check visual and nonstd to make sure it gets what
> it wanted

As several of the FOURCC formats duplicate formats you can already
specify in some
other way (e.g. the RGB and greyscale formats), and as FOURCC makes life easier
for the application writer, I'm wondering whether it makes sense to add FOURCC
support in the generic layer for drivers that don't support it? I.e.
the generic layer would
fill in fb_var_screeninfo depending on the requested FOURCC mode, if possible.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-08-01  9:49                                   ` Geert Uytterhoeven
  0 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-08-01  9:49 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Laurent Pinchart, Guennadi Liakhovetski, Paul Mundt, linux-fbdev,
	linux-media, dri-devel

On Mon, Aug 1, 2011 at 00:54, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
>> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart
>> <laurent.pinchart@ideasonboard.com>  wrote:
>>>>
>>>> As for struct fb_var_screeninfo fields to support switching to a FOURCC
>>>> mode, I also prefer an explicit dedicated flag to specify switching to
>>>> it.
>>>> Even though using FOURCC doesn't fit under the notion of a videomode,
>>>> using
>>>> one of .vmode bits is too tempting, so, I would actually take the plunge
>>>> and
>>>> use FB_VMODE_FOURCC.
>>>
>>> Another option would be to consider any grayscale>  1 value as a FOURCC.
>>> I've
>>> briefly checked the in-tree drivers: they only assign grayscale with 0 or
>>> 1,
>>> and check whether grayscale is 0 or different than 0. If a userspace
>>> application only sets grayscale>  1 when talking to a driver that
>>> supports the
>>> FOURCC-based API, we could get rid of the flag.
>>>
>>> What can't be easily found out is whether existing applications set
>>> grayscale
>>> to a>  1 value. They would break when used with FOURCC-aware drivers if
>>> we
>>> consider any grayscale>  1 value as a FOURCC. Is that a risk we can take
>>> ?
>>
>> I think we can. I'd expect applications to use either 1 or -1 (i.e.
>> all ones), both are
>> invalid FOURCC values.
>>
>> Still, I prefer the nonstd way.
>> And limiting traditional nonstd values to the lowest 24 bits (there
>> are no in-tree
>> drivers using the highest 8 bits, right?).
>
> Okay, it would be okay for me to
> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte != 0
> - not having an explicit flag to enable FOURCC
> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> - making support of FOURCC visible to userspace by capabilites |> FB_CAP_FOURCC
>
> The capabilities is not strictly necessary but I think it's very useful as
> - it allows applications to make sure the extension is supported (for
> example to adjust the UI)
> - it allows applications to distinguish whether a particular format is not
> supported or FOURCC at all
> - it allows signaling further extensions of the API
> - it does not hurt, one line per driver and still some bytes in fixinfo free
>
>
> So using it would look like this:
> - the driver must have capabilities |= FB_CAP_FOURCC
> - the application may check capabilities to know whether FOURCC is supported
> - the application may write a raw FOURCC value in nonstd to request changing
> to FOURCC mode with this format
> - when the driver switches to a FOURCC mode it must have visual > FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> - the application should check visual and nonstd to make sure it gets what
> it wanted

As several of the FOURCC formats duplicate formats you can already
specify in some
other way (e.g. the RGB and greyscale formats), and as FOURCC makes life easier
for the application writer, I'm wondering whether it makes sense to add FOURCC
support in the generic layer for drivers that don't support it? I.e.
the generic layer would
fill in fb_var_screeninfo depending on the requested FOURCC mode, if possible.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-07-31 23:58                                     ` Florian Tobias Schandinat
@ 2011-08-01 14:11                                       ` Laurent Pinchart
  -1 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-08-01 14:11 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Florian,

On Monday 01 August 2011 01:58:27 Florian Tobias Schandinat wrote:
> On 07/31/2011 11:28 PM, Laurent Pinchart wrote:
> > On Monday 01 August 2011 00:54:48 Florian Tobias Schandinat wrote:
> >> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> >>> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >>>>> As for struct fb_var_screeninfo fields to support switching to a
> >>>>> FOURCC mode, I also prefer an explicit dedicated flag to specify
> >>>>> switching to it. Even though using FOURCC doesn't fit under the
> >>>>> notion of a videomode, using one of .vmode bits is too tempting, so,
> >>>>> I would actually take the plunge and use FB_VMODE_FOURCC.
> >>>> 
> >>>> Another option would be to consider any grayscale>   1 value as a
> >>>> FOURCC. I've briefly checked the in-tree drivers: they only assign
> >>>> grayscale with 0 or 1, and check whether grayscale is 0 or different
> >>>> than 0. If a userspace application only sets grayscale>   1 when
> >>>> talking to a driver that supports the FOURCC-based API, we could get
> >>>> rid of the flag.
> >>>> 
> >>>> What can't be easily found out is whether existing applications set
> >>>> grayscale to a>   1 value. They would break when used with
> >>>> FOURCC-aware drivers if we consider any grayscale>   1 value as a
> >>>> FOURCC. Is that a risk we can take ?
> >>> 
> >>> I think we can. I'd expect applications to use either 1 or -1 (i.e.
> >>> all ones), both are
> >>> invalid FOURCC values.
> >>> 
> >>> Still, I prefer the nonstd way.
> >>> And limiting traditional nonstd values to the lowest 24 bits (there
> >>> are no in-tree
> >>> drivers using the highest 8 bits, right?).
> >> 
> >> Okay, it would be okay for me to
> >> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte !=
> >> 0 - not having an explicit flag to enable FOURCC
> >> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> >> - making support of FOURCC visible to userspace by capabilites |=
> >> FB_CAP_FOURCC
> >> 
> >> The capabilities is not strictly necessary but I think it's very useful
> >> as - it allows applications to make sure the extension is supported
> >> (for example to adjust the UI)
> >> - it allows applications to distinguish whether a particular format is
> >> not supported or FOURCC at all
> >> - it allows signaling further extensions of the API
> >> - it does not hurt, one line per driver and still some bytes in fixinfo
> >> free
> > 
> > Without a FOURCC capability applications will need to try FOURCCs
> > blindly. Drivers that are not FOURCC aware would then risk interpreting
> > the FOURCC as something else. As you mention below applications will
> > need that check that visual == FB_VISUAL_FOURCC, so it's less of an
> > issue than I initially thought, but it doesn't become a non-issue. The
> > display might still show glitches.
> 
> True.
> 
> >> So using it would look like this:
> >> - the driver must have capabilities |= FB_CAP_FOURCC
> >> - the application may check capabilities to know whether FOURCC is
> >> supported - the application may write a raw FOURCC value in nonstd to
> >> request changing to FOURCC mode with this format
> >> - when the driver switches to a FOURCC mode it must have visual =
> >> FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> >> - the application should check visual and nonstd to make sure it gets
> >> what it wanted
> >> 
> >> 
> >> So if there are no strong objections against this I think we should
> >> implement it. I do not really care whether we use a union or not but I
> >> think if we decide to have one it should cover all fields that are
> >> undefined/unused in FOURCC mode.
> >> 
> >> 
> >> Hope we can find anything that everyone considers acceptable,
> > 
> > This sounds good to me, except that I would use the grayscale field
> > instead of the nonstd field. nonstd has pretty weird usecases, while
> > grayscale is better defined. nonstd might also make sense combined with
> > FOURCC-based modes, while grayscale would be completely redundant.
> > 
> > What's your opinion on that ?
> 
> I do not really care, either one would be okay for me.
> You're right that nonstd is used for a lot of things and perhaps some of
> those should still be possible in FOURCC mode. On the other hand I think
> applications are more likely to pass random values to grayscale as its
> meaning seems globally accepted (in contrast to nonstd where the
> application needs to know the driver to get any use of it).
> Perhaps you should also say that in FOURCC mode all unused pixel format
> fields should be set to 0 by the application and other values of those may
> get a meaning in later extensions or individual drivers.

Good point. I'll add that to the documentation.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-08-01 14:11                                       ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-08-01 14:11 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Geert Uytterhoeven, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Florian,

On Monday 01 August 2011 01:58:27 Florian Tobias Schandinat wrote:
> On 07/31/2011 11:28 PM, Laurent Pinchart wrote:
> > On Monday 01 August 2011 00:54:48 Florian Tobias Schandinat wrote:
> >> On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> >>> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >>>>> As for struct fb_var_screeninfo fields to support switching to a
> >>>>> FOURCC mode, I also prefer an explicit dedicated flag to specify
> >>>>> switching to it. Even though using FOURCC doesn't fit under the
> >>>>> notion of a videomode, using one of .vmode bits is too tempting, so,
> >>>>> I would actually take the plunge and use FB_VMODE_FOURCC.
> >>>> 
> >>>> Another option would be to consider any grayscale>   1 value as a
> >>>> FOURCC. I've briefly checked the in-tree drivers: they only assign
> >>>> grayscale with 0 or 1, and check whether grayscale is 0 or different
> >>>> than 0. If a userspace application only sets grayscale>   1 when
> >>>> talking to a driver that supports the FOURCC-based API, we could get
> >>>> rid of the flag.
> >>>> 
> >>>> What can't be easily found out is whether existing applications set
> >>>> grayscale to a>   1 value. They would break when used with
> >>>> FOURCC-aware drivers if we consider any grayscale>   1 value as a
> >>>> FOURCC. Is that a risk we can take ?
> >>> 
> >>> I think we can. I'd expect applications to use either 1 or -1 (i.e.
> >>> all ones), both are
> >>> invalid FOURCC values.
> >>> 
> >>> Still, I prefer the nonstd way.
> >>> And limiting traditional nonstd values to the lowest 24 bits (there
> >>> are no in-tree
> >>> drivers using the highest 8 bits, right?).
> >> 
> >> Okay, it would be okay for me to
> >> - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte !> >> 0 - not having an explicit flag to enable FOURCC
> >> - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> >> - making support of FOURCC visible to userspace by capabilites |> >> FB_CAP_FOURCC
> >> 
> >> The capabilities is not strictly necessary but I think it's very useful
> >> as - it allows applications to make sure the extension is supported
> >> (for example to adjust the UI)
> >> - it allows applications to distinguish whether a particular format is
> >> not supported or FOURCC at all
> >> - it allows signaling further extensions of the API
> >> - it does not hurt, one line per driver and still some bytes in fixinfo
> >> free
> > 
> > Without a FOURCC capability applications will need to try FOURCCs
> > blindly. Drivers that are not FOURCC aware would then risk interpreting
> > the FOURCC as something else. As you mention below applications will
> > need that check that visual = FB_VISUAL_FOURCC, so it's less of an
> > issue than I initially thought, but it doesn't become a non-issue. The
> > display might still show glitches.
> 
> True.
> 
> >> So using it would look like this:
> >> - the driver must have capabilities |= FB_CAP_FOURCC
> >> - the application may check capabilities to know whether FOURCC is
> >> supported - the application may write a raw FOURCC value in nonstd to
> >> request changing to FOURCC mode with this format
> >> - when the driver switches to a FOURCC mode it must have visual > >> FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> >> - the application should check visual and nonstd to make sure it gets
> >> what it wanted
> >> 
> >> 
> >> So if there are no strong objections against this I think we should
> >> implement it. I do not really care whether we use a union or not but I
> >> think if we decide to have one it should cover all fields that are
> >> undefined/unused in FOURCC mode.
> >> 
> >> 
> >> Hope we can find anything that everyone considers acceptable,
> > 
> > This sounds good to me, except that I would use the grayscale field
> > instead of the nonstd field. nonstd has pretty weird usecases, while
> > grayscale is better defined. nonstd might also make sense combined with
> > FOURCC-based modes, while grayscale would be completely redundant.
> > 
> > What's your opinion on that ?
> 
> I do not really care, either one would be okay for me.
> You're right that nonstd is used for a lot of things and perhaps some of
> those should still be possible in FOURCC mode. On the other hand I think
> applications are more likely to pass random values to grayscale as its
> meaning seems globally accepted (in contrast to nonstd where the
> application needs to know the driver to get any use of it).
> Perhaps you should also say that in FOURCC mode all unused pixel format
> fields should be set to 0 by the application and other values of those may
> get a meaning in later extensions or individual drivers.

Good point. I'll add that to the documentation.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-08-01  9:49                                   ` Geert Uytterhoeven
@ 2011-08-11 17:19                                     ` Laurent Pinchart
  -1 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-08-11 17:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Florian Tobias Schandinat, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Geert,

On Monday 01 August 2011 11:49:46 Geert Uytterhoeven wrote:
> On Mon, Aug 1, 2011 at 00:54, Florian Tobias Schandinat wrote:
> > On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> >> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >>>> As for struct fb_var_screeninfo fields to support switching to a
> >>>> FOURCC mode, I also prefer an explicit dedicated flag to specify
> >>>> switching to it.
> >>>> Even though using FOURCC doesn't fit under the notion of a videomode,
> >>>> using
> >>>> one of .vmode bits is too tempting, so, I would actually take the
> >>>> plunge and
> >>>> use FB_VMODE_FOURCC.
> >>> 
> >>> Another option would be to consider any grayscale>  1 value as a
> >>> FOURCC. I've
> >>> briefly checked the in-tree drivers: they only assign grayscale with 0
> >>> or 1,
> >>> and check whether grayscale is 0 or different than 0. If a userspace
> >>> application only sets grayscale>  1 when talking to a driver that
> >>> supports the
> >>> FOURCC-based API, we could get rid of the flag.
> >>> 
> >>> What can't be easily found out is whether existing applications set
> >>> grayscale
> >>> to a>  1 value. They would break when used with FOURCC-aware drivers if
> >>> we
> >>> consider any grayscale>  1 value as a FOURCC. Is that a risk we can
> >>> take ?
> >> 
> >> I think we can. I'd expect applications to use either 1 or -1 (i.e.
> >> all ones), both are
> >> invalid FOURCC values.
> >> 
> >> Still, I prefer the nonstd way.
> >> And limiting traditional nonstd values to the lowest 24 bits (there
> >> are no in-tree
> >> drivers using the highest 8 bits, right?).
> > 
> > Okay, it would be okay for me to
> > - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte !=
> > 0 - not having an explicit flag to enable FOURCC
> > - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> > - making support of FOURCC visible to userspace by capabilites |=
> > FB_CAP_FOURCC
> > 
> > The capabilities is not strictly necessary but I think it's very useful
> > as - it allows applications to make sure the extension is supported (for
> > example to adjust the UI)
> > - it allows applications to distinguish whether a particular format is
> > not supported or FOURCC at all
> > - it allows signaling further extensions of the API
> > - it does not hurt, one line per driver and still some bytes in fixinfo
> > free
> > 
> > 
> > So using it would look like this:
> > - the driver must have capabilities |= FB_CAP_FOURCC
> > - the application may check capabilities to know whether FOURCC is
> > supported - the application may write a raw FOURCC value in nonstd to
> > request changing to FOURCC mode with this format
> > - when the driver switches to a FOURCC mode it must have visual =
> > FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> > - the application should check visual and nonstd to make sure it gets
> > what it wanted
> 
> As several of the FOURCC formats duplicate formats you can already specify
> in some other way (e.g. the RGB and greyscale formats), and as FOURCC makes
> life easier for the application writer, I'm wondering whether it makes sense
> to add FOURCC support in the generic layer for drivers that don't support
> it? I.e. the generic layer would fill in fb_var_screeninfo depending on the
> requested FOURCC mode, if possible.

That's a good idea, but I'd like to add that in a second step. I'm working on 
a proof-of-concept by porting a driver to the FOURCC-based API first.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-08-11 17:19                                     ` Laurent Pinchart
  0 siblings, 0 replies; 63+ messages in thread
From: Laurent Pinchart @ 2011-08-11 17:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Florian Tobias Schandinat, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi Geert,

On Monday 01 August 2011 11:49:46 Geert Uytterhoeven wrote:
> On Mon, Aug 1, 2011 at 00:54, Florian Tobias Schandinat wrote:
> > On 07/31/2011 08:32 PM, Geert Uytterhoeven wrote:
> >> On Thu, Jul 28, 2011 at 12:51, Laurent Pinchart wrote:
> >>>> As for struct fb_var_screeninfo fields to support switching to a
> >>>> FOURCC mode, I also prefer an explicit dedicated flag to specify
> >>>> switching to it.
> >>>> Even though using FOURCC doesn't fit under the notion of a videomode,
> >>>> using
> >>>> one of .vmode bits is too tempting, so, I would actually take the
> >>>> plunge and
> >>>> use FB_VMODE_FOURCC.
> >>> 
> >>> Another option would be to consider any grayscale>  1 value as a
> >>> FOURCC. I've
> >>> briefly checked the in-tree drivers: they only assign grayscale with 0
> >>> or 1,
> >>> and check whether grayscale is 0 or different than 0. If a userspace
> >>> application only sets grayscale>  1 when talking to a driver that
> >>> supports the
> >>> FOURCC-based API, we could get rid of the flag.
> >>> 
> >>> What can't be easily found out is whether existing applications set
> >>> grayscale
> >>> to a>  1 value. They would break when used with FOURCC-aware drivers if
> >>> we
> >>> consider any grayscale>  1 value as a FOURCC. Is that a risk we can
> >>> take ?
> >> 
> >> I think we can. I'd expect applications to use either 1 or -1 (i.e.
> >> all ones), both are
> >> invalid FOURCC values.
> >> 
> >> Still, I prefer the nonstd way.
> >> And limiting traditional nonstd values to the lowest 24 bits (there
> >> are no in-tree
> >> drivers using the highest 8 bits, right?).
> > 
> > Okay, it would be okay for me to
> > - write raw FOURCC values in nonstd, enable FOURCC mode if upper byte !> > 0 - not having an explicit flag to enable FOURCC
> > - in FOURCC mode drivers must set visual to FB_VISUAL_FOURCC
> > - making support of FOURCC visible to userspace by capabilites |> > FB_CAP_FOURCC
> > 
> > The capabilities is not strictly necessary but I think it's very useful
> > as - it allows applications to make sure the extension is supported (for
> > example to adjust the UI)
> > - it allows applications to distinguish whether a particular format is
> > not supported or FOURCC at all
> > - it allows signaling further extensions of the API
> > - it does not hurt, one line per driver and still some bytes in fixinfo
> > free
> > 
> > 
> > So using it would look like this:
> > - the driver must have capabilities |= FB_CAP_FOURCC
> > - the application may check capabilities to know whether FOURCC is
> > supported - the application may write a raw FOURCC value in nonstd to
> > request changing to FOURCC mode with this format
> > - when the driver switches to a FOURCC mode it must have visual > > FB_VISUAL_FOURCC and the current FOURCC format in nonstd
> > - the application should check visual and nonstd to make sure it gets
> > what it wanted
> 
> As several of the FOURCC formats duplicate formats you can already specify
> in some other way (e.g. the RGB and greyscale formats), and as FOURCC makes
> life easier for the application writer, I'm wondering whether it makes sense
> to add FOURCC support in the generic layer for drivers that don't support
> it? I.e. the generic layer would fill in fb_var_screeninfo depending on the
> requested FOURCC mode, if possible.

That's a good idea, but I'd like to add that in a second step. I'm working on 
a proof-of-concept by porting a driver to the FOURCC-based API first.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
  2011-08-11 17:19                                     ` Laurent Pinchart
@ 2011-08-13  9:42                                       ` Geert Uytterhoeven
  -1 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-08-13  9:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Florian Tobias Schandinat, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi LAurent,

On Thu, Aug 11, 2011 at 19:19, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 01 August 2011 11:49:46 Geert Uytterhoeven wrote:
>> As several of the FOURCC formats duplicate formats you can already specify
>> in some other way (e.g. the RGB and greyscale formats), and as FOURCC makes
>> life easier for the application writer, I'm wondering whether it makes sense
>> to add FOURCC support in the generic layer for drivers that don't support
>> it? I.e. the generic layer would fill in fb_var_screeninfo depending on the
>> requested FOURCC mode, if possible.
>
> That's a good idea, but I'd like to add that in a second step. I'm working on
> a proof-of-concept by porting a driver to the FOURCC-based API first.

Sure! I was just mentioning it, so we keep it in the back of our head and don't
make decisions now that would make it impossible to add that later.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH/RFC] fbdev: Add FOURCC-based format configuration API
@ 2011-08-13  9:42                                       ` Geert Uytterhoeven
  0 siblings, 0 replies; 63+ messages in thread
From: Geert Uytterhoeven @ 2011-08-13  9:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Florian Tobias Schandinat, Guennadi Liakhovetski, Paul Mundt,
	linux-fbdev, linux-media, dri-devel

Hi LAurent,

On Thu, Aug 11, 2011 at 19:19, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 01 August 2011 11:49:46 Geert Uytterhoeven wrote:
>> As several of the FOURCC formats duplicate formats you can already specify
>> in some other way (e.g. the RGB and greyscale formats), and as FOURCC makes
>> life easier for the application writer, I'm wondering whether it makes sense
>> to add FOURCC support in the generic layer for drivers that don't support
>> it? I.e. the generic layer would fill in fb_var_screeninfo depending on the
>> requested FOURCC mode, if possible.
>
> That's a good idea, but I'd like to add that in a second step. I'm working on
> a proof-of-concept by porting a driver to the FOURCC-based API first.

Sure! I was just mentioning it, so we keep it in the back of our head and don't
make decisions now that would make it impossible to add that later.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

end of thread, other threads:[~2011-08-13  9:42 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-17 22:07 [RFC] Standardize YUV support in the fbdev API Laurent Pinchart
2011-05-17 22:07 ` Laurent Pinchart
2011-05-17 22:44 ` Felipe Contreras
2011-05-17 22:44   ` Felipe Contreras
2011-05-18  6:53   ` Hans Verkuil
2011-05-18  6:53     ` Hans Verkuil
2011-05-23 11:55     ` Marek Szyprowski
2011-05-23 11:55       ` Marek Szyprowski
2011-05-23 12:09       ` Hans Verkuil
2011-05-23 12:09         ` Hans Verkuil
2011-05-18  0:21 ` Andy Walls
2011-05-18  0:21   ` Andy Walls
2011-05-18  1:09   ` Andy Walls
2011-05-18  1:09     ` Andy Walls
2011-05-18  1:09     ` Andy Walls
2011-05-20 22:33 ` Florian Tobias Schandinat
2011-05-20 22:33   ` Florian Tobias Schandinat
2011-05-23 21:00   ` Laurent Pinchart
2011-05-23 21:00     ` Laurent Pinchart
2011-05-23 21:00     ` Laurent Pinchart
2011-05-23 22:56     ` Florian Tobias Schandinat
2011-05-23 22:56       ` Florian Tobias Schandinat
2011-06-21 15:36       ` [PATCH/RFC] fbdev: Add FOURCC-based format configuration API Laurent Pinchart
2011-06-21 15:36         ` Laurent Pinchart
2011-06-21 20:49         ` Geert Uytterhoeven
2011-06-21 20:49           ` Geert Uytterhoeven
2011-06-21 22:31           ` Laurent Pinchart
2011-06-21 22:31             ` Laurent Pinchart
2011-06-22  5:45             ` Florian Tobias Schandinat
2011-06-22  8:50               ` Laurent Pinchart
2011-06-22  8:50                 ` Laurent Pinchart
2011-06-23 16:08               ` Geert Uytterhoeven
2011-06-23 16:08                 ` Geert Uytterhoeven
2011-06-24  6:19                 ` Paul Mundt
2011-06-24  6:19                   ` Paul Mundt
2011-06-24 18:55                   ` Geert Uytterhoeven
2011-06-24 18:55                     ` Geert Uytterhoeven
2011-06-24 19:45                     ` Florian Tobias Schandinat
2011-07-11 15:32                       ` Laurent Pinchart
2011-07-11 15:32                         ` Laurent Pinchart
2011-07-25 10:30                         ` Laurent Pinchart
2011-07-25 10:30                           ` Laurent Pinchart
2011-07-28  8:31                         ` Guennadi Liakhovetski
2011-07-28  8:31                           ` Guennadi Liakhovetski
2011-07-28 10:51                           ` Laurent Pinchart
2011-07-28 10:51                             ` Laurent Pinchart
2011-07-31 20:32                             ` Geert Uytterhoeven
2011-07-31 20:32                               ` Geert Uytterhoeven
2011-07-31 22:54                               ` Florian Tobias Schandinat
2011-07-31 23:28                                 ` Laurent Pinchart
2011-07-31 23:28                                   ` Laurent Pinchart
2011-07-31 23:58                                   ` Florian Tobias Schandinat
2011-07-31 23:58                                     ` Florian Tobias Schandinat
2011-08-01 14:11                                     ` Laurent Pinchart
2011-08-01 14:11                                       ` Laurent Pinchart
2011-08-01  9:49                                 ` Geert Uytterhoeven
2011-08-01  9:49                                   ` Geert Uytterhoeven
2011-08-11 17:19                                   ` Laurent Pinchart
2011-08-11 17:19                                     ` Laurent Pinchart
2011-08-13  9:42                                     ` Geert Uytterhoeven
2011-08-13  9:42                                       ` Geert Uytterhoeven
2011-07-31 23:30                               ` Laurent Pinchart
2011-07-31 23:30                                 ` 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.