VLV Turbo/rps + RC6 workaround

PATCH Superseded [email protected] 3 participants 13 messages Jan 27, 2014 – Jan 30, 2014 1 message not in the archive

Subsystems (2)

DRM drivers Intel i915

lore

Share & tools

lore

[email protected]

author

2014-01-27 16:07 UTC #0 lore raw

v5 cover VLV Turbo/rps + RC6 workaround

From: Deepak S <[email protected]>

Below patches addes WA to set Gfx freq while clock are down and enable/disable the pm interrupts based on cur delay.

Deepak S (2):
drm/i915: Disable/Enable PM Intrrupts based on the current freq. drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx
is power gated.

 drivers/gpu/drm/i915/i915_drv.h |  3 +++
 drivers/gpu/drm/i915/i915_irq.c | 39 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h |  4 +++
 drivers/gpu/drm/i915/intel_pm.c | 56 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 101 insertions(+), 1 deletion(-)

Signature · 1 line

1.8.5.2

[email protected]

author

2014-01-27 16:07 UTC #1 lore raw

PATCH v5 1/2 drm/i915: Disable/Enable PM Intrrupts based on the current freq.

From: Deepak S <[email protected]>

When current delay is already at max delay, Let's disable the PM UP THRESHOLD INTRRUPTS, so that we will not get further interrupts until current delay is less than max delay, Also request for the PM DOWN THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and viceversa for PM DOWN THRESHOLD INTRRUPTS.

v2: Use bool variables (Daniel)

v3: Fix Interrupt masking bit (Deepak)

v4: Use existing symbolic constants in i915_reg.h (Daniel)

v5: Add pm interrupt mask after new_delay calculation (Ville)

Signed-off-by Deepak S <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++
 drivers/gpu/drm/i915/i915_irq.c | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.c |  3 +++
 3 files changed, 45 insertions(+)

drivers/gpu/drm/i915/i915_drv.h +3 -0

index 56c720b..f19de66 100644
@@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
 	u8 rp0_delay;
 	u8 hw_max;
 
+	bool rp_up_masked;
+	bool rp_down_masked;
+
 	int last_adj;
 	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;

drivers/gpu/drm/i915/i915_irq.c +39 -0

index 01a8686..69a5214 100644
@@ -972,6 +972,43 @@ static void notify_ring(struct drm_device *dev,
 	i915_queue_hangcheck(dev);
 }
 
+static void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
+						u32 pm_iir, int *new_delay)
+{
+	if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
+		if (*new_delay >= dev_priv->rps.max_delay) {
+			/* Mask UP THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+				I915_READ(GEN6_PMINTRMSK) |
+						GEN6_PM_RP_UP_THRESHOLD);
+			dev_priv->rps.rp_up_masked = true;
+		}
+		if (dev_priv->rps.rp_down_masked) {
+			/* UnMask DOWN THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+				I915_READ(GEN6_PMINTRMSK) &
+						~GEN6_PM_RP_DOWN_THRESHOLD);
+			dev_priv->rps.rp_down_masked = false;
+		}
+	} else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD) {
+		if (*new_delay <= dev_priv->rps.min_delay) {
+			/* Mask DOWN THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) |
+						GEN6_PM_RP_DOWN_THRESHOLD);
+			dev_priv->rps.rp_down_masked = true;
+		}
+
+		if (dev_priv->rps.rp_up_masked) {
+			/* UnMask UP THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+				I915_READ(GEN6_PMINTRMSK) &
+						~GEN6_PM_RP_UP_THRESHOLD);
+			dev_priv->rps.rp_up_masked = false;
+		}
+	}
+}
+
 static void gen6_pm_rps_work(struct work_struct *work)
 {
 	drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,

@@ -1029,6 +1066,8 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	 */
 	new_delay = clamp_t(int, new_delay,
 			    dev_priv->rps.min_delay, dev_priv->rps.max_delay);
+
+	gen6_set_pm_mask(dev_priv, pm_iir, &new_delay);
 	dev_priv->rps.last_adj = new_delay - dev_priv->rps.cur_delay;
 
 	if (IS_VALLEYVIEW(dev_priv->dev))

drivers/gpu/drm/i915/intel_pm.c +3 -0

index b9b4fe4..c6a07c9 100644
@@ -3615,6 +3615,9 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
 
+	dev_priv->rps.rp_up_masked = false;
+	dev_priv->rps.rp_down_masked = false;
+
 	gen6_enable_rps_interrupts(dev);
 
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);

Signature · 1 line

1.8.5.2

A message that is not in the archive (sent off-list or not yet received): F359EE24E0B97E4A9739736D2C188EF3010089D9@BGSMSX104.gar.corp.intel.com

S, Deepak

author

2014-01-29 15:45 UTC #3 lore raw

Re: [PATCH v5 1/2] drm/i915: Disable/Enable PM Intrrupts based on the current freq.

Hi Ville,

Can you please review this patch. If Ok, Can we push this to nightly?

Thanks
Deepak

Quoted 110 lines · Deepak

-----Original Message-----
From: S, Deepak
Sent: Monday, January 27, 2014 9:35 PM
To: [email protected]
Cc: S, Deepak
Subject: [PATCH v5 1/2] drm/i915: Disable/Enable PM Intrrupts based on the current freq.

From: Deepak S <[email protected]>

v2: Use bool variables (Daniel)

v3: Fix Interrupt masking bit (Deepak)

v4: Use existing symbolic constants in i915_reg.h (Daniel)

v5: Add pm interrupt mask after new_delay calculation (Ville)

Signed-off-by Deepak S <[email protected]>

---
  drivers/gpu/drm/i915/i915_drv.h |  3 +++  drivers/gpu/drm/i915/i915_irq.c | 39 +++++++++++++++++++++++++++++++++++++++
  drivers/gpu/drm/i915/intel_pm.c |  3 +++
  3 files changed, 45 insertions(+)

drivers/gpu/drm/i915/i915_drv.h +3 -0

@@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
  	u8 rp0_delay;
  	u8 hw_max;
 
+	bool rp_up_masked;
+	bool rp_down_masked;
+
  	int last_adj;
  	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;

drivers/gpu/drm/i915/i915_irq.c +37 -0

@@ -972,6 +972,43 @@ static void notify_ring(struct drm_device *dev,
  	i915_queue_hangcheck(dev);
  }
 
+static void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
+						u32 pm_iir, int *new_delay)
+{
+	if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
+		if (*new_delay >= dev_priv->rps.max_delay) {
+			/* Mask UP THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+				I915_READ(GEN6_PMINTRMSK) |
+						GEN6_PM_RP_UP_THRESHOLD);
+			dev_priv->rps.rp_up_masked = true;
+		}
+		if (dev_priv->rps.rp_down_masked) {
+			/* UnMask DOWN THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+				I915_READ(GEN6_PMINTRMSK) &
+						~GEN6_PM_RP_DOWN_THRESHOLD);
+			dev_priv->rps.rp_down_masked = false;
+		}
+	} else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD) {
+		if (*new_delay <= dev_priv->rps.min_delay) {
+			/* Mask DOWN THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) |
+						GEN6_PM_RP_DOWN_THRESHOLD);
+			dev_priv->rps.rp_down_masked = true;
+		}
+
+		if (dev_priv->rps.rp_up_masked) {
+			/* UnMask UP THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+				I915_READ(GEN6_PMINTRMSK) &
+						~GEN6_PM_RP_UP_THRESHOLD);
+			dev_priv->rps.rp_up_masked = false;
+		}
+	}
+}
+
  static void gen6_pm_rps_work(struct work_struct *work)  {
  	drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t, @@ -1029,6 +1066,8 @@ static void gen6_pm_rps_work(struct work_struct *work)
  	 */

new_delay = clamp_t(int, new_delay,
dev_priv->rps.min_delay, dev_priv->rps.max_delay);

drivers/gpu/drm/i915/i915_irq.c +2 -0

+
+	gen6_set_pm_mask(dev_priv, pm_iir, &new_delay);
  	dev_priv->rps.last_adj = new_delay - dev_priv->rps.cur_delay;
 
  	if (IS_VALLEYVIEW(dev_priv->dev))

drivers/gpu/drm/i915/intel_pm.c +3 -1

@@ -3615,6 +3615,9 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
  	valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
 
+	dev_priv->rps.rp_up_masked = false;
+	dev_priv->rps.rp_down_masked = false;
+
  	gen6_enable_rps_interrupts(dev);
 
  	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
--

1.8.5.2

Ville Syrjälä

Reviewed-by on v5 1/2

2014-01-29 15:59 UTC #4 lore raw

Re: [PATCH v5 1/2] drm/i915: Disable/Enable PM Intrrupts based on the current freq.

In reply to [email protected]

Quoted 42 lines · On Mon, Jan 27, 2014 at 09:35:05PM +0530, [email protected] wrote:

From: Deepak S <[email protected]>
When current delay is already at max delay, Let's disable the PM UP THRESHOLD INTRRUPTS, so that we will not get further interrupts until current delay is less than max delay, Also request for the PM DOWN THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and viceversa for PM DOWN THRESHOLD INTRRUPTS.
v2: Use bool variables (Daniel)
v3: Fix Interrupt masking bit (Deepak)
v4: Use existing symbolic constants in i915_reg.h (Daniel)
v5: Add pm interrupt mask after new_delay calculation (Ville)
Signed-off-by Deepak S <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++
 drivers/gpu/drm/i915/i915_irq.c | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.c |  3 +++
 3 files changed, 45 insertions(+)
drivers/gpu/drm/i915/i915_drv.h +3 -0
index 56c720b..f19de66 100644
@@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
 	u8 rp0_delay;
 	u8 hw_max;
 
+	bool rp_up_masked;
+	bool rp_down_masked;
+
 	int last_adj;
 	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
 
drivers/gpu/drm/i915/i915_irq.c +0 -0
index 01a8686..69a5214 100644

drivers/gpu/drm/i915/i915_irq.c static void notify_ring(struct drm_device *dev, · line 976

Quoted 5 lines

drivers/gpu/drm/i915/i915_irq.c +2 -0

@@ -972,6 +972,43 @@ static void notify_ring(struct drm_device *dev,
 	i915_queue_hangcheck(dev);
 }
 
+static void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
+						u32 pm_iir, int *new_delay)

Just a minor nit here. We don't modify new_delay in this function, so passing by value would be better.

Otherwise the patch looks good to me. So if you change that, you can add:

Reviewed-by Ville Syrjälä <[email protected]>

Quoted 68 lines

drivers/gpu/drm/i915/i915_irq.c +37 -0

+{
+	if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
+		if (*new_delay >= dev_priv->rps.max_delay) {
+			/* Mask UP THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+				I915_READ(GEN6_PMINTRMSK) |
+						GEN6_PM_RP_UP_THRESHOLD);
+			dev_priv->rps.rp_up_masked = true;
+		}
+		if (dev_priv->rps.rp_down_masked) {
+			/* UnMask DOWN THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+				I915_READ(GEN6_PMINTRMSK) &
+						~GEN6_PM_RP_DOWN_THRESHOLD);
+			dev_priv->rps.rp_down_masked = false;
+		}
+	} else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD) {
+		if (*new_delay <= dev_priv->rps.min_delay) {
+			/* Mask DOWN THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) |
+						GEN6_PM_RP_DOWN_THRESHOLD);
+			dev_priv->rps.rp_down_masked = true;
+		}
+
+		if (dev_priv->rps.rp_up_masked) {
+			/* UnMask UP THRESHOLD Interrupts */
+			I915_WRITE(GEN6_PMINTRMSK,
+				I915_READ(GEN6_PMINTRMSK) &
+						~GEN6_PM_RP_UP_THRESHOLD);
+			dev_priv->rps.rp_up_masked = false;
+		}
+	}
+}
+
 static void gen6_pm_rps_work(struct work_struct *work)
 {
 	drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,

@@ -1029,6 +1066,8 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	 */
 	new_delay = clamp_t(int, new_delay,
 			    dev_priv->rps.min_delay, dev_priv->rps.max_delay);
+
+	gen6_set_pm_mask(dev_priv, pm_iir, &new_delay);
 	dev_priv->rps.last_adj = new_delay - dev_priv->rps.cur_delay;
 
 	if (IS_VALLEYVIEW(dev_priv->dev))

drivers/gpu/drm/i915/intel_pm.c +3 -0

index b9b4fe4..c6a07c9 100644
@@ -3615,6 +3615,9 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
 
+	dev_priv->rps.rp_up_masked = false;
+	dev_priv->rps.rp_down_masked = false;
+
 	gen6_enable_rps_interrupts(dev);
 
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);

Signature · 6 lines

1.8.5.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

Ville Syrjälä
Intel OTC

Daniel Vetter

2014-01-29 19:30 UTC #5 lore raw

Re: [PATCH v5 1/2] drm/i915: Disable/Enable PM Intrrupts based on the current freq.

Quoted 46 lines · Ville Syrjälä

Quoted 45 lines · On Mon, Jan 27, 2014 at 09:35:05PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When current delay is already at max delay, Let's disable the PM UP THRESHOLD INTRRUPTS, so that we will not get further interrupts until current delay is less than max delay, Also request for the PM DOWN THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and viceversa for PM DOWN THRESHOLD INTRRUPTS.
v2: Use bool variables (Daniel)
v3: Fix Interrupt masking bit (Deepak)
v4: Use existing symbolic constants in i915_reg.h (Daniel)
v5: Add pm interrupt mask after new_delay calculation (Ville)
Signed-off-by Deepak S <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++
 drivers/gpu/drm/i915/i915_irq.c | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.c |  3 +++
 3 files changed, 45 insertions(+)
drivers/gpu/drm/i915/i915_drv.h +3 -0
index 56c720b..f19de66 100644
@@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
 	u8 rp0_delay;
 	u8 hw_max;
 
+	bool rp_up_masked;
+	bool rp_down_masked;
+
 	int last_adj;
 	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
 
drivers/gpu/drm/i915/i915_irq.c +0 -0
index 01a8686..69a5214 100644
@@ -972,6 +972,43 @@ static void notify_ring(struct drm_device *dev,
 	i915_queue_hangcheck(dev);
 }
 

Quoted 5 lines

drivers/gpu/drm/i915/i915_irq.c static void notify_ring(struct drm_device *dev, · line 976
Quoted 2 lines
drivers/gpu/drm/i915/i915_irq.c +2 -0
+static void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
+						u32 pm_iir, int *new_delay)
Just a minor nit here. We don't modify new_delay in this function, so passing by value would be better.

I've fixed this up and merged the patch. I've also polished the whitespace a bit, please run patches through scripts/checkpatch.pl before submitting. I usually don't all go whitespace-nazi about this, but generally the suggestions result in more uniform and hence readable sources.

Quoted 3 lines

Otherwise the patch looks good to me. So if you change that, you can add:
Reviewed-by Ville Syrjälä <[email protected]>

Thanks, Daniel

Signature · 3 lines

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

S, Deepak

author

2014-01-30 07:03 UTC #6 lore raw

Re: [PATCH v5 1/2] drm/i915: Disable/Enable PM Intrrupts based on the current freq.

Quoted 58 lines · Daniel Vetter

Quoted 46 lines · Ville Syrjälä
Quoted 45 lines · On Mon, Jan 27, 2014 at 09:35:05PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When current delay is already at max delay, Let's disable the PM UP THRESHOLD INTRRUPTS, so that we will not get further interrupts until current delay is less than max delay, Also request for the PM DOWN THRESHOLD INTRRUPTS to indicate the decrease in clock freq. and viceversa for PM DOWN THRESHOLD INTRRUPTS.
v2: Use bool variables (Daniel)
v3: Fix Interrupt masking bit (Deepak)
v4: Use existing symbolic constants in i915_reg.h (Daniel)
v5: Add pm interrupt mask after new_delay calculation (Ville)
Signed-off-by Deepak S <[email protected]>
---
  drivers/gpu/drm/i915/i915_drv.h |  3 +++
  drivers/gpu/drm/i915/i915_irq.c | 39 +++++++++++++++++++++++++++++++++++++++
  drivers/gpu/drm/i915/intel_pm.c |  3 +++
  3 files changed, 45 insertions(+)
drivers/gpu/drm/i915/i915_drv.h +3 -0
index 56c720b..f19de66 100644
@@ -943,6 +943,9 @@ struct intel_gen6_power_mgmt {
  	u8 rp0_delay;
  	u8 hw_max;
 
+	bool rp_up_masked;
+	bool rp_down_masked;
+
  	int last_adj;
  	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
 
drivers/gpu/drm/i915/i915_irq.c +0 -0
index 01a8686..69a5214 100644
@@ -972,6 +972,43 @@ static void notify_ring(struct drm_device *dev,
  	i915_queue_hangcheck(dev);
  }
 
Quoted 5 lines
drivers/gpu/drm/i915/i915_irq.c static void notify_ring(struct drm_device *dev, · line 976
Quoted 2 lines
drivers/gpu/drm/i915/i915_irq.c +2 -0
+static void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
+						u32 pm_iir, int *new_delay)
Just a minor nit here. We don't modify new_delay in this function, so passing by value would be better.
I've fixed this up and merged the patch. I've also polished the whitespace a bit, please run patches through scripts/checkpatch.pl before submitting. I usually don't all go whitespace-nazi about this, but generally the suggestions result in more uniform and hence readable sources.

Quoted 5 lines

Quoted 3 lines
Otherwise the patch looks good to me. So if you change that, you can add:
Reviewed-by Ville Syrjälä <[email protected]>
Thanks, Daniel

Thanks Daniel.

[email protected]

author

2014-01-27 16:07 UTC #7 lore raw

PATCH v4 2/2 drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.

In reply to [email protected]

From: Deepak S <[email protected]>

When we enter RC6 and GFX Clocks are off, the voltage remains higher than Vmin. When we try to set the freq to RPn, it might fail since the Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock up and set the freq to RPn then move GFx down.

v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)

v3: Fix the timeout during wait for gfx clock (Jesse)

v4: addressed comments on set freq and punit wait (Ville)

Signed-off-by Deepak S <[email protected]>

---
 drivers/gpu/drm/i915/i915_reg.h |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c | 53 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/i915_reg.h +4 -0

index 242f540..feaa83b 100644
@@ -4944,6 +4944,10 @@
 						 GEN6_PM_RP_DOWN_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_TIMEOUT)
 
+#define VLV_GTLC_SURVIVABILITY_REG              0x130098
+#define VLV_GFX_CLK_STATUS_BIT			(1<<3)
+#define VLV_GFX_CLK_FORCE_ON_BIT		(1<<2)
+
 #define GEN6_GT_GFX_RC6_LOCKED			0x138104
 #define VLV_COUNTER_CONTROL			0x138104
 #define   VLV_COUNT_RANGE_HIGH			(1<<15)

drivers/gpu/drm/i915/intel_pm.c +52 -1

71 lines not on this page · raw message

Signature · 1 line

1.8.5.2

Daniel Vetter

2014-01-27 16:52 UTC #8 lore raw

Re: [PATCH v4 2/2] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.

Quoted 59 lines · On Mon, Jan 27, 2014 at 09:35:06PM +0530, [email protected] wrote:

From: Deepak S <[email protected]>

v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)

v3: Fix the timeout during wait for gfx clock (Jesse)

v4: addressed comments on set freq and punit wait (Ville)

Signed-off-by Deepak S <[email protected]>

---
 drivers/gpu/drm/i915/i915_reg.h |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c | 53 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/i915_reg.h +4 -0

index 242f540..feaa83b 100644
@@ -4944,6 +4944,10 @@
 						 GEN6_PM_RP_DOWN_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_TIMEOUT)
 
+#define VLV_GTLC_SURVIVABILITY_REG              0x130098
+#define VLV_GFX_CLK_STATUS_BIT			(1<<3)
+#define VLV_GFX_CLK_FORCE_ON_BIT		(1<<2)
+
 #define GEN6_GT_GFX_RC6_LOCKED			0x138104
 #define VLV_COUNTER_CONTROL			0x138104
 #define   VLV_COUNT_RANGE_HIGH			(1<<15)

drivers/gpu/drm/i915/intel_pm.c +17 -0

index c6a07c9..84e20d0 100644
@@ -3035,6 +3035,56 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 	trace_intel_gpu_freq_change(val * 50);
 }
 
+/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+{
+	/*
+	 * When we are idle.  Drop to min voltage state.
+	 */
+
+	if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay)
+		return;

drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3064

Quoted 10 lines

drivers/gpu/drm/i915/intel_pm.c +10 -0

+
+	/* Mask turbo interrupt so that they will not come in between */
+	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
+
+	/* Bring up the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
+				VLV_GFX_CLK_FORCE_ON_BIT);
+
+	if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT &

Do we really need an atomic register busy loop here? Afaics this is only called from process context, so the normal wait_for macro should be good enough ...
-Daniel

Quoted 50 lines

drivers/gpu/drm/i915/intel_pm.c +25 -1

+		I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
+			DRM_ERROR("GFX_CLK_ON request timed out\n");
+		return;
+	}
+
+	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, dev_priv->rps.min_delay);
+
+	if (wait_for_atomic(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
+				& GENFREQSTATUS) == 0, 5))
+		DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
+
+	/* Release the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
+				~VLV_GFX_CLK_FORCE_ON_BIT);
+
+	/* Unmask Turbo interrupts */
+	I915_WRITE(GEN6_PMINTRMSK, ~(GEN6_PM_RPS_EVENTS |
+					GEN6_PM_RP_UP_EI_EXPIRED));
+}
+
+
+
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;

@@ -3042,7 +3092,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
 	mutex_lock(&dev_priv->rps.hw_lock);
 	if (dev_priv->rps.enabled) {
 		if (IS_VALLEYVIEW(dev))
-			valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
+			vlv_set_rps_idle(dev_priv);
 		else
 			gen6_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
 		dev_priv->rps.last_adj = 0;

@@ -4273,6 +4323,7 @@ void intel_gpu_ips_teardown(void)
 	i915_mch_dev = NULL;
 	spin_unlock_irq(&mchdev_lock);
 }
