diff mbox

[RFCv2] drm/msm: DT support for 8960/8064

Message ID 1404835244-3735-1-git-send-email-robdclark@gmail.com (mailing list archive)
State Accepted
Headers show

Commit Message

Rob Clark July 8, 2014, 4 p.m. UTC
Now that we (almost) have enough dependencies in place (MMCC, RPM, etc),
add necessary DT support so that we can use drm/msm on upstream kernel.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
I thought I sent this already, but looks like I've forgot.

I've updated the names for the GPIO properties (and, via helper, made
driver check both with and without -gpio suffix).  Added clock-names,
etc.

For now I've removed the compatible string for 8074 hdmi phy.  Due to
integration differences between mdp4 and mdp5 devices, it may end up
being easier to document the bindings separately (even though most of
the code is in common).  When paired with mdp5 display controller, for
example, the HDMI block does not have it's own irq.

 Documentation/devicetree/bindings/drm/msm/gpu.txt  | 52 +++++++++++++++++
 Documentation/devicetree/bindings/drm/msm/hdmi.txt | 46 +++++++++++++++
 Documentation/devicetree/bindings/drm/msm/mdp.txt  | 48 +++++++++++++++
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c              |  2 +
 drivers/gpu/drm/msm/hdmi/hdmi.c                    | 68 ++++++++++++++--------
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c            | 10 +++-
 drivers/gpu/drm/msm/msm_drv.c                      | 29 +++++++--
 7 files changed, 225 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/drm/msm/gpu.txt
 create mode 100644 Documentation/devicetree/bindings/drm/msm/hdmi.txt
 create mode 100644 Documentation/devicetree/bindings/drm/msm/mdp.txt

Comments

divya ojha July 17, 2014, 8:10 a.m. UTC | #1
Hi Rob,

