[Intel-gfx] [PATCH] drm/i915: Add two-stage ILK-style watermark programming (v11)
Imre Deak
imre.deak at intel.com
Mon Feb 29 19:52:19 UTC 2016
On to, 2016-02-25 at 08:48 -0800, Matt Roper wrote:
> I never got a CI email for this (probably because fdo was down for a
> while),
> but I can see the results below in patchwork
We also have the problem, that if a patch prevents a machine from
booting that machine won't be included in the CI test result. This was
the case here on SKL.
--Imre
>
> > Test gem_cs_prefetch:
> > Subgroup basic-default:
> > incomplete -> PASS (ilk-hp8440p)
> > Test kms_flip:
> > Subgroup basic-flip-vs-dpms:
> > pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE
>
> Pre-existing watermark bug here:
> https://bugs.freedesktop.org/show_bug.cgi?id=93787
>
> I had hoped this patch might also fix that problem, but I guess
> further
> investigation will be needed; it seems to be something very
> ILK-specific, not seen on SNB, IVB, etc.
>
>
> > Test kms_force_connector_basic:
> > Subgroup force-edid:
> > skip -> PASS (snb-x220t)
> > pass -> SKIP (ivb-t430s)
>
> Pre-existing:
> https://bugs.freedesktop.org/show_bug.cgi?id=93769
>
> > Subgroup force-load-detect:
> > dmesg-fail -> FAIL (snb-x220t)
> > dmesg-fail -> FAIL (hsw-gt2)
>
> The dmesg warning is gone now (a locking warning that I don't think
> my
> patch should have affected); the remaining test failure is identical
> to
> what it was before (temp->connection != DRM_MODE_UNKNOWNCONNECTION)
> and
> unrelated.
>
> I don't see a bugzilla for this, which seems strange since it's a
> pre-existing defect. Am I overlooking it or do I need to file a new
> one?
>
> > fail -> SKIP (ilk-hp8440p)
>
> Not sure what is actually plugged into the CI machine, so the skip
> may
> be legitimate/expected? Doesn't seem related to anything in my patch
> anyway.
>
> > Test kms_pipe_crc_basic:
> > Subgroup suspend-read-crc-pipe-c:
> > pass -> DMESG-WARN (bsw-nuc-2)
>
> This one was happening sporadically before my patch; I think it's the
> same bug filed here:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=94164
>
> I'll update the bugzilla to indicate it also happens on this test.
>
> > Test pm_rpm:
> > Subgroup basic-rte:
> > pass -> DMESG-WARN (bsw-nuc-2)
> > fail -> PASS (hsw-brixbox)
>
> Same bug mentioned above (94164)
>
>
>
> Matt
>
>
> On Tue, Feb 23, 2016 at 05:20:13PM -0800, 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.
> >
> > v4:
> > - Add a wm_mutex to cover updates to intel_crtc->active and the
> > need_postvbl_update flag. Since we don't have async yet it
> > isn't
> > terribly important yet, but might as well add it now.
> > - Change interface to program watermarks. Platforms will now
> > expose
> > .initial_watermarks() and .optimize_watermarks() functions to do
> > watermark programming. These should lock wm_mutex, copy the
> > appropriate state values into intel_crtc->active, and then call
> > the internal program watermarks function.
> >
> > v5:
> > - Skip intermediate watermark calculation/check during initial
> > hardware
> > readout since we don't trust the existing HW values (and don't
> > have
> > valid values of our own yet).
> > - Don't try to call .optimize_watermarks() on platforms that don't
> > have
> > atomic watermarks yet. (Maarten)
> >
> > v6:
> > - Rebase
> >
> > v7:
> > - Further rebase
> >
> > v8:
> > - A few minor indentation and line length fixes
> >
> > v9:
> > - Yet another rebase since Maarten's patches reworked a bunch of
> > the
> > code (wm_pre, wm_post, etc.) that this was previously based on.
> >
> > v10:
> > - Move wm_mutex to dev_priv to protect against racing commits
> > against
> > disjoint CRTC sets. (Maarten)
> > - Drop unnecessary clearing of cstate->wm.need_postvbl_update
> > (Maarten)
> >
> > v11:
> > - Now that we've moved to atomic watermark updates, make sure we
> > call
> > the proper function to program watermarks in
> > {ironlake,haswell}_crtc_enable(); the failure to do so on the
> > previous patch iteration led to us not actually programming the
> > watermarks before turning on the CRTC, which was the cause of
> > the
> > underruns that the CI system was seeing.
> > - Fix inverted logic for determining when to optimize
> > watermarks. We
> > were needlessly optimizing when the intermediate/optimal values
> > were
> > the same (harmless), but not actually optimizing when they
> > differed
> > (also harmless, but wasteful from a power/bandwidth
> > perspective).
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > ---
> > 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 | 97 +++++++++++++++++++--
> > drivers/gpu/drm/i915/intel_drv.h | 28 +++++-
> > drivers/gpu/drm/i915/intel_pm.c | 162
> > ++++++++++++++++++++++++-----------
> > 6 files changed, 244 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index 1c6d227..36c0cf1 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1010,6 +1010,7 @@ 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);
> >
> > ret = i915_workqueues_init(dev_priv);
> > if (ret < 0)
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 9e76bfc..cf17d62 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -631,7 +631,11 @@ struct drm_i915_display_funcs {
> > struct dpll *best_clock);
> > int (*compute_pipe_wm)(struct intel_crtc *crtc,
> > struct drm_atomic_state *state);
> > - void (*program_watermarks)(struct intel_crtc_state
> > *cstate);
> > + 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 (*update_wm)(struct drm_crtc *crtc);
> > int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
> > void (*modeset_commit_cdclk)(struct drm_atomic_state
> > *state);
> > @@ -1980,6 +1984,13 @@ 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 4625f8a..9682d94 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -97,6 +97,7 @@ 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 deee560..423b99e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4846,7 +4846,42 @@ static void intel_pre_plane_update(struct
> > intel_crtc_state *old_crtc_state)
> > intel_set_memory_cxsr(dev_priv, false);
> > }
> >
> > - if (!needs_modeset(&pipe_config->base) && pipe_config-
> > >wm_changed)
> > + /*
> > + * 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)
> > intel_update_watermarks(&crtc->base);
> > }
> >
> > @@ -4925,7 +4960,7 @@ static void ironlake_crtc_enable(struct
> > drm_crtc *crtc)
> > */
> > intel_crtc_load_lut(crtc);
> >
> > - intel_update_watermarks(crtc);
> > + dev_priv->display.initial_watermarks(intel_crtc->config);
> > intel_enable_pipe(intel_crtc);
> >
> > if (intel_crtc->config->has_pch_encoder)
> > @@ -5024,7 +5059,7 @@ static void haswell_crtc_enable(struct
> > drm_crtc *crtc)
> > if (!intel_crtc->config->has_dsi_encoder)
> > intel_ddi_enable_transcoder_func(crtc);
> >
> > - intel_update_watermarks(crtc);
> > + dev_priv->display.initial_watermarks(pipe_config);
> > intel_enable_pipe(intel_crtc);
> >
> > if (intel_crtc->config->has_pch_encoder)
> > @@ -11800,6 +11835,7 @@ int intel_plane_atomic_calc_changes(struct
> > drm_crtc_state *crtc_state,
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > struct drm_plane *plane = plane_state->plane;
> > struct drm_device *dev = crtc->dev;
> > + struct drm_i915_private *dev_priv = to_i915(dev);
> > struct intel_plane_state *old_plane_state =
> > to_intel_plane_state(plane->state);
> > int idx = intel_crtc->base.base.id, ret;
> > @@ -11859,6 +11895,11 @@ 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;
> > @@ -11983,8 +12024,29 @@ 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)
> > + 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,
> > + in
> > tel_crtc,
> > + pi
> > pe_config);
> > + if (ret) {
> > + DRM_DEBUG_KMS("No valid intermediate pipe
> > watermarks are possible\n");
> > + return ret;
> > + }
> > }
> >
> > if (INTEL_INFO(dev)->gen >= 9) {
> > @@ -13452,6 +13514,7 @@ 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;
> >
> > @@ -13555,6 +13618,20 @@ 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);
> > @@ -15225,7 +15302,7 @@ static void sanitize_watermarks(struct
> > drm_device *dev)
> > int i;
> >
> > /* Only supported on platforms that use atomic watermark
> > design */
> > - if (!dev_priv->display.program_watermarks)
> > + if (!dev_priv->display.optimize_watermarks)
> > return;
> >
> > /*
> > @@ -15246,6 +15323,13 @@ 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) {
> > /*
> > @@ -15268,7 +15352,8 @@ retry:
> > for_each_crtc_in_state(state, crtc, cstate, i) {
> > struct intel_crtc_state *cs =
> > to_intel_crtc_state(cstate);
> >
> > - dev_priv->display.program_watermarks(cs);
> > + cs->wm.need_postvbl_update = true;
> > + dev_priv->display.optimize_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 4852049..a9f7641 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -260,6 +260,12 @@ 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 {
> > @@ -509,13 +515,29 @@ 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;
> > };
> >
> > @@ -601,6 +623,7 @@ struct intel_crtc {
> > struct intel_pipe_wm ilk;
> > struct skl_pipe_wm skl;
> > } active;
> > +
> > /* allow CxSR on this pipe */
> > bool cxsr_allowed;
> > } wm;
> > @@ -1566,6 +1589,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);
> > int sanitize_rc6_option(const struct drm_device *dev, int
> > enable_rc6);
> >
> > /* intel_sdvo.c */
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 347d4df..ccdb581 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2278,6 +2278,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 intel_crtc *intel_crtc,
> > struct drm_atomic_state *state)
> > @@ -2292,10 +2315,6 @@ 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);
> > @@ -2319,21 +2338,18 @@ static int ilk_compute_pipe_wm(struct
> > intel_crtc *intel_crtc,
> > curstate = to_intel_plane_state(ps);
> > }
> >
> > - config.sprites_enabled = sprstate->visible;
> > - config.sprites_scaled = sprstate->visible &&
> > + pipe_wm->pipe_enabled = cstate->base.active;
> > + pipe_wm->sprites_enabled = sprstate->visible;
> > + pipe_wm->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 (config.sprites_scaled)
> > + if (pipe_wm->sprites_scaled)
> > max_level = 0;
> >
> > ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
> > @@ -2342,12 +2358,8 @@ 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);
> >
> > - /* 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);
> >
> > @@ -2372,6 +2384,59 @@ 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,
> > @@ -2383,9 +2448,7 @@ static void ilk_merge_wm_level(struct
> > drm_device *dev,
> > ret_wm->enable = true;
> >
> > 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.optimal.ilk;
> > + const struct intel_pipe_wm *active = &intel_crtc-
> > >wm.active.ilk;
> > const struct intel_wm_level *wm = &active-
> > >wm[level];
> >
> > if (!active->pipe_enabled)
> > @@ -2533,15 +2596,14 @@ 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 = &cstate-
> > >wm.optimal.ilk.wm[0];
> > + const struct intel_wm_level *r =
> > + &intel_crtc->wm.active.ilk.wm[0];
> >
> > if (WARN_ON(!r->enable))
> > continue;
> >
> > - results->wm_linetime[pipe] = cstate-
> > >wm.optimal.ilk.linetime;
> > + results->wm_linetime[pipe] = intel_crtc-
> > >wm.active.ilk.linetime;
> >
> > results->wm_pipe[pipe] =
> > (r->pri_val << WM0_PIPE_PLANE_SHIFT) |
> > @@ -2748,7 +2810,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;
> >
> > @@ -3643,11 +3705,9 @@ static void ilk_compute_wm_config(struct
> > drm_device *dev,
> > }
> > }
> >
> > -static void ilk_program_watermarks(struct intel_crtc_state
> > *cstate)
> > +static void ilk_program_watermarks(struct drm_i915_private
> > *dev_priv)
> > {
> > - struct drm_crtc *crtc = cstate->base.crtc;
> > - struct drm_device *dev = crtc->dev;
> > - struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct drm_device *dev = dev_priv->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 = {};
> > @@ -3678,28 +3738,28 @@ static void ilk_program_watermarks(struct
> > intel_crtc_state *cstate)
> > ilk_write_wm_values(dev_priv, &results);
> > }
> >
> > -static void ilk_update_wm(struct drm_crtc *crtc)
> > +static void ilk_initial_watermarks(struct intel_crtc_state
> > *cstate)
> > {
> > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > - struct intel_crtc_state *cstate =
> > to_intel_crtc_state(crtc->state);
> > -
> > - WARN_ON(cstate->base.active != intel_crtc->active);
> > + struct drm_i915_private *dev_priv = to_i915(cstate-
> > >base.crtc->dev);
> > + struct intel_crtc *intel_crtc = to_intel_crtc(cstate-
> > >base.crtc);
> >
> > - /*
> > - * 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_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);
> > +}
> >
> > - intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
> > +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);
> >
> > - ilk_program_watermarks(cstate);
> > + 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);
> > + }
> > + mutex_unlock(&dev_priv->wm.wm_mutex);
> > }
> >
> > static void skl_pipe_wm_active_state(uint32_t val,
> > @@ -7076,9 +7136,13 @@ 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.program_watermarks =
> > ilk_program_watermarks;
> > + 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;
> > } else {
> > DRM_DEBUG_KMS("Failed to read display
> > plane latency. "
> > "Disable CxSR\n");
> > --
> > 2.1.4
> >
>
More information about the Intel-gfx
mailing list