+
 static void intel_init_emon(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;

Signature · 6 lines

1.8.5.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 3 lines

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

S, Deepak

author

2014-01-28 14:33 UTC #9 lore raw

Re: [PATCH v4 2/2] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.

Quoted 67 lines · Daniel Vetter

Quoted 66 lines · On Mon, Jan 27, 2014 at 09:35:06PM +0530, [email protected] wrote:

From: Deepak S <[email protected]>

v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)

v3: Fix the timeout during wait for gfx clock (Jesse)

v4: addressed comments on set freq and punit wait (Ville)

Signed-off-by Deepak S <[email protected]>

---
  drivers/gpu/drm/i915/i915_reg.h |  4 ++++
  drivers/gpu/drm/i915/intel_pm.c | 53 ++++++++++++++++++++++++++++++++++++++++-
  2 files changed, 56 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/i915_reg.h +4 -0

index 242f540..feaa83b 100644
@@ -4944,6 +4944,10 @@
  						 GEN6_PM_RP_DOWN_THRESHOLD | \
  						 GEN6_PM_RP_DOWN_TIMEOUT)
 
+#define VLV_GTLC_SURVIVABILITY_REG              0x130098
+#define VLV_GFX_CLK_STATUS_BIT			(1<<3)
+#define VLV_GFX_CLK_FORCE_ON_BIT		(1<<2)
+
  #define GEN6_GT_GFX_RC6_LOCKED			0x138104
  #define VLV_COUNTER_CONTROL			0x138104
  #define   VLV_COUNT_RANGE_HIGH			(1<<15)

drivers/gpu/drm/i915/intel_pm.c +24 -0

index c6a07c9..84e20d0 100644
@@ -3035,6 +3035,56 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
  	trace_intel_gpu_freq_change(val * 50);
  }
 
+/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+{
+	/*
+	 * When we are idle.  Drop to min voltage state.
+	 */
+
+	if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay)
+		return;
+
+	/* Mask turbo interrupt so that they will not come in between */
+	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
+
+	/* Bring up the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) |

Quoted 8 lines

drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3064
Quoted 3 lines
drivers/gpu/drm/i915/intel_pm.c +3 -0
+				VLV_GFX_CLK_FORCE_ON_BIT);
+
+	if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT &
Do we really need an atomic register busy loop here? Afaics this is only called from process context, so the normal wait_for macro should be good enough ...
-Daniel

I agree, the reason why i did the _atomic as we observed delay in scheduling the wait_for and we ended up spending lot of time here. I will wait for other comments, If i dont get any, I will address this comment and submit the patch.

Quoted 51 lines

Quoted 50 lines

drivers/gpu/drm/i915/intel_pm.c +25 -2

+		I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
+			DRM_ERROR("GFX_CLK_ON request timed out\n");
+		return;
+	}
+
+	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, dev_priv->rps.min_delay);
+
+	if (wait_for_atomic(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
+				& GENFREQSTATUS) == 0, 5))
+		DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
+
+	/* Release the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
+				~VLV_GFX_CLK_FORCE_ON_BIT);
+
+	/* Unmask Turbo interrupts */
+	I915_WRITE(GEN6_PMINTRMSK, ~(GEN6_PM_RPS_EVENTS |
+					GEN6_PM_RP_UP_EI_EXPIRED));
+}
+
+
+
  void gen6_rps_idle(struct drm_i915_private *dev_priv)
  {
  	struct drm_device *dev = dev_priv->dev;

@@ -3042,7 +3092,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
  	mutex_lock(&dev_priv->rps.hw_lock);
  	if (dev_priv->rps.enabled) {
  		if (IS_VALLEYVIEW(dev))
-			valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
+			vlv_set_rps_idle(dev_priv);
  		else
  			gen6_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
  		dev_priv->rps.last_adj = 0;

@@ -4273,6 +4323,7 @@ void intel_gpu_ips_teardown(void)
  	i915_mch_dev = NULL;
  	spin_unlock_irq(&mchdev_lock);
  }
+
  static void intel_init_emon(struct drm_device *dev)
  {
  	struct drm_i915_private *dev_priv = dev->dev_private;
--

1.8.5.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Daniel Vetter

2014-01-28 19:33 UTC #10 lore raw

Re: [PATCH v4 2/2] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.

Quoted 78 lines · Deepak

Quoted 67 lines · Daniel Vetter
Quoted 66 lines · On Mon, Jan 27, 2014 at 09:35:06PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When we enter RC6 and GFX Clocks are off, the voltage remains higher than Vmin. When we try to set the freq to RPn, it might fail since the Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock up and set the freq to RPn then move GFx down.
v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)
v3: Fix the timeout during wait for gfx clock (Jesse)
v4: addressed comments on set freq and punit wait (Ville)
Signed-off-by Deepak S <[email protected]>
---
 drivers/gpu/drm/i915/i915_reg.h |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c | 53 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_reg.h +4 -0
index 242f540..feaa83b 100644
@@ -4944,6 +4944,10 @@
 						 GEN6_PM_RP_DOWN_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_TIMEOUT)
 
+#define VLV_GTLC_SURVIVABILITY_REG              0x130098
+#define VLV_GFX_CLK_STATUS_BIT			(1<<3)
+#define VLV_GFX_CLK_FORCE_ON_BIT		(1<<2)
+
 #define GEN6_GT_GFX_RC6_LOCKED			0x138104
 #define VLV_COUNTER_CONTROL			0x138104
 #define   VLV_COUNT_RANGE_HIGH			(1<<15)
drivers/gpu/drm/i915/intel_pm.c +24 -0
index c6a07c9..84e20d0 100644
@@ -3035,6 +3035,56 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 	trace_intel_gpu_freq_change(val * 50);
 }
 
+/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+{
+	/*
+	 * When we are idle.  Drop to min voltage state.
+	 */
+
+	if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay)
+		return;
+
+	/* Mask turbo interrupt so that they will not come in between */
+	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
+
+	/* Bring up the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
Quoted 8 lines
drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3064
Quoted 3 lines
drivers/gpu/drm/i915/intel_pm.c +3 -0
+				VLV_GFX_CLK_FORCE_ON_BIT);
+
+	if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT &
Do we really need an atomic register busy loop here? Afaics this is only called from process context, so the normal wait_for macro should be good enough ...
-Daniel

Quoted 4 lines

I agree, the reason why i did the _atomic as we observed delay in scheduling the wait_for and we ended up spending lot of time here. I will wait for other comments, If i dont get any, I will address this comment and submit the patch.

Now that we're using Chris' idle-clamping rps logic this would be run from a work queue. So I hope that the potential scheduling delays here won't affect things badly. If it does you can stick with the _atomic, but then it needs a comment.

But I really hope that we only call this from worker threads, so shouldn't matter too badly if there's a bit a scheduler delay.
-Daniel

Quoted 52 lines

Quoted 51 lines

drivers/gpu/drm/i915/intel_pm.c +25 -2

+		I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
+			DRM_ERROR("GFX_CLK_ON request timed out\n");
+		return;
+	}
+
+	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, dev_priv->rps.min_delay);
+
+	if (wait_for_atomic(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
+				& GENFREQSTATUS) == 0, 5))
+		DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
+
+	/* Release the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
+				~VLV_GFX_CLK_FORCE_ON_BIT);
+
+	/* Unmask Turbo interrupts */
+	I915_WRITE(GEN6_PMINTRMSK, ~(GEN6_PM_RPS_EVENTS |
+					GEN6_PM_RP_UP_EI_EXPIRED));
+}
+
+
+
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;

@@ -3042,7 +3092,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
 	mutex_lock(&dev_priv->rps.hw_lock);
 	if (dev_priv->rps.enabled) {
 		if (IS_VALLEYVIEW(dev))
-			valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
+			vlv_set_rps_idle(dev_priv);
 		else
 			gen6_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
 		dev_priv->rps.last_adj = 0;

@@ -4273,6 +4323,7 @@ void intel_gpu_ips_teardown(void)
 	i915_mch_dev = NULL;
 	spin_unlock_irq(&mchdev_lock);
 }
+
 static void intel_init_emon(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
--

1.8.5.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 3 lines

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

S, Deepak

author

2014-01-29 04:39 UTC #11 lore raw

Re: [PATCH v4 2/2] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.

Quoted 84 lines · Daniel Vetter

Quoted 78 lines · Deepak
Quoted 67 lines · Daniel Vetter
Quoted 66 lines · On Mon, Jan 27, 2014 at 09:35:06PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When we enter RC6 and GFX Clocks are off, the voltage remains higher than Vmin. When we try to set the freq to RPn, it might fail since the Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock up and set the freq to RPn then move GFx down.
v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)
v3: Fix the timeout during wait for gfx clock (Jesse)
v4: addressed comments on set freq and punit wait (Ville)
Signed-off-by Deepak S <[email protected]>
---
  drivers/gpu/drm/i915/i915_reg.h |  4 ++++
  drivers/gpu/drm/i915/intel_pm.c | 53 ++++++++++++++++++++++++++++++++++++++++-
  2 files changed, 56 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_reg.h +4 -0
index 242f540..feaa83b 100644
@@ -4944,6 +4944,10 @@
  						 GEN6_PM_RP_DOWN_THRESHOLD | \
  						 GEN6_PM_RP_DOWN_TIMEOUT)
 
+#define VLV_GTLC_SURVIVABILITY_REG              0x130098
+#define VLV_GFX_CLK_STATUS_BIT			(1<<3)
+#define VLV_GFX_CLK_FORCE_ON_BIT		(1<<2)
+
  #define GEN6_GT_GFX_RC6_LOCKED			0x138104
  #define VLV_COUNTER_CONTROL			0x138104
  #define   VLV_COUNT_RANGE_HIGH			(1<<15)
drivers/gpu/drm/i915/intel_pm.c +24 -0
index c6a07c9..84e20d0 100644
@@ -3035,6 +3035,56 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
  	trace_intel_gpu_freq_change(val * 50);
  }
 
+/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+{
+	/*
+	 * When we are idle.  Drop to min voltage state.
+	 */
+
+	if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay)
+		return;
+
+	/* Mask turbo interrupt so that they will not come in between */
+	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
+
+	/* Bring up the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
Quoted 8 lines
drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3064
Quoted 3 lines
drivers/gpu/drm/i915/intel_pm.c +3 -0
+				VLV_GFX_CLK_FORCE_ON_BIT);
+
+	if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT &
Do we really need an atomic register busy loop here? Afaics this is only called from process context, so the normal wait_for macro should be good enough ...
-Daniel
Quoted 4 lines
I agree, the reason why i did the _atomic as we observed delay in scheduling the wait_for and we ended up spending lot of time here. I will wait for other comments, If i dont get any, I will address this comment and submit the patch.

Quoted 8 lines

Now that we're using Chris' idle-clamping rps logic this would be run from a work queue. So I hope that the potential scheduling delays here won't affect things badly. If it does you can stick with the _atomic, but then it needs a comment.
But I really hope that we only call this from worker threads, so shouldn't matter too badly if there's a bit a scheduler delay.
-Daniel

I verified and we don't have a scheduling delays, I will change this to _atomic to wait_for

-Deepak

Quoted 53 lines

Quoted 52 lines

Quoted 51 lines

drivers/gpu/drm/i915/intel_pm.c +25 -2

+		I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
+			DRM_ERROR("GFX_CLK_ON request timed out\n");
+		return;
+	}
+
+	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, dev_priv->rps.min_delay);
+
+	if (wait_for_atomic(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
+				& GENFREQSTATUS) == 0, 5))
+		DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
+
+	/* Release the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
+				~VLV_GFX_CLK_FORCE_ON_BIT);
+
+	/* Unmask Turbo interrupts */
+	I915_WRITE(GEN6_PMINTRMSK, ~(GEN6_PM_RPS_EVENTS |
+					GEN6_PM_RP_UP_EI_EXPIRED));
+}
+
+
+
  void gen6_rps_idle(struct drm_i915_private *dev_priv)
  {
  	struct drm_device *dev = dev_priv->dev;

@@ -3042,7 +3092,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
  	mutex_lock(&dev_priv->rps.hw_lock);
  	if (dev_priv->rps.enabled) {
  		if (IS_VALLEYVIEW(dev))
-			valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
+			vlv_set_rps_idle(dev_priv);
  		else
  			gen6_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
  		dev_priv->rps.last_adj = 0;

@@ -4273,6 +4323,7 @@ void intel_gpu_ips_teardown(void)
  	i915_mch_dev = NULL;
  	spin_unlock_irq(&mchdev_lock);
  }
+
  static void intel_init_emon(struct drm_device *dev)
  {
  	struct drm_i915_private *dev_priv = dev->dev_private;
--

1.8.5.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Ville Syrjälä

2014-01-27 17:08 UTC #12 lore raw

Re: [PATCH v4 2/2] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.

In reply to [email protected]

Quoted 49 lines · On Mon, Jan 27, 2014 at 09:35:06PM +0530, [email protected] wrote:

From: Deepak S <[email protected]>
When we enter RC6 and GFX Clocks are off, the voltage remains higher than Vmin. When we try to set the freq to RPn, it might fail since the Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock up and set the freq to RPn then move GFx down.
v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)
v3: Fix the timeout during wait for gfx clock (Jesse)
v4: addressed comments on set freq and punit wait (Ville)
Signed-off-by Deepak S <[email protected]>
---
 drivers/gpu/drm/i915/i915_reg.h |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c | 53 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_reg.h +4 -0
index 242f540..feaa83b 100644
@@ -4944,6 +4944,10 @@
 						 GEN6_PM_RP_DOWN_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_TIMEOUT)
 
+#define VLV_GTLC_SURVIVABILITY_REG              0x130098
+#define VLV_GFX_CLK_STATUS_BIT			(1<<3)
+#define VLV_GFX_CLK_FORCE_ON_BIT		(1<<2)
+
 #define GEN6_GT_GFX_RC6_LOCKED			0x138104
 #define VLV_COUNTER_CONTROL			0x138104
 #define   VLV_COUNT_RANGE_HIGH			(1<<15)
drivers/gpu/drm/i915/intel_pm.c +7 -0
index c6a07c9..84e20d0 100644
@@ -3035,6 +3035,56 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 	trace_intel_gpu_freq_change(val * 50);
 }
 
+/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down

drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3054

Quoted 10 lines

drivers/gpu/drm/i915/intel_pm.c +10 -0

+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+{
+	/*
+	 * When we are idle.  Drop to min voltage state.
+	 */
+
+	if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay)
+		return;

If we're already at min freq I guess there's a good chance we're at the min voltage too. But I'm not sure that's really guaranteed by anything. Maybe it's enough. If not then I guess we should track whether we've already called this function w/o going to higher voltage in between.

Quoted 6 lines

drivers/gpu/drm/i915/intel_pm.c +6 -0

+
+	/* Mask turbo interrupt so that they will not come in between */
+	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
+
+	/* Bring up the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,

drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3070

Quoted 10 lines

drivers/gpu/drm/i915/intel_pm.c +10 -0

+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
+				VLV_GFX_CLK_FORCE_ON_BIT);
+
+	if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT &
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
+			DRM_ERROR("GFX_CLK_ON request timed out\n");
+		return;
+	}
+
+	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, dev_priv->rps.min_delay);

We should update cur_delay to reflect this.

drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3083

Quoted 13 lines

drivers/gpu/drm/i915/intel_pm.c +13 -0

+
+	if (wait_for_atomic(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
+				& GENFREQSTATUS) == 0, 5))
+		DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
+
+	/* Release the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
+				~VLV_GFX_CLK_FORCE_ON_BIT);
+
+	/* Unmask Turbo interrupts */
+	I915_WRITE(GEN6_PMINTRMSK, ~(GEN6_PM_RPS_EVENTS |
+					GEN6_PM_RP_UP_EI_EXPIRED));

Wouldn't that confuse the interrupt masking logic you just introduced in the previous patch?

So looks to me like pretending we got a down threshold interrupt here is all that's needed to keep things in sync. So somehting like:
gen6_set_pm_mask(dev_priv, GEN6_PM_RP_DOWN_THRESHOLD, min_delay);

Quoted 31 lines

drivers/gpu/drm/i915/intel_pm.c +6 -1

+}
+
+
+
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;

@@ -3042,7 +3092,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
 	mutex_lock(&dev_priv->rps.hw_lock);
 	if (dev_priv->rps.enabled) {
 		if (IS_VALLEYVIEW(dev))
-			valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
+			vlv_set_rps_idle(dev_priv);
 		else
 			gen6_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
 		dev_priv->rps.last_adj = 0;

@@ -4273,6 +4323,7 @@ void intel_gpu_ips_teardown(void)
 	i915_mch_dev = NULL;
 	spin_unlock_irq(&mchdev_lock);
 }
+
 static void intel_init_emon(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;

Signature · 6 lines

1.8.5.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

Ville Syrjälä
Intel OTC

S, Deepak

author

2014-01-28 14:18 UTC #13 lore raw

Re: [PATCH v4 2/2] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.

Quoted 57 lines · Ville Syrjälä

Quoted 56 lines · On Mon, Jan 27, 2014 at 09:35:06PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
When we enter RC6 and GFX Clocks are off, the voltage remains higher than Vmin. When we try to set the freq to RPn, it might fail since the Gfx clocks are down. So to fix this in Gfx idle, Bring the GFX clock up and set the freq to RPn then move GFx down.
v2: remove vlv_update_rps_cur_delay function. Update commit message (Daniel)
v3: Fix the timeout during wait for gfx clock (Jesse)
v4: addressed comments on set freq and punit wait (Ville)
Signed-off-by Deepak S <[email protected]>
---
  drivers/gpu/drm/i915/i915_reg.h |  4 ++++
  drivers/gpu/drm/i915/intel_pm.c | 53 ++++++++++++++++++++++++++++++++++++++++-
  2 files changed, 56 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_reg.h +4 -0
index 242f540..feaa83b 100644
@@ -4944,6 +4944,10 @@
  						 GEN6_PM_RP_DOWN_THRESHOLD | \
  						 GEN6_PM_RP_DOWN_TIMEOUT)
 
+#define VLV_GTLC_SURVIVABILITY_REG              0x130098
+#define VLV_GFX_CLK_STATUS_BIT			(1<<3)
+#define VLV_GFX_CLK_FORCE_ON_BIT		(1<<2)
+
  #define GEN6_GT_GFX_RC6_LOCKED			0x138104
  #define VLV_COUNTER_CONTROL			0x138104
  #define   VLV_COUNT_RANGE_HIGH			(1<<15)
drivers/gpu/drm/i915/intel_pm.c +14 -0
index c6a07c9..84e20d0 100644
@@ -3035,6 +3035,56 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
  	trace_intel_gpu_freq_change(val * 50);
  }
 
+/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 4. Clear the Force GFX CLK ON bit so that Gfx can down
+ * 5. Unmask Turbo interrupts
+*/
+static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
+{
+	/*
+	 * When we are idle.  Drop to min voltage state.
+	 */

Quoted 8 lines

drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3054
Quoted 3 lines
drivers/gpu/drm/i915/intel_pm.c +3 -0
+
+	if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay)
+		return;
If we're already at min freq I guess there's a good chance we're at the min voltage too. But I'm not sure that's really guaranteed by anything. Maybe it's enough. If not then I guess we should track whether we've already called this function w/o going to higher voltage in between.

If we are already in min_freq we will just return right? Only if we have crossed the min_delay and if the gpu is idle we are setting is freq back to min.

Quoted 10 lines

drivers/gpu/drm/i915/intel_pm.c +10 -0

+
+	/* Mask turbo interrupt so that they will not come in between */
+	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
+
+	/* Bring up the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) |
+				VLV_GFX_CLK_FORCE_ON_BIT);
+
+	if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT &

Quoted 8 lines

drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3070
Quoted 6 lines
drivers/gpu/drm/i915/intel_pm.c +6 -0
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 5)) {
+			DRM_ERROR("GFX_CLK_ON request timed out\n");
+		return;
+	}
+
+	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, dev_priv->rps.min_delay);
We should update cur_delay to reflect this.

I will fix this issue.

Quoted 53 lines

drivers/gpu/drm/i915/intel_pm.c void gen6_set_rps(struct drm_device *dev, u8 val) · line 3083
Quoted 13 lines
drivers/gpu/drm/i915/intel_pm.c +13 -0
+
+	if (wait_for_atomic(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
+				& GENFREQSTATUS) == 0, 5))
+		DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
+
+	/* Release the Gfx clock */
+	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
+				~VLV_GFX_CLK_FORCE_ON_BIT);
+
+	/* Unmask Turbo interrupts */
+	I915_WRITE(GEN6_PMINTRMSK, ~(GEN6_PM_RPS_EVENTS |
+					GEN6_PM_RP_UP_EI_EXPIRED));
Wouldn't that confuse the interrupt masking logic you just introduced in the previous patch?
So looks to me like pretending we got a down threshold interrupt here is all that's needed to keep things in sync. So somehting like:
gen6_set_pm_mask(dev_priv, GEN6_PM_RP_DOWN_THRESHOLD, min_delay);
Quoted 31 lines
drivers/gpu/drm/i915/intel_pm.c +6 -2
+}
+
+
+
  void gen6_rps_idle(struct drm_i915_private *dev_priv)
  {
  	struct drm_device *dev = dev_priv->dev;
@@ -3042,7 +3092,7 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
  	mutex_lock(&dev_priv->rps.hw_lock);
  	if (dev_priv->rps.enabled) {
  		if (IS_VALLEYVIEW(dev))
-			valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
+			vlv_set_rps_idle(dev_priv);
  		else
  			gen6_set_rps(dev_priv->dev, dev_priv->rps.min_delay);
  		dev_priv->rps.last_adj = 0;
@@ -4273,6 +4323,7 @@ void intel_gpu_ips_teardown(void)
  	i915_mch_dev = NULL;
  	spin_unlock_irq(&mchdev_lock);
  }
+
  static void intel_init_emon(struct drm_device *dev)
  {
  	struct drm_i915_private *dev_priv = dev->dev_private;
--
1.8.5.2
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx