* [PATCH 01/14] drm/i915: Drop redundant watermark programming
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 02/14] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2) Matt Roper
` (12 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
In commit
commit e4ca061275ec6a48b66c6edebe08644e666994c0
Author: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Date: Wed Jul 8 15:31:52 2015 +0200
drm/i915: Don't forget to mark crtc as inactive after disable
we added extra watermark updates to all of the .crtc_disable()
entrypoints to avoid problems problems with system resume on SKL. Those
disable entrypoints are currently called in just two places in the
driver: intel_atomic_commit (i.e., during a modeset) and
intel_crtc_disable_noatomic (which is called during hardware readout).
It seems that this extra watermark recalculation should only be
important in the latter case (which happens during a resume operation);
the former case should always have appropriate watermark programming
happening at other points in the modeset sequence.
Let's move the watermark update out of the .crtc_disable() entrypoints
and place it directly in intel_crtc_disable_noatomic() so that it only
happens on S3 resume and not during a regular modeset (since the
existing watermark handling should properly update watermarks during
normal atomic commits).
Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fc00867..d144d29 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5031,9 +5031,6 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
ironlake_fdi_pll_disable(intel_crtc);
}
-
- intel_crtc->active = false;
- intel_update_watermarks(crtc);
}
static void haswell_crtc_disable(struct drm_crtc *crtc)
@@ -5077,9 +5074,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
for_each_encoder_on_crtc(dev, crtc, encoder)
if (encoder->post_disable)
encoder->post_disable(encoder);
-
- intel_crtc->active = false;
- intel_update_watermarks(crtc);
}
static void i9xx_pfit_enable(struct intel_crtc *crtc)
@@ -6178,9 +6172,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
if (!IS_GEN2(dev))
intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
-
- intel_crtc->active = false;
- intel_update_watermarks(crtc);
}
static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
@@ -6200,6 +6191,8 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
intel_crtc_disable_planes(crtc, crtc->state->plane_mask);
dev_priv->display.crtc_disable(crtc);
+ intel_crtc->active = false;
+ intel_update_watermarks(crtc);
intel_disable_shared_dpll(intel_crtc);
domains = intel_crtc->enabled_power_domains;
--
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] 20+ messages in thread* [PATCH 02/14] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2)
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
2015-09-15 2:19 ` [PATCH 01/14] drm/i915: Drop redundant watermark programming Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 03/14] drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM (v2) Matt Roper
` (11 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
Just pull the info out of the plane state structure rather than staging
it in an additional structure.
v2: Add 'visible' condition to sprites_scaled so that we don't limit the
WM level when the sprite isn't enabled. (Ville)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by(v1): Ander Conselvan de Oliveira <conselvan2@gmail.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 62de97e..442a1f6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1799,9 +1799,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 {
@@ -1823,25 +1820,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);
@@ -1852,20 +1849,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);
}
@@ -1875,28 +1872,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)
@@ -2061,10 +2062,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];
@@ -2076,10 +2079,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;
}
@@ -2341,10 +2363,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;
@@ -2352,32 +2371,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) {
- 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,
@@ -2405,28 +2398,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 = sprstate->visible &&
+ (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);
@@ -2443,7 +2450,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] 20+ messages in thread* [PATCH 03/14] drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM (v2)
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
2015-09-15 2:19 ` [PATCH 01/14] drm/i915: Drop redundant watermark programming Matt Roper
2015-09-15 2:19 ` [PATCH 02/14] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code (v2) Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 04/14] drm/i915: Determine I915_MAX_PLANES from plane enum Matt Roper
` (10 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
Just pull the info out of the CRTC state structure rather than staging
it in an additional structure.
Note that we use cstate->active rather than intel_crtc->active which may
appear to be a change in behavior. However since we're no longer trying
to recalculate watermarks during the "pipe off" stage of a modeset,
intel_crtc->active and cstate->active should always be identical when
watermarks are calculated (at least for ILK-style platforms).
v2: Clarify reasoning for cstate->active and add a WARN_ON to the code
to assert that it really is always identical to intel_crtc->active
as expected.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 84 ++++++++++++++---------------------------
1 file changed, 29 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 442a1f6..0caabce 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1795,12 +1795,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;
@@ -1819,7 +1813,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)
@@ -1827,16 +1821,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);
@@ -1848,19 +1842,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);
@@ -1871,30 +1865,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);
@@ -2064,7 +2058,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;
@@ -2085,18 +2079,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;
}
@@ -2360,19 +2354,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)
{
@@ -2392,10 +2373,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);
@@ -2420,8 +2401,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;
@@ -2433,7 +2413,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);
@@ -2450,7 +2430,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
@@ -3757,19 +3737,19 @@ 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);
+ WARN_ON(cstate->base.active != intel_crtc->active);
- 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;
@@ -3809,12 +3789,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] 20+ messages in thread* [PATCH 04/14] drm/i915: Determine I915_MAX_PLANES from plane enum
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (2 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 03/14] drm/i915: Eliminate usage of pipe_wm_parameters from ILK-style WM (v2) Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 05/14] drm/i915/skl: Simplify wm structures slightly (v2) Matt Roper
` (9 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
Let the compiler figure out what I915_MAX_PLANES is from 'enum plane' so
that we don't need a separate #define.
While we're at it, add the cursor plane to the enum. This will cause
I915_MAX_PLANES to now include the cursor plane in its count (it didn't
previously). This change is safe since we currently only use this
value in array declarations (never in the actual code logic); we just
wind up allocating slightly more memory than we need to. A followup
patch will cause various parts of the code to start using the extra
array element where appropriate.
(This patch probably should have been squashed with the followup patch,
but I couldn't figure out how to get Coccinelle to modify enum
declarations...)
Suggested-by: Ander Conselvan De Oliveira <conselvan2@gmail.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3bf8a9b..c800031 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -131,17 +131,17 @@ enum transcoder {
#define transcoder_name(t) ((t) + 'A')
/*
- * This is the maximum (across all platforms) number of planes (primary +
- * sprites) that can be active at the same time on one pipe.
- *
- * This value doesn't count the cursor plane.
+ * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
+ * number of planes per CRTC. Not all platforms really have this many planes,
+ * which means some arrays of size I915_MAX_PLANES may have unused entries
+ * between the topmost sprite plane and the cursor plane.
*/
-#define I915_MAX_PLANES 4
-
enum plane {
PLANE_A = 0,
PLANE_B,
PLANE_C,
+ PLANE_CURSOR,
+ I915_MAX_PLANES,
};
#define plane_name(p) ((p) + 'A')
--
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] 20+ messages in thread* [PATCH 05/14] drm/i915/skl: Simplify wm structures slightly (v2)
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (3 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 04/14] drm/i915: Determine I915_MAX_PLANES from plane enum Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 06/14] drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v2) Matt Roper
` (8 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
A bunch of SKL watermark-related structures have the cursor plane as a
separate entry from the rest of the planes. Since a previous patch
updated I915_MAX_PLANES such that those plane arrays now have a slot for
the cursor, update the code to use the new slot in the existing plane
arrays and kill off the cursor-specific structures.
There shouldn't be any functional change here; this is just shuffling
around how the data is stored in some of the data structures. The whole
patch is generated with Coccinelle via the following semantic patch:
@@ struct skl_pipe_wm_parameters WMP; @@
- WMP.cursor
+ WMP.plane[PLANE_CURSOR]
@@ struct skl_pipe_wm_parameters *WMP; @@
- WMP->cursor
+ WMP->plane[PLANE_CURSOR]
@@ @@
struct skl_pipe_wm_parameters {
...
- struct intel_plane_wm_parameters cursor;
...
};
@@
struct skl_ddb_allocation DDB;
expression E;
@@
- DDB.cursor[E]
+ DDB.plane[E][PLANE_CURSOR]
@@
struct skl_ddb_allocation *DDB;
expression E;
@@
- DDB->cursor[E]
+ DDB->plane[E][PLANE_CURSOR]
@@ @@
struct skl_ddb_allocation {
...
- struct skl_ddb_entry cursor[I915_MAX_PIPES];
...
};
@@
struct skl_wm_values WMV;
expression E1, E2;
@@
(
- WMV.cursor[E1][E2]
+ WMV.plane[E1][PLANE_CURSOR][E2]
|
- WMV.cursor_trans[E1]
+ WMV.plane_trans[E1][PLANE_CURSOR]
)
@@
struct skl_wm_values *WMV;
expression E1, E2;
@@
(
- WMV->cursor[E1][E2]
+ WMV->plane[E1][PLANE_CURSOR][E2]
|
- WMV->cursor_trans[E1]
+ WMV->plane_trans[E1][PLANE_CURSOR]
)
@@ @@
struct skl_wm_values {
...
- uint32_t cursor[I915_MAX_PIPES][8];
...
- uint32_t cursor_trans[I915_MAX_PIPES];
...
};
@@ struct skl_wm_level WML; @@
(
- WML.cursor_en
+ WML.plane_en[PLANE_CURSOR]
|
- WML.cursor_res_b
+ WML.plane_res_b[PLANE_CURSOR]
|
- WML.cursor_res_l
+ WML.plane_res_l[PLANE_CURSOR]
)
@@ struct skl_wm_level *WML; @@
(
- WML->cursor_en
+ WML->plane_en[PLANE_CURSOR]
|
- WML->cursor_res_b
+ WML->plane_res_b[PLANE_CURSOR]
|
- WML->cursor_res_l
+ WML->plane_res_l[PLANE_CURSOR]
)
@@ @@
struct skl_wm_level {
...
- bool cursor_en;
...
- uint16_t cursor_res_b;
- uint8_t cursor_res_l;
...
};
v2: Use a PLANE_CURSOR enum entry rather than making the code reference
I915_MAX_PLANES or I915_MAX_PLANES+1, which was confusing. (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 8 +---
drivers/gpu/drm/i915/intel_display.c | 4 +-
drivers/gpu/drm/i915/intel_pm.c | 87 +++++++++++++++++++-----------------
4 files changed, 49 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 72ae347..52ee2a5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3144,7 +3144,7 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
skl_ddb_entry_size(entry));
}
- entry = &ddb->cursor[pipe];
+ entry = &ddb->plane[pipe][PLANE_CURSOR];
seq_printf(m, " %-13s%8u%8u%8u\n", "Cursor", entry->start,
entry->end, skl_ddb_entry_size(entry));
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c800031..539bfa2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1585,8 +1585,7 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1,
struct skl_ddb_allocation {
struct skl_ddb_entry pipe[I915_MAX_PIPES];
struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; /* packed/uv */
- struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES]; /* y-plane */
- struct skl_ddb_entry cursor[I915_MAX_PIPES];
+ struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
};
struct skl_wm_values {
@@ -1594,18 +1593,13 @@ struct skl_wm_values {
struct skl_ddb_allocation ddb;
uint32_t wm_linetime[I915_MAX_PIPES];
uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
- uint32_t cursor[I915_MAX_PIPES][8];
uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES];
- uint32_t cursor_trans[I915_MAX_PIPES];
};
struct skl_wm_level {
bool plane_en[I915_MAX_PLANES];
- bool cursor_en;
uint16_t plane_res_b[I915_MAX_PLANES];
uint8_t plane_res_l[I915_MAX_PLANES];
- uint16_t cursor_res_b;
- uint8_t cursor_res_l;
};
/*
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d144d29..017641a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12527,8 +12527,8 @@ static void check_wm_state(struct drm_device *dev)
}
/* cursor */
- hw_entry = &hw_ddb.cursor[pipe];
- sw_entry = &sw_ddb->cursor[pipe];
+ hw_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
+ sw_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
if (skl_ddb_entry_equal(hw_entry, sw_entry))
continue;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0caabce..7115368 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1792,7 +1792,6 @@ struct skl_pipe_wm_parameters {
uint32_t pipe_htotal;
uint32_t pixel_rate; /* in KHz */
struct intel_plane_wm_parameters plane[I915_MAX_PLANES];
- struct intel_plane_wm_parameters cursor;
};
struct ilk_wm_maximums {
@@ -2906,7 +2905,8 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
}
val = I915_READ(CUR_BUF_CFG(pipe));
- skl_ddb_entry_init_from_hw(&ddb->cursor[pipe], val);
+ skl_ddb_entry_init_from_hw(&ddb->plane[pipe][PLANE_CURSOR],
+ val);
}
}
@@ -2975,13 +2975,14 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
alloc_size = skl_ddb_entry_size(alloc);
if (alloc_size == 0) {
memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
- memset(&ddb->cursor[pipe], 0, sizeof(ddb->cursor[pipe]));
+ memset(&ddb->plane[pipe][PLANE_CURSOR], 0,
+ sizeof(ddb->plane[pipe][PLANE_CURSOR]));
return;
}
cursor_blocks = skl_cursor_allocation(config);
- ddb->cursor[pipe].start = alloc->end - cursor_blocks;
- ddb->cursor[pipe].end = alloc->end;
+ ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - cursor_blocks;
+ ddb->plane[pipe][PLANE_CURSOR].end = alloc->end;
alloc_size -= cursor_blocks;
alloc->end -= cursor_blocks;
@@ -3120,8 +3121,8 @@ static bool skl_ddb_allocation_changed(const struct skl_ddb_allocation *new_ddb,
sizeof(new_ddb->plane[pipe])))
return true;
- if (memcmp(&new_ddb->cursor[pipe], &cur_ddb->cursor[pipe],
- sizeof(new_ddb->cursor[pipe])))
+ if (memcmp(&new_ddb->plane[pipe][PLANE_CURSOR], &cur_ddb->plane[pipe][PLANE_CURSOR],
+ sizeof(new_ddb->plane[pipe][PLANE_CURSOR])))
return true;
return false;
@@ -3181,17 +3182,17 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
p->plane[0].rotation = crtc->primary->state->rotation;
fb = crtc->cursor->state->fb;
- p->cursor.y_bytes_per_pixel = 0;
+ p->plane[PLANE_CURSOR].y_bytes_per_pixel = 0;
if (fb) {
- p->cursor.enabled = true;
- p->cursor.bytes_per_pixel = fb->bits_per_pixel / 8;
- p->cursor.horiz_pixels = crtc->cursor->state->crtc_w;
- p->cursor.vert_pixels = crtc->cursor->state->crtc_h;
+ p->plane[PLANE_CURSOR].enabled = true;
+ p->plane[PLANE_CURSOR].bytes_per_pixel = fb->bits_per_pixel / 8;
+ p->plane[PLANE_CURSOR].horiz_pixels = crtc->cursor->state->crtc_w;
+ p->plane[PLANE_CURSOR].vert_pixels = crtc->cursor->state->crtc_h;
} else {
- p->cursor.enabled = false;
- p->cursor.bytes_per_pixel = 0;
- p->cursor.horiz_pixels = 64;
- p->cursor.vert_pixels = 64;
+ p->plane[PLANE_CURSOR].enabled = false;
+ p->plane[PLANE_CURSOR].bytes_per_pixel = 0;
+ p->plane[PLANE_CURSOR].horiz_pixels = 64;
+ p->plane[PLANE_CURSOR].vert_pixels = 64;
}
}
@@ -3305,11 +3306,12 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
&result->plane_res_l[i]);
}
- ddb_blocks = skl_ddb_entry_size(&ddb->cursor[pipe]);
- result->cursor_en = skl_compute_plane_wm(dev_priv, p, &p->cursor,
+ ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][PLANE_CURSOR]);
+ result->plane_en[PLANE_CURSOR] = skl_compute_plane_wm(dev_priv, p,
+ &p->plane[PLANE_CURSOR],
ddb_blocks, level,
- &result->cursor_res_b,
- &result->cursor_res_l);
+ &result->plane_res_b[PLANE_CURSOR],
+ &result->plane_res_l[PLANE_CURSOR]);
}
static uint32_t
@@ -3337,7 +3339,7 @@ static void skl_compute_transition_wm(struct drm_crtc *crtc,
/* Until we know more, just disable transition WMs */
for (i = 0; i < intel_num_planes(intel_crtc); i++)
trans_wm->plane_en[i] = false;
- trans_wm->cursor_en = false;
+ trans_wm->plane_en[PLANE_CURSOR] = false;
}
static void skl_compute_pipe_wm(struct drm_crtc *crtc,
@@ -3386,13 +3388,13 @@ static void skl_compute_wm_results(struct drm_device *dev,
temp = 0;
- temp |= p_wm->wm[level].cursor_res_l << PLANE_WM_LINES_SHIFT;
- temp |= p_wm->wm[level].cursor_res_b;
+ temp |= p_wm->wm[level].plane_res_l[PLANE_CURSOR] << PLANE_WM_LINES_SHIFT;
+ temp |= p_wm->wm[level].plane_res_b[PLANE_CURSOR];
- if (p_wm->wm[level].cursor_en)
+ if (p_wm->wm[level].plane_en[PLANE_CURSOR])
temp |= PLANE_WM_EN;
- r->cursor[pipe][level] = temp;
+ r->plane[pipe][PLANE_CURSOR][level] = temp;
}
@@ -3408,12 +3410,12 @@ static void skl_compute_wm_results(struct drm_device *dev,
}
temp = 0;
- temp |= p_wm->trans_wm.cursor_res_l << PLANE_WM_LINES_SHIFT;
- temp |= p_wm->trans_wm.cursor_res_b;
- if (p_wm->trans_wm.cursor_en)
+ temp |= p_wm->trans_wm.plane_res_l[PLANE_CURSOR] << PLANE_WM_LINES_SHIFT;
+ temp |= p_wm->trans_wm.plane_res_b[PLANE_CURSOR];
+ if (p_wm->trans_wm.plane_en[PLANE_CURSOR])
temp |= PLANE_WM_EN;
- r->cursor_trans[pipe] = temp;
+ r->plane_trans[pipe][PLANE_CURSOR] = temp;
r->wm_linetime[pipe] = p_wm->linetime;
}
@@ -3447,12 +3449,13 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
I915_WRITE(PLANE_WM(pipe, i, level),
new->plane[pipe][i][level]);
I915_WRITE(CUR_WM(pipe, level),
- new->cursor[pipe][level]);
+ new->plane[pipe][PLANE_CURSOR][level]);
}
for (i = 0; i < intel_num_planes(crtc); i++)
I915_WRITE(PLANE_WM_TRANS(pipe, i),
new->plane_trans[pipe][i]);
- I915_WRITE(CUR_WM_TRANS(pipe), new->cursor_trans[pipe]);
+ I915_WRITE(CUR_WM_TRANS(pipe),
+ new->plane_trans[pipe][PLANE_CURSOR]);
for (i = 0; i < intel_num_planes(crtc); i++) {
skl_ddb_entry_write(dev_priv,
@@ -3464,7 +3467,7 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
}
skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe),
- &new->ddb.cursor[pipe]);
+ &new->ddb.plane[pipe][PLANE_CURSOR]);
}
}
@@ -3820,10 +3823,10 @@ static void skl_pipe_wm_active_state(uint32_t val,
(val >> PLANE_WM_LINES_SHIFT) &
PLANE_WM_LINES_MASK;
} else {
- active->wm[level].cursor_en = is_enabled;
- active->wm[level].cursor_res_b =
+ active->wm[level].plane_en[PLANE_CURSOR] = is_enabled;
+ active->wm[level].plane_res_b[PLANE_CURSOR] =
val & PLANE_WM_BLOCKS_MASK;
- active->wm[level].cursor_res_l =
+ active->wm[level].plane_res_l[PLANE_CURSOR] =
(val >> PLANE_WM_LINES_SHIFT) &
PLANE_WM_LINES_MASK;
}
@@ -3836,10 +3839,10 @@ static void skl_pipe_wm_active_state(uint32_t val,
(val >> PLANE_WM_LINES_SHIFT) &
PLANE_WM_LINES_MASK;
} else {
- active->trans_wm.cursor_en = is_enabled;
- active->trans_wm.cursor_res_b =
+ active->trans_wm.plane_en[PLANE_CURSOR] = is_enabled;
+ active->trans_wm.plane_res_b[PLANE_CURSOR] =
val & PLANE_WM_BLOCKS_MASK;
- active->trans_wm.cursor_res_l =
+ active->trans_wm.plane_res_l[PLANE_CURSOR] =
(val >> PLANE_WM_LINES_SHIFT) &
PLANE_WM_LINES_MASK;
}
@@ -3865,12 +3868,12 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
for (i = 0; i < intel_num_planes(intel_crtc); i++)
hw->plane[pipe][i][level] =
I915_READ(PLANE_WM(pipe, i, level));
- hw->cursor[pipe][level] = I915_READ(CUR_WM(pipe, level));
+ hw->plane[pipe][PLANE_CURSOR][level] = I915_READ(CUR_WM(pipe, level));
}
for (i = 0; i < intel_num_planes(intel_crtc); i++)
hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i));
- hw->cursor_trans[pipe] = I915_READ(CUR_WM_TRANS(pipe));
+ hw->plane_trans[pipe][PLANE_CURSOR] = I915_READ(CUR_WM_TRANS(pipe));
if (!intel_crtc->active)
return;
@@ -3885,7 +3888,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
skl_pipe_wm_active_state(temp, active, false,
false, i, level);
}
- temp = hw->cursor[pipe][level];
+ temp = hw->plane[pipe][PLANE_CURSOR][level];
skl_pipe_wm_active_state(temp, active, false, true, i, level);
}
@@ -3894,7 +3897,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
skl_pipe_wm_active_state(temp, active, true, false, i, 0);
}
- temp = hw->cursor_trans[pipe];
+ temp = hw->plane_trans[pipe][PLANE_CURSOR];
skl_pipe_wm_active_state(temp, active, true, true, i, 0);
}
--
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] 20+ messages in thread* [PATCH 06/14] drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v2)
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (4 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 05/14] drm/i915/skl: Simplify wm structures slightly (v2) Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-17 13:24 ` Maarten Lankhorst
2015-09-15 2:19 ` [PATCH 07/14] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check Matt Roper
` (7 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
Just pull the info out of the state structures rather than staging
it in an additional set of structures. To make this more
straightforward, we change the signature of several internal WM
functions to take the crtc state as a parameter.
v2:
- Don't forget to skip cursor planes on a loop in the DDB allocation
function to match original behavior. (Ander)
- Change a use of intel_crtc->active to cstate->active. They should
be identical, but it's better to be consistent. (Ander)
- Rework more function signatures to pass states rather than crtc for
consistency. (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 326 +++++++++++++++++++---------------------
1 file changed, 151 insertions(+), 175 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7115368..51dbda7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1787,13 +1787,6 @@ static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels,
return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2;
}
-struct skl_pipe_wm_parameters {
- bool active;
- uint32_t pipe_htotal;
- uint32_t pixel_rate; /* in KHz */
- struct intel_plane_wm_parameters plane[I915_MAX_PLANES];
-};
-
struct ilk_wm_maximums {
uint16_t pri;
uint16_t spr;
@@ -2834,18 +2827,40 @@ static bool ilk_disable_lp_wm(struct drm_device *dev)
#define SKL_DDB_SIZE 896 /* in blocks */
#define BXT_DDB_SIZE 512
+/*
+ * Return the index of a plane in the SKL DDB and wm result arrays. Primary
+ * plane is always in slot 0, cursor is always in slot I915_MAX_PLANES-1, and
+ * other universal planes are in indices 1..n. Note that this may leave unused
+ * indices between the top "sprite" plane and the cursor.
+ */
+static int
+skl_wm_plane_id(const struct intel_plane *plane)
+{
+ switch (plane->base.type) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ return 0;
+ case DRM_PLANE_TYPE_CURSOR:
+ return PLANE_CURSOR;
+ case DRM_PLANE_TYPE_OVERLAY:
+ return plane->plane;
+ default:
+ MISSING_CASE(plane->base.type);
+ return plane->plane;
+ }
+}
+
static void
skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
- struct drm_crtc *for_crtc,
+ const struct intel_crtc_state *cstate,
const struct intel_wm_config *config,
- const struct skl_pipe_wm_parameters *params,
struct skl_ddb_entry *alloc /* out */)
{
+ struct drm_crtc *for_crtc = cstate->base.crtc;
struct drm_crtc *crtc;
unsigned int pipe_size, ddb_size;
int nth_active_pipe;
- if (!params->active) {
+ if (!cstate->base.active) {
alloc->start = 0;
alloc->end = 0;
return;
@@ -2911,19 +2926,29 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
}
static unsigned int
-skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
+skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
+ const struct drm_plane_state *pstate,
+ int y)
{
+ struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
+ struct drm_framebuffer *fb = pstate->fb;
/* for planar format */
- if (p->y_bytes_per_pixel) {
+ if (fb->pixel_format == DRM_FORMAT_NV12) {
if (y) /* y-plane data rate */
- return p->horiz_pixels * p->vert_pixels * p->y_bytes_per_pixel;
+ return intel_crtc->config->pipe_src_w *
+ intel_crtc->config->pipe_src_h *
+ drm_format_plane_cpp(fb->pixel_format, 0);
else /* uv-plane data rate */
- return (p->horiz_pixels/2) * (p->vert_pixels/2) * p->bytes_per_pixel;
+ return (intel_crtc->config->pipe_src_w/2) *
+ (intel_crtc->config->pipe_src_h/2) *
+ drm_format_plane_cpp(fb->pixel_format, 1);
}
/* for packed formats */
- return p->horiz_pixels * p->vert_pixels * p->bytes_per_pixel;
+ return intel_crtc->config->pipe_src_w *
+ intel_crtc->config->pipe_src_h *
+ drm_format_plane_cpp(fb->pixel_format, 1);
}
/*
@@ -2932,46 +2957,51 @@ skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
* 3 * 4096 * 8192 * 4 < 2^32
*/
static unsigned int
-skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
- const struct skl_pipe_wm_parameters *params)
+skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
{
+ struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
+ struct drm_device *dev = intel_crtc->base.dev;
+ const struct intel_plane *intel_plane;
unsigned int total_data_rate = 0;
- int plane;
- for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
- const struct intel_plane_wm_parameters *p;
+ for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+ const struct drm_plane_state *pstate = intel_plane->base.state;
- p = ¶ms->plane[plane];
- if (!p->enabled)
+ if (WARN_ON(pstate->fb == NULL))
continue;
- total_data_rate += skl_plane_relative_data_rate(p, 0); /* packed/uv */
- if (p->y_bytes_per_pixel) {
- total_data_rate += skl_plane_relative_data_rate(p, 1); /* y-plane */
- }
+ /* packed/uv */
+ total_data_rate += skl_plane_relative_data_rate(cstate,
+ pstate,
+ 0);
+
+ if (pstate->fb->pixel_format == DRM_FORMAT_NV12)
+ /* y-plane */
+ total_data_rate += skl_plane_relative_data_rate(cstate,
+ pstate,
+ 1);
}
return total_data_rate;
}
static void
-skl_allocate_pipe_ddb(struct drm_crtc *crtc,
+skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
const struct intel_wm_config *config,
- const struct skl_pipe_wm_parameters *params,
struct skl_ddb_allocation *ddb /* out */)
{
+ struct drm_crtc *crtc = cstate->base.crtc;
struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_plane *intel_plane;
enum pipe pipe = intel_crtc->pipe;
struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
uint16_t alloc_size, start, cursor_blocks;
uint16_t minimum[I915_MAX_PLANES];
uint16_t y_minimum[I915_MAX_PLANES];
unsigned int total_data_rate;
- int plane;
- skl_ddb_get_pipe_allocation_limits(dev, crtc, config, params, alloc);
+ skl_ddb_get_pipe_allocation_limits(dev, cstate, config, alloc);
alloc_size = skl_ddb_entry_size(alloc);
if (alloc_size == 0) {
memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
@@ -2988,17 +3018,20 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
alloc->end -= cursor_blocks;
/* 1. Allocate the mininum required blocks for each active plane */
- for_each_plane(dev_priv, pipe, plane) {
- const struct intel_plane_wm_parameters *p;
+ for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+ struct drm_plane *plane = &intel_plane->base;
+ struct drm_framebuffer *fb = plane->fb;
+ int id = skl_wm_plane_id(intel_plane);
- p = ¶ms->plane[plane];
- if (!p->enabled)
+ if (fb == NULL)
+ continue;
+ if (plane->type == DRM_PLANE_TYPE_CURSOR)
continue;
- minimum[plane] = 8;
- alloc_size -= minimum[plane];
- y_minimum[plane] = p->y_bytes_per_pixel ? 8 : 0;
- alloc_size -= y_minimum[plane];
+ minimum[id] = 8;
+ alloc_size -= minimum[id];
+ y_minimum[id] = (fb->pixel_format == DRM_FORMAT_NV12) ? 8 : 0;
+ alloc_size -= y_minimum[id];
}
/*
@@ -3007,45 +3040,50 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
*
* FIXME: we may not allocate every single block here.
*/
- total_data_rate = skl_get_total_relative_data_rate(intel_crtc, params);
+ total_data_rate = skl_get_total_relative_data_rate(cstate);
start = alloc->start;
- for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
- const struct intel_plane_wm_parameters *p;
+ for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+ struct drm_plane *plane = &intel_plane->base;
+ struct drm_plane_state *pstate = intel_plane->base.state;
unsigned int data_rate, y_data_rate;
uint16_t plane_blocks, y_plane_blocks = 0;
+ int id = skl_wm_plane_id(intel_plane);
- p = ¶ms->plane[plane];
- if (!p->enabled)
+ if (pstate->fb == NULL)
+ continue;
+ if (plane->type == DRM_PLANE_TYPE_CURSOR)
continue;
- data_rate = skl_plane_relative_data_rate(p, 0);
+ data_rate = skl_plane_relative_data_rate(cstate, pstate, 0);
/*
* allocation for (packed formats) or (uv-plane part of planar format):
* promote the expression to 64 bits to avoid overflowing, the
* result is < available as data_rate / total_data_rate < 1
*/
- plane_blocks = minimum[plane];
+ plane_blocks = minimum[id];
plane_blocks += div_u64((uint64_t)alloc_size * data_rate,
total_data_rate);
- ddb->plane[pipe][plane].start = start;
- ddb->plane[pipe][plane].end = start + plane_blocks;
+ ddb->plane[pipe][id].start = start;
+ ddb->plane[pipe][id].end = start + plane_blocks;
start += plane_blocks;
/*
* allocation for y_plane part of planar format:
*/
- if (p->y_bytes_per_pixel) {
- y_data_rate = skl_plane_relative_data_rate(p, 1);
- y_plane_blocks = y_minimum[plane];
+ if (pstate->fb->pixel_format == DRM_FORMAT_NV12) {
+ y_data_rate = skl_plane_relative_data_rate(cstate,
+ pstate,
+ 1);
+ y_plane_blocks = y_minimum[id];
y_plane_blocks += div_u64((uint64_t)alloc_size * y_data_rate,
total_data_rate);
- ddb->y_plane[pipe][plane].start = start;
- ddb->y_plane[pipe][plane].end = start + y_plane_blocks;
+ ddb->y_plane[pipe][id].start = start;
+ ddb->y_plane[pipe][id].end = start + y_plane_blocks;
start += y_plane_blocks;
}
@@ -3146,73 +3184,16 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev,
}
}
-static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
- struct skl_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;
- struct drm_framebuffer *fb;
- int i = 1; /* Index for sprite planes start */
-
- p->active = intel_crtc->active;
- if (p->active) {
- p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
- p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
-
- fb = crtc->primary->state->fb;
- /* For planar: Bpp is for uv plane, y_Bpp is for y plane */
- if (fb) {
- p->plane[0].enabled = true;
- p->plane[0].bytes_per_pixel = fb->pixel_format == DRM_FORMAT_NV12 ?
- drm_format_plane_cpp(fb->pixel_format, 1) :
- drm_format_plane_cpp(fb->pixel_format, 0);
- p->plane[0].y_bytes_per_pixel = fb->pixel_format == DRM_FORMAT_NV12 ?
- drm_format_plane_cpp(fb->pixel_format, 0) : 0;
- p->plane[0].tiling = fb->modifier[0];
- } else {
- p->plane[0].enabled = false;
- p->plane[0].bytes_per_pixel = 0;
- p->plane[0].y_bytes_per_pixel = 0;
- p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
- }
- p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
- p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
- p->plane[0].rotation = crtc->primary->state->rotation;
-
- fb = crtc->cursor->state->fb;
- p->plane[PLANE_CURSOR].y_bytes_per_pixel = 0;
- if (fb) {
- p->plane[PLANE_CURSOR].enabled = true;
- p->plane[PLANE_CURSOR].bytes_per_pixel = fb->bits_per_pixel / 8;
- p->plane[PLANE_CURSOR].horiz_pixels = crtc->cursor->state->crtc_w;
- p->plane[PLANE_CURSOR].vert_pixels = crtc->cursor->state->crtc_h;
- } else {
- p->plane[PLANE_CURSOR].enabled = false;
- p->plane[PLANE_CURSOR].bytes_per_pixel = 0;
- p->plane[PLANE_CURSOR].horiz_pixels = 64;
- p->plane[PLANE_CURSOR].vert_pixels = 64;
- }
- }
-
- list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
- struct intel_plane *intel_plane = to_intel_plane(plane);
-
- if (intel_plane->pipe == pipe &&
- plane->type == DRM_PLANE_TYPE_OVERLAY)
- p->plane[i++] = intel_plane->wm;
- }
-}
-
static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
- struct skl_pipe_wm_parameters *p,
- struct intel_plane_wm_parameters *p_params,
+ struct intel_crtc_state *cstate,
+ struct intel_plane *intel_plane,
uint16_t ddb_allocation,
int level,
uint16_t *out_blocks, /* out */
uint8_t *out_lines /* out */)
{
+ struct drm_plane *plane = &intel_plane->base;
+ struct drm_framebuffer *fb = plane->state->fb;
uint32_t latency = dev_priv->wm.skl_latency[level];
uint32_t method1, method2;
uint32_t plane_bytes_per_line, plane_blocks_per_line;
@@ -3220,31 +3201,35 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
uint32_t selected_result;
uint8_t bytes_per_pixel;
- if (latency == 0 || !p->active || !p_params->enabled)
+ if (latency == 0 || !cstate->base.active || !fb)
return false;
- bytes_per_pixel = p_params->y_bytes_per_pixel ?
- p_params->y_bytes_per_pixel :
- p_params->bytes_per_pixel;
- method1 = skl_wm_method1(p->pixel_rate,
+ bytes_per_pixel = (fb->pixel_format == DRM_FORMAT_NV12) ?
+ drm_format_plane_cpp(DRM_FORMAT_NV12, 0) :
+ drm_format_plane_cpp(DRM_FORMAT_NV12, 1);
+ method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
bytes_per_pixel,
latency);
- method2 = skl_wm_method2(p->pixel_rate,
- p->pipe_htotal,
- p_params->horiz_pixels,
+ method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
+ cstate->base.adjusted_mode.crtc_htotal,
+ cstate->pipe_src_w,
bytes_per_pixel,
- p_params->tiling,
+ fb->modifier[0],
latency);
- plane_bytes_per_line = p_params->horiz_pixels * bytes_per_pixel;
+ plane_bytes_per_line = cstate->pipe_src_w * bytes_per_pixel;
plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
- if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
- p_params->tiling == I915_FORMAT_MOD_Yf_TILED) {
+ if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
+ fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
uint32_t min_scanlines = 4;
uint32_t y_tile_minimum;
- if (intel_rotation_90_or_270(p_params->rotation)) {
- switch (p_params->bytes_per_pixel) {
+ if (intel_rotation_90_or_270(plane->state->rotation)) {
+ int bpp = (fb->pixel_format == DRM_FORMAT_NV12) ?
+ drm_format_plane_cpp(fb->pixel_format, 1) :
+ drm_format_plane_cpp(fb->pixel_format, 0);
+
+ switch (bpp) {
case 1:
min_scanlines = 16;
break;
@@ -3268,8 +3253,8 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
res_lines = DIV_ROUND_UP(selected_result, plane_blocks_per_line);
if (level >= 1 && level <= 7) {
- if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
- p_params->tiling == I915_FORMAT_MOD_Yf_TILED)
+ if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
+ fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)
res_lines += 4;
else
res_blocks++;
@@ -3286,84 +3271,80 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb,
- struct skl_pipe_wm_parameters *p,
- enum pipe pipe,
+ struct intel_crtc_state *cstate,
int level,
- int num_planes,
struct skl_wm_level *result)
{
+ struct drm_device *dev = dev_priv->dev;
+ struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
+ struct intel_plane *intel_plane;
uint16_t ddb_blocks;
- int i;
+ enum pipe pipe = intel_crtc->pipe;
+
+ for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
+ int i = skl_wm_plane_id(intel_plane);
- for (i = 0; i < num_planes; i++) {
ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][i]);
result->plane_en[i] = skl_compute_plane_wm(dev_priv,
- p, &p->plane[i],
+ cstate,
+ intel_plane,
ddb_blocks,
level,
&result->plane_res_b[i],
&result->plane_res_l[i]);
}
-
- ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][PLANE_CURSOR]);
- result->plane_en[PLANE_CURSOR] = skl_compute_plane_wm(dev_priv, p,
- &p->plane[PLANE_CURSOR],
- ddb_blocks, level,
- &result->plane_res_b[PLANE_CURSOR],
- &result->plane_res_l[PLANE_CURSOR]);
}
static uint32_t
-skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
+skl_compute_linetime_wm(struct intel_crtc_state *cstate)
{
- if (!to_intel_crtc(crtc)->active)
+ if (!cstate->base.active)
return 0;
- if (WARN_ON(p->pixel_rate == 0))
+ if (WARN_ON(skl_pipe_pixel_rate(cstate) == 0))
return 0;
- return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
+ return DIV_ROUND_UP(8 * cstate->base.adjusted_mode.crtc_htotal * 1000,
+ skl_pipe_pixel_rate(cstate));
}
-static void skl_compute_transition_wm(struct drm_crtc *crtc,
- struct skl_pipe_wm_parameters *params,
+static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
struct skl_wm_level *trans_wm /* out */)
{
+ struct drm_crtc *crtc = cstate->base.crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int i;
+ struct intel_plane *intel_plane;
- if (!params->active)
+ if (!cstate->base.active)
return;
/* Until we know more, just disable transition WMs */
- for (i = 0; i < intel_num_planes(intel_crtc); i++)
+ for_each_intel_plane_on_crtc(crtc->dev, intel_crtc, intel_plane) {
+ int i = skl_wm_plane_id(intel_plane);
+
trans_wm->plane_en[i] = false;
- trans_wm->plane_en[PLANE_CURSOR] = false;
+ }
}
-static void skl_compute_pipe_wm(struct drm_crtc *crtc,
+static void skl_compute_pipe_wm(struct intel_crtc_state *cstate,
struct skl_ddb_allocation *ddb,
- struct skl_pipe_wm_parameters *params,
struct skl_pipe_wm *pipe_wm)
{
- struct drm_device *dev = crtc->dev;
+ struct drm_device *dev = cstate->base.crtc->dev;
const struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int level, max_level = ilk_wm_max_level(dev);
for (level = 0; level <= max_level; level++) {
- skl_compute_wm_level(dev_priv, ddb, params, intel_crtc->pipe,
- level, intel_num_planes(intel_crtc),
- &pipe_wm->wm[level]);
+ skl_compute_wm_level(dev_priv, ddb, cstate,
+ level, &pipe_wm->wm[level]);
}
- pipe_wm->linetime = skl_compute_linetime_wm(crtc, params);
+ pipe_wm->linetime = skl_compute_linetime_wm(cstate);
- skl_compute_transition_wm(crtc, params, &pipe_wm->trans_wm);
+ skl_compute_transition_wm(cstate, &pipe_wm->trans_wm);
}
static void skl_compute_wm_results(struct drm_device *dev,
- struct skl_pipe_wm_parameters *p,
struct skl_pipe_wm *p_wm,
struct skl_wm_values *r,
struct intel_crtc *intel_crtc)
@@ -3607,16 +3588,15 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
}
static bool skl_update_pipe_wm(struct drm_crtc *crtc,
- struct skl_pipe_wm_parameters *params,
struct intel_wm_config *config,
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);
+ skl_allocate_pipe_ddb(cstate, config, ddb);
+ skl_compute_pipe_wm(cstate, ddb, pipe_wm);
if (!memcmp(&intel_crtc->wm.skl_active, pipe_wm, sizeof(*pipe_wm)))
return false;
@@ -3649,7 +3629,6 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
*/
list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
base.head) {
- struct skl_pipe_wm_parameters params = {};
struct skl_pipe_wm pipe_wm = {};
bool wm_changed;
@@ -3659,8 +3638,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
if (!intel_crtc->active)
continue;
- wm_changed = skl_update_pipe_wm(&intel_crtc->base,
- ¶ms, config,
+ wm_changed = skl_update_pipe_wm(&intel_crtc->base, config,
&r->ddb, &pipe_wm);
/*
@@ -3670,7 +3648,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
*/
WARN_ON(!wm_changed);
- skl_compute_wm_results(dev, ¶ms, &pipe_wm, r, intel_crtc);
+ skl_compute_wm_results(dev, &pipe_wm, r, intel_crtc);
r->dirty[intel_crtc->pipe] = true;
}
}
@@ -3680,7 +3658,6 @@ static void skl_update_wm(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct skl_pipe_wm_parameters params = {};
struct skl_wm_values *results = &dev_priv->wm.skl_results;
struct skl_pipe_wm pipe_wm = {};
struct intel_wm_config config = {};
@@ -3689,11 +3666,10 @@ static void skl_update_wm(struct drm_crtc *crtc)
skl_compute_wm_global_parameters(dev, &config);
- if (!skl_update_pipe_wm(crtc, ¶ms, &config,
- &results->ddb, &pipe_wm))
+ if (!skl_update_pipe_wm(crtc, &config, &results->ddb, &pipe_wm))
return;
- skl_compute_wm_results(dev, ¶ms, &pipe_wm, results, intel_crtc);
+ skl_compute_wm_results(dev, &pipe_wm, results, intel_crtc);
results->dirty[intel_crtc->pipe] = true;
skl_update_other_pipe_wm(dev, crtc, &config, results);
--
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] 20+ messages in thread* Re: [PATCH 06/14] drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v2)
2015-09-15 2:19 ` [PATCH 06/14] drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v2) Matt Roper
@ 2015-09-17 13:24 ` Maarten Lankhorst
0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2015-09-17 13:24 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Hey,
Op 15-09-15 om 04:19 schreef Matt Roper:
> Just pull the info out of the state structures rather than staging
> it in an additional set of structures. To make this more
> straightforward, we change the signature of several internal WM
> functions to take the crtc state as a parameter.
>
> v2:
> - Don't forget to skip cursor planes on a loop in the DDB allocation
> function to match original behavior. (Ander)
> - Change a use of intel_crtc->active to cstate->active. They should
> be identical, but it's better to be consistent. (Ander)
> - Rework more function signatures to pass states rather than crtc for
> consistency. (Ander)
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 326 +++++++++++++++++++---------------------
> 1 file changed, 151 insertions(+), 175 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7115368..51dbda7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1787,13 +1787,6 @@ static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels,
> return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2;
> }
>
> -struct skl_pipe_wm_parameters {
> - bool active;
> - uint32_t pipe_htotal;
> - uint32_t pixel_rate; /* in KHz */
> - struct intel_plane_wm_parameters plane[I915_MAX_PLANES];
> -};
> -
> struct ilk_wm_maximums {
> uint16_t pri;
> uint16_t spr;
> @@ -2834,18 +2827,40 @@ static bool ilk_disable_lp_wm(struct drm_device *dev)
> #define SKL_DDB_SIZE 896 /* in blocks */
> #define BXT_DDB_SIZE 512
>
> +/*
> + * Return the index of a plane in the SKL DDB and wm result arrays. Primary
> + * plane is always in slot 0, cursor is always in slot I915_MAX_PLANES-1, and
> + * other universal planes are in indices 1..n. Note that this may leave unused
> + * indices between the top "sprite" plane and the cursor.
> + */
> +static int
> +skl_wm_plane_id(const struct intel_plane *plane)
> +{
> + switch (plane->base.type) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + return 0;
> + case DRM_PLANE_TYPE_CURSOR:
> + return PLANE_CURSOR;
> + case DRM_PLANE_TYPE_OVERLAY:
> + return plane->plane;
Shouldn't this be plane + 1 on skylake? Though it's not the only code in i915 that gets it wrong,
assert_sprites_disabled has the same issue on SKL.. O:-)
> + default:
> + MISSING_CASE(plane->base.type);
> + return plane->plane;
> + }
> +}
> +
> static void
> skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
> - struct drm_crtc *for_crtc,
> + const struct intel_crtc_state *cstate,
> const struct intel_wm_config *config,
> - const struct skl_pipe_wm_parameters *params,
> struct skl_ddb_entry *alloc /* out */)
> {
> + struct drm_crtc *for_crtc = cstate->base.crtc;
> struct drm_crtc *crtc;
> unsigned int pipe_size, ddb_size;
> int nth_active_pipe;
>
> - if (!params->active) {
> + if (!cstate->base.active) {
> alloc->start = 0;
> alloc->end = 0;
> return;
> @@ -2911,19 +2926,29 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> }
>
> static unsigned int
> -skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
> +skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> + const struct drm_plane_state *pstate,
> + int y)
> {
> + struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> + struct drm_framebuffer *fb = pstate->fb;
>
> /* for planar format */
> - if (p->y_bytes_per_pixel) {
> + if (fb->pixel_format == DRM_FORMAT_NV12) {
> if (y) /* y-plane data rate */
> - return p->horiz_pixels * p->vert_pixels * p->y_bytes_per_pixel;
> + return intel_crtc->config->pipe_src_w *
> + intel_crtc->config->pipe_src_h *
> + drm_format_plane_cpp(fb->pixel_format, 0);
> else /* uv-plane data rate */
> - return (p->horiz_pixels/2) * (p->vert_pixels/2) * p->bytes_per_pixel;
> + return (intel_crtc->config->pipe_src_w/2) *
> + (intel_crtc->config->pipe_src_h/2) *
> + drm_format_plane_cpp(fb->pixel_format, 1);
> }
>
> /* for packed formats */
> - return p->horiz_pixels * p->vert_pixels * p->bytes_per_pixel;
> + return intel_crtc->config->pipe_src_w *
> + intel_crtc->config->pipe_src_h *
> + drm_format_plane_cpp(fb->pixel_format, 1);
^Gives a nasty divide by zero error on my skl when total_data_rate ends up being 0..
You want to change last argument to 0 here. ;-)
> }
>
> /*
> @@ -2932,46 +2957,51 @@ skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
> * 3 * 4096 * 8192 * 4 < 2^32
> */
> static unsigned int
> -skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
> - const struct skl_pipe_wm_parameters *params)
> +skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
> {
> + struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> + struct drm_device *dev = intel_crtc->base.dev;
> + const struct intel_plane *intel_plane;
> unsigned int total_data_rate = 0;
> - int plane;
>
> - for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
> - const struct intel_plane_wm_parameters *p;
> + for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> + const struct drm_plane_state *pstate = intel_plane->base.state;
>
> - p = ¶ms->plane[plane];
> - if (!p->enabled)
> + if (WARN_ON(pstate->fb == NULL))
> continue;
^Why is this a WARN_ON?
All in all, the series looks good.
With those fixes feel free to add this to all patches:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 07/14] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (5 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 06/14] drm/i915/skl: Eliminate usage of pipe_wm_parameters from SKL-style WM (v2) Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 08/14] drm/i915: Drop intel_update_sprite_watermarks Matt Roper
` (6 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 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 | 39 +++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++---------------------
4 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index f1975f2..05b1203 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -94,6 +94,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
crtc_state->update_pipe = false;
+ 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 017641a..daa0ede 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11514,18 +11514,32 @@ 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)
{
@@ -11541,7 +11555,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;
@@ -11659,11 +11672,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
+ *
+ * cstate->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)) {
+ to_intel_crtc_state(crtc_state)->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 02a755a..3fa2e66 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -458,6 +458,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 51dbda7..d601630 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3728,6 +3728,18 @@ static void ilk_update_wm(struct drm_crtc *crtc)
WARN_ON(cstate->base.active != intel_crtc->active);
+ /*
+ * 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)))
@@ -3759,28 +3771,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,
@@ -7091,7 +7081,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");
--
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] 20+ messages in thread* [PATCH 08/14] drm/i915: Drop intel_update_sprite_watermarks
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (6 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 07/14] drm/i915/ivb: Move WaCxSRDisabledForSpriteScaling w/a to atomic check Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 09/14] drm/i915: Refactor ilk_update_wm (v3) Matt Roper
` (5 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
The only platform that still has an update_sprite_wm entrypoint is SKL;
on SKL, intel_update_sprite_watermarks just updates intel_plane->wm and
then performs a regular watermark update. However intel_plane->wm is
only used to update a couple fields in intel_wm_config, and those fields
are never used by the SKL code, so on SKL an update_sprite_wm is
effectively identical to an update_wm call. Since we're already
ensuring that the regular intel_update_wm is called any time we'd try to
call intel_update_sprite_watermarks, the whole call is redundant and can
be dropped.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ---
drivers/gpu/drm/i915/intel_display.c | 5 ----
drivers/gpu/drm/i915/intel_drv.h | 6 ----
drivers/gpu/drm/i915/intel_pm.c | 58 ------------------------------------
drivers/gpu/drm/i915/intel_sprite.c | 15 ----------
5 files changed, 88 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 539bfa2..b197d41 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -628,10 +628,6 @@ struct drm_i915_display_funcs {
struct dpll *match_clock,
struct dpll *best_clock);
void (*update_wm)(struct drm_crtc *crtc);
- void (*update_sprite_wm)(struct drm_plane *plane,
- struct drm_crtc *crtc,
- uint32_t sprite_width, uint32_t sprite_height,
- int pixel_size, bool enable, bool scaled);
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,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index daa0ede..96ceed7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4721,7 +4721,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_plane *plane;
if (atomic->wait_vblank)
intel_wait_for_vblank(dev, crtc->pipe);
@@ -4740,10 +4739,6 @@ 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);
-
memset(atomic, 0, sizeof(*atomic));
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3fa2e66..9885b25 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1380,12 +1380,6 @@ void intel_init_clock_gating(struct drm_device *dev);
void intel_suspend_hw(struct drm_device *dev);
int ilk_wm_max_level(const struct drm_device *dev);
void intel_update_watermarks(struct drm_crtc *crtc);
-void intel_update_sprite_watermarks(struct drm_plane *plane,
- struct drm_crtc *crtc,
- uint32_t sprite_width,
- uint32_t sprite_height,
- int pixel_size,
- bool enabled, bool scaled);
void intel_init_pm(struct drm_device *dev);
void intel_pm_setup(struct drm_device *dev);
void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d601630..3f9e0e2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3170,18 +3170,9 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev,
struct intel_wm_config *config)
{
struct drm_crtc *crtc;
- struct drm_plane *plane;
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
config->num_pipes_active += to_intel_crtc(crtc)->active;
-
- /* FIXME: I don't think we need those two global parameters on SKL */
- list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
- struct intel_plane *intel_plane = to_intel_plane(plane);
-
- config->sprites_enabled |= intel_plane->wm.enabled;
- config->sprites_scaled |= intel_plane->wm.scaled;
- }
}
static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
@@ -3680,39 +3671,6 @@ static void skl_update_wm(struct drm_crtc *crtc)
dev_priv->wm.skl_hw = *results;
}
-static void
-skl_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 intel_plane *intel_plane = to_intel_plane(plane);
- struct drm_framebuffer *fb = plane->state->fb;
-
- intel_plane->wm.enabled = enabled;
- intel_plane->wm.scaled = scaled;
- intel_plane->wm.horiz_pixels = sprite_width;
- intel_plane->wm.vert_pixels = sprite_height;
- intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE;
-
- /* For planar: Bpp is for UV plane, y_Bpp is for Y plane */
- intel_plane->wm.bytes_per_pixel =
- (fb && fb->pixel_format == DRM_FORMAT_NV12) ?
- drm_format_plane_cpp(plane->state->fb->pixel_format, 1) : pixel_size;
- intel_plane->wm.y_bytes_per_pixel =
- (fb && fb->pixel_format == DRM_FORMAT_NV12) ?
- drm_format_plane_cpp(plane->state->fb->pixel_format, 0) : 0;
-
- /*
- * Framebuffer can be NULL on plane disable, but it does not
- * matter for watermarks if we assume no tiling in that case.
- */
- if (fb)
- intel_plane->wm.tiling = fb->modifier[0];
- intel_plane->wm.rotation = plane->state->rotation;
-
- skl_update_wm(crtc);
-}
-
static void ilk_update_wm(struct drm_crtc *crtc)
{
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -4148,21 +4106,6 @@ void intel_update_watermarks(struct drm_crtc *crtc)
dev_priv->display.update_wm(crtc);
}
-void intel_update_sprite_watermarks(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_i915_private *dev_priv = plane->dev->dev_private;
-
- if (dev_priv->display.update_sprite_wm)
- dev_priv->display.update_sprite_wm(plane, crtc,
- sprite_width, sprite_height,
- pixel_size, enabled, scaled);
-}
-
/**
* Lock protecting IPS related data structures
*/
@@ -7072,7 +7015,6 @@ void intel_init_pm(struct drm_device *dev)
dev_priv->display.init_clock_gating =
skl_init_clock_gating;
dev_priv->display.update_wm = skl_update_wm;
- dev_priv->display.update_sprite_wm = skl_update_sprite_wm;
} else if (HAS_PCH_SPLIT(dev)) {
ilk_setup_wm_latency(dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4d27243..fb096e5 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -182,7 +182,6 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
const int pipe = intel_plane->pipe;
const int plane = intel_plane->plane + 1;
u32 plane_ctl, stride_div, stride;
- int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
const struct drm_intel_sprite_colorkey *key =
&to_intel_plane_state(drm_plane->state)->ckey;
unsigned long surf_addr;
@@ -201,10 +200,6 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
rotation = drm_plane->state->rotation;
plane_ctl |= skl_plane_ctl_rotation(rotation);
- intel_update_sprite_watermarks(drm_plane, crtc, src_w, src_h,
- pixel_size, true,
- src_w != crtc_w || src_h != crtc_h);
-
stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
fb->pixel_format);
@@ -286,8 +281,6 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
I915_WRITE(PLANE_SURF(pipe, plane), 0);
POSTING_READ(PLANE_SURF(pipe, plane));
-
- intel_update_sprite_watermarks(dplane, crtc, 0, 0, 0, false, false);
}
static void
@@ -530,10 +523,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--;
@@ -667,10 +656,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] 20+ messages in thread* [PATCH 09/14] drm/i915: Refactor ilk_update_wm (v3)
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (7 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 08/14] drm/i915: Drop intel_update_sprite_watermarks Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 10/14] drm/i915: Calculate pipe watermarks into CRTC state (v3) Matt Roper
` (4 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 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.
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 | 64 ++++++++++++++++++++++-------------------
1 file changed, 35 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3f9e0e2..20d91bd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3671,39 +3671,14 @@ static void skl_update_wm(struct drm_crtc *crtc)
dev_priv->wm.skl_hw = *results;
}
-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 = {};
-
- WARN_ON(cstate->base.active != intel_crtc->active);
-
- /*
- * 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);
@@ -3729,6 +3704,37 @@ 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 = {};
+
+ WARN_ON(cstate->base.active != intel_crtc->active);
+
+ /*
+ * 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] 20+ messages in thread* [PATCH 10/14] drm/i915: Calculate pipe watermarks into CRTC state (v3)
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (8 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 09/14] drm/i915: Refactor ilk_update_wm (v3) Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 11/14] drm/i915: Calculate ILK-style watermarks during atomic check (v3) Matt Roper
` (3 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
A future patch will calculate these during the atomic 'check' phase
rather than at WM programming time, so let's store the watermark
values we're planning to use in the CRTC state; the values actually
active on the hardware remains in intel_crtc.
While we're at it, do some minor restructuring to keep ILK and SKL
values in a union.
v2: Don't move cxsr_allowed to state (Maarten)
v3: Only calculate watermarks in state. Still keep active watermarks in
intel_crtc itself. (Ville)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 48 +++++++++++++++++++++++++---------------
drivers/gpu/drm/i915/intel_pm.c | 44 ++++++++++++++++++++++--------------
2 files changed, 57 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9885b25..6d6b275 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -324,6 +324,21 @@ struct intel_crtc_scaler_state {
/* drm_mode->private_flags */
#define I915_MODE_FLAG_INHERITED 1
+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;
@@ -461,6 +476,17 @@ struct intel_crtc_state {
/* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
bool disable_lp_wm;
+
+ struct {
+ /*
+ * optimal watermarks, programmed post-vblank when this state
+ * is committed
+ */
+ union {
+ struct intel_pipe_wm ilk;
+ struct skl_pipe_wm skl;
+ } optimal;
+ } wm;
};
struct vlv_wm_state {
@@ -472,15 +498,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;
@@ -488,12 +505,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.
@@ -559,9 +570,10 @@ struct intel_crtc {
/* 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;
+ union {
+ struct intel_pipe_wm ilk;
+ struct skl_pipe_wm skl;
+ } active;
/* 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 20d91bd..b74e2a6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2353,7 +2353,7 @@ 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_pipe_wm *wm = &intel_crtc->wm.active.ilk;
if (!wm->pipe_enabled)
continue;
@@ -2450,7 +2450,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.optimal.ilk;
const struct intel_wm_level *wm = &active->wm[level];
if (!active->pipe_enabled)
@@ -2598,14 +2600,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.optimal.ilk.wm[0];
if (WARN_ON(!r->enable))
continue;
- results->wm_linetime[pipe] = intel_crtc->wm.active.linetime;
+ results->wm_linetime[pipe] = cstate->wm.optimal.ilk.linetime;
results->wm_pipe[pipe] =
(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
@@ -3589,10 +3592,10 @@ static bool skl_update_pipe_wm(struct drm_crtc *crtc,
skl_allocate_pipe_ddb(cstate, config, ddb);
skl_compute_pipe_wm(cstate, ddb, pipe_wm);
- if (!memcmp(&intel_crtc->wm.skl_active, pipe_wm, sizeof(*pipe_wm)))
+ if (!memcmp(&intel_crtc->wm.active.skl, pipe_wm, sizeof(*pipe_wm)))
return false;
- intel_crtc->wm.skl_active = *pipe_wm;
+ intel_crtc->wm.active.skl = *pipe_wm;
return true;
}
@@ -3650,17 +3653,18 @@ static void skl_update_wm(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct skl_wm_values *results = &dev_priv->wm.skl_results;
- struct skl_pipe_wm pipe_wm = {};
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
+ struct skl_pipe_wm *pipe_wm = &cstate->wm.optimal.skl;
struct intel_wm_config config = {};
memset(results, 0, sizeof(*results));
skl_compute_wm_global_parameters(dev, &config);
- if (!skl_update_pipe_wm(crtc, &config, &results->ddb, &pipe_wm))
+ if (!skl_update_pipe_wm(crtc, &config, &results->ddb, pipe_wm))
return;
- skl_compute_wm_results(dev, &pipe_wm, results, intel_crtc);
+ skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);
results->dirty[intel_crtc->pipe] = true;
skl_update_other_pipe_wm(dev, crtc, &config, results);
@@ -3709,7 +3713,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 = {};
WARN_ON(cstate->base.active != intel_crtc->active);
@@ -3725,12 +3728,13 @@ 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);
+ intel_compute_pipe_wm(cstate, &cstate->wm.optimal.ilk);
- if (!memcmp(&intel_crtc->wm.active, &pipe_wm, sizeof(pipe_wm)))
- return;
+ if (!memcmp(&intel_crtc->wm.active.ilk,
+ &cstate->wm.optimal.ilk,
+ sizeof(cstate->wm.optimal.ilk)));
- intel_crtc->wm.active = pipe_wm;
+ intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
ilk_program_watermarks(dev_priv);
}
@@ -3785,7 +3789,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.optimal.skl;
enum pipe pipe = intel_crtc->pipe;
int level, i, max_level;
uint32_t temp;
@@ -3829,6 +3834,8 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
temp = hw->plane_trans[pipe][PLANE_CURSOR];
skl_pipe_wm_active_state(temp, active, true, true, i, 0);
+
+ intel_crtc->wm.active.skl = *active;
}
void skl_wm_get_hw_state(struct drm_device *dev)
@@ -3848,7 +3855,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.optimal.ilk;
enum pipe pipe = intel_crtc->pipe;
static const unsigned int wm0_pipe_reg[] = {
[PIPE_A] = WM0_PIPEA_ILK,
@@ -3887,6 +3895,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;
}
+
+ intel_crtc->wm.active.ilk = *active;
}
#define _FW_WM(value, plane) \
--
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] 20+ messages in thread* [PATCH 11/14] drm/i915: Calculate ILK-style watermarks during atomic check (v3)
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (9 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 10/14] drm/i915: Calculate pipe watermarks into CRTC state (v3) Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 12/14] drm/i915: Don't set plane visible during HW readout if CRTC is off Matt Roper
` (2 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 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.
v3:
- Pass intel_crtc instead of drm_crtc to .compute_pipe_wm() entrypoint
and use intel_atomic_get_crtc_state() to avoid need for extra
casting. (Ander)
- Drop unused intel_check_crtc() function prototype. (Ander)
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_pm.c | 88 ++++++++++++++++++------------------
3 files changed, 51 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b197d41..2696522 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -627,6 +627,8 @@ struct drm_i915_display_funcs {
int target, int refclk,
struct dpll *match_clock,
struct dpll *best_clock);
+ int (*compute_pipe_wm)(struct intel_crtc *crtc,
+ struct drm_atomic_state *state);
void (*update_wm)(struct drm_crtc *crtc);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 96ceed7..0f513e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11768,6 +11768,12 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
}
ret = 0;
+ if (dev_priv->display.compute_pipe_wm) {
+ ret = dev_priv->display.compute_pipe_wm(intel_crtc, state);
+ if (ret)
+ return ret;
+ }
+
if (INTEL_INFO(dev)->gen >= 9) {
if (mode_changed)
ret = skl_update_scaler_crtc(pipe_config);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b74e2a6..3c67b5f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2051,9 +2051,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];
@@ -2065,29 +2067,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;
}
@@ -2365,15 +2349,18 @@ 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 intel_crtc *intel_crtc,
+ struct drm_atomic_state *state)
{
- struct drm_crtc *crtc = cstate->base.crtc;
- struct drm_device *dev = crtc->dev;
+ struct intel_pipe_wm *pipe_wm;
+ struct drm_device *dev = intel_crtc->base.dev;
const struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ 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 = {
@@ -2381,11 +2368,24 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
};
struct ilk_wm_maximums max;
+ cstate = intel_atomic_get_crtc_state(state, intel_crtc);
+ if (IS_ERR(cstate))
+ return PTR_ERR(cstate);
+
+ pipe_wm = &cstate->wm.optimal.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;
@@ -2394,7 +2394,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 */
@@ -2405,24 +2405,27 @@ 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);
+ pipe_wm->linetime = hsw_compute_linetime_wm(dev,
+ &intel_crtc->base);
/* 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 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
@@ -2435,7 +2438,7 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate,
pipe_wm->wm[level] = wm;
}
- return true;
+ return 0;
}
/*
@@ -3728,12 +3731,6 @@ static void ilk_update_wm(struct drm_crtc *crtc)
intel_wait_for_vblank(crtc->dev, intel_crtc->pipe);
}
- intel_compute_pipe_wm(cstate, &cstate->wm.optimal.ilk);
-
- if (!memcmp(&intel_crtc->wm.active.ilk,
- &cstate->wm.optimal.ilk,
- sizeof(cstate->wm.optimal.ilk)));
-
intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
ilk_program_watermarks(dev_priv);
@@ -7039,6 +7036,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] 20+ messages in thread* [PATCH 12/14] drm/i915: Don't set plane visible during HW readout if CRTC is off
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (10 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 11/14] drm/i915: Calculate ILK-style watermarks during atomic check (v3) Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 13/14] drm/i915: Calculate watermark configuration during atomic check (v2) Matt Roper
2015-09-15 2:19 ` [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4) Matt Roper
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
We already ensure that pstate->visible = false when crtc->active = false
during runtime programming; make sure we follow the same logic when
reading out initial hardware state.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0f513e5..36eee4a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15108,7 +15108,7 @@ static void readout_plane_state(struct intel_crtc *crtc,
plane_state = to_intel_plane_state(p->base.state);
- if (p->base.type == DRM_PLANE_TYPE_PRIMARY)
+ if (p->base.type == DRM_PLANE_TYPE_PRIMARY && active)
plane_state->visible = primary_get_hw_state(crtc);
else {
if (active)
--
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] 20+ messages in thread* [PATCH 13/14] drm/i915: Calculate watermark configuration during atomic check (v2)
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (11 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 12/14] drm/i915: Don't set plane visible during HW readout if CRTC is off Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-15 2:19 ` [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4) Matt Roper
13 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
v2: Don't forget to actually check the cstate->active value when
tallying up the number of active CRTC's. (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 10 ++++++
drivers/gpu/drm/i915/intel_display.c | 52 +++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 64 +++++++-----------------------------
4 files changed, 72 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2696522..e8ef328 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1699,6 +1699,13 @@ struct i915_execbuffer_params {
struct drm_i915_gem_request *request;
};
+/* used in computing the new watermarks state */
+struct intel_wm_config {
+ unsigned int num_pipes_active;
+ bool sprites_enabled;
+ bool sprites_scaled;
+};
+
struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *objects;
@@ -1918,6 +1925,9 @@ struct drm_i915_private {
*/
uint16_t skl_latency[8];
+ /* Committed wm config */
+ struct intel_wm_config config;
+
/*
* The skl_wm_values structure is a bit too big for stack
* allocation, so we keep the staging struct where we store
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 36eee4a..3d411c2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12979,6 +12979,45 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
return 0;
}
+/*
+ * Handle calculation of various watermark data at the end of the atomic check
+ * phase. The code here should be run after the per-crtc and per-plane 'check'
+ * handlers to ensure that all derived state has been updated.
+ */
+static void calc_watermark_data(struct drm_atomic_state *state)
+{
+ struct drm_device *dev = state->dev;
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *cstate;
+ struct drm_plane *plane;
+ struct drm_plane_state *pstate;
+
+ /*
+ * Calculate watermark configuration details now that derived
+ * plane/crtc state is all properly updated.
+ */
+ drm_for_each_crtc(crtc, dev) {
+ cstate = drm_atomic_get_existing_crtc_state(state, crtc) ?:
+ crtc->state;
+
+ if (cstate->active)
+ intel_state->wm_config.num_pipes_active++;
+ }
+ drm_for_each_legacy_plane(plane, dev) {
+ pstate = drm_atomic_get_existing_plane_state(state, plane) ?:
+ plane->state;
+
+ if (!to_intel_plane_state(pstate)->visible)
+ continue;
+
+ intel_state->wm_config.sprites_enabled = true;
+ if (pstate->crtc_w != pstate->src_w >> 16 ||
+ pstate->crtc_h != pstate->src_h >> 16)
+ intel_state->wm_config.sprites_scaled = true;
+ }
+}
+
/**
* intel_atomic_check - validate state object
* @dev: drm device
@@ -12987,6 +13026,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
static int intel_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
+ struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
int ret, i;
@@ -13050,10 +13090,15 @@ static int intel_atomic_check(struct drm_device *dev,
if (ret)
return ret;
} else
- to_intel_atomic_state(state)->cdclk =
- to_i915(state->dev)->cdclk_freq;
+ intel_state->cdclk = to_i915(state->dev)->cdclk_freq;
- return drm_atomic_helper_check_planes(state->dev, state);
+ ret = drm_atomic_helper_check_planes(state->dev, state);
+ if (ret)
+ return ret;
+
+ calc_watermark_data(state);
+
+ return 0;
}
/**
@@ -13093,6 +13138,7 @@ static int intel_atomic_commit(struct drm_device *dev,
return ret;
drm_atomic_helper_swap_state(dev, state);
+ dev_priv->wm.config = to_intel_atomic_state(state)->wm_config;
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6d6b275..c40b079 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -240,6 +240,7 @@ struct intel_atomic_state {
unsigned int cdclk;
bool dpll_set;
struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
+ struct intel_wm_config wm_config;
};
struct intel_plane_state {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3c67b5f..ead0359 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1794,13 +1794,6 @@ struct ilk_wm_maximums {
uint16_t fbc;
};
-/* used in computing the new watermarks state */
-struct intel_wm_config {
- unsigned int num_pipes_active;
- bool sprites_enabled;
- bool sprites_scaled;
-};
-
/*
* For both WM_PIPE and WM_LP.
* mem_value must be in 0.1us units.
@@ -2330,24 +2323,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_config(struct drm_device *dev,
- struct intel_wm_config *config)
-{
- struct intel_crtc *intel_crtc;
-
- /* Compute the currently _active_ config */
- for_each_intel_crtc(dev, intel_crtc) {
- const struct intel_pipe_wm *wm = &intel_crtc->wm.active.ilk;
-
- if (!wm->pipe_enabled)
- continue;
-
- config->sprites_enabled |= wm->sprites_enabled;
- config->sprites_scaled |= wm->sprites_scaled;
- config->num_pipes_active++;
- }
-}
-
/* Compute new watermarks for the pipe */
static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
struct drm_atomic_state *state)
@@ -2993,11 +2968,12 @@ skl_get_total_relative_data_rate(const struct intel_crtc_state *cstate)
static void
skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
- const struct intel_wm_config *config,
struct skl_ddb_allocation *ddb /* out */)
{
struct drm_crtc *crtc = cstate->base.crtc;
struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_wm_config *config = &dev_priv->wm.config;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane;
enum pipe pipe = intel_crtc->pipe;
@@ -3172,15 +3148,6 @@ static bool skl_ddb_allocation_changed(const struct skl_ddb_allocation *new_ddb,
return false;
}
-static void skl_compute_wm_global_parameters(struct drm_device *dev,
- struct intel_wm_config *config)
-{
- struct drm_crtc *crtc;
-
- list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
- config->num_pipes_active += to_intel_crtc(crtc)->active;
-}
-
static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
struct intel_crtc_state *cstate,
struct intel_plane *intel_plane,
@@ -3585,14 +3552,13 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
}
static bool skl_update_pipe_wm(struct drm_crtc *crtc,
- struct intel_wm_config *config,
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_allocate_pipe_ddb(cstate, config, ddb);
+ skl_allocate_pipe_ddb(cstate, ddb);
skl_compute_pipe_wm(cstate, ddb, pipe_wm);
if (!memcmp(&intel_crtc->wm.active.skl, pipe_wm, sizeof(*pipe_wm)))
@@ -3605,7 +3571,6 @@ static bool skl_update_pipe_wm(struct drm_crtc *crtc,
static void skl_update_other_pipe_wm(struct drm_device *dev,
struct drm_crtc *crtc,
- struct intel_wm_config *config,
struct skl_wm_values *r)
{
struct intel_crtc *intel_crtc;
@@ -3635,7 +3600,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
if (!intel_crtc->active)
continue;
- wm_changed = skl_update_pipe_wm(&intel_crtc->base, config,
+ wm_changed = skl_update_pipe_wm(&intel_crtc->base,
&r->ddb, &pipe_wm);
/*
@@ -3658,19 +3623,16 @@ static void skl_update_wm(struct drm_crtc *crtc)
struct skl_wm_values *results = &dev_priv->wm.skl_results;
struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
struct skl_pipe_wm *pipe_wm = &cstate->wm.optimal.skl;
- struct intel_wm_config config = {};
memset(results, 0, sizeof(*results));
- skl_compute_wm_global_parameters(dev, &config);
-
- if (!skl_update_pipe_wm(crtc, &config, &results->ddb, pipe_wm))
+ if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
return;
skl_compute_wm_results(dev, pipe_wm, results, intel_crtc);
results->dirty[intel_crtc->pipe] = true;
- skl_update_other_pipe_wm(dev, crtc, &config, results);
+ skl_update_other_pipe_wm(dev, crtc, results);
skl_write_wm_values(dev_priv, results);
skl_flush_wm_values(dev_priv, results);
@@ -3683,20 +3645,18 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
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 intel_wm_config *config = &dev_priv->wm.config;
struct ilk_wm_values results = {};
enum intel_ddb_partitioning partitioning;
- ilk_compute_wm_config(dev, &config);
-
- ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_1_2, &max);
- ilk_wm_merge(dev, &config, &max, &lp_wm_1_2);
+ ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_1_2, &max);
+ ilk_wm_merge(dev, config, &max, &lp_wm_1_2);
/* 5/6 split only in single pipe config on IVB+ */
if (INTEL_INFO(dev)->gen >= 7 &&
- config.num_pipes_active == 1 && config.sprites_enabled) {
- ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_5_6, &max);
- ilk_wm_merge(dev, &config, &max, &lp_wm_5_6);
+ config->num_pipes_active == 1 && config->sprites_enabled) {
+ ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_5_6, &max);
+ ilk_wm_merge(dev, config, &max, &lp_wm_5_6);
best_lp_wm = ilk_find_best_result(dev, &lp_wm_1_2, &lp_wm_5_6);
} else {
--
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] 20+ messages in thread* [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4)
2015-09-15 2:19 [PATCH 00/14] Atomic watermark updates (v4) Matt Roper
` (12 preceding siblings ...)
2015-09-15 2:19 ` [PATCH 13/14] drm/i915: Calculate watermark configuration during atomic check (v2) Matt Roper
@ 2015-09-15 2:19 ` Matt Roper
2015-09-17 12:38 ` Maarten Lankhorst
13 siblings, 1 reply; 20+ messages in thread
From: Matt Roper @ 2015-09-15 2:19 UTC (permalink / raw)
To: intel-gfx
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.
v3:
- Move 'need_postvbl_update' flag to CRTC state (Daniel)
- Don't forget to check intermediate watermark values for validity
(Maarten)
- Don't due async watermark optimization; just do it at the end of the
atomic transaction, after waiting for vblanks. We do want it to be
async eventually, but adding that now will cause more trouble for
Maarten's in-progress work. (Maarten)
- Don't allocate space in crtc_state for intermediate watermarks on
platforms that don't need it (gen9+).
- Move WaCxSRDisabledForSpriteScaling:ivb into intel_begin_crtc_commit
now that ilk_update_wm is gone.
v4:
- Add a wm_mutex to cover updates to intel_crtc->active and the
need_postvbl_update flag. Since we don't have async yet it isn't
terribly important yet, but might as well add it now.
- Change interface to program watermarks. Platforms will now expose
.initial_watermarks() and .optimize_watermarks() functions to do
watermark programming. These should lock wm_mutex, copy the
appropriate state values into intel_crtc->active, and then call
the internal program watermarks function.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 5 ++
drivers/gpu/drm/i915/intel_atomic.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 77 +++++++++++++++++-
drivers/gpu/drm/i915/intel_drv.h | 25 +++++-
drivers/gpu/drm/i915/intel_pm.c | 154 +++++++++++++++++++++++++----------
5 files changed, 213 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e8ef328..6d08d47 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -629,6 +629,11 @@ struct drm_i915_display_funcs {
struct dpll *best_clock);
int (*compute_pipe_wm)(struct intel_crtc *crtc,
struct drm_atomic_state *state);
+ int (*compute_intermediate_wm)(struct drm_device *dev,
+ struct intel_crtc *intel_crtc,
+ struct intel_crtc_state *newstate);
+ void (*initial_watermarks)(struct intel_crtc_state *cstate);
+ void (*optimize_watermarks)(struct intel_crtc_state *cstate);
void (*update_wm)(struct drm_crtc *crtc);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 05b1203..28630f8 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -95,6 +95,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state->update_pipe = false;
crtc_state->disable_lp_wm = false;
+ crtc_state->wm.need_postvbl_update = false;
return &crtc_state->base;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3d411c2..0909485 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11613,6 +11613,12 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
intel_crtc->atomic.update_wm_pre = true;
}
+ /* Pre-gen9 platforms need two-step watermark updates */
+ if ((intel_crtc->atomic.update_wm_pre || intel_crtc->atomic.update_wm_post) &&
+ INTEL_INFO(dev)->gen < 9 &&
+ dev_priv->display.optimize_watermarks)
+ to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
+
if (visible || was_visible)
intel_crtc->atomic.fb_bits |=
to_intel_plane(plane)->frontbuffer_bit;
@@ -11770,8 +11776,28 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
ret = 0;
if (dev_priv->display.compute_pipe_wm) {
ret = dev_priv->display.compute_pipe_wm(intel_crtc, state);
- if (ret)
+ if (ret) {
+ DRM_DEBUG_KMS("Target pipe watermarks are invalid\n");
+ return ret;
+ }
+ }
+
+ if (dev_priv->display.compute_intermediate_wm) {
+ if (WARN_ON(!dev_priv->display.compute_pipe_wm))
+ return 0;
+
+ /*
+ * Calculate 'intermediate' watermarks that satisfy both the
+ * old state and the new state. We can program these
+ * immediately.
+ */
+ ret = dev_priv->display.compute_intermediate_wm(crtc->dev,
+ intel_crtc,
+ pipe_config);
+ if (ret) {
+ DRM_DEBUG_KMS("No valid intermediate pipe watermarks are possible\n");
return ret;
+ }
}
if (INTEL_INFO(dev)->gen >= 9) {
@@ -13124,6 +13150,7 @@ static int intel_atomic_commit(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
+ struct intel_crtc_state *intel_cstate;
int ret = 0;
int i;
bool any_ms = false;
@@ -13202,6 +13229,20 @@ static int intel_atomic_commit(struct drm_device *dev,
/* FIXME: add subpixel order */
drm_atomic_helper_wait_for_vblanks(dev, state);
+
+ /*
+ * Now that the vblank has passed, we can go ahead and program the
+ * optimal watermarks on platforms that need two-step watermark
+ * programming.
+ *
+ * TODO: Move this (and other cleanup) to an async worker eventually.
+ */
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ intel_cstate = to_intel_crtc_state(crtc->state);
+
+ dev_priv->display.optimize_watermarks(intel_cstate);
+ }
+
drm_atomic_helper_cleanup_planes(dev, state);
if (any_ms)
@@ -13530,13 +13571,44 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
{
struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
struct intel_crtc_state *old_intel_state =
to_intel_crtc_state(old_crtc_state);
bool modeset = needs_modeset(crtc->state);
- if (intel_crtc->atomic.update_wm_pre)
+ /*
+ * 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);
+ }
+
+ /*
+ * For platforms that support atomic watermarks, program the
+ * 'intermediate' 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 'active' 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 (dev_priv->display.initial_watermarks != NULL) {
+ dev_priv->display.initial_watermarks(cstate);
+ } else if (intel_crtc->atomic.update_wm_pre) {
intel_update_watermarks(crtc);
+ }
/* Perform vblank evasion around commit operation */
if (crtc->state->active)
@@ -13878,6 +13950,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
intel_crtc->cursor_size = ~0;
intel_crtc->wm.cxsr_allowed = true;
+ mutex_init(&intel_crtc->wm.wm_mutex);
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 c40b079..067100d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -480,13 +480,29 @@ struct intel_crtc_state {
struct {
/*
- * optimal watermarks, programmed post-vblank when this state
- * is committed
+ * Optimal watermarks, programmed post-vblank when this state
+ * is committed.
*/
union {
struct intel_pipe_wm ilk;
struct skl_pipe_wm skl;
} optimal;
+
+ /*
+ * Intermediate watermarks; these can be programmed immediately
+ * since they satisfy both the current configuration we're
+ * switching away from and the new configuration we're switching
+ * to.
+ */
+ struct intel_pipe_wm intermediate;
+
+ /*
+ * Platforms with two-step watermark programming will need to
+ * update watermark programming post-vblank to switch from the
+ * safe intermediate watermarks to the optimal final
+ * watermarks.
+ */
+ bool need_postvbl_update;
} wm;
};
@@ -575,8 +591,12 @@ struct intel_crtc {
struct intel_pipe_wm ilk;
struct skl_pipe_wm skl;
} active;
+
/* allow CxSR on this pipe */
bool cxsr_allowed;
+
+ /* Protects active and need_postvbl_update */
+ struct mutex wm_mutex;
} wm;
int scanline_offset;
@@ -1418,6 +1438,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
struct skl_ddb_allocation *ddb /* out */);
uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
+bool ilk_disable_lp_wm(struct drm_device *dev);
/* intel_sdvo.c */
bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ead0359..41a4dba 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2323,6 +2323,29 @@ static void skl_setup_wm_latency(struct drm_device *dev)
intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
}
+static bool ilk_validate_pipe_wm(struct drm_device *dev,
+ struct intel_pipe_wm *pipe_wm)
+{
+ /* LP0 watermark maximums depend on this pipe alone */
+ const struct intel_wm_config config = {
+ .num_pipes_active = 1,
+ .sprites_enabled = pipe_wm->sprites_enabled,
+ .sprites_scaled = pipe_wm->sprites_scaled,
+ };
+ 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 intel_crtc *intel_crtc,
struct drm_atomic_state *state)
@@ -2337,10 +2360,6 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
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 = {
- .num_pipes_active = 1,
- };
struct ilk_wm_maximums max;
cstate = intel_atomic_get_crtc_state(state, intel_crtc);
@@ -2363,21 +2382,18 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
curstate = to_intel_plane_state(ps);
}
- config.sprites_enabled = sprstate->visible;
- config.sprites_scaled = sprstate->visible &&
+ pipe_wm->pipe_enabled = cstate->base.active;
+ pipe_wm->sprites_enabled = sprstate->visible;
+ pipe_wm->sprites_scaled = sprstate->visible &&
(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 = cstate->base.active;
- pipe_wm->sprites_enabled = config.sprites_enabled;
- pipe_wm->sprites_scaled = config.sprites_scaled;
-
/* ILK/SNB: LP2+ watermarks only w/o sprites */
if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible)
max_level = 1;
/* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */
- if (config.sprites_scaled)
+ if (pipe_wm->sprites_scaled)
max_level = 0;
ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
@@ -2387,12 +2403,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
pipe_wm->linetime = hsw_compute_linetime_wm(dev,
&intel_crtc->base);
- /* 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, pipe_wm))
+ return false;
ilk_compute_wm_reg_maximums(dev, 1, &max);
@@ -2417,6 +2429,59 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_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.
+ */
+static int ilk_compute_intermediate_wm(struct drm_device *dev,
+ struct intel_crtc *intel_crtc,
+ struct intel_crtc_state *newstate)
+{
+ struct intel_pipe_wm *a = &newstate->wm.intermediate;
+ struct intel_pipe_wm *b = &intel_crtc->wm.active.ilk;
+ int level, max_level = ilk_wm_max_level(dev);
+
+ /*
+ * Start with the final, target watermarks, then combine with the
+ * currently active watermarks to get values that are safe both before
+ * and after the vblank.
+ */
+ *a = newstate->wm.optimal.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);
+ }
+
+ /*
+ * We need to make sure that these merged watermark values are
+ * actually a valid configuration themselves. If they're not,
+ * there's no safe way to transition from the old state to
+ * the new state, so we need to fail the atomic transaction.
+ */
+ if (!ilk_validate_pipe_wm(dev, a))
+ return -EINVAL;
+
+ /*
+ * If our intermediate WM are identical to the final WM, then we can
+ * omit the post-vblank programming; only update if it's different.
+ */
+ if (memcmp(a, &newstate->wm.optimal.ilk, sizeof *a) != 0)
+ newstate->wm.need_postvbl_update = false;
+
+ return 0;
+}
+
+/*
* Merge the watermarks from all active pipes for a specific level.
*/
static void ilk_merge_wm_level(struct drm_device *dev,
@@ -2428,9 +2493,7 @@ static void ilk_merge_wm_level(struct drm_device *dev,
ret_wm->enable = true;
for_each_intel_crtc(dev, intel_crtc) {
- const struct intel_crtc_state *cstate =
- to_intel_crtc_state(intel_crtc->base.state);
- const struct intel_pipe_wm *active = &cstate->wm.optimal.ilk;
+ const struct intel_pipe_wm *active = &intel_crtc->wm.active.ilk;
const struct intel_wm_level *wm = &active->wm[level];
if (!active->pipe_enabled)
@@ -2578,15 +2641,13 @@ 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 = &cstate->wm.optimal.ilk.wm[0];
+ const struct intel_wm_level *r = &intel_crtc->wm.active.ilk.wm[0];
if (WARN_ON(!r->enable))
continue;
- results->wm_linetime[pipe] = cstate->wm.optimal.ilk.linetime;
+ results->wm_linetime[pipe] = intel_crtc->wm.active.ilk.linetime;
results->wm_pipe[pipe] =
(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
@@ -2793,7 +2854,7 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
dev_priv->wm.hw = *results;
}
-static bool ilk_disable_lp_wm(struct drm_device *dev)
+bool ilk_disable_lp_wm(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3671,29 +3732,29 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
ilk_write_wm_values(dev_priv, &results);
}
-static void ilk_update_wm(struct drm_crtc *crtc)
+static void ilk_initial_watermarks(struct intel_crtc_state *cstate)
{
- 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);
-
- WARN_ON(cstate->base.active != intel_crtc->active);
+ struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
+ struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
- /*
- * 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);
- }
+ mutex_lock(&intel_crtc->wm.wm_mutex);
+ intel_crtc->wm.active.ilk = cstate->wm.intermediate;
+ ilk_program_watermarks(dev_priv);
+ mutex_unlock(&intel_crtc->wm.wm_mutex);
+}
- intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
+static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
+{
+ struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
+ struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
- ilk_program_watermarks(dev_priv);
+ mutex_lock(&intel_crtc->wm.wm_mutex);
+ if (cstate->wm.need_postvbl_update) {
+ intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
+ ilk_program_watermarks(dev_priv);
+ cstate->wm.need_postvbl_update = false;
+ }
+ mutex_unlock(&intel_crtc->wm.wm_mutex);
}
static void skl_pipe_wm_active_state(uint32_t val,
@@ -6995,8 +7056,11 @@ void intel_init_pm(struct drm_device *dev)
dev_priv->wm.spr_latency[1] && dev_priv->wm.cur_latency[1]) ||
(!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;
+ dev_priv->display.compute_intermediate_wm =
+ ilk_compute_intermediate_wm;
+ dev_priv->display.initial_watermarks = ilk_initial_watermarks;
+ dev_priv->display.optimize_watermarks = ilk_optimize_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] 20+ messages in thread* Re: [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4)
2015-09-15 2:19 ` [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4) Matt Roper
@ 2015-09-17 12:38 ` Maarten Lankhorst
2015-09-17 13:05 ` Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2015-09-17 12:38 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Hey,
It's worh nothing this series may have a soft dependency on Ville's cleanups,
because else pixel_rate == 0 during boot. :(
This patch breaks when the initial sprite watermarks are set on ironlake, but no sprite is enabled.
A naive fix is below, without it my laptop won't power the screen because intermediate wm calculation fails. :(
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 41a4dbaf600b..bf874ff97024 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2457,7 +2457,7 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
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->spr_val = a->sprites_enabled ? max(a_wm->spr_val, b_wm->spr_val) : 0;
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);
}
Op 15-09-15 om 04:19 schreef Matt Roper:
> <snip>
> @@ -13202,6 +13229,20 @@ static int intel_atomic_commit(struct drm_device *dev,
> /* FIXME: add subpixel order */
>
> drm_atomic_helper_wait_for_vblanks(dev, state);
> +
> + /*
> + * Now that the vblank has passed, we can go ahead and program the
> + * optimal watermarks on platforms that need two-step watermark
> + * programming.
> + *
> + * TODO: Move this (and other cleanup) to an async worker eventually.
> + */
> + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> + intel_cstate = to_intel_crtc_state(crtc->state);
> +
> + dev_priv->display.optimize_watermarks(intel_cstate);
> + }
^Breaks on my skylake with a null pointer deref; I think you might want to hide it behind a if. :-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4)
2015-09-17 12:38 ` Maarten Lankhorst
@ 2015-09-17 13:05 ` Ville Syrjälä
2015-09-17 13:24 ` Maarten Lankhorst
0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2015-09-17 13:05 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, Sep 17, 2015 at 02:38:12PM +0200, Maarten Lankhorst wrote:
> Hey,
>
> It's worh nothing this series may have a soft dependency on Ville's cleanups,
> because else pixel_rate == 0 during boot. :(
>
> This patch breaks when the initial sprite watermarks are set on ironlake, but no sprite is enabled.
>
> A naive fix is below, without it my laptop won't power the screen because intermediate wm calculation fails. :(
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 41a4dbaf600b..bf874ff97024 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2457,7 +2457,7 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
>
> 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->spr_val = a->sprites_enabled ? max(a_wm->spr_val, b_wm->spr_val) : 0;
> 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);
> }
Hmm. That could work, but feels a bit wrong. Would be nice if we could
always keep the watermarks totally consistent with the plane
configuration.
One thing we could do during init is simply calculate the optimal
watermarks for the current config and just blast them in. Then when
the first plane update/modeset happens the current state would
actually be sane wrt. the current plane config.
>
>
> Op 15-09-15 om 04:19 schreef Matt Roper:
> > <snip>
> > @@ -13202,6 +13229,20 @@ static int intel_atomic_commit(struct drm_device *dev,
> > /* FIXME: add subpixel order */
> >
> > drm_atomic_helper_wait_for_vblanks(dev, state);
> > +
> > + /*
> > + * Now that the vblank has passed, we can go ahead and program the
> > + * optimal watermarks on platforms that need two-step watermark
> > + * programming.
> > + *
> > + * TODO: Move this (and other cleanup) to an async worker eventually.
> > + */
> > + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > + intel_cstate = to_intel_crtc_state(crtc->state);
> > +
> > + dev_priv->display.optimize_watermarks(intel_cstate);
> > + }
> ^Breaks on my skylake with a null pointer deref; I think you might want to hide it behind a if. :-)
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] 20+ messages in thread* Re: [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4)
2015-09-17 13:05 ` Ville Syrjälä
@ 2015-09-17 13:24 ` Maarten Lankhorst
2015-09-23 9:16 ` Daniel Vetter
0 siblings, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2015-09-17 13:24 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 17-09-15 om 15:05 schreef Ville Syrjälä:
> On Thu, Sep 17, 2015 at 02:38:12PM +0200, Maarten Lankhorst wrote:
>> Hey,
>>
>> It's worh nothing this series may have a soft dependency on Ville's cleanups,
>> because else pixel_rate == 0 during boot. :(
>>
>> This patch breaks when the initial sprite watermarks are set on ironlake, but no sprite is enabled.
>>
>> A naive fix is below, without it my laptop won't power the screen because intermediate wm calculation fails. :(
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 41a4dbaf600b..bf874ff97024 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -2457,7 +2457,7 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
>>
>> 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->spr_val = a->sprites_enabled ? max(a_wm->spr_val, b_wm->spr_val) : 0;
>> 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);
>> }
> Hmm. That could work, but feels a bit wrong. Would be nice if we could
> always keep the watermarks totally consistent with the plane
> configuration.
>
> One thing we could do during init is simply calculate the optimal
> watermarks for the current config and just blast them in. Then when
> the first plane update/modeset happens the current state would
> actually be sane wrt. the current plane config.
>
That would work too, as long as we end up with something sane I'm all for it. :-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 14/14] drm/i915: Add two-stage ILK-style watermark programming (v4)
2015-09-17 13:24 ` Maarten Lankhorst
@ 2015-09-23 9:16 ` Daniel Vetter
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2015-09-23 9:16 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, Sep 17, 2015 at 03:24:00PM +0200, Maarten Lankhorst wrote:
> Op 17-09-15 om 15:05 schreef Ville Syrjälä:
> > On Thu, Sep 17, 2015 at 02:38:12PM +0200, Maarten Lankhorst wrote:
> >> Hey,
> >>
> >> It's worh nothing this series may have a soft dependency on Ville's cleanups,
> >> because else pixel_rate == 0 during boot. :(
> >>
> >> This patch breaks when the initial sprite watermarks are set on ironlake, but no sprite is enabled.
> >>
> >> A naive fix is below, without it my laptop won't power the screen because intermediate wm calculation fails. :(
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 41a4dbaf600b..bf874ff97024 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -2457,7 +2457,7 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev,
> >>
> >> 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->spr_val = a->sprites_enabled ? max(a_wm->spr_val, b_wm->spr_val) : 0;
> >> 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);
> >> }
> > Hmm. That could work, but feels a bit wrong. Would be nice if we could
> > always keep the watermarks totally consistent with the plane
> > configuration.
> >
> > One thing we could do during init is simply calculate the optimal
> > watermarks for the current config and just blast them in. Then when
> > the first plane update/modeset happens the current state would
> > actually be sane wrt. the current plane config.
> >
> That would work too, as long as we end up with something sane I'm all for it. :-)
Yeah blasting in our own preferred config is probably simpler - adding all
kinds of hw state readout for this seems overkill to me. And adding
special cases all over is imo bad since we're really not any good at
testing all the various corner-cases ...
-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] 20+ messages in thread