[PATCH v7 00/11] MediaTek DisplayPort: support eDP and aux-bus

[PATCH v7 00/11] MediaTek DisplayPort: support eDP and aux-bus

[AngeloGioacchino Del Regno]

Changes in v7:
 - Fixed snippet removal, moved from patch [11/11] to [9/11] where
   it actually belongs to, as in patch 9/11 I'm moving that snippet
   to a new function.

Changes in v6:
 - Added some previously missing error checking (patch [01/11])
 - Added error checks for devm_drm_bridge_add()
 - Made sure that cable_plugged_in is set to false if HPD assertion
   polling fails (timeout)
 - Support panel as module (tested with panel-edp on MT8195 Tomato)
 - Rebased over next-20230717

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 (11):
  drm/mediatek: dp: Add missing error checks in
    mtk_dp_parse_capabilities
  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 | 360 ++++++++++++++++++++----------
 2 files changed, 242 insertions(+), 119 deletions(-)

-- 
2.41.0

[PATCH v7 01/11] drm/mediatek: dp: Add missing error checks in mtk_dp_parse_capabilities

[AngeloGioacchino Del Regno]

If reading the RX capabilities fails the training pattern will be set
wrongly: add error checking for drm_dp_read_dpcd_caps() and return if
anything went wrong with it.

While at it, also add a less critical error check when writing to
clear the ESI0 IRQ vector.

Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 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 64eee77452c0..c58b775877a3 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1588,7 +1588,9 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp)
 	u8 val;
 	ssize_t ret;
 
-	drm_dp_read_dpcd_caps(&mtk_dp->aux, mtk_dp->rx_cap);
+	ret = drm_dp_read_dpcd_caps(&mtk_dp->aux, mtk_dp->rx_cap);
+	if (ret < 0)
+		return ret;
 
 	if (drm_dp_tps4_supported(mtk_dp->rx_cap))
 		mtk_dp->train_info.channel_eq_pattern = DP_TRAINING_PATTERN_4;
@@ -1615,10 +1617,13 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp)
 			return ret == 0 ? -EIO : ret;
 		}
 
-		if (val)
-			drm_dp_dpcd_writeb(&mtk_dp->aux,
-					   DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
-					   val);
+		if (val) {
+			ret = drm_dp_dpcd_writeb(&mtk_dp->aux,
+						 DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
+						 val);
+			if (ret < 0)
+				return ret;
+		}
 	}
 
 	return 0;
-- 
2.41.0

[PATCH v7 02/11] drm/mediatek: dp: Move AUX and panel poweron/off sequence to function

[AngeloGioacchino Del Regno]

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>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 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 c58b775877a3..77da0d002e9f 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,
@@ -1941,16 +1964,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
@@ -1962,16 +1978,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;
 }
@@ -1987,15 +1995,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);
@@ -2015,15 +2015,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);
 	}
 
@@ -2183,15 +2175,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.41.0

[PATCH v7 03/11] drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer()

[AngeloGioacchino Del Regno]

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>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 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 77da0d002e9f..98f63d8230e4 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;
 		}
@@ -2054,7 +2054,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;
@@ -2070,7 +2070,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.41.0

[PATCH v7 04/11] drm/mediatek: dp: Use devm variant of drm_bridge_add()

[AngeloGioacchino Del Regno]

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>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 98f63d8230e4..fc6cabf5370b 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2552,7 +2552,9 @@ 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);
+	ret = devm_drm_bridge_add(dev, &mtk_dp->bridge);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add bridge\n");
 
 	mtk_dp->need_debounce = true;
 	timer_setup(&mtk_dp->debounce_timer, mtk_dp_debounce_timer, 0);
@@ -2570,7 +2572,6 @@ 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);
-	drm_bridge_remove(&mtk_dp->bridge);
 	platform_device_unregister(mtk_dp->phy_dev);
 	if (mtk_dp->audio_pdev)
 		platform_device_unregister(mtk_dp->audio_pdev);
-- 
2.41.0

[PATCH v7 05/11] drm/mediatek: dp: Move AUX_P0 setting to mtk_dp_initialize_aux_settings()

[AngeloGioacchino Del Regno]

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>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 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 fc6cabf5370b..e8d3bf310608 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)
@@ -1826,10 +1831,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.41.0

[PATCH v7 06/11] drm/mediatek: dp: Enable event interrupt only when bridge attached

[AngeloGioacchino Del Regno]

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>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 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 e8d3bf310608..83e55f8dc84a 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];
@@ -2141,6 +2142,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;
@@ -2157,6 +2160,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);
@@ -2475,7 +2479,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)
@@ -2484,9 +2488,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);
@@ -2507,7 +2511,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.41.0

[PATCH v7 07/11] drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled

[AngeloGioacchino Del Regno]

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>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Reviewed-by: CK Hu <ck.hu@mediatek.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 83e55f8dc84a..c1d1a882f1db 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1948,6 +1948,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,
@@ -2520,11 +2523,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.41.0

[PATCH v7 08/11] drm/mediatek: dp: Move PHY registration to new function

[AngeloGioacchino Del Regno]

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>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.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 c1d1a882f1db..1b4219e6a00b 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2478,6 +2478,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;
@@ -2536,23 +2559,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.41.0

[PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[AngeloGioacchino Del Regno]

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>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
---
 drivers/gpu/drm/mediatek/Kconfig  |  1 +
 drivers/gpu/drm/mediatek/mtk_dp.c | 92 ++++++++++++++++++++++++++-----
 2 files changed, 79 insertions(+), 14 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 1b4219e6a00b..18c944bfc7ce 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;
+
 	ret = drm_dp_read_dpcd_caps(&mtk_dp->aux, mtk_dp->rx_cap);
 	if (ret < 0)
 		return ret;
@@ -2030,15 +2043,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;
 	}
@@ -2501,6 +2513,28 @@ 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;
@@ -2519,21 +2553,14 @@ static int mtk_dp_probe(struct platform_device *pdev)
 		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");
 
-	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);
 
@@ -2577,6 +2604,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.41.0

[PATCH v7 10/11] drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus

[AngeloGioacchino Del Regno]

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 and
register the bridge unconditionally at probe time only if we are
probing full DisplayPort and not eDP while, for the latter, we
register the bridge in the .done_probing() callback and only if
the panel was found and triggered HPD.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 45 ++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 18c944bfc7ce..ba750d463e41 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1920,6 +1920,31 @@ 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) {
+		mtk_dp->train_info.cable_plugged_in = false;
+		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)
 {
@@ -2532,6 +2557,12 @@ static int mtk_dp_edp_link_panel(struct drm_dp_aux *mtk_aux)
 		mtk_dp->next_bridge = NULL;
 		return ret;
 	}
+
+	/* For eDP, we add the bridge only if the panel was found */
+	ret = devm_drm_bridge_add(dev, &mtk_dp->bridge);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -2560,6 +2591,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);
@@ -2592,15 +2624,8 @@ 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;
 
