[PATCH 1/2] RFC drm/xe: check pcode init status only on root gt of root tile
Ghimiray, Himal Prasad
himal.prasad.ghimiray at intel.com
Mon Mar 11 10:18:41 UTC 2024
On 08-03-2024 14:25, Riana Tauro wrote:
> The root tile indicates the pcode initialization is complete
> when all tiles have completed their initialization.
> So the mailbox can be polled only on the root tile.
> Check pcode init status only on root tile and move it to
> device probe early as root tile is initialized there.
> Also make similar changes in resume paths.
>
> v2: Add lock/unlocked version of pcode_mailbox_rw
> to allow pcode init to be called in device
> early probe (Rodrigo)
HI Riana,
Patch looks good. Can you please add source to mentioned info in the commit?
"The root tile indicates the pcode initialization is complete
when all tiles have completed their initialization. So the mailbox can be polled only on the root tile"
>
> Signed-off-by: Riana Tauro<riana.tauro at intel.com>
> ---
> drivers/gpu/drm/xe/xe_device.c | 21 +++++---
> drivers/gpu/drm/xe/xe_pcode.c | 93 ++++++++++++++++++++--------------
> drivers/gpu/drm/xe/xe_pcode.h | 3 +-
> drivers/gpu/drm/xe/xe_pm.c | 16 +++---
> 4 files changed, 78 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 919ad88f0495..83dd60f68566 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -413,8 +413,14 @@ static int xe_set_dma_info(struct xe_device *xe)
> return err;
> }
>
> -/*
> - * Initialize MMIO resources that don't require any knowledge about tile count.
> +/**
> + * xe_device_probe_early: Device early probe
> + * @xe: xe device instance
> + *
> + * Initialize MMIO resources that don't require any
> + * knowledge about tile count. Also initialize pcode
> + *
> + * Return: 0 on success, error code on failure
> */
> int xe_device_probe_early(struct xe_device *xe)
> {
> @@ -428,6 +434,10 @@ int xe_device_probe_early(struct xe_device *xe)
> if (err)
> return err;
>
> + err = xe_pcode_ready(xe_root_mmio_gt(xe), false);
> + if (err)
> + return err;
> +
> return 0;
> }
>
> @@ -506,11 +516,8 @@ int xe_device_probe(struct xe_device *xe)
> if (err)
> return err;
>
> - for_each_gt(gt, xe, id) {
> - err = xe_pcode_probe(gt);
> - if (err)
> - return err;
> - }
> + for_each_gt(gt, xe, id)
> + xe_pcode_probe(gt);
>
> err = xe_display_init_noirq(xe);
> if (err)
> diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c
> index b324dc2a5deb..6c0009fcd2fe 100644
> --- a/drivers/gpu/drm/xe/xe_pcode.c
> +++ b/drivers/gpu/drm/xe/xe_pcode.c
> @@ -43,8 +43,6 @@ static int pcode_mailbox_status(struct xe_gt *gt)
> [PCODE_ERROR_MASK] = {-EPROTO, "Unknown"},
> };
>
> - lockdep_assert_held(>->pcode.lock);
> -
> err = xe_mmio_read32(gt, PCODE_MAILBOX) & PCODE_ERROR_MASK;
> if (err) {
> drm_err(>_to_xe(gt)->drm, "PCODE Mailbox failed: %d %s", err,
> @@ -55,17 +53,15 @@ static int pcode_mailbox_status(struct xe_gt *gt)
> return 0;
> }
>
> -static int pcode_mailbox_rw(struct xe_gt *gt, u32 mbox, u32 *data0, u32 *data1,
> - unsigned int timeout_ms, bool return_data,
> - bool atomic)
> +static int __pcode_mailbox_rw(struct xe_gt *gt, u32 mbox, u32 *data0, u32 *data1,
> + unsigned int timeout_ms, bool return_data,
> + bool atomic)
> {
> int err;
>
> if (gt_to_xe(gt)->info.skip_pcode)
> return 0;
>
> - lockdep_assert_held(>->pcode.lock);
> -
> if ((xe_mmio_read32(gt, PCODE_MAILBOX) & PCODE_READY) != 0)
> return -EAGAIN;
>
> @@ -87,6 +83,18 @@ static int pcode_mailbox_rw(struct xe_gt *gt, u32 mbox, u32 *data0, u32 *data1,
> return pcode_mailbox_status(gt);
> }
>
> +static int pcode_mailbox_rw(struct xe_gt *gt, u32 mbox, u32 *data0, u32 *data1,
> + unsigned int timeout_ms, bool return_data,
> + bool atomic)
> +{
> + if (gt_to_xe(gt)->info.skip_pcode)
> + return 0;
> +
> + lockdep_assert_held(>->pcode.lock);
> +
> + return __pcode_mailbox_rw(gt, mbox, data0, data1, timeout_ms, return_data, atomic);
> +}
> +
> int xe_pcode_write_timeout(struct xe_gt *gt, u32 mbox, u32 data, int timeout)
> {
> int err;
> @@ -109,15 +117,19 @@ int xe_pcode_read(struct xe_gt *gt, u32 mbox, u32 *val, u32 *val1)
> return err;
> }
>
> -static int xe_pcode_try_request(struct xe_gt *gt, u32 mbox,
> - u32 request, u32 reply_mask, u32 reply,
> - u32 *status, bool atomic, int timeout_us)
> +static int pcode_try_request(struct xe_gt *gt, u32 mbox,
> + u32 request, u32 reply_mask, u32 reply,
> + u32 *status, bool atomic, int timeout_us, bool locked)
> {
> int slept, wait = 10;
>
> for (slept = 0; slept < timeout_us; slept += wait) {
> - *status = pcode_mailbox_rw(gt, mbox, &request, NULL, 1, true,
> - atomic);
> + if (locked)
> + *status = pcode_mailbox_rw(gt, mbox, &request, NULL, 1, true,
> + atomic);
> + else
> + *status = __pcode_mailbox_rw(gt, mbox, &request, NULL, 1, true,
> + atomic);
> if ((*status == 0) && ((request & reply_mask) == reply))
> return 0;
>
> @@ -158,8 +170,8 @@ int xe_pcode_request(struct xe_gt *gt, u32 mbox, u32 request,
>
> mutex_lock(>->pcode.lock);
>
> - ret = xe_pcode_try_request(gt, mbox, request, reply_mask, reply, &status,
> - false, timeout_base_ms * 1000);
> + ret = pcode_try_request(gt, mbox, request, reply_mask, reply, &status,
> + false, timeout_base_ms * 1000, true);
> if (!ret)
> goto out;
>
> @@ -177,8 +189,8 @@ int xe_pcode_request(struct xe_gt *gt, u32 mbox, u32 request,
> "PCODE timeout, retrying with preemption disabled\n");
> drm_WARN_ON_ONCE(>_to_xe(gt)->drm, timeout_base_ms > 1);
> preempt_disable();
> - ret = xe_pcode_try_request(gt, mbox, request, reply_mask, reply, &status,
> - true, timeout_base_ms * 1000);
> + ret = pcode_try_request(gt, mbox, request, reply_mask, reply, &status,
> + true, timeout_base_ms * 1000, true);
> preempt_enable();
>
> out:
> @@ -238,15 +250,16 @@ int xe_pcode_init_min_freq_table(struct xe_gt *gt, u32 min_gt_freq,
> }
>
> /**
> - * xe_pcode_init - Ensure PCODE is initialized
> + * xe_pcode_ready - Ensure PCODE is initialized
> * @gt: gt instance
> + * @locked: true if lock held, false otherwise
> *
> - * This function ensures that PCODE is properly initialized. To be called during
> - * probe and resume paths.
> + * This function ensures that PCODE is properly initialized. Can be called
> + * without locks only in early probe.
> *
> * It returns 0 on success, and -error number on failure.
> */
> -int xe_pcode_init(struct xe_gt *gt)
> +int xe_pcode_ready(struct xe_gt *gt, bool locked)
> {
> u32 status, request = DGFX_GET_INIT_STATUS;
> int timeout_us = 180000000; /* 3 min */
> @@ -258,12 +271,10 @@ int xe_pcode_init(struct xe_gt *gt)
> if (!IS_DGFX(gt_to_xe(gt)))
> return 0;
>
> - mutex_lock(>->pcode.lock);
> - ret = xe_pcode_try_request(gt, DGFX_PCODE_STATUS, request,
> - DGFX_INIT_STATUS_COMPLETE,
> - DGFX_INIT_STATUS_COMPLETE,
> - &status, false, timeout_us);
> - mutex_unlock(>->pcode.lock);
> + ret = pcode_try_request(gt, DGFX_PCODE_STATUS, request,
> + DGFX_INIT_STATUS_COMPLETE,
> + DGFX_INIT_STATUS_COMPLETE,
> + &status, false, timeout_us, locked);
>
> if (ret)
> drm_err(>_to_xe(gt)->drm,
> @@ -273,24 +284,32 @@ int xe_pcode_init(struct xe_gt *gt)
> }
>
> /**
> - * xe_pcode_probe - Prepare xe_pcode and also ensure PCODE is initialized.
> + * xe_pcode_init - initialize pcode
> * @gt: gt instance
> *
> - * This function initializes the xe_pcode component, and when needed, it ensures
> - * that PCODE has properly performed its initialization and it is really ready
> - * to go. To be called once only during probe.
> + * This function initializes pcode. Used in resume paths
> *
> * It returns 0 on success, and -error number on failure.
> */
> -int xe_pcode_probe(struct xe_gt *gt)
> +int xe_pcode_init(struct xe_gt *gt)
> {
> - drmm_mutex_init(>_to_xe(gt)->drm, >->pcode.lock);
> + int ret;
>
> - if (gt_to_xe(gt)->info.skip_pcode)
> - return 0;
> + mutex_lock(>->pcode.lock);
> + ret = xe_pcode_ready(gt, true);
> + mutex_unlock(>->pcode.lock);
>
> - if (!IS_DGFX(gt_to_xe(gt)))
> - return 0;
> + return ret;
> +}
>
> - return xe_pcode_init(gt);
> +/**
> + * xe_pcode_probe - Prepare pcode component
> + * @gt: gt instance
> + *
> + * This function initializes the xe_pcode component.
> + * To be called once only during probe.
> + */
> +void xe_pcode_probe(struct xe_gt *gt)
> +{
> + drmm_mutex_init(>_to_xe(gt)->drm, >->pcode.lock);
> }
> diff --git a/drivers/gpu/drm/xe/xe_pcode.h b/drivers/gpu/drm/xe/xe_pcode.h
> index 08cb1d047cba..e597dcc8ad9f 100644
> --- a/drivers/gpu/drm/xe/xe_pcode.h
> +++ b/drivers/gpu/drm/xe/xe_pcode.h
> @@ -9,8 +9,9 @@
> #include <linux/types.h>
> struct xe_gt;
>
> -int xe_pcode_probe(struct xe_gt *gt);
> +void xe_pcode_probe(struct xe_gt *gt);
> int xe_pcode_init(struct xe_gt *gt);
> +int xe_pcode_ready(struct xe_gt *gt, bool locked);
> int xe_pcode_init_min_freq_table(struct xe_gt *gt, u32 min_gt_freq,
> u32 max_gt_freq);
> int xe_pcode_read(struct xe_gt *gt, u32 mbox, u32 *val, u32 *val1);
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 9fbb6f6c598a..83c316254e43 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -121,11 +121,9 @@ int xe_pm_resume(struct xe_device *xe)
> for_each_tile(tile, xe, id)
> xe_wa_apply_tile_workarounds(tile);
>
> - for_each_gt(gt, xe, id) {
> - err = xe_pcode_init(gt);
> - if (err)
> - return err;
> - }
> + err = xe_pcode_init(xe_root_mmio_gt(xe));
> + if (err)
> + return err;
>
> xe_display_pm_resume_early(xe);
>
> @@ -374,11 +372,9 @@ int xe_pm_runtime_resume(struct xe_device *xe)
> xe->d3cold.power_lost = xe_guc_in_reset(>->uc.guc);
>
> if (xe->d3cold.allowed && xe->d3cold.power_lost) {
> - for_each_gt(gt, xe, id) {
> - err = xe_pcode_init(gt);
> - if (err)
> - goto out;
> - }
> + err = xe_pcode_init(xe_root_mmio_gt(xe));
> + if (err)
> + goto out;
>
> /*
> * This only restores pinned memory which is the memory
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-xe/attachments/20240311/357b0d4a/attachment-0001.htm>
More information about the Intel-xe
mailing list