| Message ID | 20211215095117.176435-5-maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: Use the firmware to stop the display pipeline | expand |
Hello Maxime, On Wed, Dec 15, 2021 at 10:51 AM Maxime Ripard <maxime@cerno.tech> 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 <maxime@cerno.tech> > --- Patch looks good to me. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> Best regards, Javier
Hi Am 15.12.21 um 10:51 schrieb Maxime Ripard: > 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 <maxime@cerno.tech> > --- > 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;
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: > > 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 <maxime@cerno.tech> > > --- > > 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
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;
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 <maxime@cerno.tech> --- drivers/gpu/drm/vc4/vc4_drv.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)