On Wed, May 20, 2015 at 07:12:24PM -0700, Matt Roper wrote:
> Rather that just comparing and then throwing away the pipe watermark
> values we precomputed at atomic check time, actually use those values
> when we program watermarks.
>
> FIXME: This should be squashed into the previous patch eventually, once
> we're convinced that pre-computed watermarks get the proper values in
> all cases.
>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_atomic.c | 4 +-
> drivers/gpu/drm/i915/intel_drv.h | 3 --
> drivers/gpu/drm/i915/intel_pm.c | 77 +++++++++++--------------------------
> 4 files changed, 24 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 214ce76..d577eba 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -563,8 +563,7 @@ struct drm_i915_display_funcs {
> struct dpll *match_clock,
> struct dpll *best_clock);
> int (*compute_pipe_wm)(struct drm_crtc *crtc,
> - struct drm_atomic_state *state,
> - void *target);
> + struct drm_atomic_state *state);
> void (*update_wm)(struct drm_crtc *crtc);
> void (*update_sprite_wm)(struct drm_plane *plane,
> struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index db349dd..5294840 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -435,14 +435,12 @@ intel_check_crtc(struct drm_crtc *crtc,
> struct drm_crtc_state *state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> - struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> int ret;
>
> if (!dev_priv->display.compute_pipe_wm)
> return 0;
>
> - ret = dev_priv->display.compute_pipe_wm(crtc, state->state,
> - &intel_state->wm.tmp);
> + ret = dev_priv->display.compute_pipe_wm(crtc, state->state);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6ca3c06..627741a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -464,9 +464,6 @@ struct intel_crtc_state {
> struct intel_crtc_scaler_state scaler_state;
>
> struct {
> - /* tmp wm */
> - struct intel_pipe_wm tmp;
> -
> /* final, target watermarks for state */
> union {
> struct intel_pipe_wm ilk;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 017b184..27337fe 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2073,10 +2073,9 @@ static void ilk_compute_wm_config(struct drm_device *dev,
>
> /* Compute new watermarks for the pipe */
> static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
> - struct drm_atomic_state *state,
> - void *target)
> + struct drm_atomic_state *state)
> {
> - struct intel_pipe_wm *pipe_wm = target;
> + struct intel_pipe_wm *pipe_wm;
> struct drm_device *dev = crtc->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -2094,47 +2093,26 @@ static int ilk_compute_pipe_wm(struct drm_crtc *crtc,
> };
> struct ilk_wm_maximums max;
>
> - /*
> - * FIXME: In an upcoming patch, we'll *only* be calling this from the
> - * atomic 'check' codepath and thus will always have a top-level atomic
> - * transaction. However at the moment we still call this in the legacy
> - * 'ilk_update_wm' function, which means we need to be able to also
> - * operate on already-committed state which has been detached from any
> - * top-level transaction.
> - *
> - * Drop the 'else' block here once we only do atomic-style watermarks.
> - */
> - if (state) {
> - cs = drm_atomic_get_crtc_state(state, crtc);
> - if (IS_ERR(cs))
> - return PTR_ERR(cs);
> - else
> - cstate = to_intel_crtc_state(cs);
> -
> - for_each_intel_plane_of_crtc(intel_crtc, intel_plane) {
> - ps = drm_atomic_get_plane_state(state,
> - &intel_plane->base);
> - if (IS_ERR(ps))
> - return PTR_ERR(ps);
> -
> - if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
> - pristate = to_intel_plane_state(ps);
> - else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
> - sprstate = to_intel_plane_state(ps);
> - else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
> - curstate = to_intel_plane_state(ps);
> - }
> - } else {
> - /* Use already-committed state */
> - cstate = to_intel_crtc_state(crtc->state);
> - for_each_intel_plane_of_crtc(intel_crtc, intel_plane) {
> - if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) {
> - sprstate = to_intel_plane_state(intel_plane->base.state);
> - break;
> - }
> - }
> - pristate = to_intel_plane_state(crtc->primary->state);
> - curstate = to_intel_plane_state(crtc->cursor->state);
> + cs = drm_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(cs))
> + return PTR_ERR(cs);
> + else
> + cstate = to_intel_crtc_state(cs);
> +
> + pipe_wm = &cstate->wm.target.ilk;
> +
> + for_each_intel_plane_of_crtc(intel_crtc, intel_plane) {
> + ps = drm_atomic_get_plane_state(state,
> + &intel_plane->base);
> + if (IS_ERR(ps))
> + return PTR_ERR(ps);
> +
> + if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
> + pristate = to_intel_plane_state(ps);
> + else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
> + sprstate = to_intel_plane_state(ps);
> + else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
> + curstate = to_intel_plane_state(ps);
> }
>
> config.sprites_enabled = sprstate->visible;
> @@ -3511,17 +3489,6 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
> static void ilk_update_wm(struct drm_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> - struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> - struct intel_pipe_wm pipe_wm = {};
> -
> - ilk_compute_pipe_wm(crtc, NULL, &pipe_wm);
> - WARN(memcmp(&cstate->wm.tmp, &pipe_wm, sizeof(pipe_wm)) != 0,
> - "Pre-computed atomic watermark values do not match final values!");
> -
> - if (!memcmp(&cstate->wm.target.ilk, &pipe_wm, sizeof(pipe_wm)))
> - return;
The memcmp() is there to avoid reprogromaming needlessly. It should
stay.
> -
> - cstate->wm.target.ilk = pipe_wm;
>
> ilk_program_watermarks(dev_priv);
> }
> --
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
