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@xxxxxxxxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
To assist with review, the non-rebasing changes in this iteration vs the
last one are:
> @@ -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)
(both new additions to the patch)
and:
> + /*
> + * 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;
(replacement of a "!=" with "==")
Matt
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
