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, but that approach
has been dropped due to bindings concerns. So now we lookup the panel
node in DT and based on it's presence we can safely say if it is eDP or not.
The PHY counterpart patchset is here:
https://lore.kernel.org/all/20240324-x1e80100-phy-edp-compatible-refactor-v5-0-a0db5f3150bc@linaro.org
This patchset cannot be applied without the one mentioned above because
it relies on PHY_SUBMODE_EDP and PHY_SUBMODE_DP.
Signed-off-by: Abel Vesa <[email protected]>
---
Changes in v4:
- Reworked the dp_display_get_connector_type to be more readable, like
Bjorn suggested.
- Dropped the unrelated change w.r.t. dp_aux_get call, reported by
Dmitry.
- Re-worded the commit message for the first patch, to align with
Dmitry's suggestion.
- Added Dmitry's R-b tag to the X1E80100 specific patch
- Link to v3: https://lore.kernel.org/r/20240322-x1e80100-display-refactor-connector-v3-0-af14c29af665@linaro.org
Changes in v3:
- Dropped the bindings patch as this new solution doesn't involve
bindings update.
- Dropped R-b tags as this has been entirely reworked
- Reworked to lookup the panel node in DT and set the is_edp and
connector type based on panel node presence
- Link to v2: https://lore.kernel.org/r/20240222-x1e80100-display-refactor-connector-v2-0-bd4197dfceab@linaro.org
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 (2):
drm/msm/dp: Add support for determining the eDP/DP mode from DT
drm/msm/dp: Add support for the X1E80100
drivers/gpu/drm/msm/dp/dp_display.c | 38 ++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
---
base-commit: 13ee4a7161b6fd938aef6688ff43b163f6d83e37
change-id: 20231219-x1e80100-display-refactor-connector-e1c66548cae3
Best regards,
--
Abel Vesa <[email protected]>
Instead of relying on different compatibles for eDP and DP, lookup
the panel node in devicetree to figure out the connector type and
then pass on that information to the PHY. External DP doesn't have
a panel described in DT, therefore, assume it's eDP if panel node
is present.
Signed-off-by: Abel Vesa <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_display.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index c4cb82af5c2f..9169a739cc54 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -726,6 +726,14 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
if (IS_ERR(phy))
return PTR_ERR(phy);
+ rc = phy_set_mode_ext(phy, PHY_MODE_DP,
+ dp->dp_display.is_edp ? PHY_SUBMODE_EDP : PHY_SUBMODE_DP);
+ if (rc) {
+ DRM_ERROR("failed to set phy submode, rc = %d\n", rc);
+ dp->catalog = NULL;
+ goto error;
+ }
+
dp->catalog = dp_catalog_get(dev);
if (IS_ERR(dp->catalog)) {
rc = PTR_ERR(dp->catalog);
@@ -1241,6 +1249,25 @@ static int dp_auxbus_done_probe(struct drm_dp_aux *aux)
return dp_display_probe_tail(aux->dev);
}
+static int dp_display_get_connector_type(struct platform_device *pdev,
+ const struct msm_dp_desc *desc)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct device_node *aux_bus = of_get_child_by_name(node, "aux-bus");
+ struct device_node *panel = of_get_child_by_name(aux_bus, "panel");
+ int connector_type;
+
+ if (panel)
+ connector_type = DRM_MODE_CONNECTOR_eDP;
+ else
+ connector_type = DRM_MODE_SUBCONNECTOR_DisplayPort;
+
+ of_node_put(panel);
+ of_node_put(aux_bus);
+
+ return connector_type;
+}
+
static int dp_display_probe(struct platform_device *pdev)
{
int rc = 0;
@@ -1263,7 +1290,7 @@ static int dp_display_probe(struct platform_device *pdev)
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 = dp_display_get_connector_type(pdev, desc);
dp->wide_bus_supported = desc->wide_bus_supported;
dp->dp_display.is_edp =
(dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
--
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 based on the presence of the panel node in DT.
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 9169a739cc54..521cba76d2a0 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_supported = true },
+ { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },
+ { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .wide_bus_supported = true },
+ { .io_start = 0x0aea0000, .id = MSM_DP_CONTROLLER_3, .wide_bus_supported = 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 Sun, 24 Mar 2024 at 20:57, Abel Vesa <[email protected]> wrote:
>
> Instead of relying on different compatibles for eDP and DP, lookup
> the panel node in devicetree to figure out the connector type and
> then pass on that information to the PHY. External DP doesn't have
> a panel described in DT, therefore, assume it's eDP if panel node
> is present.
>
> Signed-off-by: Abel Vesa <[email protected]>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Sun, Mar 24, 2024 at 08:56:51PM +0200, Abel Vesa wrote:
> Instead of relying on different compatibles for eDP and DP, lookup
> the panel node in devicetree to figure out the connector type and
> then pass on that information to the PHY. External DP doesn't have
> a panel described in DT, therefore, assume it's eDP if panel node
> is present.
>
Reviewed-by: Bjorn Andersson <[email protected]>
Regards,
Bjorn
> Signed-off-by: Abel Vesa <[email protected]>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index c4cb82af5c2f..9169a739cc54 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -726,6 +726,14 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
> if (IS_ERR(phy))
> return PTR_ERR(phy);
>
> + rc = phy_set_mode_ext(phy, PHY_MODE_DP,
> + dp->dp_display.is_edp ? PHY_SUBMODE_EDP : PHY_SUBMODE_DP);
> + if (rc) {
> + DRM_ERROR("failed to set phy submode, rc = %d\n", rc);
> + dp->catalog = NULL;
> + goto error;
> + }
> +
> dp->catalog = dp_catalog_get(dev);
> if (IS_ERR(dp->catalog)) {
> rc = PTR_ERR(dp->catalog);
> @@ -1241,6 +1249,25 @@ static int dp_auxbus_done_probe(struct drm_dp_aux *aux)
> return dp_display_probe_tail(aux->dev);
> }
>
> +static int dp_display_get_connector_type(struct platform_device *pdev,
> + const struct msm_dp_desc *desc)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct device_node *aux_bus = of_get_child_by_name(node, "aux-bus");
> + struct device_node *panel = of_get_child_by_name(aux_bus, "panel");
> + int connector_type;
> +
> + if (panel)
> + connector_type = DRM_MODE_CONNECTOR_eDP;
> + else
> + connector_type = DRM_MODE_SUBCONNECTOR_DisplayPort;
> +
> + of_node_put(panel);
> + of_node_put(aux_bus);
> +
> + return connector_type;
> +}
> +
> static int dp_display_probe(struct platform_device *pdev)
> {
> int rc = 0;
> @@ -1263,7 +1290,7 @@ static int dp_display_probe(struct platform_device *pdev)
> 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 = dp_display_get_connector_type(pdev, desc);
> dp->wide_bus_supported = desc->wide_bus_supported;
> dp->dp_display.is_edp =
> (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
>
> --
> 2.34.1
>
On Sun, Mar 24, 2024 at 08:56:52PM +0200, Abel Vesa wrote:
> 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 based on the presence of the panel node in DT.
>
> 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_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 9169a739cc54..521cba76d2a0 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_supported = true },
> + { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },
> + { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .wide_bus_supported = true },
> + { .io_start = 0x0aea0000, .id = MSM_DP_CONTROLLER_3, .wide_bus_supported = 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
>