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

From: Daniel Vetter <daniel@ffwll.ch>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [RFC] drm: add support for tiled/compressed/etc modifier in addfb2
Date: Mon, 15 Dec 2014 08:33:10 +0100	[thread overview]
Message-ID: <20141215073310.GL27182@phenom.ffwll.local> (raw)
In-Reply-To: <59103478.xEx4fc2hml@avalon>

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
;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel