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

[Imre Deak <imre.deak@intel.com>]

[-- 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