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.
> +}
