Re: [PATCH 1/3] drm/nouveau: select HDMI deep-color link depth

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/nouveau: incorrect TMDS character rate calculation for HDMI deep color breaks SCDC scrambling
- [Medium] drm/nouveau: HDMI deep color permanently disabled on non-HDMI connectors
--

commit e40b56c72a75d42642f11bb0711a44b65e65a10b
Author: Daniel Campos Ramos <Capitain_Jack@yahoo.com>

drm/nouveau: select HDMI deep-color link depth

The patch enables HDMI deep-color modes by attaching an 8 to 16 max bpc
property to HDMI connectors. It then selects the appropriate 10, 12, or
16 bpc output depth if the sink advertises support and the resulting TMDS
character rate fits within the existing link limits.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index b0b0ad9a0c248..240a7b96cc519 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
[ ... ]
> @@ -298,6 +298,9 @@ nouveau_conn_attach_properties(struct drm_connector *connector)
>  		drm_object_attach_property(&connector->base, dev->mode_config.
>  					   dvi_i_subconnector_property, 0);
>  
> +	if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA)
> +		drm_connector_attach_max_bpc_property(connector, 8, 16);

[Severity: Medium]
Does this prevent HDMI deep color from being enabled on DVI or DP connectors
that are driving an HDMI sink via a passive adapter?

If the max bpc property is only attached for HDMIA connectors, then for other
connector types driving an HDMI sink, the max_requested_bpc state remains 0.
Later in nv50_outp_atomic_fix_depth(), the deep color promotion checks
(like conn_state->max_requested_bpc >= 12) will fail, locking these
configurations to 8 bpc despite both sink and GPU supporting higher depths.

> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 2c66e480b5116..9e8bfd6b78ec0 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
[ ... ]
> @@ -1776,6 +1808,27 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_commit *st
>  
>  	switch (nv_encoder->dcb->type) {
>  	case DCB_OUTPUT_TMDS:
> +		switch (asyh->or.bpc) {
> +		case 16:
> +			depth = NV837D_SOR_SET_CONTROL_PIXEL_DEPTH_BPP_48_444;
> +			break;
> +		case 12:
> +			depth = NV837D_SOR_SET_CONTROL_PIXEL_DEPTH_BPP_36_444;
> +			break;
> +		case 10:
> +			depth = NV837D_SOR_SET_CONTROL_PIXEL_DEPTH_BPP_30_444;
> +			break;
> +		case 8:
> +			depth = NV837D_SOR_SET_CONTROL_PIXEL_DEPTH_BPP_24_444;
> +			break;
> +		case 6:
> +			depth = NV837D_SOR_SET_CONTROL_PIXEL_DEPTH_BPP_18_444;
> +			break;
> +		default:
> +			depth = NV837D_SOR_SET_CONTROL_PIXEL_DEPTH_DEFAULT;
> +			break;
> +		}
> +
>  		if (disp->disp->object.oclass != NV50_DISP &&
>  		    nv_connector->base.display_info.is_hdmi)
>  			nv50_hdmi_enable(encoder, nv_crtc, nv_connector, state, mode, hda);

[Severity: High]
Will enabling deep color here cause SCDC scrambling to fail for high-bandwidth
modes?

The HDMI 2.0 specification requires TMDS scrambling and a 1/40 bit clock
ratio when the TMDS character rate exceeds 340 MHz. However, when calling
nv50_hdmi_enable(), the existing code calculates the high TMDS clock
ratio using the pixel clock rather than the new TMDS character rate:

drivers/gpu/drm/nouveau/dispnv50/disp.c:nv50_hdmi_enable() {
    ...
    const bool high_tmds_clock_ratio = mode->clock > 340000;
    ...
    ret = nvif_outp_hdmi(&nv_encoder->outp, nv_crtc->index, true,
                         max_ac_packet, rekey, mode->clock, ...);
    ...
}

For high-bandwidth modes like 4K 30Hz at 12 bpc, the pixel clock is 297 MHz
but the TMDS rate is 445.5 MHz. In this scenario, high_tmds_clock_ratio
evaluates to false and the unscaled pixel clock is passed to the hardware
sequencer.

Could this cause the driver to fail to enable SCDC scrambling and the required
clock ratio, resulting in a display link training failure and a black screen?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922192132.114546-1-Capitain_Jack@yahoo.com?part=1