[PATCH weston 3/8] compositor-drm: Don’t try to set the cursor anymore once we know it’s broken

[Pekka Paalanen]

On Mon,  2 May 2016 22:40:12 +0100
Emmanuel Gil Peyrot <emmanuel.peyrot at collabora.com> wrote:

> Signed-off-by: Emmanuel Gil Peyrot <emmanuel.peyrot at collabora.com>
> ---
>  src/compositor-drm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index ea118fa..c11562f 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -671,7 +671,8 @@ drm_output_repaint(struct weston_output *output_base,
>  
>  	output->page_flip_pending = 1;
>  
> -	drm_output_set_cursor(output);
> +	if (!backend->cursors_are_broken)
> +		drm_output_set_cursor(output);
>  
>  	/*
>  	 * Now, update all the sprite surfaces

Hi,

we talked about this in irc.

This is an easy-looking change, but there are subtle things hiding
behind that raised my eyebrow a bit.

drm_output_prepare_cursor_view() should already be bailing out, for
cursors_are_broken if nothing else. If that happens, then this patch
would only avoid unsetting the cursor on every frame. I can very well
understand the desire to not repeatedly unset the cursor, however you
told me unsetting was not the case.

Btw. even the unsetting case might be prone to a couple corner-cases:

- VT-switching in, you better hammer in all KMS state again. Even if
  cursors are broken for us, maybe something else managed to set one.
  We don't want to leave a stale cursor on screen.

- According to [1], some virtual hardware might be sometimes able to
  handle cursors and other times not. Or, maybe there are some other
  unknown limitations on when cursor can be used or not, but anyway,
  I'd like to guarantee there won't be a stale cursor on screen.

Funnily enough, the DRM-backend is already never going back from
cursors_are_broken when once set.

The current intended behaviour in Weston is to try to use the HW
cursor, and set cursors_are_broken in drm_output_set_cursor() the first
time it fails. That makes prepare_cursor_view() always bail out, so no
cursor is attempted again, though the cursor is unset (mostly
redundantly) on every frame.

Because it is unclear whether this patch is actually necessary, and we
could not figure it out in irc, and there is some remote possibility
for a rare regression, I'd leave this out for now.


Thanks,
pq

[1] https://lists.x.org/archives/xorg-devel/2016-March/048960.html

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160517/f0dc9767/attachment.sig>