| Message ID | 20230509165200.1740-2-fei.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: use pat_index instead of cache_level | expand |
On Tue, 09 May 2023, fei.yang@intel.com wrote: > From: Fei Yang <fei.yang@intel.com> > > This patch is a preparation for replacing enum i915_cache_level with PAT > index. Caching policy for buffer objects is set through the PAT index in > PTE, the old i915_cache_level is not sufficient to represent all caching > modes supported by the hardware. > > Preparing the transition by adding some platform dependent data structures > and helper functions to translate the cache_level to pat_index. > > cachelevel_to_pat: a platform dependent array mapping cache_level to > pat_index. > > max_pat_index: the maximum PAT index recommended in hardware specification > Needed for validating the PAT index passed in from user > space. > > i915_gem_get_pat_index: function to convert cache_level to PAT index. > > obj_to_i915(obj): macro moved to header file for wider usage. > > I915_MAX_CACHE_LEVEL: upper bound of i915_cache_level for the > convenience of coding. > > Cc: Chris Wilson <chris.p.wilson@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Fei Yang <fei.yang@intel.com> > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> > Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> [snip] > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > index f6a7c0bd2955..0eda8b4ee17f 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > @@ -123,7 +123,9 @@ struct drm_i915_private *mock_gem_device(void) > static struct dev_iommu fake_iommu = { .priv = (void *)-1 }; > #endif > struct drm_i915_private *i915; > + struct intel_device_info *i915_info; > struct pci_dev *pdev; > + unsigned int i; > int ret; > > pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); > @@ -180,6 +182,13 @@ struct drm_i915_private *mock_gem_device(void) > I915_GTT_PAGE_SIZE_2M; > > RUNTIME_INFO(i915)->memory_regions = REGION_SMEM; > + > + /* simply use legacy cache level for mock device */ > + i915_info = (struct intel_device_info *)INTEL_INFO(i915); This is not okay. It's not okay to modify device info at runtime. This is why we've separated runtime info from device info. This is why we've made device info const, and localized the modifications to a couple of places. If you need to modify it, it belongs in runtime info. Even if it's only ever modified for mock devices. We were at the brink of being able to finally convert INTEL_INFO() into a pointer to const rodata [1], where you are unable to modify it, but this having been merged as commit 5e352e32aec2 ("drm/i915: preparation for using PAT index") sets us back. (With [1] this oopses trying to modify read-only data.) This has been posted to the public list 20+ times, and nobody noticed or pointed this out?! Throwing away const should be a huge red flag to any developer or reviewer. Hell, *any* cast should be. I've got a patch ready moving cachelevel_to_pat and max_pat_index to runtime info. It's not great, since we'd be doing it only for the mock device. Better ideas? I'm not waiting long. BR, Jani. [1] https://patchwork.freedesktop.org/patch/msgid/0badc36ce6dd6b030507bdfd8a42ab984fb38d12.1686236840.git.jani.nikula@intel.com > + i915_info->max_pat_index = 3; > + for (i = 0; i < I915_MAX_CACHE_LEVEL; i++) > + i915_info->cachelevel_to_pat[i] = i; > + > intel_memory_regions_hw_probe(i915); > > spin_lock_init(&i915->gpu_error.lock);
On 27/06/2023 14:28, Jani Nikula wrote: > On Tue, 09 May 2023, fei.yang@intel.com wrote: >> From: Fei Yang <fei.yang@intel.com> >> >> This patch is a preparation for replacing enum i915_cache_level with PAT >> index. Caching policy for buffer objects is set through the PAT index in >> PTE, the old i915_cache_level is not sufficient to represent all caching >> modes supported by the hardware. >> >> Preparing the transition by adding some platform dependent data structures >> and helper functions to translate the cache_level to pat_index. >> >> cachelevel_to_pat: a platform dependent array mapping cache_level to >> pat_index. >> >> max_pat_index: the maximum PAT index recommended in hardware specification >> Needed for validating the PAT index passed in from user >> space. >> >> i915_gem_get_pat_index: function to convert cache_level to PAT index. >> >> obj_to_i915(obj): macro moved to header file for wider usage. >> >> I915_MAX_CACHE_LEVEL: upper bound of i915_cache_level for the >> convenience of coding. >> >> Cc: Chris Wilson <chris.p.wilson@linux.intel.com> >> Cc: Matt Roper <matthew.d.roper@intel.com> >> Signed-off-by: Fei Yang <fei.yang@intel.com> >> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> >> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> > > [snip] > >> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >> index f6a7c0bd2955..0eda8b4ee17f 100644 >> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c >> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >> @@ -123,7 +123,9 @@ struct drm_i915_private *mock_gem_device(void) >> static struct dev_iommu fake_iommu = { .priv = (void *)-1 }; >> #endif >> struct drm_i915_private *i915; >> + struct intel_device_info *i915_info; >> struct pci_dev *pdev; >> + unsigned int i; >> int ret; >> >> pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); >> @@ -180,6 +182,13 @@ struct drm_i915_private *mock_gem_device(void) >> I915_GTT_PAGE_SIZE_2M; >> >> RUNTIME_INFO(i915)->memory_regions = REGION_SMEM; >> + >> + /* simply use legacy cache level for mock device */ >> + i915_info = (struct intel_device_info *)INTEL_INFO(i915); > > This is not okay. It's not okay to modify device info at runtime. This > is why we've separated runtime info from device info. This is why we've > made device info const, and localized the modifications to a couple of > places. > > If you need to modify it, it belongs in runtime info. Even if it's only > ever modified for mock devices. > > We were at the brink of being able to finally convert INTEL_INFO() into > a pointer to const rodata [1], where you are unable to modify it, but > this having been merged as commit 5e352e32aec2 ("drm/i915: preparation > for using PAT index") sets us back. (With [1] this oopses trying to > modify read-only data.) > > This has been posted to the public list 20+ times, and nobody noticed or > pointed this out?! > > Throwing away const should be a huge red flag to any developer or > reviewer. Hell, *any* cast should be. > > I've got a patch ready moving cachelevel_to_pat and max_pat_index to > runtime info. It's not great, since we'd be doing it only for the mock > device. Better ideas? I'm not waiting long. > > > BR, > Jani. > > > [1] https://patchwork.freedesktop.org/patch/msgid/0badc36ce6dd6b030507bdfd8a42ab984fb38d12.1686236840.git.jani.nikula@intel.com > > >> + i915_info->max_pat_index = 3; >> + for (i = 0; i < I915_MAX_CACHE_LEVEL; i++) >> + i915_info->cachelevel_to_pat[i] = i; >> + I'd simply suggest having a local static const table for the mock device. It should be trivial once i915->__info becomes a pointer so in that series I guess. While I am here - Fei - any plans to work on the promised cleanup? Abstracting the caching modes with a hw agnostic sw/driver representation, if you remember what we discussed. Regards, Tvrtko
Hi Jani and Tvrtko, > > > This patch is a preparation for replacing enum i915_cache_level with PAT > > > index. Caching policy for buffer objects is set through the PAT index in > > > PTE, the old i915_cache_level is not sufficient to represent all caching > > > modes supported by the hardware. > > > > > > Preparing the transition by adding some platform dependent data structures > > > and helper functions to translate the cache_level to pat_index. > > > > > > cachelevel_to_pat: a platform dependent array mapping cache_level to > > > pat_index. > > > > > > max_pat_index: the maximum PAT index recommended in hardware specification > > > Needed for validating the PAT index passed in from user > > > space. > > > > > > i915_gem_get_pat_index: function to convert cache_level to PAT index. > > > > > > obj_to_i915(obj): macro moved to header file for wider usage. > > > > > > I915_MAX_CACHE_LEVEL: upper bound of i915_cache_level for the > > > convenience of coding. > > > > > > Cc: Chris Wilson <chris.p.wilson@linux.intel.com> > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > Signed-off-by: Fei Yang <fei.yang@intel.com> > > > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> > > > Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> > > > > [snip] > > > > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > > > index f6a7c0bd2955..0eda8b4ee17f 100644 > > > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > > > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > > > @@ -123,7 +123,9 @@ struct drm_i915_private *mock_gem_device(void) > > > static struct dev_iommu fake_iommu = { .priv = (void *)-1 }; > > > #endif > > > struct drm_i915_private *i915; > > > + struct intel_device_info *i915_info; > > > struct pci_dev *pdev; > > > + unsigned int i; > > > int ret; > > > pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); > > > @@ -180,6 +182,13 @@ struct drm_i915_private *mock_gem_device(void) > > > I915_GTT_PAGE_SIZE_2M; > > > RUNTIME_INFO(i915)->memory_regions = REGION_SMEM; > > > + > > > + /* simply use legacy cache level for mock device */ > > > + i915_info = (struct intel_device_info *)INTEL_INFO(i915); > > > > This is not okay. It's not okay to modify device info at runtime. This > > is why we've separated runtime info from device info. This is why we've > > made device info const, and localized the modifications to a couple of > > places. > > > > If you need to modify it, it belongs in runtime info. Even if it's only > > ever modified for mock devices. > > > > We were at the brink of being able to finally convert INTEL_INFO() into > > a pointer to const rodata [1], where you are unable to modify it, but > > this having been merged as commit 5e352e32aec2 ("drm/i915: preparation > > for using PAT index") sets us back. (With [1] this oopses trying to > > modify read-only data.) > > > > This has been posted to the public list 20+ times, and nobody noticed or > > pointed this out?! That's not cool, indeed. > > Throwing away const should be a huge red flag to any developer or > > reviewer. Hell, *any* cast should be. > > > > I've got a patch ready moving cachelevel_to_pat and max_pat_index to > > runtime info. It's not great, since we'd be doing it only for the mock > > device. Better ideas? I'm not waiting long. > > > > > > BR, > > Jani. > > > > > > [1] https://patchwork.freedesktop.org/patch/msgid/0badc36ce6dd6b030507bdfd8a42ab984fb38d12.1686236840.git.jani.nikula@intel.com > > > > > > > + i915_info->max_pat_index = 3; > > > + for (i = 0; i < I915_MAX_CACHE_LEVEL; i++) > > > + i915_info->cachelevel_to_pat[i] = i; > > > + > > I'd simply suggest having a local static const table for the mock device. It > should be trivial once i915->__info becomes a pointer so in that series I > guess. Fei... do you have bandwidth to look into this or do you want me to try Tvrtko's suggestion out? Thank you Jani for reporting it and thank you Tvrtko for proposing the fix. Andi
On Tue, 27 Jun 2023, Andi Shyti <andi.shyti@linux.intel.com> wrote: > Tvrtko wrote: >> I'd simply suggest having a local static const table for the mock device. It >> should be trivial once i915->__info becomes a pointer so in that series I >> guess. That's a great idea, thanks! > Fei... do you have bandwidth to look into this or do you want me > to try Tvrtko's suggestion out? https://patchwork.freedesktop.org/patch/msgid/b0db62045a96a3fd4cf123685da88cc777f9b485.1687878757.git.jani.nikula@intel.com BR, Jani.
> Hi Jani and Tvrtko, > >>>> This patch is a preparation for replacing enum i915_cache_level with PAT >>>> index. Caching policy for buffer objects is set through the PAT index in >>>> PTE, the old i915_cache_level is not sufficient to represent all caching >>>> modes supported by the hardware. >>>> >>>> Preparing the transition by adding some platform dependent data structures >>>> and helper functions to translate the cache_level to pat_index. >>>> >>>> cachelevel_to_pat: a platform dependent array mapping cache_level to >>>> pat_index. >>>> >>>> max_pat_index: the maximum PAT index recommended in hardware specification >>>> Needed for validating the PAT index passed in from user >>>> space. >>>> >>>> i915_gem_get_pat_index: function to convert cache_level to PAT index. >>>> >>>> obj_to_i915(obj): macro moved to header file for wider usage. >>>> >>>> I915_MAX_CACHE_LEVEL: upper bound of i915_cache_level for the >>>> convenience of coding. >>>> >>>> Cc: Chris Wilson <chris.p.wilson@linux.intel.com> >>>> Cc: Matt Roper <matthew.d.roper@intel.com> >>>> Signed-off-by: Fei Yang <fei.yang@intel.com> >>>> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> >>>> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> >>> >>> [snip] >>> >>>> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>>> index f6a7c0bd2955..0eda8b4ee17f 100644 >>>> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>>> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>>> @@ -123,7 +123,9 @@ struct drm_i915_private *mock_gem_device(void) >>>> static struct dev_iommu fake_iommu = { .priv = (void *)-1 }; >>>> #endif >>>> struct drm_i915_private *i915; >>>> + struct intel_device_info *i915_info; >>>> struct pci_dev *pdev; >>>> + unsigned int i; >>>> int ret; >>>> pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); >>>> @@ -180,6 +182,13 @@ struct drm_i915_private *mock_gem_device(void) >>>> I915_GTT_PAGE_SIZE_2M; >>>> RUNTIME_INFO(i915)->memory_regions = REGION_SMEM; >>>> + >>>> + /* simply use legacy cache level for mock device */ >>>> + i915_info = (struct intel_device_info *)INTEL_INFO(i915); >>> >>> This is not okay. It's not okay to modify device info at runtime. This >>> is why we've separated runtime info from device info. This is why we've >>> made device info const, and localized the modifications to a couple of >>> places. >>> >>> If you need to modify it, it belongs in runtime info. Even if it's only >>> ever modified for mock devices. >>> >>> We were at the brink of being able to finally convert INTEL_INFO() into >>> a pointer to const rodata [1], where you are unable to modify it, but >>> this having been merged as commit 5e352e32aec2 ("drm/i915: preparation >>> for using PAT index") sets us back. (With [1] this oopses trying to >>> modify read-only data.) >>> >>> This has been posted to the public list 20+ times, and nobody noticed or >>> pointed this out?! > > That's not cool, indeed. > >>> Throwing away const should be a huge red flag to any developer or >>> reviewer. Hell, *any* cast should be. >>> >>> I've got a patch ready moving cachelevel_to_pat and max_pat_index to >>> runtime info. It's not great, since we'd be doing it only for the mock >>> device. Better ideas? I'm not waiting long. >>> >>> >>> BR, >>> Jani. >>> >>> >>> [1] https://patchwork.freedesktop.org/patch/msgid/0badc36ce6dd6b030507bdfd8a42ab984fb38d12.1686236840.git.jani.nikula@intel.com >>> >>> >>>> + i915_info->max_pat_index = 3; >>>> + for (i = 0; i < I915_MAX_CACHE_LEVEL; i++) >>>> + i915_info->cachelevel_to_pat[i] = i; >>>> + >> >> I'd simply suggest having a local static const table for the mock device. It >> should be trivial once i915->__info becomes a pointer so in that series I >> guess. > > Fei... do you have bandwidth to look into this or do you want me > to try Tvrtko's suggestion out? Please go ahead Andi if you have time to help on this. > Thank you Jani for reporting it and thank you Tvrtko for > proposing the fix. Sorry for the trouble here. > Andi
> On 27/06/2023 14:28, Jani Nikula wrote: >> On Tue, 09 May 2023, fei.yang@intel.com wrote: >>> From: Fei Yang <fei.yang@intel.com> >>> >>> This patch is a preparation for replacing enum i915_cache_level with >>> PAT index. Caching policy for buffer objects is set through the PAT >>> index in PTE, the old i915_cache_level is not sufficient to represent >>> all caching modes supported by the hardware. >>> >>> Preparing the transition by adding some platform dependent data >>> structures and helper functions to translate the cache_level to pat_index. >>> >>> cachelevel_to_pat: a platform dependent array mapping cache_level to >>> pat_index. >>> >>> max_pat_index: the maximum PAT index recommended in hardware specification >>> Needed for validating the PAT index passed in from user >>> space. >>> >>> i915_gem_get_pat_index: function to convert cache_level to PAT index. >>> >>> obj_to_i915(obj): macro moved to header file for wider usage. >>> >>> I915_MAX_CACHE_LEVEL: upper bound of i915_cache_level for the >>> convenience of coding. >>> >>> Cc: Chris Wilson <chris.p.wilson@linux.intel.com> >>> Cc: Matt Roper <matthew.d.roper@intel.com> >>> Signed-off-by: Fei Yang <fei.yang@intel.com> >>> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> >>> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> >> >> [snip] >> >>> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>> b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>> index f6a7c0bd2955..0eda8b4ee17f 100644 >>> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>> @@ -123,7 +123,9 @@ struct drm_i915_private *mock_gem_device(void) >>> static struct dev_iommu fake_iommu = { .priv = (void *)-1 }; >>> #endif >>> struct drm_i915_private *i915; >>> + struct intel_device_info *i915_info; >>> struct pci_dev *pdev; >>> + unsigned int i; >>> int ret; >>> >>> pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); @@ -180,6 +182,13 @@ >>> struct drm_i915_private *mock_gem_device(void) >>> I915_GTT_PAGE_SIZE_2M; >>> >>> RUNTIME_INFO(i915)->memory_regions = REGION_SMEM; >>> + >>> + /* simply use legacy cache level for mock device */ >>> + i915_info = (struct intel_device_info *)INTEL_INFO(i915); >> >> This is not okay. It's not okay to modify device info at runtime. This >> is why we've separated runtime info from device info. This is why >> we've made device info const, and localized the modifications to a >> couple of places. >> >> If you need to modify it, it belongs in runtime info. Even if it's >> only ever modified for mock devices. >> >> We were at the brink of being able to finally convert INTEL_INFO() >> into a pointer to const rodata [1], where you are unable to modify it, >> but this having been merged as commit 5e352e32aec2 ("drm/i915: >> preparation for using PAT index") sets us back. (With [1] this oopses >> trying to modify read-only data.) >> >> This has been posted to the public list 20+ times, and nobody noticed >> or pointed this out?! >> >> Throwing away const should be a huge red flag to any developer or >> reviewer. Hell, *any* cast should be. >> >> I've got a patch ready moving cachelevel_to_pat and max_pat_index to >> runtime info. It's not great, since we'd be doing it only for the mock >> device. Better ideas? I'm not waiting long. >> >> >> BR, >> Jani. >> >> >> [1] >> https://patchwork.freedesktop.org/patch/msgid/0badc36ce6dd6b030507bdfd >> 8a42ab984fb38d12.1686236840.git.jani.nikula@intel.com >> >> >>> + i915_info->max_pat_index = 3; >>> + for (i = 0; i < I915_MAX_CACHE_LEVEL; i++) >>> + i915_info->cachelevel_to_pat[i] = i; >>> + > > I'd simply suggest having a local static const table for the mock device. > It should be trivial once i915->__info becomes a pointer so in that series > I guess. > > While I am here - Fei - any plans to work on the promised cleanup? > Abstracting the caching modes with a hw agnostic sw/driver representation, > if you remember what we discussed. Yes, I'm still working on that as a side task. Hopefully I would be able to post something to the mailing list after the July 4th holiday. > Regards, > > Tvrtko
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 4666bb82f312..8c70a0ec7d2f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -45,6 +45,15 @@ static struct kmem_cache *slab_objects; static const struct drm_gem_object_funcs i915_gem_object_funcs; +unsigned int i915_gem_get_pat_index(struct drm_i915_private *i915, + enum i915_cache_level level) +{ + if (drm_WARN_ON(&i915->drm, level >= I915_MAX_CACHE_LEVEL)) + return 0; + + return INTEL_INFO(i915)->cachelevel_to_pat[level]; +} + struct drm_i915_gem_object *i915_gem_object_alloc(void) { struct drm_i915_gem_object *obj; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index bc1291887d4f..284e1aa396cd 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -20,6 +20,8 @@ enum intel_region_id; +#define obj_to_i915(obj__) to_i915((obj__)->base.dev) + static inline bool i915_gem_object_size_2big(u64 size) { struct drm_i915_gem_object *obj; @@ -30,6 +32,8 @@ static inline bool i915_gem_object_size_2big(u64 size) return false; } +unsigned int i915_gem_get_pat_index(struct drm_i915_private *i915, + enum i915_cache_level level); void i915_gem_init__objects(struct drm_i915_private *i915); void i915_objects_module_exit(void); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 830c11431ee8..bf0bd8e11355 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -194,6 +194,13 @@ enum i915_cache_level { * engine. */ I915_CACHE_WT, + /** + * @I915_MAX_CACHE_LEVEL: + * + * Mark the last entry in the enum. Used for defining cachelevel_to_pat + * array for cache_level to pat translation table. + */ + I915_MAX_CACHE_LEVEL, }; enum i915_map_type { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c index b1672e054b21..214763942aa2 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c @@ -460,8 +460,6 @@ void i915_gem_shrinker_taints_mutex(struct drm_i915_private *i915, fs_reclaim_release(GFP_KERNEL); } -#define obj_to_i915(obj__) to_i915((obj__)->base.dev) - /** * i915_gem_object_make_unshrinkable - Hide the object from the shrinker. By * default all object types that support shrinking(see IS_SHRINKABLE), will also diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index 22ec1566d2a7..bb6998d67133 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -78,6 +78,12 @@ static u64 mtl_pte_encode(dma_addr_t addr, case I915_CACHE_WT: pte |= GEN12_PPGTT_PTE_PAT0; break; + default: + /* This should never happen. Added to deal with the compile + * error due to the addition of I915_MAX_CACHE_LEVEL. Will + * be removed by the pat_index patch. + */ + break; } return pte; diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 20915edc8bd9..c8390d03fce2 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -242,6 +242,12 @@ static u64 mtl_ggtt_pte_encode(dma_addr_t addr, case I915_CACHE_WT: pte |= MTL_GGTT_PTE_PAT0; break; + default: + /* This should never happen. Added to deal with the compile + * error due to the addition of I915_MAX_CACHE_LEVEL. Will + * be removed by the pat_index patch. + */ + break; } return pte; diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index c509ea4aa70f..72dd7b9f6dfd 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -30,6 +30,7 @@ #include "display/intel_display_driver.h" #include "gt/intel_gt_regs.h" #include "gt/intel_sa_media.h" +#include "gem/i915_gem_object_types.h" #include "i915_driver.h" #include "i915_drv.h" @@ -164,6 +165,38 @@ .gamma_lut_tests = DRM_COLOR_LUT_NON_DECREASING, \ } +#define LEGACY_CACHELEVEL \ + .cachelevel_to_pat = { \ + [I915_CACHE_NONE] = 0, \ + [I915_CACHE_LLC] = 1, \ + [I915_CACHE_L3_LLC] = 2, \ + [I915_CACHE_WT] = 3, \ + } + +#define TGL_CACHELEVEL \ + .cachelevel_to_pat = { \ + [I915_CACHE_NONE] = 3, \ + [I915_CACHE_LLC] = 0, \ + [I915_CACHE_L3_LLC] = 0, \ + [I915_CACHE_WT] = 2, \ + } + +#define PVC_CACHELEVEL \ + .cachelevel_to_pat = { \ + [I915_CACHE_NONE] = 0, \ + [I915_CACHE_LLC] = 3, \ + [I915_CACHE_L3_LLC] = 3, \ + [I915_CACHE_WT] = 2, \ + } + +#define MTL_CACHELEVEL \ + .cachelevel_to_pat = { \ + [I915_CACHE_NONE] = 2, \ + [I915_CACHE_LLC] = 3, \ + [I915_CACHE_L3_LLC] = 3, \ + [I915_CACHE_WT] = 1, \ + } + /* Keep in gen based order, and chronological order within a gen */ #define GEN_DEFAULT_PAGE_SIZES \ @@ -189,11 +222,13 @@ .has_snoop = true, \ .has_coherent_ggtt = false, \ .dma_mask_size = 32, \ + .max_pat_index = 3, \ I9XX_PIPE_OFFSETS, \ I9XX_CURSOR_OFFSETS, \ I9XX_COLORS, \ GEN_DEFAULT_PAGE_SIZES, \ - GEN_DEFAULT_REGIONS + GEN_DEFAULT_REGIONS, \ + LEGACY_CACHELEVEL #define I845_FEATURES \ GEN(2), \ @@ -210,11 +245,13 @@ .has_snoop = true, \ .has_coherent_ggtt = false, \ .dma_mask_size = 32, \ + .max_pat_index = 3, \ I845_PIPE_OFFSETS, \ I845_CURSOR_OFFSETS, \ I845_COLORS, \ GEN_DEFAULT_PAGE_SIZES, \ - GEN_DEFAULT_REGIONS + GEN_DEFAULT_REGIONS, \ + LEGACY_CACHELEVEL static const struct intel_device_info i830_info = { I830_FEATURES, @@ -249,11 +286,13 @@ static const struct intel_device_info i865g_info = { .has_snoop = true, \ .has_coherent_ggtt = true, \ .dma_mask_size = 32, \ + .max_pat_index = 3, \ I9XX_PIPE_OFFSETS, \ I9XX_CURSOR_OFFSETS, \ I9XX_COLORS, \ GEN_DEFAULT_PAGE_SIZES, \ - GEN_DEFAULT_REGIONS + GEN_DEFAULT_REGIONS, \ + LEGACY_CACHELEVEL static const struct intel_device_info i915g_info = { GEN3_FEATURES, @@ -341,11 +380,13 @@ static const struct intel_device_info pnv_m_info = { .has_snoop = true, \ .has_coherent_ggtt = true, \ .dma_mask_size = 36, \ + .max_pat_index = 3, \ I9XX_PIPE_OFFSETS, \ I9XX_CURSOR_OFFSETS, \ I9XX_COLORS, \ GEN_DEFAULT_PAGE_SIZES, \ - GEN_DEFAULT_REGIONS + GEN_DEFAULT_REGIONS, \ + LEGACY_CACHELEVEL static const struct intel_device_info i965g_info = { GEN4_FEATURES, @@ -395,11 +436,13 @@ static const struct intel_device_info gm45_info = { /* ilk does support rc6, but we do not implement [power] contexts */ \ .has_rc6 = 0, \ .dma_mask_size = 36, \ + .max_pat_index = 3, \ I9XX_PIPE_OFFSETS, \ I9XX_CURSOR_OFFSETS, \ ILK_COLORS, \ GEN_DEFAULT_PAGE_SIZES, \ - GEN_DEFAULT_REGIONS + GEN_DEFAULT_REGIONS, \ + LEGACY_CACHELEVEL static const struct intel_device_info ilk_d_info = { GEN5_FEATURES, @@ -429,13 +472,15 @@ static const struct intel_device_info ilk_m_info = { .has_rc6p = 0, \ .has_rps = true, \ .dma_mask_size = 40, \ + .max_pat_index = 3, \ .__runtime.ppgtt_type = INTEL_PPGTT_ALIASING, \ .__runtime.ppgtt_size = 31, \ I9XX_PIPE_OFFSETS, \ I9XX_CURSOR_OFFSETS, \ ILK_COLORS, \ GEN_DEFAULT_PAGE_SIZES, \ - GEN_DEFAULT_REGIONS + GEN_DEFAULT_REGIONS, \ + LEGACY_CACHELEVEL #define SNB_D_PLATFORM \ GEN6_FEATURES, \ @@ -482,13 +527,15 @@ static const struct intel_device_info snb_m_gt2_info = { .has_reset_engine = true, \ .has_rps = true, \ .dma_mask_size = 40, \ + .max_pat_index = 3, \ .__runtime.ppgtt_type = INTEL_PPGTT_ALIASING, \ .__runtime.ppgtt_size = 31, \ IVB_PIPE_OFFSETS, \ IVB_CURSOR_OFFSETS, \ IVB_COLORS, \ GEN_DEFAULT_PAGE_SIZES, \ - GEN_DEFAULT_REGIONS + GEN_DEFAULT_REGIONS, \ + LEGACY_CACHELEVEL #define IVB_D_PLATFORM \ GEN7_FEATURES, \ @@ -542,6 +589,7 @@ static const struct intel_device_info vlv_info = { .display.has_gmch = 1, .display.has_hotplug = 1, .dma_mask_size = 40, + .max_pat_index = 3, .__runtime.ppgtt_type = INTEL_PPGTT_ALIASING, .__runtime.ppgtt_size = 31, .has_snoop = true, @@ -553,6 +601,7 @@ static const struct intel_device_info vlv_info = { I9XX_COLORS, GEN_DEFAULT_PAGE_SIZES, GEN_DEFAULT_REGIONS, + LEGACY_CACHELEVEL, }; #define G75_FEATURES \ @@ -640,6 +689,7 @@ static const struct intel_device_info chv_info = { .has_logical_ring_contexts = 1, .display.has_gmch = 1, .dma_mask_size = 39, + .max_pat_index = 3, .__runtime.ppgtt_type = INTEL_PPGTT_FULL, .__runtime.ppgtt_size = 32, .has_reset_engine = 1, @@ -651,6 +701,7 @@ static const struct intel_device_info chv_info = { CHV_COLORS, GEN_DEFAULT_PAGE_SIZES, GEN_DEFAULT_REGIONS, + LEGACY_CACHELEVEL, }; #define GEN9_DEFAULT_PAGE_SIZES \ @@ -732,11 +783,13 @@ static const struct intel_device_info skl_gt4_info = { .has_snoop = true, \ .has_coherent_ggtt = false, \ .display.has_ipc = 1, \ + .max_pat_index = 3, \ HSW_PIPE_OFFSETS, \ IVB_CURSOR_OFFSETS, \ IVB_COLORS, \ GEN9_DEFAULT_PAGE_SIZES, \ - GEN_DEFAULT_REGIONS + GEN_DEFAULT_REGIONS, \ + LEGACY_CACHELEVEL static const struct intel_device_info bxt_info = { GEN9_LP_FEATURES, @@ -890,9 +943,11 @@ static const struct intel_device_info jsl_info = { [TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET, \ }, \ TGL_CURSOR_OFFSETS, \ + TGL_CACHELEVEL, \ .has_global_mocs = 1, \ .has_pxp = 1, \ - .display.has_dsb = 1 + .display.has_dsb = 1, \ + .max_pat_index = 3 static const struct intel_device_info tgl_info = { GEN12_FEATURES, @@ -1014,6 +1069,7 @@ static const struct intel_device_info adl_p_info = { .__runtime.graphics.ip.ver = 12, \ .__runtime.graphics.ip.rel = 50, \ XE_HP_PAGE_SIZES, \ + TGL_CACHELEVEL, \ .dma_mask_size = 46, \ .has_3d_pipeline = 1, \ .has_64bit_reloc = 1, \ @@ -1032,6 +1088,7 @@ static const struct intel_device_info adl_p_info = { .has_reset_engine = 1, \ .has_rps = 1, \ .has_runtime_pm = 1, \ + .max_pat_index = 3, \ .__runtime.ppgtt_size = 48, \ .__runtime.ppgtt_type = INTEL_PPGTT_FULL @@ -1108,11 +1165,13 @@ static const struct intel_device_info pvc_info = { PLATFORM(INTEL_PONTEVECCHIO), NO_DISPLAY, .has_flat_ccs = 0, + .max_pat_index = 7, .__runtime.platform_engine_mask = BIT(BCS0) | BIT(VCS0) | BIT(CCS0) | BIT(CCS1) | BIT(CCS2) | BIT(CCS3), .require_force_probe = 1, + PVC_CACHELEVEL, }; #define XE_LPDP_FEATURES \ @@ -1152,9 +1211,11 @@ static const struct intel_device_info mtl_info = { .has_llc = 0, .has_mslice_steering = 0, .has_snoop = 1, + .max_pat_index = 4, .__runtime.memory_regions = REGION_SMEM | REGION_STOLEN_LMEM, .__runtime.platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0), .require_force_probe = 1, + MTL_CACHELEVEL, }; #undef PLATFORM diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index f032f2500f50..959a4080840c 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -35,6 +35,8 @@ #include "gt/intel_context_types.h" #include "gt/intel_sseu.h" +#include "gem/i915_gem_object_types.h" + struct drm_printer; struct drm_i915_private; struct intel_gt_definition; @@ -308,6 +310,9 @@ struct intel_device_info { * Initial runtime info. Do not access outside of i915_driver_create(). */ const struct intel_runtime_info __runtime; + + u32 cachelevel_to_pat[I915_MAX_CACHE_LEVEL]; + u32 max_pat_index; }; struct intel_driver_caps { diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index f6a7c0bd2955..0eda8b4ee17f 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -123,7 +123,9 @@ struct drm_i915_private *mock_gem_device(void) static struct dev_iommu fake_iommu = { .priv = (void *)-1 }; #endif struct drm_i915_private *i915; + struct intel_device_info *i915_info; struct pci_dev *pdev; + unsigned int i; int ret; pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); @@ -180,6 +182,13 @@ struct drm_i915_private *mock_gem_device(void) I915_GTT_PAGE_SIZE_2M; RUNTIME_INFO(i915)->memory_regions = REGION_SMEM; + + /* simply use legacy cache level for mock device */ + i915_info = (struct intel_device_info *)INTEL_INFO(i915); + i915_info->max_pat_index = 3; + for (i = 0; i < I915_MAX_CACHE_LEVEL; i++) + i915_info->cachelevel_to_pat[i] = i; + intel_memory_regions_hw_probe(i915); spin_lock_init(&i915->gpu_error.lock);