[PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Tom.O'Rourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d9c5918..3d3e402 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3522,8 +3522,10 @@ static void gen8_enable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
 
 	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
-	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
-	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
+	if (IS_CHERRYVIEW(dev)) {
+		I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
+		I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
+	}
 
 	/* 5: Enable RPS */
 	I915_WRITE(GEN6_RP_CONTROL,
-- 
1.7.9.5

Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Damien Lespiau]

On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'Rourke@intel.com wrote:
> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> 
> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW.
> 
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>

A lovely catch.

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_pm.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d9c5918..3d3e402 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3522,8 +3522,10 @@ static void gen8_enable_rps(struct drm_device *dev)
>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
>  
>  	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
> -	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
> -	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
> +	if (IS_CHERRYVIEW(dev)) {
> +		I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
> +		I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
> +	}
>  
>  	/* 5: Enable RPS */
>  	I915_WRITE(GEN6_RP_CONTROL,
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Jani Nikula]

On Mon, 09 Jun 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
> On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'Rourke@intel.com wrote:
>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>> 
>> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW.
>> 
>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
>
> A lovely catch.

Sadly gen8_enable_rps does not get called on chv, so the fix is wrong.

BR,
Jani.

>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
>
> -- 
> Damien
>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index d9c5918..3d3e402 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -3522,8 +3522,10 @@ static void gen8_enable_rps(struct drm_device *dev)
>>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
>>  
>>  	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
>> -	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
>> -	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
>> +	if (IS_CHERRYVIEW(dev)) {
>> +		I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
>> +		I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
>> +	}
>>  
>>  	/* 5: Enable RPS */
>>  	I915_WRITE(GEN6_RP_CONTROL,
>> -- 
>> 1.7.9.5
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Jani Nikula]

On Tue, 10 Jun 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 09 Jun 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
>> On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'Rourke@intel.com wrote:
>>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>>> 
>>> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW.
>>> 
>>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
>>
>> A lovely catch.
>
> Sadly gen8_enable_rps does not get called on chv, so the fix is wrong.

To elaborate, I think we need a patch dropping the wa altogether (which
we can queue for 3.15 through stable because the change affects
broadwell) and another patch, if needed, adding the wa in the chv
specific function.

Thanks,
Jani.


>
> BR,
> Jani.
>
>>
>> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
>>
>> -- 
>> Damien
>>
>>> ---
>>>  drivers/gpu/drm/i915/intel_pm.c |    6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index d9c5918..3d3e402 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -3522,8 +3522,10 @@ static void gen8_enable_rps(struct drm_device *dev)
>>>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
>>>  
>>>  	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
>>> -	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
>>> -	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
>>> +	if (IS_CHERRYVIEW(dev)) {
>>> +		I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
>>> +		I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
>>> +	}
>>>  
>>>  	/* 5: Enable RPS */
>>>  	I915_WRITE(GEN6_RP_CONTROL,
>>> -- 
>>> 1.7.9.5
>>> 
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Ville Syrjälä]

On Tue, Jun 10, 2014 at 06:34:18PM +0300, Jani Nikula wrote:
> On Tue, 10 Jun 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Mon, 09 Jun 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
> >> On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'Rourke@intel.com wrote:
> >>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >>> 
> >>> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW.
> >>> 
> >>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >>
> >> A lovely catch.
> >
> > Sadly gen8_enable_rps does not get called on chv, so the fix is wrong.
> 
> To elaborate, I think we need a patch dropping the wa altogether (which
> we can queue for 3.15 through stable because the change affects
> broadwell) and another patch, if needed, adding the wa in the chv
> specific function.

This is just a merge mishap in one the chv patches. Someone just
needs to send a patch that moves the misapplied stuff to the
appropriate chv function.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Jani Nikula]

On Tue, 10 Jun 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Jun 10, 2014 at 06:34:18PM +0300, Jani Nikula wrote:
>> On Tue, 10 Jun 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
>> > On Mon, 09 Jun 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
>> >> On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'Rourke@intel.com wrote:
>> >>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>> >>> 
>> >>> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW.
>> >>> 
>> >>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
>> >>
>> >> A lovely catch.
>> >
>> > Sadly gen8_enable_rps does not get called on chv, so the fix is wrong.
>> 
>> To elaborate, I think we need a patch dropping the wa altogether (which
>> we can queue for 3.15 through stable because the change affects
>> broadwell) and another patch, if needed, adding the wa in the chv
>> specific function.
>
> This is just a merge mishap in one the chv patches. Someone just
> needs to send a patch that moves the misapplied stuff to the
> appropriate chv function.

Right. So my first comment was correct, and my elaboration total
bullcrap. This is not present in 3.15, but we've queued the screwup for
3.16. Thanks for the correction Ville.

BR,
Jani.



>
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[O'Rourke, Tom]

>> This is just a merge mishap in one the chv patches. Someone just needs
>> to send a patch that moves the misapplied stuff to the appropriate chv
>> function.
>
>Right. So my first comment was correct, and my elaboration total bullcrap. This
>is not present in 3.15, but we've queued the screwup for 3.16. Thanks for the
>correction Ville.
>
>BR,
>Jani.
>
>
>
>>
>> --
>> Ville Syrjälä
>> Intel OTC
>
>--
>Jani Nikula, Intel Open Source Technology Center
[TOR:] Hi Ville,
Do you want to be the someone to send the patch that moves the misapplied stuff?
I could guess where it goes but I do not have a CHV for testing the fix.
Thanks,
Tom
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Ville Syrjälä]

On Tue, Jun 10, 2014 at 07:03:48PM +0000, O'Rourke, Tom wrote:
> >> This is just a merge mishap in one the chv patches. Someone just needs
> >> to send a patch that moves the misapplied stuff to the appropriate chv
> >> function.
> >
> >Right. So my first comment was correct, and my elaboration total bullcrap. This
> >is not present in 3.15, but we've queued the screwup for 3.16. Thanks for the
> >correction Ville.
> >
> >BR,
> >Jani.
> >
> >
> >
> >>
> >> --
> >> Ville Syrjälä
> >> Intel OTC
> >
> >--
> >Jani Nikula, Intel Open Source Technology Center
> [TOR:] Hi Ville,
> Do you want to be the someone to send the patch that moves the misapplied stuff?

Go ahead if you want to do it. Easy +1 to your patch statistics ;)

> I could guess where it goes

Or just look at the original patch:
http://patchwork.freedesktop.org/patch/23974/

> but I do not have a CHV for testing the fix.\

You're not alone in that. In any case the patch was never really
tested, except by the compiler :P

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm/i915/chv: Fix "drm/i915/chv: Add a bunch of pre production workarounds"

[Tom.O'Rourke]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Correct a merge mishap in commit e4443e459ccf43f2c139358400365fd6a839d40d.

Wa*:chv belongs in cherryview_enable_rps, not gen8_enable_rps.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 18f0ba0..c6e893b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3520,15 +3520,11 @@ static void gen8_enable_rps(struct drm_device *dev)
 
 	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
 
-	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
-	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
-	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
-
 	/* 5: Enable RPS */
 	I915_WRITE(GEN6_RP_CONTROL,
 		   GEN6_RP_MEDIA_TURBO |
 		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
-		   GEN6_RP_MEDIA_IS_GFX | /* WaSetMaskForGfxBusyness:chv (pre-production hw ?) */
+		   GEN6_RP_MEDIA_IS_GFX |
 		   GEN6_RP_ENABLE |
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_AVG);
@@ -4022,10 +4018,14 @@ static void cherryview_enable_rps(struct drm_device *dev)
 
 	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
 
+	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
+	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
+	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
+
 	/* 5: Enable RPS */
 	I915_WRITE(GEN6_RP_CONTROL,
 		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
-		   GEN6_RP_MEDIA_IS_GFX |
+		   GEN6_RP_MEDIA_IS_GFX | /* WaSetMaskForGfxBusyness:chv (pre-production hw ?) */
 		   GEN6_RP_ENABLE |
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_AVG);
-- 
1.7.9.5

Re: [PATCH] drm/i915/chv: Fix "drm/i915/chv: Add a bunch of pre production workarounds"

[Ville Syrjälä]

On Tue, Jun 10, 2014 at 04:26:34PM -0700, Tom.O'Rourke@intel.com wrote:
> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> 
> Correct a merge mishap in commit e4443e459ccf43f2c139358400365fd6a839d40d.
> 
> Wa*:chv belongs in cherryview_enable_rps, not gen8_enable_rps.
> 
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 18f0ba0..c6e893b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3520,15 +3520,11 @@ static void gen8_enable_rps(struct drm_device *dev)
>  
>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
>  
> -	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
> -	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
> -	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
> -
>  	/* 5: Enable RPS */
>  	I915_WRITE(GEN6_RP_CONTROL,
>  		   GEN6_RP_MEDIA_TURBO |
>  		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
> -		   GEN6_RP_MEDIA_IS_GFX | /* WaSetMaskForGfxBusyness:chv (pre-production hw ?) */
> +		   GEN6_RP_MEDIA_IS_GFX |
>  		   GEN6_RP_ENABLE |
>  		   GEN6_RP_UP_BUSY_AVG |
>  		   GEN6_RP_DOWN_IDLE_AVG);
> @@ -4022,10 +4018,14 @@ static void cherryview_enable_rps(struct drm_device *dev)
>  
>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
>  
> +	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
> +	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
> +	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
> +
>  	/* 5: Enable RPS */
>  	I915_WRITE(GEN6_RP_CONTROL,
>  		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
> -		   GEN6_RP_MEDIA_IS_GFX |
> +		   GEN6_RP_MEDIA_IS_GFX | /* WaSetMaskForGfxBusyness:chv (pre-production hw ?) */
>  		   GEN6_RP_ENABLE |
>  		   GEN6_RP_UP_BUSY_AVG |
>  		   GEN6_RP_DOWN_IDLE_AVG);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Jani Nikula]

On Tue, 10 Jun 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 10 Jun 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Tue, Jun 10, 2014 at 06:34:18PM +0300, Jani Nikula wrote:
>>> On Tue, 10 Jun 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>> > On Mon, 09 Jun 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
>>> >> On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'Rourke@intel.com wrote:
>>> >>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>>> >>> 
>>> >>> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW.
>>> >>> 
>>> >>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
>>> >>
>>> >> A lovely catch.
>>> >
>>> > Sadly gen8_enable_rps does not get called on chv, so the fix is wrong.
>>> 
>>> To elaborate, I think we need a patch dropping the wa altogether (which
>>> we can queue for 3.15 through stable because the change affects
>>> broadwell) and another patch, if needed, adding the wa in the chv
>>> specific function.
>>
>> This is just a merge mishap in one the chv patches. Someone just
>> needs to send a patch that moves the misapplied stuff to the
>> appropriate chv function.
>
> Right. So my first comment was correct, and my elaboration total
> bullcrap. This is not present in 3.15, but we've queued the screwup for
> 3.16. Thanks for the correction Ville.

Argh. I'm really confusing myself and others here. Please bear with me.

So we've added

commit e4443e459ccf43f2c139358400365fd6a839d40d
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Wed Apr 9 13:28:41 2014 +0300

    drm/i915/chv: Add a bunch of pre production workarounds

which contains the chv specific w/a in bdw code. This is now going to
3.16, and we need to fix this for 3.16 through
drm-intel-fixes. Effectively the hunk touching gen8_enable_rps() from
Tom's new patch [1]. Right?

However the new patch from Tom moves those bits to
cherryview_enable_rps(), which is only present since

commit 38807746fa2ce44b79957ff07813d10fcaf3d311
Author: Deepak S <deepak.s@linux.intel.com>
Date:   Fri May 23 21:00:15 2014 +0530

    drm/i915/chv: Enable Render Standby (RC6) for Cherryview

and queued for 3.17. So we need another patch adding the bits to
cherryview_enable_rps() on top of drm-intel-next-queued, effectively the
second hunk from Tom's new patch. Right?

So Tom, please split your patch in two, one on top of drm-intel-fixes,
and another on top of drm-intel-next-queued, and I think we'll be fine.

BR,
Jani.


[1] http://mid.gmane.org/1402442794-166797-1-git-send-email-Tom.O'Rourke@intel.com


>
> BR,
> Jani.
>
>
>
>>
>> -- 
>> Ville Syrjälä
>> Intel OTC
>
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/chv: Fix "drm/i915/chv: Add a bunch of pre production workarounds"

[Daniel Vetter]

