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

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

[Egbert Eich]

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(+)

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 */
-- 
2.7.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

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


-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for sna/cursor: Make sure hw cursors are disabled before disabling secondary planes

[Patchwork]

== Series Details ==

Series: sna/cursor: Make sure hw cursors are disabled before disabling secondary planes
URL   : https://patchwork.freedesktop.org/series/9009/
State : failure

== Summary ==

Applying: sna/cursor: Make sure hw cursors are disabled before disabling secondary planes
fatal: sha1 information is lacking or useless (src/sna/sna_display.c).
error: could not build fake ancestor
Patch failed at 0001 sna/cursor: Make sure hw cursors are disabled before disabling secondary planes
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Egbert Eich]

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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx