Re: [PATCH 2/6] drm: dsi: Let host and device specify supported bus

[Paul Cercueil <paul@crapouillou.net>]

Hi Laurent,

Le lun. 27 juil. 2020 à 20:02, Laurent Pinchart 
<laurent.pinchart@ideasonboard.com> a écrit :
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Mon, Jul 27, 2020 at 06:46:09PM +0200, Paul Cercueil wrote:
>>  The current MIPI DSI framework can very well be used to support 
>> MIPI DBI
>>  panels. In order to add support for the various bus types supported 
>> by
>>  DBI, the DRM panel drivers should specify the bus type they will 
>> use,
>>  and the DSI host drivers should specify the bus types they are
>>  compatible with.
>> 
>>  The DSI host driver can then use the information provided by the 
>> DBI/DSI
>>  device driver, such as the bus type and the number of lanes, to
>>  configure its hardware properly.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   drivers/gpu/drm/drm_mipi_dsi.c |  9 +++++++++
>>   include/drm/drm_mipi_dsi.h     | 12 ++++++++++++
> 
> Use the mipi_dsi_* API for DBI panels will be very confusing to say 
> the
> least. Can we consider a global name refactoring to clarify all this ?

I was thinking that this could be done when the code is cleaned up and 
drivers/gpu/drm/drm_mipi_dbi.c is removed. I'm scared of tree-wide 
patchsets.

-Paul

> 
>>   2 files changed, 21 insertions(+)
>> 
>>  diff --git a/drivers/gpu/drm/drm_mipi_dsi.c 
>> b/drivers/gpu/drm/drm_mipi_dsi.c
>>  index 5dd475e82995..11ef885de765 100644
>>  --- a/drivers/gpu/drm/drm_mipi_dsi.c
>>  +++ b/drivers/gpu/drm/drm_mipi_dsi.c
>>  @@ -281,6 +281,9 @@ int mipi_dsi_host_register(struct mipi_dsi_host 
>> *host)
>>   {
>>   	struct device_node *node;
>> 
>>  +	if (WARN_ON_ONCE(!host->bus_types))
>>  +		host->bus_types = MIPI_DEVICE_TYPE_DSI;
>>  +
>>   	for_each_available_child_of_node(host->dev->of_node, node) {
>>   		/* skip nodes without reg property */
>>   		if (!of_find_property(node, "reg", NULL))
>>  @@ -323,6 +326,12 @@ int mipi_dsi_attach(struct mipi_dsi_device 
>> *dsi)
>>   {
>>   	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
>> 
>>  +	if (WARN_ON_ONCE(!dsi->bus_type))
>>  +		dsi->bus_type = MIPI_DEVICE_TYPE_DSI;
>>  +
>>  +	if (!(dsi->bus_type & dsi->host->bus_types))
>>  +		return -EINVAL;
>>  +
>>   	if (!ops || !ops->attach)
>>   		return -ENOSYS;
>> 
>>  diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>>  index 360e6377e84b..65d2961fc054 100644
>>  --- a/include/drm/drm_mipi_dsi.h
>>  +++ b/include/drm/drm_mipi_dsi.h
>>  @@ -63,6 +63,14 @@ struct mipi_dsi_packet {
>>   int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
>>   			   const struct mipi_dsi_msg *msg);
>> 
>>  +/* MIPI bus types */
>>  +#define MIPI_DEVICE_TYPE_DSI		BIT(0)
>>  +#define MIPI_DEVICE_TYPE_DBI_SPI_MODE1	BIT(1)
>>  +#define MIPI_DEVICE_TYPE_DBI_SPI_MODE2	BIT(2)
>>  +#define MIPI_DEVICE_TYPE_DBI_SPI_MODE3	BIT(3)
>>  +#define MIPI_DEVICE_TYPE_DBI_M6800	BIT(4)
>>  +#define MIPI_DEVICE_TYPE_DBI_I8080	BIT(5)
>>  +
>>   /**
>>    * struct mipi_dsi_host_ops - DSI bus operations
>>    * @attach: attach DSI device to DSI host
>>  @@ -94,11 +102,13 @@ struct mipi_dsi_host_ops {
>>    * struct mipi_dsi_host - DSI host device
>>    * @dev: driver model device node for this DSI host
>>    * @ops: DSI host operations
>>  + * @bus_types: Bitmask of supported MIPI bus types
>>    * @list: list management
>>    */
>>   struct mipi_dsi_host {
>>   	struct device *dev;
>>   	const struct mipi_dsi_host_ops *ops;
>>  +	unsigned int bus_types;
>>   	struct list_head list;
>>   };
>> 
>>  @@ -162,6 +172,7 @@ struct mipi_dsi_device_info {
>>    * @host: DSI host for this peripheral
>>    * @dev: driver model device node for this peripheral
>>    * @name: DSI peripheral chip type
>>  + * @bus_type: MIPI bus type (MIPI_DEVICE_TYPE_DSI/...)
>>    * @channel: virtual channel assigned to the peripheral
>>    * @format: pixel format for video mode
>>    * @lanes: number of active data lanes
>>  @@ -178,6 +189,7 @@ struct mipi_dsi_device {
>>   	struct device dev;
>> 
>>   	char name[DSI_DEV_NAME_SIZE];
>>  +	unsigned int bus_type;
>>   	unsigned int channel;
>>   	unsigned int lanes;
>>   	enum mipi_dsi_pixel_format format;
> 
> --
> Regards,
> 
> Laurent Pinchart