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

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

[Eric Anholt]

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.

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

diff --git a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
index ca02d3e4db91..bc1756f4f791 100644
--- a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
+++ b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
@@ -59,6 +59,9 @@ 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
+
 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 81d2bc08e766..08d5c2213c80 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -189,6 +189,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 7cc346ad9b0b..2442622e6bff 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"
@@ -164,6 +165,9 @@ static int vc4_v3d_runtime_suspend(struct device *dev)
 
 	vc4_irq_uninstall(vc4->dev);
 
+	if (v3d->clk)
+		clk_disable_unprepare(v3d->clk);
+
 	return 0;
 }
 
@@ -172,6 +176,13 @@ static int vc4_v3d_runtime_resume(struct device *dev)
 	struct vc4_v3d *v3d = dev_get_drvdata(dev);
 	struct vc4_dev *vc4 = v3d->vc4;
 
+	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);
 
@@ -202,12 +213,34 @@ 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 == -ENODEV) {
+			/* 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;
+	}
+
 	/* Reset the binner overflow address/size at setup, to be sure
 	 * we don't reuse an old one.
 	 */
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Eric Anholt]

The FBDEV initialization would throw an error in dmesg, when we just
want to silently not initialize fbdev on a V3D-only VC4 instance.

Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
---
 drivers/gpu/drm/vc4/vc4_kms.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index ad7925a9e0ea..237a504f11f0 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -230,10 +230,12 @@ int vc4_kms_load(struct drm_device *dev)
 
 	drm_mode_config_reset(dev);
 
-	vc4->fbdev = drm_fbdev_cma_init(dev, 32,
-					dev->mode_config.num_connector);
-	if (IS_ERR(vc4->fbdev))
-		vc4->fbdev = NULL;
+	if (dev->mode_config.num_connector) {
+		vc4->fbdev = drm_fbdev_cma_init(dev, 32,
+						dev->mode_config.num_connector);
+		if (IS_ERR(vc4->fbdev))
+			vc4->fbdev = NULL;
+	}
 
 	drm_kms_helper_poll_init(dev);
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Daniel Vetter]

On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt <eric@anholt.net> wrote:
> The FBDEV initialization would throw an error in dmesg, when we just
> want to silently not initialize fbdev on a V3D-only VC4 instance.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>

Hm, this shouldn't be an error really, you might want to hotplug more
connectors later on. What exactly complains?
-Daniel

> ---
>  drivers/gpu/drm/vc4/vc4_kms.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index ad7925a9e0ea..237a504f11f0 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -230,10 +230,12 @@ int vc4_kms_load(struct drm_device *dev)
>
>         drm_mode_config_reset(dev);
>
> -       vc4->fbdev = drm_fbdev_cma_init(dev, 32,
> -                                       dev->mode_config.num_connector);
> -       if (IS_ERR(vc4->fbdev))
> -               vc4->fbdev = NULL;
> +       if (dev->mode_config.num_connector) {
> +               vc4->fbdev = drm_fbdev_cma_init(dev, 32,
> +                                               dev->mode_config.num_connector);
> +               if (IS_ERR(vc4->fbdev))
> +                       vc4->fbdev = NULL;
> +       }
>
>         drm_kms_helper_poll_init(dev);
>
> --
> 2.11.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 544 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt <eric@anholt.net> wrote:
>> The FBDEV initialization would throw an error in dmesg, when we just
>> want to silently not initialize fbdev on a V3D-only VC4 instance.
>>
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>
> Hm, this shouldn't be an error really, you might want to hotplug more
> connectors later on. What exactly complains?

drm_fb_helper_init() throws an error if the passed in connector count is
0, so drm_fb_cma_helper() printks an error.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Daniel Vetter]

On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt <eric@anholt.net> wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
>> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt <eric@anholt.net> wrote:
>>> The FBDEV initialization would throw an error in dmesg, when we just
>>> want to silently not initialize fbdev on a V3D-only VC4 instance.
>>>
>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>
>> Hm, this shouldn't be an error really, you might want to hotplug more
>> connectors later on. What exactly complains?
>
> drm_fb_helper_init() throws an error if the passed in connector count is
> 0, so drm_fb_cma_helper() printks an error.

