| Message ID | 1529299083-151008-1-git-send-email-azhar.shaikh@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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);
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.
>-----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
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?
>-----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
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.
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);
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(+)