From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx>
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 9 +++++++-
drivers/gpu/drm/i915/intel_display.c | 41 +++++++++++++++++++++++++++++++-----
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c0f1dfc7119e..66d2950dc657 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1874,7 +1874,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;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 70f3f0b70263..d7a4bc63b05b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13946,13 +13946,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))
@@ -14018,12 +14037,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;
+ 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);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
