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

[Michael Thayer]

Hello Adam,

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

I got one of the affected users to (successfully) test this patch.  On 
X.Org Server 1.17.4, but I assume that is good enough.  Would you be 
able to re-submit it with a "signed off by" so that I can review it?  Or 
can I review it as it stands?

> ---
>
> - ajax
>

Regards,

Michael
-- 
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