Re: [Intel-gfx] [PATCH v3 2/5] drm/i915/uapi: convert drm_i915_gem_caching to kernel doc

[Ramalingam C <ramalingam.c@intel.com>]

On 2021-07-05 at 14:53:07 +0100, Matthew Auld wrote:
> Convert all the drm_i915_gem_caching bits to proper kernel doc.
> 
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
LGTM.

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> ---
>  include/uapi/drm/i915_drm.h | 70 +++++++++++++++++++++++--------------
>  1 file changed, 43 insertions(+), 27 deletions(-)
> 
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 2f70c48567c0..d13c6c5fad04 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1363,43 +1363,59 @@ struct drm_i915_gem_busy {
>  };
>  
>  /**
> - * I915_CACHING_NONE
> + * struct drm_i915_gem_caching - Set or get the caching for given object
> + * handle.
>   *
> - * GPU access is not coherent with cpu caches. Default for machines without an
> - * LLC.
> - */
> -#define I915_CACHING_NONE		0
> -/**
> - * I915_CACHING_CACHED
> + * Allow userspace to control the GTT caching bits for a given object when the
> + * object is later mapped through the ppGTT(or GGTT on older platforms lacking
> + * ppGTT support, or if the object is used for scanout). Note that this might
> + * require unbinding the object from the GTT first, if its current caching value
> + * doesn't match.
>   *
> - * GPU access is coherent with cpu caches and furthermore the data is cached in
> - * last-level caches shared between cpu cores and the gpu GT. Default on
> - * machines with HAS_LLC.
> - */
> -#define I915_CACHING_CACHED		1
> -/**
> - * I915_CACHING_DISPLAY
>   *
> - * Special GPU caching mode which is coherent with the scanout engines.
> - * Transparently falls back to I915_CACHING_NONE on platforms where no special
> - * cache mode (like write-through or gfdt flushing) is available. The kernel
> - * automatically sets this mode when using a buffer as a scanout target.
> - * Userspace can manually set this mode to avoid a costly stall and clflush in
> - * the hotpath of drawing the first frame.
>   */
> -#define I915_CACHING_DISPLAY		2
> -
>  struct drm_i915_gem_caching {
>  	/**
> -	 * Handle of the buffer to set/get the caching level of. */
> +	 * @handle: Handle of the buffer to set/get the caching level.
> +	 */
>  	__u32 handle;
>  
>  	/**
> -	 * Cacheing level to apply or return value
> +	 * @caching: The GTT caching level to apply or possible return value.
> +	 *
> +	 * The supported @caching values:
>  	 *
> -	 * bits0-15 are for generic caching control (i.e. the above defined
> -	 * values). bits16-31 are reserved for platform-specific variations
> -	 * (e.g. l3$ caching on gen7). */
> +	 * I915_CACHING_NONE:
> +	 *
> +	 * GPU access is not coherent with CPU caches.  Default for machines
> +	 * without an LLC. This means we need to manually clflush, if we want
> +	 * GPU access to be coherent.
> +	 *
> +	 * I915_CACHING_CACHED:
> +	 *
> +	 * GPU access is coherent with CPU caches and furthermore the data is
> +	 * cached in last-level caches shared between CPU cores and the GPU GT.
> +	 * Default on machines with HAS_LLC. In general the fast shared
> +	 * last-level cache(HAS_LLC) is considered much faster then platforms
> +	 * which only support snooping(HAS_SNOOP), hence by default
> +	 *
> +	 * I915_CACHING_DISPLAY:
> +	 *
> +	 * Special GPU caching mode which is coherent with the scanout engines.
> +	 * Transparently falls back to I915_CACHING_NONE on platforms where no
> +	 * special cache mode (like write-through or gfdt flushing) is
> +	 * available. The kernel automatically sets this mode when using a
> +	 * buffer as a scanout target.  Userspace can manually set this mode to
> +	 * avoid a costly stall and clflush in the hotpath of drawing the first
> +	 * frame.
> +	 *
> +	 * Side note: On gen8+ this no longer does much since we lost the GGTT
> +	 * caching bits. Although setting this is harmless, since it still
> +	 * effectively falls back to I915_CACHING_NONE.
> +	 */
> +#define I915_CACHING_NONE		0
> +#define I915_CACHING_CACHED		1
> +#define I915_CACHING_DISPLAY		2
>  	__u32 caching;
>  };
>  
> -- 
> 2.26.3
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx