On Tuesday 28 July 2015 01:27 PM, Daniel Vetter wrote:
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
Thanks alot Damien.
Animesh,
As discussed in same call, bug fix should go initial patches in this series.
also its good to add the check in skl_set_power_well function itself
than intel_display_power_put
- Sunil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
