Add sm8150/sm8250 compatibles to drm/msm and fix the sm8250 display nodes.
v2: do not remove mmcx-supply from dispcc node
Jonathan Marek (2): drm/msm: add compatibles for sm8150/sm8250 display arm64: dts: qcom: sm8250: fix display nodes
.../devicetree/bindings/display/msm/dpu.txt | 4 +-- arch/arm64/boot/dts/qcom/sm8250.dtsi | 33 +++++-------------- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++ drivers/gpu/drm/msm/msm_drv.c | 6 ++-- 4 files changed, 16 insertions(+), 29 deletions(-)
The driver already has support for sm8150/sm8250, but the compatibles were never added.
Also inverse the non-mdp4 condition in add_display_components() to avoid having to check every new compatible in the condition.
Signed-off-by: Jonathan Marek jonathan@marek.ca --- Documentation/devicetree/bindings/display/msm/dpu.txt | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++ drivers/gpu/drm/msm/msm_drv.c | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt index 551ae26f60da..5763f43200a0 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu.txt +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -8,7 +8,7 @@ The DPU display controller is found in SDM845 SoC.
MDSS: Required properties: -- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss" +- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss", "qcom,sm8150-mdss", "qcom,sm8250-mdss" - reg: physical base address and length of contoller's registers. - reg-names: register region names. The following region is required: * "mdss" @@ -41,7 +41,7 @@ Optional properties:
MDP: Required properties: -- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu" +- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu", "qcom,sm8150-dpu", "qcom,sm8250-dpu" - reg: physical base address and length of controller's registers. - reg-names : register region names. The following region is required: * "mdp" diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 5a8e3e1fc48c..fff12a4c8bfc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1219,6 +1219,8 @@ static const struct dev_pm_ops dpu_pm_ops = { static const struct of_device_id dpu_dt_match[] = { { .compatible = "qcom,sdm845-dpu", }, { .compatible = "qcom,sc7180-dpu", }, + { .compatible = "qcom,sm8150-dpu", }, + { .compatible = "qcom,sm8250-dpu", }, {} }; MODULE_DEVICE_TABLE(of, dpu_dt_match); diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 94525ac76d4e..928f13d4bfbc 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1185,9 +1185,7 @@ static int add_display_components(struct device *dev, * Populate the children devices, find the MDP5/DPU node, and then add * the interfaces to our components list. */ - if (of_device_is_compatible(dev->of_node, "qcom,mdss") || - of_device_is_compatible(dev->of_node, "qcom,sdm845-mdss") || - of_device_is_compatible(dev->of_node, "qcom,sc7180-mdss")) { + if (!of_device_is_compatible(dev->of_node, "qcom,mdp4")) { ret = of_platform_populate(dev->of_node, NULL, NULL, dev); if (ret) { DRM_DEV_ERROR(dev, "failed to populate children devices\n"); @@ -1320,6 +1318,8 @@ static const struct of_device_id dt_match[] = { { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 }, { .compatible = "qcom,sdm845-mdss", .data = (void *)KMS_DPU }, { .compatible = "qcom,sc7180-mdss", .data = (void *)KMS_DPU }, + { .compatible = "qcom,sm8150-mdss", .data = (void *)KMS_DPU }, + { .compatible = "qcom,sm8250-mdss", .data = (void *)KMS_DPU }, {} }; MODULE_DEVICE_TABLE(of, dt_match);
On Mon, 15 Feb 2021 at 19:25, Jonathan Marek jonathan@marek.ca wrote:
The driver already has support for sm8150/sm8250, but the compatibles were never added.
Also inverse the non-mdp4 condition in add_display_components() to avoid having to check every new compatible in the condition.
Signed-off-by: Jonathan Marek jonathan@marek.ca
Documentation/devicetree/bindings/display/msm/dpu.txt | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++ drivers/gpu/drm/msm/msm_drv.c | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt index 551ae26f60da..5763f43200a0 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu.txt +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -8,7 +8,7 @@ The DPU display controller is found in SDM845 SoC.
MDSS: Required properties: -- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss" +- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss", "qcom,sm8150-mdss", "qcom,sm8250-mdss"
- reg: physical base address and length of contoller's registers.
- reg-names: register region names. The following region is required:
- "mdss"
@@ -41,7 +41,7 @@ Optional properties:
MDP: Required properties: -- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu" +- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu", "qcom,sm8150-dpu", "qcom,sm8250-dpu"
- reg: physical base address and length of controller's registers.
- reg-names : register region names. The following region is required:
- "mdp"
These two chunks should probably go to the separate patch 'dt-bindings:...'.
Also, could you please pinpoint the reason for adding more compatibility strings, while they map to the same internal data? I think we might want instead to use some generic name for the dpu block, like "qcom,dpu" or "qcom,mdp-dpu" instead of specifying the platform name.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 5a8e3e1fc48c..fff12a4c8bfc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1219,6 +1219,8 @@ static const struct dev_pm_ops dpu_pm_ops = { static const struct of_device_id dpu_dt_match[] = { { .compatible = "qcom,sdm845-dpu", }, { .compatible = "qcom,sc7180-dpu", },
{ .compatible = "qcom,sm8150-dpu", },{ .compatible = "qcom,sm8250-dpu", }, {}}; MODULE_DEVICE_TABLE(of, dpu_dt_match); diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 94525ac76d4e..928f13d4bfbc 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1185,9 +1185,7 @@ static int add_display_components(struct device *dev, * Populate the children devices, find the MDP5/DPU node, and then add * the interfaces to our components list. */
if (of_device_is_compatible(dev->of_node, "qcom,mdss") ||of_device_is_compatible(dev->of_node, "qcom,sdm845-mdss") ||of_device_is_compatible(dev->of_node, "qcom,sc7180-mdss")) {
if (!of_device_is_compatible(dev->of_node, "qcom,mdp4")) { ret = of_platform_populate(dev->of_node, NULL, NULL, dev); if (ret) { DRM_DEV_ERROR(dev, "failed to populate children devices\n");@@ -1320,6 +1318,8 @@ static const struct of_device_id dt_match[] = { { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 }, { .compatible = "qcom,sdm845-mdss", .data = (void *)KMS_DPU }, { .compatible = "qcom,sc7180-mdss", .data = (void *)KMS_DPU },
{ .compatible = "qcom,sm8150-mdss", .data = (void *)KMS_DPU },{ .compatible = "qcom,sm8250-mdss", .data = (void *)KMS_DPU }, {}}; MODULE_DEVICE_TABLE(of, dt_match); -- 2.26.1
On 2/16/21 11:54 AM, Dmitry Baryshkov wrote:
On Mon, 15 Feb 2021 at 19:25, Jonathan Marek jonathan@marek.ca wrote:
The driver already has support for sm8150/sm8250, but the compatibles were never added.
Also inverse the non-mdp4 condition in add_display_components() to avoid having to check every new compatible in the condition.
Signed-off-by: Jonathan Marek jonathan@marek.ca
Documentation/devicetree/bindings/display/msm/dpu.txt | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++ drivers/gpu/drm/msm/msm_drv.c | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt index 551ae26f60da..5763f43200a0 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu.txt +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -8,7 +8,7 @@ The DPU display controller is found in SDM845 SoC.
MDSS: Required properties: -- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss" +- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss", "qcom,sm8150-mdss", "qcom,sm8250-mdss"
- reg: physical base address and length of contoller's registers.
- reg-names: register region names. The following region is required:
- "mdss"
@@ -41,7 +41,7 @@ Optional properties:
MDP: Required properties: -- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu" +- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu", "qcom,sm8150-dpu", "qcom,sm8250-dpu"
- reg: physical base address and length of controller's registers.
- reg-names : register region names. The following region is required:
- "mdp"
These two chunks should probably go to the separate patch 'dt-bindings:...'.
In this case I think its better to have this change in the same patch, but maybe one of the Robs will disagree.
Also, could you please pinpoint the reason for adding more compatibility strings, while they map to the same internal data? I think we might want instead to use some generic name for the dpu block, like "qcom,dpu" or "qcom,mdp-dpu" instead of specifying the platform name.
sdm845 and sc7180 aren't using generic compatibles, this is just being consistent with that.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 5a8e3e1fc48c..fff12a4c8bfc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1219,6 +1219,8 @@ static const struct dev_pm_ops dpu_pm_ops = { static const struct of_device_id dpu_dt_match[] = { { .compatible = "qcom,sdm845-dpu", }, { .compatible = "qcom,sc7180-dpu", },
{ .compatible = "qcom,sm8150-dpu", },{ .compatible = "qcom,sm8250-dpu", }, {}diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 94525ac76d4e..928f13d4bfbc 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1185,9 +1185,7 @@ static int add_display_components(struct device *dev, * Populate the children devices, find the MDP5/DPU node, and then add * the interfaces to our components list. */
if (of_device_is_compatible(dev->of_node, "qcom,mdss") ||of_device_is_compatible(dev->of_node, "qcom,sdm845-mdss") ||of_device_is_compatible(dev->of_node, "qcom,sc7180-mdss")) {
if (!of_device_is_compatible(dev->of_node, "qcom,mdp4")) { ret = of_platform_populate(dev->of_node, NULL, NULL, dev); if (ret) { DRM_DEV_ERROR(dev, "failed to populate children devices\n");@@ -1320,6 +1318,8 @@ static const struct of_device_id dt_match[] = { { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 }, { .compatible = "qcom,sdm845-mdss", .data = (void *)KMS_DPU }, { .compatible = "qcom,sc7180-mdss", .data = (void *)KMS_DPU },
{ .compatible = "qcom,sm8150-mdss", .data = (void *)KMS_DPU },{ .compatible = "qcom,sm8250-mdss", .data = (void *)KMS_DPU }, {}-- 2.26.1
On Tue, 16 Feb 2021 at 21:06, Jonathan Marek jonathan@marek.ca wrote:
On 2/16/21 11:54 AM, Dmitry Baryshkov wrote:
On Mon, 15 Feb 2021 at 19:25, Jonathan Marek jonathan@marek.ca wrote:
The driver already has support for sm8150/sm8250, but the compatibles were never added.
Also inverse the non-mdp4 condition in add_display_components() to avoid having to check every new compatible in the condition.
Signed-off-by: Jonathan Marek jonathan@marek.ca
Documentation/devicetree/bindings/display/msm/dpu.txt | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++ drivers/gpu/drm/msm/msm_drv.c | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt index 551ae26f60da..5763f43200a0 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu.txt +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -8,7 +8,7 @@ The DPU display controller is found in SDM845 SoC.
MDSS: Required properties: -- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss" +- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss", "qcom,sm8150-mdss", "qcom,sm8250-mdss"
- reg: physical base address and length of contoller's registers.
- reg-names: register region names. The following region is required:
- "mdss"
@@ -41,7 +41,7 @@ Optional properties:
MDP: Required properties: -- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu" +- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu", "qcom,sm8150-dpu", "qcom,sm8250-dpu"
- reg: physical base address and length of controller's registers.
- reg-names : register region names. The following region is required:
- "mdp"
These two chunks should probably go to the separate patch 'dt-bindings:...'.
In this case I think its better to have this change in the same patch, but maybe one of the Robs will disagree.
Also, could you please pinpoint the reason for adding more compatibility strings, while they map to the same internal data? I think we might want instead to use some generic name for the dpu block, like "qcom,dpu" or "qcom,mdp-dpu" instead of specifying the platform name.
sdm845 and sc7180 aren't using generic compatibles, this is just being consistent with that.
Well, I suppose the common case is to use the 'first compatible' entry if the entities are compatible. The generic compatibles is a proposal, not an affirmation. Please excuse me if it sounded in a different way.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 5a8e3e1fc48c..fff12a4c8bfc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1219,6 +1219,8 @@ static const struct dev_pm_ops dpu_pm_ops = { static const struct of_device_id dpu_dt_match[] = { { .compatible = "qcom,sdm845-dpu", }, { .compatible = "qcom,sc7180-dpu", },
{ .compatible = "qcom,sm8150-dpu", },{ .compatible = "qcom,sm8250-dpu", }, {}diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 94525ac76d4e..928f13d4bfbc 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1185,9 +1185,7 @@ static int add_display_components(struct device *dev, * Populate the children devices, find the MDP5/DPU node, and then add * the interfaces to our components list. */
if (of_device_is_compatible(dev->of_node, "qcom,mdss") ||of_device_is_compatible(dev->of_node, "qcom,sdm845-mdss") ||of_device_is_compatible(dev->of_node, "qcom,sc7180-mdss")) {
if (!of_device_is_compatible(dev->of_node, "qcom,mdp4")) { ret = of_platform_populate(dev->of_node, NULL, NULL, dev); if (ret) { DRM_DEV_ERROR(dev, "failed to populate children devices\n");@@ -1320,6 +1318,8 @@ static const struct of_device_id dt_match[] = { { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 }, { .compatible = "qcom,sdm845-mdss", .data = (void *)KMS_DPU }, { .compatible = "qcom,sc7180-mdss", .data = (void *)KMS_DPU },
{ .compatible = "qcom,sm8150-mdss", .data = (void *)KMS_DPU },{ .compatible = "qcom,sm8250-mdss", .data = (void *)KMS_DPU }, {}-- 2.26.1
On Tue, Feb 16, 2021 at 10:06 AM Jonathan Marek jonathan@marek.ca wrote:
On 2/16/21 11:54 AM, Dmitry Baryshkov wrote:
On Mon, 15 Feb 2021 at 19:25, Jonathan Marek jonathan@marek.ca wrote:
The driver already has support for sm8150/sm8250, but the compatibles were never added.
Also inverse the non-mdp4 condition in add_display_components() to avoid having to check every new compatible in the condition.
Signed-off-by: Jonathan Marek jonathan@marek.ca
Documentation/devicetree/bindings/display/msm/dpu.txt | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 ++ drivers/gpu/drm/msm/msm_drv.c | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt index 551ae26f60da..5763f43200a0 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu.txt +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -8,7 +8,7 @@ The DPU display controller is found in SDM845 SoC.
MDSS: Required properties: -- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss" +- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss", "qcom,sm8150-mdss", "qcom,sm8250-mdss"
- reg: physical base address and length of contoller's registers.
- reg-names: register region names. The following region is required:
- "mdss"
@@ -41,7 +41,7 @@ Optional properties:
MDP: Required properties: -- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu" +- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu", "qcom,sm8150-dpu", "qcom,sm8250-dpu"
- reg: physical base address and length of controller's registers.
- reg-names : register region names. The following region is required:
- "mdp"
These two chunks should probably go to the separate patch 'dt-bindings:...'.
In this case I think its better to have this change in the same patch, but maybe one of the Robs will disagree.
I *think* typically the reason to split dt bindings into their own patch is that devicetree@ list isn't interested in reviewing driver changes, just binding changes..
In this case since it is just adding a compatible I think it is ok.. (or at least ok by me, but maybe other-Rob disagrees ;-))
Also, could you please pinpoint the reason for adding more compatibility strings, while they map to the same internal data? I think we might want instead to use some generic name for the dpu block, like "qcom,dpu" or "qcom,mdp-dpu" instead of specifying the platform name.
sdm845 and sc7180 aren't using generic compatibles, this is just being consistent with that.
It is good to have a device specific compatible up front, even if we fallback to the more generic one for matching.. just in case we find a reason for needing it later
BR, -R
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 5a8e3e1fc48c..fff12a4c8bfc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1219,6 +1219,8 @@ static const struct dev_pm_ops dpu_pm_ops = { static const struct of_device_id dpu_dt_match[] = { { .compatible = "qcom,sdm845-dpu", }, { .compatible = "qcom,sc7180-dpu", },
{ .compatible = "qcom,sm8150-dpu", },{ .compatible = "qcom,sm8250-dpu", }, {}diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 94525ac76d4e..928f13d4bfbc 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1185,9 +1185,7 @@ static int add_display_components(struct device *dev, * Populate the children devices, find the MDP5/DPU node, and then add * the interfaces to our components list. */
if (of_device_is_compatible(dev->of_node, "qcom,mdss") ||of_device_is_compatible(dev->of_node, "qcom,sdm845-mdss") ||of_device_is_compatible(dev->of_node, "qcom,sc7180-mdss")) {
if (!of_device_is_compatible(dev->of_node, "qcom,mdp4")) { ret = of_platform_populate(dev->of_node, NULL, NULL, dev); if (ret) { DRM_DEV_ERROR(dev, "failed to populate children devices\n");@@ -1320,6 +1318,8 @@ static const struct of_device_id dt_match[] = { { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 }, { .compatible = "qcom,sdm845-mdss", .data = (void *)KMS_DPU }, { .compatible = "qcom,sc7180-mdss", .data = (void *)KMS_DPU },
{ .compatible = "qcom,sm8150-mdss", .data = (void *)KMS_DPU },{ .compatible = "qcom,sm8250-mdss", .data = (void *)KMS_DPU }, {}-- 2.26.1
dri-devel@lists.freedesktop.org
-
Dmitry Baryshkov -
Jonathan Marek -
Rob Clark