On 6/15/22 09:39, Thomas Zimmermann wrote: > Hi > > Am 14.06.22 um 23:06 schrieb Robin Murphy: >> On 2022-06-14 14:48, Thomas Zimmermann wrote: >>> Hi >>> >>> Am 14.06.22 um 15:04 schrieb Robin Murphy: >>>> The Arm Juno board EDK2 port has provided an EFI GOP display via HDLCD0 >>>> for some time now, which works nicely as an early framebuffer. However, >>>> once the HDLCD driver probes and takes over the hardware, it should >>>> take over the logical framebuffer as well, otherwise the now-defunct GOP >>>> device hangs about and virtual console output inevitably disappears into >>>> the wrong place most of the time. >>>> >>>> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx> >>>> --- >>>> drivers/gpu/drm/arm/hdlcd_drv.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c >>>> b/drivers/gpu/drm/arm/hdlcd_drv.c >>>> index af59077a5481..a5d04884658b 100644 >>>> --- a/drivers/gpu/drm/arm/hdlcd_drv.c >>>> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c >>>> @@ -331,6 +331,8 @@ static int hdlcd_drm_bind(struct device *dev) >>>> goto err_vblank; >>>> } >>>> + drm_fb_helper_remove_conflicting_framebuffers(NULL, "hdlcd", >>>> false); >>>> + >>> >>> In addition to what Javier said, it appears to be too late to call >>> this function. If anything her etouches hardware, you might >>> accidentally interfere with the EFI-related driver. Rather call it at >>> the top of ldlcd_drm_bind(). >> >> OK, thanks for the info. I mostly just copied the pattern from the >> simplest-looking other users (sun4i, tegra, vc4) who all seemed to call >> it fairly late, and indeed naively it seemed logical not to do it *too* >> early when there's more chance we might fail to bind and leave the user >> with no framebuffer at all. In particular, waiting until we've bound the >> HDMI encoder seems like a good idea in the case of the Juno board (which >> is the only real HDLCD user), as the I2C bus often gets stuck if the >> System Control Processor is having a bad day. I also don't believe >> there's anything here that would affect efifb more than the fact that >> once the DRM CRTC is alive we simply stop scanning out from the region >> of memory that efifb is managing, but if it's considered good practice >> to do this early then I can certainly make that change too. > We've been struggling with this a bit. If it works reliably, you're > welcome to leave it where it is. > > Historically, most drivers call this function very early. But for error > recovery it would be better to do it as late as possible. Ideally, > drivers would first initialize their DRM software state, then kickout > the generic driver, and finally take over hardware. But that would > require a careful review of each driver. :/ > We got bug reports on Fedora about regressions caused by the fact that some programs made the (wrong) assumption that /dev/dri/card0 would be the "main" display and just hard-coded that path. But removing the conflicting framebuffers after calling devm_drm_dev_alloc() breaks this assumption, since the registered device will be /dev/dri/card1. All this is to say that doing it too late, even if nothing is touching the HW yet, could still have unexpected consequences across your graphics stack. -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
