Re: [Intel-gfx] [PATCH 04/15] drm/i915: Clean up cursor junk from intel_crtc
On Mon, Mar 27, 2017 at 09:55:35PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Move cursor_base, cursor_cntl, and cursor_size from intel_crtc > into intel_plane so that we don't need the crtc for cursor stuff > so much. > > Also entirely nuke cursor_addr which IMO doesn't provide any benefit > since it's not actually used by the cursor code itself. I'm not 100% > sure what the SKL+ DDB is code is after by looking at cursor_addr so > I just make it do its checks unconditionally. If that's not correct > then we should likely replace it with somehting like > plane_state->visible. Yes, AFAICS in case it's not visible the cursor DDB and WM will be still computed (to fixed minimum and 0 accordingly) and programmed. Maybe historical left-over? The code comment about this is also stale then. > > Signed-off-by: Ville Syrjälä Reviewed-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_debugfs.c | 48 +- > drivers/gpu/drm/i915/intel_display.c | 80 > +++- > drivers/gpu/drm/i915/intel_drv.h | 9 ++-- > 3 files changed, 48 insertions(+), 89 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 316bc47a8eea..b9410cb845f3 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -3040,36 +3040,6 @@ static void intel_connector_info(struct seq_file *m, > intel_seq_print_mode(m, 2, mode); > } > > -static bool cursor_active(struct drm_i915_private *dev_priv, int pipe) > -{ > - u32 state; > - > - if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) > - state = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE; > - else > - state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE; > - > - return state; > -} > - > -static bool cursor_position(struct drm_i915_private *dev_priv, > - int pipe, int *x, int *y) > -{ > - u32 pos; > - > - pos = I915_READ(CURPOS(pipe)); > - > - *x = (pos >> CURSOR_X_SHIFT) & CURSOR_POS_MASK; > - if (pos & (CURSOR_POS_SIGN << CURSOR_X_SHIFT)) > - *x = -*x; > - > - *y = (pos >> CURSOR_Y_SHIFT) & CURSOR_POS_MASK; > - if (pos & (CURSOR_POS_SIGN << CURSOR_Y_SHIFT)) > - *y = -*y; > - > - return cursor_active(dev_priv, pipe); > -} > - > static const char *plane_type(enum drm_plane_type type) > { > switch (type) { > @@ -3191,9 +3161,7 @@ static int i915_display_info(struct seq_file *m, void > *unused) > seq_printf(m, "CRTC info\n"); > seq_printf(m, "-\n"); > for_each_intel_crtc(dev, crtc) { > - bool active; > struct intel_crtc_state *pipe_config; > - int x, y; > > drm_modeset_lock(&crtc->base.mutex, NULL); > pipe_config = to_intel_crtc_state(crtc->base.state); > @@ -3205,14 +3173,18 @@ static int i915_display_info(struct seq_file *m, void > *unused) > yesno(pipe_config->dither), pipe_config->pipe_bpp); > > if (pipe_config->base.active) { > + struct intel_plane *cursor = > + to_intel_plane(crtc->base.cursor); > + > intel_crtc_info(m, crtc); > > - active = cursor_position(dev_priv, crtc->pipe, &x, &y); > - seq_printf(m, "\tcursor visible? %s, position (%d, %d), > size %dx%d, addr 0x%08x, active? %s\n", > -yesno(crtc->cursor_base), > -x, y, crtc->base.cursor->state->crtc_w, > -crtc->base.cursor->state->crtc_h, > -crtc->cursor_addr, yesno(active)); > + seq_printf(m, "\tcursor visible? %s, position (%d, %d), > size %dx%d, addr 0x%08x\n", > +yesno(cursor->base.state->visible), > +cursor->base.state->crtc_x, > +cursor->base.state->crtc_y, > +cursor->base.state->crtc_w, > +cursor->base.state->crtc_h, > +cursor->cursor.base); > intel_scaler_info(m, crtc); > intel_plane_info(m, crtc); > } > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 5e04f64a0f76..1d55fac397ad 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9126,8 +9126,7 @@ static bool haswell_get_pipe_config(struct intel_crtc > *crtc, > return active; > } > > -static u32 intel_cursor_base(struct intel_crtc *crtc, > - const struct intel_plane_state *plane_state) > +static u32 intel_cursor_base(const struct intel_plane_state *plane_state) > { > struct drm_i915_private *dev_pr
[Intel-gfx] [PATCH 04/15] drm/i915: Clean up cursor junk from intel_crtc
From: Ville Syrjälä Move cursor_base, cursor_cntl, and cursor_size from intel_crtc into intel_plane so that we don't need the crtc for cursor stuff so much. Also entirely nuke cursor_addr which IMO doesn't provide any benefit since it's not actually used by the cursor code itself. I'm not 100% sure what the SKL+ DDB is code is after by looking at cursor_addr so I just make it do its checks unconditionally. If that's not correct then we should likely replace it with somehting like plane_state->visible. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_debugfs.c | 48 +- drivers/gpu/drm/i915/intel_display.c | 80 +++- drivers/gpu/drm/i915/intel_drv.h | 9 ++-- 3 files changed, 48 insertions(+), 89 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 316bc47a8eea..b9410cb845f3 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3040,36 +3040,6 @@ static void intel_connector_info(struct seq_file *m, intel_seq_print_mode(m, 2, mode); } -static bool cursor_active(struct drm_i915_private *dev_priv, int pipe) -{ - u32 state; - - if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) - state = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE; - else - state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE; - - return state; -} - -static bool cursor_position(struct drm_i915_private *dev_priv, - int pipe, int *x, int *y) -{ - u32 pos; - - pos = I915_READ(CURPOS(pipe)); - - *x = (pos >> CURSOR_X_SHIFT) & CURSOR_POS_MASK; - if (pos & (CURSOR_POS_SIGN << CURSOR_X_SHIFT)) - *x = -*x; - - *y = (pos >> CURSOR_Y_SHIFT) & CURSOR_POS_MASK; - if (pos & (CURSOR_POS_SIGN << CURSOR_Y_SHIFT)) - *y = -*y; - - return cursor_active(dev_priv, pipe); -} - static const char *plane_type(enum drm_plane_type type) { switch (type) { @@ -3191,9 +3161,7 @@ static int i915_display_info(struct seq_file *m, void *unused) seq_printf(m, "CRTC info\n"); seq_printf(m, "-\n"); for_each_intel_crtc(dev, crtc) { - bool active; struct intel_crtc_state *pipe_config; - int x, y; drm_modeset_lock(&crtc->base.mutex, NULL); pipe_config = to_intel_crtc_state(crtc->base.state); @@ -3205,14 +3173,18 @@ static int i915_display_info(struct seq_file *m, void *unused) yesno(pipe_config->dither), pipe_config->pipe_bpp); if (pipe_config->base.active) { + struct intel_plane *cursor = + to_intel_plane(crtc->base.cursor); + intel_crtc_info(m, crtc); - active = cursor_position(dev_priv, crtc->pipe, &x, &y); - seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x, active? %s\n", - yesno(crtc->cursor_base), - x, y, crtc->base.cursor->state->crtc_w, - crtc->base.cursor->state->crtc_h, - crtc->cursor_addr, yesno(active)); + seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x\n", + yesno(cursor->base.state->visible), + cursor->base.state->crtc_x, + cursor->base.state->crtc_y, + cursor->base.state->crtc_w, + cursor->base.state->crtc_h, + cursor->cursor.base); intel_scaler_info(m, crtc); intel_plane_info(m, crtc); } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5e04f64a0f76..1d55fac397ad 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9126,8 +9126,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, return active; } -static u32 intel_cursor_base(struct intel_crtc *crtc, -const struct intel_plane_state *plane_state) +static u32 intel_cursor_base(const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(plane_state->base.plane->dev); @@ -9140,8 +9139,6 @@ static u32 intel_cursor_base(struct intel_crtc *crtc, else base = intel_plane_ggtt_offset(plane_state); - crtc->cursor_addr = base; - /* ILK+ do this automagically */ if (HAS_GMCH_DISPLAY(dev_priv) && plane_state->base.rotation & DRM_ROTATE_180) @@ -9176,12 +9173,10 @@ static u32 i845_cursor_ctl(const struct