Re: [PATCH v3 1/3] fbdev: Add FOURCC-based format configuration API

2011-11-28 Thread Laurent Pinchart
Hi Florian,

On Friday 25 November 2011 23:09:40 Florian Tobias Schandinat wrote:
 On 11/24/2011 10:50 AM, Laurent Pinchart wrote:
  Hi Florian,
  
  Gentle ping ?
 
 Sorry, but I'm very busy at the moment and therefore time-consuming things,
 like solving challenging problems, are delayed for some time.

No worries.

  On Sunday 20 November 2011 11:55:22 Laurent Pinchart wrote:
  On Sunday 20 November 2011 03:00:33 Florian Tobias Schandinat wrote:
  Hi Laurent,
  
  On 08/31/2011 11:18 AM, Laurent Pinchart wrote:
  This API will be used to support YUV frame buffer formats in a
  standard way.
  
  looks like the union is causing problems. With this patch applied I get
  
  errors like this:
CC [M]  drivers/auxdisplay/cfag12864bfb.o
  
  drivers/auxdisplay/cfag12864bfb.c:57: error: unknown field ‘red’
  specified in initializer
  
  *ouch*
  
  gcc  4.6 chokes on anonymous unions initializers :-/
  
  [snip]
  
  @@ -246,12 +251,23 @@ struct fb_var_screeninfo {
  
   __u32 yoffset;  /* resolution   
  */
   
   __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 
  */
  +union {
  +struct {/* Legacy format API
  */
  +__u32 grayscale; /* 0 = color, 1 = grayscale
  */
  +/* bitfields in fb mem if true color, else only 
  */
  +/* length is significant
  */
  +struct fb_bitfield red;
  +struct fb_bitfield green;
  +struct fb_bitfield blue;
  +struct fb_bitfield transp;  /* transparency 
  */
  +};
  +struct {/* FOURCC-based format API  
  */
  +__u32 fourcc;   /* FOURCC format
  */
  +__u32 colorspace;
  +__u32 reserved[11];
  +} fourcc;
  +};
  
  We can't name the union, otherwise this will change the userspace API.
  
  We could fix the problem on the kernel side with
  
  #ifdef __KERNEL__
  
 } color;
  
  #else
  
 };
  
  #endif
  
  (and the structure that contains the grayscale, red, green, blue and
  transp fields would need to be similarly named, the rgb name comes to
  mind)
 
 Which, I guess, would require modifying all drivers?

Unfortunately. That can be automated using coccinelle (I wrote a semantic 
patch for that), but it will still be around 10k lines of diff.

 I don't consider that a good idea. Maybe the simplest solution would be to
 drop the union idea and just accept an utterly misleading name grayscale
 for setting the FOURCC value.

I'll see if we can add an accessor macro to make it more explicit.

 The colorspace could use one of the reserved fields at the end or do you
 worry that we need to add a lot of other things?

For FOURCC-based format configuration I don't think we will need much more. If 
we do need lots of additional fields in the future we might have to consider 
an fbdev2 API ;-)

I'll resubmit patches based on this.

-- 
Regards,

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


Re: [PATCH v3 1/3] fbdev: Add FOURCC-based format configuration API

2011-11-25 Thread Florian Tobias Schandinat
Hi Laurent,

On 11/24/2011 10:50 AM, Laurent Pinchart wrote:
 Hi Florian,
 
 Gentle ping ?

