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

Arun Siluvery
2016-01-25T16:41:45Z

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?

regards
Arun

>
> u32 val;
>
> /* Wa:bar,foo,baz */
> val = 0;
> if (is_bar(dev_priv))
> val = 1;
> else if (is_foo(dev_priv))
> val = 2;
> else if (is_baz(dev_priv))
> val = 3;
> if (val)
> I915_WRITE(REG, val);
>
> Would result in slightly less horrendous code.
> -Chris