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

[Daniel Vetter]

On Thu, Dec 18, 2014 at 10:54:14PM +0200, Laurent Pinchart wrote:
> 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 ;-)

I'm not a terrible big fan of driver flags, mostly because I've seen too
much of the horrible stuff in dri1. Imo much better to pass everything to
drivers and help them out with helpers if needed. I might be going
overboard a bit with my bias against driver flags ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch