Re: [PATCH RFC/DO NOT MERGE 09/12] drm/msm/hdmi: Add HDMI for Eliza SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The TMDS rate validation for platforms with OPP tables accepts inexact pixel clocks by skipping clock synthesizability checks.
--

commit e2e2d112c3fa04206001b8a9cc56acdda995a731
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

drm/msm/hdmi: Add HDMI for Eliza SoC
    
This commit adds HDMI support for the Qualcomm Eliza SoC (DPU v12.4). It
introduces configuration for INTF_11 with new interrupt registers and handles
Eliza's distinct clock and power requirements.

> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index 17e81628525a9..b5353a1688ddf 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c

[ ... ]

> @@ -459,21 +490,47 @@ static enum drm_mode_status msm_hdmi_bridge_tmds_char_rate_valid(const struct dr
>  	 * mdp4/dtv stuff where pixel clk is assigned to mdp/encoder
>  	 * instead):
>  	 */
> -	if (kms->funcs->round_pixclk)
> +	if (kms->funcs->round_pixclk) {
>  		actual = kms->funcs->round_pixclk(kms,
>  						  tmds_rate,
>  						  hdmi_bridge->hdmi->encoder);
> -	else if (hdmi->extp_clk)
> -		actual = clk_round_rate(hdmi->extp_clk, tmds_rate);
> -	else
> -		actual = tmds_rate;
> -
> -	DBG("requested=%lld, actual=%ld", tmds_rate, actual);
> -
> -	if (actual != tmds_rate)
> -		return MODE_CLOCK_RANGE;
> -
> -	return 0;
> +		DBG("requested=%lld, actual=%ld", tmds_rate, actual);
> +
> +		if (actual != tmds_rate)
> +			return MODE_CLOCK_RANGE;
> +		return MODE_OK;
> +	} else if (!dev_pm_opp_get_opp_count(&hdmi->pdev->dev)) {
> +		if (hdmi->extp_clk)
> +			actual = clk_round_rate(hdmi->extp_clk, tmds_rate);
> +		else
> +			actual = tmds_rate;
> +
> +		if (actual != tmds_rate)
> +			return MODE_CLOCK_RANGE;
> +		return MODE_OK;
> +	} else {
> +		struct dev_pm_opp *opp;
> +		unsigned long pclk_rate = tmds_rate;
> +
> +		if (mode->clock > 600000)
> +			return MODE_CLOCK_HIGH;
> +
> +		opp = dev_pm_opp_find_freq_ceil(&hdmi->pdev->dev, &pclk_rate);
> +		if (!IS_ERR(opp)) {
> +			dev_pm_opp_put(opp);
> +		} else if (PTR_ERR(opp) == -ERANGE) {
> +			/*
> +			 * An empty table is created by devm_pm_opp_set_clkname() even
> +			 * if there is none. Thus find_freq_ceil will still return
> +			 * -ERANGE in such case.
> +			 */
> +			return MODE_CLOCK_RANGE;
> +		} else {
> +			return MODE_ERROR;
> +		}
> +
> +		return MODE_OK;

[Severity: High]
Does this bypass clock synthesizability checks for platforms with OPP tables?

The TMDS rate validation here accepts inexact pixel clocks because it skips
the exact match check that is enforced in the non-OPP path:

	if (hdmi->extp_clk)
		actual = clk_round_rate(hdmi->extp_clk, tmds_rate);
	...
	if (actual != tmds_rate)
		return MODE_CLOCK_RANGE;

When msm_hdmi_bridge_tmds_char_rate_valid() searches for an OPP entry, it
uses dev_pm_opp_find_freq_ceil() which finds a ceiling rather than an exact
match. Because it returns MODE_OK unconditionally upon finding a ceiling,
it can accept display modes that the hardware clock generator cannot
exactly synthesize.

Could this result in configuring an inaccurate pixel clock for the display,
leading to Out of Range errors or visual corruption?

> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-drm-msm-hdmi-eliza-v1-0-67843277de17@oss.qualcomm.com?part=9

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy