CC'ed: driv-devel
On 2/2/25 13:39, Kajtár Zsolt wrote:
A series on de-duplicating the common cfb and sys drawing routines will
follow.
Some background:
It happens that I need to use both cfb and sys drawing routines.
For which driver do you need this?
At low resolution where the aperture is large enough the framebuffer
memory is directly mapped. As usual the cfb routines are used.
In the high resolution scenario defio is used to make banking
transparent, there the sys drawing routines are used.
There are packed pixels, of course in the wrong order. So that needs
CFB_REV_PIXELS_IN_BYTE, fine. Or almost.
While the sys routines are based on cfb for some reason the former lacks
pixel order reversing support. The result is that at low resolution the
console is fine, but it's unreadable at high resolution due to the wrong
pixel ordering.
First I added the pixel reversal support to sys, console looks fine.
Still I might have made mistakes when doing so, that might need
further testing just to be sure. Hacked fillrect to run in userspace,
wasn't easy and now I have to come up with edge cases...
Had another look at the cfb routines and sys is basically a straight
copy. Minus the pixel reversing, FB_READ/WRITE macro inlining by hand,
comment style updates and a few changes here and there for I/O vs.
system memory. The memory access differences could have been easily
covered with a few small macros, strange.
Cfb has a working pixel reversal as far as I know. Now all it needs is a
few changes for the memory access differences and then I have the sys
routines with pixel reversal. And can also be reasonably confident that
it actually does what it needs to in special drawing scenarios.
So the patches below take a copy of the cfb routines as header files,
and add macros for the access, text and other differences. The comment
style changes were taken from sys so that it's less different when
compared. Then the cfb and sys files were cut down to just an include
of the common code in the header plus a few defines for the macros used
in the headers.
I was thinking what to do with copyright/credits now. On the new headers
it's clear as it's basically cfb, but the new cfb and sys suffered
significant changes and not much remained. I kept the original authors
but it might be questionable on sys, it just an includes cfb code now.
I know at the moment there are no users for the pixel reversal function
in sys and could have sent such changes later when truly required.
However there are some maintainability benefits as it removes lots of
duplicate code which might be worth to have meanwhile. The pixel
reversal gets optimized out when not in use so I don't worry about that
much.
Zsolt Kajtar (12):
fbdev: core: Copy cfbcopyarea to fb_copyarea
fbdev: core: Make fb_copyarea generic
fbdev: core: Use generic copyarea for as cfb_copyarea
fbdev: core: Use generic copyarea for as sys_copyarea
fbdev: core: Copy cfbfillrect to fb_fillrect
fbdev: core: Make fb_fillrect generic
fbdev: core: Use generic fillrect for as cfb_fillrect
fbdev: core: Use generic fillrect for as sys_fillrect
fbdev: core: Copy cfbimgblt to fb_imageblit
fbdev: core: Make fb_imageblit generic
fbdev: core: Use generic imageblit for as cfb_imageblit
fbdev: core: Use generic imageblit for as sys_imageblit
drivers/video/fbdev/core/cfbcopyarea.c | 425 +-----------------------
drivers/video/fbdev/core/cfbfillrect.c | 362 +-------------------
drivers/video/fbdev/core/cfbimgblt.c | 356 +-------------------
drivers/video/fbdev/core/fb_copyarea.h | 421 +++++++++++++++++++++++
drivers/video/fbdev/core/fb_fillrect.h | 359 ++++++++++++++++++++
drivers/video/fbdev/core/fb_imageblit.h | 356 ++++++++++++++++++++
drivers/video/fbdev/core/syscopyarea.c | 356 +-------------------
drivers/video/fbdev/core/sysfillrect.c | 314 +----------------
drivers/video/fbdev/core/sysimgblt.c | 324 +-----------------
9 files changed, 1190 insertions(+), 2083 deletions(-)
create mode 100644 drivers/video/fbdev/core/fb_copyarea.h
create mode 100644 drivers/video/fbdev/core/fb_fillrect.h
create mode 100644 drivers/video/fbdev/core/fb_imageblit.h
Some notes regarding your patches:
- please add dri-devel@xxxxxxxxxxxxxxxxxxxxx mailing list
- it seems you sent your patches manually.
Using b4 on your Message-ID gives:
b4 am f921492d-6c53-ce68-16b6-dc9c21f2b738@xxxxxxxxxxxxx
Grabbing thread from lore.kernel.org/all/f921492d-6c53-ce68-16b6-dc9c21f2b738@xxxxxxxxxxxxx/t.mbox.gz
Analyzing 13 messages in the thread
Analyzing 0 code-review messages
Checking attestation on all messages, may take a moment...
---
[PATCH RFC 1/12] Deduplicate cfb/sys drawing fbops
ERROR: missing [2/12]!
[PATCH RFC 3/12] Deduplicate cfb/sys drawing fbops
[PATCH RFC 4/12] Deduplicate cfb/sys drawing fbops
[PATCH RFC 5/12] Deduplicate cfb/sys drawing fbops
[PATCH RFC 6/12] fbdev: core: Make fb_fillrect generic
[PATCH RFC 7/12] fbdev: core: Use generic fillrect for as, cfb_fillrect
[PATCH RFC 8/12] fbdev: core: Use generic fillrect for as, sys_fillrect
[PATCH RFC 9/12] fbdev: core: Copy cfbimgblt to fb_imageblit
[PATCH RFC 10/12] fbdev: core: Make fb_imageblit generic
[PATCH RFC 11/12] fbdev: core: Use generic imageblit for as, cfb_imageblit
[PATCH RFC 12/12] fbdev: core: Use generic imageblit for as, sys_imageblit
Total patches: 11
WARNING: Thread incomplete!
---
- patch #2 is missing, and patches 1-5 have the same subject.
- When applying there are warnings:
git am ./20250202_soci_deduplicate_cfb_sys_drawing_fbops.mbx
Applying: Deduplicate cfb/sys drawing fbops
.git/rebase-apply/patch:451: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Applying: Deduplicate cfb/sys drawing fbops
Applying: Deduplicate cfb/sys drawing fbops
Applying: Deduplicate cfb/sys drawing fbops
Applying: fbdev: core: Make fb_fillrect generic
Applying: fbdev: core: Use generic fillrect for as, cfb_fillrect
Applying: fbdev: core: Use generic fillrect for as, sys_fillrect
Applying: fbdev: core: Copy cfbimgblt to fb_imageblit
.git/rebase-apply/patch:199: space before tab in indent.
if (shift) {
.git/rebase-apply/patch:380: new blank line at EOF.
+
warning: 2 lines add whitespace errors.
Applying: fbdev: core: Make fb_imageblit generic
Applying: fbdev: core: Use generic imageblit for as, cfb_imageblit
Applying: fbdev: core: Use generic imageblit for as, sys_imageblit
I suggest you make yourself familiar with "git-publish" (https://github.com/stefanha/git-publish).
It's a great tool which helps a lot for sending patches.
Helge
