Re: [PATCH 2/2] drm/xe: Ensure d3cold is only allowed in DGFX

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

On Mon, Jun 03, 2024 at 06:15:46PM -0400, Zanoni, Paulo R wrote:
> On Mon, 2024-06-03 at 17:52 -0400, Rodrigo Vivi wrote:
> > For our integrated parts, the GPU is part of the CPU package,
> > hence removing the power at the root port is likely not possible.
> > 
> > Let's add this extra layer of protection to ensure that we are
> > really not seeing d3cold atempts into integrated devices.
> 
> I don't think this is the case.
> 
> It's been a while since I dealt with this, but as far as I remember you
> can put integrated cards in D3 cold by writing 0x3 to PCI register
> 0xd4.

That is D3.  Please notice that there's no difference on D3hot vs D3cold
on that register.

Bits 0-1: Power State (PS)
00: D0
01: D1
10: D2
11: D3

D3Cold is a special sub-case of D3hot, where the main Vcc power is off.
That doesn't happen at the device level, but only at the root port level,
after all the PCI devices under the same root port are in D3hot.

Although technically possible, I doubt that we have any integrated out
there with true D3Cold support. In general, on igfx, we are the device
0000:00:02.0 that is under the root port pci0000:00 with many other devices
directly bound to CPU. I honestly doubt that the power there can be cut.

Also, I recently noticed that even on most of our integrated parts, not
even D3hot is advertised as supported. The gain of the runtime pm on
the integrated comes more from the package-C state, then from the PCI
device power delivery channels actually.

Well, anyway, the rest of our code on d3cold_allowed are more prepared
to deal with discrete by checking local memory and other things. I don't
want to take the risk of unexpected behavior of something we were not
taking into account.

We can re-evaluate this on a next step if we see parts coming out with
this support in place, so we prepare the flow properly.

> 
> https://cdrdv2.intel.com/v1/dl/getcontent/703047 page 990 (printed as
> 960). On BSpec this is page 49664.

yeap, I believe it just shows the possibility, but that would depend
of other agents and hardware changes that are outside of the graphics
domain.

> 
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > Cc: Francois Dugast <francois.dugast@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_pm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index de3b5df65e48..1facb7dd8b66 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -172,6 +172,9 @@ static bool xe_pm_pci_d3cold_capable(struct xe_device *xe)
> >  	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> >  	struct pci_dev *root_pdev;
> >  
> > +	if (!IS_DGFX(xe))
> > +		return false;
> > +
> >  	root_pdev = pcie_find_root_port(pdev);
> >  	if (!root_pdev)
> >  		return false;
>