<html><body><p>
<pre>
On Tue, 2025-08-05 at 00:03 +0800, Haru Zheng wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
>
>
> When suspending and resuming DisplayPort via Type-C,
> link training will be fail.
>
> This patch backports the software IRQ handling for DP,
> as eDP uses hardware IRQ while DP uses software IRQ.
> Additionally, cable_plugged_in is flipped in
> mtk_dp_hpd_event to ensure correct hotplug detection
> during resume.
>
> These changes fix the DP training failure after suspend/resume.
>
> Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
> Signed-off-by: Haru Zheng <towwy321@gmail.com>
>
> ---
>
> Changes since v1:
> - Fixed indentation to use tabs
> - Simplified swirq_enable() logic with ternary
> - Removed unnecessary memset()
> - Replaced dev_info() with dev_dbg()
> - Add mtk_dp_bridge_hpd_notify() declaration to struct drm_bridge_funcs mtk_dp_bridge_funcs
> - Removed IRQ enable from probe() to avoid enabling IRQ for eDP
> - Corrected HW/SW IRQ logic:
> * eDP uses hardware IRQ, DP uses software IRQ
> * Previously some logic was reversed causing issues
> - Fixed hotplug detection logic in mtk_dp_hpd_event:
> * cable_plugged_in flag inverted to ensure correct detection on resume
>
> Code flow:
> - On suspend, DP disables training and IRQs accordingly.
> - On resume, IRQs for DP are re-enabled via software IRQ handler.
> - HPD events are processed with correct plug/unplug state, ensuring training succeeds.
>
> ---
> drivers/gpu/drm/mediatek/mtk_dp.c | 94 ++++++++++++++++++++++++---
> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 3 +
> 2 files changed, 87 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index bef6eeb30d3e..b0f96c7c279e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -1012,6 +1012,12 @@ static u32 mtk_dp_swirq_get_clear(struct mtk_dp *mtk_dp)
> return irq_status;
> }
>
> +static void mtk_dp_swirq_enable(struct mtk_dp *mtk_dp, bool enable)
> +{
> + mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_35C4, enable ? 0 : 0xFFFF,
> + SW_IRQ_FINAL_STATUS_DP_TRANS_P0_MASK);
> +}
> +
> static u32 mtk_dp_hwirq_get_clear(struct mtk_dp *mtk_dp)
> {
> u32 irq_status = (mtk_dp_read(mtk_dp, MTK_DP_TRANS_P0_3418) &
> @@ -2031,8 +2037,8 @@ static irqreturn_t mtk_dp_hpd_event(int hpd, void *dev)
> spin_unlock_irqrestore(&mtk_dp->irq_thread_lock, flags);
>
> if (cable_sta_chg) {
> - if (!!(mtk_dp_read(mtk_dp, MTK_DP_TRANS_P0_3414) &
> - HPD_DB_DP_TRANS_P0_MASK))
> + if (!(mtk_dp_read(mtk_dp, MTK_DP_TRANS_P0_3414) &
> + HPD_DB_DP_TRANS_P0_MASK))
Why do you change this.
Original code works normally.
This change may cause problem.
> mtk_dp->train_info.cable_plugged_in = true;
> else
> mtk_dp->train_info.cable_plugged_in = false;
> @@ -2252,7 +2258,7 @@ static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux,
> to_access, &msg->reply);
>
> if (ret) {
> - dev_info(mtk_dp->dev,
> + dev_dbg(mtk_dp->dev,
> "Failed to do AUX transfer: %d\n", ret);
This is error case. Do not use dev_dbg().
> goto err;
> }
> @@ -2265,6 +2271,35 @@ static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux,
> return ret;
> }
>
> +static void mtk_dp_swirq_hpd(struct mtk_dp *mtk_dp, u8 conn)
> +{
> + u32 data;
> +
> + data = mtk_dp_read(mtk_dp, MTK_DP_TRANS_P0_3414);
data is useless. Drop it.
> +
> + mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3414,
> + HPD_OVR_EN_DP_TRANS_P0_MASK,
> + HPD_OVR_EN_DP_TRANS_P0_MASK);
Where do you define HPD_OVR_EN_DP_TRANS_P0_MASK?
> +
> + if (conn)
> + mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3414,
> + HPD_SET_DP_TRANS_P0_MASK,
> + HPD_SET_DP_TRANS_P0_MASK);
Where do you define HPD_SET_DP_TRANS_P0_MASK?
> + else
> + mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3414,
> + 0,
> + HPD_SET_DP_TRANS_P0_MASK);
> +}
> +
> +static void mtk_dp_swirq_hpd_interrupt_set(struct mtk_dp *mtk_dp, u8 status)
> +{
> + dev_dbg(mtk_dp->dev, "[DPTX] status:%d [2:DISCONNECT, 4:CONNECT]\n", status);
> +
> + mtk_dp_swirq_hpd(mtk_dp, status == MTK_DP_HPD_CONNECT ? TRUE : FALSE);
> + mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_35C0, status,
> + SW_IRQ_SET_DP_TRANS_P0_MASK);
> +}
> +
> static int mtk_dp_poweron(struct mtk_dp *mtk_dp)
> {
> int ret;
> @@ -2326,7 +2361,13 @@ static int mtk_dp_bridge_attach(struct drm_bridge *bridge,
> if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP) {
> irq_clear_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
> enable_irq(mtk_dp->irq);
> + /* eDP does use HW IRQs */
'mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP' means here is DP.
The comment is wrong.
> mtk_dp_hwirq_enable(mtk_dp, true);
> + mtk_dp_swirq_enable(mtk_dp, false);
> + } else {
> + /* DP does use SW IRQs */
Here is eDP.
eDP does not need IRQ.
If you want to use IRQ, add comment to describe why you need.
> + mtk_dp_hwirq_enable(mtk_dp, false);
> + mtk_dp_swirq_enable(mtk_dp, true);
> }
>
> return 0;
> @@ -2534,7 +2575,7 @@ static int mtk_dp_bridge_atomic_check(struct drm_bridge *bridge,
>
> dev_dbg(mtk_dp->dev, "input format 0x%04x, output format 0x%04x\n",
> bridge_state->input_bus_cfg.format,
> - bridge_state->output_bus_cfg.format);
> + bridge_state->output_bus_cfg.format);
This is a cleanup modification.
This is not related to the title.
If you want to modify this, collect all cleanup in this file and separate to another patch.
>
> if (input_bus_format == MEDIA_BUS_FMT_YUYV8_1X16)
> mtk_dp->info.format = DP_PIXELFORMAT_YUV422;
> @@ -2552,6 +2593,30 @@ static int mtk_dp_bridge_atomic_check(struct drm_bridge *bridge,
> return 0;
> }
>
> +static void mtk_dp_bridge_hpd_notify(struct drm_bridge *bridge,
> + enum drm_connector_status status)
Why add this.
Original code could work normally.
If this is not related to the title, separate this to another patch.
> +{
> + struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> + struct mtk_dp_train_info *train_info = &mtk_dp->train_info;
> +
> + if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP) {
> + if (mtk_dp->hpd_state != status) {
> + if (status == connector_status_disconnected) {
> + train_info->cable_plugged_in = false;
> + } else {
> + mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3414,
> + HPD_OVR_EN_DP_TRANS_P0_MASK,
> + HPD_OVR_EN_DP_TRANS_P0_MASK);
> + mtk_dp_update_bits(mtk_dp, MTK_DP_TRANS_P0_3414,
> + HPD_SET_DP_TRANS_P0_MASK,
> + HPD_SET_DP_TRANS_P0_MASK);
> + train_info->cable_plugged_in = true;
> + }
> + mtk_dp->hpd_state = status;
> + }
> + }
> +}
> +
> static const struct drm_bridge_funcs mtk_dp_bridge_funcs = {
> .atomic_check = mtk_dp_bridge_atomic_check,
> .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> @@ -2566,6 +2631,7 @@ static const struct drm_bridge_funcs mtk_dp_bridge_funcs = {
> .mode_valid = mtk_dp_bridge_mode_valid,
> .edid_read = mtk_dp_edid_read,
> .detect = mtk_dp_bdg_detect,
> + .hpd_notify = mtk_dp_bridge_hpd_notify,
> };
>
> static void mtk_dp_debounce_timer(struct timer_list *t)
> @@ -2800,9 +2866,6 @@ 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
> @@ -2861,10 +2924,15 @@ static int mtk_dp_suspend(struct device *dev)
> struct mtk_dp *mtk_dp = dev_get_drvdata(dev);
>
> mtk_dp_power_disable(mtk_dp);
> - if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP)
> +
> + if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP) {
eDP does not need IRQ.
If you want to use IRQ, add comment to describe why you need.
> mtk_dp_hwirq_enable(mtk_dp, false);
> - pm_runtime_put_sync(dev);
> + } else {
> + mtk_dp_swirq_hpd_interrupt_set(mtk_dp, MTK_DP_HPD_DISCONNECT);
> + mtk_dp_swirq_enable(mtk_dp, false);
> + }
>
> + pm_runtime_put_sync(dev);
> return 0;
> }
>
> @@ -2874,8 +2942,14 @@ static int mtk_dp_resume(struct device *dev)
>
> pm_runtime_get_sync(dev);
> mtk_dp_init_port(mtk_dp);
> - if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP)
> +
> + if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP) {
> mtk_dp_hwirq_enable(mtk_dp, true);
eDP does not need IRQ.
If you want to use IRQ, add comment to describe why you need.
Regards,
CK
> + } else {
> + mtk_dp_swirq_hpd_interrupt_set(mtk_dp, MTK_DP_HPD_CONNECT);
> + mtk_dp_swirq_enable(mtk_dp, true);
> + }
> +
> mtk_dp_power_enable(mtk_dp);
>
> return 0;
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp_reg.h b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> index 8ad7a9cc259e..7c97e230be50 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> @@ -286,7 +286,10 @@
> #define POST_MISC_DATA_LANE1_OV_DP_TRANS_P0_MASK BIT(9)
> #define POST_MISC_DATA_LANE2_OV_DP_TRANS_P0_MASK BIT(10)
> #define POST_MISC_DATA_LANE3_OV_DP_TRANS_P0_MASK BIT(11)
> +#define MTK_DP_TRANS_P0_35C0 0x35c0
> +#define MTK_DP_TRANS_P0_35C4 0x35c4
> #define MTK_DP_TRANS_P0_35C8 0x35c8
> +#define SW_IRQ_SET_DP_TRANS_P0_MASK GENMASK(15, 0)
> #define SW_IRQ_CLR_DP_TRANS_P0_MASK GENMASK(15, 0)
> #define SW_IRQ_STATUS_DP_TRANS_P0_MASK GENMASK(15, 0)
> #define MTK_DP_TRANS_P0_35D0 0x35d0
> --
> 2.23.0
>
</pre>
</p></body></html><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice
********************
The information contained in this e-mail message (including any
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be
conveyed only to the designated recipient(s). Any use, dissemination,
distribution, printing, retaining or copying of this e-mail (including its
attachments) by unintended recipient(s) is strictly prohibited and may
be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender
immediately (by replying to this e-mail), delete any and all copies of
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->