[PATCH master 1/3] efi: gop: fix rectangle extents passed to fb_damage

[PATCH master 1/3] efi: gop: fix rectangle extents passed to fb_damage

[Ahmad Fatoum]

fb_damage takes the coordinates of the upper left and bottom right
pixels as can be seen in fb_rect_width() and fb_rect_height().

We erroneously passed only width and height though, so add the origin
pixel coordinates on top to fix this.

Fixes: d040f49ad0b3 ("efi: loader: protocol: add graphical output protocol support")
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 efi/loader/protocols/gop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/efi/loader/protocols/gop.c b/efi/loader/protocols/gop.c
index 9ada8a2da290..d48edac5a2d6 100644
--- a/efi/loader/protocols/gop.c
+++ b/efi/loader/protocols/gop.c
@@ -248,7 +248,7 @@ static __always_inline efi_status_t gop_blt_int(struct efi_graphics_output_proto
 	}
 
 	if (blt_to_video) {
-		struct fb_rect rect = { dx, dy, width, height };
+		struct fb_rect rect = { dx, dy, dx + width, dy + height };
 		fb_damage(gopobj->fbi, &rect);
 	}
 
-- 
2.47.3

[PATCH master 2/3] efi: gop: use correct BLT pixel type

[Ahmad Fatoum]

EFI_PIXEL_BITMASK and EFI_GRAPHICS_OUTPUT_BLT_PIXEL are distinct types:
Whereas the former is 4 32-bit masks, the latter is int total 32-bit
with 8-bit for each of red, green, blue and reserved.

The EFI_GRAPHICS_OUTPUT_PROTOCOL_BLT callback takes the BltBuffer as
EFI_GRAPHICS_OUTPUT_BLT_PIXEL, but instead we used efi_pixel_bitmask, an
oversight when I ported the U-Boot support while trying to make use of
the existent barebox EFI app types. The EFI GOP driver used in barebox
as EFI payload doesn't use the Blt callback at all.

Fixes: d040f49ad0b3 ("efi: loader: protocol: add graphical output protocol support")
Reported-by: Ben Pye # Matrix
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 efi/loader/protocols/gop.c | 64 +++++++++++++++++++-------------------
 include/efi/protocol/gop.h |  9 +++++-
 2 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/efi/loader/protocols/gop.c b/efi/loader/protocols/gop.c
index d48edac5a2d6..03995d9eee29 100644
--- a/efi/loader/protocols/gop.c
+++ b/efi/loader/protocols/gop.c
@@ -66,50 +66,50 @@ static efi_status_t EFIAPI gop_query_mode(struct efi_graphics_output_protocol *t
 	return EFI_EXIT(ret);
 }
 
-static __always_inline struct efi_pixel_bitmask efi_vid30_to_blt_col(u32 vid)
+static __always_inline struct efi_gop_pixel efi_vid30_to_blt_col(u32 vid)
 {
-	struct efi_pixel_bitmask blt = {
-		.reserved_mask = 0,
+	struct efi_gop_pixel blt = {
+		.reserved = 0,
 	};
 
-	blt.blue_mask  = (vid & 0x3ff) >> 2;
+	blt.blue  = (vid & 0x3ff) >> 2;
 	vid >>= 10;
-	blt.green_mask = (vid & 0x3ff) >> 2;
+	blt.green = (vid & 0x3ff) >> 2;
 	vid >>= 10;
-	blt.red_mask   = (vid & 0x3ff) >> 2;
+	blt.red   = (vid & 0x3ff) >> 2;
 	return blt;
 }
 
-static __always_inline u32 efi_blt_col_to_vid30(struct efi_pixel_bitmask *blt)
+static __always_inline u32 efi_blt_col_to_vid30(struct efi_gop_pixel *blt)
 {
-	return (u32)(blt->red_mask   << 2) << 20 |
-	       (u32)(blt->green_mask << 2) << 10 |
-	       (u32)(blt->blue_mask  << 2);
+	return (u32)(blt->red   << 2) << 20 |
+	       (u32)(blt->green << 2) << 10 |
+	       (u32)(blt->blue  << 2);
 }
 
-static __always_inline struct efi_pixel_bitmask efi_vid16_to_blt_col(u16 vid)
+static __always_inline struct efi_gop_pixel efi_vid16_to_blt_col(u16 vid)
 {
-	struct efi_pixel_bitmask blt = {
-		.reserved_mask = 0,
+	struct efi_gop_pixel blt = {
+		.reserved = 0,
 	};
 
-	blt.blue_mask  = (vid & 0x1f) << 3;
+	blt.blue  = (vid & 0x1f) << 3;
 	vid >>= 5;
-	blt.green_mask = (vid & 0x3f) << 2;
+	blt.green = (vid & 0x3f) << 2;
 	vid >>= 6;
-	blt.red_mask   = (vid & 0x1f) << 3;
+	blt.red   = (vid & 0x1f) << 3;
 	return blt;
 }
 
-static __always_inline u16 efi_blt_col_to_vid16(struct efi_pixel_bitmask *blt)
+static __always_inline u16 efi_blt_col_to_vid16(struct efi_gop_pixel *blt)
 {
-	return (u16)(blt->red_mask   >> 3) << 11 |
-	       (u16)(blt->green_mask >> 2) <<  5 |
-	       (u16)(blt->blue_mask  >> 3);
+	return (u16)(blt->red   >> 3) << 11 |
+	       (u16)(blt->green >> 2) <<  5 |
+	       (u16)(blt->blue  >> 3);
 }
 
 static __always_inline efi_status_t gop_blt_int(struct efi_graphics_output_protocol *this,
-						struct efi_pixel_bitmask *bufferp,
+						struct efi_gop_pixel *bufferp,
 						u32 operation, efi_uintn_t sx,
 						efi_uintn_t sy, efi_uintn_t dx,
 						efi_uintn_t dy,
@@ -122,7 +122,7 @@ static __always_inline efi_status_t gop_blt_int(struct efi_graphics_output_proto
 	efi_uintn_t i, j, linelen, slineoff = 0, dlineoff, swidth, dwidth;
 	u32 *fb32 = gopobj->fb;
 	u16 *fb16 = gopobj->fb;
-	struct efi_pixel_bitmask *buffer = __builtin_assume_aligned(bufferp, 4);
+	struct efi_gop_pixel *buffer = __builtin_assume_aligned(bufferp, 4);
 	bool blt_to_video = (operation != EFI_BLT_VIDEO_TO_BLT_BUFFER);
 
 	if (delta) {
@@ -200,7 +200,7 @@ static __always_inline efi_status_t gop_blt_int(struct efi_graphics_output_proto
 	dlineoff = dwidth * dy;
 	for (i = 0; i < height; i++) {
 		for (j = 0; j < width; j++) {
-			struct efi_pixel_bitmask pix;
+			struct efi_gop_pixel pix;
 
 			/* Read source pixel */
 			switch (operation) {
@@ -213,7 +213,7 @@ static __always_inline efi_status_t gop_blt_int(struct efi_graphics_output_proto
 			case EFI_BLT_VIDEO_TO_BLT_BUFFER:
 			case EFI_BLT_VIDEO_TO_VIDEO:
 				if (vid_bpp == 32)
-					pix = *(struct efi_pixel_bitmask *)&fb32[
+					pix = *(struct efi_gop_pixel *)&fb32[
 						slineoff + j + sx];
 				else if (vid_bpp == 30)
 					pix = efi_vid30_to_blt_col(fb32[
@@ -285,7 +285,7 @@ static efi_uintn_t gop_get_bpp(struct efi_graphics_output_protocol *this)
  * optimize for speed.
  */
 static efi_status_t gop_blt_video_fill(struct efi_graphics_output_protocol *this,
-				       struct efi_pixel_bitmask *buffer,
+				       struct efi_gop_pixel *buffer,
 				       u32 foo, efi_uintn_t sx,
 				       efi_uintn_t sy, efi_uintn_t dx,
 				       efi_uintn_t dy, efi_uintn_t width,
@@ -297,7 +297,7 @@ static efi_status_t gop_blt_video_fill(struct efi_graphics_output_protocol *this
 }
 
 static efi_status_t gop_blt_buf_to_vid16(struct efi_graphics_output_protocol *this,
-					 struct efi_pixel_bitmask *buffer,
+					 struct efi_gop_pixel *buffer,
 					 u32 foo, efi_uintn_t sx,
 					 efi_uintn_t sy, efi_uintn_t dx,
 					 efi_uintn_t dy, efi_uintn_t width,
@@ -308,7 +308,7 @@ static efi_status_t gop_blt_buf_to_vid16(struct efi_graphics_output_protocol *th
 }
 
 static efi_status_t gop_blt_buf_to_vid30(struct efi_graphics_output_protocol *this,
-					 struct efi_pixel_bitmask *buffer,
+					 struct efi_gop_pixel *buffer,
 					 u32 foo, efi_uintn_t sx,
 					 efi_uintn_t sy, efi_uintn_t dx,
 					 efi_uintn_t dy, efi_uintn_t width,
@@ -319,7 +319,7 @@ static efi_status_t gop_blt_buf_to_vid30(struct efi_graphics_output_protocol *th
 }
 
 static efi_status_t gop_blt_buf_to_vid32(struct efi_graphics_output_protocol *this,
-					 struct efi_pixel_bitmask *buffer,
+					 struct efi_gop_pixel *buffer,
 					 u32 foo, efi_uintn_t sx,
 					 efi_uintn_t sy, efi_uintn_t dx,
 					 efi_uintn_t dy, efi_uintn_t width,
@@ -330,7 +330,7 @@ static efi_status_t gop_blt_buf_to_vid32(struct efi_graphics_output_protocol *th
 }
 
 static efi_status_t gop_blt_vid_to_vid(struct efi_graphics_output_protocol *this,
-				       struct efi_pixel_bitmask *buffer,
+				       struct efi_gop_pixel *buffer,
 				       u32 foo, efi_uintn_t sx,
 				       efi_uintn_t sy, efi_uintn_t dx,
 				       efi_uintn_t dy, efi_uintn_t width,
@@ -342,7 +342,7 @@ static efi_status_t gop_blt_vid_to_vid(struct efi_graphics_output_protocol *this
 }
 
 static efi_status_t gop_blt_vid_to_buf(struct efi_graphics_output_protocol *this,
-				       struct efi_pixel_bitmask *buffer,
+				       struct efi_gop_pixel *buffer,
 				       u32 foo, efi_uintn_t sx,
 				       efi_uintn_t sy, efi_uintn_t dx,
 				       efi_uintn_t dy, efi_uintn_t width,
@@ -369,7 +369,7 @@ static efi_status_t EFIAPI gop_set_mode(struct efi_graphics_output_protocol *thi
 					u32 mode_number)
 {
 	struct efi_gop_obj *gopobj;
-	struct efi_pixel_bitmask buffer = {0, 0, 0, 0};
+	struct efi_gop_pixel buffer = {0, 0, 0, 0};
 	efi_uintn_t vid_bpp;
 	efi_status_t ret = EFI_SUCCESS;
 
@@ -412,7 +412,7 @@ static efi_status_t EFIAPI gop_set_mode(struct efi_graphics_output_protocol *thi
  * Return:	status code
  */
 static efi_status_t EFIAPI gop_blt(struct efi_graphics_output_protocol *this,
-				   struct efi_pixel_bitmask *buffer,
+				   struct efi_gop_pixel *buffer,
 				   u32 operation, efi_uintn_t sx,
 				   efi_uintn_t sy, efi_uintn_t dx,
 				   efi_uintn_t dy, efi_uintn_t width,
diff --git a/include/efi/protocol/gop.h b/include/efi/protocol/gop.h
index 12a5ea6d0fe0..3410d8e3ce67 100644
--- a/include/efi/protocol/gop.h
+++ b/include/efi/protocol/gop.h
@@ -21,6 +21,13 @@ struct efi_pixel_bitmask {
 	u32 reserved_mask;
 };
 
+struct efi_gop_pixel {
+	u8 blue;
+	u8 green;
+	u8 red;
+	u8 reserved;
+};
+
 #define EFI_BLT_VIDEO_FILL		0
 #define EFI_BLT_VIDEO_TO_BLT_BUFFER	1
 #define EFI_BLT_BUFFER_TO_VIDEO		2
@@ -51,7 +58,7 @@ struct efi_graphics_output_protocol {
 	efi_status_t (EFIAPI *set_mode) (struct efi_graphics_output_protocol *This,
 			uint32_t mode_number);
 	efi_status_t (EFIAPI *blt)(struct efi_graphics_output_protocol *This,
-			struct efi_pixel_bitmask *buffer,
+			struct efi_gop_pixel *buffer,
 			u32 operation,
 			size_t sourcex, size_t sourcey,
 			size_t destinationx, size_t destinationy,
-- 
2.47.3

[PATCH master 3/3] efi: gop: report and use framebuffer stride

[Ahmad Fatoum]

Both barebox and EFI GOP differentiate between the width of a line on
screen and in memory.

The EFI GOP driver we had imported from U-Boot loses that distinction
and reports PixelsPerScanLine to be identical to the visible horizontal
resolution. Pass the framebuffer line length instead to fix potential
issues when they differ.

Assisted-by: Codex:gpt-5.5
Fixes: d040f49ad0b3 ("efi: loader: protocol: add graphical output protocol support")
Signed-off-by: Ahmad Fatoum <a.fatoum@barebox.org>
---
 efi/loader/protocols/gop.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/efi/loader/protocols/gop.c b/efi/loader/protocols/gop.c
index 03995d9eee29..fbbcace83eef 100644
--- a/efi/loader/protocols/gop.c
+++ b/efi/loader/protocols/gop.c
@@ -174,7 +174,7 @@ static __always_inline efi_status_t gop_blt_int(struct efi_graphics_output_proto
 		break;
 	case EFI_BLT_VIDEO_TO_BLT_BUFFER:
 	case EFI_BLT_VIDEO_TO_VIDEO:
-		swidth = gopobj->info.horizontal_resolution;
+		swidth = gopobj->info.pixels_per_scan_line;
 		if (!vid_bpp)
 			return EFI_UNSUPPORTED;
 		break;
@@ -187,7 +187,7 @@ static __always_inline efi_status_t gop_blt_int(struct efi_graphics_output_proto
 	case EFI_BLT_BUFFER_TO_VIDEO:
 	case EFI_BLT_VIDEO_FILL:
 	case EFI_BLT_VIDEO_TO_VIDEO:
-		dwidth = gopobj->info.horizontal_resolution;
+		dwidth = gopobj->info.pixels_per_scan_line;
 		if (!vid_bpp)
 			return EFI_UNSUPPORTED;
 		break;
@@ -489,6 +489,7 @@ static efi_status_t efi_gop_register(void *data)
 	struct fb_info *fbi;
 	struct screen *sc;
 	struct efi_pixel_bitmask *pixel_information;
+	u32 bytes_per_pixel, pixels_per_scan_line;
 
 	sc = fb_open(fbdev);
 	if (IS_ERR(sc)) {
@@ -510,6 +511,14 @@ static efi_status_t efi_gop_register(void *data)
 		return EFI_UNSUPPORTED;
 	}
 
+	bytes_per_pixel = fbi->bits_per_pixel / BITS_PER_BYTE;
+	if (fbi->line_length % bytes_per_pixel)
+		return EFI_UNSUPPORTED;
+
+	pixels_per_scan_line = fbi->line_length / bytes_per_pixel;
+	if (pixels_per_scan_line < col)
+		return EFI_UNSUPPORTED;
+
 	gopobj = calloc(1, sizeof(*gopobj));
 	if (!gopobj) {
 		pr_err("Out of memory\n");
@@ -566,7 +575,7 @@ static efi_status_t efi_gop_register(void *data)
 		pixel_information->green_mask = 0x07e0;
 		pixel_information->blue_mask = 0x001f;
 	}
-	gopobj->info.pixels_per_scan_line = col;
+	gopobj->info.pixels_per_scan_line = pixels_per_scan_line;
 	gopobj->fb = fbi->screen_base;
 	gopobj->fbi = fbi;
 
-- 
2.47.3