Re: [PATCH 1/2] drm/xe/pm: Temporarily disable D3Cold on BMG

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

On Fri, Mar 07, 2025 at 04:15:01PM -0600, Lucas De Marchi wrote:
> On Thu, Mar 06, 2025 at 04:36:14PM -0500, Rodrigo Vivi wrote:
> > Currently, many instability cases related to D3Cold -> D0 transition
> > on BMG are under investigation. Among them some bad cases where
> > the device is lost after 1 to 3 transitions from D3Cold to D0
> > on the runtime pm, with pcieport upstream bridge port link retrain
> > failure.
> > 
> > In other cases, it works fine, but with some sudden random memory
> > corruptions after D3cold, that could be 0xffff missed ack on GT
> > forcewake or GuC reload related failures.
> > 
> > In some other cases though, D3Cold -> D0 works pretty reliably.
> > It looks like it is a combination of GPU cards and Host boards at
> > this point. So, there is no possible/available quirk at this time.
> > 
> > This patch disables the D3Cold by default on BMG by reducing the
> > vram_d3cold_threshold to 0. Users and developers who wants to enable
> > it are still able to via
> > $ echo 300 > /sys/bus/pci/devices/<addr>/vram_d3cold_threshold
> > 
> > Fixes: 3adcf970dc7e ("drm/xe/bmg: Drop force_probe requirement")
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/4037
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/4395
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/4396
> 
> are these Link: or should we use Closes: ?

I don't want to close them while we are in the investigation.
So it is either Link: or References:, which check patch doesn't like.

> 
> > Cc: Karthik Poosa <karthik.poosa@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_pm.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index 12200be7b43d..a9f61a5fc971 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -287,6 +287,7 @@ ALLOW_ERROR_INJECTION(xe_pm_init_early, ERRNO); /* See xe_pci_probe() */
> >  */
> > int xe_pm_init(struct xe_device *xe)
> > {
> > +	u32 vram_threshold;
> > 	int err;
> > 
> > 	/* For now suspend/resume is only allowed with GuC */
> > @@ -300,7 +301,11 @@ int xe_pm_init(struct xe_device *xe)
> > 		if (err)
> > 			return err;
> > 
> > -		err = xe_pm_set_vram_threshold(xe, DEFAULT_VRAM_THRESHOLD);
> > +		/* FIXME: D3Cold temporarily disabled by default on BMG */
> > +		vram_threshold = xe->info.platform == XE_BATTLEMAGE ? 0 :
> > +				DEFAULT_VRAM_THRESHOLD;
> 
> we usually have to extract this for different values per platform, so
> maybe just go ahead and do that?
> 
> 	u32 vram_threshold_value(struct xe_device *xe)
> 	{
> 		/* FIXME: D3Cold temporarily disabled by default on BMG */
> 		if (xe->info.platform == XE_BATTLEMAGE)
> 			return 0;
> 
> 		return DEFAULT_VRAM_THRESHOLD;
> 	}
> 
> 	xe_pm_init()
> 	{
> 		...
> 		vram_threshold = vram_threshold_value(xe);
> 	}
> 
> Then the second patch simply removes the first 3 lines of that function.

Good idea! I will change. Thank you!

> Anyway, I agree with the approach to get things working. We can try
> enabling d3cold again when we understand what's going on.
> 
> 
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> 
> for both patches.
> 
> thanks
> Lucas De Marchi
> 
> > +
> > +		err = xe_pm_set_vram_threshold(xe, vram_threshold);
> > 		if (err)
> > 			return err;
> > 	}
> > -- 
> > 2.48.1
> >