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:

quoted

Hi Laurent,

On 08/31/2011 11:18 AM, Laurent Pinchart wrote:

quoted

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]

quoted

quoted

@@ -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.

quoted

quoted

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

-- 
Regards,

Laurent Pinchart