Op 08-10-16 om 02:11 schreef Lyude: > Now that we've make skl_wm_levels make a little more sense, we can > remove all of the redundant wm information. Up until now we'd been > storing two copies of all of the skl watermarks: one being the > skl_pipe_wm structs, the other being the global wm struct in > drm_i915_private containing the raw register values. This is confusing > and problematic, since it means we're prone to accidentally letting the > two copies go out of sync. So, get rid of all of the functions > responsible for computing the register values and just use a single > helper, skl_write_wm_level(), to convert and write the new watermarks on > the fly. > > Changes since v1: > - Fixup skl_write_wm_level() > - Fixup skl_wm_level_from_reg_val() > - Don't forget to copy *active to intel_crtc->wm.active.skl > > Signed-off-by: Lyude <cpaul at redhat.com> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 - > drivers/gpu/drm/i915/intel_display.c | 14 ++- > drivers/gpu/drm/i915/intel_drv.h | 6 +- > drivers/gpu/drm/i915/intel_pm.c | 204 > ++++++++++++----------------------- > drivers/gpu/drm/i915/intel_sprite.c | 8 +- > 5 files changed, 90 insertions(+), 144 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 0287c93..76583b2 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1644,8 +1644,6 @@ struct skl_ddb_allocation { > struct skl_wm_values { > unsigned dirty_pipes; > struct skl_ddb_allocation ddb; > - uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8]; > - uint32_t plane_trans[I915_MAX_PIPES][I915_MAX_PLANES]; > }; > > struct skl_wm_level { > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index a71d05a..39400a0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3378,6 +3378,8 @@ static void skylake_update_primary_plane(struct > drm_plane *plane, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc); > struct drm_framebuffer *fb = plane_state->base.fb; > const struct skl_wm_values *wm = &dev_priv->wm.skl_results; > + const struct skl_plane_wm *p_wm = > + &crtc_state->wm.skl.optimal.planes[0]; > int pipe = intel_crtc->pipe; > u32 plane_ctl; > unsigned int rotation = plane_state->base.rotation; > @@ -3414,7 +3416,7 @@ static void skylake_update_primary_plane(struct > drm_plane *plane, > intel_crtc->adjusted_y = src_y; > > if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base)) > - skl_write_plane_wm(intel_crtc, wm, 0); > + skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0); > > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); > I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x); > @@ -3448,6 +3450,8 @@ static void skylake_disable_primary_plane(struct > drm_plane *primary, > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); > + const struct skl_plane_wm *p_wm = &cstate->wm.skl.optimal.planes[0]; > int pipe = intel_crtc->pipe; > > /* > @@ -3455,7 +3459,8 @@ static void skylake_disable_primary_plane(struct > drm_plane *primary, > * plane's visiblity isn't actually changing neither is its watermarks. > */ > if (!crtc->primary->state->visible) > - skl_write_plane_wm(intel_crtc, &dev_priv->wm.skl_results, 0); > + skl_write_plane_wm(intel_crtc, p_wm, > + &dev_priv->wm.skl_results.ddb, 0); > > I915_WRITE(PLANE_CTL(pipe, 0), 0); > I915_WRITE(PLANE_SURF(pipe, 0), 0); > @@ -10819,12 +10824,15 @@ static void i9xx_update_cursor(struct drm_crtc > *crtc, u32 base, > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); > const struct skl_wm_values *wm = &dev_priv->wm.skl_results; > + const struct skl_plane_wm *p_wm = > + &cstate->wm.skl.optimal.planes[PLANE_CURSOR]; > int pipe = intel_crtc->pipe; > uint32_t cntl = 0; > > if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc)) > - skl_write_cursor_wm(intel_crtc, wm); > + skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb); > > if (plane_state && plane_state->base.visible) { > cntl = MCURSOR_GAMMA_ENABLE; > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index d684f4f..958dc72 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1765,9 +1765,11 @@ bool skl_ddb_allocation_equals(const struct > skl_ddb_allocation *old, > bool skl_ddb_allocation_overlaps(struct drm_atomic_state *state, > struct intel_crtc *intel_crtc); > void skl_write_cursor_wm(struct intel_crtc *intel_crtc, > - const struct skl_wm_values *wm); > + const struct skl_plane_wm *wm, > + const struct skl_ddb_allocation *ddb); > void skl_write_plane_wm(struct intel_crtc *intel_crtc, > - const struct skl_wm_values *wm, > + const struct skl_plane_wm *wm, > + const struct skl_ddb_allocation *ddb, > int plane); > uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config); > bool ilk_disable_lp_wm(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 5dbaf12..5cb537c 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3000,6 +3000,8 @@ bool intel_can_enable_sagv(struct drm_atomic_state > *state) > 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_state *cstate; > + struct skl_plane_wm *wm; > enum pipe pipe; > int level, plane; > > @@ -3020,18 +3022,21 @@ bool intel_can_enable_sagv(struct drm_atomic_state > *state) > /* Since we're now guaranteed to only have one active CRTC... */ > pipe = ffs(intel_state->active_crtcs) - 1; > crtc = dev_priv->pipe_to_crtc_mapping[pipe]; > + cstate = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc)); Wrong cstate here, should be crtc->state since it's committed and crtc_state is the old state.
~Maarten