Re: [Freedreno] [PATCH v4 4/7] drm/msm/dp: Allow attaching a drm_panel
On 2021-10-05 19:09, Bjorn Andersson wrote: On Tue 05 Oct 17:35 PDT 2021, abhin...@codeaurora.org wrote: Hi Bjorn On 2021-10-05 16:13, Bjorn Andersson wrote: > eDP panels might need some power sequencing and backlight management, > so make it possible to associate a drm_panel with an eDP instance and > prepare and enable the panel accordingly. > > Now that we know which hardware instance is DP and which is eDP, > parser->parse() is passed the connector_type and the parser is limited > to only search for a panel in the eDP case. > > Signed-off-by: Bjorn Andersson > --- > > Changes since v3: > - Previously posted separately, now added to series > - Make the search for a panel conditional on it being an eDP connector > - Move the search to of_graph port 1 (was 2 to distinguish from DP > link to USB-C connector) > > drivers/gpu/drm/msm/dp/dp_display.c | 9 ++--- > drivers/gpu/drm/msm/dp/dp_display.h | 1 + > drivers/gpu/drm/msm/dp/dp_drm.c | 11 +++ > drivers/gpu/drm/msm/dp/dp_parser.c | 30 - > drivers/gpu/drm/msm/dp/dp_parser.h | 3 ++- > 5 files changed, 49 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index eaf08f9e7d87..bdaf227f05dc 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > > #include "msm_drv.h" > #include "msm_kms.h" > @@ -230,12 +231,14 @@ static int dp_display_bind(struct device *dev, > struct device *master, >priv = drm->dev_private; >priv->dp = &(dp->dp_display); > > - rc = dp->parser->parse(dp->parser); > + rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type); >if (rc) { >DRM_ERROR("device tree parsing failed\n"); >goto end; >} > > + dp->dp_display.panel_bridge = dp->parser->panel_bridge; > + >dp->aux->drm_dev = drm; >rc = dp_aux_register(dp->aux); >if (rc) { > @@ -822,7 +825,7 @@ static int dp_display_set_mode(struct msm_dp > *dp_display, >return 0; > } > > -static int dp_display_prepare(struct msm_dp *dp) > +static int dp_display_prepare(struct msm_dp *dp_display) > { >return 0; > } > @@ -896,7 +899,7 @@ static int dp_display_disable(struct > dp_display_private *dp, u32 data) >return 0; > } > > -static int dp_display_unprepare(struct msm_dp *dp) > +static int dp_display_unprepare(struct msm_dp *dp_display) > { >return 0; > } > diff --git a/drivers/gpu/drm/msm/dp/dp_display.h > b/drivers/gpu/drm/msm/dp/dp_display.h > index 02999408c052..24aefca66029 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.h > +++ b/drivers/gpu/drm/msm/dp/dp_display.h > @@ -15,6 +15,7 @@ struct msm_dp { >struct device *codec_dev; >struct drm_connector *connector; >struct drm_encoder *encoder; > + struct drm_bridge *panel_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 f33e31523f56..76856c4ee1d6 100644 > --- a/drivers/gpu/drm/msm/dp/dp_drm.c > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c > @@ -5,6 +5,7 @@ > > #include > #include > +#include > #include > > #include "msm_drv.h" > @@ -160,5 +161,15 @@ struct drm_connector > *dp_drm_connector_init(struct msm_dp *dp_display) > >drm_connector_attach_encoder(connector, dp_display->encoder); > > + if (dp_display->panel_bridge) { > + ret = drm_bridge_attach(dp_display->encoder, > + dp_display->panel_bridge, NULL, > + DRM_BRIDGE_ATTACH_NO_CONNECTOR); > + if (ret < 0) { > + DRM_ERROR("failed to attach panel bridge: %d\n", ret); > + return ERR_PTR(ret); > + } > + } > + >return connector; > } > diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c > b/drivers/gpu/drm/msm/dp/dp_parser.c > index 4d6e047f803d..eb6bbfbea484 100644 > --- a/drivers/gpu/drm/msm/dp/dp_parser.c > +++ b/drivers/gpu/drm/msm/dp/dp_parser.c > @@ -6,6 +6,7 @@ > #include > #include > > +#include > #include > > #include "dp_parser.h" > @@ -263,7 +264,28 @@ static int dp_parser_clock(struct dp_parser > *parser) >return 0; > } > > -static int dp_parser_parse(struct dp_parser *parser) > +static int dp_parser_find_panel(struct dp_parser *parser) > +{ > + struct device *dev = >pdev->dev; > + struct drm_panel *panel; > + int rc; > + > + rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, , NULL); > + if (rc) { > + DRM_ERROR("failed to acquire DRM panel: %d\n", rc); > + return rc; > + } > + > + 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); > + } When we add a bridge using devm_drm_panel_bridge_add(), it will
Re: [Freedreno] [PATCH v4 4/7] drm/msm/dp: Allow attaching a drm_panel
On Tue 05 Oct 17:35 PDT 2021, abhin...@codeaurora.org wrote: > Hi Bjorn > > On 2021-10-05 16:13, Bjorn Andersson wrote: > > eDP panels might need some power sequencing and backlight management, > > so make it possible to associate a drm_panel with an eDP instance and > > prepare and enable the panel accordingly. > > > > Now that we know which hardware instance is DP and which is eDP, > > parser->parse() is passed the connector_type and the parser is limited > > to only search for a panel in the eDP case. > > > > Signed-off-by: Bjorn Andersson > > --- > > > > Changes since v3: > > - Previously posted separately, now added to series > > - Make the search for a panel conditional on it being an eDP connector > > - Move the search to of_graph port 1 (was 2 to distinguish from DP > > link to USB-C connector) > > > > drivers/gpu/drm/msm/dp/dp_display.c | 9 ++--- > > drivers/gpu/drm/msm/dp/dp_display.h | 1 + > > drivers/gpu/drm/msm/dp/dp_drm.c | 11 +++ > > drivers/gpu/drm/msm/dp/dp_parser.c | 30 - > > drivers/gpu/drm/msm/dp/dp_parser.h | 3 ++- > > 5 files changed, 49 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > b/drivers/gpu/drm/msm/dp/dp_display.c > > index eaf08f9e7d87..bdaf227f05dc 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_display.c > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > > @@ -10,6 +10,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "msm_drv.h" > > #include "msm_kms.h" > > @@ -230,12 +231,14 @@ static int dp_display_bind(struct device *dev, > > struct device *master, > > priv = drm->dev_private; > > priv->dp = &(dp->dp_display); > > > > - rc = dp->parser->parse(dp->parser); > > + rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type); > > if (rc) { > > DRM_ERROR("device tree parsing failed\n"); > > goto end; > > } > > > > + dp->dp_display.panel_bridge = dp->parser->panel_bridge; > > + > > dp->aux->drm_dev = drm; > > rc = dp_aux_register(dp->aux); > > if (rc) { > > @@ -822,7 +825,7 @@ static int dp_display_set_mode(struct msm_dp > > *dp_display, > > return 0; > > } > > > > -static int dp_display_prepare(struct msm_dp *dp) > > +static int dp_display_prepare(struct msm_dp *dp_display) > > { > > return 0; > > } > > @@ -896,7 +899,7 @@ static int dp_display_disable(struct > > dp_display_private *dp, u32 data) > > return 0; > > } > > > > -static int dp_display_unprepare(struct msm_dp *dp) > > +static int dp_display_unprepare(struct msm_dp *dp_display) > > { > > return 0; > > } > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.h > > b/drivers/gpu/drm/msm/dp/dp_display.h > > index 02999408c052..24aefca66029 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_display.h > > +++ b/drivers/gpu/drm/msm/dp/dp_display.h > > @@ -15,6 +15,7 @@ struct msm_dp { > > struct device *codec_dev; > > struct drm_connector *connector; > > struct drm_encoder *encoder; > > + struct drm_bridge *panel_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 f33e31523f56..76856c4ee1d6 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_drm.c > > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c > > @@ -5,6 +5,7 @@ > > > > #include > > #include > > +#include > > #include > > > > #include "msm_drv.h" > > @@ -160,5 +161,15 @@ struct drm_connector > > *dp_drm_connector_init(struct msm_dp *dp_display) > > > > drm_connector_attach_encoder(connector, dp_display->encoder); > > > > + if (dp_display->panel_bridge) { > > + ret = drm_bridge_attach(dp_display->encoder, > > + dp_display->panel_bridge, NULL, > > + DRM_BRIDGE_ATTACH_NO_CONNECTOR); > > + if (ret < 0) { > > + DRM_ERROR("failed to attach panel bridge: %d\n", ret); > > + return ERR_PTR(ret); > > + } > > + } > > + > > return connector; > > } > > diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c > > b/drivers/gpu/drm/msm/dp/dp_parser.c > > index 4d6e047f803d..eb6bbfbea484 100644 > > --- a/drivers/gpu/drm/msm/dp/dp_parser.c > > +++ b/drivers/gpu/drm/msm/dp/dp_parser.c > > @@ -6,6 +6,7 @@ > > #include > > #include > > > > +#include > > #include > > > > #include "dp_parser.h" > > @@ -263,7 +264,28 @@ static int dp_parser_clock(struct dp_parser > > *parser) > > return 0; > > } > > > > -static int dp_parser_parse(struct dp_parser *parser) > > +static int dp_parser_find_panel(struct dp_parser *parser) > > +{ > > + struct device *dev = >pdev->dev; > > + struct drm_panel *panel; > > + int rc; > > + > > + rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, , NULL); > > + if (rc) { > > + DRM_ERROR("failed to acquire DRM panel: %d\n", rc); > > + return
Re: [Freedreno] [PATCH v4 4/7] drm/msm/dp: Allow attaching a drm_panel
Hi Bjorn On 2021-10-05 16:13, Bjorn Andersson wrote: eDP panels might need some power sequencing and backlight management, so make it possible to associate a drm_panel with an eDP instance and prepare and enable the panel accordingly. Now that we know which hardware instance is DP and which is eDP, parser->parse() is passed the connector_type and the parser is limited to only search for a panel in the eDP case. Signed-off-by: Bjorn Andersson --- Changes since v3: - Previously posted separately, now added to series - Make the search for a panel conditional on it being an eDP connector - Move the search to of_graph port 1 (was 2 to distinguish from DP link to USB-C connector) drivers/gpu/drm/msm/dp/dp_display.c | 9 ++--- drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dp/dp_drm.c | 11 +++ drivers/gpu/drm/msm/dp/dp_parser.c | 30 - drivers/gpu/drm/msm/dp/dp_parser.h | 3 ++- 5 files changed, 49 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index eaf08f9e7d87..bdaf227f05dc 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -10,6 +10,7 @@ #include #include #include +#include #include "msm_drv.h" #include "msm_kms.h" @@ -230,12 +231,14 @@ static int dp_display_bind(struct device *dev, struct device *master, priv = drm->dev_private; priv->dp = &(dp->dp_display); - rc = dp->parser->parse(dp->parser); + rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type); if (rc) { DRM_ERROR("device tree parsing failed\n"); goto end; } + dp->dp_display.panel_bridge = dp->parser->panel_bridge; + dp->aux->drm_dev = drm; rc = dp_aux_register(dp->aux); if (rc) { @@ -822,7 +825,7 @@ static int dp_display_set_mode(struct msm_dp *dp_display, return 0; } -static int dp_display_prepare(struct msm_dp *dp) +static int dp_display_prepare(struct msm_dp *dp_display) { return 0; } @@ -896,7 +899,7 @@ static int dp_display_disable(struct dp_display_private *dp, u32 data) return 0; } -static int dp_display_unprepare(struct msm_dp *dp) +static int dp_display_unprepare(struct msm_dp *dp_display) { return 0; } diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h index 02999408c052..24aefca66029 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.h +++ b/drivers/gpu/drm/msm/dp/dp_display.h @@ -15,6 +15,7 @@ struct msm_dp { struct device *codec_dev; struct drm_connector *connector; struct drm_encoder *encoder; + struct drm_bridge *panel_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 f33e31523f56..76856c4ee1d6 100644 --- a/drivers/gpu/drm/msm/dp/dp_drm.c +++ b/drivers/gpu/drm/msm/dp/dp_drm.c @@ -5,6 +5,7 @@ #include #include +#include #include #include "msm_drv.h" @@ -160,5 +161,15 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display) drm_connector_attach_encoder(connector, dp_display->encoder); + if (dp_display->panel_bridge) { + ret = drm_bridge_attach(dp_display->encoder, + dp_display->panel_bridge, NULL, + DRM_BRIDGE_ATTACH_NO_CONNECTOR); + if (ret < 0) { + DRM_ERROR("failed to attach panel bridge: %d\n", ret); + return ERR_PTR(ret); + } + } + return connector; } diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c index 4d6e047f803d..eb6bbfbea484 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.c +++ b/drivers/gpu/drm/msm/dp/dp_parser.c @@ -6,6 +6,7 @@ #include #include +#include #include #include "dp_parser.h" @@ -263,7 +264,28 @@ static int dp_parser_clock(struct dp_parser *parser) return 0; } -static int dp_parser_parse(struct dp_parser *parser) +static int dp_parser_find_panel(struct dp_parser *parser) +{ + struct device *dev = >pdev->dev; + struct drm_panel *panel; + int rc; + + rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, , NULL); + if (rc) { + DRM_ERROR("failed to acquire DRM panel: %d\n", rc); + return rc; + } + + 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); + } When we add a bridge using devm_drm_panel_bridge_add(), it will register with default bridge functions which is fine because we need the panel power to be controlled here. 140 static const struct drm_bridge_funcs
[Freedreno] [PATCH v4 4/7] drm/msm/dp: Allow attaching a drm_panel
eDP panels might need some power sequencing and backlight management, so make it possible to associate a drm_panel with an eDP instance and prepare and enable the panel accordingly. Now that we know which hardware instance is DP and which is eDP, parser->parse() is passed the connector_type and the parser is limited to only search for a panel in the eDP case. Signed-off-by: Bjorn Andersson --- Changes since v3: - Previously posted separately, now added to series - Make the search for a panel conditional on it being an eDP connector - Move the search to of_graph port 1 (was 2 to distinguish from DP link to USB-C connector) drivers/gpu/drm/msm/dp/dp_display.c | 9 ++--- drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dp/dp_drm.c | 11 +++ drivers/gpu/drm/msm/dp/dp_parser.c | 30 - drivers/gpu/drm/msm/dp/dp_parser.h | 3 ++- 5 files changed, 49 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index eaf08f9e7d87..bdaf227f05dc 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -10,6 +10,7 @@ #include #include #include +#include #include "msm_drv.h" #include "msm_kms.h" @@ -230,12 +231,14 @@ static int dp_display_bind(struct device *dev, struct device *master, priv = drm->dev_private; priv->dp = &(dp->dp_display); - rc = dp->parser->parse(dp->parser); + rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type); if (rc) { DRM_ERROR("device tree parsing failed\n"); goto end; } + dp->dp_display.panel_bridge = dp->parser->panel_bridge; + dp->aux->drm_dev = drm; rc = dp_aux_register(dp->aux); if (rc) { @@ -822,7 +825,7 @@ static int dp_display_set_mode(struct msm_dp *dp_display, return 0; } -static int dp_display_prepare(struct msm_dp *dp) +static int dp_display_prepare(struct msm_dp *dp_display) { return 0; } @@ -896,7 +899,7 @@ static int dp_display_disable(struct dp_display_private *dp, u32 data) return 0; } -static int dp_display_unprepare(struct msm_dp *dp) +static int dp_display_unprepare(struct msm_dp *dp_display) { return 0; } diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h index 02999408c052..24aefca66029 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.h +++ b/drivers/gpu/drm/msm/dp/dp_display.h @@ -15,6 +15,7 @@ struct msm_dp { struct device *codec_dev; struct drm_connector *connector; struct drm_encoder *encoder; + struct drm_bridge *panel_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 f33e31523f56..76856c4ee1d6 100644 --- a/drivers/gpu/drm/msm/dp/dp_drm.c +++ b/drivers/gpu/drm/msm/dp/dp_drm.c @@ -5,6 +5,7 @@ #include #include +#include #include #include "msm_drv.h" @@ -160,5 +161,15 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display) drm_connector_attach_encoder(connector, dp_display->encoder); + if (dp_display->panel_bridge) { + ret = drm_bridge_attach(dp_display->encoder, + dp_display->panel_bridge, NULL, + DRM_BRIDGE_ATTACH_NO_CONNECTOR); + if (ret < 0) { + DRM_ERROR("failed to attach panel bridge: %d\n", ret); + return ERR_PTR(ret); + } + } + return connector; } diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c index 4d6e047f803d..eb6bbfbea484 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.c +++ b/drivers/gpu/drm/msm/dp/dp_parser.c @@ -6,6 +6,7 @@ #include #include +#include #include #include "dp_parser.h" @@ -263,7 +264,28 @@ static int dp_parser_clock(struct dp_parser *parser) return 0; } -static int dp_parser_parse(struct dp_parser *parser) +static int dp_parser_find_panel(struct dp_parser *parser) +{ + struct device *dev = >pdev->dev; + struct drm_panel *panel; + int rc; + + rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, , NULL); + if (rc) { + DRM_ERROR("failed to acquire DRM panel: %d\n", rc); + return rc; + } + + 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); + } + + return 0; +} + +static int dp_parser_parse(struct dp_parser *parser, int connector_type) { int rc = 0; @@ -284,6 +306,12 @@ static int dp_parser_parse(struct dp_parser *parser) if (rc) return rc; + if