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

Animesh Manna
2016-04-13T13:47:25Z

On 4/8/2016 2:35 AM, Imre Deak wrote:
> 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?

Thanks Imre for review, First of all Display/Gfx is a single
function pci device. As per PMC HAS display should not put into
D3 for HPD to work. Once Punit gets a PM response with DC9 Ready,
and conditions outside display indicate the system is ready for
power down, Punit will save device 2 PCI config and device 2 MMIO
registers that are marked for save in the RDL/XML, then remove
power from display completely (Vnn). So I feel if Vnn is down
there is no chance of power leak.

> - 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)?

I think these will be based on h/w architecture. As you mentioned
below #PME is one way which I feel followed by wifi/ethernet devices.
But this can not be general solution for other devices which might
block S0ix, but without display/gfx in D3 as we can achieve S0ix
so have taken this approach.

> - 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?

Not have much idea why it is not supported and is there any plan to support
in 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?

Only BXT has a special HPD control register in PMC which helps routing
the hpd interrupt during sleep. So other platform we can not support
this feature.

>
> 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?

Need to explore with platform architect, but we already had a discussion and this
approach is recommended by platform architecture team.

>
> 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).

Ok .. I can try out. This is doable.

Regards,
Animesh

>
> --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(-)
>>