On Mon, 29 Jan 2024 at 05:17, Abhinav Kumar <quic_abhin...@quicinc.com> wrote: > > > > On 1/25/2024 2:05 PM, Dmitry Baryshkov wrote: > > On 25/01/2024 21:38, Paloma Arellano wrote: > >> All the components of YUV420 over DP are added. Therefore, let's mark the > >> connector property as true for DP connector when the DP type is not eDP > >> and when VSC SDP is supported. > >> > >> Signed-off-by: Paloma Arellano <quic_parel...@quicinc.com> > >> --- > >> drivers/gpu/drm/msm/dp/dp_display.c | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > >> b/drivers/gpu/drm/msm/dp/dp_display.c > >> index 4329435518351..97edd607400b8 100644 > >> --- a/drivers/gpu/drm/msm/dp/dp_display.c > >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c > >> @@ -370,11 +370,14 @@ static int dp_display_process_hpd_high(struct > >> dp_display_private *dp) > >> dp_link_process_request(dp->link); > >> - if (!dp->dp_display.is_edp) > >> + if (!dp->dp_display.is_edp) { > >> + if (dp_panel_vsc_sdp_supported(dp->panel)) > >> + dp->dp_display.connector->ycbcr_420_allowed = true; > > > > Please consider fixing a TODO in drm_bridge_connector_init(). > > > > I am not totally clear if that TODO can ever go for DP/HDMI usage of > drm_bridge_connector. > > We do not know if the sink supports VSC SDP till we read the DPCD and > till we know that sink supports VSC SDP, there is no reason to mark the > YUV modes as supported. This is the same logic followed across vendors. > > drm_bride_connector_init() happens much earlier than the point where we > read DPCD. The only thing which can be done is perhaps add some callback > to update_ycbcr_420_allowed once DPCD is read. But I don't think its > absolutely necessary to have a callback just for this.
After checking the drm_connector docs, I'd still hold my opinion and consider this patch to be a misuse of the property. If you check the drm_connector::ycbcr_420_allowed docs, you'll see that it describes the output from the source point of view. In other words, it should be true if the DP connector can send YUV420 rather than being set if the attached display supports such output. This matches ycbcr420_allowed usage by AMD, dw-hdmi, intel_hdmi and even intel_dp usage. > >> drm_dp_set_subconnector_property(dp->dp_display.connector, > >> connector_status_connected, > >> dp->panel->dpcd, > >> dp->panel->downstream_ports); > >> + } > >> edid = dp->panel->edid; > > -- With best wishes Dmitry