[Intel-gfx] [PATCH] drm/915: Pad GTT views of exec objects up to user specified size

[Tvrtko Ursulin]

Hi,

On 26/10/15 17:05, Chris Wilson wrote:
> On Mon, Oct 26, 2015 at 04:00:20PM +0000, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>> On 23/10/15 10:50, Tvrtko Ursulin wrote:
>>> On 22/10/15 17:07, Chris Wilson wrote:
>>>> On Thu, Oct 22, 2015 at 03:15:55PM +0100, Tvrtko Ursulin wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 21/10/15 16:24, Chris Wilson wrote:
>>>>>> Our GPUs impose certain requirements upon buffers that depend upon how
>>>>>> exactly they are used. Typically this is expressed as that they require
>>>>>> a larger surface than would be naively computed by pitch * height.
>>>>>> Normally such requirements are hidden away in the userspace driver, but
>>>>>> when we accept pointers from strangers and later impose extra
>>>>>> conditions
>>>>>> on them, the original client allocator has no idea about the
>>>>>> monstrosities in the GPU and we require the userspace driver to inform
>>>>>> the kernel how many padding pages are required beyond the client
>>>>>> allocation.
>>>>>>
>>>>>> v2: Long time, no see
>>>>>
>>>>> [snip]
>>>>>
>>>>>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>>>>>> index 08e047cba76a..678f7d5320ae 100644
>>>>>> --- a/include/uapi/drm/i915_drm.h
>>>>>> +++ b/include/uapi/drm/i915_drm.h
>>>>>> @@ -691,10 +691,11 @@ struct drm_i915_gem_exec_object2 {
>>>>>>   #define EXEC_OBJECT_NEEDS_GTT    (1<<1)
>>>>>>   #define EXEC_OBJECT_WRITE    (1<<2)
>>>>>>   #define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
>>>>>> -#define __EXEC_OBJECT_UNKNOWN_FLAGS
>>>>>> -(EXEC_OBJECT_SUPPORTS_48B_ADDRESS<<1)
>>>>>> +#define EXEC_OBJECT_PAD_TO_SIZE    (1<<4)
>>>>>> +#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_PAD_TO_SIZE<<1)
>>>>>>       __u64 flags;
>>>>>>
>>>>>> -    __u64 rsvd1;
>>>>>> +    __u64 rsvd1; /* pad_to_size */
>>>>>>       __u64 rsvd2;
>>>>>>   };
>>>>>
>>>>> What do you think about:
>>>>>
>>>>> union {
>>>>>     __u64 pad_to_size;
>>>>>     __u64 rsvd1;
>>>>> } ?
>>>>>
>>>>> Kind of like a migration path for userspace?
>>>>
>>>> Hmm, I think that just might work. Clang? Does clang support anonymous
>>>> unions yet? Do we care if it doesn't?
>>>
>>> I've found some existing examples in uapi headers so think we are covered.
>>
>> Any further thoughts on this? Would you consider re-spinning the
>> patch with this addition?
>
> I have respun it, but haven't checked it against clang nor do I know
> what others think of potential portability issues with other compilers.

As I said there are other anonymous unions in uapi headers so I think it 
should be fine.

I even think just renaming the field in the first place should be fine, 
otherwise what is the point of having reserved fields.

Regards,

Tvrtko