Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8553c18e authored by Ville Syrjälä's avatar Ville Syrjälä Committed by Daniel Vetter
Browse files

drm/i915: Try to fix the messy IVB sprite scaling workaround 



We now have a very clear method of disabling LP1+ wartermarks,
and we can actually detect if we actually did disable them, or
if they were already disabled. Use that to clean up the
WaCxSRDisabledForSpriteScaling:ivb handling.

I was hoping to apply the workaround in a way that wouldn't
require a blocking wait, but sadly IVB really does appear to
require LP1+ watermarks to be off for an entire frame before
enabling sprite scaling. Simply disabling LP1+ watermarks
during the previous frame is not enough, no matter how early
in the frame we do it :(

Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: default avatarImre Deak <imre.deak@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 96f90c54

drivers/gpu/drm/i915/i915_drv.h

+0 −1
Original line number Original line Diff line number Diff line
@@ -1401,7 +1401,6 @@ typedef struct drm_i915_private { 




	/* overlay */

	/* overlay */

	struct intel_overlay *overlay;

	struct intel_overlay *overlay;

	unsigned int sprite_scaling_enabled;





	/* backlight registers and fields in struct intel_panel */

	/* backlight registers and fields in struct intel_panel */

	spinlock_t backlight_lock;

	spinlock_t backlight_lock;

drivers/gpu/drm/i915/intel_pm.c

+45 −13
Original line number Original line Diff line number Diff line
@@ -2426,33 +2426,26 @@ static unsigned int ilk_compute_wm_dirty(struct drm_device *dev, 
	return dirty;

	return dirty;

}

}





/*

static bool _ilk_disable_lp_wm(struct drm_i915_private *dev_priv,

 * The spec says we shouldn't write when we don't need, because every write

			       unsigned int dirty)

 * causes WMs to be re-evaluated, expending some power.

 */

static void hsw_write_wm_values(struct drm_i915_private *dev_priv,

				struct hsw_wm_values *results)

{

{

	struct drm_device *dev = dev_priv->dev;

	struct hsw_wm_values *previous = &dev_priv->wm.hw;

	struct hsw_wm_values *previous = &dev_priv->wm.hw;

	unsigned int dirty;

	bool changed = false;

	uint32_t val;



	dirty = ilk_compute_wm_dirty(dev_priv->dev, previous, results);

	if (!dirty)

		return;





	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] & WM1_LP_SR_EN) {

	if (dirty & WM_DIRTY_LP(3) && previous->wm_lp[2] & WM1_LP_SR_EN) {

		previous->wm_lp[2] &= ~WM1_LP_SR_EN;

		previous->wm_lp[2] &= ~WM1_LP_SR_EN;

		I915_WRITE(WM3_LP_ILK, previous->wm_lp[2]);

		I915_WRITE(WM3_LP_ILK, previous->wm_lp[2]);

		changed = true;

	}

	}

	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] & WM1_LP_SR_EN) {

	if (dirty & WM_DIRTY_LP(2) && previous->wm_lp[1] & WM1_LP_SR_EN) {

		previous->wm_lp[1] &= ~WM1_LP_SR_EN;

		previous->wm_lp[1] &= ~WM1_LP_SR_EN;

		I915_WRITE(WM2_LP_ILK, previous->wm_lp[1]);

		I915_WRITE(WM2_LP_ILK, previous->wm_lp[1]);

		changed = true;

	}

	}

	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] & WM1_LP_SR_EN) {

	if (dirty & WM_DIRTY_LP(1) && previous->wm_lp[0] & WM1_LP_SR_EN) {

		previous->wm_lp[0] &= ~WM1_LP_SR_EN;

		previous->wm_lp[0] &= ~WM1_LP_SR_EN;

		I915_WRITE(WM1_LP_ILK, previous->wm_lp[0]);

		I915_WRITE(WM1_LP_ILK, previous->wm_lp[0]);

		changed = true;

	}

	}





	/*

	/*
@@ -2460,6 +2453,27 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv, 
	 * Doing so could cause underruns.

	 * Doing so could cause underruns.

	 */

	 */





	return changed;

}



/*

 * The spec says we shouldn't write when we don't need, because every write

 * causes WMs to be re-evaluated, expending some power.

 */

static void hsw_write_wm_values(struct drm_i915_private *dev_priv,

				struct hsw_wm_values *results)

{

	struct drm_device *dev = dev_priv->dev;

	struct hsw_wm_values *previous = &dev_priv->wm.hw;

	unsigned int dirty;

	uint32_t val;



	dirty = ilk_compute_wm_dirty(dev, previous, results);

	if (!dirty)

		return;



	_ilk_disable_lp_wm(dev_priv, dirty);



	if (dirty & WM_DIRTY_PIPE(PIPE_A))

	if (dirty & WM_DIRTY_PIPE(PIPE_A))

		I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);

		I915_WRITE(WM0_PIPEA_ILK, results->wm_pipe[0]);

	if (dirty & WM_DIRTY_PIPE(PIPE_B))

	if (dirty & WM_DIRTY_PIPE(PIPE_B))
@@ -2523,6 +2537,13 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv, 
	dev_priv->wm.hw = *results;

	dev_priv->wm.hw = *results;

}

}





static bool ilk_disable_lp_wm(struct drm_device *dev)

{

	struct drm_i915_private *dev_priv = dev->dev_private;



	return _ilk_disable_lp_wm(dev_priv, WM_DIRTY_LP_ALL);

}



static void haswell_update_wm(struct drm_crtc *crtc)

static void haswell_update_wm(struct drm_crtc *crtc)

{

{

	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -2572,6 +2593,7 @@ static void haswell_update_sprite_wm(struct drm_plane *plane, 
				     uint32_t sprite_width, int pixel_size,

				     uint32_t sprite_width, int pixel_size,

				     bool enabled, bool scaled)

				     bool enabled, bool scaled)

{

{

	struct drm_device *dev = plane->dev;

	struct intel_plane *intel_plane = to_intel_plane(plane);

	struct intel_plane *intel_plane = to_intel_plane(plane);





	intel_plane->wm.enabled = enabled;

	intel_plane->wm.enabled = enabled;
@@ -2579,6 +2601,16 @@ static void haswell_update_sprite_wm(struct drm_plane *plane, 
	intel_plane->wm.horiz_pixels = sprite_width;

	intel_plane->wm.horiz_pixels = sprite_width;

	intel_plane->wm.bytes_per_pixel = pixel_size;

	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

	 * when scaling is disabled.

	 *

	 * WaCxSRDisabledForSpriteScaling:ivb

	 */

	if (IS_IVYBRIDGE(dev) && scaled && ilk_disable_lp_wm(dev))

		intel_wait_for_vblank(dev, intel_plane->pipe);



	haswell_update_wm(crtc);

	haswell_update_wm(crtc);

}

}

drivers/gpu/drm/i915/intel_sprite.c

+1 −26
Original line number Original line Diff line number Diff line
@@ -230,7 +230,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, 
	u32 sprctl, sprscale = 0;

	u32 sprctl, sprscale = 0;

	unsigned long sprsurf_offset, linear_offset;

	unsigned long sprsurf_offset, linear_offset;

	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);

	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);

	bool scaling_was_enabled = dev_priv->sprite_scaling_enabled;





	sprctl = I915_READ(SPRCTL(pipe));

	sprctl = I915_READ(SPRCTL(pipe));
@@ -291,21 +290,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, 
	crtc_w--;

	crtc_w--;

	crtc_h--;

	crtc_h--;





	/*

	if (crtc_w != src_w || crtc_h != src_h)

	 * 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.

	 */

	if (crtc_w != src_w || crtc_h != src_h) {

		dev_priv->sprite_scaling_enabled |= 1 << pipe;



		if (!scaling_was_enabled) {

			intel_update_watermarks(crtc);

			intel_wait_for_vblank(dev, pipe);

		}

		sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;

		sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;

	} else

		dev_priv->sprite_scaling_enabled &= ~(1 << pipe);





	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);

	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);

	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);

	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
@@ -332,10 +318,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, 
	I915_MODIFY_DISPBASE(SPRSURF(pipe),

	I915_MODIFY_DISPBASE(SPRSURF(pipe),

			     i915_gem_obj_ggtt_offset(obj) + sprsurf_offset);

			     i915_gem_obj_ggtt_offset(obj) + sprsurf_offset);

	POSTING_READ(SPRSURF(pipe));

	POSTING_READ(SPRSURF(pipe));



	/* potentially re-enable LP watermarks */

	if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)

		intel_update_watermarks(crtc);

}

}





static void

static void
@@ -345,7 +327,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) 
	struct drm_i915_private *dev_priv = dev->dev_private;

	struct drm_i915_private *dev_priv = dev->dev_private;

	struct intel_plane *intel_plane = to_intel_plane(plane);

	struct intel_plane *intel_plane = to_intel_plane(plane);

	int pipe = intel_plane->pipe;

	int pipe = intel_plane->pipe;

	bool scaling_was_enabled = dev_priv->sprite_scaling_enabled;





	I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);

	I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);

	/* Can't leave the scaler enabled... */

	/* Can't leave the scaler enabled... */
@@ -355,13 +336,7 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) 
	I915_MODIFY_DISPBASE(SPRSURF(pipe), 0);

	I915_MODIFY_DISPBASE(SPRSURF(pipe), 0);

	POSTING_READ(SPRSURF(pipe));

	POSTING_READ(SPRSURF(pipe));





	dev_priv->sprite_scaling_enabled &= ~(1 << pipe);



	intel_update_sprite_watermarks(plane, crtc, 0, 0, false, false);

	intel_update_sprite_watermarks(plane, crtc, 0, 0, false, false);



	/* potentially re-enable LP watermarks */

	if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)

		intel_update_watermarks(crtc);

}

}





static int

static int