On 15.12.2023 12:09, Ville Syrjala wrote:
From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
If we fail to pin error_capture to the start of ggtt (which
is likely given the BIOS fb is usually there), we currently
fall back to pinning it at the next available low address.
This seems somewhat sub-optimal to me in case we later discard
the BIOS fb (fairly likely if there are multiple different sized
displays connected at boot). We are then then left with a
permanenly pinned object somewhere in the middle of the mappable
range of ggtt. It seems more sensible to pin the error capture
object to the end of mappable as a fallback, so even if we discard
the BIOS fb we are left with the mappable region mostly in one
piece (potentially allowing for more/larger objects to be pinned
there later).
Though I suppose we are chopping the ggtt address space as a
whole into two chunks in a slightly different way. Essentially
reducing the size of the second (larger) chunk a bit. So perhaps
pinning truly massive objects (which don't strictly need to
be mappable) could become a bit more difficult.
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 21a7e3191c18..f62008962eb5 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -876,7 +876,7 @@ static int init_ggtt(struct i915_ggtt *ggtt)
ggtt->error_capture.size, 0,
ggtt->error_capture.color,
0, ggtt->mappable_end,
- DRM_MM_INSERT_LOW);
+ DRM_MM_INSERT_HIGH);
In this case Chris raised concern that since error_capture must be
mappable we will end up with worse fragmentation in case of
DRM_MM_INSERT_HIGH.
Also see the comment above the change (my concern):
* We strongly prefer taking address 0x0 in order to protect
* other critical buffers against accidental overwrites,
* as writing to address 0 is a very common mistake.
Maybe the ultimate solution would be to relocate BIOS fb vma also if it
is at 0, I don't know.
Regards
Andrzej
}
if (drm_mm_node_allocated(&ggtt->error_capture)) {
u64 start = ggtt->error_capture.start;
