[PATCH v2 19/19] drm/msm/dp: allow YUV420 mode for DP connector when CDM available

[Dmitry Baryshkov]

On Sat, 10 Feb 2024 at 21:19, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>
>
>
> On 2/10/2024 3:33 AM, Dmitry Baryshkov wrote:
> > On Sat, 10 Feb 2024 at 03:52, Paloma Arellano <quic_parellan at quicinc.com> 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 there is a CDM block available.
> >>
> >> Changes in v2:
> >>          - Check for if dp_catalog has a CDM block available instead of
> >>            checking if VSC SDP is allowed when setting the dp connector's
> >>            ycbcr_420_allowed parameter
> >>
> >> Signed-off-by: Paloma Arellano <quic_parellan at quicinc.com>
> >> ---
> >>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 +++-
> >>   drivers/gpu/drm/msm/dp/dp_display.c     | 4 ++--
> >>   drivers/gpu/drm/msm/dp/dp_drm.c         | 8 ++++++--
> >>   drivers/gpu/drm/msm/dp/dp_drm.h         | 3 ++-
> >>   drivers/gpu/drm/msm/msm_drv.h           | 5 +++--
> >>   5 files changed, 16 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> >> index 723cc1d821431..beeaabe499abf 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> >> @@ -565,6 +565,7 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
> >>   {
> >>          struct drm_encoder *encoder = NULL;
> >>          struct msm_display_info info;
> >> +       bool yuv_supported;
> >>          int rc;
> >>          int i;
> >>
> >> @@ -583,7 +584,8 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
> >>                          return PTR_ERR(encoder);
> >>                  }
> >>
> >> -               rc = msm_dp_modeset_init(priv->dp[i], dev, encoder);
> >> +               yuv_supported = !!(dpu_kms->catalog->cdm);
> >
> > Drop parentheses please.
> >
> >> +               rc = msm_dp_modeset_init(priv->dp[i], dev, encoder, yuv_supported);
> >>                  if (rc) {
> >>                          DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
> >>                          return rc;
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> >> index ebcc76ef1d590..9b9f5f2921903 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> >> @@ -1471,7 +1471,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
> >>   }
> >>
> >>   int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
> >> -                       struct drm_encoder *encoder)
> >> +                       struct drm_encoder *encoder, bool yuv_supported)
> >>   {
> >>          struct dp_display_private *dp_priv;
> >>          int ret;
> >> @@ -1487,7 +1487,7 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
> >>                  return ret;
> >>          }
> >>
> >> -       dp_display->connector = dp_drm_connector_init(dp_display, encoder);
> >> +       dp_display->connector = dp_drm_connector_init(dp_display, encoder, yuv_supported);
> >>          if (IS_ERR(dp_display->connector)) {
> >>                  ret = PTR_ERR(dp_display->connector);
> >>                  DRM_DEV_ERROR(dev->dev,
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
> >> index 46e6889037e88..ab0d0d13b5e2c 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> >> @@ -353,7 +353,8 @@ int dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,
> >>   }
> >>
> >>   /* connector initialization */
> >> -struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display, struct drm_encoder *encoder)
> >> +struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display, struct drm_encoder *encoder,
> >> +                                           bool yuv_supported)
> >>   {
> >>          struct drm_connector *connector = NULL;
> >>
> >> @@ -361,8 +362,11 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display, struct dr
> >>          if (IS_ERR(connector))
> >>                  return connector;
> >>
> >> -       if (!dp_display->is_edp)
> >> +       if (!dp_display->is_edp) {
> >>                  drm_connector_attach_dp_subconnector_property(connector);
> >> +               if (yuv_supported)
> >> +                       connector->ycbcr_420_allowed = true;
> >
> > Is there any reason to disallow YUV420 for eDP connectors?
> >
> >> +       }
> >>
>
> Major reason was certainly validation but thinking about it more
> closely, I need to check whether CDM over eDP is even possible.
>
> Historically, CDM could output only to HDMI OR WB using the bit we
> program while setting up CDM and the same HDMI path is being used by DP
> as well. But I need to check whether CDM can even output to eDP with the
> same DP path. I dont have any documentation on this part yet.

I had the feeling that the DP / eDP difference on the chips is mostly
on the PHY and software side. So I assumed that it should be possible
to output YUV data to the eDP port in the same way as it is done for
the DP port.

-- 
With best wishes
Dmitry