Re: [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP

["Rafael J. Wysocki" <rafael.j.wysocki@intel.com>]

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 <imre.deak@intel.com> wrote:
>>>> On Tue, 2014-06-24 at 16:54 +0300, Jani Nikula wrote:
>>>>> On Mon, 23 Jun 2014, Imre Deak <imre.deak@intel.com> 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 <jbarnes@virtuousgeek.org>
>>>>>> Date:   Thu Jun 12 08:35:47 2014 -0700
>>>>>>
>>>>>>      drm/i915: send proper opregion notifications on suspend/resume
>>>>>>
>>>>>> Reported-by: Randy Dunlap <rdunlap@infradead.org>
>>>>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>>>>> ---
>>>>>>   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