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

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


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