Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
From: Stephan Gerhold <stephan@gerhold.net>
To: Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Hai Li <hali@codeaurora.org>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Stephan Gerhold <stephan@gerhold.net>,
	Jasper Korten <jja2000@gmail.com>
Subject: [PATCH] drm/msm/dsi: Delay drm_panel_enable() until dsi_mgr_bridge_enable()
Date: Fri,  8 Nov 2019 22:28:40 +0100
Message-ID: <20191108212840.13586-1-stephan@gerhold.net> (raw)

At the moment, the MSM DSI driver calls drm_panel_enable() rather early
from the DSI bridge pre_enable() function. At this point, the encoder
(e.g. MDP5) is not enabled, so we have not started transmitting
video data.

However, the drm_panel_funcs documentation states that enable()
should be called on the panel *after* video data is being transmitted:

  The .prepare() function is typically called before the display controller
  starts to transmit video data. [...] After the display controller has
  started transmitting video data, it's safe to call the .enable() function.
  This will typically enable the backlight to make the image on screen visible.

Calling drm_panel_enable() too early causes problems for some panels:
The TFT LCD panel used in the Samsung Galaxy Tab A 9.7 (2015) (APQ8016)
uses the MIPI_DCS_SET_DISPLAY_BRIGHTNESS command to control
backlight/brightness of the screen. The enable sequence is therefore:

  drm_panel_enable()
    drm_panel_funcs.enable():
      backlight_enable()
        backlight_ops.update_status():
          mipi_dsi_dcs_set_display_brightness(dsi, bl->props.brightness);

The panel seems to silently ignore the MIPI_DCS_SET_DISPLAY_BRIGHTNESS
command if it is sent too early. This prevents setting the initial brightness,
causing the display to be enabled with minimum brightness instead.
Adding various delays in the panel initialization code does not result
in any difference.

On the other hand, moving drm_panel_enable() to dsi_mgr_bridge_enable()
fixes the problem, indicating that the panel requires the video stream
to be active before the brightness command is accepted.

Therefore: Move drm_panel_enable() to dsi_mgr_bridge_enable() to
delay calling it until video data is being transmitted.

Move drm_panel_disable() to dsi_mgr_bridge_disable() for similar reasons.
(This is not strictly required for the panel affected above...)

Tested-by: Jasper Korten <jja2000@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Since this is a core change I thought it would be better to send this
early. I believe Jasper still wants to finish some other changes before
submitting the initial device tree for the Samsung Galaxy Tab A 9.7 (2015). ;)

I also tested it on msm8916-samsung-a5u-eur, its display is working fine
with or without this patch.
---
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 62 ++++++++++++++++++---------
 1 file changed, 41 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 271aa7bbca92..eea108865cd3 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -432,20 +432,8 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge)
 		}
 	}
 
-	if (panel) {
-		ret = drm_panel_enable(panel);
-		if (ret) {
-			pr_err("%s: enable panel %d failed, %d\n", __func__, id,
-									ret);
-			goto panel_en_fail;
-		}
-	}
-
 	return;
 
-panel_en_fail:
-	if (is_dual_dsi && msm_dsi1)
-		msm_dsi_host_disable(msm_dsi1->host);
 host1_en_fail:
 	msm_dsi_host_disable(host);
 host_en_fail:
@@ -464,12 +452,51 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge)
 
 static void dsi_mgr_bridge_enable(struct drm_bridge *bridge)
 {
-	DBG("");
+	int id = dsi_mgr_bridge_get_id(bridge);
+	struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
+	struct drm_panel *panel = msm_dsi->panel;
+	bool is_dual_dsi = IS_DUAL_DSI();
+	int ret;
+
+	DBG("id=%d", id);
+	if (!msm_dsi_device_connected(msm_dsi))
+		return;
+
+	/* Do nothing with the host if it is slave-DSI in case of dual DSI */
+	if (is_dual_dsi && !IS_MASTER_DSI_LINK(id))
+		return;
+
+	if (panel) {
+		ret = drm_panel_enable(panel);
+		if (ret) {
+			pr_err("%s: enable panel %d failed, %d\n", __func__, id,
+									ret);
+		}
+	}
 }
 
 static void dsi_mgr_bridge_disable(struct drm_bridge *bridge)
 {
-	DBG("");
+	int id = dsi_mgr_bridge_get_id(bridge);
+	struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
+	struct drm_panel *panel = msm_dsi->panel;
+	bool is_dual_dsi = IS_DUAL_DSI();
+	int ret;
+
+	DBG("id=%d", id);
+	if (!msm_dsi_device_connected(msm_dsi))
+		return;
+
+	/* Do nothing with the host if it is slave-DSI in case of dual DSI */
+	if (is_dual_dsi && !IS_MASTER_DSI_LINK(id))
+		return;
+
+	if (panel) {
+		ret = drm_panel_disable(panel);
+		if (ret)
+			pr_err("%s: Panel %d OFF failed, %d\n", __func__, id,
+									ret);
+	}
 }
 
 static void dsi_mgr_bridge_post_disable(struct drm_bridge *bridge)
@@ -495,13 +522,6 @@ static void dsi_mgr_bridge_post_disable(struct drm_bridge *bridge)
 	if (is_dual_dsi && !IS_MASTER_DSI_LINK(id))
 		goto disable_phy;
 
-	if (panel) {
-		ret = drm_panel_disable(panel);
-		if (ret)
-			pr_err("%s: Panel %d OFF failed, %d\n", __func__, id,
-									ret);
-	}
-
 	ret = msm_dsi_host_disable(host);
 	if (ret)
 		pr_err("%s: host %d disable failed, %d\n", __func__, id, ret);
-- 
2.23.0


             reply index

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-08 21:28 Stephan Gerhold [this message]
2019-11-08 22:12 ` [Freedreno] " Jeffrey Hugo
2019-11-08 23:46   ` Stephan Gerhold
2019-11-09  3:47     ` Jeffrey Hugo
2019-11-09 12:01       ` Stephan Gerhold
2019-11-09 23:55         ` Jeffrey Hugo

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191108212840.13586-1-stephan@gerhold.net \
    --to=stephan@gerhold.net \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=hali@codeaurora.org \
    --cc=jja2000@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robdclark@gmail.com \
    --cc=sean@poorly.run \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git