| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c10ff37d3730bb1d434958d4d7f7002730fb117e.camel@mediatek.com>
Date: Mon, 11 Aug 2025 09:28:30 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "towwy321@...il.com" <towwy321@...il.com>, "chunkuang.hu@...nel.org"
<chunkuang.hu@...nel.org>, "simona@...ll.ch" <simona@...ll.ch>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
"airlied@...il.com" <airlied@...il.com>, "msp@...libre.com"
<msp@...libre.com>, "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>, "granquet@...libre.com"
<granquet@...libre.com>
CC: Rex-BC Chen (陳柏辰) <Rex-BC.Chen@...iatek.com>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] drm/mediatek: dp: Fix suspend/resume training failure
On Sun, 2025-08-10 at 14:17 +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.
This patch just modify the flow of DP, not eDP, so do not mention eDP here.
For DP, I don't know why hardware IRQ would fail.
HDMI support suspend and resume and it does not software IRQ.
Please describe the hardware behavior when resume and point out why hardware would not trigger hardware hot plug interrupt.
> 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@...il.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 for eDP and DP
> - Fixed hotplug check logic in mtk_dp_hpd_event
>
> Changes since v2:
> - Dropped mtk_dp_bridge_hpd_notify() in favor of mtk_dp_altmode_hpd_notify()
> - Removed IRQ enabling from probe() for eDP
> - Restricted SW IRQ enabling to DP only, in bridge_attach()
> - Corrected cable_plugged_in logic in mtk_dp_hpd_event()
> - Cleaned up unused variables in mtk_dp_swirq_hpd()
> - Updated comments to reflect eDP/DP IRQ separation
> ---
> drivers/gpu/drm/mediatek/mtk_dp.c | 81 ++++++++++++++++++++++++---
> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 5 ++
> 2 files changed, 77 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 33cf9fe1ccde..5fdb319ec62c 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) &
> @@ -2265,6 +2271,31 @@ 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)
> +{
> + 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);
> +
> + 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);
> + 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;
> @@ -2324,9 +2355,9 @@ static int mtk_dp_bridge_attach(struct drm_bridge *bridge,
> mtk_dp->drm_dev = bridge->dev;
>
> 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);
> + /* DP does use SW IRQs */
> + mtk_dp_hwirq_enable(mtk_dp, false);
> + mtk_dp_swirq_enable(mtk_dp, true);
> }
>
> return 0;
> @@ -2534,7 +2565,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);
>
> if (input_bus_format == MEDIA_BUS_FMT_YUYV8_1X16)
> mtk_dp->info.format = DP_PIXELFORMAT_YUV422;
> @@ -2552,6 +2583,30 @@ static int mtk_dp_bridge_atomic_check(struct drm_bridge *bridge,
> return 0;
> }
>
> +static void mtk_dp_altmode_hpd_notify(struct drm_bridge *bridge,
> + enum drm_connector_status status)
> +{
> + 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) {
Where do you define hpd_state?
> + 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 +2621,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_altmode_hpd_notify,
Why need this?
What is the callstack to this?
I mean what is the original source to call into this function?
There is another HDP detect source and it would send HPD event and call into this function?
Why not use DP hardware as HDP detection source?
> };
>
> static void mtk_dp_debounce_timer(struct timer_list *t)
> @@ -2861,10 +2917,13 @@ 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)
> - mtk_dp_hwirq_enable(mtk_dp, false);
> - pm_runtime_put_sync(dev);
>
> + if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP) {
> + 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 +2933,12 @@ 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)
> - mtk_dp_hwirq_enable(mtk_dp, true);
> +
> + if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP) {
> + mtk_dp_swirq_hpd_interrupt_set(mtk_dp, MTK_DP_HPD_CONNECT);
Does this mean the connect status is CONNECTED?
Why resume is connected?
If the cable is plug out and system resume, it does not connect.
Describe what does this function do.
Regards,
CK
> + 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..c9a022ffd0ac 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> +++ b/drivers/gpu/drm/mediatek/mtk_dp_reg.h
> @@ -262,6 +262,8 @@
> #define HPD_DISC_THD_DP_TRANS_P0_MASK GENMASK(11, 8)
> #define HPD_CONN_THD_DP_TRANS_P0_MASK GENMASK(15, 12)
> #define MTK_DP_TRANS_P0_3414 0x3414
> +#define HPD_OVR_EN_DP_TRANS_P0_MASK BIT(0)
> +#define HPD_SET_DP_TRANS_P0_MASK BIT(1)
> #define HPD_DB_DP_TRANS_P0_MASK BIT(2)
> #define MTK_DP_TRANS_P0_3418 0x3418
> #define IRQ_CLR_DP_TRANS_P0_MASK GENMASK(3, 0)
> @@ -286,7 +288,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
>
Powered by blists - more mailing lists