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 at 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
