[PATCH 2/2] randr: add provider object (v3.1)

[Dave Airlie]

>>   /* Event selection bits */
>>   #define RRScreenChangeNotifyMask  (1L << 0)
>>   /* V1.2 additions */
>>   #define RRCrtcChangeNotifyMask     (1L << 1)
>>   #define RROutputChangeNotifyMask    (1L << 2)
>>   #define RROutputPropertyNotifyMask  (1L << 3)
>> +#define RRProviderPropertyNotifyMask        (1L << 4)
>
>
> Do we need a RRProviderChangeNotifyMask?  Can providers dynamically appear
> and disappear?  I assume they can due to things like USB hotplug.
>

Looks like it alright, will add in the next revision.

>> */
>> +    RRProvider provider B32;           /* affected output */
>
>
> s/output/provider/

fixed.

>
>
>> +    Atom atom B32;                     /* property name */
>> +    Time timestamp B32;                        /* time crtc was changed
>> */
>
>
> Time provider property was changed?

fixed.
>
> xRROutputPropertyNotifyEvent has a similar copy & paste error.

should fix separate upstream as well

>> +
>> + 2) DRI2 offload - For dual-GPU laptops, allow DRI2 applications to be
>> run
>> + on the second GPU and display on the first GPU.
>
>
> DRI2 seems like an implementation detail.  RandR clients shouldn't need to
> know how the server implements offloading.

yeah good point, I've modified the text.

>> +ROLEMASK { Master, Offload, Output }
>> +        List of roles a device can have.
>> +        Master: A primary display and primary renderer.
>> +        Offload: A DRI2 rendering offload device.
>
>
> s/DRI2//?

Done.

>
>
>> +        Output: An output device that can accept a shadow
>> +        pixmap to display as its scanout buffer.
>
>
> "Shadow" pixmaps are not defined anywhere as far as I can tell.  Maybe just
> remove the word from this description?  What restrictions are there on
> shadow pixmaps vs. normal pixmaps?

Done. yeah I cut that out complete, to just being an output slave device.

>> +┌───
>> +    RRGetProviders
>> +        window : WINDOW
>> +     ▶
>> +        timestamp: TIMESTAMP
>> +        providers: LISTofPROVIDER
>> +└───
>> +        Errors: Window
>> +
>> +        RRGetPRoviders returns the list of providers connected to the
>> screen
>> +       associated with 'window'.
>> +
>> +       'timestamp' indicates when the configuration was last set.
>> +
>> +       'providers' contains the list of PROVIDERs associated with the
>> +       screen.
>> +
>> +┌───
>> +    RRGetProviderInfo
>> +        provider: PROVIDER
>> +     ▶
>> +        abilities: ABILITYMASK
>> +        roles: ROLEMASK
>> +        current_role: ROLEMASK
>> +       name: STRING
>> +       crtcs: LISTofCRTC
>> +       outputs: LISTofOUTPUT
>> +└───
>> +       Errors: Window
>
>
> This seems surprising.  Shouldn't this throw Provider errors?

indeed.

>> +
>> +       'name' is a UTF-8 encoded strings to be presented to the user to
>
>
> s/strings/string/

Done.

>> +┌───
>> +    RRSetProviderRoles
>> +        providers: LISTofPROVIDER
>> +        role: LISTofROLEMASK
>> +     ▶
>> +└───
>> +        Errors: Window
>
>
> Errors: Provider?

indeed.
>> +
>> +┌───
>> +    RRListProviderProperties
>> +       provider:PROVIDERS
>> +      ▶
>> +       atoms: LISTof ATOM
>> +└───
>> +       Errors: Provider
>
>
> Doesn't this need to be defined in section 4 and in the protocol by bumping
> RRNumberErrors to 4 and adding a BadRRProvider = 3 define?

Totally didn't understand that area, thanks for clarifying it and
totally agree with it.

I'll send out another version in a while once I finished the edits.

Thanks for review,
Dave.