| Message ID | 20171011160455.1874-3-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 11, 2017 at 07:04:48PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@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@gmail.com> > Cc: Alex Villacís Lasso <alexvillacislasso@hotmail.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103223 > Signed-off-by: Ville Syrjälä <ville.syrjala@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@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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); +} + 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); }