Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Souza, Jose" <jose.souza@intel.com>
To: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
	"Yadav,  Sanjay Kumar" <sanjay.kumar.yadav@intel.com>
Cc: "Auld, Matthew" <matthew.auld@intel.com>
Subject: Re: [PATCH] drm/xe/uapi: Implement DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION flag
Date: Tue, 11 Nov 2025 17:05:59 +0000	[thread overview]
Message-ID: <5000eecca12c74b66d9606a41cc27431841fe5b7.camel@intel.com> (raw)
In-Reply-To: <20251028131031.2052163-2-sanjay.kumar.yadav@intel.com>

Hi Sanjay

Thanks for the patch but for Mesa to actually use it we need a way to
detect if running KMD support it or not.
So could you add a flag like DRM_XE_QUERY_CONFIG_FLAG_HAS_LOW_LATENCY 
for this feature?

thank you

On Tue, 2025-10-28 at 18:40 +0530, Sanjay Yadav wrote:
> Let userspace opt out of CCS compression on a per-BO basis. When set,
> the driver maps this to XE_BO_FLAG_NO_COMPRESSION, skips CCS metadata
> allocation/clearing, and rejects compressed PAT indices at vm_bind.
> This avoids extra memory ops and manual CCS state handling for
> buffer.
> 
> Signed-off-by: Sanjay Yadav <sanjay.kumar.yadav@intel.com>
> Suggested-by: Matthew Auld <matthew.auld@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c  | 15 +++++++++++++--
>  drivers/gpu/drm/xe/xe_bo.h  |  1 +
>  drivers/gpu/drm/xe/xe_pat.c |  9 ++++++++-
>  drivers/gpu/drm/xe/xe_pat.h | 15 +++++++++++++++
>  drivers/gpu/drm/xe/xe_vm.c  |  6 ++++++
>  include/uapi/drm/xe_drm.h   | 13 +++++++++++++
>  6 files changed, 56 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 7b6502081873..1421537ef9fc 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -3160,7 +3160,8 @@ int xe_gem_create_ioctl(struct drm_device *dev,
> void *data,
>  	if (XE_IOCTL_DBG(xe, args->flags &
>  			 ~(DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING |
>  			   DRM_XE_GEM_CREATE_FLAG_SCANOUT |
> -			  
> DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)))
> +			   DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM
> |
> +			   DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION)))
>  		return -EINVAL;
>  
>  	if (XE_IOCTL_DBG(xe, args->handle))
> @@ -3182,6 +3183,12 @@ int xe_gem_create_ioctl(struct drm_device
> *dev, void *data,
>  	if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT)
>  		bo_flags |= XE_BO_FLAG_SCANOUT;
>  
> +	if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) {
> +		if (GRAPHICS_VER(xe) < 20)
> +			return -EINVAL;
> +		bo_flags |= XE_BO_FLAG_NO_COMPRESSION;
> +	}
> +
>  	bo_flags |= args->placement << (ffs(XE_BO_FLAG_SYSTEM) - 1);
>  
>  	/* CCS formats need physical placement at a 64K alignment in
> VRAM. */
> @@ -3503,8 +3510,12 @@ bool xe_bo_needs_ccs_pages(struct xe_bo *bo)
>  	 * Compression implies coh_none, therefore we know for sure
> that WB
>  	 * memory can't currently use compression, which is likely
> one of the
>  	 * common cases.
> +	 * Additionally, userspace may explicitly request no
> compression via the
> +	 * DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION flag, which should
> also disable
> +	 * CCS usage.
>  	 */
> -	if (bo->cpu_caching == DRM_XE_GEM_CPU_CACHING_WB)
> +	if (bo->cpu_caching == DRM_XE_GEM_CPU_CACHING_WB ||
> +	    bo->flags & XE_BO_FLAG_NO_COMPRESSION)
>  		return false;
>  
>  	return true;
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index 353d607d301d..1217e386e697 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -50,6 +50,7 @@
>  #define XE_BO_FLAG_GGTT3		BIT(23)
>  #define XE_BO_FLAG_CPU_ADDR_MIRROR	BIT(24)
>  #define XE_BO_FLAG_FORCE_USER_VRAM	BIT(25)
> +#define XE_BO_FLAG_NO_COMPRESSION	BIT(26)
>  
>  /* this one is trigger internally only */
>  #define XE_BO_FLAG_INTERNAL_TEST	BIT(30)
> diff --git a/drivers/gpu/drm/xe/xe_pat.c
> b/drivers/gpu/drm/xe/xe_pat.c
> index 6e48ff84ad0a..991eeb551a72 100644
> --- a/drivers/gpu/drm/xe/xe_pat.c
> +++ b/drivers/gpu/drm/xe/xe_pat.c
> @@ -115,7 +115,8 @@ static const struct xe_pat_table_entry
> xelpg_pat_table[] = {
>  			REG_FIELD_PREP(XE2_L4_POLICY, l4_policy) | \
>  			REG_FIELD_PREP(XE2_COH_MODE, __coh_mode), \
>  		.coh_mode = (BUILD_BUG_ON_ZERO(__coh_mode &&
> comp_en) || __coh_mode) ? \
> -			XE_COH_AT_LEAST_1WAY : XE_COH_NONE \
> +			XE_COH_AT_LEAST_1WAY : XE_COH_NONE, \
> +		.compressed = comp_en, \
>  	}
>  
>  static const struct xe_pat_table_entry xe2_pat_table[] = {
> @@ -160,6 +161,12 @@ u16 xe_pat_index_get_coh_mode(struct xe_device
> *xe, u16 pat_index)
>  	return xe->pat.table[pat_index].coh_mode;
>  }
>  
> +bool xe_pat_index_has_compression(struct xe_device *xe, u16
> pat_index)
> +{
> +	xe_assert(xe, pat_index < xe->pat.n_entries);
> +	return xe->pat.table[pat_index].compressed;
> +}
> +
>  static void program_pat(struct xe_gt *gt, const struct
> xe_pat_table_entry table[],
>  			int n_entries)
>  {
> diff --git a/drivers/gpu/drm/xe/xe_pat.h
> b/drivers/gpu/drm/xe/xe_pat.h
> index 268c9a899f56..07319fca5f4c 100644
> --- a/drivers/gpu/drm/xe/xe_pat.h
> +++ b/drivers/gpu/drm/xe/xe_pat.h
> @@ -29,6 +29,11 @@ struct xe_pat_table_entry {
>  #define XE_COH_NONE          1
>  #define XE_COH_AT_LEAST_1WAY 2
>  	u16 coh_mode;
> +
> +	/**
> +	 * @compressed: Whether compression is enabled or not with
> @value.
> +	 */
> +	bool compressed;
>  };
>  
>  /**
> @@ -53,4 +58,14 @@ int xe_pat_dump(struct xe_gt *gt, struct
> drm_printer *p);
>   */
>  u16 xe_pat_index_get_coh_mode(struct xe_device *xe, u16 pat_index);
>  
> +/**
> + * xe_pat_index_has_compression - Check if the given pat_index
> enables
> + * compression.
> + * @xe: xe device
> + * @pat_index: The pat_index to query
> + *
> + * Note: Only applicable to Xe2+, where compression is part of the
> PAT index.
> + */
> +bool xe_pat_index_has_compression(struct xe_device *xe, u16
> pat_index);
> +
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 179758ca7cb8..98990ab14c85 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -3486,6 +3486,12 @@ static int xe_vm_bind_ioctl_validate_bo(struct
> xe_device *xe, struct xe_bo *bo,
>  {
>  	u16 coh_mode;
>  
> +	/* Reject compressed PAT index for BO with NO_COMPRESSION
> flag */
> +	if ((bo->flags & XE_BO_FLAG_NO_COMPRESSION) &&
> +	    xe_pat_index_has_compression(xe, pat_index))
> +		return -EINVAL;
> +
> +
>  	if (XE_IOCTL_DBG(xe, range > xe_bo_size(bo)) ||
>  	    XE_IOCTL_DBG(xe, obj_offset >
>  			 xe_bo_size(bo) - range)) {
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 40ff19f52a8d..07dea4a9dfd3 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -787,6 +787,18 @@ struct drm_xe_device_query {
>   *    need to use VRAM for display surfaces, therefore the kernel
> requires
>   *    setting this flag for such objects, otherwise an error is
> thrown on
>   *    small-bar systems.
> + *  - %DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION - Allows userspace to
> + *    hint that compression (CCS) should be disabled for the buffer
> being
> + *    created. This can avoid unnecessary memory operations and CCS
> state
> + *    management.
> + *    On pre-Xe2 platforms, this flag is currently rejected as
> compression
> + *    control is not supported via PAT index. On Xe2+ platforms,
> compression
> + *    is controlled via PAT entries. If this flag is set, the driver
> will reject
> + *    any VM bind that requests a PAT index enabling compression for
> this BO.
> + *    Note: On dGPU platforms, there is currently no change in
> behavior with
> + *    this flag, but future improvements may leverage it. The
> current benefit is
> + *    primarily applicable to iGPU platforms.
> +
>   *
>   * @cpu_caching supports the following values:
>   *  - %DRM_XE_GEM_CPU_CACHING_WB - Allocate the pages with write-
> back
> @@ -833,6 +845,7 @@ struct drm_xe_gem_create {
>  #define DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING		(1 << 0)
>  #define DRM_XE_GEM_CREATE_FLAG_SCANOUT			(1 << 1)
>  #define DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM	(1 << 2)
> +#define DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION		(1 << 3)
>  	/**
>  	 * @flags: Flags, currently a mask of memory instances of
> where BO can
>  	 * be placed

  parent reply	other threads:[~2025-11-11 17:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-28 13:10 [PATCH] drm/xe/uapi: Implement DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION flag Sanjay Yadav
2025-10-28 14:18 ` ✗ CI.checkpatch: warning for " Patchwork
2025-10-28 14:19 ` ✓ CI.KUnit: success " Patchwork
2025-10-28 15:22 ` ✓ Xe.CI.BAT: " Patchwork
2025-10-28 20:26 ` ✗ Xe.CI.Full: failure " Patchwork
2025-11-11 17:05 ` Souza, Jose [this message]
2025-11-11 17:22   ` [PATCH] " Souza, Jose

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5000eecca12c74b66d9606a41cc27431841fe5b7.camel@intel.com \
    --to=jose.souza@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    --cc=sanjay.kumar.yadav@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox