Re: [Intel-gfx] [PATCH 04/10] drm/i915: CHV DDR DVFS support and another watermark rewrite
On Mon, Jun 29, 2015 at 11:03:04AM +0300, Jani Nikula wrote:
On Fri, 26 Jun 2015, Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Fri, Jun 26, 2015 at 10:56:33AM -0700, Clint Taylor wrote:
On 06/24/2015 12:00 PM, ville.syrj...@linux.intel.com wrote:
+if (IS_CHERRYVIEW(dev_priv)) {
+dev_priv-wm.pri_latency[VLV_WM_LEVEL_PM5] = 12;
+dev_priv-wm.pri_latency[VLV_WM_LEVEL_DDR_DVFS] = 33;
nit #defines for these magic values please
What's the point of doing that? These values are not repeated anywhere
else.
Documentation.
I've seend the original watermark code which did this for the massive mess
that where gen2/3/4 wm code. It was unreadable, unreviewable and because
of that had bugs. I concur with Ville here.
What we might want to do is a macro to do the logical wm setting - hw
value encoding, since there's some surprising differences there between
platforms. But imo that's better done as some large-scale overall project.
-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
Re: [Intel-gfx] [PATCH 04/10] drm/i915: CHV DDR DVFS support and another watermark rewrite
On Fri, 26 Jun 2015, Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Fri, Jun 26, 2015 at 10:56:33AM -0700, Clint Taylor wrote:
On 06/24/2015 12:00 PM, ville.syrj...@linux.intel.com wrote:
+ if (IS_CHERRYVIEW(dev_priv)) {
+ dev_priv-wm.pri_latency[VLV_WM_LEVEL_PM5] = 12;
+ dev_priv-wm.pri_latency[VLV_WM_LEVEL_DDR_DVFS] = 33;
nit #defines for these magic values please
What's the point of doing that? These values are not repeated anywhere
else.
Documentation.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/10] drm/i915: CHV DDR DVFS support and another watermark rewrite
On 06/24/2015 12:00 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Turns out the VLV/CHV system agent doesn't understand memory
latencies, so trying to rely on the PND deadline mechanism is not
going to fly especially when DDR DVFS is enabled. Currently we try to
avoid the problems by lying to the system agent about the deadlines
and setting the FIFO watermarks to 8 cachelines. This however leads to
bad memory self refresh residency.
So in order to satosfy everyone we'll just give up on the deadline
scheme and program the watermarks old school based on the worst case
memory latency.
I've modelled this a bit on the ILK+ approach where we compute multiple
sets of watermarks for each pipe (PM2,PM5,DDR DVFS) and when merge thet
appropriate one later with the watermarks from other pipes. There isn't
too much to merge actually since each pipe has a totally independent
FIFO (well apart from the mess with the partially shared DSPARB
registers), but still decopuling the pipes from each other seems like a
good idea.
Eventually we'll want to perform the watermark update in two phases
around the plane update to avoid underruns due to the single buffered
watermark registers. But that's still in limbo for ILK+ too, so I've not
gone that far yet for VLV/CHV either.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 28 +--
drivers/gpu/drm/i915/intel_display.c | 6 +-
drivers/gpu/drm/i915/intel_drv.h | 11 ++
drivers/gpu/drm/i915/intel_pm.c | 318 ++-
4 files changed, 345 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 514adcf..37cc653 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -276,6 +276,12 @@ struct i915_hotplug {
dev-mode_config.plane_list,\
base.head)
+#define for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) \
+ list_for_each_entry(intel_plane,\
+ (dev)-mode_config.plane_list, \
+ base.head) \
+ if ((intel_plane)-pipe == (intel_crtc)-pipe)
+
#define for_each_intel_crtc(dev, intel_crtc) \
list_for_each_entry(intel_crtc, dev-mode_config.crtc_list, base.head)
@@ -1498,18 +1504,20 @@ struct ilk_wm_values {
enum intel_ddb_partitioning partitioning;
};
-struct vlv_wm_values {
- struct {
- uint16_t primary;
- uint16_t sprite[2];
- uint8_t cursor;
- } pipe[3];
+struct vlv_pipe_wm {
+ uint16_t primary;
+ uint16_t sprite[2];
+ uint8_t cursor;
+};
- struct {
- uint16_t plane;
- uint8_t cursor;
- } sr;
+struct vlv_sr_wm {
+ uint16_t plane;
+ uint8_t cursor;
+};
+struct vlv_wm_values {
+ struct vlv_pipe_wm pipe[3];
+ struct vlv_sr_wm sr;
struct {
uint8_t cursor;
uint8_t sprite[2];
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b15d57f..1424320 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4690,8 +4690,11 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
* event which is after the vblank start event, so we need to have a
* wait-for-vblank between disabling the plane and the pipe.
*/
- if (HAS_GMCH_DISPLAY(dev))
+ if (HAS_GMCH_DISPLAY(dev)) {
intel_set_memory_cxsr(dev_priv, false);
+ dev_priv-wm.vlv.cxsr = false;
+ intel_wait_for_vblank(dev, pipe);
+ }
/*
* FIXME IPS should be fine as long as one plane is
@@ -6005,7 +6008,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
intel_crtc_load_lut(crtc);
- intel_update_watermarks(crtc);
intel_enable_pipe(intel_crtc);
assert_vblank_disabled(crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3673a71..f26a680 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -462,6 +462,15 @@ struct intel_crtc_state {
enum pipe hsw_workaround_pipe;
};
+struct vlv_wm_state {
+ struct vlv_pipe_wm wm[3];
+ struct vlv_sr_wm sr[3];
+ uint8_t num_active_planes;
+ uint8_t num_levels;
+ uint8_t level;
+ bool cxsr;
+};
+
struct intel_pipe_wm {
struct intel_wm_level wm[5];
uint32_t linetime;
@@ -564,6 +573,8 @@ struct intel_crtc {
/* scalers available on this crtc */
int num_scalers;
+
+ struct vlv_wm_state wm_state;
};
struct intel_plane_wm_parameters {
diff --git a/drivers/gpu/drm/i915/intel_pm.c
Re: [Intel-gfx] [PATCH 04/10] drm/i915: CHV DDR DVFS support and another watermark rewrite
On Fri, Jun 26, 2015 at 10:56:33AM -0700, Clint Taylor wrote:
On 06/24/2015 12:00 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Turns out the VLV/CHV system agent doesn't understand memory
latencies, so trying to rely on the PND deadline mechanism is not
going to fly especially when DDR DVFS is enabled. Currently we try to
avoid the problems by lying to the system agent about the deadlines
and setting the FIFO watermarks to 8 cachelines. This however leads to
bad memory self refresh residency.
So in order to satosfy everyone we'll just give up on the deadline
scheme and program the watermarks old school based on the worst case
memory latency.
I've modelled this a bit on the ILK+ approach where we compute multiple
sets of watermarks for each pipe (PM2,PM5,DDR DVFS) and when merge thet
appropriate one later with the watermarks from other pipes. There isn't
too much to merge actually since each pipe has a totally independent
FIFO (well apart from the mess with the partially shared DSPARB
registers), but still decopuling the pipes from each other seems like a
good idea.
Eventually we'll want to perform the watermark update in two phases
around the plane update to avoid underruns due to the single buffered
watermark registers. But that's still in limbo for ILK+ too, so I've not
gone that far yet for VLV/CHV either.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 28 +--
drivers/gpu/drm/i915/intel_display.c | 6 +-
drivers/gpu/drm/i915/intel_drv.h | 11 ++
drivers/gpu/drm/i915/intel_pm.c | 318
++-
4 files changed, 345 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 514adcf..37cc653 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -276,6 +276,12 @@ struct i915_hotplug {
dev-mode_config.plane_list, \
base.head)
+#define for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) \
+ list_for_each_entry(intel_plane,\
+ (dev)-mode_config.plane_list, \
+ base.head) \
+ if ((intel_plane)-pipe == (intel_crtc)-pipe)
+
#define for_each_intel_crtc(dev, intel_crtc) \
list_for_each_entry(intel_crtc, dev-mode_config.crtc_list, base.head)
@@ -1498,18 +1504,20 @@ struct ilk_wm_values {
enum intel_ddb_partitioning partitioning;
};
-struct vlv_wm_values {
- struct {
- uint16_t primary;
- uint16_t sprite[2];
- uint8_t cursor;
- } pipe[3];
+struct vlv_pipe_wm {
+ uint16_t primary;
+ uint16_t sprite[2];
+ uint8_t cursor;
+};
- struct {
- uint16_t plane;
- uint8_t cursor;
- } sr;
+struct vlv_sr_wm {
+ uint16_t plane;
+ uint8_t cursor;
+};
+struct vlv_wm_values {
+ struct vlv_pipe_wm pipe[3];
+ struct vlv_sr_wm sr;
struct {
uint8_t cursor;
uint8_t sprite[2];
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b15d57f..1424320 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4690,8 +4690,11 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
* event which is after the vblank start event, so we need to have a
* wait-for-vblank between disabling the plane and the pipe.
*/
- if (HAS_GMCH_DISPLAY(dev))
+ if (HAS_GMCH_DISPLAY(dev)) {
intel_set_memory_cxsr(dev_priv, false);
+ dev_priv-wm.vlv.cxsr = false;
+ intel_wait_for_vblank(dev, pipe);
+ }
/*
* FIXME IPS should be fine as long as one plane is
@@ -6005,7 +6008,6 @@ static void valleyview_crtc_enable(struct drm_crtc
*crtc)
intel_crtc_load_lut(crtc);
- intel_update_watermarks(crtc);
intel_enable_pipe(intel_crtc);
assert_vblank_disabled(crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 3673a71..f26a680 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -462,6 +462,15 @@ struct intel_crtc_state {
enum pipe hsw_workaround_pipe;
};
+struct vlv_wm_state {
+ struct vlv_pipe_wm wm[3];
+ struct vlv_sr_wm sr[3];
+ uint8_t num_active_planes;
+ uint8_t num_levels;
+ uint8_t level;
+ bool cxsr;
+};
+
struct intel_pipe_wm {
struct intel_wm_level wm[5];
uint32_t linetime;
@@ -564,6 +573,8 @@ struct intel_crtc {
/* scalers available on this crtc */
int num_scalers;
+
+ struct vlv_wm_state wm_state;
Re: [Intel-gfx] [PATCH 04/10] drm/i915: CHV DDR DVFS support and another watermark rewrite
On 06/26/2015 12:48 PM, Ville Syrjälä wrote:
On Fri, Jun 26, 2015 at 10:56:33AM -0700, Clint Taylor wrote:
On 06/24/2015 12:00 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Turns out the VLV/CHV system agent doesn't understand memory
latencies, so trying to rely on the PND deadline mechanism is not
going to fly especially when DDR DVFS is enabled. Currently we try to
avoid the problems by lying to the system agent about the deadlines
and setting the FIFO watermarks to 8 cachelines. This however leads to
bad memory self refresh residency.
So in order to satosfy everyone we'll just give up on the deadline
scheme and program the watermarks old school based on the worst case
memory latency.
I've modelled this a bit on the ILK+ approach where we compute multiple
sets of watermarks for each pipe (PM2,PM5,DDR DVFS) and when merge thet
appropriate one later with the watermarks from other pipes. There isn't
too much to merge actually since each pipe has a totally independent
FIFO (well apart from the mess with the partially shared DSPARB
registers), but still decopuling the pipes from each other seems like a
good idea.
Eventually we'll want to perform the watermark update in two phases
around the plane update to avoid underruns due to the single buffered
watermark registers. But that's still in limbo for ILK+ too, so I've not
gone that far yet for VLV/CHV either.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 28 +--
drivers/gpu/drm/i915/intel_display.c | 6 +-
drivers/gpu/drm/i915/intel_drv.h | 11 ++
drivers/gpu/drm/i915/intel_pm.c | 318
++-
4 files changed, 345 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 514adcf..37cc653 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -276,6 +276,12 @@ struct i915_hotplug {
dev-mode_config.plane_list,\
base.head)
+#define for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) \
+ list_for_each_entry(intel_plane,\
+ (dev)-mode_config.plane_list, \
+ base.head) \
+ if ((intel_plane)-pipe == (intel_crtc)-pipe)
+
#define for_each_intel_crtc(dev, intel_crtc) \
list_for_each_entry(intel_crtc, dev-mode_config.crtc_list, base.head)
@@ -1498,18 +1504,20 @@ struct ilk_wm_values {
enum intel_ddb_partitioning partitioning;
};
-struct vlv_wm_values {
- struct {
- uint16_t primary;
- uint16_t sprite[2];
- uint8_t cursor;
- } pipe[3];
+struct vlv_pipe_wm {
+ uint16_t primary;
+ uint16_t sprite[2];
+ uint8_t cursor;
+};
- struct {
- uint16_t plane;
- uint8_t cursor;
- } sr;
+struct vlv_sr_wm {
+ uint16_t plane;
+ uint8_t cursor;
+};
+struct vlv_wm_values {
+ struct vlv_pipe_wm pipe[3];
+ struct vlv_sr_wm sr;
struct {
uint8_t cursor;
uint8_t sprite[2];
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b15d57f..1424320 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4690,8 +4690,11 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
* event which is after the vblank start event, so we need to have a
* wait-for-vblank between disabling the plane and the pipe.
*/
- if (HAS_GMCH_DISPLAY(dev))
+ if (HAS_GMCH_DISPLAY(dev)) {
intel_set_memory_cxsr(dev_priv, false);
+ dev_priv-wm.vlv.cxsr = false;
+ intel_wait_for_vblank(dev, pipe);
+ }
/*
* FIXME IPS should be fine as long as one plane is
@@ -6005,7 +6008,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
intel_crtc_load_lut(crtc);
- intel_update_watermarks(crtc);
intel_enable_pipe(intel_crtc);
assert_vblank_disabled(crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3673a71..f26a680 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -462,6 +462,15 @@ struct intel_crtc_state {
enum pipe hsw_workaround_pipe;
};
+struct vlv_wm_state {
+ struct vlv_pipe_wm wm[3];
+ struct vlv_sr_wm sr[3];
+ uint8_t num_active_planes;
+ uint8_t num_levels;
+ uint8_t level;
+ bool cxsr;
+};
+
struct intel_pipe_wm {
struct intel_wm_level wm[5];
uint32_t linetime;
@@ -564,6 +573,8 @@ struct intel_crtc {
/* scalers available on this crtc */
int num_scalers;
+
+ struct
[Intel-gfx] [PATCH 04/10] drm/i915: CHV DDR DVFS support and another watermark rewrite
From: Ville Syrjälä ville.syrj...@linux.intel.com
Turns out the VLV/CHV system agent doesn't understand memory
latencies, so trying to rely on the PND deadline mechanism is not
going to fly especially when DDR DVFS is enabled. Currently we try to
avoid the problems by lying to the system agent about the deadlines
and setting the FIFO watermarks to 8 cachelines. This however leads to
bad memory self refresh residency.
So in order to satosfy everyone we'll just give up on the deadline
scheme and program the watermarks old school based on the worst case
memory latency.
I've modelled this a bit on the ILK+ approach where we compute multiple
sets of watermarks for each pipe (PM2,PM5,DDR DVFS) and when merge thet
appropriate one later with the watermarks from other pipes. There isn't
too much to merge actually since each pipe has a totally independent
FIFO (well apart from the mess with the partially shared DSPARB
registers), but still decopuling the pipes from each other seems like a
good idea.
Eventually we'll want to perform the watermark update in two phases
around the plane update to avoid underruns due to the single buffered
watermark registers. But that's still in limbo for ILK+ too, so I've not
gone that far yet for VLV/CHV either.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 28 +--
drivers/gpu/drm/i915/intel_display.c | 6 +-
drivers/gpu/drm/i915/intel_drv.h | 11 ++
drivers/gpu/drm/i915/intel_pm.c | 318 ++-
4 files changed, 345 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 514adcf..37cc653 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -276,6 +276,12 @@ struct i915_hotplug {
dev-mode_config.plane_list, \
base.head)
+#define for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) \
+ list_for_each_entry(intel_plane,\
+ (dev)-mode_config.plane_list, \
+ base.head) \
+ if ((intel_plane)-pipe == (intel_crtc)-pipe)
+
#define for_each_intel_crtc(dev, intel_crtc) \
list_for_each_entry(intel_crtc, dev-mode_config.crtc_list, base.head)
@@ -1498,18 +1504,20 @@ struct ilk_wm_values {
enum intel_ddb_partitioning partitioning;
};
-struct vlv_wm_values {
- struct {
- uint16_t primary;
- uint16_t sprite[2];
- uint8_t cursor;
- } pipe[3];
+struct vlv_pipe_wm {
+ uint16_t primary;
+ uint16_t sprite[2];
+ uint8_t cursor;
+};
- struct {
- uint16_t plane;
- uint8_t cursor;
- } sr;
+struct vlv_sr_wm {
+ uint16_t plane;
+ uint8_t cursor;
+};
+struct vlv_wm_values {
+ struct vlv_pipe_wm pipe[3];
+ struct vlv_sr_wm sr;
struct {
uint8_t cursor;
uint8_t sprite[2];
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b15d57f..1424320 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4690,8 +4690,11 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
* event which is after the vblank start event, so we need to have a
* wait-for-vblank between disabling the plane and the pipe.
*/
- if (HAS_GMCH_DISPLAY(dev))
+ if (HAS_GMCH_DISPLAY(dev)) {
intel_set_memory_cxsr(dev_priv, false);
+ dev_priv-wm.vlv.cxsr = false;
+ intel_wait_for_vblank(dev, pipe);
+ }
/*
* FIXME IPS should be fine as long as one plane is
@@ -6005,7 +6008,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
intel_crtc_load_lut(crtc);
- intel_update_watermarks(crtc);
intel_enable_pipe(intel_crtc);
assert_vblank_disabled(crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3673a71..f26a680 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -462,6 +462,15 @@ struct intel_crtc_state {
enum pipe hsw_workaround_pipe;
};
+struct vlv_wm_state {
+ struct vlv_pipe_wm wm[3];
+ struct vlv_sr_wm sr[3];
+ uint8_t num_active_planes;
+ uint8_t num_levels;
+ uint8_t level;
+ bool cxsr;
+};
+
struct intel_pipe_wm {
struct intel_wm_level wm[5];
uint32_t linetime;
@@ -564,6 +573,8 @@ struct intel_crtc {
/* scalers available on this crtc */
int num_scalers;
+
+ struct vlv_wm_state wm_state;
};
struct intel_plane_wm_parameters {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e67548d..d046e5f 100644
---
