[Intel-xe] [PATCH v4 22/22] drm/xe: Initialize GuC earlier during probe
Michał Winiarski
michal.winiarski at intel.com
Wed Nov 29 22:20:27 UTC 2023
On Wed, Nov 29, 2023 at 01:48:50PM -0800, Welty, Brian wrote:
>
> On 11/28/2023 5:16 PM, Michał Winiarski wrote:
> > SR-IOV VF has limited access to MMIO registers. Fortunately, it is able
> > to access a curated subset that is needed to initialize the driver by
> > communicating with SR-IOV PF using GuC CT.
> > Initialize GuC earlier in order to keep the unified probe ordering
> > between VF and PF modes.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> > ---
> > v3 -> v4:
> > - Introduce xe_gt_init_hwconfig (Matt Brost)
> > - Init PAT before loading GuC
> >
> > drivers/gpu/drm/xe/xe_device.c | 6 ++++
> > drivers/gpu/drm/xe/xe_gt.c | 51 ++++++++++++++++++++++++----------
> > drivers/gpu/drm/xe/xe_gt.h | 1 +
> > drivers/gpu/drm/xe/xe_uc.c | 11 ++++++--
> > 4 files changed, 52 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index 65e9aa5e6c31e..a236c36cdae3c 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -425,6 +425,12 @@ int xe_device_probe(struct xe_device *xe)
> > return err;
> > }
> > + for_each_gt(gt, xe, id) {
> > + err = xe_gt_init_hwconfig(gt);
> > + if (err)
> > + return err;
> > + }
> > +
> > err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
> > if (err)
> > return err;
> > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > index 964e76f931a28..d2766c2c58f0f 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.c
> > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > @@ -297,7 +297,6 @@ int xe_gt_init_early(struct xe_gt *gt)
> > return err;
> > xe_gt_topology_init(gt);
> > - xe_gt_mcr_init(gt);
> > err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> > if (err)
> > @@ -336,22 +335,12 @@ static int gt_fw_domain_init(struct xe_gt *gt)
> > if (err)
> > goto err_hw_fence_irq;
> > - xe_pat_init(gt);
> > -
> > if (!xe_gt_is_media_type(gt)) {
> > err = xe_ggtt_init(gt_to_tile(gt)->mem.ggtt);
> > if (err)
> > goto err_force_wake;
> > }
> > - err = xe_uc_init(>->uc);
> > - if (err)
> > - goto err_force_wake;
> > -
> > - err = xe_uc_init_hwconfig(>->uc);
> > - if (err)
> > - goto err_force_wake;
> > -
> > xe_gt_idle_sysfs_init(>->gtidle);
> > /* XXX: Fake that we pull the engine mask from hwconfig blob */
> > gt->info.engine_mask = gt->info.__engine_mask;
>
>
> I've been wondering about the XXX above. I added the line above that comes
> next.
> Should that move into xe_gt_init_hwconfig() as well?
> And replace it with code to read it from hwconfig blob?
>
> I'm asking because I was looking at a change that would like to make use of
> a valid engine_mask earlier than xe_gt_init().
Yeah - makes sense, I'll move it to xe_gt_init_hwconfig().
Is that point in the codebase early enough for the change that you're
referring to?
-Michał
>
> -Brian
>
>
>
> > @@ -415,10 +404,6 @@ static int all_fw_domain_init(struct xe_gt *gt)
> > if (err)
> > goto err_force_wake;
> > - err = xe_uc_init_post_hwconfig(>->uc);
> > - if (err)
> > - goto err_force_wake;
> > -
> > if (!xe_gt_is_media_type(gt)) {
> > /*
> > * USM has its only SA pool to non-block behind user operations
> > @@ -442,6 +427,10 @@ static int all_fw_domain_init(struct xe_gt *gt)
> > }
> > }
> > + err = xe_uc_init_post_hwconfig(>->uc);
> > + if (err)
> > + goto err_force_wake;
> > +
> > err = xe_uc_init_hw(>->uc);
> > if (err)
> > goto err_force_wake;
> > @@ -462,6 +451,38 @@ static int all_fw_domain_init(struct xe_gt *gt)
> > return err;
> > }
> > +/*
> > + * Initialize enough GT to be able to load GuC in order to obtain hwconfig and enable CTB
> > + * communication.
> > + */
> > +int xe_gt_init_hwconfig(struct xe_gt *gt)
> > +{
> > + int err;
> > +
> > + xe_device_mem_access_get(gt_to_xe(gt));
> > + err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
> > + if (err)
> > + goto out;
> > +
> > + xe_gt_mcr_init(gt);
> > + xe_pat_init(gt);
> > +
> > + err = xe_uc_init(>->uc);
> > + if (err)
> > + goto out_fw;
> > +
> > + err = xe_uc_init_hwconfig(>->uc);
> > + if (err)
> > + goto out_fw;
> > +
> > +out_fw:
> > + xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> > +out:
> > + xe_device_mem_access_put(gt_to_xe(gt));
> > +
> > + return err;
> > +}
> > +
> > int xe_gt_init(struct xe_gt *gt)
> > {
> > int err;
> > diff --git a/drivers/gpu/drm/xe/xe_gt.h b/drivers/gpu/drm/xe/xe_gt.h
> > index caded203a8a03..e9d6aeceb56af 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.h
> > +++ b/drivers/gpu/drm/xe/xe_gt.h
> > @@ -31,6 +31,7 @@ static inline bool xe_fault_inject_gt_reset(void)
> > #endif
> > struct xe_gt *xe_gt_alloc(struct xe_tile *tile);
> > +int xe_gt_init_hwconfig(struct xe_gt *gt);
> > int xe_gt_init_early(struct xe_gt *gt);
> > int xe_gt_init(struct xe_gt *gt);
> > int xe_gt_record_default_lrcs(struct xe_gt *gt);
> > diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
> > index 13e76e6805ca1..123baafc9ec38 100644
> > --- a/drivers/gpu/drm/xe/xe_uc.c
> > +++ b/drivers/gpu/drm/xe/xe_uc.c
> > @@ -29,13 +29,15 @@ uc_to_xe(struct xe_uc *uc)
> > /* Should be called once at driver load only */
> > int xe_uc_init(struct xe_uc *uc)
> > {
> > + struct xe_device *xe = uc_to_xe(uc);
> > int ret;
> > + xe_device_mem_access_get(xe);
> > +
> > /*
> > * We call the GuC/HuC init functions even if GuC submission is off to
> > * correctly move our tracking of the FW state to "disabled".
> > */
> > -
> > ret = xe_guc_init(&uc->guc);
> > if (ret)
> > goto err;
> > @@ -45,7 +47,7 @@ int xe_uc_init(struct xe_uc *uc)
> > goto err;
> > if (!xe_device_uc_enabled(uc_to_xe(uc)))
> > - return 0;
> > + goto out;
> > ret = xe_wopcm_init(&uc->wopcm);
> > if (ret)
> > @@ -55,9 +57,14 @@ int xe_uc_init(struct xe_uc *uc)
> > if (ret)
> > goto err;
> > +out:
> > + xe_device_mem_access_put(xe);
> > +
> > return 0;
> > err:
> > + xe_device_mem_access_put(xe);
> > +
> > return ret;
> > }
More information about the Intel-xe
mailing list