All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
@ 2014-06-23 12:46 Imre Deak
  2014-06-24 13:54 ` Jani Nikula
  0 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2014-06-23 12:46 UTC (permalink / raw)
  To: intel-gfx

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)
+	if (acpi_target_system_state() < ACPI_STATE_S3)
 		opregion_target_state = PCI_D1;
+#endif
 	intel_opregion_notify_adapter(dev, opregion_target_state);
 
 	intel_uncore_forcewake_reset(dev, false);
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
  2014-06-23 12:46 [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP Imre Deak
@ 2014-06-24 13:54 ` Jani Nikula
  2014-06-24 14:37   ` Imre Deak
  0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2014-06-24 13:54 UTC (permalink / raw)
  To: Imre Deak, intel-gfx

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?

BR,
Jani.

> +	if (acpi_target_system_state() < ACPI_STATE_S3)
>  		opregion_target_state = PCI_D1;
> +#endif
>  	intel_opregion_notify_adapter(dev, opregion_target_state);
>  
>  	intel_uncore_forcewake_reset(dev, false);
> -- 
> 1.8.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
  2014-06-24 13:54 ` Jani Nikula
@ 2014-06-24 14:37   ` Imre Deak
  2014-06-24 14:53     ` Jani Nikula
  0 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2014-06-24 14:37 UTC (permalink / raw)
  To: Jani Nikula, Rafael J. Wysocki; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 2509 bytes --]

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?

--Imre

> 
> BR,
> Jani.
> 
> > +	if (acpi_target_system_state() < ACPI_STATE_S3)
> >  		opregion_target_state = PCI_D1;
> > +#endif
> >  	intel_opregion_notify_adapter(dev, opregion_target_state);
> >  
> >  	intel_uncore_forcewake_reset(dev, false);
> > -- 
> > 1.8.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
  2014-06-24 14:37   ` Imre Deak
@ 2014-06-24 14:53     ` Jani Nikula
  2014-06-24 15:12       ` Imre Deak
  0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2014-06-24 14:53 UTC (permalink / raw)
  To: imre.deak, Rafael J. Wysocki; +Cc: intel-gfx

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?

intel_opregion_notify_adapter() is a NOP for CONFIG_ACPI=n anyway. And
AFAICT CONFIG_ACPI=y && CONFIG_ACPI_SLEEP=n is broken.

BR,
Jani.


>
> --Imre
>
>> 
>> BR,
>> Jani.
>> 
>> > +	if (acpi_target_system_state() < ACPI_STATE_S3)
>> >  		opregion_target_state = PCI_D1;
>> > +#endif
>> >  	intel_opregion_notify_adapter(dev, opregion_target_state);
>> >  
>> >  	intel_uncore_forcewake_reset(dev, false);
>> > -- 
>> > 1.8.4
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
  2014-06-24 14:53     ` Jani Nikula
@ 2014-06-24 15:12       ` Imre Deak
  2014-07-07 23:13         ` Rafael J. Wysocki
  0 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2014-06-24 15:12 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Rafael J. Wysocki


[-- Attachment #1.1: Type: text/plain, Size: 2643 bytes --]

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?
> 
> intel_opregion_notify_adapter() is a NOP for CONFIG_ACPI=n anyway.

Ok, but the question for me is what's the target sleep state in case of
CONFIG_ACPI=y and CONFIG_ACPI_SLEEP=n.

>  And AFAICT CONFIG_ACPI=y && CONFIG_ACPI_SLEEP=n is broken.

But it seems like a valid configuration. So it needs to be fixed
separately.

--Imre

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
  2014-06-24 15:12       ` Imre Deak
@ 2014-07-07 23:13         ` Rafael J. Wysocki
  2014-07-07 23:39           ` Rafael J. Wysocki
  0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2014-07-07 23:13 UTC (permalink / raw)
  To: imre.deak, Jani Nikula; +Cc: intel-gfx

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.

>> intel_opregion_notify_adapter() is a NOP for CONFIG_ACPI=n anyway.
> Ok, but the question for me is what's the target sleep state in case of
> CONFIG_ACPI=y and CONFIG_ACPI_SLEEP=n.
>
>>   And AFAICT CONFIG_ACPI=y && CONFIG_ACPI_SLEEP=n is broken.

Broken how?

> But it seems like a valid configuration. So it needs to be fixed
> separately.

It is a valid configuration.

Rafael

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
  2014-07-07 23:13         ` Rafael J. Wysocki
@ 2014-07-07 23:39           ` Rafael J. Wysocki
  2014-07-08  8:31             ` Daniel Vetter
  0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2014-07-07 23:39 UTC (permalink / raw)
  To: imre.deak, Jani Nikula; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP
  2014-07-07 23:39           ` Rafael J. Wysocki
@ 2014-07-08  8:31             ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2014-07-08  8:31 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: intel-gfx

On Tue, Jul 08, 2014 at 01:39:23AM +0200, Rafael J. Wysocki wrote:
> 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.

Ok, I've merged Imre's original patch to not call acpi_target_system_state
then.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-07-08  8:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-23 12:46 [PATCH] drm/i915: make system freeze support depend on CONFIG_ACPI_SLEEP Imre Deak
2014-06-24 13:54 ` Jani Nikula
2014-06-24 14:37   ` Imre Deak
2014-06-24 14:53     ` Jani Nikula
2014-06-24 15:12       ` Imre Deak
2014-07-07 23:13         ` Rafael J. Wysocki
2014-07-07 23:39           ` Rafael J. Wysocki
2014-07-08  8:31             ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.