VLV Turbo/rps + RC6 workaround

PATCH Superseded [email protected] 3 participants 10 messages Jan 9, 2014 – Jan 16, 2014

Subsystems (2)

DRM drivers Intel i915

lore

Share & tools

lore

[email protected]

author

2014-01-09 14:00 UTC #0 lore raw

v2 cover VLV Turbo/rps + RC6 workaround

From: Deepak S <[email protected]>

Below patches addes WA to set Gfx freq while clock are down. Also, added a WA to enables both RC6 and Turbo work together.

Deepak S (3):
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.
drm/i915/vlv: WA for Turbo and RC6 to work together.

 drivers/gpu/drm/i915/i915_drv.h |  16 ++++
 drivers/gpu/drm/i915/i915_irq.c | 182 ++++++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h |  23 +++++
 drivers/gpu/drm/i915/intel_pm.c | 102 +++++++++++++++++++---
 4 files changed, 304 insertions(+), 19 deletions(-)

Signature · 1 line

1.8.4.2

[email protected]

author

2014-01-09 14:01 UTC #1 lore raw

PATCH v3 1/3 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)

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

---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++
 drivers/gpu/drm/i915/i915_irq.c | 31 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_pm.c |  3 +++
 3 files changed, 35 insertions(+), 2 deletions(-)

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

index cc8afff..d49e674 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 +29 -2

index 1d44c79..e87d47a 100644
@@ -988,7 +988,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			adj *= 2;
 		else
 			adj = 1;
-		new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.cur_delay >= dev_priv->rps.max_delay) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) | 1 << 5);
+			dev_priv->rps.rp_up_masked = true;
+			new_delay = dev_priv->rps.cur_delay;
+		} else
+			new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.rp_down_masked) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) & ~(1 << 4));
+			dev_priv->rps.rp_down_masked = false;
+		}
 
 		/*
 		 * For better performance, jump directly

@@ -1007,7 +1020,21 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			adj *= 2;
 		else
 			adj = -1;
-		new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) | 1 << 4);
+			dev_priv->rps.rp_down_masked = true;
+			new_delay = dev_priv->rps.cur_delay;
+		} else
+			new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.rp_up_masked) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) & ~(1 << 5));
+			dev_priv->rps.rp_up_masked = false;
+		}
+
 	} else { /* unknown event */
 		new_delay = dev_priv->rps.cur_delay;
 	}

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

index 469170c..9c950e4 100644
@@ -3628,6 +3628,9 @@ static void valleyview_enable_rps(struct drm_device *dev)
 			 vlv_gpu_freq(dev_priv, dev_priv->rps.rpe_delay),
 			 dev_priv->rps.rpe_delay);
 
+	dev_priv->rps.rp_up_masked = false;
+	dev_priv->rps.rp_down_masked = false;
+
 	valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
 
 	gen6_enable_rps_interrupts(dev);

Signature · 1 line

1.8.4.2

Jesse Barnes

Reviewed-by on v3 1/3

2014-01-13 23:01 UTC #2 lore raw

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

Quoted 46 lines · On Thu, 9 Jan 2014 19:31:08 +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)
Signed-off-by Deepak S <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++
 drivers/gpu/drm/i915/i915_irq.c | 31 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_pm.c |  3 +++
 3 files changed, 35 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +3 -0
index cc8afff..d49e674 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 +4 -1
index 1d44c79..e87d47a 100644
@@ -988,7 +988,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			adj *= 2;
 		else
 			adj = 1;
-		new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.cur_delay >= dev_priv->rps.max_delay) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) | 1 << 5);

drivers/gpu/drm/i915/i915_irq.c static void gen6_pm_rps_work(struct work_struct *work) · line 1004

Quoted 10 lines

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

+			dev_priv->rps.rp_up_masked = true;
+			new_delay = dev_priv->rps.cur_delay;
+		} else
+			new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.rp_down_masked) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) & ~(1 << 4));
+			dev_priv->rps.rp_down_masked = false;
+		}

I've looked at this too much and confused myself. It looks like the old logic of the delay numbers being in reverse order has changed (or maybe that was just ILK?)... so I guess the above
is ok (I was afraid we'd end up masking on the first interrupt everytime).

So:

Reviewed-by Jesse Barnes <[email protected]>

Thanks,
Jesse

Daniel Vetter

2014-01-14 09:21 UTC #3 lore raw

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

In reply to [email protected]

Quoted 38 lines · On Thu, Jan 09, 2014 at 07:31:08PM +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)
Signed-off-by Deepak S <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++
 drivers/gpu/drm/i915/i915_irq.c | 31 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_pm.c |  3 +++
 3 files changed, 35 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +3 -0
index cc8afff..d49e674 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 1d44c79..e87d47a 100644

drivers/gpu/drm/i915/i915_irq.c static void gen6_pm_rps_work(struct work_struct *work) · line 994

Quoted 8 lines

drivers/gpu/drm/i915/i915_irq.c +4 -1

@@ -988,7 +988,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			adj *= 2;
 		else
 			adj = 1;
-		new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.cur_delay >= dev_priv->rps.max_delay) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) | 1 << 5);

Please use the symbolic constants in i915_reg.h, we can reuse the ones below GEN6_PMIER. Also Chris had some comment to move this code around, but I didn't really understand what he wants. Chris, can you please clarify?

Thanks, Daniel

Quoted 57 lines

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

+			dev_priv->rps.rp_up_masked = true;
+			new_delay = dev_priv->rps.cur_delay;
+		} else
+			new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.rp_down_masked) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) & ~(1 << 4));
+			dev_priv->rps.rp_down_masked = false;
+		}
 
 		/*
 		 * For better performance, jump directly

@@ -1007,7 +1020,21 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			adj *= 2;
 		else
 			adj = -1;
-		new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) | 1 << 4);
+			dev_priv->rps.rp_down_masked = true;
+			new_delay = dev_priv->rps.cur_delay;
+		} else
+			new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.rp_up_masked) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) & ~(1 << 5));
+			dev_priv->rps.rp_up_masked = false;
+		}
+
 	} else { /* unknown event */
 		new_delay = dev_priv->rps.cur_delay;
 	}

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

index 469170c..9c950e4 100644
@@ -3628,6 +3628,9 @@ static void valleyview_enable_rps(struct drm_device *dev)
 			 vlv_gpu_freq(dev_priv, dev_priv->rps.rpe_delay),
 			 dev_priv->rps.rpe_delay);
 
+	dev_priv->rps.rp_up_masked = false;
+	dev_priv->rps.rp_down_masked = false;
+
 	valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
 
 	gen6_enable_rps_interrupts(dev);

Signature · 6 lines

1.8.4.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-14 09:23 UTC #4 lore raw

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

Thanks Daniel. I will wait for Chris feedback and then address the comments.

-----Original Message-----
From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter
Sent: Tuesday, January 14, 2014 2:52 PM
To: S, Deepak
Cc: [email protected]; Chris Wilson
Subject: Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: Disable/Enable PM Intrrupts based on the current freq.

Quoted 38 lines · On Thu, Jan 09, 2014 at 07:31:08PM +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)
Signed-off-by Deepak S <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++  
drivers/gpu/drm/i915/i915_irq.c | 31 +++++++++++++++++++++++++++++--  
drivers/gpu/drm/i915/intel_pm.c |  3 +++
 3 files changed, 35 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/drivers/gpu/drm/i915/i915_drv.h index cc8afff..d49e674 100644
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;
 
(quoted diff) +0 -0
b/drivers/gpu/drm/i915/i915_irq.c index 1d44c79..e87d47a 100644

drivers/gpu/drm/i915/i915_irq.c static void gen6_pm_rps_work(struct work_struct *work) · line 994

Quoted 8 lines

drivers/gpu/drm/i915/i915_irq.c +4 -1

@@ -988,7 +988,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			adj *= 2;
 		else
 			adj = 1;
-		new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.cur_delay >= dev_priv->rps.max_delay) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) | 1 << 5);

Thanks, Daniel

Quoted 53 lines

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

+			dev_priv->rps.rp_up_masked = true;
+			new_delay = dev_priv->rps.cur_delay;
+		} else
+			new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.rp_down_masked) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) & ~(1 << 4));
+			dev_priv->rps.rp_down_masked = false;
+		}
 
 		/*
 		 * For better performance, jump directly @@ -1007,7 +1020,21 @@

static void gen6_pm_rps_work(struct work_struct *work)

drivers/gpu/drm/i915/i915_irq.c +15 -1

 			adj *= 2;
 		else
 			adj = -1;
-		new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) | 1 << 4);
+			dev_priv->rps.rp_down_masked = true;
+			new_delay = dev_priv->rps.cur_delay;
+		} else
+			new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.rp_up_masked) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) & ~(1 << 5));
+			dev_priv->rps.rp_up_masked = false;
+		}
+
 	} else { /* unknown event */
 		new_delay = dev_priv->rps.cur_delay;
 	}

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/intel_pm.c index 469170c..9c950e4 100644

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

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

1.8.4.2

Quoted 4 lines

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

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

S, Deepak

author

2014-01-16 05:48 UTC #5 lore raw

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

In reply to Daniel Vetter

Quoted 1 line

Please use the symbolic constants in i915_reg.h, we can reuse the ones below GEN6_PMIER. Also Chris had some comment to move this code around, but I didn't really understand what he wants. Chris, can you please clarify?

Hi Chris,

Can you please clarify on this?

Thanks
Deepak

Quoted 38 lines · On Thu, Jan 09, 2014 at 07:31:08PM +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)
Signed-off-by Deepak S <[email protected]>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 +++  
drivers/gpu/drm/i915/i915_irq.c | 31 +++++++++++++++++++++++++++++--  
drivers/gpu/drm/i915/intel_pm.c |  3 +++
 3 files changed, 35 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/drivers/gpu/drm/i915/i915_drv.h index cc8afff..d49e674 100644
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;
 
(quoted diff) +0 -0
b/drivers/gpu/drm/i915/i915_irq.c index 1d44c79..e87d47a 100644

drivers/gpu/drm/i915/i915_irq.c static void gen6_pm_rps_work(struct work_struct *work) · line 994

Quoted 8 lines

drivers/gpu/drm/i915/i915_irq.c +4 -1

@@ -988,7 +988,20 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			adj *= 2;
 		else
 			adj = 1;
-		new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.cur_delay >= dev_priv->rps.max_delay) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) | 1 << 5);

Thanks, Daniel

Quoted 53 lines

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

+			dev_priv->rps.rp_up_masked = true;
+			new_delay = dev_priv->rps.cur_delay;
+		} else
+			new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.rp_down_masked) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) & ~(1 << 4));
+			dev_priv->rps.rp_down_masked = false;
+		}
 
 		/*
 		 * For better performance, jump directly @@ -1007,7 +1020,21 @@

static void gen6_pm_rps_work(struct work_struct *work)

drivers/gpu/drm/i915/i915_irq.c +15 -1

 			adj *= 2;
 		else
 			adj = -1;
-		new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.cur_delay <= dev_priv->rps.min_delay) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) | 1 << 4);
+			dev_priv->rps.rp_down_masked = true;
+			new_delay = dev_priv->rps.cur_delay;
+		} else
+			new_delay = dev_priv->rps.cur_delay + adj;
+
+		if (dev_priv->rps.rp_up_masked) {
+			I915_WRITE(GEN6_PMINTRMSK,
+					I915_READ(GEN6_PMINTRMSK) & ~(1 << 5));
+			dev_priv->rps.rp_up_masked = false;
+		}
+
 	} else { /* unknown event */
 		new_delay = dev_priv->rps.cur_delay;
 	}

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/intel_pm.c index 469170c..9c950e4 100644

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

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

1.8.4.2

Quoted 4 lines

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

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

[email protected]

author

2014-01-09 14:01 UTC #6 lore raw

PATCH v2 2/3 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 RPe, 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 RPe then move GFx down.

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

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

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

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

index a699efd..e37831f 100644
@@ -4940,6 +4940,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 +47 -1

66 lines not on this page · raw message

Signature · 1 line

1.8.4.2

Jesse Barnes

2014-01-13 22:53 UTC #7 lore raw

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

Quoted 96 lines · On Thu, 9 Jan 2014 19:31:09 +0530 [email protected] wrote:

From: Deepak S <[email protected]>

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

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

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

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

index a699efd..e37831f 100644
@@ -4940,6 +4940,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 +46 -1

index 9c950e4..a8e05fe 100644
@@ -3050,6 +3050,51 @@ 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 Rpe if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpe 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.rpe_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) |
+				VLV_GFX_CLK_FORCE_ON_BIT);
+
+	if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT &
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 500)) {
+			DRM_ERROR("GFX_CLK_ON request timed out\n");
+		return;
+	}
+
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
+
+	/* 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);
+}
+
+
+
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;

@@ -3057,7 +3102,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;

drivers/gpu/drm/i915/intel_pm.c void intel_gpu_ips_teardown(void) · line 4339

Quoted 7 lines

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

@@ -4288,6 +4333,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;

Yeah if we need to bring the gfx clocks up (which makes sense) then I guess we need this. I'm not sure about the wait on the punit though; that could end up penalizing us in bursty workloads, since the punit can take quite some time to update the freq, but I don't actually see a wait here?

Also, is the 500ms timeout really required for the gfx clock? That's a long time to potentially hold the mutex and delay any clock boosting or other activity...

Signature · 1 line

Jesse Barnes, Intel Open Source Technology Center

S, Deepak

author

2014-01-14 09:13 UTC #8 lore raw

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

Quoted 2 lines

Yeah if we need to bring the gfx clocks up (which makes sense) then I guess we need this. I'm not sure about the wait on the punit though; that could end up penalizing us in bursty workloads, since the punit can take quite some time to update the freq, but I don't actually see a wait here?

Ville suggested we don't need wait after requesting the freq. " AFAIK the punit will recheck the situation periodically, and it will try to use PUNIT_REG_GPU_FREQ_REQ."

Quoted 1 line

Also, is the 500ms timeout really required for the gfx clock? That's a long time to potentially hold the mutex and delay any clock boosting or other activity...

Yes agree, we don't need 500ms. I will change the delay to 5ms and submit a new command.

Thanks
Deepak

-----Original Message-----
From: Jesse Barnes [mailto:[email protected]]
Sent: Tuesday, January 14, 2014 4:23 AM
To: S, Deepak
Cc: [email protected]
Subject: Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/vlv: WA to fix Voltage not getting dropped to Vmin when Gfx is power gated.

Quoted 94 lines · On Thu, 9 Jan 2014 19:31:09 +0530 [email protected] wrote:

From: Deepak S <[email protected]>

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

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

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

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/i915_reg.h index a699efd..e37831f 100644

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

@@ -4940,6 +4940,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)

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/intel_pm.c index 9c950e4..a8e05fe 100644

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

@@ -3050,6 +3050,51 @@ 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 Rpe if Gfx clocks are down
+ *
+ * * If Gfx is Idle, then
+ * 1. Mask Turbo interrupts
+ * 2. Bring up Gfx clock
+ * 3. Change the freq to Rpe 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.rpe_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) |
+				VLV_GFX_CLK_FORCE_ON_BIT);
+
+	if (wait_for_atomic(((VLV_GFX_CLK_STATUS_BIT &
+		I915_READ(VLV_GTLC_SURVIVABILITY_REG)) != 0), 500)) {
+			DRM_ERROR("GFX_CLK_ON request timed out\n");
+		return;
+	}
+
+	valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
+
+	/* 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); }
+
+
+
 void gen6_rps_idle(struct drm_i915_private *dev_priv)  {
 	struct drm_device *dev = dev_priv->dev; @@ -3057,7 +3102,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;

drivers/gpu/drm/i915/intel_pm.c void intel_gpu_ips_teardown(void) · line 4338

Quoted 6 lines

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

@@ -4288,6 +4333,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;

Also, is the 500ms timeout really required for the gfx clock? That's a long time to potentially hold the mutex and delay any clock boosting or other activity...

--
Jesse Barnes, Intel Open Source Technology Center

Quoted 70 lines · On Sun, Dec 08, 2013 at 02:16:43PM +0530, [email protected] wrote:

From: Deepak S <[email protected]>

on VLV, P-Unit doesn't garauntee that last requested freq by driver is actually the current running frequency. We need to make sure we update the cur freq. before requesitng new freq.

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

---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_irq.c |  8 ++++++++
 drivers/gpu/drm/i915/intel_pm.c | 31 +++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

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

index 780f815..a62ac0c 100644
@@ -2416,6 +2416,7 @@ extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
 extern void intel_init_pch_refclk(struct drm_device *dev);
 extern void gen6_set_rps(struct drm_device *dev, u8 val);
 extern void valleyview_set_rps(struct drm_device *dev, u8 val);
+extern bool vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv);
 extern int valleyview_rps_max_freq(struct drm_i915_private *dev_priv);
 extern int valleyview_rps_min_freq(struct drm_i915_private *dev_priv);
 extern void intel_detect_pch(struct drm_device *dev);

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

index 2715600..4bde03a 100644
@@ -982,6 +982,14 @@ static void gen6_pm_rps_work(struct work_struct *work)
 
      mutex_lock(&dev_priv->rps.hw_lock);
 
+     /* Make sure we have current freq updated properly. Doing this
+      * here becuase, on VLV, P-Unit doesnt garauntee that last requested
+      * freq by driver is actually the current running frequency
+      */
 
