[Intel-gfx] [RFC] drm/i915: _wait_for might be called when irq is off
From: Deepak S
Sometimes, i915 might call _wait_for when irq is disabled.
If the cpu is the main cpu to process jiffies, jiffies
wouldn't be increased as this cpu disables irq. Then,
time_after(jiffies, timeout__) becomes meaningless. If
gunit doesn't work now, kernel wouldn't exit as the timeout
doesn't work.
The patch fixes it by using sched_clock instead of jiffies.
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_drv.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6036e3b..2c6ebce 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -49,10 +49,12 @@
* we've never had a chance to check the condition before the timeout.
*/
#define _wait_for(COND, MS, W) ({ \
- unsigned long timeout__ = jiffies + msecs_to_jiffies(MS) + 1; \
+ u64 timeout_ = sched_clock() + MS * ((u64) NSEC_PER_MSEC); \
+ u64 clock; \
int ret__ = 0; \
while (!(COND)) { \
- if (time_after(jiffies, timeout__)) { \
+ clock = sched_clock(); \
+ if (clock >= timeout_) {\
if (!(COND))\
ret__ = -ETIMEDOUT; \
break; \
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clean-up idr table if context create fails.
On Tuesday 07 April 2015 02:02 PM, Chris Wilson wrote: On Tue, Apr 07, 2015 at 10:20:15AM +0200, Daniel Vetter wrote: On Thu, Apr 02, 2015 at 06:49:38PM +0530, Deepak S wrote: On Monday 30 March 2015 09:13 PM, Daniel Vetter wrote: On Mon, Mar 30, 2015 at 08:03:58PM +0530, deepa...@linux.intel.com wrote: From: Deepak S Cleanup idr table if any error happens after __create_hw_context() in i915_gem_create_context() Signed-off-by: Deepak S --- drivers/gpu/drm/i915/i915_gem_context.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index f3e84c4..69bebe5 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -287,6 +287,8 @@ err_unpin: if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state); err_destroy: + if (ctx->file_priv) + idr_remove(&ctx->file_priv->context_idr, ctx->user_handle); The common approach is to add a new err_idr: label at the op of the unwind code and make the call to idr_remove unconditional. Thanks, Daniel Thanks Daniel for review. I do not think we can have a unconditional idr remove since for global ctx i915_gem_create_context called with file_priv=NULL? Hm right, the entire control-flow in there is a bit funny. I think a much cleaner solution would be to drop the file_prive from create_context and add a new i915_gem_context_create_user which wraps create_context and the idr allocation. Doing the cleanup, conditionally, in a different function than where we do the allocation is a bit too brittle imo. I suggested that it look like: http://cgit.freedesktop.org/~ickle/linux-2.6/tree/drivers/gpu/drm/i915/i915_gem_context.c#n179 -Chris Thanks Chris and Daniel, I will submit cleaned up patches ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/chv: Implement WaDisableShadowRegForCpd
From: Deepak S
This WA disable usage of shadow register during CPD/RC6 transactions on
CHV
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 4
2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c97842..bcdb16b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6206,6 +6206,7 @@ enum skl_disp_power_wells {
#define GTFIFOCTL 0x120008
#defineGT_FIFO_FREE_ENTRIES_MASK 0x7f
#defineGT_FIFO_NUM_RESERVED_ENTRIES20
+#define GT_FIFO_CTL_BLOCK_POLICY (3<<11)
#define HSW_IDICR 0x9008
#defineIDIHASHMSK(x) (((x) & 0x3f) << 16)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4dd8b41..b9d3c00 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6431,6 +6431,10 @@ static void cherryview_init_clock_gating(struct
drm_device *dev)
/* WaDisableSDEUnitClockGating:chv */
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
+
+ /* WaDisableShadowRegForCpd */
+ I915_WRITE(GTFIFOCTL, I915_READ(GTFIFOCTL) |
+ GT_FIFO_CTL_BLOCK_POLICY);
}
static void g4x_init_clock_gating(struct drm_device *dev)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Disable WaGsvRC0ResidencyMethod for vlv
On Monday 13 April 2015 05:36 PM, Jani Nikula wrote: On Thu, 19 Mar 2015, Daniel Vetter wrote: On Thu, Mar 19, 2015 at 03:38:19PM +0200, David Weinehall wrote: On Thu, Mar 19, 2015 at 06:17:00PM +0530, Deepak S wrote: On Thursday 19 March 2015 05:14 PM, David Weinehall wrote: On Thu, Mar 19, 2015 at 04:09:44PM +0530, deepa...@linux.intel.com wrote: From: Deepak S Unfortunately WaGsvRC0ResidencyMethod causing system freeze on some of the baytrail systems :(. Switching back to legacy mode rps. Is there any way to identify either what systems it's OK to use on, or to identif what Baytrail systems it isn't OK to use on? Just reverting this completely seems overly broad if it's possible to tell the difference between working and non-working systems. Restricting the changes to few system will be the right way to go. How do we get details of now working system? That depends. Who reported the non-working Baytrail-systems? And just as importantly, do we have Baytrail systems where this is confirmed to work? If we can identify the respective revisions of the systems we might be able to discern the pattern and continue from there. My experience with random crashes with rps on snb/ivb is that there's no solid systems, just systems where no one has bothered to be really evil yet. I'd be really cautious with whitelisting since it tends to not really solve the underlying bug (if there is one), usually blows up really fast in size and is long-term unmaintainable. We've been there a few times with other hacks and issues :( Plenty of talk but no final verdict on the patch either way, and the bug is still open too. :( Deepak, is this patch still needed? BR, Jani. Hi Jani, Chris has cleaned up patches for "WA for Turbo and RC6 to work" & these patches are helping to resolve freeze issue. We can ignore this patch. :) Thanks Deepak -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/chv: Implement WaDisableShadowRegForCpd
On Tuesday 14 April 2015 04:29 PM, Ville Syrjälä wrote:
On Tue, Apr 14, 2015 at 03:58:54PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
This WA disable usage of shadow register during CPD/RC6 transactions on
CHV
I suppose is a workaround for the shadow vs. wake FIFO problem... Yeah
hsd seems to agree (after a bit of extra digging).
Yes ville, this WA is avoid problem between shadow vs wake FIFO unload problem.
I will address the comments and send new patch set
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 4
2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c97842..bcdb16b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6206,6 +6206,7 @@ enum skl_disp_power_wells {
#define GTFIFOCTL0x120008
#defineGT_FIFO_FREE_ENTRIES_MASK 0x7f
#defineGT_FIFO_NUM_RESERVED_ENTRIES 20
+#define GT_FIFO_CTL_BLOCK_POLICY (3<<11)
GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL (1 << 12)
GT_FIFO_CTL_RC6_POLICY_STALL(1 << 11)
perhaps?
#define HSW_IDICR0x9008
#defineIDIHASHMSK(x) (((x) & 0x3f) << 16)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4dd8b41..b9d3c00 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6431,6 +6431,10 @@ static void cherryview_init_clock_gating(struct
drm_device *dev)
/* WaDisableSDEUnitClockGating:chv */
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
+
+ /* WaDisableShadowRegForCpd */
+ I915_WRITE(GTFIFOCTL, I915_READ(GTFIFOCTL) |
+ GT_FIFO_CTL_BLOCK_POLICY);
I think __intel_uncore_early_sanitize() might be a better place for
this.
}
static void g4x_init_clock_gating(struct drm_device *dev)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/chv: Implement WaDisableShadowRegForCpd
From: Deepak S
This WA is avoid problem between shadow vs wake FIFO unload
problem during CPD/RC6 transactions on CHV.
v2: Define individual bits GTFIFOCTL (Ville)
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 5 +
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c97842..a642a58 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6206,6 +6206,8 @@ enum skl_disp_power_wells {
#define GTFIFOCTL 0x120008
#defineGT_FIFO_FREE_ENTRIES_MASK 0x7f
#defineGT_FIFO_NUM_RESERVED_ENTRIES20
+#define GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL (1 << 12)
+#define GT_FIFO_CTL_RC6_POLICY_STALL (1 << 11)
#define HSW_IDICR 0x9008
#defineIDIHASHMSK(x) (((x) & 0x3f) << 16)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4dd8b41..2b86915 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6431,6 +6431,11 @@ static void cherryview_init_clock_gating(struct
drm_device *dev)
/* WaDisableSDEUnitClockGating:chv */
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
+
+ /* WaDisableShadowRegForCpd */
+ I915_WRITE(GTFIFOCTL, I915_READ(GTFIFOCTL) |
+
GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL |
+ GT_FIFO_CTL_RC6_POLICY_STALL);
}
static void g4x_init_clock_gating(struct drm_device *dev)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/chv: Implement WaDisableShadowRegForCpd
On Wednesday 15 April 2015 04:48 PM, Ville Syrjälä wrote:
On Wed, Apr 15, 2015 at 02:16:18PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
This WA is avoid problem between shadow vs wake FIFO unload
problem during CPD/RC6 transactions on CHV.
v2: Define individual bits GTFIFOCTL (Ville)
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 5 +
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c97842..a642a58 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6206,6 +6206,8 @@ enum skl_disp_power_wells {
#define GTFIFOCTL0x120008
#defineGT_FIFO_FREE_ENTRIES_MASK 0x7f
#defineGT_FIFO_NUM_RESERVED_ENTRIES 20
+#define GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL (1 << 12)
+#define GT_FIFO_CTL_RC6_POLICY_STALL (1 << 11)
#define HSW_IDICR0x9008
#defineIDIHASHMSK(x) (((x) & 0x3f) << 16)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4dd8b41..2b86915 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6431,6 +6431,11 @@ static void cherryview_init_clock_gating(struct
drm_device *dev)
/* WaDisableSDEUnitClockGating:chv */
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
+
+ /* WaDisableShadowRegForCpd */
+ I915_WRITE(GTFIFOCTL, I915_READ(GTFIFOCTL) |
+
GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL |
+ GT_FIFO_CTL_RC6_POLICY_STALL);
uncore early sanitize would still seem like the better place for this.
you mean intel_uncore_early_sanitize(). But func is called during early resume
right?
Since GTFIFOCTL will be saved and resorted by gunit. I think programming reg in
cherryview_init_clock_gating should be good enough?
}
static void g4x_init_clock_gating(struct drm_device *dev)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915/chv: Implement WaDisableShadowRegForCpd
From: Deepak S
This WA is avoid problem between shadow vs wake FIFO unload
problem during CPD/RC6 transactions on CHV.
v2: Define individual bits GTFIFOCTL (Ville)
v3: move WA to uncore_early_sanitize (ville)
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_uncore.c | 8
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c97842..a642a58 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6206,6 +6206,8 @@ enum skl_disp_power_wells {
#define GTFIFOCTL 0x120008
#defineGT_FIFO_FREE_ENTRIES_MASK 0x7f
#defineGT_FIFO_NUM_RESERVED_ENTRIES20
+#define GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL (1 << 12)
+#define GT_FIFO_CTL_RC6_POLICY_STALL (1 << 11)
#define HSW_IDICR 0x9008
#defineIDIHASHMSK(x) (((x) & 0x3f) << 16)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index d96d15f..26d6dda 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -360,6 +360,14 @@ static void __intel_uncore_early_sanitize(struct
drm_device *dev,
__raw_i915_write32(dev_priv, GTFIFODBG,
__raw_i915_read32(dev_priv, GTFIFODBG));
+ /* WaDisableShadowRegForCpd */
+ if (IS_CHERRYVIEW(dev)) {
+ __raw_i915_write32(dev_priv, GTFIFOCTL,
+ __raw_i915_read32(dev_priv, GTFIFOCTL) |
+
GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL |
+ GT_FIFO_CTL_RC6_POLICY_STALL);
+ }
+
intel_uncore_forcewake_reset(dev, restore_forcewake);
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/chv: Implement WaDisableShadowRegForCpd
On Thursday 16 April 2015 12:09 AM, Ville Syrjälä wrote:
On Wed, Apr 15, 2015 at 07:41:39PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
This WA is avoid problem between shadow vs wake FIFO unload
problem during CPD/RC6 transactions on CHV.
v2: Define individual bits GTFIFOCTL (Ville)
v3: move WA to uncore_early_sanitize (ville)
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_uncore.c | 8
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c97842..a642a58 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6206,6 +6206,8 @@ enum skl_disp_power_wells {
#define GTFIFOCTL0x120008
#defineGT_FIFO_FREE_ENTRIES_MASK 0x7f
#defineGT_FIFO_NUM_RESERVED_ENTRIES 20
+#define GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL (1 << 12)
+#define GT_FIFO_CTL_RC6_POLICY_STALL (1 << 11)
#define HSW_IDICR0x9008
#defineIDIHASHMSK(x) (((x) & 0x3f) << 16)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index d96d15f..26d6dda 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -360,6 +360,14 @@ static void __intel_uncore_early_sanitize(struct
drm_device *dev,
__raw_i915_write32(dev_priv, GTFIFODBG,
__raw_i915_read32(dev_priv, GTFIFODBG));
+ /* WaDisableShadowRegForCpd */
I couldn't find this w/a name anywhere official, but I guess it's fine.
Should be WaDisableShadowRegForCpd:chv though. Otherwise looks good, so
Reviewed-by: Ville Syrjälä
Thanks Ville for reviewing. I this the WA still not added to the chv list.
+ if (IS_CHERRYVIEW(dev)) {
+ __raw_i915_write32(dev_priv, GTFIFOCTL,
+ __raw_i915_read32(dev_priv, GTFIFOCTL) |
+
GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL |
+ GT_FIFO_CTL_RC6_POLICY_STALL);
+ }
+
intel_uncore_forcewake_reset(dev, restore_forcewake);
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915/chv: Implement WaDisableShadowRegForCpd
From: Deepak S
This WA is avoid problem between shadow vs wake FIFO unload
problem during CPD/RC6 transactions on CHV.
v2: Define individual bits GTFIFOCTL (Ville)
v3: move WA to uncore_early_sanitize (ville)
Signed-off-by: Deepak S
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_uncore.c | 8
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c97842..a642a58 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6206,6 +6206,8 @@ enum skl_disp_power_wells {
#define GTFIFOCTL 0x120008
#defineGT_FIFO_FREE_ENTRIES_MASK 0x7f
#defineGT_FIFO_NUM_RESERVED_ENTRIES20
+#define GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL (1 << 12)
+#define GT_FIFO_CTL_RC6_POLICY_STALL (1 << 11)
#define HSW_IDICR 0x9008
#defineIDIHASHMSK(x) (((x) & 0x3f) << 16)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index d96d15f..d7cfd0c 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -360,6 +360,14 @@ static void __intel_uncore_early_sanitize(struct
drm_device *dev,
__raw_i915_write32(dev_priv, GTFIFODBG,
__raw_i915_read32(dev_priv, GTFIFODBG));
+ /* WaDisableShadowRegForCpd:chv */
+ if (IS_CHERRYVIEW(dev)) {
+ __raw_i915_write32(dev_priv, GTFIFOCTL,
+ __raw_i915_read32(dev_priv, GTFIFOCTL) |
+
GT_FIFO_CTL_BLOCK_ALL_POLICY_STALL |
+ GT_FIFO_CTL_RC6_POLICY_STALL);
+ }
+
intel_uncore_forcewake_reset(dev, restore_forcewake);
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Avoid GPU hang when coming out of S3 or S4
Yes agreed, we need to make changes in other paths :) On Tuesday 28 April 2015 02:14 PM, Chris Wilson wrote: On Tue, Apr 28, 2015 at 08:29:13AM +, S, Deepak wrote: Thanks Chirs for review, We moved "Init_hw" to initialize WA's before any BB submission. Init_hw calls " init_clock_gating" But you appreciate the same issue exists on other paths? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove wait for for punit to updates freq.
On Monday 13 April 2015 05:40 PM, Ville Syrjälä wrote:
On Mon, Apr 13, 2015 at 02:55:12PM +0300, Jani Nikula wrote:
On Thu, 05 Mar 2015, deepa...@linux.intel.com wrote:
From: Deepak S
When GPU is idle on VLV, Request freq to punit should be good enough to
get the voltage back to VNN. Also, make sure gfx clock force applies
before requesting the freq fot vlv.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
suggested-by: Jesse Barnes
Signed-off-by: Deepak S
Deepak, these patches seem to have fallen through the cracks. Are they
still valid? Please rebase and repost if they are.
Ville, your opinion also appreciated.
I don't I have any VLVs old enough to hit this, so can't really confirm
one way or the other.
Hi Jesse, Can you please review the patch?
Thanks
Deepak
BR,
Jani.
---
drivers/gpu/drm/i915/intel_pm.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e710b43..2e1ed07 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8
val)
* * 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
+ * 3. Request the freq to Rpn.
* 4. Clear the Force GFX CLK ON bit so that Gfx can down
* 5. Unmask Turbo interrupts
*/
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
{
struct drm_device *dev = dev_priv->dev;
- /* CHV and latest VLV don't need to force the gfx clock */
- if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+ /* CHV don't need to force the gfx clock */
+ if (IS_CHERRYVIEW(dev)) {
Why was the stepping check removed?
valleyview_set_rps(dev_priv->dev,
dev_priv->rps.min_freq_softlimit);
return;
}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
gen6_sanitize_rps_pm_mask(dev_priv, ~0));
vlv_force_gfx_clock(dev_priv, true);
-
- dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
-
- vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
- dev_priv->rps.min_freq_softlimit);
-
- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
- & GENFREQSTATUS) == 0, 100))
- DRM_ERROR("timed out waiting for Punit\n");
-
+ valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
vlv_force_gfx_clock(dev_priv, false);
-
- I915_WRITE(GEN6_PMINTRMSK,
- gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
}
void gen6_rps_idle(struct drm_i915_private *dev_priv)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove wait for for punit to updates freq.
On Tuesday 28 April 2015 11:46 PM, Jesse Barnes wrote: Yeah I think this is fine (may need a rebase though, you can keep my r-b if you do that in case Jani doesn't want to deal with the merge conflicts). Reviewed-by: Jesse Barnes Sure Jesse, I will rebase the patch. Thanks Deepak ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove wait for for punit to updates freq.
On Wednesday 29 April 2015 12:02 AM, Ville Syrjälä wrote:
On Tue, Apr 28, 2015 at 11:16:29AM -0700, Jesse Barnes wrote:
On 03/04/2015 08:08 PM, deepa...@linux.intel.com wrote:
From: Deepak S
When GPU is idle on VLV, Request freq to punit should be good enough to
get the voltage back to VNN. Also, make sure gfx clock force applies
before requesting the freq fot vlv.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
suggested-by: Jesse Barnes
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_pm.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e710b43..2e1ed07 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8
val)
* * 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
+ * 3. Request the freq to Rpn.
* 4. Clear the Force GFX CLK ON bit so that Gfx can down
* 5. Unmask Turbo interrupts
*/
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
{
struct drm_device *dev = dev_priv->dev;
- /* CHV and latest VLV don't need to force the gfx clock */
- if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+ /* CHV don't need to force the gfx clock */
+ if (IS_CHERRYVIEW(dev)) {
valleyview_set_rps(dev_priv->dev,
dev_priv->rps.min_freq_softlimit);
return;
}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
gen6_sanitize_rps_pm_mask(dev_priv, ~0));
vlv_force_gfx_clock(dev_priv, true);
-
- dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
-
- vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
- dev_priv->rps.min_freq_softlimit);
-
- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
- & GENFREQSTATUS) == 0, 100))
- DRM_ERROR("timed out waiting for Punit\n");
-
+ valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
vlv_force_gfx_clock(dev_priv, false);
-
- I915_WRITE(GEN6_PMINTRMSK,
- gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
}
void gen6_rps_idle(struct drm_i915_private *dev_priv)
Yeah I think this is fine (may need a rebase though, you can keep my r-b
if you do that in case Jani doesn't want to deal with the merge conflicts).
Reviewed-by: Jesse Barnes
The removal of the stepping check is still confusing me even if the
rest would be OK.
Stepping check was added latest BYT release. On older BYT stepping, We used to
wait for punit to grant the freq in GT Idle case, (most of the cases punit is
timing out :( )
We now make the gfx clock force apply to all VLV and then request the freq to
RPn this should be good enough to get voltage to Vnn.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Setup static bias for GPU
From: Deepak S
Based on the spec, Setting up static BIAS for GPU to improve the
rps performace.
v2: rename reg defn to match spec. (Ville)
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_pm.c | 12
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b6..21c33c6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -670,6 +670,11 @@ enum skl_disp_power_wells {
#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27
#define FB_FMAX_VMIN_FREQ_LO_MASK0xf800
+#define VLV_TURBO_SOC_OVERRIDE 0x04
+#defineVLV_OVERRIDE_EN 1
+#defineVLV_SOC_TDP_EN (1 << 1)
+#defineVLV_BIAS_CPU_125_SOC_875 (6 << 2)
+
#define VLV_CZ_CLOCK_TO_MILLI_SEC 10
/* vlv2 north clock has */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 78c89ff..318f587 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
GEN6_RP_UP_BUSY_AVG |
GEN6_RP_DOWN_IDLE_AVG);
+ /* Setting Fixed Bias */
+ val = VLV_OVERRIDE_EN |
+ VLV_SOC_TDP_EN |
+ VLV_BIAS_CPU_125_SOC_875;
+ vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
/* RPS code assumes GPLL is used */
@@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
+ /* Setting Fixed Bias */
+ val = VLV_OVERRIDE_EN |
+ VLV_SOC_TDP_EN |
+ VLV_BIAS_CPU_125_SOC_875;
+ vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
/* RPS code assumes GPLL is used */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv
From: Deepak S
After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
frequency to RPn, punit is failing to change the input voltage to
minimum :(
Since Punit validates the rps range [RPe, RP0]. This patch
removes unused cherryview_rps_min_freq function.
v2: Change commit message
v3: set min_freq before idle_freq (chris)
v4: Squash 'Remove unused rps min function' patch
Signed-off-by: Deepak S
Acked-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_pm.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7516ed..78c89ff 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct
drm_i915_private *dev_priv)
return rp1;
}
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
- struct drm_device *dev = dev_priv->dev;
- u32 val, rpn;
-
- if (dev->pdev->revision >= 0x20) {
- val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
- rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
- FB_GFX_FREQ_FUSE_MASK);
- } else { /* For pre-production hardware */
- val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
- rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
- PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
- }
-
- return rpn;
-}
-
static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
{
u32 val, rp1;
@@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct
drm_device *dev)
intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
dev_priv->rps.rp1_freq);
- dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+ /* PUnit validated range is only [RPe, RP0] */
+ dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
dev_priv->rps.min_freq);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: Setup static bias for GPU
From: Deepak S
Based on the spec, Setting up static BIAS for GPU to improve the
rps performace.
v2: rename reg defn to match spec. (Ville)
v3: Updated bias setting for chv (Deepak)
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/i915_reg.h | 6 ++
drivers/gpu/drm/i915/intel_pm.c | 12
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b6..048987e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -670,6 +670,12 @@ enum skl_disp_power_wells {
#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27
#define FB_FMAX_VMIN_FREQ_LO_MASK0xf800
+#define VLV_TURBO_SOC_OVERRIDE 0x04
+#defineVLV_OVERRIDE_EN 1
+#defineVLV_SOC_TDP_EN (1 << 1)
+#defineVLV_BIAS_CPU_125_SOC_875 (6 << 2)
+#defineCHV_BIAS_CPU_50_SOC_50 (3 << 2)
+
#define VLV_CZ_CLOCK_TO_MILLI_SEC 10
/* vlv2 north clock has */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 78c89ff..3689d0e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
GEN6_RP_UP_BUSY_AVG |
GEN6_RP_DOWN_IDLE_AVG);
+ /* Setting Fixed Bias */
+ val = VLV_OVERRIDE_EN |
+ VLV_SOC_TDP_EN |
+ CHV_BIAS_CPU_50_SOC_50;
+ vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
/* RPS code assumes GPLL is used */
@@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
+ /* Setting Fixed Bias */
+ val = VLV_OVERRIDE_EN |
+ VLV_SOC_TDP_EN |
+ VLV_BIAS_CPU_125_SOC_875;
+ vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
/* RPS code assumes GPLL is used */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove wait for for punit to updates freq.
On Wednesday 29 April 2015 03:56 PM, Ville Syrjälä wrote:
On Wed, Apr 29, 2015 at 08:20:20AM +0530, Deepak S wrote:
On Wednesday 29 April 2015 12:02 AM, Ville Syrjälä wrote:
On Tue, Apr 28, 2015 at 11:16:29AM -0700, Jesse Barnes wrote:
On 03/04/2015 08:08 PM, deepa...@linux.intel.com wrote:
From: Deepak S
When GPU is idle on VLV, Request freq to punit should be good enough to
get the voltage back to VNN. Also, make sure gfx clock force applies
before requesting the freq fot vlv.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
suggested-by: Jesse Barnes
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_pm.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e710b43..2e1ed07 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8
val)
* * 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
+ * 3. Request the freq to Rpn.
* 4. Clear the Force GFX CLK ON bit so that Gfx can down
* 5. Unmask Turbo interrupts
*/
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
{
struct drm_device *dev = dev_priv->dev;
- /* CHV and latest VLV don't need to force the gfx clock */
- if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+ /* CHV don't need to force the gfx clock */
+ if (IS_CHERRYVIEW(dev)) {
valleyview_set_rps(dev_priv->dev,
dev_priv->rps.min_freq_softlimit);
return;
}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
gen6_sanitize_rps_pm_mask(dev_priv, ~0));
vlv_force_gfx_clock(dev_priv, true);
-
- dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
-
- vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
- dev_priv->rps.min_freq_softlimit);
-
- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
- & GENFREQSTATUS) == 0, 100))
- DRM_ERROR("timed out waiting for Punit\n");
-
+ valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
vlv_force_gfx_clock(dev_priv, false);
-
- I915_WRITE(GEN6_PMINTRMSK,
- gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
}
void gen6_rps_idle(struct drm_i915_private *dev_priv)
Yeah I think this is fine (may need a rebase though, you can keep my r-b
if you do that in case Jani doesn't want to deal with the merge conflicts).
Reviewed-by: Jesse Barnes
The removal of the stepping check is still confusing me even if the
rest would be OK.
Stepping check was added latest BYT release. On older BYT stepping, We used to
wait for punit to grant the freq in GT Idle case, (most of the cases punit is
timing out :( )
We now make the gfx clock force apply to all VLV and then request the freq to
RPn this should be good enough to get voltage to Vnn.
But we shouldn't need the gfx clock force for the latest VLV
stepping(s), and we certainly didn't do it before. So why do
it now?
Hi Ville, This is keep code common across all the VLV stepping. :)
Thanks
Deepak
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv
On Thursday 30 April 2015 01:23 AM, Ville Syrjälä wrote:
On Wed, Apr 29, 2015 at 06:31:56PM +0300, Ville Syrjälä wrote:
On Wed, Apr 29, 2015 at 08:23:21AM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
frequency to RPn, punit is failing to change the input voltage to
minimum :(
So far I can't reproduce this problem on my BSW. In fact what I see
that the voltage at RPn is lower than the voltage at RPe, even while
we're in rc6.
without forcewake:
RPn -> 0x66
RPe -> 0x67
RP0 -> 0x69
with forcewake:
RPn -> 0x66
RPe -> 0x76
RP0 -> 0x9d
Also asking Punit to change the frequency after the GPU has gone to
rc6 does absolutely nothing (remind anyone of VLV?). I think I need to
retest my VLV C0 to see if my earlier observations there were accurate.
The shared Vnn rail does make it harder to observe this stuff on
VLV though.
I went back to my VLVs (had a B3 and C0 actually). And I'm seeing the
exact same behaviour on both, ie. requesting a new frequency from Punit
does nothing when the GPU is in rc6, and if I let it enter rc6 with a
high frequency Vnn also remains high. Previously I had thought that C0
fixed this, but now it definitely shows the same problem here. I must
have had some accidental forcewake somewhere when I originally tested
it,
So based on that, your other patch to remove the stepping check from
vlv_set_rps_idle() is in fact correct.
The question remains however what should we do with CHV. According to my
testing to get the minimum voltage we should keep RPn around, and we
should also do the vlv_set_rps_idle() workaround on CHV.
Oh and I also observed something else on VLV. Normally when entering rc6
the GPLL ref clock gets trunk gated at CCK (by Punit I assume). However
when using the vlv_gfx_clock_force() that doesn't happen. So I'm not
entirely sure the GPLL gets turned off properly in that case. Maybe we
should just use forcewake instead? Oh and BTW, CHV doesn't do the trunk
gating in either case. I'm not sure where to check if the GPLL is
actually running or not.
Hi Ville,
Thanks Ville for verifying on VLV and CHV. Its interesting to see when Idle,
Voltage not dropping to Vnn on CHV :( This was supposed to be fixed
in BSW/CHV :(. As you suggested it would be better to extend the VLV WA to
CHV also to make sure we drop the voltage when idle.
Below is the sequence I think we should follow (based on your comments).
1. forcewake power wells
2. do gfx force clock on
3. request freq to punit
4. release gfx force clock on
5. release forcewake of power wells.
Please share your thoughts?
Thanks
Deepak
So based on my tests this patch feels a bit wrong.
Since Punit validates the rps range [RPe, RP0]. This patch
removes unused cherryview_rps_min_freq function.
v2: Change commit message
v3: set min_freq before idle_freq (chris)
v4: Squash 'Remove unused rps min function' patch
Signed-off-by: Deepak S
Acked-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_pm.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7516ed..78c89ff 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct
drm_i915_private *dev_priv)
return rp1;
}
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
- struct drm_device *dev = dev_priv->dev;
- u32 val, rpn;
-
- if (dev->pdev->revision >= 0x20) {
- val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
- rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
- FB_GFX_FREQ_FUSE_MASK);
- } else { /* For pre-production hardware */
- val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
- rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
- PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
- }
-
- return rpn;
-}
-
static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
{
u32 val, rp1;
@@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct
drm_device *dev)
intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
dev_priv->rps.rp1_freq);
- dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+ /* PUnit validated range is only [RPe, RP0] */
+ dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
dev_priv->rps.min_freq);
--
1.9.1
--
Ville Syrjälä
Intel OTC
_
Re: [Intel-gfx] [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv
On Thursday 30 April 2015 07:35 PM, Ville Syrjälä wrote:
On Thu, Apr 30, 2015 at 02:19:07PM +0300, Ville Syrjälä wrote:
On Thu, Apr 30, 2015 at 03:42:42PM +0530, Deepak S wrote:
As you suggested it would be better to extend the VLV WA to
CHV also to make sure we drop the voltage when idle.
Below is the sequence I think we should follow (based on your comments).
1. forcewake power wells
2. do gfx force clock on
3. request freq to punit
4. release gfx force clock on
5. release forcewake of power wells.
Please share your thoughts?
I'm thinking we shouldn't need the gfx clock force since forcewake
should already cause the clock to be enabled.
I've also not verified what happens if we drop the forcewake before
Punit has actually finished the frequency change. I'll try to hack
up some kind of test to see if I can make that happen.
OK, after some hacking I see that the forcewake is enough, and also
we don't seem to need to wait for the Punit to finish the frequency
change before dropping forcewake. It seems to finish the change even
after forcewake has been dropped.
Also for a bit of extra micro optimization we should perhaps wake up
the media well only, as that takes a lot less power than the render
well.
Thanks Ville. I will submit new patch to extend the WA to CHV
Thanks
Deepak
Thanks
Deepak
So based on my tests this patch feels a bit wrong.
Since Punit validates the rps range [RPe, RP0]. This patch
removes unused cherryview_rps_min_freq function.
v2: Change commit message
v3: set min_freq before idle_freq (chris)
v4: Squash 'Remove unused rps min function' patch
Signed-off-by: Deepak S
Acked-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_pm.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7516ed..78c89ff 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct
drm_i915_private *dev_priv)
return rp1;
}
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
- struct drm_device *dev = dev_priv->dev;
- u32 val, rpn;
-
- if (dev->pdev->revision >= 0x20) {
- val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
- rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
- FB_GFX_FREQ_FUSE_MASK);
- } else { /* For pre-production hardware */
- val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
- rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
- PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
- }
-
- return rpn;
-}
-
static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
{
u32 val, rp1;
@@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct
drm_device *dev)
intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
dev_priv->rps.rp1_freq);
- dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+ /* PUnit validated range is only [RPe, RP0] */
+ dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
dev_priv->rps.min_freq);
--
1.9.1
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Setup static bias for GPU
On Wednesday 29 April 2015 02:59 PM, Ville Syrjälä wrote:
On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
Based on the spec, Setting up static BIAS for GPU to improve the
rps performace.
v2: rename reg defn to match spec. (Ville)
v3: Updated bias setting for chv (Deepak)
Signed-off-by: Deepak S
Matches the spec. Whether the chosen bias is really the best, I can't
really say. But favoring the GPU does seem like a sensible idea if we
want to keep the UI stuff fluid enough while there's some CPU heavy
tasks running at the same time.
Reviewed-by: Ville Syrjälä
Thanks Ville for reviewing,
Yes our aim is to keep user experience smooth.
---
drivers/gpu/drm/i915/i915_reg.h | 6 ++
drivers/gpu/drm/i915/intel_pm.c | 12
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b6..048987e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -670,6 +670,12 @@ enum skl_disp_power_wells {
#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27
#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf800
+#define VLV_TURBO_SOC_OVERRIDE 0x04
+#defineVLV_OVERRIDE_EN 1
+#defineVLV_SOC_TDP_EN (1 << 1)
+#defineVLV_BIAS_CPU_125_SOC_875 (6 << 2)
+#defineCHV_BIAS_CPU_50_SOC_50 (3 << 2)
+
#define VLV_CZ_CLOCK_TO_MILLI_SEC 10
/* vlv2 north clock has */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 78c89ff..3689d0e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
GEN6_RP_UP_BUSY_AVG |
GEN6_RP_DOWN_IDLE_AVG);
+ /* Setting Fixed Bias */
+ val = VLV_OVERRIDE_EN |
+ VLV_SOC_TDP_EN |
+ CHV_BIAS_CPU_50_SOC_50;
+ vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
/* RPS code assumes GPLL is used */
@@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
+ /* Setting Fixed Bias */
+ val = VLV_OVERRIDE_EN |
+ VLV_SOC_TDP_EN |
+ VLV_BIAS_CPU_125_SOC_875;
+ vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
/* RPS code assumes GPLL is used */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Setup static bias for GPU
On Monday 04 May 2015 08:58 PM, Ville Syrjälä wrote:
On Mon, May 04, 2015 at 10:12:23AM +0200, Daniel Vetter wrote:
On Mon, May 04, 2015 at 10:58:02AM +0530, Deepak S wrote:
On Wednesday 29 April 2015 02:59 PM, Ville Syrjälä wrote:
On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
Based on the spec, Setting up static BIAS for GPU to improve the
rps performace.
v2: rename reg defn to match spec. (Ville)
v3: Updated bias setting for chv (Deepak)
Signed-off-by: Deepak S
Matches the spec. Whether the chosen bias is really the best, I can't
really say. But favoring the GPU does seem like a sensible idea if we
want to keep the UI stuff fluid enough while there's some CPU heavy
tasks running at the same time.
Reviewed-by: Ville Syrjälä
Thanks Ville for reviewing,
Yes our aim is to keep user experience smooth.
Since the aim is smooth UI ... how does this interact with the rps
boosting that was just enabled with Chris' patches for vlv/chv too?
A static bias seems a lot less what we want now that we should have
something dynamic. Specifically I'm thinking of
commit 6ad790c0f5ac55fd13f322c23519f0d6f0721864
Author: Chris Wilson
Date: Tue Apr 7 16:20:31 2015 +0100
drm/i915: Boost GPU frequency if we detect outstanding pageflips
Totally separate topic. This only affects how the Punit splits up the
available energy credits between the CPU and the GPU. So only relevant
when thermally constrained and both CPU and GPU would like to run
faster than the limit allows.
Thanks Ville.
This is completely a different topic, more to do with punit budget constraints & this
does not impact "Boot RPS logic"
Thanks
Deepak
Cheers, Daniel
---
drivers/gpu/drm/i915/i915_reg.h | 6 ++
drivers/gpu/drm/i915/intel_pm.c | 12
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b6..048987e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -670,6 +670,12 @@ enum skl_disp_power_wells {
#define FB_FMAX_VMIN_FREQ_LO_SHIFT 27
#define FB_FMAX_VMIN_FREQ_LO_MASK 0xf800
+#define VLV_TURBO_SOC_OVERRIDE 0x04
+#defineVLV_OVERRIDE_EN 1
+#defineVLV_SOC_TDP_EN (1 << 1)
+#defineVLV_BIAS_CPU_125_SOC_875 (6 << 2)
+#defineCHV_BIAS_CPU_50_SOC_50 (3 << 2)
+
#define VLV_CZ_CLOCK_TO_MILLI_SEC 10
/* vlv2 north clock has */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 78c89ff..3689d0e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5065,6 +5065,12 @@ static void cherryview_enable_rps(struct drm_device *dev)
GEN6_RP_UP_BUSY_AVG |
GEN6_RP_DOWN_IDLE_AVG);
+ /* Setting Fixed Bias */
+ val = VLV_OVERRIDE_EN |
+ VLV_SOC_TDP_EN |
+ CHV_BIAS_CPU_50_SOC_50;
+ vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
/* RPS code assumes GPLL is used */
@@ -5149,6 +5155,12 @@ static void valleyview_enable_rps(struct drm_device *dev)
I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
+ /* Setting Fixed Bias */
+ val = VLV_OVERRIDE_EN |
+ VLV_SOC_TDP_EN |
+ VLV_BIAS_CPU_125_SOC_875;
+ vlv_punit_write(dev_priv, VLV_TURBO_SOC_OVERRIDE, val);
+
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
/* RPS code assumes GPLL is used */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915: Implement chv display PHY lane stagger setup
/* Data lane stagger programming */ - /* FIXME: Fix up value only after power analysis */ + if (intel_crtc->config->port_clock > 27) + stagger = 0x18; + else if (intel_crtc->config->port_clock > 135000) + stagger = 0xd; + else if (intel_crtc->config->port_clock > 67500) + stagger = 0x7; + else if (intel_crtc->config->port_clock > 33750) + stagger = 0x4; + else + stagger = 0x2; + + val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW11(ch)); + val |= DPIO_TX2_STAGGER_MASK(0x1f); + vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW11(ch), val); + + val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW11(ch)); + val |= DPIO_TX2_STAGGER_MASK(0x1f); + vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW11(ch), val); + + vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW12(ch), + DPIO_LANESTAGGER_STRAP(stagger) | + DPIO_LANESTAGGER_STRAP_OVRD | + DPIO_TX1_STAGGER_MASK(0x1f) | + DPIO_TX1_STAGGER_MULT(6) | + DPIO_TX2_STAGGER_MULT(0)); + + vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW12(ch), + DPIO_LANESTAGGER_STRAP(stagger) | + DPIO_LANESTAGGER_STRAP_OVRD | + DPIO_TX1_STAGGER_MASK(0x1f) | + DPIO_TX1_STAGGER_MULT(7) | + DPIO_TX2_STAGGER_MULT(5)); /* Clear calc init */ val = vlv_dpio_read(dev_priv, pipe, VLV_PCS01_DW10(ch)); Patch does what spec says :) Reviewed-by: Deepak S ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Work around DISPLAY_PHY_CONTROL register corruption on CHV
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Sometimes (exactly when is a bit unclear) DISPLAY_PHY_CONTROL appears to
get corrupted. The values I've managed to read from it seem to have some
pattern but vary quite a lot. The corruption doesn't seem to just happen
when the register is accessed, but can also happen spontaneosly during
modeset. When this happens during a modeset things go south and the
display doesn't light up.
I've managed to hit the problemn when toggling HDMI on port D on and
off. When things get corrupted the display doesn't light up, but as soon
as I manually write the correct value to the register the display comes
up.
First I was suspicious that we ourselves accidentally overwrite it with
garbage, but didn't catch anything with the reg_rw tracepoint. Also I
sprinkled check all over the modeset path to see exactly when the
corruption happens, and eg. the read back value was fine just before
intel_dp_set_m(), and corrupted immediately after it. I also made my
check function repair the register value whenever it was wrong, and with
this approach the corruption repeated several times during the modeset
operation, always seeming to trigger in the same exact calls to the
check function, while other calls to the function never caught anything.
So far I've not seen this problem occurring when carefully avoiding all
read accesses to DISPLAY_PHY_CONTROL. Not sure if that's just pure luck
or an actual workaround, but we can hope it works. So let's avoid reading
the register and instead track the desired value of the register in dev_priv.
v2: Read out the power well state to determine initial register value
v3: Use DPIO_CHx names instead of raw numbers
Even reading once DISPLAY_PHY_CONTROL layer is getting corrupted?
I saw similar issues on my setup. On some platform access phy is causing system
behave inconsistently :(
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_reg.h | 5 -
drivers/gpu/drm/i915/intel_runtime_pm.c | 36 -
3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 822f259..288c3fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1754,6 +1754,8 @@ struct drm_i915_private {
u32 fdi_rx_config;
+ u32 chv_phy_control;
+
u32 suspend_count;
struct i915_suspend_saved_registers regfile;
struct vlv_s0ix_state vlv_s0ix_state;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cfbd5a6..98588d5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1887,7 +1887,10 @@ enum skl_disp_power_wells {
#define DPIO_PHY_STATUS (VLV_DISPLAY_BASE + 0x6240)
#define DPLL_PORTD_READY_MASK (0xf)
#define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
-#define PHY_COM_LANE_RESET_DEASSERT(phy) (1 << (phy))
+#define PHY_CH_SU_PSR0x1
+#define PHY_CH_DEEP_PSR 0x7
PHY_CH_DEEP_PSR defined but not used in this patch?
other than this, patch does what it says.
Reviewed-by: Deepak S
+#define PHY_CH_POWER_MODE(mode, phy, ch) ((mode) << (6*(phy)+3*(ch)+2))
+#define PHY_COM_LANE_RESET_DEASSERT(phy) (1 << (phy))
#define DISPLAY_PHY_STATUS (VLV_DISPLAY_BASE + 0x60104)
#define PHY_POWERGOOD(phy) (((phy) == DPIO_PHY0) ? (1<<31) : (1<<30))
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ce00e69..b73671f 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -666,8 +666,8 @@ static void chv_dpio_cmn_power_well_enable(struct
drm_i915_private *dev_priv,
if (wait_for(I915_READ(DISPLAY_PHY_STATUS) & PHY_POWERGOOD(phy), 1))
DRM_ERROR("Display PHY %d is not power up\n", phy);
- I915_WRITE(DISPLAY_PHY_CONTROL, I915_READ(DISPLAY_PHY_CONTROL) |
- PHY_COM_LANE_RESET_DEASSERT(phy));
+ dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(phy);
+ I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control);
}
static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
@@ -687,8 +687,8 @@ static void chv_dpio_cmn_power_well_disable(struct
drm_i915_private *dev_priv,
assert_pll_disabled(dev_priv, PIPE_C);
}
- I915_WRITE(DISPLAY_PHY_CONTROL, I915_READ(DISPLAY_PHY_CONTROL) &
- ~PHY_COM_LANE_RESET_DEASSERT(phy));
+ dev_priv->chv_phy_control &= ~PHY_COM_LANE_RESET_DEASSERT(phy);
+ I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control);
vlv_set_power_well(dev_priv, po
Re: [Intel-gfx] [PATCH 3/7] Revert "drm/i915: Hack to tie both common lanes together on chv"
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
With recent hardware/firmware there don't appear to be any glitches
on the other PHY when we toggle the cmnreset for the other PHY. So
detangle the cmnlane power wells from one another and let them be
controlled independently.
This reverts commit 3dd7b97458e8aa2d8985b46622d226fa635071e7.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_runtime_pm.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b73671f..1f800f8 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1181,23 +1181,13 @@ static struct i915_power_well chv_power_wells[] = {
#endif
{
.name = "dpio-common-bc",
- /*
-* XXX: cmnreset for one PHY seems to disturb the other.
-* As a workaround keep both powered on at the same
-* time for now.
-*/
- .domains = CHV_DPIO_CMN_BC_POWER_DOMAINS |
CHV_DPIO_CMN_D_POWER_DOMAINS,
+ .domains = CHV_DPIO_CMN_BC_POWER_DOMAINS,
.data = PUNIT_POWER_WELL_DPIO_CMN_BC,
.ops = &chv_dpio_cmn_power_well_ops,
},
{
.name = "dpio-common-d",
- /*
-* XXX: cmnreset for one PHY seems to disturb the other.
-* As a workaround keep both powered on at the same
-* time for now.
-*/
- .domains = CHV_DPIO_CMN_BC_POWER_DOMAINS |
CHV_DPIO_CMN_D_POWER_DOMAINS,
+ .domains = CHV_DPIO_CMN_D_POWER_DOMAINS,
.data = PUNIT_POWER_WELL_DPIO_CMN_D,
.ops = &chv_dpio_cmn_power_well_ops,
},
Right, Issue is fixed with latest FW.
Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: Use the default 600ns LDO programming sequence delay
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Not sure which LDO programming sequence delay should be used for the CHV
PHY, but the spec says that 600ns is "Used by default for initial
bringup", and the BIOS seems to use that, so let's do the same.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 4
drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 98588d5..977bad6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1887,6 +1887,10 @@ enum skl_disp_power_wells {
#define DPIO_PHY_STATUS (VLV_DISPLAY_BASE + 0x6240)
#define DPLL_PORTD_READY_MASK (0xf)
#define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
+#define PHY_LDO_DELAY_0NS0x0
+#define PHY_LDO_DELAY_200NS 0x1
PHY_LDO_DELAY_0NS & PHY_LDO_DELAY_200NS not used right?
Should we keep the definitions?
+#define PHY_LDO_DELAY_600NS 0x2
+#define PHY_LDO_SEQ_DELAY(delay, phy)((delay) <<
(2*(phy)+23))
#define PHY_CH_SU_PSR 0x1
#define PHY_CH_DEEP_PSR 0x7
#define PHY_CH_POWER_MODE(mode, phy, ch)((mode) << (6*(phy)+3*(ch)+2))
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 1f800f8..5cd8a51 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1406,6 +1406,8 @@ static void chv_phy_control_init(struct drm_i915_private
*dev_priv)
* value.
*/
dev_priv->chv_phy_control =
+ PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY0) |
+ PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY1) |
PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH0) |
PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH1) |
PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY1, DPIO_CH0);
I think we need to squash this patch to previous one?
[Intel-gfx] [PATCH 1/7] drm/i915: Implement chv display PHY lane stagger setup
http://www.spinics.net/lists/intel-gfx/msg64481.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Work around DISPLAY_PHY_CONTROL register corruption on CHV
On Friday 08 May 2015 06:49 PM, Ville Syrjälä wrote:
On Fri, May 08, 2015 at 06:24:42PM +0530, Deepak S wrote:
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Sometimes (exactly when is a bit unclear) DISPLAY_PHY_CONTROL appears to
get corrupted. The values I've managed to read from it seem to have some
pattern but vary quite a lot. The corruption doesn't seem to just happen
when the register is accessed, but can also happen spontaneosly during
modeset. When this happens during a modeset things go south and the
display doesn't light up.
I've managed to hit the problemn when toggling HDMI on port D on and
off. When things get corrupted the display doesn't light up, but as soon
as I manually write the correct value to the register the display comes
up.
First I was suspicious that we ourselves accidentally overwrite it with
garbage, but didn't catch anything with the reg_rw tracepoint. Also I
sprinkled check all over the modeset path to see exactly when the
corruption happens, and eg. the read back value was fine just before
intel_dp_set_m(), and corrupted immediately after it. I also made my
check function repair the register value whenever it was wrong, and with
this approach the corruption repeated several times during the modeset
operation, always seeming to trigger in the same exact calls to the
check function, while other calls to the function never caught anything.
So far I've not seen this problem occurring when carefully avoiding all
read accesses to DISPLAY_PHY_CONTROL. Not sure if that's just pure luck
or an actual workaround, but we can hope it works. So let's avoid reading
the register and instead track the desired value of the register in dev_priv.
v2: Read out the power well state to determine initial register value
v3: Use DPIO_CHx names instead of raw numbers
Even reading once DISPLAY_PHY_CONTROL layer is getting corrupted?
Not always. I think it somehow depends on what other register accesses
happen around it. So there is perhaps some magic sequence that might allow
reading it, but I decided that it's better to be safe and never read it.
I saw similar issues on my setup. On some platform access phy is causing system
behave inconsistently :(
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_reg.h | 5 -
drivers/gpu/drm/i915/intel_runtime_pm.c | 36
-
3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 822f259..288c3fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1754,6 +1754,8 @@ struct drm_i915_private {
u32 fdi_rx_config;
+ u32 chv_phy_control;
+
u32 suspend_count;
struct i915_suspend_saved_registers regfile;
struct vlv_s0ix_state vlv_s0ix_state;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cfbd5a6..98588d5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1887,7 +1887,10 @@ enum skl_disp_power_wells {
#define DPIO_PHY_STATUS (VLV_DISPLAY_BASE + 0x6240)
#define DPLL_PORTD_READY_MASK (0xf)
#define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
-#define PHY_COM_LANE_RESET_DEASSERT(phy) (1 << (phy))
+#define PHY_CH_SU_PSR0x1
+#define PHY_CH_DEEP_PSR 0x7
PHY_CH_DEEP_PSR defined but not used in this patch?
Just wanted to define it since it's the only other valid value, and the
doc situation is crap. I've not played around with PSR so I'm not
entirely sure how these would be used in practise. My gut is telling me
SU_PSR might be used with link standby and DEEP_PSR with link off, but
that's just a hunch at this point.
right ville, DEEP_PSR is related to link off. I will try to find the docs
related to this.
You'll see in later patches that we'll start using the override bits to
force the power state, so I think at that point these don't even matter.
But I suppose when we enter PSR we should drop the override bits to
allow the hardware to manage the power states based on the PSR mode
selected.
other than this, patch does what it says.
Reviewed-by: Deepak S
+#define PHY_CH_POWER_MODE(mode, phy, ch) ((mode) << (6*(phy)+3*(ch)+2))
+#define PHY_COM_LANE_RESET_DEASSERT(phy) (1 << (phy))
#define DISPLAY_PHY_STATUS (VLV_DISPLAY_BASE + 0x60104)
#define PHY_POWERGOOD(phy) (((phy) == DPIO_PHY0) ? (1<<31) : (1<<30))
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ce00e69..b73671f 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -666,8 +666
Re: [Intel-gfx] [PATCH 4/7] drm/i915: Use the default 600ns LDO programming sequence delay
On Friday 08 May 2015 06:52 PM, Ville Syrjälä wrote:
On Fri, May 08, 2015 at 06:31:23PM +0530, Deepak S wrote:
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Not sure which LDO programming sequence delay should be used for the CHV
PHY, but the spec says that 600ns is "Used by default for initial
bringup", and the BIOS seems to use that, so let's do the same.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 4
drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 98588d5..977bad6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1887,6 +1887,10 @@ enum skl_disp_power_wells {
#define DPIO_PHY_STATUS (VLV_DISPLAY_BASE + 0x6240)
#define DPLL_PORTD_READY_MASK (0xf)
#define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
+#define PHY_LDO_DELAY_0NS0x0
+#define PHY_LDO_DELAY_200NS 0x1
PHY_LDO_DELAY_0NS & PHY_LDO_DELAY_200NS not used right?
Should we keep the definitions?
I generally like to keep a bit of extra for VLV/CHV due to the bad doc
situation.
+#define PHY_LDO_DELAY_600NS 0x2
+#define PHY_LDO_SEQ_DELAY(delay, phy)((delay) <<
(2*(phy)+23))
#define PHY_CH_SU_PSR 0x1
#define PHY_CH_DEEP_PSR0x7
#define PHY_CH_POWER_MODE(mode, phy, ch) ((mode) << (6*(phy)+3*(ch)+2))
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 1f800f8..5cd8a51 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1406,6 +1406,8 @@ static void chv_phy_control_init(struct drm_i915_private
*dev_priv)
* value.
*/
dev_priv->chv_phy_control =
+ PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY0) |
+ PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY1) |
PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH0) |
PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH1) |
PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY1, DPIO_CH0);
I think we need to squash this patch to previous one?
[Intel-gfx] [PATCH 1/7] drm/i915: Implement chv display PHY lane stagger setup
http://www.spinics.net/lists/intel-gfx/msg64481.html
Well, IIRC I never saw any real issues with the 0ns delay either, with
or without the lane stagger setup. So not much point in squashing IMO.
Thanks for the clarification :)
Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Only wait for required lanes in vlv_wait_port_ready()
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Currently vlv_wait_port_ready() waits for all four lanes on the
appropriate channel. This no longer works on CHV when the unused
lanes may be power gated. So pass in a mask of lanes that the
caller is expecting to be ready.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 10 ++
drivers/gpu/drm/i915/intel_dp.c | 4 +++-
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_hdmi.c| 4 ++--
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7bfe2af..2aa8055 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1819,7 +1819,8 @@ static void chv_disable_pll(struct drm_i915_private
*dev_priv, enum pipe pipe)
}
void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
- struct intel_digital_port *dport)
+struct intel_digital_port *dport,
+unsigned int expected_mask)
{
u32 port_mask;
int dpll_reg;
@@ -1832,6 +1833,7 @@ void vlv_wait_port_ready(struct drm_i915_private
*dev_priv,
case PORT_C:
port_mask = DPLL_PORTC_READY_MASK;
dpll_reg = DPLL(0);
+ expected_mask <<= 4;
break;
case PORT_D:
port_mask = DPLL_PORTD_READY_MASK;
@@ -1841,9 +1843,9 @@ void vlv_wait_port_ready(struct drm_i915_private
*dev_priv,
BUG();
}
- if (wait_for((I915_READ(dpll_reg) & port_mask) == 0, 1000))
- WARN(1, "timed out waiting for port %c ready: 0x%08x\n",
-port_name(dport->port), I915_READ(dpll_reg));
+ if (wait_for((I915_READ(dpll_reg) & port_mask) == expected_mask, 1000))
+ WARN(1, "timed out waiting for port %c ready: got 0x%x, expected
0x%x\n",
+port_name(dport->port), I915_READ(dpll_reg) & port_mask,
expected_mask);
}
static void intel_prepare_shared_dpll(struct intel_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7401fa3..ac38fd8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2472,6 +2472,7 @@ static void intel_enable_dp(struct intel_encoder *encoder)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
uint32_t dp_reg = I915_READ(intel_dp->output_reg);
+ unsigned int lane_mask = 0x0;
if (WARN_ON(dp_reg & DP_PORT_EN))
return;
@@ -2490,7 +2491,8 @@ static void intel_enable_dp(struct intel_encoder *encoder)
pps_unlock(intel_dp);
if (IS_VALLEYVIEW(dev))
- vlv_wait_port_ready(dev_priv, dp_to_dig_port(intel_dp));
+ vlv_wait_port_ready(dev_priv, dp_to_dig_port(intel_dp),
+ lane_mask);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index efa53d5..3ec829a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -951,7 +951,8 @@ intel_wait_for_vblank(struct drm_device *dev, int pipe)
}
int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
-struct intel_digital_port *dport);
+struct intel_digital_port *dport,
+unsigned int expected_mask);
bool intel_get_load_detect_pipe(struct drm_connector *connector,
struct drm_display_mode *mode,
struct intel_load_detect_pipe *old,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index a24e2c8..24b0aa1 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1319,7 +1319,7 @@ static void vlv_hdmi_pre_enable(struct intel_encoder
*encoder)
intel_enable_hdmi(encoder);
- vlv_wait_port_ready(dev_priv, dport);
+ vlv_wait_port_ready(dev_priv, dport, 0x0);
}
static void vlv_hdmi_pre_pll_enable(struct intel_encoder *encoder)
@@ -1636,7 +1636,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder
*encoder)
intel_enable_hdmi(encoder);
- vlv_wait_port_ready(dev_priv, dport);
+ vlv_wait_port_ready(dev_priv, dport, 0x0);
}
static void intel_hdmi_destroy(struct drm_connector *connector)
Looks good.
Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: Implement PHY lane power gating for CHV
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Powergate the PHY lanes when they're not needed. For HDMI all four lanes
are needed always, but for DP we can enable only the needed lanes. And
when the port is not used all lanes can be power gated. This could
reduce power consumption a bit when only a subset of the lanes in the
PHY are required.
A bit of extra care is needed to reconstruct the initial state of the
DPIO_PHY_CONTROL register since we can't read it. So instead we read the
actual lane status from the DPLL/PHY_STATUS registers and use that to
determine which lanes ought to be powergated initially.
Also sprinkle a few debug prints around so that we can monitor the
DPIO_PHY_STATUS changes without having to read it and risk corrupting
it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 2 +
drivers/gpu/drm/i915/intel_dp.c | 8
drivers/gpu/drm/i915/intel_drv.h| 2 +
drivers/gpu/drm/i915/intel_hdmi.c | 5 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 69 +++--
5 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 977bad6..34c366a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1887,10 +1887,12 @@ enum skl_disp_power_wells {
#define DPIO_PHY_STATUS (VLV_DISPLAY_BASE + 0x6240)
#define DPLL_PORTD_READY_MASK (0xf)
#define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
+#define PHY_CH_POWER_DOWN_OVRD_EN(phy, ch) (1 << (2*(phy)+(ch)+27))
#define PHY_LDO_DELAY_0NS 0x0
#define PHY_LDO_DELAY_200NS 0x1
#define PHY_LDO_DELAY_600NS 0x2
#define PHY_LDO_SEQ_DELAY(delay, phy) ((delay) <<
(2*(phy)+23))
+#define PHY_CH_POWER_DOWN_OVRD(mask, phy, ch)((mask) <<
(8*(phy)+4*(ch)+11))
#define PHY_CH_SU_PSR 0x1
#define PHY_CH_DEEP_PSR 0x7
#define PHY_CH_POWER_MODE(mode, phy, ch)((mode) << (6*(phy)+3*(ch)+2))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ac38fd8..0b43f99 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2336,6 +2336,8 @@ static void chv_post_disable_dp(struct intel_encoder
*encoder)
intel_dp_link_down(intel_dp);
+ chv_powergate_phy_lanes(encoder, 0xf);
+
mutex_lock(&dev_priv->dpio_lock);
/* Propagate soft reset to data lane reset */
@@ -2482,6 +2484,12 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
if (IS_VALLEYVIEW(dev))
vlv_init_panel_power_sequencer(intel_dp);
+ if (IS_CHERRYVIEW(dev)) {
+ /* FIXME deal with lane reversal */
+ lane_mask = 0xf & ~((1 << intel_dp->lane_count) - 1);
+ chv_powergate_phy_lanes(encoder, lane_mask);
+ }
+
intel_dp_enable_port(intel_dp);
edp_panel_vdd_on(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3ec829a..54bcca8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1238,6 +1238,8 @@ void intel_runtime_pm_put(struct drm_i915_private
*dev_priv);
void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
+void chv_powergate_phy_lanes(struct intel_encoder *encoder, unsigned int mask);
+
/* intel_pm.c */
void intel_init_clock_gating(struct drm_device *dev);
void intel_suspend_hw(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 24b0aa1..f5842c3 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -916,6 +916,9 @@ static void intel_disable_hdmi(struct intel_encoder
*encoder)
I915_WRITE(intel_hdmi->hdmi_reg, temp);
POSTING_READ(intel_hdmi->hdmi_reg);
}
+
+ if (IS_CHERRYVIEW(dev))
+ chv_powergate_phy_lanes(encoder, 0xf);
}
static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit)
@@ -1634,6 +1637,8 @@ static void chv_hdmi_pre_enable(struct intel_encoder
*encoder)
intel_crtc->config->has_hdmi_sink,
adjusted_mode);
+ chv_powergate_phy_lanes(encoder, 0x0);
+
intel_enable_hdmi(encoder);
vlv_wait_port_ready(dev_priv, dport, 0x0);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 5cd8a51..d9e00d3 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -668,6 +668,9 @@ static void chv_dpio_cmn_power_well_enable(struct
drm_i915_private *dev_priv,
dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(phy);
I
Re: [Intel-gfx] [PATCH 7/7] drm/i915: Throw out WIP CHV power well definitions
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Expecting CHV power wells to be just an extended versions of the VLV
power wells, a bunch of commented out power wells were added in
anticipation when Punit folks would implement it all. Turns out they
never did, and instead CHV has fewer power wells than VLV. Rip out all
the #if 0'ed junk that's not needed.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 4 --
drivers/gpu/drm/i915/intel_runtime_pm.c | 97 +
2 files changed, 3 insertions(+), 98 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 34c366a..f2e0d58 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -595,10 +595,6 @@ enum punit_power_well {
PUNIT_POWER_WELL_DPIO_RX0 = 10,
PUNIT_POWER_WELL_DPIO_RX1 = 11,
PUNIT_POWER_WELL_DPIO_CMN_D = 12,
- /* FIXME: guesswork below */
- PUNIT_POWER_WELL_DPIO_TX_D_LANES_01 = 13,
- PUNIT_POWER_WELL_DPIO_TX_D_LANES_23 = 14,
- PUNIT_POWER_WELL_DPIO_RX2 = 15,
PUNIT_POWER_WELL_NUM,
};
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index d9e00d3..f208806 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -977,18 +977,6 @@ void intel_display_power_put(struct drm_i915_private
*dev_priv,
BIT(POWER_DOMAIN_AUX_C) | \
BIT(POWER_DOMAIN_INIT))
-#define CHV_PIPE_A_POWER_DOMAINS ( \
- BIT(POWER_DOMAIN_PIPE_A) | \
- BIT(POWER_DOMAIN_INIT))
-
-#define CHV_PIPE_B_POWER_DOMAINS ( \
- BIT(POWER_DOMAIN_PIPE_B) | \
- BIT(POWER_DOMAIN_INIT))
-
-#define CHV_PIPE_C_POWER_DOMAINS ( \
- BIT(POWER_DOMAIN_PIPE_C) | \
- BIT(POWER_DOMAIN_INIT))
-
#define CHV_DPIO_CMN_BC_POWER_DOMAINS ( \
BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) | \
BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) | \
@@ -1004,17 +992,6 @@ void intel_display_power_put(struct drm_i915_private
*dev_priv,
BIT(POWER_DOMAIN_AUX_D) | \
BIT(POWER_DOMAIN_INIT))
-#define CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS ( \
- BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) | \
- BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) | \
- BIT(POWER_DOMAIN_AUX_D) | \
- BIT(POWER_DOMAIN_INIT))
-
-#define CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS ( \
- BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) | \
- BIT(POWER_DOMAIN_AUX_D) | \
- BIT(POWER_DOMAIN_INIT))
-
static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
.sync_hw = i9xx_always_on_power_well_noop,
.enable = i9xx_always_on_power_well_noop,
@@ -1172,40 +1149,16 @@ static struct i915_power_well chv_power_wells[] = {
.domains = VLV_ALWAYS_ON_POWER_DOMAINS,
.ops = &i9xx_always_on_power_well_ops,
},
-#if 0
- {
- .name = "display",
- .domains = VLV_DISPLAY_POWER_DOMAINS,
- .data = PUNIT_POWER_WELL_DISP2D,
- .ops = &vlv_display_power_well_ops,
- },
-#endif
{
.name = "pipe-a",
/*
-* FIXME: pipe A power well seems to be the new disp2d well.
-* At least all registers seem to be housed there. Figure
-* out if this a a temporary situation in pre-production
-* hardware or a permanent state of affairs.
+* pipe A power well is the new disp2d well.
+* pipe B and C wells don't actually exist.
Can we add a comment saying "enabling pipe A sub system will also enable pipe B &
c"
Because it is confusing, We says pipe B and C wells don't actually exist,
then if we use PIPE B to drive. how is it working without powering up the well?
Other than this. patch looks fine
Reviewed-by: Deepak S
*/
- .domains = CHV_PIPE_A_POWER_DOMAINS | VLV_DISPLAY_POWER_DOMAINS,
+ .domains = VLV_DISPLAY_POWER_DOMAINS,
.data = PIPE_A,
.ops = &chv_pipe_power_well_ops,
},
-#if 0
- {
- .name = "pipe-b",
- .domains = CHV_PIPE_B_POWER_DOMAINS,
- .data = PIPE_B,
- .ops = &chv_pipe_power_well_ops,
- },
- {
- .name = "pipe-c",
- .domains = CHV_PIPE_C_POWER_DOMAINS,
- .data = PIPE_C,
- .ops = &chv_pipe_power_well_ops,
- },
-#endif
{
.name = "dpio-common-bc",
.domains = CHV_DPIO_CMN_BC_POWER_DOMAINS,
@@ -1218,50 +1171,6 @@
Re: [Intel-gfx] [PATCH v3] drm/i915: Setup static bias for GPU
On Wednesday 06 May 2015 02:32 PM, Daniel Vetter wrote: On Tue, May 05, 2015 at 01:12:41PM +0530, Deepak S wrote: On Monday 04 May 2015 08:58 PM, Ville Syrjälä wrote: On Mon, May 04, 2015 at 10:12:23AM +0200, Daniel Vetter wrote: On Mon, May 04, 2015 at 10:58:02AM +0530, Deepak S wrote: On Wednesday 29 April 2015 02:59 PM, Ville Syrjälä wrote: On Wed, Apr 29, 2015 at 08:36:24AM +0530, deepa...@linux.intel.com wrote: From: Deepak S Based on the spec, Setting up static BIAS for GPU to improve the rps performace. v2: rename reg defn to match spec. (Ville) v3: Updated bias setting for chv (Deepak) Signed-off-by: Deepak S Matches the spec. Whether the chosen bias is really the best, I can't really say. But favoring the GPU does seem like a sensible idea if we want to keep the UI stuff fluid enough while there's some CPU heavy tasks running at the same time. Reviewed-by: Ville Syrjälä Thanks Ville for reviewing, Yes our aim is to keep user experience smooth. Since the aim is smooth UI ... how does this interact with the rps boosting that was just enabled with Chris' patches for vlv/chv too? A static bias seems a lot less what we want now that we should have something dynamic. Specifically I'm thinking of commit 6ad790c0f5ac55fd13f322c23519f0d6f0721864 Author: Chris Wilson Date: Tue Apr 7 16:20:31 2015 +0100 drm/i915: Boost GPU frequency if we detect outstanding pageflips Totally separate topic. This only affects how the Punit splits up the available energy credits between the CPU and the GPU. So only relevant when thermally constrained and both CPU and GPU would like to run faster than the limit allows. Thanks Ville. This is completely a different topic, more to do with punit budget constraints & this does not impact "Boot RPS logic" Well I thought it'd be related since the justification was better UI interactivity. And that tends to not be a thermal constrained load but something really spike-y. And a static bias doesn't seem like a solution to that problem. I'll just go ahead and merge, but still feels like at least I don't know why exactly we need this. -Daniel Hi Daniel, Dynamic power bias is not supported by the HW or FW :(. This static bias was recommended by HW team after analyzing the results on IA & GT Thanks Deepak ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/2] drm/i915/vlv: Remove wait for for punit to updates freq.
From: Deepak S
When GPU is idle on VLV, Request freq to punit should be good enough to
get the voltage back to VNN. Also, make sure gfx clock force applies
before requesting the freq fot vlv.
v2: Do forcewake before setting idle frequency (ville)
Update function comments to match the code (Deepak)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
suggested-by: Jesse Barnes
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_pm.c | 41 +++--
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7516ed..3df929a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4043,51 +4043,32 @@ static void valleyview_set_rps(struct drm_device *dev,
u8 val)
trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
}
-/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+/* vlv_set_rps_idle: Set the frequency to idle, 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
+ * 1. Forcewake Media well.
+ * 2. Request idle freq.
+ * 3. Release Forcewake of Media well.
*/
static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
u32 val = dev_priv->rps.idle_freq;
- /* CHV and latest VLV don't need to force the gfx clock */
- if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+ /* CHV don't need to force the gfx clock */
+ if (IS_CHERRYVIEW(dev)) {
valleyview_set_rps(dev_priv->dev, val);
return;
}
- /*
-* When we are idle. Drop to min voltage state.
-*/
-
if (dev_priv->rps.cur_freq <= val)
return;
- /* Mask turbo interrupt so that they will not come in between */
- I915_WRITE(GEN6_PMINTRMSK,
- gen6_sanitize_rps_pm_mask(dev_priv, ~0));
-
- vlv_force_gfx_clock(dev_priv, true);
-
- dev_priv->rps.cur_freq = val;
-
- vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
-
- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
- & GENFREQSTATUS) == 0, 100))
- DRM_ERROR("timed out waiting for Punit\n");
-
- gen6_set_rps_thresholds(dev_priv, val);
- vlv_force_gfx_clock(dev_priv, false);
-
- I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
+ /* Wake up the media well, as that takes a lot less
+* power than the Render well. */
+ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
+ valleyview_set_rps(dev_priv->dev, val);
+ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
}
void gen6_rps_busy(struct drm_i915_private *dev_priv)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv
From: Deepak S
After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
frequency to RPn, punit is failing to change the input voltage to
minimum :(
Since Punit validates the rps range [RPe, RP0]. This patch
removes unused cherryview_rps_min_freq function.
v2: Change commit message
v3: set min_freq before idle_freq (chris)
v4: Squash 'Remove unused rps min function' patch
Signed-off-by: Deepak S
Acked-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_pm.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 852f756..b6b14a4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct
drm_i915_private *dev_priv)
return rp1;
}
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
- struct drm_device *dev = dev_priv->dev;
- u32 val, rpn;
-
- if (dev->pdev->revision >= 0x20) {
- val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
- rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
- FB_GFX_FREQ_FUSE_MASK);
- } else { /* For pre-production hardware */
- val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
- rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
- PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
- }
-
- return rpn;
-}
-
static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
{
u32 val, rp1;
@@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct
drm_device *dev)
intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
dev_priv->rps.rp1_freq);
- dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+ /* PUnit validated range is only [RPe, RP0] */
+ dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
dev_priv->rps.min_freq);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/2] drm/i915/chv: Extend set idle rps wa to chv
From: Deepak S
It is obsered on BSW that requesting a new frequency from Punit
does nothing when the GPU is in rc6, and if we let it enter rc6 with a
high frequency Vnn also remains high. Extending vlv_set_rps_idle()
workaround on CHV/BSW.
suggested-by: Ville Syrjälä
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_pm.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3df929a..852f756 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4052,15 +4052,8 @@ static void valleyview_set_rps(struct drm_device *dev,
u8 val)
*/
static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
{
- struct drm_device *dev = dev_priv->dev;
u32 val = dev_priv->rps.idle_freq;
- /* CHV don't need to force the gfx clock */
- if (IS_CHERRYVIEW(dev)) {
- valleyview_set_rps(dev_priv->dev, val);
- return;
- }
-
if (dev_priv->rps.cur_freq <= val)
return;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/vlv: Remove wait for for punit to updates freq.
On Friday 08 May 2015 10:04 PM, Ville Syrjälä wrote:
On Fri, May 08, 2015 at 08:43:10PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
When GPU is idle on VLV, Request freq to punit should be good enough to
get the voltage back to VNN. Also, make sure gfx clock force applies
before requesting the freq fot vlv.
v2: Do forcewake before setting idle frequency (ville)
Update function comments to match the code (Deepak)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
suggested-by: Jesse Barnes
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_pm.c | 41 +++--
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7516ed..3df929a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4043,51 +4043,32 @@ static void valleyview_set_rps(struct drm_device *dev,
u8 val)
trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
}
-/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+/* vlv_set_rps_idle: Set the frequency to idle, 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
+ * 1. Forcewake Media well.
+ * 2. Request idle freq.
+ * 3. Release Forcewake of Media well.
*/
static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
u32 val = dev_priv->rps.idle_freq;
- /* CHV and latest VLV don't need to force the gfx clock */
- if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+ /* CHV don't need to force the gfx clock */
+ if (IS_CHERRYVIEW(dev)) {
valleyview_set_rps(dev_priv->dev, val);
return;
}
- /*
-* When we are idle. Drop to min voltage state.
-*/
-
if (dev_priv->rps.cur_freq <= val)
return;
- /* Mask turbo interrupt so that they will not come in between */
- I915_WRITE(GEN6_PMINTRMSK,
- gen6_sanitize_rps_pm_mask(dev_priv, ~0));
-
- vlv_force_gfx_clock(dev_priv, true);
-
- dev_priv->rps.cur_freq = val;
-
- vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
-
- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
- & GENFREQSTATUS) == 0, 100))
- DRM_ERROR("timed out waiting for Punit\n");
-
- gen6_set_rps_thresholds(dev_priv, val);
- vlv_force_gfx_clock(dev_priv, false);
-
- I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
+ /* Wake up the media well, as that takes a lot less
+* power than the Render well. */
+ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
+ valleyview_set_rps(dev_priv->dev, val);
+ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
_put
With that fixed this is:
Reviewed-by: Ville Syrjälä
Oops :) Copy paste problem :)
}
void gen6_rps_busy(struct drm_i915_private *dev_priv)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv
On Friday 08 May 2015 10:09 PM, Ville Syrjälä wrote:
On Fri, May 08, 2015 at 08:43:12PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
frequency to RPn, punit is failing to change the input voltage to
minimum :(
As I mentioned I've been unable to reproduce that particular problem
on my BSW. Perhaps add a note about that in the commit message.
Issue is Vgg_in voltage not getting dropped :(. Vnn observation is same as what
your seeing.
We saw this issue on CHV platform and confirmed by punit team.
Let me update the commit msg.
Since Punit validates the rps range [RPe, RP0]. This patch
removes unused cherryview_rps_min_freq function.
But I can accept that we should stick to the validated range, so I
can slap an r-b on the patch anyway:
Reviewed-by: Ville Syrjälä
v2: Change commit message
v3: set min_freq before idle_freq (chris)
v4: Squash 'Remove unused rps min function' patch
Signed-off-by: Deepak S
Acked-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_pm.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 852f756..b6b14a4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct
drm_i915_private *dev_priv)
return rp1;
}
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
- struct drm_device *dev = dev_priv->dev;
- u32 val, rpn;
-
- if (dev->pdev->revision >= 0x20) {
- val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
- rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
- FB_GFX_FREQ_FUSE_MASK);
- } else { /* For pre-production hardware */
- val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
- rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
- PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
- }
-
- return rpn;
-}
-
static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
{
u32 val, rp1;
@@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct
drm_device *dev)
intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
dev_priv->rps.rp1_freq);
- dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+ /* PUnit validated range is only [RPe, RP0] */
+ dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
dev_priv->rps.min_freq);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: Implement PHY lane power gating for CHV
On Friday 08 May 2015 09:35 PM, Ville Syrjälä wrote:
On Fri, May 08, 2015 at 08:19:12PM +0530, Deepak S wrote:
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Powergate the PHY lanes when they're not needed. For HDMI all four lanes
are needed always, but for DP we can enable only the needed lanes. And
when the port is not used all lanes can be power gated. This could
reduce power consumption a bit when only a subset of the lanes in the
PHY are required.
A bit of extra care is needed to reconstruct the initial state of the
DPIO_PHY_CONTROL register since we can't read it. So instead we read the
actual lane status from the DPLL/PHY_STATUS registers and use that to
determine which lanes ought to be powergated initially.
Also sprinkle a few debug prints around so that we can monitor the
DPIO_PHY_STATUS changes without having to read it and risk corrupting
it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 2 +
drivers/gpu/drm/i915/intel_dp.c | 8
drivers/gpu/drm/i915/intel_drv.h| 2 +
drivers/gpu/drm/i915/intel_hdmi.c | 5 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 69
+++--
5 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 977bad6..34c366a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1887,10 +1887,12 @@ enum skl_disp_power_wells {
#define DPIO_PHY_STATUS (VLV_DISPLAY_BASE + 0x6240)
#define DPLL_PORTD_READY_MASK (0xf)
#define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
+#define PHY_CH_POWER_DOWN_OVRD_EN(phy, ch) (1 << (2*(phy)+(ch)+27))
#define PHY_LDO_DELAY_0NS 0x0
#define PHY_LDO_DELAY_200NS0x1
#define PHY_LDO_DELAY_600NS0x2
#define PHY_LDO_SEQ_DELAY(delay, phy) ((delay) <<
(2*(phy)+23))
+#define PHY_CH_POWER_DOWN_OVRD(mask, phy, ch)((mask) <<
(8*(phy)+4*(ch)+11))
#define PHY_CH_SU_PSR 0x1
#define PHY_CH_DEEP_PSR0x7
#define PHY_CH_POWER_MODE(mode, phy, ch) ((mode) << (6*(phy)+3*(ch)+2))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ac38fd8..0b43f99 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2336,6 +2336,8 @@ static void chv_post_disable_dp(struct intel_encoder
*encoder)
intel_dp_link_down(intel_dp);
+ chv_powergate_phy_lanes(encoder, 0xf);
+
mutex_lock(&dev_priv->dpio_lock);
/* Propagate soft reset to data lane reset */
@@ -2482,6 +2484,12 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
if (IS_VALLEYVIEW(dev))
vlv_init_panel_power_sequencer(intel_dp);
+ if (IS_CHERRYVIEW(dev)) {
+ /* FIXME deal with lane reversal */
+ lane_mask = 0xf & ~((1 << intel_dp->lane_count) - 1);
+ chv_powergate_phy_lanes(encoder, lane_mask);
+ }
+
intel_dp_enable_port(intel_dp);
edp_panel_vdd_on(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3ec829a..54bcca8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1238,6 +1238,8 @@ void intel_runtime_pm_put(struct drm_i915_private
*dev_priv);
void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
+void chv_powergate_phy_lanes(struct intel_encoder *encoder, unsigned int mask);
+
/* intel_pm.c */
void intel_init_clock_gating(struct drm_device *dev);
void intel_suspend_hw(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 24b0aa1..f5842c3 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -916,6 +916,9 @@ static void intel_disable_hdmi(struct intel_encoder
*encoder)
I915_WRITE(intel_hdmi->hdmi_reg, temp);
POSTING_READ(intel_hdmi->hdmi_reg);
}
+
+ if (IS_CHERRYVIEW(dev))
+ chv_powergate_phy_lanes(encoder, 0xf);
}
static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit)
@@ -1634,6 +1637,8 @@ static void chv_hdmi_pre_enable(struct intel_encoder
*encoder)
intel_crtc->config->has_hdmi_sink,
adjusted_mode);
+ chv_powergate_phy_lanes(encoder, 0x0);
+
intel_enable_hdmi(encoder);
vlv_wait_port_ready(dev_priv, dport, 0x0);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 5cd8a51..d9e00d3 100644
--- a/drivers/gpu/drm/i915/intel_runtime_p
[Intel-gfx] [PATCH v3] drm/i915/vlv: Remove wait for for punit to updates freq.
From: Deepak S
When GPU is idle on VLV, Request freq to punit should be good enough to
get the voltage back to VNN. Also, make sure gfx clock force applies
before requesting the freq fot vlv.
v2: Do forcewake before setting idle frequency (ville)
Update function comments to match the code (Deepak)
v3: Fix get/put across idle frequency Request. (Ville)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
suggested-by: Jesse Barnes
Signed-off-by: Deepak S
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 41 +++--
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7516ed..82dfdbc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4043,51 +4043,32 @@ static void valleyview_set_rps(struct drm_device *dev,
u8 val)
trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val));
}
-/* vlv_set_rps_idle: Set the frequency to Rpn if Gfx clocks are down
+/* vlv_set_rps_idle: Set the frequency to idle, 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
+ * 1. Forcewake Media well.
+ * 2. Request idle freq.
+ * 3. Release Forcewake of Media well.
*/
static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
u32 val = dev_priv->rps.idle_freq;
- /* CHV and latest VLV don't need to force the gfx clock */
- if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+ /* CHV don't need to force the gfx clock */
+ if (IS_CHERRYVIEW(dev)) {
valleyview_set_rps(dev_priv->dev, val);
return;
}
- /*
-* When we are idle. Drop to min voltage state.
-*/
-
if (dev_priv->rps.cur_freq <= val)
return;
- /* Mask turbo interrupt so that they will not come in between */
- I915_WRITE(GEN6_PMINTRMSK,
- gen6_sanitize_rps_pm_mask(dev_priv, ~0));
-
- vlv_force_gfx_clock(dev_priv, true);
-
- dev_priv->rps.cur_freq = val;
-
- vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
-
- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
- & GENFREQSTATUS) == 0, 100))
- DRM_ERROR("timed out waiting for Punit\n");
-
- gen6_set_rps_thresholds(dev_priv, val);
- vlv_force_gfx_clock(dev_priv, false);
-
- I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
+ /* Wake up the media well, as that takes a lot less
+* power than the Render well. */
+ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
+ valleyview_set_rps(dev_priv->dev, val);
+ intel_uncore_forcewake_put(dev_priv, FORCEWAKE_MEDIA);
}
void gen6_rps_busy(struct drm_i915_private *dev_priv)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/chv: Extend set idle rps wa to chv
From: Deepak S
It is observed on BSW that requesting a new frequency from Punit
does nothing when the GPU is in rc6, and if we let GPU enter rc6
with a high frequency, Vnn remains slightly higher than at minimum
frequency. Extending vlv_set_rps_idle() workaround on CHV/BSW.
v2: Update commit msg (Ville)
suggested-by: Ville Syrjälä
Signed-off-by: Deepak S
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 82dfdbc..064f11a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4052,15 +4052,8 @@ static void valleyview_set_rps(struct drm_device *dev,
u8 val)
*/
static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
{
- struct drm_device *dev = dev_priv->dev;
u32 val = dev_priv->rps.idle_freq;
- /* CHV don't need to force the gfx clock */
- if (IS_CHERRYVIEW(dev)) {
- valleyview_set_rps(dev_priv->dev, val);
- return;
- }
-
if (dev_priv->rps.cur_freq <= val)
return;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv
From: Deepak S
After feedback from the hardware team, now we set the GPU min/idel freq to RPe.
Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the
frequency to RPn, punit is failing to change the vgg input voltage to
minimum :(
Since Punit validates the rps range [RPe, RP0]. This patch
removes unused cherryview_rps_min_freq function.
v2: Change commit message
v3: set min_freq before idle_freq (chris)
v4: Squash 'Remove unused rps min function' patch
Signed-off-by: Deepak S
Acked-by: Chris Wilson
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 064f11a..c229d7e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4689,24 +4689,6 @@ static int cherryview_rps_guar_freq(struct
drm_i915_private *dev_priv)
return rp1;
}
-static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
-{
- struct drm_device *dev = dev_priv->dev;
- u32 val, rpn;
-
- if (dev->pdev->revision >= 0x20) {
- val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
- rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
- FB_GFX_FREQ_FUSE_MASK);
- } else { /* For pre-production hardware */
- val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
- rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
- PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
- }
-
- return rpn;
-}
-
static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
{
u32 val, rp1;
@@ -4958,7 +4940,8 @@ static void cherryview_init_gt_powersave(struct
drm_device *dev)
intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
dev_priv->rps.rp1_freq);
- dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
+ /* PUnit validated range is only [RPe, RP0] */
+ dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
dev_priv->rps.min_freq);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: Implement PHY lane power gating for CHV
On Monday 11 May 2015 05:13 PM, Ville Syrjälä wrote:
On Sat, May 09, 2015 at 11:05:27AM +0530, Deepak S wrote:
On Friday 08 May 2015 09:35 PM, Ville Syrjälä wrote:
On Fri, May 08, 2015 at 08:19:12PM +0530, Deepak S wrote:
On Friday 10 April 2015 08:51 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Powergate the PHY lanes when they're not needed. For HDMI all four lanes
are needed always, but for DP we can enable only the needed lanes. And
when the port is not used all lanes can be power gated. This could
reduce power consumption a bit when only a subset of the lanes in the
PHY are required.
A bit of extra care is needed to reconstruct the initial state of the
DPIO_PHY_CONTROL register since we can't read it. So instead we read the
actual lane status from the DPLL/PHY_STATUS registers and use that to
determine which lanes ought to be powergated initially.
Also sprinkle a few debug prints around so that we can monitor the
DPIO_PHY_STATUS changes without having to read it and risk corrupting
it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 2 +
drivers/gpu/drm/i915/intel_dp.c | 8
drivers/gpu/drm/i915/intel_drv.h| 2 +
drivers/gpu/drm/i915/intel_hdmi.c | 5 +++
drivers/gpu/drm/i915/intel_runtime_pm.c | 69
+++--
5 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 977bad6..34c366a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1887,10 +1887,12 @@ enum skl_disp_power_wells {
#define DPIO_PHY_STATUS (VLV_DISPLAY_BASE + 0x6240)
#define DPLL_PORTD_READY_MASK (0xf)
#define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
+#define PHY_CH_POWER_DOWN_OVRD_EN(phy, ch) (1 << (2*(phy)+(ch)+27))
#define PHY_LDO_DELAY_0NS 0x0
#define PHY_LDO_DELAY_200NS 0x1
#define PHY_LDO_DELAY_600NS 0x2
#define PHY_LDO_SEQ_DELAY(delay, phy) ((delay) <<
(2*(phy)+23))
+#define PHY_CH_POWER_DOWN_OVRD(mask, phy, ch)((mask) <<
(8*(phy)+4*(ch)+11))
#define PHY_CH_SU_PSR 0x1
#define PHY_CH_DEEP_PSR 0x7
#define PHY_CH_POWER_MODE(mode, phy, ch) ((mode) << (6*(phy)+3*(ch)+2))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ac38fd8..0b43f99 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2336,6 +2336,8 @@ static void chv_post_disable_dp(struct intel_encoder
*encoder)
intel_dp_link_down(intel_dp);
+ chv_powergate_phy_lanes(encoder, 0xf);
+
mutex_lock(&dev_priv->dpio_lock);
/* Propagate soft reset to data lane reset */
@@ -2482,6 +2484,12 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
if (IS_VALLEYVIEW(dev))
vlv_init_panel_power_sequencer(intel_dp);
+ if (IS_CHERRYVIEW(dev)) {
+ /* FIXME deal with lane reversal */
+ lane_mask = 0xf & ~((1 << intel_dp->lane_count) - 1);
+ chv_powergate_phy_lanes(encoder, lane_mask);
+ }
+
intel_dp_enable_port(intel_dp);
edp_panel_vdd_on(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3ec829a..54bcca8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1238,6 +1238,8 @@ void intel_runtime_pm_put(struct drm_i915_private
*dev_priv);
void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
+void chv_powergate_phy_lanes(struct intel_encoder *encoder, unsigned int mask);
+
/* intel_pm.c */
void intel_init_clock_gating(struct drm_device *dev);
void intel_suspend_hw(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 24b0aa1..f5842c3 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -916,6 +916,9 @@ static void intel_disable_hdmi(struct intel_encoder
*encoder)
I915_WRITE(intel_hdmi->hdmi_reg, temp);
POSTING_READ(intel_hdmi->hdmi_reg);
}
+
+ if (IS_CHERRYVIEW(dev))
+ chv_powergate_phy_lanes(encoder, 0xf);
}
static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit)
@@ -1634,6 +1637,8 @@ static void chv_hdmi_pre_enable(struct intel_encoder
*encoder)
intel_crtc->config->has_hdmi_sink,
adjusted_mode);
+ chv_powergate_phy_lanes(encoder, 0x0);
+
intel_enable_hdmi(encoder);
vlv_wait_port_ready(dev_priv, dport, 0x0);
diff --git a/drivers/gpu/drm/
[Intel-gfx] [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake
From: Deepak S
Based on the HW team inputs. We can should not wait for the old ack,
Waiting for old ack might fail, when other forcewake came before the
present one is desserted.
for example, if forcewake bit 0 was set and before it could get cleared
forcewake bit 1 got set, HW eventually clear bit 0, when the bit 1
is cleared. i.e, bit 1 is still sent then forcewake bit 0 will still be
set.
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_uncore.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 918b761..bcb0806 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -196,11 +196,6 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
{
/* Check for Render Engine */
if (FORCEWAKE_RENDER & fw_engine) {
- if (wait_for_atomic((__raw_i915_read32(dev_priv,
- FORCEWAKE_ACK_VLV) &
- FORCEWAKE_KERNEL) == 0,
- FORCEWAKE_ACK_TIMEOUT_MS))
- DRM_ERROR("Timed out: Render forcewake old ack to
clear.\n");
__raw_i915_write32(dev_priv, FORCEWAKE_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
@@ -214,11 +209,6 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
/* Check for Media Engine */
if (FORCEWAKE_MEDIA & fw_engine) {
- if (wait_for_atomic((__raw_i915_read32(dev_priv,
- FORCEWAKE_ACK_MEDIA_VLV) &
- FORCEWAKE_KERNEL) == 0,
- FORCEWAKE_ACK_TIMEOUT_MS))
- DRM_ERROR("Timed out: Media forcewake old ack to
clear.\n");
__raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake
On Thursday 18 September 2014 06:38 PM, Ville Syrjälä wrote:
On Thu, Sep 18, 2014 at 01:53:13PM +0200, Daniel Vetter wrote:
On Thu, Sep 18, 2014 at 11:39:25AM +0300, Ville Syrjälä wrote:
On Thu, Sep 18, 2014 at 06:51:50PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
Based on the HW team inputs. We can should not wait for the old ack,
Waiting for old ack might fail, when other forcewake came before the
present one is desserted.
for example, if forcewake bit 0 was set and before it could get cleared
forcewake bit 1 got set, HW eventually clear bit 0, when the bit 1
is cleared. i.e, bit 1 is still sent then forcewake bit 0 will still be
set.
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_uncore.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 918b761..bcb0806 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -196,11 +196,6 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
{
/* Check for Render Engine */
if (FORCEWAKE_RENDER & fw_engine) {
- if (wait_for_atomic((__raw_i915_read32(dev_priv,
- FORCEWAKE_ACK_VLV) &
- FORCEWAKE_KERNEL) == 0,
- FORCEWAKE_ACK_TIMEOUT_MS))
- DRM_ERROR("Timed out: Render forcewake old ack to
clear.\n");
Can we have a comment here? Something like this perhaps:
/*
* WaRsDontPollForAckOnClearingFWBits:vlv
* Hardware clears ack bits lazily (only when all ack
* bits become 0) so don't poll for individiual ack
* bits to be clear here.
*/
Apart from that:
Reviewed-by: Ville Syrjälä
Shouldn't this be Cc: sta...@vger.kernel.org?
Perhaps. But on the other hand LRI to display registers is busted on
VLV so the ddx doesn't do scanline waits which means it doesn't
poke at the forcewake bits either. So only the KERNEL bit gets poked
and so it should be quite impossible to encounter this issue with
current userspace. Which probably explains why no one has ever
reported it, or at least I don't remember seeing such a report
myself.
-Daniel
Your right, with current userspace we should not see the issue. But if user
space starts using other forcewake bits,
we might endup hanging the system due to wait for OLD Ack. Advice from HW team
was to avoid this check.
Thanks
Deepak
__raw_i915_write32(dev_priv, FORCEWAKE_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
@@ -214,11 +209,6 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
/* Check for Media Engine */
if (FORCEWAKE_MEDIA & fw_engine) {
- if (wait_for_atomic((__raw_i915_read32(dev_priv,
- FORCEWAKE_ACK_MEDIA_VLV) &
- FORCEWAKE_KERNEL) == 0,
- FORCEWAKE_ACK_TIMEOUT_MS))
- DRM_ERROR("Timed out: Media forcewake old ack to
clear.\n");
__raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake
On Thursday 18 September 2014 02:09 PM, Ville Syrjälä wrote:
On Thu, Sep 18, 2014 at 06:51:50PM +0530, deepa...@linux.intel.com wrote:
From: Deepak S
Based on the HW team inputs. We can should not wait for the old ack,
Waiting for old ack might fail, when other forcewake came before the
present one is desserted.
for example, if forcewake bit 0 was set and before it could get cleared
forcewake bit 1 got set, HW eventually clear bit 0, when the bit 1
is cleared. i.e, bit 1 is still sent then forcewake bit 0 will still be
set.
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_uncore.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 918b761..bcb0806 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -196,11 +196,6 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
{
/* Check for Render Engine */
if (FORCEWAKE_RENDER & fw_engine) {
- if (wait_for_atomic((__raw_i915_read32(dev_priv,
- FORCEWAKE_ACK_VLV) &
- FORCEWAKE_KERNEL) == 0,
- FORCEWAKE_ACK_TIMEOUT_MS))
- DRM_ERROR("Timed out: Render forcewake old ack to
clear.\n");
Can we have a comment here? Something like this perhaps:
/*
* WaRsDontPollForAckOnClearingFWBits:vlv
* Hardware clears ack bits lazily (only when all ack
* bits become 0) so don't poll for individiual ack
* bits to be clear here.
*/
Apart from that:
Reviewed-by: Ville Syrjälä
Thanks Ville, I will change the commit message.
__raw_i915_write32(dev_priv, FORCEWAKE_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
@@ -214,11 +209,6 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
/* Check for Media Engine */
if (FORCEWAKE_MEDIA & fw_engine) {
- if (wait_for_atomic((__raw_i915_read32(dev_priv,
- FORCEWAKE_ACK_MEDIA_VLV) &
- FORCEWAKE_KERNEL) == 0,
- FORCEWAKE_ACK_TIMEOUT_MS))
- DRM_ERROR("Timed out: Media forcewake old ack to
clear.\n");
__raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_VLV,
_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK
On Saturday 08 November 2014 01:03 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
When reading a CCK register we should obviously read it from CCK not
Punit. This problem has been present ever since this of code was
introduced in
commit 67c3bf6f55a97a0915a0f9ea07278a3073cc9601
Author: Deepak S
Date: Thu Jul 10 13:16:24 2014 +0530
drm/i915: populate mem_freq/cz_clock for chv
The problem was raised during review by Mika [1] but somehow slipped
through the cracks, and the patch got applied with the problem unfixed.
[1] http://lists.freedesktop.org/archives/intel-gfx/2014-July/048937.html
Cc: Deepak S
Cc: Mika Kuoppala
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9285dee..befad36 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5253,7 +5253,10 @@ static void cherryview_init_gt_powersave(struct
drm_device *dev)
mutex_lock(&dev_priv->rps.hw_lock);
- val = vlv_punit_read(dev_priv, CCK_FUSE_REG);
+ mutex_lock(&dev_priv->dpio_lock);
+ val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
+ mutex_unlock(&dev_priv->dpio_lock);
+
switch ((val >> 2) & 0x7) {
case 0:
case 1:
Oops i missed the comment.
Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 1/4] drm/i915/bdw: Clean up execlist queue items in retire_work
_priv);
@@ -565,7 +548,8 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
WARN(tail_req->elsp_submitted != 0,
"More than 2 already-submitted reqs queued\n");
list_del(&tail_req->execlist_link);
- queue_work(dev_priv->wq, &tail_req->work);
+ list_add_tail(&tail_req->execlist_link,
+ &ring->execlist_retired_req_list);
}
}
@@ -733,6 +717,30 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
return 0;
}
+void intel_execlists_retire_requests(struct intel_engine_cs *ring)
+{
+ struct intel_ctx_submit_request *req, *tmp;
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+ unsigned long flags;
+ struct list_head retired_list;
+
+ WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+ if (list_empty(&ring->execlist_retired_req_list))
+ return;
+
+ INIT_LIST_HEAD(&retired_list);
+ spin_lock_irqsave(&ring->execlist_lock, flags);
+ list_replace_init(&ring->execlist_retired_req_list, &retired_list);
+ spin_unlock_irqrestore(&ring->execlist_lock, flags);
+
+ list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) {
+ intel_runtime_pm_put(dev_priv);
+ i915_gem_context_unreference(req->ctx);
+ list_del(&req->execlist_link);
+ kfree(req);
Hi Thomas,
I am fine with the current changes after v5.
Reviewed-by: Deepak S
Thanks
Deepak
+ }
+}
+
void intel_logical_ring_stop(struct intel_engine_cs *ring)
{
struct drm_i915_private *dev_priv = ring->dev->dev_private;
@@ -1248,6 +1256,7 @@ static int logical_ring_init(struct drm_device *dev,
struct intel_engine_cs *rin
init_waitqueue_head(&ring->irq_queue);
INIT_LIST_HEAD(&ring->execlist_queue);
+ INIT_LIST_HEAD(&ring->execlist_retired_req_list);
spin_lock_init(&ring->execlist_lock);
ring->next_context_status_buffer = 0;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 33c3b4b..84bbf19 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -104,11 +104,11 @@ struct intel_ctx_submit_request {
u32 tail;
struct list_head execlist_link;
- struct work_struct work;
int elsp_submitted;
};
void intel_execlists_handle_ctx_events(struct intel_engine_cs *ring);
+void intel_execlists_retire_requests(struct intel_engine_cs *ring);
#endif /* _INTEL_LRC_H_ */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 96479c8..8c002d2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -235,6 +235,7 @@ struct intel_engine_cs {
/* Execlists */
spinlock_t execlist_lock;
struct list_head execlist_queue;
+ struct list_head execlist_retired_req_list;
u8 next_context_status_buffer;
u32 irq_keep_mask; /* bitmask for interrupts that should
not be masked */
int (*emit_request)(struct intel_ringbuffer *ringbuf);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 4/4] drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand
;);
+ "More than 2 already-submitted reqs queued\n");
list_del(&tail_req->execlist_link);
list_add_tail(&tail_req->execlist_link,
&ring->execlist_retired_req_list);
@@ -732,6 +746,12 @@ void intel_execlists_retire_requests(struct
intel_engine_cs *ring)
spin_unlock_irqrestore(&ring->execlist_lock, flags);
list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) {
+ struct intel_context *ctx = req->ctx;
+ struct drm_i915_gem_object *ctx_obj =
+ ctx->engine[ring->id].state;
+
+ if (ctx_obj && (ctx != ring->default_context))
+ intel_lr_context_unpin(ring, ctx);
intel_runtime_pm_put(dev_priv);
i915_gem_context_unreference(req->ctx);
list_del(&req->execlist_link);
@@ -803,6 +823,7 @@ static int intel_lr_context_pin(struct intel_engine_cs
*ring,
struct intel_context *ctx)
{
struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
+ struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
int ret = 0;
WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
@@ -810,21 +831,35 @@ static int intel_lr_context_pin(struct intel_engine_cs
*ring,
ret = i915_gem_obj_ggtt_pin(ctx_obj,
GEN8_LR_CONTEXT_ALIGN, 0);
if (ret)
- ctx->engine[ring->id].unpin_count = 0;
+ goto reset_unpin_count;
+
+ ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
+ if (ret)
+ goto unpin_ctx_obj;
}
return ret;
+
+unpin_ctx_obj:
+ i915_gem_object_ggtt_unpin(ctx_obj);
+reset_unpin_count:
+ ctx->engine[ring->id].unpin_count = 0;
+
+ return ret;
}
void intel_lr_context_unpin(struct intel_engine_cs *ring,
struct intel_context *ctx)
{
struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
+ struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
if (ctx_obj) {
WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
With pin specific mutex from previous patch set removed
Reviewed-by: Deepak S
- if (--ctx->engine[ring->id].unpin_count == 0)
+ if (--ctx->engine[ring->id].unpin_count == 0) {
+ intel_unpin_ringbuffer_obj(ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
+ }
}
}
@@ -1541,7 +1576,6 @@ populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object *ctx_o
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_i915_gem_object *ring_obj = ringbuf->obj;
struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
struct page *page;
uint32_t *reg_state;
@@ -1587,7 +1621,9 @@ populate_lr_context(struct intel_context *ctx, struct
drm_i915_gem_object *ctx_o
reg_state[CTX_RING_TAIL] = RING_TAIL(ring->mmio_base);
reg_state[CTX_RING_TAIL+1] = 0;
reg_state[CTX_RING_BUFFER_START] = RING_START(ring->mmio_base);
- reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(ring_obj);
+ /* Ring buffer start address is not known until the buffer is pinned.
+* It is written to the context image in execlists_update_context()
+*/
reg_state[CTX_RING_BUFFER_CONTROL] = RING_CTL(ring->mmio_base);
reg_state[CTX_RING_BUFFER_CONTROL+1] =
((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES) |
RING_VALID;
@@ -1669,10 +1705,12 @@ void intel_lr_context_free(struct intel_context *ctx)
ctx->engine[i].ringbuf;
struct intel_engine_cs *ring = ringbuf->ring;
+ if (ctx == ring->default_context) {
+ intel_unpin_ringbuffer_obj(ringbuf);
+ i915_gem_object_ggtt_unpin(ctx_obj);
+ }
intel_destroy_ringbuffer_obj(ringbuf);
kfree(ringbuf);
- if (ctx == ring->default_context)
- i915_gem_object_ggtt_unpin(ctx_obj);
drm_gem_object_unreference(&ctx_obj->base);
}
}
@@ -1770,11 +1808,8 @@ int intel_lr_context_deferred_create(struct
intel_context *ctx,
if (!ringbuf) {
DRM_DEBUG_DRIVER("Failed to allocate ringbuffer %s\n",
ring->name);
- if (is_global_default
Re: [Intel-gfx] [PATCH v5 4/4] drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand
On Tuesday 18 November 2014 12:07 PM, Deepak S wrote:
On Thursday 13 November 2014 03:58 PM, Thomas Daniel wrote:
Same as with the context, pinning to GGTT regardless is harmful (it
badly fragments the GGTT and can even exhaust it).
Unfortunately, this case is also more complex than the previous one
because we need to map and access the ringbuffer in several places
along the execbuffer path (and we cannot make do by leaving the
default ringbuffer pinned, as before). Also, the context object
itself contains a pointer to the ringbuffer address that we have to
keep updated if we are going to allow the ringbuffer to move around.
v2: Same as with the context pinning, we cannot really do it during
an interrupt. Also, pin the default ringbuffers objects regardless
(makes error capture a lot easier).
v3: Rebased. Take a pin reference of the ringbuffer for each item
in the execlist request queue because the hardware may still be using
the ringbuffer after the MI_USER_INTERRUPT to notify the seqno update
is executed. The ringbuffer must remain pinned until the context save
is complete. No longer pin and unpin ringbuffer in
populate_lr_context() - this transient address is meaningless and the
pinning can cause a sleep while atomic.
v4: Moved ringbuffer pin and unpin into the lr_context_pin functions.
Downgraded pinning check BUG_ONs to WARN_ONs.
v5: Reinstated WARN_ONs for unexpected execlist states. Removed unused
variable.
Issue: VIZ-4277
Signed-off-by: Oscar Mateo
Signed-off-by: Thomas Daniel
---
drivers/gpu/drm/i915/intel_lrc.c| 102
+++
drivers/gpu/drm/i915/intel_ringbuffer.c | 85
+++---
drivers/gpu/drm/i915/intel_ringbuffer.h |3 +
3 files changed, 128 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c
b/drivers/gpu/drm/i915/intel_lrc.c
index f7fa0f7..ca20f91 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -202,6 +202,9 @@ enum {
};
#define GEN8_CTX_ID_SHIFT 32
+static int intel_lr_context_pin(struct intel_engine_cs *ring,
+struct intel_context *ctx);
+
/**
* intel_sanitize_enable_execlists() - sanitize i915.enable_execlists
* @dev: DRM device.
@@ -339,7 +342,9 @@ static void execlists_elsp_write(struct
intel_engine_cs *ring,
spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
}
-static int execlists_ctx_write_tail(struct drm_i915_gem_object
*ctx_obj, u32 tail)
+static int execlists_update_context(struct drm_i915_gem_object
*ctx_obj,
+struct drm_i915_gem_object *ring_obj,
+u32 tail)
{
struct page *page;
uint32_t *reg_state;
@@ -348,6 +353,7 @@ static int execlists_ctx_write_tail(struct
drm_i915_gem_object *ctx_obj, u32 tai
reg_state = kmap_atomic(page);
reg_state[CTX_RING_TAIL+1] = tail;
+reg_state[CTX_RING_BUFFER_START+1] =
i915_gem_obj_ggtt_offset(ring_obj);
kunmap_atomic(reg_state);
@@ -358,21 +364,25 @@ static int execlists_submit_context(struct
intel_engine_cs *ring,
struct intel_context *to0, u32 tail0,
struct intel_context *to1, u32 tail1)
{
-struct drm_i915_gem_object *ctx_obj0;
+struct drm_i915_gem_object *ctx_obj0 = to0->engine[ring->id].state;
+struct intel_ringbuffer *ringbuf0 = to0->engine[ring->id].ringbuf;
struct drm_i915_gem_object *ctx_obj1 = NULL;
+struct intel_ringbuffer *ringbuf1 = NULL;
-ctx_obj0 = to0->engine[ring->id].state;
BUG_ON(!ctx_obj0);
WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
+WARN_ON(!i915_gem_obj_is_pinned(ringbuf0->obj));
-execlists_ctx_write_tail(ctx_obj0, tail0);
+execlists_update_context(ctx_obj0, ringbuf0->obj, tail0);
if (to1) {
+ringbuf1 = to1->engine[ring->id].ringbuf;
ctx_obj1 = to1->engine[ring->id].state;
BUG_ON(!ctx_obj1);
WARN_ON(!i915_gem_obj_is_pinned(ctx_obj1));
+WARN_ON(!i915_gem_obj_is_pinned(ringbuf1->obj));
-execlists_ctx_write_tail(ctx_obj1, tail1);
+execlists_update_context(ctx_obj1, ringbuf1->obj, tail1);
}
execlists_elsp_write(ring, ctx_obj0, ctx_obj1);
@@ -524,6 +534,10 @@ static int execlists_context_queue(struct
intel_engine_cs *ring,
return -ENOMEM;
req->ctx = to;
i915_gem_context_reference(req->ctx);
+
+if (to != ring->default_context)
+intel_lr_context_pin(ring, to);
+
req->ring = ring;
req->tail = tail;
@@ -544,7 +558,7 @@ static int execlists_context_queue(struct
intel_engine_cs *ring,
if (to == tail_req->ctx) {
WARN(tail_req->elsp_submitted != 0,
- "More than 2 already-submitted reqs queued\n");
+"More than 2 already-submitted reqs queued\n");
list_del(&tail_req-
Re: [Intel-gfx] [PATCH v5 3/4] drm/i915/bdw: Pin the context backing objects to GGTT on-demand
+ submit_req = list_first_entry(&ring->execlist_queue,
+ struct intel_ctx_submit_request,
+ execlist_link);
+ list_del(&submit_req->execlist_link);
+ intel_runtime_pm_put(dev_priv);
+ i915_gem_context_unreference(submit_req->ctx);
+ kfree(submit_req);
+ }
+
+ /*
* We must free the requests after all the corresponding objects have
* been moved off active lists. Which is the same order as the normal
* retire_requests function does. This is important if object hold
@@ -2570,18 +2593,6 @@ static void i915_gem_reset_ring_cleanup(struct
drm_i915_private *dev_priv,
i915_gem_free_request(request);
}
- while (!list_empty(&ring->execlist_queue)) {
- struct intel_ctx_submit_request *submit_req;
-
- submit_req = list_first_entry(&ring->execlist_queue,
- struct intel_ctx_submit_request,
- execlist_link);
- list_del(&submit_req->execlist_link);
- intel_runtime_pm_put(dev_priv);
- i915_gem_context_unreference(submit_req->ctx);
- kfree(submit_req);
- }
-
/* These may not have been flush before the reset, do so now */
kfree(ring->preallocated_lazy_request);
ring->preallocated_lazy_request = NULL;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 906b985..f7fa0f7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -139,8 +139,6 @@
#define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
#define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
-#define GEN8_LR_CONTEXT_ALIGN 4096
-
#define RING_EXECLIST_QFULL (1 << 0x2)
#define RING_EXECLIST1_VALID (1 << 0x3)
#define RING_EXECLIST0_VALID (1 << 0x4)
@@ -801,9 +799,40 @@ void intel_logical_ring_advance_and_submit(struct
intel_ringbuffer *ringbuf)
execlists_context_queue(ring, ctx, ringbuf->tail);
}
+static int intel_lr_context_pin(struct intel_engine_cs *ring,
+ struct intel_context *ctx)
+{
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
+ int ret = 0;
+
+ WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
With pin specific mutex from previous patch set removed.
Reviewed-by: Deepak S
+ if (ctx->engine[ring->id].unpin_count++ == 0) {
+ ret = i915_gem_obj_ggtt_pin(ctx_obj,
+ GEN8_LR_CONTEXT_ALIGN, 0);
+ if (ret)
+ ctx->engine[ring->id].unpin_count = 0;
+ }
+
+ return ret;
+}
+
+void intel_lr_context_unpin(struct intel_engine_cs *ring,
+ struct intel_context *ctx)
+{
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
+
+ if (ctx_obj) {
+ WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+ if (--ctx->engine[ring->id].unpin_count == 0)
+ i915_gem_object_ggtt_unpin(ctx_obj);
+ }
+}
+
static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
struct intel_context *ctx)
{
+ int ret;
+
if (ring->outstanding_lazy_seqno)
return 0;
@@ -814,6 +843,14 @@ static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
if (request == NULL)
return -ENOMEM;
+ if (ctx != ring->default_context) {
+ ret = intel_lr_context_pin(ring, ctx);
+ if (ret) {
+ kfree(request);
+ return ret;
+ }
+ }
+
/* Hold a reference to the context this request belongs to
* (we will need it when the time comes to emit/retire the
* request).
@@ -1626,12 +1663,16 @@ void intel_lr_context_free(struct intel_context *ctx)
for (i = 0; i < I915_NUM_RINGS; i++) {
struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
- struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
if (ctx_obj) {
+ struct intel_ringbuffer *ringbuf =
+ ctx->engine[i].ringbuf;
+ struct intel_engine_cs *ring = ringbuf->ring;
+
intel_destroy_ringbuffer_obj(ringbuf);
kfree(ringbuf);
- i915_gem_object_ggtt_unpin(ctx_obj);
+ if (ctx == ring->default_context)
+ i915_gem_object_ggtt_unpin(ctx_obj);
dr
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Silence valleyview_set_rps()
On Saturday 08 November 2014 01:03 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Even with the rps debug messages signficantly recuced by
commit 67956867aa07c59d6d83628bbc9ee4bd9799a1e1
Author: Ville Syrjälä
Date: Tue Sep 2 15:12:17 2014 +0300
drm/i915: Don't spam dmesg with rps messages on vlv/chv
we still get an inordinate amount of spam from this. Just kill the debug
print. If someone wants to observe it they can just use the tracepoint.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 300d7e5..9285dee 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4504,14 +4504,8 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
"Odd GPU freq value\n"))
val &= ~1;
- if (val != dev_priv->rps.cur_freq) {
- DRM_DEBUG_DRIVER("GPU freq request from %d MHz (%u) to %d MHz
(%u)\n",
-vlv_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
-dev_priv->rps.cur_freq,
-vlv_gpu_freq(dev_priv, val), val);
-
+ if (val != dev_priv->rps.cur_freq)
vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
- }
I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
:)
- Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Drop WaRsForcewakeWaitTC0:vlv
On Friday 14 November 2014 01:42 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
GEN6_GT_THREAD_STATUS_REG doesn't seem to exist on VLV. Reads just give
0x0 no matter what the state of the render and media wells.
There was also some hint in the Gunit HAS that thread status not being
needed on VLV, and hence dropped when bringing stuff over from the IVB
design. Not really a definite comment about the specific register itself
though.
Also the w/a itself is no longer listed for VLV in the database. It was
there some time ago in the past, but I guess someone figured out the
mistake and dropped it.
So let's just drop it from the code as well.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_uncore.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index f318c03..3aa5c65 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -223,10 +223,6 @@ static void __vlv_force_wake_get(struct drm_i915_private
*dev_priv,
FORCEWAKE_ACK_TIMEOUT_MS))
DRM_ERROR("Timed out: waiting for media to ack.\n");
}
-
- /* WaRsForcewakeWaitTC0:vlv */
- if (!IS_CHERRYVIEW(dev_priv->dev))
- __gen6_gt_wait_for_thread_c0(dev_priv);
}
static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
I did not find WA in DB.
S0 :) - Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Drop the HSW special case from __gen6_gt_wait_for_thread_c0()
On Friday 14 November 2014 01:42 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Bits [18:16] of GEN6_GT_THREAD_STATUS_REG have always had the same
meaning since SNB. So treating them as something special for HSW doesn't
make sense to me.
Also the bits *seem* to work exactly the same way on IVB, HSW GT2 and
HSW GT3. At least intel_reg_read gives the identical results on all
platforms with and without forcewake.
Also the HSW PM guide rev 0.99 (ww05 2013) doesn't say anything about
those bits. It just says to poll for bits [2:0]. As does the more recent
BDW PM guide.
So just drop the HSW special case and treat all platforms the same way.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 1 -
drivers/gpu/drm/i915/intel_uncore.c | 10 ++
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 97d3479..3de58ac 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2313,7 +2313,6 @@ enum punit_power_well {
#define GEN6_GT_THREAD_STATUS_REG 0x13805c
#define GEN6_GT_THREAD_STATUS_CORE_MASK 0x7
-#define GEN6_GT_THREAD_STATUS_CORE_MASK_HSW (0x7 | (0x07 << 16))
#define GEN6_GT_PERF_STATUS (MCHBAR_MIRROR_BASE_SNB + 0x5948)
#define GEN6_RP_STATE_LIMITS (MCHBAR_MIRROR_BASE_SNB + 0x5994)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 6a0c3fb..f318c03 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -49,17 +49,11 @@ assert_device_not_suspended(struct drm_i915_private
*dev_priv)
static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
{
- u32 gt_thread_status_mask;
-
- if (IS_HASWELL(dev_priv->dev))
- gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK_HSW;
- else
- gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK;
-
/* w/a for a sporadic read returning 0 by waiting for the GT
* thread to wake up.
*/
- if (wait_for_atomic_us((__raw_i915_read32(dev_priv,
GEN6_GT_THREAD_STATUS_REG) & gt_thread_status_mask) == 0, 500))
+ if (wait_for_atomic_us((__raw_i915_read32(dev_priv,
GEN6_GT_THREAD_STATUS_REG) &
+ GEN6_GT_THREAD_STATUS_CORE_MASK) == 0, 500))
DRM_ERROR("GT thread status wait timed out\n");
}
Yes, Just polling for Bits [2..0] should be good.
Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Refactor vlv/chv GPU frequency divider setup
On Tuesday 11 November 2014 02:25 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
The divider used in the GPU frequency calculations is compatible between
vlv and chv. vlv just wants doubled values compared to chv.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 104 ++--
1 file changed, 35 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ef8e055..03fbb45 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7283,99 +7283,65 @@ int sandybridge_pcode_write(struct drm_i915_private
*dev_priv, u8 mbox, u32 val)
return 0;
}
-static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
+static int vlv_gpu_freq_div(unsigned int czclk_freq)
{
- int div;
-
- /* 4 x czclk */
- switch (dev_priv->mem_freq) {
- case 800:
- div = 10;
- break;
- case 1066:
- div = 12;
- break;
- case 1333:
- div = 16;
- break;
+ switch (czclk_freq) {
+ case 200:
+ return 10;
+ case 267:
+ return 12;
+ case 320:
+ case 333:
+ case 400:
+ return 16;
default:
return -1;
}
+}
- return DIV_ROUND_CLOSEST(dev_priv->mem_freq * (val + 6 - 0xbd), 4 * div);
+static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
+{
+ int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->mem_freq, 4);
+
+ div = vlv_gpu_freq_div(czclk_freq);
+ if (div < 0)
+ return div;
+
+ return DIV_ROUND_CLOSEST(czclk_freq * (val + 6 - 0xbd), div);
}
static int byt_freq_opcode(struct drm_i915_private *dev_priv, int val)
{
- int mul;
+ int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->mem_freq, 4);
- /* 4 x czclk */
- switch (dev_priv->mem_freq) {
- case 800:
- mul = 10;
- break;
- case 1066:
- mul = 12;
- break;
- case 1333:
- mul = 16;
- break;
- default:
- return -1;
- }
+ mul = vlv_gpu_freq_div(czclk_freq);
+ if (mul < 0)
+ return mul;
- return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
+ return DIV_ROUND_CLOSEST(mul * val, czclk_freq) + 0xbd - 6;
}
static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
{
- int div, freq;
-
- switch (dev_priv->rps.cz_freq) {
- case 200:
- div = 5;
- break;
- case 267:
- div = 6;
- break;
- case 320:
- case 333:
- case 400:
- div = 8;
- break;
- default:
- return -1;
- }
+ int div, czclk_freq = dev_priv->rps.cz_freq;
- freq = (DIV_ROUND_CLOSEST((dev_priv->rps.cz_freq * val), 2 * div) / 2);
+ div = vlv_gpu_freq_div(czclk_freq) / 2;
+ if (div < 0)
+ return div;
- return freq;
+ return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div) / 2;
}
static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
{
- int mul, opcode;
+ int mul, czclk_freq = dev_priv->rps.cz_freq;
- switch (dev_priv->rps.cz_freq) {
- case 200:
- mul = 5;
- break;
- case 267:
- mul = 6;
- break;
- case 320:
- case 333:
- case 400:
- mul = 8;
- break;
- default:
- return -1;
- }
+ mul = vlv_gpu_freq_div(czclk_freq) / 2;
+ if (mul < 0)
+ return mul;
/* CHV needs even values */
- opcode = (DIV_ROUND_CLOSEST((val * 2 * mul), dev_priv->rps.cz_freq) *
2);
-
- return opcode;
+ return DIV_ROUND_CLOSEST(val * 2 * mul, czclk_freq) * 2;
}
int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val)
Nice Looks fine.
Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix chv GPU freq<->opcode conversions
On Tuesday 11 November 2014 02:25 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä Currently we miscalculate the GPU frequency on chv. This causes us to report the GPU frequency as half of what it really is. Drop the extra factor of 2 from the calculations to get the correct answer. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 03fbb45..74e4293 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -7329,7 +7329,7 @@ static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val) if (div < 0) return div; - return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div) / 2; + return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div); I think CHV is 2 X cck, shouldn't we report the cck freq and not 2 *cck? } static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val) @@ -7341,7 +7341,7 @@ static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val) return mul; /* CHV needs even values */ - return DIV_ROUND_CLOSEST(val * 2 * mul, czclk_freq) * 2; + return DIV_ROUND_CLOSEST(val * mul, czclk_freq) * 2; } int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Add missing newline to 'DDR speed' debug messages
On Tuesday 11 November 2014 02:25 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 74e4293..0f5c391 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5217,7 +5217,7 @@ static void valleyview_init_gt_powersave(struct
drm_device *dev)
dev_priv->mem_freq = 1333;
break;
}
- DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
+ DRM_DEBUG_DRIVER("DDR speed: %d MHz\n", dev_priv->mem_freq);
dev_priv->rps.max_freq = valleyview_rps_max_freq(dev_priv);
dev_priv->rps.rp0_freq = dev_priv->rps.max_freq;
@@ -5286,7 +5286,7 @@ static void cherryview_init_gt_powersave(struct
drm_device *dev)
dev_priv->mem_freq = 1600;
break;
}
- DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
+ DRM_DEBUG_DRIVER("DDR speed: %d MHz\n", dev_priv->mem_freq);
dev_priv->rps.max_freq = cherryview_rps_max_freq(dev_priv);
dev_priv->rps.rp0_freq = dev_priv->rps.max_freq;
:)
Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Change CHV SKU400 GPU freq divider to 10
On Tuesday 11 November 2014 02:25 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä According to "Cherryview_GFXclocks_y14w36d1.xlsx" the GPU frequency divider should be 10 in when the CZ clock is 400 MHz. Change the code to agree so that we report the correct frequencies. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0f5c391..b73506f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -7292,8 +7292,9 @@ static int vlv_gpu_freq_div(unsigned int czclk_freq) return 12; case 320: case 333: - case 400: return 16; + case 400: + return 20; default: return -1; } right latest spec as div is 20 Reviewed-by: Deepak S ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Improve PCBR debug information
On Saturday 08 November 2014 01:03 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Always print the final PCBR register value on both vlv and chv, and
also tell us whether the BIOS was a good citizen or not.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e8a6f92..ef8e055 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5126,12 +5126,15 @@ static void cherryview_setup_pctx(struct drm_device
*dev)
pcbr = I915_READ(VLV_PCBR);
if ((pcbr >> VLV_PCBR_ADDR_SHIFT) == 0) {
+ DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
paddr = (dev_priv->mm.stolen_base +
(gtt->stolen_size - pctx_size));
pctx_paddr = (paddr & (~4095));
I915_WRITE(VLV_PCBR, pctx_paddr);
}
+
+ DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
}
static void valleyview_setup_pctx(struct drm_device *dev)
@@ -5157,6 +5160,8 @@ static void valleyview_setup_pctx(struct drm_device *dev)
goto out;
}
+ DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
+
/*
* From the Gunit register HAS:
* The Gfx driver is expected to program this register and ensure
@@ -5175,6 +5180,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
I915_WRITE(VLV_PCBR, pctx_paddr);
out:
+ DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
dev_priv->vlv_pctx = pctx;
}
@@ -5366,8 +5372,6 @@ static void cherryview_enable_rps(struct drm_device *dev)
/* For now we assume BIOS is allocating and populating the PCBR */
pcbr = I915_READ(VLV_PCBR);
- DRM_DEBUG_DRIVER("PCBR offset : 0x%x\n", pcbr);
-
/* 3: Enable RC6 */
if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
(pcbr >> VLV_PCBR_ADDR_SHIFT))
Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915: Warn if GPLL isn't used on vlv/chv
On Saturday 08 November 2014 01:03 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Our freq<->opcode conversions assume that GPLL is always used.
Apparently that should be the case always, but let's scream if we
ever encounter something different.
Signed-off-by: Ville Syrjälä
---
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 71eb377..e8a6f92 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5397,6 +5397,9 @@ static void cherryview_enable_rps(struct drm_device *dev)
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
+ /* RPS code assumes GPLL is used */
+ WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
+
DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
@@ -5477,6 +5480,9 @@ static void valleyview_enable_rps(struct drm_device *dev)
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
+ /* RPS code assumes GPLL is used */
+ WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
+
DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
Yup better to give warning if our assumption is wrong:)
Reviewed-by: Deepak S
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915: Add a name for the Punit GPLLENABLE bit
On Saturday 08 November 2014 01:03 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Remove the magic number for the GPLLENABLE bit by adding a name for it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d43fa0e..ec4dc00 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -564,6 +564,7 @@ enum punit_power_well {
#define PUNIT_REG_GPU_LFM 0xd3
#define PUNIT_REG_GPU_FREQ_REQ0xd4
#define PUNIT_REG_GPU_FREQ_STS0xd8
+#define GPLLENABLE (1<<4)
#define GENFREQSTATUS (1<<0)
#define PUNIT_REG_MEDIA_TURBO_FREQ_REQ0xdc
#define PUNIT_REG_CZ_TIMESTAMP0xce
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index befad36..71eb377 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5397,7 +5397,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
- DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");
+ DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
dev_priv->rps.cur_freq = (val >> 8) & 0xff;
@@ -5477,7 +5477,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
- DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");
+ DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
:)
Reviewed-by: Deepak S
dev_priv->rps.cur_freq = (val >> 8) & 0xff;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix chv GPU freq<->opcode conversions
On Monday 17 November 2014 05:05 PM, Ville Syrjälä wrote: On Tue, Nov 18, 2014 at 02:38:25PM +0530, Deepak S wrote: On Tuesday 11 November 2014 02:25 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä Currently we miscalculate the GPU frequency on chv. This causes us to report the GPU frequency as half of what it really is. Drop the extra factor of 2 from the calculations to get the correct answer. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 03fbb45..74e4293 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -7329,7 +7329,7 @@ static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val) if (div < 0) return div; - return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div) / 2; + return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div); I think CHV is 2 X cck, shouldn't we report the cck freq and not 2 *cck? Hmm. Once again the docs are extremely unclear. Dropping the 2x factor gives me the same numbers that the tables in the docs have. But then the spreadsheet calls it a "2x clock" in some places, which does suggest it might get further divided down by 2. Oh, now I did find a somewhat clear note in the clock HAS: "The dedicated GPLL (Graphics PLL) sends a 2GHz gfx clock to GenLC, which gets divided inside the GenLC block to derive a 1GHz Gfx fast clock." So based on that the original code does make more sense. Do we need to mention in comment about 2 * GFX clock? } static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val) @@ -7341,7 +7341,7 @@ static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val) return mul; /* CHV needs even values */ - return DIV_ROUND_CLOSEST(val * 2 * mul, czclk_freq) * 2; + return DIV_ROUND_CLOSEST(val * mul, czclk_freq) * 2; } int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 3/4] drm/i915/bdw: Pin the context backing objects to GGTT on-demand
On Monday 17 November 2014 07:53 PM, Daniel Vetter wrote:
On Tue, Nov 18, 2014 at 12:10:51PM +0530, Deepak S wrote:
On Thursday 13 November 2014 03:58 PM, Thomas Daniel wrote:
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 906b985..f7fa0f7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -139,8 +139,6 @@
#define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
#define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
-#define GEN8_LR_CONTEXT_ALIGN 4096
-
#define RING_EXECLIST_QFULL (1 << 0x2)
#define RING_EXECLIST1_VALID (1 << 0x3)
#define RING_EXECLIST0_VALID (1 << 0x4)
@@ -801,9 +799,40 @@ void intel_logical_ring_advance_and_submit(struct
intel_ringbuffer *ringbuf)
execlists_context_queue(ring, ctx, ringbuf->tail);
}
+static int intel_lr_context_pin(struct intel_engine_cs *ring,
+ struct intel_context *ctx)
+{
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
+ int ret = 0;
+
+ WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
With pin specific mutex from previous patch set removed.
Pardon my ignorance but I'm completely lost on this review comment here.
Deepak, can you please elaborate what kind of lock on which exact version
of the previous patch you mean? I didn't find any locking at all in the
preceeding patch here ...
Thanks, Daniel
Hi Daniel,
+static int intel_lr_context_pin(struct intel_engine_cs *ring,
+ struct intel_context *ctx)
+{
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
+ int ret = 0;
+
+ mutex_lock(&ctx->engine[ring->id].unpin_lock);
+ if (ctx->engine[ring->id].unpin_count++ == 0) {
+ ret = i915_gem_obj_ggtt_pin(ctx_obj,
+ GEN8_LR_CONTEXT_ALIGN, 0);
+ if (ret)
+ ctx->engine[ring->id].unpin_count = 0;
+ }
+ mutex_unlock(&ctx->engine[ring->id].unpin_lock);
+
+ return ret;
+}
In Previous patch set we had a "mutex_lock(&ctx->engine[ring->id].unpin_lock);"
Since we "intel_lr_context_pin" is already under struct mutex, So we dont need
unpin_lock. This was the change in latest patch set :)
Thanks
Deepak
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 4/4] drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand
On Monday 17 November 2014 07:59 PM, Daniel Vetter wrote: On Tue, Nov 18, 2014 at 12:09:54PM +0530, Deepak S wrote: On Tuesday 18 November 2014 12:07 PM, Deepak S wrote: With pin specific mutex from previous patch set removed Oops This comment was for previous patch in the series :( Since i reviewed the patch offline, comments got mixed :) Please forward these comments from the private discussion to the mailing list. Review isn't just about code correctness, but about communication - yes, I (and domain experts) actually read all this stuff that floats around and will jump into the discussion if there's something important or tricky being discussed. Second reason for public review is that the important part about the r-b tag isn't that review happened, but by whom. So this is all about reputation building and playing to people's various strenght. And if you do review in private nothing of that can happen, which makes the review a lot less useful. So let's extract the most value from all that engineering time we invest into reviewing and _always_ do the review in public. Thanks, Daniel Thanks Daniel. I will make sure to add the comments to mail list :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Fix chv GPU freq<->opcode conversions
On Monday 17 November 2014 06:11 PM, Ville Syrjälä wrote: On Tue, Nov 18, 2014 at 05:59:07PM +0530, Deepak S wrote: On Monday 17 November 2014 05:05 PM, Ville Syrjälä wrote: On Tue, Nov 18, 2014 at 02:38:25PM +0530, Deepak S wrote: On Tuesday 11 November 2014 02:25 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä Currently we miscalculate the GPU frequency on chv. This causes us to report the GPU frequency as half of what it really is. Drop the extra factor of 2 from the calculations to get the correct answer. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 03fbb45..74e4293 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -7329,7 +7329,7 @@ static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val) if (div < 0) return div; - return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div) / 2; + return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div); I think CHV is 2 X cck, shouldn't we report the cck freq and not 2 *cck? Hmm. Once again the docs are extremely unclear. Dropping the 2x factor gives me the same numbers that the tables in the docs have. But then the spreadsheet calls it a "2x clock" in some places, which does suggest it might get further divided down by 2. Oh, now I did find a somewhat clear note in the clock HAS: "The dedicated GPLL (Graphics PLL) sends a 2GHz gfx clock to GenLC, which gets divided inside the GenLC block to derive a 1GHz Gfx fast clock." So based on that the original code does make more sense. Do we need to mention in comment about 2 * GFX clock? Yeah, a comment would probably be a good idea. Could avoid some confusion in the future if someone else looks at this code. Will you add the comment and submit a patch? } static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val) @@ -7341,7 +7341,7 @@ static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val) return mul; /* CHV needs even values */ - return DIV_ROUND_CLOSEST(val * 2 * mul, czclk_freq) * 2; + return DIV_ROUND_CLOSEST(val * mul, czclk_freq) * 2; } int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Bug fixes to ring 'head' updating
On Monday 03 November 2014 06:59 PM, Dave Gordon wrote:
Fixes to both the LRC and the legacy ringbuffer code to correctly
calculate and update the available space in a ring.
The logical ring code was updating the software ring 'head' value
by reading the hardware 'HEAD' register. In LRC mode, this is not
valid as the hardware is not necessarily executing the same context
that is being processed by the software. Thus reading the h/w HEAD
could put an unrelated (undefined, effectively random) value into
the s/w 'head' -- A Bad Thing for the free space calculations.
In addition, the old code could update a ringbuffer's 'head' value
from the 'last_retired_head' even when the latter hadn't been recently
updated and therefore had a value of -1; this would also confuse the
freespace calculations. Now, we consume 'last_retired_head' in just
one place, ensuring that this confusion does not arise.
Change-Id: Id7ce9096ed100a2882c68a54206f30b6c87e92fa
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/i915_dma.c |5 ++-
drivers/gpu/drm/i915/intel_lrc.c| 36 ++---
drivers/gpu/drm/i915/intel_ringbuffer.c | 53 ---
drivers/gpu/drm/i915/intel_ringbuffer.h |1 +
4 files changed, 48 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 9a73533..1646416 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -154,11 +154,10 @@ void i915_kernel_lost_context(struct drm_device *dev)
if (drm_core_check_feature(dev, DRIVER_MODESET))
return;
+ ringbuf->last_retired_head = -1;
ringbuf->head = I915_READ_HEAD(ring) & HEAD_ADDR;
ringbuf->tail = I915_READ_TAIL(ring) & TAIL_ADDR;
- ringbuf->space = ringbuf->head - (ringbuf->tail + I915_RING_FREE_SPACE);
- if (ringbuf->space < 0)
- ringbuf->space += ringbuf->size;
+ intel_ring_update_space(ringbuf);
if (!dev->primary->master)
return;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index cd74e5c..11a9047 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -827,16 +827,20 @@ static int logical_ring_wait_request(struct
intel_ringbuffer *ringbuf,
u32 seqno = 0;
int ret;
- if (ringbuf->last_retired_head != -1) {
- ringbuf->head = ringbuf->last_retired_head;
- ringbuf->last_retired_head = -1;
-
- ringbuf->space = intel_ring_space(ringbuf);
- if (ringbuf->space >= bytes)
- return 0;
- }
+ if (intel_ring_space(ringbuf) >= bytes)
+ return 0;
list_for_each_entry(request, &ring->request_list, list) {
+ /*
+* The request queue is per-engine, so can contain requests
+* from multiple ringbuffers. Here, we must ignore any that
+* aren't from the ringbuffer we're considering.
+*/
+ struct intel_context *ctx = request->ctx;
+ if (ctx->engine[ring->id].ringbuf != ringbuf)
+ continue;
+
+ /* Would completion of this request free enough space? */
if (__intel_ring_space(request->tail, ringbuf->tail,
ringbuf->size) >= bytes) {
seqno = request->seqno;
@@ -852,11 +856,8 @@ static int logical_ring_wait_request(struct
intel_ringbuffer *ringbuf,
return ret;
i915_gem_retire_requests_ring(ring);
- ringbuf->head = ringbuf->last_retired_head;
- ringbuf->last_retired_head = -1;
- ringbuf->space = intel_ring_space(ringbuf);
- return 0;
+ return intel_ring_space(ringbuf) >= bytes ? 0 : -ENOSPC;
}
static int logical_ring_wait_for_space(struct intel_ringbuffer *ringbuf,
@@ -882,13 +883,10 @@ static int logical_ring_wait_for_space(struct
intel_ringbuffer *ringbuf,
* case by choosing an insanely large timeout. */
end = jiffies + 60 * HZ;
+ ret = 0;
do {
- ringbuf->head = I915_READ_HEAD(ring);
- ringbuf->space = intel_ring_space(ringbuf);
- if (ringbuf->space >= bytes) {
- ret = 0;
+ if (intel_ring_space(ringbuf) >= bytes)
break;
- }
msleep(1);
@@ -929,7 +927,7 @@ static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf)
iowrite32(MI_NOOP, virt++);
ringbuf->tail = 0;
- ringbuf->space = intel_ring_space(ringbuf);
+ intel_ring_update_space(ringbuf);
return 0;
}
@@ -1708,8 +1706,8 @@ int intel_lr_context_deferred_create(struct intel_context
*ctx,
ringbuf->effective_size = ringbuf->size;
ringbuf->head = 0;
ringbuf->tail = 0;
- ringbuf->space = r
