* [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms
@ 2025-11-19 15:33 Jani Nikula
2025-11-19 16:08 ` Lucas De Marchi
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jani Nikula @ 2025-11-19 15:33 UTC (permalink / raw)
To: intel-gfx, intel-xe; +Cc: jani.nikula, ville.syrjala
When big joiner is enabled, it reserves the adjacent pipe as the
secondary pipe. This happens without the user space knowing, and
subsequent attempts at using the CRTC with that pipe will fail. If the
user space does not have a coping mechanism, i.e. trying another CRTC,
this leads to a black screen.
Try to reduce the impact of the problem on discrete platforms by mapping
the CRTCs to pipes in order A, C, B, and D. If the user space reserves
CRTCs in order, this should trick it to using pipes that are more likely
to be available for and after joining.
Limit this to discrete platforms, which have four pipes, and no eDP, a
combination that should benefit the most with least drawbacks.
Although there are currently no platforms with more than four pipes, add
a fallback for initializing the rest of the pipes to not miss them.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
v2: Also remove WARN_ON()
v3: Limit to discrete
There are a number of issues in IGT with assuming CRTC index == pipe, at
least with CRC and vblank waits. With them being used a lot in tests, we
won't get enough test coverage until they're fixed.
---
drivers/gpu/drm/i915/display/intel_crtc.c | 2 --
.../drm/i915/display/intel_display_driver.c | 26 ++++++++++++++++++-
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 9d2a23c96c61..11e58d07ddef 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -394,8 +394,6 @@ int intel_crtc_init(struct intel_display *display, enum pipe pipe)
cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE);
- drm_WARN_ON(display->drm, drm_crtc_index(&crtc->base) != crtc->pipe);
-
if (HAS_CASF(display))
drm_crtc_create_sharpness_strength_property(&crtc->base);
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 7e000ba3e08b..b5c9cdf14a43 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -452,6 +452,7 @@ bool intel_display_driver_check_access(struct intel_display *display)
/* part #2: call after irq install, but before gem init */
int intel_display_driver_probe_nogem(struct intel_display *display)
{
+ u8 pipe_mask = U8_MAX;
enum pipe pipe;
int ret;
@@ -470,7 +471,30 @@ int intel_display_driver_probe_nogem(struct intel_display *display)
INTEL_NUM_PIPES(display),
INTEL_NUM_PIPES(display) > 1 ? "s" : "");
- for_each_pipe(display, pipe) {
+ /*
+ * Expose the pipes in order A, C, B, D on discrete platforms to trick
+ * user space into using pipes that are more likely to be available for
+ * both a) user space if pipe B has been reserved for the joiner, and b)
+ * the joiner if pipe A doesn't need the joiner.
+ *
+ * Fall back to normal initialization for the remaining pipes, if any.
+ */
+ if (HAS_BIGJOINER(display) && display->platform.dgfx) {
+ enum pipe pipe_order[] = { PIPE_A, PIPE_C, PIPE_B, PIPE_D };
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pipe_order); i++) {
+ pipe = pipe_order[i];
+
+ ret = intel_crtc_init(display, pipe);
+ if (ret)
+ goto err_mode_config;
+
+ pipe_mask &= ~BIT(pipe);
+ }
+ }
+
+ for_each_pipe_masked(display, pipe, pipe_mask) {
ret = intel_crtc_init(display, pipe);
if (ret)
goto err_mode_config;
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms 2025-11-19 15:33 [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms Jani Nikula @ 2025-11-19 16:08 ` Lucas De Marchi 2025-11-19 16:21 ` Jani Nikula 2025-11-19 17:09 ` ✗ i915.CI.BAT: failure for " Patchwork 2025-11-19 17:24 ` [PATCH v3] " Ville Syrjälä 2 siblings, 1 reply; 6+ messages in thread From: Lucas De Marchi @ 2025-11-19 16:08 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx, intel-xe, ville.syrjala On Wed, Nov 19, 2025 at 05:33:21PM +0200, Jani Nikula wrote: >When big joiner is enabled, it reserves the adjacent pipe as the >secondary pipe. This happens without the user space knowing, and >subsequent attempts at using the CRTC with that pipe will fail. If the >user space does not have a coping mechanism, i.e. trying another CRTC, >this leads to a black screen. > >Try to reduce the impact of the problem on discrete platforms by mapping >the CRTCs to pipes in order A, C, B, and D. If the user space reserves >CRTCs in order, this should trick it to using pipes that are more likely >to be available for and after joining. > >Limit this to discrete platforms, which have four pipes, and no eDP, a >combination that should benefit the most with least drawbacks. > >Although there are currently no platforms with more than four pipes, add >a fallback for initializing the rest of the pipes to not miss them. > >Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >--- > >v2: Also remove WARN_ON() > >v3: Limit to discrete > >There are a number of issues in IGT with assuming CRTC index == pipe, at >least with CRC and vblank waits. With them being used a lot in tests, we >won't get enough test coverage until they're fixed. >--- > drivers/gpu/drm/i915/display/intel_crtc.c | 2 -- > .../drm/i915/display/intel_display_driver.c | 26 ++++++++++++++++++- > 2 files changed, 25 insertions(+), 3 deletions(-) > >diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c >index 9d2a23c96c61..11e58d07ddef 100644 >--- a/drivers/gpu/drm/i915/display/intel_crtc.c >+++ b/drivers/gpu/drm/i915/display/intel_crtc.c >@@ -394,8 +394,6 @@ int intel_crtc_init(struct intel_display *display, enum pipe pipe) > > cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE); > >- drm_WARN_ON(display->drm, drm_crtc_index(&crtc->base) != crtc->pipe); >- > if (HAS_CASF(display)) > drm_crtc_create_sharpness_strength_property(&crtc->base); > >diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c >index 7e000ba3e08b..b5c9cdf14a43 100644 >--- a/drivers/gpu/drm/i915/display/intel_display_driver.c >+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c >@@ -452,6 +452,7 @@ bool intel_display_driver_check_access(struct intel_display *display) > /* part #2: call after irq install, but before gem init */ > int intel_display_driver_probe_nogem(struct intel_display *display) > { >+ u8 pipe_mask = U8_MAX; > enum pipe pipe; > int ret; > >@@ -470,7 +471,30 @@ int intel_display_driver_probe_nogem(struct intel_display *display) > INTEL_NUM_PIPES(display), > INTEL_NUM_PIPES(display) > 1 ? "s" : ""); > >- for_each_pipe(display, pipe) { this would previously skip fused off pipes from __intel_display_device_info_runtime_init(). Now we are just going to err_mode_config if one of the (first 4) pipes are fused off. I think we should check DISPLAY_RUNTIME_INFO(__dev_priv)->pipe_mask inside your loop below or IMO it would avoid some redundancy to change that to loop twice with for_each_pipe_masked(), passing PIPE_A | PIPE_C on first iteration. Lucas De Marchi >+ /* >+ * Expose the pipes in order A, C, B, D on discrete platforms to trick >+ * user space into using pipes that are more likely to be available for >+ * both a) user space if pipe B has been reserved for the joiner, and b) >+ * the joiner if pipe A doesn't need the joiner. >+ * >+ * Fall back to normal initialization for the remaining pipes, if any. >+ */ >+ if (HAS_BIGJOINER(display) && display->platform.dgfx) { >+ enum pipe pipe_order[] = { PIPE_A, PIPE_C, PIPE_B, PIPE_D }; >+ int i; >+ >+ for (i = 0; i < ARRAY_SIZE(pipe_order); i++) { >+ pipe = pipe_order[i]; >+ >+ ret = intel_crtc_init(display, pipe); >+ if (ret) >+ goto err_mode_config; >+ >+ pipe_mask &= ~BIT(pipe); >+ } >+ } >+ >+ for_each_pipe_masked(display, pipe, pipe_mask) { > ret = intel_crtc_init(display, pipe); > if (ret) > goto err_mode_config; >-- >2.47.3 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms 2025-11-19 16:08 ` Lucas De Marchi @ 2025-11-19 16:21 ` Jani Nikula 0 siblings, 0 replies; 6+ messages in thread From: Jani Nikula @ 2025-11-19 16:21 UTC (permalink / raw) To: Lucas De Marchi; +Cc: intel-gfx, intel-xe, ville.syrjala On Wed, 19 Nov 2025, Lucas De Marchi <lucas.demarchi@intel.com> wrote: > On Wed, Nov 19, 2025 at 05:33:21PM +0200, Jani Nikula wrote: >>When big joiner is enabled, it reserves the adjacent pipe as the >>secondary pipe. This happens without the user space knowing, and >>subsequent attempts at using the CRTC with that pipe will fail. If the >>user space does not have a coping mechanism, i.e. trying another CRTC, >>this leads to a black screen. >> >>Try to reduce the impact of the problem on discrete platforms by mapping >>the CRTCs to pipes in order A, C, B, and D. If the user space reserves >>CRTCs in order, this should trick it to using pipes that are more likely >>to be available for and after joining. >> >>Limit this to discrete platforms, which have four pipes, and no eDP, a >>combination that should benefit the most with least drawbacks. >> >>Although there are currently no platforms with more than four pipes, add >>a fallback for initializing the rest of the pipes to not miss them. >> >>Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >>--- >> >>v2: Also remove WARN_ON() >> >>v3: Limit to discrete >> >>There are a number of issues in IGT with assuming CRTC index == pipe, at >>least with CRC and vblank waits. With them being used a lot in tests, we >>won't get enough test coverage until they're fixed. >>--- >> drivers/gpu/drm/i915/display/intel_crtc.c | 2 -- >> .../drm/i915/display/intel_display_driver.c | 26 ++++++++++++++++++- >> 2 files changed, 25 insertions(+), 3 deletions(-) >> >>diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c >>index 9d2a23c96c61..11e58d07ddef 100644 >>--- a/drivers/gpu/drm/i915/display/intel_crtc.c >>+++ b/drivers/gpu/drm/i915/display/intel_crtc.c >>@@ -394,8 +394,6 @@ int intel_crtc_init(struct intel_display *display, enum pipe pipe) >> >> cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE); >> >>- drm_WARN_ON(display->drm, drm_crtc_index(&crtc->base) != crtc->pipe); >>- >> if (HAS_CASF(display)) >> drm_crtc_create_sharpness_strength_property(&crtc->base); >> >>diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c >>index 7e000ba3e08b..b5c9cdf14a43 100644 >>--- a/drivers/gpu/drm/i915/display/intel_display_driver.c >>+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c >>@@ -452,6 +452,7 @@ bool intel_display_driver_check_access(struct intel_display *display) >> /* part #2: call after irq install, but before gem init */ >> int intel_display_driver_probe_nogem(struct intel_display *display) >> { >>+ u8 pipe_mask = U8_MAX; >> enum pipe pipe; >> int ret; >> >>@@ -470,7 +471,30 @@ int intel_display_driver_probe_nogem(struct intel_display *display) >> INTEL_NUM_PIPES(display), >> INTEL_NUM_PIPES(display) > 1 ? "s" : ""); >> >>- for_each_pipe(display, pipe) { > > this would previously skip fused off pipes from > __intel_display_device_info_runtime_init(). Now we are just > going to err_mode_config if one of the (first 4) pipes are > fused off. > > I think we should check DISPLAY_RUNTIME_INFO(__dev_priv)->pipe_mask > inside your loop below or IMO it would avoid some redundancy to change > that to loop twice with for_each_pipe_masked(), passing > PIPE_A | PIPE_C on first iteration. Good catch, thanks! BR, Jani. > > Lucas De Marchi > >>+ /* >>+ * Expose the pipes in order A, C, B, D on discrete platforms to trick >>+ * user space into using pipes that are more likely to be available for >>+ * both a) user space if pipe B has been reserved for the joiner, and b) >>+ * the joiner if pipe A doesn't need the joiner. >>+ * >>+ * Fall back to normal initialization for the remaining pipes, if any. >>+ */ >>+ if (HAS_BIGJOINER(display) && display->platform.dgfx) { >>+ enum pipe pipe_order[] = { PIPE_A, PIPE_C, PIPE_B, PIPE_D }; >>+ int i; >>+ >>+ for (i = 0; i < ARRAY_SIZE(pipe_order); i++) { >>+ pipe = pipe_order[i]; >>+ >>+ ret = intel_crtc_init(display, pipe); >>+ if (ret) >>+ goto err_mode_config; >>+ >>+ pipe_mask &= ~BIT(pipe); >>+ } >>+ } >>+ >>+ for_each_pipe_masked(display, pipe, pipe_mask) { >> ret = intel_crtc_init(display, pipe); >> if (ret) >> goto err_mode_config; >>-- >>2.47.3 >> -- Jani Nikula, Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
* ✗ i915.CI.BAT: failure for drm/i915/display: change pipe allocation order for discrete platforms 2025-11-19 15:33 [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms Jani Nikula 2025-11-19 16:08 ` Lucas De Marchi @ 2025-11-19 17:09 ` Patchwork 2025-11-19 17:24 ` [PATCH v3] " Ville Syrjälä 2 siblings, 0 replies; 6+ messages in thread From: Patchwork @ 2025-11-19 17:09 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 5354 bytes --] == Series Details == Series: drm/i915/display: change pipe allocation order for discrete platforms URL : https://patchwork.freedesktop.org/series/157783/ State : failure == Summary == CI Bug Log - changes from CI_DRM_17569 -> Patchwork_157783v1 ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_157783v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_157783v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/index.html Participating hosts (45 -> 44) ------------------------------ Missing (1): fi-snb-2520m Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_157783v1: ### IGT changes ### #### Possible regressions #### * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-1: - bat-dg2-8: [PASS][1] -> [FAIL][2] +20 other tests fail [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17569/bat-dg2-8/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-1.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/bat-dg2-8/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-c-dp-1.html * igt@kms_pipe_crc_basic@nonblocking-crc: - bat-dg1-7: [PASS][3] -> [FAIL][4] +20 other tests fail [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17569/bat-dg1-7/igt@kms_pipe_crc_basic@nonblocking-crc.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/bat-dg1-7/igt@kms_pipe_crc_basic@nonblocking-crc.html * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence: - bat-dg2-11: [PASS][5] -> [FAIL][6] +20 other tests fail [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17569/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1: - bat-dg2-9: [PASS][7] -> [FAIL][8] +20 other tests fail [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17569/bat-dg2-9/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/bat-dg2-9/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1.html * igt@kms_pipe_crc_basic@read-crc-frame-sequence: - bat-dg2-14: [PASS][9] -> [FAIL][10] +20 other tests fail [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17569/bat-dg2-14/igt@kms_pipe_crc_basic@read-crc-frame-sequence.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/bat-dg2-14/igt@kms_pipe_crc_basic@read-crc-frame-sequence.html Known issues ------------ Here are the changes found in Patchwork_157783v1 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@i915_selftest@live@workarounds: - bat-arlh-3: [PASS][11] -> [DMESG-FAIL][12] ([i915#12061]) +1 other test dmesg-fail [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17569/bat-arlh-3/igt@i915_selftest@live@workarounds.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/bat-arlh-3/igt@i915_selftest@live@workarounds.html #### Possible fixes #### * igt@i915_selftest@live: - bat-mtlp-8: [DMESG-FAIL][13] ([i915#12061]) -> [PASS][14] +1 other test pass [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17569/bat-mtlp-8/igt@i915_selftest@live.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/bat-mtlp-8/igt@i915_selftest@live.html #### Warnings #### * igt@i915_selftest@live: - bat-atsm-1: [DMESG-FAIL][15] ([i915#12061] / [i915#14204]) -> [DMESG-FAIL][16] ([i915#12061] / [i915#13929]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17569/bat-atsm-1/igt@i915_selftest@live.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/bat-atsm-1/igt@i915_selftest@live.html * igt@i915_selftest@live@mman: - bat-atsm-1: [DMESG-FAIL][17] ([i915#14204]) -> [DMESG-FAIL][18] ([i915#13929]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17569/bat-atsm-1/igt@i915_selftest@live@mman.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/bat-atsm-1/igt@i915_selftest@live@mman.html [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061 [i915#13929]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13929 [i915#14204]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204 Build changes ------------- * Linux: CI_DRM_17569 -> Patchwork_157783v1 CI-20190529: 20190529 CI_DRM_17569: 186229155346a45a09ce797200476a00872abbd2 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_8633: 8633 Patchwork_157783v1: 186229155346a45a09ce797200476a00872abbd2 @ git://anongit.freedesktop.org/gfx-ci/linux == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_157783v1/index.html [-- Attachment #2: Type: text/html, Size: 6560 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms 2025-11-19 15:33 [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms Jani Nikula 2025-11-19 16:08 ` Lucas De Marchi 2025-11-19 17:09 ` ✗ i915.CI.BAT: failure for " Patchwork @ 2025-11-19 17:24 ` Ville Syrjälä 2025-11-20 14:53 ` Ville Syrjälä 2 siblings, 1 reply; 6+ messages in thread From: Ville Syrjälä @ 2025-11-19 17:24 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx, intel-xe On Wed, Nov 19, 2025 at 05:33:21PM +0200, Jani Nikula wrote: > When big joiner is enabled, it reserves the adjacent pipe as the > secondary pipe. This happens without the user space knowing, and > subsequent attempts at using the CRTC with that pipe will fail. If the > user space does not have a coping mechanism, i.e. trying another CRTC, > this leads to a black screen. > > Try to reduce the impact of the problem on discrete platforms by mapping > the CRTCs to pipes in order A, C, B, and D. If the user space reserves > CRTCs in order, this should trick it to using pipes that are more likely > to be available for and after joining. > > Limit this to discrete platforms, which have four pipes, and no eDP, a > combination that should benefit the most with least drawbacks. > > Although there are currently no platforms with more than four pipes, add > a fallback for initializing the rest of the pipes to not miss them. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > v2: Also remove WARN_ON() > > v3: Limit to discrete > > There are a number of issues in IGT with assuming CRTC index == pipe, at > least with CRC and vblank waits. With them being used a lot in tests, we > won't get enough test coverage until they're fixed. > --- > drivers/gpu/drm/i915/display/intel_crtc.c | 2 -- > .../drm/i915/display/intel_display_driver.c | 26 ++++++++++++++++++- > 2 files changed, 25 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c > index 9d2a23c96c61..11e58d07ddef 100644 > --- a/drivers/gpu/drm/i915/display/intel_crtc.c > +++ b/drivers/gpu/drm/i915/display/intel_crtc.c > @@ -394,8 +394,6 @@ int intel_crtc_init(struct intel_display *display, enum pipe pipe) > > cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE); > > - drm_WARN_ON(display->drm, drm_crtc_index(&crtc->base) != crtc->pipe); > - > if (HAS_CASF(display)) > drm_crtc_create_sharpness_strength_property(&crtc->base); > > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c > index 7e000ba3e08b..b5c9cdf14a43 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c > @@ -452,6 +452,7 @@ bool intel_display_driver_check_access(struct intel_display *display) > /* part #2: call after irq install, but before gem init */ > int intel_display_driver_probe_nogem(struct intel_display *display) > { > + u8 pipe_mask = U8_MAX; > enum pipe pipe; > int ret; > > @@ -470,7 +471,30 @@ int intel_display_driver_probe_nogem(struct intel_display *display) > INTEL_NUM_PIPES(display), > INTEL_NUM_PIPES(display) > 1 ? "s" : ""); > > - for_each_pipe(display, pipe) { > + /* > + * Expose the pipes in order A, C, B, D on discrete platforms to trick > + * user space into using pipes that are more likely to be available for > + * both a) user space if pipe B has been reserved for the joiner, and b) > + * the joiner if pipe A doesn't need the joiner. > + * > + * Fall back to normal initialization for the remaining pipes, if any. > + */ > + if (HAS_BIGJOINER(display) && display->platform.dgfx) { uncompressed joiner is also a thing. > + enum pipe pipe_order[] = { PIPE_A, PIPE_C, PIPE_B, PIPE_D }; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(pipe_order); i++) { > + pipe = pipe_order[i]; > + > + ret = intel_crtc_init(display, pipe); > + if (ret) > + goto err_mode_config; > + > + pipe_mask &= ~BIT(pipe); > + } > + } I was thinking it might be easier to just do the B<->C swap inside intel_crtc_init(). Kinda similar how we we already do the plane A<->B swap in intel_primary_plane_create(). But I guess the loop here would become a bit more confusing since it would have to iterate all possible pipes and not just the ones present in the runtime info pipe_mask. > + > + for_each_pipe_masked(display, pipe, pipe_mask) { > ret = intel_crtc_init(display, pipe); > if (ret) > goto err_mode_config; > -- > 2.47.3 -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms 2025-11-19 17:24 ` [PATCH v3] " Ville Syrjälä @ 2025-11-20 14:53 ` Ville Syrjälä 0 siblings, 0 replies; 6+ messages in thread From: Ville Syrjälä @ 2025-11-20 14:53 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx, intel-xe On Wed, Nov 19, 2025 at 07:24:22PM +0200, Ville Syrjälä wrote: > On Wed, Nov 19, 2025 at 05:33:21PM +0200, Jani Nikula wrote: > > When big joiner is enabled, it reserves the adjacent pipe as the > > secondary pipe. This happens without the user space knowing, and > > subsequent attempts at using the CRTC with that pipe will fail. If the > > user space does not have a coping mechanism, i.e. trying another CRTC, > > this leads to a black screen. > > > > Try to reduce the impact of the problem on discrete platforms by mapping > > the CRTCs to pipes in order A, C, B, and D. If the user space reserves > > CRTCs in order, this should trick it to using pipes that are more likely > > to be available for and after joining. > > > > Limit this to discrete platforms, which have four pipes, and no eDP, a > > combination that should benefit the most with least drawbacks. > > > > Although there are currently no platforms with more than four pipes, add > > a fallback for initializing the rest of the pipes to not miss them. > > > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > > > --- > > > > v2: Also remove WARN_ON() > > > > v3: Limit to discrete > > > > There are a number of issues in IGT with assuming CRTC index == pipe, at > > least with CRC and vblank waits. With them being used a lot in tests, we > > won't get enough test coverage until they're fixed. > > --- > > drivers/gpu/drm/i915/display/intel_crtc.c | 2 -- > > .../drm/i915/display/intel_display_driver.c | 26 ++++++++++++++++++- > > 2 files changed, 25 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c > > index 9d2a23c96c61..11e58d07ddef 100644 > > --- a/drivers/gpu/drm/i915/display/intel_crtc.c > > +++ b/drivers/gpu/drm/i915/display/intel_crtc.c > > @@ -394,8 +394,6 @@ int intel_crtc_init(struct intel_display *display, enum pipe pipe) > > > > cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE); > > > > - drm_WARN_ON(display->drm, drm_crtc_index(&crtc->base) != crtc->pipe); > > - > > if (HAS_CASF(display)) > > drm_crtc_create_sharpness_strength_property(&crtc->base); > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c > > index 7e000ba3e08b..b5c9cdf14a43 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c > > @@ -452,6 +452,7 @@ bool intel_display_driver_check_access(struct intel_display *display) > > /* part #2: call after irq install, but before gem init */ > > int intel_display_driver_probe_nogem(struct intel_display *display) > > { > > + u8 pipe_mask = U8_MAX; > > enum pipe pipe; > > int ret; > > > > @@ -470,7 +471,30 @@ int intel_display_driver_probe_nogem(struct intel_display *display) > > INTEL_NUM_PIPES(display), > > INTEL_NUM_PIPES(display) > 1 ? "s" : ""); > > > > - for_each_pipe(display, pipe) { > > + /* > > + * Expose the pipes in order A, C, B, D on discrete platforms to trick > > + * user space into using pipes that are more likely to be available for > > + * both a) user space if pipe B has been reserved for the joiner, and b) > > + * the joiner if pipe A doesn't need the joiner. > > + * > > + * Fall back to normal initialization for the remaining pipes, if any. > > + */ > > + if (HAS_BIGJOINER(display) && display->platform.dgfx) { > > uncompressed joiner is also a thing. > > > + enum pipe pipe_order[] = { PIPE_A, PIPE_C, PIPE_B, PIPE_D }; > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(pipe_order); i++) { > > + pipe = pipe_order[i]; > > + > > + ret = intel_crtc_init(display, pipe); > > + if (ret) > > + goto err_mode_config; > > + > > + pipe_mask &= ~BIT(pipe); > > + } > > + } > > I was thinking it might be easier to just do the B<->C swap inside > intel_crtc_init(). Kinda similar how we we already do the plane > A<->B swap in intel_primary_plane_create(). But I guess the > loop here would become a bit more confusing since it would have > to iterate all possible pipes and not just the ones present in > the runtime info pipe_mask. Gave this a quick try and I don't think it looks all that bad. Shrug. diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c index 7ebbde716238..9279c76216ca 100644 --- a/drivers/gpu/drm/i915/display/intel_crtc.c +++ b/drivers/gpu/drm/i915/display/intel_crtc.c @@ -326,6 +326,21 @@ static void add_crtc_to_pipe_list(struct intel_display *display, struct intel_cr list_add_tail(&crtc->pipe_head, &display->pipe_list); } +static enum pipe reorder_pipe(struct intel_display *display, enum pipe pipe) +{ + if (!display->platform.dgfx) + return pipe; + + switch (pipe) { + case PIPE_B: + return PIPE_C; + case PIPE_C: + return PIPE_B; + default: + return pipe; + } +} + int intel_crtc_init(struct intel_display *display, enum pipe pipe) { struct intel_plane *primary, *cursor; @@ -333,6 +348,11 @@ int intel_crtc_init(struct intel_display *display, enum pipe pipe) struct intel_crtc *crtc; int sprite, ret; + pipe = reorder_pipe(display, pipe); + + if ((DISPLAY_RUNTIME_INFO(display)->pipe_mask & BIT(pipe)) == 0) + return 0; + crtc = intel_crtc_alloc(); if (IS_ERR(crtc)) return PTR_ERR(crtc); diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c index 32726906e550..cd30c6f18bb5 100644 --- a/drivers/gpu/drm/i915/display/intel_display_driver.c +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c @@ -471,7 +471,7 @@ int intel_display_driver_probe_nogem(struct intel_display *display) INTEL_NUM_PIPES(display), INTEL_NUM_PIPES(display) > 1 ? "s" : ""); - for_each_pipe(display, pipe) { + for (pipe = PIPE_A; pipe < I915_MAX_PIPES; pipe++) { ret = intel_crtc_init(display, pipe); if (ret) goto err_mode_config; -- Ville Syrjälä Intel ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-20 14:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-19 15:33 [PATCH v3] drm/i915/display: change pipe allocation order for discrete platforms Jani Nikula 2025-11-19 16:08 ` Lucas De Marchi 2025-11-19 16:21 ` Jani Nikula 2025-11-19 17:09 ` ✗ i915.CI.BAT: failure for " Patchwork 2025-11-19 17:24 ` [PATCH v3] " Ville Syrjälä 2025-11-20 14:53 ` Ville Syrjälä
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).