[PATCH v3 2/3] drm/xe: Expose display alignment requirement

[Zbigniew Kempczyński]

On Thu, Jul 11, 2024 at 05:44:24PM -0400, Rodrigo Vivi wrote:
> 
> On Thu, Jul 11, 2024 at 11:09:24AM +0200, Zbigniew Kempczyński wrote:
> 
> Please use prefix 'drm/xe/uapi:' on the commit subject for anything
> touching uapi.
> 
> And also documentation please.

Ok, I'm going to retag this to drm/xe/uapi.

Hmm, I've added this similar to MIN_ALIGNMENT (see on the bottom of the
patch). Do you want to elaborate this more? I thought xe_drm.h is
appropriate place to document this flag.

> 
> But more then that, we need to engage first with userspace and get
> them to implement this.

I'm aware that BMG display complicated things a bit. As it supports
4K on vram I think switching vram_flags to XE_VRAM_FLAGS_NEED64K will
work, but we loose 4K granularity on vram so imo there's a waste
of memory for smaller allocations.

> 
> > Scanout buffer on Battlemage requires allocation in 64K contigues
> > pages to support Tile4 + compression what differs from normal bo
> > requirements. Expose display alignment configuration to userspace
> > to ensure it will properly align requested memory.
> 
> We should probably consider something that aligns better with
> the default alignment that comes from the min_page_size
> from struct drm_xe_mem_region.

At the moment I see following options:

1. change xe_graphics_desc bmg to:
   .vram_flags = XE_VRAM_FLAGS_NEED64K
   at cost of loosing 4K granularity on vram

2. add new flag as I proposed; we may burn out one reserved u64 (u32)
   and expose min_display_page_size. But I'm not sure should we follow
   this way as display is not taken from system memory but vram
   (this field will be inappropriate set for system memory).

3. add new flag as I proposed, and expose this in via config query
   (current proposal).

4. any other ideas?

> 
> BTW: This will be a big impact in Media because right now they are
> having to use SCANOUT flag for every buffer since they have no
> ways to know upfront what buffers they will send to display.

If media will use buffer which is Tile4 + compressed I think they
will have no choice and they will need to allocate buffers in 64K contigues
chunks somehow.

At the moment I've mitigated this issue seen in kms_ccs for bmg
in series: https://patchwork.freedesktop.org/series/135994/
but this is not temporary and not fully correct solution until kernel
will switch to 64K physical alignment for scanout buffers.

--
Zbigniew

> 
> > 
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Cc: Matthew Auld <matthew.auld at intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_query.c | 4 +++-
> >  include/uapi/drm/xe_drm.h     | 3 +++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> > index 4e01df6b1b7a..c32894f3eea1 100644
> > --- a/drivers/gpu/drm/xe/xe_query.c
> > +++ b/drivers/gpu/drm/xe/xe_query.c
> > @@ -313,7 +313,7 @@ static int query_mem_regions(struct xe_device *xe,
> >  
> >  static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
> >  {
> > -	const u32 num_params = DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY + 1;
> > +	const u32 num_params = DRM_XE_QUERY_CONFIG_DISPLAY_ALIGNMENT + 1;
> >  	size_t size =
> >  		sizeof(struct drm_xe_query_config) + num_params * sizeof(u64);
> >  	struct drm_xe_query_config __user *query_ptr =
> > @@ -342,6 +342,8 @@ static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
> >  	config->info[DRM_XE_QUERY_CONFIG_VA_BITS] = xe->info.va_bits;
> >  	config->info[DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY] =
> >  		xe_exec_queue_device_get_max_priority(xe);
> > +	config->info[DRM_XE_QUERY_CONFIG_DISPLAY_ALIGNMENT] =
> > +		xe->info.vram_flags & XE_VRAM_FLAGS_DISPLAY_NEED64K ? SZ_64K : SZ_4K;
> >  
> >  	if (copy_to_user(query_ptr, config, size)) {
> >  		kfree(config);
> > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > index 19619d4952a8..c7a930dbf58c 100644
> > --- a/include/uapi/drm/xe_drm.h
> > +++ b/include/uapi/drm/xe_drm.h
> > @@ -398,6 +398,8 @@ struct drm_xe_query_mem_regions {
> >   *  - %DRM_XE_QUERY_CONFIG_VA_BITS - Maximum bits of a virtual address
> >   *  - %DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY - Value of the highest
> >   *    available exec queue priority
> > + *  - %DRM_XE_QUERY_CONFIG_DISPLAY_ALIGNMENT - Alignment of contigous physical
> > + *    memory allocation required by the display, typically SZ_4K or SZ_64K
> >   */
> >  struct drm_xe_query_config {
> >  	/** @num_params: number of parameters returned in info */
> > @@ -412,6 +414,7 @@ struct drm_xe_query_config {
> >  #define DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT		2
> >  #define DRM_XE_QUERY_CONFIG_VA_BITS			3
> >  #define DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY	4
> > +#define DRM_XE_QUERY_CONFIG_DISPLAY_ALIGNMENT		5
> >  	/** @info: array of elements containing the config info */
> >  	__u64 info[];
> >  };
> > -- 
> > 2.34.1
> >