[PATCH 11/36] xfree86: add framework for provider support in ddx.
Dave Airlie
airlied at gmail.com
Tue Jul 3 04:27:18 PDT 2012
On Mon, Jul 2, 2012 at 8:35 PM, Dave Airlie <airlied at gmail.com> wrote:
> On Mon, Jul 2, 2012 at 8:07 PM, Keith Packard <keithp at keithp.com> wrote:
>> Dave Airlie <airlied at gmail.com> writes:
>>
>>> +xf86ProviderPtr
>>> +xf86ProviderCreate(ScrnInfoPtr scrn,
>>> + const xf86ProviderFuncsRec *funcs, const char *name)
>>> +{
>>> + xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
>>> + xf86ProviderPtr provider;
>>> + int len;
>>> +
>>> + if (xf86_config->provider)
>>> + return xf86_config->provider;
>>
>> A 'create' function shouldn't return an existing object; that's more
>> like a 'get' function. Do you expect callers to not know if the object
>> exists yet?
>
> I suppose it shouldn't happen, I was just being careful, we should
> only have one provider object per device, maybe I should just make
> that an assert.
>>> * Requests that the driver resize the screen.
>>> @@ -681,6 +731,7 @@ typedef struct _xf86CrtcConfig {
>>> /* callback when crtc configuration changes */
>>> xf86_crtc_notify_proc_ptr xf86_crtc_notify;
>>>
>>> + xf86ProviderPtr provider;
>>> } xf86CrtcConfigRec, *xf86CrtcConfigPtr;
>>
>> Could the elements of 'provider' just become members of the crtc config rec?
>
> will check that tomorrow, probably could do alright, but I just liked
> keeping things in an object.
>>
>>> @@ -1552,6 +1552,19 @@ xf86RandR12CreateObjects12(ScreenPtr pScreen)
>>> output->funcs->create_resources(output);
>>> RRPostPendingProperties(output->randr_output);
>>> }
>>> +
>>> + {
>>> + xf86ProviderPtr provider = config->provider;
>>> + provider->randr_provider = RRProviderCreate(pScreen, provider->name,
>>> + strlen(provider->name), provider);
>>> +
>>> + if (provider->scrn->is_gpu) {
>>> + RRProviderSetRolesAbilities(provider->randr_provider, provider->scrn->roles,
>>> + provider->scrn->abilities);
>>> + RRProviderSetCurrentRole(provider->randr_provider, provider->scrn->current_role);
>>> + }
>>> + }
>>> +
>>
>> Ok, I'm lost here -- this assumes that config->provider exists, and yet
>> xf86ProviderCreate isn't being called with some default values in case
>> the driver doesn't do that yet.
>
> Yeah I should check here in case drivers don't create a provider object alright.
I could move the provider stuff into the toplevel struct alright,
though that would mean
a one provider per GPU model which is what I think is correct.
I suppose I need to review the randr protocol bits first before I decide.
Dave.
More information about the xorg-devel
mailing list