On 05.07.2016 22:15, Michael Thayer wrote:
On 05.07.2016 20:40, Adam Jackson wrote:
On Mon, 2016-07-04 at 21:43 +0200, Michael Thayer wrote:
When submitting dirty rectangles to the kernel driver, modesetting
checks
the return value, and if it gets ENOSYS (driver does not support
reporting)
or EINVAL (invalid data submitted to the kernel driver) it disables
reporting
for the rest of the session. The second is clearly wrong, and has
been seen
to trigger in practice when the X server submits more rectangles at
once to
the VirtualBox kernel driver than the kernel will accept. I would
expect
this too affect most or all other drivers for virtual graphics
devices and
some others.
The explanation makes sense, but I'm not sure the patch does. I'm not
especially familiar with this code, but it seems like pretending that
submitting dirty rects succeeded when it didn't is just hoping that
some later update will compensate. What about something like:
---
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -515,6 +515,17 @@ dispatch_dirty_region(ScrnInfoPtr scrn,
/* TODO query connector property to see if this is needed */
ret = drmModeDirtyFB(ms->fd, fb_id, clip, num_cliprects);
+
+ /* if we're swamping it with work, try one at a time */
+ if (ret == -EINVAL) {
+ for (i = 0; i < num_cliprects; i++) {
+ if (drmModeDirtyFB(ms->fd, fb_id, &clip[i], 1) ==
-EINVAL)
+ break;
+ }
+ if (i == num_cliprects)
+ ret = 0;
+ }
+
free(clip);
DamageEmpty(damage);
}
That certainly looks sensible to me. I must admit that the main reason
that I did not do something more elaborate was that I did not really
have time to test more than the simple patch I sent (actually I got one
of the users experiencing the problem to test it, which increased the
loop length). So I decided that my patch was already an improvement -
missing one dirty update versus missing all updates for the rest of the
server life-time.
If you are alright with your updated patch I would be happy to give it a
reviewed-by.
I asked the user to test your patch, so let's see.
Regards and thanks,
Michael
---
- ajax
--
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
