On Wed, Jun 19, 2024 at 06:39:04PM +0200, Thomas Hellström wrote:
> The caching mode for buffer objects with VRAM as a possible
> placement was forced to write-combined, regardless of placement.
> 
> However, write-combined system memory is expensive to allocate and
> even though it is pooled, the pool is expensive to shrink, since
> it involves global CPU TLB flushes.
> 
> Moreover write-combined system memory from TTM is only reliably
> available on x86 and DGFX doesn't have an x86 restriction.
> 
> So regardless of the cpu caching mode selected for a bo,
> internally use write-back caching mode for system memory on DGFX.
> 
> Coherency is maintained, but user-space clients may perceive a
> difference in cpu access speeds.

Reviewed-by: Rodrigo Vivi <rodrigo.v...@intel.com>

> 
> Signed-off-by: Thomas Hellström <thomas.hellst...@linux.intel.com>
> Fixes: 622f709ca629 ("drm/xe/uapi: Add support for CPU caching mode")
> Cc: Pallavi Mishra <pallavi.mis...@intel.com>
> Cc: Matthew Auld <matthew.a...@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Effie Yu <effie...@intel.com>
> Cc: Matthew Brost <matthew.br...@intel.com>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Jose Souza <jose.so...@intel.com>
> Cc: Michal Mrozek <michal.mro...@intel.com>
> Cc: <sta...@vger.kernel.org> # v6.8+
> ---
>  drivers/gpu/drm/xe/xe_bo.c       | 47 +++++++++++++++++++-------------
>  drivers/gpu/drm/xe/xe_bo_types.h |  3 +-
>  include/uapi/drm/xe_drm.h        |  8 +++++-
>  3 files changed, 37 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 65c696966e96..31192d983d9e 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -343,7 +343,7 @@ static struct ttm_tt *xe_ttm_tt_create(struct 
> ttm_buffer_object *ttm_bo,
>       struct xe_device *xe = xe_bo_device(bo);
>       struct xe_ttm_tt *tt;
>       unsigned long extra_pages;
> -     enum ttm_caching caching;
> +     enum ttm_caching caching = ttm_cached;
>       int err;
>  
>       tt = kzalloc(sizeof(*tt), GFP_KERNEL);
> @@ -357,26 +357,35 @@ static struct ttm_tt *xe_ttm_tt_create(struct 
> ttm_buffer_object *ttm_bo,
>               extra_pages = DIV_ROUND_UP(xe_device_ccs_bytes(xe, bo->size),
>                                          PAGE_SIZE);
>  
> -     switch (bo->cpu_caching) {
> -     case DRM_XE_GEM_CPU_CACHING_WC:
> -             caching = ttm_write_combined;
> -             break;
> -     default:
> -             caching = ttm_cached;
> -             break;
> -     }
> -
> -     WARN_ON((bo->flags & XE_BO_FLAG_USER) && !bo->cpu_caching);
> -
>       /*
> -      * Display scanout is always non-coherent with the CPU cache.
> -      *
> -      * For Xe_LPG and beyond, PPGTT PTE lookups are also non-coherent and
> -      * require a CPU:WC mapping.
> +      * DGFX system memory is always WB / ttm_cached, since
> +      * other caching modes are only supported on x86. DGFX
> +      * GPU system memory accesses are always coherent with the
> +      * CPU.
>        */
> -     if ((!bo->cpu_caching && bo->flags & XE_BO_FLAG_SCANOUT) ||
> -         (xe->info.graphics_verx100 >= 1270 && bo->flags & 
> XE_BO_FLAG_PAGETABLE))
> -             caching = ttm_write_combined;
> +     if (!IS_DGFX(xe)) {
> +             switch (bo->cpu_caching) {
> +             case DRM_XE_GEM_CPU_CACHING_WC:
> +                     caching = ttm_write_combined;
> +                     break;
> +             default:
> +                     caching = ttm_cached;
> +                     break;
> +             }
> +
> +             WARN_ON((bo->flags & XE_BO_FLAG_USER) && !bo->cpu_caching);
> +
> +             /*
> +              * Display scanout is always non-coherent with the CPU cache.
> +              *
> +              * For Xe_LPG and beyond, PPGTT PTE lookups are also
> +              * non-coherent and require a CPU:WC mapping.
> +              */
> +             if ((!bo->cpu_caching && bo->flags & XE_BO_FLAG_SCANOUT) ||
> +                 (xe->info.graphics_verx100 >= 1270 &&
> +                  bo->flags & XE_BO_FLAG_PAGETABLE))
> +                     caching = ttm_write_combined;
> +     }
>  
>       if (bo->flags & XE_BO_FLAG_NEEDS_UC) {
>               /*
> diff --git a/drivers/gpu/drm/xe/xe_bo_types.h 
> b/drivers/gpu/drm/xe/xe_bo_types.h
> index 86422e113d39..10450f1fbbde 100644
> --- a/drivers/gpu/drm/xe/xe_bo_types.h
> +++ b/drivers/gpu/drm/xe/xe_bo_types.h
> @@ -66,7 +66,8 @@ struct xe_bo {
>  
>       /**
>        * @cpu_caching: CPU caching mode. Currently only used for userspace
> -      * objects.
> +      * objects. Exceptions are system memory on DGFX, which is always
> +      * WB.
>        */
>       u16 cpu_caching;
>  
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 93e00be44b2d..1189b3044723 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -783,7 +783,13 @@ struct drm_xe_gem_create {
>  #define DRM_XE_GEM_CPU_CACHING_WC                      2
>       /**
>        * @cpu_caching: The CPU caching mode to select for this object. If
> -      * mmaping the object the mode selected here will also be used.
> +      * mmaping the object the mode selected here will also be used. The
> +      * exception is when mapping system memory (including evicted
> +      * system memory) on discrete GPUs. The caching mode selected will
> +      * then be overridden to DRM_XE_GEM_CPU_CACHING_WB, and coherency
> +      * between GPU- and CPU is guaranteed. The caching mode of
> +      * existing CPU-mappings will be updated transparently to
> +      * user-space clients.
>        */
>       __u16 cpu_caching;
>       /** @pad: MBZ */
> -- 
> 2.44.0
> 

Reply via email to