Re: [Intel-gfx] [PATCH v2 01/10] drm/i915/skl: Move per-pipe ddb allocations into crtc states

From: Paulo Zanoni
Date: Tue Oct 11 2016 - 15:36:32 EST


Em Sex, 2016-10-07 Ãs 20:11 -0400, Lyude escreveu:
> First part of cleaning up all of the skl watermark code. This moves
> the
> structures for storing the ddb allocations of each pipe into
> intel_crtc_state, along with moving the structures for storing the
> current ddb allocations active on hardware into intel_crtc.
>
> Changes since v1:
> - Don't replace alloc->start = alloc->end = 0;
>
> Signed-off-by: Lyude <cpaul@xxxxxxxxxx>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>

> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Ville SyrjÃlà <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> ---
> Âdrivers/gpu/drm/i915/i915_drv.hÂÂÂÂÂÂ|ÂÂ1 -
> Âdrivers/gpu/drm/i915/intel_display.c | 16 ++++++++-------
> Âdrivers/gpu/drm/i915/intel_drv.hÂÂÂÂÂ|ÂÂ8 +++++---
> Âdrivers/gpu/drm/i915/intel_pm.cÂÂÂÂÂÂ| 40 +++++++++++++++-----------
> ----------
> Â4 files changed, 30 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index a219a35..bb2de8d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1637,7 +1637,6 @@ static inline bool skl_ddb_entry_equal(const
> struct skl_ddb_entry *e1,
> Â}
> Â
> Âstruct skl_ddb_allocation {
> - struct skl_ddb_entry pipe[I915_MAX_PIPES];
> Â struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES];
> /* packed/uv */
> Â struct skl_ddb_entry
> y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
> Â};
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index a366656..17733af 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14235,12 +14235,11 @@ static void skl_update_crtcs(struct
> drm_atomic_state *state,
> Â ÂÂÂÂÂunsigned int *crtc_vblank_mask)
> Â{
> Â struct drm_device *dev = state->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> Â struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> Â struct drm_crtc *crtc;
> + struct intel_crtc *intel_crtc;
> Â struct drm_crtc_state *old_crtc_state;
> - struct skl_ddb_allocation *new_ddb = &intel_state-
> >wm_results.ddb;
> - struct skl_ddb_allocation *cur_ddb = &dev_priv-
> >wm.skl_hw.ddb;
> + struct intel_crtc_state *cstate;
> Â unsigned int updated = 0;
> Â bool progress;
> Â enum pipe pipe;
> @@ -14258,12 +14257,14 @@ static void skl_update_crtcs(struct
> drm_atomic_state *state,
> Â for_each_crtc_in_state(state, crtc, old_crtc_state,
> i) {
> Â bool vbl_wait = false;
> Â unsigned int cmask = drm_crtc_mask(crtc);
> - pipe = to_intel_crtc(crtc)->pipe;
> +
> + intel_crtc = to_intel_crtc(crtc);
> + cstate = to_intel_crtc_state(crtc->state);
> + pipe = intel_crtc->pipe;
> Â
> Â if (updated & cmask || !crtc->state->active)
> Â continue;
> - if (skl_ddb_allocation_overlaps(state,
> cur_ddb, new_ddb,
> - pipe))
> + if (skl_ddb_allocation_overlaps(state,
> intel_crtc))
> Â continue;
> Â
> Â updated |= cmask;
> @@ -14274,7 +14275,8 @@ static void skl_update_crtcs(struct
> drm_atomic_state *state,
> Â Â* then we need to wait for a vblank to pass
> for the
> Â Â* new ddb allocation to take effect.
> Â Â*/
> - if (!skl_ddb_allocation_equals(cur_ddb,
> new_ddb, pipe) &&
> + if (!skl_ddb_entry_equal(&cstate-
> >wm.skl.ddb,
> + Â&intel_crtc-
> >hw_ddb) &&
> Â ÂÂÂÂ!crtc->state->active_changed &&
> Â ÂÂÂÂintel_state->wm_results.dirty_pipes !=
> updated)
> Â vbl_wait = true;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f48e79a..35ba282 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -496,6 +496,7 @@ struct intel_crtc_wm_state {
> Â struct {
> Â /* gen9+ only needs 1-step wm programming */
> Â struct skl_pipe_wm optimal;
> + struct skl_ddb_entry ddb;
> Â
> Â /* cached plane data rate */
> Â unsigned plane_data_rate[I915_MAX_PLANES];
> @@ -733,6 +734,9 @@ struct intel_crtc {
> Â bool cxsr_allowed;
> Â } wm;
> Â
> + /* gen9+: ddb allocation currently being used */
> + struct skl_ddb_entry hw_ddb;
> +
> Â int scanline_offset;
> Â
> Â struct {
> @@ -1755,9 +1759,7 @@ bool skl_ddb_allocation_equals(const struct
> skl_ddb_allocation *old,
> Â ÂÂÂÂÂÂÂconst struct skl_ddb_allocation *new,
> Â ÂÂÂÂÂÂÂenum pipe pipe);
> Âbool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
> - Âconst struct skl_ddb_allocation
> *old,
> - Âconst struct skl_ddb_allocation
> *new,
> - Âenum pipe pipe);
> + Âstruct intel_crtc *intel_crtc);
> Âvoid skl_write_cursor_wm(struct intel_crtc *intel_crtc,
> Â Âconst struct skl_wm_values *wm);
> Âvoid skl_write_plane_wm(struct intel_crtc *intel_crtc,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index fe6c1c6..68355b0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3058,7 +3058,6 @@ skl_ddb_get_pipe_allocation_limits(struct
> drm_device *dev,
> Â struct drm_crtc *for_crtc = cstate->base.crtc;
> Â unsigned int pipe_size, ddb_size;
> Â int nth_active_pipe;
> - int pipe = to_intel_crtc(for_crtc)->pipe;
> Â
> Â if (WARN_ON(!state) || !cstate->base.active) {
> Â alloc->start = 0;
> @@ -3086,7 +3085,7 @@ skl_ddb_get_pipe_allocation_limits(struct
> drm_device *dev,
> Â Â* we currently hold.
> Â Â*/
> Â if (!intel_state->active_pipe_changes) {
> - *alloc = dev_priv->wm.skl_hw.ddb.pipe[pipe];
> + *alloc = to_intel_crtc(for_crtc)->hw_ddb;
> Â return;
> Â }
> Â
> @@ -3354,7 +3353,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state
> *cstate,
> Â struct drm_plane *plane;
> Â struct drm_plane_state *pstate;
> Â enum pipe pipe = intel_crtc->pipe;
> - struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
> + struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
> Â uint16_t alloc_size, start, cursor_blocks;
> Â uint16_t *minimum = cstate->wm.skl.minimum_blocks;
> Â uint16_t *y_minimum = cstate->wm.skl.minimum_y_blocks;
> @@ -3370,7 +3369,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state
> *cstate,
> Â return 0;
> Â
> Â if (!cstate->base.active) {
> - ddb->pipe[pipe].start = ddb->pipe[pipe].end = 0;
> + alloc->start = alloc->end = 0;
> Â return 0;
> Â }
> Â
> @@ -3897,14 +3896,6 @@ void skl_write_cursor_wm(struct intel_crtc
> *intel_crtc,
> Â ÂÂÂÂ&wm->ddb.plane[pipe][PLANE_CURSOR]);
> Â}
> Â
> -bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
> - ÂÂÂÂÂÂÂconst struct skl_ddb_allocation *new,
> - ÂÂÂÂÂÂÂenum pipe pipe)
> -{
> - return new->pipe[pipe].start == old->pipe[pipe].start &&
> - ÂÂÂÂÂÂÂnew->pipe[pipe].end == old->pipe[pipe].end;
> -}
> -
> Âstatic inline bool skl_ddb_entries_overlap(const struct
> skl_ddb_entry *a,
> Â ÂÂÂconst struct
> skl_ddb_entry *b)
> Â{
> @@ -3912,22 +3903,22 @@ static inline bool
> skl_ddb_entries_overlap(const struct skl_ddb_entry *a,
> Â}
> Â
> Âbool skl_ddb_allocation_overlaps(struct drm_atomic_state *state,
> - Âconst struct skl_ddb_allocation
> *old,
> - Âconst struct skl_ddb_allocation
> *new,
> - Âenum pipe pipe)
> + Âstruct intel_crtc *intel_crtc)
> Â{
> - struct drm_device *dev = state->dev;
> - struct intel_crtc *intel_crtc;
> - enum pipe otherp;
> + struct drm_crtc *other_crtc;
> + struct drm_crtc_state *other_cstate;
> + struct intel_crtc *other_intel_crtc;
> + const struct skl_ddb_entry *ddb =
> + &to_intel_crtc_state(intel_crtc->base.state)-
> >wm.skl.ddb;
> + int i;
> Â
> - for_each_intel_crtc(dev, intel_crtc) {
> - otherp = intel_crtc->pipe;
> + for_each_crtc_in_state(state, other_crtc, other_cstate, i) {
> + other_intel_crtc = to_intel_crtc(other_crtc);
> Â
> - if (otherp == pipe)
> + if (other_intel_crtc == intel_crtc)
> Â continue;
> Â
> - if (skl_ddb_entries_overlap(&new->pipe[pipe],
> - ÂÂÂÂ&old->pipe[otherp]))
> + if (skl_ddb_entries_overlap(ddb, &other_intel_crtc-
> >hw_ddb))
> Â return true;
> Â }
> Â
> @@ -4092,7 +4083,6 @@ skl_copy_wm_for_pipe(struct skl_wm_values *dst,
> Â memcpy(dst->plane_trans[pipe], src->plane_trans[pipe],
> Â ÂÂÂÂÂÂÂsizeof(dst->plane_trans[pipe]));
> Â
> - dst->ddb.pipe[pipe] = src->ddb.pipe[pipe];
> Â memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
> Â ÂÂÂÂÂÂÂsizeof(dst->ddb.y_plane[pipe]));
> Â memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
> @@ -4200,6 +4190,8 @@ static void skl_update_wm(struct drm_crtc
> *crtc)
> Â
> Â skl_copy_wm_for_pipe(hw_vals, results, pipe);
> Â
> + intel_crtc->hw_ddb = cstate->wm.skl.ddb;
> +
> Â mutex_unlock(&dev_priv->wm.wm_mutex);
> Â}
> Â