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