| Message ID | 1440119524-13867-12-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2015-08-20 at 18:12 -0700, Matt Roper wrote: > In addition to calculating final watermarks, let's also pre-calculate a > set of intermediate watermark values at atomic check time. These > intermediate watermarks are a combination of the watermarks for the old > state and the new state; they should satisfy the requirements of both > states which means they can be programmed immediately when we commit the > atomic state (without waiting for a vblank). Once the vblank does > happen, we can then re-program watermarks to the more optimal final > value. > > v2: Significant rebasing/rewriting. > > v3: > - Move 'need_postvbl_update' flag to CRTC state (Daniel) > - Don't forget to check intermediate watermark values for validity > (Maarten) > - Don't due async watermark optimization; just do it at the end of the > atomic transaction, after waiting for vblanks. We do want it to be > async eventually, but adding that now will cause more trouble for > Maarten's in-progress work. (Maarten) > - Don't allocate space in crtc_state for intermediate watermarks on > platforms that don't need it (gen9+). > - Move WaCxSRDisabledForSpriteScaling:ivb into intel_begin_crtc_commit > now that ilk_update_wm is gone. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 5 ++ > drivers/gpu/drm/i915/intel_display.c | 77 ++++++++++++++++++- > drivers/gpu/drm/i915/intel_drv.h | 33 ++++++-- > drivers/gpu/drm/i915/intel_pm.c | 144 ++++++++++++++++++++++++----------- > 4 files changed, 208 insertions(+), 51 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index ac13cd7..be42dd8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -622,6 +622,11 @@ struct drm_i915_display_funcs { > struct dpll *best_clock); > int (*compute_pipe_wm)(struct drm_crtc *crtc, > struct drm_atomic_state *state); > + int (*compute_intermediate_wm)(struct drm_device *dev, > + struct intel_crtc_state *newstate, > + const struct intel_crtc_state *oldstate); > + void (*program_watermarks)(struct drm_i915_private *dev_priv); > + void (*optimize_watermarks)(struct intel_crtc_state *cstate); > void (*update_wm)(struct drm_crtc *crtc); > int (*modeset_calc_cdclk)(struct drm_atomic_state *state); > void (*modeset_commit_cdclk)(struct drm_atomic_state *state); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 8e9d87a..f929676 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11634,6 +11634,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, > } > } else if (intel_wm_need_update(plane, plane_state)) { > intel_crtc->atomic.update_wm_pre = true; > + > + /* Pre-gen9 platforms need two-step watermark updates */ > + if (INTEL_INFO(dev)->gen < 9 && > + dev_priv->display.program_watermarks) > + cstate->wm.need_postvbl_update = true; > } > > if (visible) > @@ -11769,6 +11774,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_crtc_state *pipe_config = > to_intel_crtc_state(crtc_state); > + struct intel_crtc_state *old_pipe_config = > + to_intel_crtc_state(crtc->state); > struct drm_atomic_state *state = crtc_state->state; > int ret; > bool mode_changed = needs_modeset(crtc_state); > @@ -11793,8 +11800,28 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > ret = 0; > if (dev_priv->display.compute_pipe_wm) { > ret = dev_priv->display.compute_pipe_wm(crtc, state); > - if (ret) > + if (ret) { > + DRM_DEBUG_KMS("Target pipe watermarks are invalid\n"); > + return ret; > + } > + } > + > + if (dev_priv->display.compute_intermediate_wm) { > + if (WARN_ON(!dev_priv->display.compute_pipe_wm)) > + return 0; > + > + /* > + * Calculate 'intermediate' watermarks that satisfy both the > + * old state and the new state. We can program these > + * immediately. > + */ > + ret = dev_priv->display.compute_intermediate_wm(crtc->dev, > + pipe_config, > + old_pipe_config); > + if (ret) { > + DRM_DEBUG_KMS("No valid intermediate pipe watermarks are possible\n"); > return ret; > + } > } > > if (INTEL_INFO(dev)->gen >= 9) { > @@ -13149,6 +13176,7 @@ static int intel_atomic_commit(struct drm_device *dev, > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_crtc *crtc; > struct drm_crtc_state *crtc_state; > + struct intel_crtc_state *intel_cstate; > int ret = 0; > int i; > bool any_ms = false; > @@ -13213,6 +13241,21 @@ static int intel_atomic_commit(struct drm_device *dev, > /* FIXME: add subpixel order */ > > drm_atomic_helper_wait_for_vblanks(dev, state); > + > + /* > + * Now that the vblank has passed, we can go ahead and program the > + * optimal watermarks on platforms that need two-step watermark > + * programming. > + * > + * TODO: Move this (and other cleanup) to an async worker eventually. > + */ > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + intel_cstate = to_intel_crtc_state(crtc->state); > + > + if (intel_cstate->wm.need_postvbl_update) > + dev_priv->display.optimize_watermarks(intel_cstate); > + } > + > drm_atomic_helper_cleanup_planes(dev, state); > > if (any_ms) > @@ -13544,10 +13587,40 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, > struct drm_crtc_state *old_crtc_state) > { > struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > - if (intel_crtc->atomic.update_wm_pre) > + /* > + * IVB workaround: must disable low power watermarks for at least > + * one frame before enabling scaling. LP watermarks can be re-enabled > + * when scaling is disabled. > + * > + * WaCxSRDisabledForSpriteScaling:ivb > + */ > + if (to_intel_crtc_state(crtc->state)->disable_lp_wm) { > + ilk_disable_lp_wm(crtc->dev); > + intel_wait_for_vblank(crtc->dev, intel_crtc->pipe); > + } > + > + /* > + * For platforms that support atomic watermarks, program the 'active' > + * watermarks immediately. On pre-gen9 platforms, these will be the > + * intermediate values that are safe for both pre- and post- vblank; > + * when vblank happens, the 'active' values will be set to the final > + * 'target' values and we'll do this again to get the optimal > + * watermarks. For gen9+ platforms, the values we program here will be > + * the final target values which will get automatically latched at > + * vblank time; no further programming will be necessary. > + * > + * If a platform hasn't been transitioned to atomic watermarks yet, > + * we'll continue to update watermarks the old way, if flags tell > + * us to. > + */ > + if (dev_priv->display.program_watermarks != NULL) { > + dev_priv->display.program_watermarks(dev_priv); > + } else if (intel_crtc->atomic.update_wm_pre) { > intel_update_watermarks(crtc); > + } > > /* Perform vblank evasion around commit operation */ > if (crtc->state->active) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 6ac1010c..6a73a53 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -476,14 +476,34 @@ struct intel_crtc_state { > bool disable_lp_wm; > > struct { > - /* > - * final watermarks, programmed post-vblank when this state > - * is committed > - */ > union { > - struct intel_pipe_wm ilk; > + struct { > + /* > + * Final, target watermarks for state; this > + * might not have been programmed yet if a > + * vblank hasn't happened since this state > + * was committed. > + */ > + struct intel_pipe_wm target; > + > + /* > + * currently programmed watermark; this will be > + * the intermediate values before vblank then > + * switch to match 'target' after vblank fires > + */ > + struct intel_pipe_wm active; > + } ilk; > + > struct skl_pipe_wm skl; > - } active; > + }; > + > + /* > + * Platforms with two-step watermark programming will need to > + * update watermark programming post-vblank to switch from the > + * safe intermediate watermarks to the optimal final > + * watermarks. > + */ > + bool need_postvbl_update; > } wm; > }; > > @@ -1385,6 +1405,7 @@ void skl_wm_get_hw_state(struct drm_device *dev); > void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, > struct skl_ddb_allocation *ddb /* out */); > uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config); > +bool ilk_disable_lp_wm(struct drm_device *dev); > > /* intel_sdvo.c */ > bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index b32d6b0..e0a4c4d 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2315,6 +2315,29 @@ static void skl_setup_wm_latency(struct drm_device *dev) > intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency); > } > > +static bool ilk_validate_pipe_wm(struct drm_device *dev, > + struct intel_pipe_wm *pipe_wm) > +{ > + /* LP0 watermark maximums depend on this pipe alone */ > + const struct intel_wm_config config = { > + .num_pipes_active = 1, > + .sprites_enabled = pipe_wm->sprites_enabled, > + .sprites_scaled = pipe_wm->sprites_scaled, > + }; > + struct ilk_wm_maximums max; > + > + /* LP0 watermarks always use 1/2 DDB partitioning */ > + ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max); > + > + /* At least LP0 must be valid */ > + if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) { > + DRM_DEBUG_KMS("LP0 watermark invalid\n"); > + return false; > + } > + > + return true; > +} > + > /* Compute new watermarks for the pipe */ > static int ilk_compute_pipe_wm(struct drm_crtc *crtc, > struct drm_atomic_state *state) > @@ -2331,10 +2354,6 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, > struct intel_plane_state *sprstate = NULL; > struct intel_plane_state *curstate = NULL; > int level, max_level = ilk_wm_max_level(dev); > - /* LP0 watermark maximums depend on this pipe alone */ > - struct intel_wm_config config = { > - .num_pipes_active = 1, > - }; > struct ilk_wm_maximums max; > > cs = drm_atomic_get_crtc_state(state, crtc); > @@ -2343,7 +2362,7 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, > else > cstate = to_intel_crtc_state(cs); > > - pipe_wm = &cstate->wm.active.ilk; > + pipe_wm = &cstate->wm.ilk.target; > > for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { > ps = drm_atomic_get_plane_state(state, > @@ -2359,21 +2378,18 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, > curstate = to_intel_plane_state(ps); > } > > - config.sprites_enabled = sprstate->visible; > - config.sprites_scaled = > + pipe_wm->pipe_enabled = cstate->base.active; > + pipe_wm->sprites_enabled = sprstate->visible; > + pipe_wm->sprites_scaled = > drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 || > drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16; > > - pipe_wm->pipe_enabled = cstate->base.active; > - pipe_wm->sprites_enabled = config.sprites_enabled; > - pipe_wm->sprites_scaled = config.sprites_scaled; > - > /* ILK/SNB: LP2+ watermarks only w/o sprites */ > if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible) > max_level = 1; > > /* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */ > - if (config.sprites_scaled) > + if (pipe_wm->sprites_scaled) > max_level = 0; > > ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > @@ -2382,12 +2398,8 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, > if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc); > > - /* LP0 watermarks always use 1/2 DDB partitioning */ > - ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max); > - > - /* At least LP0 must be valid */ > - if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) > - return -EINVAL; > + if (!ilk_validate_pipe_wm(dev, pipe_wm)) > + return false; > > ilk_compute_wm_reg_maximums(dev, 1, &max); > > @@ -2412,6 +2424,60 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, > } > > /* > + * Build a set of 'intermediate' watermark values that satisfy both the old > + * state and the new state. These can be programmed to the hardware > + * immediately. > + */ > +static int ilk_compute_intermediate_wm(struct drm_device *dev, > + struct intel_crtc_state *newstate, > + const struct intel_crtc_state *oldstate) > +{ > + struct intel_pipe_wm *a = &newstate->wm.ilk.active; Shouldn't this be ilk.target? Also, I get a black screen with this patch on my HSW laptop. On boot, the level 0 wm has spr_val set to 56, but the the hw state readout code doesn't set sprites_enabled properly. When the merged active and target watermarks are checked for validity, the max value allowed for spr_val is 0 because sprites_enabled is 0, and that causes every modeset to fail. Ander > + const struct intel_pipe_wm *b = &oldstate->wm.ilk.active; > + int level, max_level = ilk_wm_max_level(dev); > + > + /* > + * Start with the final, target watermarks, then combine with the > + * current state's watermarks to get values that are safe both > + * before and after the vblank. > + */ > + *a = newstate->wm.ilk.target; > + a->pipe_enabled |= b->pipe_enabled; > + a->sprites_enabled |= b->sprites_enabled; > + a->sprites_scaled |= b->sprites_scaled; > + > + for (level = 0; level <= max_level; level++) { > + struct intel_wm_level *a_wm = &a->wm[level]; > + const struct intel_wm_level *b_wm = &b->wm[level]; > + > + a_wm->enable &= b_wm->enable; > + a_wm->pri_val = max(a_wm->pri_val, b_wm->pri_val); > + a_wm->spr_val = max(a_wm->spr_val, b_wm->spr_val); > + a_wm->cur_val = max(a_wm->cur_val, b_wm->cur_val); > + a_wm->fbc_val = max(a_wm->fbc_val, b_wm->fbc_val); > + } > + > + /* > + * We need to make sure that these merged watermark values are > + * actually a valid configuration themselves. If they're not, > + * there's no safe way to transition from the old state to > + * the new state, so we need to fail the atomic transaction. > + */ > + if (!ilk_validate_pipe_wm(dev, a)) > + return -EINVAL; > + > + /* > + * If our intermediate WM are identical to the final WM, then we can > + * omit the post-vblank programming. > + */ > + if (!memcmp(&newstate->wm.ilk.active, &newstate->wm.ilk.target, > + sizeof *a)) > + newstate->wm.need_postvbl_update = false; > + > + return 0; > +} > + > +/* > * Merge the watermarks from all active pipes for a specific level. > */ > static void ilk_merge_wm_level(struct drm_device *dev, > @@ -2425,7 +2491,7 @@ static void ilk_merge_wm_level(struct drm_device *dev, > for_each_intel_crtc(dev, intel_crtc) { > const struct intel_crtc_state *cstate = > to_intel_crtc_state(intel_crtc->base.state); > - const struct intel_pipe_wm *active = &cstate->wm.active.ilk; > + const struct intel_pipe_wm *active = &cstate->wm.ilk.active; > const struct intel_wm_level *wm = &active->wm[level]; > > if (!active->pipe_enabled) > @@ -2576,12 +2642,12 @@ static void ilk_compute_wm_results(struct drm_device *dev, > const struct intel_crtc_state *cstate = > to_intel_crtc_state(intel_crtc->base.state); > enum pipe pipe = intel_crtc->pipe; > - const struct intel_wm_level *r = &cstate->wm.active.ilk.wm[0]; > + const struct intel_wm_level *r = &cstate->wm.ilk.active.wm[0]; > > if (WARN_ON(!r->enable)) > continue; > > - results->wm_linetime[pipe] = cstate->wm.active.ilk.linetime; > + results->wm_linetime[pipe] = cstate->wm.ilk.active.linetime; > > results->wm_pipe[pipe] = > (r->pri_val << WM0_PIPE_PLANE_SHIFT) | > @@ -2788,7 +2854,7 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv, > dev_priv->wm.hw = *results; > } > > -static bool ilk_disable_lp_wm(struct drm_device *dev) > +bool ilk_disable_lp_wm(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > @@ -3544,10 +3610,10 @@ static bool skl_update_pipe_wm(struct drm_crtc *crtc, > skl_allocate_pipe_ddb(crtc, cstate, ddb); > skl_compute_pipe_wm(crtc, ddb, cstate, pipe_wm); > > - if (!memcmp(&cstate->wm.active.skl, pipe_wm, sizeof(*pipe_wm))) > + if (!memcmp(&cstate->wm.skl, pipe_wm, sizeof(*pipe_wm))) > return false; > > - cstate->wm.active.skl = *pipe_wm; > + cstate->wm.skl = *pipe_wm; > > return true; > } > @@ -3653,24 +3719,11 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv) > ilk_write_wm_values(dev_priv, &results); > } > > -static void ilk_update_wm(struct drm_crtc *crtc) > +static void ilk_optimize_watermarks(struct intel_crtc_state *cstate) > { > - struct drm_i915_private *dev_priv = to_i915(crtc->dev); > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); > - > - /* > - * IVB workaround: must disable low power watermarks for at least > - * one frame before enabling scaling. LP watermarks can be re-enabled > - * when scaling is disabled. > - * > - * WaCxSRDisabledForSpriteScaling:ivb > - */ > - if (cstate->disable_lp_wm) { > - ilk_disable_lp_wm(crtc->dev); > - intel_wait_for_vblank(crtc->dev, intel_crtc->pipe); > - } > + struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); > > + cstate->wm.ilk.active = cstate->wm.ilk.target; > ilk_program_watermarks(dev_priv); > } > > @@ -3725,7 +3778,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc) > struct skl_wm_values *hw = &dev_priv->wm.skl_hw; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); > - struct skl_pipe_wm *active = &cstate->wm.active.skl; > + struct skl_pipe_wm *active = &cstate->wm.skl; > enum pipe pipe = intel_crtc->pipe; > int level, i, max_level; > uint32_t temp; > @@ -3789,7 +3842,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc) > struct ilk_wm_values *hw = &dev_priv->wm.hw; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); > - struct intel_pipe_wm *active = &cstate->wm.active.ilk; > + struct intel_pipe_wm *active = &cstate->wm.ilk.active; > enum pipe pipe = intel_crtc->pipe; > static const unsigned int wm0_pipe_reg[] = { > [PIPE_A] = WM0_PIPEA_ILK, > @@ -3828,6 +3881,8 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc) > for (level = 0; level <= max_level; level++) > active->wm[level].enable = true; > } > + > + cstate->wm.ilk.target = *active; > } > > #define _FW_WM(value, plane) \ > @@ -6949,8 +7004,11 @@ void intel_init_pm(struct drm_device *dev) > dev_priv->wm.spr_latency[1] && dev_priv->wm.cur_latency[1]) || > (!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] && > dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) { > - dev_priv->display.update_wm = ilk_update_wm; > dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm; > + dev_priv->display.compute_intermediate_wm = > + ilk_compute_intermediate_wm; > + dev_priv->display.program_watermarks = ilk_program_watermarks; > + dev_priv->display.optimize_watermarks = ilk_optimize_watermarks; > } else { > DRM_DEBUG_KMS("Failed to read display plane latency. " > "Disable CxSR\n");
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ac13cd7..be42dd8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -622,6 +622,11 @@ struct drm_i915_display_funcs { struct dpll *best_clock); int (*compute_pipe_wm)(struct drm_crtc *crtc, struct drm_atomic_state *state); + int (*compute_intermediate_wm)(struct drm_device *dev, + struct intel_crtc_state *newstate, + const struct intel_crtc_state *oldstate); + void (*program_watermarks)(struct drm_i915_private *dev_priv); + void (*optimize_watermarks)(struct intel_crtc_state *cstate); void (*update_wm)(struct drm_crtc *crtc); int (*modeset_calc_cdclk)(struct drm_atomic_state *state); void (*modeset_commit_cdclk)(struct drm_atomic_state *state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8e9d87a..f929676 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11634,6 +11634,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, } } else if (intel_wm_need_update(plane, plane_state)) { intel_crtc->atomic.update_wm_pre = true; + + /* Pre-gen9 platforms need two-step watermark updates */ + if (INTEL_INFO(dev)->gen < 9 && + dev_priv->display.program_watermarks) + cstate->wm.need_postvbl_update = true; } if (visible) @@ -11769,6 +11774,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state); + struct intel_crtc_state *old_pipe_config = + to_intel_crtc_state(crtc->state); struct drm_atomic_state *state = crtc_state->state; int ret; bool mode_changed = needs_modeset(crtc_state); @@ -11793,8 +11800,28 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, ret = 0; if (dev_priv->display.compute_pipe_wm) { ret = dev_priv->display.compute_pipe_wm(crtc, state); - if (ret) + if (ret) { + DRM_DEBUG_KMS("Target pipe watermarks are invalid\n"); + return ret; + } + } + + if (dev_priv->display.compute_intermediate_wm) { + if (WARN_ON(!dev_priv->display.compute_pipe_wm)) + return 0; + + /* + * Calculate 'intermediate' watermarks that satisfy both the + * old state and the new state. We can program these + * immediately. + */ + ret = dev_priv->display.compute_intermediate_wm(crtc->dev, + pipe_config, + old_pipe_config); + if (ret) { + DRM_DEBUG_KMS("No valid intermediate pipe watermarks are possible\n"); return ret; + } } if (INTEL_INFO(dev)->gen >= 9) { @@ -13149,6 +13176,7 @@ static int intel_atomic_commit(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; + struct intel_crtc_state *intel_cstate; int ret = 0; int i; bool any_ms = false; @@ -13213,6 +13241,21 @@ static int intel_atomic_commit(struct drm_device *dev, /* FIXME: add subpixel order */ drm_atomic_helper_wait_for_vblanks(dev, state); + + /* + * Now that the vblank has passed, we can go ahead and program the + * optimal watermarks on platforms that need two-step watermark + * programming. + * + * TODO: Move this (and other cleanup) to an async worker eventually. + */ + for_each_crtc_in_state(state, crtc, crtc_state, i) { + intel_cstate = to_intel_crtc_state(crtc->state); + + if (intel_cstate->wm.need_postvbl_update) + dev_priv->display.optimize_watermarks(intel_cstate); + } + drm_atomic_helper_cleanup_planes(dev, state); if (any_ms) @@ -13544,10 +13587,40 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - if (intel_crtc->atomic.update_wm_pre) + /* + * IVB workaround: must disable low power watermarks for at least + * one frame before enabling scaling. LP watermarks can be re-enabled + * when scaling is disabled. + * + * WaCxSRDisabledForSpriteScaling:ivb + */ + if (to_intel_crtc_state(crtc->state)->disable_lp_wm) { + ilk_disable_lp_wm(crtc->dev); + intel_wait_for_vblank(crtc->dev, intel_crtc->pipe); + } + + /* + * For platforms that support atomic watermarks, program the 'active' + * watermarks immediately. On pre-gen9 platforms, these will be the + * intermediate values that are safe for both pre- and post- vblank; + * when vblank happens, the 'active' values will be set to the final + * 'target' values and we'll do this again to get the optimal + * watermarks. For gen9+ platforms, the values we program here will be + * the final target values which will get automatically latched at + * vblank time; no further programming will be necessary. + * + * If a platform hasn't been transitioned to atomic watermarks yet, + * we'll continue to update watermarks the old way, if flags tell + * us to. + */ + if (dev_priv->display.program_watermarks != NULL) { + dev_priv->display.program_watermarks(dev_priv); + } else if (intel_crtc->atomic.update_wm_pre) { intel_update_watermarks(crtc); + } /* Perform vblank evasion around commit operation */ if (crtc->state->active) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 6ac1010c..6a73a53 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -476,14 +476,34 @@ struct intel_crtc_state { bool disable_lp_wm; struct { - /* - * final watermarks, programmed post-vblank when this state - * is committed - */ union { - struct intel_pipe_wm ilk; + struct { + /* + * Final, target watermarks for state; this + * might not have been programmed yet if a + * vblank hasn't happened since this state + * was committed. + */ + struct intel_pipe_wm target; + + /* + * currently programmed watermark; this will be + * the intermediate values before vblank then + * switch to match 'target' after vblank fires + */ + struct intel_pipe_wm active; + } ilk; + struct skl_pipe_wm skl; - } active; + }; + + /* + * Platforms with two-step watermark programming will need to + * update watermark programming post-vblank to switch from the + * safe intermediate watermarks to the optimal final + * watermarks. + */ + bool need_postvbl_update; } wm; }; @@ -1385,6 +1405,7 @@ void skl_wm_get_hw_state(struct drm_device *dev); void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, struct skl_ddb_allocation *ddb /* out */); uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config); +bool ilk_disable_lp_wm(struct drm_device *dev); /* intel_sdvo.c */ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b32d6b0..e0a4c4d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2315,6 +2315,29 @@ static void skl_setup_wm_latency(struct drm_device *dev) intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency); } +static bool ilk_validate_pipe_wm(struct drm_device *dev, + struct intel_pipe_wm *pipe_wm) +{ + /* LP0 watermark maximums depend on this pipe alone */ + const struct intel_wm_config config = { + .num_pipes_active = 1, + .sprites_enabled = pipe_wm->sprites_enabled, + .sprites_scaled = pipe_wm->sprites_scaled, + }; + struct ilk_wm_maximums max; + + /* LP0 watermarks always use 1/2 DDB partitioning */ + ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max); + + /* At least LP0 must be valid */ + if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) { + DRM_DEBUG_KMS("LP0 watermark invalid\n"); + return false; + } + + return true; +} + /* Compute new watermarks for the pipe */ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, struct drm_atomic_state *state) @@ -2331,10 +2354,6 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, struct intel_plane_state *sprstate = NULL; struct intel_plane_state *curstate = NULL; int level, max_level = ilk_wm_max_level(dev); - /* LP0 watermark maximums depend on this pipe alone */ - struct intel_wm_config config = { - .num_pipes_active = 1, - }; struct ilk_wm_maximums max; cs = drm_atomic_get_crtc_state(state, crtc); @@ -2343,7 +2362,7 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, else cstate = to_intel_crtc_state(cs); - pipe_wm = &cstate->wm.active.ilk; + pipe_wm = &cstate->wm.ilk.target; for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { ps = drm_atomic_get_plane_state(state, @@ -2359,21 +2378,18 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, curstate = to_intel_plane_state(ps); } - config.sprites_enabled = sprstate->visible; - config.sprites_scaled = + pipe_wm->pipe_enabled = cstate->base.active; + pipe_wm->sprites_enabled = sprstate->visible; + pipe_wm->sprites_scaled = drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 || drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16; - pipe_wm->pipe_enabled = cstate->base.active; - pipe_wm->sprites_enabled = config.sprites_enabled; - pipe_wm->sprites_scaled = config.sprites_scaled; - /* ILK/SNB: LP2+ watermarks only w/o sprites */ if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible) max_level = 1; /* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */ - if (config.sprites_scaled) + if (pipe_wm->sprites_scaled) max_level = 0; ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, @@ -2382,12 +2398,8 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, if (IS_HASWELL(dev) || IS_BROADWELL(dev)) pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc); - /* LP0 watermarks always use 1/2 DDB partitioning */ - ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max); - - /* At least LP0 must be valid */ - if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) - return -EINVAL; + if (!ilk_validate_pipe_wm(dev, pipe_wm)) + return false; ilk_compute_wm_reg_maximums(dev, 1, &max); @@ -2412,6 +2424,60 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc, } /* + * Build a set of 'intermediate' watermark values that satisfy both the old + * state and the new state. These can be programmed to the hardware + * immediately. + */ +static int ilk_compute_intermediate_wm(struct drm_device *dev, + struct intel_crtc_state *newstate, + const struct intel_crtc_state *oldstate) +{ + struct intel_pipe_wm *a = &newstate->wm.ilk.active; + const struct intel_pipe_wm *b = &oldstate->wm.ilk.active; + int level, max_level = ilk_wm_max_level(dev); + + /* + * Start with the final, target watermarks, then combine with the + * current state's watermarks to get values that are safe both + * before and after the vblank. + */ + *a = newstate->wm.ilk.target; + a->pipe_enabled |= b->pipe_enabled; + a->sprites_enabled |= b->sprites_enabled; + a->sprites_scaled |= b->sprites_scaled; + + for (level = 0; level <= max_level; level++) { + struct intel_wm_level *a_wm = &a->wm[level]; + const struct intel_wm_level *b_wm = &b->wm[level]; + + a_wm->enable &= b_wm->enable; + a_wm->pri_val = max(a_wm->pri_val, b_wm->pri_val); + a_wm->spr_val = max(a_wm->spr_val, b_wm->spr_val); + a_wm->cur_val = max(a_wm->cur_val, b_wm->cur_val); + a_wm->fbc_val = max(a_wm->fbc_val, b_wm->fbc_val); + } + + /* + * We need to make sure that these merged watermark values are + * actually a valid configuration themselves. If they're not, + * there's no safe way to transition from the old state to + * the new state, so we need to fail the atomic transaction. + */ + if (!ilk_validate_pipe_wm(dev, a)) + return -EINVAL; + + /* + * If our intermediate WM are identical to the final WM, then we can + * omit the post-vblank programming. + */ + if (!memcmp(&newstate->wm.ilk.active, &newstate->wm.ilk.target, + sizeof *a)) + newstate->wm.need_postvbl_update = false; + + return 0; +} + +/* * Merge the watermarks from all active pipes for a specific level. */ static void ilk_merge_wm_level(struct drm_device *dev, @@ -2425,7 +2491,7 @@ static void ilk_merge_wm_level(struct drm_device *dev, for_each_intel_crtc(dev, intel_crtc) { const struct intel_crtc_state *cstate = to_intel_crtc_state(intel_crtc->base.state); - const struct intel_pipe_wm *active = &cstate->wm.active.ilk; + const struct intel_pipe_wm *active = &cstate->wm.ilk.active; const struct intel_wm_level *wm = &active->wm[level]; if (!active->pipe_enabled) @@ -2576,12 +2642,12 @@ static void ilk_compute_wm_results(struct drm_device *dev, const struct intel_crtc_state *cstate = to_intel_crtc_state(intel_crtc->base.state); enum pipe pipe = intel_crtc->pipe; - const struct intel_wm_level *r = &cstate->wm.active.ilk.wm[0]; + const struct intel_wm_level *r = &cstate->wm.ilk.active.wm[0]; if (WARN_ON(!r->enable)) continue; - results->wm_linetime[pipe] = cstate->wm.active.ilk.linetime; + results->wm_linetime[pipe] = cstate->wm.ilk.active.linetime; results->wm_pipe[pipe] = (r->pri_val << WM0_PIPE_PLANE_SHIFT) | @@ -2788,7 +2854,7 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv, dev_priv->wm.hw = *results; } -static bool ilk_disable_lp_wm(struct drm_device *dev) +bool ilk_disable_lp_wm(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -3544,10 +3610,10 @@ static bool skl_update_pipe_wm(struct drm_crtc *crtc, skl_allocate_pipe_ddb(crtc, cstate, ddb); skl_compute_pipe_wm(crtc, ddb, cstate, pipe_wm); - if (!memcmp(&cstate->wm.active.skl, pipe_wm, sizeof(*pipe_wm))) + if (!memcmp(&cstate->wm.skl, pipe_wm, sizeof(*pipe_wm))) return false; - cstate->wm.active.skl = *pipe_wm; + cstate->wm.skl = *pipe_wm; return true; } @@ -3653,24 +3719,11 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv) ilk_write_wm_values(dev_priv, &results); } -static void ilk_update_wm(struct drm_crtc *crtc) +static void ilk_optimize_watermarks(struct intel_crtc_state *cstate) { - struct drm_i915_private *dev_priv = to_i915(crtc->dev); - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); - - /* - * IVB workaround: must disable low power watermarks for at least - * one frame before enabling scaling. LP watermarks can be re-enabled - * when scaling is disabled. - * - * WaCxSRDisabledForSpriteScaling:ivb - */ - if (cstate->disable_lp_wm) { - ilk_disable_lp_wm(crtc->dev); - intel_wait_for_vblank(crtc->dev, intel_crtc->pipe); - } + struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); + cstate->wm.ilk.active = cstate->wm.ilk.target; ilk_program_watermarks(dev_priv); } @@ -3725,7 +3778,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc) struct skl_wm_values *hw = &dev_priv->wm.skl_hw; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); - struct skl_pipe_wm *active = &cstate->wm.active.skl; + struct skl_pipe_wm *active = &cstate->wm.skl; enum pipe pipe = intel_crtc->pipe; int level, i, max_level; uint32_t temp; @@ -3789,7 +3842,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc) struct ilk_wm_values *hw = &dev_priv->wm.hw; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); - struct intel_pipe_wm *active = &cstate->wm.active.ilk; + struct intel_pipe_wm *active = &cstate->wm.ilk.active; enum pipe pipe = intel_crtc->pipe; static const unsigned int wm0_pipe_reg[] = { [PIPE_A] = WM0_PIPEA_ILK, @@ -3828,6 +3881,8 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc) for (level = 0; level <= max_level; level++) active->wm[level].enable = true; } + + cstate->wm.ilk.target = *active; } #define _FW_WM(value, plane) \ @@ -6949,8 +7004,11 @@ void intel_init_pm(struct drm_device *dev) dev_priv->wm.spr_latency[1] && dev_priv->wm.cur_latency[1]) || (!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] && dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) { - dev_priv->display.update_wm = ilk_update_wm; dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm; + dev_priv->display.compute_intermediate_wm = + ilk_compute_intermediate_wm; + dev_priv->display.program_watermarks = ilk_program_watermarks; + dev_priv->display.optimize_watermarks = ilk_optimize_watermarks; } else { DRM_DEBUG_KMS("Failed to read display plane latency. " "Disable CxSR\n");
In addition to calculating final watermarks, let's also pre-calculate a set of intermediate watermark values at atomic check time. These intermediate watermarks are a combination of the watermarks for the old state and the new state; they should satisfy the requirements of both states which means they can be programmed immediately when we commit the atomic state (without waiting for a vblank). Once the vblank does happen, we can then re-program watermarks to the more optimal final value. v2: Significant rebasing/rewriting. v3: - Move 'need_postvbl_update' flag to CRTC state (Daniel) - Don't forget to check intermediate watermark values for validity (Maarten) - Don't due async watermark optimization; just do it at the end of the atomic transaction, after waiting for vblanks. We do want it to be async eventually, but adding that now will cause more trouble for Maarten's in-progress work. (Maarten) - Don't allocate space in crtc_state for intermediate watermarks on platforms that don't need it (gen9+). - Move WaCxSRDisabledForSpriteScaling:ivb into intel_begin_crtc_commit now that ilk_update_wm is gone. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 5 ++ drivers/gpu/drm/i915/intel_display.c | 77 ++++++++++++++++++- drivers/gpu/drm/i915/intel_drv.h | 33 ++++++-- drivers/gpu/drm/i915/intel_pm.c | 144 ++++++++++++++++++++++++----------- 4 files changed, 208 insertions(+), 51 deletions(-)