Thomas Zimmermann tzimmermann@suse.de writes:
Hello Thomas,
Hi Javier,
thanks for the patch.
Thanks for your feedback.
Am 12.09.24 um 18:33 schrieb Javier Martinez Canillas:
Julius Werner jwerner@chromium.org writes:
[...]
drivers/firmware/google/framebuffer-coreboot.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c index daadd71d8ddd..4e50da17cd7e 100644 --- a/drivers/firmware/google/framebuffer-coreboot.c +++ b/drivers/firmware/google/framebuffer-coreboot.c @@ -15,6 +15,7 @@ #include <linux/module.h> #include <linux/platform_data/simplefb.h> #include <linux/platform_device.h> +#include <linux/screen_info.h> #include "coreboot_table.h" @@ -27,6 +28,7 @@ static int framebuffer_probe(struct coreboot_device *dev) int i; u32 length; struct lb_framebuffer *fb = &dev->framebuffer;
- struct screen_info *si = &screen_info;
Probably 'const'.
Ok.
struct platform_device *pdev; struct resource res; struct simplefb_platform_data pdata = { @@ -36,6 +38,20 @@ static int framebuffer_probe(struct coreboot_device *dev) .format = NULL, };
- /*
* If the global screen_info data has been filled, the Generic* System Framebuffers (sysfb) will already register a platform* and pass the screen_info as platform_data to a driver that* could scan-out using the system provided framebuffer.** On Coreboot systems, the advertise LB_TAG_FRAMEBUFFER entry* in the Coreboot table should only be used if the payload did* not set video mode info and passed it to the Linux kernel.*/- if (si->orig_video_isVGA == VIDEO_TYPE_VLFB ||
si->orig_video_isVGA == VIDEO_TYPE_EFI)Rather call screen_info_video_type(si) [1] to get the type. If it
Indeed. I missed that helper, I'll change it.
returns 0, the screen_info is unset and the corebios code can handle the framebuffer. In any other case, the framebuffer went through a bootloader, which might have modified it. This also handles awkward cases, such as if the bootloader programs a VGA text mode.
[1] https://elixir.bootlin.com/linux/v6.10.10/source/include/linux/screen_info.h...
With these changes:
Reviewed-by: Thomas Zimmermann tzimmermann@suse.de
Thanks. I'll wait for others in this thread to comment and if all agree with the solution, I'll post a proper patch (addressing your comments).
Best regards Thomas