[PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge

[Daniel Vetter]

On Wed, Oct 29, 2014 at 10:14:37AM +0100, Thierry Reding wrote:
> On Wed, Oct 29, 2014 at 09:57:02AM +0100, Daniel Vetter wrote:
> > That makes the entire thing a bit non-trivial, which is why I think it
> > would be better as some generic helper. Which then gets embedded or
> > instantiated for specific cases, like dt&drm_panel or dt&drm_bridge.
> > Or maybe even acpi&drm_bridge, who knows ;-)
> 
> I worry a little about type safety. How will this generic helper know
> what registry to look in for? Or conversely, if all these resources are
> added to a single registry how do you know that they're of the correct
> type? Failing to ensure this could cause situations where you're asking
> for a panel and get a bridge in return because you've wrongly wired it
> up in device tree for example.
> 
> But perhaps if both the registry and the device parts are turned into
> helpers we could still have a single core implementation and then
> instantiate that for each type, something roughly like this:
> 
> 	struct registry {
> 		struct list_head list;
> 		struct mutex lock;
> 	};
> 
> 	struct registry_record {
> 		struct list_head list;
> 		struct module *owner;
> 		struct kref *ref;
> 
> 		struct device *dev;
> 	};
> 
> 	int registry_add(struct registry *registry, struct registry_record *record)
> 	{
> 		...
> 		try_module_get(record->owner);
> 		...
> 	}
> 
> 	struct registry_record *registry_find_by_of_node(struct registry *registry,
> 							 struct device_node *np)
> 	{
> 		...
> 		kref_get(...);
> 		...
> 	}
> 
> That way it should be possible to embed these into other structures,
> like so:
> 
> 	struct drm_panel {
> 		struct registry_record record;
> 		...
> 	};
> 
> 	static struct registry drm_panels;
> 
> 	int drm_panel_add(struct drm_panel *panel)
> 	{
> 		return registry_add(&drm_panels, &panel->record);
> 	}
> 
> 	struct drm_panel *of_drm_panel_find(struct device_node *np)
> 	{
> 		struct registry_record *record;
> 
> 		record = registry_find_by_of_node(&drm_panels, np);
> 
> 		return container_of(record, struct drm_panel, record);
> 	}
> 
> Is that what you had in mind?

Yeah I've thought that we should instantiate using macros even, so that we
have per-type registries. So you'd smash the usual set of
DECLARE/DEFINE_AUX_DEV_REGISTRY into headers/source files, and they'd take
a (name, key, value) tripled. For the example here(of_drm_panel, struct
device_node *, struct drm_panel *) or similar. I might be hand-waving over
a few too many details though ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch