[i-g-t,v2,2/2] lib/xe_mmio: Introduce Xe MMIO lib
Piotr Piórkowski
piotr.piorkowski at intel.com
Wed Apr 3 09:57:29 UTC 2024
Laguna, Lukasz <lukasz.laguna at intel.com> wrote on pią [2024-mar-29 10:35:34 +0100]:
> On 3/26/2024 11:09, Piórkowski, Piotr wrote:
> > From: Piotr Piórkowski <piotr.piorkowski at intel.com>
> >
> > Currently in IGT we have a library intel_mmio for simple MMIO operations
> > on intel GPU devices, but it is limited only to accessing registers,
> > has a lot of legacy code related to the i915 and offers no support for
> > multi tile.
> > Let's reuse the memory mapping from the previous library and add separate
> > helpers, dedicated to Xe, for registers and GGTT access that support multi
> > tile.
> >
> > v2:
> > - add missing std ifndef/define in header lib/xe_mmio.h
> >
> > Signed-off-by: Piotr Piórkowski <piotr.piorkowski at intel.com>
> > Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > ---
> > lib/meson.build | 1 +
> > lib/xe/xe_mmio.c | 207 ++++++++++++++++++++++++++++++++++++++++++++++
> > lib/xe/xe_mmio.h | 42 ++++++++++
> > lib/xe/xe_query.c | 19 +++++
> > lib/xe/xe_query.h | 1 +
> > 5 files changed, 270 insertions(+)
> > create mode 100644 lib/xe/xe_mmio.c
> > create mode 100644 lib/xe/xe_mmio.h
> >
> > diff --git a/lib/meson.build b/lib/meson.build
> > index a5651571b..e2f740c11 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -111,6 +111,7 @@ lib_sources = [
> > 'igt_dsc.c',
> > 'xe/xe_gt.c',
> > 'xe/xe_ioctl.c',
> > + 'xe/xe_mmio.c',
> > 'xe/xe_query.c',
> > 'xe/xe_spin.c',
> > 'xe/xe_util.c',
> > diff --git a/lib/xe/xe_mmio.c b/lib/xe/xe_mmio.c
> > new file mode 100644
> > index 000000000..bd4227f25
> > --- /dev/null
> > +++ b/lib/xe/xe_mmio.c
> > @@ -0,0 +1,207 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright(c) 2024 Intel Corporation. All rights reserved.
> > + */
> > +
> > +#include "igt_device.h"
> > +
> > +#include "xe/xe_mmio.h"
> > +#include "xe/xe_query.h"
> > +
> > +/**
> > + * xe_mmio_vf_access_init:
> > + * @pf_fd: xe device file descriptor
> > + * @vf_id: PCI virtual function number (0 if native or PF itself)
> > + * @mmio: xe mmio structure for IO operations
> > + *
> > + * This initializes the xe mmio structure, and maps the MMIO BAR owned by
> > + * the specified virtual function associated with @pf_fd.
> > + */
> > +void xe_mmio_vf_access_init(int pf_fd, int vf_id, struct xe_mmio *mmio)
> > +{
> > + struct pci_device *pci_dev = __igt_device_get_pci_device(pf_fd, vf_id);
> > +
> > + igt_assert_f(pci_dev, "No PCI device found for VF%u\n", vf_id);
> > +
> > + intel_mmio_use_pci_bar(&mmio->intel_mmio, pci_dev);
> > +
> > + igt_assert(!mmio->intel_mmio.igt_mmio);
> > +
> > + mmio->fd = pf_fd;
>
>
> no mmio->vf_id = ... ?
>
>
> > + mmio->intel_mmio.safe = false;
> > + mmio->intel_mmio.pci_device_id = pci_dev->device_id;
> > +}
> > +
> > +/**
> > + * xe_mmio_access_init:
> > + * @pf_fd: xe device file descriptor
> > + * @mmio: xe mmio structure for IO operations
> > + *
> > + * This initializes the xe mmio structure, and maps MMIO BAR for @pf_fd device.
> > + */
> > +void xe_mmio_access_init(int pf_fd, struct xe_mmio *mmio)
> > +{
> > + xe_mmio_vf_access_init(pf_fd, 0, mmio);
> > +}
> > +
> > +/**
> > + * xe_mmio_access_fini:
> > + * @mmio: xe mmio structure for IO operations
> > + *
> > + * Clean up the mmio access helper initialized with
> > + * xe_mmio_access_init()/xe_mmio_vf_access_init().
> > + */
> > +void xe_mmio_access_fini(struct xe_mmio *mmio)
> > +{
> > + mmio->intel_mmio.pci_device_id = 0;
>
>
> Hmm... maybe better to remove warn from intel_mmio_unmap_pci_bar() ? Is it
> really needed there?
I didn't want to modify it and I made it similar to intel_register_access_fini here.
>
>
> > + intel_mmio_unmap_pci_bar(&mmio->intel_mmio);
> > + igt_pci_system_cleanup();
> > +}
> > +
> > +/**
> > + * xe_mmio_read32:
> > + * @mmio: xe mmio structure for IO operations
> > + * @reg: mmio register offset
> > + *
> > + * 32-bit read of the register at @offset.
>
>
> @reg / @offset inconsistency
> s/@offset/@reg ?
>
my copy/paste mistake
>
> > + *
> > + * Returns:
> > + * The value read from the register.
> > + */
> > +uint32_t xe_mmio_read32(struct xe_mmio *mmio, uint32_t reg)
> > +{
> > + return ioread32(mmio->intel_mmio.igt_mmio, reg);
> > +}
> > +
> > +/**
> > + * xe_mmio_read64:
> > + * @mmio: xe mmio structure for IO operations
> > + * @reg: mmio register offset
> > + *
> > + * 64-bit read of the register at @offset.
> > + *
> > + * Returns:
> > + * The value read from the register.
> > + */
> > +uint64_t xe_mmio_read64(struct xe_mmio *mmio, uint32_t reg)
> > +{
> > + return ioread64(mmio->intel_mmio.igt_mmio, reg);
> > +}
> > +
> > +/**
> > + * xe_mmio_write32:
> > + * @mmio: xe mmio structure for IO operations
> > + * @reg: mmio register offset
> > + * @val: value to write
> > + *
> > + * 32-bit write to the register at @offset.
> > + */
> > +void xe_mmio_write32(struct xe_mmio *mmio, uint32_t reg, uint32_t val)
> > +{
> > + return iowrite32(mmio->intel_mmio.igt_mmio, reg, val);
> > +}
> > +
> > +/**
> > + * xe_mmio_write64:
> > + * @mmio: xe mmio structure for IO operations
> > + * @reg: mmio register offset
> > + * @val: value to write
> > + *
> > + * 64-bit write to the register at @offset.
> > + */
> > +void xe_mmio_write64(struct xe_mmio *mmio, uint32_t reg, uint64_t val)
> > +{
> > + return iowrite64(mmio->intel_mmio.igt_mmio, reg, val);
> > +}
> > +
> > +/**
> > + * xe_mmio_gt_read32:
> > + * @mmio: xe mmio structure for IO operations
> > + * @gt: gt id
> > + * @reg: mmio register offset in tile to which @gt belongs
> > + *
> > + * 32-bit read of the register at @offset in tile to which @gt belongs.
> > + *
> > + * Returns:
> > + * The value read from the register.
> > + */
> > +uint32_t xe_mmio_gt_read32(struct xe_mmio *mmio, int gt, uint32_t reg)
> > +{
> > + return xe_mmio_read32(mmio, reg + (TILE_MMIO_SIZE * xe_gt_get_tile_id(mmio->fd, gt)));
> > +}
> > +
> > +/**
> > + * xe_mmio_gt_read64:
> > + * @mmio: xe mmio structure for IO operations
> > + * @gt: gt id
> > + * @reg: mmio register offset in tile to which @gt belongs
> > + *
> > + * 64-bit read of the register at @offset in tile to which @gt belongs.
> > + *
> > + * Returns:
> > + * The value read from the register.
> > + */
> > +uint64_t xe_mmio_gt_read64(struct xe_mmio *mmio, int gt, uint32_t reg)
> > +{
> > + return xe_mmio_read64(mmio, reg + (TILE_MMIO_SIZE * xe_gt_get_tile_id(mmio->fd, gt)));
> > +}
> > +
> > +/**
> > + * xe_mmio_gt_write32:
> > + * @mmio: xe mmio structure for IO operations
> > + * @gt: gt id
> > + * @reg: mmio register offset
> > + * @val: value to write
> > + *
> > + * 32-bit write to the register at @offset in tile to which @gt belongs.
> > + */
> > +void xe_mmio_gt_write32(struct xe_mmio *mmio, int gt, uint32_t reg, uint32_t val)
> > +{
> > + return xe_mmio_write32(mmio, reg + (TILE_MMIO_SIZE * xe_gt_get_tile_id(mmio->fd, gt)),
> > + val);
> > +}
> > +
> > +/**
> > + * xe_mmio_gt_write64:
> > + * @mmio: xe mmio structure for IO operations
> > + * @gt: gt id
> > + * @reg: mmio register offset
> > + * @val: value to write
> > + *
> > + * 64-bit write to the register at @offset in tile to which @gt belongs.
> > + */
> > +void xe_mmio_gt_write64(struct xe_mmio *mmio, int gt, uint32_t reg, uint64_t val)
> > +{
> > + return xe_mmio_write64(mmio, reg + (TILE_MMIO_SIZE * xe_gt_get_tile_id(mmio->fd, gt)),
> > + val);
> > +}
> > +
> > +/**
> > + * xe_mmio_ggtt_read:
> > + * @mmio: xe mmio structure for IO operations
> > + * @gt: gt id
> > + * @offset: PTE offset from the beginning of GGTT, in tile to which @gt belongs
> > + *
> > + * Read of GGTT PTE at GGTT @offset in tile to which @gt belongs.
> > + *
> > + * Returns:
> > + * The value read from the register.
> > + */
> > +xe_ggtt_pte_t xe_mmio_ggtt_read(struct xe_mmio *mmio, int gt, uint32_t offset)
> > +{
> > + return xe_mmio_gt_read64(mmio, gt, offset + GGTT_OFFSET_IN_TILE);
> > +}
> > +
> > +/**
> > + * xe_mmio_ggtt_write:
> > + * @mmio: xe mmio structure for IO operations
> > + * @gt: gt id
> > + * @offset: PTE offset from the beginning of GGTT, in tile to which @gt belongs
> > + * @pte: PTE value to write
> > + *
> > + * Write PTE value at GGTT @offset in tile to which @gt belongs.
> > + */
> > +void xe_mmio_ggtt_write(struct xe_mmio *mmio, int gt, uint32_t offset, xe_ggtt_pte_t pte)
> > +{
> > + return xe_mmio_gt_write64(mmio, gt, offset + GGTT_OFFSET_IN_TILE, pte);
> > +}
> > diff --git a/lib/xe/xe_mmio.h b/lib/xe/xe_mmio.h
> > new file mode 100644
> > index 000000000..96bba6e16
> > --- /dev/null
> > +++ b/lib/xe/xe_mmio.h
> > @@ -0,0 +1,42 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright(c) 2024 Intel Corporation. All rights reserved.
> > + */
> > +
> > +#include "lib/intel_io.h"
> > +#include "lib/igt_sizes.h"
> > +
> > +#ifndef XE_MMIO_H
> > +#define XE_MMIO_H
> > +
> > +#define TILE_MMIO_SIZE SZ_16M
> > +#define GGTT_OFFSET_IN_TILE SZ_8M
> > +
> > +typedef uint64_t xe_ggtt_pte_t;
> > +
> > +struct xe_mmio {
> > + int fd;
> > + unsigned int vf_id;
> > + struct intel_mmio_data intel_mmio;
> > +};
> > +
> > +void xe_mmio_vf_access_init(int pf_fd, int vf_id, struct xe_mmio *mmio);
> > +void xe_mmio_access_init(int pf_fd, struct xe_mmio *mmio);
> > +void xe_mmio_access_fini(struct xe_mmio *mmio);
> > +
> > +uint32_t xe_mmio_read32(struct xe_mmio *mmio, uint32_t reg);
> > +uint64_t xe_mmio_read64(struct xe_mmio *mmio, uint32_t reg);
> > +
> > +void xe_mmio_write32(struct xe_mmio *mmio, uint32_t reg, uint32_t val);
> > +void xe_mmio_write64(struct xe_mmio *mmio, uint32_t reg, uint64_t val);
> > +
> > +uint32_t xe_mmio_gt_read32(struct xe_mmio *mmio, int gt, uint32_t reg);
> > +uint64_t xe_mmio_gt_read64(struct xe_mmio *mmio, int gt, uint32_t reg);
> > +
> > +void xe_mmio_gt_write32(struct xe_mmio *mmio, int gt, uint32_t reg, uint32_t val);
> > +void xe_mmio_gt_write64(struct xe_mmio *mmio, int gt, uint32_t reg, uint64_t val);
> > +
> > +xe_ggtt_pte_t xe_mmio_ggtt_read(struct xe_mmio *mmio, int gt, uint32_t pte_offset);
> > +void xe_mmio_ggtt_write(struct xe_mmio *mmio, int gt, uint32_t pte_offset, xe_ggtt_pte_t pte);
> > +
> > +#endif /* XE_MMIO_H */
> > diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> > index 53a2b4386..c885e3a79 100644
> > --- a/lib/xe/xe_query.c
> > +++ b/lib/xe/xe_query.c
> > @@ -731,6 +731,25 @@ bool xe_is_media_gt(int fd, int gt)
> > return false;
> > }
> > +/**
> > + * xe_gt_to_tile_id:
> > + * @fd: xe device fd
> > + * @gt: gt id
> > + *
> > + * Returns tile id for given @gt.
> > + */
> > +int xe_gt_get_tile_id(int fd, int gt)
>
>
> uint16_t xe_gt_get_tile_id(...)
Yes, it should be u16 here.
Thanks for review!
Piotr
>
>
> > +{
> > + struct xe_device *xe_dev;
> > +
> > + xe_dev = find_in_cache(fd);
> > +
> > + igt_assert(xe_dev);
> > + igt_assert(gt < xe_number_gt(fd));
> > +
> > + return xe_dev->gt_list->gt_list[gt].tile_id;
> > +}
> > +
> > igt_constructor
> > {
> > xe_device_cache_init();
> > diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
> > index 82af2706d..b1b3a989e 100644
> > --- a/lib/xe/xe_query.h
> > +++ b/lib/xe/xe_query.h
> > @@ -99,6 +99,7 @@ const char *xe_engine_class_string(uint32_t engine_class);
> > bool xe_has_engine_class(int fd, uint16_t engine_class);
> > bool xe_has_media_gt(int fd);
> > bool xe_is_media_gt(int fd, int gt);
> > +int xe_gt_get_tile_id(int fd, int gt);
> > struct xe_device *xe_device_get(int fd);
> > void xe_device_put(int fd);
--
More information about the igt-dev
mailing list