Re: [RFC v2 6/8] gpu: drm: tegra: Remove redundant host1x

From: Thierry Reding
Date: Thu Dec 20 2012 - 16:50:07 EST

On Thu, Dec 20, 2012 at 11:34:26PM +0200, Terje BergstrÃm wrote:
> On 20.12.2012 22:30, Thierry Reding wrote:
> > The problem with your proposed solution is that, even any of Stephen's
> > valid objections aside, it won't work. Once the tegra-drm module is
> > unloaded, the driver's data will be left in the current state and the
> > link to the dummy device will be lost.
> The dummy device is created by tegradrm's module init, because it's used
> only by tegradrm. When tegradrm is unloaded, all the tegradrm devices
> and drivers are unloaded and freed, including the dummy one.

No, the children platform devices of host1x are not freed, so their
driver data will still contain the data set by the previous call to
their driver's .probe() function.

But reading what you said again, you were proposing to set the children
driver data from the tegra-drm dummy device's .probe(). In that case it
would probably work.

> > And quite frankly given how all the various host1x components are
> > interlinked I think having a single function that gets the DRM dummy
> > device for DRM-related clients isn't that bad.
> I like clear responsibilities. tegradrm is responsible for the DRM
> interface, and host1x driver is responsible for host1x. tegradrm owns
> its data, and can pass its pointers to the functions that need it.
> But fine, I still don't like it, but I'll add host1x_set_tegradrm() and
> host1x_get_tegradrm(), which act as setter and getter for tegradrm's
> global data. I still think it's a solution to a problem we don't have,
> but we've wasted an order of magnitude more time debating it than it
> takes to implement.

As Stephen already pointed out, since the host1x driver will already
instantiate the dummy device, there's no point in adding a function to
set the pointer to that device.

Adding a getter should be enough. And it should return a pointer to the
dummy platform devices .dev field. Calling it host1x_get_drm_device()
may make it more obvious about what it returns.


Attachment: pgp00000.pgp
Description: PGP signature