-	ret = devm_drm_bridge_add(dev, &mtk_dp->bridge);
-	if (ret)
-		return dev_err_probe(dev, ret, "Failed to add bridge\n");
-
 	mtk_dp->need_debounce = true;
 	timer_setup(&mtk_dp->debounce_timer, mtk_dp_debounce_timer, 0);
 
@@ -2639,6 +2664,12 @@ static int mtk_dp_probe(struct platform_device *pdev)
 				return ret;
 			}
 		}
+	} else {
+		mtk_dp->bridge.ops = DRM_BRIDGE_OP_DETECT |
+				     DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
+		ret = devm_drm_bridge_add(dev, &mtk_dp->bridge);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to add bridge\n");
 	}
 
 	pm_runtime_enable(dev);
-- 
2.41.0

[PATCH v7 11/11] drm/mediatek: dp: Don't register HPD interrupt handler for eDP case

[AngeloGioacchino Del Regno]

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>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 drivers/gpu/drm/mediatek/mtk_dp.c | 73 +++++++++++++++++++------------
 1 file changed, 46 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index ba750d463e41..c06fcc7318e7 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -2182,9 +2182,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;
 
@@ -2199,8 +2201,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);
@@ -2579,32 +2583,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");
-
 	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) {
@@ -2626,9 +2642,6 @@ static int mtk_dp_probe(struct platform_device *pdev)
 	mtk_dp->bridge.of_node = dev->of_node;
 	mtk_dp->bridge.type = mtk_dp->data->bridge_type;
 
-	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
@@ -2639,6 +2652,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
@@ -2684,7 +2700,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);
 	platform_device_unregister(mtk_dp->phy_dev);
 	if (mtk_dp->audio_pdev)
 		platform_device_unregister(mtk_dp->audio_pdev);
@@ -2698,7 +2715,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;
@@ -2710,7 +2728,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.41.0

Re: [PATCH v7 06/11] drm/mediatek: dp: Enable event interrupt only when bridge attached

[CK Hu (胡俊光)]

Hi, Angelo:

On Tue, 2023-07-25 at 09:32 +0200, AngeloGioacchino Del Regno wrote:
> 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.

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>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
>  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 e8d3bf310608..83e55f8dc84a 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];
> @@ -2141,6 +2142,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;
> @@ -2157,6 +2160,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);
> @@ -2475,7 +2479,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)
> @@ -2484,9 +2488,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);
> @@ -2507,7 +2511,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);

Re: [PATCH v7 11/11] drm/mediatek: dp: Don't register HPD interrupt handler for eDP case

[CK Hu (胡俊光)]

Hi, Angelo:

On Tue, 2023-07-25 at 09:32 +0200, AngeloGioacchino Del Regno wrote:
> 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.

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>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dp.c | 73 +++++++++++++++++++--------
> ----
>  1 file changed, 46 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> b/drivers/gpu/drm/mediatek/mtk_dp.c
> index ba750d463e41..c06fcc7318e7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -2182,9 +2182,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;
>  
> @@ -2199,8 +2201,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);
> @@ -2579,32 +2583,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");
> -
>  	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) {
> @@ -2626,9 +2642,6 @@ static int mtk_dp_probe(struct platform_device
> *pdev)
>  	mtk_dp->bridge.of_node = dev->of_node;
>  	mtk_dp->bridge.type = mtk_dp->data->bridge_type;
>  
> -	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
> @@ -2639,6 +2652,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
> @@ -2684,7 +2700,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);
>  	platform_device_unregister(mtk_dp->phy_dev);
>  	if (mtk_dp->audio_pdev)
>  		platform_device_unregister(mtk_dp->audio_pdev);
> @@ -2698,7 +2715,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;
> @@ -2710,7 +2728,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;

Re: [PATCH v7 08/11] drm/mediatek: dp: Move PHY registration to new function

[CK Hu (胡俊光)]

Hi, Angelo:

On Tue, 2023-07-25 at 09:32 +0200, AngeloGioacchino Del Regno wrote:
> 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.

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>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.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 c1d1a882f1db..1b4219e6a00b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -2478,6 +2478,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;
> @@ -2536,23 +2559,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;

Re: [PATCH v7 01/11] drm/mediatek: dp: Add missing error checks in mtk_dp_parse_capabilities

[CK Hu (胡俊光)]

Hi, Angelo:

On Tue, 2023-07-25 at 09:32 +0200, AngeloGioacchino Del Regno wrote:
> If reading the RX capabilities fails the training pattern will be set
> wrongly: add error checking for drm_dp_read_dpcd_caps() and return if
> anything went wrong with it.
> 
> While at it, also add a less critical error check when writing to
> clear the ESI0 IRQ vector.

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

> 
> Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort
> driver")
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
>  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 64eee77452c0..c58b775877a3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -1588,7 +1588,9 @@ static int mtk_dp_parse_capabilities(struct
> mtk_dp *mtk_dp)
>  	u8 val;
>  	ssize_t ret;
>  
> -	drm_dp_read_dpcd_caps(&mtk_dp->aux, mtk_dp->rx_cap);
> +	ret = drm_dp_read_dpcd_caps(&mtk_dp->aux, mtk_dp->rx_cap);
> +	if (ret < 0)
> +		return ret;
>  
>  	if (drm_dp_tps4_supported(mtk_dp->rx_cap))
>  		mtk_dp->train_info.channel_eq_pattern =
> DP_TRAINING_PATTERN_4;
> @@ -1615,10 +1617,13 @@ static int mtk_dp_parse_capabilities(struct
> mtk_dp *mtk_dp)
>  			return ret == 0 ? -EIO : ret;
>  		}
>  
> -		if (val)
> -			drm_dp_dpcd_writeb(&mtk_dp->aux,
> -					   DP_DEVICE_SERVICE_IRQ_VECTOR
> _ESI0,
> -					   val);
> +		if (val) {
> +			ret = drm_dp_dpcd_writeb(&mtk_dp->aux,
> +						 DP_DEVICE_SERVICE_IRQ_
> VECTOR_ESI0,
> +						 val);
> +			if (ret < 0)
> +				return ret;
> +		}
>  	}
>  
>  	return 0;

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Michael Walle]

Hi AngeloGioacchino,

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

This patch breaks my board based on the MT8195 which only has one
DisplayPort output port. I suspect it might also break the mt8195-cherry
board.

While the mediatek-dpi driver finds the DP port:
[    3.131645] mediatek-dpi 1c113000.dp-intf: Found bridge node: /soc/dp-tx@1c600000

The probing of the eDP is deferred:
[   13.289009] platform 1c015000.dp-intf: deferred probe pending

So I don't know why, but to make dp_intf1 work, it seems that dp_intf0
must be probed successfully. After this patch, the edp (which is
connected to the dp_intf1) probe will return with an -ENODEV and
the previous call to devm_drm_bridge_add() will be rolled back.

Before this patch, bridge_add() was called in any case (in the
error case with next_bridge = NULL) and the mediatek-dpi probed
like that:

[    3.121011] mediatek-dpi 1c015000.dp-intf: Found bridge node: /soc/edp-tx@1c500000
[    3.122111] mediatek-dpi 1c113000.dp-intf: Found bridge node: /soc/dp-tx@1c600000

Eventually resulting in a framebuffer device:
[    4.451081] mediatek-drm mediatek-drm.8.auto: [drm] fb0: mediatekdrmfb frame buffer device


NB, somehow this series broke the initial display output. I always have
to replug the DisplayPort to get some output. I'll dig deeper into that
later.

..

> @@ -2519,21 +2553,14 @@ static int mtk_dp_probe(struct platform_device *pdev)
>  		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;

In my case, this branch was taken.

-michael

> -	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");
>  
> -	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);
>  

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Nícolas F. R. A. Prado]

Hi,

On Fri, Aug 25, 2023 at 02:01:09PM +0200, Michael Walle wrote:
> Hi AngeloGioacchino,
> 
> > 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.
> 
> This patch breaks my board based on the MT8195 which only has one
> DisplayPort output port. I suspect it might also break the mt8195-cherry
> board.

Do you mean that your board does not have an internal display, only the one
output port? If so, why are you enabling the nodes for the internal display path
in your board specific DT?

> 
> While the mediatek-dpi driver finds the DP port:
> [    3.131645] mediatek-dpi 1c113000.dp-intf: Found bridge node: /soc/dp-tx@1c600000
> 
> The probing of the eDP is deferred:
> [   13.289009] platform 1c015000.dp-intf: deferred probe pending
> 
> So I don't know why, but to make dp_intf1 work, it seems that dp_intf0
> must be probed successfully. After this patch, the edp (which is
> connected to the dp_intf1) probe will return with an -ENODEV and
> the previous call to devm_drm_bridge_add() will be rolled back.

The MediaTek DRM driver uses the component framework, so it waits for all of its
components to register until it binds them all (which includes both intf0 and
intf1, unless they're disabled on the DT).

It's true that before this patch no panel being found for edp-tx wouldn't
prevent it to probe, but it really should.

Thanks,
Nícolas

> 
> Before this patch, bridge_add() was called in any case (in the
> error case with next_bridge = NULL) and the mediatek-dpi probed
> like that:
> 
> [    3.121011] mediatek-dpi 1c015000.dp-intf: Found bridge node: /soc/edp-tx@1c500000
> [    3.122111] mediatek-dpi 1c113000.dp-intf: Found bridge node: /soc/dp-tx@1c600000
> 
> Eventually resulting in a framebuffer device:
> [    4.451081] mediatek-drm mediatek-drm.8.auto: [drm] fb0: mediatekdrmfb frame buffer device
> 
> 
> NB, somehow this series broke the initial display output. I always have
> to replug the DisplayPort to get some output. I'll dig deeper into that
> later.
> 
> ..
> 
> > @@ -2519,21 +2553,14 @@ static int mtk_dp_probe(struct platform_device *pdev)
> >  		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;
> 
> In my case, this branch was taken.
> 
> -michael
> 
> > -	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");
> >  
> > -	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);
> >  

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Michael Walle]

Hi Nicolas,

>> > 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.
>> 
>> This patch breaks my board based on the MT8195 which only has one
>> DisplayPort output port. I suspect it might also break the 
>> mt8195-cherry
>> board.
> 
> Do you mean that your board does not have an internal display, only the 
> one
> output port? If so, why are you enabling the nodes for the internal 
> display path
> in your board specific DT?

Well, that depends. The board actually has an eDP socket, but because we
are an OEM, there might be no display connected to it. (And I haven't
tried it yet). It should probably go into an own device tree or overlay
if it is used. I agree. But it looked like it was auto-detectable (it
even has a HDP pin on the eDP socket, not sure about its use case..)

But the real reason I've enabled it was because I'll get an kernel
oops otherwise. I thought it might be some quirk that you'll need both,
because eDP will register even if theres no display - as you've
mentioned below.

Here's the splat:
[    3.237064] mediatek-drm mediatek-drm.10.auto: bound 
1c110000.vpp-merge (ops mtk_disp_merge_component_ops)
[    3.238274] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 
because component 8 is disabled or missing
[    3.239504] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 
because component 9 is disabled or missing
[    3.240741] Unable to handle kernel NULL pointer dereference at 
virtual address 00000000000004a0
[    3.241841] Mem abort info:
[    3.242192]   ESR = 0x0000000096000004
[    3.242662]   EC = 0x25: DABT (current EL), IL = 32 bits
[    3.243328]   SET = 0, FnV = 0
[    3.243710]   EA = 0, S1PTW = 0
[    3.244104]   FSC = 0x04: level 0 translation fault
[    3.244717] Data abort info:
[    3.245078]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[    3.245765]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    3.246398]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    3.247063] [00000000000004a0] user address but active_mm is swapper
[    3.247860] Internal error: Oops: 0000000096000004 [#1] SMP
[    3.248559] Modules linked in:
[    3.248945] CPU: 4 PID: 11 Comm: kworker/u16:0 Not tainted 
6.5.0-rc7-next-20230821+ #2225
[    3.249970] Hardware name: Kontron 3.5"-SBC-i1200 (DT)
[    3.250614] Workqueue: events_unbound deferred_probe_work_func
[    3.251347] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS 
BTYPE=--)
[    3.252220] pc : mtk_drm_crtc_dma_dev_get+0x8/0x18
[    3.252824] lr : mtk_drm_bind+0x458/0x558
[    3.253326] sp : ffff800082b23a20
[    3.253741] x29: ffff800082b23a20 x28: ffff000002c78880 x27: 
ffff8000816466d0
[    3.254635] x26: ffff000002c6f010 x25: 0000000000000003 x24: 
0000000000000000
[    3.255529] x23: ffff000002c78880 x22: 0000000000000002 x21: 
0000000000000000
[    3.256423] x20: ffff000006516800 x19: ffff000002c78880 x18: 
ffffffffffffffff
[    3.257317] x17: 6f63206573756163 x16: 6562203020637472 x15: 
6320676e69746165
[    3.258211] x14: 726320746f4e203a x13: 676e697373696d20 x12: 
726f2064656c6261
[    3.259106] x11: 7369642073692039 x10: ffff80008275c0c0 x9 : 
ffff80008091ebf8
[    3.260000] x8 : 00000000ffffefff x7 : ffff80008275c0c0 x6 : 
80000000fffff000
[    3.260895] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 
ffff000006516ae0
[    3.261789] x2 : ffff000006516ae0 x1 : 0000000000000000 x0 : 
0000000000000000
[    3.262684] Call trace:
[    3.262991]  mtk_drm_crtc_dma_dev_get+0x8/0x18
[    3.263549]  try_to_bring_up_aggregate_device+0x16c/0x1e0
[    3.264227]  __component_add+0xac/0x180
[    3.264708]  component_add+0x1c/0x30
[    3.265158]  mtk_disp_rdma_probe+0x17c/0x270
[    3.265695]  platform_probe+0x70/0xd0
[    3.266155]  really_probe+0x150/0x2c0
[    3.266615]  __driver_probe_device+0x80/0x140
[    3.267162]  driver_probe_device+0x44/0x170
[    3.267687]  __device_attach_driver+0xc0/0x148
[    3.268245]  bus_for_each_drv+0x88/0xf0
[    3.268727]  __device_attach+0xa4/0x198
[    3.269208]  device_initial_probe+0x1c/0x30
[    3.269732]  bus_probe_device+0xb4/0xc0
[    3.270214]  deferred_probe_work_func+0x90/0xd0
[    3.270783]  process_one_work+0x144/0x3a0
[    3.271289]  worker_thread+0x2ac/0x4b8
[    3.271761]  kthread+0xec/0xf8
[    3.272145]  ret_from_fork+0x10/0x20
[    3.272597] Code: 814f7858 ffff8000 aa1e03e9 d503201f (f9425000)
[    3.273361] ---[ end trace 0000000000000000 ]---

Here's the complete bootlog:
https://pastebin.com/raw/SekMYBj4

Any help is where to look is appreciated.

>> While the mediatek-dpi driver finds the DP port:
>> [    3.131645] mediatek-dpi 1c113000.dp-intf: Found bridge node: 
>> /soc/dp-tx@1c600000
>> 
>> The probing of the eDP is deferred:
>> [   13.289009] platform 1c015000.dp-intf: deferred probe pending
>> 
>> So I don't know why, but to make dp_intf1 work, it seems that dp_intf0
>> must be probed successfully. After this patch, the edp (which is
>> connected to the dp_intf1) probe will return with an -ENODEV and
>> the previous call to devm_drm_bridge_add() will be rolled back.
> 
> The MediaTek DRM driver uses the component framework, so it waits for 
> all of its
> components to register until it binds them all (which includes both 
> intf0 and
> intf1, unless they're disabled on the DT).

Oh with "I don't know why, but to make dp_intf1 work.." I meant the 
splat
above. I figured that dpi won't probe if a dependency is still deferred 
;)

-michael

> It's true that before this patch no panel being found for edp-tx 
> wouldn't
> prevent it to probe, but it really should.
> 
> Thanks,
> Nícolas
> 
>> 
>> Before this patch, bridge_add() was called in any case (in the
>> error case with next_bridge = NULL) and the mediatek-dpi probed
>> like that:
>> 
>> [    3.121011] mediatek-dpi 1c015000.dp-intf: Found bridge node: 
>> /soc/edp-tx@1c500000
>> [    3.122111] mediatek-dpi 1c113000.dp-intf: Found bridge node: 
>> /soc/dp-tx@1c600000
>> 
>> Eventually resulting in a framebuffer device:
>> [    4.451081] mediatek-drm mediatek-drm.8.auto: [drm] fb0: 
>> mediatekdrmfb frame buffer device
>> 
>> 
>> NB, somehow this series broke the initial display output. I always 
>> have
>> to replug the DisplayPort to get some output. I'll dig deeper into 
>> that
>> later.
>> 
>> ..
>> 
>> > @@ -2519,21 +2553,14 @@ static int mtk_dp_probe(struct platform_device *pdev)
>> >  		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;
>> 
>> In my case, this branch was taken.
>> 
>> -michael
>> 
>> > -	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");
>> >
>> > -	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);
>> >

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Nícolas F. R. A. Prado]

On Fri, Aug 25, 2023 at 05:42:59PM +0200, Michael Walle wrote:
> Hi Nicolas,
> 
> > > > 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.
> > > 
> > > This patch breaks my board based on the MT8195 which only has one
> > > DisplayPort output port. I suspect it might also break the
> > > mt8195-cherry
> > > board.
> > 
> > Do you mean that your board does not have an internal display, only the
> > one
> > output port? If so, why are you enabling the nodes for the internal
> > display path
> > in your board specific DT?
> 
> Well, that depends. The board actually has an eDP socket, but because we
> are an OEM, there might be no display connected to it. (And I haven't
> tried it yet). It should probably go into an own device tree or overlay
> if it is used. I agree. But it looked like it was auto-detectable (it
> even has a HDP pin on the eDP socket, not sure about its use case..)

Right, given it has an HPD pin it should be possible to hotplug. Although
hotplugging a panel to eDP doesn't seem common, so Angelo even removed HPD
irq support for eDP in patch 11 of this series.

The unit I have is a mt8195-cherry-tomato, a Chromebook, so I couldn't really
test hotplugging the internal display.

> 
> But the real reason I've enabled it was because I'll get an kernel
> oops otherwise. I thought it might be some quirk that you'll need both,
> because eDP will register even if theres no display - as you've
> mentioned below.
> 
> Here's the splat:
> [    3.237064] mediatek-drm mediatek-drm.10.auto: bound 1c110000.vpp-merge
> (ops mtk_disp_merge_component_ops)
> [    3.238274] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 because
> component 8 is disabled or missing
> [    3.239504] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 because
> component 9 is disabled or missing
> [    3.240741] Unable to handle kernel NULL pointer dereference at virtual
> address 00000000000004a0
> [    3.241841] Mem abort info:
> [    3.242192]   ESR = 0x0000000096000004
> [    3.242662]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    3.243328]   SET = 0, FnV = 0
> [    3.243710]   EA = 0, S1PTW = 0
> [    3.244104]   FSC = 0x04: level 0 translation fault
> [    3.244717] Data abort info:
> [    3.245078]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [    3.245765]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [    3.246398]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [    3.247063] [00000000000004a0] user address but active_mm is swapper
> [    3.247860] Internal error: Oops: 0000000096000004 [#1] SMP
> [    3.248559] Modules linked in:
> [    3.248945] CPU: 4 PID: 11 Comm: kworker/u16:0 Not tainted
> 6.5.0-rc7-next-20230821+ #2225
> [    3.249970] Hardware name: Kontron 3.5"-SBC-i1200 (DT)
> [    3.250614] Workqueue: events_unbound deferred_probe_work_func
> [    3.251347] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> [    3.252220] pc : mtk_drm_crtc_dma_dev_get+0x8/0x18
> [    3.252824] lr : mtk_drm_bind+0x458/0x558
> [    3.253326] sp : ffff800082b23a20
> [    3.253741] x29: ffff800082b23a20 x28: ffff000002c78880 x27:
> ffff8000816466d0
> [    3.254635] x26: ffff000002c6f010 x25: 0000000000000003 x24:
> 0000000000000000
> [    3.255529] x23: ffff000002c78880 x22: 0000000000000002 x21:
> 0000000000000000
> [    3.256423] x20: ffff000006516800 x19: ffff000002c78880 x18:
> ffffffffffffffff
> [    3.257317] x17: 6f63206573756163 x16: 6562203020637472 x15:
> 6320676e69746165
> [    3.258211] x14: 726320746f4e203a x13: 676e697373696d20 x12:
> 726f2064656c6261
> [    3.259106] x11: 7369642073692039 x10: ffff80008275c0c0 x9 :
> ffff80008091ebf8
> [    3.260000] x8 : 00000000ffffefff x7 : ffff80008275c0c0 x6 :
> 80000000fffff000
> [    3.260895] x5 : 000000000000bff4 x4 : 0000000000000000 x3 :
> ffff000006516ae0
> [    3.261789] x2 : ffff000006516ae0 x1 : 0000000000000000 x0 :
> 0000000000000000
> [    3.262684] Call trace:
> [    3.262991]  mtk_drm_crtc_dma_dev_get+0x8/0x18
> [    3.263549]  try_to_bring_up_aggregate_device+0x16c/0x1e0
> [    3.264227]  __component_add+0xac/0x180
> [    3.264708]  component_add+0x1c/0x30
> [    3.265158]  mtk_disp_rdma_probe+0x17c/0x270
> [    3.265695]  platform_probe+0x70/0xd0
> [    3.266155]  really_probe+0x150/0x2c0
> [    3.266615]  __driver_probe_device+0x80/0x140
> [    3.267162]  driver_probe_device+0x44/0x170
> [    3.267687]  __device_attach_driver+0xc0/0x148
> [    3.268245]  bus_for_each_drv+0x88/0xf0
> [    3.268727]  __device_attach+0xa4/0x198
> [    3.269208]  device_initial_probe+0x1c/0x30
> [    3.269732]  bus_probe_device+0xb4/0xc0
> [    3.270214]  deferred_probe_work_func+0x90/0xd0
> [    3.270783]  process_one_work+0x144/0x3a0
> [    3.271289]  worker_thread+0x2ac/0x4b8
> [    3.271761]  kthread+0xec/0xf8
> [    3.272145]  ret_from_fork+0x10/0x20
> [    3.272597] Code: 814f7858 ffff8000 aa1e03e9 d503201f (f9425000)
> [    3.273361] ---[ end trace 0000000000000000 ]---

I tried reproducing this on mt8192-asurada-spherion and mt8195-cherry-tomato but
wasn't able to. However, I did see another issue

[    3.183314] mediatek-drm mediatek-drm.9.auto: Not creating crtc 0 because component 14 is disabled or missing
[    3.199404] Bogus possible_crtcs: [ENCODER:31:TMDS-31] possible_crtcs=0x2 (full crtc mask=0x1)
[    3.208081] WARNING: CPU: 6 PID: 68 at drivers/gpu/drm/drm_mode_config.c:626 drm_mode_config_validate+0x1c8/0x548
[    3.224789] Modules linked in:
[    3.227838] CPU: 6 PID: 68 Comm: kworker/u16:1 Not tainted 6.5.0-rc3-next-20230728+ #100
[    3.235918] Hardware name: Google Spherion (rev0 - 3) (DT)
[    3.241391] Workqueue: events_unbound deferred_probe_work_func
[    3.247216] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.254167] pc : drm_mode_config_validate+0x1c8/0x548
[    3.259209] lr : drm_mode_config_validate+0x1c8/0x548
[    3.264250] sp : ffff8000804e3970
[    3.267552] x29: ffff8000804e3980 x28: ffff4841827c1880 x27: 0000000000000001
[    3.274677] x26: 0000000000000001 x25: ffff4841825f5ab0 x24: ffff4841825f5ab0
[    3.281801] x23: ffff484182469880 x22: ffffa80dbfbdde28 x21: ffffa80dbfbddba8
[    3.288925] x20: ffff4841825f5800 x19: ffff4841825f5aa8 x18: 0000000000000030
[    3.296050] x17: 6628203278303d73 x16: 637472635f656c62 x15: 6973736f70205d31
[    3.303174] x14: 332d53444d543a31 x13: 293178303d6b7361 x12: 6d2063747263206c
[    3.310298] x11: 6c75662820327830 x10: 3d73637472635f65 x9 : ffffa80dbdd3805c
[    3.317422] x8 : 455b203a73637472 x7 : 205d343034393931 x6 : ffffa80dbe6365d8
[    3.324546] x5 : ffffa80dc0fcc48f x4 : ffffa80dc0049b40 x3 : 00000000ffffffff
[    3.331671] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff4841809d5e80
[    3.338796] Call trace:
[    3.341232]  drm_mode_config_validate+0x1c8/0x548
[    3.345924]  drm_dev_register+0x198/0x248
[    3.345931]  mtk_drm_bind+0x2cc/0x590
[    3.345936]  try_to_bring_up_aggregate_device+0x1f8/0x308
[    3.345940]  __component_add+0xac/0x1a0
[    3.345942]  component_add+0x1c/0x30
[    3.345944]  mtk_dpi_probe+0x1c0/0x300
[    3.358100]  platform_probe+0x70/0xe8
[    3.358106]  really_probe+0x18c/0x3d8
[    3.358108]  __driver_probe_device+0x84/0x180
[    3.358109]  driver_probe_device+0x44/0x120
[    3.358111]  __device_attach_driver+0xc4/0x168
[    3.358113]  bus_for_each_drv+0x8c/0xf0
[    3.367146]  __device_attach+0xb0/0x1e8
[    3.367148]  device_initial_probe+0x1c/0x30
[    3.367150]  bus_probe_device+0xb4/0xc0
[    3.367153]  deferred_probe_work_func+0xa4/0x100
[    3.367155]  process_one_work+0x1ec/0x480
[    3.374543]  worker_thread+0x74/0x448
[    3.374545]  kthread+0x120/0x130
[    3.374548]  ret_from_fork+0x10/0x20

The mtk-dpi driver populates its encoder's possible_crtcs from the result of
mtk_drm_find_possible_crtc_by_comp(), and this function assumes the CRTC for the
main path will always have ID 0, and the external path ID 1, but when the
main path components are disabled, the external path CRTC becomes the one with
ID 0.

So we'd need to make that function return the crtc id dynamically based on
whether the components for each path are enabled or not.

With that function hacked to force the right crtc ID, I was able to have a
working external DP, with the eDP pipeline disabled on both mt8192 and mt8195.

Thanks,
Nícolas

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Michael Walle]

Hi Nícolas,

