diff mbox

[v2] drm/i915/chv: Use timeout mode for RC6 on chv

Message ID 1418451207-30795-1-git-send-email-deepak.s@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

deepak.s@linux.intel.com Dec. 13, 2014, 6:13 a.m. UTC
From: Deepak S <deepak.s@linux.intel.com>

Higher RC6 residency is observed using timeout mode
instead of EI mode. It's Recommended to use TO Method for RC6.

v2: Add comment about timeout threshold. (Tom)

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Ville Syrjälä Dec. 12, 2014, 4:34 p.m. UTC | #1
On Sat, Dec 13, 2014 at 11:43:27AM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> Higher RC6 residency is observed using timeout mode
> instead of EI mode. It's Recommended to use TO Method for RC6.
> 
> v2: Add comment about timeout threshold. (Tom)

Yeah if TO is better let's just use it. The 1750us value is what
the BIOS spec recommends, so:

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

> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Why is Rodrigo's sob here?

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2316d23..2acb3de 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4689,7 +4689,8 @@ static void cherryview_enable_rps(struct drm_device *dev)
>  		I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
>  	I915_WRITE(GEN6_RC_SLEEP, 0);
>  
> -	I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
> +	/* TO threshold set to 1750 us ( 0x557 * 1.28 us) */
> +	I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
>  
>  	/* allows RC6 residency counter to work */
>  	I915_WRITE(VLV_COUNTER_CONTROL,
> @@ -4703,7 +4704,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
>  	/* 3: Enable RC6 */
>  	if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
>  						(pcbr >> VLV_PCBR_ADDR_SHIFT))
> -		rc6_mode = GEN6_RC_CTL_EI_MODE(1);
> +		rc6_mode = GEN7_RC_CTL_TO_MODE;
>  
>  	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
>  
> -- 
> 1.9.1
Daniel Vetter Dec. 15, 2014, 3:12 p.m. UTC | #2
On Tue, Dec 16, 2014 at 05:39:19PM +0530, Deepak S wrote:
> 
> On Friday 12 December 2014 10:04 PM, Ville Syrjälä wrote:
> >On Sat, Dec 13, 2014 at 11:43:27AM +0530, deepak.s@linux.intel.com wrote:
> >>From: Deepak S <deepak.s@linux.intel.com>
> >>
> >>Higher RC6 residency is observed using timeout mode
> >>instead of EI mode. It's Recommended to use TO Method for RC6.
> >>
> >>v2: Add comment about timeout threshold. (Tom)
> >Yeah if TO is better let's just use it. The 1750us value is what
> >the BIOS spec recommends, so:
> >
> >Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >>Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> >>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >Why is Rodrigo's sob here?
> 
> Rodrigo had resubmitted my patch with his sob. just retained :)

Sob should document the path a patch took, not every possible person who
every touch a patch. If you want to acknowledge indirect contributions
just mention them in the commit message (E.g. "based on a patch by" or
"squash in fixup from $person"). The bangalore team like to pile up sob
lines especially, which isn't really how it's supposed to work.

A few things to check:
- First sob should be the original author.
- Last sob should be the submitter of the patch.

Anyway, back to merging patches for me ;-)
-Daniel
deepak.s@linux.intel.com Dec. 16, 2014, 12:09 p.m. UTC | #3
On Friday 12 December 2014 10:04 PM, Ville Syrjälä wrote:
> On Sat, Dec 13, 2014 at 11:43:27AM +0530, deepak.s@linux.intel.com wrote:
>> From: Deepak S <deepak.s@linux.intel.com>
>>
>> Higher RC6 residency is observed using timeout mode
>> instead of EI mode. It's Recommended to use TO Method for RC6.
>>
>> v2: Add comment about timeout threshold. (Tom)
> Yeah if TO is better let's just use it. The 1750us value is what
> the BIOS spec recommends, so:
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Why is Rodrigo's sob here?

Rodrigo had resubmitted my patch with his sob. just retained :)

>> ---
>>   drivers/gpu/drm/i915/intel_pm.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 2316d23..2acb3de 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4689,7 +4689,8 @@ static void cherryview_enable_rps(struct drm_device *dev)
>>   		I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
>>   	I915_WRITE(GEN6_RC_SLEEP, 0);
>>   
>> -	I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
>> +	/* TO threshold set to 1750 us ( 0x557 * 1.28 us) */
>> +	I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
>>   
>>   	/* allows RC6 residency counter to work */
>>   	I915_WRITE(VLV_COUNTER_CONTROL,
>> @@ -4703,7 +4704,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
>>   	/* 3: Enable RC6 */
>>   	if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
>>   						(pcbr >> VLV_PCBR_ADDR_SHIFT))
>> -		rc6_mode = GEN6_RC_CTL_EI_MODE(1);
>> +		rc6_mode = GEN7_RC_CTL_TO_MODE;
>>   
>>   	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
>>   
>> -- 
>> 1.9.1
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2316d23..2acb3de 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4689,7 +4689,8 @@  static void cherryview_enable_rps(struct drm_device *dev)
 		I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
 	I915_WRITE(GEN6_RC_SLEEP, 0);
 
-	I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
+	/* TO threshold set to 1750 us ( 0x557 * 1.28 us) */
+	I915_WRITE(GEN6_RC6_THRESHOLD, 0x557);
 
 	/* allows RC6 residency counter to work */
 	I915_WRITE(VLV_COUNTER_CONTROL,
@@ -4703,7 +4704,7 @@  static void cherryview_enable_rps(struct drm_device *dev)
 	/* 3: Enable RC6 */
 	if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
 						(pcbr >> VLV_PCBR_ADDR_SHIFT))
-		rc6_mode = GEN6_RC_CTL_EI_MODE(1);
+		rc6_mode = GEN7_RC_CTL_TO_MODE;
 
 	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);