[Intel-gfx] [PATCH 72/89] drm/i915/skl: Enable/disable power well for aux transaction

[Ville Syrjälä]

On Fri, Nov 07, 2014 at 01:49:43PM +0000, Damien Lespiau wrote:
> On Fri, Nov 07, 2014 at 03:31:20PM +0200, Ville Syrjälä wrote:
> > On Fri, Nov 07, 2014 at 01:11:13PM +0000, Damien Lespiau wrote:
> > > On Tue, Sep 16, 2014 at 04:19:07PM +0300, Imre Deak wrote:
> > > > This patch needs to be rebased on the recent PPS changes at least
> > > > getting/putting the AUX power domain in pps_lock()/pps_unlock() too.
> > > > Also it should be squashed into 69/89. Some more comments below.
> > > 
> > > Hum, do we really need to hold a reference to the AUX power in
> > > pps_lock(), it doesn't seem that the PPS hw should a specific dependency
> > > on the AUX power domain. We might as well just get the "port" power
> > > domain if that's enough.
> > 
> > pps_lock() doesn't *really* need any power domain references. The thing
> > is that the VLV/CHV display power well hooks need to reset the pps_pipe
> > assignment which means they should grab pps_mutex. However the vdd code
> > can grab power domain references while already holding pps_mutex, which
> > gets us into a neat locking inversion with the power domain mutex.
> > 
> > The workaround I did was to grab the power domain references always around
> > pps_mutex, so that we dodge the problem. A better solution would be to
> > never grab power domain references while already holding pps_mutex, but
> > I wasn't happy with how the code started to look when I tried that. But I
> > would welcome any efforts to make that happen since the current trick is
> > rather hackish. Also the code is now otherwise cleaner than what it was
> > when I tried to do that, so maybe it's easier now.
> 
> Right, but then the remark was that we don't need the aux power domain
> to keep the power well on on VLV/CHV, so would you be happy to just take
> the port power domain around pps_lock/unlock().

Yeah. Actually just the disp2/pipe-a well is needed, so even the port
domains are pointless but I was feeling lazy when I came up with the
hack.

I (or someone) should also do some tests on vlv/chv to see which power
wells are actually needed for AUX. disp2d/pipe-a obviously just for the
registers, but then I'm not sure if the phy cmnlane well is needed. On
chv the docs do say that the AUX stuff is in the common lane, but IIRC
on vlv it was said to be more of a separate lane. Anyway would be nice
to know for sure.

-- 
Ville Syrjälä
Intel OTC