[1/3,v2] drm/vc4: Turn the V3D clock on at runtime.

Submitted by Eric Anholt on April 18, 2017, 11:38 p.m.

Details

Message ID 20170418233805.15767-1-eric@anholt.net
State New
Headers show

Commit Message

Eric Anholt April 18, 2017, 11:38 p.m.
For the Raspberry Pi's bindings, the power domain also implicitly
turns on the clock and deasserts reset, but for the new Cygnus port we
start representing the clock in the devicetree.

v2: Document the clock-names property, check for -ENOENT for no clock
    in DT.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 .../devicetree/bindings/display/brcm,bcm-vc4.txt   |  4 +++
 drivers/gpu/drm/vc4/vc4_drv.h                      |  1 +
 drivers/gpu/drm/vc4/vc4_v3d.c                      | 38 +++++++++++++++++++++-
 3 files changed, 42 insertions(+), 1 deletion(-)

Comments

Florian Fainelli April 18, 2017, 11:48 p.m.
On 04/18/2017 04:38 PM, Eric Anholt wrote:
> For the Raspberry Pi's bindings, the power domain also implicitly
> turns on the clock and deasserts reset, but for the new Cygnus port we
> start representing the clock in the devicetree.
> 
> v2: Document the clock-names property, check for -ENOENT for no clock
>     in DT.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---

> +	if (v3d->clk)
> +		clk_disable_unprepare(v3d->clk);

The clock API allows you to pass a NULL clk and do nothing in these
cases which is what you seem to have done a few lines below, you could
simplify these checks?

