Re: [PATCH 4/4] drm/i915: Only enable cursor if it can be enabled.

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/4] drm/i915: Only enable cursor if it can be enabled.
Date: Fri, 12 Jun 2015 14:49:50 +0300	[thread overview]
Message-ID: <20150612114950.GA5176@intel.com> (raw)
In-Reply-To: <557AB538.7070505@linux.intel.com>

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.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx