[PATCH xserver] modesetting: do not disable dirty rectangles on EINVAL.

[Adam Jackson]

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);
     }
---

- ajax