diff mbox

[driver/intel] sna/cursor: Make sure hw cursors are disabled before disabling secondary planes

Message ID 1466530474-11587-1-git-send-email-eich@freedesktop.org (mailing list archive)
State New, archived
Headers show

Commit Message

Egbert Eich June 21, 2016, 5:34 p.m. UTC
When the hw cursors are not disabled before the cursor planes get disabled
we may lose the cursor later on. Thus make sure the cursors are disabled
before the cursor planes are.

Signed-off-by: Egbert Eich <eich@freedesktop.org>
---
 src/sna/sna_display.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Chris Wilson June 21, 2016, 8:25 p.m. UTC | #1
On Tue, Jun 21, 2016 at 07:34:34PM +0200, Egbert Eich wrote:
> When the hw cursors are not disabled before the cursor planes get disabled
> we may lose the cursor later on. Thus make sure the cursors are disabled
> before the cursor planes are.

The cursor would already be controlled by the xf86SetDesiredModes(), so
we can skip disabling entirely. What we should do instead is add the
paranoia check, but I can't see an easy way to inquire what the kernel
thinks the legacy cursor handle should be.

commit f1c757e4518f6835bbff6c940269a5c6be75f202
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Jun 21 21:17:15 2016 +0100

    sna: Only shutdown unknown secondary planes on CRTC we control
    
    In a ZaphodHead scenario, we do not own all the CRTC and so we should
    not be making changes outside of our zone of control. Also, we only want
    to disable secondary overlay planes and ignore the secondary cursor
    planes which are controlled through the normal modesetting.
    
    As we are now tracking all sprite planes on a CRTC, this leads to much
    simpler code.
    
    Reported-by: Egbert Eich <eich@freedesktop.org>
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Egbert Eich July 4, 2016, 11:55 a.m. UTC | #2
On Tue, Jun 21, 2016 at 09:25:36PM +0100, Chris Wilson wrote:
> On Tue, Jun 21, 2016 at 07:34:34PM +0200, Egbert Eich wrote:
> > When the hw cursors are not disabled before the cursor planes get disabled
> > we may lose the cursor later on. Thus make sure the cursors are disabled
> > before the cursor planes are.
> 
> The cursor would already be controlled by the xf86SetDesiredModes(), so
> we can skip disabling entirely. What we should do instead is add the
> paranoia check, but I can't see an easy way to inquire what the kernel
> thinks the legacy cursor handle should be.
> 
> commit f1c757e4518f6835bbff6c940269a5c6be75f202
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue Jun 21 21:17:15 2016 +0100
> 
>     sna: Only shutdown unknown secondary planes on CRTC we control
>     
>     In a ZaphodHead scenario, we do not own all the CRTC and so we should
>     not be making changes outside of our zone of control. Also, we only want
>     to disable secondary overlay planes and ignore the secondary cursor
>     planes which are controlled through the normal modesetting.
>     
>     As we are now tracking all sprite planes on a CRTC, this leads to much
>     simpler code.

Chris, thanks for the patch! I've been able to test it now - it works.

Cheers,
	Egbert.
diff mbox

Patch

diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index d790975..412c192 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -8081,6 +8081,9 @@  void sna_mode_check(struct sna *sna)
 	if (sna->mode.hidden)
 		return;
 
+        /* make sure the hw cursors are disabled before disabling
+	   the secondary planes which include the cursor plane */
+	sna_hide_cursors(sna->scrn);
 	disabled = sna_mode_disable_secondary_planes(sna);
 
 	/* Validate CRTC attachments and force consistency upon the kernel */