[PATCH 2/2] randr: add provider object (v3.1)
Dave Airlie
airlied at gmail.com
Thu Jun 28 02:36:41 PDT 2012
>> /* 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.
More information about the xorg-devel
mailing list