| Message ID | 1378293610-26631-1-git-send-email-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 04, 2013 at 02:20:08PM +0300, Jani Nikula wrote: > The cursor is supposed to be disabled during crtc mode set (disabled by > ctrc disable). Assert this is the case. > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index d88057e..89243cc 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1069,6 +1069,26 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv, > pipe_name(pipe)); > } > > +static void assert_cursor(struct drm_i915_private *dev_priv, > + enum pipe pipe, bool state) > +{ > + struct drm_device *dev = dev_priv->dev; > + bool cur_state; > + > + if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) This could be (gen >=7 && !VLV), but we have the same check in intel_crtc_update_cursor() so that should be changed as well. Can be left for another patch I suppose. > + cur_state = I915_READ(CURCNTR_IVB(pipe)) & CURSOR_MODE; > + else if (IS_845G(dev) || IS_I865G(dev)) > + cur_state = I915_READ(_CURACNTR) & CURSOR_ENABLE; > + else > + cur_state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE; First I was thingking that can't be right, but looks like it is. Weird how they stuffed the cursor registers differently on mobile and desktop during the gen2 days. > + > + WARN(cur_state != state, > + "cursor on pipe %c assertion failure (expected %s, current %s)\n", > + pipe_name(pipe), state_string(state), state_string(cur_state)); > +} > +#define assert_cursor_enabled(d, p) assert_cursor(d, p, true) > +#define assert_cursor_disabled(d, p) assert_cursor(d, p, false) I guess we don't really need assert_cursor_enabled() but no real harm in having it. > + > void assert_pipe(struct drm_i915_private *dev_priv, > enum pipe pipe, bool state) > { > @@ -4856,6 +4876,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, > const intel_limit_t *limit; > int ret; > > + assert_cursor_disabled(dev_priv, pipe); > + > for_each_encoder_on_crtc(dev, crtc, encoder) { > switch (encoder->type) { > case INTEL_OUTPUT_LVDS: > @@ -5754,6 +5776,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > struct intel_shared_dpll *pll; > int ret; > > + assert_cursor_disabled(dev_priv, pipe); > + > for_each_encoder_on_crtc(dev, crtc, encoder) { > switch (encoder->type) { > case INTEL_OUTPUT_LVDS: > @@ -6270,6 +6294,8 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, > int plane = intel_crtc->plane; > int ret; > > + assert_cursor_disabled(dev_priv, intel_crtc->pipe); > + > if (!intel_ddi_pll_mode_set(crtc)) > return -EINVAL; I'd slap the asserts to the same places as the asserts for other planes. But maybe we'd want to have pipe and plane asserts in mode_set as well, just in case we manage to mess things up and call mode_set w/o disabling the pipe first. Also looks like the plane assert should be killed from ironlake_fdi_link_train(). We shouldn't need a plain to train the FDI link as the pipe will anyway pump out pixels.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d88057e..89243cc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1069,6 +1069,26 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv, pipe_name(pipe)); } +static void assert_cursor(struct drm_i915_private *dev_priv, + enum pipe pipe, bool state) +{ + struct drm_device *dev = dev_priv->dev; + bool cur_state; + + if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) + cur_state = I915_READ(CURCNTR_IVB(pipe)) & CURSOR_MODE; + else if (IS_845G(dev) || IS_I865G(dev)) + cur_state = I915_READ(_CURACNTR) & CURSOR_ENABLE; + else + cur_state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE; + + WARN(cur_state != state, + "cursor on pipe %c assertion failure (expected %s, current %s)\n", + pipe_name(pipe), state_string(state), state_string(cur_state)); +} +#define assert_cursor_enabled(d, p) assert_cursor(d, p, true) +#define assert_cursor_disabled(d, p) assert_cursor(d, p, false) + void assert_pipe(struct drm_i915_private *dev_priv, enum pipe pipe, bool state) { @@ -4856,6 +4876,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, const intel_limit_t *limit; int ret; + assert_cursor_disabled(dev_priv, pipe); + for_each_encoder_on_crtc(dev, crtc, encoder) { switch (encoder->type) { case INTEL_OUTPUT_LVDS: @@ -5754,6 +5776,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, struct intel_shared_dpll *pll; int ret; + assert_cursor_disabled(dev_priv, pipe); + for_each_encoder_on_crtc(dev, crtc, encoder) { switch (encoder->type) { case INTEL_OUTPUT_LVDS: @@ -6270,6 +6294,8 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, int plane = intel_crtc->plane; int ret; + assert_cursor_disabled(dev_priv, intel_crtc->pipe); + if (!intel_ddi_pll_mode_set(crtc)) return -EINVAL;
The cursor is supposed to be disabled during crtc mode set (disabled by ctrc disable). Assert this is the case. Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)