Re: [PATCH v2 13/30] drm/xe/gsc: Use scope-based cleanup

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Matt Roper (2025-11-10 20:20:31-03:00)
>Use scope-based cleanup for forcewake and runtime PM to eliminate some
>goto-based error handling and simplify other functions.
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>---
> drivers/gpu/drm/xe/xe_gsc.c       | 21 ++++++---------------
> drivers/gpu/drm/xe/xe_gsc_proxy.c | 17 +++++++----------
> 2 files changed, 13 insertions(+), 25 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_gsc.c b/drivers/gpu/drm/xe/xe_gsc.c
>index dd69cb834f8e..a3157b0fe791 100644
>--- a/drivers/gpu/drm/xe/xe_gsc.c
>+++ b/drivers/gpu/drm/xe/xe_gsc.c
>@@ -352,7 +352,6 @@ static void gsc_work(struct work_struct *work)
>         struct xe_gsc *gsc = container_of(work, typeof(*gsc), work);
>         struct xe_gt *gt = gsc_to_gt(gsc);
>         struct xe_device *xe = gt_to_xe(gt);
>-        unsigned int fw_ref;
>         u32 actions;
>         int ret;
> 
>@@ -361,13 +360,12 @@ static void gsc_work(struct work_struct *work)
>         gsc->work_actions = 0;
>         spin_unlock_irq(&gsc->lock);
> 
>-        xe_pm_runtime_get(xe);
>-        fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC);
>+        guard(xe_pm_runtime)(xe);
>+        CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GSC);
> 
>         if (actions & GSC_ACTION_ER_COMPLETE) {
>-                ret = gsc_er_complete(gt);
>-                if (ret)
>-                        goto out;
>+                if (gsc_er_complete(gt))
>+                        return;
>         }
> 
>         if (actions & GSC_ACTION_FW_LOAD) {
>@@ -380,10 +378,6 @@ static void gsc_work(struct work_struct *work)
> 
>         if (actions & GSC_ACTION_SW_PROXY)
>                 xe_gsc_proxy_request_handler(gsc);
>-
>-out:
>-        xe_force_wake_put(gt_to_fw(gt), fw_ref);
>-        xe_pm_runtime_put(xe);
> }
> 
> void xe_gsc_hwe_irq_handler(struct xe_hw_engine *hwe, u16 intr_vec)
>@@ -615,7 +609,6 @@ void xe_gsc_print_info(struct xe_gsc *gsc, struct drm_printer *p)
> {
>         struct xe_gt *gt = gsc_to_gt(gsc);
>         struct xe_mmio *mmio = &gt->mmio;
>-        unsigned int fw_ref;
> 
>         xe_uc_fw_print(&gsc->fw, p);
> 
>@@ -624,8 +617,8 @@ void xe_gsc_print_info(struct xe_gsc *gsc, struct drm_printer *p)
>         if (!xe_uc_fw_is_enabled(&gsc->fw))
>                 return;
> 
>-        fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC);
>-        if (!fw_ref)
>+        CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GSC);
>+        if (!fw_ref.domains)
>                 return;
> 
>         drm_printf(p, "\nHECI1 FWSTS: 0x%08x 0x%08x 0x%08x 0x%08x 0x%08x 0x%08x\n",
>@@ -635,6 +628,4 @@ void xe_gsc_print_info(struct xe_gsc *gsc, struct drm_printer *p)
>                         xe_mmio_read32(mmio, HECI_FWSTS4(MTL_GSC_HECI1_BASE)),
>                         xe_mmio_read32(mmio, HECI_FWSTS5(MTL_GSC_HECI1_BASE)),
>                         xe_mmio_read32(mmio, HECI_FWSTS6(MTL_GSC_HECI1_BASE)));
>-
>-        xe_force_wake_put(gt_to_fw(gt), fw_ref);
> }
>diff --git a/drivers/gpu/drm/xe/xe_gsc_proxy.c b/drivers/gpu/drm/xe/xe_gsc_proxy.c
>index 464282a89eef..e7573a0c5e5d 100644
>--- a/drivers/gpu/drm/xe/xe_gsc_proxy.c
>+++ b/drivers/gpu/drm/xe/xe_gsc_proxy.c
>@@ -440,22 +440,19 @@ static void xe_gsc_proxy_remove(void *arg)
>         struct xe_gsc *gsc = arg;
>         struct xe_gt *gt = gsc_to_gt(gsc);
>         struct xe_device *xe = gt_to_xe(gt);
>-        unsigned int fw_ref = 0;
> 
>         if (!gsc->proxy.component_added)
>                 return;
> 
>         /* disable HECI2 IRQs */
>-        xe_pm_runtime_get(xe);
>-        fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC);
>-        if (!fw_ref)
>-                xe_gt_err(gt, "failed to get forcewake to disable GSC interrupts\n");
>+        scoped_guard(xe_pm_runtime, xe) {
>+                CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GSC);
>+                if (!fw_ref.domains)
>+                        xe_gt_err(gt, "failed to get forcewake to disable GSC interrupts\n");
> 
>-        /* try do disable irq even if forcewake failed */
>-        gsc_proxy_irq_toggle(gsc, false);
>-
>-        xe_force_wake_put(gt_to_fw(gt), fw_ref);
>-        xe_pm_runtime_put(xe);
>+                /* try do disable irq even if forcewake failed */
>+                gsc_proxy_irq_toggle(gsc, false);
>+        }
> 
>         xe_gsc_wait_for_worker_completion(gsc);
> 
>-- 
>2.51.1
>