Re: [PATCH] drm/arm/hdlcd: Take over EFI framebuffer properly

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 3000 bytes --]

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@arm.com>
>>> ---
>>>   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. :/

Best regards
Thomas

> 
> Cheers,
> Robin.

-- 
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

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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