Message-ID: <b68237812ed3251114b4df3c27b82ee1fe9243e3.camel@mediatek.com>
Date: Fri, 22 Aug 2025 12:06:47 +0000
From: LIANKUN YANG (杨连坤) <Liankun.Yang@...iatek.com>
To: Peng Liu (刘鹏) <Peng.Liu@...iatek.com>,
	Mac Shen (沈俊) <Mac.Shen@...iatek.com>,
	"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>, "simona@...ll.ch"
	<simona@...ll.ch>, "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
	CK Hu (胡俊光) <ck.hu@...iatek.com>,
	Project_Global_Chrome_Upstream_Group
	<Project_Global_Chrome_Upstream_Group@...iatek.com>, "airlied@...il.com"
	<airlied@...il.com>, "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
CC: "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-mediatek@...ts.infradead.org"
	<linux-mediatek@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 1/1] drm/mediatek: Adjust bandwidth limit for DP

On Wed, 2025-08-06 at 05:41 +0000, CK Hu (胡俊光) wrote:
> On Mon, 2025-06-30 at 15:58 +0800, Liankun Yang wrote:
> > By adjusting the order of link training and relocating it to HPD,
> > link training can identify the usability of each lane in the
> > current link.
> > 
> > It also supports handling signal instability and weakness due to
> > environmental issues, enabling the acquisition of a stable
> > bandwidth
> > for the current link. Subsequently, DP work can proceed based on
> > the actual maximum bandwidth.
> > 
> > It should training in the hpd event thread.
> > Check the mode with lane count and link rate of training.
> > 
> > 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
> > 
> > Therefore, bridge typec judgment is required for edp training in
> > atomic_enable function.
> > 
> > Because DP will enter idle pattern state after DP training,the
> > current
> > DP link state is ready and can be prepared to output the video.
> > Therefore, the following actions do not require the wall-mounted
> > aux_panel power, and set DP enable to true.
> 
> In v4, Nicolas has point out that DP has already broken.
> Does this patch fix the broken?
> If this patch does not fix the broken, I would like you to fix the
> broken first then apply this patch.
> If you does not see the broken problem, try to discuss with Nicolas
> and help him to fix it.
> 

Yes, this patch has fixed the broken.

> > 
> > Signed-off-by: Liankun Yang <liankun.yang@...iatek.com>
> > ---
> > Change in V5:
> > - Fixed the issue that the 4th version of the patch caused DP to
> > have no screen.
> > Per suggestion from the previous thread:
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20250625095446.31726-1-liankun.yang@mediatek.com/__;!!CTRNKA9wMg0ARbw!jqgxw9-Ej1AVN-yGpx2hOm4q2tz4p6AiFAcVXGuETgpdttAlmeIXSqsQx563gSeJ77Mlv9sVjTAFKRfSpKnjXJY$
> >  
> > 
> > Change in V4:
> > - Tested the internal eDP display on MT8195 Tomato and it is fine.
> > Per suggestion from the previous thread:
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20250318140236.13650-2-liankun.yang@mediatek.com/__;!!CTRNKA9wMg0ARbw!jqgxw9-Ej1AVN-yGpx2hOm4q2tz4p6AiFAcVXGuETgpdttAlmeIXSqsQx563gSeJ77Mlv9sVjTAFKRfS05c9fJ4$
> >  
> > 
> > Change in V3:
> > - Remove 'mtk_dp->enabled = false" in atomic disable.
> > - Remove 'mtk_dp->enabled = true" in atomic enable.
> > Per suggestion from the previous thread:
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20241025083036.8829-4-liankun.yang@mediatek.com/__;!!CTRNKA9wMg0ARbw!jqgxw9-Ej1AVN-yGpx2hOm4q2tz4p6AiFAcVXGuETgpdttAlmeIXSqsQx563gSeJ77Mlv9sVjTAFKRfSER8KuRc$
> >  
> > 
> > Change in V2:
> > - Adjust DP training timing.
> > - Adjust parse capabilities timing.
> > - Add power on/off for connect/disconnect.
> > Per suggestion from the previous thread:
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20240315015233.2023-1-liankun.yang@mediatek.com/__;!!CTRNKA9wMg0ARbw!jqgxw9-Ej1AVN-yGpx2hOm4q2tz4p6AiFAcVXGuETgpdttAlmeIXSqsQx563gSeJ77Mlv9sVjTAFKRfSQiQTSO4$
> >  
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dp.c | 45 ++++++++++++++++++---------
> > ----
> >  1 file changed, 27 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> > b/drivers/gpu/drm/mediatek/mtk_dp.c
> > index a5b10b2545dc..1010263f2510 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> > @@ -1976,6 +1976,7 @@ static irqreturn_t
> > mtk_dp_hpd_event_thread(int hpd, void *dev)
> >  	struct mtk_dp *mtk_dp = dev;
> >  	unsigned long flags;
> >  	u32 status;
> > +	int ret;
> >  
> >  	if (mtk_dp->need_debounce && mtk_dp-
> > >train_info.cable_plugged_in)
> >  		msleep(100);
> > @@ -1994,9 +1995,28 @@ static irqreturn_t
> > mtk_dp_hpd_event_thread(int hpd, void *dev)
> >  			memset(&mtk_dp->info.audio_cur_cfg, 0,
> >  			       sizeof(mtk_dp->info.audio_cur_cfg));
> >  
> > +			mtk_dp->enabled = false;
> > +			/* 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->need_debounce = false;
> >  			mod_timer(&mtk_dp->debounce_timer,
> >  				  jiffies + msecs_to_jiffies(100) - 1);
> > +		} else {
> > +			mtk_dp_aux_panel_poweron(mtk_dp, true);
> 
> You power on panel here. It seems that it does not need power on in
> atomic enable. Right?
> In addition, when plug in, you power on aux and panel, but when plug
> out, you just power off aux.
> This is not symmetric. I would like it to be symmetric.
> 

No, it need power on for eDP in atomic enable. Patch update in the
sixth version.

When plug out, power off aux and panel in atomic disable.

> > +
> > +			ret = mtk_dp_parse_capabilities(mtk_dp);
> > +			if (ret)
> > +				drm_err(mtk_dp->drm_dev, "Can't parse
> > capabilities\n");
> > +
> > +			/* Training */
> > +			ret = mtk_dp_training(mtk_dp);
> > +			if (ret)
> > +				drm_err(mtk_dp->drm_dev, "Training
> > failed, %d\n", ret);
> > +
> > +			mtk_dp->enabled = true;
> >  		}
> >  	}
> >  
> > @@ -2161,17 +2181,6 @@ static const struct drm_edid
> > *mtk_dp_edid_read(struct drm_bridge *bridge,
> >  	}
> >  
> >  	drm_edid = drm_edid_read_ddc(connector, &mtk_dp->aux.ddc);
> > -
> > -	/*
> > -	 * Parse capability here to let atomic_get_input_bus_fmts and
> > -	 * mode_valid use the capability to calculate sink bitrates.
> > -	 */
> > -	if (mtk_dp_parse_capabilities(mtk_dp)) {
> 
> Where does eDP parse capabilities?
> Only DP would trigger hdp event thread.
> 

Patch update in the sixth version.

> > -		drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n");
> > -		drm_edid_free(drm_edid);
> > -		drm_edid = NULL;
> > -	}
> > -
> >  	if (drm_edid) {
> >  		/*
> >  		 * FIXME: get rid of drm_edid_raw()
> > @@ -2366,11 +2375,13 @@ static void
> > mtk_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> >  
> >  	mtk_dp_aux_panel_poweron(mtk_dp, true);
> >  
> > -	/* Training */
> > -	ret = mtk_dp_training(mtk_dp);
> > -	if (ret) {
> > -		drm_err(mtk_dp->drm_dev, "Training failed, %d\n", ret);
> > -		goto power_off_aux;
> > +	if (mtk_dp->data->bridge_type == DRM_MODE_CONNECTOR_eDP) {
> > +		/* Training */
> > +		ret = mtk_dp_training(mtk_dp);
> > +		if (ret) {
> > +			drm_err(mtk_dp->drm_dev, "Training failed,
> > %d\n", ret);
> > +			goto power_off_aux;
> > +		}
> 
> For DP, you move training to hdp event thread.
> In you commit message, I could not understand why do this.
> Does this is timing problem?
> If this is a timing problem, I would like you to describe what's
> wrong in timing.
> 
> For example, this is a time sequence 
> 
> 1. plug in panel A
> 2. atomic enable (training)
> 3. plug out panel A
> 4. plug in panel B
> 
> But panel B does not training again, so this patch make it as
> 
> 1. plug in panel A (training)
> 2. atomic enable
> 3. plug out panel A
> 4. plug in panel B (training)
> 
> If it is this case, add this information to commit message.
> If it is not this case, try to let me understand why moving training
> timing would help adjust bandwidth limit.
> 
> Regards,
> CK
> 

For example, the platform support DP 4lane 5.4G,
but panel A support DP 2lane 5.4G.

This is a time sequence:

1. Plug in panel A. According to the platform, it can output 4K 60Hz.
2. Timing mode set 4K 60Hz.
3. Atomic enable(Based on panel A ability, training pass 2lane 5.4G).
4. Finally, due to 2lane 5.4G bandwidth limitation, the platform cannot
output 4K 60Hz, resulting in a black sreen.

If apply this patch.

1. Plug in panel A.
2. Training pass 2lane 5.4G
3. Timing mode set 2K 60Hz(Based on the 2lane 5.4G bandwidth limit).
4. Atomic enable for output screen.

Best Regards,
Liankun

> >  	}
> >  
> >  	ret = mtk_dp_video_config(mtk_dp);
> > @@ -2390,7 +2401,6 @@ static void
> > mtk_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> >  		       sizeof(mtk_dp->info.audio_cur_cfg));
> >  	}
> >  
> > -	mtk_dp->enabled = true;
> >  	mtk_dp_update_plugged_status(mtk_dp);
> >  
> >  	return;
> > @@ -2405,7 +2415,6 @@ static void
> > mtk_dp_bridge_atomic_disable(struct drm_bridge *bridge,
> >  {
> >  	struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> >  
> > -	mtk_dp->enabled = false;
> >  	mtk_dp_update_plugged_status(mtk_dp);
> >  	mtk_dp_video_enable(mtk_dp, false);
> >  	mtk_dp_audio_mute(mtk_dp, true);
> 
> 

Powered by blists - more mailing lists