Re: [Intel-gfx] [PATCH 12/12] drm/i915: Disable DDR DVFS on CHV
On Mon, Mar 09, 2015 at 05:00:09PM +0200, Ville Syrjälä wrote: On Mon, Mar 09, 2015 at 10:14:05AM +0530, Arun R Murthy wrote: On Friday 06 March 2015 12:49 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com DDR DVFS introduces massive memory latencies which can't be handled by the PND deadline stuff. Instead the watermarks will need to be programmed to compensate for the latency and the deadlines will need to be programmed to tight fixed values. That means DDR DVFS can only be enabled if the display FIFOs are large enough, and that pretty much means we have to manually repartition them to suit the needs of the moment. That's a lot of change, so in the meantime let's just disable DDR DVFS to get the display(s) to be stable. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- Since this patch disabled DDR DVFS in the driver, can this be done once. Here I assume that DDR DVFS gets disabled on each update watermark call. Yeah it did occur to me that we could get away with disabling just once, but once we get the DDR DVFS enabled we're going to be changing it more, potentially at every watermark update (although having to do it every time is rather unlikely). In any case, you may have noticed that I skip the entire WM programming if the calculated watermarks didn't change from the old values, so we won't be frobbing the Punit unless something in the WM values has actually changed. So I figured that's good enough for now. My plan is to optimize away redundant Punit communication (for both PM5 and DDR DVFS) eventually, but that is part of the task to get DDR DVFS enabled in the first place. Yeah I think this plan makes sense long-term. Generally I'd like to move the wm code towards always recomputing everything and optimizing out no-op updates afterwards. Atm we have have a split between the code that decides when to recompute wm and the code that recomputes them, and that tends to be a bit fragile. Especially since we add a lot more complexity to the wm code with new features like Y-tiling or atomic updates. -Daneil -- 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 12/12] drm/i915: Disable DDR DVFS on CHV
On Mon, Mar 09, 2015 at 10:14:05AM +0530, Arun R Murthy wrote:
On Friday 06 March 2015 12:49 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
DDR DVFS introduces massive memory latencies which can't be handled by
the PND deadline stuff. Instead the watermarks will need to be
programmed to compensate for the latency and the deadlines will need to
be programmed to tight fixed values. That means DDR DVFS can only be
enabled if the display FIFOs are large enough, and that pretty much
means we have to manually repartition them to suit the needs of the
moment.
That's a lot of change, so in the meantime let's just disable DDR DVFS
to get the display(s) to be stable.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
Since this patch disabled DDR DVFS in the driver, can this be done
once. Here I assume that DDR DVFS gets disabled on each update
watermark call.
Yeah it did occur to me that we could get away with disabling just once,
but once we get the DDR DVFS enabled we're going to be changing it more,
potentially at every watermark update (although having to do it every
time is rather unlikely).
In any case, you may have noticed that I skip the entire WM programming
if the calculated watermarks didn't change from the old values, so we
won't be frobbing the Punit unless something in the WM values has actually
changed. So I figured that's good enough for now.
My plan is to optimize away redundant Punit communication (for both PM5
and DDR DVFS) eventually, but that is part of the task to get DDR DVFS
enabled in the first place.
Moreover enabling DDR DVFS should be followed with updating
watermarks and display arbiter.
Thanks and Regards,
Arun R Murthy
---
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_pm.c | 34 ++
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index 5a20f58..744d162 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -630,6 +630,11 @@ enum skl_disp_power_wells {
#define FB_GFX_FMIN_AT_VMIN_FUSE 0x137
#define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT8
+#define PUNIT_REG_DDR_SETUP2 0x139
+#define FORCE_DDR_FREQ_REQ_ACK (1 8)
+#define FORCE_DDR_LOW_FREQ (1 1)
+#define FORCE_DDR_HIGH_FREQ (1 0)
+
#define PUNIT_GPU_STATUS_REG 0xdb
#define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT 16
#define PUNIT_GPU_STATUS_MAX_FREQ_MASK0xff
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 1dd82ec..fc03e24 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -235,6 +235,28 @@ static const struct cxsr_latency
*intel_get_cxsr_latency(int is_desktop,
return NULL;
}
+static void chv_set_memory_dvfs(struct drm_i915_private *dev_priv, bool
enable)
+{
+ u32 val;
+
+ mutex_lock(dev_priv-rps.hw_lock);
+
+ val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
+ if (enable)
+ val = ~FORCE_DDR_HIGH_FREQ;
+ else
+ val |= FORCE_DDR_HIGH_FREQ;
+ val = ~FORCE_DDR_LOW_FREQ;
+ val |= FORCE_DDR_FREQ_REQ_ACK;
+ vlv_punit_write(dev_priv, PUNIT_REG_DDR_SETUP2, val);
+
+ if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2)
+ FORCE_DDR_FREQ_REQ_ACK) == 0, 3))
+ DRM_ERROR(timed out waiting for Punit DDR DVFS request\n);
+
+ mutex_unlock(dev_priv-rps.hw_lock);
+}
+
static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool
enable)
{
u32 val;
@@ -282,6 +304,7 @@ void intel_set_memory_cxsr(struct drm_i915_private
*dev_priv, bool enable)
enable ? enabled : disabled);
}
+
/*
* Latency for FIFO fetches is dependent on several factors:
* - memory configuration (speed, channels)
@@ -992,6 +1015,17 @@ static void valleyview_update_wm(struct drm_crtc
*crtc)
wm.pipe[pipe].primary, wm.pipe[pipe].cursor,
wm.sr.plane, wm.sr.cursor);
+ /*
+* FIXME DDR DVFS introduces massive memory latencies which
+* are not known to system agent so any deadline specified
+* by the display may not be respected. To support DDR DVFS
+* the watermark code needs to be rewritten to essentially
+* bypass deadline mechanism and rely solely on the
+* watermarks. For now disable DDR DVFS.
+*/
+ if (IS_CHERRYVIEW(dev_priv))
+ chv_set_memory_dvfs(dev_priv, false);
+
if (!cxsr_enabled)
intel_set_memory_cxsr(dev_priv, false);
--
Ville Syrjälä
Intel OTC
___
Re: [Intel-gfx] [PATCH 12/12] drm/i915: Disable DDR DVFS on CHV
On Friday 06 March 2015 12:49 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
DDR DVFS introduces massive memory latencies which can't be handled by
the PND deadline stuff. Instead the watermarks will need to be
programmed to compensate for the latency and the deadlines will need to
be programmed to tight fixed values. That means DDR DVFS can only be
enabled if the display FIFOs are large enough, and that pretty much
means we have to manually repartition them to suit the needs of the
moment.
That's a lot of change, so in the meantime let's just disable DDR DVFS
to get the display(s) to be stable.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
Since this patch disabled DDR DVFS in the driver, can this be done
once. Here I assume that DDR DVFS gets disabled on each update
watermark call.
Moreover enabling DDR DVFS should be followed with updating
watermarks and display arbiter.
Thanks and Regards,
Arun R Murthy
---
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_pm.c | 34 ++
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5a20f58..744d162 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -630,6 +630,11 @@ enum skl_disp_power_wells {
#define FB_GFX_FMIN_AT_VMIN_FUSE 0x137
#define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT8
+#define PUNIT_REG_DDR_SETUP2 0x139
+#define FORCE_DDR_FREQ_REQ_ACK (1 8)
+#define FORCE_DDR_LOW_FREQ (1 1)
+#define FORCE_DDR_HIGH_FREQ (1 0)
+
#define PUNIT_GPU_STATUS_REG 0xdb
#define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT 16
#define PUNIT_GPU_STATUS_MAX_FREQ_MASK0xff
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1dd82ec..fc03e24 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -235,6 +235,28 @@ static const struct cxsr_latency
*intel_get_cxsr_latency(int is_desktop,
return NULL;
}
+static void chv_set_memory_dvfs(struct drm_i915_private *dev_priv, bool enable)
+{
+ u32 val;
+
+ mutex_lock(dev_priv-rps.hw_lock);
+
+ val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
+ if (enable)
+ val = ~FORCE_DDR_HIGH_FREQ;
+ else
+ val |= FORCE_DDR_HIGH_FREQ;
+ val = ~FORCE_DDR_LOW_FREQ;
+ val |= FORCE_DDR_FREQ_REQ_ACK;
+ vlv_punit_write(dev_priv, PUNIT_REG_DDR_SETUP2, val);
+
+ if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2)
+ FORCE_DDR_FREQ_REQ_ACK) == 0, 3))
+ DRM_ERROR(timed out waiting for Punit DDR DVFS request\n);
+
+ mutex_unlock(dev_priv-rps.hw_lock);
+}
+
static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable)
{
u32 val;
@@ -282,6 +304,7 @@ void intel_set_memory_cxsr(struct drm_i915_private
*dev_priv, bool enable)
enable ? enabled : disabled);
}
+
/*
* Latency for FIFO fetches is dependent on several factors:
* - memory configuration (speed, channels)
@@ -992,6 +1015,17 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
wm.pipe[pipe].primary, wm.pipe[pipe].cursor,
wm.sr.plane, wm.sr.cursor);
+ /*
+* FIXME DDR DVFS introduces massive memory latencies which
+* are not known to system agent so any deadline specified
+* by the display may not be respected. To support DDR DVFS
+* the watermark code needs to be rewritten to essentially
+* bypass deadline mechanism and rely solely on the
+* watermarks. For now disable DDR DVFS.
+*/
+ if (IS_CHERRYVIEW(dev_priv))
+ chv_set_memory_dvfs(dev_priv, false);
+
if (!cxsr_enabled)
intel_set_memory_cxsr(dev_priv, false);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/12] drm/i915: Disable DDR DVFS on CHV
On 03/05/2015 11:19 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
DDR DVFS introduces massive memory latencies which can't be handled by
the PND deadline stuff. Instead the watermarks will need to be
programmed to compensate for the latency and the deadlines will need to
be programmed to tight fixed values. That means DDR DVFS can only be
enabled if the display FIFOs are large enough, and that pretty much
means we have to manually repartition them to suit the needs of the
moment.
That's a lot of change, so in the meantime let's just disable DDR DVFS
to get the display(s) to be stable.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_pm.c | 34 ++
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5a20f58..744d162 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -630,6 +630,11 @@ enum skl_disp_power_wells {
#define FB_GFX_FMIN_AT_VMIN_FUSE 0x137
#define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT 8
+#define PUNIT_REG_DDR_SETUP2 0x139
+#define FORCE_DDR_FREQ_REQ_ACK (1 8)
+#define FORCE_DDR_LOW_FREQ (1 1)
+#define FORCE_DDR_HIGH_FREQ(1 0)
+
#define PUNIT_GPU_STATUS_REG 0xdb
#define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT 16
#define PUNIT_GPU_STATUS_MAX_FREQ_MASK 0xff
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1dd82ec..fc03e24 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -235,6 +235,28 @@ static const struct cxsr_latency
*intel_get_cxsr_latency(int is_desktop,
return NULL;
}
+static void chv_set_memory_dvfs(struct drm_i915_private *dev_priv, bool
enable)
+{
+ u32 val;
+
+ mutex_lock(dev_priv-rps.hw_lock);
+
+ val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
+ if (enable)
+ val = ~FORCE_DDR_HIGH_FREQ;
+ else
+ val |= FORCE_DDR_HIGH_FREQ;
+ val = ~FORCE_DDR_LOW_FREQ;
+ val |= FORCE_DDR_FREQ_REQ_ACK;
+ vlv_punit_write(dev_priv, PUNIT_REG_DDR_SETUP2, val);
+
+ if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2)
+ FORCE_DDR_FREQ_REQ_ACK) == 0, 3))
+ DRM_ERROR(timed out waiting for Punit DDR DVFS request\n);
+
+ mutex_unlock(dev_priv-rps.hw_lock);
+}
+
static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool
enable)
{
u32 val;
@@ -282,6 +304,7 @@ void intel_set_memory_cxsr(struct drm_i915_private
*dev_priv, bool enable)
enable ? enabled : disabled);
}
+
/*
* Latency for FIFO fetches is dependent on several factors:
* - memory configuration (speed, channels)
@@ -992,6 +1015,17 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
wm.pipe[pipe].primary, wm.pipe[pipe].cursor,
wm.sr.plane, wm.sr.cursor);
+ /*
+ * FIXME DDR DVFS introduces massive memory latencies which
+ * are not known to system agent so any deadline specified
+ * by the display may not be respected. To support DDR DVFS
+ * the watermark code needs to be rewritten to essentially
+ * bypass deadline mechanism and rely solely on the
+ * watermarks. For now disable DDR DVFS.
+ */
+ if (IS_CHERRYVIEW(dev_priv))
+ chv_set_memory_dvfs(dev_priv, false);
+
if (!cxsr_enabled)
intel_set_memory_cxsr(dev_priv, false);
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/12] drm/i915: Disable DDR DVFS on CHV
From: Ville Syrjälä ville.syrj...@linux.intel.com
DDR DVFS introduces massive memory latencies which can't be handled by
the PND deadline stuff. Instead the watermarks will need to be
programmed to compensate for the latency and the deadlines will need to
be programmed to tight fixed values. That means DDR DVFS can only be
enabled if the display FIFOs are large enough, and that pretty much
means we have to manually repartition them to suit the needs of the
moment.
That's a lot of change, so in the meantime let's just disable DDR DVFS
to get the display(s) to be stable.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_pm.c | 34 ++
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5a20f58..744d162 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -630,6 +630,11 @@ enum skl_disp_power_wells {
#define FB_GFX_FMIN_AT_VMIN_FUSE 0x137
#define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT 8
+#define PUNIT_REG_DDR_SETUP2 0x139
+#define FORCE_DDR_FREQ_REQ_ACK (1 8)
+#define FORCE_DDR_LOW_FREQ (1 1)
+#define FORCE_DDR_HIGH_FREQ (1 0)
+
#define PUNIT_GPU_STATUS_REG 0xdb
#define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT16
#define PUNIT_GPU_STATUS_MAX_FREQ_MASK 0xff
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1dd82ec..fc03e24 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -235,6 +235,28 @@ static const struct cxsr_latency
*intel_get_cxsr_latency(int is_desktop,
return NULL;
}
+static void chv_set_memory_dvfs(struct drm_i915_private *dev_priv, bool enable)
+{
+ u32 val;
+
+ mutex_lock(dev_priv-rps.hw_lock);
+
+ val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
+ if (enable)
+ val = ~FORCE_DDR_HIGH_FREQ;
+ else
+ val |= FORCE_DDR_HIGH_FREQ;
+ val = ~FORCE_DDR_LOW_FREQ;
+ val |= FORCE_DDR_FREQ_REQ_ACK;
+ vlv_punit_write(dev_priv, PUNIT_REG_DDR_SETUP2, val);
+
+ if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2)
+ FORCE_DDR_FREQ_REQ_ACK) == 0, 3))
+ DRM_ERROR(timed out waiting for Punit DDR DVFS request\n);
+
+ mutex_unlock(dev_priv-rps.hw_lock);
+}
+
static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable)
{
u32 val;
@@ -282,6 +304,7 @@ void intel_set_memory_cxsr(struct drm_i915_private
*dev_priv, bool enable)
enable ? enabled : disabled);
}
+
/*
* Latency for FIFO fetches is dependent on several factors:
* - memory configuration (speed, channels)
@@ -992,6 +1015,17 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
wm.pipe[pipe].primary, wm.pipe[pipe].cursor,
wm.sr.plane, wm.sr.cursor);
+ /*
+* FIXME DDR DVFS introduces massive memory latencies which
+* are not known to system agent so any deadline specified
+* by the display may not be respected. To support DDR DVFS
+* the watermark code needs to be rewritten to essentially
+* bypass deadline mechanism and rely solely on the
+* watermarks. For now disable DDR DVFS.
+*/
+ if (IS_CHERRYVIEW(dev_priv))
+ chv_set_memory_dvfs(dev_priv, false);
+
if (!cxsr_enabled)
intel_set_memory_cxsr(dev_priv, false);
--
2.0.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
