[PATCH 17/17] drm/msm/dp: allow YUV420 mode for DP connector when VSC SDP supported

[Dmitry Baryshkov]

On Mon, 29 Jan 2024 at 05:17, Abhinav Kumar <quic_abhinavk at 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_parellan at 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