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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

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.

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

> 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.

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.

> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@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
>