diff mbox

[xf86-video-intel,8/8] sna/video/sprite: Try disabling plane before giving up on colorkey

Message ID 20180529183315.9823-8-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä May 29, 2018, 6:33 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When we're trying to reinstate the colorkey we might fail on account of
the plane still being enable with a configuration that prevent the
use of colorkey. This happens easily with NV12 since the plane scaler
required by even unscaled NV12 is not compatible with colorkey.

To work around the problem let's try disabling the plane first, then
re-enable the colorkey, and finally we will try to re-enable the plane.
The plane re-enable may fail, in which case we'll head to the GPU
scaling fallback path. The cost is a flash of the colorkey when the
plane blink off and then back on.

Help me atomic ioctl, you're my only hope!

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/sna/sna_video_sprite.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Chris Wilson May 31, 2018, 9:30 a.m. UTC | #1 
Quoting Ville Syrjala (2018-05-29 19:33:15)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When we're trying to reinstate the colorkey we might fail on account of
> the plane still being enable with a configuration that prevent the
> use of colorkey. This happens easily with NV12 since the plane scaler
> required by even unscaled NV12 is not compatible with colorkey.
> 
> To work around the problem let's try disabling the plane first, then
> re-enable the colorkey, and finally we will try to re-enable the plane.
> The plane re-enable may fail, in which case we'll head to the GPU
> scaling fallback path. The cost is a flash of the colorkey when the
> plane blink off and then back on.
> 
> Help me atomic ioctl, you're my only hope!
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Though I glazed over at the asm patches, I concur that merely adding
further crud is the easiest way to add colorspace encoding support. The
rest of the patches looked fine, so the series is

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks,
-Chris
Chris Wilson May 31, 2018, 7:36 p.m. UTC | #2 
Quoting Chris Wilson (2018-05-31 10:30:13)
> Quoting Ville Syrjala (2018-05-29 19:33:15)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > When we're trying to reinstate the colorkey we might fail on account of
> > the plane still being enable with a configuration that prevent the
> > use of colorkey. This happens easily with NV12 since the plane scaler
> > required by even unscaled NV12 is not compatible with colorkey.
> > 
> > To work around the problem let's try disabling the plane first, then
> > re-enable the colorkey, and finally we will try to re-enable the plane.
> > The plane re-enable may fail, in which case we'll head to the GPU
> > scaling fallback path. The cost is a flash of the colorkey when the
> > plane blink off and then back on.
> > 
> > Help me atomic ioctl, you're my only hope!
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Though I glazed over at the asm patches, I concur that merely adding
> further crud is the easiest way to add colorspace encoding support. The
> rest of the patches looked fine, so the series is
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Thanks,

And pushed, double thanks.
-Chris
diff mbox

Patch

diff --git a/src/sna/sna_video_sprite.c b/src/sna/sna_video_sprite.c
index 0f52f0328fc0..f713abcb9151 100644
--- a/src/sna/sna_video_sprite.c
+++ b/src/sna/sna_video_sprite.c
@@ -270,9 +270,19 @@  sna_video_sprite_show(struct sna *sna,
 		if (drmIoctl(sna->kgem.fd,
 			     LOCAL_IOCTL_I915_SET_SPRITE_COLORKEY,
 			     &set)) {
-			xf86DrvMsg(sna->scrn->scrnIndex, X_ERROR,
-				   "failed to update color key, disabling future updates\n");
-			video->has_color_key = false;
+			memset(&s, 0, sizeof(s));
+			s.plane_id = sna_crtc_to_sprite(crtc, video->idx);
+
+			/* try to disable the plane first */
+			if (drmIoctl(video->sna->kgem.fd, LOCAL_IOCTL_MODE_SETPLANE, &s))
+				xf86DrvMsg(video->sna->scrn->scrnIndex, X_ERROR,
+					   "failed to disable plane\n");
+
+			if (drmIoctl(sna->kgem.fd, LOCAL_IOCTL_I915_SET_SPRITE_COLORKEY, &set)) {
+				xf86DrvMsg(sna->scrn->scrnIndex, X_ERROR,
+					   "failed to update color key, disabling future updates\n");
+				video->has_color_key = false;
+			}
 		}
 
 		video->color_key_changed &= ~(1 << pipe);