On Tue, Mar 01, 2016 at 11:07:22AM +0100, Maarten Lankhorst wrote:
> Only planes that are part of the state should be used for recalculating
> watermarks. For planes not part of the state the previous patch allows
> us to re-use the old values since they're calculated even for levels
> that are not actively used.
>
> Changes since v1:
> - Remove big if from intel_crtc_atomic_check.
> - Remove extra newline.
> - Remove memset in ilk_compute_pipe_wm.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Matt Roper <matthew.d.ro...@intel.com>
I haven't thought through this too carefully yet, but off the top of my
head I'm not sure if this will work for SKL once we transition it to
also use a more atomic style. Changes to other state might result in
changes to DDB allocation, making our previously-calculated values
invalid.
I think this is okay for the current code (where only ILK is atomic), so
Acknowledged-by: Matt Roper <matthew.d.ro...@intel.com>
for the time being. I'll be back to looking at SKL-style watermarks in
the next day or two and I might have to backtrack somewhat in cases
where a DDB partitioning changes results in a full recompute, but I need
to think through the details a bit more about how best to handle that.
Matt
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 12 ++++++++
> drivers/gpu/drm/i915/intel_pm.c | 59
> ++++++++++++++++++++----------------
> 4 files changed, 47 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 671295523317..4b8f446cdf44 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -629,8 +629,7 @@ struct drm_i915_display_funcs {
> int target, int refclk,
> struct dpll *match_clock,
> struct dpll *best_clock);
> - int (*compute_pipe_wm)(struct intel_crtc *crtc,
> - struct drm_atomic_state *state);
> + int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
> int (*compute_intermediate_wm)(struct drm_device *dev,
> struct intel_crtc *intel_crtc,
> struct intel_crtc_state *newstate);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 79bf527e0a73..20a4cb3f69d1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11994,7 +11994,7 @@ 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);
> + ret = dev_priv->display.compute_pipe_wm(pipe_config);
> if (ret) {
> DRM_DEBUG_KMS("Target pipe watermarks are invalid\n");
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 5daf53c080e1..03b7d031e910 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1630,6 +1630,18 @@ intel_atomic_get_crtc_state(struct drm_atomic_state
> *state,
>
> return to_intel_crtc_state(crtc_state);
> }
> +
> +static inline struct intel_plane_state *
> +intel_atomic_get_existing_plane_state(struct drm_atomic_state *state,
> + struct intel_plane *plane)
> +{
> + struct drm_plane_state *plane_state;
> +
> + plane_state = drm_atomic_get_existing_plane_state(state, &plane->base);
> +
> + return to_intel_plane_state(plane_state);
> +}
> +
> int intel_atomic_setup_scalers(struct drm_device *dev,
> struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1b8ba777d2b3..dc1e1683f4a3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1996,11 +1996,18 @@ static void ilk_compute_wm_level(const struct
> drm_i915_private *dev_priv,
> cur_latency *= 5;
> }
>
> - result->pri_val = ilk_compute_pri_wm(cstate, pristate,
> - pri_latency, level);
> - result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency);
> - result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency);
> - result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val);
> + if (pristate) {
> + result->pri_val = ilk_compute_pri_wm(cstate, pristate,
> + pri_latency, level);
> + result->fbc_val = ilk_compute_fbc_wm(cstate, pristate,
> result->pri_val);
> + }
> +
> + if (sprstate)
> + result->spr_val = ilk_compute_spr_wm(cstate, sprstate,
> spr_latency);
> +
> + if (curstate)
> + result->cur_val = ilk_compute_cur_wm(cstate, curstate,
> cur_latency);
> +
> result->enable = true;
> }
>
> @@ -2288,51 +2295,51 @@ static bool ilk_validate_pipe_wm(struct drm_device
> *dev,
> }
>
> /* Compute new watermarks for the pipe */
> -static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
> - struct drm_atomic_state *state)
> +static int ilk_compute_pipe_wm(struct intel_crtc_state *cstate)
> {
> + struct drm_atomic_state *state = cstate->base.state;
> + struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> struct intel_pipe_wm *pipe_wm;
> - struct drm_device *dev = intel_crtc->base.dev;
> + struct drm_device *dev = state->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc_state *cstate = NULL;
> struct intel_plane *intel_plane;
> - struct drm_plane_state *ps;
> struct intel_plane_state *pristate = NULL;
> struct intel_plane_state *sprstate = NULL;
> struct intel_plane_state *curstate = NULL;
> int level, max_level = ilk_wm_max_level(dev), usable_level;
> struct ilk_wm_maximums max;
>
> - cstate = intel_atomic_get_crtc_state(state, intel_crtc);
> - if (IS_ERR(cstate))
> - return PTR_ERR(cstate);
> -
> pipe_wm = &cstate->wm.optimal.ilk;
>
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> - ps = drm_atomic_get_plane_state(state,
> - &intel_plane->base);
> - if (IS_ERR(ps))
> - return PTR_ERR(ps);
> + struct intel_plane_state *ps;
> +
> + ps = intel_atomic_get_existing_plane_state(state,
> + intel_plane);
> + if (!ps)
> + continue;
>
> if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY)
> - pristate = to_intel_plane_state(ps);
> + pristate = ps;
> else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY)
> - sprstate = to_intel_plane_state(ps);
> + sprstate = ps;
> else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)
> - curstate = to_intel_plane_state(ps);
> + curstate = ps;
> }
>
> 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);
> + if (sprstate) {
> + 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);
> + }
> +
>
> usable_level = max_level;
>
> /* ILK/SNB: LP2+ watermarks only w/o sprites */
> - if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible)
> + if (INTEL_INFO(dev)->gen <= 6 && pipe_wm->sprites_enabled)
> usable_level = 1;
>
> /* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */
> --
> 2.1.0
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
