Re: [PATCH] drm/i915/gen9: Re-allocate DDB only for changed pipes

[Matt Roper <matthew.d.roper@intel.com>]

On Tue, Jun 28, 2016 at 10:50:20AM +0200, Maarten Lankhorst wrote:
> Op 28-06-16 om 01:42 schreef Matt Roper:
> > When a display update triggers a DDB re-allocation, we should start by
> > assuming that only the updated pipes need to be re-allocated (we have
> > logic later that may add additional pipes if, e.g., a modeset triggers a
> > change to the global allocation).
> >
> > We were erroneously using the _active_ pipes as our starting point
> > rather than the changed pipes.  This causes us to grab CRTC locks that
> > we didn't actually need, reducing parallelism.  Given the recent
> > non-blocking atomic changes, it also causes legacy pageflips against one
> > CRTC to return -EBUSY if there's an outstanding pageflip against a
> > different CRTC (a situation easily triggered via compositors like
> > Weston).
> >
> > Fixes: 98d39494d3 ("drm/i915/gen9: Compute DDB allocation at atomic check time (v4)")
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index c94521cc..d7f8ba8 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3879,6 +3879,19 @@ static int skl_update_pipe_wm(struct drm_crtc_state *cstate,
> >  	return 0;
> >  }
> >  
> > +static uint32_t
> > +pipes_modified(struct drm_atomic_state *state)
> > +{
> > +	struct drm_crtc *crtc;
> > +	struct drm_crtc_state *cstate;
> > +	uint32_t i, ret = 0;
> > +
> > +	for_each_crtc_in_state(state, crtc, cstate, i)
> > +		ret |= drm_crtc_mask(crtc);
> > +
> > +	return ret;
> > +}
> > +
> >  static int
> >  skl_compute_ddb(struct drm_atomic_state *state)
> >  {
> > @@ -3887,7 +3900,7 @@ skl_compute_ddb(struct drm_atomic_state *state)
> >  	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> >  	struct intel_crtc *intel_crtc;
> >  	struct skl_ddb_allocation *ddb = &intel_state->wm_results.ddb;
> > -	unsigned realloc_pipes = dev_priv->active_crtcs;
> > +	uint32_t realloc_pipes = pipes_modified(state);
> >  	int ret;
> >  
> >  	/*
> 
> Hey,
> 
> Thanks, this is the original bug that triggered the need for "[PATCH] drm/atomic: Make drm_atomic_legacy_backoff reset crtc->acquire_ctx".
> Fixing this should remove unnecessary stalls in atomic commit. :-)
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 

Merged to dinq; thanks for the review.


Matt

-- 
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