On Thu, Jun 12, 2014 at 11:20:36AM +0300, Ville Syrjälä wrote:
> On Tue, Jun 10, 2014 at 04:26:34PM -0700, Tom.O'Rourke@intel.com wrote:
> > From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> > 
> > Correct a merge mishap in commit e4443e459ccf43f2c139358400365fd6a839d40d.
> > 
> > Wa*:chv belongs in cherryview_enable_rps, not gen8_enable_rps.
> > 
> > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c |   12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 18f0ba0..c6e893b 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3520,15 +3520,11 @@ static void gen8_enable_rps(struct drm_device *dev)
> >  
> >  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
> >  
> > -	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
> > -	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
> > -	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
> > -
> >  	/* 5: Enable RPS */
> >  	I915_WRITE(GEN6_RP_CONTROL,
> >  		   GEN6_RP_MEDIA_TURBO |
> >  		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
> > -		   GEN6_RP_MEDIA_IS_GFX | /* WaSetMaskForGfxBusyness:chv (pre-production hw ?) */
> > +		   GEN6_RP_MEDIA_IS_GFX |
> >  		   GEN6_RP_ENABLE |
> >  		   GEN6_RP_UP_BUSY_AVG |
> >  		   GEN6_RP_DOWN_IDLE_AVG);
> > @@ -4022,10 +4018,14 @@ static void cherryview_enable_rps(struct drm_device *dev)
> >  
> >  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
> >  
> > +	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
> > +	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
> > +	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
> > +
> >  	/* 5: Enable RPS */
> >  	I915_WRITE(GEN6_RP_CONTROL,
> >  		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
> > -		   GEN6_RP_MEDIA_IS_GFX |
> > +		   GEN6_RP_MEDIA_IS_GFX | /* WaSetMaskForGfxBusyness:chv (pre-production hw ?) */
> >  		   GEN6_RP_ENABLE |
> >  		   GEN6_RP_UP_BUSY_AVG |
> >  		   GEN6_RP_DOWN_IDLE_AVG);
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915/chv: WaDisablePwrmtrEvent:chv on CHV only

[Daniel Vetter]

On Thu, Jun 12, 2014 at 01:33:32PM +0300, Jani Nikula wrote:
> On Tue, 10 Jun 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Tue, 10 Jun 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >> On Tue, Jun 10, 2014 at 06:34:18PM +0300, Jani Nikula wrote:
> >>> On Tue, 10 Jun 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >>> > On Mon, 09 Jun 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
> >>> >> On Mon, Jun 09, 2014 at 10:06:49AM -0700, Tom.O'Rourke@intel.com wrote:
> >>> >>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >>> >>> 
> >>> >>> In gen8_enable_rps, don't write CHV registers unless IS_CHERRYVIEW.
> >>> >>> 
> >>> >>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> >>> >>
> >>> >> A lovely catch.
> >>> >
> >>> > Sadly gen8_enable_rps does not get called on chv, so the fix is wrong.
> >>> 
> >>> To elaborate, I think we need a patch dropping the wa altogether (which
> >>> we can queue for 3.15 through stable because the change affects
> >>> broadwell) and another patch, if needed, adding the wa in the chv
> >>> specific function.
> >>
> >> This is just a merge mishap in one the chv patches. Someone just
> >> needs to send a patch that moves the misapplied stuff to the
> >> appropriate chv function.
> >
> > Right. So my first comment was correct, and my elaboration total
> > bullcrap. This is not present in 3.15, but we've queued the screwup for
> > 3.16. Thanks for the correction Ville.
> 
> Argh. I'm really confusing myself and others here. Please bear with me.
> 
> So we've added
> 
> commit e4443e459ccf43f2c139358400365fd6a839d40d
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Wed Apr 9 13:28:41 2014 +0300
> 
>     drm/i915/chv: Add a bunch of pre production workarounds
> 
> which contains the chv specific w/a in bdw code. This is now going to
> 3.16, and we need to fix this for 3.16 through
> drm-intel-fixes. Effectively the hunk touching gen8_enable_rps() from
> Tom's new patch [1]. Right?
> 
> However the new patch from Tom moves those bits to
> cherryview_enable_rps(), which is only present since
> 
> commit 38807746fa2ce44b79957ff07813d10fcaf3d311
> Author: Deepak S <deepak.s@linux.intel.com>
> Date:   Fri May 23 21:00:15 2014 +0530
> 
>     drm/i915/chv: Enable Render Standby (RC6) for Cherryview
> 
> and queued for 3.17. So we need another patch adding the bits to
> cherryview_enable_rps() on top of drm-intel-next-queued, effectively the
> second hunk from Tom's new patch. Right?
> 
> So Tom, please split your patch in two, one on top of drm-intel-fixes,
> and another on top of drm-intel-next-queued, and I think we'll be fine.

