Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/msm/dp: late HDR capability check in atomic_enable violates atomic contract --
commit dd06afce910f68ea34833e6875a317b2680f0949 Author: Xilin Wu <[email protected]> drm/msm/dp: Add static HDR metadata support for DP and eDP This commit adds static HDR metadata processing for DP and eDP displays. It introduces Generic2 packet formatting, revalidates sink capabilities, and forces a modeset on metadata changes. > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c [ ... ] > @@ -708,7 +718,7 @@ int msm_dp_display_atomic_check(struct msm_dp > *msm_dp_display, > const struct drm_display_info *info = > &conn_state->connector->display_info; > struct msm_dp_display_private *dp; > unsigned int clock = mode->clock; > - int bpp; > + int bpp, ret; > > dp = container_of(msm_dp_display, struct msm_dp_display_private, > msm_dp_display); > > @@ -717,17 +727,25 @@ int msm_dp_display_atomic_check(struct msm_dp > *msm_dp_display, > > guard(mutex)(&dp->plugged_lock); > > + ret = msm_dp_utils_pack_hdr_sdp(conn_state, &state->hdr_sdp); > + if (ret) > + return ret; > + > + state->hdr_enabled = !!conn_state->hdr_output_metadata; [Severity: High] Should the hardware capability validation for HDR be moved here? This is the correct function where the DP controller's hw_revision and all static sink HDR capability checks must be performed. Validating here ensures that userspace receives an error prior to the commit, rather than failing late during the enable phase. [ ... ] > @@ -760,6 +778,18 @@ static int msm_dp_display_prepare_link(struct > msm_dp_display_private *dp) > > dp->stream_pm_active = true; > > + if (dp->panel->hdr_enabled) { > + const struct drm_display_info *info = > &msm_dp_display->connector->display_info; > + const struct hdr_static_metadata *hdr = > &info->hdr_sink_metadata.hdmi_type1; > + u8 eotf = dp->panel->hdr_sdp.db[2]; > + > + if (dp->panel->hw_revision < DP_HW_VERSION_1_0 || > + !(hdr->metadata_type & BIT(HDMI_STATIC_METADATA_TYPE1)) || > + !(hdr->eotf & BIT(eotf)) || > + (eotf != HDMI_EOTF_TRADITIONAL_GAMMA_SDR && info->bpc < 10)) > + return -EINVAL; > + } > + [Severity: High] Does this late hardware capability validation violate the atomic contract? Returning an error during the atomic_enable phase (from msm_dp_display_prepare_link) causes a silent modesetting failure where the hardware state diverges from the software state, leading to a wedged display pipeline and DRM core warnings. If a local user with DRM master privileges submits an atomic commit with HDR metadata on a DP controller with an older hw_revision, the commit passes validation but fails here. The atomic modesetting contract requires all such configuration constraints to be rejected during msm_dp_display_atomic_check. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=13
