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

[Tvrtko Ursulin]

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.

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

Regards,

Tvrtko