> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Rodrigo
> Vivi
> Sent: Tuesday, August 24, 2021 9:15 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Deak, Imre <imre.deak@xxxxxxxxx>;
> Tangudu, Tilak <tilak.tangudu@xxxxxxxxx>
> Subject: [PATCH 2/2] drm/i915/runtime_pm: Let's avoid the
> undocumented D1 opregion notification.
>
> At least for newer generations, let's try to do the right thing that is to notify the
> opregion that we are going into D3hot.
>
> But to avoid breaking the world let's keep the older undocumented behavior in
> place.
>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Tilak Tangudu <tilak.tangudu@xxxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_runtime_pm.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 43cdc2f3ff9e..371bbc58db92 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -706,27 +706,19 @@ int intel_runtime_pm_suspend(struct
> intel_runtime_pm *rpm)
>
> rpm->suspended = true;
>
> - /*
> - * FIXME: We really should find a document that references the
> arguments
> - * used below!
> - */
> - if (IS_BROADWELL(i915)) {
> - /*
> - * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop
> - * being detected, and the call we do at intel_runtime_resume()
> - * won't be able to restore them. Since PCI_D3hot matches the
> - * actual specification and appears to be working, use it.
> - */
> - intel_opregion_notify_adapter(i915, PCI_D3hot);
> - } else {
> + if (GRAPHICS_VER(i915) < 8) {
> /*
> - * current versions of firmware which depend on this opregion
> - * notification have repurposed the D1 definition to mean
> + * Some older versions of firmware which depend on this
> opregion
> + * notification had repurposed the D1 definition to mean
> * "runtime suspended" vs. what you would normally expect
> (D3)
> * to distinguish it from notifications that might be sent via
> - * the suspend path.
> + * the suspend path. Unfortunately there's no documentation
> + * available right now to justify this flow. However let's
> + * keep for historical reasons.
> */
> intel_opregion_notify_adapter(i915, PCI_D1);
> + } else {
> + intel_opregion_notify_adapter(i915, PCI_D3hot);
This is going to call the opregion ACPI SBCB method with function SWSCI_SBCB_ADAPTER_POWER_STATE i.e. value =7 and with input PARAM value as input power state (PCI_D0, PCI_D1, ...).
Below is the TGL SBCB method code block for command SWSCI_SBCB_ADAPTER_POWER_STATE (this method can be retrieve from one of SSDT table in /sys/firmware/acpi/tables/SSDT*)
If ((GESF == 0x07))
{
If (((S0ID == One) && (OSYS < 0x07DF)))
{
If (((PARM & 0xFF) == One))
{
ADBG ("IgSbcb:GUAM(1)")
\GUAM (One)
}
If (((PARM & 0xFF) == Zero))
{
ADBG ("IgSbcb:GUAM(0)")
\GUAM (Zero)
}
}
If ((PARM == Zero))
{
Local0 = CLID /* \_SB_.PC00.GFX0.CLID */
If ((0x80000000 & Local0))
{
CLID &= 0x0F
GLID (CLID)
}
}
GESF = Zero
PARM = Zero
Return (SUCC) /* \_SB_.PC00.GFX0.SUCC */
}
With above code block, it either checks for input PARAM value either 0 or 1.
I am not sure but passing PCI_D3hot as input parameter may affect the SBCB functionality. here
Thanks,
Anshuman Gupta
> }
>
> assert_forcewakes_inactive(&i915->uncore);
> --
> 2.31.1
