Skip to content

Commit a815362

Browse files
committed
drm/i915: Try to relocate the BIOS fb to the start of ggtt
On MTL the GOP (for whatever reason) likes to bind its framebuffer high up in the ggtt address space. This can conflict with whatever ggtt_reserve_guc_top() is trying to do, and the result is that ggtt_reserve_guc_top() fails and then we proceed to explode when trying to tear down the driver. Thus far I haven't analyzed what causes the actual fireworks, but it's not super important as even if it didn't explode we'd still fail the driver load and the user would be left with an unusable GPU. To remedy this (without having to figure out exactly what ggtt_reserve_guc_top() is trying to achieve) we can attempt to relocate the BIOS framebuffer to a lower ggtt address. We can do this at this early point in driver init because nothing else is supposed to be clobbering the ggtt yet. So we simply change where in the ggtt we pin the vma, the original PTEs will be left as is, and the new PTEs will get written with the same dma addresses. The plane will keep on scanning out from the original PTEs until we are done with the whole process, and at that point we rewrite the plane's surface address register to point at the new ggtt address. Since we don't need a specific ggtt address for the plane (apart from needing it to land in the mappable region for normal stolen objects) we'll just try to pin it without a fixed offset first. It should end up at the lowest available address (which really should be 0 at this point in the driver init). If that fails we'll fall back to just pinning it exactly to the origianal address. To make sure we don't accidentlally pin it partially over the original ggtt range (as that would corrupt the original PTEs) we reserve the original range temporarily during this process. v2: Try to pin explicitly to ggtt offset 0 as otherwise DG2 puts it even higher (atm we have no PIN_LOW flag to force it low) v3: "fix" xe Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Tested-by: Paz Zcharya <pazz@chromium.org> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240202224340.30647-16-ville.syrjala@linux.intel.com Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>
1 parent ea5e150 commit a815362

8 files changed

Lines changed: 129 additions & 2 deletions

File tree

drivers/gpu/drm/i915/display/i9xx_plane.c

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,3 +1060,33 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc,
10601060

10611061
plane_config->fb = intel_fb;
10621062
}
1063+
1064+
bool i9xx_fixup_initial_plane_config(struct intel_crtc *crtc,
1065+
const struct intel_initial_plane_config *plane_config)
1066+
{
1067+
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
1068+
struct intel_plane *plane = to_intel_plane(crtc->base.primary);
1069+
const struct intel_plane_state *plane_state =
1070+
to_intel_plane_state(plane->base.state);
1071+
enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
1072+
u32 base;
1073+
1074+
if (!plane_state->uapi.visible)
1075+
return false;
1076+
1077+
base = intel_plane_ggtt_offset(plane_state);
1078+
1079+
/*
1080+
* We may have moved the surface to a different
1081+
* part of ggtt, make the plane aware of that.
1082+
*/
1083+
if (plane_config->base == base)
1084+
return false;
1085+
1086+
if (DISPLAY_VER(dev_priv) >= 4)
1087+
intel_de_write(dev_priv, DSPSURF(i9xx_plane), base);
1088+
else
1089+
intel_de_write(dev_priv, DSPADDR(i9xx_plane), base);
1090+
1091+
return true;
1092+
}

drivers/gpu/drm/i915/display/i9xx_plane.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe);
2626

2727
void i9xx_get_initial_plane_config(struct intel_crtc *crtc,
2828
struct intel_initial_plane_config *plane_config);
29+
bool i9xx_fixup_initial_plane_config(struct intel_crtc *crtc,
30+
const struct intel_initial_plane_config *plane_config);
2931
#else
3032
static inline unsigned int i965_plane_max_stride(struct intel_plane *plane,
3133
u32 pixel_format, u64 modifier,
@@ -46,6 +48,11 @@ static inline void i9xx_get_initial_plane_config(struct intel_crtc *crtc,
4648
struct intel_initial_plane_config *plane_config)
4749
{
4850
}
51+
static inline bool i9xx_fixup_initial_plane_config(struct intel_crtc *crtc,
52+
const struct intel_initial_plane_config *plane_config)
53+
{
54+
return false;
55+
}
4956
#endif
5057

5158
#endif

drivers/gpu/drm/i915/display/intel_display.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7850,6 +7850,7 @@ static const struct intel_display_funcs skl_display_funcs = {
78507850
.crtc_disable = hsw_crtc_disable,
78517851
.commit_modeset_enables = skl_commit_modeset_enables,
78527852
.get_initial_plane_config = skl_get_initial_plane_config,
7853+
.fixup_initial_plane_config = skl_fixup_initial_plane_config,
78537854
};
78547855

78557856
static const struct intel_display_funcs ddi_display_funcs = {
@@ -7858,6 +7859,7 @@ static const struct intel_display_funcs ddi_display_funcs = {
78587859
.crtc_disable = hsw_crtc_disable,
78597860
.commit_modeset_enables = intel_commit_modeset_enables,
78607861
.get_initial_plane_config = i9xx_get_initial_plane_config,
7862+
.fixup_initial_plane_config = i9xx_fixup_initial_plane_config,
78617863
};
78627864

78637865
static const struct intel_display_funcs pch_split_display_funcs = {
@@ -7866,6 +7868,7 @@ static const struct intel_display_funcs pch_split_display_funcs = {
78667868
.crtc_disable = ilk_crtc_disable,
78677869
.commit_modeset_enables = intel_commit_modeset_enables,
78687870
.get_initial_plane_config = i9xx_get_initial_plane_config,
7871+
.fixup_initial_plane_config = i9xx_fixup_initial_plane_config,
78697872
};
78707873

78717874
static const struct intel_display_funcs vlv_display_funcs = {
@@ -7874,6 +7877,7 @@ static const struct intel_display_funcs vlv_display_funcs = {
78747877
.crtc_disable = i9xx_crtc_disable,
78757878
.commit_modeset_enables = intel_commit_modeset_enables,
78767879
.get_initial_plane_config = i9xx_get_initial_plane_config,
7880+
.fixup_initial_plane_config = i9xx_fixup_initial_plane_config,
78777881
};
78787882

78797883
static const struct intel_display_funcs i9xx_display_funcs = {
@@ -7882,6 +7886,7 @@ static const struct intel_display_funcs i9xx_display_funcs = {
78827886
.crtc_disable = i9xx_crtc_disable,
78837887
.commit_modeset_enables = intel_commit_modeset_enables,
78847888
.get_initial_plane_config = i9xx_get_initial_plane_config,
7889+
.fixup_initial_plane_config = i9xx_fixup_initial_plane_config,
78857890
};
78867891

78877892
/**

drivers/gpu/drm/i915/display/intel_display_core.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ struct intel_display_funcs {
6767
struct intel_crtc_state *);
6868
void (*get_initial_plane_config)(struct intel_crtc *,
6969
struct intel_initial_plane_config *);
70+
bool (*fixup_initial_plane_config)(struct intel_crtc *crtc,
71+
const struct intel_initial_plane_config *plane_config);
7072
void (*crtc_enable)(struct intel_atomic_state *state,
7173
struct intel_crtc *crtc);
7274
void (*crtc_disable)(struct intel_atomic_state *state,

drivers/gpu/drm/i915/display/intel_plane_initial.c

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
* Copyright © 2021 Intel Corporation
44
*/
55

6+
#include "gem/i915_gem_lmem.h"
67
#include "gem/i915_gem_region.h"
78
#include "i915_drv.h"
89
#include "intel_atomic_plane.h"
10+
#include "intel_crtc.h"
911
#include "intel_display.h"
1012
#include "intel_display_types.h"
1113
#include "intel_fb.h"
@@ -138,6 +140,7 @@ initial_plane_vma(struct drm_i915_private *i915,
138140
{
139141
struct intel_memory_region *mem;
140142
struct drm_i915_gem_object *obj;
143+
struct drm_mm_node orig_mm = {};
141144
struct i915_vma *vma;
142145
resource_size_t phys_base;
143146
u32 base, size;
@@ -195,23 +198,66 @@ initial_plane_vma(struct drm_i915_private *i915,
195198
goto err_obj;
196199
}
197200

201+
/*
202+
* MTL GOP likes to place the framebuffer high up in ggtt,
203+
* which can cause problems for ggtt_reserve_guc_top().
204+
* Try to pin it to a low ggtt address instead to avoid that.
205+
*/
206+
base = 0;
207+
208+
if (base != plane_config->base) {
209+
struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
210+
int ret;
211+
212+
/*
213+
* Make sure the original and new locations
214+
* can't overlap. That would corrupt the original
215+
* PTEs which are still being used for scanout.
216+
*/
217+
ret = i915_gem_gtt_reserve(&ggtt->vm, NULL, &orig_mm,
218+
size, plane_config->base,
219+
I915_COLOR_UNEVICTABLE, PIN_NOEVICT);
220+
if (ret)
221+
goto err_obj;
222+
}
223+
198224
vma = i915_vma_instance(obj, &to_gt(i915)->ggtt->vm, NULL);
199225
if (IS_ERR(vma))
200226
goto err_obj;
201227

228+
retry:
202229
pinctl = PIN_GLOBAL | PIN_OFFSET_FIXED | base;
203-
if (HAS_GMCH(i915))
230+
if (!i915_gem_object_is_lmem(obj))
204231
pinctl |= PIN_MAPPABLE;
205-
if (i915_vma_pin(vma, 0, 0, pinctl))
232+
if (i915_vma_pin(vma, 0, 0, pinctl)) {
233+
if (drm_mm_node_allocated(&orig_mm)) {
234+
drm_mm_remove_node(&orig_mm);
235+
/*
236+
* Try again, but this time pin
237+
* it to its original location.
238+
*/
239+
base = plane_config->base;
240+
goto retry;
241+
}
206242
goto err_obj;
243+
}
207244

208245
if (i915_gem_object_is_tiled(obj) &&
209246
!i915_vma_is_map_and_fenceable(vma))
210247
goto err_obj;
211248

249+
if (drm_mm_node_allocated(&orig_mm))
250+
drm_mm_remove_node(&orig_mm);
251+
252+
drm_dbg_kms(&i915->drm,
253+
"Initial plane fb bound to 0x%x in the ggtt (original 0x%x)\n",
254+
i915_ggtt_offset(vma), plane_config->base);
255+
212256
return vma;
213257

214258
err_obj:
259+
if (drm_mm_node_allocated(&orig_mm))
260+
drm_mm_remove_node(&orig_mm);
215261
i915_gem_object_put(obj);
216262
return NULL;
217263
}
@@ -386,6 +432,9 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
386432
*/
387433
intel_find_initial_plane_obj(crtc, plane_configs);
388434

435+
if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
436+
intel_crtc_wait_for_next_vblank(crtc);
437+
389438
plane_config_fini(plane_config);
390439
}
391440
}

drivers/gpu/drm/i915/display/skl_universal_plane.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2624,3 +2624,31 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
26242624
error:
26252625
kfree(intel_fb);
26262626
}
2627+
2628+
bool skl_fixup_initial_plane_config(struct intel_crtc *crtc,
2629+
const struct intel_initial_plane_config *plane_config)
2630+
{
2631+
struct drm_i915_private *i915 = to_i915(crtc->base.dev);
2632+
struct intel_plane *plane = to_intel_plane(crtc->base.primary);
2633+
const struct intel_plane_state *plane_state =
2634+
to_intel_plane_state(plane->base.state);
2635+
enum plane_id plane_id = plane->id;
2636+
enum pipe pipe = crtc->pipe;
2637+
u32 base;
2638+
2639+
if (!plane_state->uapi.visible)
2640+
return false;
2641+
2642+
base = intel_plane_ggtt_offset(plane_state);
2643+
2644+
/*
2645+
* We may have moved the surface to a different
2646+
* part of ggtt, make the plane aware of that.
2647+
*/
2648+
if (plane_config->base == base)
2649+
return false;
2650+
2651+
intel_de_write(i915, PLANE_SURF(pipe, plane_id), base);
2652+
2653+
return true;
2654+
}

drivers/gpu/drm/i915/display/skl_universal_plane.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
2222

2323
void skl_get_initial_plane_config(struct intel_crtc *crtc,
2424
struct intel_initial_plane_config *plane_config);
25+
bool skl_fixup_initial_plane_config(struct intel_crtc *crtc,
26+
const struct intel_initial_plane_config *plane_config);
2527

2628
int skl_format_to_fourcc(int format, bool rgb_order, bool alpha);
2729

drivers/gpu/drm/xe/display/xe_plane_initial.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include "i915_drv.h"
1212
#include "intel_atomic_plane.h"
13+
#include "intel_crtc.h"
1314
#include "intel_display.h"
1415
#include "intel_display_types.h"
1516
#include "intel_fb.h"
@@ -295,6 +296,9 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
295296
*/
296297
intel_find_initial_plane_obj(crtc, plane_configs);
297298

299+
if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
300+
intel_crtc_wait_for_next_vblank(crtc);
301+
298302
plane_config_fini(plane_config);
299303
}
300304
}

0 commit comments

Comments
 (0)