From: Ville Syrjälä Date: Thu, 6 May 2021 07:38:36 +0000 (+0300) Subject: drm/i915: Reorder skl+ scaler vs. plane updates X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=7c653e15e2ba4c476ea9aa1f5ddf14ca3ccacc17;p=uclinux-h8%2Flinux.git drm/i915: Reorder skl+ scaler vs. plane updates When scanning out NV12 if we at any time have the plane enabled while the scaler is disabled we get a pretty catastrophic underrun. Let's reorder the operations so that we try to avoid that happening even if our vblank evade fails and the scaler enable/disable and the plane enable/disable get latched during two diffent frames. This takes care of the most common cases. I suppose there is still at least a theoretical possibility of hitting this if one plane takes the scaler away from another plane before the second plane had a chance to set up another scaler for its use. But that is starting to get a bit complicated, especially since the plane commit order already has to be carefully sequenced to avoid any dbuf overlaps. So plugging this 100% may prove somewhat hard... Cc: Cooper Chiou Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20210506073836.14848-1-ville.syrjala@linux.intel.com Reviewed-by: Stanislav Lisovskiy --- diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 91e5ae157953..42f63914fa0c 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -9697,8 +9697,6 @@ static void intel_pipe_fastset(const struct intel_crtc_state *old_crtc_state, /* on skylake this is done by detaching scalers */ if (DISPLAY_VER(dev_priv) >= 9) { - skl_detach_scalers(new_crtc_state); - if (new_crtc_state->pch_pfit.enabled) skl_pfit_enable(new_crtc_state); } else if (HAS_PCH_SPLIT(dev_priv)) { @@ -9724,8 +9722,8 @@ static void intel_pipe_fastset(const struct intel_crtc_state *old_crtc_state, icl_set_pipe_chicken(crtc); } -static void commit_pipe_config(struct intel_atomic_state *state, - struct intel_crtc *crtc) +static void commit_pipe_pre_planes(struct intel_atomic_state *state, + struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(state->base.dev); const struct intel_crtc_state *old_crtc_state = @@ -9743,9 +9741,6 @@ static void commit_pipe_config(struct intel_atomic_state *state, new_crtc_state->update_pipe) intel_color_commit(new_crtc_state); - if (DISPLAY_VER(dev_priv) >= 9) - skl_detach_scalers(new_crtc_state); - if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) bdw_set_pipemisc(new_crtc_state); @@ -9759,6 +9754,23 @@ static void commit_pipe_config(struct intel_atomic_state *state, dev_priv->display.atomic_update_watermarks(state, crtc); } +static void commit_pipe_post_planes(struct intel_atomic_state *state, + struct intel_crtc *crtc) +{ + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + const struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + + /* + * Disable the scaler(s) after the plane(s) so that we don't + * get a catastrophic underrun even if the two operations + * end up happening in two different frames. + */ + if (DISPLAY_VER(dev_priv) >= 9 && + !intel_crtc_needs_modeset(new_crtc_state)) + skl_detach_scalers(new_crtc_state); +} + static void intel_enable_crtc(struct intel_atomic_state *state, struct intel_crtc *crtc) { @@ -9810,13 +9822,15 @@ static void intel_update_crtc(struct intel_atomic_state *state, /* Perform vblank evasion around commit operation */ intel_pipe_update_start(new_crtc_state); - commit_pipe_config(state, crtc); + commit_pipe_pre_planes(state, crtc); if (DISPLAY_VER(dev_priv) >= 9) skl_update_planes_on_crtc(state, crtc); else i9xx_update_planes_on_crtc(state, crtc); + commit_pipe_post_planes(state, crtc); + intel_pipe_update_end(new_crtc_state); /* diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 0d34a5ad4e2b..6ad85d7cb219 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -1033,6 +1033,14 @@ skl_program_plane(struct intel_plane *plane, intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, color_plane); /* + * Enable the scaler before the plane so that we don't + * get a catastrophic underrun even if the two operations + * end up happening in two different frames. + */ + if (plane_state->scaler_id >= 0) + skl_program_plane_scaler(plane, crtc_state, plane_state); + + /* * The control register self-arms if the plane was previously * disabled. Try to make the plane enable atomic by writing * the control register just before the surface register. @@ -1041,9 +1049,6 @@ skl_program_plane(struct intel_plane *plane, intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), intel_plane_ggtt_offset(plane_state) + surf_addr); - if (plane_state->scaler_id >= 0) - skl_program_plane_scaler(plane, crtc_state, plane_state); - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); }