| Message ID | f921492d-6c53-ce68-16b6-dc9c21f2b738@c64.rulez.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Deduplicate cfb/sys drawing fbops | expand |
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@lists.freedesktop.org mailing list - it seems you sent your patches manually. Using b4 on your Message-ID gives: b4 am f921492d-6c53-ce68-16b6-dc9c21f2b738@c64.rulez.org Grabbing thread from lore.kernel.org/all/f921492d-6c53-ce68-16b6-dc9c21f2b738@c64.rulez.org/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
> Some notes regarding your patches: > - please add dri-devel@lists.freedesktop.org mailing list Will do next time. > - it seems you sent your patches manually. Yes, I thought it will work after a bit of practice without mistakes. Sorry. > - patch #2 is missing, and patches 1-5 have the same subject. On that one I missed to delete the "Re:" prefix in the subject, but it was there. And after the 5th I noticed that possibly the subject needs to be changed as well. Also replies should have been on the first one not always on the previous one. > - When applying there are warnings: Will retab and white space clean it next time. However I noticed that even if I set up the e-mail client as advised it still removed single spaces in diffs for unchanged empty lines. > 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. That complicates things but it seems it's almost impossible to do it correctly manually. Will look into it.