* [RFC 1/8] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code
2015-07-02 2:25 [RFC 0/8] Atomic watermark updates (v2) Matt Roper
@ 2015-07-02 2:25 ` Matt Roper
2015-07-02 2:25 ` [RFC 2/8] drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM Matt Roper
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Matt Roper @ 2015-07-02 2:25 UTC (permalink / raw)
To: intel-gfx
Just pull the info out of the plane state structure rather than staging
it in an additional structure.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 133 +++++++++++++++++++++-------------------
1 file changed, 70 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6eb5d76..2f38070 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1778,9 +1778,6 @@ struct ilk_pipe_wm_parameters {
bool active;
uint32_t pipe_htotal;
uint32_t pixel_rate;
- struct intel_plane_wm_parameters pri;
- struct intel_plane_wm_parameters spr;
- struct intel_plane_wm_parameters cur;
};
struct ilk_wm_maximums {
@@ -1802,25 +1799,25 @@ struct intel_wm_config {
* mem_value must be in 0.1us units.
*/
static uint32_t ilk_compute_pri_wm(const struct ilk_pipe_wm_parameters *params,
+ const struct intel_plane_state *pstate,
uint32_t mem_value,
bool is_lp)
{
+ int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
uint32_t method1, method2;
- if (!params->active || !params->pri.enabled)
+ if (!params->active || !pstate->visible)
return 0;
- method1 = ilk_wm_method1(params->pixel_rate,
- params->pri.bytes_per_pixel,
- mem_value);
+ method1 = ilk_wm_method1(params->pixel_rate, bpp, mem_value);
if (!is_lp)
return method1;
method2 = ilk_wm_method2(params->pixel_rate,
params->pipe_htotal,
- params->pri.horiz_pixels,
- params->pri.bytes_per_pixel,
+ drm_rect_width(&pstate->dst),
+ bpp,
mem_value);
return min(method1, method2);
@@ -1831,20 +1828,20 @@ static uint32_t ilk_compute_pri_wm(const struct ilk_pipe_wm_parameters *params,
* mem_value must be in 0.1us units.
*/
static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
+ const struct intel_plane_state *pstate,
uint32_t mem_value)
{
+ int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
uint32_t method1, method2;
- if (!params->active || !params->spr.enabled)
+ if (!params->active || !pstate->visible)
return 0;
- method1 = ilk_wm_method1(params->pixel_rate,
- params->spr.bytes_per_pixel,
- mem_value);
+ method1 = ilk_wm_method1(params->pixel_rate, bpp, mem_value);
method2 = ilk_wm_method2(params->pixel_rate,
params->pipe_htotal,
- params->spr.horiz_pixels,
- params->spr.bytes_per_pixel,
+ drm_rect_width(&pstate->dst),
+ bpp,
mem_value);
return min(method1, method2);
}
@@ -1854,28 +1851,32 @@ static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
* mem_value must be in 0.1us units.
*/
static uint32_t ilk_compute_cur_wm(const struct ilk_pipe_wm_parameters *params,
+ const struct intel_plane_state *pstate,
uint32_t mem_value)
{
- if (!params->active || !params->cur.enabled)
+ int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
+
+ if (!params->active || !pstate->visible)
return 0;
return ilk_wm_method2(params->pixel_rate,
params->pipe_htotal,
- params->cur.horiz_pixels,
- params->cur.bytes_per_pixel,
+ drm_rect_width(&pstate->dst),
+ bpp,
mem_value);
}
/* Only for WM_LP. */
static uint32_t ilk_compute_fbc_wm(const struct ilk_pipe_wm_parameters *params,
+ const struct intel_plane_state *pstate,
uint32_t pri_val)
{
- if (!params->active || !params->pri.enabled)
+ int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
+
+ if (!params->active || !pstate->visible)
return 0;
- return ilk_wm_fbc(pri_val,
- params->pri.horiz_pixels,
- params->pri.bytes_per_pixel);
+ return ilk_wm_fbc(pri_val, drm_rect_width(&pstate->dst), bpp);
}
static unsigned int ilk_display_fifo_size(const struct drm_device *dev)
@@ -2040,10 +2041,12 @@ static bool ilk_validate_wm_level(int level,
}
static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
+ const struct intel_crtc *intel_crtc,
int level,
const struct ilk_pipe_wm_parameters *p,
struct intel_wm_level *result)
{
+ struct intel_plane *intel_plane;
uint16_t pri_latency = dev_priv->wm.pri_latency[level];
uint16_t spr_latency = dev_priv->wm.spr_latency[level];
uint16_t cur_latency = dev_priv->wm.cur_latency[level];
@@ -2055,10 +2058,29 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
cur_latency *= 5;
}
- result->pri_val = ilk_compute_pri_wm(p, pri_latency, level);
- result->spr_val = ilk_compute_spr_wm(p, spr_latency);
- result->cur_val = ilk_compute_cur_wm(p, cur_latency);
- result->fbc_val = ilk_compute_fbc_wm(p, result->pri_val);
+ for_each_intel_plane_on_crtc(dev_priv->dev, intel_crtc, intel_plane) {
+ struct intel_plane_state *pstate =
+ to_intel_plane_state(intel_plane->base.state);
+
+ switch (intel_plane->base.type) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ result->pri_val = ilk_compute_pri_wm(p, pstate,
+ pri_latency,
+ level);
+ result->fbc_val = ilk_compute_fbc_wm(p, pstate,
+ result->pri_val);
+ break;
+ case DRM_PLANE_TYPE_OVERLAY:
+ result->spr_val = ilk_compute_spr_wm(p, pstate,
+ spr_latency);
+ break;
+ case DRM_PLANE_TYPE_CURSOR:
+ result->cur_val = ilk_compute_cur_wm(p, pstate,
+ cur_latency);
+ break;
+ }
+ }
+
result->enable = true;
}
@@ -2320,10 +2342,7 @@ static void skl_setup_wm_latency(struct drm_device *dev)
static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
struct ilk_pipe_wm_parameters *p)
{
- struct drm_device *dev = crtc->dev;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- enum pipe pipe = intel_crtc->pipe;
- struct drm_plane *plane;
if (!intel_crtc->active)
return;
@@ -2331,32 +2350,6 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
p->active = true;
p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
p->pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
-
- if (crtc->primary->state->fb)
- p->pri.bytes_per_pixel =
- crtc->primary->state->fb->bits_per_pixel / 8;
- else
- p->pri.bytes_per_pixel = 4;
-
- p->cur.bytes_per_pixel = 4;
- /*
- * TODO: for now, assume primary and cursor planes are always enabled.
- * Setting them to false makes the screen flicker.
- */
- p->pri.enabled = true;
- p->cur.enabled = true;
-
- p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
- p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
-
- drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
- struct intel_plane *intel_plane = to_intel_plane(plane);
-
- if (intel_plane->pipe == pipe) {
- p->spr = intel_plane->wm;
- break;
- }
- }
}
static void ilk_compute_wm_config(struct drm_device *dev,
@@ -2384,28 +2377,42 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
{
struct drm_device *dev = crtc->dev;
const struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_plane *intel_plane;
+ struct intel_plane_state *sprstate = NULL;
int level, max_level = ilk_wm_max_level(dev);
/* LP0 watermark maximums depend on this pipe alone */
struct intel_wm_config config = {
.num_pipes_active = 1,
- .sprites_enabled = params->spr.enabled,
- .sprites_scaled = params->spr.scaled,
};
struct ilk_wm_maximums max;
+ for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+ if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) {
+ sprstate = to_intel_plane_state(intel_plane->base.state);
+ break;
+ }
+ }
+
+ config.sprites_enabled = sprstate->visible;
+ config.sprites_scaled =
+ drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 ||
+ drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16;
+
+
pipe_wm->pipe_enabled = params->active;
- pipe_wm->sprites_enabled = params->spr.enabled;
- pipe_wm->sprites_scaled = params->spr.scaled;
+ pipe_wm->sprites_enabled = sprstate->visible;
+ pipe_wm->sprites_scaled = config.sprites_scaled;
/* ILK/SNB: LP2+ watermarks only w/o sprites */
- if (INTEL_INFO(dev)->gen <= 6 && params->spr.enabled)
+ if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible)
max_level = 1;
/* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */
- if (params->spr.scaled)
+ if (config.sprites_scaled)
max_level = 0;
- ilk_compute_wm_level(dev_priv, 0, params, &pipe_wm->wm[0]);
+ ilk_compute_wm_level(dev_priv, intel_crtc, 0, params, &pipe_wm->wm[0]);
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);
@@ -2422,7 +2429,7 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
for (level = 1; level <= max_level; level++) {
struct intel_wm_level wm = {};
- ilk_compute_wm_level(dev_priv, level, params, &wm);
+ ilk_compute_wm_level(dev_priv, intel_crtc, level, params, &wm);
/*
* Disable any watermark level that exceeds the
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* [RFC 2/8] drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM
2015-07-02 2:25 [RFC 0/8] Atomic watermark updates (v2) Matt Roper
2015-07-02 2:25 ` [RFC 1/8] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code Matt Roper
@ 2015-07-02 2:25 ` Matt Roper
2015-07-02 2:25 ` [RFC 3/8] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check Matt Roper
` (5 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Matt Roper @ 2015-07-02 2:25 UTC (permalink / raw)
To: intel-gfx
Just pull the info out of the CRTC state structure rather than staging
it in an additional structure.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 84 ++++++++++++++---------------------------
1 file changed, 28 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2f38070..a639cc9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1774,12 +1774,6 @@ struct skl_pipe_wm_parameters {
struct intel_plane_wm_parameters cursor;
};
-struct ilk_pipe_wm_parameters {
- bool active;
- uint32_t pipe_htotal;
- uint32_t pixel_rate;
-};
-
struct ilk_wm_maximums {
uint16_t pri;
uint16_t spr;
@@ -1798,7 +1792,7 @@ struct intel_wm_config {
* For both WM_PIPE and WM_LP.
* mem_value must be in 0.1us units.
*/
-static uint32_t ilk_compute_pri_wm(const struct ilk_pipe_wm_parameters *params,
+static uint32_t ilk_compute_pri_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t mem_value,
bool is_lp)
@@ -1806,16 +1800,16 @@ static uint32_t ilk_compute_pri_wm(const struct ilk_pipe_wm_parameters *params,
int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
uint32_t method1, method2;
- if (!params->active || !pstate->visible)
+ if (!cstate->base.active || !pstate->visible)
return 0;
- method1 = ilk_wm_method1(params->pixel_rate, bpp, mem_value);
+ method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), bpp, mem_value);
if (!is_lp)
return method1;
- method2 = ilk_wm_method2(params->pixel_rate,
- params->pipe_htotal,
+ method2 = ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
+ cstate->base.adjusted_mode.crtc_htotal,
drm_rect_width(&pstate->dst),
bpp,
mem_value);
@@ -1827,19 +1821,19 @@ static uint32_t ilk_compute_pri_wm(const struct ilk_pipe_wm_parameters *params,
* For both WM_PIPE and WM_LP.
* mem_value must be in 0.1us units.
*/
-static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
+static uint32_t ilk_compute_spr_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t mem_value)
{
int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
uint32_t method1, method2;
- if (!params->active || !pstate->visible)
+ if (!cstate->base.active || !pstate->visible)
return 0;
- method1 = ilk_wm_method1(params->pixel_rate, bpp, mem_value);
- method2 = ilk_wm_method2(params->pixel_rate,
- params->pipe_htotal,
+ method1 = ilk_wm_method1(ilk_pipe_pixel_rate(cstate), bpp, mem_value);
+ method2 = ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
+ cstate->base.adjusted_mode.crtc_htotal,
drm_rect_width(&pstate->dst),
bpp,
mem_value);
@@ -1850,30 +1844,30 @@ static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters *params,
* For both WM_PIPE and WM_LP.
* mem_value must be in 0.1us units.
*/
-static uint32_t ilk_compute_cur_wm(const struct ilk_pipe_wm_parameters *params,
+static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t mem_value)
{
int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
- if (!params->active || !pstate->visible)
+ if (!cstate->base.active || !pstate->visible)
return 0;
- return ilk_wm_method2(params->pixel_rate,
- params->pipe_htotal,
+ return ilk_wm_method2(ilk_pipe_pixel_rate(cstate),
+ cstate->base.adjusted_mode.crtc_htotal,
drm_rect_width(&pstate->dst),
bpp,
mem_value);
}
/* Only for WM_LP. */
-static uint32_t ilk_compute_fbc_wm(const struct ilk_pipe_wm_parameters *params,
+static uint32_t ilk_compute_fbc_wm(const struct intel_crtc_state *cstate,
const struct intel_plane_state *pstate,
uint32_t pri_val)
{
int bpp = pstate->base.fb ? pstate->base.fb->bits_per_pixel / 8 : 0;
- if (!params->active || !pstate->visible)
+ if (!cstate->base.active || !pstate->visible)
return 0;
return ilk_wm_fbc(pri_val, drm_rect_width(&pstate->dst), bpp);
@@ -2043,7 +2037,7 @@ static bool ilk_validate_wm_level(int level,
static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
const struct intel_crtc *intel_crtc,
int level,
- const struct ilk_pipe_wm_parameters *p,
+ struct intel_crtc_state *cstate,
struct intel_wm_level *result)
{
struct intel_plane *intel_plane;
@@ -2064,18 +2058,18 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
switch (intel_plane->base.type) {
case DRM_PLANE_TYPE_PRIMARY:
- result->pri_val = ilk_compute_pri_wm(p, pstate,
+ result->pri_val = ilk_compute_pri_wm(cstate, pstate,
pri_latency,
level);
- result->fbc_val = ilk_compute_fbc_wm(p, pstate,
+ result->fbc_val = ilk_compute_fbc_wm(cstate, pstate,
result->pri_val);
break;
case DRM_PLANE_TYPE_OVERLAY:
- result->spr_val = ilk_compute_spr_wm(p, pstate,
+ result->spr_val = ilk_compute_spr_wm(cstate, pstate,
spr_latency);
break;
case DRM_PLANE_TYPE_CURSOR:
- result->cur_val = ilk_compute_cur_wm(p, pstate,
+ result->cur_val = ilk_compute_cur_wm(cstate, pstate,
cur_latency);
break;
}
@@ -2339,19 +2333,6 @@ static void skl_setup_wm_latency(struct drm_device *dev)
intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
}
-static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
- struct ilk_pipe_wm_parameters *p)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- if (!intel_crtc->active)
- return;
-
- p->active = true;
- p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
- p->pixel_rate = ilk_pipe_pixel_rate(intel_crtc->config);
-}
-
static void ilk_compute_wm_config(struct drm_device *dev,
struct intel_wm_config *config)
{
@@ -2371,10 +2352,10 @@ static void ilk_compute_wm_config(struct drm_device *dev,
}
/* Compute new watermarks for the pipe */
-static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
- const struct ilk_pipe_wm_parameters *params,
+static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
struct intel_pipe_wm *pipe_wm)
{
+ struct drm_crtc *crtc = cstate->base.crtc;
struct drm_device *dev = crtc->dev;
const struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -2399,8 +2380,7 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 ||
drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16;
-
- pipe_wm->pipe_enabled = params->active;
+ pipe_wm->pipe_enabled = cstate->base.active;
pipe_wm->sprites_enabled = sprstate->visible;
pipe_wm->sprites_scaled = config.sprites_scaled;
@@ -2412,7 +2392,7 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
if (config.sprites_scaled)
max_level = 0;
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, params, &pipe_wm->wm[0]);
+ ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, &pipe_wm->wm[0]);
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);
@@ -2429,7 +2409,7 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
for (level = 1; level <= max_level; level++) {
struct intel_wm_level wm = {};
- ilk_compute_wm_level(dev_priv, intel_crtc, level, params, &wm);
+ ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, &wm);
/*
* Disable any watermark level that exceeds the
@@ -3731,19 +3711,17 @@ skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
static void ilk_update_wm(struct drm_crtc *crtc)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct ilk_wm_maximums max;
- struct ilk_pipe_wm_parameters params = {};
struct ilk_wm_values results = {};
enum intel_ddb_partitioning partitioning;
struct intel_pipe_wm pipe_wm = {};
struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
struct intel_wm_config config = {};
- ilk_compute_wm_parameters(crtc, ¶ms);
-
- intel_compute_pipe_wm(crtc, ¶ms, &pipe_wm);
+ intel_compute_pipe_wm(cstate, &pipe_wm);
if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
return;
@@ -3783,12 +3761,6 @@ ilk_update_sprite_wm(struct drm_plane *plane,
struct drm_device *dev = plane->dev;
struct intel_plane *intel_plane = to_intel_plane(plane);
- intel_plane->wm.enabled = enabled;
- intel_plane->wm.scaled = scaled;
- intel_plane->wm.horiz_pixels = sprite_width;
- intel_plane->wm.vert_pixels = sprite_width;
- intel_plane->wm.bytes_per_pixel = pixel_size;
-
/*
* IVB workaround: must disable low power watermarks for at least
* one frame before enabling scaling. LP watermarks can be re-enabled
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* [RFC 3/8] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check
2015-07-02 2:25 [RFC 0/8] Atomic watermark updates (v2) Matt Roper
2015-07-02 2:25 ` [RFC 1/8] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code Matt Roper
2015-07-02 2:25 ` [RFC 2/8] drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM Matt Roper
@ 2015-07-02 2:25 ` Matt Roper
2015-10-02 16:03 ` Egbert Eich
2015-07-02 2:25 ` [RFC 4/8] drm/i915: Refactor ilk_update_wm (v3) Matt Roper
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Matt Roper @ 2015-07-02 2:25 UTC (permalink / raw)
To: intel-gfx
Determine whether we need to apply this workaround at atomic check time
and just set a flag that will be used by the main watermark update
routine.
Moving this workaround into the atomic framework reduces
ilk_update_sprite_wm() to just a standard watermark update, so drop it
completely and just ensure that ilk_update_wm() is called whenever a
sprite plane is updated in a way that would affect watermarks.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_atomic.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++--------------------
drivers/gpu/drm/i915/intel_sprite.c | 8 --------
5 files changed, 49 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 0aeced8..6673516 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -230,6 +230,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
crtc_state->base.crtc = crtc;
+ crtc_state->disable_lp_wm = false;
return &crtc_state->base;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5de1ded..46ef981 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11560,23 +11560,38 @@ retry:
static bool intel_wm_need_update(struct drm_plane *plane,
struct drm_plane_state *state)
{
- /* Update watermarks on tiling changes. */
+ struct intel_plane_state *new = to_intel_plane_state(state);
+ struct intel_plane_state *cur = to_intel_plane_state(plane->state);
+
+ /* Update watermarks on tiling or size changes. */
if (!plane->state->fb || !state->fb ||
plane->state->fb->modifier[0] != state->fb->modifier[0] ||
- plane->state->rotation != state->rotation)
- return true;
-
- if (plane->state->crtc_w != state->crtc_w)
+ plane->state->rotation != state->rotation ||
+ drm_rect_width(&new->src) != drm_rect_width(&cur->src) ||
+ drm_rect_height(&new->src) != drm_rect_height(&cur->src) ||
+ drm_rect_width(&new->dst) != drm_rect_width(&cur->dst) ||
+ drm_rect_height(&new->dst) != drm_rect_height(&cur->dst))
return true;
return false;
}
+static bool needs_scaling(struct intel_plane_state *state)
+{
+ int src_w = drm_rect_width(&state->src) >> 16;
+ int src_h = drm_rect_height(&state->src) >> 16;
+ int dst_w = drm_rect_width(&state->dst);
+ int dst_h = drm_rect_height(&state->dst);
+
+ return (src_w != dst_w || src_h != dst_h);
+}
+
int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
struct drm_plane_state *plane_state)
{
struct drm_crtc *crtc = crtc_state->crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc_state);
struct drm_plane *plane = plane_state->plane;
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -11587,7 +11602,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
bool mode_changed = needs_modeset(crtc_state);
bool was_crtc_enabled = crtc->state->active;
bool is_crtc_enabled = crtc_state->active;
-
bool turn_off, turn_on, visible, was_visible;
struct drm_framebuffer *fb = plane_state->fb;
@@ -11705,11 +11719,23 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
case DRM_PLANE_TYPE_CURSOR:
break;
case DRM_PLANE_TYPE_OVERLAY:
- if (turn_off && !mode_changed) {
+ /*
+ * WaCxSRDisabledForSpriteScaling:ivb
+ *
+ * atomic.update_wm was already set above, so this flag will
+ * take effect when we commit and program watermarks.
+ */
+ if (IS_IVYBRIDGE(dev) &&
+ needs_scaling(to_intel_plane_state(plane_state)) &&
+ !needs_scaling(old_plane_state)) {
+ cstate->disable_lp_wm = true;
+ } else if (turn_off && !mode_changed) {
intel_crtc->atomic.wait_vblank = true;
intel_crtc->atomic.update_sprite_watermarks |=
1 << i;
}
+
+ break;
}
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9ffacc0..cdc7d6d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -460,6 +460,9 @@ struct intel_crtc_state {
/* w/a for waiting 2 vblanks during crtc enable */
enum pipe hsw_workaround_pipe;
+
+ /* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
+ bool disable_lp_wm;
};
struct vlv_wm_state {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a639cc9..d061fcd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3721,6 +3721,18 @@ static void ilk_update_wm(struct drm_crtc *crtc)
struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
struct intel_wm_config config = {};
+ /*
+ * IVB workaround: must disable low power watermarks for at least
+ * one frame before enabling scaling. LP watermarks can be re-enabled
+ * when scaling is disabled.
+ *
+ * WaCxSRDisabledForSpriteScaling:ivb
+ */
+ if (cstate->disable_lp_wm) {
+ ilk_disable_lp_wm(dev);
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ }
+
intel_compute_pipe_wm(cstate, &pipe_wm);
if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
@@ -3752,28 +3764,6 @@ static void ilk_update_wm(struct drm_crtc *crtc)
ilk_write_wm_values(dev_priv, &results);
}
-static void
-ilk_update_sprite_wm(struct drm_plane *plane,
- struct drm_crtc *crtc,
- uint32_t sprite_width, uint32_t sprite_height,
- int pixel_size, bool enabled, bool scaled)
-{
- struct drm_device *dev = plane->dev;
- struct intel_plane *intel_plane = to_intel_plane(plane);
-
- /*
- * IVB workaround: must disable low power watermarks for at least
- * one frame before enabling scaling. LP watermarks can be re-enabled
- * when scaling is disabled.
- *
- * WaCxSRDisabledForSpriteScaling:ivb
- */
- if (IS_IVYBRIDGE(dev) && scaled && ilk_disable_lp_wm(dev))
- intel_wait_for_vblank(dev, intel_plane->pipe);
-
- ilk_update_wm(crtc);
-}
-
static void skl_pipe_wm_active_state(uint32_t val,
struct skl_pipe_wm *active,
bool is_transwm,
@@ -7045,7 +7035,6 @@ void intel_init_pm(struct drm_device *dev)
(!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] &&
dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
dev_priv->display.update_wm = ilk_update_wm;
- dev_priv->display.update_sprite_wm = ilk_update_sprite_wm;
} else {
DRM_DEBUG_KMS("Failed to read display plane latency. "
"Disable CxSR\n");
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index cd21525..cc83a2c 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -529,10 +529,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
sprctl |= SPRITE_PIPE_CSC_ENABLE;
- intel_update_sprite_watermarks(plane, crtc, src_w, src_h, pixel_size,
- true,
- src_w != crtc_w || src_h != crtc_h);
-
/* Sizes are 0 based */
src_w--;
src_h--;
@@ -666,10 +662,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
if (IS_GEN6(dev))
dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
- intel_update_sprite_watermarks(plane, crtc, src_w, src_h,
- pixel_size, true,
- src_w != crtc_w || src_h != crtc_h);
-
/* Sizes are 0 based */
src_w--;
src_h--;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC 3/8] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check
2015-07-02 2:25 ` [RFC 3/8] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check Matt Roper
@ 2015-10-02 16:03 ` Egbert Eich
2015-10-06 8:23 ` Daniel Vetter
0 siblings, 1 reply; 19+ messages in thread
From: Egbert Eich @ 2015-10-02 16:03 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
Regarding commit 7809e5ae35b9d8d0710f0874b2e3f10be144e38b
On Wed, Jul 01, 2015 at 07:25:56PM -0700, Matt Roper wrote:
> Determine whether we need to apply this workaround at atomic check time
> and just set a flag that will be used by the main watermark update
> routine.
>
> Moving this workaround into the atomic framework reduces
> ilk_update_sprite_wm() to just a standard watermark update, so drop it
> completely and just ensure that ilk_update_wm() is called whenever a
> sprite plane is updated in a way that would affect watermarks.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
This patch causes intel_update_watermarks() to be called much more
frequently although the watermark values don't change.
The change responsible for this is:
> index 5de1ded..46ef981 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11560,23 +11560,38 @@ retry:
> static bool intel_wm_need_update(struct drm_plane *plane,
> struct drm_plane_state *state)
> {
> - /* Update watermarks on tiling changes. */
> + struct intel_plane_state *new = to_intel_plane_state(state);
> + struct intel_plane_state *cur = to_intel_plane_state(plane->state);
> +
> + /* Update watermarks on tiling or size changes. */
> if (!plane->state->fb || !state->fb ||
> plane->state->fb->modifier[0] != state->fb->modifier[0] ||
> - plane->state->rotation != state->rotation)
> - return true;
> -
> - if (plane->state->crtc_w != state->crtc_w)
A quick look thru intel_pm.c reveals that this is relevant for
WM caluclations for gen=4 and any chipsets for which is_g4x is true.
Should this really be removed?
> + plane->state->rotation != state->rotation ||
> + drm_rect_width(&new->src) != drm_rect_width(&cur->src) ||
> + drm_rect_height(&new->src) != drm_rect_height(&cur->src) ||
> + drm_rect_width(&new->dst) != drm_rect_width(&cur->dst) ||
> + drm_rect_height(&new->dst) != drm_rect_height(&cur->dst))
these values seem to be used only for watermark calculations on gen < 9 when
HAS_PCH_SPLIT() is true.
Still these are responsible for most of the watermark recalculations (when the mouse
cursor is moved towards the edge for example). On the system I'm looking at the moment
(Q35) changes in these values don't change the WMs.
Since WM calculation is very chip generation specific and differs considerably between
generations, wouldn't it make sense to either have chipset specific functions to determin
whether a recalculation is needed - or even perfrom this in the update_wm() function
itself?
Cheers,
Egbert.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 3/8] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check
2015-10-02 16:03 ` Egbert Eich
@ 2015-10-06 8:23 ` Daniel Vetter
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2015-10-06 8:23 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx
On Fri, Oct 02, 2015 at 06:03:57PM +0200, Egbert Eich wrote:
> Regarding commit 7809e5ae35b9d8d0710f0874b2e3f10be144e38b
>
> On Wed, Jul 01, 2015 at 07:25:56PM -0700, Matt Roper wrote:
> > Determine whether we need to apply this workaround at atomic check time
> > and just set a flag that will be used by the main watermark update
> > routine.
> >
> > Moving this workaround into the atomic framework reduces
> > ilk_update_sprite_wm() to just a standard watermark update, so drop it
> > completely and just ensure that ilk_update_wm() is called whenever a
> > sprite plane is updated in a way that would affect watermarks.
> >
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>
>
> This patch causes intel_update_watermarks() to be called much more
> frequently although the watermark values don't change.
> The change responsible for this is:
>
> > index 5de1ded..46ef981 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11560,23 +11560,38 @@ retry:
> > static bool intel_wm_need_update(struct drm_plane *plane,
> > struct drm_plane_state *state)
> > {
> > - /* Update watermarks on tiling changes. */
> > + struct intel_plane_state *new = to_intel_plane_state(state);
> > + struct intel_plane_state *cur = to_intel_plane_state(plane->state);
> > +
> > + /* Update watermarks on tiling or size changes. */
> > if (!plane->state->fb || !state->fb ||
> > plane->state->fb->modifier[0] != state->fb->modifier[0] ||
> > - plane->state->rotation != state->rotation)
> > - return true;
> > -
>
> > - if (plane->state->crtc_w != state->crtc_w)
>
> A quick look thru intel_pm.c reveals that this is relevant for
> WM caluclations for gen=4 and any chipsets for which is_g4x is true.
> Should this really be removed?
>
> > + plane->state->rotation != state->rotation ||
>
> > + drm_rect_width(&new->src) != drm_rect_width(&cur->src) ||
> > + drm_rect_height(&new->src) != drm_rect_height(&cur->src) ||
> > + drm_rect_width(&new->dst) != drm_rect_width(&cur->dst) ||
> > + drm_rect_height(&new->dst) != drm_rect_height(&cur->dst))
>
> these values seem to be used only for watermark calculations on gen < 9 when
> HAS_PCH_SPLIT() is true.
>
> Still these are responsible for most of the watermark recalculations (when the mouse
> cursor is moved towards the edge for example). On the system I'm looking at the moment
> (Q35) changes in these values don't change the WMs.
>
> Since WM calculation is very chip generation specific and differs considerably between
> generations, wouldn't it make sense to either have chipset specific functions to determin
> whether a recalculation is needed - or even perfrom this in the update_wm() function
> itself?
The chipset-specific functions to recalc wm values should check for any
real changes and no-op out if none of the registers have changed. The idea
is that wm calculations are really complex and trying to keep both the "do
we need to recalc" and the actual calculations perfectly in sync is a hard
problem. Hence why we recalc aggressively to avoid hard-to-find bugs where
wm values are stale. So overhead should be just a bit of wasted cpu time.
Do you see bad things happening because of all these recalculations?
I guess if it's real trouble we can make a special case for cursors on
pre-gen9 on plane window changes.
-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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC 4/8] drm/i915: Refactor ilk_update_wm (v3)
2015-07-02 2:25 [RFC 0/8] Atomic watermark updates (v2) Matt Roper
` (2 preceding siblings ...)
2015-07-02 2:25 ` [RFC 3/8] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check Matt Roper
@ 2015-07-02 2:25 ` Matt Roper
2015-07-02 2:25 ` [RFC 5/8] drm/i915: Move active watermarks into CRTC state Matt Roper
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Matt Roper @ 2015-07-02 2:25 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Split ilk_update_wm() into two parts; one doing the programming
and the other the calculations.
v2: Fix typo in commit message
v3 (by Matt): Heavily rebased for current codebase.
Reviewed-by(v2): Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 60 ++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d061fcd..44e361c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3708,37 +3708,14 @@ skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
skl_update_wm(crtc);
}
-static void ilk_update_wm(struct drm_crtc *crtc)
+static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_device *dev = dev_priv->dev;
+ struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
struct ilk_wm_maximums max;
+ struct intel_wm_config config = {};
struct ilk_wm_values results = {};
enum intel_ddb_partitioning partitioning;
- struct intel_pipe_wm pipe_wm = {};
- struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
- struct intel_wm_config config = {};
-
- /*
- * IVB workaround: must disable low power watermarks for at least
- * one frame before enabling scaling. LP watermarks can be re-enabled
- * when scaling is disabled.
- *
- * WaCxSRDisabledForSpriteScaling:ivb
- */
- if (cstate->disable_lp_wm) {
- ilk_disable_lp_wm(dev);
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- }
-
- intel_compute_pipe_wm(cstate, &pipe_wm);
-
- if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
- return;
-
- intel_crtc->wm.active = pipe_wm;
ilk_compute_wm_config(dev, &config);
@@ -3764,6 +3741,35 @@ static void ilk_update_wm(struct drm_crtc *crtc)
ilk_write_wm_values(dev_priv, &results);
}
+static void ilk_update_wm(struct drm_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct intel_pipe_wm pipe_wm = {};
+
+ /*
+ * IVB workaround: must disable low power watermarks for at least
+ * one frame before enabling scaling. LP watermarks can be re-enabled
+ * when scaling is disabled.
+ *
+ * WaCxSRDisabledForSpriteScaling:ivb
+ */
+ if (cstate->disable_lp_wm) {
+ ilk_disable_lp_wm(crtc->dev);
+ intel_wait_for_vblank(crtc->dev, intel_crtc->pipe);
+ }
+
+ intel_compute_pipe_wm(cstate, &pipe_wm);
+
+ if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
+ return;
+
+ intel_crtc->wm.active = pipe_wm;
+
+ ilk_program_watermarks(dev_priv);
+}
+
static void skl_pipe_wm_active_state(uint32_t val,
struct skl_pipe_wm *active,
bool is_transwm,
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* [RFC 5/8] drm/i915: Move active watermarks into CRTC state
2015-07-02 2:25 [RFC 0/8] Atomic watermark updates (v2) Matt Roper
` (3 preceding siblings ...)
2015-07-02 2:25 ` [RFC 4/8] drm/i915: Refactor ilk_update_wm (v3) Matt Roper
@ 2015-07-02 2:25 ` Matt Roper
2015-07-20 9:19 ` Maarten Lankhorst
2015-07-02 2:25 ` [RFC 6/8] drm/i915: Calculate ILK-style watermarks during atomic check (v2) Matt Roper
` (2 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Matt Roper @ 2015-07-02 2:25 UTC (permalink / raw)
To: intel-gfx
Since we allocate a few CRTC states on the stack, also switch the 'wm'
struct here to be a union so that we're not wasting stack space with
other platforms' watermark values.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 8 ++++--
drivers/gpu/drm/i915/intel_drv.h | 54 +++++++++++++++++++-----------------
drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++---------
3 files changed, 55 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 46ef981..36ae3f7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4733,6 +4733,7 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
static void intel_post_plane_update(struct intel_crtc *crtc)
{
struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
struct drm_device *dev = crtc->base.dev;
struct drm_plane *plane;
@@ -4742,7 +4743,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
intel_frontbuffer_flip(dev, atomic->fb_bits);
if (atomic->disable_cxsr)
- crtc->wm.cxsr_allowed = true;
+ cstate->wm.cxsr_allowed = true;
if (crtc->atomic.update_wm_post)
intel_update_watermarks(&crtc->base);
@@ -4766,6 +4767,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
static void intel_pre_plane_update(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
struct drm_plane *p;
@@ -4798,7 +4800,7 @@ static void intel_pre_plane_update(struct intel_crtc *crtc)
intel_pre_disable_primary(&crtc->base);
if (atomic->disable_cxsr) {
- crtc->wm.cxsr_allowed = false;
+ cstate->wm.cxsr_allowed = false;
intel_set_memory_cxsr(dev_priv, false);
}
}
@@ -14127,7 +14129,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
intel_crtc->cursor_cntl = ~0;
intel_crtc->cursor_size = ~0;
- intel_crtc->wm.cxsr_allowed = true;
+ crtc_state->wm.cxsr_allowed = true;
BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cdc7d6d..c23cf7d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -328,6 +328,21 @@ struct intel_crtc_scaler_state {
int scaler_id;
};
+struct intel_pipe_wm {
+ struct intel_wm_level wm[5];
+ uint32_t linetime;
+ bool fbc_wm_enabled;
+ bool pipe_enabled;
+ bool sprites_enabled;
+ bool sprites_scaled;
+};
+
+struct skl_pipe_wm {
+ struct skl_wm_level wm[8];
+ struct skl_wm_level trans_wm;
+ uint32_t linetime;
+};
+
struct intel_crtc_state {
struct drm_crtc_state base;
@@ -463,6 +478,20 @@ struct intel_crtc_state {
/* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
bool disable_lp_wm;
+
+ struct {
+ /*
+ * final watermarks, programmed post-vblank when this state
+ * is committed
+ */
+ union {
+ struct intel_pipe_wm ilk;
+ struct skl_pipe_wm skl;
+ } active;
+
+ /* allow CxSR on this pipe */
+ bool cxsr_allowed;
+ } wm;
};
struct vlv_wm_state {
@@ -474,15 +503,6 @@ struct vlv_wm_state {
bool cxsr;
};
-struct intel_pipe_wm {
- struct intel_wm_level wm[5];
- uint32_t linetime;
- bool fbc_wm_enabled;
- bool pipe_enabled;
- bool sprites_enabled;
- bool sprites_scaled;
-};
-
struct intel_mmio_flip {
struct work_struct work;
struct drm_i915_private *i915;
@@ -490,12 +510,6 @@ struct intel_mmio_flip {
struct intel_crtc *crtc;
};
-struct skl_pipe_wm {
- struct skl_wm_level wm[8];
- struct skl_wm_level trans_wm;
- uint32_t linetime;
-};
-
/*
* Tracking of operations that need to be performed at the beginning/end of an
* atomic commit, outside the atomic section where interrupts are disabled.
@@ -564,16 +578,6 @@ struct intel_crtc {
bool cpu_fifo_underrun_disabled;
bool pch_fifo_underrun_disabled;
- /* per-pipe watermark state */
- struct {
- /* watermarks currently being used */
- struct intel_pipe_wm active;
- /* SKL wm values currently in use */
- struct skl_pipe_wm skl_active;
- /* allow CxSR on this pipe */
- bool cxsr_allowed;
- } wm;
-
int scanline_offset;
struct intel_crtc_atomic_commit atomic;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 44e361c..0e28806 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1116,6 +1116,7 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
static void vlv_compute_wm(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
struct vlv_wm_state *wm_state = &crtc->wm_state;
struct intel_plane *plane;
int sr_fifo_size = INTEL_INFO(dev)->num_pipes * 512 - 1;
@@ -1123,7 +1124,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
memset(wm_state, 0, sizeof(*wm_state));
- wm_state->cxsr = crtc->pipe != PIPE_C && crtc->wm.cxsr_allowed;
+ wm_state->cxsr = crtc->pipe != PIPE_C && cstate->wm.cxsr_allowed;
if (IS_CHERRYVIEW(dev))
wm_state->num_levels = CHV_WM_NUM_LEVELS;
else
@@ -2340,7 +2341,9 @@ static void ilk_compute_wm_config(struct drm_device *dev,
/* Compute the currently _active_ config */
for_each_intel_crtc(dev, intel_crtc) {
- const struct intel_pipe_wm *wm = &intel_crtc->wm.active;
+ const struct intel_crtc_state *cstate =
+ to_intel_crtc_state(intel_crtc->base.state);
+ const struct intel_pipe_wm *wm = &cstate->wm.active.ilk;
if (!wm->pipe_enabled)
continue;
@@ -2437,7 +2440,9 @@ static void ilk_merge_wm_level(struct drm_device *dev,
ret_wm->enable = true;
for_each_intel_crtc(dev, intel_crtc) {
- const struct intel_pipe_wm *active = &intel_crtc->wm.active;
+ const struct intel_crtc_state *cstate =
+ to_intel_crtc_state(intel_crtc->base.state);
+ const struct intel_pipe_wm *active = &cstate->wm.active.ilk;
const struct intel_wm_level *wm = &active->wm[level];
if (!active->pipe_enabled)
@@ -2583,14 +2588,15 @@ static void ilk_compute_wm_results(struct drm_device *dev,
/* LP0 register values */
for_each_intel_crtc(dev, intel_crtc) {
+ const struct intel_crtc_state *cstate =
+ to_intel_crtc_state(intel_crtc->base.state);
enum pipe pipe = intel_crtc->pipe;
- const struct intel_wm_level *r =
- &intel_crtc->wm.active.wm[0];
+ const struct intel_wm_level *r = &cstate->wm.active.ilk.wm[0];
if (WARN_ON(!r->enable))
continue;
- results->wm_linetime[pipe] = intel_crtc->wm.active.linetime;
+ results->wm_linetime[pipe] = cstate->wm.active.ilk.linetime;
results->wm_pipe[pipe] =
(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
@@ -3583,16 +3589,16 @@ static bool skl_update_pipe_wm(struct drm_crtc *crtc,
struct skl_ddb_allocation *ddb, /* out */
struct skl_pipe_wm *pipe_wm /* out */)
{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
skl_compute_wm_pipe_parameters(crtc, params);
skl_allocate_pipe_ddb(crtc, config, params, ddb);
skl_compute_pipe_wm(crtc, ddb, params, pipe_wm);
- if (!memcmp(&intel_crtc->wm.skl_active, pipe_wm, sizeof(*pipe_wm)))
+ if (!memcmp(&cstate->wm.active.skl, pipe_wm, sizeof(*pipe_wm)))
return false;
- intel_crtc->wm.skl_active = *pipe_wm;
+ cstate->wm.active.skl = *pipe_wm;
return true;
}
@@ -3762,10 +3768,10 @@ static void ilk_update_wm(struct drm_crtc *crtc)
intel_compute_pipe_wm(cstate, &pipe_wm);
- if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
+ if (!memcmp(&cstate->wm.active.ilk, &pipe_wm, sizeof(pipe_wm)))
return;
- intel_crtc->wm.active = pipe_wm;
+ cstate->wm.active.ilk = pipe_wm;
ilk_program_watermarks(dev_priv);
}
@@ -3820,7 +3826,8 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct skl_pipe_wm *active = &intel_crtc->wm.skl_active;
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct skl_pipe_wm *active = &cstate->wm.active.skl;
enum pipe pipe = intel_crtc->pipe;
int level, i, max_level;
uint32_t temp;
@@ -3883,7 +3890,8 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
struct ilk_wm_values *hw = &dev_priv->wm.hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_pipe_wm *active = &intel_crtc->wm.active;
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct intel_pipe_wm *active = &cstate->wm.active.ilk;
enum pipe pipe = intel_crtc->pipe;
static const unsigned int wm0_pipe_reg[] = {
[PIPE_A] = WM0_PIPEA_ILK,
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC 5/8] drm/i915: Move active watermarks into CRTC state
2015-07-02 2:25 ` [RFC 5/8] drm/i915: Move active watermarks into CRTC state Matt Roper
@ 2015-07-20 9:19 ` Maarten Lankhorst
0 siblings, 0 replies; 19+ messages in thread
From: Maarten Lankhorst @ 2015-07-20 9:19 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Op 02-07-15 om 04:25 schreef Matt Roper:
> Since we allocate a few CRTC states on the stack, also switch the 'wm'
> struct here to be a union so that we're not wasting stack space with
> other platforms' watermark values.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 8 ++++--
> drivers/gpu/drm/i915/intel_drv.h | 54 +++++++++++++++++++-----------------
> drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++---------
> 3 files changed, 55 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 46ef981..36ae3f7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4733,6 +4733,7 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
> static void intel_post_plane_update(struct intel_crtc *crtc)
> {
> struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
> + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
> struct drm_device *dev = crtc->base.dev;
> struct drm_plane *plane;
>
> @@ -4742,7 +4743,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> intel_frontbuffer_flip(dev, atomic->fb_bits);
>
> if (atomic->disable_cxsr)
> - crtc->wm.cxsr_allowed = true;
> + cstate->wm.cxsr_allowed = true;
I'm not sure that keeping cxsr_allowed in crtc_state makes much sense..
Seems like cxsr_allowed should be removed in 8/8 or before?
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC 6/8] drm/i915: Calculate ILK-style watermarks during atomic check (v2)
2015-07-02 2:25 [RFC 0/8] Atomic watermark updates (v2) Matt Roper
` (4 preceding siblings ...)
2015-07-02 2:25 ` [RFC 5/8] drm/i915: Move active watermarks into CRTC state Matt Roper
@ 2015-07-02 2:25 ` Matt Roper
2015-07-06 9:13 ` Daniel Vetter
2015-07-02 2:26 ` [RFC 7/8] drm/i915: Allow final wm programming to be scheduled after next vblank (v2) Matt Roper
2015-07-02 2:26 ` [RFC 8/8] drm/i915: Add two-stage ILK-style watermark programming (v2) Matt Roper
7 siblings, 1 reply; 19+ messages in thread
From: Matt Roper @ 2015-07-02 2:25 UTC (permalink / raw)
To: intel-gfx
Calculate pipe watermarks during atomic calculation phase, based on the
contents of the atomic transaction's state structure. We still program
the watermarks at the same time we did before, but the computation now
happens much earlier.
While this patch isn't too exciting by itself, it paves the way for
future patches. The eventual goal (which will be realized in future
patches in this series) is to calculate multiple sets up watermark
values up front, and then program them at different times (pre- vs
post-vblank) on the platforms that need a two-step watermark update.
While we're at it, s/intel_compute_pipe_wm/ilk_compute_pipe_wm/ since
this function only applies to ILK-style watermarks and we have a
completely different function for SKL-style watermarks.
Note that the original code had a memcmp() in ilk_update_wm() to avoid
calling ilk_program_watermarks() if the watermarks hadn't changed. This
memcmp vanishes here, which means we may do some unnecessary result
generation and merging in cases where watermarks didn't change, but the
lower-level function ilk_write_wm_values already makes sure that we
don't actually try to program the watermark registers again.
v2: Squash a few commits from the original series together; no longer
leave pre-calculated wm's in a separate temporary structure since
it's easier to follow the logic if we just cut over to using the
pre-calculated values directly.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/intel_display.c | 6 +++
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_pm.c | 87 ++++++++++++++++++------------------
4 files changed, 53 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6aa8083..2774976 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -621,6 +621,8 @@ struct drm_i915_display_funcs {
int target, int refclk,
struct dpll *match_clock,
struct dpll *best_clock);
+ int (*compute_pipe_wm)(struct drm_crtc *crtc,
+ struct drm_atomic_state *state);
void (*update_wm)(struct drm_crtc *crtc);
void (*update_sprite_wm)(struct drm_plane *plane,
struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 36ae3f7..46b62cc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11857,6 +11857,12 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
return ret;
}
+ if (dev_priv->display.compute_pipe_wm) {
+ ret = dev_priv->display.compute_pipe_wm(crtc, state);
+ if (ret)
+ return ret;
+ }
+
return intel_atomic_setup_scalers(dev, intel_crtc, pipe_config);
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c23cf7d..335b24b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1445,6 +1445,8 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
int intel_atomic_setup_scalers(struct drm_device *dev,
struct intel_crtc *intel_crtc,
struct intel_crtc_state *crtc_state);
+int intel_check_crtc(struct drm_crtc *crtc,
+ struct drm_crtc_state *state);
/* intel_atomic_plane.c */
struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0e28806..c6e735f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2039,9 +2039,11 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
const struct intel_crtc *intel_crtc,
int level,
struct intel_crtc_state *cstate,
+ struct intel_plane_state *pristate,
+ struct intel_plane_state *sprstate,
+ struct intel_plane_state *curstate,
struct intel_wm_level *result)
{
- struct intel_plane *intel_plane;
uint16_t pri_latency = dev_priv->wm.pri_latency[level];
uint16_t spr_latency = dev_priv->wm.spr_latency[level];
uint16_t cur_latency = dev_priv->wm.cur_latency[level];
@@ -2053,29 +2055,11 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
cur_latency *= 5;
}
- for_each_intel_plane_on_crtc(dev_priv->dev, intel_crtc, intel_plane) {
- struct intel_plane_state *pstate =
- to_intel_plane_state(intel_plane->base.state);
-
- switch (intel_plane->base.type) {
- case DRM_PLANE_TYPE_PRIMARY:
- result->pri_val = ilk_compute_pri_wm(cstate, pstate,
- pri_latency,
- level);
- result->fbc_val = ilk_compute_fbc_wm(cstate, pstate,
- result->pri_val);
- break;
- case DRM_PLANE_TYPE_OVERLAY:
- result->spr_val = ilk_compute_spr_wm(cstate, pstate,
- spr_latency);
- break;
- case DRM_PLANE_TYPE_CURSOR:
- result->cur_val = ilk_compute_cur_wm(cstate, pstate,
- cur_latency);
- break;
- }
- }
-
+ result->pri_val = ilk_compute_pri_wm(cstate, pristate,
+ pri_latency, level);
+ result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency);
+ result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency);
+ result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val);
result->enable = true;
}
@@ -2355,15 +2339,20 @@ static void ilk_compute_wm_config(struct drm_device *dev,
}
/* Compute new watermarks for the pipe */
-static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
- struct intel_pipe_wm *pipe_wm)
+static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct drm_crtc *crtc = cstate->base.crtc;
+ struct intel_pipe_wm *pipe_wm;
struct drm_device *dev = crtc->dev;
const struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_crtc_state *cs;
+ struct intel_crtc_state *cstate = NULL;
struct intel_plane *intel_plane;
+ struct drm_plane_state *ps;
+ struct intel_plane_state *pristate = NULL;
struct intel_plane_state *sprstate = NULL;
+ struct intel_plane_state *curstate = NULL;
int level, max_level = ilk_wm_max_level(dev);
/* LP0 watermark maximums depend on this pipe alone */
struct intel_wm_config config = {
@@ -2371,11 +2360,26 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
};
struct ilk_wm_maximums max;
+ cs = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(cs))
+ return PTR_ERR(cs);
+ else
+ cstate = to_intel_crtc_state(cs);
+
+ pipe_wm = &cstate->wm.active.ilk;
+
for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
- if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) {
- sprstate = to_intel_plane_state(intel_plane->base.state);
- break;
- }
+ ps = drm_atomic_get_plane_state(state,
+ &intel_plane->base);
+ if (IS_ERR(ps))
+ return PTR_ERR(ps);
+
+ if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
+ pristate = to_intel_plane_state(ps);
+ else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
+ sprstate = to_intel_plane_state(ps);
+ else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
+ curstate = to_intel_plane_state(ps);
}
config.sprites_enabled = sprstate->visible;
@@ -2384,7 +2388,7 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16;
pipe_wm->pipe_enabled = cstate->base.active;
- pipe_wm->sprites_enabled = sprstate->visible;
+ pipe_wm->sprites_enabled = config.sprites_enabled;
pipe_wm->sprites_scaled = config.sprites_scaled;
/* ILK/SNB: LP2+ watermarks only w/o sprites */
@@ -2395,7 +2399,8 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
if (config.sprites_scaled)
max_level = 0;
- ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, &pipe_wm->wm[0]);
+ ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
+ pristate, sprstate, curstate, &pipe_wm->wm[0]);
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);
@@ -2405,14 +2410,15 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
/* At least LP0 must be valid */
if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
- return false;
+ return -EINVAL;
ilk_compute_wm_reg_maximums(dev, 1, &max);
for (level = 1; level <= max_level; level++) {
struct intel_wm_level wm = {};
- ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, &wm);
+ ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate,
+ pristate, sprstate, curstate, &wm);
/*
* Disable any watermark level that exceeds the
@@ -2425,7 +2431,7 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
pipe_wm->wm[level] = wm;
}
- return true;
+ return 0;
}
/*
@@ -3752,7 +3758,6 @@ static void ilk_update_wm(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = to_i915(crtc->dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- struct intel_pipe_wm pipe_wm = {};
/*
* IVB workaround: must disable low power watermarks for at least
@@ -3766,13 +3771,6 @@ static void ilk_update_wm(struct drm_crtc *crtc)
intel_wait_for_vblank(crtc->dev, intel_crtc->pipe);
}
- intel_compute_pipe_wm(cstate, &pipe_wm);
-
- if (!memcmp(&cstate->wm.active.ilk, &pipe_wm, sizeof(pipe_wm)))
- return;
-
- cstate->wm.active.ilk = pipe_wm;
-
ilk_program_watermarks(dev_priv);
}
@@ -7049,6 +7047,7 @@ void intel_init_pm(struct drm_device *dev)
(!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] &&
dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
dev_priv->display.update_wm = ilk_update_wm;
+ dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm;
} else {
DRM_DEBUG_KMS("Failed to read display plane latency. "
"Disable CxSR\n");
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC 6/8] drm/i915: Calculate ILK-style watermarks during atomic check (v2)
2015-07-02 2:25 ` [RFC 6/8] drm/i915: Calculate ILK-style watermarks during atomic check (v2) Matt Roper
@ 2015-07-06 9:13 ` Daniel Vetter
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2015-07-06 9:13 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Wed, Jul 01, 2015 at 07:25:59PM -0700, Matt Roper wrote:
> Calculate pipe watermarks during atomic calculation phase, based on the
> contents of the atomic transaction's state structure. We still program
> the watermarks at the same time we did before, but the computation now
> happens much earlier.
>
> While this patch isn't too exciting by itself, it paves the way for
> future patches. The eventual goal (which will be realized in future
> patches in this series) is to calculate multiple sets up watermark
> values up front, and then program them at different times (pre- vs
> post-vblank) on the platforms that need a two-step watermark update.
>
> While we're at it, s/intel_compute_pipe_wm/ilk_compute_pipe_wm/ since
> this function only applies to ILK-style watermarks and we have a
> completely different function for SKL-style watermarks.
>
> Note that the original code had a memcmp() in ilk_update_wm() to avoid
> calling ilk_program_watermarks() if the watermarks hadn't changed. This
> memcmp vanishes here, which means we may do some unnecessary result
> generation and merging in cases where watermarks didn't change, but the
> lower-level function ilk_write_wm_values already makes sure that we
> don't actually try to program the watermark registers again.
>
> v2: Squash a few commits from the original series together; no longer
> leave pre-calculated wm's in a separate temporary structure since
> it's easier to follow the logic if we just cut over to using the
> pre-calculated values directly.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/intel_display.c | 6 +++
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> drivers/gpu/drm/i915/intel_pm.c | 87 ++++++++++++++++++------------------
> 4 files changed, 53 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6aa8083..2774976 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -621,6 +621,8 @@ struct drm_i915_display_funcs {
> int target, int refclk,
> struct dpll *match_clock,
> struct dpll *best_clock);
> + int (*compute_pipe_wm)(struct drm_crtc *crtc,
> + struct drm_atomic_state *state);
All the new callbacks you add here are per-crtc, but we might want to
rebalance the fifo space on some platforms where that's not perfectly
per-crtc. Otoh that can wait until someone actually writes that code,
maybe we'll decide to only rebalance wm when we need to do a modeset
anyway. I think we can stick with this for now.
-Daniel
> void (*update_wm)(struct drm_crtc *crtc);
> void (*update_sprite_wm)(struct drm_plane *plane,
> struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 36ae3f7..46b62cc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11857,6 +11857,12 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> return ret;
> }
>
> + if (dev_priv->display.compute_pipe_wm) {
> + ret = dev_priv->display.compute_pipe_wm(crtc, state);
> + if (ret)
> + return ret;
> + }
> +
> return intel_atomic_setup_scalers(dev, intel_crtc, pipe_config);
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c23cf7d..335b24b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1445,6 +1445,8 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state,
> int intel_atomic_setup_scalers(struct drm_device *dev,
> struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state);
> +int intel_check_crtc(struct drm_crtc *crtc,
> + struct drm_crtc_state *state);
>
> /* intel_atomic_plane.c */
> struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0e28806..c6e735f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2039,9 +2039,11 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
> const struct intel_crtc *intel_crtc,
> int level,
> struct intel_crtc_state *cstate,
> + struct intel_plane_state *pristate,
> + struct intel_plane_state *sprstate,
> + struct intel_plane_state *curstate,
> struct intel_wm_level *result)
> {
> - struct intel_plane *intel_plane;
> uint16_t pri_latency = dev_priv->wm.pri_latency[level];
> uint16_t spr_latency = dev_priv->wm.spr_latency[level];
> uint16_t cur_latency = dev_priv->wm.cur_latency[level];
> @@ -2053,29 +2055,11 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv,
> cur_latency *= 5;
> }
>
> - for_each_intel_plane_on_crtc(dev_priv->dev, intel_crtc, intel_plane) {
> - struct intel_plane_state *pstate =
> - to_intel_plane_state(intel_plane->base.state);
> -
> - switch (intel_plane->base.type) {
> - case DRM_PLANE_TYPE_PRIMARY:
> - result->pri_val = ilk_compute_pri_wm(cstate, pstate,
> - pri_latency,
> - level);
> - result->fbc_val = ilk_compute_fbc_wm(cstate, pstate,
> - result->pri_val);
> - break;
> - case DRM_PLANE_TYPE_OVERLAY:
> - result->spr_val = ilk_compute_spr_wm(cstate, pstate,
> - spr_latency);
> - break;
> - case DRM_PLANE_TYPE_CURSOR:
> - result->cur_val = ilk_compute_cur_wm(cstate, pstate,
> - cur_latency);
> - break;
> - }
> - }
> -
> + result->pri_val = ilk_compute_pri_wm(cstate, pristate,
> + pri_latency, level);
> + result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency);
> + result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency);
> + result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val);
> result->enable = true;
> }
>
> @@ -2355,15 +2339,20 @@ static void ilk_compute_wm_config(struct drm_device *dev,
> }
>
> /* Compute new watermarks for the pipe */
> -static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
> - struct intel_pipe_wm *pipe_wm)
> +static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> {
> - struct drm_crtc *crtc = cstate->base.crtc;
> + struct intel_pipe_wm *pipe_wm;
> struct drm_device *dev = crtc->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_crtc_state *cs;
> + struct intel_crtc_state *cstate = NULL;
> struct intel_plane *intel_plane;
> + struct drm_plane_state *ps;
> + struct intel_plane_state *pristate = NULL;
> struct intel_plane_state *sprstate = NULL;
> + struct intel_plane_state *curstate = NULL;
> int level, max_level = ilk_wm_max_level(dev);
> /* LP0 watermark maximums depend on this pipe alone */
> struct intel_wm_config config = {
> @@ -2371,11 +2360,26 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
> };
> struct ilk_wm_maximums max;
>
> + cs = drm_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(cs))
> + return PTR_ERR(cs);
> + else
> + cstate = to_intel_crtc_state(cs);
> +
> + pipe_wm = &cstate->wm.active.ilk;
> +
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> - if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) {
> - sprstate = to_intel_plane_state(intel_plane->base.state);
> - break;
> - }
> + ps = drm_atomic_get_plane_state(state,
> + &intel_plane->base);
> + if (IS_ERR(ps))
> + return PTR_ERR(ps);
> +
> + if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
> + pristate = to_intel_plane_state(ps);
> + else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
> + sprstate = to_intel_plane_state(ps);
> + else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
> + curstate = to_intel_plane_state(ps);
> }
>
> config.sprites_enabled = sprstate->visible;
> @@ -2384,7 +2388,7 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
> drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16;
>
> pipe_wm->pipe_enabled = cstate->base.active;
> - pipe_wm->sprites_enabled = sprstate->visible;
> + pipe_wm->sprites_enabled = config.sprites_enabled;
> pipe_wm->sprites_scaled = config.sprites_scaled;
>
> /* ILK/SNB: LP2+ watermarks only w/o sprites */
> @@ -2395,7 +2399,8 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
> if (config.sprites_scaled)
> max_level = 0;
>
> - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, &pipe_wm->wm[0]);
> + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
> + pristate, sprstate, curstate, &pipe_wm->wm[0]);
>
> if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);
> @@ -2405,14 +2410,15 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
>
> /* At least LP0 must be valid */
> if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
> - return false;
> + return -EINVAL;
>
> ilk_compute_wm_reg_maximums(dev, 1, &max);
>
> for (level = 1; level <= max_level; level++) {
> struct intel_wm_level wm = {};
>
> - ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, &wm);
> + ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate,
> + pristate, sprstate, curstate, &wm);
>
> /*
> * Disable any watermark level that exceeds the
> @@ -2425,7 +2431,7 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
> pipe_wm->wm[level] = wm;
> }
>
> - return true;
> + return 0;
> }
>
> /*
> @@ -3752,7 +3758,6 @@ static void ilk_update_wm(struct drm_crtc *crtc)
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> - struct intel_pipe_wm pipe_wm = {};
>
> /*
> * IVB workaround: must disable low power watermarks for at least
> @@ -3766,13 +3771,6 @@ static void ilk_update_wm(struct drm_crtc *crtc)
> intel_wait_for_vblank(crtc->dev, intel_crtc->pipe);
> }
>
> - intel_compute_pipe_wm(cstate, &pipe_wm);
> -
> - if (!memcmp(&cstate->wm.active.ilk, &pipe_wm, sizeof(pipe_wm)))
> - return;
> -
> - cstate->wm.active.ilk = pipe_wm;
> -
> ilk_program_watermarks(dev_priv);
> }
>
> @@ -7049,6 +7047,7 @@ void intel_init_pm(struct drm_device *dev)
> (!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] &&
> dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
> dev_priv->display.update_wm = ilk_update_wm;
> + dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm;
> } else {
> DRM_DEBUG_KMS("Failed to read display plane latency. "
> "Disable CxSR\n");
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC 7/8] drm/i915: Allow final wm programming to be scheduled after next vblank (v2)
2015-07-02 2:25 [RFC 0/8] Atomic watermark updates (v2) Matt Roper
` (5 preceding siblings ...)
2015-07-02 2:25 ` [RFC 6/8] drm/i915: Calculate ILK-style watermarks during atomic check (v2) Matt Roper
@ 2015-07-02 2:26 ` Matt Roper
2015-07-06 9:07 ` Daniel Vetter
2015-07-20 8:10 ` Maarten Lankhorst
2015-07-02 2:26 ` [RFC 8/8] drm/i915: Add two-stage ILK-style watermark programming (v2) Matt Roper
7 siblings, 2 replies; 19+ messages in thread
From: Matt Roper @ 2015-07-02 2:26 UTC (permalink / raw)
To: intel-gfx
Add a simple mechanism to trigger final watermark updates in an
asynchronous manner once the next vblank occurs. No platform types
actually support atomic watermark programming until a future patch, so
there should be no functional change yet; individual platforms will be
converted to use this mechanism one-by-one in future patches.
Note that we'll probably expand this to cover other post-vblank async
tasks (like unpinning) at some point in the future.
v2: Much simpler vblank mechanism than was used in the previous series;
no need to allocate new heap structures.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
drivers/gpu/drm/i915/i915_irq.c | 9 +++++++++
drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++----
drivers/gpu/drm/i915/intel_drv.h | 4 ++++
4 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2774976..5ad942e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -628,6 +628,7 @@ struct drm_i915_display_funcs {
struct drm_crtc *crtc,
uint32_t sprite_width, uint32_t sprite_height,
int pixel_size, bool enable, bool scaled);
+ void (*program_watermarks)(struct drm_i915_private *dev_priv);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
/* Returns the active state of the crtc, and if the crtc is active,
@@ -2567,6 +2568,12 @@ struct drm_i915_cmd_table {
#define HAS_L3_DPF(dev) (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
#define NUM_L3_SLICES(dev) (IS_HSW_GT3(dev) ? 2 : HAS_L3_DPF(dev))
+/*
+ * FIXME: Not all platforms have been transitioned to atomic watermark
+ * updates yet.
+ */
+#define HAS_ATOMIC_WM(dev_priv) (dev_priv->display.program_watermarks != NULL)
+
#define GT_FREQUENCY_MULTIPLIER 50
#define GEN9_FREQ_SCALER 3
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a6fbe64..20c7260 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1452,6 +1452,15 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+ if (intel_crtc->need_vblank_wm_update) {
+ queue_work(dev_priv->wq, &intel_crtc->wm_work);
+ intel_crtc->need_vblank_wm_update = false;
+ }
+
if (!drm_handle_vblank(dev, pipe))
return false;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 46b62cc..fa4373e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4737,6 +4737,10 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
struct drm_device *dev = crtc->base.dev;
struct drm_plane *plane;
+ if (HAS_ATOMIC_WM(to_i915(dev)))
+ /* vblank handler will kick off workqueue task to update wm's */
+ crtc->need_vblank_wm_update = true;
+
if (atomic->wait_vblank)
intel_wait_for_vblank(dev, crtc->pipe);
@@ -4745,7 +4749,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
if (atomic->disable_cxsr)
cstate->wm.cxsr_allowed = true;
- if (crtc->atomic.update_wm_post)
+ if (!HAS_ATOMIC_WM(to_i915(dev)) && crtc->atomic.update_wm_post)
intel_update_watermarks(&crtc->base);
if (atomic->update_fbc) {
@@ -4757,9 +4761,10 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
if (atomic->post_enable_primary)
intel_post_enable_primary(&crtc->base);
- drm_for_each_plane_mask(plane, dev, atomic->update_sprite_watermarks)
- intel_update_sprite_watermarks(plane, &crtc->base,
- 0, 0, 0, false, false);
+ if (!HAS_ATOMIC_WM(to_i915(dev)))
+ drm_for_each_plane_mask(plane, dev, atomic->update_sprite_watermarks)
+ intel_update_sprite_watermarks(plane, &crtc->base,
+ 0, 0, 0, false, false);
memset(atomic, 0, sizeof(*atomic));
}
@@ -14070,6 +14075,21 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
scaler_state->scaler_id = -1;
}
+/* FIXME: This may expand to cover other tasks like unpinning in the future... */
+static void wm_work_func(struct work_struct *work)
+{
+ struct intel_crtc *intel_crtc =
+ container_of(work, struct intel_crtc, wm_work);
+ struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+
+ if (WARN_ON(!HAS_ATOMIC_WM(dev_priv)))
+ return;
+ if (WARN_ON(!intel_crtc->base.state->active))
+ return;
+
+ dev_priv->display.program_watermarks(dev_priv);
+}
+
static void intel_crtc_init(struct drm_device *dev, int pipe)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -14142,6 +14162,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
+ INIT_WORK(&intel_crtc->wm_work, wm_work_func);
+
drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 335b24b..775e3d0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -586,6 +586,10 @@ struct intel_crtc {
int num_scalers;
struct vlv_wm_state wm_state;
+
+ /* Do we need to program watermark values after the next vblank? */
+ bool need_vblank_wm_update;
+ struct work_struct wm_work;
};
struct intel_plane_wm_parameters {
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC 7/8] drm/i915: Allow final wm programming to be scheduled after next vblank (v2)
2015-07-02 2:26 ` [RFC 7/8] drm/i915: Allow final wm programming to be scheduled after next vblank (v2) Matt Roper
@ 2015-07-06 9:07 ` Daniel Vetter
2015-07-06 11:23 ` Ville Syrjälä
2015-07-20 8:10 ` Maarten Lankhorst
1 sibling, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2015-07-06 9:07 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Wed, Jul 01, 2015 at 07:26:00PM -0700, Matt Roper wrote:
> Add a simple mechanism to trigger final watermark updates in an
> asynchronous manner once the next vblank occurs. No platform types
> actually support atomic watermark programming until a future patch, so
> there should be no functional change yet; individual platforms will be
> converted to use this mechanism one-by-one in future patches.
>
> Note that we'll probably expand this to cover other post-vblank async
> tasks (like unpinning) at some point in the future.
>
> v2: Much simpler vblank mechanism than was used in the previous series;
> no need to allocate new heap structures.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> drivers/gpu/drm/i915/i915_irq.c | 9 +++++++++
> drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++----
> drivers/gpu/drm/i915/intel_drv.h | 4 ++++
> 4 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2774976..5ad942e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -628,6 +628,7 @@ struct drm_i915_display_funcs {
> struct drm_crtc *crtc,
> uint32_t sprite_width, uint32_t sprite_height,
> int pixel_size, bool enable, bool scaled);
> + void (*program_watermarks)(struct drm_i915_private *dev_priv);
> int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
> void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
> /* Returns the active state of the crtc, and if the crtc is active,
> @@ -2567,6 +2568,12 @@ struct drm_i915_cmd_table {
> #define HAS_L3_DPF(dev) (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
> #define NUM_L3_SLICES(dev) (IS_HSW_GT3(dev) ? 2 : HAS_L3_DPF(dev))
>
> +/*
> + * FIXME: Not all platforms have been transitioned to atomic watermark
> + * updates yet.
> + */
> +#define HAS_ATOMIC_WM(dev_priv) (dev_priv->display.program_watermarks != NULL)
HAS_FOO is generally hw features. I think especially for just this vfunc
check is clearer to inline it.
> +
> #define GT_FREQUENCY_MULTIPLIER 50
> #define GEN9_FREQ_SCALER 3
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a6fbe64..20c7260 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1452,6 +1452,15 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
>
> static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
> {
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> + if (intel_crtc->need_vblank_wm_update) {
> + queue_work(dev_priv->wq, &intel_crtc->wm_work);
> + intel_crtc->need_vblank_wm_update = false;
We need some lock or some other means of sync to be able to cancel such an
update if userspace submits the next atomic update. Otherwise this work
might overwrite the intermediate wm values.
Imo best would be if we createa a cancel_work_sync-like interface which
ensures the work will be cancelled (when it's already scheduled) or won't
ever get scheduled through our vblank handler. That would mirror the
interface I have in mind for generic vblank workers, which imo should
closely resemble the interface we have for delayed_work (for vblank work
run in process context) and timers (for vblank run from hardirq context).
> + }
> +
> if (!drm_handle_vblank(dev, pipe))
> return false;
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 46b62cc..fa4373e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4737,6 +4737,10 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> struct drm_device *dev = crtc->base.dev;
> struct drm_plane *plane;
>
> + if (HAS_ATOMIC_WM(to_i915(dev)))
> + /* vblank handler will kick off workqueue task to update wm's */
> + crtc->need_vblank_wm_update = true;
> +
> if (atomic->wait_vblank)
> intel_wait_for_vblank(dev, crtc->pipe);
>
> @@ -4745,7 +4749,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> if (atomic->disable_cxsr)
> cstate->wm.cxsr_allowed = true;
>
> - if (crtc->atomic.update_wm_post)
> + if (!HAS_ATOMIC_WM(to_i915(dev)) && crtc->atomic.update_wm_post)
> intel_update_watermarks(&crtc->base);
>
> if (atomic->update_fbc) {
> @@ -4757,9 +4761,10 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> if (atomic->post_enable_primary)
> intel_post_enable_primary(&crtc->base);
>
> - drm_for_each_plane_mask(plane, dev, atomic->update_sprite_watermarks)
> - intel_update_sprite_watermarks(plane, &crtc->base,
> - 0, 0, 0, false, false);
> + if (!HAS_ATOMIC_WM(to_i915(dev)))
> + drm_for_each_plane_mask(plane, dev, atomic->update_sprite_watermarks)
> + intel_update_sprite_watermarks(plane, &crtc->base,
> + 0, 0, 0, false, false);
Can't we just move this loop into intel_update_watermark after all the
atomic rework and you've just having moved the lp_wm vs. scaling wa
around? One less thing to keep in mind when transitioning.
> memset(atomic, 0, sizeof(*atomic));
> }
> @@ -14070,6 +14075,21 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
> scaler_state->scaler_id = -1;
> }
>
> +/* FIXME: This may expand to cover other tasks like unpinning in the future... */
> +static void wm_work_func(struct work_struct *work)
> +{
> + struct intel_crtc *intel_crtc =
> + container_of(work, struct intel_crtc, wm_work);
> + struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +
> + if (WARN_ON(!HAS_ATOMIC_WM(dev_priv)))
> + return;
> + if (WARN_ON(!intel_crtc->base.state->active))
> + return;
> +
> + dev_priv->display.program_watermarks(dev_priv);
> +}
> +
> static void intel_crtc_init(struct drm_device *dev, int pipe)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -14142,6 +14162,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
> dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = &intel_crtc->base;
>
> + INIT_WORK(&intel_crtc->wm_work, wm_work_func);
> +
> drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>
> WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 335b24b..775e3d0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -586,6 +586,10 @@ struct intel_crtc {
> int num_scalers;
>
> struct vlv_wm_state wm_state;
> +
> + /* Do we need to program watermark values after the next vblank? */
> + bool need_vblank_wm_update;
Maybe vblank_wm_update_pending? Since "need" is platform-specific and
invariant at runtime and so a bit confusing (to me at least).
-Daniel
> + struct work_struct wm_work;
> };
>
> struct intel_plane_wm_parameters {
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 7/8] drm/i915: Allow final wm programming to be scheduled after next vblank (v2)
2015-07-06 9:07 ` Daniel Vetter
@ 2015-07-06 11:23 ` Ville Syrjälä
0 siblings, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2015-07-06 11:23 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Mon, Jul 06, 2015 at 11:07:52AM +0200, Daniel Vetter wrote:
> On Wed, Jul 01, 2015 at 07:26:00PM -0700, Matt Roper wrote:
> > Add a simple mechanism to trigger final watermark updates in an
> > asynchronous manner once the next vblank occurs. No platform types
> > actually support atomic watermark programming until a future patch, so
> > there should be no functional change yet; individual platforms will be
> > converted to use this mechanism one-by-one in future patches.
> >
> > Note that we'll probably expand this to cover other post-vblank async
> > tasks (like unpinning) at some point in the future.
> >
> > v2: Much simpler vblank mechanism than was used in the previous series;
> > no need to allocate new heap structures.
> >
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> > drivers/gpu/drm/i915/i915_irq.c | 9 +++++++++
> > drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++----
> > drivers/gpu/drm/i915/intel_drv.h | 4 ++++
> > 4 files changed, 46 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2774976..5ad942e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -628,6 +628,7 @@ struct drm_i915_display_funcs {
> > struct drm_crtc *crtc,
> > uint32_t sprite_width, uint32_t sprite_height,
> > int pixel_size, bool enable, bool scaled);
> > + void (*program_watermarks)(struct drm_i915_private *dev_priv);
> > int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
> > void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
> > /* Returns the active state of the crtc, and if the crtc is active,
> > @@ -2567,6 +2568,12 @@ struct drm_i915_cmd_table {
> > #define HAS_L3_DPF(dev) (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
> > #define NUM_L3_SLICES(dev) (IS_HSW_GT3(dev) ? 2 : HAS_L3_DPF(dev))
> >
> > +/*
> > + * FIXME: Not all platforms have been transitioned to atomic watermark
> > + * updates yet.
> > + */
> > +#define HAS_ATOMIC_WM(dev_priv) (dev_priv->display.program_watermarks != NULL)
>
> HAS_FOO is generally hw features. I think especially for just this vfunc
> check is clearer to inline it.
>
> > +
> > #define GT_FREQUENCY_MULTIPLIER 50
> > #define GEN9_FREQ_SCALER 3
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index a6fbe64..20c7260 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1452,6 +1452,15 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
> >
> > static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
> > {
> > + struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +
> > + if (intel_crtc->need_vblank_wm_update) {
> > + queue_work(dev_priv->wq, &intel_crtc->wm_work);
> > + intel_crtc->need_vblank_wm_update = false;
>
> We need some lock or some other means of sync to be able to cancel such an
> update if userspace submits the next atomic update. Otherwise this work
> might overwrite the intermediate wm values.
This series seems to be missing a bunch of stuff from my ILK wm rework,
including the wm.mutex.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC 7/8] drm/i915: Allow final wm programming to be scheduled after next vblank (v2)
2015-07-02 2:26 ` [RFC 7/8] drm/i915: Allow final wm programming to be scheduled after next vblank (v2) Matt Roper
2015-07-06 9:07 ` Daniel Vetter
@ 2015-07-20 8:10 ` Maarten Lankhorst
1 sibling, 0 replies; 19+ messages in thread
From: Maarten Lankhorst @ 2015-07-20 8:10 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Op 02-07-15 om 04:26 schreef Matt Roper:
> Add a simple mechanism to trigger final watermark updates in an
> asynchronous manner once the next vblank occurs. No platform types
> actually support atomic watermark programming until a future patch, so
> there should be no functional change yet; individual platforms will be
> converted to use this mechanism one-by-one in future patches.
>
> Note that we'll probably expand this to cover other post-vblank async
> tasks (like unpinning) at some point in the future.
>
> v2: Much simpler vblank mechanism than was used in the previous series;
> no need to allocate new heap structures.
>
Patch 1-6 are good. But 7 and 8 make my life harder.
There is more that needs to be done asynchronously, can you just commit the watermarks synchronously after drm_atomic_helper_wait_for_vblanks for now?
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC 8/8] drm/i915: Add two-stage ILK-style watermark programming (v2)
2015-07-02 2:25 [RFC 0/8] Atomic watermark updates (v2) Matt Roper
` (6 preceding siblings ...)
2015-07-02 2:26 ` [RFC 7/8] drm/i915: Allow final wm programming to be scheduled after next vblank (v2) Matt Roper
@ 2015-07-02 2:26 ` Matt Roper
2015-07-06 9:11 ` Daniel Vetter
2015-07-06 12:20 ` Maarten Lankhorst
7 siblings, 2 replies; 19+ messages in thread
From: Matt Roper @ 2015-07-02 2:26 UTC (permalink / raw)
To: intel-gfx
From: Matt Roper <matt@mattrope.com>
In addition to calculating final watermarks, let's also pre-calculate a
set of intermediate watermark values at atomic check time. These
intermediate watermarks are a combination of the watermarks for the old
state and the new state; they should satisfy the requirements of both
states which means they can be programmed immediately when we commit the
atomic state (without waiting for a vblank). Once the vblank does
happen, we can then re-program watermarks to the more optimal final
value.
v2: Significant rebasing/rewriting.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 9 +++++
drivers/gpu/drm/i915/i915_irq.c | 7 ++++
drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++++-
drivers/gpu/drm/i915/intel_drv.h | 26 +++++++++----
drivers/gpu/drm/i915/intel_pm.c | 75 ++++++++++++++++++++++++++++++------
5 files changed, 130 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5ad942e..42397e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -623,6 +623,9 @@ struct drm_i915_display_funcs {
struct dpll *best_clock);
int (*compute_pipe_wm)(struct drm_crtc *crtc,
struct drm_atomic_state *state);
+ void (*compute_intermediate_wm)(struct drm_device *dev,
+ struct intel_crtc_state *newstate,
+ const struct intel_crtc_state *oldstate);
void (*update_wm)(struct drm_crtc *crtc);
void (*update_sprite_wm)(struct drm_plane *plane,
struct drm_crtc *crtc,
@@ -2574,6 +2577,12 @@ struct drm_i915_cmd_table {
*/
#define HAS_ATOMIC_WM(dev_priv) (dev_priv->display.program_watermarks != NULL)
+/*
+ * Newer platforms have doublebuffered watermark registers and do not need
+ * the two-step watermark programming used by older platforms.
+ */
+#define HAS_DBLBUF_WM(dev_priv) (INTEL_INFO(dev_priv)->gen >= 9)
+
#define GT_FREQUENCY_MULTIPLIER 50
#define GEN9_FREQ_SCALER 3
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 20c7260..627c56f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1455,8 +1455,15 @@ static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
if (intel_crtc->need_vblank_wm_update) {
+ WARN_ON(HAS_DBLBUF_WM(dev_priv));
+
+ /* Latch final watermarks now that vblank is past */
+ cstate->wm.active = cstate->wm.target;
+
+ /* Queue work to actually program them asynchronously */
queue_work(dev_priv->wq, &intel_crtc->wm_work);
intel_crtc->need_vblank_wm_update = false;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fa4373e..1616d7f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4737,7 +4737,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
struct drm_device *dev = crtc->base.dev;
struct drm_plane *plane;
- if (HAS_ATOMIC_WM(to_i915(dev)))
+ if (HAS_ATOMIC_WM(to_i915(dev)) && !HAS_DBLBUF_WM(to_i915(dev)))
/* vblank handler will kick off workqueue task to update wm's */
crtc->need_vblank_wm_update = true;
@@ -11833,6 +11833,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc_state);
+ struct intel_crtc_state *old_pipe_config =
+ to_intel_crtc_state(crtc->state);
struct drm_atomic_state *state = crtc_state->state;
int ret, idx = crtc->base.id;
bool mode_changed = needs_modeset(crtc_state);
@@ -11863,9 +11865,20 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
}
if (dev_priv->display.compute_pipe_wm) {
+ if (WARN_ON(!dev_priv->display.compute_intermediate_wm))
+ return 0;
+
ret = dev_priv->display.compute_pipe_wm(crtc, state);
if (ret)
return ret;
+
+ /*
+ * Calculate 'intermediate' watermarks that satisfy both the old state
+ * and the new state. We can program these immediately.
+ */
+ dev_priv->display.compute_intermediate_wm(crtc->dev,
+ pipe_config,
+ old_pipe_config);
}
return intel_atomic_setup_scalers(dev, intel_crtc, pipe_config);
@@ -13789,8 +13802,25 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
if (!needs_modeset(crtc->state))
intel_pre_plane_update(intel_crtc);
- if (intel_crtc->atomic.update_wm_pre)
+ /*
+ * For platforms that support atomic watermarks, program the 'pending'
+ * watermarks immediately. On pre-gen9 platforms, these will be the
+ * intermediate values that are safe for both pre- and post- vblank;
+ * when vblank happens, the 'pending' values will be set to the final
+ * 'target' values and we'll do this again to get the optimal
+ * watermarks. For gen9+ platforms, the values we program here will be
+ * the final target values which will get automatically latched at
+ * vblank time; no further programming will be necessary.
+ *
+ * If a platform hasn't been transitioned to atomic watermarks yet,
+ * we'll continue to update watermarks the old way, if flags tell
+ * us to.
+ */
+ if (HAS_ATOMIC_WM(dev_priv)) {
+ dev_priv->display.program_watermarks(dev_priv);
+ } else if (intel_crtc->atomic.update_wm_pre) {
intel_update_watermarks(crtc);
+ }
intel_runtime_pm_get(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 775e3d0..7bc4e81 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -343,6 +343,11 @@ struct skl_pipe_wm {
uint32_t linetime;
};
+union pipe_wm {
+ struct intel_pipe_wm ilk;
+ struct skl_pipe_wm skl;
+};
+
struct intel_crtc_state {
struct drm_crtc_state base;
@@ -481,16 +486,21 @@ struct intel_crtc_state {
struct {
/*
- * final watermarks, programmed post-vblank when this state
- * is committed
+ * final, target watermarks for state; on pre-gen9 platforms,
+ * this might not have been programmed yet if a vblank hasn't
+ * happened since this state was committed
+ */
+ union pipe_wm target;
+
+ /*
+ * currently programmed watermark; on pre-gen9, this will be
+ * the intermediate values before vblank then switch to match
+ * 'target' after vblank fires)
*/
- union {
- struct intel_pipe_wm ilk;
- struct skl_pipe_wm skl;
- } active;
+ union pipe_wm active;
- /* allow CxSR on this pipe */
- bool cxsr_allowed;
+ /* allow CxSR on this pipe */
+ bool cxsr_allowed;
} wm;
};
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c6e735f..7942bb0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2338,6 +2338,24 @@ static void ilk_compute_wm_config(struct drm_device *dev,
}
}
+static bool ilk_validate_pipe_wm(struct drm_device *dev,
+ struct intel_wm_config *config,
+ struct intel_pipe_wm *pipe_wm)
+{
+ struct ilk_wm_maximums max;
+
+ /* LP0 watermarks always use 1/2 DDB partitioning */
+ ilk_compute_wm_maximums(dev, 0, config, INTEL_DDB_PART_1_2, &max);
+
+ /* At least LP0 must be valid */
+ if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) {
+ DRM_DEBUG_KMS("LP0 watermark invalid\n");
+ return false;
+ }
+
+ return true;
+}
+
/* Compute new watermarks for the pipe */
static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
struct drm_atomic_state *state)
@@ -2366,7 +2384,7 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
else
cstate = to_intel_crtc_state(cs);
- pipe_wm = &cstate->wm.active.ilk;
+ pipe_wm = &cstate->wm.target.ilk;
for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
ps = drm_atomic_get_plane_state(state,
@@ -2405,12 +2423,8 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);
- /* LP0 watermarks always use 1/2 DDB partitioning */
- ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max);
-
- /* At least LP0 must be valid */
- if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
- return -EINVAL;
+ if (!ilk_validate_pipe_wm(dev, &config, pipe_wm))
+ return false;
ilk_compute_wm_reg_maximums(dev, 1, &max);
@@ -2435,6 +2449,40 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
}
/*
+ * Build a set of 'intermediate' watermark values that satisfy both the old
+ * state and the new state. These can be programmed to the hardware
+ * immediately.
+ */
+void ilk_compute_intermediate_wm(struct drm_device *dev,
+ struct intel_crtc_state *newstate,
+ const struct intel_crtc_state *oldstate)
+{
+ struct intel_pipe_wm *a = &newstate->wm.active.ilk;
+ const struct intel_pipe_wm *b = &oldstate->wm.active.ilk;
+ int level, max_level = ilk_wm_max_level(dev);
+
+ /*
+ * Start with the final, target watermarks, then combine with the
+ * current state's watermarks.
+ */
+ *a = newstate->wm.target.ilk;
+ a->pipe_enabled |= b->pipe_enabled;
+ a->sprites_enabled |= b->sprites_enabled;
+ a->sprites_scaled |= b->sprites_scaled;
+
+ for (level = 0; level <= max_level; level++) {
+ struct intel_wm_level *a_wm = &a->wm[level];
+ const struct intel_wm_level *b_wm = &b->wm[level];
+
+ a_wm->enable &= b_wm->enable;
+ a_wm->pri_val = max(a_wm->pri_val, b_wm->pri_val);
+ a_wm->spr_val = max(a_wm->spr_val, b_wm->spr_val);
+ a_wm->cur_val = max(a_wm->cur_val, b_wm->cur_val);
+ a_wm->fbc_val = max(a_wm->fbc_val, b_wm->fbc_val);
+ }
+}
+
+/*
* Merge the watermarks from all active pipes for a specific level.
*/
static void ilk_merge_wm_level(struct drm_device *dev,
@@ -3601,10 +3649,10 @@ static bool skl_update_pipe_wm(struct drm_crtc *crtc,
skl_allocate_pipe_ddb(crtc, config, params, ddb);
skl_compute_pipe_wm(crtc, ddb, params, pipe_wm);
- if (!memcmp(&cstate->wm.active.skl, pipe_wm, sizeof(*pipe_wm)))
+ if (!memcmp(&cstate->wm.target.skl, pipe_wm, sizeof(*pipe_wm)))
return false;
- cstate->wm.active.skl = *pipe_wm;
+ cstate->wm.target.skl = *pipe_wm;
return true;
}
@@ -3825,7 +3873,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- struct skl_pipe_wm *active = &cstate->wm.active.skl;
+ struct skl_pipe_wm *active = &cstate->wm.target.skl;
enum pipe pipe = intel_crtc->pipe;
int level, i, max_level;
uint32_t temp;
@@ -3889,7 +3937,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
struct ilk_wm_values *hw = &dev_priv->wm.hw;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
- struct intel_pipe_wm *active = &cstate->wm.active.ilk;
+ struct intel_pipe_wm *active = &cstate->wm.target.ilk;
enum pipe pipe = intel_crtc->pipe;
static const unsigned int wm0_pipe_reg[] = {
[PIPE_A] = WM0_PIPEA_ILK,
@@ -3928,6 +3976,8 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
for (level = 0; level <= max_level; level++)
active->wm[level].enable = true;
}
+
+ cstate->wm.active.ilk = cstate->wm.target.ilk = *active;
}
#define _FW_WM(value, plane) \
@@ -7048,6 +7098,9 @@ void intel_init_pm(struct drm_device *dev)
dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
dev_priv->display.update_wm = ilk_update_wm;
dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm;
+ dev_priv->display.compute_intermediate_wm =
+ ilk_compute_intermediate_wm;
+ dev_priv->display.program_watermarks = ilk_program_watermarks;
} else {
DRM_DEBUG_KMS("Failed to read display plane latency. "
"Disable CxSR\n");
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC 8/8] drm/i915: Add two-stage ILK-style watermark programming (v2)
2015-07-02 2:26 ` [RFC 8/8] drm/i915: Add two-stage ILK-style watermark programming (v2) Matt Roper
@ 2015-07-06 9:11 ` Daniel Vetter
2015-07-06 12:20 ` Maarten Lankhorst
1 sibling, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2015-07-06 9:11 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Wed, Jul 01, 2015 at 07:26:01PM -0700, Matt Roper wrote:
> From: Matt Roper <matt@mattrope.com>
>
> In addition to calculating final watermarks, let's also pre-calculate a
> set of intermediate watermark values at atomic check time. These
> intermediate watermarks are a combination of the watermarks for the old
> state and the new state; they should satisfy the requirements of both
> states which means they can be programmed immediately when we commit the
> atomic state (without waiting for a vblank). Once the vblank does
> happen, we can then re-program watermarks to the more optimal final
> value.
>
> v2: Significant rebasing/rewriting.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 9 +++++
> drivers/gpu/drm/i915/i915_irq.c | 7 ++++
> drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++++-
> drivers/gpu/drm/i915/intel_drv.h | 26 +++++++++----
> drivers/gpu/drm/i915/intel_pm.c | 75 ++++++++++++++++++++++++++++++------
> 5 files changed, 130 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5ad942e..42397e2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -623,6 +623,9 @@ struct drm_i915_display_funcs {
> struct dpll *best_clock);
> int (*compute_pipe_wm)(struct drm_crtc *crtc,
> struct drm_atomic_state *state);
> + void (*compute_intermediate_wm)(struct drm_device *dev,
> + struct intel_crtc_state *newstate,
> + const struct intel_crtc_state *oldstate);
> void (*update_wm)(struct drm_crtc *crtc);
> void (*update_sprite_wm)(struct drm_plane *plane,
> struct drm_crtc *crtc,
> @@ -2574,6 +2577,12 @@ struct drm_i915_cmd_table {
> */
> #define HAS_ATOMIC_WM(dev_priv) (dev_priv->display.program_watermarks != NULL)
>
> +/*
> + * Newer platforms have doublebuffered watermark registers and do not need
> + * the two-step watermark programming used by older platforms.
> + */
> +#define HAS_DBLBUF_WM(dev_priv) (INTEL_INFO(dev_priv)->gen >= 9)
Just move need_vblank_wm_update into crtc_state and compute it in the
compute_pipe_wm functions?
> +
> #define GT_FREQUENCY_MULTIPLIER 50
> #define GEN9_FREQ_SCALER 3
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 20c7260..627c56f 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1455,8 +1455,15 @@ static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
>
> if (intel_crtc->need_vblank_wm_update) {
> + WARN_ON(HAS_DBLBUF_WM(dev_priv));
> +
> + /* Latch final watermarks now that vblank is past */
> + cstate->wm.active = cstate->wm.target;
> +
> + /* Queue work to actually program them asynchronously */
> queue_work(dev_priv->wq, &intel_crtc->wm_work);
> intel_crtc->need_vblank_wm_update = false;
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fa4373e..1616d7f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4737,7 +4737,7 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
> struct drm_device *dev = crtc->base.dev;
> struct drm_plane *plane;
>
> - if (HAS_ATOMIC_WM(to_i915(dev)))
> + if (HAS_ATOMIC_WM(to_i915(dev)) && !HAS_DBLBUF_WM(to_i915(dev)))
> /* vblank handler will kick off workqueue task to update wm's */
> crtc->need_vblank_wm_update = true;
>
> @@ -11833,6 +11833,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_crtc_state *pipe_config =
> to_intel_crtc_state(crtc_state);
> + struct intel_crtc_state *old_pipe_config =
> + to_intel_crtc_state(crtc->state);
> struct drm_atomic_state *state = crtc_state->state;
> int ret, idx = crtc->base.id;
> bool mode_changed = needs_modeset(crtc_state);
> @@ -11863,9 +11865,20 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> }
>
> if (dev_priv->display.compute_pipe_wm) {
> + if (WARN_ON(!dev_priv->display.compute_intermediate_wm))
> + return 0;
skl won't need this, right?
> +
> ret = dev_priv->display.compute_pipe_wm(crtc, state);
> if (ret)
> return ret;
> +
> + /*
> + * Calculate 'intermediate' watermarks that satisfy both the old state
> + * and the new state. We can program these immediately.
> + */
> + dev_priv->display.compute_intermediate_wm(crtc->dev,
> + pipe_config,
> + old_pipe_config);
> }
>
> return intel_atomic_setup_scalers(dev, intel_crtc, pipe_config);
> @@ -13789,8 +13802,25 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
> if (!needs_modeset(crtc->state))
> intel_pre_plane_update(intel_crtc);
>
> - if (intel_crtc->atomic.update_wm_pre)
> + /*
> + * For platforms that support atomic watermarks, program the 'pending'
> + * watermarks immediately. On pre-gen9 platforms, these will be the
> + * intermediate values that are safe for both pre- and post- vblank;
> + * when vblank happens, the 'pending' values will be set to the final
> + * 'target' values and we'll do this again to get the optimal
> + * watermarks. For gen9+ platforms, the values we program here will be
> + * the final target values which will get automatically latched at
> + * vblank time; no further programming will be necessary.
> + *
> + * If a platform hasn't been transitioned to atomic watermarks yet,
> + * we'll continue to update watermarks the old way, if flags tell
> + * us to.
> + */
> + if (HAS_ATOMIC_WM(dev_priv)) {
> + dev_priv->display.program_watermarks(dev_priv);
> + } else if (intel_crtc->atomic.update_wm_pre) {
> intel_update_watermarks(crtc);
> + }
>
> intel_runtime_pm_get(dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 775e3d0..7bc4e81 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -343,6 +343,11 @@ struct skl_pipe_wm {
> uint32_t linetime;
> };
>
> +union pipe_wm {
> + struct intel_pipe_wm ilk;
> + struct skl_pipe_wm skl;
> +};
> +
> struct intel_crtc_state {
> struct drm_crtc_state base;
>
> @@ -481,16 +486,21 @@ struct intel_crtc_state {
>
> struct {
> /*
> - * final watermarks, programmed post-vblank when this state
> - * is committed
> + * final, target watermarks for state; on pre-gen9 platforms,
> + * this might not have been programmed yet if a vblank hasn't
> + * happened since this state was committed
> + */
> + union pipe_wm target;
> +
> + /*
> + * currently programmed watermark; on pre-gen9, this will be
> + * the intermediate values before vblank then switch to match
> + * 'target' after vblank fires)
> */
> - union {
> - struct intel_pipe_wm ilk;
> - struct skl_pipe_wm skl;
> - } active;
> + union pipe_wm active;
>
> - /* allow CxSR on this pipe */
> - bool cxsr_allowed;
> + /* allow CxSR on this pipe */
> + bool cxsr_allowed;
> } wm;
> };
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c6e735f..7942bb0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2338,6 +2338,24 @@ static void ilk_compute_wm_config(struct drm_device *dev,
> }
> }
>
> +static bool ilk_validate_pipe_wm(struct drm_device *dev,
> + struct intel_wm_config *config,
> + struct intel_pipe_wm *pipe_wm)
> +{
> + struct ilk_wm_maximums max;
> +
> + /* LP0 watermarks always use 1/2 DDB partitioning */
> + ilk_compute_wm_maximums(dev, 0, config, INTEL_DDB_PART_1_2, &max);
> +
> + /* At least LP0 must be valid */
> + if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) {
> + DRM_DEBUG_KMS("LP0 watermark invalid\n");
> + return false;
> + }
> +
> + return true;
> +}
> +
> /* Compute new watermarks for the pipe */
> static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
> struct drm_atomic_state *state)
> @@ -2366,7 +2384,7 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
> else
> cstate = to_intel_crtc_state(cs);
>
> - pipe_wm = &cstate->wm.active.ilk;
> + pipe_wm = &cstate->wm.target.ilk;
>
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> ps = drm_atomic_get_plane_state(state,
> @@ -2405,12 +2423,8 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
> if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);
>
> - /* LP0 watermarks always use 1/2 DDB partitioning */
> - ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max);
> -
> - /* At least LP0 must be valid */
> - if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
> - return -EINVAL;
> + if (!ilk_validate_pipe_wm(dev, &config, pipe_wm))
> + return false;
>
> ilk_compute_wm_reg_maximums(dev, 1, &max);
>
> @@ -2435,6 +2449,40 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
> }
>
> /*
> + * Build a set of 'intermediate' watermark values that satisfy both the old
> + * state and the new state. These can be programmed to the hardware
> + * immediately.
> + */
> +void ilk_compute_intermediate_wm(struct drm_device *dev,
> + struct intel_crtc_state *newstate,
> + const struct intel_crtc_state *oldstate)
> +{
> + struct intel_pipe_wm *a = &newstate->wm.active.ilk;
> + const struct intel_pipe_wm *b = &oldstate->wm.active.ilk;
> + int level, max_level = ilk_wm_max_level(dev);
> +
> + /*
> + * Start with the final, target watermarks, then combine with the
> + * current state's watermarks.
> + */
> + *a = newstate->wm.target.ilk;
> + a->pipe_enabled |= b->pipe_enabled;
> + a->sprites_enabled |= b->sprites_enabled;
> + a->sprites_scaled |= b->sprites_scaled;
> +
> + for (level = 0; level <= max_level; level++) {
> + struct intel_wm_level *a_wm = &a->wm[level];
> + const struct intel_wm_level *b_wm = &b->wm[level];
> +
> + a_wm->enable &= b_wm->enable;
> + a_wm->pri_val = max(a_wm->pri_val, b_wm->pri_val);
> + a_wm->spr_val = max(a_wm->spr_val, b_wm->spr_val);
> + a_wm->cur_val = max(a_wm->cur_val, b_wm->cur_val);
> + a_wm->fbc_val = max(a_wm->fbc_val, b_wm->fbc_val);
> + }
> +}
> +
> +/*
> * Merge the watermarks from all active pipes for a specific level.
> */
> static void ilk_merge_wm_level(struct drm_device *dev,
> @@ -3601,10 +3649,10 @@ static bool skl_update_pipe_wm(struct drm_crtc *crtc,
> skl_allocate_pipe_ddb(crtc, config, params, ddb);
> skl_compute_pipe_wm(crtc, ddb, params, pipe_wm);
>
> - if (!memcmp(&cstate->wm.active.skl, pipe_wm, sizeof(*pipe_wm)))
> + if (!memcmp(&cstate->wm.target.skl, pipe_wm, sizeof(*pipe_wm)))
> return false;
>
> - cstate->wm.active.skl = *pipe_wm;
> + cstate->wm.target.skl = *pipe_wm;
>
> return true;
> }
> @@ -3825,7 +3873,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> - struct skl_pipe_wm *active = &cstate->wm.active.skl;
> + struct skl_pipe_wm *active = &cstate->wm.target.skl;
> enum pipe pipe = intel_crtc->pipe;
> int level, i, max_level;
> uint32_t temp;
> @@ -3889,7 +3937,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> struct ilk_wm_values *hw = &dev_priv->wm.hw;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> - struct intel_pipe_wm *active = &cstate->wm.active.ilk;
> + struct intel_pipe_wm *active = &cstate->wm.target.ilk;
> enum pipe pipe = intel_crtc->pipe;
> static const unsigned int wm0_pipe_reg[] = {
> [PIPE_A] = WM0_PIPEA_ILK,
> @@ -3928,6 +3976,8 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> for (level = 0; level <= max_level; level++)
> active->wm[level].enable = true;
> }
> +
> + cstate->wm.active.ilk = cstate->wm.target.ilk = *active;
> }
>
> #define _FW_WM(value, plane) \
> @@ -7048,6 +7098,9 @@ void intel_init_pm(struct drm_device *dev)
> dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
> dev_priv->display.update_wm = ilk_update_wm;
> dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm;
> + dev_priv->display.compute_intermediate_wm =
> + ilk_compute_intermediate_wm;
> + dev_priv->display.program_watermarks = ilk_program_watermarks;
> } else {
> DRM_DEBUG_KMS("Failed to read display plane latency. "
> "Disable CxSR\n");
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 8/8] drm/i915: Add two-stage ILK-style watermark programming (v2)
2015-07-02 2:26 ` [RFC 8/8] drm/i915: Add two-stage ILK-style watermark programming (v2) Matt Roper
2015-07-06 9:11 ` Daniel Vetter
@ 2015-07-06 12:20 ` Maarten Lankhorst
2015-07-06 12:26 ` Daniel Vetter
1 sibling, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2015-07-06 12:20 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Op 02-07-15 om 04:26 schreef Matt Roper:
> From: Matt Roper <matt@mattrope.com>
>
> In addition to calculating final watermarks, let's also pre-calculate a
> set of intermediate watermark values at atomic check time. These
> intermediate watermarks are a combination of the watermarks for the old
> state and the new state; they should satisfy the requirements of both
> states which means they can be programmed immediately when we commit the
> atomic state (without waiting for a vblank). Once the vblank does
> happen, we can then re-program watermarks to the more optimal final
> value.
>
> v2: Significant rebasing/rewriting.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 9 +++++
> drivers/gpu/drm/i915/i915_irq.c | 7 ++++
> drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++++-
> drivers/gpu/drm/i915/intel_drv.h | 26 +++++++++----
> drivers/gpu/drm/i915/intel_pm.c | 75 ++++++++++++++++++++++++++++++------
> 5 files changed, 130 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5ad942e..42397e2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -623,6 +623,9 @@ struct drm_i915_display_funcs {
> struct dpll *best_clock);
> int (*compute_pipe_wm)(struct drm_crtc *crtc,
> struct drm_atomic_state *state);
> + void (*compute_intermediate_wm)(struct drm_device *dev,
> + struct intel_crtc_state *newstate,
> + const struct intel_crtc_state *oldstate);
>
If this is can't fail anyway could we please do this at runtime instead of precalculating?
Something like this:
commit_intermediate_wm
vblank_evade
plane_update
vblank_end
(wait for vblank)
commit_final_wm
Of course for skylake just hammer in the final wm during vblank evasion. :-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC 8/8] drm/i915: Add two-stage ILK-style watermark programming (v2)
2015-07-06 12:20 ` Maarten Lankhorst
@ 2015-07-06 12:26 ` Daniel Vetter
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2015-07-06 12:26 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, Jul 06, 2015 at 02:20:00PM +0200, Maarten Lankhorst wrote:
> Op 02-07-15 om 04:26 schreef Matt Roper:
> > From: Matt Roper <matt@mattrope.com>
> >
> > In addition to calculating final watermarks, let's also pre-calculate a
> > set of intermediate watermark values at atomic check time. These
> > intermediate watermarks are a combination of the watermarks for the old
> > state and the new state; they should satisfy the requirements of both
> > states which means they can be programmed immediately when we commit the
> > atomic state (without waiting for a vblank). Once the vblank does
> > happen, we can then re-program watermarks to the more optimal final
> > value.
> >
> > v2: Significant rebasing/rewriting.
> >
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 9 +++++
> > drivers/gpu/drm/i915/i915_irq.c | 7 ++++
> > drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++++-
> > drivers/gpu/drm/i915/intel_drv.h | 26 +++++++++----
> > drivers/gpu/drm/i915/intel_pm.c | 75 ++++++++++++++++++++++++++++++------
> > 5 files changed, 130 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 5ad942e..42397e2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -623,6 +623,9 @@ struct drm_i915_display_funcs {
> > struct dpll *best_clock);
> > int (*compute_pipe_wm)(struct drm_crtc *crtc,
> > struct drm_atomic_state *state);
> > + void (*compute_intermediate_wm)(struct drm_device *dev,
> > + struct intel_crtc_state *newstate,
> > + const struct intel_crtc_state *oldstate);
> >
> If this is can't fail anyway could we please do this at runtime instead of precalculating?
Hm, this is possible to fail, we might not be able to get valid
intermediate wms. And therefore it needs to be in the compute phase.
-Daniel
>
> Something like this:
>
> commit_intermediate_wm
> vblank_evade
> plane_update
> vblank_end
> (wait for vblank)
> commit_final_wm
>
> Of course for skylake just hammer in the final wm during vblank evasion. :-)
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread