[PATCH RESEND v4 v5 0/4] drm/vc4: Use the firmware to stop the display pipeline

STALE1664d

Revision v4 of 6 in this series.

11 messages, 3 authors, 2022-01-11 · open the first message on its own page

[PATCH RESEND v4 v5 0/4] drm/vc4: Use the firmware to stop the display pipeline

From: Maxime Ripard <hidden>
Date: 2021-12-15 09:51:22

Hi,

The VC4 driver has had limited support to disable the HDMI controllers and
pixelvalves at boot if the firmware has enabled them.

However, this proved to be limited, and a bit unreliable so a new firmware
command has been introduced some time ago to make it free all its resources and
disable any display output it might have enabled.

This series takes advantage of that command to call it once the transition from
simplefb to the KMS driver has been done.

Let me know what you think,
Maxime

---

Changes from v4:
  - Don't register v3d with nomodeset

Changes from v3:
  - Support nomodeset

Changes from v2:
  - Switch back to rpi_firmware_get / rpi_firmware_put
  - Moved the rpi_firmware pointer to a local variable

Changes from v1:
  - Use of_find_compatible_node instead of a phandle
  - Use devm_rpi_firmware_get

Maxime Ripard (4):
  firmware: raspberrypi: Add RPI_FIRMWARE_NOTIFY_DISPLAY_DONE
  drm/vc4: Support nomodeset
  drm/vc4: Remove conflicting framebuffers before callind bind_all
  drm/vc4: Notify the firmware when DRM is in charge

 drivers/gpu/drm/vc4/vc4_drv.c              | 33 +++++++++++++++++++---
 include/soc/bcm2835/raspberrypi-firmware.h |  1 +
 2 files changed, 30 insertions(+), 4 deletions(-)

-- 
2.33.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND v4 v5 1/4] firmware: raspberrypi: Add RPI_FIRMWARE_NOTIFY_DISPLAY_DONE

From: Maxime Ripard <hidden>
Date: 2021-12-15 09:51:26

The RPI_FIRMWARE_NOTIFY_DISPLAY_DONE firmware call allows to tell the
firmware the kernel is in charge of the display now and the firmware can
free whatever resources it was using.

Acked-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
Signed-off-by: Maxime Ripard <redacted>
---
 include/soc/bcm2835/raspberrypi-firmware.h | 1 +
 1 file changed, 1 insertion(+)
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index 73ad784fca96..811ea668c4a1 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -91,6 +91,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_GET_POE_HAT_VAL =                        0x00030049,
 	RPI_FIRMWARE_SET_POE_HAT_VAL =                        0x00030050,
 	RPI_FIRMWARE_NOTIFY_XHCI_RESET =                      0x00030058,
+	RPI_FIRMWARE_NOTIFY_DISPLAY_DONE =                    0x00030066,
 
 	/* Dispmanx TAGS */
 	RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE =                   0x00040001,
-- 
2.33.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND v4 v5 2/4] drm/vc4: Support nomodeset

From: Maxime Ripard <hidden>
Date: 2021-12-15 09:51:29

If we have nomodeset on the kernel command line we should have the
firmware framebuffer driver kept as is and not try to load the
full-blown KMS driver.

In this case, let's just register the v3d driver.

Signed-off-by: Maxime Ripard <redacted>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 3 +++
 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 16abc3a3d601..d3cae84a4c4e 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -357,6 +357,9 @@ static int __init vc4_drm_register(void)
 {
 	int ret;
 
+	if (drm_firmware_drivers_only())
+		return -ENODEV;
+
 	ret = platform_register_drivers(component_drivers,
 					ARRAY_SIZE(component_drivers));
 	if (ret)
-- 
2.33.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RESEND v4 v5 2/4] drm/vc4: Support nomodeset

From: Thomas Zimmermann <tzimmermann@suse.de>
Date: 2021-12-15 10:08:13

Hi

Am 15.12.21 um 10:51 schrieb Maxime Ripard:
quoted hunk
If we have nomodeset on the kernel command line we should have the
firmware framebuffer driver kept as is and not try to load the
full-blown KMS driver.

In this case, let's just register the v3d driver.

