Re: [Intel-gfx] [PATCH v4 23/26] drm/i915/slpc: Keep RP SW Mode enabled while disabling rps

2016-09-15 Thread Kamble, Sagar A 



On 9/9/2016 10:28 PM, Chris Wilson wrote:

On Fri, Sep 09, 2016 at 06:21:42PM +0530, Sagar Arun Kamble wrote:

With SLPC, only RP SW Mode control should be left enabled by i915.
Else, SLPC requests through through RPNSWREQ will not be granted.

Signed-off-by: Sagar Arun Kamble 
---
  drivers/gpu/drm/i915/intel_pm.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 70e08d9..d06c9bb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5064,7 +5064,13 @@ static void gen9_disable_rc6(struct drm_i915_private 
*dev_priv)
  
  static void gen9_disable_rps(struct drm_i915_private *dev_priv)

  {
-   I915_WRITE(GEN6_RP_CONTROL, 0);
+   uint32_t rp_ctl = 0;

u32 rp_ctl = 0;


+
+   /* RP SW Mode Control will be needed for SLPC, Hence not clearing.*/

/* RP SW Mode Control will be needed for SLPC, so keep it enabled. */


+   if (i915.enable_slpc)

intel_slpc_enabled() ? (consistency!)

Will update this.



+   rp_ctl = I915_READ(GEN6_RP_CONTROL) & GEN6_RP_MEDIA_MODE_MASK;

Ok, so this is not doing what you describe. This is preserving state,
yes. But if we know that state is meant to be enabled for SLPC why are
we reading it back.

I am left with questions about what is happening behind our backs, and
what the code is trying to hide.
Will fix this. SLPC is going to request frequency hence we need to make 
sure host

does not tamper RP_CONTROL settings.

-Chris



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 23/26] drm/i915/slpc: Keep RP SW Mode enabled while disabling rps

2016-09-09 Thread Chris Wilson 
On Fri, Sep 09, 2016 at 06:21:42PM +0530, Sagar Arun Kamble wrote:
> With SLPC, only RP SW Mode control should be left enabled by i915.
> Else, SLPC requests through through RPNSWREQ will not be granted.
> 
> Signed-off-by: Sagar Arun Kamble 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 70e08d9..d06c9bb 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5064,7 +5064,13 @@ static void gen9_disable_rc6(struct drm_i915_private 
> *dev_priv)
>  
>  static void gen9_disable_rps(struct drm_i915_private *dev_priv)
>  {
> - I915_WRITE(GEN6_RP_CONTROL, 0);
> + uint32_t rp_ctl = 0;

u32 rp_ctl = 0;

> +
> + /* RP SW Mode Control will be needed for SLPC, Hence not clearing.*/

/* RP SW Mode Control will be needed for SLPC, so keep it enabled. */

> + if (i915.enable_slpc)

intel_slpc_enabled() ? (consistency!)

> + rp_ctl = I915_READ(GEN6_RP_CONTROL) & GEN6_RP_MEDIA_MODE_MASK;

Ok, so this is not doing what you describe. This is preserving state,
yes. But if we know that state is meant to be enabled for SLPC why are
we reading it back.

I am left with questions about what is happening behind our backs, and
what the code is trying to hide.
-Chris

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

[Intel-gfx] [PATCH v4 23/26] drm/i915/slpc: Keep RP SW Mode enabled while disabling rps

2016-09-09 Thread Sagar Arun Kamble 
With SLPC, only RP SW Mode control should be left enabled by i915.
Else, SLPC requests through through RPNSWREQ will not be granted.

Signed-off-by: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/intel_pm.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 70e08d9..d06c9bb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5064,7 +5064,13 @@ static void gen9_disable_rc6(struct drm_i915_private 
*dev_priv)
 
 static void gen9_disable_rps(struct drm_i915_private *dev_priv)
 {
-   I915_WRITE(GEN6_RP_CONTROL, 0);
+   uint32_t rp_ctl = 0;
+
+   /* RP SW Mode Control will be needed for SLPC, Hence not clearing.*/
+   if (i915.enable_slpc)
+   rp_ctl = I915_READ(GEN6_RP_CONTROL) & GEN6_RP_MEDIA_MODE_MASK;
+
+   I915_WRITE(GEN6_RP_CONTROL, rp_ctl);
 
dev_priv->rps.enabled = false;
 }
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx