[RFC] Standardize YUV support in the fbdev API

[Laurent Pinchart]

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

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

-- 
Regards,

Laurent Pinchart