On Mon, Jul 2, 2012 at 8:35 PM, Dave Airlie <[email protected]> wrote: > On Mon, Jul 2, 2012 at 8:07 PM, Keith Packard <[email protected]> wrote: >> Dave Airlie <[email protected]> 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. _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
