Re: [DPU PATCH v2 03/12] drm/msm/dpu: add MDSS top level driver for dpu

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2018-05-11 20:58, Sean Paul wrote:
On Fri, May 11, 2018 at 08:19:29PM +0530, Rajesh Yadav wrote:
SoCs containing dpu have a MDSS top level wrapper
which includes sub-blocks as dpu, dsi, phy, dp etc.
MDSS top level wrapper manages common resources like
common clocks, power and irq for its sub-blocks.

Currently, in dpu driver, all the power resource
management is part of power_handle which manages
these resources via a custom implementation. And
the resource relationships are not modelled properly
in dt.  Moreover the irq domain handling code is part
of dpu device (which is a child device) due to lack
of a dedicated driver for MDSS top level wrapper
device.

This change adds dpu_mdss top level driver to handle
common clock like - core clock, ahb clock
(for register access), main power supply (i.e. gdsc)
and irq management.
The top level mdss device/driver acts as an interrupt
controller and manage hwirq mapping for its child
devices.

It implements runtime_pm support for resource management.
Child nodes can control these resources via runtime_pm
get/put calls on their corresponding devices due to parent
child relationship defined in dt.

Changes in v2:
	- merge _dpu_mdss_hw_rev_init to dpu_mdss_init (Sean Paul)
	- merge _dpu_mdss_get_intr_sources to dpu_mdss_irq (Sean Paul)
	- fix indentation for irq_find_mapping call (Sean Paul)
	- remove unnecessary goto statements from dpu_mdss_irq (Sean Paul)
	- remove redundant param checks from
	  dpu_mdss_irq_mask/unmask (Sean Paul/Jordan Crouse)
	- remove redundant param checks from
	  dpu_mdss_irqdomain_map (Sean Paul/Jordan Crouse)
- return error code from dpu_mdss_enable/disable (Sean Paul/Jordan Crouse)
	- remove redundant param check from dpu_mdss_destroy (Sean Paul)
	- remove explicit calls to devm_kfree (Sean Paul/Jordan Crouse)
	- remove compatibility check from dpu_mdss_init as
	  it is conditionally called from msm_drv (Sean Paul)
	- reworked msm_dss_parse_clock() to add return checks for
	  of_property_read_* calls, fix log message and
	  fix alignment issues (Sean Paul/Jordan Crouse)
	- remove extra line before dpu_mdss_init (Sean Paul)
	- remove redundant param checks from __intr_offset and
	  make it a void function to avoid unnecessary error
	  handling from caller (Jordan Crouse)
	- remove redundant param check from dpu_mdss_irq (Jordan Crouse)
	- change mdss address space log message to debug and use %pK for
	  kernel pointers (Jordan Crouse)
- remove unnecessary log message from msm_dss_parse_clock (Jordan Crouse)
	- don't export msm_dss_parse_clock since it is used
	  only by dpu driver (Jordan Crouse)

Signed-off-by: Rajesh Yadav <ryadav@xxxxxxxxxxxxxx>
---
 drivers/gpu/drm/msm/Makefile                      |   1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.c      |  97 ---------
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h      |  14 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    |   9 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |   7 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c |  28 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  11 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_irq.c           |  48 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c           |   6 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h           |   2 -
drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 254 ++++++++++++++++++++++
 drivers/gpu/drm/msm/dpu_io_util.c                 |  57 +++++
 drivers/gpu/drm/msm/msm_drv.c                     |  26 ++-
 drivers/gpu/drm/msm/msm_drv.h                     |   2 +-
 drivers/gpu/drm/msm/msm_kms.h                     |   1 +
 include/linux/dpu_io_util.h                       |   2 +
 16 files changed, 339 insertions(+), 226 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c


/snip

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
new file mode 100644
index 0000000..ce680ea
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c

/snip

+
+int dpu_mdss_init(struct drm_device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev->dev);
+	struct msm_drm_private *priv = dev->dev_private;
+	struct dpu_mdss *dpu_mdss;
+	struct dss_module_power *mp;
+	int ret = 0;
+
+	dpu_mdss = devm_kzalloc(dev->dev, sizeof(*dpu_mdss), GFP_KERNEL);
+	if (!dpu_mdss)
+		return -ENOMEM;
+
+	dpu_mdss->mmio = msm_ioremap(pdev, "mdss_phys", "mdss_phys");
+	if (IS_ERR(dpu_mdss->mmio)) {
+		ret = PTR_ERR(dpu_mdss->mmio);

remove this ...

+		DPU_ERROR("mdss register memory map failed: %d\n", ret);
+		dpu_mdss->mmio = NULL;
+		return ret;

... and replace with
                return PTR_ERR(dpu_mdss->mmio);

Sure, I'll update in v3.

+	}
+	DRM_DEBUG("mapped mdss address space @%pK\n", dpu_mdss->mmio);
+	dpu_mdss->mmio_len = msm_iomap_size(pdev, "mdss_phys");
+
+	mp = &dpu_mdss->mp;
+	ret = msm_dss_parse_clock(pdev, mp);
+	if (ret) {
+		DPU_ERROR("failed to parse clocks, ret=%d\n", ret);
+		goto clk_parse_err;
+	}
+
+	ret = msm_dss_get_clk(&pdev->dev, mp->clk_config, mp->num_clk);
+	if (ret) {
+		DPU_ERROR("failed to get clocks, ret=%d\n", ret);
+		goto clk_get_error;
+	}
+
+	ret = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
+	if (ret) {
+		DPU_ERROR("failed to set clock rate, ret=%d\n", ret);
+		goto clk_rate_error;
+	}
+
+	dpu_mdss->base.dev = dev;
+	dpu_mdss->base.funcs = &mdss_funcs;
+
+	ret = _dpu_mdss_irq_domain_add(dpu_mdss);
+	if (ret)
+		goto irq_domain_error;
+
+	ret = devm_request_irq(dev->dev, platform_get_irq(pdev, 0),
+			dpu_mdss_irq, 0, "dpu_mdss_isr", dpu_mdss);
+	if (ret) {
+		DPU_ERROR("failed to init irq: %d\n", ret);
+		goto irq_error;
+	}
+
+	pm_runtime_enable(dev->dev);
+
+	pm_runtime_get_sync(dev->dev);
+	dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio);
+	pm_runtime_put_sync(dev->dev);
+
+	priv->mdss = &dpu_mdss->base;
+
+	return ret;
+
+irq_error:
+	_dpu_mdss_irq_domain_fini(dpu_mdss);
+irq_domain_error:
+clk_rate_error:
+	msm_dss_put_clk(mp->clk_config, mp->num_clk);
+clk_get_error:
+	devm_kfree(&pdev->dev, mp->clk_config);
+clk_parse_err:
+	if (dpu_mdss->mmio)
+		msm_iounmap(pdev, dpu_mdss->mmio);
+	dpu_mdss->mmio = NULL;
+	return ret;
+}
diff --git a/drivers/gpu/drm/msm/dpu_io_util.c b/drivers/gpu/drm/msm/dpu_io_util.c
index a18bc99..c44f33f 100644
--- a/drivers/gpu/drm/msm/dpu_io_util.c
+++ b/drivers/gpu/drm/msm/dpu_io_util.c
@@ -448,6 +448,63 @@ int msm_dss_enable_clk(struct dss_clk *clk_arry, int num_clk, int enable)
 } /* msm_dss_enable_clk */
 EXPORT_SYMBOL(msm_dss_enable_clk);

+int msm_dss_parse_clock(struct platform_device *pdev,
+		struct dss_module_power *mp)
+{
+	u32 i, rc = 0;
+	const char *clock_name;
+	u32 rate = 0, max_rate = 0;
+	int num_clk = 0;
+
+	if (!pdev || !mp)
+		return -EINVAL;
+
+	mp->num_clk = 0;
+ num_clk = of_property_count_strings(pdev->dev.of_node, "clock-names");
+	if (num_clk <= 0) {
+		pr_debug("clocks are not defined\n");
+		return 0;
+	}
+
+	mp->clk_config = devm_kzalloc(&pdev->dev,
+				      sizeof(struct dss_clk) * num_clk,
+				      GFP_KERNEL);
+	if (!mp->clk_config)
+		return -ENOMEM;
+
+	for (i = 0; i < num_clk; i++) {
+		rc = of_property_read_string_index(pdev->dev.of_node,
+						   "clock-names", i,
+						   &clock_name);
+		if (rc)
+			break;
+		strlcpy(mp->clk_config[i].clk_name, clock_name,
+			sizeof(mp->clk_config[i].clk_name));
+
+		rc = of_property_read_u32_index(pdev->dev.of_node,
+						"clock-rate", i,
+						&rate);
+		if (rc)
+			break;
+		mp->clk_config[i].rate = rate;
+		if (!mp->clk_config[i].rate)
+			mp->clk_config[i].type = DSS_CLK_AHB;
+		else
+			mp->clk_config[i].type = DSS_CLK_PCLK;
+
+		rc = of_property_read_u32_index(pdev->dev.of_node,
+						"clock-max-rate", i,
+						&max_rate);

Hmm, I missed these in my first review, these need new dt bindings. I'm
far from an expert on dt bindings, but I think you'll be asked to define these are clocks, and get the rate/max rate information from the clock subsystem
instead of breaking it all out like this.

Sean

I have checked the clock-bindings.txt and in the clock consumers example
I can see that clock-frequency binding is used, so I'll rename "clock-rate" to "clock-frequency". Regarding max-rate/frequency, I have not see any references for it in clock-bindings.txt.
This properties is mainly used in downstream driver, i'll remove it.

Thanks,
Rajesh

+		if (rc)
+			break;
+		mp->clk_config[i].max_rate = max_rate;
+	}
+
+	if (!rc)
+		mp->num_clk = num_clk;
+
+	return rc;
+}

 int dpu_i2c_byte_read(struct i2c_client *client, uint8_t slave_addr,
 			uint8_t reg_offset, uint8_t *read_buf)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 5d8f1b6..a0e73ea 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -503,7 +503,18 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
 	ddev->dev_private = priv;
 	priv->dev = ddev;

