On Wed, 25 Aug 2021, Rodrigo Vivi <[email protected]> wrote: > On Wed, Aug 25, 2021 at 09:04:02AM +0000, Gupta, Anshuman wrote: >> >> >> > -----Original Message----- >> > From: Intel-gfx <[email protected]> On Behalf Of >> > Rodrigo >> > Vivi >> > Sent: Tuesday, August 24, 2021 9:15 PM >> > To: [email protected] >> > Cc: Vivi, Rodrigo <[email protected]>; Deak, Imre >> > <[email protected]>; >> > Tangudu, Tilak <[email protected]> >> > Subject: [Intel-gfx] [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 <[email protected]> >> > Cc: Tilak Tangudu <[email protected]> >> > Signed-off-by: Rodrigo Vivi <[email protected]> >> > --- >> > 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 for sharing this info. > I left this out of my new series while we investigate internally why this > doesn't match > the spec and looks more like the command 8 where 0 is "on" and 1 is "standby"
I've let myself be told current gen platforms shouldn't have swsci at all, but in practice we encounter opregions with swsci mailbox flag set. BR, Jani. > > Thanks, > Rodrigo. > >> Thanks, >> Anshuman Gupta >> > } >> > >> > assert_forcewakes_inactive(&i915->uncore); >> > -- >> > 2.31.1 >> -- Jani Nikula, Intel Open Source Graphics Center
