| Message ID | 20190614103941.15399-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Check visibility in icl_build_plane_wm | expand |
On Fri, Jun 14, 2019 at 12:39:41PM +0200, Maarten Lankhorst wrote: > When a planar YUV plane is configured, but the crtc is > marked inactive, we can end up with a linked plane without > visibility. How is that possible? I don't think we should be adding the slave plane if the master is not visible. > Handle this by checking for visibility early, > instead of doing a WARN. > > <4> [201.742919] ------------[ cut here ]------------ > <4> [201.742920] WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)) > <4> [201.742947] WARNING: CPU: 7 PID: 1268 at drivers/gpu/drm/i915/intel_pm.c:5068 skl_compute_wm+0x2be/0x10a0 [i915] > <4> [201.742948] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal snd_hda_intel coretemp snd_hda_codec mei_hdcp snd_hwdep snd_hda_core crct10dif_pclmul cdc_ether usbnet crc32_pclmul mii snd_pcm ghash_clmulni_intel mei_me mei prime_numbers > <4> [201.742958] CPU: 7 PID: 1268 Comm: kms_chamelium Tainted: G U 5.2.0-rc3-CI-CI_DRM_6216+ #1 > <4> [201.742960] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3183.A00.1905020411 05/02/2019 > <4> [201.742978] RIP: 0010:skl_compute_wm+0x2be/0x10a0 [i915] > <4> [201.742980] Code: 24 10 8b 92 fc 02 00 00 0f 85 ba 04 00 00 48 c7 c6 e0 38 2e a0 48 c7 c7 93 99 31 a0 89 54 24 20 48 89 44 24 08 e8 82 a2 f5 e0 <0f> 0b 8b 54 24 20 48 8b 44 24 08 48 8b 40 48 80 78 12 00 0f 85 76 > <4> [201.742981] RSP: 0018:ffffc9000064f9a8 EFLAGS: 00010282 > <4> [201.742983] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000007 > <4> [201.742984] RDX: 000000000000175c RSI: ffff8884934d48e0 RDI: ffffffff8212df49 > <4> [201.742985] RBP: ffff888493408558 R08: 00000000b56dab44 R09: 0000000000000000 > <4> [201.742986] R10: ffff88848be00000 R11: 0000000000000000 R12: ffff88849afd89f8 > <4> [201.742987] R13: ffff88847eaf67e8 R14: ffff88848c344a88 R15: ffff88848be00000 > <4> [201.742988] FS: 00007f4d9b60b700(0000) GS:ffff88849ff80000(0000) knlGS:0000000000000000 > <4> [201.742989] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > <4> [201.742991] CR2: 00007f4d9b609ff8 CR3: 000000048c326006 CR4: 0000000000760ee0 > <4> [201.742992] PKRU: 55555554 > <4> [201.742993] Call Trace: > <4> [201.743021] ? intel_atomic_check+0x7b2/0x1440 [i915] > <4> [201.743026] ? __mutex_unlock_slowpath+0x46/0x2b0 > <4> [201.743052] intel_atomic_check+0x7ca/0x1440 [i915] > <4> [201.743060] drm_atomic_check_only+0x55a/0x7f0 > <4> [201.743064] drm_atomic_commit+0xe/0x50 > <4> [201.743067] drm_atomic_connector_commit_dpms+0xe0/0xf0 > <4> [201.743069] set_property_atomic+0xba/0x140 > <4> [201.743075] drm_mode_obj_set_property_ioctl+0x111/0x1d0 > <4> [201.743077] ? drm_dev_exit+0x8/0x40 > <4> [201.743080] ? drm_connector_set_obj_prop+0x70/0x70 > <4> [201.743082] drm_connector_property_set_ioctl+0x39/0x60 > <4> [201.743084] drm_ioctl_kernel+0x83/0xf0 > <4> [201.743087] drm_ioctl+0x2f3/0x3b0 > <4> [201.743090] ? drm_connector_set_obj_prop+0x70/0x70 > <4> [201.743096] ? lock_acquire+0xa6/0x1c0 > <4> [201.743100] do_vfs_ioctl+0xa0/0x6e0 > <4> [201.743103] ? __fget+0x10f/0x200 > <4> [201.743105] ksys_ioctl+0x35/0x60 > <4> [201.743108] __x64_sys_ioctl+0x11/0x20 > <4> [201.743110] do_syscall_64+0x55/0x1c0 > <4> [201.743112] entry_SYSCALL_64_after_hwframe+0x49/0xbe > <4> [201.743114] RIP: 0033:0x7f4da6c8d5d7 > <4> [201.743115] Code: b3 66 90 48 8b 05 b1 48 2d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 81 48 2d 00 f7 d8 64 89 01 48 > <4> [201.743116] RSP: 002b:00007f4d9b60aba8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > <4> [201.743118] RAX: ffffffffffffffda RBX: 00007f4d94001ac0 RCX: 00007f4da6c8d5d7 > <4> [201.743119] RDX: 00007f4d9b60abe0 RSI: 00000000c01064ab RDI: 0000000000000005 > <4> [201.743120] RBP: 00007f4d9b60abe0 R08: 00007f4d940015c0 R09: 00007f4d940015f0 > <4> [201.743121] R10: 0000000000000055 R11: 0000000000000246 R12: 00000000c01064ab > <4> [201.743122] R13: 0000000000000005 R14: 0000000000000005 R15: 00007f4da7a2c0c7 > <4> [201.743156] irq event stamp: 362 > <4> [201.743162] hardirqs last enabled at (361): [<ffffffff8112862c>] vprintk_emit+0xcc/0x340 > <4> [201.743168] hardirqs last disabled at (362): [<ffffffff810019e0>] trace_hardirqs_off_thunk+0x1a/0x1c > <4> [201.743174] softirqs last enabled at (0): [<ffffffff810abf78>] copy_process.part.6+0x4e8/0x1dc0 > <4> [201.743178] softirqs last disabled at (0): [<0000000000000000>] 0x0 > <4> [201.743243] WARNING: CPU: 7 PID: 1268 at drivers/gpu/drm/i915/intel_pm.c:5068 skl_compute_wm+0x2be/0x10a0 [i915] > <4> [201.743246] ---[ end trace 33e6703087376efa ]--- > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110895 > Testcase: kms_chamelium@hdmi-cmp-nv12 > --- > drivers/gpu/drm/i915/intel_pm.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 4e52dad84d64..e0e57de22388 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -5060,15 +5060,15 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state, > enum plane_id plane_id = to_intel_plane(plane_state->base.plane)->id; > int ret; > > - /* Watermarks calculated in master */ > - if (plane_state->slave) > + /* Watermarks are calculated in master */ > + if (plane_state->slave || > + !intel_wm_plane_visible(crtc_state, plane_state)) > return 0; > > if (plane_state->linked_plane) { > const struct drm_framebuffer *fb = plane_state->base.fb; > enum plane_id y_plane_id = plane_state->linked_plane->id; > > - WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)); > WARN_ON(!fb->format->is_yuv || > fb->format->num_planes == 1); > > @@ -5081,7 +5081,7 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state, > plane_id, 1); > if (ret) > return ret; > - } else if (intel_wm_plane_visible(crtc_state, plane_state)) { > + } else { > ret = skl_build_plane_wm_single(crtc_state, plane_state, > plane_id, 0); > if (ret) > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 17-06-2019 om 14:34 schreef Ville Syrjälä: > On Fri, Jun 14, 2019 at 12:39:41PM +0200, Maarten Lankhorst wrote: >> When a planar YUV plane is configured, but the crtc is >> marked inactive, we can end up with a linked plane without >> visibility. > How is that possible? I don't think we should be adding the slave plane > if the master is not visible. DPMS off, we calculate the various fields as if the CRTC is on, then disable visibility. crtc_state->nv12_planes etc still get set, so it works as if the crtc is on. It's a way of not allowing an invalid result when dpms is off, then breaking on crtc enable.
On Tue, Jun 18, 2019 at 11:16:41AM +0200, Maarten Lankhorst wrote: > Op 17-06-2019 om 14:34 schreef Ville Syrjälä: > > On Fri, Jun 14, 2019 at 12:39:41PM +0200, Maarten Lankhorst wrote: > >> When a planar YUV plane is configured, but the crtc is > >> marked inactive, we can end up with a linked plane without > >> visibility. > > How is that possible? I don't think we should be adding the slave plane > > if the master is not visible. > > > DPMS off, we calculate the various fields as if the CRTC is on, then disable visibility. > > crtc_state->nv12_planes etc still get set, so it works as if the crtc is on. > > It's a way of not allowing an invalid result when dpms is off, then breaking on crtc enable. Hmm. I wonder when we started to do that. If we're already doing this much then I wonder how far we are from just dealing with the FIXME in intel_wm_plane_visible() instead?
On Tue, Jun 18, 2019 at 02:44:00PM +0300, Ville Syrjälä wrote: > On Tue, Jun 18, 2019 at 11:16:41AM +0200, Maarten Lankhorst wrote: > > Op 17-06-2019 om 14:34 schreef Ville Syrjälä: > > > On Fri, Jun 14, 2019 at 12:39:41PM +0200, Maarten Lankhorst wrote: > > >> When a planar YUV plane is configured, but the crtc is > > >> marked inactive, we can end up with a linked plane without > > >> visibility. > > > How is that possible? I don't think we should be adding the slave plane > > > if the master is not visible. > > > > > > DPMS off, we calculate the various fields as if the CRTC is on, then disable visibility. > > > > crtc_state->nv12_planes etc still get set, so it works as if the crtc is on. > > > > It's a way of not allowing an invalid result when dpms is off, then breaking on crtc enable. > > Hmm. I wonder when we started to do that. If we're already doing this > much then I wonder how far we are from just dealing with the FIXME in > intel_wm_plane_visible() instead? Still far I guess. Would potentially need to do some surgery on the pipe ddb allocation as well. This whole thing is a bit borked. We clear active_planes but still use it when allocating the Y plane. Hence dpms on could just fail anyway due to not having a free Y plane (as well as due to insufficient watermarks). So if we want to make the Y plane allocation robust I guess we would also need to move clearing the plane visibility to happen after the crtc .atomic_check().
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4e52dad84d64..e0e57de22388 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5060,15 +5060,15 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state, enum plane_id plane_id = to_intel_plane(plane_state->base.plane)->id; int ret; - /* Watermarks calculated in master */ - if (plane_state->slave) + /* Watermarks are calculated in master */ + if (plane_state->slave || + !intel_wm_plane_visible(crtc_state, plane_state)) return 0; if (plane_state->linked_plane) { const struct drm_framebuffer *fb = plane_state->base.fb; enum plane_id y_plane_id = plane_state->linked_plane->id; - WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)); WARN_ON(!fb->format->is_yuv || fb->format->num_planes == 1); @@ -5081,7 +5081,7 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state, plane_id, 1); if (ret) return ret; - } else if (intel_wm_plane_visible(crtc_state, plane_state)) { + } else { ret = skl_build_plane_wm_single(crtc_state, plane_state, plane_id, 0); if (ret)
When a planar YUV plane is configured, but the crtc is marked inactive, we can end up with a linked plane without visibility. Handle this by checking for visibility early, instead of doing a WARN. <4> [201.742919] ------------[ cut here ]------------ <4> [201.742920] WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)) <4> [201.742947] WARNING: CPU: 7 PID: 1268 at drivers/gpu/drm/i915/intel_pm.c:5068 skl_compute_wm+0x2be/0x10a0 [i915] <4> [201.742948] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 x86_pkg_temp_thermal snd_hda_intel coretemp snd_hda_codec mei_hdcp snd_hwdep snd_hda_core crct10dif_pclmul cdc_ether usbnet crc32_pclmul mii snd_pcm ghash_clmulni_intel mei_me mei prime_numbers <4> [201.742958] CPU: 7 PID: 1268 Comm: kms_chamelium Tainted: G U 5.2.0-rc3-CI-CI_DRM_6216+ #1 <4> [201.742960] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3183.A00.1905020411 05/02/2019 <4> [201.742978] RIP: 0010:skl_compute_wm+0x2be/0x10a0 [i915] <4> [201.742980] Code: 24 10 8b 92 fc 02 00 00 0f 85 ba 04 00 00 48 c7 c6 e0 38 2e a0 48 c7 c7 93 99 31 a0 89 54 24 20 48 89 44 24 08 e8 82 a2 f5 e0 <0f> 0b 8b 54 24 20 48 8b 44 24 08 48 8b 40 48 80 78 12 00 0f 85 76 <4> [201.742981] RSP: 0018:ffffc9000064f9a8 EFLAGS: 00010282 <4> [201.742983] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000007 <4> [201.742984] RDX: 000000000000175c RSI: ffff8884934d48e0 RDI: ffffffff8212df49 <4> [201.742985] RBP: ffff888493408558 R08: 00000000b56dab44 R09: 0000000000000000 <4> [201.742986] R10: ffff88848be00000 R11: 0000000000000000 R12: ffff88849afd89f8 <4> [201.742987] R13: ffff88847eaf67e8 R14: ffff88848c344a88 R15: ffff88848be00000 <4> [201.742988] FS: 00007f4d9b60b700(0000) GS:ffff88849ff80000(0000) knlGS:0000000000000000 <4> [201.742989] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 <4> [201.742991] CR2: 00007f4d9b609ff8 CR3: 000000048c326006 CR4: 0000000000760ee0 <4> [201.742992] PKRU: 55555554 <4> [201.742993] Call Trace: <4> [201.743021] ? intel_atomic_check+0x7b2/0x1440 [i915] <4> [201.743026] ? __mutex_unlock_slowpath+0x46/0x2b0 <4> [201.743052] intel_atomic_check+0x7ca/0x1440 [i915] <4> [201.743060] drm_atomic_check_only+0x55a/0x7f0 <4> [201.743064] drm_atomic_commit+0xe/0x50 <4> [201.743067] drm_atomic_connector_commit_dpms+0xe0/0xf0 <4> [201.743069] set_property_atomic+0xba/0x140 <4> [201.743075] drm_mode_obj_set_property_ioctl+0x111/0x1d0 <4> [201.743077] ? drm_dev_exit+0x8/0x40 <4> [201.743080] ? drm_connector_set_obj_prop+0x70/0x70 <4> [201.743082] drm_connector_property_set_ioctl+0x39/0x60 <4> [201.743084] drm_ioctl_kernel+0x83/0xf0 <4> [201.743087] drm_ioctl+0x2f3/0x3b0 <4> [201.743090] ? drm_connector_set_obj_prop+0x70/0x70 <4> [201.743096] ? lock_acquire+0xa6/0x1c0 <4> [201.743100] do_vfs_ioctl+0xa0/0x6e0 <4> [201.743103] ? __fget+0x10f/0x200 <4> [201.743105] ksys_ioctl+0x35/0x60 <4> [201.743108] __x64_sys_ioctl+0x11/0x20 <4> [201.743110] do_syscall_64+0x55/0x1c0 <4> [201.743112] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4> [201.743114] RIP: 0033:0x7f4da6c8d5d7 <4> [201.743115] Code: b3 66 90 48 8b 05 b1 48 2d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 81 48 2d 00 f7 d8 64 89 01 48 <4> [201.743116] RSP: 002b:00007f4d9b60aba8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 <4> [201.743118] RAX: ffffffffffffffda RBX: 00007f4d94001ac0 RCX: 00007f4da6c8d5d7 <4> [201.743119] RDX: 00007f4d9b60abe0 RSI: 00000000c01064ab RDI: 0000000000000005 <4> [201.743120] RBP: 00007f4d9b60abe0 R08: 00007f4d940015c0 R09: 00007f4d940015f0 <4> [201.743121] R10: 0000000000000055 R11: 0000000000000246 R12: 00000000c01064ab <4> [201.743122] R13: 0000000000000005 R14: 0000000000000005 R15: 00007f4da7a2c0c7 <4> [201.743156] irq event stamp: 362 <4> [201.743162] hardirqs last enabled at (361): [<ffffffff8112862c>] vprintk_emit+0xcc/0x340 <4> [201.743168] hardirqs last disabled at (362): [<ffffffff810019e0>] trace_hardirqs_off_thunk+0x1a/0x1c <4> [201.743174] softirqs last enabled at (0): [<ffffffff810abf78>] copy_process.part.6+0x4e8/0x1dc0 <4> [201.743178] softirqs last disabled at (0): [<0000000000000000>] 0x0 <4> [201.743243] WARNING: CPU: 7 PID: 1268 at drivers/gpu/drm/i915/intel_pm.c:5068 skl_compute_wm+0x2be/0x10a0 [i915] <4> [201.743246] ---[ end trace 33e6703087376efa ]--- Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110895 Testcase: kms_chamelium@hdmi-cmp-nv12 --- drivers/gpu/drm/i915/intel_pm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)