[Intel-gfx] [PATCH v2 2/3] drm: Create a format/modifier blob

[Emil Velikov]

On 18 May 2017 at 01:46, Ben Widawsky <ben at bwidawsk.net> wrote:

>>> +       blob_size += modifiers_size;
>>> +
>>> +       blob = drm_property_create_blob(dev, blob_size, NULL);
>>> +       if (IS_ERR(blob))
>>> +               return -1;
>>> +
>>
>> Maybe propagate the exact error... Hmm we don't seem to check if
>> create_in_format_blob fails either so perhaps it's not worth it.
>>
>>
>
> In this case it's almost definitely ENOMEM. All values should be verified -
> I
> think the other errors are there for when userspace is utilizing blob
> creation.
>
> So I'll just leave it.
>
Ack, sure thing.

>>> --- a/include/uapi/drm/drm_mode.h
>>> +++ b/include/uapi/drm/drm_mode.h
>>> @@ -665,6 +665,56 @@ struct drm_mode_atomic {
>>>         __u64 user_data;
>>>  };
>>>
>>> +struct drm_format_modifier_blob {
>>> +#define FORMAT_BLOB_CURRENT 1
>>> +       /* Version of this blob format */
>>> +       u32 version;
>>> +
>>> +       /* Flags */
>>> +       u32 flags;
>>> +
>>> +       /* Number of fourcc formats supported */
>>> +       u32 count_formats;
>>> +
>>> +       /* Where in this blob the formats exist (in bytes) */
>>> +       u32 formats_offset;
>>> +
>>> +       /* Number of drm_format_modifiers */
>>> +       u32 count_modifiers;
>>> +
>>> +       /* Where in this blob the modifiers exist (in bytes) */
>>> +       u32 modifiers_offset;
>>> +
>>> +       /* u32 formats[] */
>>> +       /* struct drm_format_modifier modifiers[] */
>>> +};
>>> +
>>> +struct drm_format_modifier {
>>> +       /* Bitmask of formats in get_plane format list this info applies
>>> to. The
>>> +        * offset allows a sliding window of which 64 formats (bits).
>>> +        *
>>> +        * Some examples:
>>> +        * In today's world with < 65 formats, and formats 0, and 2 are
>>> +        * supported
>>> +        * 0x0000000000000005
>>> +        *                ^-offset = 0, formats = 5
>>> +        *
>>> +        * If the number formats grew to 128, and formats 98-102 are
>>> +        * supported with the modifier:
>
> G>> +        *
>>>
>>> +        * 0x0000003c00000000 0000000000000000
>>> +        *                ^
>>> +        *                |__offset = 64, formats = 0x3c00000000
>>> +        *
>>> +        */
>>> +       __u64 formats;
>>> +       __u32 offset;
>>> +       __u32 pad;
>>> +
>>> +       /* The modifier that applies to the >get_plane format list
>>> bitmask. */
>>> +       __u64 modifier;
>>
>> Please drop the leading __ from the type names in UAPI headers.
>>
>
> Many other structures have the __, can you explain why please (this has
> probably
> been beaten to death already; but I don't know)?
>
Got confused there for a moment, apologies.

Using the __ types is correct. It is drm_format_modifier_blob that
should be updated to follow suit.

-Emil