Re: [RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces
On 2/24/2022 12:49 PM, Dmitry Baryshkov wrote:
On Thu, 24 Feb 2022 at 23:13, Abhinav Kumar 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
---
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_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).
Ah okay, I just noticed that dp_parser_parse() returns 0 by default and
not rc.
So in this case it will still return 0.
Hence this change LGTM,
Reviewed-by: Abhinav Kumar
Re: [RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces
On Thu, 24 Feb 2022 at 23:13, Abhinav Kumar 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
> > ---
> > 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_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
Re: [RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces
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
---
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_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?
/* Map the corresponding regulator information according to
Re: [RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces
Quoting Dmitry Baryshkov (2022-02-11 14:40:04) > It is possible to supply display-connector (bridge) to the DP interface, > add support for parsing it too. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces
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
Tested-by: Kuogee Hsieh
---
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;
}
/* Map the corresponding regulator information according to
[RFC PATCH v2 3/5] drm/msm/dp: support finding next bridge even for DP interfaces
It is possible to supply display-connector (bridge) to the DP interface,
add support for parsing it too.
Signed-off-by: Dmitry Baryshkov
---
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;
}
/* Map the corresponding regulator information according to
--
2.34.1
