All of 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: 3+ 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]

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.