[Intel-gfx] [PATCH 2/9] drm/i915: Redo plane sanitation during readout
Daniel Vetter
daniel at ffwll.ch
Thu Oct 12 19:03:10 UTC 2017
On Wed, Oct 11, 2017 at 07:04:48PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Unify the plane disabling during state readout by pulling the code into
> a new helper intel_plane_disable_noatomic(). We'll also read out the
> state of all planes, so that we know which planes really need to be
> diabled.
>
> Additonally we change the plane<->pipe mapping sanitation to work by
> simply disabling the offending planes instead of entire pipes. And
> we do it before we otherwise sanitize the crtcs, which means we don't
> have to worry about misassigned planes during crtc sanitation anymore.
>
> v2: Reoder patches to not depend on enum old_plane_id
>
> Cc: Thierry Reding <thierry.reding at gmail.com>
> Cc: Alex Villacís Lasso <alexvillacislasso at hotmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103223
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 116 ++++++++++++++++++++---------------
> 1 file changed, 67 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 825ab00b6639..a9fd3b8fa922 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2729,6 +2729,23 @@ intel_set_plane_visible(struct intel_crtc_state *crtc_state,
> crtc_state->active_planes);
> }
>
> +static void intel_plane_disable_noatomic(struct intel_crtc *crtc,
> + struct intel_plane *plane)
> +{
> + struct intel_crtc_state *crtc_state =
> + to_intel_crtc_state(crtc->base.state);
> + struct intel_plane_state *plane_state =
> + to_intel_plane_state(plane->base.state);
> +
> + intel_set_plane_visible(crtc_state, plane_state, false);
> +
> + if (plane->id == PLANE_PRIMARY)
> + intel_pre_disable_primary_noatomic(&crtc->base);
> +
> + trace_intel_disable_plane(&plane->base, crtc);
> + plane->disable_plane(plane, crtc);
> +}
Wooot, I asked Maarten ages ago to extract this, thanks for doing this!
Just for that:
Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
(ah no I'm kidding, I did check a few things, but thankfully CI now at
least covers some gen3 fun).
Cheers, Daniel
> +
> static void
> intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> struct intel_initial_plane_config *plane_config)
> @@ -2786,12 +2803,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> * simplest solution is to just disable the primary plane now and
> * pretend the BIOS never had it enabled.
> */
> - intel_set_plane_visible(to_intel_crtc_state(crtc_state),
> - to_intel_plane_state(plane_state),
> - false);
> - intel_pre_disable_primary_noatomic(&intel_crtc->base);
> - trace_intel_disable_plane(primary, intel_crtc);
> - intel_plane->disable_plane(intel_plane, intel_crtc);
> + intel_plane_disable_noatomic(intel_crtc, intel_plane);
>
> return;
>
> @@ -5923,6 +5935,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> enum intel_display_power_domain domain;
> + struct intel_plane *plane;
> u64 domains;
> struct drm_atomic_state *state;
> struct intel_crtc_state *crtc_state;
> @@ -5931,11 +5944,12 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
> if (!intel_crtc->active)
> return;
>
> - if (crtc->primary->state->visible) {
> - intel_pre_disable_primary_noatomic(crtc);
> + for_each_intel_plane_on_crtc(&dev_priv->drm, intel_crtc, plane) {
> + const struct intel_plane_state *plane_state =
> + to_intel_plane_state(plane->base.state);
>
> - intel_crtc_disable_planes(crtc, 1 << drm_plane_index(crtc->primary));
> - crtc->primary->state->visible = false;
> + if (plane_state->base.visible)
> + intel_plane_disable_noatomic(intel_crtc, plane);
> }
>
> state = drm_atomic_state_alloc(crtc->dev);
> @@ -14678,22 +14692,38 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> POSTING_READ(DPLL(pipe));
> }
>
> -static bool
> -intel_check_plane_mapping(struct intel_crtc *crtc)
> +static bool intel_plane_mapping_ok(struct intel_crtc *crtc,
> + struct intel_plane *primary)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - u32 val;
> + enum plane plane = primary->plane;
> + u32 val = I915_READ(DSPCNTR(plane));
>
> - if (INTEL_INFO(dev_priv)->num_pipes == 1)
> - return true;
> + return (val & DISPLAY_PLANE_ENABLE) == 0 ||
> + (val & DISPPLANE_SEL_PIPE_MASK) == DISPPLANE_SEL_PIPE(crtc->pipe);
> +}
>
> - val = I915_READ(DSPCNTR(!crtc->plane));
> +static void
> +intel_sanitize_plane_mapping(struct drm_i915_private *dev_priv)
> +{
> + enum pipe pipe;
>
> - if ((val & DISPLAY_PLANE_ENABLE) &&
> - (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
> - return false;
> + if (INTEL_GEN(dev_priv) >= 4)
> + return;
>
> - return true;
> + for_each_pipe(dev_priv, pipe) {
> + struct intel_crtc *crtc =
> + intel_get_crtc_for_pipe(dev_priv, pipe);
> + struct intel_plane *plane =
> + to_intel_plane(crtc->base.primary);
> +
> + if (intel_plane_mapping_ok(crtc, plane))
> + continue;
> +
> + DRM_DEBUG_KMS("%s attached to the wrong pipe, disabling plane\n",
> + plane->base.name);
> + intel_plane_disable_noatomic(crtc, plane);
> + }
> }
>
> static bool intel_crtc_has_encoders(struct intel_crtc *crtc)
> @@ -14749,33 +14779,15 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>
> /* Disable everything but the primary plane */
> for_each_intel_plane_on_crtc(dev, crtc, plane) {
> - if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
> - continue;
> + const struct intel_plane_state *plane_state =
> + to_intel_plane_state(plane->base.state);
>
> - trace_intel_disable_plane(&plane->base, crtc);
> - plane->disable_plane(plane, crtc);
> + if (plane_state->base.visible &&
> + plane->base.type != DRM_PLANE_TYPE_PRIMARY)
> + intel_plane_disable_noatomic(crtc, plane);
> }
> }
>
> - /* We need to sanitize the plane -> pipe mapping first because this will
> - * disable the crtc (and hence change the state) if it is wrong. Note
> - * that gen4+ has a fixed plane -> pipe mapping. */
> - if (INTEL_GEN(dev_priv) < 4 && !intel_check_plane_mapping(crtc)) {
> - bool plane;
> -
> - DRM_DEBUG_KMS("[CRTC:%d:%s] wrong plane connection detected!\n",
> - crtc->base.base.id, crtc->base.name);
> -
> - /* Pipe has the wrong plane attached and the plane is active.
> - * Temporarily change the plane mapping and disable everything
> - * ... */
> - plane = crtc->plane;
> - crtc->base.primary->state->visible = true;
> - crtc->plane = !plane;
> - intel_crtc_disable_noatomic(&crtc->base, ctx);
> - crtc->plane = plane;
> - }
> -
> /* Adjust the state of the output pipe according to whether we
> * have active connectors/encoders. */
> if (crtc->active && !intel_crtc_has_encoders(crtc))
> @@ -14883,14 +14895,18 @@ void i915_redisable_vga(struct drm_i915_private *dev_priv)
> /* FIXME read out full plane state for all planes */
> static void readout_plane_state(struct intel_crtc *crtc)
> {
> - struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> - bool visible;
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + struct intel_crtc_state *crtc_state =
> + to_intel_crtc_state(crtc->base.state);
> + struct intel_plane *plane;
>
> - visible = crtc->active && primary->get_hw_state(primary);
> + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
> + struct intel_plane_state *plane_state =
> + to_intel_plane_state(plane->base.state);
> + bool visible = plane->get_hw_state(plane);
>
> - intel_set_plane_visible(to_intel_crtc_state(crtc->base.state),
> - to_intel_plane_state(primary->base.state),
> - visible);
> + intel_set_plane_visible(crtc_state, plane_state, visible);
> + }
> }
>
> static void intel_modeset_readout_hw_state(struct drm_device *dev)
> @@ -15079,6 +15095,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
> /* HW state is read out, now we need to sanitize this mess. */
> get_encoder_power_domains(dev_priv);
>
> + intel_sanitize_plane_mapping(dev_priv);
> +
> for_each_intel_encoder(dev, encoder) {
> intel_sanitize_encoder(encoder);
> }
> --
> 2.13.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list