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 <cooper.chiou@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210506073836.14848-1-ville.syrjala@linux.intel.com Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
This commit is contained in:
parent
e7c46e43bd
commit
7c653e15e2
|
@ -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);
|
||||
|
||||
/*
|
||||
|
|
|
@ -1032,6 +1032,14 @@ skl_program_plane(struct intel_plane *plane,
|
|||
if (!drm_atomic_crtc_needs_modeset(&crtc_state->uapi))
|
||||
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
|
||||
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue