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

Ville Syrjälä
2014-11-07T13:31:25Z

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.

--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org