2015-11-30 12:01:41

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v3 0/5] drm/dsi: DSI for devices with different control bus

We are currently restricted when it comes to supporting DSI on devices
that have a non-DSI control bus. For example, DSI encoder chips are
available in the market that are configured via i2c. Configuring their
registers via DSI bus is either optional or not available at all.

These devices still need to pass DSI parameters (data lanes, mode flags
etc) to the DSI host they are connected to. We don't have a way to do
that at the moment.

After some discussions on the previous RFC[1], we decided to support this
by providing additional API in drm_mipi_dsi which lets us create new DSI
devices without the need of them to have a DT node.

[1]: https://lkml.org/lkml/2015/6/30/42

Changes from v2 to v3:

- Incorporated misc comments by Andrzej. Changed from RFC to a PATCH set.
- Fixed htmldocs warnings.

Archit Taneja (5):
drm/dsi: Refactor device creation
drm/dsi: Try to match non-DT dsi devices
drm/dsi: Check for used channels
drm/dsi: Add routine to unregister dsi device
drm/dsi: Get DSI host by DT device node

drivers/gpu/drm/drm_mipi_dsi.c | 136 +++++++++++++++++++++++++++++++----------
include/drm/drm_mipi_dsi.h | 29 +++++++++
2 files changed, 133 insertions(+), 32 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


2015-11-30 12:01:47

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v3 1/5] drm/dsi: Refactor device creation

Simplify the mipi dsi device creation process. device_initialize and
device_add don't need to be called separately when creating
mipi_dsi_device's. Use device_register instead to simplify things.

Create a helper function mipi_dsi_device_new which takes in struct
mipi_dsi_device_info and mipi_dsi_host. It clubs the functions
mipi_dsi_device_alloc and mipi_dsi_device_add into one.

mipi_dsi_device_info acts as a template to populate the dsi device
information. This is populated by of_mipi_dsi_device_add and passed to
mipi_dsi_device_new.

Later on, we'll provide mipi_dsi_device_new as a standalone way to create
a dsi device not available via DT.

The new device creation process tries to closely follow what's been done
in i2c_new_device in i2c-core.

Reviewed-by: Andrzej Hajda <[email protected]>
Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 61 +++++++++++++++++-------------------------
include/drm/drm_mipi_dsi.h | 15 +++++++++++
2 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 2d5ca8ee..82bcdcd 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -102,9 +102,18 @@ static const struct device_type mipi_dsi_device_type = {
.release = mipi_dsi_dev_release,
};

-static struct mipi_dsi_device *mipi_dsi_device_alloc(struct mipi_dsi_host *host)
+struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
+ struct mipi_dsi_device_info *info)
{
struct mipi_dsi_device *dsi;
+ struct device *dev = host->dev;
+ int ret;
+
+ if (info->reg > 3) {
+ dev_err(dev, "device node has invalid reg property: %u\n",
+ info->reg);
+ return ERR_PTR(-EINVAL);
+ }

dsi = kzalloc(sizeof(*dsi), GFP_KERNEL);
if (!dsi)
@@ -114,26 +123,27 @@ static struct mipi_dsi_device *mipi_dsi_device_alloc(struct mipi_dsi_host *host)
dsi->dev.bus = &mipi_dsi_bus_type;
dsi->dev.parent = host->dev;
dsi->dev.type = &mipi_dsi_device_type;
+ dsi->dev.of_node = info->node;
+ dsi->channel = info->reg;

- device_initialize(&dsi->dev);
-
- return dsi;
-}
-
-static int mipi_dsi_device_add(struct mipi_dsi_device *dsi)
-{
- struct mipi_dsi_host *host = dsi->host;
+ dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg);

- dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), dsi->channel);
+ ret = device_register(&dsi->dev);
+ if (ret) {
+ dev_err(dev, "failed to register device: %d\n", ret);
+ kfree(dsi);
+ return ERR_PTR(ret);
+ }

- return device_add(&dsi->dev);
+ return dsi;
}
+EXPORT_SYMBOL(mipi_dsi_device_new);

static struct mipi_dsi_device *
of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
{
- struct mipi_dsi_device *dsi;
struct device *dev = host->dev;
+ struct mipi_dsi_device_info info = { };
int ret;
u32 reg;

@@ -144,31 +154,10 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
return ERR_PTR(-EINVAL);
}

- if (reg > 3) {
- dev_err(dev, "device node %s has invalid reg property: %u\n",
- node->full_name, reg);
- return ERR_PTR(-EINVAL);
- }
-
- dsi = mipi_dsi_device_alloc(host);
- if (IS_ERR(dsi)) {
- dev_err(dev, "failed to allocate DSI device %s: %ld\n",
- node->full_name, PTR_ERR(dsi));
- return dsi;
- }
-
- dsi->dev.of_node = of_node_get(node);
- dsi->channel = reg;
+ info.reg = reg;
+ info.node = of_node_get(node);

- ret = mipi_dsi_device_add(dsi);
- if (ret) {
- dev_err(dev, "failed to add DSI device %s: %d\n",
- node->full_name, ret);
- kfree(dsi);
- return ERR_PTR(ret);
- }
-
- return dsi;
+ return mipi_dsi_device_new(host, &info);
}

int mipi_dsi_host_register(struct mipi_dsi_host *host)
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index f1d8d0d..90f4f3c 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -140,6 +140,19 @@ enum mipi_dsi_pixel_format {
};

/**
+ * struct mipi_dsi_device_info - template for creating a mipi_dsi_device
+ * @reg: DSI virtual channel assigned to peripheral
+ * @node: pointer to OF device node
+ *
+ * This is populated and passed to mipi_dsi_device_new to create a new
+ * DSI device
+ */
+struct mipi_dsi_device_info {
+ u32 reg;
+ struct device_node *node;
+};
+
+/**
* struct mipi_dsi_device - DSI peripheral device
* @host: DSI host for this peripheral
* @dev: driver model device node for this peripheral
@@ -174,6 +187,8 @@ ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, const void *payload,
ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params,
size_t num_params, void *data, size_t size);

+struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
+ struct mipi_dsi_device_info *info);
/**
* enum mipi_dsi_dcs_tear_mode - Tearing Effect Output Line mode
* @MIPI_DSI_DCS_TEAR_MODE_VBLANK: the TE output line consists of V-Blanking
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-11-30 12:01:49

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices

Add a device name field in mipi_dsi_device. This name is different from
the actual dev name (which is of the format "hostname.reg"). When the
device is created via DT, this name is set to the modalias string.
In the non-DT case, the driver creating the DSI device provides the
name by populating a filed in mipi_dsi_device_info.

Matching for DT case would be as it was before. For the non-DT case,
we compare the device and driver names. Other buses (like i2c/spi)
perform a non-DT match by comparing the device name and entries in the
driver's id_table. Such a mechanism isn't used for the dsi bus.

Reviewed-by: Andrzej Hajda <[email protected]>
Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 25 ++++++++++++++++++++++++-
include/drm/drm_mipi_dsi.h | 6 ++++++
2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 82bcdcd..143cce4 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -45,9 +45,26 @@
* subset of the MIPI DCS command set.
*/

+static const struct device_type mipi_dsi_device_type;
+
static int mipi_dsi_device_match(struct device *dev, struct device_driver *drv)
{
- return of_driver_match_device(dev, drv);
+ struct mipi_dsi_device *dsi;
+
+ if (dev->type == &mipi_dsi_device_type)
+ dsi = to_mipi_dsi_device(dev);
+ else
+ return 0;
+
+ /* attempt OF style match */
+ if (of_driver_match_device(dev, drv))
+ return 1;
+
+ /* compare dsi device and driver names */
+ if (!strcmp(dsi->name, drv->name))
+ return 1;
+
+ return 0;
}

static const struct dev_pm_ops mipi_dsi_device_pm_ops = {
@@ -125,6 +142,7 @@ struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
dsi->dev.type = &mipi_dsi_device_type;
dsi->dev.of_node = info->node;
dsi->channel = info->reg;
+ strlcpy(dsi->name, info->type, sizeof(dsi->name));

dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg);

@@ -147,6 +165,11 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
int ret;
u32 reg;

+ if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
+ dev_err(dev, "modalias failure on %s\n", node->full_name);
+ return ERR_PTR(-EINVAL);
+ }
+
ret = of_property_read_u32(node, "reg", &reg);
if (ret) {
dev_err(dev, "device node %s has no valid reg property: %d\n",
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 90f4f3c..cb084af 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -139,8 +139,11 @@ enum mipi_dsi_pixel_format {
MIPI_DSI_FMT_RGB565,
};

+#define DSI_DEV_NAME_SIZE 20
+
/**
* struct mipi_dsi_device_info - template for creating a mipi_dsi_device
+ * @type: dsi peripheral chip type
* @reg: DSI virtual channel assigned to peripheral
* @node: pointer to OF device node
*
@@ -148,6 +151,7 @@ enum mipi_dsi_pixel_format {
* DSI device
*/
struct mipi_dsi_device_info {
+ char type[DSI_DEV_NAME_SIZE];
u32 reg;
struct device_node *node;
};
@@ -156,6 +160,7 @@ struct mipi_dsi_device_info {
* struct mipi_dsi_device - DSI peripheral device
* @host: DSI host for this peripheral
* @dev: driver model device node for this peripheral
+ * @name: dsi peripheral chip type
* @channel: virtual channel assigned to the peripheral
* @format: pixel format for video mode
* @lanes: number of active data lanes
@@ -165,6 +170,7 @@ struct mipi_dsi_device {
struct mipi_dsi_host *host;
struct device dev;

+ char name[DSI_DEV_NAME_SIZE];
unsigned int channel;
unsigned int lanes;
enum mipi_dsi_pixel_format format;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-11-30 12:01:52

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v3 3/5] drm/dsi: Check for used channels

We don't check whether a previously registered mipi_dsi_device under the
same host shares the same virtual channel.

Before registering, check if any of the registered devices doesn't
already have the same virtual channel.

This wasn't crucial when all the devices under a host were populated via
DT. Now that we also support creating devices manually, we could end up
in a situation where a driver tries to create a device with a virtual
channel already taken by a device populated in DT.

Reviewed-by: Andrzej Hajda <[email protected]>
Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 143cce4..f73e434 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -119,6 +119,22 @@ static const struct device_type mipi_dsi_device_type = {
.release = mipi_dsi_dev_release,
};

+static int __dsi_check_chan_busy(struct device *dev, void *data)
+{
+ struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);
+ u32 *reg = data;
+
+ if (dsi && dsi->channel == *reg)
+ return -EBUSY;
+
+ return 0;
+}
+
+static int mipi_dsi_check_chan_busy(struct mipi_dsi_host *host, u32 reg)
+{
+ return device_for_each_child(host->dev, &reg, __dsi_check_chan_busy);
+}
+
struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
struct mipi_dsi_device_info *info)
{
@@ -146,14 +162,20 @@ struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,

dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg);

+ ret = mipi_dsi_check_chan_busy(host, info->reg);
+ if (ret)
+ goto err;
+
ret = device_register(&dsi->dev);
if (ret) {
dev_err(dev, "failed to register device: %d\n", ret);
- kfree(dsi);
- return ERR_PTR(ret);
+ goto err;
}

return dsi;
+err:
+ kfree(dsi);
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL(mipi_dsi_device_new);

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-11-30 12:01:56

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v3 4/5] drm/dsi: Add routine to unregister dsi device

A driver calling mipi_dsi_device_new might want to unregister the device
once it's done. It might also require it in an error handling path in
case something didn't go right.

Signed-off-by: Archit Taneja <[email protected]>
---
include/drm/drm_mipi_dsi.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index cb084af..410d8b5 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -195,6 +195,11 @@ ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params,

struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
struct mipi_dsi_device_info *info);
+static inline void mipi_dsi_device_unregister(struct mipi_dsi_device *dsi)
+{
+ device_unregister(&dsi->dev);
+}
+
/**
* enum mipi_dsi_dcs_tear_mode - Tearing Effect Output Line mode
* @MIPI_DSI_DCS_TEAR_MODE_VBLANK: the TE output line consists of V-Blanking
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-11-30 12:02:02

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v3 5/5] drm/dsi: Get DSI host by DT device node

mipi_dsi_devices are inherently aware of their host because they
share a parent-child hierarchy in the device tree.

non-dsi drivers that create dsi device don't have this data. In order to
get this information, they require to a phandle to the dsi host in the
device tree.

Maintain a list of all the hosts DSI that are currently registered.

This list will be used to find the mipi_dsi_host corresponding to the
device_node passed in of_find_mipi_dsi_host_by_node.

Reviewed-by: Andrzej Hajda <[email protected]>
Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 38 ++++++++++++++++++++++++++++++++++++++
include/drm/drm_mipi_dsi.h | 3 +++
2 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index f73e434..e9c17e25 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -205,6 +205,36 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
return mipi_dsi_device_new(host, &info);
}

+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
+ * @node: device tree node
+ *
+ * Return: A pointer to the MIPI DSI host corresponding to @np or NULL if no
+ * such device exists (or has not been registered yet).
+ */
+struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node)
+{
+ struct mipi_dsi_host *host;
+
+ mutex_lock(&host_lock);
+
+ list_for_each_entry(host, &host_list, list) {
+ if (host->dev->of_node == node) {
+ mutex_unlock(&host_lock);
+ return host;
+ }
+ }
+
+ mutex_unlock(&host_lock);
+
+ return NULL;
+}
+EXPORT_SYMBOL(of_find_mipi_dsi_host_by_node);
+
int mipi_dsi_host_register(struct mipi_dsi_host *host)
{
struct device_node *node;
@@ -216,6 +246,10 @@ int mipi_dsi_host_register(struct mipi_dsi_host *host)
of_mipi_dsi_device_add(host, node);
}

+ mutex_lock(&host_lock);
+ list_add_tail(&host->list, &host_list);
+ mutex_unlock(&host_lock);
+
return 0;
}
EXPORT_SYMBOL(mipi_dsi_host_register);
@@ -232,6 +266,10 @@ static int mipi_dsi_remove_device_fn(struct device *dev, void *priv)
void mipi_dsi_host_unregister(struct mipi_dsi_host *host)
{
device_for_each_child(host->dev, NULL, mipi_dsi_remove_device_fn);
+
+ mutex_lock(&host_lock);
+ list_del_init(&host->list);
+ mutex_unlock(&host_lock);
}
EXPORT_SYMBOL(mipi_dsi_host_unregister);

diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 410d8b5..e5c1df9 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -96,14 +96,17 @@ 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
+ * @list: list management
*/
struct mipi_dsi_host {
struct device *dev;
const struct mipi_dsi_host_ops *ops;
+ struct list_head list;
};

int mipi_dsi_host_register(struct mipi_dsi_host *host);
void mipi_dsi_host_unregister(struct mipi_dsi_host *host);
+struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node);

/* DSI mode flags */

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-11-30 12:35:51

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] drm/dsi: Add routine to unregister dsi device

On 11/30/2015 01:01 PM, Archit Taneja wrote:
> A driver calling mipi_dsi_device_new might want to unregister the device
> once it's done. It might also require it in an error handling path in
> case something didn't go right.
>
> Signed-off-by: Archit Taneja <[email protected]>
Reviewed-by: Andrzej Hajda <[email protected]>

Regards
Andrzej

2015-11-30 12:46:48

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices

Hi Archit,

[auto build test ERROR on: v4.4-rc3]
[also build test ERROR on: next-20151127]

url: https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
config: x86_64-allyesdebian (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All errors (new ones prefixed by >>):

drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
^
cc1: some warnings being treated as errors

vim +/of_modalias_node +168 drivers/gpu/drm/drm_mipi_dsi.c

162 {
163 struct device *dev = host->dev;
164 struct mipi_dsi_device_info info = { };
165 int ret;
166 u32 reg;
167
> 168 if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
169 dev_err(dev, "modalias failure on %s\n", node->full_name);
170 return ERR_PTR(-EINVAL);
171 }

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.26 kB)
.config.gz (35.22 kB)
Download all attachments

2015-12-07 05:29:37

by Archit Taneja

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices

Hi,

On 11/30/2015 06:15 PM, kbuild test robot wrote:
> Hi Archit,
>
> [auto build test ERROR on: v4.4-rc3]
> [also build test ERROR on: next-20151127]
>
> url: https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
> config: x86_64-allyesdebian (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
> if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {

Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
depend on CONFIG_OF? Or is it better to wrap these funcs around
IF_ENABLED(CONFIG_OF)?

Archit

> ^
> cc1: some warnings being treated as errors
>
> vim +/of_modalias_node +168 drivers/gpu/drm/drm_mipi_dsi.c
>
> 162 {
> 163 struct device *dev = host->dev;
> 164 struct mipi_dsi_device_info info = { };
> 165 int ret;
> 166 u32 reg;
> 167
> > 168 if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
> 169 dev_err(dev, "modalias failure on %s\n", node->full_name);
> 170 return ERR_PTR(-EINVAL);
> 171 }
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation

2015-12-07 08:46:08

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices

On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
> Hi,
>
> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>> Hi Archit,
>>
>> [auto build test ERROR on: v4.4-rc3]
>> [also build test ERROR on: next-20151127]
>>
>> url: https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>> config: x86_64-allyesdebian (attached as .config)
>> reproduce:
>> # save the attached .config to linux build tree
>> make ARCH=x86_64
>>
>> All errors (new ones prefixed by >>):
>>
>> drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>> if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>
> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
> depend on CONFIG_OF?

Please don't.

BR,
Jani.

--
Jani Nikula, Intel Open Source Technology Center

2015-12-07 09:00:04

by Archit Taneja

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices



On 12/07/2015 02:15 PM, Jani Nikula wrote:
> On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
>> Hi,
>>
>> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>>> Hi Archit,
>>>
>>> [auto build test ERROR on: v4.4-rc3]
>>> [also build test ERROR on: next-20151127]
>>>
>>> url: https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>>> config: x86_64-allyesdebian (attached as .config)
>>> reproduce:
>>> # save the attached .config to linux build tree
>>> make ARCH=x86_64
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>> drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>> if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>>
>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>> depend on CONFIG_OF?
>
> Please don't.

Just curious, how did x86 use DSI if the only way to create DSI devices
until now was via DT?

Archit

>
> BR,
> Jani.
>

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation

2015-12-07 09:10:15

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices

On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
> On 12/07/2015 02:15 PM, Jani Nikula wrote:
>> On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
>>> Hi,
>>>
>>> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>>>> Hi Archit,
>>>>
>>>> [auto build test ERROR on: v4.4-rc3]
>>>> [also build test ERROR on: next-20151127]
>>>>
>>>> url: https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>>>> config: x86_64-allyesdebian (attached as .config)
>>>> reproduce:
>>>> # save the attached .config to linux build tree
>>>> make ARCH=x86_64
>>>>
>>>> All errors (new ones prefixed by >>):
>>>>
>>>> drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>>> if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>>>
>>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>>> depend on CONFIG_OF?
>>
>> Please don't.
>
> Just curious, how did x86 use DSI if the only way to create DSI devices
> until now was via DT?

Oh, you want the gory details... we use the DSI code as a library for
abstraction and helpers, without actually creating or registering the
devices.

BR,
Jani.


>
> Archit
>
>>
>> BR,
>> Jani.
>>

--
Jani Nikula, Intel Open Source Technology Center

2015-12-07 09:18:58

by Archit Taneja

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices



On 12/07/2015 02:40 PM, Jani Nikula wrote:
> On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
>> On 12/07/2015 02:15 PM, Jani Nikula wrote:
>>> On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
>>>> Hi,
>>>>
>>>> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>>>>> Hi Archit,
>>>>>
>>>>> [auto build test ERROR on: v4.4-rc3]
>>>>> [also build test ERROR on: next-20151127]
>>>>>
>>>>> url: https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>>>>> config: x86_64-allyesdebian (attached as .config)
>>>>> reproduce:
>>>>> # save the attached .config to linux build tree
>>>>> make ARCH=x86_64
>>>>>
>>>>> All errors (new ones prefixed by >>):
>>>>>
>>>>> drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>>>> if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>>>>
>>>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>>>> depend on CONFIG_OF?
>>>
>>> Please don't.
>>
>> Just curious, how did x86 use DSI if the only way to create DSI devices
>> until now was via DT?
>
> Oh, you want the gory details... we use the DSI code as a library for
> abstraction and helpers, without actually creating or registering the
> devices.

Okay, got it. I'll go with the IS_ENABLED(CONFIG_OF) approach.

Humble request: Next time if I share something that doesn't make sense,
please reply with something more than a "Please don't". That just sounds
condescending and doesn't really help me with my cause either.

Thanks,
Archit

>
> BR,
> Jani.
>
>
>>
>> Archit
>>
>>>
>>> BR,
>>> Jani.
>>>
>

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation

2015-12-07 10:07:53

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices

On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
> On 12/07/2015 02:40 PM, Jani Nikula wrote:
>> On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
>>> On 12/07/2015 02:15 PM, Jani Nikula wrote:
>>>> On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
>>>>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>>>>> depend on CONFIG_OF?
>>>>
>>>> Please don't.
>>>
>>> Just curious, how did x86 use DSI if the only way to create DSI devices
>>> until now was via DT?
>>
>> Oh, you want the gory details... we use the DSI code as a library for
>> abstraction and helpers, without actually creating or registering the
>> devices.
>
> Okay, got it. I'll go with the IS_ENABLED(CONFIG_OF) approach.

Thanks, appreciated, so i915 doesn't need to depend on OF.

> Humble request: Next time if I share something that doesn't make sense,
> please reply with something more than a "Please don't". That just sounds
> condescending and doesn't really help me with my cause either.

That's a fair request, no need to be humble about it. Apologies.

BR,
Jani.


--
Jani Nikula, Intel Open Source Technology Center

2015-12-07 16:55:08

by Rob Clark

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] drm/dsi: Try to match non-DT dsi devices

On Mon, Dec 7, 2015 at 3:45 AM, Jani Nikula <[email protected]> wrote:
> On Mon, 07 Dec 2015, Archit Taneja <[email protected]> wrote:
>> Hi,
>>
>> On 11/30/2015 06:15 PM, kbuild test robot wrote:
>>> Hi Archit,
>>>
>>> [auto build test ERROR on: v4.4-rc3]
>>> [also build test ERROR on: next-20151127]
>>>
>>> url: https://github.com/0day-ci/linux/commits/Archit-Taneja/drm-dsi-DSI-for-devices-with-different-control-bus/20151130-200725
>>> config: x86_64-allyesdebian (attached as .config)
>>> reproduce:
>>> # save the attached .config to linux build tree
>>> make ARCH=x86_64
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>> drivers/gpu/drm/drm_mipi_dsi.c: In function 'of_mipi_dsi_device_add':
>>>>> drivers/gpu/drm/drm_mipi_dsi.c:168:6: error: implicit declaration of function 'of_modalias_node' [-Werror=implicit-function-declaration]
>>> if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
>>
>> Any suggestions on how to fix this? Is it ok to make DRM_MIPI_DSI
>> depend on CONFIG_OF?
>
> Please don't.

I assume you are not using of_mipi_dsi_device_add()? We could just
put this one fxn inside #ifdef CONFIG_OF / #endif, I think..

BR,
-R

> BR,
> Jani.
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> dri-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

2015-12-10 12:41:52

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v4 0/6] drm/dsi: DSI for devices with different control bus

We are currently restricted when it comes to supporting DSI on devices
that have a non-DSI control bus. For example, DSI encoder chips are
available in the market that are configured via i2c. Configuring their
registers via DSI bus is either optional or not available at all.

These devices still need to pass DSI parameters (data lanes, mode flags
etc) to the DSI host they are connected to. We don't have a way to do
that at the moment.

After some discussions on the previous RFC[1], we decided to support this
by providing additional API in drm_mipi_dsi which lets us create new DSI
devices without the need of them to have a DT node.

[1]: https://lkml.org/lkml/2015/6/30/42

Changes in v4:
- Added a new patch that fixes build issues when CONFIG_OF is not set.

Changes in v3:

- Incorporated misc comments by Andrzej. Changed from RFC to a PATCH set.
- Fixed htmldocs warnings.

Archit Taneja (6):
drm/dsi: check for CONFIG_OF when defining of_mipi_dsi_device_add
drm/dsi: Refactor device creation
drm/dsi: Try to match non-DT dsi devices
drm/dsi: Check for used channels
drm/dsi: Add routine to unregister dsi device
drm/dsi: Get DSI host by DT device node

drivers/gpu/drm/drm_mipi_dsi.c | 144 ++++++++++++++++++++++++++++++++---------
include/drm/drm_mipi_dsi.h | 29 +++++++++
2 files changed, 141 insertions(+), 32 deletions(-)

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-12-10 12:41:56

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v4 1/6] drm/dsi: check for CONFIG_OF when defining of_mipi_dsi_device_add

of_mipi_dsi_device_add is used only when CONFIG_OF is enabled. It
currently works if OF support is disabled, but this will change
when we add more functionality to it.

Define the original func if CONFIG_OF is enabled. Define a dummy func
otherwise.

Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 2d5ca8ee..bced235 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -129,6 +129,7 @@ static int mipi_dsi_device_add(struct mipi_dsi_device *dsi)
return device_add(&dsi->dev);
}

+#if IS_ENABLED(CONFIG_OF)
static struct mipi_dsi_device *
of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
{
@@ -170,6 +171,13 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)

return dsi;
}
+#else
+static struct mipi_dsi_device *
+of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
+{
+ return NULL;
+}
+#endif

int mipi_dsi_host_register(struct mipi_dsi_host *host)
{
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-12-10 12:42:00

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v4 2/6] drm/dsi: Refactor device creation

Simplify the mipi dsi device creation process. device_initialize and
device_add don't need to be called separately when creating
mipi_dsi_device's. Use device_register instead to simplify things.

Create a helper function mipi_dsi_device_new which takes in struct
mipi_dsi_device_info and mipi_dsi_host. It clubs the functions
mipi_dsi_device_alloc and mipi_dsi_device_add into one.

mipi_dsi_device_info acts as a template to populate the dsi device
information. This is populated by of_mipi_dsi_device_add and passed to
mipi_dsi_device_new.

Later on, we'll provide mipi_dsi_device_new as a standalone way to create
a dsi device not available via DT.

The new device creation process tries to closely follow what's been done
in i2c_new_device in i2c-core.

Reviewed-by: Andrzej Hajda <[email protected]>
Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 61 +++++++++++++++++-------------------------
include/drm/drm_mipi_dsi.h | 15 +++++++++++
2 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index bced235..9434585 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -102,9 +102,18 @@ static const struct device_type mipi_dsi_device_type = {
.release = mipi_dsi_dev_release,
};

-static struct mipi_dsi_device *mipi_dsi_device_alloc(struct mipi_dsi_host *host)
+struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
+ struct mipi_dsi_device_info *info)
{
struct mipi_dsi_device *dsi;
+ struct device *dev = host->dev;
+ int ret;
+
+ if (info->reg > 3) {
+ dev_err(dev, "device node has invalid reg property: %u\n",
+ info->reg);
+ return ERR_PTR(-EINVAL);
+ }

dsi = kzalloc(sizeof(*dsi), GFP_KERNEL);
if (!dsi)
@@ -114,27 +123,28 @@ static struct mipi_dsi_device *mipi_dsi_device_alloc(struct mipi_dsi_host *host)
dsi->dev.bus = &mipi_dsi_bus_type;
dsi->dev.parent = host->dev;
dsi->dev.type = &mipi_dsi_device_type;
+ dsi->dev.of_node = info->node;
+ dsi->channel = info->reg;

- device_initialize(&dsi->dev);
-
- return dsi;
-}
-
-static int mipi_dsi_device_add(struct mipi_dsi_device *dsi)
-{
- struct mipi_dsi_host *host = dsi->host;
+ dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg);

- dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), dsi->channel);
+ ret = device_register(&dsi->dev);
+ if (ret) {
+ dev_err(dev, "failed to register device: %d\n", ret);
+ kfree(dsi);
+ return ERR_PTR(ret);
+ }

- return device_add(&dsi->dev);
+ return dsi;
}
+EXPORT_SYMBOL(mipi_dsi_device_new);

#if IS_ENABLED(CONFIG_OF)
static struct mipi_dsi_device *
of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
{
- struct mipi_dsi_device *dsi;
struct device *dev = host->dev;
+ struct mipi_dsi_device_info info = { };
int ret;
u32 reg;

@@ -145,31 +155,10 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
return ERR_PTR(-EINVAL);
}

- if (reg > 3) {
- dev_err(dev, "device node %s has invalid reg property: %u\n",
- node->full_name, reg);
- return ERR_PTR(-EINVAL);
- }
-
- dsi = mipi_dsi_device_alloc(host);
- if (IS_ERR(dsi)) {
- dev_err(dev, "failed to allocate DSI device %s: %ld\n",
- node->full_name, PTR_ERR(dsi));
- return dsi;
- }
-
- dsi->dev.of_node = of_node_get(node);
- dsi->channel = reg;
+ info.reg = reg;
+ info.node = of_node_get(node);

- ret = mipi_dsi_device_add(dsi);
- if (ret) {
- dev_err(dev, "failed to add DSI device %s: %d\n",
- node->full_name, ret);
- kfree(dsi);
- return ERR_PTR(ret);
- }
-
- return dsi;
+ return mipi_dsi_device_new(host, &info);
}
#else
static struct mipi_dsi_device *
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index f1d8d0d..90f4f3c 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -140,6 +140,19 @@ enum mipi_dsi_pixel_format {
};

/**
+ * struct mipi_dsi_device_info - template for creating a mipi_dsi_device
+ * @reg: DSI virtual channel assigned to peripheral
+ * @node: pointer to OF device node
+ *
+ * This is populated and passed to mipi_dsi_device_new to create a new
+ * DSI device
+ */
+struct mipi_dsi_device_info {
+ u32 reg;
+ struct device_node *node;
+};
+
+/**
* struct mipi_dsi_device - DSI peripheral device
* @host: DSI host for this peripheral
* @dev: driver model device node for this peripheral
@@ -174,6 +187,8 @@ ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, const void *payload,
ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params,
size_t num_params, void *data, size_t size);

+struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
+ struct mipi_dsi_device_info *info);
/**
* enum mipi_dsi_dcs_tear_mode - Tearing Effect Output Line mode
* @MIPI_DSI_DCS_TEAR_MODE_VBLANK: the TE output line consists of V-Blanking
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-12-10 12:42:06

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v4 3/6] drm/dsi: Try to match non-DT dsi devices

Add a device name field in mipi_dsi_device. This name is different from
the actual dev name (which is of the format "hostname.reg"). When the
device is created via DT, this name is set to the modalias string.
In the non-DT case, the driver creating the DSI device provides the
name by populating a filed in mipi_dsi_device_info.

Matching for DT case would be as it was before. For the non-DT case,
we compare the device and driver names. Other buses (like i2c/spi)
perform a non-DT match by comparing the device name and entries in the
driver's id_table. Such a mechanism isn't used for the dsi bus.

Reviewed-by: Andrzej Hajda <[email protected]>
Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 25 ++++++++++++++++++++++++-
include/drm/drm_mipi_dsi.h | 6 ++++++
2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 9434585..5a46802 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -45,9 +45,26 @@
* subset of the MIPI DCS command set.
*/

+static const struct device_type mipi_dsi_device_type;
+
static int mipi_dsi_device_match(struct device *dev, struct device_driver *drv)
{
- return of_driver_match_device(dev, drv);
+ struct mipi_dsi_device *dsi;
+
+ if (dev->type == &mipi_dsi_device_type)
+ dsi = to_mipi_dsi_device(dev);
+ else
+ return 0;
+
+ /* attempt OF style match */
+ if (of_driver_match_device(dev, drv))
+ return 1;
+
+ /* compare dsi device and driver names */
+ if (!strcmp(dsi->name, drv->name))
+ return 1;
+
+ return 0;
}

static const struct dev_pm_ops mipi_dsi_device_pm_ops = {
@@ -125,6 +142,7 @@ struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
dsi->dev.type = &mipi_dsi_device_type;
dsi->dev.of_node = info->node;
dsi->channel = info->reg;
+ strlcpy(dsi->name, info->type, sizeof(dsi->name));

dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg);

@@ -148,6 +166,11 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
int ret;
u32 reg;

+ if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
+ dev_err(dev, "modalias failure on %s\n", node->full_name);
+ return ERR_PTR(-EINVAL);
+ }
+
ret = of_property_read_u32(node, "reg", &reg);
if (ret) {
dev_err(dev, "device node %s has no valid reg property: %d\n",
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 90f4f3c..cb084af 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -139,8 +139,11 @@ enum mipi_dsi_pixel_format {
MIPI_DSI_FMT_RGB565,
};

+#define DSI_DEV_NAME_SIZE 20
+
/**
* struct mipi_dsi_device_info - template for creating a mipi_dsi_device
+ * @type: dsi peripheral chip type
* @reg: DSI virtual channel assigned to peripheral
* @node: pointer to OF device node
*
@@ -148,6 +151,7 @@ enum mipi_dsi_pixel_format {
* DSI device
*/
struct mipi_dsi_device_info {
+ char type[DSI_DEV_NAME_SIZE];
u32 reg;
struct device_node *node;
};
@@ -156,6 +160,7 @@ struct mipi_dsi_device_info {
* struct mipi_dsi_device - DSI peripheral device
* @host: DSI host for this peripheral
* @dev: driver model device node for this peripheral
+ * @name: dsi peripheral chip type
* @channel: virtual channel assigned to the peripheral
* @format: pixel format for video mode
* @lanes: number of active data lanes
@@ -165,6 +170,7 @@ struct mipi_dsi_device {
struct mipi_dsi_host *host;
struct device dev;

+ char name[DSI_DEV_NAME_SIZE];
unsigned int channel;
unsigned int lanes;
enum mipi_dsi_pixel_format format;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-12-10 12:42:15

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v4 4/6] drm/dsi: Check for used channels

We don't check whether a previously registered mipi_dsi_device under the
same host shares the same virtual channel.

Before registering, check if any of the registered devices doesn't
already have the same virtual channel.

This wasn't crucial when all the devices under a host were populated via
DT. Now that we also support creating devices manually, we could end up
in a situation where a driver tries to create a device with a virtual
channel already taken by a device populated in DT.

Reviewed-by: Andrzej Hajda <[email protected]>
Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 5a46802..7a81171 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -119,6 +119,22 @@ static const struct device_type mipi_dsi_device_type = {
.release = mipi_dsi_dev_release,
};

+static int __dsi_check_chan_busy(struct device *dev, void *data)
+{
+ struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);
+ u32 *reg = data;
+
+ if (dsi && dsi->channel == *reg)
+ return -EBUSY;
+
+ return 0;
+}
+
+static int mipi_dsi_check_chan_busy(struct mipi_dsi_host *host, u32 reg)
+{
+ return device_for_each_child(host->dev, &reg, __dsi_check_chan_busy);
+}
+
struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
struct mipi_dsi_device_info *info)
{
@@ -146,14 +162,20 @@ struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,

dev_set_name(&dsi->dev, "%s.%d", dev_name(host->dev), info->reg);

+ ret = mipi_dsi_check_chan_busy(host, info->reg);
+ if (ret)
+ goto err;
+
ret = device_register(&dsi->dev);
if (ret) {
dev_err(dev, "failed to register device: %d\n", ret);
- kfree(dsi);
- return ERR_PTR(ret);
+ goto err;
}

return dsi;
+err:
+ kfree(dsi);
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL(mipi_dsi_device_new);

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-12-10 12:42:13

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v4 5/6] drm/dsi: Add routine to unregister dsi device

A driver calling mipi_dsi_device_new might want to unregister the device
once it's done. It might also require it in an error handling path in
case something didn't go right.

Reviewed-by: Andrzej Hajda <[email protected]>
Signed-off-by: Archit Taneja <[email protected]>
---
include/drm/drm_mipi_dsi.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index cb084af..410d8b5 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -195,6 +195,11 @@ ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params,

struct mipi_dsi_device *mipi_dsi_device_new(struct mipi_dsi_host *host,
struct mipi_dsi_device_info *info);
+static inline void mipi_dsi_device_unregister(struct mipi_dsi_device *dsi)
+{
+ device_unregister(&dsi->dev);
+}
+
/**
* enum mipi_dsi_dcs_tear_mode - Tearing Effect Output Line mode
* @MIPI_DSI_DCS_TEAR_MODE_VBLANK: the TE output line consists of V-Blanking
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2015-12-10 12:42:26

by Archit Taneja

[permalink] [raw]
Subject: [PATCH v4 6/6] drm/dsi: Get DSI host by DT device node

mipi_dsi_devices are inherently aware of their host because they
share a parent-child hierarchy in the device tree.

non-dsi drivers that create dsi device don't have this data. In order to
get this information, they require to a phandle to the dsi host in the
device tree.

Maintain a list of all the hosts DSI that are currently registered.

This list will be used to find the mipi_dsi_host corresponding to the
device_node passed in of_find_mipi_dsi_host_by_node.

Reviewed-by: Andrzej Hajda <[email protected]>
Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/drm_mipi_dsi.c | 38 ++++++++++++++++++++++++++++++++++++++
include/drm/drm_mipi_dsi.h | 3 +++
2 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 7a81171..e40a665 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -213,6 +213,36 @@ of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
}
#endif

+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
+ * @node: device tree node
+ *
+ * Return: A pointer to the MIPI DSI host corresponding to @np or NULL if no
+ * such device exists (or has not been registered yet).
+ */
+struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node)
+{
+ struct mipi_dsi_host *host;
+
+ mutex_lock(&host_lock);
+
+ list_for_each_entry(host, &host_list, list) {
+ if (host->dev->of_node == node) {
+ mutex_unlock(&host_lock);
+ return host;
+ }
+ }
+
+ mutex_unlock(&host_lock);
+
+ return NULL;
+}
+EXPORT_SYMBOL(of_find_mipi_dsi_host_by_node);
+
int mipi_dsi_host_register(struct mipi_dsi_host *host)
{
struct device_node *node;
@@ -224,6 +254,10 @@ int mipi_dsi_host_register(struct mipi_dsi_host *host)
of_mipi_dsi_device_add(host, node);
}

+ mutex_lock(&host_lock);
+ list_add_tail(&host->list, &host_list);
+ mutex_unlock(&host_lock);
+
return 0;
}
EXPORT_SYMBOL(mipi_dsi_host_register);
@@ -240,6 +274,10 @@ static int mipi_dsi_remove_device_fn(struct device *dev, void *priv)
void mipi_dsi_host_unregister(struct mipi_dsi_host *host)
{
device_for_each_child(host->dev, NULL, mipi_dsi_remove_device_fn);
+
+ mutex_lock(&host_lock);
+ list_del_init(&host->list);
+ mutex_unlock(&host_lock);
}
EXPORT_SYMBOL(mipi_dsi_host_unregister);

diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 410d8b5..e5c1df9 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -96,14 +96,17 @@ 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
+ * @list: list management
*/
struct mipi_dsi_host {
struct device *dev;
const struct mipi_dsi_host_ops *ops;
+ struct list_head list;
};

int mipi_dsi_host_register(struct mipi_dsi_host *host);
void mipi_dsi_host_unregister(struct mipi_dsi_host *host);
+struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node);

/* DSI mode flags */

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation