* [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 [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
* 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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.