> +
>  	return 0;
>  }
>  
> @@ -318,6 +322,13 @@ static int vc4_v3d_runtime_resume(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> +	if (v3d->clk) {
> +		int ret = clk_prepare_enable(v3d->clk);
> +
> +		if (ret != 0)
> +			return ret;
> +	}
> +
>  	vc4_v3d_init_hw(vc4->dev);
>  	vc4_irq_postinstall(vc4->dev);
>  
> @@ -348,15 +359,40 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
>  	vc4->v3d = v3d;
>  	v3d->vc4 = vc4;
>  
> +	v3d->clk = devm_clk_get(dev, "v3d_clk");
> +	if (IS_ERR(v3d->clk)) {
> +		int ret = PTR_ERR(v3d->clk);
> +
> +		if (ret == -ENOENT) {
> +			/* bcm2835 didn't have a clock reference in the DT. */
> +			ret = 0;
> +			v3d->clk = NULL;
> +		} else {
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(dev, "Failed to get V3D clock: %d\n",
> +					ret);
> +			return ret;
> +		}
> +	}
> +
>  	if (V3D_READ(V3D_IDENT0) != V3D_EXPECTED_IDENT0) {
>  		DRM_ERROR("V3D_IDENT0 read 0x%08x instead of 0x%08x\n",
>  			  V3D_READ(V3D_IDENT0), V3D_EXPECTED_IDENT0);
>  		return -EINVAL;
>  	}
>  
> +	if (v3d->clk) {
> +		ret = clk_prepare_enable(v3d->clk);
> +		if (ret != 0)
> +			return ret;
> +	}
> +
>  	ret = vc4_allocate_bin_bo(drm);
> -	if (ret)
> +	if (ret) {
> +		if (v3d->clk)
> +			clk_disable_unprepare(v3d->clk);
>  		return ret;
> +	}
>  
>  	/* Reset the binner overflow address/size at setup, to be sure
>  	 * we don't reuse an old one.
>
Eric Anholt April 19, 2017, 12:02 a.m.
Florian Fainelli <f.fainelli@gmail.com> writes:

> On 04/18/2017 04:38 PM, Eric Anholt wrote:
>> For the Raspberry Pi's bindings, the power domain also implicitly
>> turns on the clock and deasserts reset, but for the new Cygnus port we
>> start representing the clock in the devicetree.
>> 
>> v2: Document the clock-names property, check for -ENOENT for no clock
>>     in DT.
>> 
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>
>> +	if (v3d->clk)
>> +		clk_disable_unprepare(v3d->clk);
>
> The clock API allows you to pass a NULL clk and do nothing in these
> cases which is what you seem to have done a few lines below, you could
> simplify these checks?

Sounds good.
Eric Anholt April 19, 2017, 12:02 a.m.
Florian Fainelli <f.fainelli@gmail.com> writes:

> On 04/18/2017 04:38 PM, Eric Anholt wrote:
>> For the Raspberry Pi's bindings, the power domain also implicitly
>> turns on the clock and deasserts reset, but for the new Cygnus port we
>> start representing the clock in the devicetree.
>> 
>> v2: Document the clock-names property, check for -ENOENT for no clock
>>     in DT.
>> 
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>
>> +	if (v3d->clk)
>> +		clk_disable_unprepare(v3d->clk);
>
> The clock API allows you to pass a NULL clk and do nothing in these
> cases which is what you seem to have done a few lines below, you could
> simplify these checks?

Sounds good.

Patch hide | download patch | download mbox

diff --git a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
index ca02d3e4db91..2318266f6481 100644
--- a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
+++ b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
@@ -59,6 +59,10 @@  Required properties for V3D:
 - interrupts:	The interrupt number
 		  See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
 
+Optional properties for V3D:
+- clocks:	The clock the unit runs on
+- clock-names:	Must be "v3d_clk"
+
 Required properties for DSI:
 - compatible:	Should be "brcm,bcm2835-dsi0" or "brcm,bcm2835-dsi1"
 - reg:		Physical base address and length of the DSI block's registers
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index b0967e2f7e88..92eb7d811bf2 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -200,6 +200,7 @@  struct vc4_v3d {
 	struct vc4_dev *vc4;
 	struct platform_device *pdev;
 	void __iomem *regs;
+	struct clk *clk;
 };
 
 struct vc4_hvs {
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index a88078d7c9d1..ca987d2800c6 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -16,6 +16,7 @@ 
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "linux/clk.h"
 #include "linux/component.h"
 #include "linux/pm_runtime.h"
 #include "vc4_drv.h"
@@ -305,6 +306,9 @@  static int vc4_v3d_runtime_suspend(struct device *dev)
 	drm_gem_object_put_unlocked(&vc4->bin_bo->base.base);
 	vc4->bin_bo = NULL;
 
+	if (v3d->clk)
+		clk_disable_unprepare(v3d->clk);
+
 	return 0;
 }
 
@@ -318,6 +322,13 @@  static int vc4_v3d_runtime_resume(struct device *dev)
 	if (ret)
 		return ret;
 
+	if (v3d->clk) {
+		int ret = clk_prepare_enable(v3d->clk);
+
+		if (ret != 0)
+			return ret;
+	}
+
 	vc4_v3d_init_hw(vc4->dev);
 	vc4_irq_postinstall(vc4->dev);
 
@@ -348,15 +359,40 @@  static int vc4_v3d_bind(struct device *dev, struct device *master, void *data)
 	vc4->v3d = v3d;
 	v3d->vc4 = vc4;
 
+	v3d->clk = devm_clk_get(dev, "v3d_clk");
+	if (IS_ERR(v3d->clk)) {
+		int ret = PTR_ERR(v3d->clk);
+
+		if (ret == -ENOENT) {
+			/* bcm2835 didn't have a clock reference in the DT. */
+			ret = 0;
+			v3d->clk = NULL;
+		} else {
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Failed to get V3D clock: %d\n",
+					ret);
+			return ret;
+		}
+	}
+
 	if (V3D_READ(V3D_IDENT0) != V3D_EXPECTED_IDENT0) {
 		DRM_ERROR("V3D_IDENT0 read 0x%08x instead of 0x%08x\n",
 			  V3D_READ(V3D_IDENT0), V3D_EXPECTED_IDENT0);
 		return -EINVAL;
 	}
 
+	if (v3d->clk) {
+		ret = clk_prepare_enable(v3d->clk);
+		if (ret != 0)
+			return ret;
+	}
+
 	ret = vc4_allocate_bin_bo(drm);
-	if (ret)
+	if (ret) {
+		if (v3d->clk)
+			clk_disable_unprepare(v3d->clk);
 		return ret;
+	}
 
 	/* Reset the binner overflow address/size at setup, to be sure
 	 * we don't reuse an old one.