Hello,
On 27.09.2016 12:06, Hans de Goede wrote:
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.
Indeed. Looking at other ways of handling this, but the whole thing is
somewhat tricky due to the differences in how the X server and the
kernel handle cursor setting. I think that the most correct way to fix
this would be to allow show and hide cursor operations to return failure
too, but that would be a very invasive change. A less invasive but also
less clean change would be to extend the first time hack to also trigger
if a load happens when the cursor is hidden globally (or perhaps better,
if it is hidden on all crtc-s at the time of the load). For VirtualBox,
if a cursor load succeeds on one crtc we know it will succeed on any,
but I wonder whether that would apply to other users. I also wonder
which other potential users there are. So far most other people use
their own DDX instead of modesetting. Alexandre, did you say this was
an issue for Tegra? And it is potentially interesting for Qemu for the
same reasons that it is for us (relative input device support).
Hans, any thoughts? Probably better not to try to force anything into 1.19.
Regards and thanks,
Michael
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;
--
Michael Thayer | VirtualBox engineer
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt
ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstraße 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603
Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister
der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
