[Intel-gfx] [PATCH 04/18] drm/i915/gen9: block disable call for pw1 if dmc firmware is present.

[Daniel Vetter]

On Mon, Jul 27, 2015 at 10:48:16AM +0200, Daniel Vetter wrote:
> On Sun, Jul 26, 2015 at 12:30:25AM +0530, Animesh Manna wrote:
> > Grabbing a runtime pm reference with intel_runtime_pm_get will only
> > prevent device D3. But dmc firmware is required even earlier (namely
> > for the skl power well 1). DMC is responsible to save the status of
> > power well 1 and shut off the power well when panel is self refresh
> > mode of display is completely off.
> > 
> > Another interesting criteria to work dmc as expected is pw1 to be
> > enabled by driver and dmc will shut it off in its execution
> > sequence. If already disabled by driver dmc will get confuse and
> > behave differently than expected found during pc10 entry issue
> > for skl.
> > 
> > So berfore we disable power-well 1, added check if dmc firmware is
> > present and driver will not disable power well 1, but for any reason
> > if firmware is not present of failed to load we can shut off the
> > power well 1 which will save some power.
> > 
> > As skl is currently fully dependent on dmc to go in lowest possible
> > power state (dc6) but the same is not applicable for bxt. Display
> > engine can enter into dc9 without dmc, hence unblocking disable call.
> > 
> > Cc: Daniel Vetter <daniel.vetter at intel.com>
> > Cc: Damien Lespiau <damien.lespiau at intel.com>
> > Cc: Imre Deak <imre.deak at intel.com>
> > Cc: Sunil Kamath <sunil.kamath at intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> > Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju at intel.com>
> 
> Please use the approach I've laid out in my original patch series with
> "drm/i915: use correct power domain for csr loading" and "drm/i915: Only
> allow rpm on gen9+ with dmc loaded". Your approach here requires a
> flush_work in the runtime pm callbacks which can deadlock.
> 
> If you want to make dmc optional on bxt then you need to adjust the 2nd
> patch a bit to no longer leak the runtime pm reference. Your approach here
> is an inversion of control and that doesn't work well since control
> inversions very easily lead to locking inversions.

Summary of what we just discussed offline:

Ok I was confused here with the intel_csr_load_status_get() check. If we
apply the above to patch from me first then we don't need that check any
more, and the patch itself looks good as a bugfix for skl dmc firmware
assumptions.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch