Re: [Intel-gfx] [PATCH 5/8] drm/i915/mtl: end support for set caching ioctl
On Wed, Apr 19, 2023 at 04:00:55PM -0700, fei.y...@intel.com wrote:
> From: Fei Yang
>
> The design is to keep Buffer Object's caching policy immutable through
> out its life cycle. This patch ends the support for set caching ioctl
> from MTL onward. While doing that we also set BO's to be 1-way coherent
> at creation time because GPU is no longer automatically snooping CPU
> cache. For UMD's need to fine tune the caching policy for BO's, a follow
> up patch will extend the GEM_CREATE uAPI to allow UMD's specify caching
> mode at BO creation time.
Nitpick: I don't think "UMD" is a term that anyone really uses outside
of Intel. It's probably better to just say "userspace" instead of
"UMD" since that's more accurate anyway.
Matt
>
> Signed-off-by: Fei Yang
> Reviewed-by: Andi Shyti
> Reviewed-by: Andrzej Hajda
> ---
> drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 9 -
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> index d2d5a24301b2..bb3575b1479f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> @@ -337,6 +337,9 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev,
> void *data,
> if (IS_DGFX(i915))
> return -ENODEV;
>
> + if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
> + return -EOPNOTSUPP;
> +
> switch (args->caching) {
> case I915_CACHING_NONE:
> level = I915_CACHE_NONE;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 37d1efcd3ca6..cad4a6017f4b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -601,7 +601,14 @@ static int shmem_object_init(struct intel_memory_region
> *mem,
> obj->write_domain = I915_GEM_DOMAIN_CPU;
> obj->read_domains = I915_GEM_DOMAIN_CPU;
>
> - if (HAS_LLC(i915))
> + /*
> + * MTL doesn't snoop CPU cache by default for GPU access (namely
> + * 1-way coherency). However some UMD's are currently depending on
> + * that. Make 1-way coherent the default setting for MTL. A follow
> + * up patch will extend the GEM_CREATE uAPI to allow UMD's specify
> + * caching mode at BO creation time
> + */
> + if (HAS_LLC(i915) || (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)))
> /* On some devices, we can have the GPU use the LLC (the CPU
>* cache) for about a 10% performance improvement
>* compared to uncached. Graphics requests other than
> --
> 2.25.1
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
Re: [Intel-gfx] [PATCH 5/8] drm/i915/mtl: end support for set caching ioctl
Hi Fei, On Wed, Apr 19, 2023 at 02:12:16PM -0700, fei.y...@intel.com wrote: > From: Fei Yang > > The design is to keep Buffer Object's caching policy immutable through > out its life cycle. This patch ends the support for set caching ioctl > from MTL onward. While doing that we also set BO's to be 1-way coherent > at creation time because GPU is no longer automatically snooping CPU > cache. For UMD's need to fine tune the caching policy for BO's, a follow > up patch will extend the GEM_CREATE uAPI to allow UMD's specify caching > mode at BO creation time. > > Signed-off-by: Fei Yang Reviewed-by: Andi Shyti Reviewed-by: Andrzej Hajda Andi
RE: [Intel-gfx] [PATCH 5/8] drm/i915/mtl: end support for set caching ioctl
> On 17.04.2023 08:25, fei.y...@intel.com wrote:
>> From: Fei Yang
>>
>> The design is to keep Buffer Object's caching policy immutable through
>> out its life cycle. This patch ends the support for set caching ioctl
>> from MTL onward. While doing that we also set BO's to be 1-way
>> coherent at creation time because GPU is no longer automatically
>> snooping CPU cache.
>>
>> Signed-off-by: Fei Yang
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
>> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 9 -
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>> b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>> index d2d5a24301b2..bb3575b1479f 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
>> @@ -337,6 +337,9 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev,
>> void *data,
>> if (IS_DGFX(i915))
>> return -ENODEV;
>>
>> +if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
>> +return -EOPNOTSUPP;
>> +
>> switch (args->caching) {
>> case I915_CACHING_NONE:
>> level = I915_CACHE_NONE;
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> index 37d1efcd3ca6..e602c323896b 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> @@ -601,7 +601,14 @@ static int shmem_object_init(struct intel_memory_region
>> *mem,
>> obj->write_domain = I915_GEM_DOMAIN_CPU;
>> obj->read_domains = I915_GEM_DOMAIN_CPU;
>>
>> -if (HAS_LLC(i915))
>> +/*
>> + * MTL doesn't snooping CPU cache by default for GPU access (namely
>
> Sounds strange, "doesn't snoop" ?
Will update.
>> + * 1-way coherency). However some UMD's are currently depending on
>> + * that. Make 1-way coherent the default setting for MTL. A follow
>> + * up patch will extend the GEM_CREATE uAPI to allow UMD's specify
>> + * caching mode at BO creation time
>
> Shouldn't such comment be a part of patch description? or at least removed by
> follow-up patch.
Will update.
-Fei
> Reviewed-by: Andrzej Hajda
>
> Regards
> Andrzej
>
>
>> + */
>> +if (HAS_LLC(i915) || (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)))
>> /* On some devices, we can have the GPU use the LLC (the CPU
>> * cache) for about a 10% performance improvement
>> * compared to uncached. Graphics requests other than
Re: [Intel-gfx] [PATCH 5/8] drm/i915/mtl: end support for set caching ioctl
On 17.04.2023 08:25, fei.y...@intel.com wrote:
From: Fei Yang
The design is to keep Buffer Object's caching policy immutable through
out its life cycle. This patch ends the support for set caching ioctl
from MTL onward. While doing that we also set BO's to be 1-way coherent
at creation time because GPU is no longer automatically snooping CPU
cache.
Signed-off-by: Fei Yang
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c | 3 +++
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 9 -
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index d2d5a24301b2..bb3575b1479f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -337,6 +337,9 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void
*data,
if (IS_DGFX(i915))
return -ENODEV;
+ if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
+ return -EOPNOTSUPP;
+
switch (args->caching) {
case I915_CACHING_NONE:
level = I915_CACHE_NONE;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 37d1efcd3ca6..e602c323896b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -601,7 +601,14 @@ static int shmem_object_init(struct intel_memory_region
*mem,
obj->write_domain = I915_GEM_DOMAIN_CPU;
obj->read_domains = I915_GEM_DOMAIN_CPU;
- if (HAS_LLC(i915))
+ /*
+* MTL doesn't snooping CPU cache by default for GPU access (namely
Sounds strange, "doesn't snoop" ?
+* 1-way coherency). However some UMD's are currently depending on
+* that. Make 1-way coherent the default setting for MTL. A follow
+* up patch will extend the GEM_CREATE uAPI to allow UMD's specify
+* caching mode at BO creation time
Shouldn't such comment be a part of patch description? or at least
removed by follow-up patch.
Reviewed-by: Andrzej Hajda
Regards
Andrzej
+*/
+ if (HAS_LLC(i915) || (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)))
/* On some devices, we can have the GPU use the LLC (the CPU
* cache) for about a 10% performance improvement
* compared to uncached. Graphics requests other than
Re: [Intel-gfx] [PATCH 5/8] drm/i915/mtl: end support for set caching ioctl
Hi Fei, On Sun, Apr 16, 2023 at 11:25:00PM -0700, fei.y...@intel.com wrote: > From: Fei Yang > > The design is to keep Buffer Object's caching policy immutable through > out its life cycle. This patch ends the support for set caching ioctl > from MTL onward. While doing that we also set BO's to be 1-way coherent > at creation time because GPU is no longer automatically snooping CPU > cache. > > Signed-off-by: Fei Yang Reviewed-by: Andi Shyti Andi
