Re: [RFC PATCH v2 2/5] drm/msm/dp: support attaching bridges to the DP encoder

[Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>]

On 19/02/2022 00:28, Kuogee Hsieh wrote:
> On 2/11/2022 2:40 PM, Dmitry Baryshkov wrote:
> > Currently DP driver will allocate panel bridge for eDP panels. This
> > supports only the following topology:
> >
> > - eDP encoder ⇒ eDP panel (wrapped using panel-bridge)
> >
> > Simplify this code to just check if there is any next bridge in the
> > chain (be it a panel bridge or regular bridge). Rename panel_bridge
> > field to next_bridge accordingly.
> >
> > This allows one to use e.g. one of the following display topologies:
> >
> > - eDP encoder ⇒ ptn3460 ⇒ fixed LVDS panel
> > - eDP encoder ⇒ ptn3460 ⇒ LVDS connector with EDID lines for panel 
> > autodetect
> > - eDP encoder ⇒ ptn3460 ⇒ THC63LVD1024 ⇒ DPI panel.
> > - eDP encoder ⇒ LT8912 ⇒ DSI panel
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
> > Tested-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>

The Tested-by got hidden by the quotation symbols. Could you please send 
another one?

> > ---
> >   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
> >   drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
> >   drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
> >   drivers/gpu/drm/msm/dp/dp_parser.c  | 31 +++++++++++++++--------------
> >   drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
> >   5 files changed, 21 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> > b/drivers/gpu/drm/msm/dp/dp_display.c
> > index 44d42c76c2a3..45f9a912ecc5 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > @@ -266,7 +266,7 @@ static int dp_display_bind(struct device *dev, 
> > struct device *master,
> >           goto end;
> >       }
> > -    dp->dp_display.panel_bridge = dp->parser->panel_bridge;
> > +    dp->dp_display.next_bridge = dp->parser->next_bridge;
> >       dp->aux->drm_dev = drm;
> >       rc = dp_aux_register(dp->aux);
> > diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
> > b/drivers/gpu/drm/msm/dp/dp_display.h
> > index e3adcd578a90..7af2b186d2d9 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_display.h
> > +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> > @@ -16,7 +16,7 @@ struct msm_dp {
> >       struct drm_bridge *bridge;
> >       struct drm_connector *connector;
> >       struct drm_encoder *encoder;
> > -    struct drm_bridge *panel_bridge;
> > +    struct drm_bridge *next_bridge;
> >       bool is_connected;
> >       bool audio_enabled;
> >       bool power_on;
> > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c 
> > b/drivers/gpu/drm/msm/dp/dp_drm.c
> > index 26ef41a4c1b6..80f59cf99089 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> > @@ -236,9 +236,9 @@ struct drm_bridge *msm_dp_bridge_init(struct 
> > msm_dp *dp_display, struct drm_devi
> >           return ERR_PTR(rc);
> >       }
> > -    if (dp_display->panel_bridge) {
> > +    if (dp_display->next_bridge) {
> >           rc = drm_bridge_attach(dp_display->encoder,
> > -                    dp_display->panel_bridge, bridge,
> > +                    dp_display->next_bridge, bridge,
> >                       DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> >           if (rc < 0) {
> >               DRM_ERROR("failed to attach panel bridge: %d\n", rc);
> > diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
> > b/drivers/gpu/drm/msm/dp/dp_parser.c
> > index a7acc23f742b..901d7967370f 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> > @@ -265,23 +265,16 @@ static int dp_parser_clock(struct dp_parser 
> > *parser)
> >       return 0;
> >   }
> > -static int dp_parser_find_panel(struct dp_parser *parser)
> > +static int dp_parser_find_next_bridge(struct dp_parser *parser)
> >   {
> >       struct device *dev = &parser->pdev->dev;
> > -    struct drm_panel *panel;
> > -    int rc;
> > +    struct drm_bridge *bridge;
> > -    rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL);
> > -    if (rc) {
> > -        DRM_ERROR("failed to acquire DRM panel: %d\n", rc);
> > -        return rc;
> > -    }
> > +    bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> > +    if (IS_ERR(bridge))
> > +        return PTR_ERR(bridge);
> > -    parser->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> > -    if (IS_ERR(parser->panel_bridge)) {
> > -        DRM_ERROR("failed to create panel bridge\n");
> > -        return PTR_ERR(parser->panel_bridge);
> > -    }
> > +    parser->next_bridge = bridge;
> >       return 0;
> >   }
> > @@ -307,10 +300,18 @@ static int dp_parser_parse(struct dp_parser 
> > *parser, int connector_type)
> >       if (rc)
> >           return rc;
> > +    /*
> > +     * Currently we support external bridges only for eDP connectors.
> > +     *
> > +     * No external bridges are expected for the DisplayPort connector,
> > +     * it is physically present in a form of a DP or USB-C connector.
> > +     */
> >       if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> > -        rc = dp_parser_find_panel(parser);
> > -        if (rc)
> > +        rc = dp_parser_find_next_bridge(parser);
> > +        if (rc) {
> > +            DRM_ERROR("DP: failed to find next bridge\n");
> >               return rc;
> > +        }
> >       }
> >       /* Map the corresponding regulator information according to
> > diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h 
> > b/drivers/gpu/drm/msm/dp/dp_parser.h
> > index 3172da089421..4cec851e38d9 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> > +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> > @@ -123,7 +123,7 @@ struct dp_parser {
> >       struct dp_display_data disp_data;
> >       const struct dp_regulator_cfg *regulator_cfg;
> >       u32 max_dp_lanes;
> > -    struct drm_bridge *panel_bridge;
> > +    struct drm_bridge *next_bridge;
> >       int (*parse)(struct dp_parser *parser, int connector_type);
> >   };


--
With best wishes
Dmitry