Re: [PATCH 09/16] drm/i915: More checks for psr.enabled

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jun 18, 2014 at 01:46:16PM +0100, Chris Wilson wrote:
> On Wed, Jun 18, 2014 at 02:41:34PM +0200, Daniel Vetter wrote:
> > On Wed, Jun 18, 2014 at 01:27:06PM +0100, Chris Wilson wrote:
> > > On Wed, Jun 18, 2014 at 01:59:10PM +0200, Daniel Vetter wrote:
> > > > We need to make sure that no one else is using this in the
> > > > enable function and also that the work item hasn't raced
> > > > with the disabled function.
> > > > 
> > > > v2: Improve bisectability by moving one hunk to an earlier patch.
> > > > 
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 910f73de3a92..870219ff1187 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -1844,6 +1844,11 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
> > > >  		return;
> > > >  	}
> > > 
> > > Is this the tail of a HAS_PSR() now made obsolete?
> > 
> > Yeah, we have a bit of redundancy here now I think. Otoh once we have
> > locking they make sense again since HAS_PSR can be checked without
> > grabbing the psr lock, while psr.enabled can't. So I think it makes sense
> > to keep them.
> 
> We do try to encourage the idiom of checking compatibility once at init,
> then checking state at runtime. If the lock becomes burdensome we can
> mitigate it.  Hmm, such as

Ok, convinced, I'll throw a cleanup patch on top.

> frontbuffer_bits &= ~fb_tracking.busy_bits; /* only notify us for changes in state */
> 
> Further promoting the idea of refactoring cpu/gpu/flip tracking. :)

The idea is that consumers like psr track the state and have the edge
detection they care about. Once we have drrs and fbc converted we can have
a look at this again and whether extracting common logic makes sense. But
I suspect that there's not much room given that we always accumulate funky
platform hacks like the hsw sprite case. So passing the unfilter events to
the consumers seemed like the right approach.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch