Re: [PATCH 11/36] xfree86: add framework for provider support in ddx.

2012-07-03 Thread Dave Airlie 
On Mon, Jul 2, 2012 at 8:35 PM, Dave Airlie airl...@gmail.com wrote:
 On Mon, Jul 2, 2012 at 8:07 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@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.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH 11/36] xfree86: add framework for provider support in ddx.

2012-07-02 Thread Dave Airlie 
From: Dave Airlie airl...@redhat.com

This adds the framework for DDX provider support.

Signed-off-by: Dave Airlie airl...@redhat.com
---
 hw/xfree86/common/xf86str.h|4 +++
 hw/xfree86/modes/xf86Crtc.c|   46 ++
 hw/xfree86/modes/xf86Crtc.h|   58 ++
 hw/xfree86/modes/xf86RandR12.c |   60 
 4 files changed, 168 insertions(+)

diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
index 6bd6a62..56397f1 100644
--- a/hw/xfree86/common/xf86str.h
+++ b/hw/xfree86/common/xf86str.h
@@ -814,6 +814,10 @@ typedef struct _ScrnInfoRec {
 funcPointer reservedFuncs[NUM_RESERVED_FUNCS];
 
 Bool is_gpu;
+uint32_t roles;
+uint32_t abilities;
+uint32_t current_role;
+   
 } ScrnInfoRec;
 
 typedef struct {
diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index 2c8878f..238fb91 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -3202,3 +3202,49 @@ xf86_crtc_supports_gamma(ScrnInfoPtr pScrn)
 
 return FALSE;
 }
+
+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;
+
+if (name)
+len = strlen(name) + 1;
+else
+len = 0;
+
+provider = calloc(sizeof(xf86ProviderRec) + len, 1);
+if (!provider)
+return NULL;
+
+provider-scrn = scrn;
+provider-funcs = funcs;
+if (name) {
+provider-name = (char *) (provider + 1);
+strcpy(provider-name, name);
+}
+#ifdef RANDR_12_INTERFACE
+provider-randr_provider = NULL;
+#endif
+
+xf86_config-provider = provider;
+return provider;
+}
+
+void
+xf86SetCurrentRole(ScrnInfoPtr scrn, uint32_t role)
+{
+xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
+scrn-current_role = role;
+
+if (config-provider)
+if (config-provider-randr_provider)
+RRProviderSetCurrentRole(config-provider-randr_provider, 
scrn-current_role);
+}
+
diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index a6a3c2e..55466b6 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -50,6 +50,7 @@
 
 typedef struct _xf86Crtc xf86CrtcRec, *xf86CrtcPtr;
 typedef struct _xf86Output xf86OutputRec, *xf86OutputPtr;
+typedef struct _xf86Provider xf86ProviderRec, *xf86ProviderPtr;
 
 /* define a standard for connector types */
 typedef enum _xf86ConnectorType {
@@ -607,6 +608,55 @@ struct _xf86Output {
 INT16 initialBorder[4];
 };
 
+typedef struct _xf86ProviderFuncs {
+/**
+ * Called to allow the output a chance to create properties after the
+ * RandR objects have been created.
+ */
+void
+ (*create_resources) (xf86ProviderPtr provider);
+
+/**
+ * Callback when an provider's property has changed.
+ */
+Bool
+(*set_property) (xf86ProviderPtr provider,
+ Atom property, RRPropertyValuePtr value);
+
+/**
+ * Callback to get an updated property value
+ */
+Bool
+(*get_property) (xf86ProviderPtr provider, Atom property);
+
+/**
+ * Clean up driver-specific bits of the provider
+ */
+void
+ (*destroy) (xf86ProviderPtr provider);
+
+} xf86ProviderFuncsRec, *xf86ProviderFuncsPtr;
+
+struct _xf86Provider {
+/**
+ * ABI versioning
+ */
+int version;
+
+ScrnInfoPtr scrn;
+
+/** Provider name */
+char *name;
+
+/** provider-specific functions */
+const xf86ProviderFuncsRec *funcs;
+#ifdef RANDR_12_INTERFACE
+RRProviderPtr randr_provider;
+#else
+void *randr_provider;
+#endif
+};
+
 typedef struct _xf86CrtcConfigFuncs {
 /**
  * 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;
 
 extern _X_EXPORT int xf86CrtcConfigPrivateIndex;
@@ -975,4 +1026,11 @@ extern _X_EXPORT void
 extern _X_EXPORT Bool
  xf86_crtc_supports_gamma(ScrnInfoPtr pScrn);
 
+extern _X_EXPORT xf86ProviderPtr
+xf86ProviderCreate(ScrnInfoPtr scrn,
+   const xf86ProviderFuncsRec * funcs, const char *name);
+
+extern _X_EXPORT void
+xf86SetCurrentRole(ScrnInfoPtr scrn, uint32_t role);
+
 #endif  /* _XF86CRTC_H_ */
diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
index 59b6f82..78cad63 100644
--- a/hw/xfree86/modes/xf86RandR12.c
+++ b/hw/xfree86/modes/xf86RandR12.c
@@ -1552,6 +1552,19 @@ xf86RandR12CreateObjects12(ScreenPtr pScreen)
 output-funcs-create_resources(output);

Re: [PATCH 11/36] xfree86: add framework for provider support in ddx.

2012-07-02 Thread Keith Packard 
Dave Airlie airl...@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?

 +struct _xf86Provider {
 +/**
 + * ABI versioning
 + */
 +int version;
 +
 +ScrnInfoPtr scrn;
 +
 +/** Provider name */
 +char *name;
 +
 +/** provider-specific functions */
 +const xf86ProviderFuncsRec *funcs;
 +#ifdef RANDR_12_INTERFACE
 +RRProviderPtr randr_provider;
 +#else
 +void *randr_provider;
 +#endif
 +};
 +
  typedef struct _xf86CrtcConfigFuncs {
  /**
   * 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?

 @@ -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.

-- 
keith.pack...@intel.com


pgpFixA0LHsZU.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 11/36] xfree86: add framework for provider support in ddx.

2012-07-02 Thread Dave Airlie 
On Mon, Jul 2, 2012 at 8:07 PM, Keith Packard kei...@keithp.com wrote:
 Dave Airlie airl...@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.

Dave.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel