[PATCH v4 7/7] drm/xe/pm: Capture errors and handle them

[Lucas De Marchi]

On Fri, Apr 12, 2024 at 11:42:11PM GMT, Himal Prasad Ghimiray wrote:
>xe_pm_init may encounter failures for various reasons, such as a failure
>in initializing drmm_mutex, or when dealing with a d3cold-capable device
>for vram_threshold sysfs creation and setting default threshold.
>Presently, all these potential failures are disregarded.
>
>Move d3cold.lock initialization to xe_pm_init_early and cause driver
>abort if mutex initialization has failed.
>
>For xe_pm_init failures cleanup the driver and return error code
>
>-v2
>Make mutex init cleaner (Lucas)
>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi

>---
> drivers/gpu/drm/xe/xe_device_sysfs.c | 12 ++++------
> drivers/gpu/drm/xe/xe_device_sysfs.h |  2 +-
> drivers/gpu/drm/xe/xe_pci.c          | 12 ++++++++--
> drivers/gpu/drm/xe/xe_pm.c           | 36 +++++++++++++++++++++-------
> drivers/gpu/drm/xe/xe_pm.h           |  4 ++--
> 5 files changed, 45 insertions(+), 21 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
>index e47c8ad1bb17..21677b8cd977 100644
>--- a/drivers/gpu/drm/xe/xe_device_sysfs.c
>+++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
>@@ -76,18 +76,14 @@ static void xe_device_sysfs_fini(struct drm_device *drm, void *arg)
> 	sysfs_remove_file(&xe->drm.dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
> }
>
>-void xe_device_sysfs_init(struct xe_device *xe)
>+int xe_device_sysfs_init(struct xe_device *xe)
> {
> 	struct device *dev = xe->drm.dev;
> 	int ret;
>
> 	ret = sysfs_create_file(&dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
>-	if (ret) {
>-		drm_warn(&xe->drm, "Failed to create sysfs file\n");
>-		return;
>-	}
>-
>-	ret = drmm_add_action_or_reset(&xe->drm, xe_device_sysfs_fini, xe);
> 	if (ret)
>-		drm_warn(&xe->drm, "Failed to add sysfs fini drm action\n");
>+		return ret;
>+
>+	return drmm_add_action_or_reset(&xe->drm, xe_device_sysfs_fini, xe);
> }
>diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.h b/drivers/gpu/drm/xe/xe_device_sysfs.h
>index 38b240684bee..f9e83d8bd2c7 100644
>--- a/drivers/gpu/drm/xe/xe_device_sysfs.h
>+++ b/drivers/gpu/drm/xe/xe_device_sysfs.h
>@@ -8,6 +8,6 @@
>
> struct xe_device;
>
>-void xe_device_sysfs_init(struct xe_device *xe);
>+int xe_device_sysfs_init(struct xe_device *xe);
>
> #endif
>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>index fa2cc80a08a3..65ea44672cab 100644
>--- a/drivers/gpu/drm/xe/xe_pci.c
>+++ b/drivers/gpu/drm/xe/xe_pci.c
>@@ -781,18 +781,26 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> 		str_yes_no(xe_device_has_sriov(xe)),
> 		xe_sriov_mode_to_string(xe_device_sriov_mode(xe)));
>
>-	xe_pm_init_early(xe);
>+	err = xe_pm_init_early(xe);
>+	if (err)
>+		return err;
>
> 	err = xe_device_probe(xe);
> 	if (err)
> 		return err;
>
>-	xe_pm_init(xe);
>+	err = xe_pm_init(xe);
>+	if (err)
>+		goto err_driver_cleanup;
>
> 	drm_dbg(&xe->drm, "d3cold: capable=%s\n",
> 		str_yes_no(xe->d3cold.capable));
>
> 	return 0;
>+
>+err_driver_cleanup:
>+	xe_pci_remove(pdev);
>+	return err;
> }
>
> static void xe_pci_shutdown(struct pci_dev *pdev)
>diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
>index f1fc83845c01..a9decdf266a8 100644
>--- a/drivers/gpu/drm/xe/xe_pm.c
>+++ b/drivers/gpu/drm/xe/xe_pm.c
>@@ -208,10 +208,21 @@ static void xe_pm_runtime_init(struct xe_device *xe)
> 	pm_runtime_put(dev);
> }
>
>-void xe_pm_init_early(struct xe_device *xe)
>+int xe_pm_init_early(struct xe_device *xe)
> {
>+	int err;
>+
> 	INIT_LIST_HEAD(&xe->mem_access.vram_userfault.list);
>-	drmm_mutex_init(&xe->drm, &xe->mem_access.vram_userfault.lock);
>+
>+	err = drmm_mutex_init(&xe->drm, &xe->mem_access.vram_userfault.lock);
>+	if (err)
>+		return err;
>+
>+	err = drmm_mutex_init(&xe->drm, &xe->d3cold.lock);
>+	if (err)
>+		return err;
>+
>+	return 0;
> }
>
> /**
>@@ -219,23 +230,32 @@ void xe_pm_init_early(struct xe_device *xe)
>  * @xe: xe device instance
>  *
>  * This component is responsible for System and Device sleep states.
>+ *
>+ * Returns 0 for success, negative error code otherwise.
>  */
>-void xe_pm_init(struct xe_device *xe)
>+int xe_pm_init(struct xe_device *xe)
> {
>+	int err;
>+
> 	/* For now suspend/resume is only allowed with GuC */
> 	if (!xe_device_uc_enabled(xe))
>-		return;
>-
>-	drmm_mutex_init(&xe->drm, &xe->d3cold.lock);
>+		return 0;
>
> 	xe->d3cold.capable = xe_pm_pci_d3cold_capable(xe);
>
> 	if (xe->d3cold.capable) {
>-		xe_device_sysfs_init(xe);
>-		xe_pm_set_vram_threshold(xe, DEFAULT_VRAM_THRESHOLD);
>+		err = xe_device_sysfs_init(xe);
>+		if (err)
>+			return err;
>+
>+		err = xe_pm_set_vram_threshold(xe, DEFAULT_VRAM_THRESHOLD);
>+		if (err)
>+			return err;
> 	}
>
> 	xe_pm_runtime_init(xe);
>+
>+	return 0;
> }
>
> /**
>diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
>index 0cb38ca244fe..2cefe8624474 100644
>--- a/drivers/gpu/drm/xe/xe_pm.h
>+++ b/drivers/gpu/drm/xe/xe_pm.h
>@@ -20,8 +20,8 @@ struct xe_device;
> int xe_pm_suspend(struct xe_device *xe);
> int xe_pm_resume(struct xe_device *xe);
>
>-void xe_pm_init_early(struct xe_device *xe);
>-void xe_pm_init(struct xe_device *xe);
>+int xe_pm_init_early(struct xe_device *xe);
>+int xe_pm_init(struct xe_device *xe);
> void xe_pm_runtime_fini(struct xe_device *xe);
> bool xe_pm_runtime_suspended(struct xe_device *xe);
> int xe_pm_runtime_suspend(struct xe_device *xe);
>-- 
>2.25.1
>