[Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)

[Daniel Vetter]

On Thu, Jan 29, 2015 at 12:55:48PM +0000, Tvrtko Ursulin wrote:
> 
> On 01/29/2015 11:57 AM, Daniel Vetter wrote:
> >On Thu, Jan 29, 2015 at 11:43:07AM +0000, Tvrtko Ursulin wrote:
> >>
> >>On 01/29/2015 11:30 AM, Daniel Vetter wrote:
> >>>On Wed, Jan 28, 2015 at 05:57:56PM +0000, Tvrtko Ursulin wrote:
> >>>>
> >>>>On 01/28/2015 05:37 PM, Daniel Vetter wrote:
> >>>>>From: Rob Clark <robdclark at gmail.com>
> >>>>>
> >>>>>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?
> >>>>
> >>>>Maybe:
> >>>>	__u64 modifier[4];
> >>>>	__u64 vendor_modifier[4];
> >>>
> >>>Seems rendundant since the modifier added in this patch is already vendor
> >>>specific. Or what exactly are you trying to accomplish here?
> >>
> >>I am trying to avoid packet-in-a-packet (bitmasks) mumbo-jumbo and vendor id
> >>on the head followed by maybe standardized or maybe vendor specific tag.
> >>Feels funny. Would it not be simpler to put a struct in there?
> >
> >The u64 modifier is just an opaque thing, with 8 bit to identifier the
> >vendor (for easier number management really) and the low 56 bits can be
> >whatever we want them. On i915 I think we should just enumerate all the
> >various tiling modes we have. And if the modifiers aren't set we use the
> >tiling mode of the underlying gem bo. We already have code in place to
> >guarantee that the underlying bo's tiling can't change as long as there's
> >a kms fb around, which means all code which checks for tiling can switch
> >over to these flags.
> >
> >struct won't work since by definition this is vendor-specific, and every
> >vendor is slightly insane in a different way.
> 
> Well 8 + 56 bits is a "struct" already and not fully opaque. Are the bits
> expensive? :) That was first part of my point.
> 
> Secondly, "vendor who registers first" part of discussion is what came to my
> attention as well.
> 
> With this, a hypothetical standard format would be under a vendor id which
> looks funny to me. While you could split standard formats/modifiers and
> vendor specific modifiers.

If some standard body actually manages to pull this off we can always add
a new enum value for KHRONOS or MICROSOFT or ISO or whatever it ends up
being. The split really is just to make number assignements less
conflit-y.

> I don't care really, it was just an idea, but I don't get this arguments why
> it is, not only not better, but worse than a bitfield. :)

Just from your struct without any explanation what your idea was (namely
modifiers which are standardized across vendors it seems) it looked like
both a plain modifier and a vendor_modifier was possible. Which didn't
make a lot of sense to me, so I asked.

Going with an opaque u64 has the benefits that it matches kms property
values. So if we ever extend this and allow generic properties for
framebuffers it'll still fit. A struct is more painful. The idea of fb
properties was one of the longer-term ideas tossed around in the v1
thread.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch