Re: [PATCH 1/3] drm/i915: add function for GT related workarounds

Arun Siluvery
2016-01-25T18:04:33Z

On 25/01/2016 17:10, Chris Wilson wrote:
> On Mon, Jan 25, 2016 at 04:41:42PM +0000, Arun Siluvery wrote:
>> On 25/01/2016 16:17, Chris Wilson wrote:
>>> On Mon, Jan 25, 2016 at 02:43:06PM +0000, Gore, Tim wrote:
>>>>
>>>>
>>>> Tim Gore
>>>> Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Mika Kuoppala [mailto:mika.kuoppala@linux.intel.com]
>>>>> Sent: Monday, January 25, 2016 2:39 PM
>>>>> To: Gore, Tim; intel-gfx@lists.freedesktop.org
>>>>> Cc: Gore, Tim; arun.siluvery@linux.intel.com
>>>>> Subject: Re: [PATCH 1/3] drm/i915: add function for GT related workarounds
>>>>>
>>>>> tim.gore@intel.com writes:
>>>>>
>>>>>> From: Tim Gore <tim.gore@intel.com>
>>>>>>
>>>>>> Add a function that is a place for workarounds that are GT related but
>>>>>> not required per ring. This function is called on driver load and also
>>>>>> after a reset and on resume, so it is safe for workarounds that get
>>>>>> clobbered in these situations.
>>>>>>
>>>>>> Signed-off-by: Tim Gore <tim.gore@intel.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 12 ++++++++++++
>>>>>> 1 file changed, 12 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>> index 7377b67..fe960d5 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>> @@ -2132,6 +2132,16 @@ static void i915_address_space_init(struct
>>>>> i915_address_space *vm,
>>>>>> list_add_tail(&vm->global_link, &dev_priv->vm_list); }
>>>>>>
>>>>>> +void gtt_write_workarounds(struct drm_device *dev) {
>>>>>
>>>>> static void
>>>>>
>>>>> This can be squashed with 2/3.
>>>>>
>>>>> -Mika
>>>>>
>>>> Do you mean all squashed together, into a single patch?
>>>
>>> I would. They are all setting the same register to a nominal value, for
>>> the same purpose.
>>
>> Don't we normally split WA into individual patches or is this only
>> for this WA?
>
> Is it not the same w/a applied to different generations? You either
> split it per device, so that a bisect + revert only affects one machine,
> or not all. Choose your poison.

yes but the value programmed is different for each device.

I think as Mika suggested, squashing 1, 2 which covers gen8 and another
patch for gen9 is a good split.

regards
Arun

> -Chris