Commit 3ef00284 authored by Matt Roper's avatar Matt Roper Committed by Daniel Vetter

drm/i915: Use crtc->state->active in ilk/skl watermark calculations (v3)

Existing watermark code calls intel_crtc_active() to determine whether a CRTC
is active for the purpose of watermark calculations (and bails out early if it
determines the CRTC is not active).  However intel_crtc_active() only returns
true if crtc->primary->fb is non-NULL, which isn't appropriate in the modern
age of universal planes and atomic modeset since userspace can now disable the
primary plane, but leave the CRTC (and other planes) running.

Note that commit

        commit 0fda6568
        Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
        Date:   Fri Feb 27 15:12:35 2015 +0000

            drm/i915/skl: Update watermarks for Y tiling

adds a test for primary plane enable/disable to trigger a watermark update
(previously we ignored updates to primary planes, which wasn't really correct,
but we got lucky since we always pretended the primary plane was on).  Tvrtko's
patch tries to update watermarks when we re-enable the primary plane, but that
watermark computation gets aborted early because intel_crtc_active() returns
false due to the disabled primary plane.

Switch the ILK and SKL watermark code over to use crtc->state->active rather
than calling intel_crtc_active() so that we'll properly compute watermarks when
re-enabling the primary plane.

Note that this commit doesn't touch callsites in the watermark code for
older platforms since there were concerns that doing so would lead to
other types of breakage.

Also note that all of the watermark calculation at the moment takes place after
new crtc/plane states are swapped into the DRM objects.  This will change in
the future, so we'll be working with in-flight state objects, but for the time
being, crtc->state is what we want to operate on.

v2: Don't drop primary->fb check from intel_crtc_active(), but rather replace
    ILK/SKL callsites with direct tests of crtc->state->active.  There is
    concern that messing with intel_crtc_active() will lead to other breakage for
    old hardware platforms.  (Ville)

v3: Use intel_crtc->active for now rather than crtc->state->active since
    we don't have CRTC states properly hooked up and initialized yet.
    We'll defer the switch to crtc->state->active until the atomic CRTC
    state work is farther along. (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: default avatarMatt Roper <matthew.d.roper@intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent c3d1f436
...@@ -1701,7 +1701,7 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) ...@@ -1701,7 +1701,7 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
struct drm_display_mode *mode = &intel_crtc->config->base.adjusted_mode; struct drm_display_mode *mode = &intel_crtc->config->base.adjusted_mode;
u32 linetime, ips_linetime; u32 linetime, ips_linetime;
if (!intel_crtc_active(crtc)) if (!intel_crtc->active)
return 0; return 0;
/* The WM are computed with base on how long it takes to fill a single /* The WM are computed with base on how long it takes to fill a single
...@@ -1956,7 +1956,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc, ...@@ -1956,7 +1956,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
enum pipe pipe = intel_crtc->pipe; enum pipe pipe = intel_crtc->pipe;
struct drm_plane *plane; struct drm_plane *plane;
if (!intel_crtc_active(crtc)) if (!intel_crtc->active)
return; return;
p->active = true; p->active = true;
...@@ -2468,7 +2468,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev, ...@@ -2468,7 +2468,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
nth_active_pipe = 0; nth_active_pipe = 0;
for_each_crtc(dev, crtc) { for_each_crtc(dev, crtc) {
if (!intel_crtc_active(crtc)) if (!to_intel_crtc(crtc)->active)
continue; continue;
if (crtc == for_crtc) if (crtc == for_crtc)
...@@ -2708,7 +2708,7 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev, ...@@ -2708,7 +2708,7 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev,
struct drm_plane *plane; struct drm_plane *plane;
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
config->num_pipes_active += intel_crtc_active(crtc); config->num_pipes_active += to_intel_crtc(crtc)->active;
/* FIXME: I don't think we need those two global parameters on SKL */ /* FIXME: I don't think we need those two global parameters on SKL */
list_for_each_entry(plane, &dev->mode_config.plane_list, head) { list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
...@@ -2729,7 +2729,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, ...@@ -2729,7 +2729,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
struct drm_framebuffer *fb; struct drm_framebuffer *fb;
int i = 1; /* Index for sprite planes start */ int i = 1; /* Index for sprite planes start */
p->active = intel_crtc_active(crtc); p->active = intel_crtc->active;
if (p->active) { if (p->active) {
p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal; p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config); p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
...@@ -2860,7 +2860,7 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv, ...@@ -2860,7 +2860,7 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
static uint32_t static uint32_t
skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p) skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
{ {
if (!intel_crtc_active(crtc)) if (!to_intel_crtc(crtc)->active)
return 0; return 0;
return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate); return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
...@@ -3407,7 +3407,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc) ...@@ -3407,7 +3407,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i)); hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i));
hw->cursor_trans[pipe] = I915_READ(CUR_WM_TRANS(pipe)); hw->cursor_trans[pipe] = I915_READ(CUR_WM_TRANS(pipe));
if (!intel_crtc_active(crtc)) if (!intel_crtc->active)
return; return;
hw->dirty[pipe] = true; hw->dirty[pipe] = true;
...@@ -3462,7 +3462,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc) ...@@ -3462,7 +3462,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
if (IS_HASWELL(dev) || IS_BROADWELL(dev)) if (IS_HASWELL(dev) || IS_BROADWELL(dev))
hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe)); hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
active->pipe_enabled = intel_crtc_active(crtc); active->pipe_enabled = intel_crtc->active;
if (active->pipe_enabled) { if (active->pipe_enabled) {
u32 tmp = hw->wm_pipe[pipe]; u32 tmp = hw->wm_pipe[pipe];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment