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

[Michael Thayer]

On 10.07.2016 12:23, Michael Thayer wrote:
> 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 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?

I know it has just been two days, but still, a polite ping.  I would 
also generally be interested in the question of what I can do to get 
this sort of patch moving.  Would reviewing other people's patches be 
the right way to go, or creating a tree on fd.o?  Or something else?

Regards and thanks,

Michael

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