Sorry, but I'm very busy at the moment and therefore time-consuming things, like
solving challenging problems, are delayed for some time.

 
 On Sunday 20 November 2011 11:55:22 Laurent Pinchart wrote:
 On Sunday 20 November 2011 03:00:33 Florian Tobias Schandinat wrote:
 Hi Laurent,

 On 08/31/2011 11:18 AM, Laurent Pinchart wrote:
 This API will be used to support YUV frame buffer formats in a standard
 way.

 looks like the union is causing problems. With this patch applied I get

 errors like this:
   CC [M]  drivers/auxdisplay/cfag12864bfb.o

 drivers/auxdisplay/cfag12864bfb.c:57: error: unknown field ‘red’
 specified in initializer

 *ouch*

 gcc  4.6 chokes on anonymous unions initializers :-/

 [snip]

 @@ -246,12 +251,23 @@ struct fb_var_screeninfo {

__u32 yoffset;  /* resolution   */

__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 */
 +  union {
 +  struct {/* Legacy format API*/
 +  __u32 grayscale; /* 0 = color, 1 = grayscale*/
 +  /* bitfields in fb mem if true color, else only */
 +  /* length is significant*/
 +  struct fb_bitfield red;
 +  struct fb_bitfield green;
 +  struct fb_bitfield blue;
 +  struct fb_bitfield transp;  /* transparency */
 +  };
 +  struct {/* FOURCC-based format API  */
 +  __u32 fourcc;   /* FOURCC format*/
 +  __u32 colorspace;
 +  __u32 reserved[11];
 +  } fourcc;
 +  };

 We can't name the union, otherwise this will change the userspace API.

 We could fix the problem on the kernel side with

 #ifdef __KERNEL__
  } color;
 #else
  };
 #endif
 
 (and the structure that contains the grayscale, red, green, blue and transp 
 fields would need to be similarly named, the rgb name comes to mind)

Which, I guess, would require modifying all drivers?
I don't consider that a good idea. Maybe the simplest solution would be to drop
the union idea and just accept an utterly misleading name grayscale for
setting the FOURCC value. The colorspace could use one of the reserved fields at
the end or do you worry that we need to add a lot of other things?


Best regards,

Florian Tobias Schandinat

 
 That's quite hackish though... What's your opinion ?

 It would also not handle userspace code that initializes an
 fb_var_screeninfo structure with named initializers, but that shouldn't
 happen, as application should read fb_var_screeninfo , modify it and write
 it back.

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

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


Re: [PATCH v3 1/3] fbdev: Add FOURCC-based format configuration API

2011-11-24 Thread Laurent Pinchart
Hi Florian,

Gentle ping ?

On Sunday 20 November 2011 11:55:22 Laurent Pinchart wrote:
 On Sunday 20 November 2011 03:00:33 Florian Tobias Schandinat wrote:
  Hi Laurent,
  
  On 08/31/2011 11:18 AM, Laurent Pinchart wrote:
   This API will be used to support YUV frame buffer formats in a standard
   way.
  
  looks like the union is causing problems. With this patch applied I get
  
  errors like this:
CC [M]  drivers/auxdisplay/cfag12864bfb.o
  
  drivers/auxdisplay/cfag12864bfb.c:57: error: unknown field ‘red’
  specified in initializer
 
 *ouch*
 
 gcc  4.6 chokes on anonymous unions initializers :-/
 
 [snip]
 
   @@ -246,12 +251,23 @@ struct fb_var_screeninfo {
   
 __u32 yoffset;  /* resolution   */
 
 __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 */
   + union {
   + struct {/* Legacy format API*/
   + __u32 grayscale; /* 0 = color, 1 = grayscale*/
   + /* bitfields in fb mem if true color, else only */
   + /* length is significant*/
   + struct fb_bitfield red;
   + struct fb_bitfield green;
   + struct fb_bitfield blue;
   + struct fb_bitfield transp;  /* transparency */
   + };
   + struct {/* FOURCC-based format API  */
   + __u32 fourcc;   /* FOURCC format*/
   + __u32 colorspace;
   + __u32 reserved[11];
   + } fourcc;
   + };
 
 We can't name the union, otherwise this will change the userspace API.
 
 We could fix the problem on the kernel side with
 
 #ifdef __KERNEL__
   } color;
 #else
   };
 #endif

(and the structure that contains the grayscale, red, green, blue and transp 
fields would need to be similarly named, the rgb name comes to mind)

 That's quite hackish though... What's your opinion ?
 
 It would also not handle userspace code that initializes an
 fb_var_screeninfo structure with named initializers, but that shouldn't
 happen, as application should read fb_var_screeninfo , modify it and write
 it back.
 
 __u32 nonstd;   /* != 0 Non standard pixel format */

-- 
Regards,

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


Re: [PATCH v3 1/3] fbdev: Add FOURCC-based format configuration API

2011-11-20 Thread Laurent Pinchart
Hi Florian,

On Sunday 20 November 2011 03:00:33 Florian Tobias Schandinat wrote:
 Hi Laurent,
 
 On 08/31/2011 11:18 AM, Laurent Pinchart wrote:
  This API will be used to support YUV frame buffer formats in a standard
  way.
 
 looks like the union is causing problems. With this patch applied I get
 errors like this:
 
   CC [M]  drivers/auxdisplay/cfag12864bfb.o
 drivers/auxdisplay/cfag12864bfb.c:57: error: unknown field ‘red’ specified
 in initializer

*ouch*

gcc  4.6 chokes on anonymous unions initializers :-/

[snip]

  @@ -246,12 +251,23 @@ struct fb_var_screeninfo {
  
  __u32 yoffset;  /* resolution   */
  
  __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 */
  +   union {
  +   struct {/* Legacy format API*/
  +   __u32 grayscale; /* 0 = color, 1 = grayscale*/
  +   /* bitfields in fb mem if true color, else only */
  +   /* length is significant*/
  +   struct fb_bitfield red;
  +   struct fb_bitfield green;
  +   struct fb_bitfield blue;
  +   struct fb_bitfield transp;  /* transparency */
  +   };
  +   struct {/* FOURCC-based format API  */
  +   __u32 fourcc;   /* FOURCC format*/
  +   __u32 colorspace;
  +   __u32 reserved[11];
  +   } fourcc;
  +   };

We can't name the union, otherwise this will change the userspace API.

We could fix the problem on the kernel side with

#ifdef __KERNEL__
} color;
#else
};
#endif

That's quite hackish though... What's your opinion ?

It would also not handle userspace code that initializes an fb_var_screeninfo 
structure with named initializers, but that shouldn't happen, as application 
should read fb_var_screeninfo , modify it and write it back.

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

-- 
Regards,

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


Re: [PATCH v3 1/3] fbdev: Add FOURCC-based format configuration API

2011-11-19 Thread Florian Tobias Schandinat
Hi Laurent,

On 08/31/2011 11:18 AM, Laurent Pinchart wrote:
 This API will be used to support YUV frame buffer formats in a standard
 way.

looks like the union is causing problems. With this patch applied I get errors
like this:

  CC [M]  drivers/auxdisplay/cfag12864bfb.o
drivers/auxdisplay/cfag12864bfb.c:57: error: unknown field ‘red’ specified in
initializer
drivers/auxdisplay/cfag12864bfb.c:57: warning: missing braces around initializer
drivers/auxdisplay/cfag12864bfb.c:57: warning: (near initialization for
‘cfag12864bfb_var.anonymous.anonymous’)
drivers/auxdisplay/cfag12864bfb.c:58: error: unknown field ‘green’ specified in
initializer
drivers/auxdisplay/cfag12864bfb.c:58: warning: braces around scalar initializer
drivers/auxdisplay/cfag12864bfb.c:58: warning: (near initialization for
‘cfag12864bfb_var.nonstd’)
drivers/auxdisplay/cfag12864bfb.c:58: warning: excess elements in scalar 
initializer
drivers/auxdisplay/cfag12864bfb.c:58: warning: (near initialization for
‘cfag12864bfb_var.nonstd’)
drivers/auxdisplay/cfag12864bfb.c:58: warning: excess elements in scalar 
initializer
drivers/auxdisplay/cfag12864bfb.c:58: warning: (near initialization for
‘cfag12864bfb_var.nonstd’)
drivers/auxdisplay/cfag12864bfb.c:59: error: unknown field ‘blue’ specified in
initializer
drivers/auxdisplay/cfag12864bfb.c:59: warning: braces around scalar initializer
drivers/auxdisplay/cfag12864bfb.c:59: warning: (near initialization for
‘cfag12864bfb_var.activate’)
drivers/auxdisplay/cfag12864bfb.c:59: warning: excess elements in scalar 
initializer
drivers/auxdisplay/cfag12864bfb.c:59: warning: (near initialization for
‘cfag12864bfb_var.activate’)
drivers/auxdisplay/cfag12864bfb.c:59: warning: excess elements in scalar 
initializer
drivers/auxdisplay/cfag12864bfb.c:59: warning: (near initialization for
‘cfag12864bfb_var.activate’)
make[2]: *** [drivers/auxdisplay/cfag12864bfb.o] Error 1

Any idea?


Best regards,

Florian Tobias Schandinat

 
 Last but not least, create a much needed fbdev API documentation and
 document the format setting APIs.
 
 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 ---
  Documentation/fb/api.txt |  317 
 ++
  include/linux/fb.h   |   28 +++-
  2 files changed, 339 insertions(+), 6 deletions(-)
  create mode 100644 Documentation/fb/api.txt
 
 diff --git a/Documentation/fb/api.txt b/Documentation/fb/api.txt
 new file mode 100644
 index 000..d842534
 --- /dev/null
 +++ b/Documentation/fb/api.txt
 @@ -0,0 +1,317 @@
 + 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.
 +
 +Visuals describe how color information is encoded and assembled to create
 +macropixels. Types describe how macropixels are stored in memory. The 
 following
 +types and visuals are supported.
 +
 +- FB_TYPE_PACKED_PIXELS
 +
 +Macropixels are stored contiguously in a single plane. If the number of bits
 +per macropixel is not a multiple of 8, whether macropixels are padded to the
 +next multiple of 8 bits or packed together into bytes depends on the visual.
 +
 +Padding at end of lines may be present and is then reported through the fixed
 +screen information line_length field.
 +
 +- FB_TYPE_PLANES
 +
 +Macropixels are split across multiple planes. The number of planes is 

[PATCH v3 1/3] fbdev: Add FOURCC-based format configuration API

2011-08-31 Thread Laurent Pinchart
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.pinch...@ideasonboard.com
---
 Documentation/fb/api.txt |  317 ++
 include/linux/fb.h   |   28 +++-
 2 files changed, 339 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/fb/api.txt

diff --git a/Documentation/fb/api.txt b/Documentation/fb/api.txt
new file mode 100644
index 000..d842534
--- /dev/null
+++ b/Documentation/fb/api.txt
@@ -0,0 +1,317 @@
+   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.
+
+Visuals describe how color information is encoded and assembled to create
+macropixels. Types describe how macropixels are stored in memory. The following
+types and visuals are supported.
+
+- FB_TYPE_PACKED_PIXELS
+
+Macropixels are stored contiguously in a single plane. If the number of bits
+per macropixel is not a multiple of 8, whether macropixels are padded to the
+next multiple of 8 bits or packed together into bytes depends on the visual.
+
+Padding at end of lines may be present and is then reported through the fixed
+screen information line_length field.
+
+- FB_TYPE_PLANES
+
+Macropixels are split across multiple planes. The number of planes is equal to
+the number of bits per macropixel, with plane i'th storing i'th bit from all
+macropixels.
+
+Planes are located contiguously in memory.
+
+- FB_TYPE_INTERLEAVED_PLANES
+
+Macropixels are split across multiple planes. The number of planes is equal to
+the number of bits per macropixel, with plane i'th storing i'th bit from all
+macropixels.
+
+Planes are interleaved in memory. The interleave factor, defined as the
+distance in bytes between the beginning of two consecutive interleaved blocks
+belonging to different planes, is stored in the fixed screen information
+type_aux field.
+
+- FB_TYPE_FOURCC
+
+Macropixels are stored in memory as described by the format FOURCC identifier
+stored in the variable screen information fourcc field.
+
+- FB_VISUAL_MONO01
+
+Pixels are black or white and stored on a number of bits (typically one)
+specified by the variable screen information bpp field.
+
+Black pixels are represented by all bits set to 1 and white pixels by all bits
+set to 0. When the number of bits per pixel is smaller than 8, several pixels
+are packed together in a byte.
+
+FB_VISUAL_MONO01 is currently used with FB_TYPE_PACKED_PIXELS only.
+
+- FB_VISUAL_MONO10
+
+Pixels are black or white and stored on a number of bits (typically one)
+specified by the variable screen information bpp field.
+
+Black pixels are represented by all bits set to 0 and white pixels by all bits
+set to 1. When the number of bits per pixel is smaller than 8, several pixels
+are packed together in a byte.
+
+FB_VISUAL_MONO01 is currently 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 a macropixel according to the variable screen
+information red, green, blue and transp fields.
+