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

[Javier Martinez Canillas]

Hello Robin,

On 6/15/22 18:09, Robin Murphy wrote:
> 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.
> 
> We'll do this after binding the HDMI encoder, since that's the most
> likely thing to fail, and the EFI console is still better than nothing
> when that happens. However, the two HDLCD controllers on Juno are
> independent, and many users will still be using older firmware without
> any display support, so we'll only bother if we find that the HDLCD
> we're probing is already enabled. And if it is, then we'll also stop it,
> since otherwise the display can end up shifted if it's still scanning
> out while the rest of the registers are subsequently reconfigured.
> 
> Signed-off-by: Robin Murphy <robin.murphy at arm.com>
> ---
> 
> Since I ended up adding (relatively) a lot here, I didn't want to
> second-guess Javier's opinion so left off the R-b tag from v1.
> 

Yes, v2 looks good to me so feel free to keep my R-b:

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

[snip]

>  
> +	/* If EFI left us running, take over from efifb/sysfb */

There are other drivers such as simplefb and simpledrm that also use
a simple platform-provided framebuffer. So instead of mentioning all
drivers maybe you could just have something like the following ?

	/* If EFI left us running, take over from simple framebuffer drivers */

And maybe you can also add some words about why you are checking the
HDLCD_REG_COMMAND register ? Liviu gave more context in this thread,
I believe some of this info could be in the comment.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat