* [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix
@ 2019-07-02 15:44 ` Rob Clark
2019-07-02 15:44 ` [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet Rob Clark
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Rob Clark @ 2019-07-02 15:44 UTC (permalink / raw)
To: dri-devel; +Cc: linux-arm-msm, Sean Paul, Rob Clark
From: Rob Clark <robdclark@chromium.org>
In process of debugging panel on my lenovo yoga c630, I noticed some
errors in the dsi->mode_flags, which I guess were just cargo-cult'd?
Since dumping the status regs was useful to notice this problem, I
cleaned it up and turned it into debugfs. The last patch corrects the
mode_flags.
Tested on cheza and c630.
Rob Clark (3):
drm/bridge: ti-sn65dsi86: add link to datasheet
drm/bridge: ti-sn65dsi86: add debugfs
drm/bridge: ti-sn65dsi86: correct dsi mode_flags
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 46 +++++++++++++++++++++++++--
1 file changed, 44 insertions(+), 2 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet
2019-07-02 15:44 ` [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix Rob Clark
@ 2019-07-02 15:44 ` Rob Clark
2019-07-02 17:03 ` Jeffrey Hugo
` (2 more replies)
2019-07-02 15:44 ` [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs Rob Clark
` (2 subsequent siblings)
3 siblings, 3 replies; 17+ messages in thread
From: Rob Clark @ 2019-07-02 15:44 UTC (permalink / raw)
To: dri-devel
Cc: linux-arm-msm, Sean Paul, Rob Clark, Andrzej Hajda,
Laurent Pinchart, David Airlie, Daniel Vetter, linux-kernel
From: Rob Clark <robdclark@chromium.org>
The bridge has pretty good docs, lets add a link to make them easier to
find.
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index bcca9173c72a..f1a2493b86d9 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * datasheet: http://www.ti.com/lit/ds/symlink/sn65dsi86.pdf
*/
#include <linux/clk.h>
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs
2019-07-02 15:44 ` [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix Rob Clark
2019-07-02 15:44 ` [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet Rob Clark
@ 2019-07-02 15:44 ` Rob Clark
2019-07-02 17:05 ` Jeffrey Hugo
2019-07-04 12:31 ` Andrzej Hajda
2019-07-02 15:44 ` [PATCH 3/3] drm/bridge: ti-sn65dsi86: correct dsi mode_flags Rob Clark
2019-07-04 13:18 ` [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix Andrzej Hajda
3 siblings, 2 replies; 17+ messages in thread
From: Rob Clark @ 2019-07-02 15:44 UTC (permalink / raw)
To: dri-devel
Cc: linux-arm-msm, Sean Paul, Rob Clark, Andrzej Hajda,
Laurent Pinchart, David Airlie, Daniel Vetter, linux-kernel
From: Rob Clark <robdclark@chromium.org>
Add a debugfs file to show status registers.
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index f1a2493b86d9..a6f27648c015 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -5,6 +5,7 @@
*/
#include <linux/clk.h>
+#include <linux/debugfs.h>
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/iopoll.h>
@@ -109,6 +110,7 @@ struct ti_sn_bridge {
struct drm_dp_aux aux;
struct drm_bridge bridge;
struct drm_connector connector;
+ struct dentry *debugfs;
struct device_node *host_node;
struct mipi_dsi_device *dsi;
struct clk *refclk;
@@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
};
+static int status_show(struct seq_file *s, void *data)
+{
+ struct ti_sn_bridge *pdata = s->private;
+ unsigned int reg, val;
+
+ seq_puts(s, "STATUS REGISTERS:\n");
+
+ pm_runtime_get_sync(pdata->dev);
+
+ /* IRQ Status Registers, see Table 31 in datasheet */
+ for (reg = 0xf0; reg <= 0xf8; reg++) {
+ regmap_read(pdata->regmap, reg, &val);
+ seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
+ }
+
+ pm_runtime_put(pdata->dev);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(status);
+
+static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
+{
+ pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
+
+ debugfs_create_file("status", 0600, pdata->debugfs, pdata,
+ &status_fops);
+}
+
+static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
+{
+ debugfs_remove_recursive(pdata->debugfs);
+ pdata->debugfs = NULL;
+}
+
/* Connector funcs */
static struct ti_sn_bridge *
connector_to_ti_sn_bridge(struct drm_connector *connector)
@@ -869,6 +907,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
drm_bridge_add(&pdata->bridge);
+ ti_sn_debugfs_init(pdata);
+
return 0;
}
@@ -879,6 +919,8 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
if (!pdata)
return -EINVAL;
+ ti_sn_debugfs_remove(pdata);
+
of_node_put(pdata->host_node);
pm_runtime_disable(pdata->dev);
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] drm/bridge: ti-sn65dsi86: correct dsi mode_flags
2019-07-02 15:44 ` [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix Rob Clark
2019-07-02 15:44 ` [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet Rob Clark
2019-07-02 15:44 ` [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs Rob Clark
@ 2019-07-02 15:44 ` Rob Clark
2019-07-02 17:09 ` Jeffrey Hugo
2019-07-04 13:18 ` [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix Andrzej Hajda
3 siblings, 1 reply; 17+ messages in thread
From: Rob Clark @ 2019-07-02 15:44 UTC (permalink / raw)
To: dri-devel
Cc: linux-arm-msm, Sean Paul, Rob Clark, Andrzej Hajda,
Laurent Pinchart, David Airlie, Daniel Vetter, linux-kernel
From: Rob Clark <robdclark@chromium.org>
Noticed while comparing register dump of how bootloader configures DSI
vs how kernel configures. It seems the bridge still works either way,
but fixing this clears the 'CHA_DATATYPE_ERR' error status bit.
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index a6f27648c015..c8fb45e7b06d 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -342,8 +342,7 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge)
/* TODO: setting to 4 lanes always for now */
dsi->lanes = 4;
dsi->format = MIPI_DSI_FMT_RGB888;
- dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
- MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
+ dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
/* check if continuous dsi clock is required or not */
pm_runtime_get_sync(pdata->dev);
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet
2019-07-02 15:44 ` [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet Rob Clark
@ 2019-07-02 17:03 ` Jeffrey Hugo
2019-07-04 12:26 ` Andrzej Hajda
2019-07-04 12:32 ` Laurent Pinchart
2 siblings, 0 replies; 17+ messages in thread
From: Jeffrey Hugo @ 2019-07-02 17:03 UTC (permalink / raw)
To: Rob Clark
Cc: open list:DRM PANEL DRIVERS, MSM, Sean Paul, Rob Clark,
Andrzej Hajda, Laurent Pinchart, David Airlie, Daniel Vetter,
lkml
On Tue, Jul 2, 2019 at 9:45 AM Rob Clark <robdclark@gmail.com> wrote:
>
> From: Rob Clark <robdclark@chromium.org>
>
> The bridge has pretty good docs, lets add a link to make them easier to
> find.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
This is in the DT binding, but having it in the driver as well is a nice touch.
Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs
2019-07-02 15:44 ` [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs Rob Clark
@ 2019-07-02 17:05 ` Jeffrey Hugo
2019-07-04 12:31 ` Andrzej Hajda
1 sibling, 0 replies; 17+ messages in thread
From: Jeffrey Hugo @ 2019-07-02 17:05 UTC (permalink / raw)
To: Rob Clark
Cc: open list:DRM PANEL DRIVERS, MSM, Sean Paul, Rob Clark,
Andrzej Hajda, Laurent Pinchart, David Airlie, Daniel Vetter,
lkml
On Tue, Jul 2, 2019 at 9:46 AM Rob Clark <robdclark@gmail.com> wrote:
>
> From: Rob Clark <robdclark@chromium.org>
>
> Add a debugfs file to show status registers.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] drm/bridge: ti-sn65dsi86: correct dsi mode_flags
2019-07-02 15:44 ` [PATCH 3/3] drm/bridge: ti-sn65dsi86: correct dsi mode_flags Rob Clark
@ 2019-07-02 17:09 ` Jeffrey Hugo
2019-07-02 17:12 ` Rob Clark
0 siblings, 1 reply; 17+ messages in thread
From: Jeffrey Hugo @ 2019-07-02 17:09 UTC (permalink / raw)
To: Rob Clark
Cc: open list:DRM PANEL DRIVERS, MSM, Sean Paul, Rob Clark,
Andrzej Hajda, Laurent Pinchart, David Airlie, Daniel Vetter,
lkml
On Tue, Jul 2, 2019 at 9:46 AM Rob Clark <robdclark@gmail.com> wrote:
>
> From: Rob Clark <robdclark@chromium.org>
>
> Noticed while comparing register dump of how bootloader configures DSI
> vs how kernel configures. It seems the bridge still works either way,
> but fixing this clears the 'CHA_DATATYPE_ERR' error status bit.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index a6f27648c015..c8fb45e7b06d 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -342,8 +342,7 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge)
> /* TODO: setting to 4 lanes always for now */
> dsi->lanes = 4;
> dsi->format = MIPI_DSI_FMT_RGB888;
> - dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> - MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
Did you check this against the datasheet? Per my reading, EOT_PACKET
and VIDEO_HSE appear valid. I don't know about VIDEO_SYNC_PULSE.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] drm/bridge: ti-sn65dsi86: correct dsi mode_flags
2019-07-02 17:09 ` Jeffrey Hugo
@ 2019-07-02 17:12 ` Rob Clark
2019-07-02 17:23 ` Jeffrey Hugo
0 siblings, 1 reply; 17+ messages in thread
From: Rob Clark @ 2019-07-02 17:12 UTC (permalink / raw)
To: Jeffrey Hugo
Cc: open list:DRM PANEL DRIVERS, MSM, Sean Paul, Rob Clark,
Andrzej Hajda, Laurent Pinchart, David Airlie, Daniel Vetter,
lkml
On Tue, Jul 2, 2019 at 10:09 AM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
>
> On Tue, Jul 2, 2019 at 9:46 AM Rob Clark <robdclark@gmail.com> wrote:
> >
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Noticed while comparing register dump of how bootloader configures DSI
> > vs how kernel configures. It seems the bridge still works either way,
> > but fixing this clears the 'CHA_DATATYPE_ERR' error status bit.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index a6f27648c015..c8fb45e7b06d 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -342,8 +342,7 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge)
> > /* TODO: setting to 4 lanes always for now */
> > dsi->lanes = 4;
> > dsi->format = MIPI_DSI_FMT_RGB888;
> > - dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> > - MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
> > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
>
> Did you check this against the datasheet? Per my reading, EOT_PACKET
> and VIDEO_HSE appear valid. I don't know about VIDEO_SYNC_PULSE.
The EOT flat is badly named:
/* disable EoT packets in HS mode */
#define MIPI_DSI_MODE_EOT_PACKET BIT(9)
I can double check out HSE, but this was one of the setting
differences between bootloader and kernel
BR,
-R
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] drm/bridge: ti-sn65dsi86: correct dsi mode_flags
2019-07-02 17:12 ` Rob Clark
@ 2019-07-02 17:23 ` Jeffrey Hugo
2019-07-04 12:33 ` Andrzej Hajda
0 siblings, 1 reply; 17+ messages in thread
From: Jeffrey Hugo @ 2019-07-02 17:23 UTC (permalink / raw)
To: Rob Clark
Cc: open list:DRM PANEL DRIVERS, MSM, Sean Paul, Rob Clark,
Andrzej Hajda, Laurent Pinchart, David Airlie, Daniel Vetter,
lkml
On Tue, Jul 2, 2019 at 11:12 AM Rob Clark <robdclark@gmail.com> wrote:
>
> On Tue, Jul 2, 2019 at 10:09 AM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
> >
> > On Tue, Jul 2, 2019 at 9:46 AM Rob Clark <robdclark@gmail.com> wrote:
> > >
> > > - dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> > > - MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
> > > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
> >
> > Did you check this against the datasheet? Per my reading, EOT_PACKET
> > and VIDEO_HSE appear valid. I don't know about VIDEO_SYNC_PULSE.
>
> The EOT flat is badly named:
>
> /* disable EoT packets in HS mode */
> #define MIPI_DSI_MODE_EOT_PACKET BIT(9)
>
> I can double check out HSE, but this was one of the setting
> differences between bootloader and kernel
Ah yeah, you are right. My eyes apparently skipped over the "disable".
If the bootloader is not setting the HSE, then I can't think of a
reason why we would be having an issue also not setting it.
Seems good to me
Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet
2019-07-02 15:44 ` [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet Rob Clark
2019-07-02 17:03 ` Jeffrey Hugo
@ 2019-07-04 12:26 ` Andrzej Hajda
2019-07-04 12:32 ` Laurent Pinchart
2 siblings, 0 replies; 17+ messages in thread
From: Andrzej Hajda @ 2019-07-04 12:26 UTC (permalink / raw)
To: Rob Clark, dri-devel
Cc: linux-arm-msm, Sean Paul, Rob Clark, Laurent Pinchart,
David Airlie, Daniel Vetter, linux-kernel
On 02.07.2019 17:44, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> The bridge has pretty good docs, lets add a link to make them easier to
> find.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index bcca9173c72a..f1a2493b86d9 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + * datasheet: http://www.ti.com/lit/ds/symlink/sn65dsi86.pdf
> */
>
> #include <linux/clk.h>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs
2019-07-02 15:44 ` [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs Rob Clark
2019-07-02 17:05 ` Jeffrey Hugo
@ 2019-07-04 12:31 ` Andrzej Hajda
2019-07-04 12:35 ` Laurent Pinchart
1 sibling, 1 reply; 17+ messages in thread
From: Andrzej Hajda @ 2019-07-04 12:31 UTC (permalink / raw)
To: Rob Clark, dri-devel
Cc: linux-arm-msm, Sean Paul, Rob Clark, Laurent Pinchart,
David Airlie, Daniel Vetter, linux-kernel
On 02.07.2019 17:44, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> Add a debugfs file to show status registers.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index f1a2493b86d9..a6f27648c015 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -5,6 +5,7 @@
> */
>
> #include <linux/clk.h>
> +#include <linux/debugfs.h>
> #include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/iopoll.h>
> @@ -109,6 +110,7 @@ struct ti_sn_bridge {
> struct drm_dp_aux aux;
> struct drm_bridge bridge;
> struct drm_connector connector;
> + struct dentry *debugfs;
> struct device_node *host_node;
> struct mipi_dsi_device *dsi;
> struct clk *refclk;
> @@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> };
>
> +static int status_show(struct seq_file *s, void *data)
> +{
> + struct ti_sn_bridge *pdata = s->private;
> + unsigned int reg, val;
> +
> + seq_puts(s, "STATUS REGISTERS:\n");
> +
> + pm_runtime_get_sync(pdata->dev);
> +
> + /* IRQ Status Registers, see Table 31 in datasheet */
> + for (reg = 0xf0; reg <= 0xf8; reg++) {
> + regmap_read(pdata->regmap, reg, &val);
> + seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
> + }
> +
> + pm_runtime_put(pdata->dev);
> +
> + return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(status);
> +
> +static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
> +{
> + pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
If some day we will have board with two such bridges there will be a
problem.
Anyway:
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej
> +
> + debugfs_create_file("status", 0600, pdata->debugfs, pdata,
> + &status_fops);
> +}
> +
> +static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
> +{
> + debugfs_remove_recursive(pdata->debugfs);
> + pdata->debugfs = NULL;
> +}
> +
> /* Connector funcs */
> static struct ti_sn_bridge *
> connector_to_ti_sn_bridge(struct drm_connector *connector)
> @@ -869,6 +907,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
>
> drm_bridge_add(&pdata->bridge);
>
> + ti_sn_debugfs_init(pdata);
> +
> return 0;
> }
>
> @@ -879,6 +919,8 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
> if (!pdata)
> return -EINVAL;
>
> + ti_sn_debugfs_remove(pdata);
> +
> of_node_put(pdata->host_node);
>
> pm_runtime_disable(pdata->dev);
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet
2019-07-02 15:44 ` [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet Rob Clark
2019-07-02 17:03 ` Jeffrey Hugo
2019-07-04 12:26 ` Andrzej Hajda
@ 2019-07-04 12:32 ` Laurent Pinchart
2 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2019-07-04 12:32 UTC (permalink / raw)
To: Rob Clark
Cc: dri-devel, linux-arm-msm, Sean Paul, Rob Clark, Andrzej Hajda,
David Airlie, Daniel Vetter, linux-kernel
Hi Rob,
Thank you for the patch.
On Tue, Jul 02, 2019 at 08:44:16AM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> The bridge has pretty good docs, lets add a link to make them easier to
> find.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index bcca9173c72a..f1a2493b86d9 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + * datasheet: http://www.ti.com/lit/ds/symlink/sn65dsi86.pdf
It's in the DT bindings and it's easy to find so I'm not sure it adds
lots of value to mention it here, but I don't feel very strongly about
it, so if you think it can be useful I don't mind. I would however put a
blank line after the copyright line in that case, and
s/datasheet/Datasheet/
> */
>
> #include <linux/clk.h>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] drm/bridge: ti-sn65dsi86: correct dsi mode_flags
2019-07-02 17:23 ` Jeffrey Hugo
@ 2019-07-04 12:33 ` Andrzej Hajda
0 siblings, 0 replies; 17+ messages in thread
From: Andrzej Hajda @ 2019-07-04 12:33 UTC (permalink / raw)
To: Jeffrey Hugo, Rob Clark
Cc: open list:DRM PANEL DRIVERS, MSM, Sean Paul, Rob Clark,
Laurent Pinchart, David Airlie, Daniel Vetter, lkml
On 02.07.2019 19:23, Jeffrey Hugo wrote:
> On Tue, Jul 2, 2019 at 11:12 AM Rob Clark <robdclark@gmail.com> wrote:
>> On Tue, Jul 2, 2019 at 10:09 AM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote:
>>> On Tue, Jul 2, 2019 at 9:46 AM Rob Clark <robdclark@gmail.com> wrote:
>>>> - dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
>>>> - MIPI_DSI_MODE_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;
>>>> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
>>> Did you check this against the datasheet? Per my reading, EOT_PACKET
>>> and VIDEO_HSE appear valid. I don't know about VIDEO_SYNC_PULSE.
>> The EOT flat is badly named:
>>
>> /* disable EoT packets in HS mode */
>> #define MIPI_DSI_MODE_EOT_PACKET BIT(9)
>>
>> I can double check out HSE, but this was one of the setting
>> differences between bootloader and kernel
> Ah yeah, you are right. My eyes apparently skipped over the "disable".
>
> If the bootloader is not setting the HSE, then I can't think of a
> reason why we would be having an issue also not setting it.
>
> Seems good to me
>
> Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
>
>
Yes, the flags require cleanup.
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs
2019-07-04 12:31 ` Andrzej Hajda
@ 2019-07-04 12:35 ` Laurent Pinchart
2019-07-04 13:56 ` Rob Clark
0 siblings, 1 reply; 17+ messages in thread
From: Laurent Pinchart @ 2019-07-04 12:35 UTC (permalink / raw)
To: Andrzej Hajda
Cc: Rob Clark, dri-devel, linux-arm-msm, Sean Paul, Rob Clark,
David Airlie, Daniel Vetter, linux-kernel
Hello,
On Thu, Jul 04, 2019 at 02:31:20PM +0200, Andrzej Hajda wrote:
> On 02.07.2019 17:44, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Add a debugfs file to show status registers.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index f1a2493b86d9..a6f27648c015 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -5,6 +5,7 @@
> > */
> >
> > #include <linux/clk.h>
> > +#include <linux/debugfs.h>
> > #include <linux/gpio/consumer.h>
> > #include <linux/i2c.h>
> > #include <linux/iopoll.h>
> > @@ -109,6 +110,7 @@ struct ti_sn_bridge {
> > struct drm_dp_aux aux;
> > struct drm_bridge bridge;
> > struct drm_connector connector;
> > + struct dentry *debugfs;
> > struct device_node *host_node;
> > struct mipi_dsi_device *dsi;
> > struct clk *refclk;
> > @@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> > SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> > };
> >
> > +static int status_show(struct seq_file *s, void *data)
> > +{
> > + struct ti_sn_bridge *pdata = s->private;
> > + unsigned int reg, val;
> > +
> > + seq_puts(s, "STATUS REGISTERS:\n");
NO NEED TO SHOUT :-)
> > +
> > + pm_runtime_get_sync(pdata->dev);
> > +
> > + /* IRQ Status Registers, see Table 31 in datasheet */
> > + for (reg = 0xf0; reg <= 0xf8; reg++) {
> > + regmap_read(pdata->regmap, reg, &val);
> > + seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
> > + }
> > +
> > + pm_runtime_put(pdata->dev);
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_SHOW_ATTRIBUTE(status);
> > +
> > +static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
> > +{
> > + pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
>
> If some day we will have board with two such bridges there will be a
> problem.
Could we use the platform device name for this ?
> Anyway:
>
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
>
> > +
> > + debugfs_create_file("status", 0600, pdata->debugfs, pdata,
> > + &status_fops);
> > +}
> > +
> > +static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
> > +{
> > + debugfs_remove_recursive(pdata->debugfs);
> > + pdata->debugfs = NULL;
> > +}
> > +
You need to conditionally-compile this based on CONFIG_DEBUG_FS.
> > /* Connector funcs */
> > static struct ti_sn_bridge *
> > connector_to_ti_sn_bridge(struct drm_connector *connector)
> > @@ -869,6 +907,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
> >
> > drm_bridge_add(&pdata->bridge);
> >
> > + ti_sn_debugfs_init(pdata);
> > +
> > return 0;
> > }
> >
> > @@ -879,6 +919,8 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
> > if (!pdata)
> > return -EINVAL;
> >
> > + ti_sn_debugfs_remove(pdata);
> > +
> > of_node_put(pdata->host_node);
> >
> > pm_runtime_disable(pdata->dev);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix
2019-07-02 15:44 ` [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix Rob Clark
` (2 preceding siblings ...)
2019-07-02 15:44 ` [PATCH 3/3] drm/bridge: ti-sn65dsi86: correct dsi mode_flags Rob Clark
@ 2019-07-04 13:18 ` Andrzej Hajda
3 siblings, 0 replies; 17+ messages in thread
From: Andrzej Hajda @ 2019-07-04 13:18 UTC (permalink / raw)
To: Rob Clark, dri-devel; +Cc: Rob Clark, linux-arm-msm, Sean Paul
On 02.07.2019 17:44, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> In process of debugging panel on my lenovo yoga c630, I noticed some
> errors in the dsi->mode_flags, which I guess were just cargo-cult'd?
>
> Since dumping the status regs was useful to notice this problem, I
> cleaned it up and turned it into debugfs. The last patch corrects the
> mode_flags.
>
> Tested on cheza and c630.
>
> Rob Clark (3):
> drm/bridge: ti-sn65dsi86: add link to datasheet
> drm/bridge: ti-sn65dsi86: add debugfs
> drm/bridge: ti-sn65dsi86: correct dsi mode_flags
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 46 +++++++++++++++++++++++++--
> 1 file changed, 44 insertions(+), 2 deletions(-)
>
Queued to drm-misc-next.
--
Regards
Andrzej
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs
2019-07-04 12:35 ` Laurent Pinchart
@ 2019-07-04 13:56 ` Rob Clark
2019-07-04 21:10 ` Laurent Pinchart
0 siblings, 1 reply; 17+ messages in thread
From: Rob Clark @ 2019-07-04 13:56 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Andrzej Hajda, dri-devel, linux-arm-msm, Sean Paul, Rob Clark,
David Airlie, Daniel Vetter, Linux Kernel Mailing List
On Thu, Jul 4, 2019 at 5:35 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hello,
>
> On Thu, Jul 04, 2019 at 02:31:20PM +0200, Andrzej Hajda wrote:
> > On 02.07.2019 17:44, Rob Clark wrote:
> > > From: Rob Clark <robdclark@chromium.org>
> > >
> > > Add a debugfs file to show status registers.
> > >
> > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > ---
> > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
> > > 1 file changed, 42 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > index f1a2493b86d9..a6f27648c015 100644
> > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > @@ -5,6 +5,7 @@
> > > */
> > >
> > > #include <linux/clk.h>
> > > +#include <linux/debugfs.h>
> > > #include <linux/gpio/consumer.h>
> > > #include <linux/i2c.h>
> > > #include <linux/iopoll.h>
> > > @@ -109,6 +110,7 @@ struct ti_sn_bridge {
> > > struct drm_dp_aux aux;
> > > struct drm_bridge bridge;
> > > struct drm_connector connector;
> > > + struct dentry *debugfs;
> > > struct device_node *host_node;
> > > struct mipi_dsi_device *dsi;
> > > struct clk *refclk;
> > > @@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> > > SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> > > };
> > >
> > > +static int status_show(struct seq_file *s, void *data)
> > > +{
> > > + struct ti_sn_bridge *pdata = s->private;
> > > + unsigned int reg, val;
> > > +
> > > + seq_puts(s, "STATUS REGISTERS:\n");
>
> NO NEED TO SHOUT :-)
>
> > > +
> > > + pm_runtime_get_sync(pdata->dev);
> > > +
> > > + /* IRQ Status Registers, see Table 31 in datasheet */
> > > + for (reg = 0xf0; reg <= 0xf8; reg++) {
> > > + regmap_read(pdata->regmap, reg, &val);
> > > + seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
> > > + }
> > > +
> > > + pm_runtime_put(pdata->dev);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +DEFINE_SHOW_ATTRIBUTE(status);
> > > +
> > > +static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
> > > +{
> > > + pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
> >
> > If some day we will have board with two such bridges there will be a
> > problem.
>
> Could we use the platform device name for this ?
hmm, yeah, that would solve the 2x bridges issue
> > Anyway:
> >
> > Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
> >
> > > +
> > > + debugfs_create_file("status", 0600, pdata->debugfs, pdata,
> > > + &status_fops);
> > > +}
> > > +
> > > +static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
> > > +{
> > > + debugfs_remove_recursive(pdata->debugfs);
> > > + pdata->debugfs = NULL;
> > > +}
> > > +
>
> You need to conditionally-compile this based on CONFIG_DEBUG_FS.
Hmm, is that really true? Debugfs appears to be sufficently stub'd w/
inline no-ops in the !CONFIG_DEBUG_FS case
BR,
-R
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs
2019-07-04 13:56 ` Rob Clark
@ 2019-07-04 21:10 ` Laurent Pinchart
0 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2019-07-04 21:10 UTC (permalink / raw)
To: Rob Clark
Cc: Andrzej Hajda, dri-devel, linux-arm-msm, Sean Paul, Rob Clark,
David Airlie, Daniel Vetter, Linux Kernel Mailing List
Hi Rob,
On Thu, Jul 04, 2019 at 06:56:56AM -0700, Rob Clark wrote:
> On Thu, Jul 4, 2019 at 5:35 AM Laurent Pinchartwrote:
> > On Thu, Jul 04, 2019 at 02:31:20PM +0200, Andrzej Hajda wrote:
> > > On 02.07.2019 17:44, Rob Clark wrote:
> > > > From: Rob Clark <robdclark@chromium.org>
> > > >
> > > > Add a debugfs file to show status registers.
> > > >
> > > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > > ---
> > > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 42 +++++++++++++++++++++++++++
> > > > 1 file changed, 42 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > index f1a2493b86d9..a6f27648c015 100644
> > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > @@ -5,6 +5,7 @@
> > > > */
> > > >
> > > > #include <linux/clk.h>
> > > > +#include <linux/debugfs.h>
> > > > #include <linux/gpio/consumer.h>
> > > > #include <linux/i2c.h>
> > > > #include <linux/iopoll.h>
> > > > @@ -109,6 +110,7 @@ struct ti_sn_bridge {
> > > > struct drm_dp_aux aux;
> > > > struct drm_bridge bridge;
> > > > struct drm_connector connector;
> > > > + struct dentry *debugfs;
> > > > struct device_node *host_node;
> > > > struct mipi_dsi_device *dsi;
> > > > struct clk *refclk;
> > > > @@ -178,6 +180,42 @@ static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
> > > > SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> > > > };
> > > >
> > > > +static int status_show(struct seq_file *s, void *data)
> > > > +{
> > > > + struct ti_sn_bridge *pdata = s->private;
> > > > + unsigned int reg, val;
> > > > +
> > > > + seq_puts(s, "STATUS REGISTERS:\n");
> >
> > NO NEED TO SHOUT :-)
> >
> > > > +
> > > > + pm_runtime_get_sync(pdata->dev);
> > > > +
> > > > + /* IRQ Status Registers, see Table 31 in datasheet */
> > > > + for (reg = 0xf0; reg <= 0xf8; reg++) {
> > > > + regmap_read(pdata->regmap, reg, &val);
> > > > + seq_printf(s, "[0x%02x] = 0x%08x\n", reg, val);
> > > > + }
> > > > +
> > > > + pm_runtime_put(pdata->dev);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +DEFINE_SHOW_ATTRIBUTE(status);
> > > > +
> > > > +static void ti_sn_debugfs_init(struct ti_sn_bridge *pdata)
> > > > +{
> > > > + pdata->debugfs = debugfs_create_dir("ti_sn65dsi86", NULL);
> > >
> > > If some day we will have board with two such bridges there will be a
> > > problem.
> >
> > Could we use the platform device name for this ?
>
> hmm, yeah, that would solve the 2x bridges issue
>
> > > Anyway:
> > >
> > > Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
> > >
> > > > +
> > > > + debugfs_create_file("status", 0600, pdata->debugfs, pdata,
> > > > + &status_fops);
> > > > +}
> > > > +
> > > > +static void ti_sn_debugfs_remove(struct ti_sn_bridge *pdata)
> > > > +{
> > > > + debugfs_remove_recursive(pdata->debugfs);
> > > > + pdata->debugfs = NULL;
> > > > +}
> > > > +
> >
> > You need to conditionally-compile this based on CONFIG_DEBUG_FS.
>
> Hmm, is that really true? Debugfs appears to be sufficently stub'd w/
> inline no-ops in the !CONFIG_DEBUG_FS case
You're right, my bad. I wonder if the compiler will optimise the above
two functions out. It might warrant a CONFIG_DEBUG_FS check here for
that reason, but that's really bikeshedding. So with the 2x bridges
issue addressed, I think the patch will be good.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-07-04 21:10 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20190702154436epcas2p3fdaaa095c15a116a0be7e5313469151f@epcas2p3.samsung.com>
2019-07-02 15:44 ` [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix Rob Clark
2019-07-02 15:44 ` [PATCH 1/3] drm/bridge: ti-sn65dsi86: add link to datasheet Rob Clark
2019-07-02 17:03 ` Jeffrey Hugo
2019-07-04 12:26 ` Andrzej Hajda
2019-07-04 12:32 ` Laurent Pinchart
2019-07-02 15:44 ` [PATCH 2/3] drm/bridge: ti-sn65dsi86: add debugfs Rob Clark
2019-07-02 17:05 ` Jeffrey Hugo
2019-07-04 12:31 ` Andrzej Hajda
2019-07-04 12:35 ` Laurent Pinchart
2019-07-04 13:56 ` Rob Clark
2019-07-04 21:10 ` Laurent Pinchart
2019-07-02 15:44 ` [PATCH 3/3] drm/bridge: ti-sn65dsi86: correct dsi mode_flags Rob Clark
2019-07-02 17:09 ` Jeffrey Hugo
2019-07-02 17:12 ` Rob Clark
2019-07-02 17:23 ` Jeffrey Hugo
2019-07-04 12:33 ` Andrzej Hajda
2019-07-04 13:18 ` [PATCH 0/3] drm/bridge: ti-sn65dsi86: debugfs and mode_flags fix Andrzej Hajda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).