Message ID | 1453232584-8543-1-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Wed, Jan 20, 2016 at 08:20:50AM -0000, Patchwork wrote: > == Summary == > > Built on 7d26528d30b0d8119c858115b6e5e22deb74ec71 drm-intel-nightly: 2016y-01m-19d-19h-38m-52s UTC integration manifest > > Test drv_module_reload_basic: > dmesg-warn -> PASS (snb-x220t) > dmesg-warn -> PASS (snb-dellxps) > dmesg-warn -> PASS (ivb-t430s) > Test kms_flip: > Subgroup basic-flip-vs-dpms: > pass -> DMESG-WARN (ilk-hp8440p) Hm, so these fifo underruns on ilk aren't removed with this patch :( Looking at long-term history this happened even before we merged the patch this reverts again, but somehow we failed to create a BAT bugzilla for this. Fixed now: https://bugs.freedesktop.org/show_bug.cgi?id=93787 > Subgroup basic-flip-vs-wf_vblank: > dmesg-warn -> PASS (snb-x220t) > dmesg-warn -> PASS (snb-dellxps) > dmesg-warn -> PASS (ilk-hp8440p) > dmesg-warn -> PASS (ivb-t430s) > Subgroup basic-plain-flip: > dmesg-warn -> PASS (ivb-t430s) > Test kms_pipe_crc_basic: > Subgroup nonblocking-crc-pipe-b: > dmesg-warn -> PASS (snb-x220t) > dmesg-warn -> PASS (snb-dellxps) > Subgroup read-crc-pipe-b: > dmesg-warn -> PASS (snb-x220t) > dmesg-warn -> PASS (snb-dellxps) > dmesg-warn -> PASS (ilk-hp8440p) > dmesg-fail -> PASS (ivb-t430s) > Subgroup read-crc-pipe-c: > dmesg-fail -> PASS (ivb-t430s) > Subgroup suspend-read-crc-pipe-b: > dmesg-warn -> PASS (snb-x220t) > dmesg-warn -> PASS (snb-dellxps) > dmesg-warn -> PASS (ilk-hp8440p) > dmesg-fail -> PASS (ivb-t430s) > Subgroup suspend-read-crc-pipe-c: > dmesg-fail -> PASS (ivb-t430s) All the other fixes are expected. so CI at least confirmed our bisect! -Daniel > > bdw-nuci7 total:140 pass:131 dwarn:0 dfail:0 fail:0 skip:9 > bdw-ultra total:140 pass:132 dwarn:1 dfail:1 fail:0 skip:6 > byt-nuc total:143 pass:125 dwarn:3 dfail:0 fail:0 skip:15 > hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 > hsw-gt2 total:143 pass:139 dwarn:0 dfail:0 fail:0 skip:4 > ilk-hp8440p total:143 pass:103 dwarn:2 dfail:0 fail:0 skip:38 > ivb-t430s total:137 pass:131 dwarn:0 dfail:0 fail:0 skip:6 > skl-i5k-2 total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 > skl-i7k-2 total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 > snb-dellxps total:143 pass:129 dwarn:0 dfail:0 fail:0 skip:14 > snb-x220t total:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 > > Results at /archive/results/CI_IGT_test/Patchwork_1226/ > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jan 19, 2016 at 11:43:04AM -0800, Matt Roper wrote: > This reverts commit 396e33ae204f52abebec9e578f44c749305500f4. > > This commit was triggering some FIFO underrun warnings on ILK-IVB > platforms (but surprisingly not on HSW/BDW that share more or less the > same codepaths). These underruns were caught by the continuous > integration (CI) system and could be reproduced consistently when > running the basic acceptance tests (BAT) on the affected platforms. > > Note that this revert will cause a visible regression for some > end-users; the "flicker when mouse moves between monitors in X" issue > that was reported before this patch was merged will now return. However > regressions that are visible to CI have higher priority since they > prevent proper testing of future patches on those platforms. Hopefully > we'll be able to figure out the cause of the underruns quickly and > remerge an improved version of this patch to fix the regression. > > Cc: Daniel Vetter <daniel@ffwll.ch> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93640 > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > Reviewed-by: Daniel Vetter <daniel@ffwll.ch> Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_dma.c | 1 - > drivers/gpu/drm/i915/i915_drv.h | 13 +-- > drivers/gpu/drm/i915/intel_atomic.c | 1 - > drivers/gpu/drm/i915/intel_display.c | 92 +------------------- > drivers/gpu/drm/i915/intel_drv.h | 28 +----- > drivers/gpu/drm/i915/intel_pm.c | 162 +++++++++++------------------------ > 6 files changed, 56 insertions(+), 241 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index a0f5659..d70d96f 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -896,7 +896,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > mutex_init(&dev_priv->sb_lock); > mutex_init(&dev_priv->modeset_restore_lock); > mutex_init(&dev_priv->av_mutex); > - mutex_init(&dev_priv->wm.wm_mutex); > > intel_pm_setup(dev); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index af30148..d3b98c2 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -628,11 +628,7 @@ struct drm_i915_display_funcs { > struct dpll *best_clock); > int (*compute_pipe_wm)(struct intel_crtc *crtc, > struct drm_atomic_state *state); > - int (*compute_intermediate_wm)(struct drm_device *dev, > - struct intel_crtc *intel_crtc, > - struct intel_crtc_state *newstate); > - void (*initial_watermarks)(struct intel_crtc_state *cstate); > - void (*optimize_watermarks)(struct intel_crtc_state *cstate); > + void (*program_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); > @@ -1938,13 +1934,6 @@ struct drm_i915_private { > }; > > uint8_t max_level; > - > - /* > - * Should be held around atomic WM register writing; also > - * protects * intel_crtc->wm.active and > - * cstate->wm.need_postvbl_update. > - */ > - struct mutex wm_mutex; > } wm; > > struct i915_runtime_pm pm; > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index 9682d94..4625f8a 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -97,7 +97,6 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) > crtc_state->disable_lp_wm = false; > crtc_state->disable_cxsr = false; > crtc_state->wm_changed = false; > - crtc_state->wm.need_postvbl_update = false; > > return &crtc_state->base; > } > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index a851cb7..3260fc6 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4833,42 +4833,7 @@ static void intel_pre_plane_update(struct intel_crtc *crtc) > intel_set_memory_cxsr(dev_priv, false); > } > > - /* > - * 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 (pipe_config->disable_lp_wm) { > - ilk_disable_lp_wm(dev); > - intel_wait_for_vblank(dev, crtc->pipe); > - } > - > - /* > - * If we're doing a modeset, we're done. No need to do any pre-vblank > - * watermark programming here. > - */ > - if (needs_modeset(&pipe_config->base)) > - return; > - > - /* > - * For platforms that support atomic watermarks, program the > - * 'intermediate' 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.initial_watermarks != NULL) > - dev_priv->display.initial_watermarks(pipe_config); > - else if (pipe_config->wm_changed) > + if (!needs_modeset(&pipe_config->base) && pipe_config->wm_changed) > intel_update_watermarks(&crtc->base); > } > > @@ -11914,11 +11879,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, > pipe_config->wm_changed = true; > } > > - /* Pre-gen9 platforms need two-step watermark updates */ > - if (pipe_config->wm_changed && INTEL_INFO(dev)->gen < 9 && > - dev_priv->display.optimize_watermarks) > - to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true; > - > if (visible || was_visible) > intel_crtc->atomic.fb_bits |= > to_intel_plane(plane)->frontbuffer_bit; > @@ -12075,29 +12035,8 @@ 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(intel_crtc, state); > - if (ret) { > - DRM_DEBUG_KMS("Target pipe watermarks are invalid\n"); > - return ret; > - } > - } > - > - if (dev_priv->display.compute_intermediate_wm && > - !to_intel_atomic_state(state)->skip_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, > - intel_crtc, > - pipe_config); > - if (ret) { > - DRM_DEBUG_KMS("No valid intermediate pipe watermarks are possible\n"); > + if (ret) > return ret; > - } > } > > if (INTEL_INFO(dev)->gen >= 9) { > @@ -13562,7 +13501,6 @@ static int intel_atomic_commit(struct drm_device *dev, > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_crtc_state *crtc_state; > struct drm_crtc *crtc; > - struct intel_crtc_state *intel_cstate; > int ret = 0, i; > bool hw_check = intel_state->modeset; > > @@ -13662,20 +13600,6 @@ static int intel_atomic_commit(struct drm_device *dev, > > 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 (dev_priv->display.optimize_watermarks) > - dev_priv->display.optimize_watermarks(intel_cstate); > - } > - > mutex_lock(&dev->struct_mutex); > drm_atomic_helper_cleanup_planes(dev, state); > mutex_unlock(&dev->struct_mutex); > @@ -15342,7 +15266,7 @@ static void sanitize_watermarks(struct drm_device *dev) > int i; > > /* Only supported on platforms that use atomic watermark design */ > - if (!dev_priv->display.optimize_watermarks) > + if (!dev_priv->display.program_watermarks) > return; > > /* > @@ -15363,13 +15287,6 @@ retry: > if (WARN_ON(IS_ERR(state))) > goto fail; > > - /* > - * Hardware readout is the only time we don't want to calculate > - * intermediate watermarks (since we don't trust the current > - * watermarks). > - */ > - to_intel_atomic_state(state)->skip_intermediate_wm = true; > - > ret = intel_atomic_check(dev, state); > if (ret) { > /* > @@ -15392,8 +15309,7 @@ retry: > for_each_crtc_in_state(state, crtc, cstate, i) { > struct intel_crtc_state *cs = to_intel_crtc_state(cstate); > > - cs->wm.need_postvbl_update = true; > - dev_priv->display.optimize_watermarks(cs); > + dev_priv->display.program_watermarks(cs); > } > > drm_atomic_state_free(state); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 059b46e..15917e3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -260,12 +260,6 @@ struct intel_atomic_state { > > struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS]; > struct intel_wm_config wm_config; > - > - /* > - * Current watermarks can't be trusted during hardware readout, so > - * don't bother calculating intermediate watermarks. > - */ > - bool skip_intermediate_wm; > }; > > struct intel_plane_state { > @@ -513,29 +507,13 @@ struct intel_crtc_state { > > struct { > /* > - * Optimal watermarks, programmed post-vblank when this state > - * is committed. > + * optimal watermarks, programmed post-vblank when this state > + * is committed > */ > union { > struct intel_pipe_wm ilk; > struct skl_pipe_wm skl; > } optimal; > - > - /* > - * Intermediate watermarks; these can be programmed immediately > - * since they satisfy both the current configuration we're > - * switching away from and the new configuration we're switching > - * to. > - */ > - struct intel_pipe_wm intermediate; > - > - /* > - * 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; > }; > > @@ -622,7 +600,6 @@ struct intel_crtc { > struct intel_pipe_wm ilk; > struct skl_pipe_wm skl; > } active; > - > /* allow CxSR on this pipe */ > bool cxsr_allowed; > } wm; > @@ -1583,7 +1560,6 @@ 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, > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 6b2b3e3..ad393a7 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2273,29 +2273,6 @@ 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 intel_crtc *intel_crtc, > struct drm_atomic_state *state) > @@ -2310,6 +2287,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_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; > > cstate = intel_atomic_get_crtc_state(state, intel_crtc); > @@ -2333,18 +2314,21 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, > curstate = to_intel_plane_state(ps); > } > > - pipe_wm->pipe_enabled = cstate->base.active; > - pipe_wm->sprites_enabled = sprstate->visible; > - pipe_wm->sprites_scaled = sprstate->visible && > + config.sprites_enabled = sprstate->visible; > + config.sprites_scaled = sprstate->visible && > (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 (pipe_wm->sprites_scaled) > + if (config.sprites_scaled) > max_level = 0; > > ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > @@ -2353,8 +2337,12 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, > if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > pipe_wm->linetime = hsw_compute_linetime_wm(dev, cstate); > > - if (!ilk_validate_pipe_wm(dev, pipe_wm)) > - return false; > + /* 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; > > ilk_compute_wm_reg_maximums(dev, 1, &max); > > @@ -2379,59 +2367,6 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_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 *intel_crtc, > - struct intel_crtc_state *newstate) > -{ > - struct intel_pipe_wm *a = &newstate->wm.intermediate; > - struct intel_pipe_wm *b = &intel_crtc->wm.active.ilk; > - int level, max_level = ilk_wm_max_level(dev); > - > - /* > - * Start with the final, target watermarks, then combine with the > - * currently active watermarks to get values that are safe both before > - * and after the vblank. > - */ > - *a = newstate->wm.optimal.ilk; > - 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; only update if it's different. > - */ > - if (memcmp(a, &newstate->wm.optimal.ilk, sizeof(*a)) != 0) > - 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, > @@ -2443,7 +2378,9 @@ static void ilk_merge_wm_level(struct drm_device *dev, > ret_wm->enable = true; > > for_each_intel_crtc(dev, intel_crtc) { > - const struct intel_pipe_wm *active = &intel_crtc->wm.active.ilk; > + const struct intel_crtc_state *cstate = > + to_intel_crtc_state(intel_crtc->base.state); > + const struct intel_pipe_wm *active = &cstate->wm.optimal.ilk; > const struct intel_wm_level *wm = &active->wm[level]; > > if (!active->pipe_enabled) > @@ -2591,14 +2528,15 @@ static void ilk_compute_wm_results(struct drm_device *dev, > > /* LP0 register values */ > for_each_intel_crtc(dev, intel_crtc) { > + 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 = > - &intel_crtc->wm.active.ilk.wm[0]; > + const struct intel_wm_level *r = &cstate->wm.optimal.ilk.wm[0]; > > if (WARN_ON(!r->enable)) > continue; > > - results->wm_linetime[pipe] = intel_crtc->wm.active.ilk.linetime; > + results->wm_linetime[pipe] = cstate->wm.optimal.ilk.linetime; > > results->wm_pipe[pipe] = > (r->pri_val << WM0_PIPE_PLANE_SHIFT) | > @@ -2805,7 +2743,7 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv, > dev_priv->wm.hw = *results; > } > > -bool ilk_disable_lp_wm(struct drm_device *dev) > +static bool ilk_disable_lp_wm(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > @@ -3698,9 +3636,11 @@ static void ilk_compute_wm_config(struct drm_device *dev, > } > } > > -static void ilk_program_watermarks(struct drm_i915_private *dev_priv) > +static void ilk_program_watermarks(struct intel_crtc_state *cstate) > { > - struct drm_device *dev = dev_priv->dev; > + struct drm_crtc *crtc = cstate->base.crtc; > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm; > struct ilk_wm_maximums max; > struct intel_wm_config config = {}; > @@ -3731,28 +3671,28 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv) > ilk_write_wm_values(dev_priv, &results); > } > > -static void ilk_initial_watermarks(struct intel_crtc_state *cstate) > +static void ilk_update_wm(struct drm_crtc *crtc) > { > - struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); > - struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc); > - > - mutex_lock(&dev_priv->wm.wm_mutex); > - intel_crtc->wm.active.ilk = cstate->wm.intermediate; > - ilk_program_watermarks(dev_priv); > - mutex_unlock(&dev_priv->wm.wm_mutex); > -} > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); > > -static void ilk_optimize_watermarks(struct intel_crtc_state *cstate) > -{ > - struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); > - struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc); > + WARN_ON(cstate->base.active != intel_crtc->active); > > - mutex_lock(&dev_priv->wm.wm_mutex); > - if (cstate->wm.need_postvbl_update) { > - intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk; > - ilk_program_watermarks(dev_priv); > + /* > + * 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); > } > - mutex_unlock(&dev_priv->wm.wm_mutex); > + > + intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk; > + > + ilk_program_watermarks(cstate); > } > > static void skl_pipe_wm_active_state(uint32_t val, > @@ -7079,13 +7019,9 @@ 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.initial_watermarks = > - ilk_initial_watermarks; > - dev_priv->display.optimize_watermarks = > - ilk_optimize_watermarks; > + dev_priv->display.program_watermarks = ilk_program_watermarks; > } else { > DRM_DEBUG_KMS("Failed to read display plane latency. " > "Disable CxSR\n"); > -- > 2.1.4 >
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index a0f5659..d70d96f 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -896,7 +896,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) mutex_init(&dev_priv->sb_lock); mutex_init(&dev_priv->modeset_restore_lock); mutex_init(&dev_priv->av_mutex); - mutex_init(&dev_priv->wm.wm_mutex); intel_pm_setup(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index af30148..d3b98c2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -628,11 +628,7 @@ struct drm_i915_display_funcs { struct dpll *best_clock); int (*compute_pipe_wm)(struct intel_crtc *crtc, struct drm_atomic_state *state); - int (*compute_intermediate_wm)(struct drm_device *dev, - struct intel_crtc *intel_crtc, - struct intel_crtc_state *newstate); - void (*initial_watermarks)(struct intel_crtc_state *cstate); - void (*optimize_watermarks)(struct intel_crtc_state *cstate); + void (*program_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); @@ -1938,13 +1934,6 @@ struct drm_i915_private { }; uint8_t max_level; - - /* - * Should be held around atomic WM register writing; also - * protects * intel_crtc->wm.active and - * cstate->wm.need_postvbl_update. - */ - struct mutex wm_mutex; } wm; struct i915_runtime_pm pm; diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 9682d94..4625f8a 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -97,7 +97,6 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) crtc_state->disable_lp_wm = false; crtc_state->disable_cxsr = false; crtc_state->wm_changed = false; - crtc_state->wm.need_postvbl_update = false; return &crtc_state->base; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a851cb7..3260fc6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4833,42 +4833,7 @@ static void intel_pre_plane_update(struct intel_crtc *crtc) intel_set_memory_cxsr(dev_priv, false); } - /* - * 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 (pipe_config->disable_lp_wm) { - ilk_disable_lp_wm(dev); - intel_wait_for_vblank(dev, crtc->pipe); - } - - /* - * If we're doing a modeset, we're done. No need to do any pre-vblank - * watermark programming here. - */ - if (needs_modeset(&pipe_config->base)) - return; - - /* - * For platforms that support atomic watermarks, program the - * 'intermediate' 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.initial_watermarks != NULL) - dev_priv->display.initial_watermarks(pipe_config); - else if (pipe_config->wm_changed) + if (!needs_modeset(&pipe_config->base) && pipe_config->wm_changed) intel_update_watermarks(&crtc->base); } @@ -11914,11 +11879,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, pipe_config->wm_changed = true; } - /* Pre-gen9 platforms need two-step watermark updates */ - if (pipe_config->wm_changed && INTEL_INFO(dev)->gen < 9 && - dev_priv->display.optimize_watermarks) - to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true; - if (visible || was_visible) intel_crtc->atomic.fb_bits |= to_intel_plane(plane)->frontbuffer_bit; @@ -12075,29 +12035,8 @@ 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(intel_crtc, state); - if (ret) { - DRM_DEBUG_KMS("Target pipe watermarks are invalid\n"); - return ret; - } - } - - if (dev_priv->display.compute_intermediate_wm && - !to_intel_atomic_state(state)->skip_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, - intel_crtc, - pipe_config); - if (ret) { - DRM_DEBUG_KMS("No valid intermediate pipe watermarks are possible\n"); + if (ret) return ret; - } } if (INTEL_INFO(dev)->gen >= 9) { @@ -13562,7 +13501,6 @@ static int intel_atomic_commit(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc_state *crtc_state; struct drm_crtc *crtc; - struct intel_crtc_state *intel_cstate; int ret = 0, i; bool hw_check = intel_state->modeset; @@ -13662,20 +13600,6 @@ static int intel_atomic_commit(struct drm_device *dev, 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 (dev_priv->display.optimize_watermarks) - dev_priv->display.optimize_watermarks(intel_cstate); - } - mutex_lock(&dev->struct_mutex); drm_atomic_helper_cleanup_planes(dev, state); mutex_unlock(&dev->struct_mutex); @@ -15342,7 +15266,7 @@ static void sanitize_watermarks(struct drm_device *dev) int i; /* Only supported on platforms that use atomic watermark design */ - if (!dev_priv->display.optimize_watermarks) + if (!dev_priv->display.program_watermarks) return; /* @@ -15363,13 +15287,6 @@ retry: if (WARN_ON(IS_ERR(state))) goto fail; - /* - * Hardware readout is the only time we don't want to calculate - * intermediate watermarks (since we don't trust the current - * watermarks). - */ - to_intel_atomic_state(state)->skip_intermediate_wm = true; - ret = intel_atomic_check(dev, state); if (ret) { /* @@ -15392,8 +15309,7 @@ retry: for_each_crtc_in_state(state, crtc, cstate, i) { struct intel_crtc_state *cs = to_intel_crtc_state(cstate); - cs->wm.need_postvbl_update = true; - dev_priv->display.optimize_watermarks(cs); + dev_priv->display.program_watermarks(cs); } drm_atomic_state_free(state); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 059b46e..15917e3 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -260,12 +260,6 @@ struct intel_atomic_state { struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS]; struct intel_wm_config wm_config; - - /* - * Current watermarks can't be trusted during hardware readout, so - * don't bother calculating intermediate watermarks. - */ - bool skip_intermediate_wm; }; struct intel_plane_state { @@ -513,29 +507,13 @@ struct intel_crtc_state { struct { /* - * Optimal watermarks, programmed post-vblank when this state - * is committed. + * optimal watermarks, programmed post-vblank when this state + * is committed */ union { struct intel_pipe_wm ilk; struct skl_pipe_wm skl; } optimal; - - /* - * Intermediate watermarks; these can be programmed immediately - * since they satisfy both the current configuration we're - * switching away from and the new configuration we're switching - * to. - */ - struct intel_pipe_wm intermediate; - - /* - * 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; }; @@ -622,7 +600,6 @@ struct intel_crtc { struct intel_pipe_wm ilk; struct skl_pipe_wm skl; } active; - /* allow CxSR on this pipe */ bool cxsr_allowed; } wm; @@ -1583,7 +1560,6 @@ 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, diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 6b2b3e3..ad393a7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2273,29 +2273,6 @@ 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 intel_crtc *intel_crtc, struct drm_atomic_state *state) @@ -2310,6 +2287,10 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_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; cstate = intel_atomic_get_crtc_state(state, intel_crtc); @@ -2333,18 +2314,21 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, curstate = to_intel_plane_state(ps); } - pipe_wm->pipe_enabled = cstate->base.active; - pipe_wm->sprites_enabled = sprstate->visible; - pipe_wm->sprites_scaled = sprstate->visible && + config.sprites_enabled = sprstate->visible; + config.sprites_scaled = sprstate->visible && (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 (pipe_wm->sprites_scaled) + if (config.sprites_scaled) max_level = 0; ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, @@ -2353,8 +2337,12 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, if (IS_HASWELL(dev) || IS_BROADWELL(dev)) pipe_wm->linetime = hsw_compute_linetime_wm(dev, cstate); - if (!ilk_validate_pipe_wm(dev, pipe_wm)) - return false; + /* 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; ilk_compute_wm_reg_maximums(dev, 1, &max); @@ -2379,59 +2367,6 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_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 *intel_crtc, - struct intel_crtc_state *newstate) -{ - struct intel_pipe_wm *a = &newstate->wm.intermediate; - struct intel_pipe_wm *b = &intel_crtc->wm.active.ilk; - int level, max_level = ilk_wm_max_level(dev); - - /* - * Start with the final, target watermarks, then combine with the - * currently active watermarks to get values that are safe both before - * and after the vblank. - */ - *a = newstate->wm.optimal.ilk; - 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; only update if it's different. - */ - if (memcmp(a, &newstate->wm.optimal.ilk, sizeof(*a)) != 0) - 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, @@ -2443,7 +2378,9 @@ static void ilk_merge_wm_level(struct drm_device *dev, ret_wm->enable = true; for_each_intel_crtc(dev, intel_crtc) { - const struct intel_pipe_wm *active = &intel_crtc->wm.active.ilk; + const struct intel_crtc_state *cstate = + to_intel_crtc_state(intel_crtc->base.state); + const struct intel_pipe_wm *active = &cstate->wm.optimal.ilk; const struct intel_wm_level *wm = &active->wm[level]; if (!active->pipe_enabled) @@ -2591,14 +2528,15 @@ static void ilk_compute_wm_results(struct drm_device *dev, /* LP0 register values */ for_each_intel_crtc(dev, intel_crtc) { + 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 = - &intel_crtc->wm.active.ilk.wm[0]; + const struct intel_wm_level *r = &cstate->wm.optimal.ilk.wm[0]; if (WARN_ON(!r->enable)) continue; - results->wm_linetime[pipe] = intel_crtc->wm.active.ilk.linetime; + results->wm_linetime[pipe] = cstate->wm.optimal.ilk.linetime; results->wm_pipe[pipe] = (r->pri_val << WM0_PIPE_PLANE_SHIFT) | @@ -2805,7 +2743,7 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv, dev_priv->wm.hw = *results; } -bool ilk_disable_lp_wm(struct drm_device *dev) +static bool ilk_disable_lp_wm(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -3698,9 +3636,11 @@ static void ilk_compute_wm_config(struct drm_device *dev, } } -static void ilk_program_watermarks(struct drm_i915_private *dev_priv) +static void ilk_program_watermarks(struct intel_crtc_state *cstate) { - struct drm_device *dev = dev_priv->dev; + struct drm_crtc *crtc = cstate->base.crtc; + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm; struct ilk_wm_maximums max; struct intel_wm_config config = {}; @@ -3731,28 +3671,28 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv) ilk_write_wm_values(dev_priv, &results); } -static void ilk_initial_watermarks(struct intel_crtc_state *cstate) +static void ilk_update_wm(struct drm_crtc *crtc) { - struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); - struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc); - - mutex_lock(&dev_priv->wm.wm_mutex); - intel_crtc->wm.active.ilk = cstate->wm.intermediate; - ilk_program_watermarks(dev_priv); - mutex_unlock(&dev_priv->wm.wm_mutex); -} + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); -static void ilk_optimize_watermarks(struct intel_crtc_state *cstate) -{ - struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); - struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc); + WARN_ON(cstate->base.active != intel_crtc->active); - mutex_lock(&dev_priv->wm.wm_mutex); - if (cstate->wm.need_postvbl_update) { - intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk; - ilk_program_watermarks(dev_priv); + /* + * 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); } - mutex_unlock(&dev_priv->wm.wm_mutex); + + intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk; + + ilk_program_watermarks(cstate); } static void skl_pipe_wm_active_state(uint32_t val, @@ -7079,13 +7019,9 @@ 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.initial_watermarks = - ilk_initial_watermarks; - dev_priv->display.optimize_watermarks = - ilk_optimize_watermarks; + dev_priv->display.program_watermarks = ilk_program_watermarks; } else { DRM_DEBUG_KMS("Failed to read display plane latency. " "Disable CxSR\n");