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

Gore, Tim
2016-01-26T09:31:16Z

> -----Original Message-----
> From: Arun Siluvery [mailto:arun.siluvery@linux.intel.com]
> Sent: Monday, January 25, 2016 6:04 PM
> To: Chris Wilson; Gore, Tim; Mika Kuoppala; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: add function for GT related
> workarounds
>
> 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
>
I kept the introduction of the new w/a function as a separate patch so that the gen8 and gen9 patches are independent.
If you squash 1 and 2, then You can't revert just the gen8 changes because this would break the gen9 stuff.
>
Tim Gore
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ>
>
> > -Chris
> >