Skip to content

Commit

Permalink
Revert "drm/xe: make gt_remove use devm"
Browse files Browse the repository at this point in the history
This reverts commit cd506a3.

The gt_remove function was explicitly added as part of the remove flow
instead of using drmm/devm automatic cleanup due to it being illegal
to remove a component after the driver has been detached from the pci
device; the GSC proxy component is removed as part of gt_remove, so we
need to do it in the pci cleanup flow. The function already has a
comment above it to explain this.

Note that the change to use the devm also caused an invalid pointer
deref in the gsc_proxy unbind function, but I didn't bother to debug
which pointer was bad since we shouldn't be calling the unbind that
late anyway and this revert fixes it.

Both issue were not seen in CI because the GSC loading is temporarily
disabled due to a critical bug, which means we're not binding the
component.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240528182354.1200424-1-daniele.ceraolospurio@intel.com
  • Loading branch information
Daniele Ceraolo Spurio committed May 30, 2024
1 parent 9bbfab1 commit 1c43247
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 9 deletions.
22 changes: 20 additions & 2 deletions drivers/gpu/drm/xe/xe_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ int xe_device_probe(struct xe_device *xe)
struct xe_tile *tile;
struct xe_gt *gt;
int err;
u8 last_gt;
u8 id;

xe_pat_init_early(xe);
Expand Down Expand Up @@ -642,16 +643,18 @@ int xe_device_probe(struct xe_device *xe)
goto err_irq_shutdown;

for_each_gt(gt, xe, id) {
last_gt = id;

err = xe_gt_init(gt);
if (err)
goto err_irq_shutdown;
goto err_fini_gt;
}

xe_heci_gsc_init(xe);

err = xe_display_init(xe);
if (err)
goto err_irq_shutdown;
goto err_fini_gt;

err = drm_dev_register(&xe->drm, 0);
if (err)
Expand All @@ -667,6 +670,15 @@ int xe_device_probe(struct xe_device *xe)

err_fini_display:
xe_display_driver_remove(xe);

err_fini_gt:
for_each_gt(gt, xe, id) {
if (id < last_gt)
xe_gt_remove(gt);
else
break;
}

err_irq_shutdown:
xe_irq_shutdown(xe);
err:
Expand All @@ -684,12 +696,18 @@ static void xe_device_remove_display(struct xe_device *xe)

void xe_device_remove(struct xe_device *xe)
{
struct xe_gt *gt;
u8 id;

xe_device_remove_display(xe);

xe_display_fini(xe);

xe_heci_gsc_fini(xe);

for_each_gt(gt, xe, id)
xe_gt_remove(gt);

xe_irq_shutdown(xe);
}

Expand Down
16 changes: 9 additions & 7 deletions drivers/gpu/drm/xe/xe_gt.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,16 @@ void xe_gt_sanitize(struct xe_gt *gt)
gt->uc.guc.submission_state.enabled = false;
}

/*
* Clean up the GT structures before driver removal. This function should only
* act on objects/structures that must be cleaned before the driver removal
* callback is complete and therefore can't be deferred to a drmm action.
/**
* xe_gt_remove() - Clean up the GT structures before driver removal
* @gt: the GT object
*
* This function should only act on objects/structures that must be cleaned
* before the driver removal callback is complete and therefore can't be
* deferred to a drmm action.
*/
static void gt_remove(void *arg)
void xe_gt_remove(struct xe_gt *gt)
{
struct xe_gt *gt = arg;
int i;

xe_uc_remove(&gt->uc);
Expand Down Expand Up @@ -566,7 +568,7 @@ int xe_gt_init(struct xe_gt *gt)

xe_gt_record_user_engines(gt);

return devm_add_action_or_reset(gt_to_xe(gt)->drm.dev, gt_remove, gt);
return 0;
}

void xe_gt_record_user_engines(struct xe_gt *gt)
Expand Down
1 change: 1 addition & 0 deletions drivers/gpu/drm/xe/xe_gt.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ int xe_gt_suspend(struct xe_gt *gt);
int xe_gt_resume(struct xe_gt *gt);
void xe_gt_reset_async(struct xe_gt *gt);
void xe_gt_sanitize(struct xe_gt *gt);
void xe_gt_remove(struct xe_gt *gt);

/**
* xe_gt_any_hw_engine_by_reset_domain - scan the list of engines and return the
Expand Down

0 comments on commit 1c43247

Please sign in to comment.