[Intel-gfx] [PATCH 2/3] drm/i915: Use crtc->state->active in ilk/skl watermark calculations (v3)

Matt Roper matthew.d.roper at intel.com
Mon Mar 9 10:19:24 PDT 2015


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 0fda65680e92545caea5be7805a7f0a617fb6c20
        Author: Tvrtko Ursulin <tvrtko.ursulin at 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 at linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4f04fab..a06a2c7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -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;
 	u32 linetime, ips_linetime;
 
-	if (!intel_crtc_active(crtc))
+	if (!intel_crtc->active)
 		return 0;
 
 	/* 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,
 	enum pipe pipe = intel_crtc->pipe;
 	struct drm_plane *plane;
 
-	if (!intel_crtc_active(crtc))
+	if (!intel_crtc->active)
 		return;
 
 	p->active = true;
@@ -2468,7 +2468,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
 
 	nth_active_pipe = 0;
 	for_each_crtc(dev, crtc) {
-		if (!intel_crtc_active(crtc))
+		if (!to_intel_crtc(crtc)->active)
 			continue;
 
 		if (crtc == for_crtc)
@@ -2708,7 +2708,7 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev,
 	struct drm_plane *plane;
 
 	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 */
 	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,
 	struct drm_framebuffer *fb;
 	int i = 1; /* Index for sprite planes start */
 
-	p->active = intel_crtc_active(crtc);
+	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);
@@ -2860,7 +2860,7 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
 static uint32_t
 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 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)
 		hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i));
 	hw->cursor_trans[pipe] = I915_READ(CUR_WM_TRANS(pipe));
 
-	if (!intel_crtc_active(crtc))
+	if (!intel_crtc->active)
 		return;
 
 	hw->dirty[pipe] = true;
@@ -3462,7 +3462,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		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) {
 		u32 tmp = hw->wm_pipe[pipe];
-- 
1.8.5.1



More information about the Intel-gfx mailing list