dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jackie Dong <xy-jackie@139.com>
To: sashiko-reviews@lists.linux.dev
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/display:Check dp_128b_132b_supported_link_rates.raw before link_rate setting
Date: Wed, 01 Jul 2026 09:45:50 +0800	[thread overview]
Message-ID: <792c4a4e145a7aaa289b6d3bc8bb8c6996591152.camel@139.com> (raw)
In-Reply-To: <20260630151147.CB9631F000E9@smtp.kernel.org>

On Tue, 2026-06-30 at 15:11 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [High] Checking `dp_128b_132b_supported_link_rates.raw` before it
> is read from the DPCD register completely disables DP 2.0 UHBR
> support.
> --
> 
> commit 11d06153280ec5dae6682a402606818b4f84b79d
> Author: Jackie Dong <xy-jackie@139.com>
> 
> drm/amd/display:Check dp_128b_132b_supported_link_rates.raw before
> link_rate setting
> 
> This patch modifies the DP 128b/132b capability check to also verify
> that
> dp_128b_132b_supported_link_rates.raw is non-zero before attempting
> to read
> and parse the specific UHBR link rates.
> 
> > diff --git
> > a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.
> > c
> > b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.
> > c
> > ---
> > a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.
> > c
> > +++
> > b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.
> > c
> > @@ -2079,7 +2079,8 @@ static bool retrieve_link_cap(struct dc_link
> > *link)
> >  	if (!dpcd_read_sink_ext_caps(link))
> >  		link->dpcd_sink_ext_caps.raw = 0;
> >  
> > -	if (link-
> > >dpcd_caps.channel_coding_cap.bits.DP_128b_132b_SUPPORTED) {
> > +	if (link-
> > >dpcd_caps.channel_coding_cap.bits.DP_128b_132b_SUPPORTED &&
> > +			link-
> > >dpcd_caps.dp_128b_132b_supported_link_rates.raw) {
> 
> [Severity: High]
> Will this change completely disable DP 2.0 UHBR support? 
> 
> In retrieve_link_cap() in
> drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c,
> dp_128b_132b_supported_link_rates.raw is now checked before it is
> actually 
> read from the DPCD register. Since the dc_link struct is zero-
> initialized and
> the read hasn't occurred yet, won't this raw value always evaluate to
> 0 upon
> connection of any DP 2.0 monitor? 
> 
> This would cause the if condition to always fail, bypassing the
> actual read
> block below and permanently preventing UHBR feature detection.
> 
Hi Sashiko,
 I have verified it as below. To add
dp_128b_132b_supported_link_rates.raw  as a new judgment doesn's impact
the value of dp_128b_132b_supported_link_rates.raw. If the monitor can
support UHBR and it can be into feature detection and assigned the
right UHBR rate.

root@yoga-Yoga-Pro-7-15ASH11:/media/yoga/Data/git/linux-502d801f0ab03-
20260622# grep -B23 "Invalid RX 128b_132b_supported_link_rates"
drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c
 
	//if (link-
>dpcd_caps.channel_coding_cap.bits.DP_128b_132b_SUPPORTED) {
	link->dpcd_caps.dp_128b_132b_supported_link_rates.raw = 0x10;
	pr_info("Jackie %x is SUPPORTED, %x: RX
128b_132b_supported_link_rates.raw\n", link-
>dpcd_caps.channel_coding_cap.bits.DP_128b_132b_SUPPORTED, link-
>dpcd_caps.dp_128b_132b_supported_link_rates.raw) ;
	//pr_info("Jackie  %x: RX
128b_132b_supported_link_rates.raw\n", link-
>dpcd_caps.dp_128b_132b_supported_link_rates.raw) ;
	//pr_info("Jackie RX 128b_132b_supported_link_rates.raw\n") ;
	if ( (link-
>dpcd_caps.channel_coding_cap.bits.DP_128b_132b_SUPPORTED) && (link-
>dpcd_caps.dp_128b_132b_supported_link_rates.raw) ) {
		DC_LOG_DP2("128b/132b encoding is supported at link
%d", link->link_index);
 
		/* Read 128b/132b suppoerted link rates */
		core_link_read_dpcd(link,
				DP_128B132B_SUPPORTED_LINK_RATES,
&link->dpcd_caps.dp_128b_132b_supported_link_rates.raw,
				sizeof(link-
>dpcd_caps.dp_128b_132b_supported_link_rates.raw));
		if (link-
>dpcd_caps.dp_128b_132b_supported_link_rates.bits.UHBR20) {
			pr_info("Jackie RX
128b_132b_supported_link_rates is UHBR20\n") ;
			link->reported_link_cap.link_rate =
LINK_RATE_UHBR20;
		} else if (link-
>dpcd_caps.dp_128b_132b_supported_link_rates.bits.UHBR13_5)
			link->reported_link_cap.link_rate =
LINK_RATE_UHBR13_5;
		else if (link-
>dpcd_caps.dp_128b_132b_supported_link_rates.bits.UHBR10)
			link->reported_link_cap.link_rate =
LINK_RATE_UHBR10;
		else
			//link->reported_link_cap.link_rate =
LINK_RATE_UHBR10;//Jackie Add
			dm_error("%s: Invalid RX
128b_132b_supported_link_rates\n", __func__);
root@yoga-Yoga-Pro-7-15ASH11:/media/yoga/Data/git/linux-502d801f0ab03-
20260622# dmesg |grep -i "fail\|error\|Jackie"
[    0.025954] KHO: Failed to reserve lowmem scratch buffer
[    0.025956] KHO: Failed to reserve scratch area, disabling kexec
handover
[    1.731523] RAS: Correctable Errors collector initialized.
[    7.629667] Jackie 1 is SUPPORTED, 10: RX
128b_132b_supported_link_rates.raw
[    7.630067] [drm:retrieve_link_cap [amdgpu]] *ERROR*
retrieve_link_cap: Invalid RX 128b_132b_supported_link_rates

Above is my verified source code and test result, FYR.

Thanks for your comments.

> >  		DC_LOG_DP2("128b/132b encoding is supported at
> > link %d", link->link_index);
> >  
> >  		/* Read 128b/132b suppoerted link rates */
> >  		core_link_read_dpcd(link,
> >  				DP_128B132B_SUPPORTED_LINK_RATES,
> >  				&link-
> > >dpcd_caps.dp_128b_132b_supported_link_rates.raw,
> 



  reply	other threads:[~2026-07-01  7:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30 14:59 [PATCH] drm/amd/display:Check dp_128b_132b_supported_link_rates.raw before link_rate setting Jackie Dong
2026-06-30 15:11 ` sashiko-bot
2026-07-01  1:45   ` Jackie Dong [this message]
2026-07-17 20:15 ` George Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=792c4a4e145a7aaa289b6d3bc8bb8c6996591152.camel@139.com \
    --to=xy-jackie@139.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox