[PATCH v2 0/4] drm/panel: jh057n00900: Move dsi init sequence to prepare

[PATCH v2 0/4] drm/panel: jh057n00900: Move dsi init sequence to prepare

[Guido Günther]

If the panel is wrapped in a panel_bridge it gets prepar()ed before the
upstream DSI bridge which can cause hangs (e.g. with imx-nwl since clocks
are not enabled yet). To avoid this move the panel's first DSI access to
enable() so the upstream bridge can prepare the DSI host controller in
it's pre_enable().

The second patch makes the disable() call symmetric to the above and the third
one just eases debugging.

Changes from v1:
* As per review comments by Sam Ravnborg
  * Ignore failures to disable the backlight in jh057n_disable()
  * Add 'drm/panel: jh057n00900: Use drm_panel_{unprepare,disable} consistently'
* Collected Reviewed-By: Thanks Sam!

To: "Guido Günther" <agx@sigxcpu.org>,Purism Kernel Team <kernel@puri.sm>,Thierry Reding <thierry.reding@gmail.com>,Sam Ravnborg <sam@ravnborg.org>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org


Guido Günther (4):
  drm/panel: jh057n00900: Move panel DSI init to enable()
  drm/panel: jh057n00900: Move mipi_dsi_dcs_set_display_off to disable()
  drm/panel: jh057n00900: Print error code on all DRM_DEV_ERROR()s
  drm/panel: jh057n00900: Use drm_panel_{unprepare,disable} consistently

 .../drm/panel/panel-rocktech-jh057n00900.c    | 31 ++++++++++---------
 1 file changed, 17 insertions(+), 14 deletions(-)

-- 
2.20.1

[PATCH v2 1/4] drm/panel: jh057n00900: Move panel DSI init to enable()

[Guido Günther]

If the panel is wrapped in a panel_bridge it gets prepar()ed before the
upstream DSI bridge which can cause hangs (e.g. with imx-nwl since clocks
are not enabled yet). To avoid this move the panel's first DSI access to
enable() so the upstream bridge can prepare the DSI host controller in
it's pre_enable().

This is also in line with other panel drivers.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 .../gpu/drm/panel/panel-rocktech-jh057n00900.c    | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
index 1274b54f2672..c6b4bfd79fde 100644
--- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
+++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
@@ -143,6 +143,14 @@ static int jh057n_init_sequence(struct jh057n *ctx)
 static int jh057n_enable(struct drm_panel *panel)
 {
 	struct jh057n *ctx = panel_to_jh057n(panel);
+	int ret;
+
+	ret = jh057n_init_sequence(ctx);
+	if (ret < 0) {
+		DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d\n",
+			      ret);
+		return ret;
+	}
 
 	return backlight_enable(ctx->backlight);
 }
@@ -197,13 +205,6 @@ static int jh057n_prepare(struct drm_panel *panel)
 	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
 	msleep(20);
 
-	ret = jh057n_init_sequence(ctx);
-	if (ret < 0) {
-		DRM_DEV_ERROR(ctx->dev, "Panel init sequence failed: %d\n",
-			      ret);
-		return ret;
-	}
-
 	ctx->prepared = true;
 
 	return 0;
-- 
2.20.1

[PATCH v2 2/4] drm/panel: jh057n00900: Move mipi_dsi_dcs_set_display_off to disable()

[Guido Günther]

This makes it symmetric with the panel init happening in enable().

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
index c6b4bfd79fde..fed24c51d48b 100644
--- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
+++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
@@ -158,19 +158,19 @@ static int jh057n_enable(struct drm_panel *panel)
 static int jh057n_disable(struct drm_panel *panel)
 {
 	struct jh057n *ctx = panel_to_jh057n(panel);
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
 
-	return backlight_disable(ctx->backlight);
+	backlight_disable(ctx->backlight);
+	return mipi_dsi_dcs_set_display_off(dsi);
 }
 
 static int jh057n_unprepare(struct drm_panel *panel)
 {
 	struct jh057n *ctx = panel_to_jh057n(panel);
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
 
 	if (!ctx->prepared)
 		return 0;
 
-	mipi_dsi_dcs_set_display_off(dsi);
 	regulator_disable(ctx->iovcc);
 	regulator_disable(ctx->vcc);
 	ctx->prepared = false;
-- 
2.20.1

[PATCH v2 3/4] drm/panel: jh057n00900: Print error code on all DRM_DEV_ERROR()s

[Guido Günther]

Most of them had these already but two mere missing. This eases
debugging.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
index fed24c51d48b..1037a201b4bb 100644
--- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
+++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
@@ -127,7 +127,7 @@ static int jh057n_init_sequence(struct jh057n *ctx)
 
 	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
 	if (ret < 0) {
-		DRM_DEV_ERROR(dev, "Failed to exit sleep mode\n");
+		DRM_DEV_ERROR(dev, "Failed to exit sleep mode: %d\n", ret);
 		return ret;
 	}
 	/* Panel is operational 120 msec after reset */
@@ -351,7 +351,9 @@ static int jh057n_probe(struct mipi_dsi_device *dsi)
 
 	ret = mipi_dsi_attach(dsi);
 	if (ret < 0) {
-		DRM_DEV_ERROR(dev, "mipi_dsi_attach failed. Is host ready?\n");
+		DRM_DEV_ERROR(dev,
+			      "mipi_dsi_attach failed (%d). Is host ready?\n",
+			      ret);
 		drm_panel_remove(&ctx->panel);
 		return ret;
 	}
-- 
2.20.1

[PATCH v2 4/4] drm/panel: jh057n00900: Use drm_panel_{unprepare,disable} consistently

[Guido Günther]

We were already using the generic functions in our debugfs code, do the
same in jh057n_shutdown. This was suggested by Sam Ravnborg.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
index 1037a201b4bb..b9109922397f 100644
--- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
+++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
@@ -372,12 +372,12 @@ static void jh057n_shutdown(struct mipi_dsi_device *dsi)
 	struct jh057n *ctx = mipi_dsi_get_drvdata(dsi);
 	int ret;
 
-	ret = jh057n_unprepare(&ctx->panel);
+	ret = drm_panel_unprepare(&ctx->panel);
 	if (ret < 0)
 		DRM_DEV_ERROR(&dsi->dev, "Failed to unprepare panel: %d\n",
 			      ret);
 
-	ret = jh057n_disable(&ctx->panel);
+	ret = drm_panel_disable(&ctx->panel);
 	if (ret < 0)
 		DRM_DEV_ERROR(&dsi->dev, "Failed to disable panel: %d\n",
 			      ret);
-- 
2.20.1

Re: [PATCH v2 0/4] drm/panel: jh057n00900: Move dsi init sequence to prepare

[Sam Ravnborg]

Hi Guido.

On Fri, Jul 26, 2019 at 03:14:35PM +0200, Guido Günther wrote:
> 
> If the panel is wrapped in a panel_bridge it gets prepar()ed before the
> upstream DSI bridge which can cause hangs (e.g. with imx-nwl since clocks
> are not enabled yet). To avoid this move the panel's first DSI access to
> enable() so the upstream bridge can prepare the DSI host controller in
> it's pre_enable().
> 
> The second patch makes the disable() call symmetric to the above and the third
> one just eases debugging.
> 
> Changes from v1:
> * As per review comments by Sam Ravnborg
>   * Ignore failures to disable the backlight in jh057n_disable()
>   * Add 'drm/panel: jh057n00900: Use drm_panel_{unprepare,disable} consistently'
> * Collected Reviewed-By: Thanks Sam!
> 
> To: "Guido Günther" <agx@sigxcpu.org>,Purism Kernel Team <kernel@puri.sm>,Thierry Reding <thierry.reding@gmail.com>,Sam Ravnborg <sam@ravnborg.org>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org
> 
> 
> Guido Günther (4):
>   drm/panel: jh057n00900: Move panel DSI init to enable()
>   drm/panel: jh057n00900: Move mipi_dsi_dcs_set_display_off to disable()
>   drm/panel: jh057n00900: Print error code on all DRM_DEV_ERROR()s
>   drm/panel: jh057n00900: Use drm_panel_{unprepare,disable} consistently

Looks good. Series applied to drm-misc-next and pushed out.

	Sam