Re: [PATCH 4/5] gpu: host1x: Provide a proper struct bus_type

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3640 bytes --]

On Tue, Dec 23, 2014 at 03:30:20PM +0800, Mark Zhang wrote:
> On 12/19/2014 11:24 PM, Thierry Reding wrote:
> > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > 
> > Previously the struct bus_type exported by the host1x infrastructure was
> > only a very basic skeleton. Turn that implementation into a more full-
> > fledged bus to support proper probe ordering and power management.
> > 
> > Note that the bus infrastructure needs to be available before any of the
> > drivers can be registered, so the bus needs to be registered before the
> > host1x module. Otherwise drivers could be registered before the module
> > is loaded and trigger a BUG_ON() in driver_register(). To ensure the bus
> > infrastructure is always there, always build the code into the kernel
> > when enabled and register it with a postcore initcall.
> > 
> 
> So this means there is no chance that host1x can be built as a kernel
> module, right? I'm fine with that, just asking.

No, it means that not all of host1x can be built as a module. The host1x
bus infrastructure will always be built-in when TEGRA_HOST1X is enabled.

> > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> [...]
> > diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile
> > index c1189f004441..a3e667a1b6f5 100644
> > --- a/drivers/gpu/host1x/Makefile
> > +++ b/drivers/gpu/host1x/Makefile
> > @@ -1,5 +1,4 @@
> >  host1x-y = \
> > -	bus.o \
> >  	syncpt.o \
> >  	dev.o \
> >  	intr.o \
> > @@ -13,3 +12,5 @@ host1x-y = \
> >  	hw/host1x04.o
> >  
> >  obj-$(CONFIG_TEGRA_HOST1X) += host1x.o
> > +
> > +obj-y += bus.o
> 
> I didn't get it, why we need to do this?

If CONFIG_TEGRA_HOST1X=m, then obj-$(CONFIG_TEGRA_HOST1X) builds the
bus.o into a module. But we want it to always be built-in. The build
system will descend into the drivers/gpu/host1x directory only if the
TEGRA_HOST1X symbol is selected (either =y or =m), therefore obj-y here
will result in bus.o being built-in whether the rest of host1x is built
as a module or built-in.

> > diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> > index 0b52f0ea8871..28630a5e9397 100644
> > --- a/drivers/gpu/host1x/bus.c
> > +++ b/drivers/gpu/host1x/bus.c
> > @@ -72,13 +72,14 @@ static void host1x_subdev_del(struct host1x_subdev *subdev)
> [...]
> >  
> >  static inline struct host1x_device *to_host1x_device(struct device *dev)
> > 
> 
> The change looks good to me. Just one thing I feel not comfortable:
> "struct host1x_device" is not a real device, it represents the drm
> device actually. The real tegra host1x device is represented by "struct
> host1x". But the name "host1x_device" makes people confusing, I mean, it
> will make people thinking it's the real "tegra host1x" device then bring
> the reading difficulty.

The reason behind that name is that host1x provides a bus (real to some
degree, but also virtual). host1x is the device that provides the bus
whereas a host1x_device is a "device" on the "host1x" bus. That's just
like an i2c_client is a "client" on the "I2C" bus. Or an spi_device is a
"device" on the "SPI" bus.

> Why don't we change this to something like "drm_device" or
> "tegra_drm_device"?

Other devices can be host1x devices. Some time ago work was being done
on a driver for the CSI/VI hardware (for camera or video input). The
idea was that it would also be instantiated as a host1x_device in some
other subsystem (V4L2 at the time).

The functionality here is generic and in no way DRM specific.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]