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

Commit 8d96561a authored by Ville Syrjälä's avatar Ville Syrjälä
Browse files

drm/i915: Protect dev_priv->atomic_cdclk_freq with all the crtc locks 



A modeset on one pipe can update dev_priv->atomic_cdclk_freq without
actually touching the hardware, in which case we won't force a modeset
on all the pipes, and thus won't lock any of the other pipes either.
That means a parallel plane update on another pipe could be looking at
a stale dev_priv->atomic_cdcdlk_freq and thus fail to notice when the
plane configuration is invalid, or potentially reject a valid update.

To overcome this we must protect writes to atomic_cdclk_freq with
all the crtc locks, and thus for reads any single crtc lock will
be sufficient protection.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1479141311-11904-3-git-send-email-ville.syrjala@linux.intel.com


Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent e0ca7a6b

drivers/gpu/drm/i915/i915_drv.h

+8 −1
Original line number Diff line number Diff line
@@ -2016,7 +2016,14 @@ struct drm_i915_private { 


	unsigned int fsb_freq, mem_freq, is_ddr3;

	unsigned int skl_preferred_vco_freq;

	unsigned int cdclk_freq, max_cdclk_freq, atomic_cdclk_freq;

	unsigned int cdclk_freq, max_cdclk_freq;



	/*

	 * For reading holding any crtc lock is sufficient,

	 * for writing must hold all of them.

	 */

	unsigned int atomic_cdclk_freq;



	unsigned int max_dotclk_freq;

	unsigned int rawclk_freq;

	unsigned int hpll_freq;

drivers/gpu/drm/i915/intel_display.c

+36 −5
Original line number Diff line number Diff line
@@ -13914,13 +13914,32 @@ static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state) 
	return 0;

}



static int intel_lock_all_pipes(struct drm_atomic_state *state)

{

	struct drm_crtc *crtc;



	/* Add all pipes to the state */

	for_each_crtc(state->dev, crtc) {

		struct drm_crtc_state *crtc_state;



		crtc_state = drm_atomic_get_crtc_state(state, crtc);

		if (IS_ERR(crtc_state))

			return PTR_ERR(crtc_state);

	}



	return 0;

}



static int intel_modeset_all_pipes(struct drm_atomic_state *state)

{

	struct drm_crtc *crtc;

	struct drm_crtc_state *crtc_state;

	int ret = 0;



	/* add all active pipes to the state */

	/*

	 * Add all pipes to the state, and force

	 * a modeset on all the active ones.

	 */

	for_each_crtc(state->dev, crtc) {

		crtc_state = drm_atomic_get_crtc_state(state, crtc);

		if (IS_ERR(crtc_state))
@@ -13986,12 +14005,24 @@ static int intel_modeset_checks(struct drm_atomic_state *state) 
		if (ret < 0)

			return ret;



		/*

		 * Writes to dev_priv->atomic_cdclk_freq must protected by

		 * holding all the crtc locks, even if we don't end up

		 * touching the hardware

		 */

		if (intel_state->cdclk != dev_priv->atomic_cdclk_freq) {

			ret = intel_lock_all_pipes(state);

			if (ret < 0)

				return ret;

		}



		/* All pipes must be switched off while we change the cdclk. */

		if (intel_state->dev_cdclk != dev_priv->cdclk_freq ||

		    intel_state->cdclk_pll_vco != dev_priv->cdclk_pll.vco)

		    intel_state->cdclk_pll_vco != dev_priv->cdclk_pll.vco) {

			ret = intel_modeset_all_pipes(state);



			if (ret < 0)

				return ret;

		}



		DRM_DEBUG_KMS("New cdclk calculated to be atomic %u, actual %u\n",

			      intel_state->cdclk, intel_state->dev_cdclk);