* [PATCH] drm/i915: Restore rps/rc6 on reset @ 2013-10-31 19:52 jeff.mcgee 2013-10-31 19:52 ` jeff.mcgee 2013-11-01 10:47 ` [PATCH] " Jani Nikula 0 siblings, 2 replies; 9+ messages in thread From: jeff.mcgee @ 2013-10-31 19:52 UTC (permalink / raw) To: intel-gfx From: Jeff McGee <jeff.mcgee@intel.com> Hello all, Posting my first patch to the list. I have tested this fix on Android kernel 3.9.1 and only compile-checked on drm-intel-nightly. I have reviewed the changes to reset and rps/rc6 stuff and believe there should be no hidden gotchas. As I hope this to be the first of many patches to upstream, I will probably need a bleeding-edge kernel dev environment to complement my Android testing. Any tips on what to use would be appreciated. I was instructed to keep the JIRA issue codes on the patch, but have to wonder why I don't see any of these in the history. Let me know if those shouldn't be there. Thanks, Jeff Jeff McGee (1): drm/i915: Restore rps/rc6 on reset drivers/gpu/drm/i915/i915_debugfs.c | 49 +++++++++++++++++++++++++++++++------ drivers/gpu/drm/i915/i915_drv.c | 11 +++++++++ drivers/gpu/drm/i915/intel_pm.c | 31 ++++++++++++++++------- 3 files changed, 74 insertions(+), 17 deletions(-) -- 1.8.4.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] drm/i915: Restore rps/rc6 on reset 2013-10-31 19:52 [PATCH] drm/i915: Restore rps/rc6 on reset jeff.mcgee @ 2013-10-31 19:52 ` jeff.mcgee 2013-11-04 8:44 ` Daniel Vetter 2013-11-01 10:47 ` [PATCH] " Jani Nikula 1 sibling, 1 reply; 9+ messages in thread From: jeff.mcgee @ 2013-10-31 19:52 UTC (permalink / raw) To: intel-gfx From: Jeff McGee <jeff.mcgee@intel.com> A check of rps/rc6 state after i915_reset determined that the ring MAX_IDLE registers were returned to their hardware defaults and that the GEN6_PMIMR register was set to mask all interrupts. This change restores those values to their pre-reset states by re-initializing rps/rc6 in i915_reset. A full re-initialization was opted for versus a targeted set of restore operations for simplicity and maintain- ability. Note that the re-initialization is not done for Ironlake, due to a past comment that it causes problems. Also updated the rps initialization sequence to preserve existing min/max values in the case of a re-init. We assume the values were validated upon being set and do not do further range checking. The debugfs interface for changing min/max was updated with range checking to ensure this condition (already present in sysfs interface). Issue: VIZ-3142 Issue: AXIA-4676 OTC-Tracker: VIZ-3143 Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 49 +++++++++++++++++++++++++++++++------ drivers/gpu/drm/i915/i915_drv.c | 11 +++++++++ drivers/gpu/drm/i915/intel_pm.c | 31 ++++++++++++++++------- 3 files changed, 74 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 5c45e9e..68710f3 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2565,6 +2565,7 @@ i915_max_freq_set(void *data, u64 val) { struct drm_device *dev = data; struct drm_i915_private *dev_priv = dev->dev_private; + u32 rp_state_cap, hw_max, hw_min; int ret; if (!(IS_GEN6(dev) || IS_GEN7(dev))) @@ -2583,14 +2584,29 @@ i915_max_freq_set(void *data, u64 val) */ if (IS_VALLEYVIEW(dev)) { val = vlv_freq_opcode(dev_priv->mem_freq, val); - dev_priv->rps.max_delay = val; - gen6_set_rps(dev, val); + + hw_max = valleyview_rps_max_freq(dev_priv); + hw_min = valleyview_rps_min_freq(dev_priv); } else { do_div(val, GT_FREQUENCY_MULTIPLIER); - dev_priv->rps.max_delay = val; - gen6_set_rps(dev, val); + + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); + hw_max = dev_priv->rps.hw_max; + hw_min = (rp_state_cap >> 16) & 0xff; + } + + if (val < hw_min || val > hw_max || val < dev_priv->rps.min_delay) { + mutex_unlock(&dev_priv->rps.hw_lock); + return -EINVAL; } + dev_priv->rps.max_delay = val; + + if (IS_VALLEYVIEW(dev)) + valleyview_set_rps(dev, val); + else + gen6_set_rps(dev, val); + mutex_unlock(&dev_priv->rps.hw_lock); return 0; @@ -2631,6 +2647,7 @@ i915_min_freq_set(void *data, u64 val) { struct drm_device *dev = data; struct drm_i915_private *dev_priv = dev->dev_private; + u32 rp_state_cap, hw_max, hw_min; int ret; if (!(IS_GEN6(dev) || IS_GEN7(dev))) @@ -2649,13 +2666,29 @@ i915_min_freq_set(void *data, u64 val) */ if (IS_VALLEYVIEW(dev)) { val = vlv_freq_opcode(dev_priv->mem_freq, val); - dev_priv->rps.min_delay = val; - valleyview_set_rps(dev, val); + + hw_max = valleyview_rps_max_freq(dev_priv); + hw_min = valleyview_rps_min_freq(dev_priv); } else { do_div(val, GT_FREQUENCY_MULTIPLIER); - dev_priv->rps.min_delay = val; - gen6_set_rps(dev, val); + + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); + hw_max = dev_priv->rps.hw_max; + hw_min = (rp_state_cap >> 16) & 0xff; + } + + if (val < hw_min || val > hw_max || val > dev_priv->rps.max_delay) { + mutex_unlock(&dev_priv->rps.hw_lock); + return -EINVAL; } + + dev_priv->rps.min_delay = val; + + if (IS_VALLEYVIEW(dev)) + valleyview_set_rps(dev, val); + else + gen6_set_rps(dev, val); + mutex_unlock(&dev_priv->rps.hw_lock); return 0; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a0804fa..7137ae7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -769,6 +769,17 @@ int i915_reset(struct drm_device *dev) drm_irq_uninstall(dev); drm_irq_install(dev); + + /* rps/rc6 re-init is necessary to restore state lost after the + * reset and the re-install of drm irq. Skip for ironlake per + * previous concerns that it doesn't respond well to some forms + * of re-init after reset. */ + if (INTEL_INFO(dev)->gen > 5) { + mutex_lock(&dev->struct_mutex); + intel_enable_gt_powersave(dev); + mutex_unlock(&dev->struct_mutex); + } + intel_hpd_init(dev); } else { mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a0c907f..b079ab8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3751,7 +3751,7 @@ static void gen6_enable_rps(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring; - u32 rp_state_cap; + u32 rp_state_cap, hw_max, hw_min; u32 gt_perf_status; u32 rc6vids, pcu_mbox, rc6_mask = 0; u32 gtfifodbg; @@ -3780,13 +3780,20 @@ static void gen6_enable_rps(struct drm_device *dev) gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS); /* In units of 50MHz */ - dev_priv->rps.hw_max = dev_priv->rps.max_delay = rp_state_cap & 0xff; - dev_priv->rps.min_delay = (rp_state_cap >> 16) & 0xff; + dev_priv->rps.hw_max = hw_max = rp_state_cap & 0xff; + hw_min = (rp_state_cap >> 16) & 0xff; dev_priv->rps.rp1_delay = (rp_state_cap >> 8) & 0xff; dev_priv->rps.rp0_delay = (rp_state_cap >> 0) & 0xff; dev_priv->rps.rpe_delay = dev_priv->rps.rp1_delay; dev_priv->rps.cur_delay = 0; + /* Preserve min/max settings in case of re-init */ + if (dev_priv->rps.max_delay == 0) + dev_priv->rps.max_delay = hw_max; + + if (dev_priv->rps.min_delay == 0) + dev_priv->rps.min_delay = hw_min; + /* disable the counters and set deterministic thresholds */ I915_WRITE(GEN6_RC_CONTROL, 0); @@ -4012,7 +4019,7 @@ static void valleyview_enable_rps(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring; - u32 gtfifodbg, val, rc6_mode = 0; + u32 gtfifodbg, val, hw_max, hw_min, rc6_mode = 0; int i; WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); @@ -4087,12 +4094,11 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.cur_delay), dev_priv->rps.cur_delay); - dev_priv->rps.max_delay = valleyview_rps_max_freq(dev_priv); - dev_priv->rps.hw_max = dev_priv->rps.max_delay; + dev_priv->rps.hw_max = hw_max = valleyview_rps_max_freq(dev_priv); DRM_DEBUG_DRIVER("max GPU freq: %d MHz (%u)\n", vlv_gpu_freq(dev_priv->mem_freq, dev_priv->rps.max_delay), - dev_priv->rps.max_delay); + hw_max); dev_priv->rps.rpe_delay = valleyview_rps_rpe_freq(dev_priv); DRM_DEBUG_DRIVER("RPe GPU freq: %d MHz (%u)\n", @@ -4100,11 +4106,18 @@ static void valleyview_enable_rps(struct drm_device *dev) dev_priv->rps.rpe_delay), dev_priv->rps.rpe_delay); - dev_priv->rps.min_delay = valleyview_rps_min_freq(dev_priv); + hw_min = valleyview_rps_min_freq(dev_priv); DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n", vlv_gpu_freq(dev_priv->mem_freq, dev_priv->rps.min_delay), - dev_priv->rps.min_delay); + hw_min); + + /* Preserve min/max settings in case of re-init */ + if (dev_priv->rps.max_delay == 0) + dev_priv->rps.max_delay = hw_max; + + if (dev_priv->rps.min_delay == 0) + dev_priv->rps.min_delay = hw_min; DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n", vlv_gpu_freq(dev_priv->mem_freq, -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Restore rps/rc6 on reset 2013-10-31 19:52 ` jeff.mcgee @ 2013-11-04 8:44 ` Daniel Vetter 2013-11-05 20:38 ` Mcgee, Jeff 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2013-11-04 8:44 UTC (permalink / raw) To: jeff.mcgee; +Cc: intel-gfx On Thu, Oct 31, 2013 at 8:52 PM, <jeff.mcgee@intel.com> wrote: > From: Jeff McGee <jeff.mcgee@intel.com> > > A check of rps/rc6 state after i915_reset determined that the ring > MAX_IDLE registers were returned to their hardware defaults and that > the GEN6_PMIMR register was set to mask all interrupts. This change > restores those values to their pre-reset states by re-initializing > rps/rc6 in i915_reset. A full re-initialization was opted for versus > a targeted set of restore operations for simplicity and maintain- > ability. Note that the re-initialization is not done for Ironlake, > due to a past comment that it causes problems. > > Also updated the rps initialization sequence to preserve existing > min/max values in the case of a re-init. We assume the values were > validated upon being set and do not do further range checking. The > debugfs interface for changing min/max was updated with range > checking to ensure this condition (already present in sysfs > interface). > > Issue: VIZ-3142 > Issue: AXIA-4676 > OTC-Tracker: VIZ-3143 > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> Can I have a testcase in i-g-t for this please? I think the following should work: 1. Throw a dummy load onto the gpu, check that cagf goes up. 2. Limit min/max to a non-default value (and install an igt atexit handler to undo this). 3. Throw a dummy load onto the gpu, check that cagf jumps from the idle freq to the selected one directly. 4. Inject a gpu hang with the stop_rings stuff (see e.g. kms_flip.c or ZZ_hangman). 5. Reject that the limts still work as in step 3. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Restore rps/rc6 on reset 2013-11-04 8:44 ` Daniel Vetter @ 2013-11-05 20:38 ` Mcgee, Jeff 2013-11-06 19:37 ` Mcgee, Jeff 0 siblings, 1 reply; 9+ messages in thread From: Mcgee, Jeff @ 2013-11-05 20:38 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Mon, 2013-11-04 at 09:44 +0100, Daniel Vetter wrote: > On Thu, Oct 31, 2013 at 8:52 PM, <jeff.mcgee@intel.com> wrote: > > From: Jeff McGee <jeff.mcgee@intel.com> > > > > A check of rps/rc6 state after i915_reset determined that the ring > > MAX_IDLE registers were returned to their hardware defaults and that > > the GEN6_PMIMR register was set to mask all interrupts. This change > > restores those values to their pre-reset states by re-initializing > > rps/rc6 in i915_reset. A full re-initialization was opted for versus > > a targeted set of restore operations for simplicity and maintain- > > ability. Note that the re-initialization is not done for Ironlake, > > due to a past comment that it causes problems. > > > > Also updated the rps initialization sequence to preserve existing > > min/max values in the case of a re-init. We assume the values were > > validated upon being set and do not do further range checking. The > > debugfs interface for changing min/max was updated with range > > checking to ensure this condition (already present in sysfs > > interface). > > > > Issue: VIZ-3142 > > Issue: AXIA-4676 > > OTC-Tracker: VIZ-3143 > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> > > Can I have a testcase in i-g-t for this please? I think the following > should work: > > 1. Throw a dummy load onto the gpu, check that cagf goes up. > > 2. Limit min/max to a non-default value (and install an igt atexit > handler to undo this). > > 3. Throw a dummy load onto the gpu, check that cagf jumps from the > idle freq to the selected one directly. > > 4. Inject a gpu hang with the stop_rings stuff (see e.g. kms_flip.c or > ZZ_hangman). > > 5. Reject that the limts still work as in step 3. > > Cheers, Daniel I'll see what can be done. I understand the emphasis on adding formalized tests. There will have to be some resourcing discussions on the product side if this is now a requirement for upstream patch acceptance. Jeff ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Restore rps/rc6 on reset 2013-11-05 20:38 ` Mcgee, Jeff @ 2013-11-06 19:37 ` Mcgee, Jeff 2014-02-04 17:32 ` [PATCH v2] " jeff.mcgee 0 siblings, 1 reply; 9+ messages in thread From: Mcgee, Jeff @ 2013-11-06 19:37 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Tue, 2013-11-05 at 20:38 +0000, Mcgee, Jeff wrote: > On Mon, 2013-11-04 at 09:44 +0100, Daniel Vetter wrote: > > On Thu, Oct 31, 2013 at 8:52 PM, <jeff.mcgee@intel.com> wrote: > > > From: Jeff McGee <jeff.mcgee@intel.com> > > > > > > A check of rps/rc6 state after i915_reset determined that the ring > > > MAX_IDLE registers were returned to their hardware defaults and that > > > the GEN6_PMIMR register was set to mask all interrupts. This change > > > restores those values to their pre-reset states by re-initializing > > > rps/rc6 in i915_reset. A full re-initialization was opted for versus > > > a targeted set of restore operations for simplicity and maintain- > > > ability. Note that the re-initialization is not done for Ironlake, > > > due to a past comment that it causes problems. > > > > > > Also updated the rps initialization sequence to preserve existing > > > min/max values in the case of a re-init. We assume the values were > > > validated upon being set and do not do further range checking. The > > > debugfs interface for changing min/max was updated with range > > > checking to ensure this condition (already present in sysfs > > > interface). > > > > > > Issue: VIZ-3142 > > > Issue: AXIA-4676 > > > OTC-Tracker: VIZ-3143 > > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> > > > > Can I have a testcase in i-g-t for this please? I think the following > > should work: > > > > 1. Throw a dummy load onto the gpu, check that cagf goes up. > > > > 2. Limit min/max to a non-default value (and install an igt atexit > > handler to undo this). > > > > 3. Throw a dummy load onto the gpu, check that cagf jumps from the > > idle freq to the selected one directly. > > > > 4. Inject a gpu hang with the stop_rings stuff (see e.g. kms_flip.c or > > ZZ_hangman). > > > > 5. Reject that the limts still work as in step 3. > > > > Cheers, Daniel > > I'll see what can be done. I understand the emphasis on adding > formalized tests. There will have to be some resourcing discussions on > the product side if this is now a requirement for upstream patch > acceptance. > > Jeff This discussion is starting in VPG Android, but it may be a while until test code gets submitted. This is a fairly serious bug and easy to reproduce manually. So please consider the patch without the test case for now. Thanks Jeff ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] drm/i915: Restore rps/rc6 on reset 2013-11-06 19:37 ` Mcgee, Jeff @ 2014-02-04 17:32 ` jeff.mcgee 2014-02-07 9:25 ` Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: jeff.mcgee @ 2014-02-04 17:32 UTC (permalink / raw) To: intel-gfx From: Jeff McGee <jeff.mcgee@intel.com> A check of rps/rc6 state after i915_reset determined that the ring MAX_IDLE registers were returned to their hardware defaults and that the GEN6_PMIMR register was set to mask all interrupts. This change restores those values to their pre-reset states by re-initializing rps/rc6 in i915_reset. A full re-initialization was opted for versus a targeted set of restore operations for simplicity and maintain- ability. Note that the re-initialization is not done for Ironlake, due to a past comment that it causes problems. Also updated the rps initialization sequence to preserve existing min/max values in the case of a re-init. We assume the values were validated upon being set and do not do further range checking. The debugfs interface for changing min/max was updated with range checking to ensure this condition (already present in sysfs interface). v2: fix rps logging to output hw_max and hw_min, not rps.max_delay and rps.min_delay which don't strictly represent hardware limits. Add igt testcase to signed-off-by section. Testcase: igt/pm_rps/reset Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 49 +++++++++++++++++++++++++++++++------ drivers/gpu/drm/i915/i915_drv.c | 11 +++++++++ drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++++++++--------- 3 files changed, 76 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index bc8707f..2dc05c3 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3223,6 +3223,7 @@ i915_max_freq_set(void *data, u64 val) { struct drm_device *dev = data; struct drm_i915_private *dev_priv = dev->dev_private; + u32 rp_state_cap, hw_max, hw_min; int ret; if (!(IS_GEN6(dev) || IS_GEN7(dev))) @@ -3241,14 +3242,29 @@ i915_max_freq_set(void *data, u64 val) */ if (IS_VALLEYVIEW(dev)) { val = vlv_freq_opcode(dev_priv, val); - dev_priv->rps.max_delay = val; - valleyview_set_rps(dev, val); + + hw_max = valleyview_rps_max_freq(dev_priv); + hw_min = valleyview_rps_min_freq(dev_priv); } else { do_div(val, GT_FREQUENCY_MULTIPLIER); - dev_priv->rps.max_delay = val; - gen6_set_rps(dev, val); + + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); + hw_max = dev_priv->rps.hw_max; + hw_min = (rp_state_cap >> 16) & 0xff; + } + + if (val < hw_min || val > hw_max || val < dev_priv->rps.min_delay) { + mutex_unlock(&dev_priv->rps.hw_lock); + return -EINVAL; } + dev_priv->rps.max_delay = val; + + if (IS_VALLEYVIEW(dev)) + valleyview_set_rps(dev, val); + else + gen6_set_rps(dev, val); + mutex_unlock(&dev_priv->rps.hw_lock); return 0; @@ -3288,6 +3304,7 @@ i915_min_freq_set(void *data, u64 val) { struct drm_device *dev = data; struct drm_i915_private *dev_priv = dev->dev_private; + u32 rp_state_cap, hw_max, hw_min; int ret; if (!(IS_GEN6(dev) || IS_GEN7(dev))) @@ -3306,13 +3323,29 @@ i915_min_freq_set(void *data, u64 val) */ if (IS_VALLEYVIEW(dev)) { val = vlv_freq_opcode(dev_priv, val); - dev_priv->rps.min_delay = val; - valleyview_set_rps(dev, val); + + hw_max = valleyview_rps_max_freq(dev_priv); + hw_min = valleyview_rps_min_freq(dev_priv); } else { do_div(val, GT_FREQUENCY_MULTIPLIER); - dev_priv->rps.min_delay = val; - gen6_set_rps(dev, val); + + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP); + hw_max = dev_priv->rps.hw_max; + hw_min = (rp_state_cap >> 16) & 0xff; + } + + if (val < hw_min || val > hw_max || val > dev_priv->rps.max_delay) { + mutex_unlock(&dev_priv->rps.hw_lock); + return -EINVAL; } + + dev_priv->rps.min_delay = val; + + if (IS_VALLEYVIEW(dev)) + valleyview_set_rps(dev, val); + else + gen6_set_rps(dev, val); + mutex_unlock(&dev_priv->rps.hw_lock); return 0; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a071748..2449364 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -691,6 +691,17 @@ int i915_reset(struct drm_device *dev) drm_irq_uninstall(dev); drm_irq_install(dev); + + /* rps/rc6 re-init is necessary to restore state lost after the + * reset and the re-install of drm irq. Skip for ironlake per + * previous concerns that it doesn't respond well to some forms + * of re-init after reset. */ + if (INTEL_INFO(dev)->gen > 5) { + mutex_lock(&dev->struct_mutex); + intel_enable_gt_powersave(dev); + mutex_unlock(&dev->struct_mutex); + } + intel_hpd_init(dev); } else { mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 9ab3883..6af58cd 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3322,7 +3322,7 @@ static void gen6_enable_rps(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring; - u32 rp_state_cap; + u32 rp_state_cap, hw_max, hw_min; u32 gt_perf_status; u32 rc6vids, pcu_mbox, rc6_mask = 0; u32 gtfifodbg; @@ -3351,13 +3351,20 @@ static void gen6_enable_rps(struct drm_device *dev) gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS); /* In units of 50MHz */ - dev_priv->rps.hw_max = dev_priv->rps.max_delay = rp_state_cap & 0xff; - dev_priv->rps.min_delay = (rp_state_cap >> 16) & 0xff; + dev_priv->rps.hw_max = hw_max = rp_state_cap & 0xff; + hw_min = (rp_state_cap >> 16) & 0xff; dev_priv->rps.rp1_delay = (rp_state_cap >> 8) & 0xff; dev_priv->rps.rp0_delay = (rp_state_cap >> 0) & 0xff; dev_priv->rps.rpe_delay = dev_priv->rps.rp1_delay; dev_priv->rps.cur_delay = 0; + /* Preserve min/max settings in case of re-init */ + if (dev_priv->rps.max_delay == 0) + dev_priv->rps.max_delay = hw_max; + + if (dev_priv->rps.min_delay == 0) + dev_priv->rps.min_delay = hw_min; + /* disable the counters and set deterministic thresholds */ I915_WRITE(GEN6_RC_CONTROL, 0); @@ -3586,7 +3593,7 @@ static void valleyview_enable_rps(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring; - u32 gtfifodbg, val, rc6_mode = 0; + u32 gtfifodbg, val, hw_max, hw_min, rc6_mode = 0; int i; WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); @@ -3648,21 +3655,27 @@ static void valleyview_enable_rps(struct drm_device *dev) vlv_gpu_freq(dev_priv, dev_priv->rps.cur_delay), dev_priv->rps.cur_delay); - dev_priv->rps.max_delay = valleyview_rps_max_freq(dev_priv); - dev_priv->rps.hw_max = dev_priv->rps.max_delay; + dev_priv->rps.hw_max = hw_max = valleyview_rps_max_freq(dev_priv); DRM_DEBUG_DRIVER("max GPU freq: %d MHz (%u)\n", - vlv_gpu_freq(dev_priv, dev_priv->rps.max_delay), - dev_priv->rps.max_delay); + vlv_gpu_freq(dev_priv, hw_max), + hw_max); dev_priv->rps.rpe_delay = valleyview_rps_rpe_freq(dev_priv); DRM_DEBUG_DRIVER("RPe GPU freq: %d MHz (%u)\n", vlv_gpu_freq(dev_priv, dev_priv->rps.rpe_delay), dev_priv->rps.rpe_delay); - dev_priv->rps.min_delay = valleyview_rps_min_freq(dev_priv); + hw_min = valleyview_rps_min_freq(dev_priv); DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n", - vlv_gpu_freq(dev_priv, dev_priv->rps.min_delay), - dev_priv->rps.min_delay); + vlv_gpu_freq(dev_priv, hw_min), + hw_min); + + /* Preserve min/max settings in case of re-init */ + if (dev_priv->rps.max_delay == 0) + dev_priv->rps.max_delay = hw_max; + + if (dev_priv->rps.min_delay == 0) + dev_priv->rps.min_delay = hw_min; DRM_DEBUG_DRIVER("setting GPU freq to %d MHz (%u)\n", vlv_gpu_freq(dev_priv, dev_priv->rps.rpe_delay), -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Restore rps/rc6 on reset 2014-02-04 17:32 ` [PATCH v2] " jeff.mcgee @ 2014-02-07 9:25 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2014-02-07 9:25 UTC (permalink / raw) To: jeff.mcgee; +Cc: intel-gfx On Tue, Feb 04, 2014 at 11:32:31AM -0600, jeff.mcgee@intel.com wrote: > From: Jeff McGee <jeff.mcgee@intel.com> > > A check of rps/rc6 state after i915_reset determined that the ring > MAX_IDLE registers were returned to their hardware defaults and that > the GEN6_PMIMR register was set to mask all interrupts. This change > restores those values to their pre-reset states by re-initializing > rps/rc6 in i915_reset. A full re-initialization was opted for versus > a targeted set of restore operations for simplicity and maintain- > ability. Note that the re-initialization is not done for Ironlake, > due to a past comment that it causes problems. > > Also updated the rps initialization sequence to preserve existing > min/max values in the case of a re-init. We assume the values were > validated upon being set and do not do further range checking. The > debugfs interface for changing min/max was updated with range > checking to ensure this condition (already present in sysfs > interface). > > v2: fix rps logging to output hw_max and hw_min, not rps.max_delay > and rps.min_delay which don't strictly represent hardware limits. > Add igt testcase to signed-off-by section. > > Testcase: igt/pm_rps/reset > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Restore rps/rc6 on reset 2013-10-31 19:52 [PATCH] drm/i915: Restore rps/rc6 on reset jeff.mcgee 2013-10-31 19:52 ` jeff.mcgee @ 2013-11-01 10:47 ` Jani Nikula 2013-11-01 11:05 ` Daniel Vetter 1 sibling, 1 reply; 9+ messages in thread From: Jani Nikula @ 2013-11-01 10:47 UTC (permalink / raw) To: jeff.mcgee, intel-gfx On Thu, 31 Oct 2013, jeff.mcgee@intel.com wrote: > From: Jeff McGee <jeff.mcgee@intel.com> > > Hello all, > > Posting my first patch to the list. I have tested this fix on Android > kernel 3.9.1 and only compile-checked on drm-intel-nightly. I have > reviewed the changes to reset and rps/rc6 stuff and believe there > should be no hidden gotchas. > > As I hope this to be the first of many patches to upstream, I will > probably need a bleeding-edge kernel dev environment to complement > my Android testing. Any tips on what to use would be appreciated. It would be awesome if you could run our -nightly kernels on Android, but I can understand if that's not feasible (missing upstream drivers etc.) Otherwise any distro you feel comfortable working with should be fine. For a start, the userspace doesn't really have to be bleeding edge (indeed for the most parts I run vanilla userspace from the distro). You should always be able to switch to a newer kernel without regressing the userspace. > I was instructed to keep the JIRA issue codes on the patch, but have > to wonder why I don't see any of these in the history. Let me know if > those shouldn't be there. Personally I don't think references to internal systems provide much value, though others may disagree. In any case the commit message has to be self sufficient without the referenced info. (And no complaints about that in your patch!) Keep the patches coming! ;) HTH, Jani. > > Thanks, > Jeff > > Jeff McGee (1): > drm/i915: Restore rps/rc6 on reset > > drivers/gpu/drm/i915/i915_debugfs.c | 49 +++++++++++++++++++++++++++++++------ > drivers/gpu/drm/i915/i915_drv.c | 11 +++++++++ > drivers/gpu/drm/i915/intel_pm.c | 31 ++++++++++++++++------- > 3 files changed, 74 insertions(+), 17 deletions(-) > > -- > 1.8.4.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Restore rps/rc6 on reset 2013-11-01 10:47 ` [PATCH] " Jani Nikula @ 2013-11-01 11:05 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2013-11-01 11:05 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Fri, Nov 1, 2013 at 11:47 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote: >> I was instructed to keep the JIRA issue codes on the patch, but have >> to wonder why I don't see any of these in the history. Let me know if >> those shouldn't be there. > > Personally I don't think references to internal systems provide much > value, though others may disagree. In any case the commit message has to > be self sufficient without the referenced info. (And no complaints about > that in your patch!) Yeah, if crucial information is hidden behind internal references that's not great. And I think for that reason such stuff is a bit frowned upon. But if it helps you to keep track of the patches I don't mind. I think the usual approach though is to smash a Changeset-Id: tag onto the patch and use that to track the patch flow. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-02-07 9:25 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-31 19:52 [PATCH] drm/i915: Restore rps/rc6 on reset jeff.mcgee 2013-10-31 19:52 ` jeff.mcgee 2013-11-04 8:44 ` Daniel Vetter 2013-11-05 20:38 ` Mcgee, Jeff 2013-11-06 19:37 ` Mcgee, Jeff 2014-02-04 17:32 ` [PATCH v2] " jeff.mcgee 2014-02-07 9:25 ` Daniel Vetter 2013-11-01 10:47 ` [PATCH] " Jani Nikula 2013-11-01 11:05 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox