[PATCH 2/8] drm/xe/svm: Add DRM_XE_SVM kernel config entry
Zeng, Oak
oak.zeng at intel.com
Tue Mar 19 18:27:43 UTC 2024
> -----Original Message-----
> From: Hellstrom, Thomas <thomas.hellstrom at intel.com>
> Sent: Tuesday, March 19, 2024 5:26 AM
> To: intel-xe at lists.freedesktop.org; Zeng, Oak <oak.zeng at intel.com>
> Cc: Brost, Matthew <matthew.brost at intel.com>; Welty, Brian
> <brian.welty at intel.com>; Ghimiray, Himal Prasad
> <himal.prasad.ghimiray at intel.com>
> Subject: Re: [PATCH 2/8] drm/xe/svm: Add DRM_XE_SVM kernel config entry
>
> Hi, Oak,
>
> On Mon, 2024-03-18 at 22:55 -0400, Oak Zeng wrote:
> > DRM_XE_SVM kernel config entry is added so
> > xe svm feature can be configured before kernel
> > compilation.
>
> Please use imperative language in commit messages.
>
>
> >
> > Signed-off-by: Oak Zeng <oak.zeng at intel.com>
> > Co-developed-by: Niranjana Vishwanathapura
> > <niranjana.vishwanathapura at intel.com>
> > Signed-off-by: Niranjana Vishwanathapura
> > <niranjana.vishwanathapura at intel.com>
> > Cc: Matthew Brost <matthew.brost at intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom at intel.com>
> > Cc: Brian Welty <brian.welty at intel.com>
> > ---
> > drivers/gpu/drm/xe/Kconfig | 21 +++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_tile.c | 4 ++++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
> > index 1a556d087e63..e244165459c5 100644
> > --- a/drivers/gpu/drm/xe/Kconfig
> > +++ b/drivers/gpu/drm/xe/Kconfig
> > @@ -83,6 +83,27 @@ config DRM_XE_FORCE_PROBE
> > 4571.
> >
> > Use "!*" to block the probe of the driver for all known
> > devices.
> > +config DRM_XE_SVM
> > + bool "Enable Shared Virtual Memory support in xe"
> > + depends on DRM_XE
> > + depends on ARCH_ENABLE_MEMORY_HOTPLUG
> > + depends on ARCH_ENABLE_MEMORY_HOTREMOVE
> > + depends on MEMORY_HOTPLUG
> > + depends on MEMORY_HOTREMOVE
> > + depends on ARCH_HAS_PTE_DEVMAP
> > + depends on SPARSEMEM_VMEMMAP
> > + depends on ZONE_DEVICE
> > + depends on DEVICE_PRIVATE
> > + depends on MMU
> > + select HMM_MIRROR
> > + select MMU_NOTIFIER
> > + default y
> > + help
> > + Choose this option if you want Shared Virtual Memory (SVM)
> > + support in xe. With SVM, virtual address space is shared
> > + between CPU and GPU. This means any virtual address such
> > + as malloc or mmap returns, variables on stack, or global
> > + memory pointers, can be used for GPU transparently.
> >
> > menu "drm/Xe Debugging"
> > depends on DRM_XE
> > diff --git a/drivers/gpu/drm/xe/xe_tile.c
> > b/drivers/gpu/drm/xe/xe_tile.c
> > index f1c4f9de51df..b52a00a6b5d5 100644
> > --- a/drivers/gpu/drm/xe/xe_tile.c
> > +++ b/drivers/gpu/drm/xe/xe_tile.c
> > @@ -159,7 +159,9 @@ static int tile_ttm_mgr_init(struct xe_tile
> > *tile)
> > */
> > int xe_tile_init_noalloc(struct xe_tile *tile)
> > {
> > +#if IS_ENABLED(CONFIG_DRM_XE_SVM)
> > struct xe_device *xe = tile_to_xe(tile);
> > +#endif
>
> Avoid using conditional compilation in this way inside functions if
> possible, please see below:
Below is doable.
For above, do you want something like:
#if IS_ENABLED(CONFIG_DRM_XE_SVM)
struct xe_device *xe;
#endif
If (IS_ENABLED(CONFIG_DRM_XE_SVM))
Xe = tile_to_xe(tile);
It looks like we have to use #if in above code... otherwise, the definition of xe will be not used when DRM_XE_SVM is not defined...
Maybe keep the original for above?
>
> > int err;
> >
> > xe_device_mem_access_get(tile_to_xe(tile));
> > @@ -177,8 +179,10 @@ int xe_tile_init_noalloc(struct xe_tile *tile)
> >
> > xe_tile_sysfs_init(tile);
> >
> > +#if IS_ENABLED(CONFIG_DRM_XE_SVM)
> > if (xe->info.has_usm)
>
> Could be:
> if (IS_ENABLED(CONFIG_DRM_XE_SVM) && tile_to_xe(tile)-
> >info.has_usm)
Will fix.
Oak
>
> /Thomas
>
>
> > xe_devm_add(tile, &tile->mem.vram);
> > +#endif
> > err_mem_access:
> > xe_device_mem_access_put(tile_to_xe(tile));
> > return err;
More information about the Intel-xe
mailing list