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@xxxxxxxxx> > > Cc: Damien Lespiau <damien.lespiau@xxxxxxxxx> > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > Cc: Sunil Kamath <sunil.kamath@xxxxxxxxx> > > Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx> > > Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@xxxxxxxxx> > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
