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 <[email protected]> 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/[email protected]?part=1
