| Message ID | 1434100542-15301-5-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 12, 2015 at 11:15:42AM +0200, Maarten Lankhorst wrote: > The cursor should only be enabled if it's visible. This fixes > igt/kms_cursor_crc, which may otherwise produce the following > warning: > > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 intel_crtc_update_cursor+0x14c/0x4d0 [i915]() > Missing switch case (0) in i9xx_update_cursor > Modules linked in: i915 > CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: G W 4.1.0-rc7-patser+ #4079 > Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 > ffffffffc01aad10 ffff8800b083faa8 ffffffff817f7827 0000000080000001 > ffff8800b083faf8 ffff8800b083fae8 ffffffff81084955 ffff8800b083fad8 > ffff8800c4931148 0000000001200000 ffff8800c48b0000 0000000000000000 > Call Trace: > [<ffffffff817f7827>] dump_stack+0x4f/0x7b > [<ffffffff81084955>] warn_slowpath_common+0x85/0xc0 > [<ffffffff810849d1>] warn_slowpath_fmt+0x41/0x50 > [<ffffffffc0139f2c>] intel_crtc_update_cursor+0x14c/0x4d0 [i915] > [<ffffffffc01497f4>] __intel_set_mode+0x6c4/0x750 [i915] > [<ffffffffc0150143>] intel_crtc_set_config+0x473/0x5c0 [i915] > [<ffffffff81467da9>] drm_mode_set_config_internal+0x69/0x120 > [<ffffffff8146c1b9>] drm_mode_setcrtc+0x189/0x540 > [<ffffffff8145c7e0>] drm_ioctl+0x1a0/0x6a0 > [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50 > [<ffffffff811e9c28>] do_vfs_ioctl+0x2f8/0x530 > [<ffffffff810d0f7d>] ? trace_hardirqs_on+0xd/0x10 > [<ffffffff812e7746>] ? selinux_file_ioctl+0x56/0x100 > [<ffffffff811e9ee1>] SyS_ioctl+0x81/0xa0 > [<ffffffff81801617>] system_call_fastpath+0x12/0x6f > ---[ end trace abf0f71163290a96 ]--- > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 14ccf49b9067..afe91a8f7e36 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) > > intel_enable_primary_hw_plane(crtc->primary, crtc); > intel_enable_sprite_planes(crtc); > - intel_crtc_update_cursor(crtc, true); > + if (to_intel_plane_state(crtc->cursor->state)->visible) > + intel_crtc_update_cursor(crtc, true); Can we actually trust it now? Last time I looked we couldn't, and Daniel didn't want my fixes to make it so. So if we can't trust 'visible' I suppose you should just do something a bit more ugly and look at 'crtc_w' or something. > > intel_post_enable_primary(crtc); > > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 12-06-15 om 12:26 schreef Ville Syrjälä: > On Fri, Jun 12, 2015 at 11:15:42AM +0200, Maarten Lankhorst wrote: >> The cursor should only be enabled if it's visible. This fixes >> igt/kms_cursor_crc, which may otherwise produce the following >> warning: >> >> ------------[ cut here ]------------ >> WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 intel_crtc_update_cursor+0x14c/0x4d0 [i915]() >> Missing switch case (0) in i9xx_update_cursor >> Modules linked in: i915 >> CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: G W 4.1.0-rc7-patser+ #4079 >> Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 >> ffffffffc01aad10 ffff8800b083faa8 ffffffff817f7827 0000000080000001 >> ffff8800b083faf8 ffff8800b083fae8 ffffffff81084955 ffff8800b083fad8 >> ffff8800c4931148 0000000001200000 ffff8800c48b0000 0000000000000000 >> Call Trace: >> [<ffffffff817f7827>] dump_stack+0x4f/0x7b >> [<ffffffff81084955>] warn_slowpath_common+0x85/0xc0 >> [<ffffffff810849d1>] warn_slowpath_fmt+0x41/0x50 >> [<ffffffffc0139f2c>] intel_crtc_update_cursor+0x14c/0x4d0 [i915] >> [<ffffffffc01497f4>] __intel_set_mode+0x6c4/0x750 [i915] >> [<ffffffffc0150143>] intel_crtc_set_config+0x473/0x5c0 [i915] >> [<ffffffff81467da9>] drm_mode_set_config_internal+0x69/0x120 >> [<ffffffff8146c1b9>] drm_mode_setcrtc+0x189/0x540 >> [<ffffffff8145c7e0>] drm_ioctl+0x1a0/0x6a0 >> [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50 >> [<ffffffff811e9c28>] do_vfs_ioctl+0x2f8/0x530 >> [<ffffffff810d0f7d>] ? trace_hardirqs_on+0xd/0x10 >> [<ffffffff812e7746>] ? selinux_file_ioctl+0x56/0x100 >> [<ffffffff811e9ee1>] SyS_ioctl+0x81/0xa0 >> [<ffffffff81801617>] system_call_fastpath+0x12/0x6f >> ---[ end trace abf0f71163290a96 ]--- >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 14ccf49b9067..afe91a8f7e36 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) >> >> intel_enable_primary_hw_plane(crtc->primary, crtc); >> intel_enable_sprite_planes(crtc); >> - intel_crtc_update_cursor(crtc, true); >> + if (to_intel_plane_state(crtc->cursor->state)->visible) >> + intel_crtc_update_cursor(crtc, true); > Can we actually trust it now? Last time I looked we couldn't, and > Daniel didn't want my fixes to make it so. > > So if we can't trust 'visible' I suppose you should just do > something a bit more ugly and look at 'crtc_w' or something. > We add all planes during a modeset, so state->visible should be sane.
On Fri, Jun 12, 2015 at 12:32:24PM +0200, Maarten Lankhorst wrote: > Op 12-06-15 om 12:26 schreef Ville Syrjälä: > > On Fri, Jun 12, 2015 at 11:15:42AM +0200, Maarten Lankhorst wrote: > >> The cursor should only be enabled if it's visible. This fixes > >> igt/kms_cursor_crc, which may otherwise produce the following > >> warning: > >> > >> ------------[ cut here ]------------ > >> WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 intel_crtc_update_cursor+0x14c/0x4d0 [i915]() > >> Missing switch case (0) in i9xx_update_cursor > >> Modules linked in: i915 > >> CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: G W 4.1.0-rc7-patser+ #4079 > >> Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 > >> ffffffffc01aad10 ffff8800b083faa8 ffffffff817f7827 0000000080000001 > >> ffff8800b083faf8 ffff8800b083fae8 ffffffff81084955 ffff8800b083fad8 > >> ffff8800c4931148 0000000001200000 ffff8800c48b0000 0000000000000000 > >> Call Trace: > >> [<ffffffff817f7827>] dump_stack+0x4f/0x7b > >> [<ffffffff81084955>] warn_slowpath_common+0x85/0xc0 > >> [<ffffffff810849d1>] warn_slowpath_fmt+0x41/0x50 > >> [<ffffffffc0139f2c>] intel_crtc_update_cursor+0x14c/0x4d0 [i915] > >> [<ffffffffc01497f4>] __intel_set_mode+0x6c4/0x750 [i915] > >> [<ffffffffc0150143>] intel_crtc_set_config+0x473/0x5c0 [i915] > >> [<ffffffff81467da9>] drm_mode_set_config_internal+0x69/0x120 > >> [<ffffffff8146c1b9>] drm_mode_setcrtc+0x189/0x540 > >> [<ffffffff8145c7e0>] drm_ioctl+0x1a0/0x6a0 > >> [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50 > >> [<ffffffff811e9c28>] do_vfs_ioctl+0x2f8/0x530 > >> [<ffffffff810d0f7d>] ? trace_hardirqs_on+0xd/0x10 > >> [<ffffffff812e7746>] ? selinux_file_ioctl+0x56/0x100 > >> [<ffffffff811e9ee1>] SyS_ioctl+0x81/0xa0 > >> [<ffffffff81801617>] system_call_fastpath+0x12/0x6f > >> ---[ end trace abf0f71163290a96 ]--- > >> > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_display.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >> index 14ccf49b9067..afe91a8f7e36 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) > >> > >> intel_enable_primary_hw_plane(crtc->primary, crtc); > >> intel_enable_sprite_planes(crtc); > >> - intel_crtc_update_cursor(crtc, true); > >> + if (to_intel_plane_state(crtc->cursor->state)->visible) > >> + intel_crtc_update_cursor(crtc, true); > > Can we actually trust it now? Last time I looked we couldn't, and > > Daniel didn't want my fixes to make it so. > > > > So if we can't trust 'visible' I suppose you should just do > > something a bit more ugly and look at 'crtc_w' or something. > > > We add all planes during a modeset, so state->visible should be sane. If we don't already have it, I'd like to see a test case that does: - set a big mode - set up all kinds of planes near the bottom right corner - set a small mode This should test that all active planes get clipped properly during the modeset.
On Fri, Jun 12, 2015 at 02:49:50PM +0300, Ville Syrjälä wrote: > On Fri, Jun 12, 2015 at 12:32:24PM +0200, Maarten Lankhorst wrote: > > Op 12-06-15 om 12:26 schreef Ville Syrjälä: > > > On Fri, Jun 12, 2015 at 11:15:42AM +0200, Maarten Lankhorst wrote: > > >> The cursor should only be enabled if it's visible. This fixes > > >> igt/kms_cursor_crc, which may otherwise produce the following > > >> warning: > > >> > > >> ------------[ cut here ]------------ > > >> WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 intel_crtc_update_cursor+0x14c/0x4d0 [i915]() > > >> Missing switch case (0) in i9xx_update_cursor > > >> Modules linked in: i915 > > >> CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: G W 4.1.0-rc7-patser+ #4079 > > >> Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 > > >> ffffffffc01aad10 ffff8800b083faa8 ffffffff817f7827 0000000080000001 > > >> ffff8800b083faf8 ffff8800b083fae8 ffffffff81084955 ffff8800b083fad8 > > >> ffff8800c4931148 0000000001200000 ffff8800c48b0000 0000000000000000 > > >> Call Trace: > > >> [<ffffffff817f7827>] dump_stack+0x4f/0x7b > > >> [<ffffffff81084955>] warn_slowpath_common+0x85/0xc0 > > >> [<ffffffff810849d1>] warn_slowpath_fmt+0x41/0x50 > > >> [<ffffffffc0139f2c>] intel_crtc_update_cursor+0x14c/0x4d0 [i915] > > >> [<ffffffffc01497f4>] __intel_set_mode+0x6c4/0x750 [i915] > > >> [<ffffffffc0150143>] intel_crtc_set_config+0x473/0x5c0 [i915] > > >> [<ffffffff81467da9>] drm_mode_set_config_internal+0x69/0x120 > > >> [<ffffffff8146c1b9>] drm_mode_setcrtc+0x189/0x540 > > >> [<ffffffff8145c7e0>] drm_ioctl+0x1a0/0x6a0 > > >> [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50 > > >> [<ffffffff811e9c28>] do_vfs_ioctl+0x2f8/0x530 > > >> [<ffffffff810d0f7d>] ? trace_hardirqs_on+0xd/0x10 > > >> [<ffffffff812e7746>] ? selinux_file_ioctl+0x56/0x100 > > >> [<ffffffff811e9ee1>] SyS_ioctl+0x81/0xa0 > > >> [<ffffffff81801617>] system_call_fastpath+0x12/0x6f > > >> ---[ end trace abf0f71163290a96 ]--- > > >> > > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > >> --- > > >> drivers/gpu/drm/i915/intel_display.c | 3 ++- > > >> 1 file changed, 2 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > >> index 14ccf49b9067..afe91a8f7e36 100644 > > >> --- a/drivers/gpu/drm/i915/intel_display.c > > >> +++ b/drivers/gpu/drm/i915/intel_display.c > > >> @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) > > >> > > >> intel_enable_primary_hw_plane(crtc->primary, crtc); > > >> intel_enable_sprite_planes(crtc); > > >> - intel_crtc_update_cursor(crtc, true); > > >> + if (to_intel_plane_state(crtc->cursor->state)->visible) > > >> + intel_crtc_update_cursor(crtc, true); > > > Can we actually trust it now? Last time I looked we couldn't, and > > > Daniel didn't want my fixes to make it so. > > > > > > So if we can't trust 'visible' I suppose you should just do > > > something a bit more ugly and look at 'crtc_w' or something. > > > > > We add all planes during a modeset, so state->visible should be sane. > > If we don't already have it, I'd like to see a test case that does: > - set a big mode > - set up all kinds of planes near the bottom right corner > - set a small mode > > This should test that all active planes get clipped properly during the > modeset. Yeah sounds like a great idea. Adding Annie to make sure we don't forget to track this properly in Jira. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 14ccf49b9067..afe91a8f7e36 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) intel_enable_primary_hw_plane(crtc->primary, crtc); intel_enable_sprite_planes(crtc); - intel_crtc_update_cursor(crtc, true); + if (to_intel_plane_state(crtc->cursor->state)->visible) + intel_crtc_update_cursor(crtc, true); intel_post_enable_primary(crtc);
The cursor should only be enabled if it's visible. This fixes igt/kms_cursor_crc, which may otherwise produce the following warning: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 intel_crtc_update_cursor+0x14c/0x4d0 [i915]() Missing switch case (0) in i9xx_update_cursor Modules linked in: i915 CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: G W 4.1.0-rc7-patser+ #4079 Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 ffffffffc01aad10 ffff8800b083faa8 ffffffff817f7827 0000000080000001 ffff8800b083faf8 ffff8800b083fae8 ffffffff81084955 ffff8800b083fad8 ffff8800c4931148 0000000001200000 ffff8800c48b0000 0000000000000000 Call Trace: [<ffffffff817f7827>] dump_stack+0x4f/0x7b [<ffffffff81084955>] warn_slowpath_common+0x85/0xc0 [<ffffffff810849d1>] warn_slowpath_fmt+0x41/0x50 [<ffffffffc0139f2c>] intel_crtc_update_cursor+0x14c/0x4d0 [i915] [<ffffffffc01497f4>] __intel_set_mode+0x6c4/0x750 [i915] [<ffffffffc0150143>] intel_crtc_set_config+0x473/0x5c0 [i915] [<ffffffff81467da9>] drm_mode_set_config_internal+0x69/0x120 [<ffffffff8146c1b9>] drm_mode_setcrtc+0x189/0x540 [<ffffffff8145c7e0>] drm_ioctl+0x1a0/0x6a0 [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50 [<ffffffff811e9c28>] do_vfs_ioctl+0x2f8/0x530 [<ffffffff810d0f7d>] ? trace_hardirqs_on+0xd/0x10 [<ffffffff812e7746>] ? selinux_file_ioctl+0x56/0x100 [<ffffffff811e9ee1>] SyS_ioctl+0x81/0xa0 [<ffffffff81801617>] system_call_fastpath+0x12/0x6f ---[ end trace abf0f71163290a96 ]--- Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)