Re: [PATCH v2 0/6] HPD support during suspend.

Imre Deak
2016-04-07T21:05:41Z

On Thu, 2016-04-07 at 20:22 +0530, Animesh Manna wrote:
> Along with below patches sharing some background details/design.
> - On BXT, Display cannot generate an interrupt when in D3.
> - Without display in D3, S0ix can be achieved, Power impact
> will be zero if d3 is blocked. PMCSR for Graphics/Display
> is irrelevant, as the power management for them is taken
> care by the PUNIT using RC6/DC9 hints and *not* through
> PMCSR write trigger.
>
> So solution is based on below principles:
> - Display should not be put into D3 for HPD to work.
> - With D0+DC9 we can achieve S0ix and at the same time
> helps to get the interrupt.
> - Using pci_save_state() during suspend to take control
> from OSPM and blocking D3, while resuming, giving back
> to OSPM by pci_restore_state().
> - _DSM method is used to program pmc hpd control register
> to enable hpd during suspend.
>
> Please have a look and send your comments/suggestions.

Good to see this getting fixed. Some notes:

Although I also suspect that D3 doesn't provide any power saving over
RC6/DC9, D3 is still a standard PCI device state shown as supported by
the GFX PCI device capability descriptor. So before going ahead with
this approach we'd need a better explanation on why preventing D3 is
ok, instead of just stating it:
- What's the exact effect of D3? We know that it at least masks the
device interrupts, but is it guaranteed (by some specification for
example) that there aren't and (and in the future won't be) any other
effects, especially power related?
- How does the GFX device compare to other devices in the system in
this regard, are there devices with a similar limitation? If so, is the
solution on those the same (preventing D3)?
- Why isn't the standard PCI wake-up mechanism supported (that is PME)?
Just some design-time overlook, or some practical problem with its
implementation? Can we expect that PME will be supported in the future?
- How does BXT compare to the other platforms regarding this problem?
We know that all of them with runtime power management support have the
same problem. On some of them (at least HSW, BDW) we could use the same
workaround (preventing D3) as a solution, but what's the exact list of
these platforms? Does preventing D3 on any of these have a negative
impact?

I noticed that the BXT HPD pins can be configured to work in GPIO mode
(GPIO 199, 200). Could we instead of the above approach use these as a
wake-up source while putting the device into D3?

About the actual patch: now we'll do the whole interrupt processing
while the device is still runtime suspended. This may work, but is
unusual which I'd rather avoid. Instead I'd just handle the PCU IRQ
itself and do the rest of processing after resuming the device (from a
scheduled work).

--Imre

> Animesh Manna (6):
>   drm/i915/bxt: VBT changes for hpd as wakeup feature
>   drm/i915/bxt: Added _DSM call to set HPD_CTL.
>   drm/i915/bxt: Corrected the guid for bxt.
>   drm/i915/bxt: Block D3 during suspend.
>   drm/i915: Enable HPD interrupts with master ctl interrupt
>   drm/i915/bxt: Enable HPD during suspend.
>
>  drivers/gpu/drm/i915/i915_drv.c       |  6 ++++
>  drivers/gpu/drm/i915/i915_drv.h       |  8 +++++
>  drivers/gpu/drm/i915/i915_irq.c       | 56
> +++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/i915_reg.h       | 13 ++++++++
>  drivers/gpu/drm/i915/intel_acpi.c     | 53
> ++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_bios.c     |  7 +++++
>  drivers/gpu/drm/i915/intel_vbt_defs.h |  3 +-
>  7 files changed, 134 insertions(+), 12 deletions(-)