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

Animesh Manna
2016-04-13T14:00:15Z

On 4/13/2016 7:17 PM, Animesh Manna wrote:
>
>
> 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.

In my previous response using "this" I wanted to mean as "Blocking 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?
>
> 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(-)
>>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx