[Intel-xe] [PATCH 11/12] drm/xe/device: Introduce xe_device_probe_early
Michał Winiarski
michal.winiarski at intel.com
Tue Nov 14 10:02:15 UTC 2023
On Mon, Nov 13, 2023 at 09:11:18AM -0600, Lucas De Marchi wrote:
> On Wed, Nov 08, 2023 at 01:33:43AM +0100, Michał Winiarski wrote:
> > SR-IOV VF doesn't have access to MMIO registers used to determine
> > graphics/media ID. It can however communicate with GuC.
> > Introduce xe_device_probe_early, which will eventually initialize enough
> > hardware to allow the driver to communicate with GuC early during probe.
> > This will allow both VF and PF/native driver to have unified probe
> > ordering.
> > Initialize root tile MMIO and TTM sys mgr early, as both are needed to
> > communicate with GuC firmware earlier during probe.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> > ---
> > drivers/gpu/drm/xe/tests/xe_pci.c | 1 +
> > drivers/gpu/drm/xe/xe_device.c | 21 +++++++++++++++------
> > drivers/gpu/drm/xe/xe_device.h | 5 +++++
> > drivers/gpu/drm/xe/xe_mmio.c | 17 +++++++++--------
> > drivers/gpu/drm/xe/xe_mmio.h | 1 +
> > drivers/gpu/drm/xe/xe_pci.c | 26 +++++++++++++++++++++-----
> > 6 files changed, 52 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
> > index a40879da2fbe1..733df9a270db2 100644
> > --- a/drivers/gpu/drm/xe/tests/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/tests/xe_pci.c
> > @@ -143,6 +143,7 @@ int xe_pci_fake_device_init(struct xe_device *xe, enum xe_platform platform,
> > return -ENODEV;
> >
> > done:
> > + xe_info_init_early(xe, desc, subplatform_desc);
> > xe_info_init(xe, desc, subplatform_desc);
> >
> > return 0;
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index 696f997bd2c93..19faddb779808 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -364,6 +364,21 @@ static int xe_set_dma_info(struct xe_device *xe)
> > return err;
> > }
> >
>
> The _early variants have different meaning all throughout this patch
> series. Keep that consistent in future will be challenge if we don't
> document what the split is between the _early (which used to be sw-only
> part) and the normal init/probe.
>
> Please add a comment on top of each _early() variant you add with the
> split of what should be there and what should rather be in the "normal"
> init.
Note that even in the "future" we might want to avoid having a rigid
split between sw-only and HW communication.
We do use MMIO regs for Graphics/Media IP version detection. Same thing
will apply to SR-IOV VF mode detection. Those are pretty basic things
that need to be done very early during probe.
If we go sw-only route for "early" probe - we will end up initializing
data structures that won't even be used by the driver, which is wasteful
and makes the probe harder to understand. We would probably also need
extra code (asserts) to warn on uninitialized resource usage.
Or, we could just map everything as early as possible. Early would then
just mean "early". Which is also what it means today, as the existing
code prior to this series already has "early" functions that are not
sw-only.
Thanks,
-Michał
>
> Lucas De Marchi
>
> > +int xe_device_probe_early(struct xe_device *xe)
> > +{
> > + int err;
> > +
> > + err = xe_mmio_init(xe);
> > + if (err)
> > + return err;
> > +
> > + xe_mmio_root_tile_init(xe);
> > +
> > + xe_ttm_sys_mgr_init(xe);
> > +
> > + return 0;
> > +}
> > +
> > int xe_device_probe(struct xe_device *xe)
> > {
> > struct xe_tile *tile;
> > @@ -382,10 +397,6 @@ int xe_device_probe(struct xe_device *xe)
> > if (err)
> > return err;
> >
> > - err = xe_mmio_init(xe);
> > - if (err)
> > - return err;
> > -
> > xe_mmio_probe_tiles(xe);
> >
> > err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
> > @@ -418,8 +429,6 @@ int xe_device_probe(struct xe_device *xe)
> > if (err)
> > goto err_irq_shutdown;
> >
> > - xe_ttm_sys_mgr_init(xe);
> > -
> > for_each_tile(tile, xe, id) {
> > err = xe_tile_init_noalloc(tile);
> > if (err)
> > diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> > index ee89af6b1ea9c..5d7fd16f545c6 100644
> > --- a/drivers/gpu/drm/xe/xe_device.h
> > +++ b/drivers/gpu/drm/xe/xe_device.h
> > @@ -37,6 +37,7 @@ static inline struct xe_device *ttm_to_xe_device(struct ttm_device *ttm)
> >
> > struct xe_device *xe_device_create(struct pci_dev *pdev,
> > const struct pci_device_id *ent);
> > +int xe_device_probe_early(struct xe_device *xe);
> > int xe_device_probe(struct xe_device *xe);
> > void xe_device_remove(struct xe_device *xe);
> >
> > @@ -122,6 +123,10 @@ static inline bool xe_device_uc_enabled(struct xe_device *xe)
> > for ((id__) = 0; (id__) < (xe__)->info.tile_count; (id__)++) \
> > for_each_if((tile__) = &(xe__)->tiles[(id__)])
> >
> > +#define for_each_remote_tile(tile__, xe__, id__) \
> > + for ((id__) = 1; (id__) < (xe__)->info.tile_count; (id__)++) \
> > + for_each_if((tile__) = &(xe__)->tiles[(id__)])
> > +
> > /*
> > * FIXME: This only works for now since multi-tile and standalone media
> > * happen to be mutually exclusive. Future platforms may change this...
> > diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> > index 8945c601e1c54..526a6ac6be56a 100644
> > --- a/drivers/gpu/drm/xe/xe_mmio.c
> > +++ b/drivers/gpu/drm/xe/xe_mmio.c
> > @@ -15,10 +15,12 @@
> > #include "regs/xe_regs.h"
> > #include "xe_bo.h"
> > #include "xe_device.h"
> > +#include "xe_ggtt.h"
> > #include "xe_gt.h"
> > #include "xe_gt_mcr.h"
> > #include "xe_macros.h"
> > #include "xe_module.h"
> > +#include "xe_tile.h"
> >
> > #define XEHP_MTCFG_ADDR XE_REG(0x101800)
> > #define TILE_COUNT REG_GENMASK(15, 8)
> > @@ -369,10 +371,8 @@ static void mmio_fini(struct drm_device *drm, void *arg)
> >
> > int xe_mmio_init(struct xe_device *xe)
> > {
> > - struct xe_tile *root_tile = xe_device_get_root_tile(xe);
> > struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> > const int mmio_bar = 0;
> > - int err;
> >
> > /*
> > * Map the entire BAR.
> > @@ -386,15 +386,16 @@ int xe_mmio_init(struct xe_device *xe)
> > return -EIO;
> > }
> >
> > - err = drmm_add_action_or_reset(&xe->drm, mmio_fini, xe);
> > - if (err)
> > - return err;
> > + return drmm_add_action_or_reset(&xe->drm, mmio_fini, xe);
> > +}
> > +
> > +void xe_mmio_root_tile_init(struct xe_device *xe)
> > +{
> > + struct xe_tile *root_tile = xe_device_get_root_tile(xe);
> >
> > /* Setup first tile; other tiles (if present) will be setup later. */
> > - root_tile->mmio.size = xe->mmio.size;
> > + root_tile->mmio.size = SZ_16M;
> > root_tile->mmio.regs = xe->mmio.regs;
> > -
> > - return 0;
> > }
> >
> > /**
> > diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
> > index 9f1a25cfd2cf0..894a792ac8abe 100644
> > --- a/drivers/gpu/drm/xe/xe_mmio.h
> > +++ b/drivers/gpu/drm/xe/xe_mmio.h
> > @@ -21,6 +21,7 @@ struct xe_device;
> > #define GEN12_LMEM_BAR 2
> >
> > int xe_mmio_init(struct xe_device *xe);
> > +void xe_mmio_root_tile_init(struct xe_device *xe);
> > void xe_mmio_probe_tiles(struct xe_device *xe);
> >
> > static inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
> > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> > index 87c45c6e08aec..2f04f07dc17a4 100644
> > --- a/drivers/gpu/drm/xe/xe_pci.c
> > +++ b/drivers/gpu/drm/xe/xe_pci.c
> > @@ -532,10 +532,12 @@ static void handle_gmdid(struct xe_device *xe,
> > }
> > }
> >
> > -static void xe_info_init_early(struct xe_device *xe,
> > - const struct xe_device_desc *desc,
> > - const struct xe_subplatform_desc *subplatform_desc)
> > +static int xe_info_init_early(struct xe_device *xe,
> > + const struct xe_device_desc *desc,
> > + const struct xe_subplatform_desc *subplatform_desc)
> > {
> > + int err;
> > +
> > xe->info.platform = desc->platform;
> > xe->info.subplatform = subplatform_desc ?
> > subplatform_desc->subplatform : XE_SUBPLATFORM_NONE;
> > @@ -549,6 +551,12 @@ static void xe_info_init_early(struct xe_device *xe,
> > xe->info.enable_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
> > enable_display &&
> > desc->has_display;
> > +
> > + err = xe_tile_init_early(xe_device_get_root_tile(xe), xe, 0);
> > + if (err)
> > + return err;
> > +
> > + return 0;
> > }
> >
> > static int xe_info_init(struct xe_device *xe,
> > @@ -611,13 +619,15 @@ static int xe_info_init(struct xe_device *xe,
> > */
> > xe->info.tile_count = 1 + graphics_desc->max_remote_tiles;
> >
> > - for_each_tile(tile, xe, id) {
> > + for_each_remote_tile(tile, xe, id) {
> > int err;
> >
> > err = xe_tile_init_early(tile, xe, id);
> > if (err)
> > return err;
> > + }
> >
> > + for_each_tile(tile, xe, id) {
> > gt = tile->primary_gt;
> > gt->info.id = xe->info.gt_count++;
> > gt->info.type = XE_GT_TYPE_MAIN;
> > @@ -717,7 +727,13 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > pci_set_master(pdev);
> > devm_add_action(&pdev->dev, xe_pci_clear_master, pdev);
> >
> > - xe_info_init_early(xe, desc, subplatform_desc);
> > + err = xe_info_init_early(xe, desc, subplatform_desc);
> > + if (err)
> > + return err;
> > +
> > + err = xe_device_probe_early(xe);
> > + if (err)
> > + return err;
> >
> > err = xe_info_init(xe, desc, subplatform_desc);
> > if (err)
> > --
> > 2.42.0
> >
More information about the Intel-xe
mailing list