[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6fbaa797-1040-48e8-c361-dad4363cd30d@codeaurora.org>
Date: Thu, 29 Jun 2017 10:33:17 +0530
From: Archit Taneja <architt@...eaurora.org>
To: Eric Anholt <eric@...olt.net>, dri-devel@...ts.freedesktop.org,
Andrzej Hajda <a.hajda@...sung.com>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Thierry Reding <thierry.reding@...il.com>
Cc: linux-kernel@...r.kernel.org,
Boris Brezillon <boris.brezillon@...e-electrons.com>
Subject: Re: [PATCH 6/8] drm: Allow DSI devices to be registered before the
host registers.
On 06/28/2017 01:28 AM, Eric Anholt wrote:
> When a mipi_dsi_host is registered, the DT is walked to find any child
> nodes with compatible strings. Those get registered as DSI devices,
> and most DSI panel drivers are mipi_dsi_drivers that attach to those nodes.
>
> There is one special case currently, the adv7533 bridge, where the
> bridge probes on I2C, and during the bridge attach step it looks up
> the mipi_dsi_host and registers the mipi_dsi_device (for its own stub
> mipi_dsi_driver).
>
> For the Raspberry Pi panel, though, we also need to attach on I2C (our
> control bus), but don't have a bridge driver. The lack of a bridge's
> attach() step like adv7533 uses means that we aren't able to delay the
> mipi_dsi_device creation until the mipi_dsi_host is present.
>
> To fix this, we extend mipi_dsi_device_register_full() to allow being
> called with a NULL host, which puts the device on a queue waiting for
> a host to appear. When a new host is registered, we fill in the host
> value and finish the device creation process.
This is quite a nice idea. The only bothering thing is the info.of_node usage
varies between child nodes (mipi_dsi_devs) and non-child nodes (i2c control
bus).
For DSI children expressed in DT, the of_node in info holds the DT node
corresponding to the DSI child itself. For non-DT ones, this patch assumes
that info.of_node stores the DSI host DT node. I think it should be okay as
long as we mention the usage in a comment somewhere. The other option is to
have a new info.host_node field to keep a track of the host DT node.
Thanks,
Archit
>
> Signed-off-by: Eric Anholt <eric@...olt.net>
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 49 ++++++++++++++++++++++++++++++++----------
> include/drm/drm_mipi_dsi.h | 3 +++
> 2 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 1160a579e0dc..9cdd68a7dc0d 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -45,6 +45,13 @@
> * subset of the MIPI DCS command set.
> */
>
> +static DEFINE_MUTEX(host_lock);
> +static LIST_HEAD(host_list);
> +/* List of struct mipi_dsi_device which were registered while no host
> + * was available.
> + */
> +static LIST_HEAD(unattached_device_list);
> +
> static int mipi_dsi_device_match(struct device *dev, struct device_driver *drv)
> {
> struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);
> @@ -138,10 +145,12 @@ static struct mipi_dsi_device *mipi_dsi_device_alloc(struct mipi_dsi_host *host)
>
> dsi->host = host;
> dsi->dev.bus = &mipi_dsi_bus_type;
> - dsi->dev.parent = host->dev;
> dsi->dev.type = &mipi_dsi_device_type;
>
> - device_initialize(&dsi->dev);
> + if (dsi->host) {
> + dsi->dev.parent = host->dev;
> + device_initialize(&dsi->dev);
> + }
>
> return dsi;
> }
> @@ -206,7 +215,7 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
> const struct mipi_dsi_device_info *info)
> {
> struct mipi_dsi_device *dsi;
> - struct device *dev = host->dev;
> + struct device *dev = host ? host->dev : NULL;
> int ret;
>
> if (!info) {
> @@ -230,11 +239,17 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
> dsi->channel = info->channel;
> strlcpy(dsi->name, info->type, sizeof(dsi->name));
>
> - ret = mipi_dsi_device_add(dsi);
> - if (ret) {
> - dev_err(dev, "failed to add DSI device %d\n", ret);
> - kfree(dsi);
> - return ERR_PTR(ret);
> + if (!dsi->host) {
> + mutex_lock(&host_lock);
> + list_add(&dsi->list, &unattached_device_list);
> + mutex_unlock(&host_lock);
> + } else {
> + ret = mipi_dsi_device_add(dsi);
> + if (ret) {
> + dev_err(dev, "failed to add DSI device %d\n", ret);
> + kfree(dsi);
> + return ERR_PTR(ret);
> + }
> }
>
> return dsi;
> @@ -251,9 +266,6 @@ void mipi_dsi_device_unregister(struct mipi_dsi_device *dsi)
> }
> EXPORT_SYMBOL(mipi_dsi_device_unregister);
>
> -static DEFINE_MUTEX(host_lock);
> -static LIST_HEAD(host_list);
> -
> /**
> * of_find_mipi_dsi_host_by_node() - find the MIPI DSI host matching a
> * device tree node
> @@ -285,6 +297,7 @@ EXPORT_SYMBOL(of_find_mipi_dsi_host_by_node);
> int mipi_dsi_host_register(struct mipi_dsi_host *host)
> {
> struct device_node *node;
> + struct mipi_dsi_device *dsi, *temp;
>
> for_each_available_child_of_node(host->dev->of_node, node) {
> /* skip nodes without reg property */
> @@ -295,6 +308,20 @@ int mipi_dsi_host_register(struct mipi_dsi_host *host)
>
> mutex_lock(&host_lock);
> list_add_tail(&host->list, &host_list);
> +
> + /* If any DSI devices were registered under our OF node, then
> + * connect our host to it and probe them now.
> + */
> + list_for_each_entry_safe(dsi, temp, &unattached_device_list, list) {
> + if (of_get_parent(dsi->dev.of_node) == host->dev->of_node) {
> + dsi->host = host;
> + dsi->dev.parent = host->dev;
> + device_initialize(&dsi->dev);
> +
> + mipi_dsi_device_add(dsi);
> + list_del_init(&dsi->list);
> + }
> + }
> mutex_unlock(&host_lock);
>
> return 0;
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 4fef19064b0f..699ea4acd5b6 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -178,6 +178,9 @@ struct mipi_dsi_device {
> unsigned int lanes;
> enum mipi_dsi_pixel_format format;
> unsigned long mode_flags;
> +
> + /* Entry on the unattached_device_list */
> + struct list_head list;
> };
>
> #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Powered by blists - more mailing lists