[v2,4/7] drivers/base: Add interface framework

From: Thierry Reding <treding@nvidia.com>

Some drivers, such as graphics drivers in the DRM subsystem, do not have
a real device that they can bind to. They are often composed of several
devices, each having their own driver. The master/component framework
can be used in these situations to collect the devices pertaining to one
logical device, wait until all of them have registered and then bind
them all at once.

For some situations this is only a partial solution. An implementation
of a master still needs to be registered with the system somehow. Many
drivers currently resort to creating a dummy device that a driver can
bind to and register the master against. This is problematic since it
requires (and presumes) knowledge about the system within drivers.

Furthermore there are setups where a suitable device already exists, but
is already bound to a driver. For example, on Tegra the following device
tree extract (simplified) represents the host1x device along with child
devices:

	host1x {
		display-controller {
			...
		};

		display-controller {
			...
		};

		hdmi {
			...
		};

		dsi {
			...
		};

		csi {
			...
		};

		video-input {
			...
		};
	};

Each of the child devices is in turn a client of host1x, in that it can
request resources (command stream DMA channels and syncpoints) from it.
To implement the DMA channel and syncpoint infrastructure, host1x comes
with its own driver. Children are implemented in separate drivers. In
Linux this set of devices would be exposed by DRM and V4L2 drivers.

However, neither the DRM nor the V4L2 drivers have a single device that
they can bind to. The DRM device is composed of the display controllers
and the various output devices, whereas the V4L2 device is composed of
one or more video input devices.

This patch introduces the concept of an interface and drivers that can
bind to a given interface. An interface can be exposed by any device,
and interface drivers can bind to these interfaces. Multiple drivers can
bind against a single interface. When a device is removed, interfaces
exposed by it will be removed as well, thereby removing the drivers that
were bound to those interfaces.

In the example above, the host1x device would expose the "tegra-host1x"
interface. DRM and V4L2 drivers can then bind to that interface and
instantiate the respective subsystem objects from there.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Note that I'd like to merge this through the Tegra DRM tree so that the
changes to the Tegra DRM driver later in this series can be merged at
the same time and are not delayed for another release cycle.

In particular that means that I'm looking for an Acked-by from Greg.

 drivers/base/Makefile     |   2 +-
 drivers/base/interface.c  | 186 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/interface.h |  40 ++++++++++
 3 files changed, 227 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/interface.c
 create mode 100644 include/linux/interface.h

On Tue, May 13, 2014 at 05:30:47PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Some drivers, such as graphics drivers in the DRM subsystem, do not have
> a real device that they can bind to. They are often composed of several
> devices, each having their own driver. The master/component framework
> can be used in these situations to collect the devices pertaining to one
> logical device, wait until all of them have registered and then bind
> them all at once.
> 
> For some situations this is only a partial solution. An implementation
> of a master still needs to be registered with the system somehow. Many
> drivers currently resort to creating a dummy device that a driver can
> bind to and register the master against. This is problematic since it
> requires (and presumes) knowledge about the system within drivers.
> 
> Furthermore there are setups where a suitable device already exists, but
> is already bound to a driver. For example, on Tegra the following device
> tree extract (simplified) represents the host1x device along with child
> devices:
> 
> 	host1x {
> 		display-controller {
> 			...
> 		};
> 
> 		display-controller {
> 			...
> 		};
> 
> 		hdmi {
> 			...
> 		};
> 
> 		dsi {
> 			...
> 		};
> 
> 		csi {
> 			...
> 		};
> 
> 		video-input {
> 			...
> 		};
> 	};
> 
> Each of the child devices is in turn a client of host1x, in that it can
> request resources (command stream DMA channels and syncpoints) from it.
> To implement the DMA channel and syncpoint infrastructure, host1x comes
> with its own driver. Children are implemented in separate drivers. In
> Linux this set of devices would be exposed by DRM and V4L2 drivers.
> 
> However, neither the DRM nor the V4L2 drivers have a single device that
> they can bind to. The DRM device is composed of the display controllers
> and the various output devices, whereas the V4L2 device is composed of
> one or more video input devices.
> 
> This patch introduces the concept of an interface and drivers that can
> bind to a given interface. An interface can be exposed by any device,
> and interface drivers can bind to these interfaces. Multiple drivers can
> bind against a single interface. When a device is removed, interfaces
> exposed by it will be removed as well, thereby removing the drivers that
> were bound to those interfaces.
> 
> In the example above, the host1x device would expose the "tegra-host1x"
> interface. DRM and V4L2 drivers can then bind to that interface and
> instantiate the respective subsystem objects from there.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Note that I'd like to merge this through the Tegra DRM tree so that the
> changes to the Tegra DRM driver later in this series can be merged at
> the same time and are not delayed for another release cycle.
> 
> In particular that means that I'm looking for an Acked-by from Greg.
> 
>  drivers/base/Makefile     |   2 +-
>  drivers/base/interface.c  | 186 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/interface.h |  40 ++++++++++
>  3 files changed, 227 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/base/interface.c
>  create mode 100644 include/linux/interface.h

Hm, this interface stuff smells like bus drivers light. Should we instead
have a pile of helpers to make creating new buses with match methods more
trivial? There's a fairly big pile of small use-cases where this might be
useful. In your case here all the host1x children would sit on a host1x
bus. Admittedly I didn't look into the details.
-Daniel

> 
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 04b314e0fa51..b5278904e443 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -4,7 +4,7 @@ obj-y			:= component.o core.o bus.o dd.o syscore.o \
>  			   driver.o class.o platform.o \
>  			   cpu.o firmware.o init.o map.o devres.o \
>  			   attribute_container.o transport_class.o \
> -			   topology.o container.o
> +			   topology.o container.o interface.o
>  obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
>  obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
>  obj-y			+= power/
> diff --git a/drivers/base/interface.c b/drivers/base/interface.c
> new file mode 100644
> index 000000000000..411f6cdf90e7
> --- /dev/null
> +++ b/drivers/base/interface.c
> @@ -0,0 +1,186 @@
> +/*
> + * Copyright (C) 2014 NVIDIA Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "interface: " fmt
> +
> +#include <linux/device.h>
> +#include <linux/interface.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +
> +static DEFINE_MUTEX(interfaces_lock);
> +static LIST_HEAD(interfaces);
> +static LIST_HEAD(drivers);
> +
> +struct interface_attachment {
> +	struct interface_driver *driver;
> +	struct list_head list;
> +};
> +
> +static bool interface_match(struct interface *intf,
> +			    struct interface_driver *driver)
> +{
> +	return strcmp(intf->name, driver->name) == 0;
> +}
> +
> +static bool interface_attached(struct interface *intf,
> +			       struct interface_driver *driver)
> +{
> +	struct interface_attachment *attach;
> +
> +	list_for_each_entry(attach, &intf->drivers, list)
> +		if (attach->driver == driver)
> +			return true;
> +
> +	return false;
> +}
> +
> +static int interface_attach(struct interface *intf,
> +			    struct interface_driver *driver)
> +{
> +	struct interface_attachment *attach;
> +
> +	attach = kzalloc(sizeof(*attach), GFP_KERNEL);
> +	if (!attach)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&attach->list);
> +	attach->driver = driver;
> +
> +	list_add_tail(&attach->list, &intf->drivers);
> +
> +	return 0;
> +}
> +
> +static void interface_detach(struct interface *intf,
> +			     struct interface_driver *driver)
> +{
> +	struct interface_attachment *attach;
> +
> +	list_for_each_entry(attach, &intf->drivers, list) {
> +		if (attach->driver == driver) {
> +			list_del(&attach->list);
> +			kfree(attach);
> +			return;
> +		}
> +	}
> +}
> +
> +static void interface_bind(struct interface *intf,
> +			   struct interface_driver *driver)
> +{
> +	int err;
> +
> +	if (interface_attached(intf, driver))
> +		return;
> +
> +	if (!interface_match(intf, driver))
> +		return;
> +
> +	err = interface_attach(intf, driver);
> +	if (err < 0) {
> +		pr_err("failed to attach driver %ps to interface %s: %d\n",
> +		       driver, intf->name, err);
> +		return;
> +	}
> +
> +	err = driver->bind(intf);
> +	if (err < 0) {
> +		pr_err("failed to bind driver %ps to interface %s: %d\n",
> +		       driver, intf->name, err);
> +		interface_detach(intf, driver);
> +		return;
> +	}
> +
> +	pr_debug("driver %ps bound to interface %s\n", driver, intf->name);
> +}
> +
> +static void interface_unbind(struct interface *intf,
> +			     struct interface_driver *driver)
> +{
> +	if (!interface_attached(intf, driver))
> +		return;
> +
> +	interface_detach(intf, driver);
> +	driver->unbind(intf);
> +
> +	pr_debug("driver %ps unbound from interface %s\n", driver, intf->name);
> +}
> +
> +int interface_add(struct interface *intf)
> +{
> +	struct interface_driver *driver;
> +
> +	INIT_LIST_HEAD(&intf->drivers);
> +	INIT_LIST_HEAD(&intf->list);
> +
> +	mutex_lock(&interfaces_lock);
> +
> +	list_add_tail(&intf->list, &interfaces);
> +
> +	pr_debug("interface %s added for device %s\n", intf->name,
> +		 dev_name(intf->dev));
> +
> +	list_for_each_entry(driver, &drivers, list)
> +		interface_bind(intf, driver);
> +
> +	mutex_unlock(&interfaces_lock);
> +
> +	return 0;
> +}
> +
> +void interface_remove(struct interface *intf)
> +{
> +	struct interface_driver *driver;
> +
> +	mutex_lock(&interfaces_lock);
> +
> +	list_for_each_entry(driver, &drivers, list)
> +		interface_unbind(intf, driver);
> +
> +	list_del_init(&intf->list);
> +
> +	pr_debug("interface %s removed for device %s\n", intf->name,
> +		 dev_name(intf->dev));
> +
> +	mutex_unlock(&interfaces_lock);
> +}
> +
> +int interface_register_driver(struct interface_driver *driver)
> +{
> +	struct interface *intf;
> +
> +	mutex_lock(&interfaces_lock);
> +
> +	list_add_tail(&driver->list, &drivers);
> +
> +	pr_debug("driver %ps added\n", driver);
> +
> +	list_for_each_entry(intf, &interfaces, list)
> +		interface_bind(intf, driver);
> +
> +	mutex_unlock(&interfaces_lock);
> +
> +	return 0;
> +}
> +
> +void interface_unregister_driver(struct interface_driver *driver)
> +{
> +	struct interface *intf;
> +
> +	mutex_lock(&interfaces_lock);
> +
> +	list_for_each_entry(intf, &interfaces, list)
> +		interface_unbind(intf, driver);
> +
> +	list_del_init(&driver->list);
> +
> +	pr_debug("driver %ps removed\n", driver);
> +
> +	mutex_unlock(&interfaces_lock);
> +}
> diff --git a/include/linux/interface.h b/include/linux/interface.h
> new file mode 100644
> index 000000000000..2c49ad3912fe
> --- /dev/null
> +++ b/include/linux/interface.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright (C) 2014 NVIDIA Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef INTERFACE_H
> +#define INTERFACE_H
> +
> +#include <linux/list.h>
> +
> +struct device;
> +struct interface;
> +
> +struct interface_driver {
> +	const char *name;
> +
> +	int (*bind)(struct interface *intf);
> +	void (*unbind)(struct interface *intf);
> +
> +	struct list_head list;
> +};
> +
> +int interface_register_driver(struct interface_driver *driver);
> +void interface_unregister_driver(struct interface_driver *driver);
> +
> +struct interface {
> +	const char *name;
> +	struct device *dev;
> +
> +	struct list_head drivers;
> +	struct list_head list;
> +};
> +
> +int interface_add(struct interface *intf);
> +void interface_remove(struct interface *intf);
> +
> +#endif
> -- 
> 1.9.2
>

On Tue, May 13, 2014 at 07:57:13PM +0200, Daniel Vetter wrote:
> On Tue, May 13, 2014 at 05:30:47PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Some drivers, such as graphics drivers in the DRM subsystem, do not have
> > a real device that they can bind to. They are often composed of several
> > devices, each having their own driver. The master/component framework
> > can be used in these situations to collect the devices pertaining to one
> > logical device, wait until all of them have registered and then bind
> > them all at once.
> > 
> > For some situations this is only a partial solution. An implementation
> > of a master still needs to be registered with the system somehow. Many
> > drivers currently resort to creating a dummy device that a driver can
> > bind to and register the master against. This is problematic since it
> > requires (and presumes) knowledge about the system within drivers.
> > 
> > Furthermore there are setups where a suitable device already exists, but
> > is already bound to a driver. For example, on Tegra the following device
> > tree extract (simplified) represents the host1x device along with child
> > devices:
> > 
> > 	host1x {
> > 		display-controller {
> > 			...
> > 		};
> > 
> > 		display-controller {
> > 			...
> > 		};
> > 
> > 		hdmi {
> > 			...
> > 		};
> > 
> > 		dsi {
> > 			...
> > 		};
> > 
> > 		csi {
> > 			...
> > 		};
> > 
> > 		video-input {
> > 			...
> > 		};
> > 	};
> > 
> > Each of the child devices is in turn a client of host1x, in that it can
> > request resources (command stream DMA channels and syncpoints) from it.
> > To implement the DMA channel and syncpoint infrastructure, host1x comes
> > with its own driver. Children are implemented in separate drivers. In
> > Linux this set of devices would be exposed by DRM and V4L2 drivers.
> > 
> > However, neither the DRM nor the V4L2 drivers have a single device that
> > they can bind to. The DRM device is composed of the display controllers
> > and the various output devices, whereas the V4L2 device is composed of
> > one or more video input devices.
> > 
> > This patch introduces the concept of an interface and drivers that can
> > bind to a given interface. An interface can be exposed by any device,
> > and interface drivers can bind to these interfaces. Multiple drivers can
> > bind against a single interface. When a device is removed, interfaces
> > exposed by it will be removed as well, thereby removing the drivers that
> > were bound to those interfaces.
> > 
> > In the example above, the host1x device would expose the "tegra-host1x"
> > interface. DRM and V4L2 drivers can then bind to that interface and
> > instantiate the respective subsystem objects from there.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Note that I'd like to merge this through the Tegra DRM tree so that the
> > changes to the Tegra DRM driver later in this series can be merged at
> > the same time and are not delayed for another release cycle.
> > 
> > In particular that means that I'm looking for an Acked-by from Greg.
> > 
> >  drivers/base/Makefile     |   2 +-
> >  drivers/base/interface.c  | 186 ++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/interface.h |  40 ++++++++++
> >  3 files changed, 227 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/base/interface.c
> >  create mode 100644 include/linux/interface.h
> 
> Hm, this interface stuff smells like bus drivers light. Should we instead
> have a pile of helpers to make creating new buses with match methods more
> trivial? There's a fairly big pile of small use-cases where this might be
> useful. In your case here all the host1x children would sit on a host1x
> bus. Admittedly I didn't look into the details.

The problem that this is trying to solve is slightly different. The
host1x children do already sit on a kind of bus, only that there's
currently (and likely never) a real need to turn it into a custom bus
implementation, so we reuse platform_bus instead (host1x is the parent
of the child devices).

What this interface framework tries to solve is that host1x itself is a
device with a proper driver. Child devices in turn are implemented as
separate drivers as well. The result is that there is no device that can
be used to register the DRM driver against. DRM doesn't specifically
require this since it doesn't take ownership of a device, but with the
driver model every driver needs a device to bind against, otherwise no
.probe() function is called.

Two solutions have previously been proposed, and Tegra DRM has used each
of them at one point, but it never really seemed like the right
solution. Also at each point I was told to look into fixing it.

The first solution is to have everything in one driver. That way the DRM
device could be instantiated from the host1x driver. Similarily a future
V4L2 driver could be instantiated from the same host1x driver. That has
the obvious disadvantage that the driver becomes really big and
monolithic.

A different solution, which seems to be fairly common for DRM drivers
for SoCs, is to instantiate a dummy device so that the DRM driver can
bind to it. This can happen in two forms: add the dummy device directly
in device tree (which goes against pretty much everything that's been
preached about device tree in the past) or the dummy device can be
instantiated in code, which is what the current Tegra DRM/host1x driver
does.

The interface framework solution is a way to combine both of the above,
while dropping the ugly parts at the same time. What it tries to achieve
is to allow a specific device (the host1x parent device in this case) to
expose an interface (which is essentially just a name of a set of
functions that the device implements). For host1x this interface would
be "tegra-host1x". An interface can be bound to by more than one driver,
which is probably the most fundamental difference between an interface
and a device (as in the driver model).

So for host1x, there could be two drivers that bind to the tegra-host1x
interface: DRM and V4L2 (there could potentially be others). None of the
drivers would need to take ownership of a device, since they are meta-
drivers that would typically use the component/master framework. The
only reason why it's needed is so that the drivers get an entry point so
that they can register the master of a componentized device.

I guess that reduces the number of use-cases compared to what you had in
mind, so perhaps moving this from the driver core into host1x would be a
better option. Still perhaps it's useful for other scenarios that I just
haven't thought about and having it in the core would make it easier to
find.

Thierry

On Tue, May 13, 2014 at 07:57:13PM +0200, Daniel Vetter wrote:
> On Tue, May 13, 2014 at 05:30:47PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Some drivers, such as graphics drivers in the DRM subsystem, do not have
> > a real device that they can bind to. They are often composed of several
> > devices, each having their own driver. The master/component framework
> > can be used in these situations to collect the devices pertaining to one
> > logical device, wait until all of them have registered and then bind
> > them all at once.
> > 
> > For some situations this is only a partial solution. An implementation
> > of a master still needs to be registered with the system somehow. Many
> > drivers currently resort to creating a dummy device that a driver can
> > bind to and register the master against. This is problematic since it
> > requires (and presumes) knowledge about the system within drivers.
> > 
> > Furthermore there are setups where a suitable device already exists, but
> > is already bound to a driver. For example, on Tegra the following device
> > tree extract (simplified) represents the host1x device along with child
> > devices:
> > 
> > 	host1x {
> > 		display-controller {
> > 			...
> > 		};
> > 
> > 		display-controller {
> > 			...
> > 		};
> > 
> > 		hdmi {
> > 			...
> > 		};
> > 
> > 		dsi {
> > 			...
> > 		};
> > 
> > 		csi {
> > 			...
> > 		};
> > 
> > 		video-input {
> > 			...
> > 		};
> > 	};
> > 
> > Each of the child devices is in turn a client of host1x, in that it can
> > request resources (command stream DMA channels and syncpoints) from it.
> > To implement the DMA channel and syncpoint infrastructure, host1x comes
> > with its own driver. Children are implemented in separate drivers. In
> > Linux this set of devices would be exposed by DRM and V4L2 drivers.
> > 
> > However, neither the DRM nor the V4L2 drivers have a single device that
> > they can bind to. The DRM device is composed of the display controllers
> > and the various output devices, whereas the V4L2 device is composed of
> > one or more video input devices.
> > 
> > This patch introduces the concept of an interface and drivers that can
> > bind to a given interface. An interface can be exposed by any device,
> > and interface drivers can bind to these interfaces. Multiple drivers can
> > bind against a single interface. When a device is removed, interfaces
> > exposed by it will be removed as well, thereby removing the drivers that
> > were bound to those interfaces.
> > 
> > In the example above, the host1x device would expose the "tegra-host1x"
> > interface. DRM and V4L2 drivers can then bind to that interface and
> > instantiate the respective subsystem objects from there.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Note that I'd like to merge this through the Tegra DRM tree so that the
> > changes to the Tegra DRM driver later in this series can be merged at
> > the same time and are not delayed for another release cycle.
> > 
> > In particular that means that I'm looking for an Acked-by from Greg.
> > 
> >  drivers/base/Makefile     |   2 +-
> >  drivers/base/interface.c  | 186 ++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/interface.h |  40 ++++++++++
> >  3 files changed, 227 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/base/interface.c
> >  create mode 100644 include/linux/interface.h
> 
> Hm, this interface stuff smells like bus drivers light. Should we instead
> have a pile of helpers to make creating new buses with match methods more
> trivial? There's a fairly big pile of small use-cases where this might be
> useful. In your case here all the host1x children would sit on a host1x
> bus. Admittedly I didn't look into the details.

I have no problem adding such "bus-light" functions, to make it easier
to create and implement a bus in the driver core, as I know it's really
heavy.  That's been on my "todo" list for over a decade now...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, May 13, 2014 at 11:31:07PM +0200, Thierry Reding wrote:
> A different solution, which seems to be fairly common for DRM drivers
> for SoCs, is to instantiate a dummy device so that the DRM driver can
> bind to it. This can happen in two forms: add the dummy device directly
> in device tree (which goes against pretty much everything that's been
> preached about device tree in the past) or the dummy device can be
> instantiated in code, which is what the current Tegra DRM/host1x driver
> does.

Actually the dummy device seems to be an acceptable solution and iirc was
even acked by DT maintainers in the last KS. I'm not on top of things, but
iirc the thinking was that a dummy device which just pulls in all the
relevant real bits with phandles is justified in the board file since it
tells you how the board is intended to be used. The justification was that
on SoCs where assigning stuff between v4l and drm is really flexible (e.g.
on omap or so where you can assign the display pipes essentially freely)
the use-case of the board is what matters, and that's a somewhat physical
property of it (i.e. in what kind of device it's sitting.)

So in your case you'd have a fake display and a fake video device which
pulls everything the drm/v4l driver need together. So if the issue is how
to get at a master device I think that's simple to solve.

If the issue otoh is how the various drivers can get at the bus-like
services host1x exposes, then I think a real bus driver makes a lot more
sense. And Greg seems to have ideas about that already.
-Daniel

On Tue, May 13, 2014 at 05:32:15PM -0700, Greg Kroah-Hartman wrote:
> On Tue, May 13, 2014 at 07:57:13PM +0200, Daniel Vetter wrote:
> > On Tue, May 13, 2014 at 05:30:47PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > > 
> > > Some drivers, such as graphics drivers in the DRM subsystem, do not have
> > > a real device that they can bind to. They are often composed of several
> > > devices, each having their own driver. The master/component framework
> > > can be used in these situations to collect the devices pertaining to one
> > > logical device, wait until all of them have registered and then bind
> > > them all at once.
> > > 
> > > For some situations this is only a partial solution. An implementation
> > > of a master still needs to be registered with the system somehow. Many
> > > drivers currently resort to creating a dummy device that a driver can
> > > bind to and register the master against. This is problematic since it
> > > requires (and presumes) knowledge about the system within drivers.
> > > 
> > > Furthermore there are setups where a suitable device already exists, but
> > > is already bound to a driver. For example, on Tegra the following device
> > > tree extract (simplified) represents the host1x device along with child
> > > devices:
> > > 
> > > 	host1x {
> > > 		display-controller {
> > > 			...
> > > 		};
> > > 
> > > 		display-controller {
> > > 			...
> > > 		};
> > > 
> > > 		hdmi {
> > > 			...
> > > 		};
> > > 
> > > 		dsi {
> > > 			...
> > > 		};
> > > 
> > > 		csi {
> > > 			...
> > > 		};
> > > 
> > > 		video-input {
> > > 			...
> > > 		};
> > > 	};
> > > 
> > > Each of the child devices is in turn a client of host1x, in that it can
> > > request resources (command stream DMA channels and syncpoints) from it.
> > > To implement the DMA channel and syncpoint infrastructure, host1x comes
> > > with its own driver. Children are implemented in separate drivers. In
> > > Linux this set of devices would be exposed by DRM and V4L2 drivers.
> > > 
> > > However, neither the DRM nor the V4L2 drivers have a single device that
> > > they can bind to. The DRM device is composed of the display controllers
> > > and the various output devices, whereas the V4L2 device is composed of
> > > one or more video input devices.
> > > 
> > > This patch introduces the concept of an interface and drivers that can
> > > bind to a given interface. An interface can be exposed by any device,
> > > and interface drivers can bind to these interfaces. Multiple drivers can
> > > bind against a single interface. When a device is removed, interfaces
> > > exposed by it will be removed as well, thereby removing the drivers that
> > > were bound to those interfaces.
> > > 
> > > In the example above, the host1x device would expose the "tegra-host1x"
> > > interface. DRM and V4L2 drivers can then bind to that interface and
> > > instantiate the respective subsystem objects from there.
> > > 
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > > Note that I'd like to merge this through the Tegra DRM tree so that the
> > > changes to the Tegra DRM driver later in this series can be merged at
> > > the same time and are not delayed for another release cycle.
> > > 
> > > In particular that means that I'm looking for an Acked-by from Greg.
> > > 
> > >  drivers/base/Makefile     |   2 +-
> > >  drivers/base/interface.c  | 186 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/interface.h |  40 ++++++++++
> > >  3 files changed, 227 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/base/interface.c
> > >  create mode 100644 include/linux/interface.h
> > 
> > Hm, this interface stuff smells like bus drivers light. Should we instead
> > have a pile of helpers to make creating new buses with match methods more
> > trivial? There's a fairly big pile of small use-cases where this might be
> > useful. In your case here all the host1x children would sit on a host1x
> > bus. Admittedly I didn't look into the details.
> 
> I have no problem adding such "bus-light" functions, to make it easier
> to create and implement a bus in the driver core, as I know it's really
> heavy.  That's been on my "todo" list for over a decade now...

Hm, I've victimized^Wvolunteered a few internal people to look into a
hdmi/dp sink bus type of thing so that we can move away from all those
ad-hoc hacks currently used to coordinate between drm display drivers and
the audio side of things. So I'm interested.

Do you have some ideas somewhere already about how you think this should
look like? Or is this more a "hey, this would be really cool, eventually"
kind of thing?

Thanks, Daniel

On Wed, May 14, 2014 at 04:34:21PM +0200, Daniel Vetter wrote:
> On Tue, May 13, 2014 at 11:31:07PM +0200, Thierry Reding wrote:
> > A different solution, which seems to be fairly common for DRM drivers
> > for SoCs, is to instantiate a dummy device so that the DRM driver can
> > bind to it. This can happen in two forms: add the dummy device directly
> > in device tree (which goes against pretty much everything that's been
> > preached about device tree in the past) or the dummy device can be
> > instantiated in code, which is what the current Tegra DRM/host1x driver
> > does.
> 
> Actually the dummy device seems to be an acceptable solution and iirc was
> even acked by DT maintainers in the last KS. I'm not on top of things, but
> iirc the thinking was that a dummy device which just pulls in all the
> relevant real bits with phandles is justified in the board file since it
> tells you how the board is intended to be used. The justification was that
> on SoCs where assigning stuff between v4l and drm is really flexible (e.g.
> on omap or so where you can assign the display pipes essentially freely)
> the use-case of the board is what matters, and that's a somewhat physical
> property of it (i.e. in what kind of device it's sitting.)

I disagree. If there's a way to derive all that information within the
driver (like we do for Tegra) then there should be no need to add
redundant content to the device tree. Also note that Tegra DRM and its
device tree binding predate the last KS, and prior to that the gospel
was to *not* add this kind of data to the device tree. Hence I was
forced to spend a *lot* of time working out a solution in the driver.
Therefore I hope it's understandable that I don't take well to being
told to just go use a bloody dummy device.

> So in your case you'd have a fake display and a fake video device which
> pulls everything the drm/v4l driver need together. So if the issue is how
> to get at a master device I think that's simple to solve.

But that's completely redundant. The driver is perfectly capable of
determining what devices belong to a DRM device and which should be part
of a V4L2 device.

Fake devices are only workarounds for a problem that we currently have
no better solution for in the kernel. And as a matter of fact we already
do create a dummy device (albeit in the kernel rather than in device
tree) on Tegra. But I consider that suboptimal, hence why I want to find
a better way to do this. This interface framework is the result.

If people don't see a need to fix this then I'll just stick with what I
currently have, since it isn't any worse than what everybody else is
doing.

> If the issue otoh is how the various drivers can get at the bus-like
> services host1x exposes, then I think a real bus driver makes a lot more
> sense. And Greg seems to have ideas about that already.

There aren't really any bus-like services exposed by host1x. It really
just exposes resources such as DMA channels or syncpoints. But that's
not even a problem because we know that host1x children are always,
well, children of host1x and therefore we can simply get access to
host1x via the parent device.

Thierry

On Wed, May 14, 2014 at 04:37:19PM +0200, Daniel Vetter wrote:
> On Tue, May 13, 2014 at 05:32:15PM -0700, Greg Kroah-Hartman wrote:
> > On Tue, May 13, 2014 at 07:57:13PM +0200, Daniel Vetter wrote:
> > > On Tue, May 13, 2014 at 05:30:47PM +0200, Thierry Reding wrote:
> > > > From: Thierry Reding <treding@nvidia.com>
> > > > 
> > > > Some drivers, such as graphics drivers in the DRM subsystem, do not have
> > > > a real device that they can bind to. They are often composed of several
> > > > devices, each having their own driver. The master/component framework
> > > > can be used in these situations to collect the devices pertaining to one
> > > > logical device, wait until all of them have registered and then bind
> > > > them all at once.
> > > > 
> > > > For some situations this is only a partial solution. An implementation
> > > > of a master still needs to be registered with the system somehow. Many
> > > > drivers currently resort to creating a dummy device that a driver can
> > > > bind to and register the master against. This is problematic since it
> > > > requires (and presumes) knowledge about the system within drivers.
> > > > 
> > > > Furthermore there are setups where a suitable device already exists, but
> > > > is already bound to a driver. For example, on Tegra the following device
> > > > tree extract (simplified) represents the host1x device along with child
> > > > devices:
> > > > 
> > > > 	host1x {
> > > > 		display-controller {
> > > > 			...
> > > > 		};
> > > > 
> > > > 		display-controller {
> > > > 			...
> > > > 		};
> > > > 
> > > > 		hdmi {
> > > > 			...
> > > > 		};
> > > > 
> > > > 		dsi {
> > > > 			...
> > > > 		};
> > > > 
> > > > 		csi {
> > > > 			...
> > > > 		};
> > > > 
> > > > 		video-input {
> > > > 			...
> > > > 		};
> > > > 	};
> > > > 
> > > > Each of the child devices is in turn a client of host1x, in that it can
> > > > request resources (command stream DMA channels and syncpoints) from it.
> > > > To implement the DMA channel and syncpoint infrastructure, host1x comes
> > > > with its own driver. Children are implemented in separate drivers. In
> > > > Linux this set of devices would be exposed by DRM and V4L2 drivers.
> > > > 
> > > > However, neither the DRM nor the V4L2 drivers have a single device that
> > > > they can bind to. The DRM device is composed of the display controllers
> > > > and the various output devices, whereas the V4L2 device is composed of
> > > > one or more video input devices.
> > > > 
> > > > This patch introduces the concept of an interface and drivers that can
> > > > bind to a given interface. An interface can be exposed by any device,
> > > > and interface drivers can bind to these interfaces. Multiple drivers can
> > > > bind against a single interface. When a device is removed, interfaces
> > > > exposed by it will be removed as well, thereby removing the drivers that
> > > > were bound to those interfaces.
> > > > 
> > > > In the example above, the host1x device would expose the "tegra-host1x"
> > > > interface. DRM and V4L2 drivers can then bind to that interface and
> > > > instantiate the respective subsystem objects from there.
> > > > 
> > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > ---
> > > > Note that I'd like to merge this through the Tegra DRM tree so that the
> > > > changes to the Tegra DRM driver later in this series can be merged at
> > > > the same time and are not delayed for another release cycle.
> > > > 
> > > > In particular that means that I'm looking for an Acked-by from Greg.
> > > > 
> > > >  drivers/base/Makefile     |   2 +-
> > > >  drivers/base/interface.c  | 186 ++++++++++++++++++++++++++++++++++++++++++++++
> > > >  include/linux/interface.h |  40 ++++++++++
> > > >  3 files changed, 227 insertions(+), 1 deletion(-)
> > > >  create mode 100644 drivers/base/interface.c
> > > >  create mode 100644 include/linux/interface.h
> > > 
> > > Hm, this interface stuff smells like bus drivers light. Should we instead
> > > have a pile of helpers to make creating new buses with match methods more
> > > trivial? There's a fairly big pile of small use-cases where this might be
> > > useful. In your case here all the host1x children would sit on a host1x
> > > bus. Admittedly I didn't look into the details.
> > 
> > I have no problem adding such "bus-light" functions, to make it easier
> > to create and implement a bus in the driver core, as I know it's really
> > heavy.  That's been on my "todo" list for over a decade now...
> 
> Hm, I've victimized^Wvolunteered a few internal people to look into a
> hdmi/dp sink bus type of thing so that we can move away from all those
> ad-hoc hacks currently used to coordinate between drm display drivers and
> the audio side of things. So I'm interested.
> 
> Do you have some ideas somewhere already about how you think this should
> look like? Or is this more a "hey, this would be really cool, eventually"
> kind of thing?

It was very much a "hey, this would be really cool" type of thing,
nothing specific at all, sorry.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, May 13, 2014 at 05:32:15PM -0700, Greg Kroah-Hartman wrote:
> On Tue, May 13, 2014 at 07:57:13PM +0200, Daniel Vetter wrote:
> > On Tue, May 13, 2014 at 05:30:47PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding <treding@nvidia.com>
> > > 
> > > Some drivers, such as graphics drivers in the DRM subsystem, do not have
> > > a real device that they can bind to. They are often composed of several
> > > devices, each having their own driver. The master/component framework
> > > can be used in these situations to collect the devices pertaining to one
> > > logical device, wait until all of them have registered and then bind
> > > them all at once.
> > > 
> > > For some situations this is only a partial solution. An implementation
> > > of a master still needs to be registered with the system somehow. Many
> > > drivers currently resort to creating a dummy device that a driver can
> > > bind to and register the master against. This is problematic since it
> > > requires (and presumes) knowledge about the system within drivers.
> > > 
> > > Furthermore there are setups where a suitable device already exists, but
> > > is already bound to a driver. For example, on Tegra the following device
> > > tree extract (simplified) represents the host1x device along with child
> > > devices:
> > > 
> > > 	host1x {
> > > 		display-controller {
> > > 			...
> > > 		};
> > > 
> > > 		display-controller {
> > > 			...
> > > 		};
> > > 
> > > 		hdmi {
> > > 			...
> > > 		};
> > > 
> > > 		dsi {
> > > 			...
> > > 		};
> > > 
> > > 		csi {
> > > 			...
> > > 		};
> > > 
> > > 		video-input {
> > > 			...
> > > 		};
> > > 	};
> > > 
> > > Each of the child devices is in turn a client of host1x, in that it can
> > > request resources (command stream DMA channels and syncpoints) from it.
> > > To implement the DMA channel and syncpoint infrastructure, host1x comes
> > > with its own driver. Children are implemented in separate drivers. In
> > > Linux this set of devices would be exposed by DRM and V4L2 drivers.
> > > 
> > > However, neither the DRM nor the V4L2 drivers have a single device that
> > > they can bind to. The DRM device is composed of the display controllers
> > > and the various output devices, whereas the V4L2 device is composed of
> > > one or more video input devices.
> > > 
> > > This patch introduces the concept of an interface and drivers that can
> > > bind to a given interface. An interface can be exposed by any device,
> > > and interface drivers can bind to these interfaces. Multiple drivers can
> > > bind against a single interface. When a device is removed, interfaces
> > > exposed by it will be removed as well, thereby removing the drivers that
> > > were bound to those interfaces.
> > > 
> > > In the example above, the host1x device would expose the "tegra-host1x"
> > > interface. DRM and V4L2 drivers can then bind to that interface and
> > > instantiate the respective subsystem objects from there.
> > > 
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > > Note that I'd like to merge this through the Tegra DRM tree so that the
> > > changes to the Tegra DRM driver later in this series can be merged at
> > > the same time and are not delayed for another release cycle.
> > > 
> > > In particular that means that I'm looking for an Acked-by from Greg.
> > > 
> > >  drivers/base/Makefile     |   2 +-
> > >  drivers/base/interface.c  | 186 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/interface.h |  40 ++++++++++
> > >  3 files changed, 227 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/base/interface.c
> > >  create mode 100644 include/linux/interface.h
> > 
> > Hm, this interface stuff smells like bus drivers light. Should we instead
> > have a pile of helpers to make creating new buses with match methods more
> > trivial? There's a fairly big pile of small use-cases where this might be
> > useful. In your case here all the host1x children would sit on a host1x
> > bus. Admittedly I didn't look into the details.
> 
> I have no problem adding such "bus-light" functions, to make it easier
> to create and implement a bus in the driver core, as I know it's really
> heavy.  That's been on my "todo" list for over a decade now...

Greg,

Do you think you could find the time to look at this patch in a little
more detail? This isn't about creating a light alternative to busses at
all. It is an attempt to solve a different problem.

Consider the following: you have a collection of hardware devices that
together can implement functionality in a given subsystem such as DRM or
V4L2. In many cases all these devices have their own driver and they are
glued together using the component helpers. This results in a situation
where people are instantiating dummy devices for the sole purpose of
getting a driver probed, since all of the existing devices have already
had a driver bind to them.

Another downside of using dummy devices is that they mess up the device
hierarchy. All of a sudden you have a situation where the dummy device
is the logical parent for its aunts and uncles (or siblings).

The solution proposed here is to allow any device to expose an interface
that interface drivers can bind to. This removes the need for dummy
devices. As opposed to device drivers, an interface can be bound to by
multiple drivers. That's a feature that is needed specifically by host1x
on Tegra since two drivers need to hang off of the host1x device (DRM
and V4L2), but I can easily imagine this to be useful in other cases.
Interfaces are exposed explicitly at probe time by the device drivers
for the devices they drive.

Even though this was designed with the above use-case in mind I can
imagine this to be useful for other things as well. For example a set of
generic interfaces could be created and devices (or even whole classes
of devices) could be made to expose these interfaces. This would cause
interfaces to be created for each of these devices. That's functionality
similar to what can be done with notifiers, albeit somewhat more
structured. That could for example be useful to apply policy to a class
of devices or collect statistics, etc.

If you think that I'm on a wild-goose chase, please let me know so that
I don't waste any more time on this.

Thierry