+
+     if (IS_VALLEYVIEW(dev_priv->dev))
+             vlv_update_rps_cur_delay(dev_priv);
+
      adj = dev_priv->rps.last_adj;
      if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {

              if (adj > 0)

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

index e6d98fe..7f6c747 100644
@@ -3607,6 +3607,35 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv)
      mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+/*
+ * Wait until the previous freq change has completed,
+ * or the timeout elapsed, and then update our notion
+ * of the current GPU frequency.
+ */
+bool vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv)
+{
+     u32 pval;
+
+     WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
+
+     if (wait_for(((pval = vlv_punit_read(dev_priv,
+                             PUNIT_REG_GPU_FREQ_STS)) &
+                                     GENFREQSTATUS) == 0, 10))
+             DRM_DEBUG_DRIVER("timed out waiting for Punit\n");
+
+     pval >>= 8;

drivers/gpu/drm/i915/intel_pm.c void gen6_rps_boost(struct drm_i915_private *dev_priv) · line 3636

Quoted 10 lines

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

+
+     if (pval != dev_priv->rps.cur_delay)
+             DRM_DEBUG_DRIVER("Punit overrode GPU freq: %d MHz (%u) requested, but got %d Mhz (%u)\n",
+                             vlv_gpu_freq(dev_priv, dev_priv->rps.cur_delay),
+                             dev_priv->rps.cur_delay,
+                             vlv_gpu_freq(dev_priv, pval), pval);
+
+     dev_priv->rps.cur_delay = pval;
+     return true;
+}

I just killed this guys a while ago. If you think we need to resurrect it, you should do it w/ git revert to make it clear where it came from.

But I'd want more justification than what you have provided. My understanding is that PUNIT_REG_GPU_FREQ_STS alwasy reflects the current operating frequency of the GPU, and that can be affected by thermal conditions (and media turbo, which I'll ignore for simplicity) in addition to the frequency requested by the driver. AFAIK the punit will recheck the situation periodically, and it will try to use PUNIT_REG_GPU_FREQ_REQ. It will check the thermal conditions to figure out if it needs to further limit the frequency. Once the thermal conditions permit it, the frequency should return back to the last requested turbo frequency, without the driver having to rewrite PUNIT_REG_GPU_FREQ_REQ.

If I'm right updating cur_delay based on PUNIT_REG_GPU_FREQ_STS is clearly the wrong thing to do. So I think we need more details on what the punit does in order to figure out what's the right thing to do here.

Quoted 17 lines

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

+
+
 void valleyview_set_rps(struct drm_device *dev, u8 val)
 {
      struct drm_i915_private *dev_priv = dev->dev_private;

@@ -3615,6 +3644,8 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
      WARN_ON(val > dev_priv->rps.max_delay);
      WARN_ON(val < dev_priv->rps.min_delay);
 
+     vlv_update_rps_cur_delay(dev_priv);
+
      DRM_DEBUG_DRIVER("GPU freq request from %d MHz (%u) to %d MHz (%u)\n",
                       vlv_gpu_freq(dev_priv, dev_priv->rps.cur_delay),
                       dev_priv->rps.cur_delay,
--

1.8.4.2

Quoted 4 lines

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

--
Ville Syrjälä
Intel OTC

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

[email protected]

author

2014-01-09 14:01 UTC #9 lore raw

PATCH v2 3/3 drm/i915/vlv: WA for Turbo and RC6 to work together.

In reply to [email protected]

From: Deepak S <[email protected]>

With RC6 enabled, BYT has an HW issue in determining the right Gfx busyness.
WA for Turbo + RC6: Use SW based Gfx busy-ness detection to decide on increasing/decreasing the freq. This logic will monitor C0 counters of render/media power-wells over EI period and takes necessary action based on these values

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

---
 drivers/gpu/drm/i915/i915_drv.h |  13 ++++
 drivers/gpu/drm/i915/i915_irq.c | 151 ++++++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h |  19 +++++
 drivers/gpu/drm/i915/intel_pm.c |  53 ++++++++++----
 4 files changed, 219 insertions(+), 17 deletions(-)

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

index d49e674..0bb4aa6 100644
@@ -942,10 +942,23 @@ struct intel_gen6_power_mgmt {
 	u8 rp1_delay;
 	u8 rp0_delay;
 	u8 hw_max;
+	u8 hw_min;
 
 	bool rp_up_masked;
 	bool rp_down_masked;
 
+	u32 cz_freq;
+	u32 ei_interrupt_count;
+
+	u32 cz_ts_up_ei;
+	u32 render_up_EI_C0;
+	u32 media_up_EI_C0;
+	u32 cz_ts_down_ei;
+	u32 render_down_EI_C0;
+	u32 media_down_EI_C0;
+
+	bool use_RC0_residency_for_turbo;
+
 	int last_adj;
 	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;

drivers/gpu/drm/i915/i915_irq.c +146 -5

192 lines not on this page · raw message

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

index e37831f..370e8d6 100644
@@ -391,6 +391,7 @@
 #define PUNIT_REG_GPU_FREQ_STS			0xd8
 #define   GENFREQSTATUS				(1<<0)
 #define PUNIT_REG_MEDIA_TURBO_FREQ_REQ		0xdc
+#define PUNIT_REG_CZ_TIMESTAMP			0xce
 
 #define PUNIT_FUSE_BUS2				0xf6 /* bits 47:40 */
 #define PUNIT_FUSE_BUS1				0xf5 /* bits 55:48 */

@@ -406,6 +407,15 @@
 #define   FB_FMAX_VMIN_FREQ_LO_SHIFT		27
 #define   FB_FMAX_VMIN_FREQ_LO_MASK		0xf8000000
 
+#define VLV_CZ_CLOCK_FREQ_DDR_MODE_800		200000000
+#define VLV_CZ_CLOCK_FREQ_DDR_MODE_1066	266666666
+#define VLV_CZ_CLOCK_FREQ_DDR_MODE_1333	333333333
+
+#define VLV_CZ_CLOCK_TO_MILLI_SEC		100000
+#define VLV_RP_UP_EI_THRESHOLD			90
+#define VLV_RP_DOWN_EI_THRESHOLD		70
+#define VLV_INT_COUNT_FOR_DOWN_EI		5
+
 /* vlv2 north clock has */
 #define CCK_FUSE_REG				0x8
 #define  CCK_FUSE_HPLL_FREQ_MASK		0x3

@@ -4822,6 +4832,7 @@
 #define  FORCEWAKE_ACK				0x130090
 #define  VLV_GTLC_WAKE_CTRL			0x130090
 #define  VLV_GTLC_PW_STATUS			0x130094
+#define VLV_GTLC_SURVIVABILITY_REG              0x130098
 #define  FORCEWAKE_MT				0xa188 /* multi-threaded */
 #define   FORCEWAKE_KERNEL			0x1
 #define   FORCEWAKE_USER			0x2

@@ -4829,6 +4840,11 @@
 #define  ECOBUS					0xa180
 #define    FORCEWAKE_MT_ENABLE			(1<<5)
 
+#define VLV_GFX_CLK_FORCE_ON_BIT                (1<<2)
+#define VLV_GFX_CLK_STATUS_BIT                  (1<<3)
+
+#define VLV_RC_COUNTER_CONTROL                  0xFFFF00FF
+
 #define  GTFIFODBG				0x120000
 #define    GT_FIFO_SBDROPERR			(1<<6)
 #define    GT_FIFO_BLOBDROPERR			(1<<5)

@@ -4944,6 +4960,9 @@
 #define VLV_GFX_CLK_STATUS_BIT			(1<<3)
 #define VLV_GFX_CLK_FORCE_ON_BIT		(1<<2)
 
+#define VLV_RENDER_C0_COUNT_REG		0x138118
+#define VLV_MEDIA_C0_COUNT_REG			0x13811C
+
 #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 +41 ~~-12~~

119 lines not on this page · raw message

Signature · 1 line

1.8.4.2