From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP Date: Tue, 08 Jul 2014 01:39:23 +0200 Message-ID: <53BB2FAB.7060701@intel.com> References: <1403527562-28919-1-git-send-email-imre.deak@intel.com> <87a9928lbu.fsf@intel.com> <1403620632.24758.26.camel@intelbox> <877g468ilc.fsf@intel.com> <1403622726.24758.29.camel@intelbox> <53BB2983.3030509@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 1B1F989F77 for ; Mon, 7 Jul 2014 16:39:26 -0700 (PDT) In-Reply-To: <53BB2983.3030509@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: imre.deak@intel.com, Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 7/8/2014 1:13 AM, Rafael J. Wysocki wrote: > On 6/24/2014 5:12 PM, Imre Deak wrote: >> On Tue, 2014-06-24 at 17:53 +0300, Jani Nikula wrote: >>> On Tue, 24 Jun 2014, Imre Deak wrote: >>>> On Tue, 2014-06-24 at 16:54 +0300, Jani Nikula wrote: >>>>> On Mon, 23 Jun 2014, Imre Deak wrote: >>>>>> To achieve further power savings during system freeze (aka connected >>>>>> standby, or s0ix) we have to send a PCI_D1 opregion notification. As >>>>>> the information about the state we're entering (system freeze, >>>>>> suspend to ram or suspend to disk) is only available through the >>>>>> ACPI >>>>>> subsystem, make this support depend on the relevant kconfig option. >>>>>> Things will still work if this option isn't set, albeit with less >>>>>> than >>>>>> optimial power saving. >>>>>> >>>>>> This also fixes a compile breakage when the option is not set >>>>>> introduced >>>>>> in >>>>>> >>>>>> commit e5747e3adcd67ae27105003ec99fb58cba180105 >>>>>> Author: Jesse Barnes >>>>>> Date: Thu Jun 12 08:35:47 2014 -0700 >>>>>> >>>>>> drm/i915: send proper opregion notifications on suspend/resume >>>>>> >>>>>> Reported-by: Randy Dunlap >>>>>> Signed-off-by: Imre Deak >>>>>> --- >>>>>> drivers/gpu/drm/i915/i915_drv.c | 7 ++++--- >>>>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c >>>>>> b/drivers/gpu/drm/i915/i915_drv.c >>>>>> index 7ae4e2a..43dc8f7 100644 >>>>>> --- a/drivers/gpu/drm/i915/i915_drv.c >>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.c >>>>>> @@ -544,10 +544,11 @@ static int i915_drm_freeze(struct >>>>>> drm_device *dev) >>>>>> i915_save_state(dev); >>>>>> - if (acpi_target_system_state() >= ACPI_STATE_S3) >>>>>> - opregion_target_state = PCI_D3cold; >>>>>> - else >>>>>> + opregion_target_state = PCI_D3cold; >>>>>> +#if IS_ENABLED(CONFIG_ACPI_SLEEP) >>>>> Maybe this should just check for CONFIG_ACPI? >>>> I wanted to send the PCI_D1 signal only if we are sure that the target >>>> sleep state is S0ix (or S1/2) and fall back to the old behavior to >>>> send >>>> PCI_D3cold in all other cases. >>>> >>>> But you are right, it would make much sense if CONFIG_ACPI_SLEEP=n the >>>> target state would be always S0ix. Rafael could you confirm this? > > The target state should be S0 for CONFIG_ACPI_SLEEP unset. > Actually, no, it shouldn't. Or rather it depends on why CONFIG_ACPI_SLEEP is unset. If that's because CONFIG_ACPI is unset, the target sleep state is undefined and acpi_target_system_state() should not be called then. Rafael