Re: [Intel-gfx] [PATCH] drm/i915: Don't use stolen memory for ring buffers

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 15/02/2023 01:56, Ceraolo Spurio, Daniele wrote:
> 
> 
> On 2/14/2023 3:48 PM, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Direction from hardware is that stolen memory should never be used for
>> ring buffer allocations. There are too many caching pitfalls due to the
>> way stolen memory accesses are routed. So it is safest to just not use
>> it.
> 
> I'm wondering if this applies to machines in ringbuffer mode as well, as 
> some of the caching stuff that according to the HW team may not work 
> properly with stolen mem accesses from the CS (mocs, ppat) came with 
> gen8/gen9.
> Maybe limit this change to gen8+, to avoid changing the behavior for 
> very old platforms?

If Gen8+ can have bugs due this then:

Fixes: c58b735fc762 ("drm/i915: Allocate rings from stolen")
Cc: <stable@vger.kernel.org> # v4.9+

Or even before:

Fixes: ebc052e0c65f ("drm/i915: Allocate ringbuffers from stolen memory")
Cc: <stable@vger.kernel.org> # v3.9+

Hm lets see when BDW when out of force probe:

Fixes: babb1903511f ("drm/i915/bdw: remove preliminary_hw_support flag from BDW")
Cc: <stable@vger.kernel.org> # v3.14+

Depends also how the problem statement interacts with LLC. If !LLC platforms are okay then the first one from the above list is enough.

Because

Regards,

Tvrtko

>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_ring.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c 
>> b/drivers/gpu/drm/i915/gt/intel_ring.c
>> index 15ec64d881c44..d1a47e1ae6452 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_ring.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_ring.c
>> @@ -116,8 +116,6 @@ static struct i915_vma *create_ring_vma(struct 
>> i915_ggtt *ggtt, int size)
>>       obj = i915_gem_object_create_lmem(i915, size, 
>> I915_BO_ALLOC_VOLATILE |
>>                         I915_BO_ALLOC_PM_VOLATILE);
>> -    if (IS_ERR(obj) && i915_ggtt_has_aperture(ggtt))
>> -        obj = i915_gem_object_create_stolen(i915, size);
> 
> There is code in ring_pin/unpin() that only applies to rings in stolen 
> memory, so you need to remove that as well if you drop stolen for rings 
> on all platforms.
> 
> Daniele
> 
>>       if (IS_ERR(obj))
>>           obj = i915_gem_object_create_internal(i915, size);
>>       if (IS_ERR(obj))
>