| Message ID | 20220211224006.1797846-3-dmitry.baryshkov@linaro.org |
|---|---|
| State | Accepted |
| Commit | 9aa924688095e55a727f11228d73ce8df01d2a40 |
| Headers | show |
| Series | Simplify and correct msm/dp bridge implementation | expand |
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@linaro.org> > Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > 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); > };
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@linaro.org> > >> Tested-by: Kuogee Hsieh <quic_khsieh@quicinc.com> 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); >> };
Quoting Dmitry Baryshkov (2022-02-11 14:40:03) > 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@linaro.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
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); };
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@linaro.org> --- 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(-)