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. */
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 --- 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);
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 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.
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 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?
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
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
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" }, {} };
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
Eric Anholt eric@anholt.net 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;}- }
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.
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.
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.
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.
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.
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
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?
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Daniel Vetter -
Eric Anholt -
Florian Fainelli -
Rob Herring