Oh, _that_ thing. The error in there is correct, but (almost) everyone
gets this parameter wrong. This isn't the max number of connectors the
fb helper will light up, but just the max number of connectors _per_
crtc when driving in hw clone mode. There's two problems with that:
- fb helpers don't support hw clone mode, we select 1:1 crtcs for each
active connector
- I mentioned that everyone gets this wrong?

If you're moderately bored it'd be great to nuke the max_connector
argument from drm_fb_helper_init, and hard-code it to 1 (with a big
comment explaining that this needs to be changed, probably with
dynamic reallocation, once someone gets around to implementing hw
clone mode).

If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
TODO.rst entry would be great in that case.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]

Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org> writes:

> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> wrote:
>> Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org> writes:
>>> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> wrote:
>>>> The FBDEV initialization would throw an error in dmesg, when we just
>>>> want to silently not initialize fbdev on a V3D-only VC4 instance.
>>>>
>>>> Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
>>>
>>> Hm, this shouldn't be an error really, you might want to hotplug more
>>> connectors later on. What exactly complains?
>>
>> drm_fb_helper_init() throws an error if the passed in connector count is
>> 0, so drm_fb_cma_helper() printks an error.
>
> Oh, _that_ thing. The error in there is correct, but (almost) everyone
> gets this parameter wrong. This isn't the max number of connectors the
> fb helper will light up, but just the max number of connectors _per_
> crtc when driving in hw clone mode. There's two problems with that:
> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each
> active connector
> - I mentioned that everyone gets this wrong?
>
> If you're moderately bored it'd be great to nuke the max_connector
> argument from drm_fb_helper_init, and hard-code it to 1 (with a big
> comment explaining that this needs to be changed, probably with
> dynamic reallocation, once someone gets around to implementing hw
> clone mode).
>
> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
> TODO.rst entry would be great in that case.

If I'm driving a GPU with no display subsystem at all, it seems like I
shouldn't initialize fbdev for it, right?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Alex Deucher]

On Fri, Apr 21, 2017 at 6:53 PM, Eric Anholt <eric@anholt.net> wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
>
>> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt <eric@anholt.net> wrote:
>>> Daniel Vetter <daniel@ffwll.ch> writes:
>>>> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt <eric@anholt.net> wrote:
>>>>> The FBDEV initialization would throw an error in dmesg, when we just
>>>>> want to silently not initialize fbdev on a V3D-only VC4 instance.
>>>>>
>>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>>>
>>>> Hm, this shouldn't be an error really, you might want to hotplug more
>>>> connectors later on. What exactly complains?
>>>
>>> drm_fb_helper_init() throws an error if the passed in connector count is
>>> 0, so drm_fb_cma_helper() printks an error.
>>
>> Oh, _that_ thing. The error in there is correct, but (almost) everyone
>> gets this parameter wrong. This isn't the max number of connectors the
>> fb helper will light up, but just the max number of connectors _per_
>> crtc when driving in hw clone mode. There's two problems with that:
>> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each
>> active connector
>> - I mentioned that everyone gets this wrong?
>>
>> If you're moderately bored it'd be great to nuke the max_connector
>> argument from drm_fb_helper_init, and hard-code it to 1 (with a big
>> comment explaining that this needs to be changed, probably with
>> dynamic reallocation, once someone gets around to implementing hw
>> clone mode).
>>
>> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
>> TODO.rst entry would be great in that case.
>
> If I'm driving a GPU with no display subsystem at all, it seems like I
> shouldn't initialize fbdev for it, right?

That's what we do for radeon/amdgpu on hw without display blocks.

Alex
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.

[Daniel Vetter]