On Tue, Jul 8, 2014 at 9:30 PM, Rob Clark <robdclark@gmail.com> wrote:
> Now that we (almost) have enough dependencies in place (MMCC, RPM, etc),
> add necessary DT support so that we can use drm/msm on upstream kernel.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> I thought I sent this already, but looks like I've forgot.
..
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 7f7aade..041c2fc 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -123,7 +123,8 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder)
>         for (i = 0; i < config->hpd_reg_cnt; i++) {
>                 struct regulator *reg;
>
> -               reg = devm_regulator_get(&pdev->dev, config->hpd_reg_names[i]);
> +               reg = devm_regulator_get_exclusive(&pdev->dev,
> +                               config->hpd_reg_names[i]);
>                 if (IS_ERR(reg)) {
>                         ret = PTR_ERR(reg);
>                         dev_err(dev->dev, "failed to get hpd regulator: %s (%d)\n",
> @@ -138,7 +139,8 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder)
>         for (i = 0; i < config->pwr_reg_cnt; i++) {
>                 struct regulator *reg;
>
> -               reg = devm_regulator_get(&pdev->dev, config->pwr_reg_names[i]);
> +               reg = devm_regulator_get_exclusive(&pdev->dev,
> +                               config->pwr_reg_names[i]);
>                 if (IS_ERR(reg)) {
>                         ret = PTR_ERR(reg);
>                         dev_err(dev->dev, "failed to get pwr regulator: %s (%d)\n",

Don't we need to have a if(regulator_enabled) check after
devm_regulator_get function ?
I see a similar test after camera regulator_get function call.

-Regards
Divya
Rob Clark July 17, 2014, 3:29 p.m. UTC | #2
On Thu, Jul 17, 2014 at 4:10 AM, divya ojha <odivya77@gmail.com> wrote:
> Hi Rob,
>
> On Tue, Jul 8, 2014 at 9:30 PM, Rob Clark <robdclark@gmail.com> wrote:
>> Now that we (almost) have enough dependencies in place (MMCC, RPM, etc),
>> add necessary DT support so that we can use drm/msm on upstream kernel.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>> I thought I sent this already, but looks like I've forgot.
> ..
>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
>> index 7f7aade..041c2fc 100644
>> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
>> @@ -123,7 +123,8 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder)
>>         for (i = 0; i < config->hpd_reg_cnt; i++) {
>>                 struct regulator *reg;
>>
>> -               reg = devm_regulator_get(&pdev->dev, config->hpd_reg_names[i]);
>> +               reg = devm_regulator_get_exclusive(&pdev->dev,
>> +                               config->hpd_reg_names[i]);
>>                 if (IS_ERR(reg)) {
>>                         ret = PTR_ERR(reg);
>>                         dev_err(dev->dev, "failed to get hpd regulator: %s (%d)\n",
>> @@ -138,7 +139,8 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder)
>>         for (i = 0; i < config->pwr_reg_cnt; i++) {
>>                 struct regulator *reg;
>>
>> -               reg = devm_regulator_get(&pdev->dev, config->pwr_reg_names[i]);
>> +               reg = devm_regulator_get_exclusive(&pdev->dev,
>> +                               config->pwr_reg_names[i]);
>>                 if (IS_ERR(reg)) {
>>                         ret = PTR_ERR(reg);
>>                         dev_err(dev->dev, "failed to get pwr regulator: %s (%d)\n",
>
> Don't we need to have a if(regulator_enabled) check after
> devm_regulator_get function ?
> I see a similar test after camera regulator_get function call.

tbh, I'm not 100% sure.

Normally I would say that any driver using some resource (regulator,
etc) should take it's own reference irrespective of whether some other
driver already has the regulator/etc, enabled.  Otherwise the
reference counting doesn't work out.

The thing I'm not 100% sure about is interaction w/ bootloader, in
cases where bootloader does initial modeset.  I've had some problems
in this area in the past when I was trying to get DSI working on my
nexus4.  I doubt the correct solution is for the driver to check if
regulator is already enabled by bootloader before enabling it, but I'm
not sure if something else somewhere needs to drop references to
resources enabled by bootloader.

BR,
-R

> -Regards
> Divya
Bjorn Andersson July 17, 2014, 4:08 p.m. UTC | #3
On Thu, Jul 17, 2014 at 8:29 AM, Rob Clark <robdclark@gmail.com> wrote:
> On Thu, Jul 17, 2014 at 4:10 AM, divya ojha <odivya77@gmail.com> wrote:
[...]
>> Don't we need to have a if(regulator_enabled) check after
>> devm_regulator_get function ?
>> I see a similar test after camera regulator_get function call.
>
> tbh, I'm not 100% sure.
>

Regulators in the Qualcomm platform works by casting votes with the
RPM and there is no way to query the current state of a regulator, so
the regulator code will only return information regarding the Linux
systems votes.

So, we can unfortunately not check this.

This setup also makes it important that if we want a regulator on and
set to a certain voltage then we need to cast our vote with these
values, or some other subsystem might lower it by releasing their
votes.

Regards,
Bjorn
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt b/Documentation/devicetree/bindings/drm/msm/gpu.txt
new file mode 100644
index 0000000..67d0a58
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt
@@ -0,0 +1,52 @@ 
+Qualcomm adreno/snapdragon GPU
+
+Required properties:
+- compatible: "qcom,adreno-3xx"
+- reg: Physical base address and length of the controller's registers.
+- interrupts: The interrupt signal from the gpu.
+- clocks: device clocks
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: the following clocks are required:
+  * "core_clk"
+  * "iface_clk"
+  * "mem_iface_clk"
+- qcom,chipid: gpu chip-id.  Note this may become optional for future
+  devices if we can reliably read the chipid from hw
+- qcom,gpu-pwrlevels: list of operating points
+  - compatible: "qcom,gpu-pwrlevels"
+  - for each qcom,gpu-pwrlevel:
+    - qcom,gpu-freq: requested gpu clock speed
+    - NOTE: downstream android driver defines additional parameters to
+      configure memory bandwidth scaling per OPP.
+
+Example:
+
+/ {
+	...
+
+	gpu: qcom,kgsl-3d0@4300000 {
+		compatible = "qcom,adreno-3xx";
+		reg = <0x04300000 0x20000>;
+		reg-names = "kgsl_3d0_reg_memory";
+		interrupts = <GIC_SPI 80 0>;
+		interrupt-names = "kgsl_3d0_irq";
+		clock-names =
+		    "core_clk",
+		    "iface_clk",
+		    "mem_iface_clk";
+		clocks =
+		    <&mmcc GFX3D_CLK>,
+		    <&mmcc GFX3D_AHB_CLK>,
+		    <&mmcc MMSS_IMEM_AHB_CLK>;
+		qcom,chipid = <0x03020100>;
+		qcom,gpu-pwrlevels {
+			compatible = "qcom,gpu-pwrlevels";
+			qcom,gpu-pwrlevel@0 {
+				qcom,gpu-freq = <450000000>;
+			};
+			qcom,gpu-pwrlevel@1 {
+				qcom,gpu-freq = <27000000>;
+			};
+		};
+	};
+};
diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt
new file mode 100644
index 0000000..aca917f
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt
@@ -0,0 +1,46 @@ 
+Qualcomm adreno/snapdragon hdmi output
+
+Required properties:
+- compatible: one of the following
+   * "qcom,hdmi-tx-8660"
+   * "qcom,hdmi-tx-8960"
+- reg: Physical base address and length of the controller's registers
+- reg-names: "core_physical"
+- interrupts: The interrupt signal from the hdmi block.
+- clocks: device clocks
+  See ../clocks/clock-bindings.txt for details.
+- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin
+- qcom,hdmi-tx-ddc-data-gpio: ddc data pin
+- qcom,hdmi-tx-hpd-gpio: hpd pin
+- core-vdda-supply: phandle to supply regulator
+- hdmi-mux-supply: phandle to mux regulator
+
+Optional properties:
+- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin
+- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin
+
+Example:
+
+/ {
+	...
+
+	hdmi: qcom,hdmi-tx-8960@4a00000 {
+		compatible = "qcom,hdmi-tx-8960";
+		reg-names = "core_physical";
+		reg = <0x04a00000 0x1000>;
+		interrupts = <GIC_SPI 79 0>;
+		clock-names =
+		    "core_clk",
+		    "master_iface_clk",
+		    "slave_iface_clk";
+		clocks =
+		    <&mmcc HDMI_APP_CLK>,
+		    <&mmcc HDMI_M_AHB_CLK>,
+		    <&mmcc HDMI_S_AHB_CLK>;
+		qcom,hdmi-tx-ddc-clk = <&msmgpio 70 GPIO_ACTIVE_HIGH>;
+		qcom,hdmi-tx-ddc-data = <&msmgpio 71 GPIO_ACTIVE_HIGH>;
+		qcom,hdmi-tx-hpd = <&msmgpio 72 GPIO_ACTIVE_HIGH>;
+		core-vdda-supply = <&pm8921_hdmi_mvs>;
+		hdmi-mux-supply = <&ext_3p3v>;
+	};
+};
diff --git a/Documentation/devicetree/bindings/drm/msm/mdp.txt b/Documentation/devicetree/bindings/drm/msm/mdp.txt
new file mode 100644
index 0000000..1a0598e
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/msm/mdp.txt
@@ -0,0 +1,48 @@ 
+Qualcomm adreno/snapdragon display controller
+
+Required properties:
+- compatible:
+  * "qcom,mdp" - mdp4
+- reg: Physical base address and length of the controller's registers.
+- interrupts: The interrupt signal from the display controller.
+- connectors: array of phandles for output device(s)
+- clocks: device clocks
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: the following clocks are required:
+  * "core_clk"
+  * "iface_clk"
+  * "lut_clk"
+  * "src_clk"
+  * "hdmi_clk"
+  * "mpd_clk"
+
+Optional properties:
+- gpus: phandle for gpu device
+
+Example:
+
+/ {
+	...
+
+	mdp: qcom,mdp@5100000 {
+		compatible = "qcom,mdp";
+		reg = <0x05100000 0xf0000>;
+		interrupts = <GIC_SPI 75 0>;
+		connectors = <&hdmi>;
+		gpus = <&gpu>;
+		clock-names =
+		    "core_clk",
+		    "iface_clk",
+		    "lut_clk",
+		    "src_clk",
+		    "hdmi_clk",
+		    "mdp_clk";
+		clocks =
+		    <&mmcc MDP_SRC>,
+		    <&mmcc MDP_AHB_CLK>,
+		    <&mmcc MDP_LUT_CLK>,
+		    <&mmcc TV_SRC>,
+		    <&mmcc HDMI_TV_CLK>,
+		    <&mmcc MDP_TV_CLK>;
+	};
+};
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index a2cee06..2773600 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -680,6 +680,8 @@  static int a3xx_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id dt_match[] = {
+	{ .compatible = "qcom,adreno-3xx" },
+	/* for backwards compat w/ downstream kgsl DT files: */
 	{ .compatible = "qcom,kgsl-3d0" },
 	{}
 };
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 7f7aade..041c2fc 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -123,7 +123,8 @@  struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder)
 	for (i = 0; i < config->hpd_reg_cnt; i++) {
 		struct regulator *reg;
 
-		reg = devm_regulator_get(&pdev->dev, config->hpd_reg_names[i]);
+		reg = devm_regulator_get_exclusive(&pdev->dev,
+				config->hpd_reg_names[i]);
 		if (IS_ERR(reg)) {
 			ret = PTR_ERR(reg);
 			dev_err(dev->dev, "failed to get hpd regulator: %s (%d)\n",
@@ -138,7 +139,8 @@  struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder)
 	for (i = 0; i < config->pwr_reg_cnt; i++) {
 		struct regulator *reg;
 
-		reg = devm_regulator_get(&pdev->dev, config->pwr_reg_names[i]);
+		reg = devm_regulator_get_exclusive(&pdev->dev,
+				config->pwr_reg_names[i]);
 		if (IS_ERR(reg)) {
 			ret = PTR_ERR(reg);
 			dev_err(dev->dev, "failed to get pwr regulator: %s (%d)\n",
@@ -266,37 +268,55 @@  static int hdmi_bind(struct device *dev, struct device *master, void *data)
 	{
 		int gpio = of_get_named_gpio(of_node, name, 0);
 		if (gpio < 0) {
-			dev_err(dev, "failed to get gpio: %s (%d)\n",
-					name, gpio);
-			gpio = -1;
+			char name2[32];
+			snprintf(name2, sizeof(name2), "%s-gpio", name);
+			gpio = of_get_named_gpio(of_node, name2, 0);
+			if (gpio < 0) {
+				dev_err(dev, "failed to get gpio: %s (%d)\n",
+						name, gpio);
+				gpio = -1;
+			}
 		}
 		return gpio;
 	}
 
-	/* TODO actually use DT.. */
-	static const char *hpd_reg_names[] = {"hpd-gdsc", "hpd-5v"};
-	static const char *pwr_reg_names[] = {"core-vdda", "core-vcc"};
-	static const char *hpd_clk_names[] = {"iface_clk", "core_clk", "mdp_core_clk"};
-	static unsigned long hpd_clk_freq[] = {0, 19200000, 0};
-	static const char *pwr_clk_names[] = {"extp_clk", "alt_iface_clk"};
+	if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8074")) {
+		static const char *hpd_reg_names[] = {"hpd-gdsc", "hpd-5v"};
+		static const char *pwr_reg_names[] = {"core-vdda", "core-vcc"};
+		static const char *hpd_clk_names[] = {"iface_clk", "core_clk", "mdp_core_clk"};
+		static unsigned long hpd_clk_freq[] = {0, 19200000, 0};
+		static const char *pwr_clk_names[] = {"extp_clk", "alt_iface_clk"};
+		config.phy_init      = hdmi_phy_8x74_init;
+		config.hpd_reg_names = hpd_reg_names;
+		config.hpd_reg_cnt   = ARRAY_SIZE(hpd_reg_names);
+		config.pwr_reg_names = pwr_reg_names;
+		config.pwr_reg_cnt   = ARRAY_SIZE(pwr_reg_names);
+		config.hpd_clk_names = hpd_clk_names;
+		config.hpd_freq      = hpd_clk_freq;
+		config.hpd_clk_cnt   = ARRAY_SIZE(hpd_clk_names);
+		config.pwr_clk_names = pwr_clk_names;
+		config.pwr_clk_cnt   = ARRAY_SIZE(pwr_clk_names);
+		config.shared_irq    = true;
+	} else if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8960")) {
+		static const char *hpd_clk_names[] = {"core_clk", "master_iface_clk", "slave_iface_clk"};
+		static const char *hpd_reg_names[] = {"core-vdda", "hdmi-mux"};
+		config.phy_init      = hdmi_phy_8960_init;
+		config.hpd_reg_names = hpd_reg_names;
+		config.hpd_reg_cnt   = ARRAY_SIZE(hpd_reg_names);
+		config.hpd_clk_names = hpd_clk_names;
+		config.hpd_clk_cnt   = ARRAY_SIZE(hpd_clk_names);
+	} else if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8660")) {
+		config.phy_init      = hdmi_phy_8x60_init;
+	} else {
+		dev_err(dev, "unknown phy: %s\n", of_node->name);
+	}
 
-	config.phy_init      = hdmi_phy_8x74_init;
 	config.mmio_name     = "core_physical";
-	config.hpd_reg_names = hpd_reg_names;
-	config.hpd_reg_cnt   = ARRAY_SIZE(hpd_reg_names);
-	config.pwr_reg_names = pwr_reg_names;
-	config.pwr_reg_cnt   = ARRAY_SIZE(pwr_reg_names);
-	config.hpd_clk_names = hpd_clk_names;
-	config.hpd_freq      = hpd_clk_freq;
-	config.hpd_clk_cnt   = ARRAY_SIZE(hpd_clk_names);
-	config.pwr_clk_names = pwr_clk_names;
-	config.pwr_clk_cnt   = ARRAY_SIZE(pwr_clk_names);
 	config.ddc_clk_gpio  = get_gpio("qcom,hdmi-tx-ddc-clk");
 	config.ddc_data_gpio = get_gpio("qcom,hdmi-tx-ddc-data");
 	config.hpd_gpio      = get_gpio("qcom,hdmi-tx-hpd");
 	config.mux_en_gpio   = get_gpio("qcom,hdmi-tx-mux-en");
 	config.mux_sel_gpio  = get_gpio("qcom,hdmi-tx-mux-sel");
-	config.shared_irq    = true;
 
 #else
 	static const char *hpd_clk_names[] = {
@@ -373,7 +393,9 @@  static int hdmi_dev_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id dt_match[] = {
-	{ .compatible = "qcom,hdmi-tx" },
+	{ .compatible = "qcom,hdmi-tx-8074" },
+	{ .compatible = "qcom,hdmi-tx-8960" },
+	{ .compatible = "qcom,hdmi-tx-8660" },
 	{}
 };
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
index 0bb4faa..5a7bfd4 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
@@ -294,15 +294,17 @@  struct msm_kms *mdp4_kms_init(struct drm_device *dev)
 		goto fail;
 	}
 
-	mdp4_kms->dsi_pll_vdda = devm_regulator_get(&pdev->dev, "dsi_pll_vdda");
+	mdp4_kms->dsi_pll_vdda =
+			devm_regulator_get_optional(&pdev->dev, "dsi_pll_vdda");
 	if (IS_ERR(mdp4_kms->dsi_pll_vdda))
 		mdp4_kms->dsi_pll_vdda = NULL;
 
-	mdp4_kms->dsi_pll_vddio = devm_regulator_get(&pdev->dev, "dsi_pll_vddio");
+	mdp4_kms->dsi_pll_vddio =
+			devm_regulator_get_optional(&pdev->dev, "dsi_pll_vddio");
 	if (IS_ERR(mdp4_kms->dsi_pll_vddio))
 		mdp4_kms->dsi_pll_vddio = NULL;
 
-	mdp4_kms->vdd = devm_regulator_get(&pdev->dev, "vdd");
+	mdp4_kms->vdd = devm_regulator_get_exclusive(&pdev->dev, "vdd");
 	if (IS_ERR(mdp4_kms->vdd))
 		mdp4_kms->vdd = NULL;
 
@@ -406,6 +408,8 @@  static struct mdp4_platform_config *mdp4_get_config(struct platform_device *dev)
 	static struct mdp4_platform_config config = {};
 #ifdef CONFIG_OF
 	/* TODO */
+	config.max_clk = 266667000;
+	config.iommu = iommu_domain_alloc(&platform_bus_type);
 #else
 	if (cpu_is_apq8064())
 		config.max_clk = 266667000;
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9a5d87d..1ad0155 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -906,7 +906,8 @@  static int compare_of(struct device *dev, void *data)
 	return dev->of_node == data;
 }
 
-static int msm_drm_add_components(struct device *master, struct master *m)
+static int add_components(struct device *master, struct master *m,
+		const char *name)
 {
 	struct device_node *np = master->of_node;
 	unsigned i;
@@ -915,16 +916,35 @@  static int msm_drm_add_components(struct device *master, struct master *m)
 	for (i = 0; ; i++) {
 		struct device_node *node;
 
-		node = of_parse_phandle(np, "connectors", i);
+		node = of_parse_phandle(np, name, i);
 		if (!node)
 			break;
 
 		ret = component_master_add_child(m, compare_of, node);
 		of_node_put(node);
 
-		if (ret)
+		if (ret) {
+			dev_err(master, "could not add %s child %s: %d\n",
+					name, node->name, ret);
 			return ret;
+		}
 	}
+
+	return 0;
+}
+
+static int msm_drm_add_components(struct device *master, struct master *m)
+{
+	int ret;
+
+	ret = add_components(master, m, "connectors");
+	if (ret)
+		return ret;
+
+	ret = add_components(master, m, "gpus");
+	if (ret)
+		return ret;
+
 	return 0;
 }
 #else
@@ -1008,7 +1028,8 @@  static const struct platform_device_id msm_id[] = {
 };
 
 static const struct of_device_id dt_match[] = {
-	{ .compatible = "qcom,mdss_mdp" },
+	{ .compatible = "qcom,mdp" },      /* mdp4 */
+	{ .compatible = "qcom,mdss_mdp" }, /* mdp5 */
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);