Dave Airlie <[email protected]> writes:

> +struct _rrProvider {
> +    RRProvider id;
> +    ScreenPtr pScreen; /* gpu screen more than likely */
> +    int current_role;

uint32_t current_role

> diff --git a/randr/rrprovider.c b/randr/rrprovider.c
> new file mode 100644
> index 0000000..549db5d
> --- /dev/null
> +++ b/randr/rrprovider.c
> @@ -0,0 +1,338 @@
> +/*
> + * Copyright © 2006 Keith Packard

Please fix this.


> +#define ADD_PROVIDER(iter) do {                                 \
> +    pScrPriv = rrGetScrPriv((iter));                            \
> +    if (pScrPriv->provider) {                                   \
> +        providers[count_providers] = pScrPriv->provider->id;    \
> +        if (client->swapped)                                    \
> +            swapl(&providers[count_providers]);                 \
> +        count_providers++;                                      \
> +    }                                                           \
> +    } while(0)

'iter' seems like a poor name choice - it's really 'pScreen'. And, I
assume you'll be re-using this macro in a future patch, because,
otherwise, ick.

> +    if (rep.allowed_roles & RR_Role_Slave_Output) {
> +        rep.nCrtcs = pScrPriv->numCrtcs;
> +        rep.nOutputs = pScrPriv->numOutputs;
...
> +    if (rep.allowed_roles & RR_Role_Slave_Output) {
> +        for (i = 0; i < pScrPriv->numCrtcs; i++) {

Do masters not get crtcs and outputs in this list?


> +    for (i = 0 ; i < stuff->numProviders; i++)
> +    {
> +        VERIFY_RR_PROVIDER(providers[i], provider, DixReadAccess);
> +
> +        pScreen = provider->pScreen;
> +        pScrPriv = rrGetScrPriv(pScreen);
> +
> +        pScrPriv->rrProviderSetRole(pScreen, provider, roles[i]);

I think this needs to be an atomic function so that the driver can swap
roles all at once. Otherwise, I don't know how you'd switch the LVDS
connection in your laptop from one GPU to the other, without going
through an intermediate black state.

> diff --git a/randr/rrproviderproperty.c b/randr/rrproviderproperty.c

I assume this is essentially cut&paste from the other randr property
code, so I'm not going to waste time looking at it...

-- 
[email protected]

Attachment: pgpN0XvvlxS4d.pgp
Description: PGP signature

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to