On Mon, Apr 24, 2017 at 10:26:45AM -0400, Alex Deucher wrote:
> On Fri, Apr 21, 2017 at 6:53 PM, Eric Anholt <eric@anholt.net> wrote:
> > Daniel Vetter <daniel@ffwll.ch> writes:
> >
> >> On Wed, Apr 19, 2017 at 7:55 PM, Eric Anholt <eric@anholt.net> wrote:
> >>> Daniel Vetter <daniel@ffwll.ch> writes:
> >>>> On Tue, Apr 18, 2017 at 9:11 PM, Eric Anholt <eric@anholt.net> wrote:
> >>>>> The FBDEV initialization would throw an error in dmesg, when we just
> >>>>> want to silently not initialize fbdev on a V3D-only VC4 instance.
> >>>>>
> >>>>> Signed-off-by: Eric Anholt <eric@anholt.net>
> >>>>
> >>>> Hm, this shouldn't be an error really, you might want to hotplug more
> >>>> connectors later on. What exactly complains?
> >>>
> >>> drm_fb_helper_init() throws an error if the passed in connector count is
> >>> 0, so drm_fb_cma_helper() printks an error.
> >>
> >> Oh, _that_ thing. The error in there is correct, but (almost) everyone
> >> gets this parameter wrong. This isn't the max number of connectors the
> >> fb helper will light up, but just the max number of connectors _per_
> >> crtc when driving in hw clone mode. There's two problems with that:
> >> - fb helpers don't support hw clone mode, we select 1:1 crtcs for each
> >> active connector
> >> - I mentioned that everyone gets this wrong?
> >>
> >> If you're moderately bored it'd be great to nuke the max_connector
> >> argument from drm_fb_helper_init, and hard-code it to 1 (with a big
> >> comment explaining that this needs to be changed, probably with
> >> dynamic reallocation, once someone gets around to implementing hw
> >> clone mode).
> >>
> >> If you're less bored, just hardcode this to 1 in vc4 and done. Plus a
> >> TODO.rst entry would be great in that case.
> >
> > If I'm driving a GPU with no display subsystem at all, it seems like I
> > shouldn't initialize fbdev for it, right?
> 
> That's what we do for radeon/amdgpu on hw without display blocks.

Yeah I got slightly distracted with the error :-)

Still might make sense to put this logic into the helpers, if there's no
crtc then don't bother initializing the fbcon. But imo the better cleanup
is getting rid of the connectors parameter, since most drivers get it
wrong, then respin the vc4 patch to explain what it's really doing (it's
not about the error really, it's about not registering and fbdev that's
not doing anything).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> writes:

> 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.

> +	v3d->clk = devm_clk_get(dev, "v3d_clk");
> +	if (IS_ERR(v3d->clk)) {
> +		int ret = PTR_ERR(v3d->clk);
> +
> +		if (ret == -ENODEV) {

Apparently I hadn't booted this on RPi yet.  This is supposed to be
-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;
> +		}
> +	}

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[PATCH 3/3] drm/vc4: Add specific compatible strings for Cygnus.

[Eric Anholt]

Cygnus has V3D 2.6 instead of 2.1, and doesn't use the VC4 display
modules.  The V3D can be uniquely identified by the IDENT[01]
registers, and there's nothing to key off of for the display change
other than the lack of DT nodes for the display components, but it's
convention to have new compatible strings anyway.

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

diff --git a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
index bc1756f4f791..284e2b14cfbe 100644
--- a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
+++ b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
@@ -5,7 +5,7 @@ with HDMI output and the HVS (Hardware Video Scaler) for compositing
 display planes.
 
 Required properties for VC4:
-- compatible:	Should be "brcm,bcm2835-vc4"
+- compatible:	Should be "brcm,bcm2835-vc4" or "brcm,cygnus-vc4"
 
 Required properties for Pixel Valve:
 - compatible:	Should be one of "brcm,bcm2835-pixelvalve0",
@@ -54,7 +54,7 @@ Required properties for VEC:
 		  See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
 
 Required properties for V3D:
-- compatible:	Should be "brcm,bcm2835-v3d"
+- compatible:	Should be "brcm,bcm2835-v3d" or "brcm,cygnus-v3d"
 - reg:		Physical base address and length of the V3D's registers
 - interrupts:	The interrupt number
 		  See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 92fb9a41fe7c..754ce76d4b98 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -335,6 +335,7 @@ static int vc4_platform_drm_remove(struct platform_device *pdev)
 
 static const struct of_device_id vc4_of_match[] = {
 	{ .compatible = "brcm,bcm2835-vc4", },
+	{ .compatible = "brcm,cygnus-vc4", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, vc4_of_match);
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 2442622e6bff..3abcd27aa46f 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -304,6 +304,7 @@ static int vc4_v3d_dev_remove(struct platform_device *pdev)
 
 static const struct of_device_id vc4_v3d_dt_match[] = {
 	{ .compatible = "brcm,bcm2835-v3d" },
+	{ .compatible = "brcm,cygnus-v3d" },
 	{ .compatible = "brcm,vc4-v3d" },
 	{}
 };
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/vc4: Add specific compatible strings for Cygnus.

[Rob Herring]

On Tue, Apr 18, 2017 at 12:11:57PM -0700, Eric Anholt wrote:
> Cygnus has V3D 2.6 instead of 2.1, and doesn't use the VC4 display
> modules.  The V3D can be uniquely identified by the IDENT[01]
> registers, and there's nothing to key off of for the display change
> other than the lack of DT nodes for the display components, but it's
> convention to have new compatible strings anyway.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt | 4 ++--
>  drivers/gpu/drm/vc4/vc4_drv.c                              | 1 +
>  drivers/gpu/drm/vc4/vc4_v3d.c                              | 1 +
>  3 files changed, 4 insertions(+), 2 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Eric Anholt]

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(-)

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.
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Florian Fainelli]

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-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> ---

> +	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.
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
>> ---
>
>> +	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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 703 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Eric Anholt]

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.
v3: Drop NULL checks around clk calls which embed NULL checks.

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                      | 31 +++++++++++++++++++++-
 3 files changed, 35 insertions(+), 1 deletion(-)

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..465405586591 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,8 @@ static int vc4_v3d_runtime_suspend(struct device *dev)
 	drm_gem_object_put_unlocked(&vc4->bin_bo->base.base);
 	vc4->bin_bo = NULL;
 
+	clk_disable_unprepare(v3d->clk);
+
 	return 0;
 }
 
@@ -318,6 +321,10 @@ static int vc4_v3d_runtime_resume(struct device *dev)
 	if (ret)
 		return ret;
 
+	ret = clk_prepare_enable(v3d->clk);
+	if (ret != 0)
+		return ret;
+
 	vc4_v3d_init_hw(vc4->dev);
 	vc4_irq_postinstall(vc4->dev);
 
@@ -348,15 +355,37 @@ 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;
 	}
 
+	ret = clk_prepare_enable(v3d->clk);
+	if (ret != 0)
+		return ret;
+
 	ret = vc4_allocate_bin_bo(drm);
-	if (ret)
+	if (ret) {
+		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.
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3 v3] drm/vc4: Turn the V3D clock on at runtime.

[Rob Herring]

On Mon, Apr 24, 2017 at 01:12:09PM -0700, 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.
> v3: Drop NULL checks around clk calls which embed NULL checks.
> 
> 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                      | 31 +++++++++++++++++++++-
>  3 files changed, 35 insertions(+), 1 deletion(-)
> 
> 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"

clock-names is pointless for a single clock. 

> +
>  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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3 v3] drm/vc4: Turn the V3D clock on at runtime.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1608 bytes --]

Rob Herring <robh@kernel.org> writes:

> On Mon, Apr 24, 2017 at 01:12:09PM -0700, 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.
>> v3: Drop NULL checks around clk calls which embed NULL checks.
>> 
>> 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                      | 31 +++++++++++++++++++++-
>>  3 files changed, 35 insertions(+), 1 deletion(-)
>> 
>> 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"
>
> clock-names is pointless for a single clock. 

I thought the "-names" was the current style of future-proofing against
finding another clock to put in the list.  If I drop it, is the DT
change acked?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel