On Wed, Dec 05, 2012 at 12:10:50PM +0200, Terje Bergström wrote: > On 05.12.2012 10:33, Thierry Reding wrote: > > I've been thinking about this some more and came to the conclusion that > > since we will already have a tight coupling between host1x and tegra-drm > > we may just as well keep the client registration code in host1x. The way > > I imagine this to work would be to export a public API from tegra-drm, > > say tegra_drm_init() and tegra_drm_exit(), that could be called in place > > of drm_platform_init() and drm_platform_exit() in the current code. > > > > tegra_drm_init() could then be passed the host1x platform device to bind > > to. The only thing that would need to be done is move the fields in the > > host1x structure specific to DRM into a separate structure. host1x would > > have to export host1x_drm_init/exit() which the DRM can invoke to have > > all DRM clients register to the DRM subsystem. > > > > From a hierarchical point of view this makes sense, with host1x being > > the parent of all DRM subdevices. It allows us to reuse the current code > > from tegra-drm that has been tested and works properly even for module > > unload/reload. We also get to keep the proper encapsulation and the > > switch to the separate host1x driver will require a much smaller patch. > > > > Does anybody see a disadvantage in this approach? > > I'm a bit confused about the scope. You mention host1x several times, > but I'm not sure if you mean the file drivers/gpu/drm/tegra/host1x.c or > the host1x driver. So I might be babbling when I answer this. Could you > please clarify that? What I propose is to move the client registration code that is currently in drivers/gpu/drm/tegra/host1x.c to the host1x driver, which may or may not end up in drivers/gpu/host1x. > host1x hardware access must be encapsulated in host1x driver > (drivers/gpu/host1x if that's the location we prefer). As for the > management of the list of DRM clients, we proposed the move to drm.c, > because host1x hardware would anyway be controlled by a different > driver. Having file called host1x.c in tegradrm didn't sound logical, as > its not really controlling host1x, and its probe wouldn't be called. 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. > If your proposal is that we'd move the management of the list of host1x > devices from tegradrm to host1x driver, we'd have a tight circular > dependency between two drivers and that's almost always a bad idea. So > far all ideas have revolved around tegradrm calling host1x, and host1x > calling a bit of DRM (for CMA, would be fixed in later version) but not > host1x calling tegradrm. 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. Thierry
Attachment:
pgphrramKwKpu.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
