diff mbox series

[v4,55/80] drm/panel: panel-dsi-cm: use MIPI_DCS_GET_ERROR_COUNT_ON_DSI

Message ID 20201124124538.660710-56-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show
Series Convert DSI code to use drm_mipi_dsi and drm_panel | expand

Commit Message

Tomi Valkeinen Nov. 24, 2020, 12:45 p.m. UTC
Use the common MIPI_DCS_GET_ERROR_COUNT_ON_DSI define instead of
driver's own.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/panel/panel-dsi-cm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Sam Ravnborg Nov. 24, 2020, 4:18 p.m. UTC | #1
Hi Tomi,

On Tue, Nov 24, 2020 at 02:45:13PM +0200, Tomi Valkeinen wrote:
> Use the common MIPI_DCS_GET_ERROR_COUNT_ON_DSI define instead of
> driver's own.
> 
They are both 5 - OK

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

IMO you should get all the patches at least up including this patch applied.
They are all reviewed/acked. And then you have a much smaller stack of
patches to spam us with.

	Sam
Tomi Valkeinen Nov. 24, 2020, 4:26 p.m. UTC | #2
Hi Sam,

On 24/11/2020 18:18, Sam Ravnborg wrote:
> Hi Tomi,
> 
> On Tue, Nov 24, 2020 at 02:45:13PM +0200, Tomi Valkeinen wrote:
>> Use the common MIPI_DCS_GET_ERROR_COUNT_ON_DSI define instead of
>> driver's own.
>>
> They are both 5 - OK
> 
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> IMO you should get all the patches at least up including this patch applied.
> They are all reviewed/acked. And then you have a much smaller stack of
> patches to spam us with.

Yes, I think that makes sense. I did not want to merge them earlier, as with the v3, I could not get
videomode panels work at all (while cmd mode panels did work). So I was not sure if something is
totally silly and broken in the series.

Now that I can get video mode panels work with some hacks on top, I'm fine with merging these.

But it's too late for 5.11, as we need testing and work on the video mode panels. So targeting 5.12.

 Tomi
Sam Ravnborg Nov. 24, 2020, 4:38 p.m. UTC | #3
Hi Tomi

On Tue, Nov 24, 2020 at 06:26:47PM +0200, Tomi Valkeinen wrote:
> Hi Sam,
> 
> On 24/11/2020 18:18, Sam Ravnborg wrote:
> > Hi Tomi,
> > 
> > On Tue, Nov 24, 2020 at 02:45:13PM +0200, Tomi Valkeinen wrote:
> >> Use the common MIPI_DCS_GET_ERROR_COUNT_ON_DSI define instead of
> >> driver's own.
> >>
> > They are both 5 - OK
> > 
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> > 
> > IMO you should get all the patches at least up including this patch applied.
> > They are all reviewed/acked. And then you have a much smaller stack of
> > patches to spam us with.
> 
> Yes, I think that makes sense. I did not want to merge them earlier, as with the v3, I could not get
> videomode panels work at all (while cmd mode panels did work). So I was not sure if something is
> totally silly and broken in the series.
> 
> Now that I can get video mode panels work with some hacks on top, I'm fine with merging these.
> 
> But it's too late for 5.11, as we need testing and work on the video mode panels. So targeting 5.12.
Obviously your call, but I see no reason to wait for working videomode
panles if what you have now do not introduce any (known) regressions.

ofc I assume videomode panels is something new and not something that worked
before.

	Sam
Tomi Valkeinen Nov. 25, 2020, 8:52 a.m. UTC | #4
On 24/11/2020 18:38, Sam Ravnborg wrote:

>>> IMO you should get all the patches at least up including this patch applied.
>>> They are all reviewed/acked. And then you have a much smaller stack of
>>> patches to spam us with.
>>
>> Yes, I think that makes sense. I did not want to merge them earlier, as with the v3, I could not get
>> videomode panels work at all (while cmd mode panels did work). So I was not sure if something is
>> totally silly and broken in the series.
>>
>> Now that I can get video mode panels work with some hacks on top, I'm fine with merging these.
>>
>> But it's too late for 5.11, as we need testing and work on the video mode panels. So targeting 5.12.
> Obviously your call, but I see no reason to wait for working videomode
> panles if what you have now do not introduce any (known) regressions.
> 
> ofc I assume videomode panels is something new and not something that worked
> before.
It gets a bit muddy here. The omap dsi host driver has had videomode support for a long time, but
there has been no upstream videomode panel drivers (omapdrm specific drivers, as omapdrm had its own
panel framework) and no board dts files using it.

I have a board with a custom made DSI videomode panel setup, but it's broken (cable, I think) and
works only randomly. I have an old 4.14 based branch with a hacky panel driver and dts file which
get the panel working. I don't know if videomode works on current upstream, or has it been broken
between 4.14 and current upstream, as the 4.14 panel driver doesn't work without modifications on
current upstream.

In this series we convert the omap dsi host driver to be a proper DRM citizen, removing support for
omapdrm specific panels, so new DRM panel drivers are needed to replace the omapdrm specific ones.

With this series applied, and adding a new panel driver and dts changes, videomode works (Nikolaus
confirmed that his panel works. Mine doesn't, as afaics it needs more finetuned initialization which
may not be possible with the current DRM bridge/panel callbacks. But mine works with some hacks).
But I'm sure in the middle of this series videomode won't work.

So, I think one can argue that this causes regressions in the middle of the series to non-upstream
panel drivers, but at the end of the series, they probably work, presuming you have a new DRM panel
driver for it.

 Tomi
Laurent Pinchart Nov. 30, 2020, 9:50 a.m. UTC | #5
Hi Tomi,

Thank you for the patch.

On Tue, Nov 24, 2020 at 02:45:13PM +0200, Tomi Valkeinen wrote:
> Use the common MIPI_DCS_GET_ERROR_COUNT_ON_DSI define instead of
> driver's own.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/panel/panel-dsi-cm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c
> index 35a0c7da1974..cb0d27a38555 100644
> --- a/drivers/gpu/drm/panel/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c
> @@ -27,7 +27,6 @@
>  #include <video/of_display_timing.h>
>  #include <video/videomode.h>
>  
> -#define DCS_READ_NUM_ERRORS	0x05
>  #define DCS_GET_ID1		0xda
>  #define DCS_GET_ID2		0xdb
>  #define DCS_GET_ID3		0xdc
> @@ -225,7 +224,7 @@ static ssize_t num_dsi_errors_show(struct device *dev,
>  	mutex_lock(&ddata->lock);
>  
>  	if (ddata->enabled)
> -		r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS, &errors);
> +		r = dsicm_dcs_read_1(ddata, MIPI_DCS_GET_ERROR_COUNT_ON_DSI, &errors);
>  
>  	mutex_unlock(&ddata->lock);
>
Sebastian Reichel Dec. 14, 2020, 12:54 p.m. UTC | #6
Hi,

On Tue, Nov 24, 2020 at 02:45:13PM +0200, Tomi Valkeinen wrote:
> Use the common MIPI_DCS_GET_ERROR_COUNT_ON_DSI define instead of
> driver's own.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/gpu/drm/panel/panel-dsi-cm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c
> index 35a0c7da1974..cb0d27a38555 100644
> --- a/drivers/gpu/drm/panel/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c
> @@ -27,7 +27,6 @@
>  #include <video/of_display_timing.h>
>  #include <video/videomode.h>
>  
> -#define DCS_READ_NUM_ERRORS	0x05
>  #define DCS_GET_ID1		0xda
>  #define DCS_GET_ID2		0xdb
>  #define DCS_GET_ID3		0xdc
> @@ -225,7 +224,7 @@ static ssize_t num_dsi_errors_show(struct device *dev,
>  	mutex_lock(&ddata->lock);
>  
>  	if (ddata->enabled)
> -		r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS, &errors);
> +		r = dsicm_dcs_read_1(ddata, MIPI_DCS_GET_ERROR_COUNT_ON_DSI, &errors);
>  
>  	mutex_unlock(&ddata->lock);
>  
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c
index 35a0c7da1974..cb0d27a38555 100644
--- a/drivers/gpu/drm/panel/panel-dsi-cm.c
+++ b/drivers/gpu/drm/panel/panel-dsi-cm.c
@@ -27,7 +27,6 @@ 
 #include <video/of_display_timing.h>
 #include <video/videomode.h>
 
-#define DCS_READ_NUM_ERRORS	0x05
 #define DCS_GET_ID1		0xda
 #define DCS_GET_ID2		0xdb
 #define DCS_GET_ID3		0xdc
@@ -225,7 +224,7 @@  static ssize_t num_dsi_errors_show(struct device *dev,
 	mutex_lock(&ddata->lock);
 
 	if (ddata->enabled)
-		r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS, &errors);
+		r = dsicm_dcs_read_1(ddata, MIPI_DCS_GET_ERROR_COUNT_ON_DSI, &errors);
 
 	mutex_unlock(&ddata->lock);