linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime.
@ 2017-04-18 19:11 Eric Anholt
  2017-04-18 19:11 ` [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D Eric Anholt
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Eric Anholt @ 2017-04-18 19:11 UTC (permalink / raw)
  To: dri-devel, Rob Herring, Mark Rutland, devicetree
  Cc: linux-kernel, 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
  2017-04-18 19:11 [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime Eric Anholt
@ 2017-04-18 19:11 ` Eric Anholt
  2017-04-19  4:59   ` Daniel Vetter
  2017-04-18 19:11 ` [PATCH 3/3] drm/vc4: Add specific compatible strings for Cygnus Eric Anholt
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Eric Anholt @ 2017-04-18 19:11 UTC (permalink / raw)
  To: dri-devel, Rob Herring, Mark Rutland, devicetree
  Cc: linux-kernel, 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@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);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] drm/vc4: Add specific compatible strings for Cygnus.
  2017-04-18 19:11 [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime Eric Anholt
  2017-04-18 19:11 ` [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D Eric Anholt
@ 2017-04-18 19:11 ` Eric Anholt
  2017-04-20 20:33   ` Rob Herring
  2017-04-18 19:23 ` [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime Eric Anholt
  2017-04-18 23:38 ` [PATCH 1/3 v2] " Eric Anholt
  3 siblings, 1 reply; 18+ messages in thread
From: Eric Anholt @ 2017-04-18 19:11 UTC (permalink / raw)
  To: dri-devel, Rob Herring, Mark Rutland, devicetree
  Cc: linux-kernel, 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime.
  2017-04-18 19:11 [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime Eric Anholt
  2017-04-18 19:11 ` [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D Eric Anholt
  2017-04-18 19:11 ` [PATCH 3/3] drm/vc4: Add specific compatible strings for Cygnus Eric Anholt
@ 2017-04-18 19:23 ` Eric Anholt
  2017-04-18 23:38 ` [PATCH 1/3 v2] " Eric Anholt
  3 siblings, 0 replies; 18+ messages in thread
From: Eric Anholt @ 2017-04-18 19:23 UTC (permalink / raw)
  To: dri-devel, Rob Herring, Mark Rutland, devicetree; +Cc: linux-kernel

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

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;
> +		}
> +	}

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3 v2] drm/vc4: Turn the V3D clock on at runtime.
  2017-04-18 19:11 [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime Eric Anholt
                   ` (2 preceding siblings ...)
  2017-04-18 19:23 ` [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime Eric Anholt
@ 2017-04-18 23:38 ` Eric Anholt
  2017-04-18 23:48   ` Florian Fainelli
  3 siblings, 1 reply; 18+ messages in thread
From: Eric Anholt @ 2017-04-18 23:38 UTC (permalink / raw)
  To: dri-devel, Rob Herring, Mark Rutland, devicetree
  Cc: linux-kernel, 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3 v2] drm/vc4: Turn the V3D clock on at runtime.
  2017-04-18 23:38 ` [PATCH 1/3 v2] " Eric Anholt
@ 2017-04-18 23:48   ` Florian Fainelli
  2017-04-19  0:02     ` Eric Anholt
                       ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Florian Fainelli @ 2017-04-18 23:48 UTC (permalink / raw)
  To: Eric Anholt, dri-devel, Rob Herring, Mark Rutland, devicetree
  Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3 v2] drm/vc4: Turn the V3D clock on at runtime.
  2017-04-18 23:48   ` Florian Fainelli
@ 2017-04-19  0:02     ` Eric Anholt
  2017-04-19  0:02     ` Eric Anholt
  2017-04-24 20:12     ` [PATCH 1/3 v3] " Eric Anholt
  2 siblings, 0 replies; 18+ messages in thread
From: Eric Anholt @ 2017-04-19  0:02 UTC (permalink / raw)
  To: Florian Fainelli, dri-devel, Rob Herring, Mark Rutland, devicetree
  Cc: linux-kernel

[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3 v2] drm/vc4: Turn the V3D clock on at runtime.
  2017-04-18 23:48   ` Florian Fainelli
  2017-04-19  0:02     ` Eric Anholt
@ 2017-04-19  0:02     ` Eric Anholt
  2017-04-24 20:12     ` [PATCH 1/3 v3] " Eric Anholt
  2 siblings, 0 replies; 18+ messages in thread
From: Eric Anholt @ 2017-04-19  0:02 UTC (permalink / raw)
  To: Florian Fainelli, dri-devel, Rob Herring, Mark Rutland, devicetree
  Cc: linux-kernel

[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
  2017-04-18 19:11 ` [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D Eric Anholt
@ 2017-04-19  4:59   ` Daniel Vetter
  2017-04-19 17:55     ` Eric Anholt
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2017-04-19  4:59 UTC (permalink / raw)
  To: Eric Anholt
  Cc: dri-devel, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
  2017-04-19  4:59   ` Daniel Vetter
@ 2017-04-19 17:55     ` Eric Anholt
  2017-04-19 19:36       ` Daniel Vetter
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Anholt @ 2017-04-19 17:55 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: dri-devel, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List

[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
  2017-04-19 17:55     ` Eric Anholt
@ 2017-04-19 19:36       ` Daniel Vetter
  2017-04-21 22:53         ` Eric Anholt
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2017-04-19 19:36 UTC (permalink / raw)
  To: Eric Anholt
  Cc: dri-devel, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/3] drm/vc4: Add specific compatible strings for Cygnus.
  2017-04-18 19:11 ` [PATCH 3/3] drm/vc4: Add specific compatible strings for Cygnus Eric Anholt
@ 2017-04-20 20:33   ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2017-04-20 20:33 UTC (permalink / raw)
  To: Eric Anholt; +Cc: dri-devel, Mark Rutland, devicetree, linux-kernel

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>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
  2017-04-19 19:36       ` Daniel Vetter
@ 2017-04-21 22:53         ` Eric Anholt
  2017-04-24 14:26           ` Alex Deucher
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Anholt @ 2017-04-21 22:53 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: dri-devel, Rob Herring, Mark Rutland, devicetree,
	Linux Kernel Mailing List

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

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?

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
  2017-04-21 22:53         ` Eric Anholt
@ 2017-04-24 14:26           ` Alex Deucher
  2017-05-02  8:16             ` Daniel Vetter
  0 siblings, 1 reply; 18+ messages in thread
From: Alex Deucher @ 2017-04-24 14:26 UTC (permalink / raw)
  To: Eric Anholt
  Cc: Daniel Vetter, Mark Rutland, devicetree, Rob Herring,
	Linux Kernel Mailing List, dri-devel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3 v3] drm/vc4: Turn the V3D clock on at runtime.
  2017-04-18 23:48   ` Florian Fainelli
  2017-04-19  0:02     ` Eric Anholt
  2017-04-19  0:02     ` Eric Anholt
@ 2017-04-24 20:12     ` Eric Anholt
  2017-04-28 18:29       ` Rob Herring
  2 siblings, 1 reply; 18+ messages in thread
From: Eric Anholt @ 2017-04-24 20:12 UTC (permalink / raw)
  To: dri-devel, Rob Herring, Mark Rutland, devicetree
  Cc: linux-kernel, 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3 v3] drm/vc4: Turn the V3D clock on at runtime.
  2017-04-24 20:12     ` [PATCH 1/3 v3] " Eric Anholt
@ 2017-04-28 18:29       ` Rob Herring
  2017-04-28 21:41         ` Eric Anholt
  0 siblings, 1 reply; 18+ messages in thread
From: Rob Herring @ 2017-04-28 18:29 UTC (permalink / raw)
  To: Eric Anholt; +Cc: dri-devel, Mark Rutland, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/3 v3] drm/vc4: Turn the V3D clock on at runtime.
  2017-04-28 18:29       ` Rob Herring
@ 2017-04-28 21:41         ` Eric Anholt
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Anholt @ 2017-04-28 21:41 UTC (permalink / raw)
  To: Rob Herring; +Cc: dri-devel, Mark Rutland, devicetree, linux-kernel

[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D.
  2017-04-24 14:26           ` Alex Deucher
@ 2017-05-02  8:16             ` Daniel Vetter
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2017-05-02  8:16 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Eric Anholt, Daniel Vetter, Mark Rutland, devicetree,
	Rob Herring, Linux Kernel Mailing List, dri-devel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-05-02  8:16 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 19:11 [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime Eric Anholt
2017-04-18 19:11 ` [PATCH 2/3] drm/vc4: Don't try to initialize FBDEV if we're only bound to V3D Eric Anholt
2017-04-19  4:59   ` Daniel Vetter
2017-04-19 17:55     ` Eric Anholt
2017-04-19 19:36       ` Daniel Vetter
2017-04-21 22:53         ` Eric Anholt
2017-04-24 14:26           ` Alex Deucher
2017-05-02  8:16             ` Daniel Vetter
2017-04-18 19:11 ` [PATCH 3/3] drm/vc4: Add specific compatible strings for Cygnus Eric Anholt
2017-04-20 20:33   ` Rob Herring
2017-04-18 19:23 ` [PATCH 1/3] drm/vc4: Turn the V3D clock on at runtime Eric Anholt
2017-04-18 23:38 ` [PATCH 1/3 v2] " Eric Anholt
2017-04-18 23:48   ` Florian Fainelli
2017-04-19  0:02     ` Eric Anholt
2017-04-19  0:02     ` Eric Anholt
2017-04-24 20:12     ` [PATCH 1/3 v3] " Eric Anholt
2017-04-28 18:29       ` Rob Herring
2017-04-28 21:41         ` Eric Anholt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).