dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus
@ 2023-07-13  9:01 AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function AngeloGioacchino Del Regno
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

Changes in v5:
 - Added .wait_hpd_asserted() callback for aux-bus
 - Avoid enabling and registering HPD interrupt + handlers for
   eDP case only (keeps HPD interrupts enabled for full DP case)
 - Support not always-on eDP panels (boot with regulator off,
   suspend with regulator off) for power saving in PM suspend.

Changes in v4:
 - Set data lanes to idle to prevent stalls if bootloader didn't
   properly close the eDP port
 - Now using the .done_probing() callback for AUX bus to prevent
   probe deferral loops in case the panel-edp driver is a module
   as previously seen with another bridge driver (ANX7625) on
   some other SoCs (MT8192 and others)
 - Rebased over next-20230706
 - Dropped Chen-Yu's T-b tag on last patch as some logic changed
   (before, I wasn't using the .done_probing() callback).

Changes in v3:
 - Added DPTX AUX block initialization before trying to communicate
   to stop relying on the bootloader keeping it initialized before
   booting Linux.
 - Fixed commit description for patch [09/09] and removed commented
   out code (that slipped from dev phase.. sorry!).

This series adds "real" support for eDP in the mtk-dp DisplayPort driver.

Explaining the "real":
Before this change, the DisplayPort driver did support eDP to some
extent, but it was treating it entirely like a regular DP interface
which is partially fine, after all, embedded DisplayPort *is* actually
DisplayPort, but there might be some differences to account for... and
this is for both small performance improvements and, more importantly,
for correct functionality in some systems.

Functionality first:

One of the common differences found in various boards implementing eDP
and machines using an eDP panel is that many times the HPD line is not
connected. This *must* be accounted for: at startup, this specific IP
will raise a HPD interrupt (which should maybe be ignored... as it does
not appear to be a "real" event...) that will make the eDP panel to be
detected and to actually work but, after a suspend-resume cycle, there
will be no HPD interrupt (as there's no HPD line in my case!) producing
a functionality issue - specifically, the DP Link Training fails because
the panel doesn't get powered up, then it stays black and won't work
until rebooting the machine (or removing and reinserting the module I
think, but I haven't tried that).

Now for.. both:
eDP panels are *e*DP because they are *not* removable (in the sense that
you can't unplug the cable without disassembling the machine, in which
case, the machine shall be powered down..!): this (correct) assumption
makes us able to solve some issues and to also gain a little performance
during PM operations.

What was done here is:
 - Caching the EDID if the panel is eDP: we're always going to read the
   same data everytime, so we can just cache that (as it's small enough)
   shortening PM resume times for the eDP driver instance;
 - Always return connector_status_connected if it's eDP: non-removable
   means connector_status_disconnected can't happen during runtime...
   this also saves us some time and even power, as we won't have to
   perform yet another power cycle of the HW;
 - Added aux-bus support!
   This makes us able to rely on panel autodetection from the EDID,
   avoiding to add more and more panel timings to panel-edp and, even
   better, allowing to use one panel node in devicetrees for multiple
   variants of the same machine since, at that point, it's not important
   to "preventively know" what panel we have (eh, it's autodetected...!).

This was tested on a MT8195 Cherry Tomato Chromebook (panel-edp on aux-bus)


P.S.: For your own testing commodity, here's a reference devicetree:

pp3300_disp_x: regulator-pp3300-disp-x {
	compatible = "regulator-fixed";
	regulator-name = "pp3300_disp_x";
	regulator-min-microvolt = <3300000>;
	regulator-max-microvolt = <3300000>;
	enable-active-high;
	gpio = <&pio 55 GPIO_ACTIVE_HIGH>;
	pinctrl-names = "default";
	pinctrl-0 = <&panel_fixed_pins>;
};

&edp_tx {
	status = "okay";

	pinctrl-names = "default";
	pinctrl-0 = <&edptx_pins_default>;

	ports {
		#address-cells = <1>;
		#size-cells = <0>;

		port@0 {
			reg = <0>;
			edp_in: endpoint {
				remote-endpoint = <&dp_intf0_out>;
			};
		};

		port@1 {
			reg = <1>;
			edp_out: endpoint {
				data-lanes = <0 1 2 3>;
				remote-endpoint = <&panel_in>;
			};
		};
	};

	aux-bus {
		panel: panel {
			compatible = "edp-panel";
			power-supply = <&pp3300_disp_x>;
			backlight = <&backlight_lcd0>;
			port {
				panel_in: endpoint {
					remote-endpoint = <&edp_out>;
				};
			};
		};
	};
};

AngeloGioacchino Del Regno (10):
  drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
  drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer()
  drm/mediatek: dp: Use devm variant of drm_bridge_add()
  drm/mediatek: dp: Move AUX_P0 setting to
    mtk_dp_initialize_aux_settings()
  drm/mediatek: dp: Enable event interrupt only when bridge attached
  drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled
  drm/mediatek: dp: Move PHY registration to new function
  drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
  drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus
  drm/mediatek: dp: Don't register HPD interrupt handler for eDP case

 drivers/gpu/drm/mediatek/Kconfig  |   1 +
 drivers/gpu/drm/mediatek/mtk_dp.c | 335 ++++++++++++++++++++----------
 2 files changed, 224 insertions(+), 112 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-18  7:40   ` CK Hu (胡俊光)
  2023-07-13  9:01 ` [PATCH v5 02/10] drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer() AngeloGioacchino Del Regno
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

Everytime we run bridge detection and/or EDID read we run a poweron
and poweroff sequence for both the AUX and the panel; moreover, this
is also done when enabling the bridge in the .atomic_enable() callback.

Move this power on/off sequence to a new mtk_dp_aux_panel_poweron()
function as to commonize it.
Note that, before this commit, in mtk_dp_bridge_atomic_enable() only
the AUX was getting powered on but the panel was left powered off if
the DP cable wasn't plugged in while now we unconditionally send a D0
request and this is done for two reasons:
 - First, whether this request fails or not, it takes the same time
   and anyway the DP hardware won't produce any error (or, if it
   does, it's ignorable because it won't block further commands)
 - Second, training the link between a sleeping/standby/unpowered
   display makes little sense.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 76 ++++++++++++-------------------
 1 file changed, 30 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 64eee77452c0..8b7ded1746f3 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1251,6 +1251,29 @@ static void mtk_dp_audio_mute(struct mtk_dp *mtk_dp, bool mute)
 			   val[2], AU_TS_CFG_DP_ENC0_P0_MASK);
 }
 
+static void mtk_dp_aux_panel_poweron(struct mtk_dp *mtk_dp, bool pwron)
+{
+	if (pwron) {
+		/* power on aux */
+		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
+				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
+				   DP_PWR_STATE_MASK);
+
+		/* power on panel */
+		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
+		usleep_range(2000, 5000);
+	} else {
+		/* power off panel */
+		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D3);
+		usleep_range(2000, 3000);
+
+		/* power off aux */
+		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
+				   DP_PWR_STATE_BANDGAP_TPLL,
+				   DP_PWR_STATE_MASK);
+	}
+}
+
 static void mtk_dp_power_enable(struct mtk_dp *mtk_dp)
 {
 	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_RESET_AND_PROBE,
@@ -1936,16 +1959,9 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
 	if (!mtk_dp->train_info.cable_plugged_in)
 		return ret;
 
-	if (!enabled) {
-		/* power on aux */
-		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
-				   DP_PWR_STATE_MASK);
+	if (!enabled)
+		mtk_dp_aux_panel_poweron(mtk_dp, true);
 
-		/* power on panel */
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
-		usleep_range(2000, 5000);
-	}
 	/*
 	 * Some dongles still source HPD when they do not connect to any
 	 * sink device. To avoid this, we need to read the sink count
@@ -1957,16 +1973,8 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge)
 	if (DP_GET_SINK_COUNT(sink_count))
 		ret = connector_status_connected;
 
-	if (!enabled) {
-		/* power off panel */
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D3);
-		usleep_range(2000, 3000);
-
-		/* power off aux */
-		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-				   DP_PWR_STATE_BANDGAP_TPLL,
-				   DP_PWR_STATE_MASK);
-	}
+	if (!enabled)
+		mtk_dp_aux_panel_poweron(mtk_dp, false);
 
 	return ret;
 }
@@ -1982,15 +1990,7 @@ static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
 
 	if (!enabled) {
 		drm_atomic_bridge_chain_pre_enable(bridge, connector->state->state);
-
-		/* power on aux */
-		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
-				   DP_PWR_STATE_MASK);
-
-		/* power on panel */
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
-		usleep_range(2000, 5000);
+		mtk_dp_aux_panel_poweron(mtk_dp, true);
 	}
 
 	new_edid = drm_get_edid(connector, &mtk_dp->aux.ddc);
@@ -2010,15 +2010,7 @@ static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
 	}
 
 	if (!enabled) {
-		/* power off panel */
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D3);
-		usleep_range(2000, 3000);
-
-		/* power off aux */
-		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-				   DP_PWR_STATE_BANDGAP_TPLL,
-				   DP_PWR_STATE_MASK);
-
+		mtk_dp_aux_panel_poweron(mtk_dp, false);
 		drm_atomic_bridge_chain_post_disable(bridge, connector->state->state);
 	}
 
@@ -2178,15 +2170,7 @@ static void mtk_dp_bridge_atomic_enable(struct drm_bridge *bridge,
 		return;
 	}
 
-	/* power on aux */
-	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
-			   DP_PWR_STATE_BANDGAP_TPLL_LANE,
-			   DP_PWR_STATE_MASK);
-
-	if (mtk_dp->train_info.cable_plugged_in) {
-		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
-		usleep_range(2000, 5000);
-	}
+	mtk_dp_aux_panel_poweron(mtk_dp, true);
 
 	/* Training */
 	ret = mtk_dp_training(mtk_dp);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 02/10] drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer()
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 03/10] drm/mediatek: dp: Use devm variant of drm_bridge_add() AngeloGioacchino Del Regno
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

Change logging from drm_{err,info}() to dev_{err,info}() in functions
mtk_dp_aux_transfer() and mtk_dp_aux_do_transfer(): this will be
essential to avoid getting NULL pointer kernel panics if any kind
of error happens during AUX transfers happening before the bridge
is attached.

This may potentially start happening in a later commit implementing
aux-bus support, as AUX transfers will be triggered from the panel
driver (for EDID) before the mtk-dp bridge gets attached, and it's
done in preparation for the same.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 8b7ded1746f3..a6a05ea6a53c 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -847,7 +847,7 @@ static int mtk_dp_aux_do_transfer(struct mtk_dp *mtk_dp, bool is_read, u8 cmd,
 		u32 phy_status = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3628) &
 				 AUX_RX_PHY_STATE_AUX_TX_P0_MASK;
 		if (phy_status != AUX_RX_PHY_STATE_AUX_TX_P0_RX_IDLE) {
-			drm_err(mtk_dp->drm_dev,
+			dev_err(mtk_dp->dev,
 				"AUX Rx Aux hang, need SW reset\n");
 			return -EIO;
 		}
@@ -2049,7 +2049,7 @@ static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux,
 		is_read = true;
 		break;
 	default:
-		drm_err(mtk_aux->drm_dev, "invalid aux cmd = %d\n",
+		dev_err(mtk_dp->dev, "invalid aux cmd = %d\n",
 			msg->request);
 		ret = -EINVAL;
 		goto err;
@@ -2065,7 +2065,7 @@ static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux,
 					     to_access, &msg->reply);
 
 		if (ret) {
-			drm_info(mtk_dp->drm_dev,
+			dev_info(mtk_dp->dev,
 				 "Failed to do AUX transfer: %d\n", ret);
 			goto err;
 		}
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 03/10] drm/mediatek: dp: Use devm variant of drm_bridge_add()
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 02/10] drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer() AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 04/10] drm/mediatek: dp: Move AUX_P0 setting to mtk_dp_initialize_aux_settings() AngeloGioacchino Del Regno
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

In preparation for adding support for aux-bus, which will add a code
path that may fail after the drm_bridge_add() call, change that to
devm_drm_bridge_add() to simplify failure paths later.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index a6a05ea6a53c..9dea78529697 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2547,7 +2547,7 @@ static int mtk_dp_probe(struct platform_device *pdev)
 		DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
 	mtk_dp->bridge.type = mtk_dp->data->bridge_type;
 
-	drm_bridge_add(&mtk_dp->bridge);
+	devm_drm_bridge_add(dev, &mtk_dp->bridge);
 
 	mtk_dp->need_debounce = true;
 	timer_setup(&mtk_dp->debounce_timer, mtk_dp_debounce_timer, 0);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 04/10] drm/mediatek: dp: Move AUX_P0 setting to mtk_dp_initialize_aux_settings()
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
                   ` (2 preceding siblings ...)
  2023-07-13  9:01 ` [PATCH v5 03/10] drm/mediatek: dp: Use devm variant of drm_bridge_add() AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 05/10] drm/mediatek: dp: Enable event interrupt only when bridge attached AngeloGioacchino Del Regno
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

Move the register write to MTK_DP_AUX_P0_3690 to set the AUX reply mode
to function mtk_dp_initialize_aux_settings(), as this is effectively
part of the DPTX AUX setup sequence.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 9dea78529697..b4e775e040ee 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1009,6 +1009,11 @@ static void mtk_dp_initialize_aux_settings(struct mtk_dp *mtk_dp)
 	mtk_dp_update_bits(mtk_dp, MTK_DP_AUX_P0_37C8,
 			   MTK_ATOP_EN_AUX_TX_P0,
 			   MTK_ATOP_EN_AUX_TX_P0);
+
+	/* Set complete reply mode for AUX */
+	mtk_dp_update_bits(mtk_dp, MTK_DP_AUX_P0_3690,
+			   RX_REPLY_COMPLETE_MODE_AUX_TX_P0,
+			   RX_REPLY_COMPLETE_MODE_AUX_TX_P0);
 }
 
 static void mtk_dp_initialize_digital_settings(struct mtk_dp *mtk_dp)
@@ -1821,10 +1826,6 @@ static void mtk_dp_init_port(struct mtk_dp *mtk_dp)
 	mtk_dp_initialize_settings(mtk_dp);
 	mtk_dp_initialize_aux_settings(mtk_dp);
 	mtk_dp_initialize_digital_settings(mtk_dp);
-
-	mtk_dp_update_bits(mtk_dp, MTK_DP_AUX_P0_3690,
-			   RX_REPLY_COMPLETE_MODE_AUX_TX_P0,
-			   RX_REPLY_COMPLETE_MODE_AUX_TX_P0);
 	mtk_dp_initialize_hpd_detect_settings(mtk_dp);
 
 	mtk_dp_digital_sw_reset(mtk_dp);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 05/10] drm/mediatek: dp: Enable event interrupt only when bridge attached
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
                   ` (3 preceding siblings ...)
  2023-07-13  9:01 ` [PATCH v5 04/10] drm/mediatek: dp: Move AUX_P0 setting to mtk_dp_initialize_aux_settings() AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 06/10] drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled AngeloGioacchino Del Regno
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

It is useless and error-prone to enable the DisplayPort event interrupt
before finishing to probe and install the driver, as the DP training
cannot happen before the entire pipeline is correctly set up, as the
interrupt handler also requires the full hardware to be initialized by
mtk_dp_bridge_attach().

Anyway, depending in which state the controller is left from the
bootloader, this may cause an interrupt storm and consequently hang
the kernel during boot, so, avoid enabling the interrupt until we
reach a clean state by adding the IRQ_NOAUTOEN flag before requesting
it at probe time and manage the enablement of the ISR in the .attach()
and .detach() handlers for the DP bridge.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index b4e775e040ee..d67dbafbac8e 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -100,6 +100,7 @@ struct mtk_dp_efuse_fmt {
 struct mtk_dp {
 	bool enabled;
 	bool need_debounce;
+	int irq;
 	u8 max_lanes;
 	u8 max_linkrate;
 	u8 rx_cap[DP_RECEIVER_CAP_SIZE];
@@ -2136,6 +2137,8 @@ static int mtk_dp_bridge_attach(struct drm_bridge *bridge,
 
 	mtk_dp->drm_dev = bridge->dev;
 
+	irq_clear_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
+	enable_irq(mtk_dp->irq);
 	mtk_dp_hwirq_enable(mtk_dp, true);
 
 	return 0;
@@ -2152,6 +2155,7 @@ static void mtk_dp_bridge_detach(struct drm_bridge *bridge)
 	struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
 
 	mtk_dp_hwirq_enable(mtk_dp, false);
+	disable_irq(mtk_dp->irq);
 	mtk_dp->drm_dev = NULL;
 	mtk_dp_poweroff(mtk_dp);
 	drm_dp_aux_unregister(&mtk_dp->aux);
@@ -2470,7 +2474,7 @@ static int mtk_dp_probe(struct platform_device *pdev)
 {
 	struct mtk_dp *mtk_dp;
 	struct device *dev = &pdev->dev;
-	int ret, irq_num;
+	int ret;
 
 	mtk_dp = devm_kzalloc(dev, sizeof(*mtk_dp), GFP_KERNEL);
 	if (!mtk_dp)
@@ -2479,9 +2483,9 @@ static int mtk_dp_probe(struct platform_device *pdev)
 	mtk_dp->dev = dev;
 	mtk_dp->data = (struct mtk_dp_data *)of_device_get_match_data(dev);
 
-	irq_num = platform_get_irq(pdev, 0);
-	if (irq_num < 0)
-		return dev_err_probe(dev, irq_num,
+	mtk_dp->irq = platform_get_irq(pdev, 0);
+	if (mtk_dp->irq < 0)
+		return dev_err_probe(dev, mtk_dp->irq,
 				     "failed to request dp irq resource\n");
 
 	mtk_dp->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
@@ -2502,7 +2506,8 @@ static int mtk_dp_probe(struct platform_device *pdev)
 
 	spin_lock_init(&mtk_dp->irq_thread_lock);
 
-	ret = devm_request_threaded_irq(dev, irq_num, mtk_dp_hpd_event,
+	irq_set_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
+	ret = devm_request_threaded_irq(dev, mtk_dp->irq, mtk_dp_hpd_event,
 					mtk_dp_hpd_event_thread,
 					IRQ_TYPE_LEVEL_HIGH, dev_name(dev),
 					mtk_dp);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 06/10] drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
                   ` (4 preceding siblings ...)
  2023-07-13  9:01 ` [PATCH v5 05/10] drm/mediatek: dp: Enable event interrupt only when bridge attached AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 07/10] drm/mediatek: dp: Move PHY registration to new function AngeloGioacchino Del Regno
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

If a controller (usually, eDP!) does not support audio, or audio is not
enabled because the endpoint has no audio support, it's useless to lock
a mutex only to unlock it right after because there's no .plugged_cb().

Check if the audio is supported and enabled before locking the mutex in
mtk_dp_update_plugged_status(): if not, we simply return immediately.

While at it, since the update_plugged_status_lock mutex would not be
used if the controller doesn't support audio at all, initialize it
only if `audio_supported` is true.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index d67dbafbac8e..3cb234b502a5 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1943,6 +1943,9 @@ static int mtk_dp_dt_parse(struct mtk_dp *mtk_dp,
 
 static void mtk_dp_update_plugged_status(struct mtk_dp *mtk_dp)
 {
+	if (!mtk_dp->data->audio_supported || !mtk_dp->audio_enable)
+		return;
+
 	mutex_lock(&mtk_dp->update_plugged_status_lock);
 	if (mtk_dp->plugged_cb && mtk_dp->codec_dev)
 		mtk_dp->plugged_cb(mtk_dp->codec_dev,
@@ -2515,11 +2518,11 @@ static int mtk_dp_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret,
 				     "failed to request mediatek dptx irq\n");
 
-	mutex_init(&mtk_dp->update_plugged_status_lock);
-
 	platform_set_drvdata(pdev, mtk_dp);
 
 	if (mtk_dp->data->audio_supported) {
+		mutex_init(&mtk_dp->update_plugged_status_lock);
+
 		ret = mtk_dp_register_audio_driver(dev);
 		if (ret) {
 			dev_err(dev, "Failed to register audio driver: %d\n",
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 07/10] drm/mediatek: dp: Move PHY registration to new function
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
                   ` (5 preceding siblings ...)
  2023-07-13  9:01 ` [PATCH v5 06/10] drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 08/10] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus AngeloGioacchino Del Regno
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

In preparation for adding support for eDP, move the PHY registration
code to a new mtk_dp_register_phy() function for better readability.

This commit brings no functional changes.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 43 +++++++++++++++++++------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 3cb234b502a5..1684dbf7bbff 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2473,6 +2473,29 @@ static int mtk_dp_register_audio_driver(struct device *dev)
 	return PTR_ERR_OR_ZERO(mtk_dp->audio_pdev);
 }
 
+static int mtk_dp_register_phy(struct mtk_dp *mtk_dp)
+{
+	struct device *dev = mtk_dp->dev;
+
+	mtk_dp->phy_dev = platform_device_register_data(dev, "mediatek-dp-phy",
+							PLATFORM_DEVID_AUTO,
+							&mtk_dp->regs,
+							sizeof(struct regmap *));
+	if (IS_ERR(mtk_dp->phy_dev))
+		return dev_err_probe(dev, PTR_ERR(mtk_dp->phy_dev),
+				     "Failed to create device mediatek-dp-phy\n");
+
+	mtk_dp_get_calibration_data(mtk_dp);
+
+	mtk_dp->phy = devm_phy_get(&mtk_dp->phy_dev->dev, "dp");
+	if (IS_ERR(mtk_dp->phy)) {
+		platform_device_unregister(mtk_dp->phy_dev);
+		return dev_err_probe(dev, PTR_ERR(mtk_dp->phy), "Failed to get phy\n");
+	}
+
+	return 0;
+}
+
 static int mtk_dp_probe(struct platform_device *pdev)
 {
 	struct mtk_dp *mtk_dp;
@@ -2531,23 +2554,9 @@ static int mtk_dp_probe(struct platform_device *pdev)
 		}
 	}
 
-	mtk_dp->phy_dev = platform_device_register_data(dev, "mediatek-dp-phy",
-							PLATFORM_DEVID_AUTO,
-							&mtk_dp->regs,
-							sizeof(struct regmap *));
-	if (IS_ERR(mtk_dp->phy_dev))
-		return dev_err_probe(dev, PTR_ERR(mtk_dp->phy_dev),
-				     "Failed to create device mediatek-dp-phy\n");
-
-	mtk_dp_get_calibration_data(mtk_dp);
-
-	mtk_dp->phy = devm_phy_get(&mtk_dp->phy_dev->dev, "dp");
-
-	if (IS_ERR(mtk_dp->phy)) {
-		platform_device_unregister(mtk_dp->phy_dev);
-		return dev_err_probe(dev, PTR_ERR(mtk_dp->phy),
-				     "Failed to get phy\n");
-	}
+	ret = mtk_dp_register_phy(mtk_dp);
+	if (ret)
+		return ret;
 
 	mtk_dp->bridge.funcs = &mtk_dp_bridge_funcs;
 	mtk_dp->bridge.of_node = dev->of_node;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 08/10] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
                   ` (6 preceding siblings ...)
  2023-07-13  9:01 ` [PATCH v5 07/10] drm/mediatek: dp: Move PHY registration to new function AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 09/10] drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus AngeloGioacchino Del Regno
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

For the eDP case we can support using aux-bus on MediaTek DP: this
gives us the possibility to declare our panel as generic "panel-edp"
which will automatically configure the timings and available modes
via the EDID that we read from it.

To do this, move the panel parsing at the end of the probe function
so that the hardware is initialized beforehand and also initialize
the DPTX AUX block and power both on as, when we populate the
aux-bus, the panel driver will trigger an EDID read to perform
panel detection.

Last but not least, since now the AUX transfers can happen in the
separated aux-bus, it was necessary to add an exclusion for the
cable_plugged_in check in `mtk_dp_aux_transfer()` and the easiest
way to do this is to simply ignore checking that when the bridge
type is eDP.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/Kconfig  |  1 +
 drivers/gpu/drm/mediatek/mtk_dp.c | 85 ++++++++++++++++++++++++++++---
 2 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig
index b451dee64d34..76cab28e010c 100644
--- a/drivers/gpu/drm/mediatek/Kconfig
+++ b/drivers/gpu/drm/mediatek/Kconfig
@@ -26,6 +26,7 @@ config DRM_MEDIATEK_DP
 	select PHY_MTK_DP
 	select DRM_DISPLAY_HELPER
 	select DRM_DISPLAY_DP_HELPER
+	select DRM_DP_AUX_BUS
 	help
 	  DRM/KMS Display Port driver for MediaTek SoCs.
 
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 1684dbf7bbff..8668ab17135d 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2022 BayLibre
  */
 
+#include <drm/display/drm_dp_aux_bus.h>
 #include <drm/display/drm_dp.h>
 #include <drm/display/drm_dp_helper.h>
 #include <drm/drm_atomic_helper.h>
@@ -1313,9 +1314,11 @@ static void mtk_dp_power_disable(struct mtk_dp *mtk_dp)
 
 static void mtk_dp_initialize_priv_data(struct mtk_dp *mtk_dp)
 {
+	bool plugged_in = (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP);
+
 	mtk_dp->train_info.link_rate = DP_LINK_BW_5_4;
 	mtk_dp->train_info.lane_count = mtk_dp->max_lanes;
-	mtk_dp->train_info.cable_plugged_in = false;
+	mtk_dp->train_info.cable_plugged_in = plugged_in;
 
 	mtk_dp->info.format = DP_PIXELFORMAT_RGB;
 	memset(&mtk_dp->info.vm, 0, sizeof(struct videomode));
@@ -1617,6 +1620,16 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp)
 	u8 val;
 	ssize_t ret;
 
+	/*
+	 * If we're eDP and capabilities were already parsed we can skip
+	 * reading again because eDP panels aren't hotpluggable hence the
+	 * caps and training information won't ever change in a boot life
+	 */
+	if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP &&
+	    mtk_dp->rx_cap[DP_MAX_LINK_RATE] &&
+	    mtk_dp->train_info.sink_ssc)
+		return 0;
+
 	drm_dp_read_dpcd_caps(&mtk_dp->aux, mtk_dp->rx_cap);
 
 	if (drm_dp_tps4_supported(mtk_dp->rx_cap))
@@ -2025,15 +2038,14 @@ static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
 static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux,
 				   struct drm_dp_aux_msg *msg)
 {
-	struct mtk_dp *mtk_dp;
+	struct mtk_dp *mtk_dp = container_of(mtk_aux, struct mtk_dp, aux);
 	bool is_read;
 	u8 request;
 	size_t accessed_bytes = 0;
 	int ret;
 
-	mtk_dp = container_of(mtk_aux, struct mtk_dp, aux);
-
-	if (!mtk_dp->train_info.cable_plugged_in) {
+	if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP &&
+	    !mtk_dp->train_info.cable_plugged_in) {
 		ret = -EAGAIN;
 		goto err;
 	}
@@ -2496,6 +2508,29 @@ static int mtk_dp_register_phy(struct mtk_dp *mtk_dp)
 	return 0;
 }
 
+static int mtk_dp_edp_link_panel(struct drm_dp_aux *mtk_aux)
+{
+	struct mtk_dp *mtk_dp = container_of(mtk_aux, struct mtk_dp, aux);
+	struct device *dev = mtk_aux->dev;
+	int ret;
+
+	mtk_dp->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+
+	/* Power off the DP and AUX: either detection is done, or no panel present */
+	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
+			   DP_PWR_STATE_BANDGAP_TPLL,
+			   DP_PWR_STATE_MASK);
+	mtk_dp_power_disable(mtk_dp);
+
+	if (IS_ERR(mtk_dp->next_bridge)) {
+		ret = PTR_ERR(mtk_dp->next_bridge);
+		mtk_dp->next_bridge = NULL;
+		return ret;
+	}
+
+	return 0;
+}
+
 static int mtk_dp_probe(struct platform_device *pdev)
 {
 	struct mtk_dp *mtk_dp;
@@ -2526,9 +2561,10 @@ static int mtk_dp_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to parse dt\n");
 
-	drm_dp_aux_init(&mtk_dp->aux);
 	mtk_dp->aux.name = "aux_mtk_dp";
+	mtk_dp->aux.dev = dev;
 	mtk_dp->aux.transfer = mtk_dp_aux_transfer;
+	drm_dp_aux_init(&mtk_dp->aux);
 
 	spin_lock_init(&mtk_dp->irq_thread_lock);
 
@@ -2570,6 +2606,43 @@ static int mtk_dp_probe(struct platform_device *pdev)
 	mtk_dp->need_debounce = true;
 	timer_setup(&mtk_dp->debounce_timer, mtk_dp_debounce_timer, 0);
 
+	if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP) {
+		/*
+		 * Set the data lanes to idle in case the bootloader didn't
+		 * properly close the eDP port to avoid stalls and then
+		 * reinitialize, reset and power on the AUX block.
+		 */
+		mtk_dp_set_idle_pattern(mtk_dp, true);
+		mtk_dp_initialize_aux_settings(mtk_dp);
+		mtk_dp_power_enable(mtk_dp);
+
+		/*
+		 * Power on the AUX to allow reading the EDID from aux-bus:
+		 * please note that it is necessary to call power off in the
+		 * .done_probing() callback (mtk_dp_edp_link_panel), as only
+		 * there we can safely assume that we finished reading EDID.
+		 */
+		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
+				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
+				   DP_PWR_STATE_MASK);
+
+		ret = devm_of_dp_aux_populate_bus(&mtk_dp->aux, mtk_dp_edp_link_panel);
+		if (ret) {
+			/* -ENODEV this means that the panel is not on the aux-bus */
+			if (ret == -ENODEV) {
+				ret = mtk_dp_edp_link_panel(&mtk_dp->aux);
+				if (ret)
+					return ret;
+			} else {
+				mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
+						   DP_PWR_STATE_BANDGAP_TPLL,
+						   DP_PWR_STATE_MASK);
+				mtk_dp_power_disable(mtk_dp);
+				return ret;
+			}
+		}
+	}
+
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 09/10] drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
                   ` (7 preceding siblings ...)
  2023-07-13  9:01 ` [PATCH v5 08/10] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-13  9:01 ` [PATCH v5 10/10] drm/mediatek: dp: Don't register HPD interrupt handler for eDP case AngeloGioacchino Del Regno
  2023-07-13  9:54 ` [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus Chen-Yu Tsai
  10 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

In order to support usecases in which the panel regulator can be
switched on and off to save power, and usecases in which the panel
regulator is off at boot, add a .wait_hpd_asserted() callback for
the AUX bus: this will make sure to wait until the panel is fully
ready after power-on before trying to communicate with it.

Also, parse the eDP display capabilities in that callback, so that
we can also avoid using the .get_edid() callback from this bridge.

Since at this point the hpd machinery is performed in the new hpd
callback and the detection and edid reading are done outside of
this driver, assign the DRM_BRIDGE_OP_{DETECT, EDID, HPD} ops only
if we're probing full DisplayPort and not eDP.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 8668ab17135d..a00bf6693b28 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1915,6 +1915,29 @@ static irqreturn_t mtk_dp_hpd_event(int hpd, void *dev)
 	return IRQ_WAKE_THREAD;
 }
 
+static int mtk_dp_wait_hpd_asserted(struct drm_dp_aux *mtk_aux, unsigned long wait_us)
+{
+	struct mtk_dp *mtk_dp = container_of(mtk_aux, struct mtk_dp, aux);
+	u32 val;
+	int ret;
+
+	ret = regmap_read_poll_timeout(mtk_dp->regs, MTK_DP_TRANS_P0_3414,
+				       val, !!(val & HPD_DB_DP_TRANS_P0_MASK),
+				       wait_us / 100, wait_us);
+	if (ret)
+		return ret;
+
+	mtk_dp->train_info.cable_plugged_in = true;
+
+	ret = mtk_dp_parse_capabilities(mtk_dp);
+	if (ret) {
+		drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static int mtk_dp_dt_parse(struct mtk_dp *mtk_dp,
 			   struct platform_device *pdev)
 {
@@ -2564,6 +2587,7 @@ static int mtk_dp_probe(struct platform_device *pdev)
 	mtk_dp->aux.name = "aux_mtk_dp";
 	mtk_dp->aux.dev = dev;
 	mtk_dp->aux.transfer = mtk_dp_aux_transfer;
+	mtk_dp->aux.wait_hpd_asserted = mtk_dp_wait_hpd_asserted;
 	drm_dp_aux_init(&mtk_dp->aux);
 
 	spin_lock_init(&mtk_dp->irq_thread_lock);
@@ -2596,11 +2620,12 @@ static int mtk_dp_probe(struct platform_device *pdev)
 
 	mtk_dp->bridge.funcs = &mtk_dp_bridge_funcs;
 	mtk_dp->bridge.of_node = dev->of_node;
-
-	mtk_dp->bridge.ops =
-		DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
 	mtk_dp->bridge.type = mtk_dp->data->bridge_type;
 
+	if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP)
+		mtk_dp->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID |
+				     DRM_BRIDGE_OP_HPD;
+
 	devm_drm_bridge_add(dev, &mtk_dp->bridge);
 
 	mtk_dp->need_debounce = true;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 10/10] drm/mediatek: dp: Don't register HPD interrupt handler for eDP case
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
                   ` (8 preceding siblings ...)
  2023-07-13  9:01 ` [PATCH v5 09/10] drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus AngeloGioacchino Del Regno
@ 2023-07-13  9:01 ` AngeloGioacchino Del Regno
  2023-07-13  9:54 ` [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus Chen-Yu Tsai
  10 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:01 UTC (permalink / raw)
  To: chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

The interrupt handler for HPD is useful only if a display is actually
supposed to be hotpluggable, as that manages the machinery to perform
cable (un)plug detection, debouncing and setup for re-training.

Since eDP panels are not supposed to be hotpluggable we can avoid
using the HPD interrupts altogether and rely on HPD polling only
for the suspend/resume case, saving us some spinlocking action and
the overhead of interrupts firing at every suspend/resume cycle,
achieving a faster (even if just slightly) display resume.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 81 ++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index a00bf6693b28..14eeb4a74191 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2175,9 +2175,11 @@ static int mtk_dp_bridge_attach(struct drm_bridge *bridge,
 
 	mtk_dp->drm_dev = bridge->dev;
 
-	irq_clear_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
-	enable_irq(mtk_dp->irq);
-	mtk_dp_hwirq_enable(mtk_dp, true);
+	if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP) {
+		irq_clear_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
+		enable_irq(mtk_dp->irq);
+		mtk_dp_hwirq_enable(mtk_dp, true);
+	}
 
 	return 0;
 
@@ -2192,8 +2194,10 @@ static void mtk_dp_bridge_detach(struct drm_bridge *bridge)
 {
 	struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
 
-	mtk_dp_hwirq_enable(mtk_dp, false);
-	disable_irq(mtk_dp->irq);
+	if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP) {
+		mtk_dp_hwirq_enable(mtk_dp, false);
+		disable_irq(mtk_dp->irq);
+	}
 	mtk_dp->drm_dev = NULL;
 	mtk_dp_poweroff(mtk_dp);
 	drm_dp_aux_unregister(&mtk_dp->aux);
@@ -2567,40 +2571,44 @@ static int mtk_dp_probe(struct platform_device *pdev)
 	mtk_dp->dev = dev;
 	mtk_dp->data = (struct mtk_dp_data *)of_device_get_match_data(dev);
 
-	mtk_dp->irq = platform_get_irq(pdev, 0);
-	if (mtk_dp->irq < 0)
-		return dev_err_probe(dev, mtk_dp->irq,
-				     "failed to request dp irq resource\n");
-
-	mtk_dp->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
-	if (IS_ERR(mtk_dp->next_bridge) &&
-	    PTR_ERR(mtk_dp->next_bridge) == -ENODEV)
-		mtk_dp->next_bridge = NULL;
-	else if (IS_ERR(mtk_dp->next_bridge))
-		return dev_err_probe(dev, PTR_ERR(mtk_dp->next_bridge),
-				     "Failed to get bridge\n");
-
 	ret = mtk_dp_dt_parse(mtk_dp, pdev);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to parse dt\n");
 
+	/*
+	 * Request the interrupt and install service routine only if we are
+	 * on full DisplayPort.
+	 * For eDP, polling the HPD instead is more convenient because we
+	 * don't expect any (un)plug events during runtime, hence we can
+	 * avoid some locking.
+	 */
+	if (mtk_dp->data->bridge_type != DRM_MODE_CONNECTOR_eDP) {
+		mtk_dp->irq = platform_get_irq(pdev, 0);
+		if (mtk_dp->irq < 0)
+			return dev_err_probe(dev, mtk_dp->irq,
+					     "failed to request dp irq resource\n");
+
+		spin_lock_init(&mtk_dp->irq_thread_lock);
+
+		irq_set_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
+		ret = devm_request_threaded_irq(dev, mtk_dp->irq, mtk_dp_hpd_event,
+						mtk_dp_hpd_event_thread,
+						IRQ_TYPE_LEVEL_HIGH, dev_name(dev),
+						mtk_dp);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to request mediatek dptx irq\n");
+
+		mtk_dp->need_debounce = true;
+		timer_setup(&mtk_dp->debounce_timer, mtk_dp_debounce_timer, 0);
+	}
+
 	mtk_dp->aux.name = "aux_mtk_dp";
 	mtk_dp->aux.dev = dev;
 	mtk_dp->aux.transfer = mtk_dp_aux_transfer;
 	mtk_dp->aux.wait_hpd_asserted = mtk_dp_wait_hpd_asserted;
 	drm_dp_aux_init(&mtk_dp->aux);
 
-	spin_lock_init(&mtk_dp->irq_thread_lock);
-
-	irq_set_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
-	ret = devm_request_threaded_irq(dev, mtk_dp->irq, mtk_dp_hpd_event,
-					mtk_dp_hpd_event_thread,
-					IRQ_TYPE_LEVEL_HIGH, dev_name(dev),
-					mtk_dp);
-	if (ret)
-		return dev_err_probe(dev, ret,
-				     "failed to request mediatek dptx irq\n");
-
 	platform_set_drvdata(pdev, mtk_dp);
 
 	if (mtk_dp->data->audio_supported) {
@@ -2628,9 +2636,6 @@ static int mtk_dp_probe(struct platform_device *pdev)
 
 	devm_drm_bridge_add(dev, &mtk_dp->bridge);
 
-	mtk_dp->need_debounce = true;
-	timer_setup(&mtk_dp->debounce_timer, mtk_dp_debounce_timer, 0);
-
 	if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP) {
 		/*
 		 * Set the data lanes to idle in case the bootloader didn't
@@ -2641,6 +2646,9 @@ static int mtk_dp_probe(struct platform_device *pdev)
 		mtk_dp_initialize_aux_settings(mtk_dp);
 		mtk_dp_power_enable(mtk_dp);
 
+		/* Disable HW interrupts: we don't need any for eDP */
+		mtk_dp_hwirq_enable(mtk_dp, false);
+
 		/*
 		 * Power on the AUX to allow reading the EDID from aux-bus:
 		 * please note that it is necessary to call power off in the
@@ -2680,7 +2688,8 @@ static int mtk_dp_remove(struct platform_device *pdev)
 
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-	del_timer_sync(&mtk_dp->debounce_timer);
+	if (mtk_dp->data->bridge_type != DRM_MODE_CONNECTOR_eDP)
+		del_timer_sync(&mtk_dp->debounce_timer);
 	drm_bridge_remove(&mtk_dp->bridge);
 	platform_device_unregister(mtk_dp->phy_dev);
 	if (mtk_dp->audio_pdev)
@@ -2695,7 +2704,8 @@ static int mtk_dp_suspend(struct device *dev)
 	struct mtk_dp *mtk_dp = dev_get_drvdata(dev);
 
 	mtk_dp_power_disable(mtk_dp);
-	mtk_dp_hwirq_enable(mtk_dp, false);
+	if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP)
+		mtk_dp_hwirq_enable(mtk_dp, false);
 	pm_runtime_put_sync(dev);
 
 	return 0;
@@ -2707,7 +2717,8 @@ static int mtk_dp_resume(struct device *dev)
 
 	pm_runtime_get_sync(dev);
 	mtk_dp_init_port(mtk_dp);
-	mtk_dp_hwirq_enable(mtk_dp, true);
+	if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP)
+		mtk_dp_hwirq_enable(mtk_dp, true);
 	mtk_dp_power_enable(mtk_dp);
 
 	return 0;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus
  2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
                   ` (9 preceding siblings ...)
  2023-07-13  9:01 ` [PATCH v5 10/10] drm/mediatek: dp: Don't register HPD interrupt handler for eDP case AngeloGioacchino Del Regno
@ 2023-07-13  9:54 ` Chen-Yu Tsai
  2023-07-13  9:57   ` AngeloGioacchino Del Regno
  10 siblings, 1 reply; 16+ messages in thread
From: Chen-Yu Tsai @ 2023-07-13  9:54 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: chunkuang.hu, nfraprado, linux-kernel, dri-devel, linux-mediatek,
	matthias.bgg, kernel, linux-arm-kernel

On Thu, Jul 13, 2023 at 5:01 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Changes in v5:
>  - Added .wait_hpd_asserted() callback for aux-bus
>  - Avoid enabling and registering HPD interrupt + handlers for
>    eDP case only (keeps HPD interrupts enabled for full DP case)
>  - Support not always-on eDP panels (boot with regulator off,
>    suspend with regulator off) for power saving in PM suspend.

This still doesn't work when the DRM driver is builtin, and the panel
driver is a module. This is still with regulator-always-on.

From what I can tell from the kernel logs, the DRM driver is not waiting
for eDP panel to probe (which sort of makes sense?), and simply uses
the default EDID. When the panel does probe, nothing triggers the DRM
driver to update its connector.

[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:32:eDP-1]
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:32:eDP-1]
status updated from unknown to connected
[drm:drm_mode_debug_printmodeline] Modeline "640x480": 60 25175 640
656 752 800 480 490 492 525 0x40 0xa
[drm:drm_mode_prune_invalid] Not using 640x480 mode: CLOCK_HIGH
[drm:drm_mode_debug_printmodeline] Modeline "800x600": 56 36000 800
824 896 1024 600 601 603 625 0x40 0x5
[drm:drm_mode_prune_invalid] Not using 800x600 mode: CLOCK_HIGH
[drm:drm_mode_debug_printmodeline] Modeline "800x600": 60 40000 800
840 968 1056 600 601 605 628 0x40 0x5
[drm:drm_mode_prune_invalid] Not using 800x600 mode: CLOCK_HIGH
[drm:drm_mode_debug_printmodeline] Modeline "848x480": 60 33750 848
864 976 1088 480 486 494 517 0x40 0x5
[drm:drm_mode_prune_invalid] Not using 848x480 mode: CLOCK_HIGH
[drm:drm_mode_debug_printmodeline] Modeline "1024x768": 60 65000 1024
1048 1184 1344 768 771 777 806 0x40 0xa
[drm:drm_mode_prune_invalid] Not using 1024x768 mode: CLOCK_HIGH
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1]
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1]
status updated from unknown to disconnected
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1] disconnected
[drm:drm_client_modeset_probe] No connectors reported connected with modes
[drm:drm_client_modeset_probe] connector 32 enabled? yes
[drm:drm_client_modeset_probe] connector 34 enabled? no
[drm:drm_client_firmware_config.constprop.0.isra.0] Not using firmware
configuration
[drm:drm_client_modeset_probe] looking for cmdline mode on connector 32
[drm:drm_client_modeset_probe] looking for preferred mode on connector 32 0
[drm:drm_client_modeset_probe] found mode none
[drm:drm_client_modeset_probe] picking CRTCs for 4096x4096 config
mediatek-drm mediatek-drm.12.auto:
[drm:__drm_fb_helper_initial_config_and_unlock] test CRTC 0 primary
plane
mediatek-drm mediatek-drm.12.auto: [drm] Cannot find any crtc or sizes
mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp:
0x00000 AUX -> (ret=  1) 14
mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp:
0x00000 AUX -> (ret= 15) 14 0a 84 41 00 00 01 80 02 00 00 00 0f 09 80
mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp:
0x00000 AUX -> (ret=  1) 14
mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp:
0x02200 AUX -> (ret= 15) 14 0a 84 41 00 00 01 80 02 00 00 00 0f 01 80
mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_read_dpcd_caps]
aux_mtk_dp: Base DPCD: 14 0a 84 41 00 00 01 80 02 00 00 00 0f 09 80
mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_read_dpcd_caps]
aux_mtk_dp: DPCD: 14 0a 84 41 00 00 01 80 02 00 00 00 0f 01 80
mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp:
0x00000 AUX -> (ret=  1) 14
mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp:
0x00021 AUX -> (ret=  1) 00
panel-simple-dp-aux aux-1c500000.edp-tx: Detected BOE NE135FBM-N41 v8.1 (0x095f)

If the panel is also built-in, then the eDP panel probe happens between
the drm driver adding components and binding to them, and everything seems
to work OK.


ChenYu

> Changes in v4:
>  - Set data lanes to idle to prevent stalls if bootloader didn't
>    properly close the eDP port
>  - Now using the .done_probing() callback for AUX bus to prevent
>    probe deferral loops in case the panel-edp driver is a module
>    as previously seen with another bridge driver (ANX7625) on
>    some other SoCs (MT8192 and others)
>  - Rebased over next-20230706
>  - Dropped Chen-Yu's T-b tag on last patch as some logic changed
>    (before, I wasn't using the .done_probing() callback).
>
> Changes in v3:
>  - Added DPTX AUX block initialization before trying to communicate
>    to stop relying on the bootloader keeping it initialized before
>    booting Linux.
>  - Fixed commit description for patch [09/09] and removed commented
>    out code (that slipped from dev phase.. sorry!).
>
> This series adds "real" support for eDP in the mtk-dp DisplayPort driver.
>
> Explaining the "real":
> Before this change, the DisplayPort driver did support eDP to some
> extent, but it was treating it entirely like a regular DP interface
> which is partially fine, after all, embedded DisplayPort *is* actually
> DisplayPort, but there might be some differences to account for... and
> this is for both small performance improvements and, more importantly,
> for correct functionality in some systems.
>
> Functionality first:
>
> One of the common differences found in various boards implementing eDP
> and machines using an eDP panel is that many times the HPD line is not
> connected. This *must* be accounted for: at startup, this specific IP
> will raise a HPD interrupt (which should maybe be ignored... as it does
> not appear to be a "real" event...) that will make the eDP panel to be
> detected and to actually work but, after a suspend-resume cycle, there
> will be no HPD interrupt (as there's no HPD line in my case!) producing
> a functionality issue - specifically, the DP Link Training fails because
> the panel doesn't get powered up, then it stays black and won't work
> until rebooting the machine (or removing and reinserting the module I
> think, but I haven't tried that).
>
> Now for.. both:
> eDP panels are *e*DP because they are *not* removable (in the sense that
> you can't unplug the cable without disassembling the machine, in which
> case, the machine shall be powered down..!): this (correct) assumption
> makes us able to solve some issues and to also gain a little performance
> during PM operations.
>
> What was done here is:
>  - Caching the EDID if the panel is eDP: we're always going to read the
>    same data everytime, so we can just cache that (as it's small enough)
>    shortening PM resume times for the eDP driver instance;
>  - Always return connector_status_connected if it's eDP: non-removable
>    means connector_status_disconnected can't happen during runtime...
>    this also saves us some time and even power, as we won't have to
>    perform yet another power cycle of the HW;
>  - Added aux-bus support!
>    This makes us able to rely on panel autodetection from the EDID,
>    avoiding to add more and more panel timings to panel-edp and, even
>    better, allowing to use one panel node in devicetrees for multiple
>    variants of the same machine since, at that point, it's not important
>    to "preventively know" what panel we have (eh, it's autodetected...!).
>
> This was tested on a MT8195 Cherry Tomato Chromebook (panel-edp on aux-bus)
>
>
> P.S.: For your own testing commodity, here's a reference devicetree:
>
> pp3300_disp_x: regulator-pp3300-disp-x {
>         compatible = "regulator-fixed";
>         regulator-name = "pp3300_disp_x";
>         regulator-min-microvolt = <3300000>;
>         regulator-max-microvolt = <3300000>;
>         enable-active-high;
>         gpio = <&pio 55 GPIO_ACTIVE_HIGH>;
>         pinctrl-names = "default";
>         pinctrl-0 = <&panel_fixed_pins>;
> };
>
> &edp_tx {
>         status = "okay";
>
>         pinctrl-names = "default";
>         pinctrl-0 = <&edptx_pins_default>;
>
>         ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
>                 port@0 {
>                         reg = <0>;
>                         edp_in: endpoint {
>                                 remote-endpoint = <&dp_intf0_out>;
>                         };
>                 };
>
>                 port@1 {
>                         reg = <1>;
>                         edp_out: endpoint {
>                                 data-lanes = <0 1 2 3>;
>                                 remote-endpoint = <&panel_in>;
>                         };
>                 };
>         };
>
>         aux-bus {
>                 panel: panel {
>                         compatible = "edp-panel";
>                         power-supply = <&pp3300_disp_x>;
>                         backlight = <&backlight_lcd0>;
>                         port {
>                                 panel_in: endpoint {
>                                         remote-endpoint = <&edp_out>;
>                                 };
>                         };
>                 };
>         };
> };
>
> AngeloGioacchino Del Regno (10):
>   drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
>   drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer()
>   drm/mediatek: dp: Use devm variant of drm_bridge_add()
>   drm/mediatek: dp: Move AUX_P0 setting to
>     mtk_dp_initialize_aux_settings()
>   drm/mediatek: dp: Enable event interrupt only when bridge attached
>   drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled
>   drm/mediatek: dp: Move PHY registration to new function
>   drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
>   drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus
>   drm/mediatek: dp: Don't register HPD interrupt handler for eDP case
>
>  drivers/gpu/drm/mediatek/Kconfig  |   1 +
>  drivers/gpu/drm/mediatek/mtk_dp.c | 335 ++++++++++++++++++++----------
>  2 files changed, 224 insertions(+), 112 deletions(-)
>
> --
> 2.40.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus
  2023-07-13  9:54 ` [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus Chen-Yu Tsai
@ 2023-07-13  9:57   ` AngeloGioacchino Del Regno
  2023-07-13 10:03     ` Chen-Yu Tsai
  0 siblings, 1 reply; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-13  9:57 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: chunkuang.hu, nfraprado, linux-kernel, dri-devel, linux-mediatek,
	matthias.bgg, kernel, linux-arm-kernel

Il 13/07/23 11:54, Chen-Yu Tsai ha scritto:
> On Thu, Jul 13, 2023 at 5:01 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Changes in v5:
>>   - Added .wait_hpd_asserted() callback for aux-bus
>>   - Avoid enabling and registering HPD interrupt + handlers for
>>     eDP case only (keeps HPD interrupts enabled for full DP case)
>>   - Support not always-on eDP panels (boot with regulator off,
>>     suspend with regulator off) for power saving in PM suspend.
> 
> This still doesn't work when the DRM driver is builtin, and the panel
> driver is a module. This is still with regulator-always-on.
> 
>  From what I can tell from the kernel logs, the DRM driver is not waiting
> for eDP panel to probe (which sort of makes sense?), and simply uses
> the default EDID. When the panel does probe, nothing triggers the DRM
> driver to update its connector.
> 
> [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:32:eDP-1]
> [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:32:eDP-1]
> status updated from unknown to connected
> [drm:drm_mode_debug_printmodeline] Modeline "640x480": 60 25175 640
> 656 752 800 480 490 492 525 0x40 0xa
> [drm:drm_mode_prune_invalid] Not using 640x480 mode: CLOCK_HIGH
> [drm:drm_mode_debug_printmodeline] Modeline "800x600": 56 36000 800
> 824 896 1024 600 601 603 625 0x40 0x5
> [drm:drm_mode_prune_invalid] Not using 800x600 mode: CLOCK_HIGH
> [drm:drm_mode_debug_printmodeline] Modeline "800x600": 60 40000 800
> 840 968 1056 600 601 605 628 0x40 0x5
> [drm:drm_mode_prune_invalid] Not using 800x600 mode: CLOCK_HIGH
> [drm:drm_mode_debug_printmodeline] Modeline "848x480": 60 33750 848
> 864 976 1088 480 486 494 517 0x40 0x5
> [drm:drm_mode_prune_invalid] Not using 848x480 mode: CLOCK_HIGH
> [drm:drm_mode_debug_printmodeline] Modeline "1024x768": 60 65000 1024
> 1048 1184 1344 768 771 777 806 0x40 0xa
> [drm:drm_mode_prune_invalid] Not using 1024x768 mode: CLOCK_HIGH
> [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1]
> [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1]
> status updated from unknown to disconnected
> [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1] disconnected
> [drm:drm_client_modeset_probe] No connectors reported connected with modes
> [drm:drm_client_modeset_probe] connector 32 enabled? yes
> [drm:drm_client_modeset_probe] connector 34 enabled? no
> [drm:drm_client_firmware_config.constprop.0.isra.0] Not using firmware
> configuration
> [drm:drm_client_modeset_probe] looking for cmdline mode on connector 32
> [drm:drm_client_modeset_probe] looking for preferred mode on connector 32 0
> [drm:drm_client_modeset_probe] found mode none
> [drm:drm_client_modeset_probe] picking CRTCs for 4096x4096 config
> mediatek-drm mediatek-drm.12.auto:
> [drm:__drm_fb_helper_initial_config_and_unlock] test CRTC 0 primary
> plane
> mediatek-drm mediatek-drm.12.auto: [drm] Cannot find any crtc or sizes
> mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp:
> 0x00000 AUX -> (ret=  1) 14
> mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp:
> 0x00000 AUX -> (ret= 15) 14 0a 84 41 00 00 01 80 02 00 00 00 0f 09 80
> mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp:
> 0x00000 AUX -> (ret=  1) 14
> mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp:
> 0x02200 AUX -> (ret= 15) 14 0a 84 41 00 00 01 80 02 00 00 00 0f 01 80
> mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_read_dpcd_caps]
> aux_mtk_dp: Base DPCD: 14 0a 84 41 00 00 01 80 02 00 00 00 0f 09 80
> mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_read_dpcd_caps]
> aux_mtk_dp: DPCD: 14 0a 84 41 00 00 01 80 02 00 00 00 0f 01 80
> mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp:
> 0x00000 AUX -> (ret=  1) 14
> mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp:
> 0x00021 AUX -> (ret=  1) 00
> panel-simple-dp-aux aux-1c500000.edp-tx: Detected BOE NE135FBM-N41 v8.1 (0x095f)
> 
> If the panel is also built-in, then the eDP panel probe happens between
> the drm driver adding components and binding to them, and everything seems
> to work OK.
> 

Argh. I actually forgot to test that case. Sorry about that.

Anyway, you don't need regulator-always-on anymore, nor regulator-boot-on...
...I'll recheck with panel-edp as module and fix.

Cheers,
Angelo

> 
> ChenYu
> 
>> Changes in v4:
>>   - Set data lanes to idle to prevent stalls if bootloader didn't
>>     properly close the eDP port
>>   - Now using the .done_probing() callback for AUX bus to prevent
>>     probe deferral loops in case the panel-edp driver is a module
>>     as previously seen with another bridge driver (ANX7625) on
>>     some other SoCs (MT8192 and others)
>>   - Rebased over next-20230706
>>   - Dropped Chen-Yu's T-b tag on last patch as some logic changed
>>     (before, I wasn't using the .done_probing() callback).
>>
>> Changes in v3:
>>   - Added DPTX AUX block initialization before trying to communicate
>>     to stop relying on the bootloader keeping it initialized before
>>     booting Linux.
>>   - Fixed commit description for patch [09/09] and removed commented
>>     out code (that slipped from dev phase.. sorry!).
>>
>> This series adds "real" support for eDP in the mtk-dp DisplayPort driver.
>>
>> Explaining the "real":
>> Before this change, the DisplayPort driver did support eDP to some
>> extent, but it was treating it entirely like a regular DP interface
>> which is partially fine, after all, embedded DisplayPort *is* actually
>> DisplayPort, but there might be some differences to account for... and
>> this is for both small performance improvements and, more importantly,
>> for correct functionality in some systems.
>>
>> Functionality first:
>>
>> One of the common differences found in various boards implementing eDP
>> and machines using an eDP panel is that many times the HPD line is not
>> connected. This *must* be accounted for: at startup, this specific IP
>> will raise a HPD interrupt (which should maybe be ignored... as it does
>> not appear to be a "real" event...) that will make the eDP panel to be
>> detected and to actually work but, after a suspend-resume cycle, there
>> will be no HPD interrupt (as there's no HPD line in my case!) producing
>> a functionality issue - specifically, the DP Link Training fails because
>> the panel doesn't get powered up, then it stays black and won't work
>> until rebooting the machine (or removing and reinserting the module I
>> think, but I haven't tried that).
>>
>> Now for.. both:
>> eDP panels are *e*DP because they are *not* removable (in the sense that
>> you can't unplug the cable without disassembling the machine, in which
>> case, the machine shall be powered down..!): this (correct) assumption
>> makes us able to solve some issues and to also gain a little performance
>> during PM operations.
>>
>> What was done here is:
>>   - Caching the EDID if the panel is eDP: we're always going to read the
>>     same data everytime, so we can just cache that (as it's small enough)
>>     shortening PM resume times for the eDP driver instance;
>>   - Always return connector_status_connected if it's eDP: non-removable
>>     means connector_status_disconnected can't happen during runtime...
>>     this also saves us some time and even power, as we won't have to
>>     perform yet another power cycle of the HW;
>>   - Added aux-bus support!
>>     This makes us able to rely on panel autodetection from the EDID,
>>     avoiding to add more and more panel timings to panel-edp and, even
>>     better, allowing to use one panel node in devicetrees for multiple
>>     variants of the same machine since, at that point, it's not important
>>     to "preventively know" what panel we have (eh, it's autodetected...!).
>>
>> This was tested on a MT8195 Cherry Tomato Chromebook (panel-edp on aux-bus)
>>
>>
>> P.S.: For your own testing commodity, here's a reference devicetree:
>>
>> pp3300_disp_x: regulator-pp3300-disp-x {
>>          compatible = "regulator-fixed";
>>          regulator-name = "pp3300_disp_x";
>>          regulator-min-microvolt = <3300000>;
>>          regulator-max-microvolt = <3300000>;
>>          enable-active-high;
>>          gpio = <&pio 55 GPIO_ACTIVE_HIGH>;
>>          pinctrl-names = "default";
>>          pinctrl-0 = <&panel_fixed_pins>;
>> };
>>
>> &edp_tx {
>>          status = "okay";
>>
>>          pinctrl-names = "default";
>>          pinctrl-0 = <&edptx_pins_default>;
>>
>>          ports {
>>                  #address-cells = <1>;
>>                  #size-cells = <0>;
>>
>>                  port@0 {
>>                          reg = <0>;
>>                          edp_in: endpoint {
>>                                  remote-endpoint = <&dp_intf0_out>;
>>                          };
>>                  };
>>
>>                  port@1 {
>>                          reg = <1>;
>>                          edp_out: endpoint {
>>                                  data-lanes = <0 1 2 3>;
>>                                  remote-endpoint = <&panel_in>;
>>                          };
>>                  };
>>          };
>>
>>          aux-bus {
>>                  panel: panel {
>>                          compatible = "edp-panel";
>>                          power-supply = <&pp3300_disp_x>;
>>                          backlight = <&backlight_lcd0>;
>>                          port {
>>                                  panel_in: endpoint {
>>                                          remote-endpoint = <&edp_out>;
>>                                  };
>>                          };
>>                  };
>>          };
>> };
>>
>> AngeloGioacchino Del Regno (10):
>>    drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
>>    drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer()
>>    drm/mediatek: dp: Use devm variant of drm_bridge_add()
>>    drm/mediatek: dp: Move AUX_P0 setting to
>>      mtk_dp_initialize_aux_settings()
>>    drm/mediatek: dp: Enable event interrupt only when bridge attached
>>    drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled
>>    drm/mediatek: dp: Move PHY registration to new function
>>    drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
>>    drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus
>>    drm/mediatek: dp: Don't register HPD interrupt handler for eDP case
>>
>>   drivers/gpu/drm/mediatek/Kconfig  |   1 +
>>   drivers/gpu/drm/mediatek/mtk_dp.c | 335 ++++++++++++++++++++----------
>>   2 files changed, 224 insertions(+), 112 deletions(-)
>>
>> --
>> 2.40.1
>>
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus
  2023-07-13  9:57   ` AngeloGioacchino Del Regno
@ 2023-07-13 10:03     ` Chen-Yu Tsai
  0 siblings, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2023-07-13 10:03 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: chunkuang.hu, nfraprado, linux-kernel, dri-devel, linux-mediatek,
	matthias.bgg, kernel, linux-arm-kernel

On Thu, Jul 13, 2023 at 5:57 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 13/07/23 11:54, Chen-Yu Tsai ha scritto:
> > On Thu, Jul 13, 2023 at 5:01 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Changes in v5:
> >>   - Added .wait_hpd_asserted() callback for aux-bus
> >>   - Avoid enabling and registering HPD interrupt + handlers for
> >>     eDP case only (keeps HPD interrupts enabled for full DP case)
> >>   - Support not always-on eDP panels (boot with regulator off,
> >>     suspend with regulator off) for power saving in PM suspend.
> >
> > This still doesn't work when the DRM driver is builtin, and the panel
> > driver is a module. This is still with regulator-always-on.
> >
> >  From what I can tell from the kernel logs, the DRM driver is not waiting
> > for eDP panel to probe (which sort of makes sense?), and simply uses
> > the default EDID. When the panel does probe, nothing triggers the DRM
> > driver to update its connector.
> >
> > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:32:eDP-1]
> > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:32:eDP-1]
> > status updated from unknown to connected
> > [drm:drm_mode_debug_printmodeline] Modeline "640x480": 60 25175 640
> > 656 752 800 480 490 492 525 0x40 0xa
> > [drm:drm_mode_prune_invalid] Not using 640x480 mode: CLOCK_HIGH
> > [drm:drm_mode_debug_printmodeline] Modeline "800x600": 56 36000 800
> > 824 896 1024 600 601 603 625 0x40 0x5
> > [drm:drm_mode_prune_invalid] Not using 800x600 mode: CLOCK_HIGH
> > [drm:drm_mode_debug_printmodeline] Modeline "800x600": 60 40000 800
> > 840 968 1056 600 601 605 628 0x40 0x5
> > [drm:drm_mode_prune_invalid] Not using 800x600 mode: CLOCK_HIGH
> > [drm:drm_mode_debug_printmodeline] Modeline "848x480": 60 33750 848
> > 864 976 1088 480 486 494 517 0x40 0x5
> > [drm:drm_mode_prune_invalid] Not using 848x480 mode: CLOCK_HIGH
> > [drm:drm_mode_debug_printmodeline] Modeline "1024x768": 60 65000 1024
> > 1048 1184 1344 768 771 777 806 0x40 0xa
> > [drm:drm_mode_prune_invalid] Not using 1024x768 mode: CLOCK_HIGH
> > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1]
> > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1]
> > status updated from unknown to disconnected
> > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1] disconnected
> > [drm:drm_client_modeset_probe] No connectors reported connected with modes
> > [drm:drm_client_modeset_probe] connector 32 enabled? yes
> > [drm:drm_client_modeset_probe] connector 34 enabled? no
> > [drm:drm_client_firmware_config.constprop.0.isra.0] Not using firmware
> > configuration
> > [drm:drm_client_modeset_probe] looking for cmdline mode on connector 32
> > [drm:drm_client_modeset_probe] looking for preferred mode on connector 32 0
> > [drm:drm_client_modeset_probe] found mode none
> > [drm:drm_client_modeset_probe] picking CRTCs for 4096x4096 config
> > mediatek-drm mediatek-drm.12.auto:
> > [drm:__drm_fb_helper_initial_config_and_unlock] test CRTC 0 primary
> > plane
> > mediatek-drm mediatek-drm.12.auto: [drm] Cannot find any crtc or sizes
> > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp:
> > 0x00000 AUX -> (ret=  1) 14
> > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp:
> > 0x00000 AUX -> (ret= 15) 14 0a 84 41 00 00 01 80 02 00 00 00 0f 09 80
> > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp:
> > 0x00000 AUX -> (ret=  1) 14
> > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp:
> > 0x02200 AUX -> (ret= 15) 14 0a 84 41 00 00 01 80 02 00 00 00 0f 01 80
> > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_read_dpcd_caps]
> > aux_mtk_dp: Base DPCD: 14 0a 84 41 00 00 01 80 02 00 00 00 0f 09 80
> > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_read_dpcd_caps]
> > aux_mtk_dp: DPCD: 14 0a 84 41 00 00 01 80 02 00 00 00 0f 01 80
> > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp:
> > 0x00000 AUX -> (ret=  1) 14
> > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp:
> > 0x00021 AUX -> (ret=  1) 00
> > panel-simple-dp-aux aux-1c500000.edp-tx: Detected BOE NE135FBM-N41 v8.1 (0x095f)
> >
> > If the panel is also built-in, then the eDP panel probe happens between
> > the drm driver adding components and binding to them, and everything seems
> > to work OK.
> >
>
> Argh. I actually forgot to test that case. Sorry about that.
>
> Anyway, you don't need regulator-always-on anymore, nor regulator-boot-on...

Confirmed this case works now.

> ...I'll recheck with panel-edp as module and fix.

With the module case, I also get an devlink error:

mediatek-drm-dp 1c500000.edp-tx: Failed to create device link (0x180)
with regulator-pp3300-disp-x

which doesn't happen with the builtin case. Not sure if it gets a retry
or anything later.

ChenYu

> Cheers,
> Angelo
>
> >
> > ChenYu
> >
> >> Changes in v4:
> >>   - Set data lanes to idle to prevent stalls if bootloader didn't
> >>     properly close the eDP port
> >>   - Now using the .done_probing() callback for AUX bus to prevent
> >>     probe deferral loops in case the panel-edp driver is a module
> >>     as previously seen with another bridge driver (ANX7625) on
> >>     some other SoCs (MT8192 and others)
> >>   - Rebased over next-20230706
> >>   - Dropped Chen-Yu's T-b tag on last patch as some logic changed
> >>     (before, I wasn't using the .done_probing() callback).
> >>
> >> Changes in v3:
> >>   - Added DPTX AUX block initialization before trying to communicate
> >>     to stop relying on the bootloader keeping it initialized before
> >>     booting Linux.
> >>   - Fixed commit description for patch [09/09] and removed commented
> >>     out code (that slipped from dev phase.. sorry!).
> >>
> >> This series adds "real" support for eDP in the mtk-dp DisplayPort driver.
> >>
> >> Explaining the "real":
> >> Before this change, the DisplayPort driver did support eDP to some
> >> extent, but it was treating it entirely like a regular DP interface
> >> which is partially fine, after all, embedded DisplayPort *is* actually
> >> DisplayPort, but there might be some differences to account for... and
> >> this is for both small performance improvements and, more importantly,
> >> for correct functionality in some systems.
> >>
> >> Functionality first:
> >>
> >> One of the common differences found in various boards implementing eDP
> >> and machines using an eDP panel is that many times the HPD line is not
> >> connected. This *must* be accounted for: at startup, this specific IP
> >> will raise a HPD interrupt (which should maybe be ignored... as it does
> >> not appear to be a "real" event...) that will make the eDP panel to be
> >> detected and to actually work but, after a suspend-resume cycle, there
> >> will be no HPD interrupt (as there's no HPD line in my case!) producing
> >> a functionality issue - specifically, the DP Link Training fails because
> >> the panel doesn't get powered up, then it stays black and won't work
> >> until rebooting the machine (or removing and reinserting the module I
> >> think, but I haven't tried that).
> >>
> >> Now for.. both:
> >> eDP panels are *e*DP because they are *not* removable (in the sense that
> >> you can't unplug the cable without disassembling the machine, in which
> >> case, the machine shall be powered down..!): this (correct) assumption
> >> makes us able to solve some issues and to also gain a little performance
> >> during PM operations.
> >>
> >> What was done here is:
> >>   - Caching the EDID if the panel is eDP: we're always going to read the
> >>     same data everytime, so we can just cache that (as it's small enough)
> >>     shortening PM resume times for the eDP driver instance;
> >>   - Always return connector_status_connected if it's eDP: non-removable
> >>     means connector_status_disconnected can't happen during runtime...
> >>     this also saves us some time and even power, as we won't have to
> >>     perform yet another power cycle of the HW;
> >>   - Added aux-bus support!
> >>     This makes us able to rely on panel autodetection from the EDID,
> >>     avoiding to add more and more panel timings to panel-edp and, even
> >>     better, allowing to use one panel node in devicetrees for multiple
> >>     variants of the same machine since, at that point, it's not important
> >>     to "preventively know" what panel we have (eh, it's autodetected...!).
> >>
> >> This was tested on a MT8195 Cherry Tomato Chromebook (panel-edp on aux-bus)
> >>
> >>
> >> P.S.: For your own testing commodity, here's a reference devicetree:
> >>
> >> pp3300_disp_x: regulator-pp3300-disp-x {
> >>          compatible = "regulator-fixed";
> >>          regulator-name = "pp3300_disp_x";
> >>          regulator-min-microvolt = <3300000>;
> >>          regulator-max-microvolt = <3300000>;
> >>          enable-active-high;
> >>          gpio = <&pio 55 GPIO_ACTIVE_HIGH>;
> >>          pinctrl-names = "default";
> >>          pinctrl-0 = <&panel_fixed_pins>;
> >> };
> >>
> >> &edp_tx {
> >>          status = "okay";
> >>
> >>          pinctrl-names = "default";
> >>          pinctrl-0 = <&edptx_pins_default>;
> >>
> >>          ports {
> >>                  #address-cells = <1>;
> >>                  #size-cells = <0>;
> >>
> >>                  port@0 {
> >>                          reg = <0>;
> >>                          edp_in: endpoint {
> >>                                  remote-endpoint = <&dp_intf0_out>;
> >>                          };
> >>                  };
> >>
> >>                  port@1 {
> >>                          reg = <1>;
> >>                          edp_out: endpoint {
> >>                                  data-lanes = <0 1 2 3>;
> >>                                  remote-endpoint = <&panel_in>;
> >>                          };
> >>                  };
> >>          };
> >>
> >>          aux-bus {
> >>                  panel: panel {
> >>                          compatible = "edp-panel";
> >>                          power-supply = <&pp3300_disp_x>;
> >>                          backlight = <&backlight_lcd0>;
> >>                          port {
> >>                                  panel_in: endpoint {
> >>                                          remote-endpoint = <&edp_out>;
> >>                                  };
> >>                          };
> >>                  };
> >>          };
> >> };
> >>
> >> AngeloGioacchino Del Regno (10):
> >>    drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
> >>    drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer()
> >>    drm/mediatek: dp: Use devm variant of drm_bridge_add()
> >>    drm/mediatek: dp: Move AUX_P0 setting to
> >>      mtk_dp_initialize_aux_settings()
> >>    drm/mediatek: dp: Enable event interrupt only when bridge attached
> >>    drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled
> >>    drm/mediatek: dp: Move PHY registration to new function
> >>    drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
> >>    drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus
> >>    drm/mediatek: dp: Don't register HPD interrupt handler for eDP case
> >>
> >>   drivers/gpu/drm/mediatek/Kconfig  |   1 +
> >>   drivers/gpu/drm/mediatek/mtk_dp.c | 335 ++++++++++++++++++++----------
> >>   2 files changed, 224 insertions(+), 112 deletions(-)
> >>
> >> --
> >> 2.40.1
> >>
> >
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
  2023-07-13  9:01 ` [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function AngeloGioacchino Del Regno
@ 2023-07-18  7:40   ` CK Hu (胡俊光)
  2023-07-18  7:41     ` Chen-Yu Tsai
  0 siblings, 1 reply; 16+ messages in thread
From: CK Hu (胡俊光) @ 2023-07-18  7:40 UTC (permalink / raw)
  To: angelogioacchino.delregno, chunkuang.hu
  Cc: nfraprado, linux-kernel, dri-devel, linux-mediatek, wenst,
	matthias.bgg, kernel, linux-arm-kernel

[-- Attachment #1: Type: text/html, Size: 9624 bytes --]

[-- Attachment #2: Type: text/plain, Size: 5698 bytes --]

Hi, Angelo:

On Thu, 2023-07-13 at 11:01 +0200, AngeloGioacchino Del Regno wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Everytime we run bridge detection and/or EDID read we run a poweron
> and poweroff sequence for both the AUX and the panel; moreover, this
> is also done when enabling the bridge in the .atomic_enable()
> callback.
> 
> Move this power on/off sequence to a new mtk_dp_aux_panel_poweron()
> function as to commonize it.
> Note that, before this commit, in mtk_dp_bridge_atomic_enable() only
> the AUX was getting powered on but the panel was left powered off if
> the DP cable wasn't plugged in while now we unconditionally send a D0
> request and this is done for two reasons:
>  - First, whether this request fails or not, it takes the same time
>    and anyway the DP hardware won't produce any error (or, if it
>    does, it's ignorable because it won't block further commands)
>  - Second, training the link between a sleeping/standby/unpowered
>    display makes little sense.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>  drivers/gpu/drm/mediatek/mtk_dp.c | 76 ++++++++++++-----------------
> --
>  1 file changed, 30 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 64eee77452c0..8b7ded1746f3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -1251,6 +1251,29 @@ static void mtk_dp_audio_mute(struct mtk_dp
> *mtk_dp, bool mute)
>  			   val[2], AU_TS_CFG_DP_ENC0_P0_MASK);
>  }
>  
> +static void mtk_dp_aux_panel_poweron(struct mtk_dp *mtk_dp, bool
> pwron)
> +{
> +	if (pwron) {
> +		/* power on aux */
> +		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> +				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
> +				   DP_PWR_STATE_MASK);
> +
> +		/* power on panel */
> +		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
> +		usleep_range(2000, 5000);
> +	} else {
> +		/* power off panel */
> +		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D3);
> +		usleep_range(2000, 3000);
> +
> +		/* power off aux */
> +		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> +				   DP_PWR_STATE_BANDGAP_TPLL,
> +				   DP_PWR_STATE_MASK);
> +	}
> +}
> +
>  static void mtk_dp_power_enable(struct mtk_dp *mtk_dp)
>  {
>  	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_RESET_AND_PROBE,
> @@ -1936,16 +1959,9 @@ static enum drm_connector_status
> mtk_dp_bdg_detect(struct drm_bridge *bridge)
>  	if (!mtk_dp->train_info.cable_plugged_in)
>  		return ret;
>  
> -	if (!enabled) {
> -		/* power on aux */
> -		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
> -				   DP_PWR_STATE_MASK);
> +	if (!enabled)
> +		mtk_dp_aux_panel_poweron(mtk_dp, true);
>  
> -		/* power on panel */
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
> -		usleep_range(2000, 5000);
> -	}
>  	/*
>  	 * Some dongles still source HPD when they do not connect to
> any
>  	 * sink device. To avoid this, we need to read the sink count
> @@ -1957,16 +1973,8 @@ static enum drm_connector_status
> mtk_dp_bdg_detect(struct drm_bridge *bridge)
>  	if (DP_GET_SINK_COUNT(sink_count))
>  		ret = connector_status_connected;
>  
> -	if (!enabled) {
> -		/* power off panel */
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D3);
> -		usleep_range(2000, 3000);
> -
> -		/* power off aux */
> -		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -				   DP_PWR_STATE_BANDGAP_TPLL,
> -				   DP_PWR_STATE_MASK);
> -	}
> +	if (!enabled)
> +		mtk_dp_aux_panel_poweron(mtk_dp, false);
>  
>  	return ret;
>  }
> @@ -1982,15 +1990,7 @@ static struct edid *mtk_dp_get_edid(struct
> drm_bridge *bridge,
>  
>  	if (!enabled) {
>  		drm_atomic_bridge_chain_pre_enable(bridge, connector-
> >state->state);
> -
> -		/* power on aux */
> -		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -				   DP_PWR_STATE_BANDGAP_TPLL_LANE,
> -				   DP_PWR_STATE_MASK);
> -
> -		/* power on panel */
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
> -		usleep_range(2000, 5000);
> +		mtk_dp_aux_panel_poweron(mtk_dp, true);
>  	}
>  
>  	new_edid = drm_get_edid(connector, &mtk_dp->aux.ddc);
> @@ -2010,15 +2010,7 @@ static struct edid *mtk_dp_get_edid(struct
> drm_bridge *bridge,
>  	}
>  
>  	if (!enabled) {
> -		/* power off panel */
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D3);
> -		usleep_range(2000, 3000);
> -
> -		/* power off aux */
> -		mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -				   DP_PWR_STATE_BANDGAP_TPLL,
> -				   DP_PWR_STATE_MASK);
> -
> +		mtk_dp_aux_panel_poweron(mtk_dp, false);
>  		drm_atomic_bridge_chain_post_disable(bridge, connector-
> >state->state);
>  	}
>  
> @@ -2178,15 +2170,7 @@ static void mtk_dp_bridge_atomic_enable(struct
> drm_bridge *bridge,
>  		return;
>  	}
>  
> -	/* power on aux */
> -	mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> -			   DP_PWR_STATE_BANDGAP_TPLL_LANE,
> -			   DP_PWR_STATE_MASK);
> -
> -	if (mtk_dp->train_info.cable_plugged_in) {
> -		drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
> -		usleep_range(2000, 5000);
> -	}
> +	mtk_dp_aux_panel_poweron(mtk_dp, true);
>  
>  	/* Training */
>  	ret = mtk_dp_training(mtk_dp);
> -- 
> 2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function
  2023-07-18  7:40   ` CK Hu (胡俊光)
@ 2023-07-18  7:41     ` Chen-Yu Tsai
  0 siblings, 0 replies; 16+ messages in thread
From: Chen-Yu Tsai @ 2023-07-18  7:41 UTC (permalink / raw)
  To: CK Hu (胡俊光)
  Cc: chunkuang.hu, nfraprado, linux-kernel, dri-devel, linux-mediatek,
	matthias.bgg, kernel, linux-arm-kernel,
	angelogioacchino.delregno

Hi CK,

On Tue, Jul 18, 2023 at 3:40 PM CK Hu (胡俊光) <ck.hu@mediatek.com> wrote:
>
> Hi, Angelo:
>
> On Thu, 2023-07-13 at 11:01 +0200, AngeloGioacchino Del Regno wrote:
> >
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >  Everytime we run bridge detection and/or EDID read we run a poweron
> > and poweroff sequence for both the AUX and the panel; moreover, this
> > is also done when enabling the bridge in the .atomic_enable()
> > callback.
> >
> > Move this power on/off sequence to a new mtk_dp_aux_panel_poweron()
> > function as to commonize it.
> > Note that, before this commit, in mtk_dp_bridge_atomic_enable() only
> > the AUX was getting powered on but the panel was left powered off if
> > the DP cable wasn't plugged in while now we unconditionally send a D0
> > request and this is done for two reasons:
> >  - First, whether this request fails or not, it takes the same time
> >    and anyway the DP hardware won't produce any error (or, if it
> >    does, it's ignorable because it won't block further commands)
> >  - Second, training the link between a sleeping/standby/unpowered
> >    display makes little sense.
>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>

There is a v6 already.

> >
> > Signed-off-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dp.c | 76 ++++++++++++-----------------
> > --
> >  1 file changed, 30 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> > b/drivers/gpu/drm/mediatek/mtk_dp.c
> > index 64eee77452c0..8b7ded1746f3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> > @@ -1251,6 +1251,29 @@ static void mtk_dp_audio_mute(struct mtk_dp
> > *mtk_dp, bool mute)
> >                          val[2], AU_TS_CFG_DP_ENC0_P0_MASK);
> >  }
> >
> > +static void mtk_dp_aux_panel_poweron(struct mtk_dp *mtk_dp, bool
> > pwron)
> > +{
> > +     if (pwron) {
> > +             /* power on aux */
> > +             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > +                                DP_PWR_STATE_BANDGAP_TPLL_LANE,
> > +                                DP_PWR_STATE_MASK);
> > +
> > +             /* power on panel */
> > +             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
> > +             usleep_range(2000, 5000);
> > +     } else {
> > +             /* power off panel */
> > +             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D3);
> > +             usleep_range(2000, 3000);
> > +
> > +             /* power off aux */
> > +             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > +                                DP_PWR_STATE_BANDGAP_TPLL,
> > +                                DP_PWR_STATE_MASK);
> > +     }
> > +}
> > +
> >  static void mtk_dp_power_enable(struct mtk_dp *mtk_dp)
> >  {
> >       mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_RESET_AND_PROBE,
> > @@ -1936,16 +1959,9 @@ static enum drm_connector_status
> > mtk_dp_bdg_detect(struct drm_bridge *bridge)
> >       if (!mtk_dp->train_info.cable_plugged_in)
> >               return ret;
> >
> > -     if (!enabled) {
> > -             /* power on aux */
> > -             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                                DP_PWR_STATE_BANDGAP_TPLL_LANE,
> > -                                DP_PWR_STATE_MASK);
> > +     if (!enabled)
> > +             mtk_dp_aux_panel_poweron(mtk_dp, true);
> >
> > -             /* power on panel */
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
> > -             usleep_range(2000, 5000);
> > -     }
> >       /*
> >        * Some dongles still source HPD when they do not connect to
> > any
> >        * sink device. To avoid this, we need to read the sink count
> > @@ -1957,16 +1973,8 @@ static enum drm_connector_status
> > mtk_dp_bdg_detect(struct drm_bridge *bridge)
> >       if (DP_GET_SINK_COUNT(sink_count))
> >               ret = connector_status_connected;
> >
> > -     if (!enabled) {
> > -             /* power off panel */
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D3);
> > -             usleep_range(2000, 3000);
> > -
> > -             /* power off aux */
> > -             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                                DP_PWR_STATE_BANDGAP_TPLL,
> > -                                DP_PWR_STATE_MASK);
> > -     }
> > +     if (!enabled)
> > +             mtk_dp_aux_panel_poweron(mtk_dp, false);
> >
> >       return ret;
> >  }
> > @@ -1982,15 +1990,7 @@ static struct edid *mtk_dp_get_edid(struct
> > drm_bridge *bridge,
> >
> >       if (!enabled) {
> >               drm_atomic_bridge_chain_pre_enable(bridge, connector-
> > >state->state);
> > -
> > -             /* power on aux */
> > -             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                                DP_PWR_STATE_BANDGAP_TPLL_LANE,
> > -                                DP_PWR_STATE_MASK);
> > -
> > -             /* power on panel */
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
> > -             usleep_range(2000, 5000);
> > +             mtk_dp_aux_panel_poweron(mtk_dp, true);
> >       }
> >
> >       new_edid = drm_get_edid(connector, &mtk_dp->aux.ddc);
> > @@ -2010,15 +2010,7 @@ static struct edid *mtk_dp_get_edid(struct
> > drm_bridge *bridge,
> >       }
> >
> >       if (!enabled) {
> > -             /* power off panel */
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D3);
> > -             usleep_range(2000, 3000);
> > -
> > -             /* power off aux */
> > -             mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                                DP_PWR_STATE_BANDGAP_TPLL,
> > -                                DP_PWR_STATE_MASK);
> > -
> > +             mtk_dp_aux_panel_poweron(mtk_dp, false);
> >               drm_atomic_bridge_chain_post_disable(bridge, connector-
> > >state->state);
> >       }
> >
> > @@ -2178,15 +2170,7 @@ static void mtk_dp_bridge_atomic_enable(struct
> > drm_bridge *bridge,
> >               return;
> >       }
> >
> > -     /* power on aux */
> > -     mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
> > -                        DP_PWR_STATE_BANDGAP_TPLL_LANE,
> > -                        DP_PWR_STATE_MASK);
> > -
> > -     if (mtk_dp->train_info.cable_plugged_in) {
> > -             drm_dp_dpcd_writeb(&mtk_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
> > -             usleep_range(2000, 5000);
> > -     }
> > +     mtk_dp_aux_panel_poweron(mtk_dp, true);
> >
> >       /* Training */
> >       ret = mtk_dp_training(mtk_dp);
> > --
> > 2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-07-18  7:41 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-13  9:01 [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus AngeloGioacchino Del Regno
2023-07-13  9:01 ` [PATCH v5 01/10] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function AngeloGioacchino Del Regno
2023-07-18  7:40   ` CK Hu (胡俊光)
2023-07-18  7:41     ` Chen-Yu Tsai
2023-07-13  9:01 ` [PATCH v5 02/10] drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer() AngeloGioacchino Del Regno
2023-07-13  9:01 ` [PATCH v5 03/10] drm/mediatek: dp: Use devm variant of drm_bridge_add() AngeloGioacchino Del Regno
2023-07-13  9:01 ` [PATCH v5 04/10] drm/mediatek: dp: Move AUX_P0 setting to mtk_dp_initialize_aux_settings() AngeloGioacchino Del Regno
2023-07-13  9:01 ` [PATCH v5 05/10] drm/mediatek: dp: Enable event interrupt only when bridge attached AngeloGioacchino Del Regno
2023-07-13  9:01 ` [PATCH v5 06/10] drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled AngeloGioacchino Del Regno
2023-07-13  9:01 ` [PATCH v5 07/10] drm/mediatek: dp: Move PHY registration to new function AngeloGioacchino Del Regno
2023-07-13  9:01 ` [PATCH v5 08/10] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus AngeloGioacchino Del Regno
2023-07-13  9:01 ` [PATCH v5 09/10] drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus AngeloGioacchino Del Regno
2023-07-13  9:01 ` [PATCH v5 10/10] drm/mediatek: dp: Don't register HPD interrupt handler for eDP case AngeloGioacchino Del Regno
2023-07-13  9:54 ` [PATCH v5 00/10] MediaTek DisplayPort: support eDP and aux-bus Chen-Yu Tsai
2023-07-13  9:57   ` AngeloGioacchino Del Regno
2023-07-13 10:03     ` Chen-Yu Tsai

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).