[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/mtl: Update cache coherency setting for context structure
== Series Details ==
Series: drm/i915/mtl: Update cache coherency setting for context structure
URL : https://patchwork.freedesktop.org/series/120315/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_13351_full -> Patchwork_120315v1_full
Summary
---
**FAILURE**
Serious unknown changes coming with Patchwork_120315v1_full absolutely need
to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_120315v1_full, please notify your bug team to allow
them
to document this new failure mode, which will reduce false positives in CI.
Participating hosts (9 -> 9)
--
No changes in participating hosts
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_120315v1_full:
### IGT changes ###
Possible regressions
* igt@i915_pm_rpm@gem-execbuf-stress:
- shard-dg2: NOTRUN -> [SKIP][1] +1 similar issue
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/shard-dg2-3/igt@i915_pm_...@gem-execbuf-stress.html
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a3:
- {shard-dg1}:NOTRUN -> [FAIL][2]
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/shard-dg1-12/igt@kms_flip@flip-vs-expired-vbl...@c-hdmi-a3.html
New tests
-
New tests have been introduced between CI_DRM_13351_full and
Patchwork_120315v1_full:
### New IGT tests (16) ###
* igt@kms_flip@2x-blocking-absolute-wf_vblank@ab-vga1-hdmi-a1:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@2x-blocking-wf_vblank@ab-vga1-hdmi-a1:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@2x-dpms-vs-vblank-race@ab-vga1-hdmi-a1:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@2x-flip-vs-rmfb@ab-vga1-hdmi-a1:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@2x-flip-vs-suspend@ab-vga1-hdmi-a1:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@2x-plain-flip@ab-vga1-hdmi-a1:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@bo-too-big-interruptible@d-hdmi-a2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@dpms-vs-vblank-race@d-hdmi-a2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@flip-vs-expired-vblank@d-hdmi-a2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@flip-vs-panning@a-dp2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@flip-vs-panning@b-dp2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@flip-vs-panning@c-dp2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@flip-vs-rmfb@d-hdmi-a2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
*
igt@kms_flip@single-buffer-flip-vs-dpms-off-vs-modeset-interruptible@d-hdmi-a2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@wf_vblank-ts-check-interruptible@d-hdmi-a2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
* igt@kms_flip@wf_vblank-ts-check@d-hdmi-a2:
- Statuses : 1 pass(s)
- Exec time: [0.0] s
Known issues
Here are the changes found in Patchwork_120315v1_full that come from known
issues:
### IGT changes ###
Issues hit
* igt@drm_fdinfo@busy-hang@bcs0:
- shard-dg2: NOTRUN -> [SKIP][3] ([i915#8414]) +10 similar issues
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/shard-dg2-6/igt@drm_fdinfo@busy-h...@bcs0.html
* igt@gem_basic@multigpu-create-close:
- shard-dg2: NOTRUN -> [SKIP][4] ([i915#7697])
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/shard-dg2-6/igt@gem_ba...@multigpu-create-close.html
* igt@gem_eio@hibernate:
- shard-dg2: NOTRUN -> [ABORT][5] ([i915#7975] / [i915#8213])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/shard-dg2-6/igt@gem_...@hibernate.html
* igt@gem_exec_balancer@bonded-pair:
- shard-dg2: NOTRUN -> [SKIP][6] ([i915#4771])
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/shard-dg2-6/igt@gem_exec_balan...@bonded-pair.html
* igt@gem_exec_balancer@full-pulse:
- shard-dg2: [PASS][7] -> [FAIL][8] ([i915#6032])
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-dg2-5/igt@gem_exec_balan...@full-pulse.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/shard-dg2-2/igt@gem_exec_balan...@full-pulse.html
* igt@gem_exec_balancer@sliced:
- shard-dg2: NOTRUN -> [SKIP][9] ([i915#4812])
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/shard-dg2-6/igt@gem_exec_balan...@sliced.html
*
Re: [Intel-gfx] ✗ Fi.CI.BUILD: failure for PCI/VGA: Improve the default VGA device selection
Hi, This patch set could only be applied if a prerequisite patch[1] set applied [1] https://patchwork.freedesktop.org/series/120053/ Sorry, its my bad, I will respin my patch. On 2023/7/7 03:34, Patchwork wrote: == Series Details == Series: PCI/VGA: Improve the default VGA device selection URL : https://patchwork.freedesktop.org/series/120294/ State : failure == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/120294/revisions/1/mbox/ not applied Applying: video/aperture: Add a helper to detect if an aperture contains firmware FB Applying: PCI/VGA: Improve the default VGA device selection error: sha1 information is lacking or useless (drivers/pci/vgaarb.c). error: could not build fake ancestor hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0002 PCI/VGA: Improve the default VGA device selection When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Build failed, no error log produced
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gt: update request engine before removing virtual GuC engine (rev2)
== Series Details == Series: drm/i915/gt: update request engine before removing virtual GuC engine (rev2) URL : https://patchwork.freedesktop.org/series/120238/ State : failure == Summary == CI Bug Log - changes from CI_DRM_13351_full -> Patchwork_120238v2_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_120238v2_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_120238v2_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (9 -> 10) -- Additional (1): shard-tglu0 Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_120238v2_full: ### IGT changes ### Possible regressions * igt@gem_exec_reloc@basic-wc-cpu-active: - shard-apl: [PASS][1] -> [DMESG-WARN][2] +4 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-apl1/igt@gem_exec_re...@basic-wc-cpu-active.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/shard-apl1/igt@gem_exec_re...@basic-wc-cpu-active.html * igt@kms_plane_multiple@tiling-y: - shard-dg2: NOTRUN -> [SKIP][3] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/shard-dg2-11/igt@kms_plane_multi...@tiling-y.html New tests - New tests have been introduced between CI_DRM_13351_full and Patchwork_120238v2_full: ### New IGT tests (34) ### * igt@gem_exec_basic@basic@bcs0-lmem0: - Statuses : 2 pass(s) - Exec time: [0.0] s * igt@gem_exec_basic@basic@rcs0-lmem0: - Statuses : 2 pass(s) - Exec time: [0.0] s * igt@gem_exec_basic@basic@vcs0-lmem0: - Statuses : 2 pass(s) - Exec time: [0.0] s * igt@gem_exec_basic@basic@vcs1-lmem0: - Statuses : 2 pass(s) - Exec time: [0.0] s * igt@gem_exec_basic@basic@vecs0-lmem0: - Statuses : 2 pass(s) - Exec time: [0.0] s * igt@kms_flip@2x-blocking-absolute-wf_vblank@ab-vga1-hdmi-a1: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@2x-blocking-wf_vblank@ab-vga1-hdmi-a1: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@2x-dpms-vs-vblank-race@ab-vga1-hdmi-a1: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@2x-flip-vs-panning-interruptible@ab-vga1-hdmi-a1: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@2x-modeset-vs-vblank-race@ab-vga1-hdmi-a1: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@2x-plain-flip-ts-check@ab-vga1-hdmi-a1: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@basic-flip-vs-dpms@d-hdmi-a2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@blocking-absolute-wf_vblank@d-hdmi-a2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@bo-too-big@a-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@bo-too-big@b-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@bo-too-big@c-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@flip-vs-expired-vblank-interruptible@d-hdmi-a2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@flip-vs-panning-vs-hang@d-hdmi-a2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@flip-vs-panning@a-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@flip-vs-panning@b-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@flip-vs-panning@c-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@flip-vs-suspend@a-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@flip-vs-suspend@b-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@flip-vs-suspend@c-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@modeset-vs-vblank-race-interruptible@a-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@modeset-vs-vblank-race-interruptible@b-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@modeset-vs-vblank-race-interruptible@c-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@nonexisting-fb-interruptible@a-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@nonexisting-fb-interruptible@b-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@nonexisting-fb-interruptible@c-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@nonexisting-fb@d-hdmi-a2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@plain-flip-fb-recreate@a-dp2: - Statuses : 1 pass(s) - Exec time: [0.0] s * igt@kms_flip@plain-flip-fb-recreate@b-dp2:
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gt: Convert to use time_before macro
== Series Details == Series: drm/i915/gt: Convert to use time_before macro URL : https://patchwork.freedesktop.org/series/120297/ State : success == Summary == CI Bug Log - changes from CI_DRM_13351_full -> Patchwork_120297v1_full Summary --- **SUCCESS** No regressions found. Participating hosts (9 -> 9) -- No changes in participating hosts Known issues Here are the changes found in Patchwork_120297v1_full that come from known issues: ### IGT changes ### Issues hit * igt@drm_fdinfo@virtual-busy-idle-all: - shard-dg2: NOTRUN -> [SKIP][1] ([i915#8414]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-6/igt@drm_fdi...@virtual-busy-idle-all.html * igt@gem_exec_balancer@full-pulse: - shard-dg2: [PASS][2] -> [FAIL][3] ([i915#6032]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-dg2-5/igt@gem_exec_balan...@full-pulse.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-1/igt@gem_exec_balan...@full-pulse.html * igt@gem_exec_fair@basic-none-rrul: - shard-dg2: NOTRUN -> [SKIP][4] ([i915#3539] / [i915#4852]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-6/igt@gem_exec_f...@basic-none-rrul.html * igt@gem_exec_fair@basic-pace-solo@rcs0: - shard-glk: [PASS][5] -> [FAIL][6] ([i915#2842]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-glk4/igt@gem_exec_fair@basic-pace-s...@rcs0.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-glk2/igt@gem_exec_fair@basic-pace-s...@rcs0.html * igt@gem_exec_reloc@basic-wc: - shard-dg2: NOTRUN -> [SKIP][7] ([i915#3281]) +3 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-6/igt@gem_exec_re...@basic-wc.html * igt@gem_exec_schedule@preempt-queue: - shard-dg2: NOTRUN -> [SKIP][8] ([i915#4537] / [i915#4812]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-6/igt@gem_exec_sched...@preempt-queue.html * igt@gem_exec_suspend@basic-s4-devices@lmem0: - shard-dg2: [PASS][9] -> [ABORT][10] ([i915#7975] / [i915#8213] / [i915#8682]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-dg2-11/igt@gem_exec_suspend@basic-s4-devi...@lmem0.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-1/igt@gem_exec_suspend@basic-s4-devi...@lmem0.html * igt@gem_exec_suspend@basic-s4-devices@smem: - shard-tglu: [PASS][11] -> [ABORT][12] ([i915#7975] / [i915#8213]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-tglu-3/igt@gem_exec_suspend@basic-s4-devi...@smem.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-tglu-10/igt@gem_exec_suspend@basic-s4-devi...@smem.html * igt@gem_exec_whisper@basic-fds-all: - shard-mtlp: [PASS][13] -> [FAIL][14] ([i915#6363]) +1 similar issue [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-mtlp-8/igt@gem_exec_whis...@basic-fds-all.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-mtlp-5/igt@gem_exec_whis...@basic-fds-all.html * igt@gem_fence_thrash@bo-write-verify-y: - shard-dg2: NOTRUN -> [SKIP][15] ([i915#4860]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-6/igt@gem_fence_thr...@bo-write-verify-y.html * igt@gem_lmem_swapping@parallel-random-verify-ccs: - shard-glk: NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#4613]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-glk9/igt@gem_lmem_swapp...@parallel-random-verify-ccs.html * igt@gem_mmap_wc@pf-nonblock: - shard-dg2: NOTRUN -> [SKIP][17] ([i915#4083]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-6/igt@gem_mmap...@pf-nonblock.html * igt@gem_pwrite@basic-exhaustion: - shard-snb: NOTRUN -> [WARN][18] ([i915#2658]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-snb5/igt@gem_pwr...@basic-exhaustion.html * igt@gem_tiled_pread_basic: - shard-dg2: NOTRUN -> [SKIP][19] ([i915#4079]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-6/igt@gem_tiled_pread_basic.html * igt@gem_tiled_wb: - shard-dg2: NOTRUN -> [SKIP][20] ([i915#4077]) +4 similar issues [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-6/igt@gem_tiled_wb.html * igt@gem_userptr_blits@forbidden-operations: - shard-dg2: NOTRUN -> [SKIP][21] ([i915#3282]) +1 similar issue [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/shard-dg2-6/igt@gem_userptr_bl...@forbidden-operations.html *
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read
== Series Details ==
Series: drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read
URL : https://patchwork.freedesktop.org/series/120296/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_13351_full -> Patchwork_120296v1_full
Summary
---
**FAILURE**
Serious unknown changes coming with Patchwork_120296v1_full absolutely need
to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_120296v1_full, please notify your bug team to allow
them
to document this new failure mode, which will reduce false positives in CI.
Participating hosts (9 -> 9)
--
No changes in participating hosts
Possible new issues
---
Here are the unknown changes that may have been introduced in
Patchwork_120296v1_full:
### IGT changes ###
Possible regressions
* igt@perf_pmu@busy-idle-no-semaphores@rcs0:
- shard-rkl: [PASS][1] -> [ABORT][2]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-rkl-1/igt@perf_pmu@busy-idle-no-semapho...@rcs0.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-rkl-7/igt@perf_pmu@busy-idle-no-semapho...@rcs0.html
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_flip@flip-vs-suspend-interruptible@d-hdmi-a4:
- {shard-dg1}:NOTRUN -> [ABORT][3]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-dg1-14/igt@kms_flip@flip-vs-suspend-interrupti...@d-hdmi-a4.html
Known issues
Here are the changes found in Patchwork_120296v1_full that come from known
issues:
### IGT changes ###
Issues hit
* igt@drm_fdinfo@most-busy-idle-check-all@rcs0:
- shard-rkl: [PASS][4] -> [FAIL][5] ([i915#7742])
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-rkl-6/igt@drm_fdinfo@most-busy-idle-check-...@rcs0.html
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-rkl-1/igt@drm_fdinfo@most-busy-idle-check-...@rcs0.html
* igt@drm_fdinfo@virtual-busy-idle-all:
- shard-dg2: NOTRUN -> [SKIP][6] ([i915#8414])
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-dg2-2/igt@drm_fdi...@virtual-busy-idle-all.html
* igt@gem_ctx_isolation@preservation-s3@bcs0:
- shard-dg2: [PASS][7] -> [FAIL][8] ([fdo#103375] / [i915#6121])
+3 similar issues
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-dg2-3/igt@gem_ctx_isolation@preservation...@bcs0.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-dg2-5/igt@gem_ctx_isolation@preservation...@bcs0.html
* igt@gem_exec_balancer@full-pulse:
- shard-dg2: [PASS][9] -> [FAIL][10] ([i915#6032])
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-dg2-5/igt@gem_exec_balan...@full-pulse.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-dg2-11/igt@gem_exec_balan...@full-pulse.html
* igt@gem_exec_fair@basic-none-rrul:
- shard-dg2: NOTRUN -> [SKIP][11] ([i915#3539] / [i915#4852])
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-dg2-2/igt@gem_exec_f...@basic-none-rrul.html
* igt@gem_exec_fair@basic-none@vcs0:
- shard-rkl: [PASS][12] -> [FAIL][13] ([i915#2842])
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-rkl-2/igt@gem_exec_fair@basic-n...@vcs0.html
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-rkl-7/igt@gem_exec_fair@basic-n...@vcs0.html
* igt@gem_exec_fair@basic-pace-solo@rcs0:
- shard-glk: [PASS][14] -> [FAIL][15] ([i915#2842])
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-glk4/igt@gem_exec_fair@basic-pace-s...@rcs0.html
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-glk9/igt@gem_exec_fair@basic-pace-s...@rcs0.html
* igt@gem_exec_reloc@basic-wc:
- shard-dg2: NOTRUN -> [SKIP][16] ([i915#3281]) +3 similar issues
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-dg2-2/igt@gem_exec_re...@basic-wc.html
* igt@gem_exec_schedule@deep@vecs0:
- shard-mtlp: [PASS][17] -> [FAIL][18] ([i915#8606])
[17]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/shard-mtlp-3/igt@gem_exec_schedule@d...@vecs0.html
[18]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-mtlp-8/igt@gem_exec_schedule@d...@vecs0.html
* igt@gem_exec_schedule@preempt-queue:
- shard-dg2: NOTRUN -> [SKIP][19] ([i915#4537] / [i915#4812])
[19]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/shard-dg2-2/igt@gem_exec_sched...@preempt-queue.html
* igt@gem_exec_suspend@basic-s4-devices@lmem0:
- shard-dg2:
Re: [Intel-gfx] [PATCH v2] drm/i915: Refactor PAT/cache handling
> @@ -27,15 +28,8 @@ static bool gpu_write_needs_clflush(struct
> drm_i915_gem_object *obj)
> if (IS_DGFX(i915))
> return false;
> - /*
> -* For objects created by userspace through GEM_CREATE with
> pat_index
> -* set by set_pat extension, i915_gem_object_has_cache_level()
> will
> -* always return true, because the coherency of such object is
> managed
> -* by userspace. Othereise the call here would fall back to
> checking
> -* whether the object is un-cached or write-through.
> -*/
> - return !(i915_gem_object_has_cache_level(obj, I915_CACHE_NONE) ||
> -i915_gem_object_has_cache_level(obj, I915_CACHE_WT));
> + return i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_UC) !=
> 1 &&
> + i915_gem_object_has_cache_mode(obj, I915_CACHE_MODE_WT) !=
> 1;
This logic was changed for objects with pat index set by user here. It
used to return false regardless the cache mode. But now it returns true
if its cache mode is neither UC nor WT.
>>>
>>> Yes, that was half of the motivation of the refactory. Before the PAT
>>> index series code was like this:
>>>
>>>return !(obj->cache_level == I915_CACHE_NONE ||
>>> obj->cache_level == I915_CACHE_WT);
>>> So kernel knew it needs to flush only if caching mode is neither UC or WT.
>>> With the PAT index series you changed it to:
>>>
>>>return !(i915_gem_object_has_cache_level(obj, I915_CACHE_NONE) ||
>>> i915_gem_object_has_cache_level(obj, I915_CACHE_WT));
>>> And as i915_gem_object_has_cache_level was changed to always return true
>>> if PAT was set by user, that made the check meaningless for such objects.
>>
>> But the point is that the KMD should not flush the cache for objects
>> with PAT set by user because UMD is handling the cache conherency.
>> That is the design. Well doing so wouldn't cause functional issue, but
>> it's unecessary and might have performance impact.
>
> Not all i915_gem_object_has_cache_level() are even about flushing the cache
> and if the kernel doesn't know what is behind a PAT index
> (i915_gem_object_has_cache_level lies by always returning true) are we 100%
> sure everything is functionally correct?
>
> flush_write_domain() for instance uses it to determine whether to set
> obj->cache_dirty after GPU activity. How does the UMD manage that?
>
> Then use_cpu_reloc(). Another pointless/misleading question.
>
> Finally vm_fault_gtt() rejects access based on it.
>
> Perhaps the question is moot since the set pat extension is restricted to
> MTL so some other conditions used in the above checks, like HAS_LLC and such,
> make for no practical difference. Even if so, what if the extension was
> allowed
> on other platforms as it was the plan until it was discovered there is no
> userspace code for other platforms. Would the plan work on all platforms? And
> even if it would I think the implementation is very non-obvious.
>
Understand your point, perhaps we should let i915_gem_object_has_cache_mode()
do what it supposed to do, and add a separate check for obj->pat_set_by_user
in functions like gpu_write_needs_clflush(), use_cpu_reloc(), etc. Anyway,
the design is to let UMD handle coherency for objects with pat set by user.
>>> With my refactoring it becomes meaningful again and restores to the
>>> original behaviour. That's the intent at least.
>>>
> bool i915_gem_cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
> @@ -255,9 +249,9 @@ i915_gem_object_set_to_gtt_domain(struct
> drm_i915_gem_object *obj, bool write)
> }
>
> /**
> - * i915_gem_object_set_cache_level - Changes the cache-level of an
> object across all VMA.
> + * i915_gem_object_set_cache - Changes the cache-level of an object
> across all VMA.
[...]
> - if (i915_gem_object_has_cache_level(obj, cache_level))
> + ret = i915_cache_find_pat(i915, cache);
> + if (ret < 0) {
> + struct drm_printer p =
> +drm_err_printer("Attempting to use unknown caching mode
> ");
> +
> + i915_cache_print(, cache);
> + drm_puts(, "!\n");
> +
> + return -EINVAL;
> + } else if (ret == obj->pat_index) {
> return 0;
We will have to do this conversion and checking again later in this
function when calling i915_gem_object_set_cache_coherency().
>>>
>>> Yes the double lookup part is a bit naff. It is not super required
>>> apart for validating internal callers (could be a debug build only
>>> feature perhaps) and to see if PAT index has changed and so whether
>>> it needs to call i915_gem_object_wait before proceeding to
>>> i915_gem_object_set_cache_coherency...
>>>
My thought was to simply remove the use of
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/mtl: Update cache coherency setting for context structure
== Series Details ==
Series: drm/i915/mtl: Update cache coherency setting for context structure
URL : https://patchwork.freedesktop.org/series/120315/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13351 -> Patchwork_120315v1
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/index.html
Participating hosts (40 -> 39)
--
Additional (1): fi-pnv-d510
Missing(2): fi-kbl-soraka fi-snb-2520m
Known issues
Here are the changes found in Patchwork_120315v1 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_selftest@live@slpc:
- bat-mtlp-6: [PASS][1] -> [DMESG-WARN][2] ([i915#6367])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-mtlp-6/igt@i915_selftest@l...@slpc.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/bat-mtlp-6/igt@i915_selftest@l...@slpc.html
- bat-rpls-2: NOTRUN -> [DMESG-WARN][3] ([i915#6367])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/bat-rpls-2/igt@i915_selftest@l...@slpc.html
* igt@kms_psr@primary_page_flip:
- fi-pnv-d510:NOTRUN -> [SKIP][4] ([fdo#109271]) +38 similar issues
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/fi-pnv-d510/igt@kms_psr@primary_page_flip.html
* igt@kms_psr@sprite_plane_onoff:
- bat-rplp-1: NOTRUN -> [ABORT][5] ([i915#8442] / [i915#8668] /
[i915#8712])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html
Possible fixes
* igt@i915_selftest@live@gt_heartbeat:
- bat-dg2-9: [DMESG-FAIL][6] ([i915#7699]) -> [PASS][7]
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@migrate:
- bat-atsm-1: [DMESG-FAIL][8] ([i915#7699] / [i915#7913]) ->
[PASS][9]
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-atsm-1/igt@i915_selftest@l...@migrate.html
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/bat-atsm-1/igt@i915_selftest@l...@migrate.html
* igt@i915_selftest@live@mman:
- bat-rpls-2: [TIMEOUT][10] ([i915#6794] / [i915#7392]) ->
[PASS][11]
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-rpls-2/igt@i915_selftest@l...@mman.html
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/bat-rpls-2/igt@i915_selftest@l...@mman.html
Warnings
* igt@core_auth@basic-auth:
- bat-adlp-11:[ABORT][12] ([i915#8011]) -> [ABORT][13] ([i915#4423]
/ [i915#8011])
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-adlp-11/igt@core_a...@basic-auth.html
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/bat-adlp-11/igt@core_a...@basic-auth.html
* igt@kms_psr@cursor_plane_move:
- bat-rplp-1: [ABORT][14] ([i915#8434]) -> [SKIP][15] ([i915#1072])
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-rplp-1/igt@kms_psr@cursor_plane_move.html
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120315v1/bat-rplp-1/igt@kms_psr@cursor_plane_move.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
[i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
[i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
[i915#8434]: https://gitlab.freedesktop.org/drm/intel/issues/8434
[i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
[i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
[i915#8712]: https://gitlab.freedesktop.org/drm/intel/issues/8712
Build changes
-
* Linux: CI_DRM_13351 -> Patchwork_120315v1
CI-20190529: 20190529
CI_DRM_13351: c5262da740fe00ef30394118a108dcf0723a0318 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_7376: 38af51c0ce6d9573793f53fc32782b77061bf169 @
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_120315v1: c5262da740fe00ef30394118a108dcf0723a0318 @
git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
8c01f072f367 drm/i915/mtl:
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: update request engine before removing virtual GuC engine (rev2)
== Series Details ==
Series: drm/i915/gt: update request engine before removing virtual GuC engine
(rev2)
URL : https://patchwork.freedesktop.org/series/120238/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13351 -> Patchwork_120238v2
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/index.html
Participating hosts (40 -> 39)
--
Additional (1): fi-pnv-d510
Missing(2): fi-kbl-soraka fi-snb-2520m
Known issues
Here are the changes found in Patchwork_120238v2 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_selftest@live@hangcheck:
- bat-rpls-1: [PASS][1] -> [ABORT][2] ([i915#7677])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-rpls-1/igt@i915_selftest@l...@hangcheck.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/bat-rpls-1/igt@i915_selftest@l...@hangcheck.html
* igt@kms_pipe_crc_basic@suspend-read-crc@pipe-c-dp-1:
- fi-kbl-7567u: [PASS][3] -> [ABORT][4] ([i915#8218])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/fi-kbl-7567u/igt@kms_pipe_crc_basic@suspend-read-...@pipe-c-dp-1.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/fi-kbl-7567u/igt@kms_pipe_crc_basic@suspend-read-...@pipe-c-dp-1.html
* igt@kms_psr@primary_page_flip:
- fi-pnv-d510:NOTRUN -> [SKIP][5] ([fdo#109271]) +38 similar issues
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/fi-pnv-d510/igt@kms_psr@primary_page_flip.html
* igt@kms_psr@sprite_plane_onoff:
- bat-rplp-1: NOTRUN -> [ABORT][6] ([i915#8442] / [i915#8668] /
[i915#8712])
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html
Possible fixes
* igt@i915_selftest@live@gt_heartbeat:
- bat-dg2-9: [DMESG-FAIL][7] ([i915#7699]) -> [PASS][8]
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@migrate:
- bat-atsm-1: [DMESG-FAIL][9] ([i915#7699] / [i915#7913]) ->
[PASS][10]
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-atsm-1/igt@i915_selftest@l...@migrate.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/bat-atsm-1/igt@i915_selftest@l...@migrate.html
* igt@i915_selftest@live@mman:
- bat-rpls-2: [TIMEOUT][11] ([i915#6794] / [i915#7392]) ->
[PASS][12]
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-rpls-2/igt@i915_selftest@l...@mman.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/bat-rpls-2/igt@i915_selftest@l...@mman.html
Warnings
* igt@i915_module_load@load:
- bat-adlp-11:[DMESG-WARN][13] ([i915#4423]) -> [ABORT][14]
([i915#4423])
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-adlp-11/igt@i915_module_l...@load.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/bat-adlp-11/igt@i915_module_l...@load.html
* igt@kms_psr@cursor_plane_move:
- bat-rplp-1: [ABORT][15] ([i915#8434]) -> [SKIP][16] ([i915#1072])
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-rplp-1/igt@kms_psr@cursor_plane_move.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120238v2/bat-rplp-1/igt@kms_psr@cursor_plane_move.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
[i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
[i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
[i915#7677]: https://gitlab.freedesktop.org/drm/intel/issues/7677
[i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#8218]: https://gitlab.freedesktop.org/drm/intel/issues/8218
[i915#8434]: https://gitlab.freedesktop.org/drm/intel/issues/8434
[i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
[i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
[i915#8712]: https://gitlab.freedesktop.org/drm/intel/issues/8712
Build changes
-
* Linux: CI_DRM_13351 -> Patchwork_120238v2
CI-20190529: 20190529
CI_DRM_13351: c5262da740fe00ef30394118a108dcf0723a0318 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_7376:
[Intel-gfx] ✗ Fi.CI.IGT: failure for Covert to use time_before macro
== Series Details == Series: Covert to use time_before macro URL : https://patchwork.freedesktop.org/series/120295/ State : failure == Summary == CI Bug Log - changes from CI_DRM_13349_full -> Patchwork_120295v1_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_120295v1_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_120295v1_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (10 -> 10) -- No changes in participating hosts Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_120295v1_full: ### IGT changes ### Possible regressions * igt@gem_ccs@ctrl-surf-copy-new-ctx: - shard-mtlp: NOTRUN -> [SKIP][1] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-mtlp-7/igt@gem_...@ctrl-surf-copy-new-ctx.html * igt@kms_rotation_crc@multiplane-rotation-cropping-top: - shard-rkl: [PASS][2] -> [ABORT][3] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/shard-rkl-7/igt@kms_rotation_...@multiplane-rotation-cropping-top.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-rkl-1/igt@kms_rotation_...@multiplane-rotation-cropping-top.html Known issues Here are the changes found in Patchwork_120295v1_full that come from known issues: ### IGT changes ### Issues hit * igt@drm_fdinfo@most-busy-idle-check-all@rcs0: - shard-rkl: [PASS][4] -> [FAIL][5] ([i915#7742]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/shard-rkl-1/igt@drm_fdinfo@most-busy-idle-check-...@rcs0.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-rkl-2/igt@drm_fdinfo@most-busy-idle-check-...@rcs0.html * igt@gem_eio@in-flight-contexts-1us: - shard-mtlp: [PASS][6] -> [ABORT][7] ([i915#8503]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/shard-mtlp-2/igt@gem_...@in-flight-contexts-1us.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-mtlp-6/igt@gem_...@in-flight-contexts-1us.html * igt@gem_eio@kms: - shard-dg2: [PASS][8] -> [INCOMPLETE][9] ([i915#1982] / [i915#7892]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/shard-dg2-6/igt@gem_...@kms.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-dg2-8/igt@gem_...@kms.html * igt@gem_exec_endless@dispatch@vcs1: - shard-tglu: [PASS][10] -> [TIMEOUT][11] ([i915#3778]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/shard-tglu-5/igt@gem_exec_endless@dispa...@vcs1.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-tglu-6/igt@gem_exec_endless@dispa...@vcs1.html * igt@gem_exec_fair@basic-deadline: - shard-glk: [PASS][12] -> [FAIL][13] ([i915#2846]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/shard-glk7/igt@gem_exec_f...@basic-deadline.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-glk9/igt@gem_exec_f...@basic-deadline.html * igt@gem_exec_reloc@basic-gtt-noreloc: - shard-mtlp: NOTRUN -> [SKIP][14] ([i915#3281]) +1 similar issue [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-mtlp-7/igt@gem_exec_re...@basic-gtt-noreloc.html * igt@gem_exec_whisper@basic-contexts-forked-all: - shard-mtlp: [PASS][15] -> [ABORT][16] ([i915#8131]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/shard-mtlp-7/igt@gem_exec_whis...@basic-contexts-forked-all.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-mtlp-7/igt@gem_exec_whis...@basic-contexts-forked-all.html * igt@gem_exec_whisper@basic-normal-all: - shard-mtlp: [PASS][17] -> [FAIL][18] ([i915#6363]) +2 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/shard-mtlp-1/igt@gem_exec_whis...@basic-normal-all.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-mtlp-5/igt@gem_exec_whis...@basic-normal-all.html * igt@gem_gpgpu_fill@basic@smem: - shard-mtlp: NOTRUN -> [FAIL][19] ([i915#5464]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-mtlp-7/igt@gem_gpgpu_fill@ba...@smem.html * igt@gem_media_vme: - shard-mtlp: NOTRUN -> [SKIP][20] ([i915#284]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-mtlp-7/igt@gem_media_vme.html * igt@gem_pread@self: - shard-mtlp: NOTRUN -> [SKIP][21] ([i915#3282]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/shard-mtlp-7/igt@gem_pr...@self.html
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: update request engine before removing virtual GuC engine (rev2)
== Series Details == Series: drm/i915/gt: update request engine before removing virtual GuC engine (rev2) URL : https://patchwork.freedesktop.org/series/120238/ State : warning == Summary == Error: dim checkpatch failed 35bc390bc9f2 drm/i915/gt: update request engine before removing virtual GuC engine -:13: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #13: i915 :00:02.0: [drm:i915_drop_caches_set [i915]] Dropping caches: 0x005c [0x005c] total: 0 errors, 1 warnings, 0 checks, 23 lines checked
[Intel-gfx] ✗ Fi.CI.BUILD: failure for PCI/VGA: Improve the default VGA device selection
== Series Details == Series: PCI/VGA: Improve the default VGA device selection URL : https://patchwork.freedesktop.org/series/120294/ State : failure == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/120294/revisions/1/mbox/ not applied Applying: video/aperture: Add a helper to detect if an aperture contains firmware FB Applying: PCI/VGA: Improve the default VGA device selection error: sha1 information is lacking or useless (drivers/pci/vgaarb.c). error: could not build fake ancestor hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0002 PCI/VGA: Improve the default VGA device selection When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Build failed, no error log produced
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Convert to use time_before macro
== Series Details == Series: drm/i915/gt: Convert to use time_before macro URL : https://patchwork.freedesktop.org/series/120297/ State : success == Summary == CI Bug Log - changes from CI_DRM_13351 -> Patchwork_120297v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/index.html Participating hosts (40 -> 39) -- Additional (1): fi-pnv-d510 Missing(2): fi-kbl-soraka fi-snb-2520m Known issues Here are the changes found in Patchwork_120297v1 that come from known issues: ### IGT changes ### Issues hit * igt@debugfs_test@basic-hwmon: - bat-adlp-11:NOTRUN -> [SKIP][1] ([i915#7456]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@debugfs_t...@basic-hwmon.html * igt@gem_tiled_pread_basic: - bat-adlp-11:NOTRUN -> [SKIP][2] ([i915#3282]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@gem_tiled_pread_basic.html * igt@i915_pm_backlight@basic-brightness: - bat-adlp-11:NOTRUN -> [SKIP][3] ([i915#3546] / [i915#7561]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@i915_pm_backli...@basic-brightness.html * igt@i915_pm_rpm@basic-pci-d3-state: - bat-adlp-11:NOTRUN -> [ABORT][4] ([i915#7977] / [i915#8434] / [i915#8668]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@i915_pm_...@basic-pci-d3-state.html * igt@i915_selftest@live@execlists: - fi-bsw-nick:[PASS][5] -> [ABORT][6] ([i915#7911] / [i915#7913]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/fi-bsw-nick/igt@i915_selftest@l...@execlists.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/fi-bsw-nick/igt@i915_selftest@l...@execlists.html * igt@i915_selftest@live@gt_mocs: - bat-mtlp-6: [PASS][7] -> [DMESG-FAIL][8] ([i915#7059]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html * igt@i915_selftest@live@reset: - bat-rpls-1: [PASS][9] -> [ABORT][10] ([i915#4983] / [i915#7461] / [i915#8347] / [i915#8384]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-rpls-1/igt@i915_selftest@l...@reset.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-rpls-1/igt@i915_selftest@l...@reset.html * igt@i915_selftest@live@slpc: - bat-mtlp-6: [PASS][11] -> [DMESG-WARN][12] ([i915#6367]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-mtlp-6/igt@i915_selftest@l...@slpc.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-mtlp-6/igt@i915_selftest@l...@slpc.html * igt@kms_chamelium_frames@hdmi-crc-fast: - bat-adlp-11:NOTRUN -> [SKIP][13] ([i915#7828]) +7 similar issues [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@kms_chamelium_fra...@hdmi-crc-fast.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - bat-adlp-11:NOTRUN -> [SKIP][14] ([i915#4103]) +1 similar issue [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html * igt@kms_flip@basic-plain-flip@b-dp6: - bat-adlp-11:NOTRUN -> [FAIL][15] ([i915#6121]) +4 similar issues [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@kms_flip@basic-plain-f...@b-dp6.html * igt@kms_flip@basic-plain-flip@c-dp5: - bat-adlp-11:NOTRUN -> [DMESG-WARN][16] ([i915#6868]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@kms_flip@basic-plain-f...@c-dp5.html * igt@kms_force_connector_basic@prune-stale-modes: - bat-adlp-11:NOTRUN -> [SKIP][17] ([i915#4093]) +3 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@kms_force_connector_ba...@prune-stale-modes.html * igt@kms_frontbuffer_tracking@basic: - bat-adlp-11:NOTRUN -> [SKIP][18] ([i915#4342]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@kms_frontbuffer_track...@basic.html * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12: - bat-adlp-11:NOTRUN -> [SKIP][19] ([i915#3546]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120297v1/bat-adlp-11/igt@kms_pipe_crc_ba...@compare-crc-sanitycheck-nv12.html * igt@kms_psr@primary_mmap_gtt: - bat-rplp-1: NOTRUN -> [ABORT][20] ([i915#8434] / [i915#8442] / [i915#8668]) [20]:
[Intel-gfx] ✗ Fi.CI.BUILD: failure for PCI/VGA: Improve the default VGA device selection
== Series Details == Series: PCI/VGA: Improve the default VGA device selection URL : https://patchwork.freedesktop.org/series/120293/ State : failure == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/120293/revisions/1/mbox/ not applied Applying: video/aperture: Add a helper to detect if an aperture contains firmware FB Applying: PCI/VGA: Improve the default VGA device selection error: sha1 information is lacking or useless (drivers/pci/vgaarb.c). error: could not build fake ancestor hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0002 PCI/VGA: Improve the default VGA device selection When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Build failed, no error log produced
Re: [Intel-gfx] [PATCH] drm/i915/mtl: Update cache coherency setting for context structure
> As context structure is shared memory for CPU/GPU, Wa_22016122933 is
> needed for this memory block as well.
>
> Signed-off-by: Zhanjun Dong
> CC: Fei Yang
Reviewed-by: Fei Yang
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a4ec20aaafe2..1b710102390b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1092,8 +1092,15 @@ __lrc_alloc_state(struct intel_context *ce, struct
> intel_engine_cs *engine)
>
> obj = i915_gem_object_create_lmem(engine->i915, context_size,
>I915_BO_ALLOC_PM_VOLATILE);
> - if (IS_ERR(obj))
> + if (IS_ERR(obj)) {
> obj = i915_gem_object_create_shmem(engine->i915,
> context_size);
> + /*
> +* Wa_22016122933: For MTL the shared memory needs to be
> mapped
> +* as WC on CPU side and UC (PAT index 2) on GPU side
> +*/
> + if (IS_METEORLAKE(engine->i915))
> + i915_gem_object_set_cache_coherency(obj,
> I915_CACHE_NONE);
> + }
> if (IS_ERR(obj))
> return ERR_CAST(obj);
>
> --
> 2.34.1
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read
== Series Details ==
Series: drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read
URL : https://patchwork.freedesktop.org/series/120296/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13351 -> Patchwork_120296v1
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/index.html
Participating hosts (40 -> 39)
--
Additional (1): fi-pnv-d510
Missing(2): fi-kbl-soraka fi-snb-2520m
Known issues
Here are the changes found in Patchwork_120296v1 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [PASS][1] -> [DMESG-FAIL][2] ([i915#5334])
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
- fi-glk-j4005: [PASS][3] -> [DMESG-FAIL][4] ([i915#5334])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@reset:
- bat-rpls-2: NOTRUN -> [ABORT][5] ([i915#4983] / [i915#7461] /
[i915#7913] / [i915#8347])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/bat-rpls-2/igt@i915_selftest@l...@reset.html
* igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
- bat-rplp-1: [PASS][6] -> [ABORT][7] ([i915#8442] / [i915#8668])
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-seque...@pipe-d-edp-1.html
* igt@kms_psr@primary_page_flip:
- fi-pnv-d510:NOTRUN -> [SKIP][8] ([fdo#109271]) +38 similar issues
[8]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/fi-pnv-d510/igt@kms_psr@primary_page_flip.html
Possible fixes
* igt@i915_selftest@live@gt_heartbeat:
- bat-dg2-9: [DMESG-FAIL][9] ([i915#7699]) -> [PASS][10]
[9]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html
[10]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/bat-dg2-9/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@migrate:
- bat-atsm-1: [DMESG-FAIL][11] ([i915#7699] / [i915#7913]) ->
[PASS][12]
[11]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-atsm-1/igt@i915_selftest@l...@migrate.html
[12]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/bat-atsm-1/igt@i915_selftest@l...@migrate.html
* igt@i915_selftest@live@mman:
- bat-rpls-2: [TIMEOUT][13] ([i915#6794] / [i915#7392]) ->
[PASS][14]
[13]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-rpls-2/igt@i915_selftest@l...@mman.html
[14]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/bat-rpls-2/igt@i915_selftest@l...@mman.html
Warnings
* igt@core_auth@basic-auth:
- bat-adlp-11:[ABORT][15] ([i915#8011]) -> [ABORT][16] ([i915#4423]
/ [i915#8011])
[15]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13351/bat-adlp-11/igt@core_a...@basic-auth.html
[16]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120296v1/bat-adlp-11/igt@core_a...@basic-auth.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
[i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
[i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
[i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
[i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
[i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
[i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
Build changes
-
* Linux: CI_DRM_13351 -> Patchwork_120296v1
CI-20190529: 20190529
CI_DRM_13351: c5262da740fe00ef30394118a108dcf0723a0318 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_7376:
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read
== Series Details == Series: drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read URL : https://patchwork.freedesktop.org/series/120296/ State : warning == Summary == Error: dim checkpatch failed f8d5cc6a6ba8 drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read -:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #7: in i915_pmu_event_read. x86 CMPXCHG instruction returns success in ZF flag, total: 0 errors, 1 warnings, 0 checks, 16 lines checked
[Intel-gfx] ✗ Fi.CI.BUILD: failure for use refcount+RCU method to implement lockless slab shrink (rev2)
== Series Details == Series: use refcount+RCU method to implement lockless slab shrink (rev2) URL : https://patchwork.freedesktop.org/series/119926/ State : failure == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/119926/revisions/2/mbox/ not applied Applying: mm: shrinker: add shrinker::private_data field Applying: mm: vmscan: introduce some helpers for dynamically allocating shrinker Applying: drm/i915: dynamically allocate the i915_gem_mm shrinker Applying: drm/msm: dynamically allocate the drm-msm_gem shrinker Applying: drm/panfrost: dynamically allocate the drm-panfrost shrinker Applying: dm: dynamically allocate the dm-bufio shrinker Applying: dm zoned: dynamically allocate the dm-zoned-meta shrinker Applying: md/raid5: dynamically allocate the md-raid5 shrinker Applying: bcache: dynamically allocate the md-bcache shrinker Applying: vmw_balloon: dynamically allocate the vmw-balloon shrinker Applying: virtio_balloon: dynamically allocate the virtio-balloon shrinker Applying: mbcache: dynamically allocate the mbcache shrinker Applying: ext4: dynamically allocate the ext4-es shrinker Applying: jbd2, ext4: dynamically allocate the jbd2-journal shrinker Applying: NFSD: dynamically allocate the nfsd-client shrinker Applying: NFSD: dynamically allocate the nfsd-reply shrinker Applying: xfs: dynamically allocate the xfs-buf shrinker Applying: xfs: dynamically allocate the xfs-inodegc shrinker Applying: xfs: dynamically allocate the xfs-qm shrinker Applying: zsmalloc: dynamically allocate the mm-zspool shrinker Using index info to reconstruct a base tree... M mm/zsmalloc.c Falling back to patching base and 3-way merge... Auto-merging mm/zsmalloc.c Applying: fs: super: dynamically allocate the s_shrink Using index info to reconstruct a base tree... M fs/btrfs/super.c M fs/super.c M include/linux/fs.h Falling back to patching base and 3-way merge... Auto-merging include/linux/fs.h Auto-merging fs/super.c Auto-merging fs/btrfs/super.c Applying: drm/ttm: introduce pool_shrink_rwsem Applying: mm: shrinker: add refcount and completion_wait fields Applying: mm: vmscan: make global slab shrink lockless error: patch fragment without header at line 15: @@ -4725,15 +4725,7 @@ void __init mnt_init(void) error: could not build fake ancestor hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0024 mm: vmscan: make global slab shrink lockless When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Build failed, no error log produced
Re: [Intel-gfx] [PATCH] drm/i915: Fix the disabling sequence for Bigjoiner
On Thu, Jul 06, 2023 at 01:32:17PM +0300, Lisovskiy, Stanislav wrote: > On Thu, Jul 06, 2023 at 11:47:26AM +0300, Imre Deak wrote: > > On Thu, Jul 06, 2023 at 11:24:21AM +0300, Lisovskiy, Stanislav wrote: > > > On Wed, Jul 05, 2023 at 06:32:51PM +0300, Imre Deak wrote: > > > > On Thu, May 25, 2023 at 01:10:36PM +0300, Stanislav Lisovskiy wrote: > > > > > According to BSpec 49190, when enabling crtcs, we first setup > > > > > slave and then master crtc, however for disabling it should go > > > > > vice versa, i.e first master, then slave, however current code > > > > > does disabling in a same way as enabling. Fix this, by skipping > > > > > non-master crtcs, instead of non-slaves. > > > > > > > > > > Signed-off-by: Stanislav Lisovskiy > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > > index 0490c6412ab5..68958ba0ef49 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > @@ -6662,7 +6662,7 @@ static void > > > > > intel_commit_modeset_disables(struct intel_atomic_state *state) > > > > >*/ > > > > > if (!is_trans_port_sync_slave(old_crtc_state) && > > > > > !intel_dp_mst_is_slave_trans(old_crtc_state) && > > > > > - !intel_crtc_is_bigjoiner_slave(old_crtc_state)) > > > > > + !intel_crtc_is_bigjoiner_master(old_crtc_state)) > > > > > > > > I don't see what does this fix. The sequence is correct at the moment > > > > and this change would break it, leaving the encoder PLL enabled > > > > incorrectly when the encoder->post_pll_disable() hook is called. Hence > > > > it's NAK from side. > > > > > > Well, as I pointed out the BSpec 49190 instructs us to disable master > > > first, then slave. Current code skips all non-slaves in first cycle, > > > i.e it disables first slaves and then masters. Which is _wrong_. > > > > This is correct at the moment, followed in the encoder's disable hook > > which is only assigned to the master CRTC. > > Yep, I see now why it was implemented this way. > We basically handle everything in a single hook, taking care of the correct > sequence. As I understood otherwise we are going to have problems with the pll > subsystem, i.e we can't disable pll for master before the slaves(basically > means > our pll subsystem contradicts what the crtc/pipe/encoder sequence requires). > > I still think this is bery counterintuitive implementation, i.e when there is > a single > hook for master taking care of everything, while slaves are just noop. > This makes the whole thing very prone for screwing things up. > Ideally we should still have fully functional hooks for all slaves. > If the pll stuff requires special treatment, that probably should be dealt > somehow > separately(don't have any solution for that yet), but definitely we shouldn't > live > further like that. Things might get even more complicated in future. IMO what we should aim for is to call the high level crtc hooks only for the master crtc (ie. essentially the transcoder), and then iterate through the pipes inside the hooks at the approptiate points. To do that cleanly we want to split the code along the pipe-transcoder boundary as much as possible. -- Ville Syrjälä Intel
[Intel-gfx] [PATCH] drm/i915/mtl: Update cache coherency setting for context structure
As context structure is shared memory for CPU/GPU, Wa_22016122933 is
needed for this memory block as well.
Signed-off-by: Zhanjun Dong
CC: Fei Yang
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index a4ec20aaafe2..1b710102390b 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1092,8 +1092,15 @@ __lrc_alloc_state(struct intel_context *ce, struct
intel_engine_cs *engine)
obj = i915_gem_object_create_lmem(engine->i915, context_size,
I915_BO_ALLOC_PM_VOLATILE);
- if (IS_ERR(obj))
+ if (IS_ERR(obj)) {
obj = i915_gem_object_create_shmem(engine->i915, context_size);
+ /*
+* Wa_22016122933: For MTL the shared memory needs to be mapped
+* as WC on CPU side and UC (PAT index 2) on GPU side
+*/
+ if (IS_METEORLAKE(engine->i915))
+ i915_gem_object_set_cache_coherency(obj,
I915_CACHE_NONE);
+ }
if (IS_ERR(obj))
return ERR_CAST(obj);
--
2.34.1
Re: [Intel-gfx] [v2] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
> -Original Message-
> From: Bhadane, Dnyaneshwar
> Sent: Friday, June 30, 2023 4:40 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Ursulin, Tvrtko ; jani.nik...@linux.intel.com;
> Srivatsa, Anusha ; Bhadane, Dnyaneshwar
>
> Subject: [v2] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform
> defines
>
> Follow consistent naming convention. Replace MTL with METEORLAKE. Added
> defines that are replacing IS_MTL_GRAPHICS_STEP with
> IS_METEORLAKE_P_GRAPHICS_STEP and IS_METEORLAKE_M_GRAPHICS_STEP.
The patch also changes MTL_MEDIA macros. That should be mentioned in the commit
message.
> v2:
> - Replace IS_MLT_GRAPHICS_STEP with
Typo ^ s/MLT/MTL
> IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko).
> - Changed subject prefix mtl with MTL (Anusha)
Rewording:"mtl instead of MTL" or "use lower case instead of upper case"
Anusha
>
> Cc: Tvrtko Ursulin
> Cc: Jani Nikula
> Cc: Anusha Srivatsa
>
> Signed-off-by: Dnyaneshwar Bhadane
> ---
> drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
> drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +-
> drivers/gpu/drm/i915/display/intel_psr.c | 10 ++---
> .../drm/i915/display/skl_universal_plane.c| 4 +-
> drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++--
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
> .../drm/i915/gt/intel_execlists_submission.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 4 +-
> drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +-
> drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 44 +--
> drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 +-
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 +-
> drivers/gpu/drm/i915/i915_drv.h | 15 +--
> drivers/gpu/drm/i915/i915_perf.c | 4 +-
> 15 files changed, 60 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 7f8b2d7713c7..6358a8b26172 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1093,7 +1093,7 @@ static int intel_fbc_check_plane(struct
> intel_atomic_state *state,
>
> /* Wa_14016291713 */
> if ((IS_DISPLAY_VER(i915, 12, 13) ||
> - IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) &&
> + IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) &&
> crtc_state->has_psr) {
> plane_state->no_fbc_reason = "PSR1 enabled
> (Wa_14016291713)";
> return 0;
> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c
> b/drivers/gpu/drm/i915/display/intel_pmdemand.c
> index f7608d363634..8c3158b188ef 100644
> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
> @@ -92,7 +92,7 @@ int intel_pmdemand_init(struct drm_i915_private *i915)
>_state->base,
>_pmdemand_funcs);
>
> - if (IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0))
> + if (IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0))
> /* Wa_14016740474 */
> intel_de_rmw(i915, XELPD_CHICKEN_DCPR_3, 0,
> DMD_RSP_TIMEOUT_DISABLE);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 62151abe4748..ecd4e36119b2 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1247,7 +1247,7 @@ static void wm_optimization_wa(struct intel_dp
> *intel_dp,
> bool set_wa_bit = false;
>
> /* Wa_14015648006 */
> - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) ||
> + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) ||
> IS_DISPLAY_VER(dev_priv, 11, 13))
> set_wa_bit |= crtc_state->wm_level_disabled;
>
> @@ -1320,7 +1320,7 @@ static void intel_psr_enable_source(struct intel_dp
> *intel_dp,
>* All supported adlp panels have 1-based X granularity, this
> may
>* cause issues if non-supported panels are used.
>*/
> - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0))
> + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0,
> STEP_B0))
> intel_de_rmw(dev_priv,
> MTL_CHICKEN_TRANS(cpu_transcoder), 0,
>ADLP_1_BASED_X_GRANULARITY);
> else if (IS_ALDERLAKE_P(dev_priv))
> @@ -1328,7 +1328,7 @@ static void intel_psr_enable_source(struct intel_dp
> *intel_dp,
>ADLP_1_BASED_X_GRANULARITY);
>
> /* Wa_16012604467:adlp,mtl[a0,b0] */
> - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0))
> + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0,
> STEP_B0))
> intel_de_rmw(dev_priv,
>
Re: [Intel-gfx] [PATCH v2] drm/i915: Refactor PAT/cache handling
On 05/07/2023 01:09, Yang, Fei wrote: >>> From: Tvrtko Ursulin >>> >>> Informal commit message for now. >>> >>> I got a bit impatient and curious to see if the idea we discussed would >>> work so sketched something out. I think it is what I was describing back >>> then.. >>> >>> So high level idea is to teach the driver what caching modes are hidden >>> behind PAT indices. Given you already had that in static tables, if we >>> just turn the tables a bit around and add a driver abstraction of caching >>> modes this is what happens: >>> >>> * We can lose the ugly runtime i915_gem_get_pat_index. >>> >>> * We can have a smarter i915_gem_object_has_cache_level, which now can >>> use the above mentioned table to understand the caching modes and so >>> does not have to pessimistically return true for _any_ input when user >>> has set the PAT index. This may improve things even for MTL. >>> >>> * We can simplify the debugfs printout to be platform agnostic. >>> >>> * We are perhaps opening the door to un-regress the dodgy addition >>> made to i915_gem_object_can_bypass_llc? See QQQ/FIXME in the patch. >>> >>> I hope I did not forget anything, but anyway, please have a read and see >>> what you think. I think it has potential. >>> >>> Proper commit message can come later. >>> >>> Signed-off-by: Tvrtko Ursulin >>> Cc: Fei Yang >>> --- >>> drivers/gpu/drm/i915/Makefile | 1 + >>> drivers/gpu/drm/i915/display/intel_dpt.c | 2 +- >>> drivers/gpu/drm/i915/display/intel_fb_pin.c | 2 +- >>> .../drm/i915/display/intel_plane_initial.c | 4 +- >>> drivers/gpu/drm/i915/gem/i915_gem_domain.c | 66 +- >>> drivers/gpu/drm/i915/gem/i915_gem_domain.h | 7 +- >>> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 13 +- >>> drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 +- >>> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 10 +- >>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 116 + >>> drivers/gpu/drm/i915/gem/i915_gem_object.h | 9 +- >>> .../gpu/drm/i915/gem/i915_gem_object_types.h | 117 +++--- >>> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 10 +- >>> drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 13 +- >>> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 43 --- >>> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +- >>> .../drm/i915/gem/selftests/huge_gem_object.c | 6 +- >>> .../gpu/drm/i915/gem/selftests/huge_pages.c | 8 +- >>> drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 4 +- >>> drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 19 +-- >>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- >>> drivers/gpu/drm/i915/gt/intel_ggtt.c | 33 ++--- >>> drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 4 +- >>> drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +- >>> drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +- >>> drivers/gpu/drm/i915/gt/intel_migrate.c | 11 +- >>> drivers/gpu/drm/i915/gt/intel_ppgtt.c | 6 +- >>> .../gpu/drm/i915/gt/intel_ring_submission.c | 2 +- >>> drivers/gpu/drm/i915/gt/intel_timeline.c | 2 +- >>> drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +- >>> drivers/gpu/drm/i915/gt/selftest_migrate.c | 9 +- >>> drivers/gpu/drm/i915/gt/selftest_reset.c | 14 +-- >>> drivers/gpu/drm/i915/gt/selftest_timeline.c | 1 + >>> drivers/gpu/drm/i915/gt/selftest_tlb.c | 5 +- >>> .../gpu/drm/i915/gt/selftest_workarounds.c | 2 +- >>> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 2 +- >>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 8 +- >>> drivers/gpu/drm/i915/i915_cache.c | 72 +++ >>> drivers/gpu/drm/i915/i915_cache.h | 49 >>> drivers/gpu/drm/i915/i915_debugfs.c | 65 +++--- >>> drivers/gpu/drm/i915/i915_driver.c | 5 + >>> drivers/gpu/drm/i915/i915_drv.h | 3 + >>> drivers/gpu/drm/i915/i915_gem.c | 21 +--- >>> drivers/gpu/drm/i915/i915_gpu_error.c | 7 +- >>> drivers/gpu/drm/i915/i915_pci.c | 83 +++-- >>> drivers/gpu/drm/i915/i915_perf.c | 2 +- >>> drivers/gpu/drm/i915/intel_device_info.h | 6 +- >>> drivers/gpu/drm/i915/selftests/i915_gem.c | 5 +- >>> .../gpu/drm/i915/selftests/i915_gem_evict.c | 8 +- >>> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 13 +- >>> drivers/gpu/drm/i915/selftests/igt_spinner.c | 2 +- >>> .../drm/i915/selftests/intel_memory_region.c | 4 +- >>> .../gpu/drm/i915/selftests/mock_gem_device.c | 12 +- >>> drivers/gpu/drm/i915/selftests/mock_region.c | 2 +- >>> 54 files changed, 440 insertions(+), 485 deletions(-) >>> create mode 100644 drivers/gpu/drm/i915/i915_cache.c >>> create mode 100644 drivers/gpu/drm/i915/i915_cache.h >>> >>> diff --git
[Intel-gfx] [PATCH v2] drm/i915/gt: update request engine before removing virtual GuC engine
GuC virtual engines can be removed before request removal. On the other
side driver expects rq->engine to be a valid pointer for a whole life of
request. Setting rq->engine to an always valid engine should solve
the issue.
The patch fixes bug detected by KASAN with following signature:
i915 :00:02.0: [drm:i915_drop_caches_set [i915]] Dropping caches:
0x005c [0x005c]
BUG: KASAN: slab-use-after-free in i915_fence_release+0x2a2/0x2f0 [i915]
Read of size 4 at addr 88813ffda6e8 by task kworker/u32:10/157
...
Allocated by task 1184:
...
guc_create_virtual+0x4d/0x1160 [i915]
i915_gem_create_context+0x11ee/0x18c0 [i915]
...
Freed by task 151:
...
intel_guc_deregister_done_process_msg+0x324/0x6d0 [i915]
...
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7926
Signed-off-by: Andrzej Hajda
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a0e3ef1c65d246..2c877ea5eda6f0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -3461,6 +3461,8 @@ static void guc_prio_fini(struct i915_request *rq, struct
intel_context *ce)
static void remove_from_context(struct i915_request *rq)
{
struct intel_context *ce = request_to_scheduling_context(rq);
+ struct intel_engine_cs *engine;
+ intel_engine_mask_t tmp;
GEM_BUG_ON(intel_context_is_child(ce));
@@ -3478,6 +3480,15 @@ static void remove_from_context(struct i915_request *rq)
atomic_dec(>guc_id.ref);
i915_request_notify_execute_cb_imm(rq);
+
+ /*
+* GuC virtual engine can disappear after this call, so let's assign
+* something valid, as driver expects this to be always valid pointer.
+*/
+ for_each_engine_masked(engine, rq->engine->gt, rq->execution_mask, tmp)
{
+ rq->engine = engine;
+ break;
+ }
}
static const struct intel_context_ops guc_context_ops = {
--
2.34.1
[Intel-gfx] ✓ Fi.CI.BAT: success for Covert to use time_before macro
== Series Details == Series: Covert to use time_before macro URL : https://patchwork.freedesktop.org/series/120295/ State : success == Summary == CI Bug Log - changes from CI_DRM_13349 -> Patchwork_120295v1 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/index.html Participating hosts (41 -> 37) -- Missing(4): fi-kbl-soraka fi-tgl-1115g4 fi-snb-2520m fi-pnv-d510 Known issues Here are the changes found in Patchwork_120295v1 that come from known issues: ### IGT changes ### Issues hit * igt@core_auth@basic-auth: - bat-adlp-11:[PASS][1] -> [ABORT][2] ([i915#4423] / [i915#8011]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/bat-adlp-11/igt@core_a...@basic-auth.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/bat-adlp-11/igt@core_a...@basic-auth.html * igt@i915_module_load@load: - bat-adlp-11:[PASS][3] -> [DMESG-WARN][4] ([i915#4423]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/bat-adlp-11/igt@i915_module_l...@load.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/bat-adlp-11/igt@i915_module_l...@load.html * igt@i915_selftest@live@gt_heartbeat: - fi-apl-guc: [PASS][5] -> [DMESG-FAIL][6] ([i915#5334]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html * igt@i915_selftest@live@requests: - bat-rpls-1: [PASS][7] -> [ABORT][8] ([i915#7920] / [i915#7982]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/bat-rpls-1/igt@i915_selftest@l...@requests.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/bat-rpls-1/igt@i915_selftest@l...@requests.html * igt@i915_suspend@basic-s2idle-without-i915: - bat-rpls-2: NOTRUN -> [ABORT][9] ([i915#6687] / [i915#8668]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/bat-rpls-2/igt@i915_susp...@basic-s2idle-without-i915.html Possible fixes * igt@i915_selftest@live@migrate: - bat-dg2-11: [DMESG-WARN][10] ([i915#7699]) -> [PASS][11] [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/bat-dg2-11/igt@i915_selftest@l...@migrate.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/bat-dg2-11/igt@i915_selftest@l...@migrate.html * igt@i915_selftest@live@requests: - bat-rpls-2: [ABORT][12] ([i915#7913] / [i915#7982]) -> [PASS][13] [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/bat-rpls-2/igt@i915_selftest@l...@requests.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/bat-rpls-2/igt@i915_selftest@l...@requests.html - bat-mtlp-6: [DMESG-FAIL][14] ([i915#7269]) -> [PASS][15] [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13349/bat-mtlp-6/igt@i915_selftest@l...@requests.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/bat-mtlp-6/igt@i915_selftest@l...@requests.html [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423 [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334 [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687 [i915#7269]: https://gitlab.freedesktop.org/drm/intel/issues/7269 [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699 [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913 [i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920 [i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982 [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011 [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668 Build changes - * Linux: CI_DRM_13349 -> Patchwork_120295v1 CI-20190529: 20190529 CI_DRM_13349: 7ffb88c57a657e902d4060565c15d30654721a31 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7374: 470370c8c25d419eafa627e54edcf0af3b116d92 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_120295v1: 7ffb88c57a657e902d4060565c15d30654721a31 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits c12d27cd937e Covert to use time_before macro == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120295v1/index.html
[Intel-gfx] ✗ Fi.CI.BUILD: failure for nightly.conf: drop sound tree from drm-tip altogether
== Series Details == Series: nightly.conf: drop sound tree from drm-tip altogether URL : https://patchwork.freedesktop.org/series/120278/ State : failure == Summary == Error: patch https://patchwork.freedesktop.org/api/1.0/series/120278/revisions/1/mbox/ not applied Applying: nightly.conf: drop sound tree from drm-tip altogether error: sha1 information is lacking or useless (nightly.conf). error: could not build fake ancestor hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0001 nightly.conf: drop sound tree from drm-tip altogether When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Build failed, no error log produced
Re: [Intel-gfx] [drm-rerere] nightly.conf: drop sound tree from drm-tip altogether
On Thu, Jul 6, 2023 at 5:29 AM Jani Nikula wrote: > > We used to have the sound branches be part of drm-tip to help > development of DP and HDMI audio. However, we always used to run into > problems with the sound branches merging Linus' master at non-tagged > random commits, wreaking havoc especially during the merge windows. We > only ever want to have tagged stuff merged back from Linus' tree to > drm-tip. > > We introduced a mechanism in dim to hold back branches at certain > commits, just to hold back sound branches when problems arise. We moved > it along, but in the end nobody has updated this in literally years, and > sound branches have been held back at v5.13. > > The merge window is currently open, and AFAICT the sound/for-linus > branch again contains commits from the merge window. > > Let's just forget about the sound tree, as nobody has really missed it > since v5.13, and focus on the drm branches. > > Signed-off-by: Jani Nikula Acked-by: Alex Deucher > --- > nightly.conf | 7 --- > 1 file changed, 7 deletions(-) > > diff --git a/nightly.conf b/nightly.conf > index 73aec820e98f..c1e22800e276 100644 > --- a/nightly.conf > +++ b/nightly.conf > @@ -46,11 +46,6 @@ git://anongit.freedesktop.org/drm/drm > https://anongit.freedesktop.org/git/drm/drm > https://anongit.freedesktop.org/git/drm/drm.git > " > -drm_tip_repos[sound-upstream]=" > -git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git > -https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git > -https://kernel.googlesource.com/pub/scm/linux/kernel/git/tiwai/sound.git > -" > drm_tip_repos[linux-upstream]=" > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > @@ -79,8 +74,6 @@ drm_tip_config=( > "drm-intel drm-intel-next" > "drm-intel drm-intel-gt-next" > > - "sound-upstream for-linus v5.13" > - "sound-upstream for-nextv5.13" > "drm-intel topic/core-for-CI" > "drm-misc topic/i915-ttm" > "drmtopic/nouveau-misc" > -- > 2.39.2 >
Re: [Intel-gfx] [PATCH 3/3] drm/i915/uncore: optimize CONFIG_DRM_I915_DEBUG_MMIO=n more
On 06/07/2023 13:06, Jani Nikula wrote:
On Thu, 06 Jul 2023, Tvrtko Ursulin wrote:
On 04/07/2023 10:48, Jani Nikula wrote:
While the default for the mmio_debug parameter depends on
CONFIG_DRM_I915_DEBUG_MMIO, we look it up and include all the code for
unclaimed reg debugging even when CONFIG_DRM_I915_DEBUG_MMIO=n. Fix it.
Cc: Lee Shawn C
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_uncore.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index dfefad5a5fec..da2edde4b6f6 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1929,7 +1929,8 @@ static inline bool __must_check
unclaimed_reg_debug_header(struct intel_uncore *uncore,
const i915_reg_t reg, const bool read)
{
- if (likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
+ if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO) ||
+ likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
return false;
But now it would not be possible to enable mmio_debug, if Kconfig
_default_ is 'n'. What am I missing?
You're not missing anything, I am. *facepalm*
The question is, are the first two acceptable without the third?
What are 1st, 2nd and 3rd in your counting?
This area is confusing me a little bit.
If I look at unclaimed_reg_debug it appears unclaimed register debug
depends on mmio_debug.
But if I look at the message output by
intel_uncore_arm_unclaimed_mmio_detection it appears that on detecting
an unclaimed register we suggest to enable mmio_debug.
Isn't that a contradiction?
Regards,
Tvrtko
Re: [Intel-gfx] [PATCH 2/2] drm/i915/display: Do not use stolen on MTL
On 06/07/2023 14:35, Nirmoy Das wrote:
On 7/6/2023 3:32 PM, Tvrtko Ursulin wrote:
On 30/06/2023 18:01, Nirmoy Das wrote:
Use smem on MTL due to a HW bug in MTL that prevents
reading from stolen memory using LMEM BAR.
Does anything remain in stolen or could the memory region just not be
created?
GSC requires DSM which can't use smem for another bug.
Okay, thanks.
As a related comment, these if-if-if object creation ladders were always
a bit ugly and some years ago I was suggesting we create a helper with
some "intent/usage" flags. Which could then dtrt ie. create the right
object for that intent/usage and platform. I *think* I possibly even had
a RFC... need to try and find it.
Regards,
Tvrtko
Regards,
Nirmoy
Regards,
Tvrtko
Cc: Oak Zeng
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Andi Shyti
Cc: Andrzej Hajda
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++
drivers/gpu/drm/i915/display/intel_overlay.c | 7 ---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 1cc0ddc6a310..10e38d60f9ef 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper
*helper,
obj = i915_gem_object_create_lmem(dev_priv, size,
I915_BO_ALLOC_CONTIGUOUS |
I915_BO_ALLOC_USER);
+ } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */
+ obj = i915_gem_object_create_shmem(dev_priv, size);
} else {
/*
* If the FB is too big, just don't use it since fbdev is
not very
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c
b/drivers/gpu/drm/i915/display/intel_overlay.c
index d6fe2bbabe55..05ae446c8a56 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct
drm_device *dev, void *data,
static int get_registers(struct intel_overlay *overlay, bool use_phys)
{
struct drm_i915_private *i915 = overlay->i915;
- struct drm_i915_gem_object *obj;
+ struct drm_i915_gem_object *obj = NULL;
struct i915_vma *vma;
int err;
- obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
- if (IS_ERR(obj))
+ if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */
+ obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
+ if (IS_ERR_OR_NULL(obj))
obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
if (IS_ERR(obj))
return PTR_ERR(obj);
Re: [Intel-gfx] [PATCH 2/2] drm/i915/display: Do not use stolen on MTL
On 7/6/2023 3:32 PM, Tvrtko Ursulin wrote:
On 30/06/2023 18:01, Nirmoy Das wrote:
Use smem on MTL due to a HW bug in MTL that prevents
reading from stolen memory using LMEM BAR.
Does anything remain in stolen or could the memory region just not be
created?
GSC requires DSM which can't use smem for another bug.
Regards,
Nirmoy
Regards,
Tvrtko
Cc: Oak Zeng
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Andi Shyti
Cc: Andrzej Hajda
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++
drivers/gpu/drm/i915/display/intel_overlay.c | 7 ---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 1cc0ddc6a310..10e38d60f9ef 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper
*helper,
obj = i915_gem_object_create_lmem(dev_priv, size,
I915_BO_ALLOC_CONTIGUOUS |
I915_BO_ALLOC_USER);
+ } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */
+ obj = i915_gem_object_create_shmem(dev_priv, size);
} else {
/*
* If the FB is too big, just don't use it since fbdev is
not very
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c
b/drivers/gpu/drm/i915/display/intel_overlay.c
index d6fe2bbabe55..05ae446c8a56 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct
drm_device *dev, void *data,
static int get_registers(struct intel_overlay *overlay, bool use_phys)
{
struct drm_i915_private *i915 = overlay->i915;
- struct drm_i915_gem_object *obj;
+ struct drm_i915_gem_object *obj = NULL;
struct i915_vma *vma;
int err;
- obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
- if (IS_ERR(obj))
+ if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */
+ obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
+ if (IS_ERR_OR_NULL(obj))
obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
if (IS_ERR(obj))
return PTR_ERR(obj);
Re: [Intel-gfx] [PATCH 2/2] drm/i915/display: Do not use stolen on MTL
On 30/06/2023 18:01, Nirmoy Das wrote:
Use smem on MTL due to a HW bug in MTL that prevents
reading from stolen memory using LMEM BAR.
Does anything remain in stolen or could the memory region just not be
created?
Regards,
Tvrtko
Cc: Oak Zeng
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Andi Shyti
Cc: Andrzej Hajda
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/i915/display/intel_fbdev.c | 2 ++
drivers/gpu/drm/i915/display/intel_overlay.c | 7 ---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 1cc0ddc6a310..10e38d60f9ef 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -182,6 +182,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
obj = i915_gem_object_create_lmem(dev_priv, size,
I915_BO_ALLOC_CONTIGUOUS |
I915_BO_ALLOC_USER);
+ } else if (IS_METEORLAKE(dev_priv)) { /* Wa_22018444074 */
+ obj = i915_gem_object_create_shmem(dev_priv, size);
} else {
/*
* If the FB is too big, just don't use it since fbdev is not
very
diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c
b/drivers/gpu/drm/i915/display/intel_overlay.c
index d6fe2bbabe55..05ae446c8a56 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -1348,12 +1348,13 @@ int intel_overlay_attrs_ioctl(struct drm_device *dev,
void *data,
static int get_registers(struct intel_overlay *overlay, bool use_phys)
{
struct drm_i915_private *i915 = overlay->i915;
- struct drm_i915_gem_object *obj;
+ struct drm_i915_gem_object *obj = NULL;
struct i915_vma *vma;
int err;
- obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
- if (IS_ERR(obj))
+ if (!IS_METEORLAKE(i915)) /* Wa_22018444074 */
+ obj = i915_gem_object_create_stolen(i915, PAGE_SIZE);
+ if (IS_ERR_OR_NULL(obj))
obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
if (IS_ERR(obj))
return PTR_ERR(obj);
[Intel-gfx] [PATCH v2 1/6] video/aperture: Add a helper to detect if an aperture contains firmware FB
From: Sui Jingfeng
This patch adds the aperture_contain_firmware_fb() function to do the
determination. Unfortunately due to the fact that apertures list will be
freed dynamically, the location and size information of the firmware fb
will be lost after dedicated drivers call
aperture_remove_conflicting_devices(),
aperture_remove_conflicting_pci_devices() or
aperture_remove_all_conflicting_devices() functions
We handle this problem by introducing two static variables which record the
firmware framebuffer's start addrness and end addrness. It assumes that the
system has only one active firmware framebuffer driver at a time.
We don't use the global structure screen_info here, because PCI resource
may get reallocated(the VRAM BAR could be moved) at kernel boot stage.
Cc: Thomas Zimmermann
Cc: Javier Martinez Canillas
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: David Airlie
Cc: Daniel Vetter
Cc: Helge Deller
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/drm_aperture.c | 16
drivers/video/aperture.c | 29 +
include/drm/drm_aperture.h | 2 ++
include/linux/aperture.h | 7 +++
4 files changed, 54 insertions(+)
diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
index 5729f3bb4398..6e5d8a08683c 100644
--- a/drivers/gpu/drm/drm_aperture.c
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -190,3 +190,19 @@ int
drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
return aperture_remove_conflicting_pci_devices(pdev, req_driver->name);
}
EXPORT_SYMBOL(drm_aperture_remove_conflicting_pci_framebuffers);
+
+/**
+ * drm_aperture_contain_firmware_fb - Determine if a aperture contains
firmware framebuffer
+ *
+ * @base: the aperture's base address in physical memory
+ * @size: aperture size in bytes
+ *
+ * Returns:
+ * true on if there is a firmware framebuffer belong to the aperture passed in,
+ * or false otherwise.
+ */
+bool drm_aperture_contain_firmware_fb(resource_size_t base, resource_size_t
size)
+{
+ return aperture_contain_firmware_fb(base, base + size);
+}
+EXPORT_SYMBOL(drm_aperture_contain_firmware_fb);
diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 561be8feca96..5a5422cec669 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -141,6 +141,9 @@ struct aperture_range {
static LIST_HEAD(apertures);
static DEFINE_MUTEX(apertures_lock);
+static resource_size_t firm_fb_start;
+static resource_size_t firm_fb_end;
+
static bool overlap(resource_size_t base1, resource_size_t end1,
resource_size_t base2, resource_size_t end2)
{
@@ -170,6 +173,9 @@ static int devm_aperture_acquire(struct device *dev,
mutex_lock(_lock);
+ firm_fb_start = base;
+ firm_fb_end = end;
+
list_for_each(pos, ) {
ap = container_of(pos, struct aperture_range, lh);
if (overlap(base, end, ap->base, ap->base + ap->size)) {
@@ -377,3 +383,26 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev
*pdev, const char *na
}
EXPORT_SYMBOL(aperture_remove_conflicting_pci_devices);
+
+/**
+ * aperture_contain_firmware_fb - Detect if the firmware framebuffer belong to
+ *a aperture.
+ * @ap_start: the aperture's start address in physical memory
+ * @ap_end: the aperture's end address in physical memory
+ *
+ * Returns:
+ * true on if there is a firmware framebuffer belong to the aperture passed in,
+ * or false otherwise.
+ */
+bool aperture_contain_firmware_fb(resource_size_t ap_start, resource_size_t
ap_end)
+{
+ /* No firmware framebuffer support */
+ if (!firm_fb_start || !firm_fb_end)
+ return false;
+
+ if (firm_fb_start >= ap_start && firm_fb_end <= ap_end)
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(aperture_contain_firmware_fb);
diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
index cbe33b49fd5d..6a0b9bacb081 100644
--- a/include/drm/drm_aperture.h
+++ b/include/drm/drm_aperture.h
@@ -35,4 +35,6 @@ drm_aperture_remove_framebuffers(const struct drm_driver
*req_driver)
req_driver);
}
+bool drm_aperture_contain_firmware_fb(resource_size_t base, resource_size_t
size);
+
#endif
diff --git a/include/linux/aperture.h b/include/linux/aperture.h
index 1a9a88b11584..d4dc5917c49b 100644
--- a/include/linux/aperture.h
+++ b/include/linux/aperture.h
@@ -19,6 +19,8 @@ int aperture_remove_conflicting_devices(resource_size_t base,
resource_size_t si
int __aperture_remove_legacy_vga_devices(struct pci_dev *pdev);
int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char
*name);
+
+bool aperture_contain_firmware_fb(resource_size_t ap_start, resource_size_t
ap_end);
#else
static inline int devm_aperture_acquire_for_platform_device(struct
platform_device *pdev,
[Intel-gfx] [PATCH v1 4/4] drm/radeon: Implement the is_boot_device callback function
From: Sui Jingfeng
[why]
The vga_is_firmware_default() defined in drivers/pci/vgaarb.c is
arch-dependent, it's a dummy on non-x86 architectures currently.
This made VGAARB lost an important condition for the arbitration.
It could still be wrong even if we remove the #ifdef and #endif guards.
because the PCI bar will move (resource re-allocation).
[how]
The device that owns the firmware framebuffer should be the default boot
device. This patch adds an arch-independent function to enforce this rule.
The vgaarb subsystem will call back to radeon_is_boot_device() function
when drm/radeon is successfully bound to a radeon GPU device.
Cc: Alex Deucher
Cc: Christian Konig
Cc: Pan Xinhui
Cc: David Airlie
Cc: Daniel Vetter
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/radeon/radeon_device.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index 71f2ff39d6a1..afb49000830c 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -34,6 +34,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1263,6 +1264,15 @@ static const struct vga_switcheroo_client_ops
radeon_switcheroo_ops = {
.can_switch = radeon_switcheroo_can_switch,
};
+static bool radeon_is_boot_device(struct pci_dev *pdev)
+{
+ struct drm_device *dev = pci_get_drvdata(pdev);
+ struct radeon_device *rdev = dev->dev_private;
+ struct radeon_mc *gmc = >mc;
+
+ return drm_aperture_contain_firmware_fb(gmc->aper_base, gmc->aper_size);
+}
+
/**
* radeon_device_init - initialize the driver
*
@@ -1425,7 +1435,7 @@ int radeon_device_init(struct radeon_device *rdev,
/* if we have > 1 VGA cards, then disable the radeon VGA resources */
/* this will fail for cards that aren't VGA class devices, just
* ignore it */
- vga_client_register(rdev->pdev, radeon_vga_set_decode, NULL);
+ vga_client_register(rdev->pdev, radeon_vga_set_decode,
radeon_is_boot_device);
if (rdev->flags & RADEON_IS_PX)
runtime = true;
--
2.25.1
[Intel-gfx] [PATCH v2] drm/i915/gt: Convert to use time_before macro
Use time_before macro instead of open it for readability. Signed-off-by: Zehao Zhang --- v2: -update subject drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c index 86b5a9ba323d..9145f9e22860 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c @@ -57,7 +57,7 @@ static bool pool_free_older_than(struct intel_gt_buffer_pool *pool, long keep) node = list_entry(pos, typeof(*node), link); age = READ_ONCE(node->age); - if (!age || jiffies - age < keep) + if (!age || time_before(jiffies, age + keep)) break; /* Check we are the first to claim this node */ -- 2.35.3
Re: [Intel-gfx] [PATCH v2 00/24] use vmalloc_array and vcalloc
Julia,
> The functions vmalloc_array and vcalloc were introduced in
>
> commit a8749a35c399 ("mm: vmalloc: introduce array allocation functions")
>
> but are not used much yet. This series introduces uses of
> these functions, to protect against multiplication overflows.
Applied #7 and #24 to 6.5/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
Re: [Intel-gfx] [PATCH 24/29] mm: vmscan: make global slab shrink lockless
On Fri, Jun 23, 2023 at 04:29:39PM +1000, Dave Chinner wrote:
> On Thu, Jun 22, 2023 at 05:12:02PM +0200, Vlastimil Babka wrote:
> > On 6/22/23 10:53, Qi Zheng wrote:
> > > @@ -1067,33 +1068,27 @@ static unsigned long shrink_slab(gfp_t gfp_mask,
> > > int nid,
> > > if (!mem_cgroup_disabled() && !mem_cgroup_is_root(memcg))
> > > return shrink_slab_memcg(gfp_mask, nid, memcg, priority);
> > >
> > > - if (!down_read_trylock(_rwsem))
> > > - goto out;
> > > -
> > > - list_for_each_entry(shrinker, _list, list) {
> > > + rcu_read_lock();
> > > + list_for_each_entry_rcu(shrinker, _list, list) {
> > > struct shrink_control sc = {
> > > .gfp_mask = gfp_mask,
> > > .nid = nid,
> > > .memcg = memcg,
> > > };
> > >
> > > + if (!shrinker_try_get(shrinker))
> > > + continue;
> > > + rcu_read_unlock();
> >
> > I don't think you can do this unlock?
Sorry to be slow to respond here, this one fell through the cracks.
And thank you to Qi for reminding me!
If you do this unlock, you had jolly well better nail down the current
element (the one referenced by shrinker), for example, by acquiring an
explicit reference count on the object. And presumably this is exactly
what shrinker_try_get() is doing. And a look at your 24/29 confirms this,
at least assuming that shrinker->refcount is set to zero before the call
to synchronize_rcu() in free_module() *and* that synchronize_rcu() doesn't
start until *after* shrinker_put() calls complete(). Plus, as always,
the object must be removed from the list before the synchronize_rcu()
starts. (On these parts of the puzzle, I defer to those more familiar
with this code path. And I strongly suggest carefully commenting this
type of action-at-a-distance design pattern.)
Why is this important? Because otherwise that object might be freed
before you get to the call to rcu_read_lock() at the end of this loop.
And if that happens, list_for_each_entry_rcu() will be walking the
freelist, which is quite bad for the health and well-being of your kernel.
There are a few other ways to make this sort of thing work:
1. Defer the shrinker_put() to the beginning of the loop.
You would need a flag initially set to zero, and then set to
one just before (or just after) the rcu_read_lock() above.
You would also need another shrinker_old pointer to track the
old pointer. Then at the top of the loop, if the flag is set,
invoke shrinker_put() on shrinker_old. This ensures that the
previous shrinker structure stays around long enough to allow
the loop to find the next shrinker structure in the list.
This approach is attractive when the removal code path
can invoke shrinker_put() after the grace period ends.
2. Make shrinker_put() invoke call_rcu() when ->refcount reaches
zero, and have the callback function free the object. This of
course requires adding an rcu_head structure to the shrinker
structure, which might or might not be a reasonable course of
action. If adding that rcu_head is reasonable, this simplifies
the logic quite a bit.
3. For the shrinker-structure-removal code path, remove the shrinker
structure, then remove the initial count from ->refcount,
and then keep doing grace periods until ->refcount is zero,
then do one more. Of course, if the result of removing the
initial count was zero, then only a single additional grace
period is required.
This would need to be carefully commented, as it is a bit
unconventional.
There are probably many other ways, but just to give an idea of a few
other ways to do this.
> > > +
> > > ret = do_shrink_slab(, shrinker, priority);
> > > if (ret == SHRINK_EMPTY)
> > > ret = 0;
> > > freed += ret;
> > > - /*
> > > - * Bail out if someone want to register a new shrinker to
> > > - * prevent the registration from being stalled for long periods
> > > - * by parallel ongoing shrinking.
> > > - */
> > > - if (rwsem_is_contended(_rwsem)) {
> > > - freed = freed ? : 1;
> > > - break;
> > > - }
> > > - }
> > >
> > > - up_read(_rwsem);
> > > -out:
> > > + rcu_read_lock();
> >
> > That new rcu_read_lock() won't help AFAIK, the whole
> > list_for_each_entry_rcu() needs to be under the single rcu_read_lock() to be
> > safe.
>
> Yeah, that's the pattern we've been taught and the one we can look
> at and immediately say "this is safe".
>
> This is a different pattern, as has been explained bi Qi, and I
> think it *might* be safe.
>
> *However.*
>
> Right now I don't have time to go through a novel RCU list iteration
> pattern it one step at to determine the correctness of the
> algorithm. I'm mostly
Re: [Intel-gfx] [PATCH] drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read
On Tue, Jul 4, 2023 at 9:28 AM Jani Nikula wrote:
>
> On Mon, 03 Jul 2023, Uros Bizjak wrote:
> > Use local64_try_cmpxchg instead of local64_cmpxchg (*ptr, old, new) == old
> > in i915_pmu_event_read. x86 CMPXCHG instruction returns success in ZF flag,
> > so this change saves a compare after cmpxchg (and related move instruction
> > in front of cmpxchg).
> >
> > Also, try_cmpxchg implicitly assigns old *ptr value to "old" when cmpxchg
> > fails. There is no need to re-read the value in the loop.
> >
> > No functional change intended.
> >
> > Cc: Jani Nikula
> > Cc: Joonas Lahtinen
> > Cc: Rodrigo Vivi
> > Cc: Tvrtko Ursulin
> > Cc: David Airlie
> > Cc: Daniel Vetter
> > Signed-off-by: Uros Bizjak
> > ---
> > drivers/gpu/drm/i915/i915_pmu.c | 9 -
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c
> > b/drivers/gpu/drm/i915/i915_pmu.c
> > index d35973b41186..108b675088ba 100644
> > --- a/drivers/gpu/drm/i915/i915_pmu.c
> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
> > @@ -696,12 +696,11 @@ static void i915_pmu_event_read(struct perf_event
> > *event)
> > event->hw.state = PERF_HES_STOPPED;
> > return;
> > }
> > -again:
> > - prev = local64_read(>prev_count);
> > - new = __i915_pmu_event_read(event);
> >
> > - if (local64_cmpxchg(>prev_count, prev, new) != prev)
> > - goto again;
> > + prev = local64_read(>prev_count);
> > + do {
> > + new = __i915_pmu_event_read(event);
> > + } while (!local64_try_cmpxchg(>prev_count, , new));
>
> You could save everyone a lot of time by actually documenting what these
> functions do. Assume you don't know what local64_try_cmpxchg() does, and
> see how many calls you have to go through to figure it out.
These functions are documented in Documentation/atomic_t.txt (under
"RMW ops:" section), and the difference is explained in a separate
section "CMPXCHG vs TRY_CMPXCGS" in the same file.
Uros.
> Because the next time I encounter this code or a patch like this, I'm
> probably going to have to do that again.
>
> To me, the old one was more readable. The optimization is meaningless to
> me if it's not quantified but reduces readability.
>
>
> BR,
> Jani.
>
>
> >
> > local64_add(new - prev, >count);
> > }
>
> --
> Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PATCH v2 6/6] drm/ast: Register as a vga client to vgaarb by calling vga_client_register()
From: Sui Jingfeng
Becasuse the VGA Display Controller in the ASpeed BMC chip is also a PCIe
device, the Software Programming guide of AST2400 say that it is Fully
IBM VGA compliant, thus, it should also particiate in the arbitration.
Cc: Thomas Zimmermann
Cc: Jocelyn Falempe
Cc: David Airlie
Cc: Daniel Vetter
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/ast/ast_drv.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index e1224ef4ad83..5ce681142679 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -28,6 +28,7 @@
#include
#include
+#include
#include
#include
@@ -89,6 +90,32 @@ static const struct pci_device_id ast_pciidlist[] = {
MODULE_DEVICE_TABLE(pci, ast_pciidlist);
+static bool ast_is_boot_device(struct pci_dev *pdev)
+{
+ struct drm_device *drm = pci_get_drvdata(pdev);
+ struct ast_device *ast = to_ast_device(drm);
+
+ return drm_aperture_contain_firmware_fb(ast->vram_base, ast->vram_size);
+}
+
+static unsigned int ast_vga_set_decode(struct pci_dev *pdev, bool state)
+{
+ struct drm_device *drm = pci_get_drvdata(pdev);
+ struct ast_device *ast = to_ast_device(drm);
+
+ if (state) {
+ /* Enable standard VGA decode and Enable normal VGA decode */
+ ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
+
+ return VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM |
+ VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
+ }
+
+ ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x07);
+
+ return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
+}
+
static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
struct ast_device *ast;
@@ -112,6 +139,8 @@ static int ast_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
if (ret)
return ret;
+ vga_client_register(pdev, ast_vga_set_decode, ast_is_boot_device);
+
drm_fbdev_generic_setup(dev, 32);
return 0;
--
2.25.1
[Intel-gfx] [PATCH v1 2/4] PCI/VGA: Improve the default VGA device selection
From: Sui Jingfeng
Currently, the default VGA device selection is not perfect. Potential
problems are:
1) This function is a no-op on non-x86 architectures.
2) It does not take the PCI Bar may get relocated into consideration.
3) It is not effective for the PCI device without a dedicated VRAM Bar.
4) It is device-agnostic, thus it has to waste the effort to iterate all
of the PCI Bar to find the VRAM aperture.
5) It has invented lots of methods to determine which one is the default
boot device on a multiple video card coexistence system. But this is
still a policy because it doesn't give the user a choice to override.
With the observation that device drivers or video aperture helpers may
have better knowledge about which PCI bar contains the firmware FB,
This patch tries to solve the above problems by introducing a function
callback to the vga_client_register() function interface. DRM device
drivers for the PCI device need to register the is_boot_device() function
callback during the driver loading time. Once the driver binds the device
successfully, VRAARB will call back to the driver. This gives the device
drivers a chance to provide accurate boot device identification. Which in
turn unlock the abitration service to non-x86 architectures. A device
driver can also pass a NULL pointer to the keep the original behavior.
This patch is to introduce the mechanism only, while the implementation
is left to the authors of various device driver. Also honor the comment:
"Clients have two callback mechanisms they can use"
Cc: Alex Deucher
Cc: Christian Konig
Cc: Pan Xinhui
Cc: David Airlie
Cc: Daniel Vetter
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: Ben Skeggs
Cc: Karol Herbst
Cc: Lyude Paul
Cc: Bjorn Helgaas
Cc: Alex Williamson
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: Hawking Zhang
Cc: Mario Limonciello
Cc: Lijo Lazar
Cc: YiPeng Chai
Cc: Bokun Zhang
Cc: Likun Gao
Cc: Ville Syrjala
Cc: Jason Gunthorpe
CC: Kevin Tian
Cc: Cornelia Huck
Cc: Yishai Hadas
Cc: Abhishek Sahu
Cc: Yi Liu
Reviewed-by: Lyude Paul # nouveau
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/i915/display/intel_vga.c | 3 +--
drivers/gpu/drm/nouveau/nouveau_vga.c | 2 +-
drivers/gpu/drm/radeon/radeon_device.c | 2 +-
drivers/pci/vgaarb.c | 21 -
drivers/vfio/pci/vfio_pci_core.c | 2 +-
include/linux/vgaarb.h | 8 +---
7 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index e25f085ee886..c5bdf6eff29e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4082,7 +4082,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
/* this will fail for cards that aren't VGA class devices, just
* ignore it */
if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
- vga_client_register(adev->pdev, amdgpu_device_vga_set_decode);
+ vga_client_register(adev->pdev, amdgpu_device_vga_set_decode,
NULL);
px = amdgpu_device_supports_px(ddev);
diff --git a/drivers/gpu/drm/i915/display/intel_vga.c
b/drivers/gpu/drm/i915/display/intel_vga.c
index 286a0bdd28c6..98d7d4dffe9f 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.c
+++ b/drivers/gpu/drm/i915/display/intel_vga.c
@@ -115,7 +115,6 @@ intel_vga_set_decode(struct pci_dev *pdev, bool
enable_decode)
int intel_vga_register(struct drm_i915_private *i915)
{
-
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
int ret;
@@ -127,7 +126,7 @@ int intel_vga_register(struct drm_i915_private *i915)
* then we do not take part in VGA arbitration and the
* vga_client_register() fails with -ENODEV.
*/
- ret = vga_client_register(pdev, intel_vga_set_decode);
+ ret = vga_client_register(pdev, intel_vga_set_decode, NULL);
if (ret && ret != -ENODEV)
return ret;
diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c
b/drivers/gpu/drm/nouveau/nouveau_vga.c
index f8bf0ec26844..162b4f4676c7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -92,7 +92,7 @@ nouveau_vga_init(struct nouveau_drm *drm)
return;
pdev = to_pci_dev(dev->dev);
- vga_client_register(pdev, nouveau_vga_set_decode);
+ vga_client_register(pdev, nouveau_vga_set_decode, NULL);
/* don't register Thunderbolt eGPU with vga_switcheroo */
if (pci_is_thunderbolt_attached(pdev))
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index afbb3a80c0c6..71f2ff39d6a1 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1425,7 +1425,7 @@ int
[Intel-gfx] [PATCH] drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read
Use local64_try_cmpxchg instead of local64_cmpxchg (*ptr, old, new) == old
in i915_pmu_event_read. x86 CMPXCHG instruction returns success in ZF flag,
so this change saves a compare after cmpxchg (and related move instruction
in front of cmpxchg).
Also, try_cmpxchg implicitly assigns old *ptr value to "old" when cmpxchg
fails. There is no need to re-read the value in the loop.
No functional change intended.
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: David Airlie
Cc: Daniel Vetter
Signed-off-by: Uros Bizjak
---
drivers/gpu/drm/i915/i915_pmu.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index d35973b41186..108b675088ba 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -696,12 +696,11 @@ static void i915_pmu_event_read(struct perf_event *event)
event->hw.state = PERF_HES_STOPPED;
return;
}
-again:
- prev = local64_read(>prev_count);
- new = __i915_pmu_event_read(event);
- if (local64_cmpxchg(>prev_count, prev, new) != prev)
- goto again;
+ prev = local64_read(>prev_count);
+ do {
+ new = __i915_pmu_event_read(event);
+ } while (!local64_try_cmpxchg(>prev_count, , new));
local64_add(new - prev, >count);
}
--
2.41.0
Re: [Intel-gfx] [PATCH 24/29] mm: vmscan: make global slab shrink lockless
On 2023/7/4 11:45, Qi Zheng wrote:
On 2023/7/4 00:39, Paul E. McKenney wrote:
On Fri, Jun 23, 2023 at 04:29:39PM +1000, Dave Chinner wrote:
On Thu, Jun 22, 2023 at 05:12:02PM +0200, Vlastimil Babka wrote:
On 6/22/23 10:53, Qi Zheng wrote:
@@ -1067,33 +1068,27 @@ static unsigned long shrink_slab(gfp_t
gfp_mask, int nid,
if (!mem_cgroup_disabled() && !mem_cgroup_is_root(memcg))
return shrink_slab_memcg(gfp_mask, nid, memcg, priority);
- if (!down_read_trylock(_rwsem))
- goto out;
-
- list_for_each_entry(shrinker, _list, list) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(shrinker, _list, list) {
struct shrink_control sc = {
.gfp_mask = gfp_mask,
.nid = nid,
.memcg = memcg,
};
+ if (!shrinker_try_get(shrinker))
+ continue;
+ rcu_read_unlock();
I don't think you can do this unlock?
Sorry to be slow to respond here, this one fell through the cracks.
And thank you to Qi for reminding me!
If you do this unlock, you had jolly well better nail down the current
element (the one referenced by shrinker), for example, by acquiring an
explicit reference count on the object. And presumably this is exactly
what shrinker_try_get() is doing. And a look at your 24/29 confirms
this,
at least assuming that shrinker->refcount is set to zero before the call
to synchronize_rcu() in free_module() *and* that synchronize_rcu()
doesn't
start until *after* shrinker_put() calls complete(). Plus, as always,
the object must be removed from the list before the synchronize_rcu()
starts. (On these parts of the puzzle, I defer to those more familiar
with this code path. And I strongly suggest carefully commenting this
type of action-at-a-distance design pattern.)
Yeah, I think I've done it like above. A more detailed timing diagram is
below.
Why is this important? Because otherwise that object might be freed
before you get to the call to rcu_read_lock() at the end of this loop.
And if that happens, list_for_each_entry_rcu() will be walking the
freelist, which is quite bad for the health and well-being of your
kernel.
There are a few other ways to make this sort of thing work:
1. Defer the shrinker_put() to the beginning of the loop.
You would need a flag initially set to zero, and then set to
one just before (or just after) the rcu_read_lock() above.
You would also need another shrinker_old pointer to track the
old pointer. Then at the top of the loop, if the flag is set,
invoke shrinker_put() on shrinker_old. This ensures that the
previous shrinker structure stays around long enough to allow
the loop to find the next shrinker structure in the list.
This approach is attractive when the removal code path
can invoke shrinker_put() after the grace period ends.
2. Make shrinker_put() invoke call_rcu() when ->refcount reaches
zero, and have the callback function free the object. This of
course requires adding an rcu_head structure to the shrinker
structure, which might or might not be a reasonable course of
action. If adding that rcu_head is reasonable, this simplifies
the logic quite a bit.
3. For the shrinker-structure-removal code path, remove the shrinker
structure, then remove the initial count from ->refcount,
and then keep doing grace periods until ->refcount is zero,
then do one more. Of course, if the result of removing the
initial count was zero, then only a single additional grace
period is required.
This would need to be carefully commented, as it is a bit
unconventional.
Thanks for such a detailed addition!
There are probably many other ways, but just to give an idea of a few
other ways to do this.
+
ret = do_shrink_slab(, shrinker, priority);
if (ret == SHRINK_EMPTY)
ret = 0;
freed += ret;
- /*
- * Bail out if someone want to register a new shrinker to
- * prevent the registration from being stalled for long
periods
- * by parallel ongoing shrinking.
- */
- if (rwsem_is_contended(_rwsem)) {
- freed = freed ? : 1;
- break;
- }
- }
- up_read(_rwsem);
-out:
+ rcu_read_lock();
That new rcu_read_lock() won't help AFAIK, the whole
list_for_each_entry_rcu() needs to be under the single
rcu_read_lock() to be
safe.
Yeah, that's the pattern we've been taught and the one we can look
at and immediately say "this is safe".
This is a different pattern, as has been explained bi Qi, and I
think it *might* be safe.
*However.*
Right now I don't have time to go through a novel RCU list iteration
pattern it one step at to determine the correctness of the
algorithm. I'm mostly worried about list manipulations that can
occur outside rcu_read_lock() section bleeding into the RCU
critical section because rcu_read_lock() by itself is not a memory
Re: [Intel-gfx] [PATCH 24/29] mm: vmscan: make global slab shrink lockless
On 2023/7/4 00:39, Paul E. McKenney wrote:
On Fri, Jun 23, 2023 at 04:29:39PM +1000, Dave Chinner wrote:
On Thu, Jun 22, 2023 at 05:12:02PM +0200, Vlastimil Babka wrote:
On 6/22/23 10:53, Qi Zheng wrote:
@@ -1067,33 +1068,27 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int
nid,
if (!mem_cgroup_disabled() && !mem_cgroup_is_root(memcg))
return shrink_slab_memcg(gfp_mask, nid, memcg, priority);
- if (!down_read_trylock(_rwsem))
- goto out;
-
- list_for_each_entry(shrinker, _list, list) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(shrinker, _list, list) {
struct shrink_control sc = {
.gfp_mask = gfp_mask,
.nid = nid,
.memcg = memcg,
};
+ if (!shrinker_try_get(shrinker))
+ continue;
+ rcu_read_unlock();
I don't think you can do this unlock?
Sorry to be slow to respond here, this one fell through the cracks.
And thank you to Qi for reminding me!
If you do this unlock, you had jolly well better nail down the current
element (the one referenced by shrinker), for example, by acquiring an
explicit reference count on the object. And presumably this is exactly
what shrinker_try_get() is doing. And a look at your 24/29 confirms this,
at least assuming that shrinker->refcount is set to zero before the call
to synchronize_rcu() in free_module() *and* that synchronize_rcu() doesn't
start until *after* shrinker_put() calls complete(). Plus, as always,
the object must be removed from the list before the synchronize_rcu()
starts. (On these parts of the puzzle, I defer to those more familiar
with this code path. And I strongly suggest carefully commenting this
type of action-at-a-distance design pattern.)
Yeah, I think I've done it like above. A more detailed timing diagram is
below.
Why is this important? Because otherwise that object might be freed
before you get to the call to rcu_read_lock() at the end of this loop.
And if that happens, list_for_each_entry_rcu() will be walking the
freelist, which is quite bad for the health and well-being of your kernel.
There are a few other ways to make this sort of thing work:
1. Defer the shrinker_put() to the beginning of the loop.
You would need a flag initially set to zero, and then set to
one just before (or just after) the rcu_read_lock() above.
You would also need another shrinker_old pointer to track the
old pointer. Then at the top of the loop, if the flag is set,
invoke shrinker_put() on shrinker_old. This ensures that the
previous shrinker structure stays around long enough to allow
the loop to find the next shrinker structure in the list.
This approach is attractive when the removal code path
can invoke shrinker_put() after the grace period ends.
2. Make shrinker_put() invoke call_rcu() when ->refcount reaches
zero, and have the callback function free the object. This of
course requires adding an rcu_head structure to the shrinker
structure, which might or might not be a reasonable course of
action. If adding that rcu_head is reasonable, this simplifies
the logic quite a bit.
3. For the shrinker-structure-removal code path, remove the shrinker
structure, then remove the initial count from ->refcount,
and then keep doing grace periods until ->refcount is zero,
then do one more. Of course, if the result of removing the
initial count was zero, then only a single additional grace
period is required.
This would need to be carefully commented, as it is a bit
unconventional.
Thanks for such a detailed addition!
There are probably many other ways, but just to give an idea of a few
other ways to do this.
+
ret = do_shrink_slab(, shrinker, priority);
if (ret == SHRINK_EMPTY)
ret = 0;
freed += ret;
- /*
-* Bail out if someone want to register a new shrinker to
-* prevent the registration from being stalled for long periods
-* by parallel ongoing shrinking.
-*/
- if (rwsem_is_contended(_rwsem)) {
- freed = freed ? : 1;
- break;
- }
- }
- up_read(_rwsem);
-out:
+ rcu_read_lock();
That new rcu_read_lock() won't help AFAIK, the whole
list_for_each_entry_rcu() needs to be under the single rcu_read_lock() to be
safe.
Yeah, that's the pattern we've been taught and the one we can look
at and immediately say "this is safe".
This is a different pattern, as has been explained bi Qi, and I
think it *might* be safe.
*However.*
Right now I don't have time to go through a novel RCU list iteration
pattern it one step
Re: [Intel-gfx] [PATCH 24/29] mm: vmscan: make global slab shrink lockless
Hi Dave,
On 2023/6/24 19:08, Qi Zheng wrote:
Hi Dave,
On 2023/6/24 06:19, Dave Chinner wrote:
On Fri, Jun 23, 2023 at 09:10:57PM +0800, Qi Zheng wrote:
On 2023/6/23 14:29, Dave Chinner wrote:
On Thu, Jun 22, 2023 at 05:12:02PM +0200, Vlastimil Babka wrote:
On 6/22/23 10:53, Qi Zheng wrote:
Yes, I suggested the IDR route because radix tree lookups under RCU
with reference counted objects are a known safe pattern that we can
easily confirm is correct or not. Hence I suggested the unification
+ IDR route because it makes the life of reviewers so, so much
easier...
In fact, I originally planned to try the unification + IDR method you
suggested at the beginning. But in the case of CONFIG_MEMCG disabled,
the struct mem_cgroup is not even defined, and root_mem_cgroup and
shrinker_info will not be allocated. This required more code
changes, so
I ended up keeping the shrinker_list and implementing the above pattern.
Yes. Go back and read what I originally said needed to be done
first. In the case of CONFIG_MEMCG=n, a dummy root memcg still needs
to exist that holds all of the global shrinkers. Then shrink_slab()
is only ever passed a memcg that should be iterated.
Yes, it needs changes external to the shrinker code itself to be
made to work. And even if memcg's are not enabled, we can still use
the memcg structures to ensure a common abstraction is used for the
shrinker tracking infrastructure
Yeah, what I imagined before was to define a more concise struct
mem_cgroup in the case of CONFIG_MEMCG=n, then allocate a dummy root
memcg on system boot:
#ifdef !CONFIG_MEMCG
struct shrinker_info {
struct rcu_head rcu;
atomic_long_t *nr_deferred;
unsigned long *map;
int map_nr_max;
};
struct mem_cgroup_per_node {
struct shrinker_info __rcu *shrinker_info;
};
struct mem_cgroup {
struct mem_cgroup_per_node *nodeinfo[];
};
#endif
These days I tried doing this:
1. CONFIG_MEMCG && !mem_cgroup_disabled()
track all global shrinkers with root_mem_cgroup.
2. CONFIG_MEMCG && mem_cgroup_disabled()
the root_mem_cgroup is also allocated in this case, so still use
root_mem_cgroup to track all global shrinkers.
3. !CONFIG_MEMCG
allocate a dummy memcg during system startup (after cgroup_init())
and use it to track all global shrinkers
This works, but needs to modify the startup order of some subsystems,
because some shrinkers will be registered before root_mem_cgroup is
allocated, such as:
1. rcu-kfree shrinker in rcu_init()
2. super block shrinkers in vfs_caches_init()
And cgroup_init() also depends on some file system infrastructure, so
I made some changes (rough and unorganized):
diff --git a/fs/namespace.c b/fs/namespace.c
index e157efc54023..6a12d3d0064e 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4706,7 +4706,7 @@ static void __init init_mount_tree(void)
void __init mnt_init(void)
{
- int err;
+ //int err;
mnt_cache = kmem_cache_create("mnt_cache", sizeof(struct mount),
0, SLAB_HWCACHE_ALIGN|SLAB_PANIC|SLAB_ACCOUNT,
NULL);
@@ -4725,15 +4725,7 @@ void __init mnt_init(void)
if (!mount_hashtable || !mountpoint_hashtable)
panic("Failed to allocate mount hash table\n");
- kernfs_init();
-
- err = sysfs_init();
- if (err)
- printk(KERN_WARNING "%s: sysfs_init error: %d\n",
- __func__, err);
- fs_kobj = kobject_create_and_add("fs", NULL);
- if (!fs_kobj)
- printk(KERN_WARNING "%s: kobj create error\n", __func__);
shmem_init();
init_rootfs();
init_mount_tree();
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 7d9c2a63b7cd..d87c67f6f66e 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -119,6 +119,7 @@ static inline void call_rcu_hurry(struct rcu_head
*head, rcu_callback_t func)
/* Internal to kernel */
void rcu_init(void);
+void rcu_shrinker_init(void);
extern int rcu_scheduler_active;
void rcu_sched_clock_irq(int user);
void rcu_report_dead(unsigned int cpu);
diff --git a/init/main.c b/init/main.c
index ad920fac325c..4190fc6d10ad 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1049,14 +1049,22 @@ void start_kernel(void)
security_init();
dbg_late_init();
net_ns_init();
+ kernfs_init();
+ if (sysfs_init())
+ printk(KERN_WARNING "%s: sysfs_init error\n",
+ __func__);
+ fs_kobj = kobject_create_and_add("fs", NULL);
+ if (!fs_kobj)
+ printk(KERN_WARNING "%s: kobj create error\n", __func__);
+ proc_root_init();
+ cgroup_init();
vfs_caches_init();
pagecache_init();
signals_init();
seq_file_init();
- proc_root_init();
nsfs_init();
cpuset_init();
- cgroup_init();
+ rcu_shrinker_init();
taskstats_init_early();
delayacct_init();
diff --git
[Intel-gfx] [PATCH v2 4/6] drm/radeon: Implement the is_boot_device callback function
From: Sui Jingfeng
[why]
The vga_is_firmware_default() defined in drivers/pci/vgaarb.c is
arch-dependent, it's a dummy on non-x86 architectures currently.
This made VGAARB lost an important condition for the arbitration.
It could still be wrong even if we remove the #ifdef and #endif guards.
because the PCI bar will move (resource re-allocation).
[how]
The device that owns the firmware framebuffer should be the default boot
device. This patch adds an arch-independent function to enforce this rule.
The vgaarb subsystem will call back to radeon_is_boot_device() function
when drm/radeon is successfully bound to a radeon GPU device.
Cc: Alex Deucher
Cc: Christian Konig
Cc: Pan Xinhui
Cc: David Airlie
Cc: Daniel Vetter
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/radeon/radeon_device.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index 71f2ff39d6a1..afb49000830c 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -34,6 +34,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1263,6 +1264,15 @@ static const struct vga_switcheroo_client_ops
radeon_switcheroo_ops = {
.can_switch = radeon_switcheroo_can_switch,
};
+static bool radeon_is_boot_device(struct pci_dev *pdev)
+{
+ struct drm_device *dev = pci_get_drvdata(pdev);
+ struct radeon_device *rdev = dev->dev_private;
+ struct radeon_mc *gmc = >mc;
+
+ return drm_aperture_contain_firmware_fb(gmc->aper_base, gmc->aper_size);
+}
+
/**
* radeon_device_init - initialize the driver
*
@@ -1425,7 +1435,7 @@ int radeon_device_init(struct radeon_device *rdev,
/* if we have > 1 VGA cards, then disable the radeon VGA resources */
/* this will fail for cards that aren't VGA class devices, just
* ignore it */
- vga_client_register(rdev->pdev, radeon_vga_set_decode, NULL);
+ vga_client_register(rdev->pdev, radeon_vga_set_decode,
radeon_is_boot_device);
if (rdev->flags & RADEON_IS_PX)
runtime = true;
--
2.25.1
[Intel-gfx] [PATCH v2 2/6] PCI/VGA: Improve the default VGA device selection
From: Sui Jingfeng
Currently, the default VGA device selection is not perfect. Potential
problems are:
1) This function is a no-op on non-x86 architectures.
2) It does not take the PCI Bar may get relocated into consideration.
3) It is not effective for the PCI device without a dedicated VRAM Bar.
4) It is device-agnostic, thus it has to waste the effort to iterate all
of the PCI Bar to find the VRAM aperture.
5) It has invented lots of methods to determine which one is the default
boot device on a multiple video card coexistence system. But this is
still a policy because it doesn't give the user a choice to override.
With the observation that device drivers or video aperture helpers may
have better knowledge about which PCI bar contains the firmware FB,
This patch tries to solve the above problems by introducing a function
callback to the vga_client_register() function interface. DRM device
drivers for the PCI device need to register the is_boot_device() function
callback during the driver loading time. Once the driver binds the device
successfully, VRAARB will call back to the driver. This gives the device
drivers a chance to provide accurate boot device identification. Which in
turn unlock the abitration service to non-x86 architectures. A device
driver can also pass a NULL pointer to keep the original behavior.
This patch is to introduce the mechanism only, while the implementation
is left to the authors of various device driver. Also honor the comment:
"Clients have two callback mechanisms they can use"
Cc: Alex Deucher
Cc: Christian Konig
Cc: Pan Xinhui
Cc: David Airlie
Cc: Daniel Vetter
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: Ben Skeggs
Cc: Karol Herbst
Cc: Lyude Paul
Cc: Bjorn Helgaas
Cc: Alex Williamson
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: Hawking Zhang
Cc: Mario Limonciello
Cc: Lijo Lazar
Cc: YiPeng Chai
Cc: Bokun Zhang
Cc: Likun Gao
Cc: Ville Syrjala
Cc: Jason Gunthorpe
CC: Kevin Tian
Cc: Cornelia Huck
Cc: Yishai Hadas
Cc: Abhishek Sahu
Cc: Yi Liu
Acked-by: Jani Nikula # i915
Reviewed-by: Lyude Paul # nouveau
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/i915/display/intel_vga.c | 3 +--
drivers/gpu/drm/nouveau/nouveau_vga.c | 2 +-
drivers/gpu/drm/radeon/radeon_device.c | 2 +-
drivers/pci/vgaarb.c | 21 -
drivers/vfio/pci/vfio_pci_core.c | 2 +-
include/linux/vgaarb.h | 8 +---
7 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index e25f085ee886..c5bdf6eff29e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4082,7 +4082,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
/* this will fail for cards that aren't VGA class devices, just
* ignore it */
if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
- vga_client_register(adev->pdev, amdgpu_device_vga_set_decode);
+ vga_client_register(adev->pdev, amdgpu_device_vga_set_decode,
NULL);
px = amdgpu_device_supports_px(ddev);
diff --git a/drivers/gpu/drm/i915/display/intel_vga.c
b/drivers/gpu/drm/i915/display/intel_vga.c
index 286a0bdd28c6..98d7d4dffe9f 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.c
+++ b/drivers/gpu/drm/i915/display/intel_vga.c
@@ -115,7 +115,6 @@ intel_vga_set_decode(struct pci_dev *pdev, bool
enable_decode)
int intel_vga_register(struct drm_i915_private *i915)
{
-
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
int ret;
@@ -127,7 +126,7 @@ int intel_vga_register(struct drm_i915_private *i915)
* then we do not take part in VGA arbitration and the
* vga_client_register() fails with -ENODEV.
*/
- ret = vga_client_register(pdev, intel_vga_set_decode);
+ ret = vga_client_register(pdev, intel_vga_set_decode, NULL);
if (ret && ret != -ENODEV)
return ret;
diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c
b/drivers/gpu/drm/nouveau/nouveau_vga.c
index f8bf0ec26844..162b4f4676c7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -92,7 +92,7 @@ nouveau_vga_init(struct nouveau_drm *drm)
return;
pdev = to_pci_dev(dev->dev);
- vga_client_register(pdev, nouveau_vga_set_decode);
+ vga_client_register(pdev, nouveau_vga_set_decode, NULL);
/* don't register Thunderbolt eGPU with vga_switcheroo */
if (pci_is_thunderbolt_attached(pdev))
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index afbb3a80c0c6..71f2ff39d6a1 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@
[Intel-gfx] [PATCH v2 3/6] drm/amdgpu: Implement the is_boot_device callback function
From: Sui Jingfeng
[why]
The vga_is_firmware_default() defined in drivers/pci/vgaarb.c is
arch-dependent, it's a dummy on non-x86 architectures currently.
This made VGAARB lost an important condition for the arbitration.
It could still be wrong even if we remove the #ifdef and #endif guards.
because the PCI bar will move (resource re-allocation).
[how]
The device that owns the firmware framebuffer should be the default boot
device. This patch adds an arch-independent function to enforce this rule.
The vgaarb subsystem will call back to amdgpu_is_boot_device() function
when drm/amdgpu is successfully bound to an AMDGPU device.
Cc: Alex Deucher
Cc: Christian Konig
Cc: Pan Xinhui
Cc: David Airlie
Cc: Daniel Vetter
Cc: Hawking Zhang
Cc: Mario Limonciello
Cc: Lijo Lazar
Cc: YiPeng Chai
Cc: Bokun Zhang
CC: Likun Gao
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c5bdf6eff29e..2f54250f9d58 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3673,6 +3673,15 @@ static const struct attribute *amdgpu_dev_attributes[] =
{
NULL
};
+static bool amdgpu_is_boot_device(struct pci_dev *pdev)
+{
+ struct drm_device *dev = pci_get_drvdata(pdev);
+ struct amdgpu_device *adev = drm_to_adev(dev);
+ struct amdgpu_gmc *gmc = >gmc;
+
+ return drm_aperture_contain_firmware_fb(gmc->aper_base, gmc->aper_size);
+}
+
/**
* amdgpu_device_init - initialize the driver
*
@@ -4082,7 +4091,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
/* this will fail for cards that aren't VGA class devices, just
* ignore it */
if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
- vga_client_register(adev->pdev, amdgpu_device_vga_set_decode,
NULL);
+ vga_client_register(adev->pdev, amdgpu_device_vga_set_decode,
+ amdgpu_is_boot_device);
px = amdgpu_device_supports_px(ddev);
--
2.25.1
[Intel-gfx] [PATCH v1 3/4] drm/amdgpu: Implement the is_boot_device callback function
From: Sui Jingfeng
[why]
The vga_is_firmware_default() defined in drivers/pci/vgaarb.c is
arch-dependent, it's a dummy on non-x86 architectures currently.
This made VGAARB lost an important condition for the arbitration.
It could still be wrong even if we remove the #ifdef and #endif guards.
because the PCI bar will move (resource re-allocation).
[how]
The device that owns the firmware framebuffer should be the default boot
device. This patch adds an arch-independent function to enforce this rule.
The vgaarb subsystem will call back to amdgpu_is_boot_device() function
when drm/amdgpu is successfully bound to an AMDGPU device.
Cc: Alex Deucher
Cc: Christian Konig
Cc: Pan Xinhui
Cc: David Airlie
Cc: Daniel Vetter
Cc: Hawking Zhang
Cc: Mario Limonciello
Cc: Lijo Lazar
Cc: YiPeng Chai
Cc: Bokun Zhang
CC: Likun Gao
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c5bdf6eff29e..2f54250f9d58 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3673,6 +3673,15 @@ static const struct attribute *amdgpu_dev_attributes[] =
{
NULL
};
+static bool amdgpu_is_boot_device(struct pci_dev *pdev)
+{
+ struct drm_device *dev = pci_get_drvdata(pdev);
+ struct amdgpu_device *adev = drm_to_adev(dev);
+ struct amdgpu_gmc *gmc = >gmc;
+
+ return drm_aperture_contain_firmware_fb(gmc->aper_base, gmc->aper_size);
+}
+
/**
* amdgpu_device_init - initialize the driver
*
@@ -4082,7 +4091,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
/* this will fail for cards that aren't VGA class devices, just
* ignore it */
if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
- vga_client_register(adev->pdev, amdgpu_device_vga_set_decode,
NULL);
+ vga_client_register(adev->pdev, amdgpu_device_vga_set_decode,
+ amdgpu_is_boot_device);
px = amdgpu_device_supports_px(ddev);
--
2.25.1
[Intel-gfx] [PATCH v1 1/4] video/aperture: Add a helper to detect if an aperture contains firmware FB
From: Sui Jingfeng
This patch adds the aperture_contain_firmware_fb() function to do the
determination. Unfortunately due to the fact that apertures list will be
freed dynamically, the location and size information of the firmware fb
will be lost after dedicated drivers call
aperture_remove_conflicting_devices(),
aperture_remove_conflicting_pci_devices() or
aperture_remove_all_conflicting_devices() functions
We handle this problem by introducing two static variables which record the
firmware framebuffer's start addrness and end addrness. It assumes that the
system has only one active firmware framebuffer driver at a time.
We don't use the global structure screen_info here, because PCI resource
may get reallocated(the VRAM BAR could be moved) at kernel boot stage.
Cc: Thomas Zimmermann
Cc: Javier Martinez Canillas
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: David Airlie
Cc: Daniel Vetter
Cc: Helge Deller
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/drm_aperture.c | 16
drivers/video/aperture.c | 29 +
include/drm/drm_aperture.h | 2 ++
include/linux/aperture.h | 7 +++
4 files changed, 54 insertions(+)
diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
index 5729f3bb4398..6e5d8a08683c 100644
--- a/drivers/gpu/drm/drm_aperture.c
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -190,3 +190,19 @@ int
drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
return aperture_remove_conflicting_pci_devices(pdev, req_driver->name);
}
EXPORT_SYMBOL(drm_aperture_remove_conflicting_pci_framebuffers);
+
+/**
+ * drm_aperture_contain_firmware_fb - Determine if a aperture contains
firmware framebuffer
+ *
+ * @base: the aperture's base address in physical memory
+ * @size: aperture size in bytes
+ *
+ * Returns:
+ * true on if there is a firmware framebuffer belong to the aperture passed in,
+ * or false otherwise.
+ */
+bool drm_aperture_contain_firmware_fb(resource_size_t base, resource_size_t
size)
+{
+ return aperture_contain_firmware_fb(base, base + size);
+}
+EXPORT_SYMBOL(drm_aperture_contain_firmware_fb);
diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 561be8feca96..5a5422cec669 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -141,6 +141,9 @@ struct aperture_range {
static LIST_HEAD(apertures);
static DEFINE_MUTEX(apertures_lock);
+static resource_size_t firm_fb_start;
+static resource_size_t firm_fb_end;
+
static bool overlap(resource_size_t base1, resource_size_t end1,
resource_size_t base2, resource_size_t end2)
{
@@ -170,6 +173,9 @@ static int devm_aperture_acquire(struct device *dev,
mutex_lock(_lock);
+ firm_fb_start = base;
+ firm_fb_end = end;
+
list_for_each(pos, ) {
ap = container_of(pos, struct aperture_range, lh);
if (overlap(base, end, ap->base, ap->base + ap->size)) {
@@ -377,3 +383,26 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev
*pdev, const char *na
}
EXPORT_SYMBOL(aperture_remove_conflicting_pci_devices);
+
+/**
+ * aperture_contain_firmware_fb - Detect if the firmware framebuffer belong to
+ *a aperture.
+ * @ap_start: the aperture's start address in physical memory
+ * @ap_end: the aperture's end address in physical memory
+ *
+ * Returns:
+ * true on if there is a firmware framebuffer belong to the aperture passed in,
+ * or false otherwise.
+ */
+bool aperture_contain_firmware_fb(resource_size_t ap_start, resource_size_t
ap_end)
+{
+ /* No firmware framebuffer support */
+ if (!firm_fb_start || !firm_fb_end)
+ return false;
+
+ if (firm_fb_start >= ap_start && firm_fb_end <= ap_end)
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(aperture_contain_firmware_fb);
diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
index cbe33b49fd5d..6a0b9bacb081 100644
--- a/include/drm/drm_aperture.h
+++ b/include/drm/drm_aperture.h
@@ -35,4 +35,6 @@ drm_aperture_remove_framebuffers(const struct drm_driver
*req_driver)
req_driver);
}
+bool drm_aperture_contain_firmware_fb(resource_size_t base, resource_size_t
size);
+
#endif
diff --git a/include/linux/aperture.h b/include/linux/aperture.h
index 1a9a88b11584..d4dc5917c49b 100644
--- a/include/linux/aperture.h
+++ b/include/linux/aperture.h
@@ -19,6 +19,8 @@ int aperture_remove_conflicting_devices(resource_size_t base,
resource_size_t si
int __aperture_remove_legacy_vga_devices(struct pci_dev *pdev);
int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char
*name);
+
+bool aperture_contain_firmware_fb(resource_size_t ap_start, resource_size_t
ap_end);
#else
static inline int devm_aperture_acquire_for_platform_device(struct
platform_device *pdev,
Re: [Intel-gfx] [PATCH] Covert to use time_before macro
Correct the format of the Subject in v2: https://lore.kernel.org/all/20230706072924.2562-1-zhangze...@vivo.com/
[Intel-gfx] [PATCH v2 0/6] PCI/VGA: Improve the default VGA device selection
From: Sui Jingfeng Currently, the default VGA device selection is not perfect. Potential problems are: 1) This function is a no-op on non-x86 architectures. 2) It does not take the PCI Bar may get relocated into consideration. 3) It is not effective for the PCI device without a dedicated VRAM Bar. 4) It is device-agnostic, thus it has to waste the effort to iterate all of the PCI Bar to find the VRAM aperture. 5) It has invented lots of methods to determine which one is the default boot device on a multiple video card coexistence system. But this is still a policy because it doesn't give the user a choice to override. With the observation that device drivers or video aperture helpers may have better knowledge about which PCI bar contains the firmware FB, This patch tries to solve the above problems by introducing a function callback to the vga_client_register() function interface. DRM device drivers for the PCI device need to register the is_boot_device() function callback during the driver loading time. Once the driver binds the device successfully, VRAARB will call back to the driver. This gives the device drivers a chance to provide accurate boot device identification. Which in turn unlock the abitration service to non-x86 architectures. A device driver can also pass a NULL pointer to keep the original behavior. This series is applied on the drm-tip branch (with a cleanup patch set[1] applied beforehand) [1] https://patchwork.freedesktop.org/series/120053/ v2: * Add a simple implemment for drm/i915 and drm/ast * Pick up all tags (Mario) Sui Jingfeng (6): video/aperture: Add a helper to detect if an aperture contains firmware FB PCI/VGA: Improve the default VGA device selection drm/amdgpu: Implement the is_boot_device callback function drm/radeon: Implement the is_boot_device callback function drm/i915: Implement the is_boot_device callback function drm/ast: Register as a vga client to vgaarb by calling vga_client_register() drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++- drivers/gpu/drm/ast/ast_drv.c | 29 drivers/gpu/drm/drm_aperture.c | 16 +++ drivers/gpu/drm/i915/display/intel_vga.c | 32 -- drivers/gpu/drm/nouveau/nouveau_vga.c | 2 +- drivers/gpu/drm/radeon/radeon_device.c | 12 +++- drivers/pci/vgaarb.c | 21 +- drivers/vfio/pci/vfio_pci_core.c | 2 +- drivers/video/aperture.c | 29 include/drm/drm_aperture.h | 2 ++ include/linux/aperture.h | 7 + include/linux/vgaarb.h | 8 -- 12 files changed, 162 insertions(+), 10 deletions(-) -- 2.25.1
[Intel-gfx] [PATCH] Covert to use time_before macro
Use time_before macro instead of open it for readability. Signed-off-by: Zehao Zhang --- drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c index 86b5a9ba323d..9145f9e22860 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c @@ -57,7 +57,7 @@ static bool pool_free_older_than(struct intel_gt_buffer_pool *pool, long keep) node = list_entry(pos, typeof(*node), link); age = READ_ONCE(node->age); - if (!age || jiffies - age < keep) + if (!age || time_before(jiffies, age + keep)) break; /* Check we are the first to claim this node */ -- 2.35.3
[Intel-gfx] [PATCH v2 5/6] drm/i915: Implement the is_boot_device callback function
From: Sui Jingfeng
This patch add a function to identify if a device is the default boot
selected by the firmware, it require the GPU has firmware framebuffer
driver support (such as efifb). If a specific hardware doesn't have
firmware framebuffer support, then the introduced function will just
return false. But even in this case, it still do no harms.
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: David Airlie
Cc: Daniel Vetter
Cc: "Ville Syrjälä"
Cc: Lyude Paul
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/i915/display/intel_vga.c | 31 +++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_vga.c
b/drivers/gpu/drm/i915/display/intel_vga.c
index 98d7d4dffe9f..7a0ba677c932 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.c
+++ b/drivers/gpu/drm/i915/display/intel_vga.c
@@ -6,6 +6,8 @@
#include
#include
+#include
+
#include
#include "soc/intel_gmch.h"
@@ -113,6 +115,32 @@ intel_vga_set_decode(struct pci_dev *pdev, bool
enable_decode)
return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
}
+static bool intel_vga_is_default_boot_device(struct pci_dev *pdev)
+{
+ struct drm_i915_private *i915 = pdev_to_i915(pdev);
+ struct drm_device *drm = >drm;
+ struct i915_gem_mm *mm = >mm;
+ struct intel_memory_region *mr;
+ unsigned int i;
+ bool ret;
+
+ for (i = 0; i < ARRAY_SIZE(mm->regions); i++) {
+ mr = mm->regions[i];
+ if (mr) {
+ ret = drm_aperture_contain_firmware_fb(mr->io_start,
+ mr->io_size);
+
+ if (ret) {
+ drm_dbg(drm, "%s contains firmware fb\n",
+ mr->name);
+ return true;
+ }
+ }
+ }
+
+ return false;
+}
+
int intel_vga_register(struct drm_i915_private *i915)
{
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
@@ -126,7 +154,8 @@ int intel_vga_register(struct drm_i915_private *i915)
* then we do not take part in VGA arbitration and the
* vga_client_register() fails with -ENODEV.
*/
- ret = vga_client_register(pdev, intel_vga_set_decode, NULL);
+ ret = vga_client_register(pdev, intel_vga_set_decode,
+ intel_vga_is_default_boot_device);
if (ret && ret != -ENODEV)
return ret;
--
2.25.1
[Intel-gfx] [PATCH v1 0/4] PCI/VGA: Improve the default VGA device selection
From: Sui Jingfeng Currently, the default VGA device selection is not perfect. Potential problems are: 1) This function is a no-op on non-x86 architectures. 2) It does not take the PCI Bar may get relocated into consideration. 3) It is not effective for the PCI device without a dedicated VRAM Bar. 4) It is device-agnostic, thus it has to waste the effort to iterate all of the PCI Bar to find the VRAM aperture. 5) It has invented lots of methods to determine which one is the default boot device on a multiple video card coexistence system. But this is still a policy because it doesn't give the user a choice to override. With the observation that device drivers or video aperture helpers may have better knowledge about which PCI bar contains the firmware FB, This patch tries to solve the above problems by introducing a function callback to the vga_client_register() function interface. DRM device drivers for the PCI device need to register the is_boot_device() function callback during the driver loading time. Once the driver binds the device successfully, VRAARB will call back to the driver. This gives the device drivers a chance to provide accurate boot device identification. Which in turn unlock the abitration service to non-x86 architectures. A device driver can also pass a NULL pointer to the keep the original behavior. Sui Jingfeng (4): video/aperture: Add a helper to detect if an aperture contains firmware FB PCI/VGA: Improve the default VGA device selection drm/amdgpu: Implement the is_boot_device callback function drm/radeon: Implement the is_boot_device callback function drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 - drivers/gpu/drm/drm_aperture.c | 16 drivers/gpu/drm/i915/display/intel_vga.c | 3 +-- drivers/gpu/drm/nouveau/nouveau_vga.c | 2 +- drivers/gpu/drm/radeon/radeon_device.c | 12 - drivers/pci/vgaarb.c | 21 +++- drivers/vfio/pci/vfio_pci_core.c | 2 +- drivers/video/aperture.c | 29 ++ include/drm/drm_aperture.h | 2 ++ include/linux/aperture.h | 7 ++ include/linux/vgaarb.h | 8 +++--- 11 files changed, 104 insertions(+), 10 deletions(-) -- 2.25.1
Re: [Intel-gfx] [PATCH] drm/i915/pmu: Use local64_try_cmpxchg in i915_pmu_event_read
On Tue, Jul 4, 2023 at 10:37 AM Jani Nikula wrote: > > On Tue, 04 Jul 2023, Uros Bizjak wrote: > > On Tue, Jul 4, 2023 at 9:28 AM Jani Nikula > > wrote: > >> You could save everyone a lot of time by actually documenting what these > >> functions do. Assume you don't know what local64_try_cmpxchg() does, and > >> see how many calls you have to go through to figure it out. > > > > These functions are documented in Documentation/atomic_t.txt (under > > "RMW ops:" section), and the difference is explained in a separate > > section "CMPXCHG vs TRY_CMPXCGS" in the same file. > > Thanks, but *sigh*. > > No kernel-doc above the functions, not even a regular comment > referencing atomic_t.txt. > > $ git grep local.*_try -- Documentation > [nothing] Unfortunately, this was always the state w.r.t. local.* atomic functions. There is an effort to improve the documentation of atomics, perhaps it will be also extended to local variants. Uros.
Re: [Intel-gfx] [PATCH 3/3] drm/i915/uncore: optimize CONFIG_DRM_I915_DEBUG_MMIO=n more
On Thu, 06 Jul 2023, Tvrtko Ursulin wrote:
> On 04/07/2023 10:48, Jani Nikula wrote:
>> While the default for the mmio_debug parameter depends on
>> CONFIG_DRM_I915_DEBUG_MMIO, we look it up and include all the code for
>> unclaimed reg debugging even when CONFIG_DRM_I915_DEBUG_MMIO=n. Fix it.
>>
>> Cc: Lee Shawn C
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/i915/intel_uncore.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> b/drivers/gpu/drm/i915/intel_uncore.c
>> index dfefad5a5fec..da2edde4b6f6 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -1929,7 +1929,8 @@ static inline bool __must_check
>> unclaimed_reg_debug_header(struct intel_uncore *uncore,
>> const i915_reg_t reg, const bool read)
>> {
>> -if (likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
>> +if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO) ||
>> +likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
>> return false;
>
> But now it would not be possible to enable mmio_debug, if Kconfig
> _default_ is 'n'. What am I missing?
You're not missing anything, I am. *facepalm*
The question is, are the first two acceptable without the third?
BR,
Jani.
>
> Regards,
>
> Tvrtko
>
>>
>> /* interrupts are disabled and re-enabled around uncore->lock usage */
--
Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PULL] drm-misc-next-fixes
Hi Dave and Daniel, here's the weekly PR for drm-misc-next-fixes. Best regards Thomas drm-misc-next-fixes-2023-07-06: Short summary of fixes pull: * panel: Fix mode on Starry-ili9882t The following changes since commit 861c249cd782cb9f2d5a881bbb32e8da7f0c1192: arch/sparc: Add module license and description for fbdev helpers (2023-06-29 13:30:02 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2023-07-06 for you to fetch changes up to 59bba51ec2a50e3dc5c3ee80f0a23207346303ff: drm/panel: Fine tune Starry-ili9882t panel HFP and HBP (2023-06-29 17:35:34 -0700) Short summary of fixes pull: * panel: Fix mode on Starry-ili9882t Cong Yang (1): drm/panel: Fine tune Starry-ili9882t panel HFP and HBP drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Re: [Intel-gfx] [PATCH] drm/i915: Remove some dead "code"
On 05/07/2023 13:08, Jani Nikula wrote:
On Wed, 05 Jul 2023, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Commit 2caffbf11762 ("drm/i915: Revoke mmaps and prevent access to fence
registers across reset") removed the temporary implementation of a reset
under stop machine but forgot to remove this one commented out define.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Jani Nikula
Thanks, pushed!
Regards,
Tvrtko
---
drivers/gpu/drm/i915/gt/intel_reset.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c
b/drivers/gpu/drm/i915/gt/intel_reset.c
index 6916eba3bd33..cdbc08dad7ae 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -35,9 +35,6 @@
#define RESET_MAX_RETRIES 3
-/* XXX How to handle concurrent GGTT updates using tiling registers? */
-#define RESET_UNDER_STOP_MACHINE 0
-
static void client_mark_guilty(struct i915_gem_context *ctx, bool banned)
{
struct drm_i915_file_private *file_priv = ctx->file_priv;
Re: [Intel-gfx] [PATCH 3/3] drm/i915/uncore: optimize CONFIG_DRM_I915_DEBUG_MMIO=n more
On 04/07/2023 10:48, Jani Nikula wrote:
While the default for the mmio_debug parameter depends on
CONFIG_DRM_I915_DEBUG_MMIO, we look it up and include all the code for
unclaimed reg debugging even when CONFIG_DRM_I915_DEBUG_MMIO=n. Fix it.
Cc: Lee Shawn C
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_uncore.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index dfefad5a5fec..da2edde4b6f6 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1929,7 +1929,8 @@ static inline bool __must_check
unclaimed_reg_debug_header(struct intel_uncore *uncore,
const i915_reg_t reg, const bool read)
{
- if (likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
+ if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_MMIO) ||
+ likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
return false;
But now it would not be possible to enable mmio_debug, if Kconfig
_default_ is 'n'. What am I missing?
Regards,
Tvrtko
/* interrupts are disabled and re-enabled around uncore->lock usage */
Re: [Intel-gfx] [PATCH 2/3] drm/i915/uncore: fix race around i915->params.mmio_debug
On 04/07/2023 10:48, Jani Nikula wrote:
Only check the conditions for unclaimed reg debug once to avoid locking
problems when i915->params.mmio_debug changes between header and footer.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8749
Cc: Lee Shawn C
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_uncore.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index a88aa342b623..dfefad5a5fec 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1925,27 +1925,26 @@ __unclaimed_previous_reg_debug(struct intel_uncore
*uncore,
i915_mmio_reg_offset(reg));
}
-static inline void
+static inline bool __must_check
unclaimed_reg_debug_header(struct intel_uncore *uncore,
const i915_reg_t reg, const bool read)
{
if (likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
- return;
+ return false;
/* interrupts are disabled and re-enabled around uncore->lock usage */
lockdep_assert_held(>lock);
spin_lock(>debug->lock);
__unclaimed_previous_reg_debug(uncore, reg, read);
+
+ return true;
}
static inline void
unclaimed_reg_debug_footer(struct intel_uncore *uncore,
const i915_reg_t reg, const bool read)
{
- if (likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
- return;
-
/* interrupts are disabled and re-enabled around uncore->lock usage */
lockdep_assert_held(>lock);
@@ -2008,13 +2007,15 @@ __gen2_read(64)
#define GEN6_READ_HEADER(x) \
u32 offset = i915_mmio_reg_offset(reg); \
unsigned long irqflags; \
+ bool unclaimed_reg_debug; \
u##x val = 0; \
assert_rpm_wakelock_held(uncore->rpm); \
spin_lock_irqsave(>lock, irqflags); \
- unclaimed_reg_debug_header(uncore, reg, true)
+ unclaimed_reg_debug = unclaimed_reg_debug_header(uncore, reg, true)
#define GEN6_READ_FOOTER \
- unclaimed_reg_debug_footer(uncore, reg, true); \
+ if (unclaimed_reg_debug) \
+ unclaimed_reg_debug_footer(uncore, reg, true); \
spin_unlock_irqrestore(>lock, irqflags); \
trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
return val
@@ -2112,13 +2113,15 @@ __gen2_write(32)
#define GEN6_WRITE_HEADER \
u32 offset = i915_mmio_reg_offset(reg); \
unsigned long irqflags; \
+ bool unclaimed_reg_debug; \
trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
assert_rpm_wakelock_held(uncore->rpm); \
spin_lock_irqsave(>lock, irqflags); \
- unclaimed_reg_debug_header(uncore, reg, false)
+ unclaimed_reg_debug = unclaimed_reg_debug_header(uncore, reg, false)
#define GEN6_WRITE_FOOTER \
- unclaimed_reg_debug_footer(uncore, reg, false); \
+ if (unclaimed_reg_debug) \
+ unclaimed_reg_debug_footer(uncore, reg, false); \
spin_unlock_irqrestore(>lock, irqflags)
#define __gen6_write(x) \
Reviewed-by: Tvrtko Ursulin
Regards,
Tvrtko
Re: [Intel-gfx] [PATCH 1/3] drm/i915/uncore: split unclaimed_reg_debug() to header and footer
On 04/07/2023 10:48, Jani Nikula wrote:
Make it easier to have different logic for the two for follow-up.
Cc: Lee Shawn C
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_uncore.c | 37 +
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 796ebfe6c550..a88aa342b623 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1926,10 +1926,8 @@ __unclaimed_previous_reg_debug(struct intel_uncore
*uncore,
}
static inline void
-unclaimed_reg_debug(struct intel_uncore *uncore,
- const i915_reg_t reg,
- const bool read,
- const bool before)
+unclaimed_reg_debug_header(struct intel_uncore *uncore,
+ const i915_reg_t reg, const bool read)
{
if (likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
return;
@@ -1937,13 +1935,22 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
/* interrupts are disabled and re-enabled around uncore->lock usage */
lockdep_assert_held(>lock);
- if (before) {
- spin_lock(>debug->lock);
- __unclaimed_previous_reg_debug(uncore, reg, read);
- } else {
- __unclaimed_reg_debug(uncore, reg, read);
- spin_unlock(>debug->lock);
- }
+ spin_lock(>debug->lock);
+ __unclaimed_previous_reg_debug(uncore, reg, read);
+}
+
+static inline void
+unclaimed_reg_debug_footer(struct intel_uncore *uncore,
+ const i915_reg_t reg, const bool read)
+{
+ if (likely(!uncore->i915->params.mmio_debug) || !uncore->debug)
+ return;
+
+ /* interrupts are disabled and re-enabled around uncore->lock usage */
+ lockdep_assert_held(>lock);
+
+ __unclaimed_reg_debug(uncore, reg, read);
+ spin_unlock(>debug->lock);
}
#define __vgpu_read(x) \
@@ -2004,10 +2011,10 @@ __gen2_read(64)
u##x val = 0; \
assert_rpm_wakelock_held(uncore->rpm); \
spin_lock_irqsave(>lock, irqflags); \
- unclaimed_reg_debug(uncore, reg, true, true)
+ unclaimed_reg_debug_header(uncore, reg, true)
#define GEN6_READ_FOOTER \
- unclaimed_reg_debug(uncore, reg, true, false); \
+ unclaimed_reg_debug_footer(uncore, reg, true); \
spin_unlock_irqrestore(>lock, irqflags); \
trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
return val
@@ -2108,10 +2115,10 @@ __gen2_write(32)
trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
assert_rpm_wakelock_held(uncore->rpm); \
spin_lock_irqsave(>lock, irqflags); \
- unclaimed_reg_debug(uncore, reg, false, true)
+ unclaimed_reg_debug_header(uncore, reg, false)
#define GEN6_WRITE_FOOTER \
- unclaimed_reg_debug(uncore, reg, false, false); \
+ unclaimed_reg_debug_footer(uncore, reg, false); \
spin_unlock_irqrestore(>lock, irqflags)
#define __gen6_write(x) \
Reviewed-by: Tvrtko Ursulin
Regards,
Tvrtko
Re: [Intel-gfx] [PATCH] drm/i915: Fix the disabling sequence for Bigjoiner
On Thu, Jul 06, 2023 at 11:47:26AM +0300, Imre Deak wrote: > On Thu, Jul 06, 2023 at 11:24:21AM +0300, Lisovskiy, Stanislav wrote: > > On Wed, Jul 05, 2023 at 06:32:51PM +0300, Imre Deak wrote: > > > On Thu, May 25, 2023 at 01:10:36PM +0300, Stanislav Lisovskiy wrote: > > > > According to BSpec 49190, when enabling crtcs, we first setup > > > > slave and then master crtc, however for disabling it should go > > > > vice versa, i.e first master, then slave, however current code > > > > does disabling in a same way as enabling. Fix this, by skipping > > > > non-master crtcs, instead of non-slaves. > > > > > > > > Signed-off-by: Stanislav Lisovskiy > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > index 0490c6412ab5..68958ba0ef49 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -6662,7 +6662,7 @@ static void intel_commit_modeset_disables(struct > > > > intel_atomic_state *state) > > > > */ > > > > if (!is_trans_port_sync_slave(old_crtc_state) && > > > > !intel_dp_mst_is_slave_trans(old_crtc_state) && > > > > - !intel_crtc_is_bigjoiner_slave(old_crtc_state)) > > > > + !intel_crtc_is_bigjoiner_master(old_crtc_state)) > > > > > > I don't see what does this fix. The sequence is correct at the moment > > > and this change would break it, leaving the encoder PLL enabled > > > incorrectly when the encoder->post_pll_disable() hook is called. Hence > > > it's NAK from side. > > > > Well, as I pointed out the BSpec 49190 instructs us to disable master > > first, then slave. Current code skips all non-slaves in first cycle, > > i.e it disables first slaves and then masters. Which is _wrong_. > > This is correct at the moment, followed in the encoder's disable hook > which is only assigned to the master CRTC. Yep, I see now why it was implemented this way. We basically handle everything in a single hook, taking care of the correct sequence. As I understood otherwise we are going to have problems with the pll subsystem, i.e we can't disable pll for master before the slaves(basically means our pll subsystem contradicts what the crtc/pipe/encoder sequence requires). I still think this is bery counterintuitive implementation, i.e when there is a single hook for master taking care of everything, while slaves are just noop. This makes the whole thing very prone for screwing things up. Ideally we should still have fully functional hooks for all slaves. If the pll stuff requires special treatment, that probably should be dealt somehow separately(don't have any solution for that yet), but definitely we shouldn't live further like that. Things might get even more complicated in future. Stan > > > Anything else in particular, where do you need clarifications? > > > > Stan > > > > > > > > > continue; > > > > > > > > intel_old_crtc_state_disables(state, old_crtc_state, > > > > -- > > > > 2.37.3 > > > >
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Remove some dead "code" (rev2)
== Series Details == Series: drm/i915: Remove some dead "code" (rev2) URL : https://patchwork.freedesktop.org/series/120222/ State : failure == Summary == CI Bug Log - changes from CI_DRM_13347_full -> Patchwork_120222v2_full Summary --- **FAILURE** Serious unknown changes coming with Patchwork_120222v2_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_120222v2_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. Participating hosts (9 -> 9) -- No changes in participating hosts Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_120222v2_full: ### IGT changes ### Possible regressions * igt@gem_softpin@allocator-basic-reserve: - shard-mtlp: [PASS][1] -> [ABORT][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/shard-mtlp-8/igt@gem_soft...@allocator-basic-reserve.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-mtlp-1/igt@gem_soft...@allocator-basic-reserve.html Known issues Here are the changes found in Patchwork_120222v2_full that come from known issues: ### IGT changes ### Issues hit * igt@drm_fdinfo@most-busy-check-all@rcs0: - shard-rkl: [PASS][3] -> [FAIL][4] ([i915#7742]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/shard-rkl-1/igt@drm_fdinfo@most-busy-check-...@rcs0.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-rkl-7/igt@drm_fdinfo@most-busy-check-...@rcs0.html * igt@gem_basic@multigpu-create-close: - shard-tglu: NOTRUN -> [SKIP][5] ([i915#7697]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-tglu-9/igt@gem_ba...@multigpu-create-close.html * igt@gem_close_race@multigpu-basic-process: - shard-rkl: NOTRUN -> [SKIP][6] ([i915#7697]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-rkl-2/igt@gem_close_r...@multigpu-basic-process.html * igt@gem_ctx_sseu@mmap-args: - shard-rkl: NOTRUN -> [SKIP][7] ([i915#280]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-rkl-2/igt@gem_ctx_s...@mmap-args.html * igt@gem_eio@hibernate: - shard-dg2: [PASS][8] -> [ABORT][9] ([i915#7975] / [i915#8213]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/shard-dg2-11/igt@gem_...@hibernate.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-dg2-1/igt@gem_...@hibernate.html * igt@gem_exec_fair@basic-flow@rcs0: - shard-tglu: [PASS][10] -> [FAIL][11] ([i915#2842]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/shard-tglu-7/igt@gem_exec_fair@basic-f...@rcs0.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-tglu-4/igt@gem_exec_fair@basic-f...@rcs0.html * igt@gem_exec_fair@basic-none-vip@rcs0: - shard-rkl: NOTRUN -> [FAIL][12] ([i915#2842]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-rkl-2/igt@gem_exec_fair@basic-none-...@rcs0.html * igt@gem_exec_fair@basic-none@bcs0: - shard-rkl: [PASS][13] -> [FAIL][14] ([i915#2842]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/shard-rkl-6/igt@gem_exec_fair@basic-n...@bcs0.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-rkl-1/igt@gem_exec_fair@basic-n...@bcs0.html * igt@gem_exec_fair@basic-pace-share: - shard-mtlp: NOTRUN -> [SKIP][15] ([i915#4473] / [i915#4771]) +1 similar issue [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-mtlp-5/igt@gem_exec_f...@basic-pace-share.html * igt@gem_exec_reloc@basic-cpu-read-noreloc: - shard-mtlp: NOTRUN -> [SKIP][16] ([i915#3281]) +4 similar issues [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-mtlp-5/igt@gem_exec_re...@basic-cpu-read-noreloc.html * igt@gem_exec_schedule@preempt-queue: - shard-mtlp: NOTRUN -> [SKIP][17] ([i915#4812]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-mtlp-5/igt@gem_exec_sched...@preempt-queue.html * igt@gem_exec_schedule@wide@rcs0: - shard-glk: [PASS][18] -> [FAIL][19] ([i915#6659]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/shard-glk3/igt@gem_exec_schedule@w...@rcs0.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/shard-glk3/igt@gem_exec_schedule@w...@rcs0.html * igt@gem_exec_whisper@basic-fds-forked-all: - shard-mtlp: [PASS][20] -> [FAIL][21] ([i915#6363]) [20]:
[Intel-gfx] [drm-rerere] nightly.conf: drop sound tree from drm-tip altogether
We used to have the sound branches be part of drm-tip to help development of DP and HDMI audio. However, we always used to run into problems with the sound branches merging Linus' master at non-tagged random commits, wreaking havoc especially during the merge windows. We only ever want to have tagged stuff merged back from Linus' tree to drm-tip. We introduced a mechanism in dim to hold back branches at certain commits, just to hold back sound branches when problems arise. We moved it along, but in the end nobody has updated this in literally years, and sound branches have been held back at v5.13. The merge window is currently open, and AFAICT the sound/for-linus branch again contains commits from the merge window. Let's just forget about the sound tree, as nobody has really missed it since v5.13, and focus on the drm branches. Signed-off-by: Jani Nikula --- nightly.conf | 7 --- 1 file changed, 7 deletions(-) diff --git a/nightly.conf b/nightly.conf index 73aec820e98f..c1e22800e276 100644 --- a/nightly.conf +++ b/nightly.conf @@ -46,11 +46,6 @@ git://anongit.freedesktop.org/drm/drm https://anongit.freedesktop.org/git/drm/drm https://anongit.freedesktop.org/git/drm/drm.git " -drm_tip_repos[sound-upstream]=" -git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git -https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git -https://kernel.googlesource.com/pub/scm/linux/kernel/git/tiwai/sound.git -" drm_tip_repos[linux-upstream]=" git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git @@ -79,8 +74,6 @@ drm_tip_config=( "drm-intel drm-intel-next" "drm-intel drm-intel-gt-next" - "sound-upstream for-linus v5.13" - "sound-upstream for-nextv5.13" "drm-intel topic/core-for-CI" "drm-misc topic/i915-ttm" "drmtopic/nouveau-misc" -- 2.39.2
Re: [Intel-gfx] [PATCH] drm/i915: Fix the disabling sequence for Bigjoiner
On Thu, Jul 06, 2023 at 11:47:26AM +0300, Imre Deak wrote: > On Thu, Jul 06, 2023 at 11:24:21AM +0300, Lisovskiy, Stanislav wrote: > > On Wed, Jul 05, 2023 at 06:32:51PM +0300, Imre Deak wrote: > > > On Thu, May 25, 2023 at 01:10:36PM +0300, Stanislav Lisovskiy wrote: > > > > According to BSpec 49190, when enabling crtcs, we first setup > > > > slave and then master crtc, however for disabling it should go > > > > vice versa, i.e first master, then slave, however current code > > > > does disabling in a same way as enabling. Fix this, by skipping > > > > non-master crtcs, instead of non-slaves. > > > > > > > > Signed-off-by: Stanislav Lisovskiy > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > index 0490c6412ab5..68958ba0ef49 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -6662,7 +6662,7 @@ static void intel_commit_modeset_disables(struct > > > > intel_atomic_state *state) > > > > */ > > > > if (!is_trans_port_sync_slave(old_crtc_state) && > > > > !intel_dp_mst_is_slave_trans(old_crtc_state) && > > > > - !intel_crtc_is_bigjoiner_slave(old_crtc_state)) > > > > + !intel_crtc_is_bigjoiner_master(old_crtc_state)) > > > > > > I don't see what does this fix. The sequence is correct at the moment > > > and this change would break it, leaving the encoder PLL enabled > > > incorrectly when the encoder->post_pll_disable() hook is called. Hence > > > it's NAK from side. > > > > Well, as I pointed out the BSpec 49190 instructs us to disable master > > first, then slave. Current code skips all non-slaves in first cycle, > > i.e it disables first slaves and then masters. Which is _wrong_. > > This is correct at the moment, followed in the encoder's disable hook > which is only assigned to the master CRTC. Hmmm... I will check it. > > > Anything else in particular, where do you need clarifications? > > > > Stan > > > > > > > > > continue; > > > > > > > > intel_old_crtc_state_disables(state, old_crtc_state, > > > > -- > > > > 2.37.3 > > > >
Re: [Intel-gfx] [PATCH] drm/i915: Fix the disabling sequence for Bigjoiner
On Thu, Jul 06, 2023 at 11:24:21AM +0300, Lisovskiy, Stanislav wrote: > On Wed, Jul 05, 2023 at 06:32:51PM +0300, Imre Deak wrote: > > On Thu, May 25, 2023 at 01:10:36PM +0300, Stanislav Lisovskiy wrote: > > > According to BSpec 49190, when enabling crtcs, we first setup > > > slave and then master crtc, however for disabling it should go > > > vice versa, i.e first master, then slave, however current code > > > does disabling in a same way as enabling. Fix this, by skipping > > > non-master crtcs, instead of non-slaves. > > > > > > Signed-off-by: Stanislav Lisovskiy > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > index 0490c6412ab5..68958ba0ef49 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -6662,7 +6662,7 @@ static void intel_commit_modeset_disables(struct > > > intel_atomic_state *state) > > >*/ > > > if (!is_trans_port_sync_slave(old_crtc_state) && > > > !intel_dp_mst_is_slave_trans(old_crtc_state) && > > > - !intel_crtc_is_bigjoiner_slave(old_crtc_state)) > > > + !intel_crtc_is_bigjoiner_master(old_crtc_state)) > > > > I don't see what does this fix. The sequence is correct at the moment > > and this change would break it, leaving the encoder PLL enabled > > incorrectly when the encoder->post_pll_disable() hook is called. Hence > > it's NAK from side. > > Well, as I pointed out the BSpec 49190 instructs us to disable master > first, then slave. Current code skips all non-slaves in first cycle, > i.e it disables first slaves and then masters. Which is _wrong_. This is correct at the moment, followed in the encoder's disable hook which is only assigned to the master CRTC. > Anything else in particular, where do you need clarifications? > > Stan > > > > > > continue; > > > > > > intel_old_crtc_state_disables(state, old_crtc_state, > > > -- > > > 2.37.3 > > >
Re: [Intel-gfx] [PATCH 00/13] drm/i915/sdvo: DDC rework and fixes
On Wed, 05 Jul 2023, Ville Syrjala wrote: > From: Ville Syrjälä > > I have plans to switch the whole driver over to using > drm_connector_init_with_ddc(), and thus populate the > sysfs "ddc" consistently. The biggest hurdle is the > SDVO DDC handling, so start by cleaning that up. I support the goal of moving towards drm_connector_init_with_ddc(), and after that, to drm_edid_read() where applicable. I admit to being a notch less diligent in reviewing all the details of legacy encoder code, but there are no obvious mistakes here that I could spot, and overall it makes sense. Thanks, Jani. > > I also found some other issues with the SDVO code so > some additional fixes are also included. > > Ville Syrjälä (13): > drm/i915/sdvo: Issue SetTargetOutput prior ot GetAttachedDisplays > drm/i915/sdvo: Protect macro args > drm/i915/sdvo: s/sdvo_inputs_mask/sdvo_num_inputs/ > drm/i915: Don't warn about zero N/P in *_calc_dpll_params() > drm/i915: Fully populate crtc_state->dpll > drm/i915/sdvo: Pick the TV dotclock from adjusted_mode > drm/i915/sdvo: Fail gracefully if the TV dotclock is out of range > drm/i915/sdvo: Nuke attached_output tracking > drm/i915/sdvo: Initialize the encoder ealier > drm/i915/sdvo: Nuke the duplicate sdvo->port > drm/i915/sdvo: Get rid of the per-connector i2c symlink > drm/i915/sdvo: Rework DDC bus handling > drm/i915/sdvo: Print out the i2c pin and slave address > > drivers/gpu/drm/i915/display/intel_dpll.c | 54 ++- > drivers/gpu/drm/i915/display/intel_sdvo.c | 357 +- > .../gpu/drm/i915/display/intel_sdvo_regs.h| 2 +- > 3 files changed, 219 insertions(+), 194 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 05/13] drm/i915: Fully populate crtc_state->dpll
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Call *_calc_dpll_params() even in cases where the encoder has
> computed the DPLL params for us.
>
> The SDVO TV output code doesn't populate crtc_state->dpll.dot
> leading to the dotclock getting calculated as zero, and that
> leads to all kinds of real problems. The g4x DP code also
> doesn't populate the derived dividers nor .vco, which could
> also create some confusion.
>
> Signed-off-by: Ville Syrjälä
It's entirely possible there are corner cases I missed, but overall
makes sense.
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_dpll.c | 17 +++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c
> b/drivers/gpu/drm/i915/display/intel_dpll.c
> index 71bfeea4cef2..2255ad651486 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll.c
> @@ -1182,6 +1182,8 @@ static int ilk_crtc_compute_clock(struct
> intel_atomic_state *state,
> refclk, NULL, _state->dpll))
> return -EINVAL;
>
> + i9xx_calc_dpll_params(refclk, _state->dpll);
> +
> ilk_compute_dpll(crtc_state, _state->dpll,
>_state->dpll);
>
> @@ -1256,6 +1258,8 @@ static int chv_crtc_compute_clock(struct
> intel_atomic_state *state,
> refclk, NULL, _state->dpll))
> return -EINVAL;
>
> + chv_calc_dpll_params(refclk, _state->dpll);
> +
> chv_compute_dpll(crtc_state);
>
> /* FIXME this is a mess */
> @@ -1278,9 +1282,10 @@ static int vlv_crtc_compute_clock(struct
> intel_atomic_state *state,
>
> if (!crtc_state->clock_set &&
> !vlv_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> - refclk, NULL, _state->dpll)) {
> + refclk, NULL, _state->dpll))
> return -EINVAL;
> - }
> +
> + vlv_calc_dpll_params(refclk, _state->dpll);
>
> vlv_compute_dpll(crtc_state);
>
> @@ -1330,6 +1335,8 @@ static int g4x_crtc_compute_clock(struct
> intel_atomic_state *state,
> refclk, NULL, _state->dpll))
> return -EINVAL;
>
> + i9xx_calc_dpll_params(refclk, _state->dpll);
> +
> i9xx_compute_dpll(crtc_state, _state->dpll,
> _state->dpll);
>
> @@ -1368,6 +1375,8 @@ static int pnv_crtc_compute_clock(struct
> intel_atomic_state *state,
> refclk, NULL, _state->dpll))
> return -EINVAL;
>
> + pnv_calc_dpll_params(refclk, _state->dpll);
> +
> i9xx_compute_dpll(crtc_state, _state->dpll,
> _state->dpll);
>
> @@ -1404,6 +1413,8 @@ static int i9xx_crtc_compute_clock(struct
> intel_atomic_state *state,
>refclk, NULL, _state->dpll))
> return -EINVAL;
>
> + i9xx_calc_dpll_params(refclk, _state->dpll);
> +
> i9xx_compute_dpll(crtc_state, _state->dpll,
> _state->dpll);
>
> @@ -1444,6 +1455,8 @@ static int i8xx_crtc_compute_clock(struct
> intel_atomic_state *state,
>refclk, NULL, _state->dpll))
> return -EINVAL;
>
> + i9xx_calc_dpll_params(refclk, _state->dpll);
> +
> i8xx_compute_dpll(crtc_state, _state->dpll,
> _state->dpll);
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 13/13] drm/i915/sdvo: Print out the i2c pin and slave address
On Wed, 05 Jul 2023, Ville Syrjala wrote: > From: Ville Syrjälä > > To reduce the guesswork a bit let's print out the SDVO > device i2c bus and slave address during init. > > Signed-off-by: Ville Syrjälä Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/display/intel_sdvo.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c > b/drivers/gpu/drm/i915/display/intel_sdvo.c > index d7edb5714c4c..f0494b9aefe5 100644 > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c > @@ -2616,6 +2616,10 @@ intel_sdvo_select_i2c_bus(struct intel_sdvo *sdvo) > else > pin = GMBUS_PIN_DPB; > > + drm_dbg_kms(_priv->drm, "[ENCODER:%d:%s] I2C pin %d, slave addr > 0x%x\n", > + sdvo->base.base.base.id, sdvo->base.base.name, > + pin, sdvo->slave_addr); > + > sdvo->i2c = intel_gmbus_get_adapter(dev_priv, pin); > > /* -- Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 12/13] drm/i915/sdvo: Rework DDC bus handling
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Each SDVO device can have up to three sets of DDC pins.
> Currently we just register a single i2c_adapter for the
> entire SDVO device and semi-randomly pick the "correct"
> set of DDC pins during intel_sdvo_tmds_sink_detect().
> This doesn't make any real sense especially if we have
> multiple outputs each with their own dedicated DDC bus.
>
> Let's clean up this mess and register a dedicated
> i2c_adapter for each of the possible pin pairs. Each
> output (ie. connector) can then pick the correct i2c_adapter
> to use for its DDC bus. And we can just switch over to
> drm_connector_init_with_ddc() to take care of the
> connector->ddc association, which also populates the
> "ddc" sysfs symlink as a bonus.
>
> And now that things are based on the actual connector we can
> also nuke the sketchy sdvo->controller_output thing.
>
> Signed-off-by: Ville Syrjälä
Okay, so I'm not going to claim I considered all the possible aspects of
the changes here, but I did not spot anything wrong with it
either. Maybe there's a bit much shoved in one patch, but not sure if it
would be feasible or productive to chop it up. The direction looks good.
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 205 --
> 1 file changed, 114 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 5c25417d256a..d7edb5714c4c 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -65,6 +65,8 @@
> #define IS_TV_OR_LVDS(c) ((c)->output_flag & (SDVO_TV_MASK |
> SDVO_LVDS_MASK))
> #define IS_DIGITAL(c)((c)->output_flag & (SDVO_TMDS_MASK |
> SDVO_LVDS_MASK))
>
> +#define HAS_DDC(c) ((c)->output_flag & (SDVO_RGB_MASK |
> SDVO_TMDS_MASK | \
> + SDVO_LVDS_MASK))
>
> static const char * const tv_format_names[] = {
> "NTSC_M" , "NTSC_J" , "NTSC_443",
> @@ -78,20 +80,25 @@ static const char * const tv_format_names[] = {
>
> #define TV_FORMAT_NUM ARRAY_SIZE(tv_format_names)
>
> +struct intel_sdvo;
> +
> +struct intel_sdvo_ddc {
> + struct i2c_adapter ddc;
> + struct intel_sdvo *sdvo;
> + u8 ddc_bus;
> +};
> +
> struct intel_sdvo {
> struct intel_encoder base;
>
> struct i2c_adapter *i2c;
> u8 slave_addr;
>
> - struct i2c_adapter ddc;
> + struct intel_sdvo_ddc ddc[3];
>
> /* Register for the SDVO device: SDVOB or SDVOC */
> i915_reg_t sdvo_reg;
>
> - /* Active outputs controlled by this SDVO output */
> - u16 controlled_output;
> -
> /*
>* Capabilities of the SDVO device returned by
>* intel_sdvo_get_capabilities()
> @@ -108,9 +115,6 @@ struct intel_sdvo {
>*/
> u16 hotplug_active;
>
> - /* DDC bus used by this SDVO encoder */
> - u8 ddc_bus;
> -
> /*
>* the sdvo flag gets lost in round trip: dtd->adjusted_mode->dtd
>*/
> @@ -2035,18 +2039,15 @@ intel_sdvo_hotplug(struct intel_encoder *encoder,
> return intel_encoder_hotplug(encoder, connector);
> }
>
> -static bool
> -intel_sdvo_multifunc_encoder(struct intel_sdvo *intel_sdvo)
> -{
> - /* Is there more than one type of output? */
> - return hweight16(intel_sdvo->caps.output_flags) > 1;
> -}
> -
> static const struct drm_edid *
> intel_sdvo_get_edid(struct drm_connector *connector)
> {
> - struct intel_sdvo *sdvo =
> intel_attached_sdvo(to_intel_connector(connector));
> - return drm_edid_read_ddc(connector, >ddc);
> + struct i2c_adapter *ddc = connector->ddc;
> +
> + if (!ddc)
> + return NULL;
> +
> + return drm_edid_read_ddc(connector, ddc);
> }
>
> /* Mac mini hack -- use the same DDC as the analog connector */
> @@ -2054,43 +2055,23 @@ static const struct drm_edid *
> intel_sdvo_get_analog_edid(struct drm_connector *connector)
> {
> struct drm_i915_private *i915 = to_i915(connector->dev);
> - struct i2c_adapter *i2c;
> + struct i2c_adapter *ddc;
>
> - i2c = intel_gmbus_get_adapter(i915, i915->display.vbt.crt_ddc_pin);
> + ddc = intel_gmbus_get_adapter(i915, i915->display.vbt.crt_ddc_pin);
> + if (!ddc)
> + return NULL;
>
> - return drm_edid_read_ddc(connector, i2c);
> + return drm_edid_read_ddc(connector, ddc);
> }
>
> static enum drm_connector_status
> intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
> {
> - struct intel_sdvo *intel_sdvo =
> intel_attached_sdvo(to_intel_connector(connector));
> enum drm_connector_status status;
> const struct drm_edid *drm_edid;
>
> drm_edid = intel_sdvo_get_edid(connector);
>
> - if (!drm_edid && intel_sdvo_multifunc_encoder(intel_sdvo)) {
> - u8 ddc, saved_ddc = intel_sdvo->ddc_bus;
> -
> -
Re: [Intel-gfx] [PATCH] drm/i915: Fix the disabling sequence for Bigjoiner
On Wed, Jul 05, 2023 at 06:32:51PM +0300, Imre Deak wrote: > On Thu, May 25, 2023 at 01:10:36PM +0300, Stanislav Lisovskiy wrote: > > According to BSpec 49190, when enabling crtcs, we first setup > > slave and then master crtc, however for disabling it should go > > vice versa, i.e first master, then slave, however current code > > does disabling in a same way as enabling. Fix this, by skipping > > non-master crtcs, instead of non-slaves. > > > > Signed-off-by: Stanislav Lisovskiy > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 0490c6412ab5..68958ba0ef49 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -6662,7 +6662,7 @@ static void intel_commit_modeset_disables(struct > > intel_atomic_state *state) > > */ > > if (!is_trans_port_sync_slave(old_crtc_state) && > > !intel_dp_mst_is_slave_trans(old_crtc_state) && > > - !intel_crtc_is_bigjoiner_slave(old_crtc_state)) > > + !intel_crtc_is_bigjoiner_master(old_crtc_state)) > > I don't see what does this fix. The sequence is correct at the moment > and this change would break it, leaving the encoder PLL enabled > incorrectly when the encoder->post_pll_disable() hook is called. Hence > it's NAK from side. Citing BSpec: Follow the steps to disable Planes, Pipe, and Transcoder for pipe B and transcoder B(master) Follow the steps to Disable Planes and Pipe for pipe C(slave) Disable port associated with Transcoder B however because of !intel_crtc_is_bigjoiner_slave(old_crtc_state), we are skipping masters and do disabling for the slave crtc first. If fixing that going to break something - that is another story(anyway doesn't mean we shouldn't fix) Stan > > > continue; > > > > intel_old_crtc_state_disables(state, old_crtc_state, > > -- > > 2.37.3 > >
Re: [Intel-gfx] [PATCH 11/13] drm/i915/sdvo: Get rid of the per-connector i2c symlink
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> We should switch over to the standard "ddc" per-connector
> symlink instead of rolling our own thing. The i2c specific
> symlink is also in the way of reworking the SDVO DDC handling
> (which is a mess atm) so get rid of it.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 29 ++-
> 1 file changed, 2 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 383f8b1547a1..5c25417d256a 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2468,31 +2468,6 @@ intel_sdvo_connector_atomic_set_property(struct
> drm_connector *connector,
> return 0;
> }
>
> -static int
> -intel_sdvo_connector_register(struct drm_connector *connector)
> -{
> - struct intel_sdvo *sdvo =
> intel_attached_sdvo(to_intel_connector(connector));
> - int ret;
> -
> - ret = intel_connector_register(connector);
> - if (ret)
> - return ret;
> -
> - return sysfs_create_link(>kdev->kobj,
> - >ddc.dev.kobj,
> - sdvo->ddc.dev.kobj.name);
> -}
> -
> -static void
> -intel_sdvo_connector_unregister(struct drm_connector *connector)
> -{
> - struct intel_sdvo *sdvo =
> intel_attached_sdvo(to_intel_connector(connector));
> -
> - sysfs_remove_link(>kdev->kobj,
> - sdvo->ddc.dev.kobj.name);
> - intel_connector_unregister(connector);
> -}
> -
> static struct drm_connector_state *
> intel_sdvo_connector_duplicate_state(struct drm_connector *connector)
> {
> @@ -2511,8 +2486,8 @@ static const struct drm_connector_funcs
> intel_sdvo_connector_funcs = {
> .fill_modes = drm_helper_probe_single_connector_modes,
> .atomic_get_property = intel_sdvo_connector_atomic_get_property,
> .atomic_set_property = intel_sdvo_connector_atomic_set_property,
> - .late_register = intel_sdvo_connector_register,
> - .early_unregister = intel_sdvo_connector_unregister,
> + .late_register = intel_connector_register,
> + .early_unregister = intel_connector_unregister,
> .destroy = intel_connector_destroy,
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> .atomic_duplicate_state = intel_sdvo_connector_duplicate_state,
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 10/13] drm/i915/sdvo: Nuke the duplicate sdvo->port
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> We already have encoder->port so get rid of the duplicate
> sdvo->port.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 19 ---
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index e6c558416a6b..383f8b1547a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -108,8 +108,6 @@ struct intel_sdvo {
>*/
> u16 hotplug_active;
>
> - enum port port;
> -
> /* DDC bus used by this SDVO encoder */
> u8 ddc_bus;
>
> @@ -226,7 +224,7 @@ static void intel_sdvo_write_sdvox(struct intel_sdvo
> *intel_sdvo, u32 val)
> return;
> }
>
> - if (intel_sdvo->port == PORT_B)
> + if (intel_sdvo->base.port == PORT_B)
> cval = intel_de_read(dev_priv, GEN3_SDVOC);
> else
> bval = intel_de_read(dev_priv, GEN3_SDVOB);
> @@ -403,7 +401,7 @@ static const char *sdvo_cmd_name(u8 cmd)
> return NULL;
> }
>
> -#define SDVO_NAME(svdo) ((svdo)->port == PORT_B ? "SDVOB" : "SDVOC")
> +#define SDVO_NAME(svdo) ((svdo)->base.port == PORT_B ? "SDVOB" : "SDVOC")
>
> static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
> const void *args, int args_len)
> @@ -1604,7 +1602,7 @@ static void intel_sdvo_pre_enable(struct
> intel_atomic_state *state,
> sdvox |= SDVO_BORDER_ENABLE;
> } else {
> sdvox = intel_de_read(dev_priv, intel_sdvo->sdvo_reg);
> - if (intel_sdvo->port == PORT_B)
> + if (intel_sdvo->base.port == PORT_B)
> sdvox &= SDVOB_PRESERVE_MASK;
> else
> sdvox &= SDVOC_PRESERVE_MASK;
> @@ -2618,7 +2616,7 @@ intel_sdvo_select_ddc_bus(struct intel_sdvo *sdvo)
> struct drm_i915_private *dev_priv = to_i915(sdvo->base.base.dev);
> struct sdvo_device_mapping *mapping;
>
> - if (sdvo->port == PORT_B)
> + if (sdvo->base.port == PORT_B)
> mapping = _priv->display.vbt.sdvo_mappings[0];
> else
> mapping = _priv->display.vbt.sdvo_mappings[1];
> @@ -2636,7 +2634,7 @@ intel_sdvo_select_i2c_bus(struct intel_sdvo *sdvo)
> struct sdvo_device_mapping *mapping;
> u8 pin;
>
> - if (sdvo->port == PORT_B)
> + if (sdvo->base.port == PORT_B)
> mapping = _priv->display.vbt.sdvo_mappings[0];
> else
> mapping = _priv->display.vbt.sdvo_mappings[1];
> @@ -2676,7 +2674,7 @@ intel_sdvo_get_slave_addr(struct intel_sdvo *sdvo)
> struct drm_i915_private *dev_priv = to_i915(sdvo->base.base.dev);
> struct sdvo_device_mapping *my_mapping, *other_mapping;
>
> - if (sdvo->port == PORT_B) {
> + if (sdvo->base.port == PORT_B) {
> my_mapping = _priv->display.vbt.sdvo_mappings[0];
> other_mapping = _priv->display.vbt.sdvo_mappings[1];
> } else {
> @@ -2703,7 +2701,7 @@ intel_sdvo_get_slave_addr(struct intel_sdvo *sdvo)
>* No SDVO device info is found for another DVO port,
>* so use mapping assumption we had before BIOS parsing.
>*/
> - if (sdvo->port == PORT_B)
> + if (sdvo->base.port == PORT_B)
> return 0x70;
> else
> return 0x72;
> @@ -3367,7 +3365,6 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
>"SDVO %c", port_name(port));
>
> intel_sdvo->sdvo_reg = sdvo_reg;
> - intel_sdvo->port = port;
> intel_sdvo->slave_addr = intel_sdvo_get_slave_addr(intel_sdvo) >> 1;
> intel_sdvo_select_i2c_bus(intel_sdvo);
> if (!intel_sdvo_init_ddc_proxy(intel_sdvo))
> @@ -3417,7 +3414,7 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
>* hotplug lines.
>*/
> if (intel_sdvo->hotplug_active) {
> - if (intel_sdvo->port == PORT_B)
> + if (intel_sdvo->base.port == PORT_B)
> intel_encoder->hpd_pin = HPD_SDVO_B;
> else
> intel_encoder->hpd_pin = HPD_SDVO_C;
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 09/13] drm/i915/sdvo: Initialize the encoder ealier
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Call drm_encoder_init() earlier so that we don't have to keep passing
> the i915/dev_priv around separately.
*earlier in the subject.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 35 +++
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 29762716a067..e6c558416a6b 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2613,9 +2613,9 @@ intel_sdvo_guess_ddc_bus(struct intel_sdvo *sdvo)
> * outputs, then LVDS outputs.
> */
> static void
> -intel_sdvo_select_ddc_bus(struct drm_i915_private *dev_priv,
> - struct intel_sdvo *sdvo)
> +intel_sdvo_select_ddc_bus(struct intel_sdvo *sdvo)
> {
> + struct drm_i915_private *dev_priv = to_i915(sdvo->base.base.dev);
> struct sdvo_device_mapping *mapping;
>
> if (sdvo->port == PORT_B)
> @@ -2630,9 +2630,9 @@ intel_sdvo_select_ddc_bus(struct drm_i915_private
> *dev_priv,
> }
>
> static void
> -intel_sdvo_select_i2c_bus(struct drm_i915_private *dev_priv,
> - struct intel_sdvo *sdvo)
> +intel_sdvo_select_i2c_bus(struct intel_sdvo *sdvo)
> {
> + struct drm_i915_private *dev_priv = to_i915(sdvo->base.base.dev);
> struct sdvo_device_mapping *mapping;
> u8 pin;
>
> @@ -2671,9 +2671,9 @@ intel_sdvo_is_hdmi_connector(struct intel_sdvo
> *intel_sdvo)
> }
>
> static u8
> -intel_sdvo_get_slave_addr(struct drm_i915_private *dev_priv,
> - struct intel_sdvo *sdvo)
> +intel_sdvo_get_slave_addr(struct intel_sdvo *sdvo)
> {
> + struct drm_i915_private *dev_priv = to_i915(sdvo->base.base.dev);
> struct sdvo_device_mapping *my_mapping, *other_mapping;
>
> if (sdvo->port == PORT_B) {
> @@ -2994,7 +2994,6 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo)
> SDVO_OUTPUT_LVDS0,
> SDVO_OUTPUT_LVDS1,
> };
> - struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
> u16 flags;
> int i;
>
> @@ -3008,7 +3007,7 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo)
>
> intel_sdvo->controlled_output = flags;
>
> - intel_sdvo_select_ddc_bus(i915, intel_sdvo);
> + intel_sdvo_select_ddc_bus(intel_sdvo);
>
> for (i = 0; i < ARRAY_SIZE(probe_order); i++) {
> u16 type = flags & probe_order[i];
> @@ -3309,9 +3308,9 @@ static const struct i2c_lock_operations proxy_lock_ops
> = {
> };
>
> static bool
> -intel_sdvo_init_ddc_proxy(struct intel_sdvo *sdvo,
> - struct drm_i915_private *dev_priv)
> +intel_sdvo_init_ddc_proxy(struct intel_sdvo *sdvo)
> {
> + struct drm_i915_private *dev_priv = to_i915(sdvo->base.base.dev);
> struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
>
> sdvo->ddc.owner = THIS_MODULE;
> @@ -3357,23 +3356,23 @@ bool intel_sdvo_init(struct drm_i915_private
> *dev_priv,
> if (!intel_sdvo)
> return false;
>
> - intel_sdvo->sdvo_reg = sdvo_reg;
> - intel_sdvo->port = port;
> - intel_sdvo->slave_addr =
> - intel_sdvo_get_slave_addr(dev_priv, intel_sdvo) >> 1;
> - intel_sdvo_select_i2c_bus(dev_priv, intel_sdvo);
> - if (!intel_sdvo_init_ddc_proxy(intel_sdvo, dev_priv))
> - goto err_i2c_bus;
> -
> /* encoder type will be decided later */
> intel_encoder = _sdvo->base;
> intel_encoder->type = INTEL_OUTPUT_SDVO;
> intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
> intel_encoder->port = port;
> +
> drm_encoder_init(_priv->drm, _encoder->base,
>_sdvo_enc_funcs, 0,
>"SDVO %c", port_name(port));
>
> + intel_sdvo->sdvo_reg = sdvo_reg;
> + intel_sdvo->port = port;
> + intel_sdvo->slave_addr = intel_sdvo_get_slave_addr(intel_sdvo) >> 1;
> + intel_sdvo_select_i2c_bus(intel_sdvo);
> + if (!intel_sdvo_init_ddc_proxy(intel_sdvo))
> + goto err_i2c_bus;
drm_encoder_cleanup() in the error path?
Other than that,
Reviewed-by: Jani Nikula
> +
> /* Read the regs to test if we can talk to the device */
> for (i = 0; i < 0x40; i++) {
> u8 byte;
--
Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PULL] drm-intel-next-fixes
Hi Dave, Daniel, A weekly collection of fixes for the drm-next/6.5 merge window. Mostly small display fixups, one for GuC/SLPC and one header file tidy. I see last week did not get pulled so this week contains those ones plus two more fixups - one code tidy actually and one fixup. Regards, Tvrtko drm-intel-next-fixes-2023-06-29: - Allow DC states along with PW2 only for PWB functionality [adlp+] (Imre Deak) - Fix SSC selection for MPLLA [mtl] (Radhakrishna Sripada) - Use hw.adjusted mode when calculating io/fast wake times [psr] (Jouni Högander) - Apply min softlimit correctly [guc/slpc] (Vinay Belgaumkar) - Assign correct hdcp content type [hdcp] (Suraj Kandpal) - Add missing forward declarations/includes to display power headers (Imre Deak) drm-intel-next-fixes-2023-07-06: - Fix BDW PSR AUX CH data register offsets [psr] (Ville Syrjälä) - Use mock device info for creating mock device (Jani Nikula) The following changes since commit 274d4b96b12f78cef4f72a97a4967032233f6cae: drm/i915: Fix a NULL vs IS_ERR() bug (2023-06-20 08:54:47 +0100) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-fixes-2023-07-06 for you to fetch changes up to f6cf3883df471abbcf1553127681dc244c8ff8dd: drm/i915: use mock device info for creating mock device (2023-07-04 10:40:29 +0100) drm-intel-next-fixes-2023-06-29: - Allow DC states along with PW2 only for PWB functionality [adlp+] (Imre Deak) - Fix SSC selection for MPLLA [mtl] (Radhakrishna Sripada) - Use hw.adjusted mode when calculating io/fast wake times [psr] (Jouni Högander) - Apply min softlimit correctly [guc/slpc] (Vinay Belgaumkar) - Assign correct hdcp content type [hdcp] (Suraj Kandpal) - Add missing forward declarations/includes to display power headers (Imre Deak) drm-intel-next-fixes-2023-07-06: - Fix BDW PSR AUX CH data register offsets [psr] (Ville Syrjälä) - Use mock device info for creating mock device (Jani Nikula) Imre Deak (2): drm/i915/adlp+: Allow DC states along with PW2 only for PWB functionality drm/i915: Add missing forward declarations/includes to display power headers Jani Nikula (1): drm/i915: use mock device info for creating mock device Jouni Högander (1): drm/i915/psr: Use hw.adjusted mode when calculating io/fast wake times Radhakrishna Sripada (1): drm/i915/mtl: Fix SSC selection for MPLLA Suraj Kandpal (1): drm/i915/hdcp: Assign correct hdcp content type Ville Syrjälä (1): drm/i915/psr: Fix BDW PSR AUX CH data register offsets Vinay Belgaumkar (1): drm/i915/guc/slpc: Apply min softlimit correctly drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 +- drivers/gpu/drm/i915/display/intel_display_power.h | 4 ++ .../gpu/drm/i915/display/intel_display_power_map.c | 16 .../drm/i915/display/intel_display_power_well.h| 2 + drivers/gpu/drm/i915/display/intel_hdcp.c | 2 +- drivers/gpu/drm/i915/display/intel_psr.c | 4 +- drivers/gpu/drm/i915/display/intel_psr_regs.h | 2 +- drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c| 2 +- drivers/gpu/drm/i915/selftests/mock_gem_device.c | 45 -- 9 files changed, 45 insertions(+), 35 deletions(-)
Re: [Intel-gfx] [PATCH] drm/i915: Fix the disabling sequence for Bigjoiner
On Wed, Jul 05, 2023 at 06:32:51PM +0300, Imre Deak wrote: > On Thu, May 25, 2023 at 01:10:36PM +0300, Stanislav Lisovskiy wrote: > > According to BSpec 49190, when enabling crtcs, we first setup > > slave and then master crtc, however for disabling it should go > > vice versa, i.e first master, then slave, however current code > > does disabling in a same way as enabling. Fix this, by skipping > > non-master crtcs, instead of non-slaves. > > > > Signed-off-by: Stanislav Lisovskiy > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 0490c6412ab5..68958ba0ef49 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -6662,7 +6662,7 @@ static void intel_commit_modeset_disables(struct > > intel_atomic_state *state) > > */ > > if (!is_trans_port_sync_slave(old_crtc_state) && > > !intel_dp_mst_is_slave_trans(old_crtc_state) && > > - !intel_crtc_is_bigjoiner_slave(old_crtc_state)) > > + !intel_crtc_is_bigjoiner_master(old_crtc_state)) > > I don't see what does this fix. The sequence is correct at the moment > and this change would break it, leaving the encoder PLL enabled > incorrectly when the encoder->post_pll_disable() hook is called. Hence > it's NAK from side. Well, as I pointed out the BSpec 49190 instructs us to disable master first, then slave. Current code skips all non-slaves in first cycle, i.e it disables first slaves and then masters. Which is _wrong_. Anything else in particular, where do you need clarifications? Stan > > > continue; > > > > intel_old_crtc_state_disables(state, old_crtc_state, > > -- > > 2.37.3 > >
Re: [Intel-gfx] [PATCH 08/13] drm/i915/sdvo: Nuke attached_output tracking
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Instead of operating on the output the user specified (via the
> connector) the current code tends to operate on whichever outputs
> it has detected as attached. That is not how the kms uapi is supposed
> to work. So simply get rid of attached_outputs and instead directly
> operate on the output the user has specified.
>
> Signed-off-by: Ville Syrjälä
Acked-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 31 ---
> 1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index fcf3a95393d9..29762716a067 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -103,12 +103,6 @@ struct intel_sdvo {
> /* Pixel clock limitations reported by the SDVO device, in kHz */
> int pixel_clock_min, pixel_clock_max;
>
> - /*
> - * For multiple function SDVO device,
> - * this is for current attached outputs.
> - */
> - u16 attached_output;
> -
> /*
>* Hotplug activation bits for this device
>*/
> @@ -1223,12 +1217,13 @@ static bool intel_sdvo_set_tv_format(struct
> intel_sdvo *intel_sdvo,
>
> static bool
> intel_sdvo_set_output_timings_from_mode(struct intel_sdvo *intel_sdvo,
> + struct intel_sdvo_connector
> *intel_sdvo_connector,
> const struct drm_display_mode *mode)
> {
> struct intel_sdvo_dtd output_dtd;
>
> if (!intel_sdvo_set_target_output(intel_sdvo,
> - intel_sdvo->attached_output))
> + intel_sdvo_connector->output_flag))
> return false;
>
> intel_sdvo_get_dtd_from_mode(_dtd, mode);
> @@ -1369,7 +1364,9 @@ static int intel_sdvo_compute_config(struct
> intel_encoder *encoder,
>* the sequence to do it. Oh well.
>*/
> if (IS_TV(intel_sdvo_connector)) {
> - if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo, mode))
> + if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo,
> +
> intel_sdvo_connector,
> + mode))
> return -EINVAL;
>
> (void) intel_sdvo_get_preferred_input_mode(intel_sdvo,
> @@ -1387,7 +1384,9 @@ static int intel_sdvo_compute_config(struct
> intel_encoder *encoder,
> if (ret)
> return ret;
>
> - if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo,
> fixed_mode))
> + if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo,
> +
> intel_sdvo_connector,
> + fixed_mode))
> return -EINVAL;
>
> (void) intel_sdvo_get_preferred_input_mode(intel_sdvo,
> @@ -1528,7 +1527,7 @@ static void intel_sdvo_pre_enable(struct
> intel_atomic_state *state,
>* channel on the motherboard. In a two-input device, the first input
>* will be SDVOB and the second SDVOC.
>*/
> - in_out.in0 = intel_sdvo->attached_output;
> + in_out.in0 = intel_sdvo_connector->output_flag;
> in_out.in1 = 0;
>
> intel_sdvo_set_value(intel_sdvo,
> @@ -1537,7 +1536,7 @@ static void intel_sdvo_pre_enable(struct
> intel_atomic_state *state,
>
> /* Set the output timings to the screen */
> if (!intel_sdvo_set_target_output(intel_sdvo,
> - intel_sdvo->attached_output))
> + intel_sdvo_connector->output_flag))
> return;
>
> /* lvds has a special fixed output timing. */
> @@ -1874,6 +1873,8 @@ static void intel_enable_sdvo(struct intel_atomic_state
> *state,
> struct drm_device *dev = encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
> + struct intel_sdvo_connector *intel_sdvo_connector =
> + to_intel_sdvo_connector(conn_state->connector);
> struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
> u32 temp;
> bool input1, input2;
> @@ -1903,7 +1904,7 @@ static void intel_enable_sdvo(struct intel_atomic_state
> *state,
> if (0)
> intel_sdvo_set_encoder_power_state(intel_sdvo,
> DRM_MODE_DPMS_ON);
> - intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo->attached_output);
> + intel_sdvo_set_active_outputs(intel_sdvo,
> intel_sdvo_connector->output_flag);
>
> if (pipe_config->has_audio)
> intel_sdvo_enable_audio(intel_sdvo, pipe_config,
Re: [Intel-gfx] [PATCH 07/13] drm/i915/sdvo: Fail gracefully if the TV dotclock is out of range
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Instead of warning and continuing with bogus state when the
> requested dotclock isn't acceptable just print some debug
> spew and fail gracefully.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 75a8e5583358..fcf3a95393d9 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -1269,7 +1269,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo
> *intel_sdvo,
> return true;
> }
>
> -static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
> +static int i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
> {
> struct drm_i915_private *dev_priv =
> to_i915(pipe_config->uapi.crtc->dev);
> unsigned int dotclock = pipe_config->hw.adjusted_mode.crtc_clock;
> @@ -1292,11 +1292,14 @@ static void i9xx_adjust_sdvo_tv_clock(struct
> intel_crtc_state *pipe_config)
> clock->m1 = 12;
> clock->m2 = 8;
> } else {
> - drm_WARN(_priv->drm, 1,
> - "SDVO TV clock out of range: %i\n", dotclock);
> + drm_dbg_kms(_priv->drm,
> + "SDVO TV clock out of range: %i\n", dotclock);
> + return -EINVAL;
> }
>
> pipe_config->clock_set = true;
> +
> + return 0;
> }
>
> static bool intel_has_hdmi_sink(struct intel_sdvo_connector
> *intel_sdvo_connector,
> @@ -1414,8 +1417,13 @@ static int intel_sdvo_compute_config(struct
> intel_encoder *encoder,
> conn_state);
>
> /* Clock computation needs to happen after pixel multiplier. */
> - if (IS_TV(intel_sdvo_connector))
> - i9xx_adjust_sdvo_tv_clock(pipe_config);
> + if (IS_TV(intel_sdvo_connector)) {
> + int ret;
> +
> + ret = i9xx_adjust_sdvo_tv_clock(pipe_config);
> + if (ret)
> + return ret;
> + }
>
> if (conn_state->picture_aspect_ratio)
> adjusted_mode->picture_aspect_ratio =
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 06/13] drm/i915/sdvo: Pick the TV dotclock from adjusted_mode
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> port_clock is what the encoder/dpll code is supposed to calculate,
> it is not the input clock. Use the dotclock as the target we're
> trying to achieve instead.
>
> TODO: the SDVO TV clocking is a mess atm and needs further work
>
> Signed-off-by: Ville Syrjälä
Acked-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 76eed11e9fed..75a8e5583358 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -1272,7 +1272,7 @@ intel_sdvo_get_preferred_input_mode(struct intel_sdvo
> *intel_sdvo,
> static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc_state *pipe_config)
> {
> struct drm_i915_private *dev_priv =
> to_i915(pipe_config->uapi.crtc->dev);
> - unsigned dotclock = pipe_config->port_clock;
> + unsigned int dotclock = pipe_config->hw.adjusted_mode.crtc_clock;
> struct dpll *clock = _config->dpll;
>
> /*
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 04/13] drm/i915: Don't warn about zero N/P in *_calc_dpll_params()
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Allow *_calc_dpll_params() to be called even if the N/P dividers
> are zero without warning. We'll want to call these to make sure the
> derived values are fully computed, but not all users (VLV DSI in
> particular) don't even enable the DPLL and thus the dividers will
> be left at zero.
>
> It could also be possible that the BIOS has misprogrammed the DPLL
> (IIRC happened with some SNB machines with 4k+ displays) and thus
> we'll currently generate a lot of dmesg spew. Better be silent and
> just let the normal state checker/etc. deal with any driver bugs.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_dpll.c | 37 ---
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c
> b/drivers/gpu/drm/i915/display/intel_dpll.c
> index 999badfe2906..71bfeea4cef2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll.c
> @@ -314,10 +314,11 @@ int pnv_calc_dpll_params(int refclk, struct dpll *clock)
> {
> clock->m = clock->m2 + 2;
> clock->p = clock->p1 * clock->p2;
> - if (WARN_ON(clock->n == 0 || clock->p == 0))
> - return 0;
> - clock->vco = DIV_ROUND_CLOSEST(refclk * clock->m, clock->n);
> - clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
> +
> + clock->vco = clock->n == 0 ? 0 :
> + DIV_ROUND_CLOSEST(refclk * clock->m, clock->n);
> + clock->dot = clock->p == 0 ? 0 :
> + DIV_ROUND_CLOSEST(clock->vco, clock->p);
>
> return clock->dot;
> }
> @@ -331,10 +332,11 @@ int i9xx_calc_dpll_params(int refclk, struct dpll
> *clock)
> {
> clock->m = i9xx_dpll_compute_m(clock);
> clock->p = clock->p1 * clock->p2;
> - if (WARN_ON(clock->n + 2 == 0 || clock->p == 0))
> - return 0;
> - clock->vco = DIV_ROUND_CLOSEST(refclk * clock->m, clock->n + 2);
> - clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
> +
> + clock->vco = clock->n + 2 == 0 ? 0 :
> + DIV_ROUND_CLOSEST(refclk * clock->m, clock->n + 2);
> + clock->dot = clock->p == 0 ? 0 :
> + DIV_ROUND_CLOSEST(clock->vco, clock->p);
>
> return clock->dot;
> }
> @@ -343,10 +345,11 @@ int vlv_calc_dpll_params(int refclk, struct dpll *clock)
> {
> clock->m = clock->m1 * clock->m2;
> clock->p = clock->p1 * clock->p2 * 5;
> - if (WARN_ON(clock->n == 0 || clock->p == 0))
> - return 0;
> - clock->vco = DIV_ROUND_CLOSEST(refclk * clock->m, clock->n);
> - clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
> +
> + clock->vco = clock->n == 0 ? 0 :
> + DIV_ROUND_CLOSEST(refclk * clock->m, clock->n);
> + clock->dot = clock->p == 0 ? 0 :
> + DIV_ROUND_CLOSEST(clock->vco, clock->p);
>
> return clock->dot;
> }
> @@ -355,11 +358,11 @@ int chv_calc_dpll_params(int refclk, struct dpll *clock)
> {
> clock->m = clock->m1 * clock->m2;
> clock->p = clock->p1 * clock->p2 * 5;
> - if (WARN_ON(clock->n == 0 || clock->p == 0))
> - return 0;
> - clock->vco = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(refclk, clock->m),
> -clock->n << 22);
> - clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
> +
> + clock->vco = clock->n == 0 ? 0 :
> + DIV_ROUND_CLOSEST_ULL(mul_u32_u32(refclk, clock->m), clock->n
> << 22);
> + clock->dot = clock->p == 0 ? 0 :
> + DIV_ROUND_CLOSEST(clock->vco, clock->p);
>
> return clock->dot;
> }
--
Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Remove some dead "code" (rev2)
== Series Details == Series: drm/i915: Remove some dead "code" (rev2) URL : https://patchwork.freedesktop.org/series/120222/ State : success == Summary == CI Bug Log - changes from CI_DRM_13347 -> Patchwork_120222v2 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/index.html Participating hosts (41 -> 39) -- Missing(2): bat-atsm-1 fi-snb-2520m Known issues Here are the changes found in Patchwork_120222v2 that come from known issues: ### IGT changes ### Issues hit * igt@i915_selftest@live@gt_mocs: - bat-mtlp-6: [PASS][1] -> [DMESG-FAIL][2] ([i915#7059]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-mtlp-6/igt@i915_selftest@live@gt_mocs.html - bat-rpls-2: [PASS][3] -> [INCOMPLETE][4] ([i915#4983]) [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/bat-rpls-2/igt@i915_selftest@live@gt_mocs.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-rpls-2/igt@i915_selftest@live@gt_mocs.html * igt@i915_selftest@live@requests: - bat-mtlp-8: [PASS][5] -> [DMESG-FAIL][6] ([i915#7269]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/bat-mtlp-8/igt@i915_selftest@l...@requests.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-mtlp-8/igt@i915_selftest@l...@requests.html - bat-rpls-1: [PASS][7] -> [ABORT][8] ([i915#7920]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/bat-rpls-1/igt@i915_selftest@l...@requests.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-rpls-1/igt@i915_selftest@l...@requests.html - bat-mtlp-6: [PASS][9] -> [ABORT][10] ([i915#7982]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/bat-mtlp-6/igt@i915_selftest@l...@requests.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-mtlp-6/igt@i915_selftest@l...@requests.html - bat-dg2-11: [PASS][11] -> [ABORT][12] ([i915#7913]) [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/bat-dg2-11/igt@i915_selftest@l...@requests.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-dg2-11/igt@i915_selftest@l...@requests.html * igt@kms_busy@basic@modeset: - bat-adlp-11:[PASS][13] -> [ABORT][14] ([i915#4423]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/bat-adlp-11/igt@kms_busy@ba...@modeset.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-adlp-11/igt@kms_busy@ba...@modeset.html * igt@kms_chamelium_hpd@common-hpd-after-suspend: - bat-adlp-9: NOTRUN -> [SKIP][15] ([i915#7828]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-adlp-9/igt@kms_chamelium_...@common-hpd-after-suspend.html Possible fixes * igt@i915_selftest@live@gt_lrc: - bat-adlp-9: [INCOMPLETE][16] ([i915#4983] / [i915#7913]) -> [PASS][17] [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/bat-adlp-9/igt@i915_selftest@live@gt_lrc.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-adlp-9/igt@i915_selftest@live@gt_lrc.html Warnings * igt@kms_psr@cursor_plane_move: - bat-rplp-1: [SKIP][18] ([i915#1072]) -> [ABORT][19] ([i915#8434] / [i915#8668]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13347/bat-rplp-1/igt@kms_psr@cursor_plane_move.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_120222v2/bat-rplp-1/igt@kms_psr@cursor_plane_move.html [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072 [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423 [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983 [i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059 [i915#7269]: https://gitlab.freedesktop.org/drm/intel/issues/7269 [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828 [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913 [i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920 [i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982 [i915#8434]: https://gitlab.freedesktop.org/drm/intel/issues/8434 [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668 Build changes - * Linux: CI_DRM_13347 -> Patchwork_120222v2 CI-20190529: 20190529 CI_DRM_13347: ee9b323b764f1f14ae4e6e8213164bd250160770 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7372: c25f0c2ed0d9b44dcd48f4ba93bdc1fc0dfec625 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_120222v2: ee9b323b764f1f14ae4e6e8213164bd250160770 @
Re: [Intel-gfx] [PATCH 03/13] drm/i915/sdvo: s/sdvo_inputs_mask/sdvo_num_inputs/
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> The SDVO inputs are reportes a simple number, not a bitmask.
*reported as a
> Adjust the code to match reality.
>
> Note that we don't actually support dual input SDVO devices,
> and we just always use the first input.
>
> Signed-off-by: Ville Syrjälä
Didn't actually check this against any spec, but looks reasonable.
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 9 -
> drivers/gpu/drm/i915/display/intel_sdvo_regs.h | 2 +-
> 2 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index ec94da6cb7bb..76eed11e9fed 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -1955,7 +1955,7 @@ static bool intel_sdvo_get_capabilities(struct
> intel_sdvo *intel_sdvo, struct in
> " device_rev_id: %d\n"
> " sdvo_version_major: %d\n"
> " sdvo_version_minor: %d\n"
> - " sdvo_inputs_mask: %d\n"
> + " sdvo_num_inputs: %d\n"
> " smooth_scaling: %d\n"
> " sharp_scaling: %d\n"
> " up_scaling: %d\n"
> @@ -1967,7 +1967,7 @@ static bool intel_sdvo_get_capabilities(struct
> intel_sdvo *intel_sdvo, struct in
> caps->device_rev_id,
> caps->sdvo_version_major,
> caps->sdvo_version_minor,
> - caps->sdvo_inputs_mask,
> + caps->sdvo_num_inputs,
> caps->smooth_scaling,
> caps->sharp_scaling,
> caps->up_scaling,
> @@ -3436,15 +3436,14 @@ bool intel_sdvo_init(struct drm_i915_private
> *dev_priv,
>
> drm_dbg_kms(_priv->drm, "%s device VID/DID: %02X:%02X.%02X, "
> "clock range %dMHz - %dMHz, "
> - "input 1: %c, input 2: %c, "
> + "num inputs: %d, "
> "output 1: %c, output 2: %c\n",
> SDVO_NAME(intel_sdvo),
> intel_sdvo->caps.vendor_id, intel_sdvo->caps.device_id,
> intel_sdvo->caps.device_rev_id,
> intel_sdvo->pixel_clock_min / 1000,
> intel_sdvo->pixel_clock_max / 1000,
> - (intel_sdvo->caps.sdvo_inputs_mask & 0x1) ? 'Y' : 'N',
> - (intel_sdvo->caps.sdvo_inputs_mask & 0x2) ? 'Y' : 'N',
> + intel_sdvo->caps.sdvo_num_inputs,
> /* check currently supported outputs */
> intel_sdvo->caps.output_flags &
> (SDVO_OUTPUT_TMDS0 | SDVO_OUTPUT_RGB0 |
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo_regs.h
> b/drivers/gpu/drm/i915/display/intel_sdvo_regs.h
> index 74dc6c042b6e..54f099abefeb 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo_regs.h
> @@ -57,7 +57,7 @@ struct intel_sdvo_caps {
> u8 device_rev_id;
> u8 sdvo_version_major;
> u8 sdvo_version_minor;
> - unsigned int sdvo_inputs_mask:2;
> + unsigned int sdvo_num_inputs:2;
> unsigned int smooth_scaling:1;
> unsigned int sharp_scaling:1;
> unsigned int up_scaling:1;
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 02/13] drm/i915/sdvo: Protect macro args
On Wed, 05 Jul 2023, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Put parens around macro argument evaluation for safety.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_sdvo.c | 13 ++---
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 9ac4c0b6055b..ec94da6cb7bb 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -57,14 +57,13 @@
> #define SDVO_LVDS_MASK (SDVO_OUTPUT_LVDS0 | SDVO_OUTPUT_LVDS1)
> #define SDVO_TV_MASK (SDVO_OUTPUT_CVBS0 | SDVO_OUTPUT_SVID0 |
> SDVO_OUTPUT_YPRPB0)
>
> -#define SDVO_OUTPUT_MASK (SDVO_TMDS_MASK | SDVO_RGB_MASK | SDVO_LVDS_MASK |\
> - SDVO_TV_MASK)
> +#define SDVO_OUTPUT_MASK (SDVO_TMDS_MASK | SDVO_RGB_MASK | SDVO_LVDS_MASK |
> SDVO_TV_MASK)
>
> -#define IS_TV(c) (c->output_flag & SDVO_TV_MASK)
> -#define IS_TMDS(c) (c->output_flag & SDVO_TMDS_MASK)
> -#define IS_LVDS(c) (c->output_flag & SDVO_LVDS_MASK)
> -#define IS_TV_OR_LVDS(c) (c->output_flag & (SDVO_TV_MASK | SDVO_LVDS_MASK))
> -#define IS_DIGITAL(c) (c->output_flag & (SDVO_TMDS_MASK | SDVO_LVDS_MASK))
> +#define IS_TV(c) ((c)->output_flag & SDVO_TV_MASK)
> +#define IS_TMDS(c) ((c)->output_flag & SDVO_TMDS_MASK)
> +#define IS_LVDS(c) ((c)->output_flag & SDVO_LVDS_MASK)
> +#define IS_TV_OR_LVDS(c) ((c)->output_flag & (SDVO_TV_MASK |
> SDVO_LVDS_MASK))
> +#define IS_DIGITAL(c)((c)->output_flag & (SDVO_TMDS_MASK |
> SDVO_LVDS_MASK))
>
>
> static const char * const tv_format_names[] = {
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH 01/13] drm/i915/sdvo: Issue SetTargetOutput prior ot GetAttachedDisplays
On Wed, 05 Jul 2023, Ville Syrjala wrote: > From: Ville Syrjälä > > I have at least one SDVO device (some Lenovo DVI-I ADD2 card, > based on Conexant CX25904) where GetAttachedDisplays returns > success but fails to report any attached displays unless wet *we Also s/ot/to/ in subject. > precede the command with a SetTargetOutput. Make it so. > > I wasn't able to spot anything in the SDVO spec stating that > this should be necessary, but real world wins over spec. > > Signed-off-by: Ville Syrjälä Acked-by: Jani Nikula > --- > drivers/gpu/drm/i915/display/intel_sdvo.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c > b/drivers/gpu/drm/i915/display/intel_sdvo.c > index 8298a86d1334..9ac4c0b6055b 100644 > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c > @@ -2135,6 +2135,10 @@ intel_sdvo_detect(struct drm_connector *connector, > bool force) > if (!INTEL_DISPLAY_ENABLED(i915)) > return connector_status_disconnected; > > + if (!intel_sdvo_set_target_output(intel_sdvo, > + intel_sdvo_connector->output_flag)) > + return connector_status_unknown; > + > if (!intel_sdvo_get_value(intel_sdvo, > SDVO_CMD_GET_ATTACHED_DISPLAYS, > , 2)) -- Jani Nikula, Intel Open Source Graphics Center
