Re: [PATCH 10/26] drm/xe: Add sa/guc_buf_cache sync interface

Michał Winiarski
2025-10-21T00:45:27Z

On Sun, Oct 12, 2025 at 11:06:28AM -0700, Matthew Brost wrote:
> On Sat, Oct 11, 2025 at 09:38:31PM +0200, Michał Winiarski wrote:
> > In upcoming changes the cached buffers are going to be used to read data
> > produced by the GuC. Add a counterpart to flush, which synchronizes the
> > CPU-side of suballocation with the GPU data and propagate the interface
> > to GuC Buffer Cache.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_guc_buf.c | 9 +++++++++
> > drivers/gpu/drm/xe/xe_guc_buf.h | 1 +
> > drivers/gpu/drm/xe/xe_sa.c | 21 +++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_sa.h | 1 +
> > 4 files changed, 32 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_guc_buf.c b/drivers/gpu/drm/xe/xe_guc_buf.c
> > index 502ca3a4ee606..1be26145f0b98 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_buf.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_buf.c
> > @@ -127,6 +127,15 @@ u64 xe_guc_buf_flush(const struct xe_guc_buf buf)
> > return xe_sa_bo_gpu_addr(buf.sa);
> > }
> >
> > +/**
> > + * xe_guc_buf_sync() - Copy the data from the GPU memory to the sub-allocation.
> > + * @buf: the &xe_guc_buf to sync
> > + */
> > +void xe_guc_buf_sync(const struct xe_guc_buf buf)
>
> s/sync/sync_read ?
>
> or
>
> s/sync/flush_read ?
>
> Patch itself LGTM.
>
> Matt

I'll rename it to sync_read.

Thanks,
-Michał

>
> > +{
> > + xe_sa_bo_sync(buf.sa);
> > +}
> > +
> > /**
> > * xe_guc_buf_cpu_ptr() - Obtain a CPU pointer to the sub-allocation.
> > * @buf: the &xe_guc_buf to query
> > diff --git a/drivers/gpu/drm/xe/xe_guc_buf.h b/drivers/gpu/drm/xe/xe_guc_buf.h
> > index 0d67604d96bdd..fe6b5ffe0d6eb 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_buf.h
> > +++ b/drivers/gpu/drm/xe/xe_guc_buf.h
> > @@ -31,6 +31,7 @@ static inline bool xe_guc_buf_is_valid(const struct xe_guc_buf buf)
> >
> > void *xe_guc_buf_cpu_ptr(const struct xe_guc_buf buf);
> > u64 xe_guc_buf_flush(const struct xe_guc_buf buf);
> > +void xe_guc_buf_sync(const struct xe_guc_buf buf);
> > u64 xe_guc_buf_gpu_addr(const struct xe_guc_buf buf);
> > u64 xe_guc_cache_gpu_addr_from_ptr(struct xe_guc_buf_cache *cache, const void *ptr, u32 size);
> >
> > diff --git a/drivers/gpu/drm/xe/xe_sa.c b/drivers/gpu/drm/xe/xe_sa.c
> > index fedd017d6dd36..2115789c2bfb7 100644
> > --- a/drivers/gpu/drm/xe/xe_sa.c
> > +++ b/drivers/gpu/drm/xe/xe_sa.c
> > @@ -110,6 +110,10 @@ struct drm_suballoc *__xe_sa_bo_new(struct xe_sa_manager *sa_manager, u32 size,
> > return drm_suballoc_new(&sa_manager->base, size, gfp, true, 0);
> > }
> >
> > +/**
> > + * xe_sa_bo_flush_write() - Copy the data from the sub-allocation to the GPU memory.
> > + * @sa_bo: the &drm_suballoc to flush
> > + */
> > void xe_sa_bo_flush_write(struct drm_suballoc *sa_bo)
> > {
> > struct xe_sa_manager *sa_manager = to_xe_sa_manager(sa_bo->manager);
> > @@ -123,6 +127,23 @@ void xe_sa_bo_flush_write(struct drm_suballoc *sa_bo)
> > drm_suballoc_size(sa_bo));
> > }
> >
> > +/**
> > + * xe_sa_bo_sync() - Copy the data from GPU memory to the sub-allocation.
> > + * @sa_bo: the &drm_suballoc to sync
> > + */
> > +void xe_sa_bo_sync(struct drm_suballoc *sa_bo)
> > +{
> > + struct xe_sa_manager *sa_manager = to_xe_sa_manager(sa_bo->manager);
> > + struct xe_device *xe = tile_to_xe(sa_manager->bo->tile);
> > +
> > + if (!sa_manager->bo->vmap.is_iomem)
> > + return;
> > +
> > + xe_map_memcpy_from(xe, xe_sa_bo_cpu_addr(sa_bo), &sa_manager->bo->vmap,
> > + drm_suballoc_soffset(sa_bo),
> > + drm_suballoc_size(sa_bo));
> > +}
> > +
> > void xe_sa_bo_free(struct drm_suballoc *sa_bo,
> > struct dma_fence *fence)
> > {
> > diff --git a/drivers/gpu/drm/xe/xe_sa.h b/drivers/gpu/drm/xe/xe_sa.h
> > index 99dbf0eea5402..28fd8bb6450c2 100644
> > --- a/drivers/gpu/drm/xe/xe_sa.h
> > +++ b/drivers/gpu/drm/xe/xe_sa.h
> > @@ -37,6 +37,7 @@ static inline struct drm_suballoc *xe_sa_bo_new(struct xe_sa_manager *sa_manager
> > }
> >
> > void xe_sa_bo_flush_write(struct drm_suballoc *sa_bo);
> > +void xe_sa_bo_sync(struct drm_suballoc *sa_bo);
> > void xe_sa_bo_free(struct drm_suballoc *sa_bo, struct dma_fence *fence);
> >
> > static inline struct xe_sa_manager *
> > --
> > 2.50.1
> >