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:

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

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

quoted

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

-- 
Regards,

Laurent Pinchart