>> But the real reason I've enabled it was because I'll get an kernel
>> oops otherwise. I thought it might be some quirk that you'll need 
>> both,
>> because eDP will register even if theres no display - as you've
>> mentioned below.
>> 
>> Here's the splat:
>> [    3.237064] mediatek-drm mediatek-drm.10.auto: bound 
>> 1c110000.vpp-merge
>> (ops mtk_disp_merge_component_ops)
>> [    3.238274] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 
>> because
>> component 8 is disabled or missing
>> [    3.239504] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 
>> because
>> component 9 is disabled or missing
>> [    3.240741] Unable to handle kernel NULL pointer dereference at 
>> virtual
>> address 00000000000004a0
>> [    3.241841] Mem abort info:
>> [    3.242192]   ESR = 0x0000000096000004
>> [    3.242662]   EC = 0x25: DABT (current EL), IL = 32 bits
>> [    3.243328]   SET = 0, FnV = 0
>> [    3.243710]   EA = 0, S1PTW = 0
>> [    3.244104]   FSC = 0x04: level 0 translation fault
>> [    3.244717] Data abort info:
>> [    3.245078]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>> [    3.245765]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>> [    3.246398]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> [    3.247063] [00000000000004a0] user address but active_mm is 
>> swapper
>> [    3.247860] Internal error: Oops: 0000000096000004 [#1] SMP
>> [    3.248559] Modules linked in:
>> [    3.248945] CPU: 4 PID: 11 Comm: kworker/u16:0 Not tainted
>> 6.5.0-rc7-next-20230821+ #2225
>> [    3.249970] Hardware name: Kontron 3.5"-SBC-i1200 (DT)
>> [    3.250614] Workqueue: events_unbound deferred_probe_work_func
>> [    3.251347] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS
>> BTYPE=--)
>> [    3.252220] pc : mtk_drm_crtc_dma_dev_get+0x8/0x18
>> [    3.252824] lr : mtk_drm_bind+0x458/0x558
>> [    3.253326] sp : ffff800082b23a20
>> [    3.253741] x29: ffff800082b23a20 x28: ffff000002c78880 x27:
>> ffff8000816466d0
>> [    3.254635] x26: ffff000002c6f010 x25: 0000000000000003 x24:
>> 0000000000000000
>> [    3.255529] x23: ffff000002c78880 x22: 0000000000000002 x21:
>> 0000000000000000
>> [    3.256423] x20: ffff000006516800 x19: ffff000002c78880 x18:
>> ffffffffffffffff
>> [    3.257317] x17: 6f63206573756163 x16: 6562203020637472 x15:
>> 6320676e69746165
>> [    3.258211] x14: 726320746f4e203a x13: 676e697373696d20 x12:
>> 726f2064656c6261
>> [    3.259106] x11: 7369642073692039 x10: ffff80008275c0c0 x9 :
>> ffff80008091ebf8
>> [    3.260000] x8 : 00000000ffffefff x7 : ffff80008275c0c0 x6 :
>> 80000000fffff000
>> [    3.260895] x5 : 000000000000bff4 x4 : 0000000000000000 x3 :
>> ffff000006516ae0
>> [    3.261789] x2 : ffff000006516ae0 x1 : 0000000000000000 x0 :
>> 0000000000000000
>> [    3.262684] Call trace:
>> [    3.262991]  mtk_drm_crtc_dma_dev_get+0x8/0x18
>> [    3.263549]  try_to_bring_up_aggregate_device+0x16c/0x1e0
>> [    3.264227]  __component_add+0xac/0x180
>> [    3.264708]  component_add+0x1c/0x30
>> [    3.265158]  mtk_disp_rdma_probe+0x17c/0x270
>> [    3.265695]  platform_probe+0x70/0xd0
>> [    3.266155]  really_probe+0x150/0x2c0
>> [    3.266615]  __driver_probe_device+0x80/0x140
>> [    3.267162]  driver_probe_device+0x44/0x170
>> [    3.267687]  __device_attach_driver+0xc0/0x148
>> [    3.268245]  bus_for_each_drv+0x88/0xf0
>> [    3.268727]  __device_attach+0xa4/0x198
>> [    3.269208]  device_initial_probe+0x1c/0x30
>> [    3.269732]  bus_probe_device+0xb4/0xc0
>> [    3.270214]  deferred_probe_work_func+0x90/0xd0
>> [    3.270783]  process_one_work+0x144/0x3a0
>> [    3.271289]  worker_thread+0x2ac/0x4b8
>> [    3.271761]  kthread+0xec/0xf8
>> [    3.272145]  ret_from_fork+0x10/0x20
>> [    3.272597] Code: 814f7858 ffff8000 aa1e03e9 d503201f (f9425000)
>> [    3.273361] ---[ end trace 0000000000000000 ]---
> 
> I tried reproducing this on mt8192-asurada-spherion and 
> mt8195-cherry-tomato but
> wasn't able to. However, I did see another issue

Yeah sorry, I tried to reproduce my initial oops but messed my DT up
and ended up with no path enabled at all.


> [    3.183314] mediatek-drm mediatek-drm.9.auto: Not creating crtc 0 
> because component 14 is disabled or missing
> [    3.199404] Bogus possible_crtcs: [ENCODER:31:TMDS-31] 
> possible_crtcs=0x2 (full crtc mask=0x1)
> [    3.208081] WARNING: CPU: 6 PID: 68 at 
> drivers/gpu/drm/drm_mode_config.c:626 
> drm_mode_config_validate+0x1c8/0x548
> [    3.224789] Modules linked in:
> [    3.227838] CPU: 6 PID: 68 Comm: kworker/u16:1 Not tainted 
> 6.5.0-rc3-next-20230728+ #100
> [    3.235918] Hardware name: Google Spherion (rev0 - 3) (DT)
> [    3.241391] Workqueue: events_unbound deferred_probe_work_func
> [    3.247216] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS 
> BTYPE=--)
> [    3.254167] pc : drm_mode_config_validate+0x1c8/0x548
> [    3.259209] lr : drm_mode_config_validate+0x1c8/0x548
> [    3.264250] sp : ffff8000804e3970
> [    3.267552] x29: ffff8000804e3980 x28: ffff4841827c1880 x27: 
> 0000000000000001
> [    3.274677] x26: 0000000000000001 x25: ffff4841825f5ab0 x24: 
> ffff4841825f5ab0
> [    3.281801] x23: ffff484182469880 x22: ffffa80dbfbdde28 x21: 
> ffffa80dbfbddba8
> [    3.288925] x20: ffff4841825f5800 x19: ffff4841825f5aa8 x18: 
> 0000000000000030
> [    3.296050] x17: 6628203278303d73 x16: 637472635f656c62 x15: 
> 6973736f70205d31
> [    3.303174] x14: 332d53444d543a31 x13: 293178303d6b7361 x12: 
> 6d2063747263206c
> [    3.310298] x11: 6c75662820327830 x10: 3d73637472635f65 x9 : 
> ffffa80dbdd3805c
> [    3.317422] x8 : 455b203a73637472 x7 : 205d343034393931 x6 : 
> ffffa80dbe6365d8
> [    3.324546] x5 : ffffa80dc0fcc48f x4 : ffffa80dc0049b40 x3 : 
> 00000000ffffffff
> [    3.331671] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 
> ffff4841809d5e80
> [    3.338796] Call trace:
> [    3.341232]  drm_mode_config_validate+0x1c8/0x548
> [    3.345924]  drm_dev_register+0x198/0x248
> [    3.345931]  mtk_drm_bind+0x2cc/0x590
> [    3.345936]  try_to_bring_up_aggregate_device+0x1f8/0x308
> [    3.345940]  __component_add+0xac/0x1a0
> [    3.345942]  component_add+0x1c/0x30
> [    3.345944]  mtk_dpi_probe+0x1c0/0x300
> [    3.358100]  platform_probe+0x70/0xe8
> [    3.358106]  really_probe+0x18c/0x3d8
> [    3.358108]  __driver_probe_device+0x84/0x180
> [    3.358109]  driver_probe_device+0x44/0x120
> [    3.358111]  __device_attach_driver+0xc4/0x168
> [    3.358113]  bus_for_each_drv+0x8c/0xf0
> [    3.367146]  __device_attach+0xb0/0x1e8
> [    3.367148]  device_initial_probe+0x1c/0x30
> [    3.367150]  bus_probe_device+0xb4/0xc0
> [    3.367153]  deferred_probe_work_func+0xa4/0x100
> [    3.367155]  process_one_work+0x1ec/0x480
> [    3.374543]  worker_thread+0x74/0x448
> [    3.374545]  kthread+0x120/0x130
> [    3.374548]  ret_from_fork+0x10/0x20

That was what I was seeing in the first place, yes. (Any yeah, no oops,
but a WARN()).

> The mtk-dpi driver populates its encoder's possible_crtcs from the 
> result of
> mtk_drm_find_possible_crtc_by_comp(), and this function assumes the 
> CRTC for the
> main path will always have ID 0, and the external path ID 1, but when 
> the
> main path components are disabled, the external path CRTC becomes the 
> one with
> ID 0.
> 
> So we'd need to make that function return the crtc id dynamically based 
> on
> whether the components for each path are enabled or not.
> 
> With that function hacked to force the right crtc ID, I was able to 
> have a
> working external DP, with the eDP pipeline disabled on both mt8192 and 
> mt8195.

Thanks for the hint, where to look at.

While digging through the code I realized that all the outputs and 
pipelines
are harcoded. Doh. For all the mediatek SoCs. Looks like major 
restriction to
me. E.g. there is also DSI and HDMI output on the mt8195. I looked at 
the
downstream linux and there, the output is not part of the pipeline. Are 
you
aware of any work in that direction?

-michael

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Nícolas F. R. A. Prado]

On Tue, Aug 29, 2023 at 03:30:24PM +0200, Michael Walle wrote:
> Hi Nícolas,
> 
> > > But the real reason I've enabled it was because I'll get an kernel
> > > oops otherwise. I thought it might be some quirk that you'll need
> > > both,
> > > because eDP will register even if theres no display - as you've
> > > mentioned below.
> > > 
> > > Here's the splat:
> > > [    3.237064] mediatek-drm mediatek-drm.10.auto: bound
> > > 1c110000.vpp-merge
> > > (ops mtk_disp_merge_component_ops)
> > > [    3.238274] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0
> > > because
> > > component 8 is disabled or missing
> > > [    3.239504] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0
> > > because
> > > component 9 is disabled or missing
> > > [    3.240741] Unable to handle kernel NULL pointer dereference at
> > > virtual
> > > address 00000000000004a0
> > > [    3.241841] Mem abort info:
> > > [    3.242192]   ESR = 0x0000000096000004
> > > [    3.242662]   EC = 0x25: DABT (current EL), IL = 32 bits
> > > [    3.243328]   SET = 0, FnV = 0
> > > [    3.243710]   EA = 0, S1PTW = 0
> > > [    3.244104]   FSC = 0x04: level 0 translation fault
> > > [    3.244717] Data abort info:
> > > [    3.245078]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> > > [    3.245765]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> > > [    3.246398]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> > > [    3.247063] [00000000000004a0] user address but active_mm is
> > > swapper
> > > [    3.247860] Internal error: Oops: 0000000096000004 [#1] SMP
> > > [    3.248559] Modules linked in:
> > > [    3.248945] CPU: 4 PID: 11 Comm: kworker/u16:0 Not tainted
> > > 6.5.0-rc7-next-20230821+ #2225
> > > [    3.249970] Hardware name: Kontron 3.5"-SBC-i1200 (DT)
> > > [    3.250614] Workqueue: events_unbound deferred_probe_work_func
> > > [    3.251347] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS
> > > BTYPE=--)
> > > [    3.252220] pc : mtk_drm_crtc_dma_dev_get+0x8/0x18
> > > [    3.252824] lr : mtk_drm_bind+0x458/0x558
> > > [    3.253326] sp : ffff800082b23a20
> > > [    3.253741] x29: ffff800082b23a20 x28: ffff000002c78880 x27:
> > > ffff8000816466d0
> > > [    3.254635] x26: ffff000002c6f010 x25: 0000000000000003 x24:
> > > 0000000000000000
> > > [    3.255529] x23: ffff000002c78880 x22: 0000000000000002 x21:
> > > 0000000000000000
> > > [    3.256423] x20: ffff000006516800 x19: ffff000002c78880 x18:
> > > ffffffffffffffff
> > > [    3.257317] x17: 6f63206573756163 x16: 6562203020637472 x15:
> > > 6320676e69746165
> > > [    3.258211] x14: 726320746f4e203a x13: 676e697373696d20 x12:
> > > 726f2064656c6261
> > > [    3.259106] x11: 7369642073692039 x10: ffff80008275c0c0 x9 :
> > > ffff80008091ebf8
> > > [    3.260000] x8 : 00000000ffffefff x7 : ffff80008275c0c0 x6 :
> > > 80000000fffff000
> > > [    3.260895] x5 : 000000000000bff4 x4 : 0000000000000000 x3 :
> > > ffff000006516ae0
> > > [    3.261789] x2 : ffff000006516ae0 x1 : 0000000000000000 x0 :
> > > 0000000000000000
> > > [    3.262684] Call trace:
> > > [    3.262991]  mtk_drm_crtc_dma_dev_get+0x8/0x18
> > > [    3.263549]  try_to_bring_up_aggregate_device+0x16c/0x1e0
> > > [    3.264227]  __component_add+0xac/0x180
> > > [    3.264708]  component_add+0x1c/0x30
> > > [    3.265158]  mtk_disp_rdma_probe+0x17c/0x270
> > > [    3.265695]  platform_probe+0x70/0xd0
> > > [    3.266155]  really_probe+0x150/0x2c0
> > > [    3.266615]  __driver_probe_device+0x80/0x140
> > > [    3.267162]  driver_probe_device+0x44/0x170
> > > [    3.267687]  __device_attach_driver+0xc0/0x148
> > > [    3.268245]  bus_for_each_drv+0x88/0xf0
> > > [    3.268727]  __device_attach+0xa4/0x198
> > > [    3.269208]  device_initial_probe+0x1c/0x30
> > > [    3.269732]  bus_probe_device+0xb4/0xc0
> > > [    3.270214]  deferred_probe_work_func+0x90/0xd0
> > > [    3.270783]  process_one_work+0x144/0x3a0
> > > [    3.271289]  worker_thread+0x2ac/0x4b8
> > > [    3.271761]  kthread+0xec/0xf8
> > > [    3.272145]  ret_from_fork+0x10/0x20
> > > [    3.272597] Code: 814f7858 ffff8000 aa1e03e9 d503201f (f9425000)
> > > [    3.273361] ---[ end trace 0000000000000000 ]---
> > 
> > I tried reproducing this on mt8192-asurada-spherion and
> > mt8195-cherry-tomato but
> > wasn't able to. However, I did see another issue
> 
> Yeah sorry, I tried to reproduce my initial oops but messed my DT up
> and ended up with no path enabled at all.

No worries, thanks for sending fixes for both!

> 
[..]
> > The mtk-dpi driver populates its encoder's possible_crtcs from the
> > result of
> > mtk_drm_find_possible_crtc_by_comp(), and this function assumes the CRTC
> > for the
> > main path will always have ID 0, and the external path ID 1, but when
> > the
> > main path components are disabled, the external path CRTC becomes the
> > one with
> > ID 0.
> > 
> > So we'd need to make that function return the crtc id dynamically based
> > on
> > whether the components for each path are enabled or not.
> > 
> > With that function hacked to force the right crtc ID, I was able to have
> > a
> > working external DP, with the eDP pipeline disabled on both mt8192 and
> > mt8195.
> 
> Thanks for the hint, where to look at.
> 
> While digging through the code I realized that all the outputs and pipelines
> are harcoded. Doh. For all the mediatek SoCs. Looks like major restriction
> to
> me. E.g. there is also DSI and HDMI output on the mt8195. I looked at the
> downstream linux and there, the output is not part of the pipeline. Are you
> aware of any work in that direction?

I'm not sure I get what output and pipelines are hardcoded that you're referring
to (besides the one in the mtk-dsi/dpi driver that you already sent the patch
fixing).

And I'm not familiar with the DSI and HDMI output support on MT8195, so I can't
help with that.

Thanks,
Nícolas

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Michael Walle]

>> While digging through the code I realized that all the outputs and 
>> pipelines
>> are harcoded. Doh. For all the mediatek SoCs. Looks like major 
>> restriction
>> to
>> me. E.g. there is also DSI and HDMI output on the mt8195. I looked at 
>> the
>> downstream linux and there, the output is not part of the pipeline. 
>> Are you
>> aware of any work in that direction?
> 
> I'm not sure I get what output and pipelines are hardcoded that you're 
> referring
> to (besides the one in the mtk-dsi/dpi driver that you already sent the 
> patch
> fixing).

Have a look at [1]. That main path ends with the DP_INTF0 which is the
eDP output. But from what I understand that path can also use the DSI
output. But that is a pattern for all the paths in that file. Looks like
all supported boards in the kernel will have the output type for a given
SoC/path (or maybe the mt8195 is the first one which supports different
output interfaces).

If you have a look at the mediatek kernel instead [2], there the last
part of the path is not fixed, but there is also a .conn_routes property
by which you seem to be able to choose the actual output interface.

I was just curious if you know of any development for that (or similar)
in the kernel.

-michael

> And I'm not familiar with the DSI and HDMI output support on MT8195, so 
> I can't
> help with that.
> 
> Thanks,
> Nícolas

[1] 
https://elixir.bootlin.com/linux/v6.5/source/drivers/gpu/drm/mediatek/mtk_drm_drv.c#L210
[2] 
https://gitlab.com/mediatek/aiot/bsp/linux/-/blob/mtk-v5.15-dev/drivers/gpu/drm/mediatek/mtk_drm_drv.c?ref_type=heads#L425

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Chen-Yu Tsai]

On Wed, Aug 30, 2023 at 7:11 PM Michael Walle <mwalle@kernel.org> wrote:
>
> >> While digging through the code I realized that all the outputs and
> >> pipelines
> >> are harcoded. Doh. For all the mediatek SoCs. Looks like major
> >> restriction
> >> to
> >> me. E.g. there is also DSI and HDMI output on the mt8195. I looked at
> >> the
> >> downstream linux and there, the output is not part of the pipeline.
> >> Are you
> >> aware of any work in that direction?
> >
> > I'm not sure I get what output and pipelines are hardcoded that you're
> > referring
> > to (besides the one in the mtk-dsi/dpi driver that you already sent the
> > patch
> > fixing).
>
> Have a look at [1]. That main path ends with the DP_INTF0 which is the
> eDP output. But from what I understand that path can also use the DSI
> output. But that is a pattern for all the paths in that file. Looks like
> all supported boards in the kernel will have the output type for a given
> SoC/path (or maybe the mt8195 is the first one which supports different
> output interfaces).
>
> If you have a look at the mediatek kernel instead [2], there the last
> part of the path is not fixed, but there is also a .conn_routes property
> by which you seem to be able to choose the actual output interface.
>
> I was just curious if you know of any development for that (or similar)
> in the kernel.

This is probably because support for this SoC began with Chromebooks,
which have fixed and defined uses for the pipelines. I suspect that
what you are working on is much more flexible.

The driver should be made to allow dynamic selection of outputs, as
is commonly seen with other drivers, but I don't know if that's on
anyone's TODO list.

ChenYu

> > And I'm not familiar with the DSI and HDMI output support on MT8195, so
> > I can't
> > help with that.
> >
> > Thanks,
> > Nícolas
>
> [1]
> https://elixir.bootlin.com/linux/v6.5/source/drivers/gpu/drm/mediatek/mtk_drm_drv.c#L210
> [2]
> https://gitlab.com/mediatek/aiot/bsp/linux/-/blob/mtk-v5.15-dev/drivers/gpu/drm/mediatek/mtk_drm_drv.c?ref_type=heads#L425

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Michael Walle]

Hi,

>> I was just curious if you know of any development for that (or 
>> similar)
>> in the kernel.
> 
> This is probably because support for this SoC began with Chromebooks,
> which have fixed and defined uses for the pipelines. I suspect that
> what you are working on is much more flexible.

Yes. that is correct.

> The driver should be made to allow dynamic selection of outputs, as
> is commonly seen with other drivers, but I don't know if that's on
> anyone's TODO list.

Do you have any pointers where to look at?

-michael

Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

[Chen-Yu Tsai]

On Fri, Sep 1, 2023 at 6:00 PM Michael Walle <mwalle@kernel.org> wrote:
>
> Hi,
>
> >> I was just curious if you know of any development for that (or
> >> similar)
> >> in the kernel.
> >
> > This is probably because support for this SoC began with Chromebooks,
> > which have fixed and defined uses for the pipelines. I suspect that
> > what you are working on is much more flexible.
>
> Yes. that is correct.
>
> > The driver should be made to allow dynamic selection of outputs, as
> > is commonly seen with other drivers, but I don't know if that's on
> > anyone's TODO list.
>
> Do you have any pointers where to look at?

There's a series [1] called "Add connector dynamic selection capability".
Not sure if it does what you want though. I haven't looked into it.

ChenYu

[1] https://lore.kernel.org/all/20230809181525.7561-1-jason-jh.lin@mediatek.com/