| Message ID | 20230214234856.744573-1-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Don't use stolen memory for ring buffers | expand |
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? > > 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))
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)) >
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); if (IS_ERR(obj)) obj = i915_gem_object_create_internal(i915, size); if (IS_ERR(obj))