[PATCH v2 3/8] randr: Add ability to turn PRIME sync off

[Alex Goins]

Hey Adam,

I recently got around to implementing this when putting the v3 patch set
together (with some changes of my own,) and I'm finding that it harms the user
experience significantly.

It's sort of annoying that you currently have to select from either 0 or <XID>
instead of 0 or 1, but more importantly, xrandr uses hex to print provider XIDs
and decimal to print/set output properties.

It could probably be resolved with an xrandr patch, but as it stands, you have
to look up the provider source XID with xrandr --listproviders, convert it to
decimal, and then set it.

I think that it would make more sense to hide this from the user with some sort
of private data attached to output properties, rather than using the actual
property value to store the XID. Having the user set the XID to turn PRIME
Synchronization on and off would probably be redundant with the setup to link
the output and source provider.

Would you object to simply including a /* TODO */ for making
RR(Init/Fini)PrimeSyncProps correctly handle different sources for different
outputs, if and when that is implemented?

Thanks,
Alex

On Sat, 16 Jan 2016, Alex Goins wrote:

> Thanks Adam.
> 
> > > +static void
> > > +RRFiniPrimeSyncProps(ScreenPtr pScreen)
> > > +{
> > > +    rrScrPrivPtr pScrPriv = rrGetScrPriv(pScreen);
> > > +    int i;
> > > +
> > > +    const char *syncStr = PRIME_SYNC_PROP;
> > > +    Atom syncProp = MakeAtom(syncStr, strlen(syncStr), FALSE);
> > > +    if (syncProp == None)
> > > +        return;
> > > +
> > > +    for (i = 0; i < pScrPriv->numOutputs; i++) {
> > > +        RRDeleteOutputProperty(pScrPriv->outputs[i], syncProp);
> > > +    }
> > > +}
> > 
> > This is sort of academic, but you're tearing down the output prop for
> > every output, which isn't really correct if we ever had different
> > sources for different outputs.  That's not really an issue since the
> > old per-crtc scanout pixmap work never landed [1], so we still have the
> > notion of _the_ screen pixmap.
> > 
> > But I suppose if we ever _did_ try to make that go, you could set the
> > property value to the XID of the source you wanted, and for
> > compatibility TRUE would mean "pick one".  And at that point we'd have
> > enough info to walk backwards to detach only the right properties at
> > provider destroy.
> 
> Good point. I will take this into account in the next revision, to avoid
> introducing a potential future bug.
> 
> Thanks,
> Alex