Messages in this thread | | | Date | Thu, 11 May 2023 15:05:03 +0200 | Subject | Re: [PATCH v6 1/6] fbdev/matrox: Remove trailing whitespaces | From | Helge Deller <> |
| |
On 5/11/23 09:55, Thomas Zimmermann wrote: > Hi > > Am 10.05.23 um 20:20 schrieb Sui Jingfeng: >> Hi, Thomas >> >> >> I love your patch, yet something to improve: >> >> >> On 2023/5/10 19:05, Thomas Zimmermann wrote: >>> Fix coding style. No functional changes. >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>> Reviewed-by: Arnd Bergmann <arnd@arndb.de> >>> Reviewed-by: Sam Ravnborg <sam@ravnborg.org> >>> Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn> >>> Tested-by: Sui Jingfeng <suijingfeng@loongson.cn> >>> --- >>> drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +++--- >>> drivers/video/fbdev/matrox/matroxfb_base.h | 4 ++-- >>> 2 files changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c b/drivers/video/fbdev/matrox/matroxfb_accel.c >>> index 9cb0685feddd..ce51227798a1 100644 >>> --- a/drivers/video/fbdev/matrox/matroxfb_accel.c >>> +++ b/drivers/video/fbdev/matrox/matroxfb_accel.c >>> @@ -88,7 +88,7 @@ >>> static inline void matrox_cfb4_pal(u_int32_t* pal) { >>> unsigned int i; >>> - >>> + >>> for (i = 0; i < 16; i++) { >>> pal[i] = i * 0x11111111U; >>> } >>> @@ -96,7 +96,7 @@ static inline void matrox_cfb4_pal(u_int32_t* pal) { >>> static inline void matrox_cfb8_pal(u_int32_t* pal) { >>> unsigned int i; >>> - >>> + >>> for (i = 0; i < 16; i++) { >>> pal[i] = i * 0x01010101U; >>> } >>> @@ -482,7 +482,7 @@ static void matroxfb_1bpp_imageblit(struct matrox_fb_info *minfo, u_int32_t fgx, >>> /* Tell... well, why bother... */ >>> while (height--) { >>> size_t i; >>> - >>> + >>> for (i = 0; i < step; i += 4) { >>> /* Hope that there are at least three readable bytes beyond the end of bitmap */ >>> fb_writel(get_unaligned((u_int32_t*)(chardata + i)),mmio.vaddr); >>> diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h b/drivers/video/fbdev/matrox/matroxfb_base.h >>> index 958be6805f87..c93c69bbcd57 100644 >>> --- a/drivers/video/fbdev/matrox/matroxfb_base.h >>> +++ b/drivers/video/fbdev/matrox/matroxfb_base.h >>> @@ -301,9 +301,9 @@ struct matrox_altout { >>> int (*verifymode)(void* altout_dev, u_int32_t mode); >>> int (*getqueryctrl)(void* altout_dev, >>> struct v4l2_queryctrl* ctrl); >> >> Noticed that there are plenty of coding style problems in matroxfb_base.h, >> >> why you only fix a few of them? Take this two line as an example, shouldn't >> >> they be fixed also as following? > > I configured my text editor to remove trailing whitespaces > automatically. That keeps my own patches free of them. But the > editor removes all trailing whitespaces, including those that have > been there before. If I encounter such a case, I split out the > whitespace fix and submit it separately. > > But the work I do within fbdev is mostly for improving DRM.
Sure.
> For the > other issues in this file, I don't think that matroxfb should even be > around any longer. Fbdev has been deprecated for a long time. But a > small number of drivers are still in use and we still need its > framebuffer console. So someone should either put significant effort > into maintaining fbdev, or it should be phased out. But neither is > happening.
You're wrong.
You don't mention that for most older machines DRM isn't an acceptable way to go due to it's limitations, e.g. it's low-speed due to missing 2D-acceleration for older cards and and it's incapability to change screen resolution at runtime (just to name two of the bigger limitations here). So, unless we somehow find a good way to move such drivers over to DRM (with a set of minimal 2D acceleration), they are still important.
Actually, I just did test matroxfb and pm2fb successfully a few days back, and they worked. For some smaller issues I've prepared patches, which are on hold due conflicts with your latest file-move-around- and whitespace-changes which are partly in drm-misc. And I do have some upcoming additional patches for console support.
Helge
| |