From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 905AEC4338F for ; Wed, 25 Aug 2021 15:58:40 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4542A610FD for ; Wed, 25 Aug 2021 15:58:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4542A610FD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A93A86E39B; Wed, 25 Aug 2021 15:58:39 +0000 (UTC) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id C5EE56E39B for ; Wed, 25 Aug 2021 15:58:37 +0000 (UTC) X-IronPort-AV: E=McAfee;i="6200,9189,10087"; a="204749079" X-IronPort-AV: E=Sophos;i="5.84,351,1620716400"; d="scan'208";a="204749079" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Aug 2021 08:57:19 -0700 X-IronPort-AV: E=Sophos;i="5.84,351,1620716400"; d="scan'208";a="527377726" Received: from mburkard-mobl1.ger.corp.intel.com (HELO localhost) ([10.251.213.64]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Aug 2021 08:57:15 -0700 From: Jani Nikula To: Rodrigo Vivi , "Gupta\, Anshuman" Cc: "intel-gfx\@lists.freedesktop.org" , "Deak\, Imre" , "Tangudu\, Tilak" In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20210824154452.2066678-1-rodrigo.vivi@intel.com> <20210824154452.2066678-2-rodrigo.vivi@intel.com> Date: Wed, 25 Aug 2021 18:57:12 +0300 Message-ID: <878s0pecvb.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/runtime_pm: Let's avoid the undocumented D1 opregion notification. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Wed, 25 Aug 2021, Rodrigo Vivi wrote: > On Wed, Aug 25, 2021 at 09:04:02AM +0000, Gupta, Anshuman wrote: >> >> >> > -----Original Message----- >> > From: Intel-gfx On Behalf Of Rodrigo >> > Vivi >> > Sent: Tuesday, August 24, 2021 9:15 PM >> > To: intel-gfx@lists.freedesktop.org >> > Cc: Vivi, Rodrigo ; Deak, Imre ; >> > Tangudu, Tilak >> > 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 >> > Cc: Tilak Tangudu >> > Signed-off-by: Rodrigo Vivi >> > --- >> > 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