On Sat, 27 Jan 2024 at 02:58, Paloma Arellano <quic_parellan@xxxxxxxxxxx> wrote: > > > On 1/25/2024 1:23 PM, Dmitry Baryshkov wrote: > > On 25/01/2024 21:38, Paloma Arellano wrote: > >> YUV420 format is supported only in the VSC SDP packet and not through > >> MSA. Hence add an API which indicates the sink support which can be used > >> by the rest of the DP programming. > > > > This API ideally should go to drm/display/drm_dp_helper.c > I'm not familiar how other vendors are checking if VSC SDP is supported. > So in moving this API, I'm going to let the other vendors make the > changes themselves. Let me show it for you: bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp) { u8 dprx = 0; if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST, &dprx) != 1) return false; return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED; } > > > >> > >> Signed-off-by: Paloma Arellano <quic_parellan@xxxxxxxxxxx> > >> --- > >> drivers/gpu/drm/msm/dp/dp_display.c | 3 ++- > >> drivers/gpu/drm/msm/dp/dp_panel.c | 35 +++++++++++++++++++++++++---- > >> drivers/gpu/drm/msm/dp/dp_panel.h | 1 + > >> 3 files changed, 34 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > >> b/drivers/gpu/drm/msm/dp/dp_display.c > >> index ddac55f45a722..f6b3b6ca242f8 100644 > >> --- a/drivers/gpu/drm/msm/dp/dp_display.c > >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c > >> @@ -1617,7 +1617,8 @@ void dp_bridge_mode_set(struct drm_bridge > >> *drm_bridge, > >> !!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC); > >> dp_display->dp_mode.out_fmt_is_yuv_420 = > >> - drm_mode_is_420_only(&dp->connector->display_info, adjusted_mode); > >> + drm_mode_is_420_only(&dp->connector->display_info, adjusted_mode) && > >> + dp_panel_vsc_sdp_supported(dp_display->panel); > >> /* populate wide_bus_support to different layers */ > >> dp_display->ctrl->wide_bus_en = > >> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c > >> b/drivers/gpu/drm/msm/dp/dp_panel.c > >> index 127f6af995cd1..af7820b6d35ec 100644 > >> --- a/drivers/gpu/drm/msm/dp/dp_panel.c > >> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c > >> @@ -17,6 +17,9 @@ struct dp_panel_private { > >> struct dp_link *link; > >> struct dp_catalog *catalog; > >> bool panel_on; > >> + bool vsc_supported; > >> + u8 major; > >> + u8 minor; > >> }; > >> static void dp_panel_read_psr_cap(struct dp_panel_private *panel) > >> @@ -43,9 +46,10 @@ static void dp_panel_read_psr_cap(struct > >> dp_panel_private *panel) > >> static int dp_panel_read_dpcd(struct dp_panel *dp_panel) > >> { > >> int rc; > >> + ssize_t rlen; > >> struct dp_panel_private *panel; > >> struct dp_link_info *link_info; > >> - u8 *dpcd, major, minor; > >> + u8 *dpcd, rx_feature; > >> panel = container_of(dp_panel, struct dp_panel_private, > >> dp_panel); > >> dpcd = dp_panel->dpcd; > >> @@ -53,10 +57,19 @@ static int dp_panel_read_dpcd(struct dp_panel > >> *dp_panel) > >> if (rc) > >> return rc; > >> + rlen = drm_dp_dpcd_read(panel->aux, > >> DP_DPRX_FEATURE_ENUMERATION_LIST, &rx_feature, 1); > >> + if (rlen != 1) { > >> + panel->vsc_supported = false; > >> + pr_debug("failed to read DP_DPRX_FEATURE_ENUMERATION_LIST\n"); > >> + } else { > >> + panel->vsc_supported = !!(rx_feature & > >> DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED); > >> + pr_debug("vsc=%d\n", panel->vsc_supported); > >> + } > >> + > >> link_info = &dp_panel->link_info; > >> link_info->revision = dpcd[DP_DPCD_REV]; > >> - major = (link_info->revision >> 4) & 0x0f; > >> - minor = link_info->revision & 0x0f; > >> + panel->major = (link_info->revision >> 4) & 0x0f; > >> + panel->minor = link_info->revision & 0x0f; > >> link_info->rate = drm_dp_max_link_rate(dpcd); > >> link_info->num_lanes = drm_dp_max_lane_count(dpcd); > >> @@ -69,7 +82,7 @@ static int dp_panel_read_dpcd(struct dp_panel > >> *dp_panel) > >> if (link_info->rate > dp_panel->max_dp_link_rate) > >> link_info->rate = dp_panel->max_dp_link_rate; > >> - drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor); > >> + drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", panel->major, > >> panel->minor); > >> drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate); > >> drm_dbg_dp(panel->drm_dev, "lane_count=%d\n", > >> link_info->num_lanes); > >> @@ -280,6 +293,20 @@ void dp_panel_tpg_config(struct dp_panel > >> *dp_panel, bool enable) > >> dp_catalog_panel_tpg_enable(catalog, > >> &panel->dp_panel.dp_mode.drm_mode); > >> } > >> +bool dp_panel_vsc_sdp_supported(struct dp_panel *dp_panel) > >> +{ > >> + struct dp_panel_private *panel; > >> + > >> + if (!dp_panel) { > >> + pr_err("invalid input\n"); > >> + return false; > >> + } > >> + > >> + panel = container_of(dp_panel, struct dp_panel_private, dp_panel); > >> + > >> + return panel->major >= 1 && panel->minor >= 3 && > >> panel->vsc_supported; Anyway, this check is incorrect. Please compare the whole revision against DP_DPCD_REV_13 instead of doing a maj/min comparison. > >> +} > >> + > >> void dp_panel_dump_regs(struct dp_panel *dp_panel) > >> { > >> struct dp_catalog *catalog; > >> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h > >> b/drivers/gpu/drm/msm/dp/dp_panel.h > >> index 6ec68be9f2366..590eca5ce304b 100644 > >> --- a/drivers/gpu/drm/msm/dp/dp_panel.h > >> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h > >> @@ -66,6 +66,7 @@ int dp_panel_get_modes(struct dp_panel *dp_panel, > >> struct drm_connector *connector); > >> void dp_panel_handle_sink_request(struct dp_panel *dp_panel); > >> void dp_panel_tpg_config(struct dp_panel *dp_panel, bool enable); > >> +bool dp_panel_vsc_sdp_supported(struct dp_panel *dp_panel); > >> /** > >> * is_link_rate_valid() - validates the link rate > > -- With best wishes Dmitry