[Intel-gfx] [PATCH 1/7] drm/i915: Record GT workarounds in a list

[Tvrtko Ursulin]

On 03/12/2018 12:38, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-12-03 12:34:24)
>>
>> On 03/12/2018 11:54, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2018-12-03 11:46:11)
>>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>> @@ -575,160 +587,240 @@ int intel_ctx_workarounds_emit(struct i915_request *rq)
>>>>           return 0;
>>>>    }
>>>>    
>>>> -static void bdw_gt_workarounds_apply(struct drm_i915_private *dev_priv)
>>>> +static void
>>>> +wal_add(struct i915_wa_list *wal, const struct i915_wa *wa)
>>>> +{
>>>> +       const unsigned int grow = 1 << 4;
>>>> +
>>>> +       GEM_BUG_ON(!is_power_of_2(grow));
>>>> +
>>>> +       if (IS_ALIGNED(wal->count, grow)) { /* Either uninitialized or full. */
>>>
>>> Neat.
>>>
>>>> +               struct i915_wa *list;
>>>> +
>>>> +               list = kcalloc(ALIGN(wal->count + 1, grow), sizeof(*wa),
>>>> +                              GFP_KERNEL);
>>>
>>> (Quietly comments on the calloc here ;)
>>
>> Oh I don't want to complicate things with zeroing the tail. Or you
>> wouldn't bother with zeroing at all since I always copy over used
>> entries? So unused, who cares about them?
> 
> Exactly, nothing after wal->count is accessed, so no need to zero it as
> it will always be initialised by wal_add().

Ok.

>>>> +struct i915_wa_list {
>>>> +       const char      *name;
>>>> +       unsigned int    count;
>>>> +       struct i915_wa  *list;
>>>
>>> Oh well, didn't save anything after all.
>>
>> How nothing, one unsigned int per wa_list instance! :)
> 
> struct is padded to 8 bytes for the pointer. There's a hole after count,
> even if we place it at the end. :|

Another unsigned int gets in there later in the series - for the benefit 
of being able to log the exact, and not merged, number of workarounds. 
But it makes sense to move the count at the end anyway so I'll do that.

Regards,

Tvrtko