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

Maxime Ripard
2022-01-11T13:01:53Z

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

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

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