[RFC] drm: add support for tiled/compressed/etc modifier in addfb2

[Laurent Pinchart]

Hi Daniel,

On Monday 15 December 2014 08:33:10 Daniel Vetter wrote:
> On Fri, Dec 12, 2014 at 10:56:53PM +0200, Laurent Pinchart wrote:
> > On Wednesday 10 December 2014 18:30:10 Daniel Vetter wrote:
> > > On Wed, Dec 10, 2014 at 12:17:51PM -0500, Rob Clark wrote:
> > > > In DRM/KMS we are lacking a good way to deal with tiled/compressed
> > > > formats.  Especially in the case of dmabuf/prime buffer sharing, where
> > > > we cannot always rely on under-the-hood flags passed to driver
> > > > specific gem-create ioctl to pass around these extra flags.
> > > > 
> > > > The proposal is to add a per-plane format modifier.  This allows to,
> > > > if necessary, use different tiling patters for sub-sampled planes,
> > > > etc. The format modifiers are added at the end of the ioctl struct, so
> > > > for legacy userspace it will be zero padded.
> > > > 
> > > > TODO how best to deal with assignment of modifier token values?  The
> > > > rough idea was to namespace things with an 8bit vendor-id, and then
> > > > beyond that it is treated as an opaque value.  But that was a
> > > > relatively arbitrary choice.  There are cases where same tiling
> > > > pattern and/or compression is supported by various different vendors. 
> > > > So we should standardize to use the vendor-id and value of the first
> > > > one who documents the format?
> > > 
> > > 8bits should be enough, will take a while until we have more than 250
> > > gpu drivers in the linux kernel ;-) I'm leaning a bit towards using
> > > 64bits though to make sure that there's enough space in the bitfiel to
> > > encode substrides and stuff like that, in case anyone needs it. For
> > > vendor ids I'd just go with first come and starting at 1 (i.e. rename
> > > yours). That way we make it clear that until a patch is merged upstream
> > > the id isn't reserved yet. drm-next should be sufficient as registry
> > > imo.
> > > 
> > > > TODO move definition of tokens to drm_fourcc.h?
> > > 
> > > Seems orthogonal imo. Another todo is to add checking to all drivers to
> > > reject it if it's not 0 with -EINVAL. Otherwise we have yet another case
> > > of an ioctl with fields that can't actually be used everywhere.
> > 
> > Could we please add the check in core code instead of drivers ?
> 
> Nope since then no driver could ever use that extension. Defeats the point
> ;-)

Except if we follow the proposal of adding a flag to tell whether a driver 
supports the extension ;-)

-- 
Regards,

Laurent Pinchart