A while back, Laurent raised some comments about the component helper, which this patch set starts to address.
The first point it addresses is the repeated parsing inefficiency when deferred probing occurs. When DT is used, the structure of the component helper today means that masters end up parsing the device tree for each attempt to re-bind the driver. We remove this inefficiency by creating an array of matching data and functions, which the component helper can use internally to match up components to their master.
The second point was the inefficiency of destroying the list of components each time we saw a failure. We did this to ensure that we kept things correctly ordered: component bind order matters. As we have an array instead, the array is already ordered, so we use this array to store the component pointers instead of a list, and remember which are duplicates (and thus should be avoided.) Avoiding the right duplicates matters as we walk the array in the opposite direction at tear down.
I hope that this is the final submission in patch form. Ultimately, these patches need to be handled carefully:
Patch Trees 1, 2, 3 Driver, Staging, DRM 4 Driver, DRM 5 Driver, Staging 6, 7, 8 To be held back until all users are updated (Exynos DRM)
drivers/base/component.c | 249 ++++++++++++++++++++++----------- drivers/gpu/drm/msm/msm_drv.c | 83 +++++------ drivers/staging/imx-drm/imx-drm-core.c | 57 +------- include/linux/component.h | 8 +- 4 files changed, 208 insertions(+), 189 deletions(-)
Update MSM's DRM driver to use the component match support rather than add_components.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- This patch is destined for Greg's driver and David's DRM trees.
drivers/gpu/drm/msm/msm_drv.c | 83 ++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 9a5d87db5c23..a322029983ce 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -905,12 +905,41 @@ static int compare_of(struct device *dev, void *data) { return dev->of_node == data; } +#else +static int compare_dev(struct device *dev, void *data) +{ + return dev == data; +} +#endif + +static int msm_drm_bind(struct device *dev) +{ + return drm_platform_init(&msm_driver, to_platform_device(dev)); +} + +static void msm_drm_unbind(struct device *dev) +{ + drm_put_dev(platform_get_drvdata(to_platform_device(dev))); +} + +static const struct component_master_ops msm_drm_ops = { + .bind = msm_drm_bind, + .unbind = msm_drm_unbind, +}; + +/* + * Platform driver: + */
-static int msm_drm_add_components(struct device *master, struct master *m) +static int msm_pdev_probe(struct platform_device *pdev) { - struct device_node *np = master->of_node; + struct component_match *match = NULL; +#ifdef CONFIG_OF + /* NOTE: the CONFIG_OF case duplicates the same code as exynos or imx + * (or probably any other).. so probably some room for some helpers + */ + struct device_node *np = pdev->dev.of_node; unsigned i; - int ret;
for (i = 0; ; i++) { struct device_node *node; @@ -919,22 +948,9 @@ static int msm_drm_add_components(struct device *master, struct master *m) if (!node) break;
- ret = component_master_add_child(m, compare_of, node); - of_node_put(node); - - if (ret) - return ret; + component_match_add(&pdev->dev, &match, compare_of, node); } - return 0; -} #else -static int compare_dev(struct device *dev, void *data) -{ - return dev == data; -} - -static int msm_drm_add_components(struct device *master, struct master *m) -{ /* 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 * they are simply not present). But for non-DT we only need to @@ -958,41 +974,12 @@ static int msm_drm_add_components(struct device *master, struct master *m) return -EPROBE_DEFER; }
- ret = component_master_add_child(m, compare_dev, dev); - if (ret) { - DBG("could not add child: %d", ret); - return ret; - } + component_match_add(&pdev->dev, &match, compare_dev, dev); } - - return 0; -} #endif
-static int msm_drm_bind(struct device *dev) -{ - return drm_platform_init(&msm_driver, to_platform_device(dev)); -} - -static void msm_drm_unbind(struct device *dev) -{ - drm_put_dev(platform_get_drvdata(to_platform_device(dev))); -} - -static const struct component_master_ops msm_drm_ops = { - .add_components = msm_drm_add_components, - .bind = msm_drm_bind, - .unbind = msm_drm_unbind, -}; - -/* - * Platform driver: - */ - -static int msm_pdev_probe(struct platform_device *pdev) -{ pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); - return component_master_add(&pdev->dev, &msm_drm_ops); + return component_master_add_with_match(&pdev->dev, &msm_drm_ops, match); }
static int msm_pdev_remove(struct platform_device *pdev)
Update MSM's DRM driver to use the component match support rather than add_components.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- This patch is destined for Greg's driver and David's DRM trees.
drivers/gpu/drm/msm/msm_drv.c | 83 ++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 9a5d87db5c23..a322029983ce 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -905,12 +905,41 @@ static int compare_of(struct device *dev, void *data) { return dev->of_node == data; } +#else +static int compare_dev(struct device *dev, void *data) +{ + return dev == data; +} +#endif + +static int msm_drm_bind(struct device *dev) +{ + return drm_platform_init(&msm_driver, to_platform_device(dev)); +} + +static void msm_drm_unbind(struct device *dev) +{ + drm_put_dev(platform_get_drvdata(to_platform_device(dev))); +} + +static const struct component_master_ops msm_drm_ops = { + .bind = msm_drm_bind, + .unbind = msm_drm_unbind, +}; + +/* + * Platform driver: + */
-static int msm_drm_add_components(struct device *master, struct master *m) +static int msm_pdev_probe(struct platform_device *pdev) { - struct device_node *np = master->of_node; + struct component_match *match = NULL; +#ifdef CONFIG_OF + /* NOTE: the CONFIG_OF case duplicates the same code as exynos or imx + * (or probably any other).. so probably some room for some helpers + */ + struct device_node *np = pdev->dev.of_node; unsigned i; - int ret;
for (i = 0; ; i++) { struct device_node *node; @@ -919,22 +948,9 @@ static int msm_drm_add_components(struct device *master, struct master *m) if (!node) break;
- ret = component_master_add_child(m, compare_of, node); - of_node_put(node); - - if (ret) - return ret; + component_match_add(&pdev->dev, &match, compare_of, node); } - return 0; -} #else -static int compare_dev(struct device *dev, void *data) -{ - return dev == data; -} - -static int msm_drm_add_components(struct device *master, struct master *m) -{ /* 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 * they are simply not present). But for non-DT we only need to @@ -958,41 +974,12 @@ static int msm_drm_add_components(struct device *master, struct master *m) return -EPROBE_DEFER; }
- ret = component_master_add_child(m, compare_dev, dev); - if (ret) { - DBG("could not add child: %d", ret); - return ret; - } + component_match_add(&pdev->dev, &match, compare_dev, dev); } - - return 0; -} #endif
-static int msm_drm_bind(struct device *dev) -{ - return drm_platform_init(&msm_driver, to_platform_device(dev)); -} - -static void msm_drm_unbind(struct device *dev) -{ - drm_put_dev(platform_get_drvdata(to_platform_device(dev))); -} - -static const struct component_master_ops msm_drm_ops = { - .add_components = msm_drm_add_components, - .bind = msm_drm_bind, - .unbind = msm_drm_unbind, -}; - -/* - * Platform driver: - */ - -static int msm_pdev_probe(struct platform_device *pdev) -{ pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); - return component_master_add(&pdev->dev, &msm_drm_ops); + return component_master_add_with_match(&pdev->dev, &msm_drm_ops, match); }
static int msm_pdev_remove(struct platform_device *pdev)
On Tue, Jul 01, 2014 at 03:40:11PM +0100, Russell King - ARM Linux wrote:
A while back, Laurent raised some comments about the component helper, which this patch set starts to address.
I looked back over the two other times which this series has posted, and noticed that two patches had been reviewed, so I've added those tags.
Unless there's any objections from anyone, I'll send the first three off to Greg either tonight or tomorrow night, which at least gets us moving forward on this.
If anyone has any objections, please shout ASAP. Thanks.
Hi Russell,
Sorry for the late review.
On Wednesday 02 July 2014 15:59:04 Russell King - ARM Linux wrote:
On Tue, Jul 01, 2014 at 03:40:11PM +0100, Russell King - ARM Linux wrote:
A while back, Laurent raised some comments about the component helper, which this patch set starts to address.
I looked back over the two other times which this series has posted, and noticed that two patches had been reviewed, so I've added those tags.
Unless there's any objections from anyone, I'll send the first three off to Greg either tonight or tomorrow night, which at least gets us moving forward on this.
If anyone has any objections, please shout ASAP. Thanks.
No objection from my side at all, this is a nice improvement. For the first three patches,
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
On Thu, Jul 03, 2014 at 01:51:19AM +0200, Laurent Pinchart wrote:
On Wednesday 02 July 2014 15:59:04 Russell King - ARM Linux wrote:
On Tue, Jul 01, 2014 at 03:40:11PM +0100, Russell King - ARM Linux wrote:
A while back, Laurent raised some comments about the component helper, which this patch set starts to address.
I looked back over the two other times which this series has posted, and noticed that two patches had been reviewed, so I've added those tags.
Unless there's any objections from anyone, I'll send the first three off to Greg either tonight or tomorrow night, which at least gets us moving forward on this.
If anyone has any objections, please shout ASAP. Thanks.
No objection from my side at all, this is a nice improvement. For the first three patches,
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
Thanks, ack added.
dri-devel@lists.freedesktop.org
-
Laurent Pinchart -
Russell King -
Russell King - ARM Linux