Since this new platform supports both DP and eDP, it's the perfect time
to drop the dual compatible (eDP and DP) and figure out a different way
to specify the mode. After some off-list discussion, one suggested way
was to add a 'is-edp' property to the controller node and call
phy_set_mode to let the PHY know which mode it should configure itself.
The PHY counterpart patchset is here:
https://lore.kernel.org/all/20240220-x1e80100-phy-edp-compatible-refactor-v5-0-e8658adf5461@linaro.org/
Signed-off-by: Abel Vesa <[email protected]>
---
Changes in v2:
- Added Dmitry's R-b tag to both driver patches
- Dropped the if statement around assigning the is_edp in
dp_display_probe, and fixed said assignment by using the connector
type from match data instead.
- Moved the qcom,x1e80100-dp compatible where it belongs
- Re-worded the bindings commit message to follow Bjorn's suggestion
- Dropped the RFC tag as the approach doesn't seem to be questioned
anymore
- Link to v1: https://lore.kernel.org/r/20240221-x1e80100-display-refactor-connector-v1-0-86c0e1ebd5ec@linaro.org
---
Abel Vesa (3):
dt-bindings: display: msm: dp-controller: document X1E80100 compatible
drm/msm/dp: Add support for setting the eDP mode from devicetree
drm/msm/dp: Add support for the X1E80100
.../bindings/display/msm/dp-controller.yaml | 6 +++++
drivers/gpu/drm/msm/dp/dp_ctrl.c | 11 +++++++++
drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 26 +++++++++++++++++++---
4 files changed, 41 insertions(+), 3 deletions(-)
---
base-commit: 4893c639cc3659cefaa675bf1e59f4e7571afb5c
change-id: 20231219-x1e80100-display-refactor-connector-e1c66548cae3
Best regards,
--
Abel Vesa <[email protected]>
Add the X1E80100 to the list of compatibles and document the is-edp
flag. The controllers are expected to operate in DP mode by default,
and this flag can be used to select eDP mode.
Signed-off-by: Abel Vesa <[email protected]>
---
Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
index ae53cbfb2193..ed11852e403d 100644
--- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
@@ -27,6 +27,7 @@ properties:
- qcom,sdm845-dp
- qcom,sm8350-dp
- qcom,sm8650-dp
+ - qcom,x1e80100-dp
- items:
- enum:
- qcom,sm8150-dp
@@ -73,6 +74,11 @@ properties:
- description: phy 0 parent
- description: phy 1 parent
+ is-edp:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Tells the controller to switch to eDP mode
+
phys:
maxItems: 1
--
2.34.1
Instead of relying on different compatibles for eDP and DP, use
the is-edp property from DT to figure out the connector type and
then pass on that information to the PHY.
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 11 +++++++++++
drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 17 ++++++++++++++---
3 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 320f17fce9a6..bd81cc6bd5e3 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1533,6 +1533,17 @@ void dp_ctrl_set_psr(struct dp_ctrl *dp_ctrl, bool enter)
}
}
+int dp_ctrl_phy_set_mode(struct dp_ctrl *dp_ctrl, int submode)
+{
+ struct dp_ctrl_private *ctrl;
+ struct phy *phy;
+
+ ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+ phy = ctrl->phy;
+
+ return phy_set_mode_ext(phy, PHY_MODE_DP, submode);
+}
+
void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
{
struct dp_ctrl_private *ctrl;
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index fa014cee7e21..a10d1b19d172 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -32,6 +32,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
struct phy *phy);
void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, bool enable);
+int dp_ctrl_phy_set_mode(struct dp_ctrl *dp_ctrl, int mode);
void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl);
void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl);
void dp_ctrl_irq_phy_exit(struct dp_ctrl *dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index e4433891becb..e01b41ad2e2a 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1229,6 +1229,7 @@ static int dp_display_probe(struct platform_device *pdev)
int rc = 0;
struct dp_display_private *dp;
const struct msm_dp_desc *desc;
+ bool is_edp;
if (!pdev || !pdev->dev.of_node) {
DRM_ERROR("pdev not found\n");
@@ -1243,13 +1244,17 @@ static int dp_display_probe(struct platform_device *pdev)
if (!desc)
return -EINVAL;
+ is_edp = (desc->connector_type == DRM_MODE_CONNECTOR_eDP) ||
+ of_property_read_bool(pdev->dev.of_node, "is-edp");
+
dp->dp_display.pdev = pdev;
dp->name = "drm_dp";
dp->id = desc->id;
- dp->dp_display.connector_type = desc->connector_type;
+ dp->dp_display.connector_type = is_edp ?
+ DRM_MODE_CONNECTOR_eDP :
+ DRM_MODE_CONNECTOR_DisplayPort;
dp->wide_bus_en = desc->wide_bus_en;
- dp->dp_display.is_edp =
- (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
+ dp->dp_display.is_edp = is_edp;
rc = dp_init_sub_modules(dp);
if (rc) {
@@ -1257,6 +1262,12 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ rc = dp_ctrl_phy_set_mode(dp->ctrl, is_edp ? PHY_SUBMODE_EDP : PHY_SUBMODE_DP);
+ if (rc) {
+ DRM_ERROR("setting PHY submode failed\n");
+ goto err;
+ }
+
/* setup event q */
mutex_init(&dp->event_mutex);
init_waitqueue_head(&dp->event_q);
--
2.34.1
Add the X1E80100 DP descs and compatible. This platform will be using
a single compatible for both eDP and DP mode. The actual mode will
be set in devicetree via is-edp flag.
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_display.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index e01b41ad2e2a..fb1969617617 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -171,6 +171,14 @@ static const struct msm_dp_desc sm8650_dp_descs[] = {
{}
};
+static const struct msm_dp_desc x1e80100_dp_descs[] = {
+ { .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, .wide_bus_en = true },
+ { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .wide_bus_en = true },
+ { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .wide_bus_en = true },
+ { .io_start = 0x0aea0000, .id = MSM_DP_CONTROLLER_3, .wide_bus_en = true },
+ {}
+};
+
static const struct of_device_id dp_dt_match[] = {
{ .compatible = "qcom,sc7180-dp", .data = &sc7180_dp_descs },
{ .compatible = "qcom,sc7280-dp", .data = &sc7280_dp_descs },
@@ -182,6 +190,7 @@ static const struct of_device_id dp_dt_match[] = {
{ .compatible = "qcom,sdm845-dp", .data = &sc7180_dp_descs },
{ .compatible = "qcom,sm8350-dp", .data = &sm8350_dp_descs },
{ .compatible = "qcom,sm8650-dp", .data = &sm8650_dp_descs },
+ { .compatible = "qcom,x1e80100-dp", .data = &x1e80100_dp_descs },
{}
};
--
2.34.1
On Thu, 22 Feb 2024 at 17:55, Abel Vesa <[email protected]> wrote:
>
> Add the X1E80100 to the list of compatibles and document the is-edp
> flag. The controllers are expected to operate in DP mode by default,
> and this flag can be used to select eDP mode.
>
> Signed-off-by: Abel Vesa <[email protected]>
Rob, Krzysztof, Connor, gracious ping for the review. It would be
really nice to merge this patchset during the next cycle. It also
unbreaks several other patches.
> ---
> Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> index ae53cbfb2193..ed11852e403d 100644
> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> @@ -27,6 +27,7 @@ properties:
> - qcom,sdm845-dp
> - qcom,sm8350-dp
> - qcom,sm8650-dp
> + - qcom,x1e80100-dp
> - items:
> - enum:
> - qcom,sm8150-dp
> @@ -73,6 +74,11 @@ properties:
> - description: phy 0 parent
> - description: phy 1 parent
>
> + is-edp:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Tells the controller to switch to eDP mode
> +
> phys:
> maxItems: 1
>
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
On 24/02/2024 23:34, Dmitry Baryshkov wrote:
> On Thu, 22 Feb 2024 at 17:55, Abel Vesa <[email protected]> wrote:
>>
>> Add the X1E80100 to the list of compatibles and document the is-edp
>> flag. The controllers are expected to operate in DP mode by default,
>> and this flag can be used to select eDP mode.
>>
>> Signed-off-by: Abel Vesa <[email protected]>
>
> Rob, Krzysztof, Connor, gracious ping for the review. It would be
> really nice to merge this patchset during the next cycle. It also
> unbreaks several other patches.
That's not a netdev... or do you have the same subsystem profile
expecting reviews *from everyone* within two days?
Best regards,
Krzysztof
On 22/02/2024 16:55, Abel Vesa wrote:
> Add the X1E80100 to the list of compatibles and document the is-edp
> flag. The controllers are expected to operate in DP mode by default,
> and this flag can be used to select eDP mode.
>
> Signed-off-by: Abel Vesa <[email protected]>
> ---
> Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> index ae53cbfb2193..ed11852e403d 100644
> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> @@ -27,6 +27,7 @@ properties:
> - qcom,sdm845-dp
> - qcom,sm8350-dp
> - qcom,sm8650-dp
> + - qcom,x1e80100-dp
> - items:
> - enum:
> - qcom,sm8150-dp
> @@ -73,6 +74,11 @@ properties:
> - description: phy 0 parent
> - description: phy 1 parent
>
> + is-edp:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Tells the controller to switch to eDP mode
DP controller cannot be edp, so property "is-edp" is confusing. Probably
you want to choose some phy mode, so you should rather use "phy-mode"
property. I am sure we've been here...
Anyway, if you define completely new property without vendor prefix,
that's a generic property, so you need to put it in some common schema
for all Display Controllers, not only Qualcomm.
Best regards,
Krzysztof
On Thu, Feb 22, 2024 at 05:55:07PM +0200, Abel Vesa wrote:
> Instead of relying on different compatibles for eDP and DP, use
> the is-edp property from DT to figure out the connector type and
> then pass on that information to the PHY.
>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Abel Vesa <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
> ---
> drivers/gpu/drm/msm/dp/dp_ctrl.c | 11 +++++++++++
> drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 +
> drivers/gpu/drm/msm/dp/dp_display.c | 17 ++++++++++++++---
> 3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 320f17fce9a6..bd81cc6bd5e3 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1533,6 +1533,17 @@ void dp_ctrl_set_psr(struct dp_ctrl *dp_ctrl, bool enter)
> }
> }
>
> +int dp_ctrl_phy_set_mode(struct dp_ctrl *dp_ctrl, int submode)
> +{
> + struct dp_ctrl_private *ctrl;
> + struct phy *phy;
> +
> + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
> + phy = ctrl->phy;
> +
> + return phy_set_mode_ext(phy, PHY_MODE_DP, submode);
> +}
> +
> void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
> {
> struct dp_ctrl_private *ctrl;
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index fa014cee7e21..a10d1b19d172 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -32,6 +32,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
> struct phy *phy);
>
> void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl, bool enable);
> +int dp_ctrl_phy_set_mode(struct dp_ctrl *dp_ctrl, int mode);
> void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl);
> void dp_ctrl_phy_exit(struct dp_ctrl *dp_ctrl);
> void dp_ctrl_irq_phy_exit(struct dp_ctrl *dp_ctrl);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index e4433891becb..e01b41ad2e2a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1229,6 +1229,7 @@ static int dp_display_probe(struct platform_device *pdev)
> int rc = 0;
> struct dp_display_private *dp;
> const struct msm_dp_desc *desc;
> + bool is_edp;
>
> if (!pdev || !pdev->dev.of_node) {
> DRM_ERROR("pdev not found\n");
> @@ -1243,13 +1244,17 @@ static int dp_display_probe(struct platform_device *pdev)
> if (!desc)
> return -EINVAL;
>
> + is_edp = (desc->connector_type == DRM_MODE_CONNECTOR_eDP) ||
> + of_property_read_bool(pdev->dev.of_node, "is-edp");
> +
> dp->dp_display.pdev = pdev;
> dp->name = "drm_dp";
> dp->id = desc->id;
> - dp->dp_display.connector_type = desc->connector_type;
> + dp->dp_display.connector_type = is_edp ?
> + DRM_MODE_CONNECTOR_eDP :
> + DRM_MODE_CONNECTOR_DisplayPort;
> dp->wide_bus_en = desc->wide_bus_en;
> - dp->dp_display.is_edp =
> - (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
> + dp->dp_display.is_edp = is_edp;
>
> rc = dp_init_sub_modules(dp);
> if (rc) {
> @@ -1257,6 +1262,12 @@ static int dp_display_probe(struct platform_device *pdev)
> return -EPROBE_DEFER;
> }
>
> + rc = dp_ctrl_phy_set_mode(dp->ctrl, is_edp ? PHY_SUBMODE_EDP : PHY_SUBMODE_DP);
> + if (rc) {
> + DRM_ERROR("setting PHY submode failed\n");
> + goto err;
> + }
> +
> /* setup event q */
> mutex_init(&dp->event_mutex);
> init_waitqueue_head(&dp->event_q);
>
> --
> 2.34.1
>
On Sun, Feb 25, 2024 at 12:34:34AM +0200, Dmitry Baryshkov wrote:
> On Thu, 22 Feb 2024 at 17:55, Abel Vesa <[email protected]> wrote:
> >
> > Add the X1E80100 to the list of compatibles and document the is-edp
> > flag. The controllers are expected to operate in DP mode by default,
> > and this flag can be used to select eDP mode.
> >
> > Signed-off-by: Abel Vesa <[email protected]>
>
> Rob, Krzysztof, Connor, gracious ping for the review. It would be
> really nice to merge this patchset during the next cycle. It also
> unbreaks several other patches.
The only thing that speeds up my review is reviewing whatever is ahead
of this patch in the queue[1].
Rob
[1] https://patchwork.ozlabs.org/project/devicetree-bindings/list/
On Tue, Feb 27, 2024 at 04:45:25PM +0100, Krzysztof Kozlowski wrote:
> On 22/02/2024 16:55, Abel Vesa wrote:
> > Add the X1E80100 to the list of compatibles and document the is-edp
> > flag. The controllers are expected to operate in DP mode by default,
> > and this flag can be used to select eDP mode.
> >
> > Signed-off-by: Abel Vesa <[email protected]>
> > ---
> > Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > index ae53cbfb2193..ed11852e403d 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > @@ -27,6 +27,7 @@ properties:
> > - qcom,sdm845-dp
> > - qcom,sm8350-dp
> > - qcom,sm8650-dp
> > + - qcom,x1e80100-dp
> > - items:
> > - enum:
> > - qcom,sm8150-dp
> > @@ -73,6 +74,11 @@ properties:
> > - description: phy 0 parent
> > - description: phy 1 parent
> >
> > + is-edp:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + Tells the controller to switch to eDP mode
>
>
> DP controller cannot be edp, so property "is-edp" is confusing. Probably
> you want to choose some phy mode, so you should rather use "phy-mode"
> property. I am sure we've been here...
phy-mode belongs in the phy node though. Not that you couldn't look in
the phy node and see, but everyone likes all the properties they need
nicely packaged up in their driver's node.
> Anyway, if you define completely new property without vendor prefix,
> that's a generic property, so you need to put it in some common schema
> for all Display Controllers, not only Qualcomm.
I'm trying to unsee what the driver is doing... Hard-coding the
connector type and some instance indices. Uhhhh! I'm sure I'm to blame
for rejecting those in DT.
I've suggested connector nodes in the past. More generally, whatever is
attached at the other end (as it could be a bridge rather than a
connector) knows what mode is needed. It's simple negotiation. Each end
presents what they support. You take the union of the list(s) and get
the mode. If there's more than one, then the kernel or user gets to
choose.
Qualcomm is not the only one with this problem. Solve it for everyone...
Rob
On 24-02-27 16:45:25, Krzysztof Kozlowski wrote:
> On 22/02/2024 16:55, Abel Vesa wrote:
> > Add the X1E80100 to the list of compatibles and document the is-edp
> > flag. The controllers are expected to operate in DP mode by default,
> > and this flag can be used to select eDP mode.
> >
> > Signed-off-by: Abel Vesa <[email protected]>
> > ---
> > Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > index ae53cbfb2193..ed11852e403d 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > @@ -27,6 +27,7 @@ properties:
> > - qcom,sdm845-dp
> > - qcom,sm8350-dp
> > - qcom,sm8650-dp
> > + - qcom,x1e80100-dp
> > - items:
> > - enum:
> > - qcom,sm8150-dp
> > @@ -73,6 +74,11 @@ properties:
> > - description: phy 0 parent
> > - description: phy 1 parent
> >
> > + is-edp:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + Tells the controller to switch to eDP mode
>
>
> DP controller cannot be edp, so property "is-edp" is confusing. Probably
> you want to choose some phy mode, so you should rather use "phy-mode"
> property. I am sure we've been here...
phy-mode in controller node or in the phy node?
>
> Anyway, if you define completely new property without vendor prefix,
> that's a generic property, so you need to put it in some common schema
> for all Display Controllers, not only Qualcomm.
I can re-spin with qcom,is-edp if that's better.
>
>
> Best regards,
> Krzysztof
>
On Fri, 1 Mar 2024 at 19:52, Rob Herring <[email protected]> wrote:
>
> On Tue, Feb 27, 2024 at 04:45:25PM +0100, Krzysztof Kozlowski wrote:
> > On 22/02/2024 16:55, Abel Vesa wrote:
> > > Add the X1E80100 to the list of compatibles and document the is-edp
> > > flag. The controllers are expected to operate in DP mode by default,
> > > and this flag can be used to select eDP mode.
> > >
> > > Signed-off-by: Abel Vesa <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > > index ae53cbfb2193..ed11852e403d 100644
> > > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > > @@ -27,6 +27,7 @@ properties:
> > > - qcom,sdm845-dp
> > > - qcom,sm8350-dp
> > > - qcom,sm8650-dp
> > > + - qcom,x1e80100-dp
> > > - items:
> > > - enum:
> > > - qcom,sm8150-dp
> > > @@ -73,6 +74,11 @@ properties:
> > > - description: phy 0 parent
> > > - description: phy 1 parent
> > >
> > > + is-edp:
> > > + $ref: /schemas/types.yaml#/definitions/flag
> > > + description:
> > > + Tells the controller to switch to eDP mode
> >
> >
> > DP controller cannot be edp, so property "is-edp" is confusing. Probably
> > you want to choose some phy mode, so you should rather use "phy-mode"
> > property. I am sure we've been here...
>
> phy-mode belongs in the phy node though. Not that you couldn't look in
> the phy node and see, but everyone likes all the properties they need
> nicely packaged up in their driver's node.
>
> > Anyway, if you define completely new property without vendor prefix,
> > that's a generic property, so you need to put it in some common schema
> > for all Display Controllers, not only Qualcomm.
Is there a generic schema for DisplayPort controllers? I think there
is none at this point. We can probably add it, declaring is-edp
property, link-frequencies, etc.
However Mediatek already uses a different option to specify supported
link frequencies.
>
> I'm trying to unsee what the driver is doing... Hard-coding the
> connector type and some instance indices. Uhhhh! I'm sure I'm to blame
> for rejecting those in DT.
Once this patchset is accepted (in this or that or whatever else
form), we will cleanup most of those hardcoded types.
>
> I've suggested connector nodes in the past. More generally, whatever is
> attached at the other end (as it could be a bridge rather than a
> connector) knows what mode is needed. It's simple negotiation. Each end
> presents what they support. You take the union of the list(s) and get
> the mode. If there's more than one, then the kernel or user gets to
> choose.
It's not that easy. First, probing of the bridge chain differs
slightly depending on whether the controller is eDP or DP controller.
eDP should use AUX BUS, while DP (currently) doesn't use it. More
importantly, error conditions differ too. For example, in the DP case
it is perfectly fine to have nothing attached to the controller. It
just means that the display chain needs no additional handling and the
HPD pin will be handled by the controller itself. In the eDP case if
neither a panel nor a bridge are attached, it is considered to be an
error and thus the driver will return probe error.
> Qualcomm is not the only one with this problem. Solve it for everyone...
--
With best wishes
Dmitry