* [Intel-gfx] [PATCH] [v2] drm/i915/display: Check GGTT to determine phys_base
@ 2023-12-06 18:46 Paz Zcharya
2023-12-07 0:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2) Patchwork
2023-12-08 1:00 ` [Intel-gfx] [PATCH] [v2] drm/i915/display: Check GGTT to determine phys_base Almahallawy, Khaled
0 siblings, 2 replies; 8+ messages in thread
From: Paz Zcharya @ 2023-12-06 18:46 UTC (permalink / raw)
To: Andrzej Hajda, Tvrtko Ursulin
Cc: Subrata Banik, intel-gfx, Marcin Wojtas, linux-kernel, dri-devel,
Sean Paul, matthew.auld, Daniel Vetter, Rodrigo Vivi,
Drew Davenport, David Airlie, Paz Zcharya, Nirmoy Das
There was an assumption that for iGPU there should be a 1:1 mapping
of GGTT to physical address pointing to the framebuffer.
This assumption is not strictly true effective generation 8 or newer.
Fix that by checking GGTT to determine the phys address on gen8+.
The following algorithm for phys_base should be valid for all platforms:
1. Find pte
2. if(IS_DGFX(i915) && pte & GEN12_GGTT_PTE_LM) mem =
i915->mm.regions[INTEL_REGION_LMEM_0] else mem = i915->mm.stolen_region
3. phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;
- On older platforms, stolen_region points to system memory, starting at 0
- on DG2, it uses lmem region which starts at 0 as well
- on MTL, stolen_region points to stolen-local which starts at 0x800000
Changes from v1:
- Add an if statement for gen7-, where there is a 1:1 mapping
Signed-off-by: Paz Zcharya <pazz@chromium.org>
---
.../drm/i915/display/intel_plane_initial.c | 64 +++++++++++--------
1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index a55c09cbd0e4..7d9bb631b93b 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -59,44 +59,58 @@ initial_plane_vma(struct drm_i915_private *i915,
return NULL;
base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT);
- if (IS_DGFX(i915)) {
+
+ if (GRAPHICS_VER(i915) < 8) {
+ /*
+ * In gen7-, there is a 1:1 mapping
+ * between GSM and physical address.
+ */
+ phys_base = base;
+ mem = i915->mm.stolen_region;
+ } else {
+ /*
+ * In gen8+, there is no 1:1 mapping between
+ * GSM and physical address, so we need to
+ * check GGTT to determine the physical address.
+ */
gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
gen8_pte_t pte;
gte += base / I915_GTT_PAGE_SIZE;
-
pte = ioread64(gte);
- if (!(pte & GEN12_GGTT_PTE_LM)) {
- drm_err(&i915->drm,
- "Initial plane programming missing PTE_LM bit\n");
- return NULL;
- }
-
- phys_base = pte & I915_GTT_PAGE_MASK;
- mem = i915->mm.regions[INTEL_REGION_LMEM_0];
- /*
- * We don't currently expect this to ever be placed in the
- * stolen portion.
- */
- if (phys_base >= resource_size(&mem->region)) {
- drm_err(&i915->drm,
- "Initial plane programming using invalid range, phys_base=%pa\n",
- &phys_base);
- return NULL;
+ if (IS_DGFX(i915)) {
+ if (!(pte & GEN12_GGTT_PTE_LM)) {
+ drm_err(&i915->drm,
+ "Initial plane programming missing PTE_LM bit\n");
+ return NULL;
+ }
+ mem = i915->mm.regions[INTEL_REGION_LMEM_0];
+ } else {
+ mem = i915->mm.stolen_region;
}
- drm_dbg(&i915->drm,
- "Using phys_base=%pa, based on initial plane programming\n",
- &phys_base);
- } else {
- phys_base = base;
- mem = i915->mm.stolen_region;
+ phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;
}
if (!mem)
return NULL;
+ /*
+ * We don't currently expect this to ever be placed in the
+ * stolen portion.
+ */
+ if (phys_base >= resource_size(&mem->region)) {
+ drm_err(&i915->drm,
+ "Initial plane programming using invalid range, phys_base=%pa\n",
+ &phys_base);
+ return NULL;
+ }
+
+ drm_dbg(&i915->drm,
+ "Using phys_base=%pa, based on initial plane programming\n",
+ &phys_base);
+
size = round_up(plane_config->base + plane_config->size,
mem->min_page_size);
size -= base;
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2)
2023-12-06 18:46 [Intel-gfx] [PATCH] [v2] drm/i915/display: Check GGTT to determine phys_base Paz Zcharya
@ 2023-12-07 0:18 ` Patchwork
2023-12-07 10:10 ` [Intel-gfx] â " Andrzej Hajda
2023-12-08 1:00 ` [Intel-gfx] [PATCH] [v2] drm/i915/display: Check GGTT to determine phys_base Almahallawy, Khaled
1 sibling, 1 reply; 8+ messages in thread
From: Patchwork @ 2023-12-07 0:18 UTC (permalink / raw)
To: Paz Zcharya; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 4082 bytes --]
== Series Details ==
Series: drm/i915/display: Check GGTT to determine phys_base (rev2)
URL : https://patchwork.freedesktop.org/series/127130/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_13990 -> Patchwork_127130v2
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_127130v2 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_127130v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html
Participating hosts (37 -> 34)
------------------------------
Missing (3): fi-pnv-d510 fi-snb-2520m bat-dg1-5
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_127130v2:
### IGT changes ###
#### Possible regressions ####
* igt@i915_module_load@load:
- bat-mtlp-8: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-mtlp-8/igt@i915_module_load@load.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-mtlp-8/igt@i915_module_load@load.html
Known issues
------------
Here are the changes found in Patchwork_127130v2 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_pm_backlight@basic-brightness@edp-1:
- bat-rplp-1: NOTRUN -> [ABORT][3] ([i915#8668])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pm_backlight@basic-brightness@edp-1.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s0@lmem0:
- bat-dg2-9: [INCOMPLETE][4] ([i915#9275]) -> [PASS][5]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html
* igt@kms_flip@basic-flip-vs-dpms@d-dp6:
- bat-adlp-11: [DMESG-FAIL][6] ([i915#6868]) -> [PASS][7]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html
* igt@kms_flip@basic-flip-vs-modeset@d-dp6:
- bat-adlp-11: [DMESG-WARN][8] -> [PASS][9]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html
* igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
- bat-rplp-1: [ABORT][10] ([i915#8668]) -> [PASS][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
[i915#6868]: https://gitlab.freedesktop.org/drm/intel/issues/6868
[i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
[i915#9275]: https://gitlab.freedesktop.org/drm/intel/issues/9275
Build changes
-------------
* Linux: CI_DRM_13990 -> Patchwork_127130v2
CI-20190529: 20190529
CI_DRM_13990: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7626: 154b7288552cd7ed3033f8ef396e88d0bd1b7646 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_127130v2: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
43f210e851cd drm/i915/display: Check GGTT to determine phys_base
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html
[-- Attachment #2: Type: text/html, Size: 4906 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] â Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2)
2023-12-07 0:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2) Patchwork
@ 2023-12-07 10:10 ` Andrzej Hajda
2023-12-07 11:26 ` [Intel-gfx] â " Andrzej Hajda
0 siblings, 1 reply; 8+ messages in thread
From: Andrzej Hajda @ 2023-12-07 10:10 UTC (permalink / raw)
To: intel-gfx, Patchwork, Paz Zcharya; +Cc: Rodrigo Vivi
On 07.12.2023 01:18, Patchwork wrote:
> *Patch Details*
> *Series:* drm/i915/display: Check GGTT to determine phys_base (rev2)
> *URL:* https://patchwork.freedesktop.org/series/127130/
> <https://patchwork.freedesktop.org/series/127130/>
> *State:* failure
> *Details:*
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html
> <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html>
>
>
> CI Bug Log - changes from CI_DRM_13990 -> Patchwork_127130v2
>
>
> Summary
>
> *FAILURE*
>
> Serious unknown changes coming with Patchwork_127130v2 absolutely need to be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in Patchwork_127130v2, please notify your bug team
> (I915-ci-infra@lists.freedesktop.org) to allow them
> to document this new failure mode, which will reduce false positives in CI.
>
> External URL:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html
>
>
> Participating hosts (37 -> 34)
>
> Missing (3): fi-pnv-d510 fi-snb-2520m bat-dg1-5
>
>
> Possible new issues
>
> Here are the unknown changes that may have been introduced in
> Patchwork_127130v2:
>
>
> IGT changes
>
>
> Possible regressions
>
> * igt@i915_module_load@load:
> o bat-mtlp-8: PASS
> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-mtlp-8/igt@i915_module_load@load.html> -> INCOMPLETE <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-mtlp-8/igt@i915_module_load@load.html>
It seems related. I think the patch is correct but it just unveils other
display take-over issues.
Ie with this patch initial_plane_vma returns valid buffer, but
subsequent display code fails miserably with kernel panic.
So until this is not solved, we shouldn't merge the patch, IMO.
CC: i915 maintainers and display developers
Regards
Andrzej
>
>
> Known issues
>
> Here are the changes found in Patchwork_127130v2 that come from known
> issues:
>
>
> IGT changes
>
>
> Issues hit
>
> * igt@kms_pm_backlight@basic-brightness@edp-1:
> o bat-rplp-1: NOTRUN -> ABORT
> <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pm_backlight@basic-brightness@edp-1.html> (i915#8668 <https://gitlab.freedesktop.org/drm/intel/issues/8668>)
>
>
> Possible fixes
>
> *
>
> igt@gem_exec_suspend@basic-s0@lmem0:
>
> o bat-dg2-9: INCOMPLETE
> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html> (i915#9275 <https://gitlab.freedesktop.org/drm/intel/issues/9275>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html>
> *
>
> igt@kms_flip@basic-flip-vs-dpms@d-dp6:
>
> o bat-adlp-11: DMESG-FAIL
> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html> (i915#6868 <https://gitlab.freedesktop.org/drm/intel/issues/6868>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html>
> *
>
> igt@kms_flip@basic-flip-vs-modeset@d-dp6:
>
> o bat-adlp-11: DMESG-WARN
> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html> -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html>
> *
>
> igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
>
> o bat-rplp-1: ABORT
> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html> (i915#8668 <https://gitlab.freedesktop.org/drm/intel/issues/8668>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html>
>
>
> Build changes
>
> * Linux: CI_DRM_13990 -> Patchwork_127130v2
>
> CI-20190529: 20190529
> CI_DRM_13990: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @
> git://anongit.freedesktop.org/gfx-ci/linux
> IGT_7626: 154b7288552cd7ed3033f8ef396e88d0bd1b7646 @
> https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
> Patchwork_127130v2: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @
> git://anongit.freedesktop.org/gfx-ci/linux
>
>
> Linux commits
>
> 43f210e851cd drm/i915/display: Check GGTT to determine phys_base
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] â Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2)
2023-12-07 10:10 ` [Intel-gfx] â " Andrzej Hajda
@ 2023-12-07 11:26 ` Andrzej Hajda
2023-12-07 12:05 ` Imre Deak
2023-12-08 8:32 ` [Intel-gfx] â " Andrzej Hajda
0 siblings, 2 replies; 8+ messages in thread
From: Andrzej Hajda @ 2023-12-07 11:26 UTC (permalink / raw)
To: intel-gfx, Patchwork, Paz Zcharya; +Cc: Rodrigo Vivi
On 07.12.2023 11:10, Andrzej Hajda wrote:
> On 07.12.2023 01:18, Patchwork wrote:
>> *Patch Details*
>> *Series:* drm/i915/display: Check GGTT to determine phys_base (rev2)
>> *URL:* https://patchwork.freedesktop.org/series/127130/
>> <https://patchwork.freedesktop.org/series/127130/>
>> *State:* failure
>> *Details:*
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html
>> <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html>
>>
>>
>> CI Bug Log - changes from CI_DRM_13990 -> Patchwork_127130v2
>>
>>
>> Summary
>>
>> *FAILURE*
>>
>> Serious unknown changes coming with Patchwork_127130v2 absolutely need
>> to be
>> verified manually.
>>
>> If you think the reported changes have nothing to do with the changes
>> introduced in Patchwork_127130v2, please notify your bug team
>> (I915-ci-infra@lists.freedesktop.org) to allow them
>> to document this new failure mode, which will reduce false positives
>> in CI.
>>
>> External URL:
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html
>>
>>
>> Participating hosts (37 -> 34)
>>
>> Missing (3): fi-pnv-d510 fi-snb-2520m bat-dg1-5
>>
>>
>> Possible new issues
>>
>> Here are the unknown changes that may have been introduced in
>> Patchwork_127130v2:
>>
>>
>> IGT changes
>>
>>
>> Possible regressions
>>
>> * igt@i915_module_load@load:
>> o bat-mtlp-8: PASS
>>
>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-mtlp-8/igt@i915_module_load@load.html> -> INCOMPLETE <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-mtlp-8/igt@i915_module_load@load.html>
>
>
> It seems related. I think the patch is correct but it just unveils other
> display take-over issues.
> Ie with this patch initial_plane_vma returns valid buffer, but
> subsequent display code fails miserably with kernel panic.
>
> So until this is not solved, we shouldn't merge the patch, IMO.
>
> CC: i915 maintainers and display developers
After taking a look on panic log [1], I have found:
[drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT for GuC
I don't know why it is only debug level? It seems serious failure, as a
result i915_init_ggtt fails and probe fails.
The cause is that initial framebuffer is located at the end of GGTT and
it overlaps with reserved area (see ggtt_reserve_guc_top).
I am not sure how it can be properly fixed, I guess dirty fix could be
just relocation of vma (hopefully into free area), sth like:
new_gte = gsm + (ggtt->vm.total - GUC_TOP_RESERVE_SIZE - size) /
I915_GTT_PAGE_SIZE;
memmove(new_gte, gte, size / I915_GTT_PAGE_SIZE);
but I have no idea of possible side effects :)
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-mtlp-8/pstore0-2849851684_Panic_1.txt
Regards
Andrzej
>
> Regards
> Andrzej
>
>>
>>
>> Known issues
>>
>> Here are the changes found in Patchwork_127130v2 that come from known
>> issues:
>>
>>
>> IGT changes
>>
>>
>> Issues hit
>>
>> * igt@kms_pm_backlight@basic-brightness@edp-1:
>> o bat-rplp-1: NOTRUN -> ABORT
>>
>> <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pm_backlight@basic-brightness@edp-1.html> (i915#8668 <https://gitlab.freedesktop.org/drm/intel/issues/8668>)
>>
>>
>> Possible fixes
>>
>> *
>>
>> igt@gem_exec_suspend@basic-s0@lmem0:
>>
>> o bat-dg2-9: INCOMPLETE
>>
>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html> (i915#9275 <https://gitlab.freedesktop.org/drm/intel/issues/9275>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html>
>> *
>>
>> igt@kms_flip@basic-flip-vs-dpms@d-dp6:
>>
>> o bat-adlp-11: DMESG-FAIL
>>
>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html> (i915#6868 <https://gitlab.freedesktop.org/drm/intel/issues/6868>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html>
>> *
>>
>> igt@kms_flip@basic-flip-vs-modeset@d-dp6:
>>
>> o bat-adlp-11: DMESG-WARN
>>
>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html> -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html>
>> *
>>
>> igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
>>
>> o bat-rplp-1: ABORT
>>
>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html> (i915#8668 <https://gitlab.freedesktop.org/drm/intel/issues/8668>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html>
>>
>>
>> Build changes
>>
>> * Linux: CI_DRM_13990 -> Patchwork_127130v2
>>
>> CI-20190529: 20190529
>> CI_DRM_13990: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @
>> git://anongit.freedesktop.org/gfx-ci/linux
>> IGT_7626: 154b7288552cd7ed3033f8ef396e88d0bd1b7646 @
>> https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
>> Patchwork_127130v2: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @
>> git://anongit.freedesktop.org/gfx-ci/linux
>>
>>
>> Linux commits
>>
>> 43f210e851cd drm/i915/display: Check GGTT to determine phys_base
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] â Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2)
2023-12-07 11:26 ` [Intel-gfx] â " Andrzej Hajda
@ 2023-12-07 12:05 ` Imre Deak
2023-12-08 8:32 ` [Intel-gfx] â " Andrzej Hajda
1 sibling, 0 replies; 8+ messages in thread
From: Imre Deak @ 2023-12-07 12:05 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi, Paz Zcharya
On Thu, Dec 07, 2023 at 12:26:25PM +0100, Andrzej Hajda wrote:
>
>
> On 07.12.2023 11:10, Andrzej Hajda wrote:
> > On 07.12.2023 01:18, Patchwork wrote:
> > > *Patch Details*
> > > *Series:* drm/i915/display: Check GGTT to determine phys_base (rev2)
> > > *URL:* https://patchwork.freedesktop.org/series/127130/
> > > <https://patchwork.freedesktop.org/series/127130/>
> > > *State:* failure
> > > *Details:*
> > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html
> > > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html>
> > >
> > >
> > > CI Bug Log - changes from CI_DRM_13990 -> Patchwork_127130v2
> > >
> > >
> > > Summary
> > >
> > > *FAILURE*
> > >
> > > Serious unknown changes coming with Patchwork_127130v2 absolutely
> > > need to be
> > > verified manually.
> > >
> > > If you think the reported changes have nothing to do with the changes
> > > introduced in Patchwork_127130v2, please notify your bug team
> > > (I915-ci-infra@lists.freedesktop.org) to allow them
> > > to document this new failure mode, which will reduce false positives
> > > in CI.
> > >
> > > External URL:
> > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html
> > >
> > >
> > > Participating hosts (37 -> 34)
> > >
> > > Missing (3): fi-pnv-d510 fi-snb-2520m bat-dg1-5
> > >
> > >
> > > Possible new issues
> > >
> > > Here are the unknown changes that may have been introduced in
> > > Patchwork_127130v2:
> > >
> > >
> > > IGT changes
> > >
> > >
> > > Possible regressions
> > >
> > > * igt@i915_module_load@load:
> > > o bat-mtlp-8: PASS
> > > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-mtlp-8/igt@i915_module_load@load.html> -> INCOMPLETE <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-mtlp-8/igt@i915_module_load@load.html>
> >
> >
> > It seems related. I think the patch is correct but it just unveils other
> > display take-over issues.
> > Ie with this patch initial_plane_vma returns valid buffer, but
> > subsequent display code fails miserably with kernel panic.
> >
> > So until this is not solved, we shouldn't merge the patch, IMO.
> >
> > CC: i915 maintainers and display developers
>
>
> After taking a look on panic log [1], I have found:
> [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT for GuC
>
> I don't know why it is only debug level? It seems serious failure, as a
> result i915_init_ggtt fails and probe fails.
>
> The cause is that initial framebuffer is located at the end of GGTT and it
> overlaps with reserved area (see ggtt_reserve_guc_top).
>
> I am not sure how it can be properly fixed, I guess dirty fix could be
> just relocation of vma (hopefully into free area), sth like:
> new_gte = gsm + (ggtt->vm.total - GUC_TOP_RESERVE_SIZE - size) /
> I915_GTT_PAGE_SIZE;
> memmove(new_gte, gte, size / I915_GTT_PAGE_SIZE);
>
> but I have no idea of possible side effects :)
>
> [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-mtlp-8/pstore0-2849851684_Panic_1.txt
fwiw, the following hack should fix the error path:
@@ -822,6 +823,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
i915_gem_driver_release(i915);
out_cleanup_modeset2:
/* FIXME clean up the error path */
+ if (HAS_DISPLAY(i915))
+ drm_atomic_helper_shutdown(&i915->drm);
intel_display_driver_remove(i915);
intel_irq_uninstall(i915);
intel_display_driver_remove_noirq(i915);
> Regards
> Andrzej
>
>
> >
> > Regards
> > Andrzej
> >
> > >
> > >
> > > Known issues
> > >
> > > Here are the changes found in Patchwork_127130v2 that come from
> > > known issues:
> > >
> > >
> > > IGT changes
> > >
> > >
> > > Issues hit
> > >
> > > * igt@kms_pm_backlight@basic-brightness@edp-1:
> > > o bat-rplp-1: NOTRUN -> ABORT
> > > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pm_backlight@basic-brightness@edp-1.html> (i915#8668 <https://gitlab.freedesktop.org/drm/intel/issues/8668>)
> > >
> > >
> > > Possible fixes
> > >
> > > *
> > >
> > > igt@gem_exec_suspend@basic-s0@lmem0:
> > >
> > > o bat-dg2-9: INCOMPLETE
> > > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html> (i915#9275 <https://gitlab.freedesktop.org/drm/intel/issues/9275>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html>
> > > *
> > >
> > > igt@kms_flip@basic-flip-vs-dpms@d-dp6:
> > >
> > > o bat-adlp-11: DMESG-FAIL
> > > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html> (i915#6868 <https://gitlab.freedesktop.org/drm/intel/issues/6868>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html>
> > > *
> > >
> > > igt@kms_flip@basic-flip-vs-modeset@d-dp6:
> > >
> > > o bat-adlp-11: DMESG-WARN
> > > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html> -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html>
> > > *
> > >
> > > igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
> > >
> > > o bat-rplp-1: ABORT
> > > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html> (i915#8668 <https://gitlab.freedesktop.org/drm/intel/issues/8668>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html>
> > >
> > >
> > > Build changes
> > >
> > > * Linux: CI_DRM_13990 -> Patchwork_127130v2
> > >
> > > CI-20190529: 20190529
> > > CI_DRM_13990: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @
> > > git://anongit.freedesktop.org/gfx-ci/linux
> > > IGT_7626: 154b7288552cd7ed3033f8ef396e88d0bd1b7646 @
> > > https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
> > > Patchwork_127130v2: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @
> > > git://anongit.freedesktop.org/gfx-ci/linux
> > >
> > >
> > > Linux commits
> > >
> > > 43f210e851cd drm/i915/display: Check GGTT to determine phys_base
> > >
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH] [v2] drm/i915/display: Check GGTT to determine phys_base
2023-12-06 18:46 [Intel-gfx] [PATCH] [v2] drm/i915/display: Check GGTT to determine phys_base Paz Zcharya
2023-12-07 0:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2) Patchwork
@ 2023-12-08 1:00 ` Almahallawy, Khaled
1 sibling, 0 replies; 8+ messages in thread
From: Almahallawy, Khaled @ 2023-12-08 1:00 UTC (permalink / raw)
To: pazz, Ursulin, Tvrtko, Hajda, Andrzej
Cc: Banik, Subrata, intel-gfx, mwojtas, linux-kernel, dri-devel,
seanpaul, Auld, Matthew, daniel, Vivi, Rodrigo, ddavenport,
airlied, Das, Nirmoy
Thank You for the patch. We noticed a break in the customer board with
the latest GOP + this patch.
Thank You
Khaled
On Wed, 2023-12-06 at 18:46 +0000, Paz Zcharya wrote:
> There was an assumption that for iGPU there should be a 1:1 mapping
> of GGTT to physical address pointing to the framebuffer.
> This assumption is not strictly true effective generation 8 or newer.
> Fix that by checking GGTT to determine the phys address on gen8+.
>
> The following algorithm for phys_base should be valid for all
> platforms:
> 1. Find pte
> 2. if(IS_DGFX(i915) && pte & GEN12_GGTT_PTE_LM) mem =
> i915->mm.regions[INTEL_REGION_LMEM_0] else mem = i915-
> >mm.stolen_region
> 3. phys_base = (pte & I915_GTT_PAGE_MASK) - mem->region.start;
>
> - On older platforms, stolen_region points to system memory, starting
> at 0
> - on DG2, it uses lmem region which starts at 0 as well
> - on MTL, stolen_region points to stolen-local which starts at
> 0x800000
>
> Changes from v1:
> - Add an if statement for gen7-, where there is a 1:1 mapping
>
> Signed-off-by: Paz Zcharya <pazz@chromium.org>
> ---
>
> .../drm/i915/display/intel_plane_initial.c | 64 +++++++++++----
> ----
> 1 file changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> index a55c09cbd0e4..7d9bb631b93b 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> @@ -59,44 +59,58 @@ initial_plane_vma(struct drm_i915_private *i915,
> return NULL;
>
> base = round_down(plane_config->base, I915_GTT_MIN_ALIGNMENT);
> - if (IS_DGFX(i915)) {
> +
> + if (GRAPHICS_VER(i915) < 8) {
> + /*
> + * In gen7-, there is a 1:1 mapping
> + * between GSM and physical address.
> + */
> + phys_base = base;
> + mem = i915->mm.stolen_region;
> + } else {
> + /*
> + * In gen8+, there is no 1:1 mapping between
> + * GSM and physical address, so we need to
> + * check GGTT to determine the physical address.
> + */
> gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm;
> gen8_pte_t pte;
>
> gte += base / I915_GTT_PAGE_SIZE;
> -
> pte = ioread64(gte);
> - if (!(pte & GEN12_GGTT_PTE_LM)) {
> - drm_err(&i915->drm,
> - "Initial plane programming missing
> PTE_LM bit\n");
> - return NULL;
> - }
> -
> - phys_base = pte & I915_GTT_PAGE_MASK;
> - mem = i915->mm.regions[INTEL_REGION_LMEM_0];
>
> - /*
> - * We don't currently expect this to ever be placed in
> the
> - * stolen portion.
> - */
> - if (phys_base >= resource_size(&mem->region)) {
> - drm_err(&i915->drm,
> - "Initial plane programming using
> invalid range, phys_base=%pa\n",
> - &phys_base);
> - return NULL;
> + if (IS_DGFX(i915)) {
> + if (!(pte & GEN12_GGTT_PTE_LM)) {
> + drm_err(&i915->drm,
> + "Initial plane programming
> missing PTE_LM bit\n");
> + return NULL;
> + }
> + mem = i915->mm.regions[INTEL_REGION_LMEM_0];
> + } else {
> + mem = i915->mm.stolen_region;
> }
>
> - drm_dbg(&i915->drm,
> - "Using phys_base=%pa, based on initial plane
> programming\n",
> - &phys_base);
> - } else {
> - phys_base = base;
> - mem = i915->mm.stolen_region;
> + phys_base = (pte & I915_GTT_PAGE_MASK) - mem-
> >region.start;
> }
>
> if (!mem)
> return NULL;
>
> + /*
> + * We don't currently expect this to ever be placed in the
> + * stolen portion.
> + */
> + if (phys_base >= resource_size(&mem->region)) {
> + drm_err(&i915->drm,
> + "Initial plane programming using invalid range,
> phys_base=%pa\n",
> + &phys_base);
> + return NULL;
> + }
> +
> + drm_dbg(&i915->drm,
> + "Using phys_base=%pa, based on initial plane
> programming\n",
> + &phys_base);
> +
> size = round_up(plane_config->base + plane_config->size,
> mem->min_page_size);
> size -= base;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] â Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2)
2023-12-07 11:26 ` [Intel-gfx] â " Andrzej Hajda
2023-12-07 12:05 ` Imre Deak
@ 2023-12-08 8:32 ` Andrzej Hajda
2023-12-13 16:03 ` Ville Syrjälä
1 sibling, 1 reply; 8+ messages in thread
From: Andrzej Hajda @ 2023-12-08 8:32 UTC (permalink / raw)
To: intel-gfx, Patchwork, Paz Zcharya; +Cc: Rodrigo Vivi
On 07.12.2023 12:26, Andrzej Hajda wrote:
>
>
> On 07.12.2023 11:10, Andrzej Hajda wrote:
>> On 07.12.2023 01:18, Patchwork wrote:
>>> *Patch Details*
>>> *Series:* drm/i915/display: Check GGTT to determine phys_base (rev2)
>>> *URL:* https://patchwork.freedesktop.org/series/127130/
>>> <https://patchwork.freedesktop.org/series/127130/>
>>> *State:* failure
>>> *Details:*
>>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html>
>>>
>>>
>>> CI Bug Log - changes from CI_DRM_13990 -> Patchwork_127130v2
>>>
>>>
>>> Summary
>>>
>>> *FAILURE*
>>>
>>> Serious unknown changes coming with Patchwork_127130v2 absolutely
>>> need to be
>>> verified manually.
>>>
>>> If you think the reported changes have nothing to do with the changes
>>> introduced in Patchwork_127130v2, please notify your bug team
>>> (I915-ci-infra@lists.freedesktop.org) to allow them
>>> to document this new failure mode, which will reduce false positives
>>> in CI.
>>>
>>> External URL:
>>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/index.html
>>>
>>>
>>> Participating hosts (37 -> 34)
>>>
>>> Missing (3): fi-pnv-d510 fi-snb-2520m bat-dg1-5
>>>
>>>
>>> Possible new issues
>>>
>>> Here are the unknown changes that may have been introduced in
>>> Patchwork_127130v2:
>>>
>>>
>>> IGT changes
>>>
>>>
>>> Possible regressions
>>>
>>> * igt@i915_module_load@load:
>>> o bat-mtlp-8: PASS
>>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-mtlp-8/igt@i915_module_load@load.html> -> INCOMPLETE <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-mtlp-8/igt@i915_module_load@load.html>
>>
>>
>> It seems related. I think the patch is correct but it just unveils
>> other display take-over issues.
>> Ie with this patch initial_plane_vma returns valid buffer, but
>> subsequent display code fails miserably with kernel panic.
>>
>> So until this is not solved, we shouldn't merge the patch, IMO.
>>
>> CC: i915 maintainers and display developers
>
>
> After taking a look on panic log [1], I have found:
> [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT for GuC
>
> I don't know why it is only debug level? It seems serious failure, as a
> result i915_init_ggtt fails and probe fails.
>
> The cause is that initial framebuffer is located at the end of GGTT and
> it overlaps with reserved area (see ggtt_reserve_guc_top).
>
> I am not sure how it can be properly fixed, I guess dirty fix could be
> just relocation of vma (hopefully into free area), sth like:
> new_gte = gsm + (ggtt->vm.total - GUC_TOP_RESERVE_SIZE - size) /
> I915_GTT_PAGE_SIZE;
> memmove(new_gte, gte, size / I915_GTT_PAGE_SIZE);
>
> but I have no idea of possible side effects :)
I looked once more into the code and maybe you can just pin the buffer
to earlier address (not overlapping with GuC reservation and current vma
of the fb):
@@ -143,6 +143,9 @@ initial_plane_vma(struct drm_i915_private *i915,
if (IS_ERR(vma))
goto err_obj;
+ if (base + size > GUC_GGTT_TOP)
+ base = min(base, GUC_GGTT_TOP) - size;
+
pinctl = PIN_GLOBAL | PIN_OFFSET_FIXED | base;
if (HAS_GMCH(i915))
pinctl |= PIN_MAPPABLE;
Regards
Andrzej
>
> [1]:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-mtlp-8/pstore0-2849851684_Panic_1.txt
>
> Regards
> Andrzej
>
>
>>
>> Regards
>> Andrzej
>>
>>>
>>>
>>> Known issues
>>>
>>> Here are the changes found in Patchwork_127130v2 that come from known
>>> issues:
>>>
>>>
>>> IGT changes
>>>
>>>
>>> Issues hit
>>>
>>> * igt@kms_pm_backlight@basic-brightness@edp-1:
>>> o bat-rplp-1: NOTRUN -> ABORT
>>> <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pm_backlight@basic-brightness@edp-1.html> (i915#8668 <https://gitlab.freedesktop.org/drm/intel/issues/8668>)
>>>
>>>
>>> Possible fixes
>>>
>>> *
>>>
>>> igt@gem_exec_suspend@basic-s0@lmem0:
>>>
>>> o bat-dg2-9: INCOMPLETE
>>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html> (i915#9275 <https://gitlab.freedesktop.org/drm/intel/issues/9275>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html>
>>> *
>>>
>>> igt@kms_flip@basic-flip-vs-dpms@d-dp6:
>>>
>>> o bat-adlp-11: DMESG-FAIL
>>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html> (i915#6868 <https://gitlab.freedesktop.org/drm/intel/issues/6868>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html>
>>> *
>>>
>>> igt@kms_flip@basic-flip-vs-modeset@d-dp6:
>>>
>>> o bat-adlp-11: DMESG-WARN
>>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html> -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@d-dp6.html>
>>> *
>>>
>>> igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
>>>
>>> o bat-rplp-1: ABORT
>>> <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13990/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html> (i915#8668 <https://gitlab.freedesktop.org/drm/intel/issues/8668>) -> PASS <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_127130v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html>
>>>
>>>
>>> Build changes
>>>
>>> * Linux: CI_DRM_13990 -> Patchwork_127130v2
>>>
>>> CI-20190529: 20190529
>>> CI_DRM_13990: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @
>>> git://anongit.freedesktop.org/gfx-ci/linux
>>> IGT_7626: 154b7288552cd7ed3033f8ef396e88d0bd1b7646 @
>>> https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
>>> Patchwork_127130v2: 85d33d0ad82a5c1a71492f14a5ceb67ada6a22d8 @
>>> git://anongit.freedesktop.org/gfx-ci/linux
>>>
>>>
>>> Linux commits
>>>
>>> 43f210e851cd drm/i915/display: Check GGTT to determine phys_base
>>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] â Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2)
2023-12-08 8:32 ` [Intel-gfx] â " Andrzej Hajda
@ 2023-12-13 16:03 ` Ville Syrjälä
0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2023-12-13 16:03 UTC (permalink / raw)
To: Andrzej Hajda; +Cc: intel-gfx, Rodrigo Vivi, Paz Zcharya
On Fri, Dec 08, 2023 at 09:32:00AM +0100, Andrzej Hajda wrote:
> On 07.12.2023 12:26, Andrzej Hajda wrote:
> > On 07.12.2023 11:10, Andrzej Hajda wrote:
> >
> > After taking a look on panic log [1], I have found:
> > [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT for GuC
> >
> > I don't know why it is only debug level? It seems serious failure, as a
> > result i915_init_ggtt fails and probe fails.
> >
> > The cause is that initial framebuffer is located at the end of GGTT and
> > it overlaps with reserved area (see ggtt_reserve_guc_top).
> >
> > I am not sure how it can be properly fixed, I guess dirty fix could be
> > just relocation of vma (hopefully into free area), sth like:
> > new_gte = gsm + (ggtt->vm.total - GUC_TOP_RESERVE_SIZE - size) /
> > I915_GTT_PAGE_SIZE;
> > memmove(new_gte, gte, size / I915_GTT_PAGE_SIZE);
> >
> > but I have no idea of possible side effects :)
>
> I looked once more into the code and maybe you can just pin the buffer
> to earlier address (not overlapping with GuC reservation and current vma
> of the fb):
> @@ -143,6 +143,9 @@ initial_plane_vma(struct drm_i915_private *i915,
> if (IS_ERR(vma))
> goto err_obj;
>
> + if (base + size > GUC_GGTT_TOP)
> + base = min(base, GUC_GGTT_TOP) - size;
> +
> pinctl = PIN_GLOBAL | PIN_OFFSET_FIXED | base;
> if (HAS_GMCH(i915))
> pinctl |= PIN_MAPPABLE;
This is not a solution. The correct fix is either:
1. fix the guc code to not insist on a fixed chunk of the ggtt
and instead allocate it normally like a good boy. It could
still try to allocate as high as possible to ideally
land in the GUC_GGTT_TOP range
2. relocate the display vma to a different part of the ggtt
1. should be far simpler by the looks of it, as 2. would involve:
a) pinning the same object into two places in the ggtt simultanously
I don't think we have support for that except for special ggtt views,
and xe doesn't have even that (should we need this trick there as well)
b) flip the plane(s) over to the relocated vma
c) wait for vblank(s)
d) discard the original vma
e) all of that would need to happen pretty early so we may not have
a lot of the required normal machinery fully up and running yet
Anyways, I ran into much of the same issues when debugging an MTL
machine. This is the result of my efforts (partially overlaps with
the proposed patch here):
https://patchwork.freedesktop.org/series/127721/
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-12-13 16:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-06 18:46 [Intel-gfx] [PATCH] [v2] drm/i915/display: Check GGTT to determine phys_base Paz Zcharya
2023-12-07 0:18 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2) Patchwork
2023-12-07 10:10 ` [Intel-gfx] â " Andrzej Hajda
2023-12-07 11:26 ` [Intel-gfx] â " Andrzej Hajda
2023-12-07 12:05 ` Imre Deak
2023-12-08 8:32 ` [Intel-gfx] â " Andrzej Hajda
2023-12-13 16:03 ` Ville Syrjälä
2023-12-08 1:00 ` [Intel-gfx] [PATCH] [v2] drm/i915/display: Check GGTT to determine phys_base Almahallawy, Khaled
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).