From: Thierry Reding treding@nvidia.com
Hi,
This series converts the Tegra DRM driver to the master/component framework. The length of the series and the list of people in Cc is mostly due to the fact that Tegra has some special requirements as opposed to other drivers and therefore requires some changes outside of DRM.
Patch 1 introduces a helper function that can be used by new DRM drivers that don't rely on legacy userspace ABIs. It allows them to register DRM devices much more easily and without much of the midlayer that currently exists in the DRM subsystem.
Patches 2 and 3 make some changes to the master/component framework which are necessary to convert Tegra DRM to use it. All current users of the API have been converted as part of this patch. Note that at the same time the drivers are converted to no longer use drm_platform_init() in favour of the drm_set_unique() from patch 1 in conjunction with drm_dev_alloc() and drm_dev_register(). I would've liked to avoid such invasive changes in a single patch, but unfortunately I couldn't think of a way how to do that. I'm open to suggestions.
Patch 4 adds a new interface framework that supplements the master/ component framework and can be used in situations where there is no struct device * that a driver can bind to.
The Tegra DRM driver is converted to using the master/component framework in patch 5 using the above four patches.
Finally patches 6 and 7 add some documentation about the new way of registering DRM devices that don't need legacy support.
Each patch has a somewhat more elaborate description of why it is needed or what problem it solves. The patchset applies on top of linux-next.
I welcome any questions or comments you might have.
Thierry
Thierry Reding (7): drm: Introduce drm_set_unique() drivers/base: Allow driver-data to be attached to a master drivers/base: Allow multiple masters per device drivers/base: Add interface framework drm/tegra: Convert to master/component framework drm: Add device registration documentation drm: Document how to register devices without struct drm_bus
Documentation/DocBook/drm.tmpl | 38 ++ drivers/base/Makefile | 2 +- drivers/base/component.c | 31 +- drivers/base/interface.c | 186 ++++++++++ drivers/gpu/drm/drm_ioctl.c | 23 +- drivers/gpu/drm/drm_pci.c | 80 ++--- drivers/gpu/drm/drm_platform.c | 15 +- drivers/gpu/drm/drm_stub.c | 43 ++- drivers/gpu/drm/drm_usb.c | 20 +- drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 8 +- drivers/gpu/drm/msm/hdmi/hdmi.c | 8 +- drivers/gpu/drm/msm/msm_drv.c | 75 ++-- drivers/gpu/drm/msm/msm_drv.h | 1 + drivers/gpu/drm/tegra/Makefile | 1 - drivers/gpu/drm/tegra/bus.c | 64 ---- drivers/gpu/drm/tegra/dc.c | 58 ++- drivers/gpu/drm/tegra/drm.c | 171 ++++++--- drivers/gpu/drm/tegra/drm.h | 27 +- drivers/gpu/drm/tegra/dsi.c | 144 ++++---- drivers/gpu/drm/tegra/gr2d.c | 78 ++-- drivers/gpu/drm/tegra/gr3d.c | 77 ++-- drivers/gpu/drm/tegra/hdmi.c | 69 ++-- drivers/gpu/drm/tegra/sor.c | 71 ++-- drivers/gpu/host1x/Makefile | 1 - drivers/gpu/host1x/bus.c | 553 ----------------------------- drivers/gpu/host1x/bus.h | 29 -- drivers/gpu/host1x/dev.c | 21 +- drivers/gpu/host1x/dev.h | 7 +- drivers/staging/imx-drm/imx-drm-core.c | 58 +-- drivers/staging/imx-drm/imx-hdmi.c | 4 +- drivers/staging/imx-drm/imx-ldb.c | 4 +- drivers/staging/imx-drm/imx-tve.c | 4 +- drivers/staging/imx-drm/ipuv3-crtc.c | 4 +- drivers/staging/imx-drm/parallel-display.c | 4 +- include/drm/drmP.h | 3 + include/linux/component.h | 20 +- include/linux/host1x.h | 64 +--- include/linux/interface.h | 40 +++ 38 files changed, 849 insertions(+), 1257 deletions(-) create mode 100644 drivers/base/interface.c delete mode 100644 drivers/gpu/drm/tegra/bus.c delete mode 100644 drivers/gpu/host1x/bus.c delete mode 100644 drivers/gpu/host1x/bus.h create mode 100644 include/linux/interface.h
From: Thierry Reding treding@nvidia.com
Add a helper function that allows drivers to statically set the unique name of the device. This will allow platform and USB drivers to get rid of their DRM bus implementations and directly use drm_dev_alloc() and drm_dev_register().
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - move drm_set_unique() to drivers/gpu/drm/drm_stub.c - add kernel-doc
drivers/gpu/drm/drm_ioctl.c | 23 +++++++++++++++++------ drivers/gpu/drm/drm_stub.c | 24 ++++++++++++++++++++++++ include/drm/drmP.h | 3 +++ 3 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 38269d5aa333..a59f6c2d7586 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -131,13 +131,24 @@ static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv) if (master->unique != NULL) drm_unset_busid(dev, master);
- ret = dev->driver->bus->set_busid(dev, master); - if (ret) - goto err; + if (dev->driver->bus && dev->driver->bus->set_busid) { + ret = dev->driver->bus->set_busid(dev, master); + if (ret) { + drm_unset_busid(dev, master); + return ret; + } + } else { + WARN(dev->unique == NULL, + "No drm_bus.set_busid() implementation provided by %ps. " + "Set the unique name explicitly using drm_set_unique().", + dev->driver); + + master->unique = kstrdup(dev->unique, GFP_KERNEL); + if (master->unique) + master->unique_len = strlen(dev->unique); + } + return 0; -err: - drm_unset_busid(dev, master); - return ret; }
/** diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 1447b0ee3676..64cf97dc4ce4 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -535,6 +535,29 @@ static void drm_fs_inode_free(struct inode *inode) }
/** + * drm_set_unique - Set the unique name of a DRM device + * @dev: device of which to set the unique name + * @fmt: format string for unique name + * + * Sets the unique name of a DRM device using the specified format string and + * a variable list of arguments. Drivers can use this at driver probe time if + * the unique name of the devices they drive is static. + */ +int drm_set_unique(struct drm_device *dev, const char *fmt, ...) +{ + va_list ap; + + kfree(dev->unique); + + va_start(ap, fmt); + dev->unique = kvasprintf(GFP_KERNEL, fmt, ap); + va_end(ap); + + return dev->unique ? 0 : -ENOMEM; +} +EXPORT_SYMBOL(drm_set_unique); + +/** * drm_dev_alloc - Allocate new drm device * @driver: DRM driver to allocate device for * @parent: Parent device object @@ -649,6 +672,7 @@ static void drm_dev_release(struct kref *ref) drm_minor_free(dev, DRM_MINOR_CONTROL);
mutex_destroy(&dev->master_mutex); + kfree(dev->unique); kfree(dev); }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 6b4ed9e84e6f..dcec96b1154c 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1235,6 +1235,8 @@ struct drm_device { struct drm_vma_offset_manager *vma_offset_manager; /*@} */ int switch_power_state; + + char *unique; };
#define DRM_SWITCH_POWER_ON 0 @@ -1680,6 +1682,7 @@ static __inline__ void drm_core_dropmap(struct drm_local_map *map)
#include <drm/drm_mem_util.h>
+int drm_set_unique(struct drm_device *dev, const char *fmt, ...); struct drm_device *drm_dev_alloc(struct drm_driver *driver, struct device *parent); void drm_dev_ref(struct drm_device *dev);
Hi
Some minor nitpicks below:
On Tue, May 13, 2014 at 5:30 PM, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Add a helper function that allows drivers to statically set the unique name of the device. This will allow platform and USB drivers to get rid of their DRM bus implementations and directly use drm_dev_alloc() and drm_dev_register().
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Thierry Reding treding@nvidia.com
Changes in v2:
- move drm_set_unique() to drivers/gpu/drm/drm_stub.c
- add kernel-doc
drivers/gpu/drm/drm_ioctl.c | 23 +++++++++++++++++------ drivers/gpu/drm/drm_stub.c | 24 ++++++++++++++++++++++++ include/drm/drmP.h | 3 +++ 3 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 38269d5aa333..a59f6c2d7586 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -131,13 +131,24 @@ static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv) if (master->unique != NULL) drm_unset_busid(dev, master);
ret = dev->driver->bus->set_busid(dev, master);if (ret)goto err;
if (dev->driver->bus && dev->driver->bus->set_busid) {ret = dev->driver->bus->set_busid(dev, master);if (ret) {drm_unset_busid(dev, master);return ret;}} else {WARN(dev->unique == NULL,"No drm_bus.set_busid() implementation provided by %ps. ""Set the unique name explicitly using drm_set_unique().",dev->driver);
return -EINVAL?
If you don't bail out here, the kstrdup() below will cause a kernel-oops. There's no reason to WARN() if you purposely cause an oops below.
master->unique = kstrdup(dev->unique, GFP_KERNEL);if (master->unique)master->unique_len = strlen(dev->unique);}return 0;-err:
drm_unset_busid(dev, master);return ret;}
/** diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 1447b0ee3676..64cf97dc4ce4 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -535,6 +535,29 @@ static void drm_fs_inode_free(struct inode *inode) }
/**
- drm_set_unique - Set the unique name of a DRM device
- @dev: device of which to set the unique name
- @fmt: format string for unique name
- Sets the unique name of a DRM device using the specified format string and
- a variable list of arguments. Drivers can use this at driver probe time if
- the unique name of the devices they drive is static.
- */
+int drm_set_unique(struct drm_device *dev, const char *fmt, ...)
I tried renaming all the device-management APIs to "drm_dev_*", so how about moving this below drm_dev_unregister() and renaming it to drm_dev_set_unique()? I prefer prefixes which describe the object the function works on. set_unique() works on drm_device, so lets use the known drm_dev_* prefix.
Just my opinion, others might disagree..
+{
va_list ap;kfree(dev->unique);va_start(ap, fmt);dev->unique = kvasprintf(GFP_KERNEL, fmt, ap);va_end(ap);return dev->unique ? 0 : -ENOMEM;+} +EXPORT_SYMBOL(drm_set_unique);
+/**
- drm_dev_alloc - Allocate new drm device
- @driver: DRM driver to allocate device for
- @parent: Parent device object
@@ -649,6 +672,7 @@ static void drm_dev_release(struct kref *ref) drm_minor_free(dev, DRM_MINOR_CONTROL);
mutex_destroy(&dev->master_mutex);
kfree(dev->unique); kfree(dev);}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 6b4ed9e84e6f..dcec96b1154c 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1235,6 +1235,8 @@ struct drm_device { struct drm_vma_offset_manager *vma_offset_manager; /*@} */ int switch_power_state;
char *unique;
Can you add this to the group at the top of the object? This somehow belongs to /** \name Lifetime Management */ as it is part of device initialization and API.
};
#define DRM_SWITCH_POWER_ON 0 @@ -1680,6 +1682,7 @@ static __inline__ void drm_core_dropmap(struct drm_local_map *map)
#include <drm/drm_mem_util.h>
+int drm_set_unique(struct drm_device *dev, const char *fmt, ...);
Again just a minor style-nitpick: Move that below the drm_dev_*() declarations, not above. Usual kernel-style is to have allocators/lifetime-management first, followed by runtime management.
Otherwise looks good:
Reviewed-by: David Herrmann dh.herrmann@gmail.com
Thanks David
struct drm_device *drm_dev_alloc(struct drm_driver *driver, struct device *parent); void drm_dev_ref(struct drm_device *dev); -- 1.9.2
From: Thierry Reding treding@nvidia.com
Similarly to what can be done for device drivers, allow driver-specific data to be attached to a master. This is necessary for masters whose device is already bound to by a different driver and therefore cannot be used to store the driver-specific data.
Signed-off-by: Thierry Reding treding@nvidia.com --- Note that I'd like to merge this patch through the Tegra DRM tree so that the Tegra DRM changes later in this patch can be merged at the same time to avoid them being delayed for another release cycle.
In particular I'm looking for an Acked-by from Greg and Russell.
drivers/base/component.c | 13 +++++++++++++ include/linux/component.h | 3 +++ 2 files changed, 16 insertions(+)
diff --git a/drivers/base/component.c b/drivers/base/component.c index c4778995cd72..fb4510707281 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -25,6 +25,8 @@ struct master {
const struct component_master_ops *ops; struct device *dev; + + void *drvdata; };
struct component { @@ -53,6 +55,17 @@ static struct master *__master_find(struct device *dev, return NULL; }
+void master_set_drvdata(struct master *master, void *data) +{ + if (master) + master->drvdata = data; +} + +void *master_get_drvdata(struct master *master) +{ + return master ? master->drvdata : NULL; +} + /* Attach an unattached component to a master. */ static void component_attach_master(struct master *master, struct component *c) { diff --git a/include/linux/component.h b/include/linux/component.h index 68870182ca1e..f130b34b1575 100644 --- a/include/linux/component.h +++ b/include/linux/component.h @@ -26,6 +26,9 @@ int component_master_add(struct device *, const struct component_master_ops *); void component_master_del(struct device *, const struct component_master_ops *);
+void master_set_drvdata(struct master *, void *); +void *master_get_drvdata(struct master *); + int component_master_add_child(struct master *master, int (*compare)(struct device *, void *), void *compare_data);
From: Thierry Reding treding@nvidia.com
Currently the component/master framework allows only a single master to be registered against a struct device. A master is uniquely identified by the device and the master operations table, but the current API does not pass enough information along to allow a master to be uniquely identified when calling component_unbind_all().
To make it possible to register multiple masters on one device, instead of passing around the device associated with a master, pass around the master directly. That way it can always be uniquely identified.
The two existing users of the component/master framework need to be converted as part of this patch because the API is changed in an incompatible way. Since the master needs to be associated with the DRM devices at a precise point, this also drops the drm_platform_init() call and replaces it by direct drm_dev_alloc()/drm_dev_register() usage.
Signed-off-by: Thierry Reding treding@nvidia.com --- Changes in v2: - convert existing users (staging/imx-drm, gpu/drm/msm)
Notes: I'd like to take this through the Tegra DRM tree if possible so that the Tegra DRM changes later in this series can be merged at the same time so as not to delay them for another release cycle.
In particular I'm looking for an Acked-by from Greg and Russell.
drivers/base/component.c | 18 +++---- drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 8 ++-- drivers/gpu/drm/msm/hdmi/hdmi.c | 8 ++-- drivers/gpu/drm/msm/msm_drv.c | 75 ++++++++++++++++++++---------- drivers/gpu/drm/msm/msm_drv.h | 1 + drivers/staging/imx-drm/imx-drm-core.c | 58 ++++++++++++++--------- drivers/staging/imx-drm/imx-hdmi.c | 4 +- drivers/staging/imx-drm/imx-ldb.c | 4 +- drivers/staging/imx-drm/imx-tve.c | 4 +- drivers/staging/imx-drm/ipuv3-crtc.c | 4 +- drivers/staging/imx-drm/parallel-display.c | 4 +- include/linux/component.h | 17 ++++--- 12 files changed, 120 insertions(+), 85 deletions(-)
diff --git a/drivers/base/component.c b/drivers/base/component.c index fb4510707281..e3693c6d552f 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -133,7 +133,7 @@ static int try_to_bring_up_master(struct master *master, * Search the list of components, looking for components that * belong to this master, and attach them to the master. */ - if (master->ops->add_components(master->dev, master)) { + if (master->ops->add_components(master, master->dev)) { /* Failed to find all components */ master_remove_components(master); ret = 0; @@ -152,7 +152,7 @@ static int try_to_bring_up_master(struct master *master, }
/* Found all components */ - ret = master->ops->bind(master->dev); + ret = master->ops->bind(master, master->dev); if (ret < 0) { devres_release_group(master->dev, NULL); dev_info(master->dev, "master bind failed: %d\n", ret); @@ -185,7 +185,7 @@ static int try_to_bring_up_masters(struct component *component) static void take_down_master(struct master *master) { if (master->bound) { - master->ops->unbind(master->dev); + master->ops->unbind(master, master->dev); devres_release_group(master->dev, NULL); master->bound = false; } @@ -246,21 +246,19 @@ static void component_unbind(struct component *component, { WARN_ON(!component->bound);
- component->ops->unbind(component->dev, master->dev, data); + component->ops->unbind(component->dev, master, data); component->bound = false;
/* Release all resources claimed in the binding of this component */ devres_release_group(component->dev, component); }
-void component_unbind_all(struct device *master_dev, void *data) +void component_unbind_all(struct master *master, void *data) { - struct master *master; struct component *c;
WARN_ON(!mutex_is_locked(&component_mutex));
- master = __master_find(master_dev, NULL); if (!master) return;
@@ -295,7 +293,7 @@ static int component_bind(struct component *component, struct master *master, dev_dbg(master->dev, "binding %s (ops %ps)\n", dev_name(component->dev), component->ops);
- ret = component->ops->bind(component->dev, master->dev, data); + ret = component->ops->bind(component->dev, master, data); if (!ret) { component->bound = true;
@@ -321,15 +319,13 @@ static int component_bind(struct component *component, struct master *master, return ret; }
-int component_bind_all(struct device *master_dev, void *data) +int component_bind_all(struct master *master, void *data) { - struct master *master; struct component *c; int ret = 0;
WARN_ON(!mutex_is_locked(&component_mutex));
- master = __master_find(master_dev, NULL); if (!master) return -EINVAL;
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c index f20fbde5dc49..abd70fa8fed8 100644 --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c @@ -560,7 +560,7 @@ static void set_gpu_pdev(struct drm_device *dev, priv->gpu_pdev = pdev; }
-static int a3xx_bind(struct device *dev, struct device *master, void *data) +static int a3xx_bind(struct device *dev, struct master *master, void *data) { static struct adreno_platform_config config = {}; #ifdef CONFIG_OF @@ -648,14 +648,14 @@ static int a3xx_bind(struct device *dev, struct device *master, void *data) # endif #endif dev->platform_data = &config; - set_gpu_pdev(dev_get_drvdata(master), to_platform_device(dev)); + set_gpu_pdev(master_get_drvdata(master), to_platform_device(dev)); return 0; }
-static void a3xx_unbind(struct device *dev, struct device *master, +static void a3xx_unbind(struct device *dev, struct master *master, void *data) { - set_gpu_pdev(dev_get_drvdata(master), NULL); + set_gpu_pdev(master_get_drvdata(master), NULL); }
static const struct component_ops a3xx_ops = { diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index ae750f6928c1..1bde0ed59795 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -256,7 +256,7 @@ static void set_hdmi_pdev(struct drm_device *dev, priv->hdmi_pdev = pdev; }
-static int hdmi_bind(struct device *dev, struct device *master, void *data) +static int hdmi_bind(struct device *dev, struct master *master, void *data) { static struct hdmi_platform_config config = {}; #ifdef CONFIG_OF @@ -344,14 +344,14 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data) } #endif dev->platform_data = &config; - set_hdmi_pdev(dev_get_drvdata(master), to_platform_device(dev)); + set_hdmi_pdev(master_get_drvdata(master), to_platform_device(dev)); return 0; }
-static void hdmi_unbind(struct device *dev, struct device *master, +static void hdmi_unbind(struct device *dev, struct master *master, void *data) { - set_hdmi_pdev(dev_get_drvdata(master), NULL); + set_hdmi_pdev(master_get_drvdata(master), NULL); }
static const struct component_ops hdmi_ops = { diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index bcee218b150b..f2fcd17b439f 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -147,7 +147,7 @@ static int msm_unload(struct drm_device *dev) priv->vram.paddr, &attrs); }
- component_unbind_all(dev->dev, dev); + component_unbind_all(priv->master, dev);
dev->dev_private = NULL;
@@ -177,19 +177,10 @@ static int get_mdp_ver(struct platform_device *pdev) static int msm_load(struct drm_device *dev, unsigned long flags) { struct platform_device *pdev = dev->platformdev; - struct msm_drm_private *priv; + struct msm_drm_private *priv = dev->dev_private; struct msm_kms *kms; int ret;
- - priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (!priv) { - dev_err(dev->dev, "failed to allocate private data\n"); - return -ENOMEM; - } - - dev->dev_private = priv; - priv->wq = alloc_ordered_workqueue("msm", 0); init_waitqueue_head(&priv->fence_event);
@@ -236,7 +227,7 @@ static int msm_load(struct drm_device *dev, unsigned long flags) platform_set_drvdata(pdev, dev);
/* Bind all our sub-components: */ - ret = component_bind_all(dev->dev, dev); + ret = component_bind_all(priv->master, dev); if (ret) return ret;
@@ -889,9 +880,9 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == data; }
-static int msm_drm_add_components(struct device *master, struct master *m) +static int msm_drm_add_components(struct master *master, struct device *dev) { - struct device_node *np = master->of_node; + struct device_node *np = dev->of_node; unsigned i; int ret;
@@ -902,7 +893,7 @@ static int msm_drm_add_components(struct device *master, struct master *m) if (!node) break;
- ret = component_master_add_child(m, compare_of, node); + ret = component_master_add_child(master, compare_of, node); of_node_put(node);
if (ret) @@ -916,7 +907,7 @@ static int compare_dev(struct device *dev, void *data) return dev == data; }
-static int msm_drm_add_components(struct device *master, struct master *m) +static int msm_drm_add_components(struct master *master, struct device *dev) { /* For non-DT case, it kinda sucks. We don't actually have a way * to know whether or not we are waiting for certain devices (or if @@ -931,17 +922,17 @@ static int msm_drm_add_components(struct device *master, struct master *m) DBG("Adding components..");
for (i = 0; i < ARRAY_SIZE(devnames); i++) { - struct device *dev; + struct device *component; int ret;
- dev = bus_find_device_by_name(&platform_bus_type, + component = bus_find_device_by_name(&platform_bus_type, NULL, devnames[i]); - if (!dev) { - dev_info(master, "still waiting for %s\n", devnames[i]); + if (!component) { + dev_info(dev, "still waiting for %s\n", devnames[i]); return -EPROBE_DEFER; }
- ret = component_master_add_child(m, compare_dev, dev); + ret = component_master_add_child(master, compare_dev, component); if (ret) { DBG("could not add child: %d", ret); return ret; @@ -952,14 +943,48 @@ static int msm_drm_add_components(struct device *master, struct master *m) } #endif
-static int msm_drm_bind(struct device *dev) +static int msm_drm_bind(struct master *master, struct device *dev) { - return drm_platform_init(&msm_driver, to_platform_device(dev)); + struct msm_drm_private *priv; + struct drm_device *drm; + int err; + + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) { + dev_err(dev, "failed to allocate private data\n"); + return -ENOMEM; + } + + priv->master = master; + + drm = drm_dev_alloc(&msm_driver, dev); + if (!drm) { + err = -ENOMEM; + goto free; + } + + drm_set_unique(drm, "%s", dev_name(dev)); + master_set_drvdata(master, drm); + drm->dev_private = priv; + + err = drm_dev_register(drm, 0); + if (err < 0) + goto unref; + + return 0; + +unref: + drm_dev_unref(drm); +free: + kfree(priv); + return err; }
-static void msm_drm_unbind(struct device *dev) +static void msm_drm_unbind(struct master *master, struct device *dev) { - drm_put_dev(platform_get_drvdata(to_platform_device(dev))); + struct drm_device *drm = master_get_drvdata(master); + + drm_put_dev(drm); }
static const struct component_master_ops msm_drm_ops = { diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index e5797d8ad927..3b5ca492c598 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -68,6 +68,7 @@ struct msm_file_private { };
struct msm_drm_private { + struct master *master;
struct msm_kms *kms;
diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c index c270c9ae6d27..ded69beb0652 100644 --- a/drivers/staging/imx-drm/imx-drm-core.c +++ b/drivers/staging/imx-drm/imx-drm-core.c @@ -37,6 +37,7 @@ struct imx_drm_component { };
struct imx_drm_device { + struct master *master; struct drm_device *drm; struct imx_drm_crtc *crtc[MAX_CRTC]; int pipes; @@ -71,9 +72,7 @@ static void imx_drm_driver_lastclose(struct drm_device *drm)
static int imx_drm_driver_unload(struct drm_device *drm) { -#if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER) struct imx_drm_device *imxdrm = drm->dev_private; -#endif
drm_kms_helper_poll_fini(drm);
@@ -82,7 +81,7 @@ static int imx_drm_driver_unload(struct drm_device *drm) drm_fbdev_cma_fini(imxdrm->fbhelper); #endif
- component_unbind_all(drm->dev, drm); + component_unbind_all(imxdrm->master, drm);
drm_vblank_cleanup(drm); drm_mode_config_cleanup(drm); @@ -240,18 +239,10 @@ static struct drm_mode_config_funcs imx_drm_mode_config_funcs = { */ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags) { - struct imx_drm_device *imxdrm; + struct imx_drm_device *imxdrm = drm->dev_private; struct drm_connector *connector; int ret;
- imxdrm = devm_kzalloc(drm->dev, sizeof(*imxdrm), GFP_KERNEL); - if (!imxdrm) - return -ENOMEM; - - imxdrm->drm = drm; - - drm->dev_private = imxdrm; - /* * enable drm irq mode. * - with irq_enabled = true, we can use the vblank feature. @@ -287,10 +278,8 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags) */ drm->vblank_disable_allowed = true;
- platform_set_drvdata(drm->platformdev, drm); - /* Now try and bind all our sub-components */ - ret = component_bind_all(drm->dev, drm); + ret = component_bind_all(imxdrm->master, drm); if (ret) goto err_vblank;
@@ -334,7 +323,7 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags) return 0;
err_unbind: - component_unbind_all(drm->dev, drm); + component_unbind_all(imxdrm->master, drm); err_vblank: drm_vblank_cleanup(drm); err_kms: @@ -579,13 +568,13 @@ static int compare_of(struct device *dev, void *data)
static LIST_HEAD(imx_drm_components);
-static int imx_drm_add_components(struct device *master, struct master *m) +static int imx_drm_add_components(struct master *master, struct device *dev) { struct imx_drm_component *component; int ret;
list_for_each_entry(component, &imx_drm_components, list) { - ret = component_master_add_child(m, compare_of, + ret = component_master_add_child(master, compare_of, component->of_node); if (ret) return ret; @@ -593,14 +582,39 @@ static int imx_drm_add_components(struct device *master, struct master *m) return 0; }
-static int imx_drm_bind(struct device *dev) +static int imx_drm_bind(struct master *master, struct device *dev) { - return drm_platform_init(&imx_drm_driver, to_platform_device(dev)); + struct imx_drm_device *imxdrm; + int err; + + imxdrm = devm_kzalloc(dev, sizeof(*imxdrm), GFP_KERNEL); + if (!imxdrm) + return -ENOMEM; + + imxdrm->master = master; + + imxdrm->drm = drm_dev_alloc(&imx_drm_driver, dev); + if (!imxdrm->drm) + return -ENOMEM; + + drm_set_unique(imxdrm->drm, "%s", dev_name(dev)); + master_set_drvdata(master, imxdrm); + imxdrm->drm->dev_private = imxdrm; + + err = drm_dev_register(imxdrm->drm, 0); + if (err < 0) { + drm_dev_unref(imxdrm->drm); + return err; + } + + return 0; }
-static void imx_drm_unbind(struct device *dev) +static void imx_drm_unbind(struct master *master, struct device *dev) { - drm_put_dev(dev_get_drvdata(dev)); + struct imx_drm_device *imxdrm = master_get_drvdata(master); + + drm_put_dev(imxdrm->drm); }
static const struct component_master_ops imx_drm_ops = { diff --git a/drivers/staging/imx-drm/imx-hdmi.c b/drivers/staging/imx-drm/imx-hdmi.c index 886a0d49031b..9ec69a25ade5 100644 --- a/drivers/staging/imx-drm/imx-hdmi.c +++ b/drivers/staging/imx-drm/imx-hdmi.c @@ -1584,7 +1584,7 @@ static const struct of_device_id imx_hdmi_dt_ids[] = { }; MODULE_DEVICE_TABLE(of, imx_hdmi_dt_ids);
-static int imx_hdmi_bind(struct device *dev, struct device *master, void *data) +static int imx_hdmi_bind(struct device *dev, struct master *master, void *data) { struct platform_device *pdev = to_platform_device(dev); const struct of_device_id *of_id = @@ -1717,7 +1717,7 @@ err_isfr: return ret; }
-static void imx_hdmi_unbind(struct device *dev, struct device *master, +static void imx_hdmi_unbind(struct device *dev, struct master *master, void *data) { struct imx_hdmi *hdmi = dev_get_drvdata(dev); diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c index fe4c1ef4e7a5..b42bc2e01b60 100644 --- a/drivers/staging/imx-drm/imx-ldb.c +++ b/drivers/staging/imx-drm/imx-ldb.c @@ -436,7 +436,7 @@ static const struct of_device_id imx_ldb_dt_ids[] = { }; MODULE_DEVICE_TABLE(of, imx_ldb_dt_ids);
-static int imx_ldb_bind(struct device *dev, struct device *master, void *data) +static int imx_ldb_bind(struct device *dev, struct master *master, void *data) { struct drm_device *drm = data; struct device_node *np = dev->of_node; @@ -566,7 +566,7 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) return 0; }
-static void imx_ldb_unbind(struct device *dev, struct device *master, +static void imx_ldb_unbind(struct device *dev, struct master *master, void *data) { struct imx_ldb *imx_ldb = dev_get_drvdata(dev); diff --git a/drivers/staging/imx-drm/imx-tve.c b/drivers/staging/imx-drm/imx-tve.c index a23f4f773146..5e6821461f9b 100644 --- a/drivers/staging/imx-drm/imx-tve.c +++ b/drivers/staging/imx-drm/imx-tve.c @@ -562,7 +562,7 @@ static const int of_get_tve_mode(struct device_node *np) return -EINVAL; }
-static int imx_tve_bind(struct device *dev, struct device *master, void *data) +static int imx_tve_bind(struct device *dev, struct master *master, void *data) { struct platform_device *pdev = to_platform_device(dev); struct drm_device *drm = data; @@ -689,7 +689,7 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) return 0; }
-static void imx_tve_unbind(struct device *dev, struct device *master, +static void imx_tve_unbind(struct device *dev, struct master *master, void *data) { struct imx_tve *tve = dev_get_drvdata(dev); diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c index 47bec5e17358..866b2a74108c 100644 --- a/drivers/staging/imx-drm/ipuv3-crtc.c +++ b/drivers/staging/imx-drm/ipuv3-crtc.c @@ -429,7 +429,7 @@ static struct device_node *ipu_drm_get_port_by_id(struct device_node *parent, return NULL; }
-static int ipu_drm_bind(struct device *dev, struct device *master, void *data) +static int ipu_drm_bind(struct device *dev, struct master *master, void *data) { struct ipu_client_platformdata *pdata = dev->platform_data; struct drm_device *drm = data; @@ -451,7 +451,7 @@ static int ipu_drm_bind(struct device *dev, struct device *master, void *data) return 0; }
-static void ipu_drm_unbind(struct device *dev, struct device *master, +static void ipu_drm_unbind(struct device *dev, struct master *master, void *data) { struct ipu_crtc *ipu_crtc = dev_get_drvdata(dev); diff --git a/drivers/staging/imx-drm/parallel-display.c b/drivers/staging/imx-drm/parallel-display.c index eaf4dda1a0c4..d2bfe1e9d32d 100644 --- a/drivers/staging/imx-drm/parallel-display.c +++ b/drivers/staging/imx-drm/parallel-display.c @@ -193,7 +193,7 @@ static int imx_pd_register(struct drm_device *drm, return 0; }
-static int imx_pd_bind(struct device *dev, struct device *master, void *data) +static int imx_pd_bind(struct device *dev, struct master *master, void *data) { struct drm_device *drm = data; struct device_node *np = dev->of_node; @@ -238,7 +238,7 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) return 0; }
-static void imx_pd_unbind(struct device *dev, struct device *master, +static void imx_pd_unbind(struct device *dev, struct master *master, void *data) { struct imx_parallel_display *imxpd = dev_get_drvdata(dev); diff --git a/include/linux/component.h b/include/linux/component.h index f130b34b1575..9c2ec9fe48d6 100644 --- a/include/linux/component.h +++ b/include/linux/component.h @@ -2,24 +2,23 @@ #define COMPONENT_H
struct device; +struct master;
struct component_ops { - int (*bind)(struct device *, struct device *, void *); - void (*unbind)(struct device *, struct device *, void *); + int (*bind)(struct device *, struct master *, void *); + void (*unbind)(struct device *, struct master *, void *); };
int component_add(struct device *, const struct component_ops *); void component_del(struct device *, const struct component_ops *);
-int component_bind_all(struct device *, void *); -void component_unbind_all(struct device *, void *); - -struct master; +int component_bind_all(struct master *, void *); +void component_unbind_all(struct master *, void *);
struct component_master_ops { - int (*add_components)(struct device *, struct master *); - int (*bind)(struct device *); - void (*unbind)(struct device *); + int (*add_components)(struct master *, struct device *); + int (*bind)(struct master *, struct device *); + void (*unbind)(struct master *, struct device *); };
int component_master_add(struct device *, const struct component_master_ops *);
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
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
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.oobj-$(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 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 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 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
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: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
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
Hi Thierry, Greg,
On 05/15/2014 10:53 AM, Thierry Reding 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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
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...
I have posted some times ago RFC for interface_tracker framework [1]. It seems with interface_tracker you can achieve similar functionality and it seems to be more generic.
[1]: https://lkml.org/lkml/2014/4/30/345
Two small things should be added to interface_tracker framework: - matching objects using string comparison, - before notifier unregistration call notifier to inform that observed interface will disappear for him.
Greg, this is another use case for interface_tracker framework.
To show how it could be achieved I present pseudo patch which converts tegra drivers to interface_tracker. Obviously I have not tested it.
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 11d0deb..79fcb43 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -728,27 +728,27 @@ static const struct component_master_ops tegra_drm_master_ops = { .unbind = tegra_drm_unbind, };
-static int host1x_interface_bind(struct interface *intf) -{ - return component_master_add(intf->dev, &tegra_drm_master_ops); -} - -static void host1x_interface_unbind(struct interface *intf) -{ - component_master_del(intf->dev, &tegra_drm_master_ops); -} - -static struct interface_driver host1x_interface_driver = { - .name = "nvidia,tegra-host1x", - .bind = host1x_interface_bind, - .unbind = host1x_interface_unbind, -}; +void itb_callback(struct interface_tracker_block *itb, const void *object, + unsigned long type, bool on, void *data) +{ + struct device *dev = data; + + if (on) + component_master_add(dev, &tegra_drm_master_ops); + else + component_master_del(dev, &tegra_drm_master_ops); +} +static struct interface_tracker_block itb = { + .callback = itb_callback +};
static int __init tegra_drm_init(void) { int err;
- err = interface_register_driver(&host1x_interface_driver); + err = interface_tracker_register("nvidia,tegra-host1x", + INTERFACE_TRACKER_TYPE_PARENT_DEV, &itb); if (err < 0) return err;
@@ -795,7 +795,8 @@ unregister_dsi: unregister_dc: platform_driver_unregister(&tegra_dc_driver); unregister_host1x: - interface_unregister_driver(&host1x_interface_driver); + interface_tracker_unregister("nvidia,tegra-host1x", + INTERFACE_TRACKER_TYPE_PARENT_DEV, &itb); return err; } module_init(tegra_drm_init); @@ -809,7 +810,8 @@ static void __exit tegra_drm_exit(void) platform_driver_unregister(&tegra_sor_driver); platform_driver_unregister(&tegra_dsi_driver); platform_driver_unregister(&tegra_dc_driver); - interface_unregister_driver(&host1x_interface_driver); + interface_tracker_unregister("nvidia,tegra-host1x", + INTERFACE_TRACKER_TYPE_PARENT_DEV, &itb); } module_exit(tegra_drm_exit);
diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index b92812b..f4455c5 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -175,10 +175,9 @@ static int host1x_probe(struct platform_device *pdev)
host1x_debug_init(host);
- host->interface.name = "nvidia,tegra-host1x"; - host->interface.dev = &pdev->dev; - - err = interface_add(&host->interface); + err = interface_tracker_ifup("nvidia,tegra-host1x", + INTERFACE_TRACKER_TYPE_PARENT_DEV, &pdev->dev); if (err < 0) goto fail_deinit_intr;
@@ -197,7 +196,9 @@ static int host1x_remove(struct platform_device *pdev) { struct host1x *host = platform_get_drvdata(pdev);
- interface_remove(&host->interface); + err = interface_tracker_ifdown("nvidia,tegra-host1x", + INTERFACE_TRACKER_TYPE_PARENT_DEV, &pdev->dev); host1x_intr_deinit(host); host1x_syncpt_deinit(host); clk_disable_unprepare(host->clk);
Regards Andrzej
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
From: Thierry Reding treding@nvidia.com
Instead of the current implementation, reuse the recently introduced master/component framework, which is equivalent in most regards. One issue is that there is no device to bind the DRM driver to. In order to still allow the driver to be probed, expose an interface from the host1x device and provide an interface driver to bind to that.
The interface driver then registers (or removes) the master that will be used to instantiate and bind the DRM driver. Since the drm_host1x bus implementation is now gone the dummy device instantiated by it can no longer be used as the parent for the DRM device. However since the parent device doesn't need to be modified, the host1x parent device that exposes the interface can be used instead.
Signed-off-by: Thierry Reding treding@nvidia.com --- drivers/gpu/drm/tegra/Makefile | 1 - drivers/gpu/drm/tegra/bus.c | 64 ----- drivers/gpu/drm/tegra/dc.c | 58 ++--- drivers/gpu/drm/tegra/drm.c | 171 +++++++++---- drivers/gpu/drm/tegra/drm.h | 27 +- drivers/gpu/drm/tegra/dsi.c | 144 +++++------ drivers/gpu/drm/tegra/gr2d.c | 78 +++--- drivers/gpu/drm/tegra/gr3d.c | 77 +++--- drivers/gpu/drm/tegra/hdmi.c | 69 ++--- drivers/gpu/drm/tegra/sor.c | 71 ++---- drivers/gpu/host1x/Makefile | 1 - drivers/gpu/host1x/bus.c | 553 ----------------------------------------- drivers/gpu/host1x/bus.h | 29 --- drivers/gpu/host1x/dev.c | 21 +- drivers/gpu/host1x/dev.h | 7 +- include/linux/host1x.h | 64 +---- 16 files changed, 332 insertions(+), 1103 deletions(-) delete mode 100644 drivers/gpu/drm/tegra/bus.c delete mode 100644 drivers/gpu/host1x/bus.c delete mode 100644 drivers/gpu/host1x/bus.h
diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile index 905f6cad1061..e24584eb2e58 100644 --- a/drivers/gpu/drm/tegra/Makefile +++ b/drivers/gpu/drm/tegra/Makefile @@ -1,7 +1,6 @@ ccflags-$(CONFIG_DRM_TEGRA_DEBUG) += -DDEBUG
tegra-drm-y := \ - bus.o \ drm.o \ gem.o \ fb.o \ diff --git a/drivers/gpu/drm/tegra/bus.c b/drivers/gpu/drm/tegra/bus.c deleted file mode 100644 index b3a66d65cb53..000000000000 --- a/drivers/gpu/drm/tegra/bus.c +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright (C) 2013 NVIDIA Corporation - * - * 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. - */ - -#include "drm.h" - -static int drm_host1x_set_busid(struct drm_device *dev, - struct drm_master *master) -{ - const char *device = dev_name(dev->dev); - const char *bus = dev->dev->bus->name; - - master->unique_len = strlen(bus) + 1 + strlen(device); - master->unique_size = master->unique_len; - - master->unique = kmalloc(master->unique_len + 1, GFP_KERNEL); - if (!master->unique) - return -ENOMEM; - - snprintf(master->unique, master->unique_len + 1, "%s:%s", bus, device); - - return 0; -} - -static struct drm_bus drm_host1x_bus = { - .set_busid = drm_host1x_set_busid, -}; - -int drm_host1x_init(struct drm_driver *driver, struct host1x_device *device) -{ - struct drm_device *drm; - int ret; - - driver->bus = &drm_host1x_bus; - - drm = drm_dev_alloc(driver, &device->dev); - if (!drm) - return -ENOMEM; - - ret = drm_dev_register(drm, 0); - if (ret) - goto err_free; - - DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n", driver->name, - driver->major, driver->minor, driver->patchlevel, - driver->date, drm->primary->index); - - return 0; - -err_free: - drm_dev_unref(drm); - return ret; -} - -void drm_host1x_exit(struct drm_driver *driver, struct host1x_device *device) -{ - struct tegra_drm *tegra = dev_get_drvdata(&device->dev); - - drm_put_dev(tegra->drm); -} diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 859e424e15e5..03f8520ed35c 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -1102,10 +1102,10 @@ static int tegra_dc_debugfs_exit(struct tegra_dc *dc) return 0; }
-static int tegra_dc_init(struct host1x_client *client) +static int tegra_dc_bind(struct device *dev, struct master *master, void *data) { - struct tegra_drm *tegra = dev_get_drvdata(client->parent); - struct tegra_dc *dc = host1x_client_to_dc(client); + struct tegra_drm *tegra = master_get_drvdata(master); + struct tegra_dc *dc = dev_get_drvdata(dev); int err;
drm_crtc_init(tegra->drm, &dc->base, &tegra_crtc_funcs); @@ -1114,7 +1114,7 @@ static int tegra_dc_init(struct host1x_client *client)
err = tegra_dc_rgb_init(tegra->drm, dc); if (err < 0 && err != -ENODEV) { - dev_err(dc->dev, "failed to initialize RGB output: %d\n", err); + dev_err(dev, "failed to initialize RGB output: %d\n", err); return err; }
@@ -1125,23 +1125,23 @@ static int tegra_dc_init(struct host1x_client *client) if (IS_ENABLED(CONFIG_DEBUG_FS)) { err = tegra_dc_debugfs_init(dc, tegra->drm->primary); if (err < 0) - dev_err(dc->dev, "debugfs setup failed: %d\n", err); + dev_err(dev, "debugfs setup failed: %d\n", err); }
- err = devm_request_irq(dc->dev, dc->irq, tegra_dc_irq, 0, - dev_name(dc->dev), dc); + err = devm_request_irq(dev, dc->irq, tegra_dc_irq, 0, dev_name(dev), + dc); if (err < 0) { - dev_err(dc->dev, "failed to request IRQ#%u: %d\n", dc->irq, - err); + dev_err(dev, "failed to request IRQ#%u: %d\n", dc->irq, err); return err; }
return 0; }
-static int tegra_dc_exit(struct host1x_client *client) +static void tegra_dc_unbind(struct device *dev, struct master *master, + void *data) { - struct tegra_dc *dc = host1x_client_to_dc(client); + struct tegra_dc *dc = dev_get_drvdata(dev); int err;
devm_free_irq(dc->dev, dc->irq, dc); @@ -1149,21 +1149,17 @@ static int tegra_dc_exit(struct host1x_client *client) if (IS_ENABLED(CONFIG_DEBUG_FS)) { err = tegra_dc_debugfs_exit(dc); if (err < 0) - dev_err(dc->dev, "debugfs cleanup failed: %d\n", err); + dev_err(dev, "debugfs cleanup failed: %d\n", err); }
err = tegra_dc_rgb_exit(dc); - if (err) { - dev_err(dc->dev, "failed to shutdown RGB output: %d\n", err); - return err; - } - - return 0; + if (err) + dev_err(dev, "failed to shutdown RGB output: %d\n", err); }
-static const struct host1x_client_ops dc_client_ops = { - .init = tegra_dc_init, - .exit = tegra_dc_exit, +static const struct component_ops tegra_dc_component_ops = { + .bind = tegra_dc_bind, + .unbind = tegra_dc_unbind, };
static const struct tegra_dc_soc_info tegra20_dc_soc_info = { @@ -1239,6 +1235,8 @@ static int tegra_dc_probe(struct platform_device *pdev) if (!dc) return -ENOMEM;
+ platform_set_drvdata(pdev, dc); + id = of_match_node(tegra_dc_of_match, pdev->dev.of_node); if (!id) return -ENODEV; @@ -1279,25 +1277,18 @@ static int tegra_dc_probe(struct platform_device *pdev) return -ENXIO; }
- INIT_LIST_HEAD(&dc->client.list); - dc->client.ops = &dc_client_ops; - dc->client.dev = &pdev->dev; - err = tegra_dc_rgb_probe(dc); if (err < 0 && err != -ENODEV) { dev_err(&pdev->dev, "failed to probe RGB output: %d\n", err); return err; }
- err = host1x_client_register(&dc->client); + err = component_add(&pdev->dev, &tegra_dc_component_ops); if (err < 0) { - dev_err(&pdev->dev, "failed to register host1x client: %d\n", - err); + dev_err(&pdev->dev, "failed to register component: %d\n", err); return err; }
- platform_set_drvdata(pdev, dc); - return 0; }
@@ -1306,12 +1297,7 @@ static int tegra_dc_remove(struct platform_device *pdev) struct tegra_dc *dc = platform_get_drvdata(pdev); int err;
- err = host1x_client_unregister(&dc->client); - if (err < 0) { - dev_err(&pdev->dev, "failed to unregister host1x client: %d\n", - err); - return err; - } + component_del(&pdev->dev, &tegra_dc_component_ops);
err = tegra_dc_rgb_remove(dc); if (err < 0) { diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index d492c2f12ca8..78f7ab6d355b 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -7,7 +7,9 @@ * published by the Free Software Foundation. */
+#include <linux/component.h> #include <linux/host1x.h> +#include <linux/interface.h>
#include "drm.h" #include "gem.h" @@ -25,20 +27,9 @@ struct tegra_drm_file {
static int tegra_drm_load(struct drm_device *drm, unsigned long flags) { - struct host1x_device *device = to_host1x_device(drm->dev); - struct tegra_drm *tegra; + struct tegra_drm *tegra = drm->dev_private; int err;
- tegra = kzalloc(sizeof(*tegra), GFP_KERNEL); - if (!tegra) - return -ENOMEM; - - dev_set_drvdata(drm->dev, tegra); - mutex_init(&tegra->clients_lock); - INIT_LIST_HEAD(&tegra->clients); - drm->dev_private = tegra; - tegra->drm = drm; - drm_mode_config_init(drm);
err = tegra_drm_fb_prepare(drm); @@ -47,7 +38,7 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
drm_kms_helper_poll_init(drm);
- err = host1x_device_init(device); + err = component_bind_all(tegra->master, drm); if (err < 0) return err;
@@ -71,17 +62,14 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
static int tegra_drm_unload(struct drm_device *drm) { - struct host1x_device *device = to_host1x_device(drm->dev); - int err; + struct tegra_drm *tegra = drm->dev_private;
drm_kms_helper_poll_fini(drm); tegra_drm_fb_exit(drm); - drm_vblank_cleanup(drm); drm_mode_config_cleanup(drm); + drm_vblank_cleanup(drm);
- err = host1x_device_exit(device); - if (err < 0) - return err; + component_unbind_all(tegra->master, drm);
return 0; } @@ -162,7 +150,7 @@ int tegra_drm_submit(struct tegra_drm_context *context, job->num_relocs = args->num_relocs; job->num_waitchk = args->num_waitchks; job->client = (u32)args->context; - job->class = context->client->base.class; + job->class = context->client->class; job->serialize = true;
while (num_cmdbufs) { @@ -227,7 +215,7 @@ int tegra_drm_submit(struct tegra_drm_context *context, if (args->timeout && args->timeout < 10000) job->timeout = args->timeout;
- err = host1x_job_pin(job, context->client->base.dev); + err = host1x_job_pin(job, context->client->dev); if (err) goto fail;
@@ -303,7 +291,7 @@ static int tegra_gem_mmap(struct drm_device *drm, void *data, static int tegra_syncpt_read(struct drm_device *drm, void *data, struct drm_file *file) { - struct host1x *host = dev_get_drvdata(drm->dev->parent); + struct host1x *host = dev_get_drvdata(drm->dev); struct drm_tegra_syncpt_read *args = data; struct host1x_syncpt *sp;
@@ -318,7 +306,7 @@ static int tegra_syncpt_read(struct drm_device *drm, void *data, static int tegra_syncpt_incr(struct drm_device *drm, void *data, struct drm_file *file) { - struct host1x *host1x = dev_get_drvdata(drm->dev->parent); + struct host1x *host1x = dev_get_drvdata(drm->dev); struct drm_tegra_syncpt_incr *args = data; struct host1x_syncpt *sp;
@@ -332,7 +320,7 @@ static int tegra_syncpt_incr(struct drm_device *drm, void *data, static int tegra_syncpt_wait(struct drm_device *drm, void *data, struct drm_file *file) { - struct host1x *host1x = dev_get_drvdata(drm->dev->parent); + struct host1x *host1x = dev_get_drvdata(drm->dev); struct drm_tegra_syncpt_wait *args = data; struct host1x_syncpt *sp;
@@ -359,7 +347,7 @@ static int tegra_open_channel(struct drm_device *drm, void *data, return -ENOMEM;
list_for_each_entry(client, &tegra->clients, list) - if (client->base.class == args->client) { + if (client->class == args->client) { err = client->ops->open_channel(client, context); if (err) break; @@ -405,10 +393,10 @@ static int tegra_get_syncpt(struct drm_device *drm, void *data, if (!tegra_drm_file_owns_context(fpriv, context)) return -ENODEV;
- if (args->index >= context->client->base.num_syncpts) + if (args->index >= context->client->num_syncpts) return -EINVAL;
- syncpt = context->client->base.syncpts[args->index]; + syncpt = context->client->syncpts[args->index]; args->id = host1x_syncpt_id(syncpt);
return 0; @@ -443,10 +431,10 @@ static int tegra_get_syncpt_base(struct drm_device *drm, void *data, if (!tegra_drm_file_owns_context(fpriv, context)) return -ENODEV;
- if (args->syncpt >= context->client->base.num_syncpts) + if (args->syncpt >= context->client->num_syncpts) return -EINVAL;
- syncpt = context->client->base.syncpts[args->syncpt]; + syncpt = context->client->syncpts[args->syncpt];
base = host1x_syncpt_get_base(syncpt); if (!base) @@ -644,19 +632,7 @@ int tegra_drm_unregister_client(struct tegra_drm *tegra, return 0; }
-static int host1x_drm_probe(struct host1x_device *device) -{ - return drm_host1x_init(&tegra_drm_driver, device); -} - -static int host1x_drm_remove(struct host1x_device *device) -{ - drm_host1x_exit(&tegra_drm_driver, device); - - return 0; -} - -static const struct of_device_id host1x_drm_subdevs[] = { +static const struct of_device_id tegra_drm_subdevs[] = { { .compatible = "nvidia,tegra20-dc", }, { .compatible = "nvidia,tegra20-hdmi", }, { .compatible = "nvidia,tegra20-gr2d", }, @@ -674,18 +650,105 @@ static const struct of_device_id host1x_drm_subdevs[] = { { /* sentinel */ } };
-static struct host1x_driver host1x_drm_driver = { - .name = "drm", - .probe = host1x_drm_probe, - .remove = host1x_drm_remove, - .subdevs = host1x_drm_subdevs, +static int tegra_drm_compare(struct device *dev, void *data) +{ + struct device_node *np = data; + + return dev->of_node == np; +} + +static int tegra_drm_add_components(struct master *master, struct device *dev) +{ + struct device_node *np; + int err = 0; + + for_each_child_of_node(dev->of_node, np) { + if (!of_device_is_available(np)) + continue; + + if (!of_match_node(tegra_drm_subdevs, np)) + continue; + + err = component_master_add_child(master, tegra_drm_compare, np); + if (err < 0) + break; + } + + return err; +} + +static int tegra_drm_bind(struct master *master, struct device *dev) +{ + struct tegra_drm *tegra; + struct drm_device *drm; + int err; + + tegra = kzalloc(sizeof(*tegra), GFP_KERNEL); + if (!tegra) + return -ENOMEM; + + mutex_init(&tegra->clients_lock); + INIT_LIST_HEAD(&tegra->clients); + tegra->master = master; + + drm = drm_dev_alloc(&tegra_drm_driver, dev); + if (!dev) { + err = -ENOMEM; + goto free; + } + + drm_set_unique(drm, "%s", dev_name(dev)); + master_set_drvdata(master, tegra); + drm->dev_private = tegra; + tegra->drm = drm; + + err = drm_dev_register(drm, 0); + if (err < 0) + goto unref; + + return 0; + +unref: + drm_dev_unref(drm); +free: + kfree(tegra); + return err; +} + +static void tegra_drm_unbind(struct master *master, struct device *dev) +{ + struct tegra_drm *tegra = master_get_drvdata(master); + + drm_put_dev(tegra->drm); +} + +static const struct component_master_ops tegra_drm_master_ops = { + .add_components = tegra_drm_add_components, + .bind = tegra_drm_bind, + .unbind = tegra_drm_unbind, +}; + +static int host1x_interface_bind(struct interface *intf) +{ + return component_master_add(intf->dev, &tegra_drm_master_ops); +} + +static void host1x_interface_unbind(struct interface *intf) +{ + component_master_del(intf->dev, &tegra_drm_master_ops); +} + +static struct interface_driver host1x_interface_driver = { + .name = "nvidia,tegra-host1x", + .bind = host1x_interface_bind, + .unbind = host1x_interface_unbind, };
-static int __init host1x_drm_init(void) +static int __init tegra_drm_init(void) { int err;
- err = host1x_driver_register(&host1x_drm_driver); + err = interface_register_driver(&host1x_interface_driver); if (err < 0) return err;
@@ -732,12 +795,12 @@ unregister_dsi: unregister_dc: platform_driver_unregister(&tegra_dc_driver); unregister_host1x: - host1x_driver_unregister(&host1x_drm_driver); + interface_unregister_driver(&host1x_interface_driver); return err; } -module_init(host1x_drm_init); +module_init(tegra_drm_init);
-static void __exit host1x_drm_exit(void) +static void __exit tegra_drm_exit(void) { platform_driver_unregister(&tegra_gr3d_driver); platform_driver_unregister(&tegra_gr2d_driver); @@ -746,9 +809,9 @@ static void __exit host1x_drm_exit(void) platform_driver_unregister(&tegra_sor_driver); platform_driver_unregister(&tegra_dsi_driver); platform_driver_unregister(&tegra_dc_driver); - host1x_driver_unregister(&host1x_drm_driver); + interface_unregister_driver(&host1x_interface_driver); } -module_exit(host1x_drm_exit); +module_exit(tegra_drm_exit);
MODULE_AUTHOR("Thierry Reding thierry.reding@avionic-design.de"); MODULE_DESCRIPTION("NVIDIA Tegra DRM driver"); diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index d100f706d818..ff29611e8a97 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -36,6 +36,7 @@ struct tegra_fbdev {
struct tegra_drm { struct drm_device *drm; + struct master *master;
struct mutex clients_lock; struct list_head clients; @@ -68,31 +69,25 @@ int tegra_drm_submit(struct tegra_drm_context *context, struct drm_file *file);
struct tegra_drm_client { - struct host1x_client base; struct list_head list; + struct device *dev; + + enum host1x_class class; + struct host1x_syncpt **syncpts; + unsigned int num_syncpts;
const struct tegra_drm_client_ops *ops; };
-static inline struct tegra_drm_client * -host1x_to_drm_client(struct host1x_client *client) -{ - return container_of(client, struct tegra_drm_client, base); -} - int tegra_drm_register_client(struct tegra_drm *tegra, struct tegra_drm_client *client); int tegra_drm_unregister_client(struct tegra_drm *tegra, struct tegra_drm_client *client);
-int tegra_drm_init(struct tegra_drm *tegra, struct drm_device *drm); -int tegra_drm_exit(struct tegra_drm *tegra); - struct tegra_dc_soc_info; struct tegra_output;
struct tegra_dc { - struct host1x_client client; struct device *dev; spinlock_t lock;
@@ -118,12 +113,6 @@ struct tegra_dc { const struct tegra_dc_soc_info *soc; };
-static inline struct tegra_dc * -host1x_client_to_dc(struct host1x_client *client) -{ - return container_of(client, struct tegra_dc, client); -} - static inline struct tegra_dc *to_tegra_dc(struct drm_crtc *crtc) { return crtc ? container_of(crtc, struct tegra_dc, base) : NULL; @@ -249,10 +238,6 @@ static inline int tegra_output_check_mode(struct tegra_output *output, return output ? -ENOSYS : -EINVAL; }
-/* from bus.c */ -int drm_host1x_init(struct drm_driver *driver, struct host1x_device *device); -void drm_host1x_exit(struct drm_driver *driver, struct host1x_device *device); - /* from rgb.c */ int tegra_dc_rgb_probe(struct tegra_dc *dc); int tegra_dc_rgb_remove(struct tegra_dc *dc); diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c index 3838575f71c6..0325e5206b66 100644 --- a/drivers/gpu/drm/tegra/dsi.c +++ b/drivers/gpu/drm/tegra/dsi.c @@ -30,7 +30,6 @@ #define DSI_HOST_FIFO_DEPTH 64
struct tegra_dsi { - struct host1x_client client; struct tegra_output output; struct device *dev;
@@ -56,12 +55,6 @@ struct tegra_dsi { bool enabled; };
-static inline struct tegra_dsi * -host1x_client_to_dsi(struct host1x_client *client) -{ - return container_of(client, struct tegra_dsi, client); -} - static inline struct tegra_dsi *host_to_tegra(struct mipi_dsi_host *host) { return container_of(host, struct tegra_dsi, host); @@ -713,68 +706,6 @@ static int tegra_dsi_pad_calibrate(struct tegra_dsi *dsi) return tegra_mipi_calibrate(dsi->mipi); }
-static int tegra_dsi_init(struct host1x_client *client) -{ - struct tegra_drm *tegra = dev_get_drvdata(client->parent); - struct tegra_dsi *dsi = host1x_client_to_dsi(client); - int err; - - dsi->output.type = TEGRA_OUTPUT_DSI; - dsi->output.dev = client->dev; - dsi->output.ops = &dsi_ops; - - err = tegra_output_init(tegra->drm, &dsi->output); - if (err < 0) { - dev_err(client->dev, "output setup failed: %d\n", err); - return err; - } - - if (IS_ENABLED(CONFIG_DEBUG_FS)) { - err = tegra_dsi_debugfs_init(dsi, tegra->drm->primary); - if (err < 0) - dev_err(dsi->dev, "debugfs setup failed: %d\n", err); - } - - err = tegra_dsi_pad_calibrate(dsi); - if (err < 0) { - dev_err(dsi->dev, "MIPI calibration failed: %d\n", err); - return err; - } - - return 0; -} - -static int tegra_dsi_exit(struct host1x_client *client) -{ - struct tegra_dsi *dsi = host1x_client_to_dsi(client); - int err; - - if (IS_ENABLED(CONFIG_DEBUG_FS)) { - err = tegra_dsi_debugfs_exit(dsi); - if (err < 0) - dev_err(dsi->dev, "debugfs cleanup failed: %d\n", err); - } - - err = tegra_output_disable(&dsi->output); - if (err < 0) { - dev_err(client->dev, "output failed to disable: %d\n", err); - return err; - } - - err = tegra_output_exit(&dsi->output); - if (err < 0) { - dev_err(client->dev, "output cleanup failed: %d\n", err); - return err; - } - - return 0; -} - -static const struct host1x_client_ops dsi_client_ops = { - .init = tegra_dsi_init, - .exit = tegra_dsi_exit, -}; - static int tegra_dsi_setup_clocks(struct tegra_dsi *dsi) { struct clk *parent; @@ -831,6 +762,62 @@ static const struct mipi_dsi_host_ops tegra_dsi_host_ops = { .detach = tegra_dsi_host_detach, };
+static int dsi_bind(struct device *dev, struct master *master, void *data) +{ + struct tegra_drm *tegra = master_get_drvdata(master); + struct tegra_dsi *dsi = dev_get_drvdata(dev); + int err; + + dsi->output.type = TEGRA_OUTPUT_DSI; + dsi->output.ops = &dsi_ops; + dsi->output.dev = dev; + + err = tegra_output_init(tegra->drm, &dsi->output); + if (err < 0) { + dev_err(dev, "output setup failed: %d\n", err); + return err; + } + + if (IS_ENABLED(CONFIG_DEBUG_FS)) { + err = tegra_dsi_debugfs_init(dsi, tegra->drm->primary); + if (err < 0) + dev_err(dev, "debugfs setup failed: %d\n", err); + } + + err = tegra_dsi_pad_calibrate(dsi); + if (err < 0) { + dev_err(dev, "MIPI calibration failed: %d\n", err); + return err; + } + + return 0; +} + +static void dsi_unbind(struct device *dev, struct master *master, void *data) +{ + struct tegra_dsi *dsi = dev_get_drvdata(dev); + int err; + + if (IS_ENABLED(CONFIG_DEBUG_FS)) { + err = tegra_dsi_debugfs_exit(dsi); + if (err < 0) + dev_err(dev, "debugfs cleanup failed: %d\n", err); + } + + err = tegra_output_disable(&dsi->output); + if (err < 0) + dev_err(dev, "output failed to disable: %d\n", err); + + err = tegra_output_exit(&dsi->output); + if (err < 0) + dev_err(dev, "output cleanup failed: %d\n", err); +} + +static const struct component_ops dsi_component_ops = { + .bind = dsi_bind, + .unbind = dsi_unbind, +}; + static int tegra_dsi_probe(struct platform_device *pdev) { struct tegra_dsi *dsi; @@ -842,6 +829,7 @@ static int tegra_dsi_probe(struct platform_device *pdev) return -ENOMEM;
dsi->output.dev = dsi->dev = &pdev->dev; + platform_set_drvdata(pdev, dsi);
err = tegra_output_probe(&dsi->output); if (err < 0) @@ -932,19 +920,12 @@ static int tegra_dsi_probe(struct platform_device *pdev) return err; }
- INIT_LIST_HEAD(&dsi->client.list); - dsi->client.ops = &dsi_client_ops; - dsi->client.dev = &pdev->dev; - - err = host1x_client_register(&dsi->client); + err = component_add(&pdev->dev, &dsi_component_ops); if (err < 0) { - dev_err(&pdev->dev, "failed to register host1x client: %d\n", - err); + dev_err(&pdev->dev, "failed to register component: %d\n", err); return err; }
- platform_set_drvdata(pdev, dsi); - return 0; }
@@ -953,12 +934,7 @@ static int tegra_dsi_remove(struct platform_device *pdev) struct tegra_dsi *dsi = platform_get_drvdata(pdev); int err;
- err = host1x_client_unregister(&dsi->client); - if (err < 0) { - dev_err(&pdev->dev, "failed to unregister host1x client: %d\n", - err); - return err; - } + component_del(&pdev->dev, &dsi_component_ops);
mipi_dsi_host_unregister(&dsi->host); tegra_mipi_free(dsi->mipi); diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c index 2c7ca748edf5..f4bfdd0079d8 100644 --- a/drivers/gpu/drm/tegra/gr2d.c +++ b/drivers/gpu/drm/tegra/gr2d.c @@ -25,46 +25,49 @@ static inline struct gr2d *to_gr2d(struct tegra_drm_client *client) return container_of(client, struct gr2d, client); }
-static int gr2d_init(struct host1x_client *client) +static int gr2d_bind(struct device *dev, struct master *master, void *data) { - struct tegra_drm_client *drm = host1x_to_drm_client(client); - struct tegra_drm *tegra = dev_get_drvdata(client->parent); + struct tegra_drm *tegra = master_get_drvdata(master); unsigned long flags = HOST1X_SYNCPT_HAS_BASE; - struct gr2d *gr2d = to_gr2d(drm); + struct gr2d *gr2d = dev_get_drvdata(dev); + int err;
- gr2d->channel = host1x_channel_request(client->dev); + gr2d->channel = host1x_channel_request(dev); if (!gr2d->channel) return -ENOMEM;
- client->syncpts[0] = host1x_syncpt_request(client->dev, flags); - if (!client->syncpts[0]) { + gr2d->client.syncpts[0] = host1x_syncpt_request(dev, flags); + if (!gr2d->client.syncpts[0]) { host1x_channel_free(gr2d->channel); return -ENOMEM; }
- return tegra_drm_register_client(tegra, drm); + err = tegra_drm_register_client(tegra, &gr2d->client); + if (err < 0) { + dev_err(dev, "failed to register client: %d\n", err); + return err; + } + + return 0; }
-static int gr2d_exit(struct host1x_client *client) +static void gr2d_unbind(struct device *dev, struct master *master, void *data) { - struct tegra_drm_client *drm = host1x_to_drm_client(client); - struct tegra_drm *tegra = dev_get_drvdata(client->parent); - struct gr2d *gr2d = to_gr2d(drm); + struct tegra_drm *tegra = master_get_drvdata(master); + struct gr2d *gr2d = dev_get_drvdata(dev); int err;
- err = tegra_drm_unregister_client(tegra, drm); + err = tegra_drm_unregister_client(tegra, &gr2d->client); if (err < 0) - return err; + dev_err(dev, "failed to unregister client: %d\n", err);
- host1x_syncpt_free(client->syncpts[0]); + host1x_syncpt_free(gr2d->client.syncpts[0]); host1x_channel_free(gr2d->channel); - - return 0; }
-static const struct host1x_client_ops gr2d_client_ops = { - .init = gr2d_init, - .exit = gr2d_exit, +static const struct component_ops tegra_gr2d_component_ops = { + .bind = gr2d_bind, + .unbind = gr2d_unbind, };
static int gr2d_open_channel(struct tegra_drm_client *client, @@ -150,6 +153,8 @@ static int gr2d_probe(struct platform_device *pdev) if (!gr2d) return -ENOMEM;
+ platform_set_drvdata(pdev, gr2d); + syncpts = devm_kzalloc(dev, sizeof(*syncpts), GFP_KERNEL); if (!syncpts) return -ENOMEM; @@ -166,28 +171,22 @@ static int gr2d_probe(struct platform_device *pdev) return err; }
- INIT_LIST_HEAD(&gr2d->client.base.list); - gr2d->client.base.ops = &gr2d_client_ops; - gr2d->client.base.dev = dev; - gr2d->client.base.class = HOST1X_CLASS_GR2D; - gr2d->client.base.syncpts = syncpts; - gr2d->client.base.num_syncpts = 1; - INIT_LIST_HEAD(&gr2d->client.list); + gr2d->client.class = HOST1X_CLASS_GR2D; + gr2d->client.syncpts = syncpts; + gr2d->client.num_syncpts = 1; gr2d->client.ops = &gr2d_ops; - - err = host1x_client_register(&gr2d->client.base); - if (err < 0) { - dev_err(dev, "failed to register host1x client: %d\n", err); - clk_disable_unprepare(gr2d->clk); - return err; - } + gr2d->client.dev = dev;
/* initialize address register map */ for (i = 0; i < ARRAY_SIZE(gr2d_addr_regs); i++) set_bit(gr2d_addr_regs[i], gr2d->addr_regs);
- platform_set_drvdata(pdev, gr2d); + err = component_add(&pdev->dev, &tegra_gr2d_component_ops); + if (err < 0) { + dev_err(&pdev->dev, "failed to register component: %d\n", err); + return err; + }
return 0; } @@ -195,15 +194,8 @@ static int gr2d_probe(struct platform_device *pdev) static int gr2d_remove(struct platform_device *pdev) { struct gr2d *gr2d = platform_get_drvdata(pdev); - int err; - - err = host1x_client_unregister(&gr2d->client.base); - if (err < 0) { - dev_err(&pdev->dev, "failed to unregister host1x client: %d\n", - err); - return err; - }
+ component_del(&pdev->dev, &tegra_gr2d_component_ops); clk_disable_unprepare(gr2d->clk);
return 0; diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c index 0cbb24b1ae04..c45502ad684e 100644 --- a/drivers/gpu/drm/tegra/gr3d.c +++ b/drivers/gpu/drm/tegra/gr3d.c @@ -34,46 +34,49 @@ static inline struct gr3d *to_gr3d(struct tegra_drm_client *client) return container_of(client, struct gr3d, client); }
-static int gr3d_init(struct host1x_client *client) +static int gr3d_bind(struct device *dev, struct master *master, void *data) { - struct tegra_drm_client *drm = host1x_to_drm_client(client); - struct tegra_drm *tegra = dev_get_drvdata(client->parent); + struct tegra_drm *tegra = master_get_drvdata(master); unsigned long flags = HOST1X_SYNCPT_HAS_BASE; - struct gr3d *gr3d = to_gr3d(drm); + struct gr3d *gr3d = dev_get_drvdata(dev); + int err;
- gr3d->channel = host1x_channel_request(client->dev); + gr3d->channel = host1x_channel_request(dev); if (!gr3d->channel) return -ENOMEM;
- client->syncpts[0] = host1x_syncpt_request(client->dev, flags); - if (!client->syncpts[0]) { + gr3d->client.syncpts[0] = host1x_syncpt_request(dev, flags); + if (!gr3d->client.syncpts[0]) { host1x_channel_free(gr3d->channel); return -ENOMEM; }
- return tegra_drm_register_client(tegra, drm); + err = tegra_drm_register_client(tegra, &gr3d->client); + if (err < 0) { + dev_err(dev, "failed to register client: %d\n", err); + return err; + } + + return 0; }
-static int gr3d_exit(struct host1x_client *client) +static void gr3d_unbind(struct device *dev, struct master *master, void *data) { - struct tegra_drm_client *drm = host1x_to_drm_client(client); - struct tegra_drm *tegra = dev_get_drvdata(client->parent); - struct gr3d *gr3d = to_gr3d(drm); + struct tegra_drm *tegra = master_get_drvdata(master); + struct gr3d *gr3d = dev_get_drvdata(dev); int err;
- err = tegra_drm_unregister_client(tegra, drm); + err = tegra_drm_unregister_client(tegra, &gr3d->client); if (err < 0) - return err; + dev_err(dev, "failed to unregister client: %d\n", err);
- host1x_syncpt_free(client->syncpts[0]); + host1x_syncpt_free(gr3d->client.syncpts[0]); host1x_channel_free(gr3d->channel); - - return 0; }
-static const struct host1x_client_ops gr3d_client_ops = { - .init = gr3d_init, - .exit = gr3d_exit, +static const struct component_ops tegra_gr3d_component_ops = { + .bind = gr3d_bind, + .unbind = gr3d_unbind, };
static int gr3d_open_channel(struct tegra_drm_client *client, @@ -248,6 +251,8 @@ static int gr3d_probe(struct platform_device *pdev) if (!gr3d) return -ENOMEM;
+ platform_set_drvdata(pdev, gr3d); + syncpts = devm_kzalloc(&pdev->dev, sizeof(*syncpts), GFP_KERNEL); if (!syncpts) return -ENOMEM; @@ -297,28 +302,22 @@ static int gr3d_probe(struct platform_device *pdev) } }
- INIT_LIST_HEAD(&gr3d->client.base.list); - gr3d->client.base.ops = &gr3d_client_ops; - gr3d->client.base.dev = &pdev->dev; - gr3d->client.base.class = HOST1X_CLASS_GR3D; - gr3d->client.base.syncpts = syncpts; - gr3d->client.base.num_syncpts = 1; - INIT_LIST_HEAD(&gr3d->client.list); + gr3d->client.class = HOST1X_CLASS_GR3D; + gr3d->client.syncpts = syncpts; + gr3d->client.num_syncpts = 1; gr3d->client.ops = &gr3d_ops; - - err = host1x_client_register(&gr3d->client.base); - if (err < 0) { - dev_err(&pdev->dev, "failed to register host1x client: %d\n", - err); - return err; - } + gr3d->client.dev = &pdev->dev;
/* initialize address register map */ for (i = 0; i < ARRAY_SIZE(gr3d_addr_regs); i++) set_bit(gr3d_addr_regs[i], gr3d->addr_regs);
- platform_set_drvdata(pdev, gr3d); + err = component_add(&pdev->dev, &tegra_gr3d_component_ops); + if (err < 0) { + dev_err(&pdev->dev, "failed to register component: %d\n", err); + return err; + }
return 0; } @@ -326,14 +325,8 @@ static int gr3d_probe(struct platform_device *pdev) static int gr3d_remove(struct platform_device *pdev) { struct gr3d *gr3d = platform_get_drvdata(pdev); - int err;
- err = host1x_client_unregister(&gr3d->client.base); - if (err < 0) { - dev_err(&pdev->dev, "failed to unregister host1x client: %d\n", - err); - return err; - } + component_del(&pdev->dev, &tegra_gr3d_component_ops);
if (gr3d->clk_secondary) { tegra_powergate_power_off(TEGRA_POWERGATE_3D1); diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c index fec1a63d32c9..c27149718753 100644 --- a/drivers/gpu/drm/tegra/hdmi.c +++ b/drivers/gpu/drm/tegra/hdmi.c @@ -37,7 +37,6 @@ struct tegra_hdmi_config { };
struct tegra_hdmi { - struct host1x_client client; struct tegra_output output; struct device *dev; bool enabled; @@ -65,12 +64,6 @@ struct tegra_hdmi { struct dentry *debugfs; };
-static inline struct tegra_hdmi * -host1x_client_to_hdmi(struct host1x_client *client) -{ - return container_of(client, struct tegra_hdmi, client); -} - static inline struct tegra_hdmi *to_hdmi(struct tegra_output *output) { return container_of(output, struct tegra_hdmi, output); @@ -1345,41 +1338,40 @@ static int tegra_hdmi_debugfs_exit(struct tegra_hdmi *hdmi) return 0; }
-static int tegra_hdmi_init(struct host1x_client *client) +static int hdmi_bind(struct device *dev, struct master *master, void *data) { - struct tegra_drm *tegra = dev_get_drvdata(client->parent); - struct tegra_hdmi *hdmi = host1x_client_to_hdmi(client); + struct tegra_drm *tegra = master_get_drvdata(master); + struct tegra_hdmi *hdmi = dev_get_drvdata(dev); int err;
hdmi->output.type = TEGRA_OUTPUT_HDMI; - hdmi->output.dev = client->dev; hdmi->output.ops = &hdmi_ops; + hdmi->output.dev = dev;
err = tegra_output_init(tegra->drm, &hdmi->output); if (err < 0) { - dev_err(client->dev, "output setup failed: %d\n", err); + dev_err(dev, "output setup failed: %d\n", err); return err; }
if (IS_ENABLED(CONFIG_DEBUG_FS)) { err = tegra_hdmi_debugfs_init(hdmi, tegra->drm->primary); if (err < 0) - dev_err(client->dev, "debugfs setup failed: %d\n", err); + dev_err(dev, "debugfs setup failed: %d\n", err); }
err = regulator_enable(hdmi->hdmi); if (err < 0) { - dev_err(client->dev, "failed to enable HDMI regulator: %d\n", - err); + dev_err(dev, "failed to enable HDMI regulator: %d\n", err); return err; }
return 0; }
-static int tegra_hdmi_exit(struct host1x_client *client) +static void hdmi_unbind(struct device *dev, struct master *master, void *data) { - struct tegra_hdmi *hdmi = host1x_client_to_hdmi(client); + struct tegra_hdmi *hdmi = dev_get_drvdata(dev); int err;
regulator_disable(hdmi->hdmi); @@ -1387,28 +1379,21 @@ static int tegra_hdmi_exit(struct host1x_client *client) if (IS_ENABLED(CONFIG_DEBUG_FS)) { err = tegra_hdmi_debugfs_exit(hdmi); if (err < 0) - dev_err(client->dev, "debugfs cleanup failed: %d\n", - err); + dev_err(dev, "debugfs cleanup failed: %d\n", err); }
err = tegra_output_disable(&hdmi->output); - if (err < 0) { - dev_err(client->dev, "output failed to disable: %d\n", err); - return err; - } + if (err < 0) + dev_err(dev, "output failed to disable: %d\n", err);
err = tegra_output_exit(&hdmi->output); - if (err < 0) { - dev_err(client->dev, "output cleanup failed: %d\n", err); - return err; - } - - return 0; + if (err < 0) + dev_err(dev, "output cleanup failed: %d\n", err); }
-static const struct host1x_client_ops hdmi_client_ops = { - .init = tegra_hdmi_init, - .exit = tegra_hdmi_exit, +static const struct component_ops hdmi_component_ops = { + .bind = hdmi_bind, + .unbind = hdmi_unbind, };
static const struct tegra_hdmi_config tegra20_hdmi_config = { @@ -1466,6 +1451,8 @@ static int tegra_hdmi_probe(struct platform_device *pdev) if (!hdmi) return -ENOMEM;
+ platform_set_drvdata(pdev, hdmi); + hdmi->config = match->data; hdmi->dev = &pdev->dev; hdmi->audio_source = AUTO; @@ -1530,19 +1517,12 @@ static int tegra_hdmi_probe(struct platform_device *pdev)
hdmi->irq = err;
- INIT_LIST_HEAD(&hdmi->client.list); - hdmi->client.ops = &hdmi_client_ops; - hdmi->client.dev = &pdev->dev; - - err = host1x_client_register(&hdmi->client); + err = component_add(&pdev->dev, &hdmi_component_ops); if (err < 0) { - dev_err(&pdev->dev, "failed to register host1x client: %d\n", - err); + dev_err(&pdev->dev, "failed to register component: %d\n", err); return err; }
- platform_set_drvdata(pdev, hdmi); - return 0; }
@@ -1551,12 +1531,7 @@ static int tegra_hdmi_remove(struct platform_device *pdev) struct tegra_hdmi *hdmi = platform_get_drvdata(pdev); int err;
- err = host1x_client_unregister(&hdmi->client); - if (err < 0) { - dev_err(&pdev->dev, "failed to unregister host1x client: %d\n", - err); - return err; - } + component_del(&pdev->dev, &hdmi_component_ops);
err = tegra_output_remove(&hdmi->output); if (err < 0) { diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index 7d66f6e53919..60535e380d18 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -20,7 +20,6 @@ #include "sor.h"
struct tegra_sor { - struct host1x_client client; struct tegra_output output; struct device *dev;
@@ -40,12 +39,6 @@ struct tegra_sor { struct dentry *debugfs; };
-static inline struct tegra_sor * -host1x_client_to_sor(struct host1x_client *client) -{ - return container_of(client, struct tegra_sor, client); -} - static inline struct tegra_sor *to_sor(struct tegra_output *output) { return container_of(output, struct tegra_sor, output); @@ -1042,10 +1035,10 @@ static int tegra_sor_debugfs_exit(struct tegra_sor *sor) return 0; }
-static int tegra_sor_init(struct host1x_client *client) +static int sor_bind(struct device *dev, struct master *master, void *data) { - struct tegra_drm *tegra = dev_get_drvdata(client->parent); - struct tegra_sor *sor = host1x_client_to_sor(client); + struct tegra_drm *tegra = master_get_drvdata(master); + struct tegra_sor *sor = dev_get_drvdata(dev); int err;
if (!sor->dpaux) @@ -1058,7 +1051,7 @@ static int tegra_sor_init(struct host1x_client *client)
err = tegra_output_init(tegra->drm, &sor->output); if (err < 0) { - dev_err(sor->dev, "output setup failed: %d\n", err); + dev_err(dev, "output setup failed: %d\n", err); return err; }
@@ -1067,13 +1060,13 @@ static int tegra_sor_init(struct host1x_client *client)
err = tegra_sor_debugfs_init(sor, root); if (err < 0) - dev_err(sor->dev, "debugfs setup failed: %d\n", err); + dev_err(dev, "debugfs setup failed: %d\n", err); }
if (sor->dpaux) { err = tegra_dpaux_attach(sor->dpaux, &sor->output); if (err < 0) { - dev_err(sor->dev, "failed to attach DP: %d\n", err); + dev_err(dev, "failed to attach DP: %d\n", err); return err; } } @@ -1081,43 +1074,35 @@ static int tegra_sor_init(struct host1x_client *client) return 0; }
-static int tegra_sor_exit(struct host1x_client *client) +static void sor_unbind(struct device *dev, struct master *master, void *data) { - struct tegra_sor *sor = host1x_client_to_sor(client); + struct tegra_sor *sor = dev_get_drvdata(dev); int err;
err = tegra_output_disable(&sor->output); - if (err < 0) { - dev_err(sor->dev, "output failed to disable: %d\n", err); - return err; - } + if (err < 0) + dev_err(dev, "output failed to disable: %d\n", err);
if (sor->dpaux) { err = tegra_dpaux_detach(sor->dpaux); - if (err < 0) { - dev_err(sor->dev, "failed to detach DP: %d\n", err); - return err; - } + if (err < 0) + dev_err(dev, "failed to detach DP: %d\n", err); }
if (IS_ENABLED(CONFIG_DEBUG_FS)) { err = tegra_sor_debugfs_exit(sor); if (err < 0) - dev_err(sor->dev, "debugfs cleanup failed: %d\n", err); + dev_err(dev, "debugfs cleanup failed: %d\n", err); }
err = tegra_output_exit(&sor->output); - if (err < 0) { - dev_err(sor->dev, "output cleanup failed: %d\n", err); - return err; - } - - return 0; + if (err < 0) + dev_err(dev, "output cleanup failed: %d\n", err); }
-static const struct host1x_client_ops sor_client_ops = { - .init = tegra_sor_init, - .exit = tegra_sor_exit, +static const struct component_ops tegra_sor_component_ops = { + .bind = sor_bind, + .unbind = sor_unbind, };
static int tegra_sor_probe(struct platform_device *pdev) @@ -1132,6 +1117,7 @@ static int tegra_sor_probe(struct platform_device *pdev) return -ENOMEM;
sor->output.dev = sor->dev = &pdev->dev; + platform_set_drvdata(pdev, sor);
np = of_parse_phandle(pdev->dev.of_node, "nvidia,dpaux", 0); if (np) { @@ -1183,35 +1169,22 @@ static int tegra_sor_probe(struct platform_device *pdev) if (err < 0) return err;
- INIT_LIST_HEAD(&sor->client.list); - sor->client.ops = &sor_client_ops; - sor->client.dev = &pdev->dev; - mutex_init(&sor->lock);
- err = host1x_client_register(&sor->client); + err = component_add(&pdev->dev, &tegra_sor_component_ops); if (err < 0) { - dev_err(&pdev->dev, "failed to register host1x client: %d\n", - err); + dev_err(&pdev->dev, "failed to register component: %d\n", err); return err; }
- platform_set_drvdata(pdev, sor); - return 0; }
static int tegra_sor_remove(struct platform_device *pdev) { struct tegra_sor *sor = platform_get_drvdata(pdev); - int err;
- err = host1x_client_unregister(&sor->client); - if (err < 0) { - dev_err(&pdev->dev, "failed to unregister host1x client: %d\n", - err); - return err; - } + component_del(&pdev->dev, &tegra_sor_component_ops);
clk_disable_unprepare(sor->clk_parent); clk_disable_unprepare(sor->clk_safe); diff --git a/drivers/gpu/host1x/Makefile b/drivers/gpu/host1x/Makefile index c1189f004441..2ff37ab71fd2 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 \ diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c deleted file mode 100644 index ccdd2e6da5e3..000000000000 --- a/drivers/gpu/host1x/bus.c +++ /dev/null @@ -1,553 +0,0 @@ -/* - * Copyright (C) 2012 Avionic Design GmbH - * Copyright (C) 2012-2013, NVIDIA Corporation - * - * This program is free software; you can redistribute it and/or modify it - * under the terms and conditions of the GNU General Public License, - * version 2, as published by the Free Software Foundation. - * - * This program is distributed in the hope it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see http://www.gnu.org/licenses/. - */ - -#include <linux/host1x.h> -#include <linux/of.h> -#include <linux/slab.h> - -#include "bus.h" -#include "dev.h" - -static DEFINE_MUTEX(clients_lock); -static LIST_HEAD(clients); - -static DEFINE_MUTEX(drivers_lock); -static LIST_HEAD(drivers); - -static DEFINE_MUTEX(devices_lock); -static LIST_HEAD(devices); - -struct host1x_subdev { - struct host1x_client *client; - struct device_node *np; - struct list_head list; -}; - -/** - * host1x_subdev_add() - add a new subdevice with an associated device node - */ -static int host1x_subdev_add(struct host1x_device *device, - struct device_node *np) -{ - struct host1x_subdev *subdev; - - subdev = kzalloc(sizeof(*subdev), GFP_KERNEL); - if (!subdev) - return -ENOMEM; - - INIT_LIST_HEAD(&subdev->list); - subdev->np = of_node_get(np); - - mutex_lock(&device->subdevs_lock); - list_add_tail(&subdev->list, &device->subdevs); - mutex_unlock(&device->subdevs_lock); - - return 0; -} - -/** - * host1x_subdev_del() - remove subdevice - */ -static void host1x_subdev_del(struct host1x_subdev *subdev) -{ - list_del(&subdev->list); - of_node_put(subdev->np); - kfree(subdev); -} - -/** - * host1x_device_parse_dt() - scan device tree and add matching subdevices - */ -static int host1x_device_parse_dt(struct host1x_device *device) -{ - struct device_node *np; - int err; - - for_each_child_of_node(device->dev.parent->of_node, np) { - if (of_match_node(device->driver->subdevs, np) && - of_device_is_available(np)) { - err = host1x_subdev_add(device, np); - if (err < 0) - return err; - } - } - - return 0; -} - -static void host1x_subdev_register(struct host1x_device *device, - struct host1x_subdev *subdev, - struct host1x_client *client) -{ - int err; - - /* - * Move the subdevice to the list of active (registered) subdevices - * and associate it with a client. At the same time, associate the - * client with its parent device. - */ - mutex_lock(&device->subdevs_lock); - mutex_lock(&device->clients_lock); - list_move_tail(&client->list, &device->clients); - list_move_tail(&subdev->list, &device->active); - client->parent = &device->dev; - subdev->client = client; - mutex_unlock(&device->clients_lock); - mutex_unlock(&device->subdevs_lock); - - /* - * When all subdevices have been registered, the composite device is - * ready to be probed. - */ - if (list_empty(&device->subdevs)) { - err = device->driver->probe(device); - if (err < 0) - dev_err(&device->dev, "probe failed: %d\n", err); - } -} - -static void __host1x_subdev_unregister(struct host1x_device *device, - struct host1x_subdev *subdev) -{ - struct host1x_client *client = subdev->client; - int err; - - /* - * If all subdevices have been activated, we're about to remove the - * first active subdevice, so unload the driver first. - */ - if (list_empty(&device->subdevs)) { - err = device->driver->remove(device); - if (err < 0) - dev_err(&device->dev, "remove failed: %d\n", err); - } - - /* - * Move the subdevice back to the list of idle subdevices and remove - * it from list of clients. - */ - mutex_lock(&device->clients_lock); - subdev->client = NULL; - client->parent = NULL; - list_move_tail(&subdev->list, &device->subdevs); - /* - * XXX: Perhaps don't do this here, but rather explicitly remove it - * when the device is about to be deleted. - * - * This is somewhat complicated by the fact that this function is - * used to remove the subdevice when a client is unregistered but - * also when the composite device is about to be removed. - */ - list_del_init(&client->list); - mutex_unlock(&device->clients_lock); -} - -static void host1x_subdev_unregister(struct host1x_device *device, - struct host1x_subdev *subdev) -{ - mutex_lock(&device->subdevs_lock); - __host1x_subdev_unregister(device, subdev); - mutex_unlock(&device->subdevs_lock); -} - -int host1x_device_init(struct host1x_device *device) -{ - struct host1x_client *client; - int err; - - mutex_lock(&device->clients_lock); - - list_for_each_entry(client, &device->clients, list) { - if (client->ops && client->ops->init) { - err = client->ops->init(client); - if (err < 0) { - dev_err(&device->dev, - "failed to initialize %s: %d\n", - dev_name(client->dev), err); - mutex_unlock(&device->clients_lock); - return err; - } - } - } - - mutex_unlock(&device->clients_lock); - - return 0; -} -EXPORT_SYMBOL(host1x_device_init); - -int host1x_device_exit(struct host1x_device *device) -{ - struct host1x_client *client; - int err; - - mutex_lock(&device->clients_lock); - - list_for_each_entry_reverse(client, &device->clients, list) { - if (client->ops && client->ops->exit) { - err = client->ops->exit(client); - if (err < 0) { - dev_err(&device->dev, - "failed to cleanup %s: %d\n", - dev_name(client->dev), err); - mutex_unlock(&device->clients_lock); - return err; - } - } - } - - mutex_unlock(&device->clients_lock); - - return 0; -} -EXPORT_SYMBOL(host1x_device_exit); - -static int host1x_register_client(struct host1x *host1x, - struct host1x_client *client) -{ - struct host1x_device *device; - struct host1x_subdev *subdev; - - mutex_lock(&host1x->devices_lock); - - list_for_each_entry(device, &host1x->devices, list) { - list_for_each_entry(subdev, &device->subdevs, list) { - if (subdev->np == client->dev->of_node) { - host1x_subdev_register(device, subdev, client); - mutex_unlock(&host1x->devices_lock); - return 0; - } - } - } - - mutex_unlock(&host1x->devices_lock); - return -ENODEV; -} - -static int host1x_unregister_client(struct host1x *host1x, - struct host1x_client *client) -{ - struct host1x_device *device, *dt; - struct host1x_subdev *subdev; - - mutex_lock(&host1x->devices_lock); - - list_for_each_entry_safe(device, dt, &host1x->devices, list) { - list_for_each_entry(subdev, &device->active, list) { - if (subdev->client == client) { - host1x_subdev_unregister(device, subdev); - mutex_unlock(&host1x->devices_lock); - return 0; - } - } - } - - mutex_unlock(&host1x->devices_lock); - return -ENODEV; -} - -static struct bus_type host1x_bus_type = { - .name = "host1x", -}; - -int host1x_bus_init(void) -{ - return bus_register(&host1x_bus_type); -} - -void host1x_bus_exit(void) -{ - bus_unregister(&host1x_bus_type); -} - -static void host1x_device_release(struct device *dev) -{ - struct host1x_device *device = to_host1x_device(dev); - - kfree(device); -} - -static int host1x_device_add(struct host1x *host1x, - struct host1x_driver *driver) -{ - struct host1x_client *client, *tmp; - struct host1x_subdev *subdev; - struct host1x_device *device; - int err; - - device = kzalloc(sizeof(*device), GFP_KERNEL); - if (!device) - return -ENOMEM; - - mutex_init(&device->subdevs_lock); - INIT_LIST_HEAD(&device->subdevs); - INIT_LIST_HEAD(&device->active); - mutex_init(&device->clients_lock); - INIT_LIST_HEAD(&device->clients); - INIT_LIST_HEAD(&device->list); - device->driver = driver; - - device->dev.coherent_dma_mask = host1x->dev->coherent_dma_mask; - device->dev.dma_mask = &device->dev.coherent_dma_mask; - device->dev.release = host1x_device_release; - dev_set_name(&device->dev, "%s", driver->name); - device->dev.bus = &host1x_bus_type; - device->dev.parent = host1x->dev; - - err = device_register(&device->dev); - if (err < 0) - return err; - - err = host1x_device_parse_dt(device); - if (err < 0) { - device_unregister(&device->dev); - return err; - } - - mutex_lock(&host1x->devices_lock); - list_add_tail(&device->list, &host1x->devices); - mutex_unlock(&host1x->devices_lock); - - mutex_lock(&clients_lock); - - list_for_each_entry_safe(client, tmp, &clients, list) { - list_for_each_entry(subdev, &device->subdevs, list) { - if (subdev->np == client->dev->of_node) { - host1x_subdev_register(device, subdev, client); - break; - } - } - } - - mutex_unlock(&clients_lock); - - return 0; -} - -/* - * Removes a device by first unregistering any subdevices and then removing - * itself from the list of devices. - * - * This function must be called with the host1x->devices_lock held. - */ -static void host1x_device_del(struct host1x *host1x, - struct host1x_device *device) -{ - struct host1x_subdev *subdev, *sd; - struct host1x_client *client, *cl; - - mutex_lock(&device->subdevs_lock); - - /* unregister subdevices */ - list_for_each_entry_safe(subdev, sd, &device->active, list) { - /* - * host1x_subdev_unregister() will remove the client from - * any lists, so we'll need to manually add it back to the - * list of idle clients. - * - * XXX: Alternatively, perhaps don't remove the client from - * any lists in host1x_subdev_unregister() and instead do - * that explicitly from host1x_unregister_client()? - */ - client = subdev->client; - - __host1x_subdev_unregister(device, subdev); - - /* add the client to the list of idle clients */ - mutex_lock(&clients_lock); - list_add_tail(&client->list, &clients); - mutex_unlock(&clients_lock); - } - - /* remove subdevices */ - list_for_each_entry_safe(subdev, sd, &device->subdevs, list) - host1x_subdev_del(subdev); - - mutex_unlock(&device->subdevs_lock); - - /* move clients to idle list */ - mutex_lock(&clients_lock); - mutex_lock(&device->clients_lock); - - list_for_each_entry_safe(client, cl, &device->clients, list) - list_move_tail(&client->list, &clients); - - mutex_unlock(&device->clients_lock); - mutex_unlock(&clients_lock); - - /* finally remove the device */ - list_del_init(&device->list); - device_unregister(&device->dev); -} - -static void host1x_attach_driver(struct host1x *host1x, - struct host1x_driver *driver) -{ - struct host1x_device *device; - int err; - - mutex_lock(&host1x->devices_lock); - - list_for_each_entry(device, &host1x->devices, list) { - if (device->driver == driver) { - mutex_unlock(&host1x->devices_lock); - return; - } - } - - mutex_unlock(&host1x->devices_lock); - - err = host1x_device_add(host1x, driver); - if (err < 0) - dev_err(host1x->dev, "failed to allocate device: %d\n", err); -} - -static void host1x_detach_driver(struct host1x *host1x, - struct host1x_driver *driver) -{ - struct host1x_device *device, *tmp; - - mutex_lock(&host1x->devices_lock); - - list_for_each_entry_safe(device, tmp, &host1x->devices, list) - if (device->driver == driver) - host1x_device_del(host1x, device); - - mutex_unlock(&host1x->devices_lock); -} - -int host1x_register(struct host1x *host1x) -{ - struct host1x_driver *driver; - - mutex_lock(&devices_lock); - list_add_tail(&host1x->list, &devices); - mutex_unlock(&devices_lock); - - mutex_lock(&drivers_lock); - - list_for_each_entry(driver, &drivers, list) - host1x_attach_driver(host1x, driver); - - mutex_unlock(&drivers_lock); - - return 0; -} - -int host1x_unregister(struct host1x *host1x) -{ - struct host1x_driver *driver; - - mutex_lock(&drivers_lock); - - list_for_each_entry(driver, &drivers, list) - host1x_detach_driver(host1x, driver); - - mutex_unlock(&drivers_lock); - - mutex_lock(&devices_lock); - list_del_init(&host1x->list); - mutex_unlock(&devices_lock); - - return 0; -} - -int host1x_driver_register(struct host1x_driver *driver) -{ - struct host1x *host1x; - - INIT_LIST_HEAD(&driver->list); - - mutex_lock(&drivers_lock); - list_add_tail(&driver->list, &drivers); - mutex_unlock(&drivers_lock); - - mutex_lock(&devices_lock); - - list_for_each_entry(host1x, &devices, list) - host1x_attach_driver(host1x, driver); - - mutex_unlock(&devices_lock); - - return 0; -} -EXPORT_SYMBOL(host1x_driver_register); - -void host1x_driver_unregister(struct host1x_driver *driver) -{ - mutex_lock(&drivers_lock); - list_del_init(&driver->list); - mutex_unlock(&drivers_lock); -} -EXPORT_SYMBOL(host1x_driver_unregister); - -int host1x_client_register(struct host1x_client *client) -{ - struct host1x *host1x; - int err; - - mutex_lock(&devices_lock); - - list_for_each_entry(host1x, &devices, list) { - err = host1x_register_client(host1x, client); - if (!err) { - mutex_unlock(&devices_lock); - return 0; - } - } - - mutex_unlock(&devices_lock); - - mutex_lock(&clients_lock); - list_add_tail(&client->list, &clients); - mutex_unlock(&clients_lock); - - return 0; -} -EXPORT_SYMBOL(host1x_client_register); - -int host1x_client_unregister(struct host1x_client *client) -{ - struct host1x_client *c; - struct host1x *host1x; - int err; - - mutex_lock(&devices_lock); - - list_for_each_entry(host1x, &devices, list) { - err = host1x_unregister_client(host1x, client); - if (!err) { - mutex_unlock(&devices_lock); - return 0; - } - } - - mutex_unlock(&devices_lock); - mutex_lock(&clients_lock); - - list_for_each_entry(c, &clients, list) { - if (c == client) { - list_del_init(&c->list); - break; - } - } - - mutex_unlock(&clients_lock); - - return 0; -} -EXPORT_SYMBOL(host1x_client_unregister); diff --git a/drivers/gpu/host1x/bus.h b/drivers/gpu/host1x/bus.h deleted file mode 100644 index 4099e99212c8..000000000000 --- a/drivers/gpu/host1x/bus.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (C) 2012 Avionic Design GmbH - * Copyright (C) 2012-2013, NVIDIA Corporation - * - * This program is free software; you can redistribute it and/or modify it - * under the terms and conditions of the GNU General Public License, - * version 2, as published by the Free Software Foundation. - * - * This program is distributed in the hope it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see http://www.gnu.org/licenses/. - */ - -#ifndef HOST1X_BUS_H -#define HOST1X_BUS_H - -struct host1x; - -int host1x_bus_init(void); -void host1x_bus_exit(void); - -int host1x_register(struct host1x *host1x); -int host1x_unregister(struct host1x *host1x); - -#endif diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index 2529908d304b..b92812ba6f18 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -27,7 +27,6 @@ #define CREATE_TRACE_POINTS #include <trace/events/host1x.h>
-#include "bus.h" #include "dev.h" #include "intr.h" #include "channel.h" @@ -124,8 +123,8 @@ static int host1x_probe(struct platform_device *pdev) if (!host) return -ENOMEM;
- mutex_init(&host->devices_lock); - INIT_LIST_HEAD(&host->devices); + mutex_init(&host->masters_lock); + INIT_LIST_HEAD(&host->masters); INIT_LIST_HEAD(&host->list); host->dev = &pdev->dev; host->info = id->data; @@ -176,7 +175,10 @@ static int host1x_probe(struct platform_device *pdev)
host1x_debug_init(host);
- err = host1x_register(host); + host->interface.name = "nvidia,tegra-host1x"; + host->interface.dev = &pdev->dev; + + err = interface_add(&host->interface); if (err < 0) goto fail_deinit_intr;
@@ -195,7 +197,7 @@ static int host1x_remove(struct platform_device *pdev) { struct host1x *host = platform_get_drvdata(pdev);
- host1x_unregister(host); + interface_remove(&host->interface); host1x_intr_deinit(host); host1x_syncpt_deinit(host); clk_disable_unprepare(host->clk); @@ -216,13 +218,9 @@ static int __init tegra_host1x_init(void) { int err;
- err = host1x_bus_init(); - if (err < 0) - return err; - err = platform_driver_register(&tegra_host1x_driver); if (err < 0) - goto unregister_bus; + return err;
err = platform_driver_register(&tegra_mipi_driver); if (err < 0) @@ -232,8 +230,6 @@ static int __init tegra_host1x_init(void)
unregister_host1x: platform_driver_unregister(&tegra_host1x_driver); -unregister_bus: - host1x_bus_exit(); return err; } module_init(tegra_host1x_init); @@ -242,7 +238,6 @@ static void __exit tegra_host1x_exit(void) { platform_driver_unregister(&tegra_mipi_driver); platform_driver_unregister(&tegra_host1x_driver); - host1x_bus_exit(); } module_exit(tegra_host1x_exit);
diff --git a/drivers/gpu/host1x/dev.h b/drivers/gpu/host1x/dev.h index 0b6e8e9629c5..90e4716ee220 100644 --- a/drivers/gpu/host1x/dev.h +++ b/drivers/gpu/host1x/dev.h @@ -17,6 +17,7 @@ #ifndef HOST1X_DEV_H #define HOST1X_DEV_H
+#include <linux/interface.h> #include <linux/platform_device.h> #include <linux/device.h>
@@ -127,9 +128,9 @@ struct host1x {
struct dentry *debugfs;
- struct mutex devices_lock; - struct list_head devices; - + struct interface interface; + struct mutex masters_lock; + struct list_head masters; struct list_head list; };
diff --git a/include/linux/host1x.h b/include/linux/host1x.h index b432b3ddbc13..14b6161328df 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -19,6 +19,7 @@ #ifndef __LINUX_HOST1X_H #define __LINUX_HOST1X_H
+#include <linux/component.h> #include <linux/device.h> #include <linux/types.h>
@@ -30,27 +31,6 @@ enum host1x_class { HOST1X_CLASS_GPU = 0x61, };
-struct host1x_client; - -struct host1x_client_ops { - int (*init)(struct host1x_client *client); - int (*exit)(struct host1x_client *client); -}; - -struct host1x_client { - struct list_head list; - struct device *parent; - struct device *dev; - - const struct host1x_client_ops *ops; - - enum host1x_class class; - struct host1x_channel *channel; - - struct host1x_syncpt **syncpts; - unsigned int num_syncpts; -}; - /* * host1x buffer objects */ @@ -241,48 +221,6 @@ void host1x_job_put(struct host1x_job *job); int host1x_job_pin(struct host1x_job *job, struct device *dev); void host1x_job_unpin(struct host1x_job *job);
-/* - * subdevice probe infrastructure - */ - -struct host1x_device; - -struct host1x_driver { - const struct of_device_id *subdevs; - struct list_head list; - const char *name; - - int (*probe)(struct host1x_device *device); - int (*remove)(struct host1x_device *device); -}; - -int host1x_driver_register(struct host1x_driver *driver); -void host1x_driver_unregister(struct host1x_driver *driver); - -struct host1x_device { - struct host1x_driver *driver; - struct list_head list; - struct device dev; - - struct mutex subdevs_lock; - struct list_head subdevs; - struct list_head active; - - struct mutex clients_lock; - struct list_head clients; -}; - -static inline struct host1x_device *to_host1x_device(struct device *dev) -{ - return container_of(dev, struct host1x_device, dev); -} - -int host1x_device_init(struct host1x_device *device); -int host1x_device_exit(struct host1x_device *device); - -int host1x_client_register(struct host1x_client *client); -int host1x_client_unregister(struct host1x_client *client); - struct tegra_mipi_device;
struct tegra_mipi_device *tegra_mipi_request(struct device *device);
2014-05-13 17:30 GMT+02:00 Thierry Reding thierry.reding@gmail.com:
From: Thierry Reding treding@nvidia.com
Instead of the current implementation, reuse the recently introduced master/component framework, which is equivalent in most regards. One issue is that there is no device to bind the DRM driver to. In order to still allow the driver to be probed, expose an interface from the host1x device and provide an interface driver to bind to that.
The interface driver then registers (or removes) the master that will be used to instantiate and bind the DRM driver. Since the drm_host1x bus implementation is now gone the dummy device instantiated by it can no longer be used as the parent for the DRM device. However since the parent device doesn't need to be modified, the host1x parent device that exposes the interface can be used instead.
Signed-off-by: Thierry Reding treding@nvidia.com
Is there a git tree somewhere to have a look at? I am interested in the usage of the master/component framework.
thanks -- Christian Gmeiner, MSc
On Mon, May 19, 2014 at 10:56:08AM +0200, Christian Gmeiner wrote:
2014-05-13 17:30 GMT+02:00 Thierry Reding thierry.reding@gmail.com:
From: Thierry Reding treding@nvidia.com
Instead of the current implementation, reuse the recently introduced master/component framework, which is equivalent in most regards. One issue is that there is no device to bind the DRM driver to. In order to still allow the driver to be probed, expose an interface from the host1x device and provide an interface driver to bind to that.
The interface driver then registers (or removes) the master that will be used to instantiate and bind the DRM driver. Since the drm_host1x bus implementation is now gone the dummy device instantiated by it can no longer be used as the parent for the DRM device. However since the parent device doesn't need to be modified, the host1x parent device that exposes the interface can be used instead.
Signed-off-by: Thierry Reding treding@nvidia.com
Is there a git tree somewhere to have a look at? I am interested in the usage of the master/component framework.
I've pushed my latest working branches to gitorious[0]. staging/work is the branch that has everything.
Thierry
From: Thierry Reding treding@nvidia.com
Describe how devices are registered using the drm_*_init() functions. Adding this to docbook requires a largish set of changes to the comments in drm_{pci,usb,platform}.c since they are doxygen-style rather than proper kernel-doc and therefore mess with the docbook generation.
Signed-off-by: Thierry Reding treding@nvidia.com --- Documentation/DocBook/drm.tmpl | 9 +++++ drivers/gpu/drm/drm_pci.c | 80 ++++++++++++++++++++---------------------- drivers/gpu/drm/drm_platform.c | 15 ++++---- drivers/gpu/drm/drm_stub.c | 19 +++++----- drivers/gpu/drm/drm_usb.c | 20 ++++++++++- 5 files changed, 81 insertions(+), 62 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index f4028c3a28a2..f26a8e4fbe47 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -282,6 +282,15 @@ char *date;</synopsis> </sect3> </sect2> <sect2> + <title>Device Registration</title> + <para> + A number of functions are provided to help with device registration. + The functions deal with PCI, USB and platform devices, respectively. + </para> +!Fdrivers/gpu/drm/drm_pci.c drm_pci_init drm_pci_exit +!Fdrivers/gpu/drm/drm_usb.c drm_usb_init drm_usb_exit +!Fdrivers/gpu/drm/drm_platform.c drm_platform_init + <sect2> <title>Driver Load</title> <para> The <methodname>load</methodname> method is the driver and device diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index d237de36a07a..020cfd934854 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -1,17 +1,3 @@ -/* drm_pci.h -- PCI DMA memory management wrappers for DRM -*- linux-c -*- */ -/** - * \file drm_pci.c - * \brief Functions and ioctls to manage PCI memory - * - * \warning These interfaces aren't stable yet. - * - * \todo Implement the remaining ioctl's for the PCI pools. - * \todo The wrappers here are so thin that they would be better off inlined.. - * - * \author José Fonseca jrfonseca@tungstengraphics.com - * \author Leif Delgass ldelgass@retinalburn.net - */ - /* * Copyright 2003 José Fonseca. * Copyright 2003 Leif Delgass. @@ -42,12 +28,14 @@ #include <linux/export.h> #include <drm/drmP.h>
-/**********************************************************************/ -/** \name PCI memory */ -/*@{*/ - /** - * \brief Allocate a PCI consistent memory block, for DMA. + * drm_pci_alloc - Allocate a PCI consistent memory block, for DMA. + * @dev: DRM device + * @size: size of block to allocate + * @align: alignment of block + * + * Return: A handle to the allocated memory block on success or NULL on + * failure. */ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align) { @@ -88,8 +76,8 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
EXPORT_SYMBOL(drm_pci_alloc);
-/** - * \brief Free a PCI consistent memory block without freeing its descriptor. +/* + * Free a PCI consistent memory block without freeing its descriptor. * * This function is for internal use in the Linux-specific DRM core code. */ @@ -111,7 +99,9 @@ void __drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) }
/** - * \brief Free a PCI consistent memory block + * drm_pci_free - Free a PCI consistent memory block + * @dev: DRM device + * @dmah: handle to memory block */ void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) { @@ -226,17 +216,16 @@ static int drm_pci_irq_by_busid(struct drm_device *dev, struct drm_irq_busid *p) }
/** - * Get interrupt from bus id. - * - * \param inode device inode. - * \param file_priv DRM file private. - * \param cmd command. - * \param arg user argument, pointing to a drm_irq_busid structure. - * \return zero on success or a negative number on failure. + * drm_irq_by_busid - Get interrupt from bus ID + * @dev: DRM device + * @data: IOCTL parameter pointing to a drm_irq_busid structure + * @file_priv: DRM file private. * * Finds the PCI device with the specified bus id and gets its IRQ number. * This IOCTL is deprecated, and will now return EINVAL for any busid not equal * to that of the device that this DRM instance attached to. + * + * Return: 0 on success or a negative error code on failure. */ int drm_irq_by_busid(struct drm_device *dev, void *data, struct drm_file *file_priv) @@ -285,15 +274,16 @@ static struct drm_bus drm_pci_bus = { };
/** - * Register. - * - * \param pdev - PCI device structure - * \param ent entry from the PCI ID table with device type flags - * \return zero on success or a negative number on failure. + * drm_get_pci_dev - Register a PCI device with the DRM subsystem + * @pdev: PCI device + * @ent: entry from the PCI ID table that matches @pdev + * @driver: DRM device driver * * Attempt to gets inter module "drm" information. If we are first * then register the character device and inter module information. * Try and register, if we fail to register, backout previous work. + * + * Return: 0 on success or a negative error code on failure. */ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, struct drm_driver *driver) @@ -346,15 +336,14 @@ err_free: EXPORT_SYMBOL(drm_get_pci_dev);
/** - * PCI device initialization. Called direct from modules at load time. + * drm_pci_init - Register matching PCI devices with the DRM subsystem + * @driver: DRM device driver + * @pdriver: PCI device driver * - * \return zero on success or a negative number on failure. + * Initializes a drm_device structures, registering the stubs and initializing + * the AGP device. * - * Initializes a drm_device structures,registering the - * stubs and initializing the AGP device. - * - * Expands the \c DRIVER_PREINIT and \c DRIVER_POST_INIT macros before and - * after the initialization for driver customization. + * Return: 0 on success or a negative error code on failure. */ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) { @@ -458,7 +447,14 @@ int drm_pci_set_unique(struct drm_device *dev,
EXPORT_SYMBOL(drm_pci_init);
-/*@}*/ +/** + * drm_pci_exit - Unregister matching PCI devices from the DRM subsystem + * @driver: DRM device driver + * @pdriver: PCI device driver + * + * Unregisters one or more devices matched by a PCI driver from the DRM + * subsystem. + */ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) { struct drm_device *dev, *tmp; diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c index 234e0bc1ae51..d5b76f148c12 100644 --- a/drivers/gpu/drm/drm_platform.c +++ b/drivers/gpu/drm/drm_platform.c @@ -106,17 +106,16 @@ static struct drm_bus drm_platform_bus = { };
/** - * Platform device initialization. Called direct from modules. + * drm_platform_init - Register a platform device with the DRM subsystem + * @driver: DRM device driver + * @platform_device: platform device to register * - * \return zero on success or a negative number on failure. - * - * Initializes a drm_device structures,registering the - * stubs + * Registers the specified DRM device driver and platform device with the DRM + * subsystem, initializing a drm_device structure and calling the driver's + * .load() function. * - * Expands the \c DRIVER_PREINIT and \c DRIVER_POST_INIT macros before and - * after the initialization for driver customization. + * Return: 0 on success or a negative error code on failure. */ - int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device) { DRM_DEBUG("\n"); diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 64cf97dc4ce4..88bada850b71 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -1,16 +1,11 @@ -/** - * \file drm_stub.h - * Stub support - * - * \author Rickard E. (Rik) Faith faith@valinux.com - */ - /* * Created: Fri Jan 19 10:48:35 2001 by faith@acm.org * * Copyright 2001 VA Linux Systems, Inc., Sunnyvale, California. * All Rights Reserved. * + * Author Rickard E. (Rik) Faith faith@valinux.com + * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), * to deal in the Software without restriction, including without limitation @@ -424,11 +419,12 @@ void drm_minor_release(struct drm_minor *minor) }
/** - * Called via drm_exit() at module unload time or when pci device is - * unplugged. + * drm_put_dev - Unregister and release a DRM device + * @dev: DRM device * - * Cleans up all DRM device, calling drm_lastclose(). + * Called at module unload time or when a PCI device is unplugged. * + * Cleans up all DRM device, calling drm_lastclose(). */ void drm_put_dev(struct drm_device *dev) { @@ -558,7 +554,7 @@ int drm_set_unique(struct drm_device *dev, const char *fmt, ...) EXPORT_SYMBOL(drm_set_unique);
/** - * drm_dev_alloc - Allocate new drm device + * drm_dev_alloc - Allocate new DRM device * @driver: DRM driver to allocate device for * @parent: Parent device object * @@ -712,6 +708,7 @@ EXPORT_SYMBOL(drm_dev_unref); /** * drm_dev_register - Register DRM device * @dev: Device to register + * @flags: Flags passed to the driver's .load() function * * Register the DRM device @dev with the system, advertise device to user-space * and start normal device operation. @dev must be allocated via drm_dev_alloc() diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c index c6c7c29ad46f..f2fe94aab901 100644 --- a/drivers/gpu/drm/drm_usb.c +++ b/drivers/gpu/drm/drm_usb.c @@ -45,7 +45,17 @@ static int drm_usb_set_busid(struct drm_device *dev, static struct drm_bus drm_usb_bus = { .set_busid = drm_usb_set_busid, }; - + +/** + * drm_usb_init - Register matching USB devices with the DRM subsystem + * @driver: DRM device driver + * @udriver: USB device driver + * + * Registers one or more devices matched by a USB driver with the DRM + * subsystem. + * + * Return: 0 on success or a negative error code on failure. + */ int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver) { int res; @@ -58,6 +68,14 @@ int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver) } EXPORT_SYMBOL(drm_usb_init);
+/** + * drm_usb_exit - Unregister matching USB devices from the DRM subsystem + * @driver: DRM device driver + * @udriver: USB device driver + * + * Unregisters one or more devices matched by a USB driver from the DRM + * subsystem. + */ void drm_usb_exit(struct drm_driver *driver, struct usb_driver *udriver) {
On Tue, May 13, 2014 at 05:30:49PM +0200, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Describe how devices are registered using the drm_*_init() functions. Adding this to docbook requires a largish set of changes to the comments in drm_{pci,usb,platform}.c since they are doxygen-style rather than proper kernel-doc and therefore mess with the docbook generation.
Signed-off-by: Thierry Reding treding@nvidia.com
Documentation/DocBook/drm.tmpl | 9 +++++ drivers/gpu/drm/drm_pci.c | 80 ++++++++++++++++++++---------------------- drivers/gpu/drm/drm_platform.c | 15 ++++---- drivers/gpu/drm/drm_stub.c | 19 +++++----- drivers/gpu/drm/drm_usb.c | 20 ++++++++++- 5 files changed, 81 insertions(+), 62 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index f4028c3a28a2..f26a8e4fbe47 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -282,6 +282,15 @@ char *date;</synopsis> </sect3> </sect2> <sect2>
<title>Device Registration</title><para>A number of functions are provided to help with device registration.The functions deal with PCI, USB and platform devices, respectively.</para>+!Fdrivers/gpu/drm/drm_pci.c drm_pci_init drm_pci_exit +!Fdrivers/gpu/drm/drm_usb.c drm_usb_init drm_usb_exit +!Fdrivers/gpu/drm/drm_platform.c drm_platform_init
Generally I prefer to use !E to pull in all exported functions - those are the ones driver authors care about. And using this has the upside that if anyone renames them or adds/removes some it will be kept in sync automatically. Same for the next patch. -Daniel
<sect2> <title>Driver Load</title> <para> The <methodname>load</methodname> method is the driver and device
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index d237de36a07a..020cfd934854 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -1,17 +1,3 @@ -/* drm_pci.h -- PCI DMA memory management wrappers for DRM -*- linux-c -*- */ -/**
- \file drm_pci.c
- \brief Functions and ioctls to manage PCI memory
- \warning These interfaces aren't stable yet.
- \todo Implement the remaining ioctl's for the PCI pools.
- \todo The wrappers here are so thin that they would be better off inlined..
- \author José Fonseca jrfonseca@tungstengraphics.com
- \author Leif Delgass ldelgass@retinalburn.net
- */
/*
- Copyright 2003 José Fonseca.
- Copyright 2003 Leif Delgass.
@@ -42,12 +28,14 @@ #include <linux/export.h> #include <drm/drmP.h>
-/**********************************************************************/ -/** \name PCI memory */ -/*@{*/
/**
- \brief Allocate a PCI consistent memory block, for DMA.
- drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
- @dev: DRM device
- @size: size of block to allocate
- @align: alignment of block
- Return: A handle to the allocated memory block on success or NULL on
*/
- failure.
drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align) { @@ -88,8 +76,8 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
EXPORT_SYMBOL(drm_pci_alloc);
-/**
- \brief Free a PCI consistent memory block without freeing its descriptor.
+/*
*/
- Free a PCI consistent memory block without freeing its descriptor.
- This function is for internal use in the Linux-specific DRM core code.
@@ -111,7 +99,9 @@ void __drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) }
/**
- \brief Free a PCI consistent memory block
- drm_pci_free - Free a PCI consistent memory block
- @dev: DRM device
*/
- @dmah: handle to memory block
void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) { @@ -226,17 +216,16 @@ static int drm_pci_irq_by_busid(struct drm_device *dev, struct drm_irq_busid *p) }
/**
- Get interrupt from bus id.
- \param inode device inode.
- \param file_priv DRM file private.
- \param cmd command.
- \param arg user argument, pointing to a drm_irq_busid structure.
- \return zero on success or a negative number on failure.
- drm_irq_by_busid - Get interrupt from bus ID
- @dev: DRM device
- @data: IOCTL parameter pointing to a drm_irq_busid structure
- @file_priv: DRM file private.
- Finds the PCI device with the specified bus id and gets its IRQ number.
- This IOCTL is deprecated, and will now return EINVAL for any busid not equal
- to that of the device that this DRM instance attached to.
*/
- Return: 0 on success or a negative error code on failure.
int drm_irq_by_busid(struct drm_device *dev, void *data, struct drm_file *file_priv) @@ -285,15 +274,16 @@ static struct drm_bus drm_pci_bus = { };
/**
- Register.
- \param pdev - PCI device structure
- \param ent entry from the PCI ID table with device type flags
- \return zero on success or a negative number on failure.
- drm_get_pci_dev - Register a PCI device with the DRM subsystem
- @pdev: PCI device
- @ent: entry from the PCI ID table that matches @pdev
- @driver: DRM device driver
- Attempt to gets inter module "drm" information. If we are first
- then register the character device and inter module information.
- Try and register, if we fail to register, backout previous work.
*/
- Return: 0 on success or a negative error code on failure.
int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, struct drm_driver *driver) @@ -346,15 +336,14 @@ err_free: EXPORT_SYMBOL(drm_get_pci_dev);
/**
- PCI device initialization. Called direct from modules at load time.
- drm_pci_init - Register matching PCI devices with the DRM subsystem
- @driver: DRM device driver
- @pdriver: PCI device driver
- \return zero on success or a negative number on failure.
- Initializes a drm_device structures, registering the stubs and initializing
- the AGP device.
- Initializes a drm_device structures,registering the
- stubs and initializing the AGP device.
- Expands the \c DRIVER_PREINIT and \c DRIVER_POST_INIT macros before and
- after the initialization for driver customization.
*/
- Return: 0 on success or a negative error code on failure.
int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) { @@ -458,7 +447,14 @@ int drm_pci_set_unique(struct drm_device *dev,
EXPORT_SYMBOL(drm_pci_init);
-/*@}*/ +/**
- drm_pci_exit - Unregister matching PCI devices from the DRM subsystem
- @driver: DRM device driver
- @pdriver: PCI device driver
- Unregisters one or more devices matched by a PCI driver from the DRM
- subsystem.
- */
void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) { struct drm_device *dev, *tmp; diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c index 234e0bc1ae51..d5b76f148c12 100644 --- a/drivers/gpu/drm/drm_platform.c +++ b/drivers/gpu/drm/drm_platform.c @@ -106,17 +106,16 @@ static struct drm_bus drm_platform_bus = { };
/**
- Platform device initialization. Called direct from modules.
- drm_platform_init - Register a platform device with the DRM subsystem
- @driver: DRM device driver
- @platform_device: platform device to register
- \return zero on success or a negative number on failure.
- Initializes a drm_device structures,registering the
- stubs
- Registers the specified DRM device driver and platform device with the DRM
- subsystem, initializing a drm_device structure and calling the driver's
- .load() function.
- Expands the \c DRIVER_PREINIT and \c DRIVER_POST_INIT macros before and
- after the initialization for driver customization.
*/
- Return: 0 on success or a negative error code on failure.
int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device) { DRM_DEBUG("\n"); diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 64cf97dc4ce4..88bada850b71 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -1,16 +1,11 @@ -/**
- \file drm_stub.h
- Stub support
- \author Rickard E. (Rik) Faith faith@valinux.com
- */
/*
- Created: Fri Jan 19 10:48:35 2001 by faith@acm.org
- Copyright 2001 VA Linux Systems, Inc., Sunnyvale, California.
- All Rights Reserved.
- Author Rickard E. (Rik) Faith faith@valinux.com
- Permission is hereby granted, free of charge, to any person obtaining a
- copy of this software and associated documentation files (the "Software"),
- to deal in the Software without restriction, including without limitation
@@ -424,11 +419,12 @@ void drm_minor_release(struct drm_minor *minor) }
/**
- Called via drm_exit() at module unload time or when pci device is
- unplugged.
- drm_put_dev - Unregister and release a DRM device
- @dev: DRM device
- Cleans up all DRM device, calling drm_lastclose().
- Called at module unload time or when a PCI device is unplugged.
*/
- Cleans up all DRM device, calling drm_lastclose().
void drm_put_dev(struct drm_device *dev) { @@ -558,7 +554,7 @@ int drm_set_unique(struct drm_device *dev, const char *fmt, ...) EXPORT_SYMBOL(drm_set_unique);
/**
- drm_dev_alloc - Allocate new drm device
- drm_dev_alloc - Allocate new DRM device
- @driver: DRM driver to allocate device for
- @parent: Parent device object
@@ -712,6 +708,7 @@ EXPORT_SYMBOL(drm_dev_unref); /**
- drm_dev_register - Register DRM device
- @dev: Device to register
- @flags: Flags passed to the driver's .load() function
- Register the DRM device @dev with the system, advertise device to user-space
- and start normal device operation. @dev must be allocated via drm_dev_alloc()
diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c index c6c7c29ad46f..f2fe94aab901 100644 --- a/drivers/gpu/drm/drm_usb.c +++ b/drivers/gpu/drm/drm_usb.c @@ -45,7 +45,17 @@ static int drm_usb_set_busid(struct drm_device *dev, static struct drm_bus drm_usb_bus = { .set_busid = drm_usb_set_busid, };
+/**
- drm_usb_init - Register matching USB devices with the DRM subsystem
- @driver: DRM device driver
- @udriver: USB device driver
- Registers one or more devices matched by a USB driver with the DRM
- subsystem.
- Return: 0 on success or a negative error code on failure.
- */
int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver) { int res; @@ -58,6 +68,14 @@ int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver) } EXPORT_SYMBOL(drm_usb_init);
+/**
- drm_usb_exit - Unregister matching USB devices from the DRM subsystem
- @driver: DRM device driver
- @udriver: USB device driver
- Unregisters one or more devices matched by a USB driver from the DRM
- subsystem.
- */
void drm_usb_exit(struct drm_driver *driver, struct usb_driver *udriver) { -- 1.9.2
From: Thierry Reding treding@nvidia.com
With the recent addition of the drm_set_unique() function, devices can now be registered without requiring a drm_bus. Add a brief description to the DRM docbook to show how that can be achieved.
Signed-off-by: Thierry Reding treding@nvidia.com --- Documentation/DocBook/drm.tmpl | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index f26a8e4fbe47..f48227caf41a 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -142,6 +142,12 @@ to register it with the DRM subsystem. </para> <para> + Newer drivers that no longer require a <structname>drm_bus</structname> + structure can alternatively use the low-level device initialization and + registration functions such as <function>drm_dev_alloc()</function> and + <function>drm_dev_register()</function> directly. + </para> + <para> The <structname>drm_driver</structname> structure contains static information that describes the driver and features it supports, and pointers to methods that the DRM core will call to implement the DRM API. @@ -290,6 +296,29 @@ char *date;</synopsis> !Fdrivers/gpu/drm/drm_pci.c drm_pci_init drm_pci_exit !Fdrivers/gpu/drm/drm_usb.c drm_usb_init drm_usb_exit !Fdrivers/gpu/drm/drm_platform.c drm_platform_init + <para> + New drivers that no longer rely on the services provided by the + <structname>drm_bus</structname> structure can call the low-level + device registration functions directly. The + <function>drm_dev_alloc()</function> function can be used to allocate + and initialize a new <structname>drm_device</structname> structure. + Drivers will typically want to perform some additional setup on this + structure, such as allocating driver-specific data and storing a + pointer to it in the DRM device's <structfield>dev_private</structfield> + field. Drivers should also set the device's unique name using the + <function>drm_set_unique()</function> function. After it has been set up + a device can be registered with the DRM subsystem by calling + <function>drm_dev_register()</function>. This will cause the device to + be exposed to userspace and will call the driver's + <structfield>.load()</structfield> implementation. When a device is + removed, the DRM device can safely be unregistered and freed using a + call to <function>drm_put_dev()</function>. + </para> +!Fdrivers/gpu/drm/drm_stub.c drm_dev_alloc +!Fdrivers/gpu/drm/drm_stub.c drm_set_unique +!Fdrivers/gpu/drm/drm_stub.c drm_dev_register +!Fdrivers/gpu/drm/drm_stub.c drm_put_dev + </sect2> <sect2> <title>Driver Load</title> <para>
Hi
On Tue, May 13, 2014 at 5:30 PM, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
With the recent addition of the drm_set_unique() function, devices can now be registered without requiring a drm_bus. Add a brief description to the DRM docbook to show how that can be achieved.
Signed-off-by: Thierry Reding treding@nvidia.com
Documentation/DocBook/drm.tmpl | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index f26a8e4fbe47..f48227caf41a 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -142,6 +142,12 @@ to register it with the DRM subsystem. </para> <para>
Newer drivers that no longer require a <structname>drm_bus</structname>structure can alternatively use the low-level device initialization andregistration functions such as <function>drm_dev_alloc()</function> and<function>drm_dev_register()</function> directly.</para>
<para> The <structname>drm_driver</structname> structure contains static information that describes the driver and features it supports, and pointers to methods that the DRM core will call to implement the DRM API.
@@ -290,6 +296,29 @@ char *date;</synopsis> !Fdrivers/gpu/drm/drm_pci.c drm_pci_init drm_pci_exit !Fdrivers/gpu/drm/drm_usb.c drm_usb_init drm_usb_exit !Fdrivers/gpu/drm/drm_platform.c drm_platform_init
<para>New drivers that no longer rely on the services provided by the<structname>drm_bus</structname> structure can call the low-leveldevice registration functions directly. The<function>drm_dev_alloc()</function> function can be used to allocateand initialize a new <structname>drm_device</structname> structure.Drivers will typically want to perform some additional setup on thisstructure, such as allocating driver-specific data and storing apointer to it in the DRM device's <structfield>dev_private</structfield>field. Drivers should also set the device's unique name using the<function>drm_set_unique()</function> function. After it has been set upa device can be registered with the DRM subsystem by calling<function>drm_dev_register()</function>. This will cause the device tobe exposed to userspace and will call the driver's<structfield>.load()</structfield> implementation. When a device isremoved, the DRM device can safely be unregistered and freed using a
That's not true. drm_put_dev() is anything but safe.. but that's an implementation bug and I'm working on it. Not that easy, though. Furthermore, please use drm_dev_unregister() and drm_dev_unref() directly. No reason to use drm_put_dev() in new drivers. Once I fixed the runtime issues, a call to drm_dev_unregister() will _immediately_ remove a device and detach all active user-space connections.
call to <function>drm_put_dev()</function>.</para>+!Fdrivers/gpu/drm/drm_stub.c drm_dev_alloc
Please change this to:
+!Fdrivers/gpu/drm/drm_stub.c drm_dev_alloc +!Fdrivers/gpu/drm/drm_stub.c drm_dev_ref +!Fdrivers/gpu/drm/drm_stub.c drm_dev_unref +!Fdrivers/gpu/drm/drm_stub.c drm_dev_register +!Fdrivers/gpu/drm/drm_stub.c drm_dev_unregister +!Fdrivers/gpu/drm/drm_stub.c drm_dev_set_unique
All these helpers are needed and exported to drivers.
Otherwise, looks all good! Thanks! David
</sect2> <sect2> <title>Driver Load</title> <para>
-- 1.9.2
On Tue, May 13, 2014 at 05:30:43PM +0200, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Hi,
This series converts the Tegra DRM driver to the master/component framework. The length of the series and the list of people in Cc is mostly due to the fact that Tegra has some special requirements as opposed to other drivers and therefore requires some changes outside of DRM.
Patch 1 introduces a helper function that can be used by new DRM drivers that don't rely on legacy userspace ABIs. It allows them to register DRM devices much more easily and without much of the midlayer that currently exists in the DRM subsystem.
Patches 2 and 3 make some changes to the master/component framework which are necessary to convert Tegra DRM to use it. All current users of the API have been converted as part of this patch. Note that at the same time the drivers are converted to no longer use drm_platform_init() in favour of the drm_set_unique() from patch 1 in conjunction with drm_dev_alloc() and drm_dev_register(). I would've liked to avoid such invasive changes in a single patch, but unfortunately I couldn't think of a way how to do that. I'm open to suggestions.
Patch 4 adds a new interface framework that supplements the master/ component framework and can be used in situations where there is no struct device * that a driver can bind to.
The Tegra DRM driver is converted to using the master/component framework in patch 5 using the above four patches.
Finally patches 6 and 7 add some documentation about the new way of registering DRM devices that don't need legacy support.
Each patch has a somewhat more elaborate description of why it is needed or what problem it solves. The patchset applies on top of linux-next.
I welcome any questions or comments you might have.
Thierry
Thierry Reding (7): drm: Introduce drm_set_unique() drivers/base: Allow driver-data to be attached to a master drivers/base: Allow multiple masters per device drivers/base: Add interface framework drm/tegra: Convert to master/component framework drm: Add device registration documentation drm: Document how to register devices without struct drm_bus
Greg, Russell,
Any comments on this? I've been blocking a bunch of patches in the hopes of getting this merged since it allows the Tegra DRM driver to be cleaned up a great deal, but if nobody's going to look at or comment on this I'll abandon this series and keep using the custom solution we've had for a while now.
Thierry
On Thu, May 22, 2014 at 11:56:23AM +0200, Thierry Reding wrote:
On Tue, May 13, 2014 at 05:30:43PM +0200, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Hi,
This series converts the Tegra DRM driver to the master/component framework. The length of the series and the list of people in Cc is mostly due to the fact that Tegra has some special requirements as opposed to other drivers and therefore requires some changes outside of DRM.
Patch 1 introduces a helper function that can be used by new DRM drivers that don't rely on legacy userspace ABIs. It allows them to register DRM devices much more easily and without much of the midlayer that currently exists in the DRM subsystem.
Patches 2 and 3 make some changes to the master/component framework which are necessary to convert Tegra DRM to use it. All current users of the API have been converted as part of this patch. Note that at the same time the drivers are converted to no longer use drm_platform_init() in favour of the drm_set_unique() from patch 1 in conjunction with drm_dev_alloc() and drm_dev_register(). I would've liked to avoid such invasive changes in a single patch, but unfortunately I couldn't think of a way how to do that. I'm open to suggestions.
Patch 4 adds a new interface framework that supplements the master/ component framework and can be used in situations where there is no struct device * that a driver can bind to.
The Tegra DRM driver is converted to using the master/component framework in patch 5 using the above four patches.
Finally patches 6 and 7 add some documentation about the new way of registering DRM devices that don't need legacy support.
Each patch has a somewhat more elaborate description of why it is needed or what problem it solves. The patchset applies on top of linux-next.
I welcome any questions or comments you might have.
Thierry
Thierry Reding (7): drm: Introduce drm_set_unique() drivers/base: Allow driver-data to be attached to a master drivers/base: Allow multiple masters per device drivers/base: Add interface framework drm/tegra: Convert to master/component framework drm: Add device registration documentation drm: Document how to register devices without struct drm_bus
Greg, Russell,
Any comments on this? I've been blocking a bunch of patches in the hopes of getting this merged since it allows the Tegra DRM driver to be cleaned up a great deal, but if nobody's going to look at or comment on this I'll abandon this series and keep using the custom solution we've had for a while now.
I'd like Russell to comment on this, as he just added some driver core work to do drm device work that seems to me to be much the same here.
There's also the work that the media developers are working on to handle power management issues that seem to also be hitting the same area. Due to all of that, I really don't want to add yet-another-driver-core addition for just one drm driver, all of them should be able to use it.
thanks,
greg k-h
On Fri, May 23, 2014 at 08:03:00PM +0900, Greg Kroah-Hartman wrote:
On Thu, May 22, 2014 at 11:56:23AM +0200, Thierry Reding wrote:
On Tue, May 13, 2014 at 05:30:43PM +0200, Thierry Reding wrote:
From: Thierry Reding treding@nvidia.com
Hi,
This series converts the Tegra DRM driver to the master/component framework. The length of the series and the list of people in Cc is mostly due to the fact that Tegra has some special requirements as opposed to other drivers and therefore requires some changes outside of DRM.
Patch 1 introduces a helper function that can be used by new DRM drivers that don't rely on legacy userspace ABIs. It allows them to register DRM devices much more easily and without much of the midlayer that currently exists in the DRM subsystem.
Patches 2 and 3 make some changes to the master/component framework which are necessary to convert Tegra DRM to use it. All current users of the API have been converted as part of this patch. Note that at the same time the drivers are converted to no longer use drm_platform_init() in favour of the drm_set_unique() from patch 1 in conjunction with drm_dev_alloc() and drm_dev_register(). I would've liked to avoid such invasive changes in a single patch, but unfortunately I couldn't think of a way how to do that. I'm open to suggestions.
Patch 4 adds a new interface framework that supplements the master/ component framework and can be used in situations where there is no struct device * that a driver can bind to.
The Tegra DRM driver is converted to using the master/component framework in patch 5 using the above four patches.
Finally patches 6 and 7 add some documentation about the new way of registering DRM devices that don't need legacy support.
Each patch has a somewhat more elaborate description of why it is needed or what problem it solves. The patchset applies on top of linux-next.
I welcome any questions or comments you might have.
Thierry
Thierry Reding (7): drm: Introduce drm_set_unique() drivers/base: Allow driver-data to be attached to a master drivers/base: Allow multiple masters per device drivers/base: Add interface framework drm/tegra: Convert to master/component framework drm: Add device registration documentation drm: Document how to register devices without struct drm_bus
Greg, Russell,
Any comments on this? I've been blocking a bunch of patches in the hopes of getting this merged since it allows the Tegra DRM driver to be cleaned up a great deal, but if nobody's going to look at or comment on this I'll abandon this series and keep using the custom solution we've had for a while now.
I'd like Russell to comment on this, as he just added some driver core work to do drm device work that seems to me to be much the same here.
Yes. In fact that's what this series is all about. It converts the Tegra DRM driver to use Russell's master/component helpers. But there are also special requirements on Tegra DRM that require some enhancements to the helpers, which this series does. Furthermore the interface framework introduced here is supposed to complement the master/components helpers. It is not a duplicate implementation but attempts to solves a completely different problem.
There's also the work that the media developers are working on to handle power management issues that seem to also be hitting the same area. Due to all of that, I really don't want to add yet-another-driver-core addition for just one drm driver, all of them should be able to use it.
Like I said, this "interface" framework is meant to address something different and was specifically designed to be useful for more than just one DRM driver. However, people seem to prefer to make assumptions about this code rather than actually go look (or even read the description) what it really is about.
Besides I already have a working solution for the "one DRM driver". And replacing it with the component/master helpers as-is will make it worse than it is. Hence in order to avoid wasting more of everyone's time I have now dropped patches 2-4 and changed patch 5 to only convert to the drm_dev_set_unique() function, which is the more important piece of the series anyway.
Thierry
dri-devel@lists.freedesktop.org
-
Andrzej Hajda -
Christian Gmeiner -
Daniel Vetter -
David Herrmann -
Greg Kroah-Hartman -
Thierry Reding