On Wed, Dec 05, 2012 at 01:47:38PM +0200, Terje Bergström wrote: > On 05.12.2012 13:13, Thierry Reding wrote: [...] > > Oh well, at the time nobody from NVIDIA was involved so I wrote that > > code in preparation for proper host1x support that I thought I would > > have to add myself at some point. I'm more than glad that I don't have > > to do this all by myself. However the patch proposed in this series > > breaks a number of requirements such as proper encapsulation, which I > > already mentioned in more detail in another mail. > > Hmm, I'm not sure if I remember that you refer to by the proper > encapsulation. Is that the fact that we bind DRM to a sub-client? Yes, but there's more. For instance I went to great lengths to make sure there's no global data whatsoever. So all the data is bound to the host1x device in the current code. I know many other drivers like to take a shortcut and just put these things into global variables but I didn't want to. > > The problem that this solves is that the DRM driver needs to be bound to > > a specific platform device. None of the DRM subdevices are suitable > > because they are only part of the whole DRM device. I think that host1x > > is the only device that fits here. > > > > Note that this is only an administrative problem. It shouldn't interfere > > with the way host1x works. The goal is that the DRM device is registered > > at the proper hierarchical location. > > > > The circular dependency is indeed a problem, though. Quite frankly I > > have no idea how to solve this. However the approach taken in the > > current patch will break several other requirements as I already > > explained. > > The problem with doing drm_platform_init() with host1x device as > parameter is that drm_get_platform_dev() will take control of drvdata. > We'd need to put host1x specific struct host1x pointer to some other > place and I'm not sure what that place could be. Not anymore. I submitted a patch so that it no longer does that. The patch was merged about a month and a half ago. > You're right in that binding to a sub-device is not a nice way. DRM > framework just needs a "struct device" to bind to. exynos seems to solve > this by introducing a virtual device and bind to that. I'm not sure if > this is the best way, but worth considering? That was discussed a few months back already and nobody seemed to like the idea. In fact it was as a result of that discussion that Stephen brought up the idea to register the DRM driver from a central host1x driver (it may also have been part of a discussion on IRC, I don't remember exactly). At the time I spent some time on a patch that introduced drm_soc_init() to solve this by creating a dummy struct device and registering the driver on top of that. But I abandoned it in favour of fixing the DRM platform support code. The approach also didn't provide for the proper encapsulation. Thierry
Attachment:
pgpbzZAQLiaxd.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
