[RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces

[Dmitry Baryshkov]

On Thu, 24 Feb 2022 at 23:13, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>
>
>
> On 2/11/2022 2:40 PM, Dmitry Baryshkov wrote:
> > It is possible to supply display-connector (bridge) to the DP interface,
> > add support for parsing it too.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> > ---
> >   drivers/gpu/drm/msm/dp/dp_parser.c | 19 ++++++++++++-------
> >   1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
> > index 901d7967370f..1056b8d5755b 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> > @@ -301,17 +301,22 @@ static int dp_parser_parse(struct dp_parser *parser, int connector_type)
> >               return rc;
> >
> >       /*
> > -      * Currently we support external bridges only for eDP connectors.
> > +      * External bridges are mandatory for eDP interfaces: one has to
> > +      * provide at least an eDP panel (which gets wrapped into panel-bridge).
> >        *
> > -      * No external bridges are expected for the DisplayPort connector,
> > -      * it is physically present in a form of a DP or USB-C connector.
> > +      * For DisplayPort interfaces external bridges are optional, so
> > +      * silently ignore an error if one is not present (-ENODEV).
> >        */
> > -     if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> > -             rc = dp_parser_find_next_bridge(parser);
> > -             if (rc) {
> > -                     DRM_ERROR("DP: failed to find next bridge\n");
> > +     rc = dp_parser_find_next_bridge(parser);
> > +     if (rc == -ENODEV) {
> > +             if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> > +                     DRM_ERROR("eDP: next bridge is not present\n");
> >                       return rc;
> >               }
> > +     } else if (rc) {
> > +             if (rc != -EPROBE_DEFER)
> > +                     DRM_ERROR("DP: error parsing next bridge: %d\n", rc);
> > +             return rc;
> >       }
>
> How is this silently ignoring?
>
> static int dp_display_bind(struct device *dev, struct device *master,
>                 void *data)
> {
>      int rc = 0;
>      struct dp_display_private *dp = dev_get_dp_display_private(dev);
>      struct msm_drm_private *priv = dev_get_drvdata(master);
>      struct drm_device *drm = priv->dev;
>
>      dp->dp_display.drm_dev = drm;
>      priv->dp[dp->id] = &dp->dp_display;
>
>      rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
>      if (rc) {
>          DRM_ERROR("device tree parsing failed\n");
>          goto end;
>      }
>
> dp_display_bind will still fail if a bridge is not found.
>
> If supplying a bridge is optional even this should succeed right?

It will succeed as dp_parser_parse() will not return -ENODEV if the
connector is not eDP.
To rephrase the comment:
For the dp_parser_find_next_bridge() result:
- for eDP the driver passes all errors to the calling function.
- for DP the driver ignores -ENODEV (no external bridge is supplied),
but passes all other errors (which can mean e.g. that the bridge is
not properly declared or that it did hasn't been probed yet).

-- 
With best wishes
Dmitry