Re: [Intel-gfx] [PATCH 2/3] drm/i915: Disallow D3Cold.

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

On Mon, 2021-09-13 at 16:22 +0530, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> > Of Rodrigo
> > Vivi
> > Sent: Friday, September 10, 2021 11:15 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Tangudu, Tilak
> > <tilak.tangudu@intel.com>; Deak, Imre <imre.deak@intel.com>
> > Subject: [Intel-gfx] [PATCH 2/3] drm/i915: Disallow D3Cold.
> > 
> > During runtime or s2idle suspend and resume cases on discrete
> > cards, if D3Cold
> > is really achieved, we will blow everything up and freeze the
> > machine because
> > we are not yet handling the pci states properly.
> > 
> > On Integrated it simply doesn't matter because D3hot is the maximum
> > that we
> > will get anyway, unless the system is on S3/S4 and our power is
> > cut.
> > 
> > Let's put this hammer for now everywhere. So we can work to enable
> > the auto-
> > suspend by default without blowing up the world.
> > 
> > Then, this should be removed when we finally fix the D3Cold flow.
> > 
> > Cc: Tilak Tangudu <tilak.tangudu@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Acked-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index a40b5d806321..086a9a475ce8 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -301,6 +301,7 @@ static void sanitize_gpu(struct
> > drm_i915_private *i915)
> >   */
> >  static int i915_driver_early_probe(struct drm_i915_private
> > *dev_priv)  {
> > +       struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> >         int ret = 0;
> > 
> >         if (i915_inject_probe_failure(dev_priv))
> > @@ -331,6 +332,13 @@ static int i915_driver_early_probe(struct
> > drm_i915_private *dev_priv)
> >         if (ret < 0)
> >                 return ret;
> > 
> > +       /*
> > +        * FIXME: Temporary hammer to avoid freezing the machine on
> > our
> > DGFX
> > +        * This should be totally removed when we handle the pci
> > states
> > properly
> > +        * on runtime PM and on s2idle cases.
> > +        */
> > +       pci_d3cold_disable(pdev);
>         This still doesn't protect, if user space enables d3cold via
> sys-fs.
>         d3cold_allowed_store() may call pci_d3cold_enable()
>         Is it possible to disable it via PCI PM Caps at early probe?
>         Otherwise it should done in respective suspend callback to
> make sure
>         d3cold is disabled.

Well, I'm not too concerned about the case that user goes and tweak
something that he shouldn't. In especial a temporary hack like this,
and with us actively working to remove that.

However you have a point and Tilak also is trying to convince me that
his version which disables it on the relevant suspend paths and re-
enables it on the resume is safer for s3 and s4.

I'm also not fully convinced there becasue on s3/s4 we lose power
ayways, however I'm not going to be stubborn and listen to both of you
;)

Please let's go with Tilak's version.

I just believe that we need a way with a temporary kernel parameter to
be able to allow d3cold on rpm and s2idle so while we are working to
fix d3cold paths properly.

Thanks,
Rodrigo.

> 
> Thanks,
> Anshuman Gupta.
> > +
> >         ret = vlv_suspend_init(dev_priv);
> >         if (ret < 0)
> >                 goto err_workqueues;
> > --
> > 2.31.1
>