[Intel-gfx] [PATCH 2/2] drm/i915: Only recalculate wm's for planes part of the state, v2.
Zanoni, Paulo R
paulo.r.zanoni at intel.com
Wed Mar 2 21:08:31 UTC 2016
Em Ter, 2016-03-01 às 14:28 -0800, Matt Roper escreveu:
> 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.lankhorst at linux.intel.com
> > >
> > Cc: Matt Roper <matthew.d.roper at 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.roper at 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.
I spent some time looking at that early return from invalid pipe
watermarks, but I suppose that return means we'll completely discard
anything just computed by the function, right?
The patch seems to do what it says on the box, so if we assume we
actually want the patch:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
I'll let you and Matt decide whether we actually want the patch or not.
> 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_p
> > lane);
> > + 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
> >
>
More information about the Intel-gfx
mailing list