On Mon, Oct 27, 2014 at 11:26:30PM +0100, Daniel Vetter wrote:
On Mon, Oct 27, 2014 at 11:20 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Mon, Oct 27, 2014 at 8:58 PM, Sean Paul seanpaul@chromium.org wrote:
@@ -660,8 +662,11 @@ struct drm_bridge_funcs {
- @driver_private: pointer to the bridge driver's internal context
*/ struct drm_bridge {
struct drm_device *dev;
struct device *dev;Please don't rename the ->dev pointer into drm. Because _all_ the other drm structures still call it ->dev. Also, can't we use struct device_node here like we do in the of helpers Russell added? See 7e435aad38083
I think this is modeled after the naming in drm_panel, FWIW. However, seems reasonable to keep the device_node instead.
Hm, indeed. Tbh I vote to rename drm_panel->drm to ->dev and like with drm_crtc drop the struct device and go directly to a struct device_node. Since we don't really need the sturct device, the only thing we care about is the of_node. For added bonus wrap an #ifdef CONFIG_OF around all the various struct device_node in drm_foo.h. Should be all fairly simple to pull off with cocci.
Thierry?
Looking at the of_drm_find_panel function I actually wonder how that works - the drm_panel doesn't really need to stick around afaics. After all panel_list is global so some other driver can unload. Russell's of support for possible_crtcs code works differently since it only looks at per-drm_device lists.
I don't understand. Panels are global resources that get registered by some driver that isn't tied to the DRM device until attached. It can't be in a per-DRM device list, because it's external to the device.
And yes, they can go away when a driver is unloaded, though it's not the typical use-case.
This bridge code here though suffers from the same. So to me this looks rather fishy.
Well, this version of the DRM bridge support was written to be close to DRM panel, so it isn't really surprising that it's similar =), but like I said, I don't really understand what you think is wrong with it.
It doesn't help that we have drm_of.[hc] around but not all the of code is in there. Adding Russell too.
DRM panel and DRM bridge aren't just OF helpers. They can be used with whatever type of device you want.
Thierry