Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The function `sink_supports_format_bpc` explicitly permits 16 BPC for RGB444 and YCBCR444 formats without verifying if the sink's EDID actually supports 48-bit deep color. -- commit 767fa3cc832e8b93f697ad91bcc66e5b3b1ed85d Author: Nicolas Frattaroli <[email protected]> drm/display: hdmi: Only allow BPC values of 8, 10, 12 and 16 As per the comment in sink_supports_format_bpc(), CTA-861-F defines that only bits-per-channel values of 8, 10, 12 and 16 are allowed for HDMI. Allowing more than this has surprising consequences for the atomic check phase. > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > index 4867edbf2622b..b06499dbfe8b3 100644 > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > @@ -419,6 +419,16 @@ sink_supports_format_bpc(const struct drm_connector > *connector, > return false; > } > > + switch (bpc) { > + case 8: > + case 10: > + case 12: > + case 16: [Severity: High] This is a pre-existing issue, but does explicitly allowing 16 BPC here expose a missing validation check for 48-bit deep color further down in sink_supports_format_bpc()? When checking DRM_OUTPUT_COLOR_FORMAT_RGB444 and DRM_OUTPUT_COLOR_FORMAT_YCBCR444, the code validates 10 BPC and 12 BPC against the EDID flags, but completely omits checking 16 BPC against DRM_EDID_HDMI_DC_48: drivers/gpu/drm/display/drm_hdmi_state_helper.c:sink_supports_format_bpc() { ... case DRM_OUTPUT_COLOR_FORMAT_RGB444: ... if (bpc == 10 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30)) { drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n"); return false; } if (bpc == 12 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36)) { drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n"); return false; } drm_dbg_kms(dev, "RGB format supported in that configuration.\n"); return true; ... } If the TMDS clock limit permits it, the driver might incorrectly conclude that the sink supports 16 BPC and program the hardware accordingly, which could result in a black screen if the monitor doesn't actually support 48-bit deep color. Should a check for (info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_48) be added for the RGB444 and YCBCR444 cases? > + break; > + default: > + return false; > + } > + > if (!info->is_hdmi && > (format != DRM_OUTPUT_COLOR_FORMAT_RGB444 || bpc != 8)) { > drm_dbg_kms(dev, "DVI Monitors require an RGB output at 8 > bpc\n"); -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
