On Mon, 2021-09-13 at 16:22 +0530, Gupta, Anshuman wrote:
>
>
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf
> > Of Rodrigo
> > Vivi
> > Sent: Friday, September 10, 2021 11:15 PM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Tangudu, Tilak
> > <tilak.tangudu@xxxxxxxxx>; Deak, Imre <imre.deak@xxxxxxxxx>
> > Subject: [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@xxxxxxxxx>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > Acked-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > 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
>
