> -----Original Message-----
> From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Sent: Wednesday, April 2, 2025 1:27 AM
> 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>; Gupta, Varun <varun.gupta@xxxxxxxxx>;
> ville.syrjala@xxxxxxxxxxxxxxx; Shankar, Uma <uma.shankar@xxxxxxxxx>
> Subject: Re: [PATCH 06/12] drm/xe/vrsr: Initialize VRSR feature
>
> On Tue, Apr 01, 2025 at 09:02:19PM +0530, Anshuman Gupta wrote:
> > From: Badal Nilawar <badal.nilawar@xxxxxxxxx>
> >
> > Initialize VRSR feature by requesting Auxilary Power and PERST# assertion
> > delay. Include an API to enable VRSR feature.
>
> s/Auxilary/Auxiliary/
>
> I would include the name of the API directly.
>
> > +#define PCODE_D3_VRSR_PERST_SHIFT 16
>
> PCODE_D3_VRSR_PERST_SHIFT is not used by this series; maybe omit it?
>
> > +#define POWER_D3_VRSR_PSERST_MASK REG_GENMASK(31,
> 16)
>
> s/PSERST/PERST/ (looks like a typo?)
>
> > +static void xe_pm_vrsr_init(struct xe_device *xe)
> > +{
> > + int ret;
> > +
> > + /* Check if platform support d3cold vrsr */
>
> Nicer to consistently capitalize as "VRSR" in comments, commit
> logs, and messages.
>
> Similar with "D3cold" ("D3cold" is used ~100 times in the tree,
> "D3Cold" ~20, mostly in xe).
>
> > + if (!xe->info.has_vrsr)
> > + return;
> > +
> > + if (!xe_pm_vrsr_capable(xe))
> > + return;
> > +
> > + /*
> > + * If the VRSR initialization fails, the device will proceed with the regular
> > + * D3 Cold flow
> > + */
> > + ret = pci_acpi_aux_power_setup(xe);
> > + if (ret) {
> > + drm_info(&xe->drm, "VRSR capable %s\n", "No");
>
> Kinda weird to use %s when the text is a known constant.
>
> > + return;
> > + }
> > +
> > + xe->d3cold.vrsr_capable = true;
> > + drm_info(&xe->drm, "VRSR capable %s\n", "Yes");
>
> Same.
Thanks for review comment, will address these in v2.
Thanks,
Anshuman.
>
> > +}
