Re: [Intel-gfx] [PATCH 1/4] drm/i915: update current freq properly before requesting new freq.
On Sun, Dec 08, 2013 at 02:16:43PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
on VLV, P-Unit doesn't garauntee that last requested freq by driver
is actually the current running frequency. We need to make sure we update
the cur freq. before requesitng new freq.
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 8
drivers/gpu/drm/i915/intel_pm.c | 31 +++
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 780f815..a62ac0c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2416,6 +2416,7 @@ extern bool ironlake_set_drps(struct drm_device *dev,
u8 val);
extern void intel_init_pch_refclk(struct drm_device *dev);
extern void gen6_set_rps(struct drm_device *dev, u8 val);
extern void valleyview_set_rps(struct drm_device *dev, u8 val);
+extern bool vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv);
extern int valleyview_rps_max_freq(struct drm_i915_private *dev_priv);
extern int valleyview_rps_min_freq(struct drm_i915_private *dev_priv);
extern void intel_detect_pch(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2715600..4bde03a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -982,6 +982,14 @@ static void gen6_pm_rps_work(struct work_struct *work)
mutex_lock(dev_priv-rps.hw_lock);
+ /* Make sure we have current freq updated properly. Doing this
+ * here becuase, on VLV, P-Unit doesnt garauntee that last requested
+ * freq by driver is actually the current running frequency
+ */
+
+ if (IS_VALLEYVIEW(dev_priv-dev))
+ vlv_update_rps_cur_delay(dev_priv);
+
adj = dev_priv-rps.last_adj;
if (pm_iir GEN6_PM_RP_UP_THRESHOLD) {
if (adj 0)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e6d98fe..7f6c747 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3607,6 +3607,35 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv)
mutex_unlock(dev_priv-rps.hw_lock);
}
+/*
+ * Wait until the previous freq change has completed,
+ * or the timeout elapsed, and then update our notion
+ * of the current GPU frequency.
+ */
+bool vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv)
+{
+ u32 pval;
+
+ WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
+
+ if (wait_for(((pval = vlv_punit_read(dev_priv,
+ PUNIT_REG_GPU_FREQ_STS))
+ GENFREQSTATUS) == 0, 10))
+ DRM_DEBUG_DRIVER(timed out waiting for Punit\n);
+
+ pval = 8;
+
+ if (pval != dev_priv-rps.cur_delay)
+ DRM_DEBUG_DRIVER(Punit overrode GPU freq: %d MHz (%u)
requested, but got %d Mhz (%u)\n,
+ vlv_gpu_freq(dev_priv, dev_priv-rps.cur_delay),
+ dev_priv-rps.cur_delay,
+ vlv_gpu_freq(dev_priv, pval), pval);
+
+ dev_priv-rps.cur_delay = pval;
+ return true;
+}
I just killed this guys a while ago. If you think we need to resurrect
it, you should do it w/ git revert to make it clear where it came from.
But I'd want more justification than what you have provided. My
understanding is that PUNIT_REG_GPU_FREQ_STS alwasy reflects the
current operating frequency of the GPU, and that can be affected by
thermal conditions (and media turbo, which I'll ignore for simplicity)
in addition to the frequency requested by the driver. AFAIK the punit
will recheck the situation periodically, and it will try to use
PUNIT_REG_GPU_FREQ_REQ. It will check the thermal conditions to
figure out if it needs to further limit the frequency. Once the
thermal conditions permit it, the frequency should return back to the
last requested turbo frequency, without the driver having to rewrite
PUNIT_REG_GPU_FREQ_REQ.
If I'm right updating cur_delay based on PUNIT_REG_GPU_FREQ_STS is
clearly the wrong thing to do. So I think we need more details on
what the punit does in order to figure out what's the right thing
to do here.
+
+
void valleyview_set_rps(struct drm_device *dev, u8 val)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -3615,6 +3644,8 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
WARN_ON(val dev_priv-rps.max_delay);
WARN_ON(val dev_priv-rps.min_delay);
+ vlv_update_rps_cur_delay(dev_priv);
+
DRM_DEBUG_DRIVER(GPU freq request from %d MHz (%u) to %d MHz (%u)\n,
vlv_gpu_freq(dev_priv, dev_priv-rps.cur_delay),
dev_priv-rps.cur_delay,
--
1.8.4.2
___
Intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: update current freq properly before requesting new freq.
I am trying to get more details. I will update the thread once I have some
clarification. Thanks for reviewing.
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Monday, December 9, 2013 3:34 PM
To: S, Deepak
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/4] drm/i915: update current freq properly
before requesting new freq.
On Sun, Dec 08, 2013 at 02:16:43PM +0530, deepa...@intel.com wrote:
From: Deepak S deepa...@intel.com
on VLV, P-Unit doesn't garauntee that last requested freq by driver is
actually the current running frequency. We need to make sure we update
the cur freq. before requesitng new freq.
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 8
drivers/gpu/drm/i915/intel_pm.c | 31 +++
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h index 780f815..a62ac0c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2416,6 +2416,7 @@ extern bool ironlake_set_drps(struct drm_device
*dev, u8 val); extern void intel_init_pch_refclk(struct drm_device
*dev); extern void gen6_set_rps(struct drm_device *dev, u8 val);
extern void valleyview_set_rps(struct drm_device *dev, u8 val);
+extern bool vlv_update_rps_cur_delay(struct drm_i915_private
+*dev_priv);
extern int valleyview_rps_max_freq(struct drm_i915_private
*dev_priv); extern int valleyview_rps_min_freq(struct
drm_i915_private *dev_priv); extern void intel_detect_pch(struct
drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c index 2715600..4bde03a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -982,6 +982,14 @@ static void gen6_pm_rps_work(struct work_struct
*work)
mutex_lock(dev_priv-rps.hw_lock);
+ /* Make sure we have current freq updated properly. Doing this
+ * here becuase, on VLV, P-Unit doesnt garauntee that last requested
+ * freq by driver is actually the current running frequency
+ */
+
+ if (IS_VALLEYVIEW(dev_priv-dev))
+ vlv_update_rps_cur_delay(dev_priv);
+
adj = dev_priv-rps.last_adj;
if (pm_iir GEN6_PM_RP_UP_THRESHOLD) {
if (adj 0)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c index e6d98fe..7f6c747 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3607,6 +3607,35 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv)
mutex_unlock(dev_priv-rps.hw_lock);
}
+/*
+ * Wait until the previous freq change has completed,
+ * or the timeout elapsed, and then update our notion
+ * of the current GPU frequency.
+ */
+bool vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv) {
+ u32 pval;
+
+ WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
+
+ if (wait_for(((pval = vlv_punit_read(dev_priv,
+ PUNIT_REG_GPU_FREQ_STS))
+ GENFREQSTATUS) == 0, 10))
+ DRM_DEBUG_DRIVER(timed out waiting for Punit\n);
+
+ pval = 8;
+
+ if (pval != dev_priv-rps.cur_delay)
+ DRM_DEBUG_DRIVER(Punit overrode GPU freq: %d MHz (%u)
requested, but got %d Mhz (%u)\n,
+ vlv_gpu_freq(dev_priv, dev_priv-rps.cur_delay),
+ dev_priv-rps.cur_delay,
+ vlv_gpu_freq(dev_priv, pval), pval);
+
+ dev_priv-rps.cur_delay = pval;
+ return true;
+}
I just killed this guys a while ago. If you think we need to resurrect it, you
should do it w/ git revert to make it clear where it came from.
But I'd want more justification than what you have provided. My understanding
is that PUNIT_REG_GPU_FREQ_STS alwasy reflects the current operating frequency
of the GPU, and that can be affected by thermal conditions (and media turbo,
which I'll ignore for simplicity) in addition to the frequency requested by the
driver. AFAIK the punit will recheck the situation periodically, and it will
try to use PUNIT_REG_GPU_FREQ_REQ. It will check the thermal conditions to
figure out if it needs to further limit the frequency. Once the thermal
conditions permit it, the frequency should return back to the last requested
turbo frequency, without the driver having to rewrite PUNIT_REG_GPU_FREQ_REQ.
If I'm right updating cur_delay based on PUNIT_REG_GPU_FREQ_STS is clearly the
wrong thing to do. So I think we need more details on what the punit does in
order to figure out what's the right thing to do here.
+
+
void valleyview_set_rps(struct drm_device *dev, u8 val) {
struct drm_i915_private *dev_priv = dev-dev_private; @@ -3615,6
+3644,8 @@ void valleyview_set_rps(struct drm_device
[Intel-gfx] [PATCH 1/4] drm/i915: update current freq properly before requesting new freq.
From: Deepak S deepa...@intel.com
on VLV, P-Unit doesn't garauntee that last requested freq by driver
is actually the current running frequency. We need to make sure we update
the cur freq. before requesitng new freq.
Signed-off-by: Deepak S deepa...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 8
drivers/gpu/drm/i915/intel_pm.c | 31 +++
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 780f815..a62ac0c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2416,6 +2416,7 @@ extern bool ironlake_set_drps(struct drm_device *dev, u8
val);
extern void intel_init_pch_refclk(struct drm_device *dev);
extern void gen6_set_rps(struct drm_device *dev, u8 val);
extern void valleyview_set_rps(struct drm_device *dev, u8 val);
+extern bool vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv);
extern int valleyview_rps_max_freq(struct drm_i915_private *dev_priv);
extern int valleyview_rps_min_freq(struct drm_i915_private *dev_priv);
extern void intel_detect_pch(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2715600..4bde03a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -982,6 +982,14 @@ static void gen6_pm_rps_work(struct work_struct *work)
mutex_lock(dev_priv-rps.hw_lock);
+ /* Make sure we have current freq updated properly. Doing this
+* here becuase, on VLV, P-Unit doesnt garauntee that last requested
+* freq by driver is actually the current running frequency
+*/
+
+ if (IS_VALLEYVIEW(dev_priv-dev))
+ vlv_update_rps_cur_delay(dev_priv);
+
adj = dev_priv-rps.last_adj;
if (pm_iir GEN6_PM_RP_UP_THRESHOLD) {
if (adj 0)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e6d98fe..7f6c747 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3607,6 +3607,35 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv)
mutex_unlock(dev_priv-rps.hw_lock);
}
+/*
+ * Wait until the previous freq change has completed,
+ * or the timeout elapsed, and then update our notion
+ * of the current GPU frequency.
+ */
+bool vlv_update_rps_cur_delay(struct drm_i915_private *dev_priv)
+{
+ u32 pval;
+
+ WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock));
+
+ if (wait_for(((pval = vlv_punit_read(dev_priv,
+ PUNIT_REG_GPU_FREQ_STS))
+ GENFREQSTATUS) == 0, 10))
+ DRM_DEBUG_DRIVER(timed out waiting for Punit\n);
+
+ pval = 8;
+
+ if (pval != dev_priv-rps.cur_delay)
+ DRM_DEBUG_DRIVER(Punit overrode GPU freq: %d MHz (%u)
requested, but got %d Mhz (%u)\n,
+ vlv_gpu_freq(dev_priv, dev_priv-rps.cur_delay),
+ dev_priv-rps.cur_delay,
+ vlv_gpu_freq(dev_priv, pval), pval);
+
+ dev_priv-rps.cur_delay = pval;
+ return true;
+}
+
+
void valleyview_set_rps(struct drm_device *dev, u8 val)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -3615,6 +3644,8 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
WARN_ON(val dev_priv-rps.max_delay);
WARN_ON(val dev_priv-rps.min_delay);
+ vlv_update_rps_cur_delay(dev_priv);
+
DRM_DEBUG_DRIVER(GPU freq request from %d MHz (%u) to %d MHz (%u)\n,
vlv_gpu_freq(dev_priv, dev_priv-rps.cur_delay),
dev_priv-rps.cur_delay,
--
1.8.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
