Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Missing DP-to-HDMI PCON capability check for deep color configuration causes atomic check failures. - [Medium] PCON detection logic fails to correctly identify HDMI ports on MST hubs, excluding them from deep color support. --
commit 2c95bd6347ca4e92cbbbdffd678f070e97b84a9d Author: Stephen <[email protected]> drm/amd/display: use HDMI RGB deep color for PCONs This commit updates the DRM AMD display driver to support HDMI RGB deep color for PCON connectors. It derives the candidate bpc from edid_hdmi_rgb444_dc_modes when an RGB output is routed through a recognized DP-to-HDMI converter. > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index f8c13bad4ac23..02ffc1563e699 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c [ ... ] > @@ -6582,22 +6582,44 @@ static void update_stream_scaling_settings(struct > drm_device *dev, > > static enum dc_color_depth > convert_color_depth_from_display_info(const struct drm_connector *connector, > - bool is_y420, int requested_bpc) > + enum dc_pixel_encoding pixel_encoding, > + int requested_bpc) > { > + const struct drm_display_info *info = &connector->display_info; > + bool is_hdmi_pcon = false; > u8 bpc; > > - if (is_y420) { > + if (connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) { > + const struct amdgpu_dm_connector *aconnector = > + to_amdgpu_dm_connector(connector); > + > + is_hdmi_pcon = aconnector->dc_link && > + aconnector->dc_link->dpcd_caps.dongle_type == > + DISPLAY_DONGLE_DP_HDMI_CONVERTER; [Severity: Medium] For MST leaf connectors, aconnector->dc_link represents the primary link to the MST hub rather than the downstream leaf port. Since the root link typically identifies as a branch device, will this evaluate to false for MST setups and exclude HDMI sinks connected via MST hubs from deep color support? > + } > + > + if (pixel_encoding == PIXEL_ENCODING_YCBCR420) { > bpc = 8; > > /* Cap display bpc based on HDMI 2.0 HF-VSDB */ > - if (connector->display_info.hdmi.y420_dc_modes & > DRM_EDID_YCBCR420_DC_48) > + if (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_48) > + bpc = 16; > + else if (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_36) > + bpc = 12; > + else if (info->hdmi.y420_dc_modes & DRM_EDID_YCBCR420_DC_30) > + bpc = 10; > + } else if (pixel_encoding == PIXEL_ENCODING_RGB && is_hdmi_pcon) { > + bpc = 8; > + > + /* The downstream sink is HDMI even though DRM exposes DP. */ > + if (info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_48) > bpc = 16; > - else if (connector->display_info.hdmi.y420_dc_modes & > DRM_EDID_YCBCR420_DC_36) > + else if (info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36) > bpc = 12; > - else if (connector->display_info.hdmi.y420_dc_modes & > DRM_EDID_YCBCR420_DC_30) > + else if (info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30) > bpc = 10; [Severity: High] This logic sets the bpc based on the downstream HDMI sink's capabilities, but does it also need to be constrained by the PCON's maximum bpc limit? If a sink supports 12 bpc but the PCON only supports 8 bpc, will skipping a check against aconnector->dc_link->dpcd_caps.dongle_caps.dp_hdmi_max_bpc cause dp_active_dongle_validate_timing() to fail and reject the atomic commit? > } else { > - bpc = (uint8_t)connector->display_info.bpc; > + bpc = (uint8_t)info->bpc; > /* Assume 8 bpc by default if no bpc is specified. */ > bpc = bpc ? bpc : 8; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
