[PATCH] drm/panel-notatek-nt35510: Fix enable/disable sequence

Linus Walleij linus.walleij at linaro.org
Wed Aug 5 14:28:08 UTC 2020


The driver was relying on only prepare()/unprepare() to
enable/disable the display.

This does not work because prepare() will be called
before the DSI host/bridge is ready to send any DSI
commands and disable() will be called after the DSI
host/bridge is disabled and thus unable to send any
DSI commands.

Move all DCS command sending to the enable() and
disable() callbacks, as is proper.

The prepare() and unprepare() is now only used to
enable and disable the regulators() and
asserting/de-asserting the reset line so we inline the
regulator and reset code here.

While developing this it was discovered that during
powercycling the device looses its ability to read
the MTP ID:s. We were lucky before as the display
came up with MTP reading enabled, but as this makes
powercycling work, we also need to send two small
sequences that enable the reading of the MTP ID
after powering on the regulators.

This makes it possible to use the user space UI
stack Phosh on the Samsung GT-S7710 as Phosh
enables/disables the display when starting.

Rename the nt35510_power_on() function to
nt35510_power_on_sequence() to reflect the fact
that this is not really about regulators but a
DCS command sequence.

Cc: Stephan Gerhold <stephan at gerhold.net>
Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
---
 drivers/gpu/drm/panel/panel-novatek-nt35510.c | 107 +++++++++++-------
 1 file changed, 68 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
index 4a8fa908a2cf..39113601e944 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
@@ -376,6 +376,10 @@ struct nt35510 {
 };
 
 /* Manufacturer command has strictly this byte sequence */
+static const u8 nt35510_mauc_mtp_read_param[] = { 0xAA, 0x55, 0x25, 0x01 };
+static const u8 nt35510_mauc_mtp_read_setting[] = { 0x01, 0x02, 0x00, 0x20,
+						    0x33, 0x13, 0x00, 0x40,
+						    0x00, 0x00, 0x23, 0x02 };
 static const u8 nt35510_mauc_select_page_0[] = { 0x55, 0xAA, 0x52, 0x08, 0x00 };
 static const u8 nt35510_mauc_select_page_1[] = { 0x55, 0xAA, 0x52, 0x08, 0x01 };
 static const u8 nt35510_vgh_on[] = { 0x01 };
@@ -674,29 +678,22 @@ static const struct backlight_ops nt35510_bl_ops = {
 /*
  * This power-on sequence
  */
-static int nt35510_power_on(struct nt35510 *nt)
+static int nt35510_power_on_sequence(struct nt35510 *nt)
 {
 	struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
 	int ret;
 
-	ret = regulator_bulk_enable(ARRAY_SIZE(nt->supplies), nt->supplies);
-	if (ret < 0) {
-		dev_err(nt->dev, "unable to enable regulators\n");
+	ret = nt35510_send_long(nt, dsi, MCS_CMD_MTP_READ_PARAM,
+				ARRAY_SIZE(nt35510_mauc_mtp_read_param),
+				nt35510_mauc_mtp_read_param);
+	if (ret)
 		return ret;
-	}
 
-	/* Toggle RESET in accordance with datasheet page 370 */
-	if (nt->reset_gpio) {
-		gpiod_set_value(nt->reset_gpio, 1);
-		/* Active min 10 us according to datasheet, let's say 20 */
-		usleep_range(20, 1000);
-		gpiod_set_value(nt->reset_gpio, 0);
-		/*
-		 * 5 ms during sleep mode, 120 ms during sleep out mode
-		 * according to datasheet, let's use 120-140 ms.
-		 */
-		usleep_range(120000, 140000);
-	}
+	ret = nt35510_send_long(nt, dsi, MCS_CMD_MTP_READ_SETTING,
+				ARRAY_SIZE(nt35510_mauc_mtp_read_setting),
+				nt35510_mauc_mtp_read_setting);
+	if (ret)
+		return ret;
 
 	ret = nt35510_read_id(nt);
 	if (ret)
@@ -758,26 +755,16 @@ static int nt35510_power_on(struct nt35510 *nt)
 	return 0;
 }
 
-static int nt35510_power_off(struct nt35510 *nt)
-{
-	int ret;
-
-	ret = regulator_bulk_disable(ARRAY_SIZE(nt->supplies), nt->supplies);
-	if (ret)
-		return ret;
-
-	if (nt->reset_gpio)
-		gpiod_set_value(nt->reset_gpio, 1);
-
-	return 0;
-}
-
-static int nt35510_unprepare(struct drm_panel *panel)
+static int nt35510_disable(struct drm_panel *panel)
 {
 	struct nt35510 *nt = panel_to_nt35510(panel);
 	struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
 	int ret;
 
+	/*
+	 * The final DCS traffic must happen here, at unprepare() the
+	 * DSI host will not accept traffic any more.
+	 */
 	ret = mipi_dsi_dcs_set_display_off(dsi);
 	if (ret) {
 		DRM_DEV_ERROR(nt->dev, "failed to turn display off (%d)\n",
@@ -797,20 +784,36 @@ static int nt35510_unprepare(struct drm_panel *panel)
 	/* Wait 4 frames, how much is that 5ms in the vendor driver */
 	usleep_range(5000, 10000);
 
-	ret = nt35510_power_off(nt);
+	return 0;
+}
+
+static int nt35510_unprepare(struct drm_panel *panel)
+{
+	struct nt35510 *nt = panel_to_nt35510(panel);
+	int ret;
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(nt->supplies), nt->supplies);
 	if (ret)
 		return ret;
 
+	if (nt->reset_gpio)
+		gpiod_set_value(nt->reset_gpio, 1);
+
+
 	return 0;
 }
 
-static int nt35510_prepare(struct drm_panel *panel)
+static int nt35510_enable(struct drm_panel *panel)
 {
 	struct nt35510 *nt = panel_to_nt35510(panel);
 	struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
 	int ret;
 
-	ret = nt35510_power_on(nt);
+	/*
+	 * At this point the DSI host is up and clocked and we can start
+	 * sending DCS commands.
+	 */
+	ret = nt35510_power_on_sequence(nt);
 	if (ret)
 		return ret;
 
@@ -836,6 +839,33 @@ static int nt35510_prepare(struct drm_panel *panel)
 	return 0;
 }
 
+static int nt35510_prepare(struct drm_panel *panel)
+{
+	struct nt35510 *nt = panel_to_nt35510(panel);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(nt->supplies), nt->supplies);
+	if (ret < 0) {
+		dev_err(nt->dev, "unable to enable regulators\n");
+		return ret;
+	}
+
+	/* Toggle RESET in accordance with datasheet page 370 */
+	if (nt->reset_gpio) {
+		gpiod_set_value(nt->reset_gpio, 1);
+		/* Active min 10 us according to datasheet, let's say 20 */
+		usleep_range(20, 1000);
+		gpiod_set_value(nt->reset_gpio, 0);
+		/*
+		 * 5 ms during sleep mode, 120 ms during sleep out mode
+		 * according to datasheet, let's use 120-140 ms.
+		 */
+		usleep_range(120000, 140000);
+	}
+
+	return 0;
+}
+
 static int nt35510_get_modes(struct drm_panel *panel,
 			     struct drm_connector *connector)
 {
@@ -862,6 +892,8 @@ static int nt35510_get_modes(struct drm_panel *panel,
 }
 
 static const struct drm_panel_funcs nt35510_drm_funcs = {
+	.enable = nt35510_enable,
+	.disable = nt35510_disable,
 	.unprepare = nt35510_unprepare,
 	.prepare = nt35510_prepare,
 	.get_modes = nt35510_get_modes,
@@ -970,14 +1002,11 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)
 static int nt35510_remove(struct mipi_dsi_device *dsi)
 {
 	struct nt35510 *nt = mipi_dsi_get_drvdata(dsi);
-	int ret;
 
 	mipi_dsi_detach(dsi);
-	/* Power off */
-	ret = nt35510_power_off(nt);
 	drm_panel_remove(&nt->panel);
 
-	return ret;
+	return 0;
 }
 
 /*
-- 
2.26.2



More information about the dri-devel mailing list