[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