On Tue, Feb 25, 2025 at 06:00:04PM +0000, Gupta, Anshuman wrote:
>
>
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Sent: Tuesday, February 25, 2025 11:20 PM
> > To: Gupta, Anshuman <anshuman.gupta@xxxxxxxxx>
> > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; linux-
> > pci@xxxxxxxxxxxxxxx; rafael@xxxxxxxxxx; lenb@xxxxxxxxxx;
> > bhelgaas@xxxxxxxxxx; ilpo.jarvinen@xxxxxxxxxxxxxxx; De Marchi, Lucas
> > <lucas.demarchi@xxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Nilawar,
> > Badal <badal.nilawar@xxxxxxxxx>; Nasim, Kam <kam.nasim@xxxxxxxxx>
> > Subject: Re: [RFC 5/6] drm/xe/pm: D3Cold target state
> >
> > On Mon, Feb 24, 2025 at 10:18:48PM +0530, Anshuman Gupta wrote:
> > > Trade-off D3Cold target state based upon current vram usages.
> > > if vram usages is greater than vram_d3cold_threshold and GPU has
> > > display connected
> >
> > Why would anyone care about displays being connected or not?
> As per specs we got to enable vrsr only when there is display connected,
What specs? And why do they say connected displays should be
a factor here?
I think the only thing that makes any sense for this kind of stuff
would be sysfs power/ knobs that allow the system administrator to
tune the behaviour for their specific use case. And if no such knobs
exist currently then perhaps they should be added.
> We can check that in probe but a drm connector status can change after completion of probe. That is the reason to put a check for display connected in idle callback.
> Thanks,
> Anshuman
> >
> > > then target D3Cold state is D3Cold-VRSR otherwise target state is
> > > D3COLD-Off.
> > >
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/xe/display/xe_display.c | 22 ++++++++++++++++++++++
> > > drivers/gpu/drm/xe/display/xe_display.h | 1 +
> > > drivers/gpu/drm/xe/xe_pm.c | 12 ++++++++++++
> > > 3 files changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c
> > > b/drivers/gpu/drm/xe/display/xe_display.c
> > > index 02a413a07382..140a43d6b1b6 100644
> > > --- a/drivers/gpu/drm/xe/display/xe_display.c
> > > +++ b/drivers/gpu/drm/xe/display/xe_display.c
> > > @@ -548,3 +548,25 @@ int xe_display_probe(struct xe_device *xe)
> > > unset_display_features(xe);
> > > return 0;
> > > }
> > > +
> > > +bool xe_display_connected(struct xe_device *xe) {
> > > + struct drm_connector *list_connector;
> > > + struct drm_connector_list_iter iter;
> > > + bool ret = false;
> > > +
> > > + mutex_lock(&xe->drm.mode_config.mutex);
> > > + drm_connector_list_iter_begin(&xe->drm, &iter);
> > > +
> > > + drm_for_each_connector_iter(list_connector, &iter) {
> > > + if (list_connector->status == connector_status_connected) {
> > > + ret = true;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + drm_connector_list_iter_end(&iter);
> > > + mutex_unlock(&xe->drm.mode_config.mutex);
> > > +
> > > + return ret;
> > > +}
> > > diff --git a/drivers/gpu/drm/xe/display/xe_display.h
> > > b/drivers/gpu/drm/xe/display/xe_display.h
> > > index 685dc74402fb..c6bc54323084 100644
> > > --- a/drivers/gpu/drm/xe/display/xe_display.h
> > > +++ b/drivers/gpu/drm/xe/display/xe_display.h
> > > @@ -40,6 +40,7 @@ void xe_display_pm_resume(struct xe_device *xe);
> > > void xe_display_pm_runtime_suspend(struct xe_device *xe); void
> > > xe_display_pm_runtime_suspend_late(struct xe_device *xe); void
> > > xe_display_pm_runtime_resume(struct xe_device *xe);
> > > +bool xe_display_connected(struct xe_device *xe);
> > >
> > > #else
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > > index 81e67b5693dc..6d28aedcb062 100644
> > > --- a/drivers/gpu/drm/xe/xe_pm.c
> > > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > > @@ -198,6 +198,14 @@ static void xe_rpm_lockmap_release(const struct
> > xe_device *xe)
> > > &xe_pm_runtime_d3cold_map);
> > > }
> > >
> > > +static void xe_pm_suspend_prepare(struct xe_device *xe) {
> > > + if (pm_suspend_target_state == PM_SUSPEND_TO_IDLE)
> > > + xe_pm_d3cold_allowed_toggle(xe);
> > > + else
> > > + xe->d3cold.allowed = XE_D3COLD_OFF; }
> > > +
> > > /**
> > > * xe_pm_suspend - Helper for System suspend, i.e. S0->S3 / S0->S2idle
> > > * @xe: xe device instance
> > > @@ -213,6 +221,8 @@ int xe_pm_suspend(struct xe_device *xe)
> > > drm_dbg(&xe->drm, "Suspending device\n");
> > > trace_xe_pm_suspend(xe, __builtin_return_address(0));
> > >
> > > + xe_pm_suspend_prepare(xe);
> > > +
> > > err = xe_pxp_pm_suspend(xe->pxp);
> > > if (err)
> > > goto err;
> > > @@ -875,6 +885,8 @@ void xe_pm_d3cold_allowed_toggle(struct
> > xe_device
> > > *xe)
> > >
> > > if (total_vram_used_mb < xe->d3cold.vram_threshold)
> > > xe->d3cold.allowed = XE_D3COLD_OFF;
> > > + else if (xe->d3cold.vrsr_capable && xe_display_connected(xe))
> > > + xe->d3cold.allowed = XE_D3COLD_VRSR;
> > > else
> > > xe->d3cold.allowed = XE_D3HOT;
> > >
> > > --
> > > 2.34.1
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
