Re: [PATCH 1/6] drm/i915: Bring UP Power Wells before disabling RC6.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Wednesday 09 April 2014 06:32 PM, Daniel Vetter wrote:
On Wed, Apr 09, 2014 at 09:51:38AM +0530, S, Deepak wrote:

On 4/9/2014 9:43 AM, Ben Widawsky wrote:
On Tue, Apr 08, 2014 at 06:22:52PM +0530, S, Deepak wrote:

On 4/8/2014 6:13 PM, Ville Syrjälä wrote:
On Mon, Apr 07, 2014 at 02:36:20PM -0700, Ben Widawsky wrote:
On Mon, Apr 07, 2014 at 05:01:46PM -0300, Rodrigo Vivi wrote:
From: Deepak S <deepak.s@xxxxxxxxx>

We need do forcewake before Disabling RC6, This is what the BIOS
expects while going into suspend.

v2: updated commit message. (Daniel)

Signed-off-by: Deepak S <deepak.s@xxxxxxxxx>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 04af065..ad2ff99 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3198,8 +3198,14 @@ static void valleyview_disable_rps(struct drm_device *dev)
  {
  	struct drm_i915_private *dev_priv = dev->dev_private;

+	/* we're doing forcewake before Disabling RC6,
+	 * This what the BIOS expects when going into suspend */
+	gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+
  	I915_WRITE(GEN6_RC_CONTROL, 0);

+	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
+
  	gen6_disable_rps_interrupts(dev);
  }

Isn't the forcewake done as part of I915_WRITE sufficient?
Writes don't do forcewake, nor is the register even part of the
VLV forcewake ranges.

I guess the rationale for this patche is still a bit vague. But if it's
really needed, I wonder whether we should do this same dance for !VLV
too? Do we have any "GPU stuck in wrong power state after suspend" type of
bugs still around?
One of suggestion form the HW team was to Bring the wells up before we
disable RC6 at run-time. We did see some issue when we enabled D0ix.

I think the is a good practice to make sure we bring-up the wells before we
disable RC6. At least this avoids the cases where wells are not up before we
can access the Next register after disable.
Ville was totally right. I do think a POSTING_READ is still sufficient.
Don't care much either way.

If feel this patch is not adding any value. I OK dropping this patch.
I think it makes a lot of sense - on the enable side we also grab the
wells (in case the bios has enabled rc6 already) to make sure we change
the rc6/rps state while everything is around.

Can you please update your patch to also roll this out for gen6/8 rps
disable functions?

Thanks, Daniel

Sure Daniel, I will add this to gen6/8 and submit the patch

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





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux