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 <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 9 +++++++++
include/drm/drm_mipi_dsi.h | 12 ++++++++++++
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;
--
2.27.0
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 <[email protected]>
> ---
> 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 ?
> 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
Hi Laurent,
Le lun. 27 juil. 2020 ? 20:02, Laurent Pinchart
<[email protected]> 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 <[email protected]>
>> ---
>> 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
Hi Paul.
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 <[email protected]>
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 9 +++++++++
> include/drm/drm_mipi_dsi.h | 12 ++++++++++++
> 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;
> +
So all 14 users need to specify bus_types.
Seems doable.
> 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;
We have ~50 users of mipi_dsi_attach() - doable. But a bit more work.
> +
> + 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 */
If you define this as an enum then kernel-doc syntax will be picked up.
See for example: enum drm_driver_feature
> +#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
Please add some kind of reference to MIPI_DEVICE_TYPE_* - so the reader
knows for sure this is the bits used here.
> * @list: list management
> */
> struct mipi_dsi_host {
> struct device *dev;
> const struct mipi_dsi_host_ops *ops;
> + unsigned int bus_types;
Use u32. Shorter and we know this is 32 bits wide.
> 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;
Use u32.
> unsigned int channel;
> unsigned int lanes;
> enum mipi_dsi_pixel_format format;
> --
> 2.27.0