Hi,
On 23-09-16 10:20, Hans de Goede wrote:
Hi All,
On 09/16/2016 06:52 PM, Michael Thayer wrote:
When the X server asks us to load a hardware cursor, that request is always
followed up by a request to show it if we report success, or to hide it if
we report failure. Therefore it makes no sense to suppress the request if
the cursor is not currently visible.
Ok, I've just spend the last half hour tracing (using grep) all callers of
load_cursor_argb_check and you're right, either the cursor is already
visible (XRecolorCursor does this), or it will get shown directly after
the drmmode_load_cursor_argb_check() call.
And despite double checking I still missed a trouble-some scenario here.
If we've 2 monitors active, then as the cursor moves from one to
the other show()/hide() gets called on them.
If the cursor then changes load_cursor_argb_check() gets called on
all active crtc-s causing the cursor to now be shown on both monitors
at the same time, not good.
So this patch is:
Reviewed-by: Hans de Goede <[email protected]>
So I hereby withdraw my Reviewed-by, and have to NACK this in its
current form as it is causing regressions in multi-monitor
setups.
I'll send a v2 my pull-req with fixes for 1.19, dropping this one
and related follow-up patches.
Regards,
Hans
Signed-off-by: Michael Thayer <[email protected]>
---
Tested that both hardware and software cursors still work after applying the
patch.
hw/xfree86/drivers/modesetting/drmmode_display.c | 11 +----------
hw/xfree86/drivers/modesetting/drmmode_display.h | 2 --
2 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 41810d9..7d171a3 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -813,14 +813,7 @@ drmmode_load_cursor_argb_check(xf86CrtcPtr crtc, CARD32
*image)
for (i = 0; i < ms->cursor_width * ms->cursor_height; i++)
ptr[i] = image[i]; // cpu_to_le32(image[i]);
- if (drmmode_crtc->cursor_up || !drmmode_crtc->first_cursor_load_done) {
- Bool ret = drmmode_set_cursor(crtc);
- if (!drmmode_crtc->cursor_up)
- drmmode_hide_cursor(crtc);
- drmmode_crtc->first_cursor_load_done = TRUE;
- return ret;
- }
- return TRUE;
+ return drmmode_set_cursor(crtc);
}
static void
@@ -830,7 +823,6 @@ drmmode_hide_cursor(xf86CrtcPtr crtc)
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
drmmode_ptr drmmode = drmmode_crtc->drmmode;
- drmmode_crtc->cursor_up = FALSE;
drmModeSetCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, 0,
ms->cursor_width, ms->cursor_height);
}
@@ -839,7 +831,6 @@ static void
drmmode_show_cursor(xf86CrtcPtr crtc)
{
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
- drmmode_crtc->cursor_up = TRUE;
drmmode_set_cursor(crtc);
}
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h
b/hw/xfree86/drivers/modesetting/drmmode_display.h
index 50976b8..bd82968 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.h
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
@@ -91,9 +91,7 @@ typedef struct {
uint32_t vblank_pipe;
int dpms_mode;
struct dumb_bo *cursor_bo;
- Bool cursor_up;
Bool set_cursor2_failed;
- Bool first_cursor_load_done;
uint16_t lut_r[256], lut_g[256], lut_b[256];
drmmode_bo rotate_bo;
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