Signed-off-by: Maxime Ripard <redacted>
---
  drivers/gpu/drm/vc4/vc4_drv.c | 3 +++
  1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 16abc3a3d601..d3cae84a4c4e 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -357,6 +357,9 @@ static int __init vc4_drm_register(void)
  {
  	int ret;
  
+	if (drm_firmware_drivers_only())
+		return -ENODEV;
+
Just now that you've updated it, Lucas Stach had a similar concern about 
etnaviv, which doesn't do modesetting either. So we probably want to 
discuss what to do about drivers that have no modesetting separately.

If you want to land the patch, for either version of the code, with or 
without vc3:

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regard
Thomas
  	ret = platform_register_drivers(component_drivers,
  					ARRAY_SIZE(component_drivers));
  	if (ret)
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[PATCH RESEND v4 v5 3/4] drm/vc4: Remove conflicting framebuffers before callind bind_all

From: Maxime Ripard <hidden>
Date: 2021-12-15 09:51:30

The bind hooks will modify their controller registers, so simplefb is
going to be unusable anyway. Let's avoid any transient state where it
could still be in the system but no longer functionnal.

Acked-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
Signed-off-by: Maxime Ripard <redacted>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index d3cae84a4c4e..86c61ee120b7 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -251,6 +251,10 @@ static int vc4_drm_bind(struct device *dev)
 	if (ret)
 		return ret;
 
+	ret = drm_aperture_remove_framebuffers(false, &vc4_drm_driver);
+	if (ret)
+		return ret;
+
 	ret = component_bind_all(dev, drm);
 	if (ret)
 		return ret;
@@ -259,10 +263,6 @@ static int vc4_drm_bind(struct device *dev)
 	if (ret)
 		goto unbind_all;
 
-	ret = drm_aperture_remove_framebuffers(false, &vc4_drm_driver);
-	if (ret)
-		goto unbind_all;
-
 	ret = vc4_kms_load(drm);
 	if (ret < 0)
 		goto unbind_all;
-- 
2.33.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND v4 v5 4/4] drm/vc4: Notify the firmware when DRM is in charge

From: Maxime Ripard <hidden>
Date: 2021-12-15 09:51:31

Once the call to drm_fb_helper_remove_conflicting_framebuffers() has
been made, simplefb has been unregistered and the KMS driver is entirely
in charge of the display.

Thus, we can notify the firmware it can free whatever resource it was
using to maintain simplefb functional.

Signed-off-by: Maxime Ripard <redacted>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 86c61ee120b7..a03053c8e22c 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -37,6 +37,8 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_vblank.h>
 
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
 #include "uapi/drm/vc4_drm.h"
 
 #include "vc4_drv.h"
@@ -215,6 +217,7 @@ static void vc4_match_add_drivers(struct device *dev,
 static int vc4_drm_bind(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
+	struct rpi_firmware *firmware = NULL;
 	struct drm_device *drm;
 	struct vc4_dev *vc4;
 	struct device_node *node;
@@ -251,10 +254,29 @@ static int vc4_drm_bind(struct device *dev)
 	if (ret)
 		return ret;
 
+	node = of_find_compatible_node(NULL, NULL, "raspberrypi,bcm2835-firmware");
+	if (node) {
+		firmware = rpi_firmware_get(node);
+		of_node_put(node);
+
+		if (!firmware)
+			return -EPROBE_DEFER;
+	}
+
 	ret = drm_aperture_remove_framebuffers(false, &vc4_drm_driver);
 	if (ret)
 		return ret;
 
+	if (firmware) {
+		ret = rpi_firmware_property(firmware,
+					    RPI_FIRMWARE_NOTIFY_DISPLAY_DONE,
+					    NULL, 0);
+		if (ret)
+			drm_warn(drm, "Couldn't stop firmware display driver: %d\n", ret);
+
+		rpi_firmware_put(firmware);
+	}
+
 	ret = component_bind_all(dev, drm);
 	if (ret)
 		return ret;
-- 
2.33.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RESEND v4 v5 4/4] drm/vc4: Notify the firmware when DRM is in charge

From: Javier Martinez Canillas <javier@dowhile0.org>
Date: 2022-01-11 09:26:55

Hello Maxime,

On Wed, Dec 15, 2021 at 10:51 AM Maxime Ripard [off-list ref] wrote:
Once the call to drm_fb_helper_remove_conflicting_framebuffers() has
been made, simplefb has been unregistered and the KMS driver is entirely
in charge of the display.

Thus, we can notify the firmware it can free whatever resource it was
using to maintain simplefb functional.

Signed-off-by: Maxime Ripard <redacted>
---
Patch looks good to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Best regards,
Javier

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RESEND v4 v5 4/4] drm/vc4: Notify the firmware when DRM is in charge

From: Thomas Zimmermann <tzimmermann@suse.de>
Date: 2022-01-11 09:38:41

Hi

Am 15.12.21 um 10:51 schrieb Maxime Ripard:
quoted hunk
Once the call to drm_fb_helper_remove_conflicting_framebuffers() has
been made, simplefb has been unregistered and the KMS driver is entirely
in charge of the display.

Thus, we can notify the firmware it can free whatever resource it was
using to maintain simplefb functional.

Signed-off-by: Maxime Ripard <redacted>
---
  drivers/gpu/drm/vc4/vc4_drv.c | 22 ++++++++++++++++++++++
  1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 86c61ee120b7..a03053c8e22c 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -37,6 +37,8 @@
  #include <drm/drm_fb_helper.h>
  #include <drm/drm_vblank.h>
  
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
  #include "uapi/drm/vc4_drm.h"
  
  #include "vc4_drv.h"
@@ -215,6 +217,7 @@ static void vc4_match_add_drivers(struct device *dev,
  static int vc4_drm_bind(struct device *dev)
  {
  	struct platform_device *pdev = to_platform_device(dev);
+	struct rpi_firmware *firmware = NULL;
  	struct drm_device *drm;
  	struct vc4_dev *vc4;
  	struct device_node *node;
@@ -251,10 +254,29 @@ static int vc4_drm_bind(struct device *dev)
  	if (ret)
  		return ret;
  
+	node = of_find_compatible_node(NULL, NULL, "raspberrypi,bcm2835-firmware");
+	if (node) {
+		firmware = rpi_firmware_get(node);
+		of_node_put(node);
+
+		if (!firmware)
+			return -EPROBE_DEFER;
+	}
+
The code is

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

Just for my understanding:

You retrieve the firmware before killing simpledrm simply to keep the 
display on if it fails, right?

What's the possible error that would justify a retry (via EPROBE_DEFER)?

Best regards
Thomas
  	ret = drm_aperture_remove_framebuffers(false, &vc4_drm_driver);
  	if (ret)
  		return ret;
  
+	if (firmware) {
+		ret = rpi_firmware_property(firmware,
+					    RPI_FIRMWARE_NOTIFY_DISPLAY_DONE,
+					    NULL, 0);
+		if (ret)
+			drm_warn(drm, "Couldn't stop firmware display driver: %d\n", ret);
+
+		rpi_firmware_put(firmware);
+	}
+
  	ret = component_bind_all(dev, drm);
  	if (ret)
  		return ret;
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Re: [PATCH RESEND v4 v5 4/4] drm/vc4: Notify the firmware when DRM is in charge

From: Maxime Ripard <hidden>
Date: 2022-01-11 13:01:53

Hi Thomas,

On Tue, Jan 11, 2022 at 10:38:36AM +0100, Thomas Zimmermann wrote:
Hi

Am 15.12.21 um 10:51 schrieb Maxime Ripard:
quoted
Once the call to drm_fb_helper_remove_conflicting_framebuffers() has
been made, simplefb has been unregistered and the KMS driver is entirely
in charge of the display.

Thus, we can notify the firmware it can free whatever resource it was
using to maintain simplefb functional.

Signed-off-by: Maxime Ripard <redacted>
---
  drivers/gpu/drm/vc4/vc4_drv.c | 22 ++++++++++++++++++++++
  1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 86c61ee120b7..a03053c8e22c 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -37,6 +37,8 @@
  #include <drm/drm_fb_helper.h>
  #include <drm/drm_vblank.h>
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
  #include "uapi/drm/vc4_drm.h"
  #include "vc4_drv.h"
@@ -215,6 +217,7 @@ static void vc4_match_add_drivers(struct device *dev,
  static int vc4_drm_bind(struct device *dev)
  {
  	struct platform_device *pdev = to_platform_device(dev);
+	struct rpi_firmware *firmware = NULL;
  	struct drm_device *drm;
  	struct vc4_dev *vc4;
  	struct device_node *node;
@@ -251,10 +254,29 @@ static int vc4_drm_bind(struct device *dev)
  	if (ret)
  		return ret;
+	node = of_find_compatible_node(NULL, NULL, "raspberrypi,bcm2835-firmware");
+	if (node) {
+		firmware = rpi_firmware_get(node);
+		of_node_put(node);
+
+		if (!firmware)
+			return -EPROBE_DEFER;
+	}
+
The code is

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Thanks for your review
Just for my understanding:

You retrieve the firmware before killing simpledrm simply to keep the
display on if it fails, right?
Exactly
What's the possible error that would justify a retry (via EPROBE_DEFER)?
The firmware there is backed by a driver that might not have probed yet,
in which case we just want to retry later on

Maxime

Re: [PATCH RESEND v4 v5 0/4] drm/vc4: Use the firmware to stop the display pipeline

From: Maxime Ripard <hidden>
Date: 2021-12-15 09:57:15

On Wed, Dec 15, 2021 at 10:51:13AM +0100, Maxime Ripard wrote:
Hi,

The VC4 driver has had limited support to disable the HDMI controllers and
pixelvalves at boot if the firmware has enabled them.

However, this proved to be limited, and a bit unreliable so a new firmware
command has been introduced some time ago to make it free all its resources and
disable any display output it might have enabled.

This series takes advantage of that command to call it once the transition from
simplefb to the KMS driver has been done.

Let me know what you think,
Maxime
And I forgot to drop the RESEND v4 prefix.. This should obviously read PATCH v5

Maxime

Re: [PATCH RESEND v4 v5 0/4] drm/vc4: Use the firmware to stop the display pipeline

From: Maxime Ripard <hidden>
Date: 2022-01-11 12:55:06

On Wed, 15 Dec 2021 10:51:13 +0100, Maxime Ripard wrote:
The VC4 driver has had limited support to disable the HDMI controllers and
pixelvalves at boot if the firmware has enabled them.

However, this proved to be limited, and a bit unreliable so a new firmware
command has been introduced some time ago to make it free all its resources and
disable any display output it might have enabled.

[...]
Applied to drm/drm-misc (drm-misc-next).

Thanks!
Maxime

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help