After discussions with Art, Ville and others internally on how DC5/6 should normally work and existing open issues related to the HW automatic DC5/6 functionality, I have the following understanding. In the future we will require the firmware for any display side runtime power management (and possibly some power savings during system power states like S0ix and S3), so in the following we can ignore the case when the firmware is not available. Normally we need to run the BSpec display init sequence only when loading the driver and resuming from S4 if the BIOS hasn't done so already. PW1 and CDCLK is enabled as part of this sequence and will be toggled by the DMC firmware on demand, so we must not toggle these manually during runtime, since that would race with the FW. The same goes goes for other resources enabled/reset in the init sequence: PCH reset handshake, DBUF, Misc IO. The manual controls are DC5, DC6 and PW2 which the driver needs to toggle based on the active outputs. We also need to disable DC5/6 while doing modeset and according to our tests DPAUX transfers. When HW enters DC6 and the GT side is idle too (in RC6) the system as a whole can enter a deeper power state (PC9/10). Due to a HW issue atm we cannot rely on the HW transitioning automatically to DC6 and we need to use a manual way instead. We do this by keeping DC6 disabled all the time and running a special deinitialization sequence when both the display and GT side is idle, and the reverse when something becomes active. (that is in the driver's runtime suspend/resume hooks). Currently the driver is not following the normal HW assisted flow, since it toggles DBUF, CDCLK and PW1 manually during runtime. It also lacks the manual sequence to enable PC9/10. This patchset solves some of the above issues, by removing the manual CDCLK and PW1 control and disabling DC6. This will solve some of the stability issues, but leave PC9/10 disabled. As a follow-up we need to - move the rest of the display init/uninit sequence out of the power_well/suspend/resume path and call it only during driver loading/S4 resume as necessary - add separate DC5/DC6 power wells - add the manual sequence to enable PC9/10 - add a module parameter to enable either HW assisted DC6, or the manual way, or disable DC6 - disable DC5/6 during modeset and other places like DPAUX as needed I'll comment on the individual patches based on the above. I think with those addressed we could merge this patchset and do the rest as a follow-up. --Imre On ke, 2015-08-26 at 01:36 +0530, Animesh Manna wrote: > The following patches helps to solve PC10 entry issue for SKL. > Detailed description about the changes done to solve the issue > is mentioned in commit message of each patch. > > v1: http://lists.freedesktop.org/archives/intel-gfx/2015-August/072870.html > > v2: Based on review comments from Daniel, changes made in the current version. > > DMC redesign patch series has dependencies with current patch series. Need > to rework on few patches, planning to send after initial review feedback > of the current patch series. > http://lists.freedesktop.org/archives/intel-gfx/2015-August/072921.html > > > Animesh Manna (5): > drm/i915/skl: Added a check for the hardware status of csr fw before > loading. > drm/i915/skl Remove the call for csr uninitialization from suspend > path > drm/i915/skl: Making DC6 entry is the last call in suspend flow. > drm/i915/skl: Do not disable cdclk PLL if csr firmware is present > drm/i915/skl: Block disable call for pw1 if dmc firmware is present. > > drivers/gpu/drm/i915/i915_drv.c | 19 +++++++++++++------ > drivers/gpu/drm/i915/intel_csr.c | 9 +++++++++ > drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++---- > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 31 ++++++++++++++++--------------- > 5 files changed, 50 insertions(+), 25 deletions(-) > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx