mbox series

[v3,0/5] drm/i915: Allow user to set cache at BO creation

Message ID 20230428054737.1765778-1-fei.yang@intel.com (mailing list archive)
Headers show
Series drm/i915: Allow user to set cache at BO creation | expand

Message

Yang, Fei April 28, 2023, 5:47 a.m. UTC
From: Fei Yang <fei.yang@intel.com>

The first three patches in this series are taken from
https://patchwork.freedesktop.org/series/116868/
These patches are included here because the last patch
has dependency on the pat_index refactor.

This series is focusing on uAPI changes,
1. end support for set caching ioctl [PATCH 4/5]
2. add set_pat extension for gem_create [PATCH 5/5]

v2: drop one patch that was merged separately
    341ad0e8e254 drm/i915/mtl: Add PTE encode function
v3: rebase on https://patchwork.freedesktop.org/series/117082/

Fei Yang (5):
  drm/i915: preparation for using PAT index
  drm/i915: use pat_index instead of cache_level
  drm/i915: make sure correct pte encode is used
  drm/i915/mtl: end support for set caching ioctl
  drm/i915: Allow user to set cache at BO creation

 drivers/gpu/drm/i915/display/intel_dpt.c      | 12 +--
 drivers/gpu/drm/i915/gem/i915_gem_create.c    | 36 +++++++++
 drivers/gpu/drm/i915/gem/i915_gem_domain.c    | 46 ++++++-----
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 10 ++-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      |  3 +-
 drivers/gpu/drm/i915/gem/i915_gem_object.c    | 67 +++++++++++++++-
 drivers/gpu/drm/i915/gem/i915_gem_object.h    |  8 ++
 .../gpu/drm/i915/gem/i915_gem_object_types.h  | 26 +++++-
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c     |  9 ++-
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  |  2 -
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c    |  4 +-
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c  | 16 ++--
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
 .../drm/i915/gem/selftests/i915_gem_migrate.c |  2 +-
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  2 +-
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c          | 10 ++-
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c          | 73 +++++++++--------
 drivers/gpu/drm/i915/gt/gen8_ppgtt.h          |  3 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c          | 76 +++++++++---------
 drivers/gpu/drm/i915/gt/intel_gtt.h           | 20 +++--
 drivers/gpu/drm/i915/gt/intel_migrate.c       | 47 ++++++-----
 drivers/gpu/drm/i915/gt/intel_migrate.h       | 13 ++-
 drivers/gpu/drm/i915/gt/intel_ppgtt.c         |  6 +-
 drivers/gpu/drm/i915/gt/selftest_migrate.c    | 47 +++++------
 drivers/gpu/drm/i915/gt/selftest_reset.c      |  8 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |  2 +-
 drivers/gpu/drm/i915/gt/selftest_tlb.c        |  4 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      | 10 ++-
 drivers/gpu/drm/i915/i915_debugfs.c           | 55 ++++++++++---
 drivers/gpu/drm/i915/i915_gem.c               | 16 +++-
 drivers/gpu/drm/i915/i915_gpu_error.c         |  8 +-
 drivers/gpu/drm/i915/i915_pci.c               | 79 ++++++++++++++++---
 drivers/gpu/drm/i915/i915_vma.c               | 16 ++--
 drivers/gpu/drm/i915/i915_vma.h               |  2 +-
 drivers/gpu/drm/i915/i915_vma_types.h         |  2 -
 drivers/gpu/drm/i915/intel_device_info.h      |  5 ++
 drivers/gpu/drm/i915/selftests/i915_gem.c     |  5 +-
 .../gpu/drm/i915/selftests/i915_gem_evict.c   |  4 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 15 ++--
 .../drm/i915/selftests/intel_memory_region.c  |  4 +-
 .../gpu/drm/i915/selftests/mock_gem_device.c  |  9 +++
 drivers/gpu/drm/i915/selftests/mock_gtt.c     |  8 +-
 include/uapi/drm/i915_drm.h                   | 36 +++++++++
 tools/include/uapi/drm/i915_drm.h             | 36 +++++++++
 44 files changed, 621 insertions(+), 243 deletions(-)

Comments

Thomas Hellström (Intel) April 28, 2023, 8:06 a.m. UTC | #1
On 4/28/23 07:47, fei.yang@intel.com wrote:
> From: Fei Yang <fei.yang@intel.com>
>
> The first three patches in this series are taken from
> https://patchwork.freedesktop.org/series/116868/
> These patches are included here because the last patch
> has dependency on the pat_index refactor.
>
> This series is focusing on uAPI changes,
> 1. end support for set caching ioctl [PATCH 4/5]
> 2. add set_pat extension for gem_create [PATCH 5/5]
>
> v2: drop one patch that was merged separately
>      341ad0e8e254 drm/i915/mtl: Add PTE encode function
> v3: rebase on https://patchwork.freedesktop.org/series/117082/

Hi, Fei.

Does this uAPI update also affect any discrete GPUs supported by i915, 
And in that case, does it allow setting non-snooping PAT indices on 
those devices?

If so, since the uAPI for discrete GPU devices doesn't allow incoherency 
between GPU and CPU (apart from write-combining buffering), the correct 
CPU caching mode matching the PAT index needs to be selected for the 
buffer object in i915_ttm_select_tt_caching().

Thanks,
Thomas

>
> Fei Yang (5):
>    drm/i915: preparation for using PAT index
>    drm/i915: use pat_index instead of cache_level
>    drm/i915: make sure correct pte encode is used
>    drm/i915/mtl: end support for set caching ioctl
>    drm/i915: Allow user to set cache at BO creation
>
>   drivers/gpu/drm/i915/display/intel_dpt.c      | 12 +--
>   drivers/gpu/drm/i915/gem/i915_gem_create.c    | 36 +++++++++
>   drivers/gpu/drm/i915/gem/i915_gem_domain.c    | 46 ++++++-----
>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 10 ++-
>   drivers/gpu/drm/i915/gem/i915_gem_mman.c      |  3 +-
>   drivers/gpu/drm/i915/gem/i915_gem_object.c    | 67 +++++++++++++++-
>   drivers/gpu/drm/i915/gem/i915_gem_object.h    |  8 ++
>   .../gpu/drm/i915/gem/i915_gem_object_types.h  | 26 +++++-
>   drivers/gpu/drm/i915/gem/i915_gem_shmem.c     |  9 ++-
>   drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  |  2 -
>   drivers/gpu/drm/i915/gem/i915_gem_stolen.c    |  4 +-
>   drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c  | 16 ++--
>   .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
>   .../drm/i915/gem/selftests/i915_gem_migrate.c |  2 +-
>   .../drm/i915/gem/selftests/i915_gem_mman.c    |  2 +-
>   drivers/gpu/drm/i915/gt/gen6_ppgtt.c          | 10 ++-
>   drivers/gpu/drm/i915/gt/gen8_ppgtt.c          | 73 +++++++++--------
>   drivers/gpu/drm/i915/gt/gen8_ppgtt.h          |  3 +-
>   drivers/gpu/drm/i915/gt/intel_ggtt.c          | 76 +++++++++---------
>   drivers/gpu/drm/i915/gt/intel_gtt.h           | 20 +++--
>   drivers/gpu/drm/i915/gt/intel_migrate.c       | 47 ++++++-----
>   drivers/gpu/drm/i915/gt/intel_migrate.h       | 13 ++-
>   drivers/gpu/drm/i915/gt/intel_ppgtt.c         |  6 +-
>   drivers/gpu/drm/i915/gt/selftest_migrate.c    | 47 +++++------
>   drivers/gpu/drm/i915/gt/selftest_reset.c      |  8 +-
>   drivers/gpu/drm/i915/gt/selftest_timeline.c   |  2 +-
>   drivers/gpu/drm/i915/gt/selftest_tlb.c        |  4 +-
>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      | 10 ++-
>   drivers/gpu/drm/i915/i915_debugfs.c           | 55 ++++++++++---
>   drivers/gpu/drm/i915/i915_gem.c               | 16 +++-
>   drivers/gpu/drm/i915/i915_gpu_error.c         |  8 +-
>   drivers/gpu/drm/i915/i915_pci.c               | 79 ++++++++++++++++---
>   drivers/gpu/drm/i915/i915_vma.c               | 16 ++--
>   drivers/gpu/drm/i915/i915_vma.h               |  2 +-
>   drivers/gpu/drm/i915/i915_vma_types.h         |  2 -
>   drivers/gpu/drm/i915/intel_device_info.h      |  5 ++
>   drivers/gpu/drm/i915/selftests/i915_gem.c     |  5 +-
>   .../gpu/drm/i915/selftests/i915_gem_evict.c   |  4 +-
>   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 15 ++--
>   .../drm/i915/selftests/intel_memory_region.c  |  4 +-
>   .../gpu/drm/i915/selftests/mock_gem_device.c  |  9 +++
>   drivers/gpu/drm/i915/selftests/mock_gtt.c     |  8 +-
>   include/uapi/drm/i915_drm.h                   | 36 +++++++++
>   tools/include/uapi/drm/i915_drm.h             | 36 +++++++++
>   44 files changed, 621 insertions(+), 243 deletions(-)
>
Yang, Fei April 28, 2023, 3:19 p.m. UTC | #2
> On 4/28/23 07:47, fei.yang@intel.com wrote:
>> From: Fei Yang <fei.yang@intel.com>
>>
>> The first three patches in this series are taken from
>> https://patchwork.freedesktop.org/series/116868/
>> These patches are included here because the last patch
>> has dependency on the pat_index refactor.
>>
>> This series is focusing on uAPI changes,
>> 1. end support for set caching ioctl [PATCH 4/5]
>> 2. add set_pat extension for gem_create [PATCH 5/5]
>>
>> v2: drop one patch that was merged separately
>>      341ad0e8e254 drm/i915/mtl: Add PTE encode function
>> v3: rebase on https://patchwork.freedesktop.org/series/117082/
>
> Hi, Fei.
>
> Does this uAPI update also affect any discrete GPUs supported by i915,

It does.

> And in that case, does it allow setting non-snooping PAT indices on
> those devices?

It allows setting PAT indices specified in https://gfxspecs.intel.com/Predator/Home/Index/45101 .
KMD does a sanity check so that it won't go over the max recommended
by bspec.

> If so, since the uAPI for discrete GPU devices doesn't allow incoherency
> between GPU and CPU (apart from write-combining buffering), the correct
> CPU caching mode matching the PAT index needs to be selected for the
> buffer object in i915_ttm_select_tt_caching().

The patch doesn't affect the CPU caching mode setting logic though.
And the caching settings for objects created by kernel should remain
the same for both CPU and GPU, objects created by userspace are
managed completely by userspace.

One question though, what do you mean by non-snooping PAT indices?
The PAT index registers don't really control coherency mode in the past,
I believe MTL is the first one that has COH_MODE in the PAT registers.
Aren't discrete GPUs snooping CPU cache automatically?

-Fei

> Thanks,
> Thomas
>
>>
>> Fei Yang (5):
>>    drm/i915: preparation for using PAT index
>>    drm/i915: use pat_index instead of cache_level
>>    drm/i915: make sure correct pte encode is used
>>    drm/i915/mtl: end support for set caching ioctl
>>    drm/i915: Allow user to set cache at BO creation
>>
>>   drivers/gpu/drm/i915/display/intel_dpt.c      | 12 +--
>>   drivers/gpu/drm/i915/gem/i915_gem_create.c    | 36 +++++++++
>>   drivers/gpu/drm/i915/gem/i915_gem_domain.c    | 46 ++++++-----
>>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 10 ++-
>>   drivers/gpu/drm/i915/gem/i915_gem_mman.c      |  3 +-
>>   drivers/gpu/drm/i915/gem/i915_gem_object.c    | 67 +++++++++++++++-
>>   drivers/gpu/drm/i915/gem/i915_gem_object.h    |  8 ++
>>   .../gpu/drm/i915/gem/i915_gem_object_types.h  | 26 +++++-
>>   drivers/gpu/drm/i915/gem/i915_gem_shmem.c     |  9 ++-
>>   drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  |  2 -
>>   drivers/gpu/drm/i915/gem/i915_gem_stolen.c    |  4 +-
>>   drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c  | 16 ++--
>>   .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
>>   .../drm/i915/gem/selftests/i915_gem_migrate.c |  2 +-
>>   .../drm/i915/gem/selftests/i915_gem_mman.c    |  2 +-
>>   drivers/gpu/drm/i915/gt/gen6_ppgtt.c          | 10 ++-
>>   drivers/gpu/drm/i915/gt/gen8_ppgtt.c          | 73 +++++++++--------
>>   drivers/gpu/drm/i915/gt/gen8_ppgtt.h          |  3 +-
>>   drivers/gpu/drm/i915/gt/intel_ggtt.c          | 76 +++++++++---------
>>   drivers/gpu/drm/i915/gt/intel_gtt.h           | 20 +++--
>>   drivers/gpu/drm/i915/gt/intel_migrate.c       | 47 ++++++-----
>>   drivers/gpu/drm/i915/gt/intel_migrate.h       | 13 ++-
>>   drivers/gpu/drm/i915/gt/intel_ppgtt.c         |  6 +-
>>   drivers/gpu/drm/i915/gt/selftest_migrate.c    | 47 +++++------
>>   drivers/gpu/drm/i915/gt/selftest_reset.c      |  8 +-
>>   drivers/gpu/drm/i915/gt/selftest_timeline.c   |  2 +-
>>   drivers/gpu/drm/i915/gt/selftest_tlb.c        |  4 +-
>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      | 10 ++-
>>   drivers/gpu/drm/i915/i915_debugfs.c           | 55 ++++++++++---
>>   drivers/gpu/drm/i915/i915_gem.c               | 16 +++-
>>   drivers/gpu/drm/i915/i915_gpu_error.c         |  8 +-
>>   drivers/gpu/drm/i915/i915_pci.c               | 79 ++++++++++++++++---
>>   drivers/gpu/drm/i915/i915_vma.c               | 16 ++--
>>   drivers/gpu/drm/i915/i915_vma.h               |  2 +-
>>   drivers/gpu/drm/i915/i915_vma_types.h         |  2 -
>>   drivers/gpu/drm/i915/intel_device_info.h      |  5 ++
>>   drivers/gpu/drm/i915/selftests/i915_gem.c     |  5 +-
>>   .../gpu/drm/i915/selftests/i915_gem_evict.c   |  4 +-
>>   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 15 ++--
>>   .../drm/i915/selftests/intel_memory_region.c  |  4 +-
>>   .../gpu/drm/i915/selftests/mock_gem_device.c  |  9 +++
>>   drivers/gpu/drm/i915/selftests/mock_gtt.c     |  8 +-
>>   include/uapi/drm/i915_drm.h                   | 36 +++++++++
>>   tools/include/uapi/drm/i915_drm.h             | 36 +++++++++
>>   44 files changed, 621 insertions(+), 243 deletions(-)
>>
Thomas Hellström (Intel) April 28, 2023, 4 p.m. UTC | #3
On 4/28/23 17:19, Yang, Fei wrote:
> > On 4/28/23 07:47, fei.yang@intel.com wrote:
> >> From: Fei Yang <fei.yang@intel.com>
> >>
> >> The first three patches in this series are taken from
> >> https://patchwork.freedesktop.org/series/116868/
> >> These patches are included here because the last patch
> >> has dependency on the pat_index refactor.
> >>
> >> This series is focusing on uAPI changes,
> >> 1. end support for set caching ioctl [PATCH 4/5]
> >> 2. add set_pat extension for gem_create [PATCH 5/5]
> >>
> >> v2: drop one patch that was merged separately
> >>      341ad0e8e254 drm/i915/mtl: Add PTE encode function
> >> v3: rebase on https://patchwork.freedesktop.org/series/117082/
> >
> > Hi, Fei.
> >
> > Does this uAPI update also affect any discrete GPUs supported by i915,
>
> It does.
>
> > And in that case, does it allow setting non-snooping PAT indices on
> > those devices?
>
> It allows setting PAT indices specified in
> KMD does a sanity check so that it won't go over the max recommended
> by bspec.
>
> > If so, since the uAPI for discrete GPU devices doesn't allow incoherency
> > between GPU and CPU (apart from write-combining buffering), the correct
> > CPU caching mode matching the PAT index needs to be selected for the
> > buffer object in i915_ttm_select_tt_caching().
>
> The patch doesn't affect the CPU caching mode setting logic though.
> And the caching settings for objects created by kernel should remain
> the same for both CPU and GPU, objects created by userspace are
> managed completely by userspace.
>
> One question though, what do you mean by non-snooping PAT indices?
> The PAT index registers don't really control coherency mode in the past,
> I believe MTL is the first one that has COH_MODE in the PAT registers.
> Aren't discrete GPUs snooping CPU cache automatically?

Yes, that was actually the bottom question: What do these PAT settings 
allow you to do WRT the snooping on supported discrete devices (DG2) on 
i915?

If they indeed don't allow disabling snooping, then that's not a 
problem. If they do, the ttm code there needs some modification.


Thanks,

Thomas



>
> -Fei
>
> > Thanks,
> > Thomas
> >
> >>
> >> Fei Yang (5):
> >>    drm/i915: preparation for using PAT index
> >>    drm/i915: use pat_index instead of cache_level
> >>    drm/i915: make sure correct pte encode is used
> >>    drm/i915/mtl: end support for set caching ioctl
> >>    drm/i915: Allow user to set cache at BO creation
> >>
> >> drivers/gpu/drm/i915/display/intel_dpt.c      | 12 +--
> >> drivers/gpu/drm/i915/gem/i915_gem_create.c    | 36 +++++++++
> >> drivers/gpu/drm/i915/gem/i915_gem_domain.c    | 46 ++++++-----
> >> .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 10 ++-
> >> drivers/gpu/drm/i915/gem/i915_gem_mman.c      |  3 +-
> >> drivers/gpu/drm/i915/gem/i915_gem_object.c    | 67 +++++++++++++++-
> >> drivers/gpu/drm/i915/gem/i915_gem_object.h    |  8 ++
> >> .../gpu/drm/i915/gem/i915_gem_object_types.h  | 26 +++++-
> >> drivers/gpu/drm/i915/gem/i915_gem_shmem.c     |  9 ++-
> >> drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  |  2 -
> >> drivers/gpu/drm/i915/gem/i915_gem_stolen.c    |  4 +-
> >> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c  | 16 ++--
> >> .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
> >> .../drm/i915/gem/selftests/i915_gem_migrate.c |  2 +-
> >> .../drm/i915/gem/selftests/i915_gem_mman.c    |  2 +-
> >> drivers/gpu/drm/i915/gt/gen6_ppgtt.c          | 10 ++-
> >> drivers/gpu/drm/i915/gt/gen8_ppgtt.c          | 73 +++++++++--------
> >> drivers/gpu/drm/i915/gt/gen8_ppgtt.h          |  3 +-
> >> drivers/gpu/drm/i915/gt/intel_ggtt.c          | 76 +++++++++---------
> >> drivers/gpu/drm/i915/gt/intel_gtt.h           | 20 +++--
> >>   drivers/gpu/drm/i915/gt/intel_migrate.c       | 47 ++++++-----
> >> drivers/gpu/drm/i915/gt/intel_migrate.h       | 13 ++-
> >> drivers/gpu/drm/i915/gt/intel_ppgtt.c         |  6 +-
> >> drivers/gpu/drm/i915/gt/selftest_migrate.c    | 47 +++++------
> >> drivers/gpu/drm/i915/gt/selftest_reset.c      |  8 +-
> >> drivers/gpu/drm/i915/gt/selftest_timeline.c   |  2 +-
> >> drivers/gpu/drm/i915/gt/selftest_tlb.c        |  4 +-
> >> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      | 10 ++-
> >> drivers/gpu/drm/i915/i915_debugfs.c           | 55 ++++++++++---
> >> drivers/gpu/drm/i915/i915_gem.c               | 16 +++-
> >> drivers/gpu/drm/i915/i915_gpu_error.c         |  8 +-
> >> drivers/gpu/drm/i915/i915_pci.c               | 79 ++++++++++++++++---
> >> drivers/gpu/drm/i915/i915_vma.c               | 16 ++--
> >> drivers/gpu/drm/i915/i915_vma.h               |  2 +-
> >> drivers/gpu/drm/i915/i915_vma_types.h         |  2 -
> >> drivers/gpu/drm/i915/intel_device_info.h      |  5 ++
> >> drivers/gpu/drm/i915/selftests/i915_gem.c     |  5 +-
> >> .../gpu/drm/i915/selftests/i915_gem_evict.c   |  4 +-
> >> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 15 ++--
> >> .../drm/i915/selftests/intel_memory_region.c  |  4 +-
> >> .../gpu/drm/i915/selftests/mock_gem_device.c  |  9 +++
> >> drivers/gpu/drm/i915/selftests/mock_gtt.c     |  8 +-
> >> include/uapi/drm/i915_drm.h                   | 36 +++++++++
> >> tools/include/uapi/drm/i915_drm.h             | 36 +++++++++
> >>   44 files changed, 621 insertions(+), 243 deletions(-)
> >>
>
Yang, Fei April 28, 2023, 5:43 p.m. UTC | #4
>> On 4/28/23 17:19, Yang, Fei wrote:
>>> On 4/28/23 07:47, fei.yang@intel.com wrote:
>>>> From: Fei Yang <fei.yang@intel.com>
>>>>
>>>> The first three patches in this series are taken from
>>>> https://patchwork.freedesktop.org/series/116868/
>>>> These patches are included here because the last patch
>>>> has dependency on the pat_index refactor.
>>>>
>>>> This series is focusing on uAPI changes,
>>>> 1. end support for set caching ioctl [PATCH 4/5]
>>>> 2. add set_pat extension for gem_create [PATCH 5/5]
>>>>
>>>> v2: drop one patch that was merged separately
>>>>      341ad0e8e254 drm/i915/mtl: Add PTE encode function
>>>> v3: rebase on https://patchwork.freedesktop.org/series/117082/
>>>
>>> Hi, Fei.
>>>
>>> Does this uAPI update also affect any discrete GPUs supported by i915,
>>
>> It does.
>>
>>> And in that case, does it allow setting non-snooping PAT indices on
>>> those devices?
>>
>> It allows setting PAT indices specified in
>> KMD does a sanity check so that it won't go over the max recommended
>> by bspec.
>>
>>> If so, since the uAPI for discrete GPU devices doesn't allow incoherency
>>> between GPU and CPU (apart from write-combining buffering), the correct
>>> CPU caching mode matching the PAT index needs to be selected for the
>>> buffer object in i915_ttm_select_tt_caching().
>>
>> The patch doesn't affect the CPU caching mode setting logic though.
>> And the caching settings for objects created by kernel should remain
>> the same for both CPU and GPU, objects created by userspace are
>> managed completely by userspace.
>>
>> One question though, what do you mean by non-snooping PAT indices?
>
> Yes, that was actually the bottom question: What do these PAT settings
> allow you to do WRT the snooping on supported discrete devices (DG2) on
> i915?
> If they indeed don't allow disabling snooping, then that's not a problem.

When dGPU's access SysMem, the PCIe default is for that access to snoop the
host's caches. All of our current dGPU's do that -- independent of PAT setting.

> If they do, the ttm code there needs some modification.

I'm not familiar with ttm, but if your concern is that certain PAT index
could disable snooping, that is not possible for current dGPU's.
I think it is possible for Xe2/3 though, because there will be COH_MODE
defined in the PAT registers going forward.

-Fei
Thomas Hellström (Intel) April 29, 2023, 8:55 a.m. UTC | #5
On 4/28/23 19:43, Yang, Fei wrote:
> >> On 4/28/23 17:19, Yang, Fei wrote:
> >>> On 4/28/23 07:47, fei.yang@intel.com wrote:
> >>>> From: Fei Yang <fei.yang@intel.com>
> >>>>
> >>>> The first three patches in this series are taken from
> >>>> https://patchwork.freedesktop.org/series/116868/
> >>>> These patches are included here because the last patch
> >>>> has dependency on the pat_index refactor.
> >>>>
> >>>> This series is focusing on uAPI changes,
> >>>> 1. end support for set caching ioctl [PATCH 4/5]
> >>>> 2. add set_pat extension for gem_create [PATCH 5/5]
> >>>>
> >>>> v2: drop one patch that was merged separately
> >>>>      341ad0e8e254 drm/i915/mtl: Add PTE encode function
> >>>> v3: rebase on https://patchwork.freedesktop.org/series/117082/
> >>>
> >>> Hi, Fei.
> >>>
> >>> Does this uAPI update also affect any discrete GPUs supported by i915,
> >>
> >> It does.
> >>
> >>> And in that case, does it allow setting non-snooping PAT indices on
> >>> those devices?
> >>
> >> It allows setting PAT indices specified in
> >> KMD does a sanity check so that it won't go over the max recommended
> >> by bspec.
> >>
> >>> If so, since the uAPI for discrete GPU devices doesn't allow 
> incoherency
> >>> between GPU and CPU (apart from write-combining buffering), the 
> correct
> >>> CPU caching mode matching the PAT index needs to be selected for the
> >>> buffer object in i915_ttm_select_tt_caching().
> >>
> >> The patch doesn't affect the CPU caching mode setting logic though.
> >> And the caching settings for objects created by kernel should remain
> >> the same for both CPU and GPU, objects created by userspace are
> >> managed completely by userspace.
> >>
> >> One question though, what do you mean by non-snooping PAT indices?
> >
> > Yes, that was actually the bottom question: What do these PAT settings
> > allow you to do WRT the snooping on supported discrete devices (DG2) on
> > i915?
> > If they indeed don't allow disabling snooping, then that's not a 
> problem.
>
> When dGPU's access SysMem, the PCIe default is for that access to 
> snoop the
> host's caches. All of our current dGPU's do that -- independent of PAT 
> setting.
>
> > If they do, the ttm code there needs some modification.
>
> I'm not familiar with ttm, but if your concern is that certain PAT index
> could disable snooping, that is not possible for current dGPU's.
> I think it is possible for Xe2/3 though, because there will be COH_MODE
> defined in the PAT registers going forward.


OK. If that's the case, then it should be safe to disregard this concern.

Thanks,


Thomas



>
> -Fei
>