Re: [PATCH 6/8] drm: Allow DSI devices to be registered before the host registers.

[Archit Taneja]

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@xxxxxxxxxx>
> ---
>   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