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

[Thomas Hellström (Intel) <thomas_os@xxxxxxxxxxxx>]

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@xxxxxxxxx wrote:

>>>> From: Fei Yang <fei.yang@xxxxxxxxx>

>>>>

>>>> 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