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

[Rob Clark]

On Fri, Dec 12, 2014 at 3:57 PM, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
> Hi Rob,
>
> On Wednesday 10 December 2014 12:17:51 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.
>
> But it will change the size of the structure, and thus the ioctl value. You
> can't extend existing structures used in ioctls I'm afraid.

Actually, that is why it will work.  Old userspace passes smaller
size, drm_ioctl() zero pads the difference..

The issue is (potentially) in the other direction (new userspace, old
kernel) since the old kernel will ignore the new fields.  But that can
be sorted w/ a cap/query

BR,
-R


> By the way, is thus calls for an addfb3, I would add reserved fields at the
> end of the structure to make future extensions possible without a new ioctl.
>
>> 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?
>>
>> TODO move definition of tokens to drm_fourcc.h?
>>
>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> ---
>>  include/uapi/drm/drm_mode.h | 30 ++++++++++++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index aae71cb..c43648c 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -330,6 +330,28 @@ struct drm_mode_fb_cmd {
>>
>>  #define DRM_MODE_FB_INTERLACED       (1<<0) /* for interlaced framebuffers */
>>
>> +/*
>> + * Format Modifiers:
>> + *
>> + * Format modifiers describe, typically, a re-ordering or modification
>> + * of the data in a plane of an FB.  This can be used to express tiled/
>> + * swizzled formats, or compression, or a combination of the two.
>> + *
>> + * The upper 8 bits of the format modifier are a vendor-id as assigned
>> + * below.  The lower 24 bits are assigned as vendor sees fit.
>> + */
>> +
>> +/* Vendor Ids: */
>> +#define DRM_FOURCC_MOD_VENDOR_SAMSUNG 0x03
>> +/* ... more */
>> +
>> +#define DRM_FOURCC_MOD(vendor, name) (((DRM_FOURCC_MOD_VENDOR_## vendor) <<
>> 24) | val)
>> +
>> +/* Modifier values: */
>> +/* 64x32 macroblock, ie NV12MT: */
>> +#define DRM_FOURCC_MOD_SAMSUNG_64_32_TILE DRM_FOURCC_MOD(SAMSUNG, 123)
>> +/* ... more */
>> +
>>  struct drm_mode_fb_cmd2 {
>>       __u32 fb_id;
>>       __u32 width, height;
>> @@ -349,10 +371,18 @@ struct drm_mode_fb_cmd2 {
>>        * So it would consist of Y as offsets[0] and UV as
>>        * offsets[1].  Note that offsets[0] will generally
>>        * be 0 (but this is not required).
>> +      *
>> +      * To accommodate tiled, compressed, etc formats, a per-plane
>> +      * modifier can be specified.  The default value of zero
>> +      * indicates "native" format as specified by the fourcc.
>> +      * Vendor specific modifier token.  This allows, for example,
>> +      * different tiling/swizzling pattern on different planes.
>> +      * See discussion above of DRM_FOURCC_MOD_xxx.
>>        */
>>       __u32 handles[4];
>>       __u32 pitches[4]; /* pitch for each plane */
>>       __u32 offsets[4]; /* offset of each plane */
>> +     __u32 modifier[4]; /* ie, tiling, compressed (per plane) */
>>  };
>>
>>  #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01
>
> --
> Regards,
>
> Laurent Pinchart
>