I've already merged Tom's patch to dinq since it's fully needed there. So
I think we just need a new patch version for -fixes only and resolve the
mess in a merge (shouldn't cause one). If we split the patch also for dinq
then I need to rebase/merge again. I guess you could simply apply Tom's
fix to -fixes and drop the unecessary hunk yourself.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915/bdw: remove erroneous chv specific workarounds from bdw code

[Jani Nikula]

From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Correct a merge mishap in

commit e4443e459ccf43f2c139358400365fd6a839d40d
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Wed Apr 9 13:28:41 2014 +0300

    drm/i915/chv: Add a bunch of pre production workarounds

Remove the the chv specific workarounds from bdw code, specifically
gen8_enable_rps().

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
[Jani: extract hunk #1 for 3.16 from Tom's patch, clarify commit message]
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

All, I intend to push this to drm-intel-fixes, any objections?

Jani.
---
 drivers/gpu/drm/i915/intel_pm.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7e2db9abd810..769caea97c21 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3505,15 +3505,11 @@ static void gen8_enable_rps(struct drm_device *dev)
 
 	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
 
-	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
-	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
-	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
-
 	/* 5: Enable RPS */
 	I915_WRITE(GEN6_RP_CONTROL,
 		   GEN6_RP_MEDIA_TURBO |
 		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
-		   GEN6_RP_MEDIA_IS_GFX | /* WaSetMaskForGfxBusyness:chv (pre-production hw ?) */
+		   GEN6_RP_MEDIA_IS_GFX |
 		   GEN6_RP_ENABLE |
 		   GEN6_RP_UP_BUSY_AVG |
 		   GEN6_RP_DOWN_IDLE_AVG);
-- 
1.9.1

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

Re: [PATCH] drm/i915/bdw: remove erroneous chv specific workarounds from bdw code

[Jani Nikula]

On Fri, 13 Jun 2014, Jani Nikula <jani.nikula@intel.com> wrote:
> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>
> Correct a merge mishap in
>
> commit e4443e459ccf43f2c139358400365fd6a839d40d
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date:   Wed Apr 9 13:28:41 2014 +0300
>
>     drm/i915/chv: Add a bunch of pre production workarounds
>
> Remove the the chv specific workarounds from bdw code, specifically
> gen8_enable_rps().
>
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> [Jani: extract hunk #1 for 3.16 from Tom's patch, clarify commit message]
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ---
>
> All, I intend to push this to drm-intel-fixes, any objections?

And pushed with Daniel's IRC ack. Thanks everyone.

BR,
Jani.

>
> Jani.
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7e2db9abd810..769caea97c21 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3505,15 +3505,11 @@ static void gen8_enable_rps(struct drm_device *dev)
>  
>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
>  
> -	/* WaDisablePwrmtrEvent:chv (pre-production hw) */
> -	I915_WRITE(0xA80C, I915_READ(0xA80C) & 0x00ffffff);
> -	I915_WRITE(0xA810, I915_READ(0xA810) & 0xffffff00);
> -
>  	/* 5: Enable RPS */
>  	I915_WRITE(GEN6_RP_CONTROL,
>  		   GEN6_RP_MEDIA_TURBO |
>  		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
> -		   GEN6_RP_MEDIA_IS_GFX | /* WaSetMaskForGfxBusyness:chv (pre-production hw ?) */
> +		   GEN6_RP_MEDIA_IS_GFX |
>  		   GEN6_RP_ENABLE |
>  		   GEN6_RP_UP_BUSY_AVG |
>  		   GEN6_RP_DOWN_IDLE_AVG);
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/bdw: remove erroneous chv specific workarounds from bdw code

[O'Rourke, Tom]

>---
>
>All, I intend to push this to drm-intel-fixes, any objections?
>
>Jani.
>---
[TOR:] I have no objections.  Looks good to me.