-	ret = mdp5_mdss_init(ddev);
+	switch (get_mdp_ver(pdev)) {
+	case KMS_MDP5:
+		ret = mdp5_mdss_init(ddev);
+		break;
+	case KMS_DPU:
+		ret = dpu_mdss_init(ddev);
+		break;
+	default:
+		ret = 0;
+		break;
+	}
+
 	if (ret)
 		goto mdss_init_fail;

@@ -1539,12 +1550,13 @@ static int add_display_components(struct device *dev,
 	int ret;

 	/*
- * MDP5 based devices don't have a flat hierarchy. There is a top level - * parent: MDSS, and children: MDP5, DSI, HDMI, eDP etc. Populate the
-	 * children devices, find the MDP5 node, and then add the interfaces
-	 * to our components list.
+ * MDP5/DPU based devices don't have a flat hierarchy. There is a top
+	 * level parent: MDSS, and children: MDP5/DPU, DSI, HDMI, eDP etc.
+ * 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")) {
+	if (of_device_is_compatible(dev->of_node, "qcom,mdss") ||
+		of_device_is_compatible(dev->of_node, "qcom,dpu-mdss")) {
 		ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
 		if (ret) {
 			dev_err(dev, "failed to populate children devices\n");
@@ -1686,7 +1698,7 @@ static int msm_pdev_remove(struct platform_device *pdev)
 	{ .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
 	{ .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
 #ifdef CONFIG_DRM_MSM_DPU
-	{ .compatible = "qcom,dpu-kms", .data = (void *)KMS_DPU },
+	{ .compatible = "qcom,dpu-mdss", .data = (void *)KMS_DPU },
 #endif
 	{}
 };
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 90a2521..e8e5e73 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -381,7 +381,7 @@ struct msm_drm_private {
 	/* subordinate devices, if present: */
 	struct platform_device *gpu_pdev;

-	/* top level MDSS wrapper device (for MDP5 only) */
+	/* top level MDSS wrapper device (for MDP5/DPU only) */
 	struct msm_mdss *mdss;

 	/* possibly this should be in the kms component, but it is
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 9a7bc7d..5e1de85 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -144,6 +144,7 @@ struct msm_mdss {
 };

 int mdp5_mdss_init(struct drm_device *dev);
+int dpu_mdss_init(struct drm_device *dev);

 /**
  * Mode Set Utility Functions
diff --git a/include/linux/dpu_io_util.h b/include/linux/dpu_io_util.h
index 7c73899..45e606f 100644
--- a/include/linux/dpu_io_util.h
+++ b/include/linux/dpu_io_util.h
@@ -104,6 +104,8 @@ int msm_dss_config_vreg(struct device *dev, struct dss_vreg *in_vreg,
 void msm_dss_put_clk(struct dss_clk *clk_arry, int num_clk);
 int msm_dss_clk_set_rate(struct dss_clk *clk_arry, int num_clk);
int msm_dss_enable_clk(struct dss_clk *clk_arry, int num_clk, int enable);
+int msm_dss_parse_clock(struct platform_device *pdev,
+		struct dss_module_power *mp);

 int dpu_i2c_byte_read(struct i2c_client *client, uint8_t slave_addr,
 		       uint8_t reg_offset, uint8_t *read_buf);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux