[PATCH 0/3] drm/i915: add WaIncreaseDefaultTLBEntries

[PATCH 0/3] drm/i915: add WaIncreaseDefaultTLBEntries

[tim.gore]

From: Tim Gore <tim.gore@intel.com>

These 3 patches introduce a performance workaround that
can have significant impact (> 10%) on some OCL workloads.
It is split into 3 patches as follows.
 1) The first patch introduces a new function to collect
   together workarounds that are GT specific and apply
   globally across the GPU (as opposed to per ring or per
   context). This function is called at driver load and
   on resume (from suspend) and after a GPU reset, so it
   can host worarounds that are clobbered by these events.
   This new function was suggested by Mika Kuoppala.
2) The second patch adds WaIncreaseDefaultTLBEntries for
   gen8 GPU's to the above function.
3) Finally add WaIncreaseDefaultTLBEntries for gen9.


Tim Gore (3):
  drm/i915: add function for GT related workarounds
  drm/i915/gen8: add WaIncreaseDefaultTLBEntries
  drm/i915/gen9: add WaIncreaseDefaultTLBEntries

 drivers/gpu/drm/i915/i915_gem_gtt.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h     |  7 +++++++
 2 files changed, 28 insertions(+)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[tim.gore]

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)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* This function is for gtt related workarounds. This function is
+	 * called on driver load and after a GPU reset, so you can place
+	 * workarounds here even if they get overwritten by GPU reset.
+	 */
+}
+
 int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2148,6 +2158,8 @@ int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
 
 int i915_ppgtt_init_hw(struct drm_device *dev)
 {
+	gtt_write_workarounds(dev);
+
 	/* In the case of execlists, PPGTT is enabled by the context descriptor
 	 * and the PDPs are contained within the context itself.  We don't
 	 * need to do anything here. */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915/gen8: add WaIncreaseDefaultTLBEntries

[tim.gore]

From: Tim Gore <tim.gore@intel.com>

Add WaIncreaseDefaultTLBEntries:chv,bdw.
This workaround provides significant perfromance boost
for some OpenCL workloads by adjusting the L3_LRA_1_GPGPU
register.
This is for bdw/chv. Further patches will provide the
workaround for other sku's.

Signed-off-by: Tim Gore <tim.gore@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++++
 drivers/gpu/drm/i915/i915_reg.h     | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index fe960d5..be87318 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2140,6 +2140,11 @@ void gtt_write_workarounds(struct drm_device *dev)
 	 * called on driver load and after a GPU reset, so you can place
 	 * workarounds here even if they get overwritten by GPU reset.
 	 */
+	/* WaIncreaseDefaultTLBEntries:chv,bdw */
+	if (IS_BROADWELL(dev))
+		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW);
+	else if (IS_CHERRYVIEW(dev))
+		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV);
 }
 
 int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0a98889..513e2980 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8159,4 +8159,9 @@ enum skl_disp_power_wells {
 #define GEN9_VEBOX_MOCS(i)	_MMIO(0xcb00 + (i) * 4)	/* Video MOCS registers */
 #define GEN9_BLT_MOCS(i)	_MMIO(0xcc00 + (i) * 4)	/* Blitter MOCS registers */
 
+/* gamt regs */
+#define GEN8_L3_LRA_1_GPGPU _MMIO(0x4dd4)
+#define   GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW  0x67F1427F /* max/min for LRA1/2 */
+#define   GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV  0x5FF101FF /* max/min for LRA1/2 */
+
 #endif /* _I915_REG_H_ */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915/gen9: add WaIncreaseDefaultTLBEntries

[tim.gore]

From: Tim Gore <tim.gore@intel.com>

Add WaIncreaseDefaultTLBEntries:skl,bxt.
This workaround provides significant perfromance boost
for some OpenCL workloads by adjusting the L3_LRA_1_GPGPU
register.
This is for skl/bxt. Further patches will provide the
workaround for other sku's.

Signed-off-by: Tim Gore <tim.gore@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++++
 drivers/gpu/drm/i915/i915_reg.h     | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index be87318..6c8f936 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2145,6 +2145,10 @@ void gtt_write_workarounds(struct drm_device *dev)
 		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW);
 	else if (IS_CHERRYVIEW(dev))
 		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV);
+	else if (IS_SKYLAKE(dev))
+		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL);
+	else if (IS_BROXTON(dev))
+		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT);
 }
 
 int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 513e2980..b0516e2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8163,5 +8163,7 @@ enum skl_disp_power_wells {
 #define GEN8_L3_LRA_1_GPGPU _MMIO(0x4dd4)
 #define   GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW  0x67F1427F /* max/min for LRA1/2 */
 #define   GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV  0x5FF101FF /* max/min for LRA1/2 */
+#define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL  0x67F1427F /*    "        " */
+#define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT  0x5FF101FF /*    "        " */
 
 #endif /* _I915_REG_H_ */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Mika Kuoppala]

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

> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/* This function is for gtt related workarounds. This function is
> +	 * called on driver load and after a GPU reset, so you can place
> +	 * workarounds here even if they get overwritten by GPU reset.
> +	 */
> +}
> +
>  int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2148,6 +2158,8 @@ int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>  
>  int i915_ppgtt_init_hw(struct drm_device *dev)
>  {
> +	gtt_write_workarounds(dev);
> +
>  	/* In the case of execlists, PPGTT is enabled by the context descriptor
>  	 * and the PDPs are contained within the context itself.  We don't
>  	 * need to do anything here. */
> -- 
> 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/gen9: add WaIncreaseDefaultTLBEntries

[Mika Kuoppala]

tim.gore@intel.com writes:

> From: Tim Gore <tim.gore@intel.com>
>
> Add WaIncreaseDefaultTLBEntries:skl,bxt.
> This workaround provides significant perfromance boost
> for some OpenCL workloads by adjusting the L3_LRA_1_GPGPU
> register.
> This is for skl/bxt. Further patches will provide the
> workaround for other sku's.
>
> Signed-off-by: Tim Gore <tim.gore@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++++
>  drivers/gpu/drm/i915/i915_reg.h     | 2 ++
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index be87318..6c8f936 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2145,6 +2145,10 @@ void gtt_write_workarounds(struct drm_device *dev)
>  		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW);
>  	else if (IS_CHERRYVIEW(dev))
>  		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV);
> +	else if (IS_SKYLAKE(dev))
> +		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL);
> +	else if (IS_BROXTON(dev))
> +		I915_WRITE(GEN8_L3_LRA_1_GPGPU, GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT);
>  }

You forgot to add these to the Wa comment.

-Mika

>  
>  int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 513e2980..b0516e2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8163,5 +8163,7 @@ enum skl_disp_power_wells {
>  #define GEN8_L3_LRA_1_GPGPU _MMIO(0x4dd4)
>  #define   GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW  0x67F1427F /* max/min for LRA1/2 */
>  #define   GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV  0x5FF101FF /* max/min for LRA1/2 */
> +#define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL  0x67F1427F /*    "        " */
> +#define   GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT  0x5FF101FF /*    "        " */
>  
>  #endif /* _I915_REG_H_ */
> -- 
> 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Gore, Tim]

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?

  Tim

> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> > +	/* This function is for gtt related workarounds. This function is
> > +	 * called on driver load and after a GPU reset, so you can place
> > +	 * workarounds here even if they get overwritten by GPU reset.
> > +	 */
> > +}
> > +
> >  int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt
> > *ppgtt)  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private; @@ -2148,6
> > +2158,8 @@ int i915_ppgtt_init(struct drm_device *dev, struct
> > i915_hw_ppgtt *ppgtt)
> >
> >  int i915_ppgtt_init_hw(struct drm_device *dev)  {
> > +	gtt_write_workarounds(dev);
> > +
> >  	/* In the case of execlists, PPGTT is enabled by the context
> descriptor
> >  	 * and the PDPs are contained within the context itself.  We don't
> >  	 * need to do anything here. */
> > --
> > 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

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.

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Arun Siluvery]

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
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

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.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Arun Siluvery]

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
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Gore, Tim]

> -----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
> >

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: add WaIncreaseDefaultTLBEntries

[Patchwork]

== Summary ==

Built on 5ae916607e3e12ba18c848dff42baaad5b118c4b drm-intel-nightly: 2016y-01m-27d-12h-48m-36s UTC integration manifest


bdw-nuci7        total:141  pass:132  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:144  pass:138  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:144  pass:120  dwarn:0   dfail:0   fail:0   skip:24 
hsw-brixbox      total:144  pass:137  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:144  pass:140  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:144  pass:105  dwarn:0   dfail:0   fail:1   skip:38 
ivb-t430s        total:144  pass:138  dwarn:0   dfail:0   fail:0   skip:6  
skl-i5k-2        total:144  pass:135  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps      total:144  pass:130  dwarn:0   dfail:0   fail:0   skip:14 
snb-x220t        total:144  pass:130  dwarn:0   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1261/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: add WaIncreaseDefaultTLBEntries

[Patchwork]

== Summary ==

Series 2811v1 drm/i915: add WaIncreaseDefaultTLBEntries

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE

bdw-nuci7        total:156  pass:147  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:159  pass:153  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:159  pass:135  dwarn:0   dfail:0   fail:0   skip:24 
byt-nuc          total:159  pass:142  dwarn:0   dfail:0   fail:0   skip:17 
hsw-brixbox      total:159  pass:152  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:159  pass:155  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:159  pass:113  dwarn:1   dfail:0   fail:1   skip:44 
ivb-t430s        total:159  pass:151  dwarn:0   dfail:0   fail:0   skip:8  
skl-i5k-2        total:159  pass:150  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps      total:159  pass:141  dwarn:0   dfail:0   fail:0   skip:18 
snb-x220t        total:159  pass:141  dwarn:0   dfail:0   fail:1   skip:17 

Results at /archive/results/CI_IGT_test/Patchwork_1292/

HEAD is now at b3f8ad6 drm-intel-nightly: 2016y-01m-28d-10h-26m-23s UTC integration manifest
Applying: drm/i915: add function for GT related workarounds
Applying: drm/i915/gen8: add WaIncreaseDefaultTLBEntries
Applying: drm/i915/gen9: add WaIncreaseDefaultTLBEntries

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx