* [RFC] drm/i915: Fix assert_plane() warning on bootup with external display @ 2018-06-18 5:18 Azhar Shaikh 2018-06-18 5:46 ` ✗ Fi.CI.BAT: failure for " Patchwork 2018-06-18 8:18 ` [RFC] " Jani Nikula 0 siblings, 2 replies; 8+ messages in thread From: Azhar Shaikh @ 2018-06-18 5:18 UTC (permalink / raw) To: intel-gfx On KBL, WHL RVPs, booting up with an external display connected, triggers below warning. This warning is not seen during hotplug. [ 3.615226] ------------[ cut here ]------------ [ 3.619829] plane 1A assertion failure (expected on, current off) [ 3.632039] WARNING: CPU: 2 PID: 354 at drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb [ 3.633920] iwlwifi 0000:00:14.3: loaded firmware version 38.c0e03d94.0 op_mode iwlmvm [ 3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm btintel bluetooth ecdh_generic [ 3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 4.17.0-rc7-50176-g655af12d39c2 #3 [ 3.647165] Hardware name: Intel Corporation CoffeeLake Client Platform/WhiskeyLake U DDR4 ERB, BIOS CNLSFWR1.R00.X140.B00.1804040304 04/04/2018 [ 3.684509] RIP: 0010:assert_plane+0x71/0xbb [ 3.764451] Call Trace: [ 3.766888] intel_atomic_commit_tail+0xa97/0xb77 [ 3.771569] intel_atomic_commit+0x26a/0x279 [ 3.771572] drm_atomic_helper_set_config+0x5c/0x76 [ 3.780670] __drm_mode_set_config_internal+0x66/0x109 [ 3.780672] drm_mode_setcrtc+0x4c9/0x5cc [ 3.780674] ? drm_mode_getcrtc+0x162/0x162 [ 3.789774] ? drm_mode_getcrtc+0x162/0x162 [ 3.798108] drm_ioctl_kernel+0x8d/0xe4 [ 3.801926] drm_ioctl+0x27d/0x368 [ 3.805311] ? drm_mode_getcrtc+0x162/0x162 [ 3.805314] ? selinux_file_ioctl+0x14e/0x199 [ 3.805317] vfs_ioctl+0x21/0x2f [ 3.813812] do_vfs_ioctl+0x491/0x4b4 [ 3.813813] ? security_file_ioctl+0x37/0x4b [ 3.813816] ksys_ioctl+0x55/0x75 [ 3.820672] __x64_sys_ioctl+0x1a/0x1e [ 3.820674] do_syscall_64+0x51/0x5f [ 3.820678] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 3.828221] RIP: 0033:0x7b5e04953967 [ 3.835504] RSP: 002b:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007b5e04953967 [ 3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: 000000000000000f [ 3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: 0000000000000000 [ 3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: 000000000000000f [ 3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: 00000000c06864a2 [ 3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 f9 48 0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff <0f> 0b eb 2b 48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48 [ 3.905845] WARNING: CPU: 2 PID: 354 at drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb [ 3.920964] ---[ end trace dac692f4ac46391a ]--- The warning is seen when mode_setcrtc() is called for pipeB during bootup and before we get a mode_setcrtc() for pipeA, while doing update_crtcs() in intel_atomic_commit_tail(). Now since, plane1A is still active after commit, update_crtcs() is done for pipeA and eventually update_plane() for plane1A. intel_plane_state->ctl for plane1A is not updated since set_modecrtc() is called for pipeB. So intel_plane_state->ctl for plane 1A will be 0x0. So doing an update_plane() for plane1A, will result in clearing PLANE_CTL_ENABLE bit, and hence the warning. To fix this warning, prior to updating the PLANE_CTL register with intel_plane_state->ctl value, read PLANE_CTL register, OR it with intel_plane_state->ctl and then write it to PLANE_CTL register instead of just relying on intel_plane_state->ctl value. Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com> --- drivers/gpu/drm/i915/intel_sprite.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 344c0e709b19..b491b1fbdea1 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -254,6 +254,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state) uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16; unsigned long irqflags; + u32 val; /* Sizes are 0 based */ src_w--; @@ -322,6 +323,8 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state) I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x); } + val = I915_READ_FW(PLANE_CTL(pipe, plane_id)); + plane_ctl |= val; I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl); I915_WRITE_FW(PLANE_SURF(pipe, plane_id), intel_plane_ggtt_offset(plane_state) + surf_addr); -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Fix assert_plane() warning on bootup with external display 2018-06-18 5:18 [RFC] drm/i915: Fix assert_plane() warning on bootup with external display Azhar Shaikh @ 2018-06-18 5:46 ` Patchwork 2018-06-18 8:18 ` [RFC] " Jani Nikula 1 sibling, 0 replies; 8+ messages in thread From: Patchwork @ 2018-06-18 5:46 UTC (permalink / raw) To: Azhar Shaikh; +Cc: intel-gfx == Series Details == Series: drm/i915: Fix assert_plane() warning on bootup with external display URL : https://patchwork.freedesktop.org/series/44909/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_4331 -> Patchwork_9344 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_9344 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9344, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/44909/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9344: === IGT changes === ==== Possible regressions ==== igt@pm_backlight@basic-brightness: fi-cnl-psr: PASS -> DMESG-WARN == Known issues == Here are the changes found in Patchwork_9344 that come from known issues: === IGT changes === ==== Issues hit ==== igt@gem_exec_gttfill@basic: fi-byt-n2820: PASS -> FAIL (fdo#106744) igt@kms_flip@basic-flip-vs-dpms: fi-glk-j4005: PASS -> DMESG-WARN (fdo#106097) +1 igt@kms_flip@basic-flip-vs-modeset: fi-glk-j4005: PASS -> DMESG-WARN (fdo#106000) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-bxt-dsi: NOTRUN -> INCOMPLETE (fdo#103927) ==== Possible fixes ==== igt@gem_ringfill@basic-default-hang: fi-elk-e7500: DMESG-WARN -> PASS igt@kms_chamelium@dp-crc-fast: fi-kbl-7500u: DMESG-FAIL (fdo#103841) -> PASS igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: fi-glk-j4005: FAIL (fdo#106765) -> PASS fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000 fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097 fdo#106744 https://bugs.freedesktop.org/show_bug.cgi?id=106744 fdo#106765 https://bugs.freedesktop.org/show_bug.cgi?id=106765 == Participating hosts (41 -> 35) == Additional (1): fi-bxt-dsi Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-ilk-650 fi-ctg-p8600 == Build changes == * Linux: CI_DRM_4331 -> Patchwork_9344 CI_DRM_4331: e47233f783ee68a864c2a10216735aca62d642da @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4522: 077c6f7c3786334c5e5c34888ab446fdb4347331 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9344: ff8437bc0ea867a61d3c5ee8c73a0eec6335df4d @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == ff8437bc0ea8 drm/i915: Fix assert_plane() warning on bootup with external display == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9344/issues.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] drm/i915: Fix assert_plane() warning on bootup with external display 2018-06-18 5:18 [RFC] drm/i915: Fix assert_plane() warning on bootup with external display Azhar Shaikh 2018-06-18 5:46 ` ✗ Fi.CI.BAT: failure for " Patchwork @ 2018-06-18 8:18 ` Jani Nikula 2018-06-18 11:57 ` Ville Syrjälä 1 sibling, 1 reply; 8+ messages in thread From: Jani Nikula @ 2018-06-18 8:18 UTC (permalink / raw) To: Azhar Shaikh, intel-gfx On Sun, 17 Jun 2018, Azhar Shaikh <azhar.shaikh@intel.com> wrote: > On KBL, WHL RVPs, booting up with an external display connected, triggers > below warning. This warning is not seen during hotplug. > > [ 3.615226] ------------[ cut here ]------------ > [ 3.619829] plane 1A assertion failure (expected on, current off) > [ 3.632039] WARNING: CPU: 2 PID: 354 at drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb > [ 3.633920] iwlwifi 0000:00:14.3: loaded firmware version 38.c0e03d94.0 op_mode iwlmvm > [ 3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm btintel bluetooth ecdh_generic > [ 3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 4.17.0-rc7-50176-g655af12d39c2 #3 > [ 3.647165] Hardware name: Intel Corporation CoffeeLake Client Platform/WhiskeyLake U DDR4 ERB, BIOS CNLSFWR1.R00.X140.B00.1804040304 04/04/2018 > [ 3.684509] RIP: 0010:assert_plane+0x71/0xbb > [ 3.764451] Call Trace: > [ 3.766888] intel_atomic_commit_tail+0xa97/0xb77 > [ 3.771569] intel_atomic_commit+0x26a/0x279 > [ 3.771572] drm_atomic_helper_set_config+0x5c/0x76 > [ 3.780670] __drm_mode_set_config_internal+0x66/0x109 > [ 3.780672] drm_mode_setcrtc+0x4c9/0x5cc > [ 3.780674] ? drm_mode_getcrtc+0x162/0x162 > [ 3.789774] ? drm_mode_getcrtc+0x162/0x162 > [ 3.798108] drm_ioctl_kernel+0x8d/0xe4 > [ 3.801926] drm_ioctl+0x27d/0x368 > [ 3.805311] ? drm_mode_getcrtc+0x162/0x162 > [ 3.805314] ? selinux_file_ioctl+0x14e/0x199 > [ 3.805317] vfs_ioctl+0x21/0x2f > [ 3.813812] do_vfs_ioctl+0x491/0x4b4 > [ 3.813813] ? security_file_ioctl+0x37/0x4b > [ 3.813816] ksys_ioctl+0x55/0x75 > [ 3.820672] __x64_sys_ioctl+0x1a/0x1e > [ 3.820674] do_syscall_64+0x51/0x5f > [ 3.820678] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > [ 3.828221] RIP: 0033:0x7b5e04953967 > [ 3.835504] RSP: 002b:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > [ 3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007b5e04953967 > [ 3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: 000000000000000f > [ 3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: 0000000000000000 > [ 3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: 000000000000000f > [ 3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: 00000000c06864a2 > [ 3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 f9 48 0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff <0f> 0b eb 2b 48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48 > [ 3.905845] WARNING: CPU: 2 PID: 354 at drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb > [ 3.920964] ---[ end trace dac692f4ac46391a ]--- > > The warning is seen when mode_setcrtc() is called for pipeB > during bootup and before we get a mode_setcrtc() for pipeA, > while doing update_crtcs() in intel_atomic_commit_tail(). > Now since, plane1A is still active after commit, update_crtcs() > is done for pipeA and eventually update_plane() for plane1A. > > intel_plane_state->ctl for plane1A is not updated since set_modecrtc() is > called for pipeB. So intel_plane_state->ctl for plane 1A will be 0x0. > So doing an update_plane() for plane1A, will result in clearing > PLANE_CTL_ENABLE bit, and hence the warning. > > To fix this warning, prior to updating the PLANE_CTL register with > intel_plane_state->ctl value, read PLANE_CTL register, OR it with > intel_plane_state->ctl and then write it to PLANE_CTL register > instead of just relying on intel_plane_state->ctl value. > > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com> > --- > drivers/gpu/drm/i915/intel_sprite.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 344c0e709b19..b491b1fbdea1 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -254,6 +254,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state) > uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; > uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16; > unsigned long irqflags; > + u32 val; > > /* Sizes are 0 based */ > src_w--; > @@ -322,6 +323,8 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state) > I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x); > } > > + val = I915_READ_FW(PLANE_CTL(pipe, plane_id)); > + plane_ctl |= val; You get the warning backtrace from our state checker, the purpose of which is to ensure that our software state and hardware state match. This change defeats the purpose of the state checker by relying on the hardware state using a read-modify-write. The states will be out of sync and we won't even know. The real fix, I think at least before I grab another cup of desperately needed coffee, is to fix the state readout during probe. BR, Jani. > I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl); > I915_WRITE_FW(PLANE_SURF(pipe, plane_id), > intel_plane_ggtt_offset(plane_state) + surf_addr); -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] drm/i915: Fix assert_plane() warning on bootup with external display 2018-06-18 8:18 ` [RFC] " Jani Nikula @ 2018-06-18 11:57 ` Ville Syrjälä 2018-06-18 21:40 ` Shaikh, Azhar 0 siblings, 1 reply; 8+ messages in thread From: Ville Syrjälä @ 2018-06-18 11:57 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Mon, Jun 18, 2018 at 11:18:52AM +0300, Jani Nikula wrote: > On Sun, 17 Jun 2018, Azhar Shaikh <azhar.shaikh@intel.com> wrote: > > On KBL, WHL RVPs, booting up with an external display connected, triggers > > below warning. This warning is not seen during hotplug. > > > > [ 3.615226] ------------[ cut here ]------------ > > [ 3.619829] plane 1A assertion failure (expected on, current off) > > [ 3.632039] WARNING: CPU: 2 PID: 354 at drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb > > [ 3.633920] iwlwifi 0000:00:14.3: loaded firmware version 38.c0e03d94.0 op_mode iwlmvm > > [ 3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm btintel bluetooth ecdh_generic > > [ 3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 4.17.0-rc7-50176-g655af12d39c2 #3 > > [ 3.647165] Hardware name: Intel Corporation CoffeeLake Client Platform/WhiskeyLake U DDR4 ERB, BIOS CNLSFWR1.R00.X140.B00.1804040304 04/04/2018 > > [ 3.684509] RIP: 0010:assert_plane+0x71/0xbb > > [ 3.764451] Call Trace: > > [ 3.766888] intel_atomic_commit_tail+0xa97/0xb77 > > [ 3.771569] intel_atomic_commit+0x26a/0x279 > > [ 3.771572] drm_atomic_helper_set_config+0x5c/0x76 > > [ 3.780670] __drm_mode_set_config_internal+0x66/0x109 > > [ 3.780672] drm_mode_setcrtc+0x4c9/0x5cc > > [ 3.780674] ? drm_mode_getcrtc+0x162/0x162 > > [ 3.789774] ? drm_mode_getcrtc+0x162/0x162 > > [ 3.798108] drm_ioctl_kernel+0x8d/0xe4 > > [ 3.801926] drm_ioctl+0x27d/0x368 > > [ 3.805311] ? drm_mode_getcrtc+0x162/0x162 > > [ 3.805314] ? selinux_file_ioctl+0x14e/0x199 > > [ 3.805317] vfs_ioctl+0x21/0x2f > > [ 3.813812] do_vfs_ioctl+0x491/0x4b4 > > [ 3.813813] ? security_file_ioctl+0x37/0x4b > > [ 3.813816] ksys_ioctl+0x55/0x75 > > [ 3.820672] __x64_sys_ioctl+0x1a/0x1e > > [ 3.820674] do_syscall_64+0x51/0x5f > > [ 3.820678] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > [ 3.828221] RIP: 0033:0x7b5e04953967 > > [ 3.835504] RSP: 002b:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > > [ 3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007b5e04953967 > > [ 3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: 000000000000000f > > [ 3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: 0000000000000000 > > [ 3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: 000000000000000f > > [ 3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: 00000000c06864a2 > > [ 3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 f9 48 0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff <0f> 0b eb 2b 48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48 > > [ 3.905845] WARNING: CPU: 2 PID: 354 at drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb > > [ 3.920964] ---[ end trace dac692f4ac46391a ]--- > > > > The warning is seen when mode_setcrtc() is called for pipeB > > during bootup and before we get a mode_setcrtc() for pipeA, > > while doing update_crtcs() in intel_atomic_commit_tail(). > > Now since, plane1A is still active after commit, update_crtcs() > > is done for pipeA and eventually update_plane() for plane1A. > > > > intel_plane_state->ctl for plane1A is not updated since set_modecrtc() is > > called for pipeB. So intel_plane_state->ctl for plane 1A will be 0x0. > > So doing an update_plane() for plane1A, will result in clearing > > PLANE_CTL_ENABLE bit, and hence the warning. > > > > To fix this warning, prior to updating the PLANE_CTL register with > > intel_plane_state->ctl value, read PLANE_CTL register, OR it with > > intel_plane_state->ctl and then write it to PLANE_CTL register > > instead of just relying on intel_plane_state->ctl value. > > > > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com> > > --- > > drivers/gpu/drm/i915/intel_sprite.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index 344c0e709b19..b491b1fbdea1 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -254,6 +254,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state) > > uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; > > uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16; > > unsigned long irqflags; > > + u32 val; > > > > /* Sizes are 0 based */ > > src_w--; > > @@ -322,6 +323,8 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state) > > I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x); > > } > > > > + val = I915_READ_FW(PLANE_CTL(pipe, plane_id)); > > + plane_ctl |= val; > > You get the warning backtrace from our state checker, the purpose of > which is to ensure that our software state and hardware state > match. This change defeats the purpose of the state checker by relying > on the hardware state using a read-modify-write. The states will be out > of sync and we won't even know. > > The real fix, I think at least before I grab another cup of desperately > needed coffee, is to fix the state readout during probe. Full plane state readout would require a bit of work. Would be nice to have indeed, but probably quicker to just force all planes to recompute their states. I'm thinking that should be happening already, but maybe I'm mistaken. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] drm/i915: Fix assert_plane() warning on bootup with external display 2018-06-18 11:57 ` Ville Syrjälä @ 2018-06-18 21:40 ` Shaikh, Azhar 2018-06-19 11:59 ` Ville Syrjälä 0 siblings, 1 reply; 8+ messages in thread From: Shaikh, Azhar @ 2018-06-18 21:40 UTC (permalink / raw) To: Ville Syrjälä, Jani Nikula; +Cc: intel-gfx@lists.freedesktop.org >-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Monday, June 18, 2018 4:57 AM >To: Jani Nikula <jani.nikula@linux.intel.com> >Cc: Shaikh, Azhar <azhar.shaikh@intel.com>; intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [RFC] drm/i915: Fix assert_plane() warning on bootup >with external display > >On Mon, Jun 18, 2018 at 11:18:52AM +0300, Jani Nikula wrote: >> On Sun, 17 Jun 2018, Azhar Shaikh <azhar.shaikh@intel.com> wrote: >> > On KBL, WHL RVPs, booting up with an external display connected, >> > triggers below warning. This warning is not seen during hotplug. >> > >> > [ 3.615226] ------------[ cut here ]------------ >> > [ 3.619829] plane 1A assertion failure (expected on, current off) >> > [ 3.632039] WARNING: CPU: 2 PID: 354 at >drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb >> > [ 3.633920] iwlwifi 0000:00:14.3: loaded firmware version 38.c0e03d94.0 >op_mode iwlmvm >> > [ 3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm btintel >bluetooth ecdh_generic >> > [ 3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 4.17.0-rc7-50176- >g655af12d39c2 #3 >> > [ 3.647165] Hardware name: Intel Corporation CoffeeLake Client >Platform/WhiskeyLake U DDR4 ERB, BIOS >CNLSFWR1.R00.X140.B00.1804040304 04/04/2018 >> > [ 3.684509] RIP: 0010:assert_plane+0x71/0xbb >> > [ 3.764451] Call Trace: >> > [ 3.766888] intel_atomic_commit_tail+0xa97/0xb77 >> > [ 3.771569] intel_atomic_commit+0x26a/0x279 >> > [ 3.771572] drm_atomic_helper_set_config+0x5c/0x76 >> > [ 3.780670] __drm_mode_set_config_internal+0x66/0x109 >> > [ 3.780672] drm_mode_setcrtc+0x4c9/0x5cc >> > [ 3.780674] ? drm_mode_getcrtc+0x162/0x162 >> > [ 3.789774] ? drm_mode_getcrtc+0x162/0x162 >> > [ 3.798108] drm_ioctl_kernel+0x8d/0xe4 >> > [ 3.801926] drm_ioctl+0x27d/0x368 >> > [ 3.805311] ? drm_mode_getcrtc+0x162/0x162 >> > [ 3.805314] ? selinux_file_ioctl+0x14e/0x199 >> > [ 3.805317] vfs_ioctl+0x21/0x2f >> > [ 3.813812] do_vfs_ioctl+0x491/0x4b4 >> > [ 3.813813] ? security_file_ioctl+0x37/0x4b >> > [ 3.813816] ksys_ioctl+0x55/0x75 >> > [ 3.820672] __x64_sys_ioctl+0x1a/0x1e >> > [ 3.820674] do_syscall_64+0x51/0x5f >> > [ 3.820678] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> > [ 3.828221] RIP: 0033:0x7b5e04953967 >> > [ 3.835504] RSP: 002b:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: >0000000000000010 >> > [ 3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: >00007b5e04953967 >> > [ 3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: >000000000000000f >> > [ 3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: >0000000000000000 >> > [ 3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: >000000000000000f >> > [ 3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: >00000000c06864a2 >> > [ 3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 f9 48 >0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff <0f> 0b eb 2b >48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48 >> > [ 3.905845] WARNING: CPU: 2 PID: 354 at >drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb >> > [ 3.920964] ---[ end trace dac692f4ac46391a ]--- >> > >> > The warning is seen when mode_setcrtc() is called for pipeB during >> > bootup and before we get a mode_setcrtc() for pipeA, while doing >> > update_crtcs() in intel_atomic_commit_tail(). >> > Now since, plane1A is still active after commit, update_crtcs() is >> > done for pipeA and eventually update_plane() for plane1A. >> > >> > intel_plane_state->ctl for plane1A is not updated since >> > set_modecrtc() is called for pipeB. So intel_plane_state->ctl for plane 1A >will be 0x0. >> > So doing an update_plane() for plane1A, will result in clearing >> > PLANE_CTL_ENABLE bit, and hence the warning. >> > >> > To fix this warning, prior to updating the PLANE_CTL register with >> > intel_plane_state->ctl value, read PLANE_CTL register, OR it with >> > intel_plane_state->ctl and then write it to PLANE_CTL register >> > instead of just relying on intel_plane_state->ctl value. >> > >> > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com> >> > --- >> > drivers/gpu/drm/i915/intel_sprite.c | 3 +++ >> > 1 file changed, 3 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c >> > b/drivers/gpu/drm/i915/intel_sprite.c >> > index 344c0e709b19..b491b1fbdea1 100644 >> > --- a/drivers/gpu/drm/i915/intel_sprite.c >> > +++ b/drivers/gpu/drm/i915/intel_sprite.c >> > @@ -254,6 +254,7 @@ void intel_pipe_update_end(struct >intel_crtc_state *new_crtc_state) >> > uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; >> > uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16; >> > unsigned long irqflags; >> > + u32 val; >> > >> > /* Sizes are 0 based */ >> > src_w--; >> > @@ -322,6 +323,8 @@ void intel_pipe_update_end(struct >intel_crtc_state *new_crtc_state) >> > I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | >crtc_x); >> > } >> > >> > + val = I915_READ_FW(PLANE_CTL(pipe, plane_id)); >> > + plane_ctl |= val; >> >> You get the warning backtrace from our state checker, the purpose of >> which is to ensure that our software state and hardware state match. >> This change defeats the purpose of the state checker by relying on the >> hardware state using a read-modify-write. The states will be out of >> sync and we won't even know. >> >> The real fix, I think at least before I grab another cup of >> desperately needed coffee, is to fix the state readout during probe. > >Full plane state readout would require a bit of work. Would be nice to have >indeed, but probably quicker to just force all planes to recompute their states. >I'm thinking that should be happening already, but maybe I'm mistaken. > > force all planes to recompute their states All planes on all pipes need to recompute or only the pipe which is in modeset? Also recompute the sw state, is that right? In this case pipeB is the only one which is doing a modeset and after commit, pipeB is not active. Its only pipeA is active when the warning is seen. >-- >Ville Syrjälä >Intel Regards, Azhar Shaikh _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] drm/i915: Fix assert_plane() warning on bootup with external display 2018-06-18 21:40 ` Shaikh, Azhar @ 2018-06-19 11:59 ` Ville Syrjälä 2018-06-21 1:00 ` Shaikh, Azhar 0 siblings, 1 reply; 8+ messages in thread From: Ville Syrjälä @ 2018-06-19 11:59 UTC (permalink / raw) To: Shaikh, Azhar; +Cc: intel-gfx@lists.freedesktop.org On Mon, Jun 18, 2018 at 09:40:38PM +0000, Shaikh, Azhar wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >Sent: Monday, June 18, 2018 4:57 AM > >To: Jani Nikula <jani.nikula@linux.intel.com> > >Cc: Shaikh, Azhar <azhar.shaikh@intel.com>; intel-gfx@lists.freedesktop.org > >Subject: Re: [Intel-gfx] [RFC] drm/i915: Fix assert_plane() warning on bootup > >with external display > > > >On Mon, Jun 18, 2018 at 11:18:52AM +0300, Jani Nikula wrote: > >> On Sun, 17 Jun 2018, Azhar Shaikh <azhar.shaikh@intel.com> wrote: > >> > On KBL, WHL RVPs, booting up with an external display connected, > >> > triggers below warning. This warning is not seen during hotplug. > >> > > >> > [ 3.615226] ------------[ cut here ]------------ > >> > [ 3.619829] plane 1A assertion failure (expected on, current off) > >> > [ 3.632039] WARNING: CPU: 2 PID: 354 at > >drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb > >> > [ 3.633920] iwlwifi 0000:00:14.3: loaded firmware version 38.c0e03d94.0 > >op_mode iwlmvm > >> > [ 3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm btintel > >bluetooth ecdh_generic > >> > [ 3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 4.17.0-rc7-50176- > >g655af12d39c2 #3 > >> > [ 3.647165] Hardware name: Intel Corporation CoffeeLake Client > >Platform/WhiskeyLake U DDR4 ERB, BIOS > >CNLSFWR1.R00.X140.B00.1804040304 04/04/2018 > >> > [ 3.684509] RIP: 0010:assert_plane+0x71/0xbb > >> > [ 3.764451] Call Trace: > >> > [ 3.766888] intel_atomic_commit_tail+0xa97/0xb77 > >> > [ 3.771569] intel_atomic_commit+0x26a/0x279 > >> > [ 3.771572] drm_atomic_helper_set_config+0x5c/0x76 > >> > [ 3.780670] __drm_mode_set_config_internal+0x66/0x109 > >> > [ 3.780672] drm_mode_setcrtc+0x4c9/0x5cc > >> > [ 3.780674] ? drm_mode_getcrtc+0x162/0x162 > >> > [ 3.789774] ? drm_mode_getcrtc+0x162/0x162 > >> > [ 3.798108] drm_ioctl_kernel+0x8d/0xe4 > >> > [ 3.801926] drm_ioctl+0x27d/0x368 > >> > [ 3.805311] ? drm_mode_getcrtc+0x162/0x162 > >> > [ 3.805314] ? selinux_file_ioctl+0x14e/0x199 > >> > [ 3.805317] vfs_ioctl+0x21/0x2f > >> > [ 3.813812] do_vfs_ioctl+0x491/0x4b4 > >> > [ 3.813813] ? security_file_ioctl+0x37/0x4b > >> > [ 3.813816] ksys_ioctl+0x55/0x75 > >> > [ 3.820672] __x64_sys_ioctl+0x1a/0x1e > >> > [ 3.820674] do_syscall_64+0x51/0x5f > >> > [ 3.820678] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > >> > [ 3.828221] RIP: 0033:0x7b5e04953967 > >> > [ 3.835504] RSP: 002b:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: > >0000000000000010 > >> > [ 3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: > >00007b5e04953967 > >> > [ 3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: > >000000000000000f > >> > [ 3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: > >0000000000000000 > >> > [ 3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: > >000000000000000f > >> > [ 3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: > >00000000c06864a2 > >> > [ 3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 f9 48 > >0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff <0f> 0b eb 2b > >48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48 > >> > [ 3.905845] WARNING: CPU: 2 PID: 354 at > >drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb > >> > [ 3.920964] ---[ end trace dac692f4ac46391a ]--- > >> > > >> > The warning is seen when mode_setcrtc() is called for pipeB during > >> > bootup and before we get a mode_setcrtc() for pipeA, while doing > >> > update_crtcs() in intel_atomic_commit_tail(). > >> > Now since, plane1A is still active after commit, update_crtcs() is > >> > done for pipeA and eventually update_plane() for plane1A. > >> > > >> > intel_plane_state->ctl for plane1A is not updated since > >> > set_modecrtc() is called for pipeB. So intel_plane_state->ctl for plane 1A > >will be 0x0. > >> > So doing an update_plane() for plane1A, will result in clearing > >> > PLANE_CTL_ENABLE bit, and hence the warning. > >> > > >> > To fix this warning, prior to updating the PLANE_CTL register with > >> > intel_plane_state->ctl value, read PLANE_CTL register, OR it with > >> > intel_plane_state->ctl and then write it to PLANE_CTL register > >> > instead of just relying on intel_plane_state->ctl value. > >> > > >> > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com> > >> > --- > >> > drivers/gpu/drm/i915/intel_sprite.c | 3 +++ > >> > 1 file changed, 3 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > >> > b/drivers/gpu/drm/i915/intel_sprite.c > >> > index 344c0e709b19..b491b1fbdea1 100644 > >> > --- a/drivers/gpu/drm/i915/intel_sprite.c > >> > +++ b/drivers/gpu/drm/i915/intel_sprite.c > >> > @@ -254,6 +254,7 @@ void intel_pipe_update_end(struct > >intel_crtc_state *new_crtc_state) > >> > uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; > >> > uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16; > >> > unsigned long irqflags; > >> > + u32 val; > >> > > >> > /* Sizes are 0 based */ > >> > src_w--; > >> > @@ -322,6 +323,8 @@ void intel_pipe_update_end(struct > >intel_crtc_state *new_crtc_state) > >> > I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | > >crtc_x); > >> > } > >> > > >> > + val = I915_READ_FW(PLANE_CTL(pipe, plane_id)); > >> > + plane_ctl |= val; > >> > >> You get the warning backtrace from our state checker, the purpose of > >> which is to ensure that our software state and hardware state match. > >> This change defeats the purpose of the state checker by relying on the > >> hardware state using a read-modify-write. The states will be out of > >> sync and we won't even know. > >> > >> The real fix, I think at least before I grab another cup of > >> desperately needed coffee, is to fix the state readout during probe. > > > >Full plane state readout would require a bit of work. Would be nice to have > >indeed, but probably quicker to just force all planes to recompute their states. > >I'm thinking that should be happening already, but maybe I'm mistaken. > > > > > force all planes to recompute their states > > All planes on all pipes need to recompute or only the pipe which is in modeset? Any active pipe taking part in the commit that hasn't had its state fully recomputed yet. Hmm. I915_MODE_FLAG_INHERITED should force this on the first commit. But apparently that's not happening in your case. We need to figure out why that is. > Also recompute the sw state, is that right? What else is there? -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] drm/i915: Fix assert_plane() warning on bootup with external display 2018-06-19 11:59 ` Ville Syrjälä @ 2018-06-21 1:00 ` Shaikh, Azhar 2018-06-21 15:46 ` Ville Syrjälä 0 siblings, 1 reply; 8+ messages in thread From: Shaikh, Azhar @ 2018-06-21 1:00 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx@lists.freedesktop.org >-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Tuesday, June 19, 2018 5:00 AM >To: Shaikh, Azhar <azhar.shaikh@intel.com> >Cc: Jani Nikula <jani.nikula@linux.intel.com>; intel-gfx@lists.freedesktop.org >Subject: Re: [Intel-gfx] [RFC] drm/i915: Fix assert_plane() warning on bootup >with external display > >On Mon, Jun 18, 2018 at 09:40:38PM +0000, Shaikh, Azhar wrote: >> >> >> >-----Original Message----- >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >> >Sent: Monday, June 18, 2018 4:57 AM >> >To: Jani Nikula <jani.nikula@linux.intel.com> >> >Cc: Shaikh, Azhar <azhar.shaikh@intel.com>; >> >intel-gfx@lists.freedesktop.org >> >Subject: Re: [Intel-gfx] [RFC] drm/i915: Fix assert_plane() warning >> >on bootup with external display >> > >> >On Mon, Jun 18, 2018 at 11:18:52AM +0300, Jani Nikula wrote: >> >> On Sun, 17 Jun 2018, Azhar Shaikh <azhar.shaikh@intel.com> wrote: >> >> > On KBL, WHL RVPs, booting up with an external display connected, >> >> > triggers below warning. This warning is not seen during hotplug. >> >> > >> >> > [ 3.615226] ------------[ cut here ]------------ >> >> > [ 3.619829] plane 1A assertion failure (expected on, current off) >> >> > [ 3.632039] WARNING: CPU: 2 PID: 354 at >> >drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb >> >> > [ 3.633920] iwlwifi 0000:00:14.3: loaded firmware version >38.c0e03d94.0 >> >op_mode iwlmvm >> >> > [ 3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm >btintel >> >bluetooth ecdh_generic >> >> > [ 3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 4.17.0-rc7- >50176- >> >g655af12d39c2 #3 >> >> > [ 3.647165] Hardware name: Intel Corporation CoffeeLake Client >> >Platform/WhiskeyLake U DDR4 ERB, BIOS >> >CNLSFWR1.R00.X140.B00.1804040304 04/04/2018 >> >> > [ 3.684509] RIP: 0010:assert_plane+0x71/0xbb >> >> > [ 3.764451] Call Trace: >> >> > [ 3.766888] intel_atomic_commit_tail+0xa97/0xb77 >> >> > [ 3.771569] intel_atomic_commit+0x26a/0x279 >> >> > [ 3.771572] drm_atomic_helper_set_config+0x5c/0x76 >> >> > [ 3.780670] __drm_mode_set_config_internal+0x66/0x109 >> >> > [ 3.780672] drm_mode_setcrtc+0x4c9/0x5cc >> >> > [ 3.780674] ? drm_mode_getcrtc+0x162/0x162 >> >> > [ 3.789774] ? drm_mode_getcrtc+0x162/0x162 >> >> > [ 3.798108] drm_ioctl_kernel+0x8d/0xe4 >> >> > [ 3.801926] drm_ioctl+0x27d/0x368 >> >> > [ 3.805311] ? drm_mode_getcrtc+0x162/0x162 >> >> > [ 3.805314] ? selinux_file_ioctl+0x14e/0x199 >> >> > [ 3.805317] vfs_ioctl+0x21/0x2f >> >> > [ 3.813812] do_vfs_ioctl+0x491/0x4b4 >> >> > [ 3.813813] ? security_file_ioctl+0x37/0x4b >> >> > [ 3.813816] ksys_ioctl+0x55/0x75 >> >> > [ 3.820672] __x64_sys_ioctl+0x1a/0x1e >> >> > [ 3.820674] do_syscall_64+0x51/0x5f >> >> > [ 3.820678] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> >> > [ 3.828221] RIP: 0033:0x7b5e04953967 >> >> > [ 3.835504] RSP: 002b:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: >> >0000000000000010 >> >> > [ 3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: >> >00007b5e04953967 >> >> > [ 3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: >> >000000000000000f >> >> > [ 3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: >> >0000000000000000 >> >> > [ 3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: >> >000000000000000f >> >> > [ 3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: >> >00000000c06864a2 >> >> > [ 3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 >f9 48 >> >0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff >> ><0f> 0b eb 2b >> >48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48 >> >> > [ 3.905845] WARNING: CPU: 2 PID: 354 at >> >drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb >> >> > [ 3.920964] ---[ end trace dac692f4ac46391a ]--- >> >> > >> >> > The warning is seen when mode_setcrtc() is called for pipeB >> >> > during bootup and before we get a mode_setcrtc() for pipeA, while >> >> > doing >> >> > update_crtcs() in intel_atomic_commit_tail(). >> >> > Now since, plane1A is still active after commit, update_crtcs() >> >> > is done for pipeA and eventually update_plane() for plane1A. >> >> > >> >> > intel_plane_state->ctl for plane1A is not updated since >> >> > set_modecrtc() is called for pipeB. So intel_plane_state->ctl for >> >> > plane 1A >> >will be 0x0. >> >> > So doing an update_plane() for plane1A, will result in clearing >> >> > PLANE_CTL_ENABLE bit, and hence the warning. >> >> > >> >> > To fix this warning, prior to updating the PLANE_CTL register >> >> > with intel_plane_state->ctl value, read PLANE_CTL register, OR it >> >> > with intel_plane_state->ctl and then write it to PLANE_CTL >> >> > register instead of just relying on intel_plane_state->ctl value. >> >> > >> >> > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com> >> >> > --- >> >> > drivers/gpu/drm/i915/intel_sprite.c | 3 +++ >> >> > 1 file changed, 3 insertions(+) >> >> > >> >> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c >> >> > b/drivers/gpu/drm/i915/intel_sprite.c >> >> > index 344c0e709b19..b491b1fbdea1 100644 >> >> > --- a/drivers/gpu/drm/i915/intel_sprite.c >> >> > +++ b/drivers/gpu/drm/i915/intel_sprite.c >> >> > @@ -254,6 +254,7 @@ void intel_pipe_update_end(struct >> >intel_crtc_state *new_crtc_state) >> >> > uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; >> >> > uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16; >> >> > unsigned long irqflags; >> >> > + u32 val; >> >> > >> >> > /* Sizes are 0 based */ >> >> > src_w--; >> >> > @@ -322,6 +323,8 @@ void intel_pipe_update_end(struct >> >intel_crtc_state *new_crtc_state) >> >> > I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | >> >crtc_x); >> >> > } >> >> > >> >> > + val = I915_READ_FW(PLANE_CTL(pipe, plane_id)); >> >> > + plane_ctl |= val; >> >> >> >> You get the warning backtrace from our state checker, the purpose >> >> of which is to ensure that our software state and hardware state match. >> >> This change defeats the purpose of the state checker by relying on >> >> the hardware state using a read-modify-write. The states will be >> >> out of sync and we won't even know. >> >> >> >> The real fix, I think at least before I grab another cup of >> >> desperately needed coffee, is to fix the state readout during probe. >> > >> >Full plane state readout would require a bit of work. Would be nice >> >to have indeed, but probably quicker to just force all planes to recompute >their states. >> >I'm thinking that should be happening already, but maybe I'm mistaken. >> > >> >> > force all planes to recompute their states >> >> All planes on all pipes need to recompute or only the pipe which is in >modeset? > >Any active pipe taking part in the commit that hasn't had its state fully >recomputed yet. > For now I tried readout of PLANE_CTL and PLANE_COLOR_CTL, updating the new_intel_plane_state with these values in intel_pre_plane_update_plane() before calling skl_update_plane(). With this change eDP is corrupted and external display fails to come up. If I call skl_plane_ctl() before calling skl_update_plane(), I do not see any corruption, no warning and everything "seems" to works fine. So do I need to call skl_plane_ctl() + other values which need to be recomputed from readout HW state and then update the intel_plane_state()? Also what all values do we need to recompute from HW readout, other than PLANE_CTL and PLANE_COLOR_CTL? >Hmm. I915_MODE_FLAG_INHERITED should force this on the first commit. >But apparently that's not happening in your case. We need to figure out why >that is. > >> Also recompute the sw state, is that right? > >What else is there? > >-- >Ville Syrjälä >Intel Regards, Azhar Shaikh _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC] drm/i915: Fix assert_plane() warning on bootup with external display 2018-06-21 1:00 ` Shaikh, Azhar @ 2018-06-21 15:46 ` Ville Syrjälä 0 siblings, 0 replies; 8+ messages in thread From: Ville Syrjälä @ 2018-06-21 15:46 UTC (permalink / raw) To: Shaikh, Azhar; +Cc: intel-gfx@lists.freedesktop.org On Thu, Jun 21, 2018 at 01:00:48AM +0000, Shaikh, Azhar wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >Sent: Tuesday, June 19, 2018 5:00 AM > >To: Shaikh, Azhar <azhar.shaikh@intel.com> > >Cc: Jani Nikula <jani.nikula@linux.intel.com>; intel-gfx@lists.freedesktop.org > >Subject: Re: [Intel-gfx] [RFC] drm/i915: Fix assert_plane() warning on bootup > >with external display > > > >On Mon, Jun 18, 2018 at 09:40:38PM +0000, Shaikh, Azhar wrote: > >> > >> > >> >-----Original Message----- > >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] > >> >Sent: Monday, June 18, 2018 4:57 AM > >> >To: Jani Nikula <jani.nikula@linux.intel.com> > >> >Cc: Shaikh, Azhar <azhar.shaikh@intel.com>; > >> >intel-gfx@lists.freedesktop.org > >> >Subject: Re: [Intel-gfx] [RFC] drm/i915: Fix assert_plane() warning > >> >on bootup with external display > >> > > >> >On Mon, Jun 18, 2018 at 11:18:52AM +0300, Jani Nikula wrote: > >> >> On Sun, 17 Jun 2018, Azhar Shaikh <azhar.shaikh@intel.com> wrote: > >> >> > On KBL, WHL RVPs, booting up with an external display connected, > >> >> > triggers below warning. This warning is not seen during hotplug. > >> >> > > >> >> > [ 3.615226] ------------[ cut here ]------------ > >> >> > [ 3.619829] plane 1A assertion failure (expected on, current off) > >> >> > [ 3.632039] WARNING: CPU: 2 PID: 354 at > >> >drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb > >> >> > [ 3.633920] iwlwifi 0000:00:14.3: loaded firmware version > >38.c0e03d94.0 > >> >op_mode iwlmvm > >> >> > [ 3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm > >btintel > >> >bluetooth ecdh_generic > >> >> > [ 3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 4.17.0-rc7- > >50176- > >> >g655af12d39c2 #3 > >> >> > [ 3.647165] Hardware name: Intel Corporation CoffeeLake Client > >> >Platform/WhiskeyLake U DDR4 ERB, BIOS > >> >CNLSFWR1.R00.X140.B00.1804040304 04/04/2018 > >> >> > [ 3.684509] RIP: 0010:assert_plane+0x71/0xbb > >> >> > [ 3.764451] Call Trace: > >> >> > [ 3.766888] intel_atomic_commit_tail+0xa97/0xb77 > >> >> > [ 3.771569] intel_atomic_commit+0x26a/0x279 > >> >> > [ 3.771572] drm_atomic_helper_set_config+0x5c/0x76 > >> >> > [ 3.780670] __drm_mode_set_config_internal+0x66/0x109 > >> >> > [ 3.780672] drm_mode_setcrtc+0x4c9/0x5cc > >> >> > [ 3.780674] ? drm_mode_getcrtc+0x162/0x162 > >> >> > [ 3.789774] ? drm_mode_getcrtc+0x162/0x162 > >> >> > [ 3.798108] drm_ioctl_kernel+0x8d/0xe4 > >> >> > [ 3.801926] drm_ioctl+0x27d/0x368 > >> >> > [ 3.805311] ? drm_mode_getcrtc+0x162/0x162 > >> >> > [ 3.805314] ? selinux_file_ioctl+0x14e/0x199 > >> >> > [ 3.805317] vfs_ioctl+0x21/0x2f > >> >> > [ 3.813812] do_vfs_ioctl+0x491/0x4b4 > >> >> > [ 3.813813] ? security_file_ioctl+0x37/0x4b > >> >> > [ 3.813816] ksys_ioctl+0x55/0x75 > >> >> > [ 3.820672] __x64_sys_ioctl+0x1a/0x1e > >> >> > [ 3.820674] do_syscall_64+0x51/0x5f > >> >> > [ 3.820678] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > >> >> > [ 3.828221] RIP: 0033:0x7b5e04953967 > >> >> > [ 3.835504] RSP: 002b:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: > >> >0000000000000010 > >> >> > [ 3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: > >> >00007b5e04953967 > >> >> > [ 3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: > >> >000000000000000f > >> >> > [ 3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: > >> >0000000000000000 > >> >> > [ 3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: > >> >000000000000000f > >> >> > [ 3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: > >> >00000000c06864a2 > >> >> > [ 3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 > >f9 48 > >> >0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff > >> ><0f> 0b eb 2b > >> >48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48 > >> >> > [ 3.905845] WARNING: CPU: 2 PID: 354 at > >> >drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb > >> >> > [ 3.920964] ---[ end trace dac692f4ac46391a ]--- > >> >> > > >> >> > The warning is seen when mode_setcrtc() is called for pipeB > >> >> > during bootup and before we get a mode_setcrtc() for pipeA, while > >> >> > doing > >> >> > update_crtcs() in intel_atomic_commit_tail(). > >> >> > Now since, plane1A is still active after commit, update_crtcs() > >> >> > is done for pipeA and eventually update_plane() for plane1A. > >> >> > > >> >> > intel_plane_state->ctl for plane1A is not updated since > >> >> > set_modecrtc() is called for pipeB. So intel_plane_state->ctl for > >> >> > plane 1A > >> >will be 0x0. > >> >> > So doing an update_plane() for plane1A, will result in clearing > >> >> > PLANE_CTL_ENABLE bit, and hence the warning. > >> >> > > >> >> > To fix this warning, prior to updating the PLANE_CTL register > >> >> > with intel_plane_state->ctl value, read PLANE_CTL register, OR it > >> >> > with intel_plane_state->ctl and then write it to PLANE_CTL > >> >> > register instead of just relying on intel_plane_state->ctl value. > >> >> > > >> >> > Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com> > >> >> > --- > >> >> > drivers/gpu/drm/i915/intel_sprite.c | 3 +++ > >> >> > 1 file changed, 3 insertions(+) > >> >> > > >> >> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > >> >> > b/drivers/gpu/drm/i915/intel_sprite.c > >> >> > index 344c0e709b19..b491b1fbdea1 100644 > >> >> > --- a/drivers/gpu/drm/i915/intel_sprite.c > >> >> > +++ b/drivers/gpu/drm/i915/intel_sprite.c > >> >> > @@ -254,6 +254,7 @@ void intel_pipe_update_end(struct > >> >intel_crtc_state *new_crtc_state) > >> >> > uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; > >> >> > uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16; > >> >> > unsigned long irqflags; > >> >> > + u32 val; > >> >> > > >> >> > /* Sizes are 0 based */ > >> >> > src_w--; > >> >> > @@ -322,6 +323,8 @@ void intel_pipe_update_end(struct > >> >intel_crtc_state *new_crtc_state) > >> >> > I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | > >> >crtc_x); > >> >> > } > >> >> > > >> >> > + val = I915_READ_FW(PLANE_CTL(pipe, plane_id)); > >> >> > + plane_ctl |= val; > >> >> > >> >> You get the warning backtrace from our state checker, the purpose > >> >> of which is to ensure that our software state and hardware state match. > >> >> This change defeats the purpose of the state checker by relying on > >> >> the hardware state using a read-modify-write. The states will be > >> >> out of sync and we won't even know. > >> >> > >> >> The real fix, I think at least before I grab another cup of > >> >> desperately needed coffee, is to fix the state readout during probe. > >> > > >> >Full plane state readout would require a bit of work. Would be nice > >> >to have indeed, but probably quicker to just force all planes to recompute > >their states. > >> >I'm thinking that should be happening already, but maybe I'm mistaken. > >> > > >> > >> > force all planes to recompute their states > >> > >> All planes on all pipes need to recompute or only the pipe which is in > >modeset? > > > >Any active pipe taking part in the commit that hasn't had its state fully > >recomputed yet. > > > > For now I tried readout of PLANE_CTL and PLANE_COLOR_CTL, updating the new_intel_plane_state with these values in intel_pre_plane_update_plane() before calling skl_update_plane(). With this change eDP is corrupted and external display fails to come up. The readout should go to the initial_plane_config() thing, which is immplemented in a sort of ass backwards way. Ideally we'd want to do a full plane state readout first, then reconstruct the fb based on that state. Currently we do it basically the other way around. One thing I was also thinking is that eventually we might want to create some kind of placeholder fb for every enabled plane, even if aren't going to take over that fb later. That way we could use the normal atomic paths to disable all planes we don't want enabled. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-06-21 15:46 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-18 5:18 [RFC] drm/i915: Fix assert_plane() warning on bootup with external display Azhar Shaikh 2018-06-18 5:46 ` ✗ Fi.CI.BAT: failure for " Patchwork 2018-06-18 8:18 ` [RFC] " Jani Nikula 2018-06-18 11:57 ` Ville Syrjälä 2018-06-18 21:40 ` Shaikh, Azhar 2018-06-19 11:59 ` Ville Syrjälä 2018-06-21 1:00 ` Shaikh, Azhar 2018-06-21 15:46 ` Ville Syrjälä
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.