| Message ID | 20230710130113.14563-1-tzimmermann@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | fbdev: Remove FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT flags | expand |
Hi Thomas, On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote: > Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from > fbdev and drivers, as briefly discussed at [1]. Both flags were maybe > useful when fbdev had special handling for driver modules. With > commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0 > and have no further effect. > > Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5 > split this by the way the fb_info struct is being allocated. All flags > are cleared to zero during the allocation. > > Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes > an actual bug in how arch/sh uses the tokne for struct fb_videomode, > which is unrelated. > > Patch 17 removes both flag constants from <linux/fb.h> We have a few more flags that are unused - should they be nuked too? FBINFO_HWACCEL_FILLRECT FBINFO_HWACCEL_ROTATE FBINFO_HWACCEL_XPAN Unused as in no references from fbdev/core/* I would rather see one series nuke all unused FBINFO flags in one go. Assuming my quick grep are right and the above can be dropped. Sam
Hi Sam Am 10.07.23 um 19:19 schrieb Sam Ravnborg: > Hi Thomas, > > On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote: >> Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from >> fbdev and drivers, as briefly discussed at [1]. Both flags were maybe >> useful when fbdev had special handling for driver modules. With >> commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0 >> and have no further effect. >> >> Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5 >> split this by the way the fb_info struct is being allocated. All flags >> are cleared to zero during the allocation. >> >> Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes >> an actual bug in how arch/sh uses the tokne for struct fb_videomode, >> which is unrelated. >> >> Patch 17 removes both flag constants from <linux/fb.h> > > We have a few more flags that are unused - should they be nuked too? > FBINFO_HWACCEL_FILLRECT > FBINFO_HWACCEL_ROTATE > FBINFO_HWACCEL_XPAN It seems those are there for completeness. Nothing sets _ROTATE, the others are simply never checked. According to the comments, some are required, some are optional. I don't know what that means. IIRC there were complains about performance when Daniel tried to remove fbcon acceleration, so not all _HWACCEL_ flags are unneeded. Leaving them in for reference/completeness might be an option; or not. I have no strong feelings about those flags. > > Unused as in no references from fbdev/core/* > > I would rather see one series nuke all unused FBINFO flags in one go. > Assuming my quick grep are right and the above can be dropped. I would not want to extend this series. I'm removing _DEFAULT as it's absolutely pointless and confusing. Best regards Thomas > > Sam
Hi Thomas, On Tue, Jul 11, 2023 at 08:24:40AM +0200, Thomas Zimmermann wrote: > Hi Sam > > Am 10.07.23 um 19:19 schrieb Sam Ravnborg: > > Hi Thomas, > > > > On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote: > > > Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from > > > fbdev and drivers, as briefly discussed at [1]. Both flags were maybe > > > useful when fbdev had special handling for driver modules. With > > > commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0 > > > and have no further effect. > > > > > > Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5 > > > split this by the way the fb_info struct is being allocated. All flags > > > are cleared to zero during the allocation. > > > > > > Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes > > > an actual bug in how arch/sh uses the tokne for struct fb_videomode, > > > which is unrelated. > > > > > > Patch 17 removes both flag constants from <linux/fb.h> > > > > We have a few more flags that are unused - should they be nuked too? > > FBINFO_HWACCEL_FILLRECT > > FBINFO_HWACCEL_ROTATE > > FBINFO_HWACCEL_XPAN > > It seems those are there for completeness. Nothing sets _ROTATE, the others > are simply never checked. According to the comments, some are required, some > are optional. I don't know what that means. > > IIRC there were complains about performance when Daniel tried to remove > fbcon acceleration, so not all _HWACCEL_ flags are unneeded. > > Leaving them in for reference/completeness might be an option; or not. I > have no strong feelings about those flags. > > > > > Unused as in no references from fbdev/core/* > > > > I would rather see one series nuke all unused FBINFO flags in one go. > > Assuming my quick grep are right and the above can be dropped. > > I would not want to extend this series. I'm removing _DEFAULT as it's > absolutely pointless and confusing. OK, makes sense and thanks for the explanation. The series is: Acked-by: Sam Ravnborg <sam@ravnborg.org>
On 7/11/23 16:47, Sam Ravnborg wrote: > Hi Thomas, > > On Tue, Jul 11, 2023 at 08:24:40AM +0200, Thomas Zimmermann wrote: >> Hi Sam >> >> Am 10.07.23 um 19:19 schrieb Sam Ravnborg: >>> Hi Thomas, >>> >>> On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote: >>>> Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from >>>> fbdev and drivers, as briefly discussed at [1]. Both flags were maybe >>>> useful when fbdev had special handling for driver modules. With >>>> commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0 >>>> and have no further effect. >>>> >>>> Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5 >>>> split this by the way the fb_info struct is being allocated. All flags >>>> are cleared to zero during the allocation. >>>> >>>> Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes >>>> an actual bug in how arch/sh uses the tokne for struct fb_videomode, >>>> which is unrelated. >>>> >>>> Patch 17 removes both flag constants from <linux/fb.h> >>> >>> We have a few more flags that are unused - should they be nuked too? >>> FBINFO_HWACCEL_FILLRECT >>> FBINFO_HWACCEL_ROTATE >>> FBINFO_HWACCEL_XPAN >> >> It seems those are there for completeness. Nothing sets _ROTATE, I think some fbdev drivers had hardware acceleration for ROTATE in the past. HWACCEL_XPAN is still in some drivers. >> the others are simply never checked. According to the comments, >> some are required, some are optional. I don't know what that >> means. I think it's OK if you remove those flags which aren't used anywhere, e.g. FBINFO_HWACCEL_ROTATE. >> IIRC there were complains about performance when Daniel tried to remove >> fbcon acceleration, so not all _HWACCEL_ flags are unneeded. Correct. I think COPYAREA and FILLRECT are the bare minimum to accelerate fbcon, IMAGEBLIT is for showing the tux penguin (?), XPAN/YPAN and YWRAP for some hardware screen panning needed by some drivers (not sure if this is still used as I don't have such hardware, Geert?). >> Leaving them in for reference/completeness might be an option; or not. I >> have no strong feelings about those flags. I'd say drop FBINFO_HWACCEL_ROTATE at least ? >>> Unused as in no references from fbdev/core/* >>> >>> I would rather see one series nuke all unused FBINFO flags in one go. >>> Assuming my quick grep are right and the above can be dropped. >> >> I would not want to extend this series. I'm removing _DEFAULT as it's >> absolutely pointless and confusing. Yes, Ok. Helge
Hi Helge, On Tue, Jul 11, 2023 at 5:26 PM Helge Deller <deller@gmx.de> wrote: > On 7/11/23 16:47, Sam Ravnborg wrote: > > On Tue, Jul 11, 2023 at 08:24:40AM +0200, Thomas Zimmermann wrote: > >> Am 10.07.23 um 19:19 schrieb Sam Ravnborg: > >>> On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote: > >>>> Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from > >>>> fbdev and drivers, as briefly discussed at [1]. Both flags were maybe > >>>> useful when fbdev had special handling for driver modules. With > >>>> commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0 > >>>> and have no further effect. > >>>> > >>>> Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5 > >>>> split this by the way the fb_info struct is being allocated. All flags > >>>> are cleared to zero during the allocation. > >>>> > >>>> Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes > >>>> an actual bug in how arch/sh uses the tokne for struct fb_videomode, > >>>> which is unrelated. > >>>> > >>>> Patch 17 removes both flag constants from <linux/fb.h> > >>> > >>> We have a few more flags that are unused - should they be nuked too? > >>> FBINFO_HWACCEL_FILLRECT > >>> FBINFO_HWACCEL_ROTATE > >>> FBINFO_HWACCEL_XPAN > >> > >> It seems those are there for completeness. Nothing sets _ROTATE, > > I think some fbdev drivers had hardware acceleration for ROTATE in the > past. HWACCEL_XPAN is still in some drivers. > > >> the others are simply never checked. According to the comments, > >> some are required, some are optional. I don't know what that > >> means. > > I think it's OK if you remove those flags which aren't used anywhere, > e.g. FBINFO_HWACCEL_ROTATE. Indeed. > >> IIRC there were complains about performance when Daniel tried to remove > >> fbcon acceleration, so not all _HWACCEL_ flags are unneeded. > > Correct. I think COPYAREA and FILLRECT are the bare minimum to accelerate > fbcon, IMAGEBLIT is for showing the tux penguin (?), > XPAN/YPAN and YWRAP for some hardware screen panning needed by some drivers > (not sure if this is still used as I don't have such hardware, Geert?). Yes, they are used. Anything that is handled in drivers/video/fbdev/core/ is used: $ git grep HWACCEL_ -- drivers/video/fbdev/core/ drivers/video/fbdev/core/fbcon.c: if ((info->flags & FBINFO_HWACCEL_COPYAREA) && drivers/video/fbdev/core/fbcon.c: !(info->flags & FBINFO_HWACCEL_DISABLED)) drivers/video/fbdev/core/fbcon.c: int good_pan = (cap & FBINFO_HWACCEL_YPAN) && drivers/video/fbdev/core/fbcon.c: int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) && drivers/video/fbdev/core/fbcon.c: int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) && drivers/video/fbdev/core/fbcon.c: !(cap & FBINFO_HWACCEL_DISABLED); drivers/video/fbdev/core/fbcon.c: int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) && drivers/video/fbdev/core/fbcon.c: !(cap & FBINFO_HWACCEL_DISABLED); BTW, I'm surprised FBINFO_HWACCEL_FILLRECT is not handled. But looking at the full history, it never was... > >> Leaving them in for reference/completeness might be an option; or not. I > >> have no strong feelings about those flags. > > I'd say drop FBINFO_HWACCEL_ROTATE at least ? Agreed. Gr{oetje,eeting}s, Geert
Am 11.07.23 um 16:47 schrieb Sam Ravnborg: > Hi Thomas, > > On Tue, Jul 11, 2023 at 08:24:40AM +0200, Thomas Zimmermann wrote: >> Hi Sam >> >> Am 10.07.23 um 19:19 schrieb Sam Ravnborg: >>> Hi Thomas, >>> >>> On Mon, Jul 10, 2023 at 02:50:04PM +0200, Thomas Zimmermann wrote: >>>> Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from >>>> fbdev and drivers, as briefly discussed at [1]. Both flags were maybe >>>> useful when fbdev had special handling for driver modules. With >>>> commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0 >>>> and have no further effect. >>>> >>>> Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5 >>>> split this by the way the fb_info struct is being allocated. All flags >>>> are cleared to zero during the allocation. >>>> >>>> Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes >>>> an actual bug in how arch/sh uses the tokne for struct fb_videomode, >>>> which is unrelated. >>>> >>>> Patch 17 removes both flag constants from <linux/fb.h> >>> >>> We have a few more flags that are unused - should they be nuked too? >>> FBINFO_HWACCEL_FILLRECT >>> FBINFO_HWACCEL_ROTATE >>> FBINFO_HWACCEL_XPAN >> >> It seems those are there for completeness. Nothing sets _ROTATE, the others >> are simply never checked. According to the comments, some are required, some >> are optional. I don't know what that means. >> >> IIRC there were complains about performance when Daniel tried to remove >> fbcon acceleration, so not all _HWACCEL_ flags are unneeded. >> >> Leaving them in for reference/completeness might be an option; or not. I >> have no strong feelings about those flags. >> >>> >>> Unused as in no references from fbdev/core/* >>> >>> I would rather see one series nuke all unused FBINFO flags in one go. >>> Assuming my quick grep are right and the above can be dropped. >> >> I would not want to extend this series. I'm removing _DEFAULT as it's >> absolutely pointless and confusing. > > OK, makes sense and thanks for the explanation. > > The series is: > Acked-by: Sam Ravnborg <sam@ravnborg.org> Thanks a lot. >
Remove the unused flags FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT from fbdev and drivers, as briefly discussed at [1]. Both flags were maybe useful when fbdev had special handling for driver modules. With commit 376b3ff54c9a ("fbdev: Nuke FBINFO_MODULE"), they are both 0 and have no further effect. Patches 1 to 7 remove FBINFO_DEFAULT from drivers. Patches 2 to 5 split this by the way the fb_info struct is being allocated. All flags are cleared to zero during the allocation. Patches 8 to 16 do the same for FBINFO_FLAG_DEFAULT. Patch 8 fixes an actual bug in how arch/sh uses the tokne for struct fb_videomode, which is unrelated. Patch 17 removes both flag constants from <linux/fb.h> [1] https://lore.kernel.org/dri-devel/877crer8fm.fsf@minerva.mail-host-address-is-not-set/ Thomas Zimmermann (17): drm: Remove flag FBINFO_DEFAULT from fbdev emulation fbdev: Remove flag FBINFO_DEFAULT from fbdev drivers fbdev: Remove flag FBINFO_DEFAULT from fbdev drivers fbdev: Remove flag FBINFO_DEFAULT from fbdev drivers fbdev: Remove flag FBINFO_DEFAULT from fbdev drivers fbdev/fsl-diu-fb: Remove flag FBINFO_DEFAULT vfio-mdev: Remove flag FBINFO_DEFAULT from fbdev sample driver arch/sh: Do not assign FBINFO_FLAG_DEFAULT to fb_videomode.flag auxdisplay: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers hid/picolcd: Remove flag FBINFO_FLAG_DEFAULT from fbdev driver media: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers staging: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers fbdev: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers fbdev: Remove flag FBINFO_FLAG_DEFAULT from fbdev drivers fbdev/atafb: Remove flag FBINFO_FLAG_DEFAULT fbdev/pxafb: Remove flag FBINFO_FLAG_DEFAULT fbdev: Remove FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT arch/sh/boards/mach-sh7763rdp/setup.c | 1 - drivers/auxdisplay/cfag12864bfb.c | 1 - drivers/auxdisplay/ht16k33.c | 1 - drivers/gpu/drm/drm_fbdev_dma.c | 1 - drivers/gpu/drm/drm_fbdev_generic.c | 1 - drivers/gpu/drm/gma500/fbdev.c | 2 +- drivers/gpu/drm/radeon/radeon_fbdev.c | 2 +- drivers/hid/hid-picolcd_fb.c | 1 - drivers/media/pci/ivtv/ivtvfb.c | 1 - drivers/media/test-drivers/vivid/vivid-osd.c | 1 - drivers/staging/fbtft/fbtft-core.c | 2 +- drivers/staging/sm750fb/sm750.c | 1 - drivers/video/fbdev/68328fb.c | 2 +- drivers/video/fbdev/acornfb.c | 2 +- drivers/video/fbdev/amba-clcd.c | 1 - drivers/video/fbdev/amifb.c | 5 ++--- drivers/video/fbdev/arcfb.c | 1 - drivers/video/fbdev/asiliantfb.c | 1 - drivers/video/fbdev/atafb.c | 1 - drivers/video/fbdev/atmel_lcdfb.c | 2 +- drivers/video/fbdev/aty/aty128fb.c | 1 - drivers/video/fbdev/aty/atyfb_base.c | 3 +-- drivers/video/fbdev/aty/radeon_base.c | 3 +-- drivers/video/fbdev/broadsheetfb.c | 2 +- drivers/video/fbdev/bw2.c | 1 - drivers/video/fbdev/carminefb.c | 1 - drivers/video/fbdev/cg14.c | 2 +- drivers/video/fbdev/cg3.c | 1 - drivers/video/fbdev/cg6.c | 2 +- drivers/video/fbdev/chipsfb.c | 1 - drivers/video/fbdev/cirrusfb.c | 3 +-- drivers/video/fbdev/clps711x-fb.c | 1 - drivers/video/fbdev/cobalt_lcdfb.c | 1 - drivers/video/fbdev/controlfb.c | 2 +- drivers/video/fbdev/cyber2000fb.c | 2 +- drivers/video/fbdev/da8xx-fb.c | 1 - drivers/video/fbdev/efifb.c | 1 - drivers/video/fbdev/ep93xx-fb.c | 1 - drivers/video/fbdev/ffb.c | 3 +-- drivers/video/fbdev/fm2fb.c | 1 - drivers/video/fbdev/fsl-diu-fb.c | 2 +- drivers/video/fbdev/g364fb.c | 2 +- drivers/video/fbdev/gbefb.c | 1 - drivers/video/fbdev/geode/gx1fb_core.c | 1 - drivers/video/fbdev/geode/gxfb_core.c | 1 - drivers/video/fbdev/geode/lxfb_core.c | 1 - drivers/video/fbdev/goldfishfb.c | 1 - drivers/video/fbdev/grvga.c | 2 +- drivers/video/fbdev/gxt4500.c | 3 +-- drivers/video/fbdev/hecubafb.c | 2 +- drivers/video/fbdev/hgafb.c | 2 +- drivers/video/fbdev/hitfb.c | 2 +- drivers/video/fbdev/hpfb.c | 1 - drivers/video/fbdev/hyperv_fb.c | 2 -- drivers/video/fbdev/i740fb.c | 2 +- drivers/video/fbdev/i810/i810_main.c | 4 ++-- drivers/video/fbdev/imsttfb.c | 3 +-- drivers/video/fbdev/imxfb.c | 3 +-- drivers/video/fbdev/intelfb/intelfbdrv.c | 5 ++--- drivers/video/fbdev/kyro/fbdev.c | 1 - drivers/video/fbdev/leo.c | 1 - drivers/video/fbdev/macfb.c | 1 - drivers/video/fbdev/matrox/matroxfb_crtc2.c | 5 ++--- drivers/video/fbdev/maxinefb.c | 1 - drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 2 +- drivers/video/fbdev/metronomefb.c | 2 +- drivers/video/fbdev/mmp/fb/mmpfb.c | 2 +- drivers/video/fbdev/mx3fb.c | 1 - drivers/video/fbdev/neofb.c | 2 +- drivers/video/fbdev/nvidia/nvidia.c | 4 ++-- drivers/video/fbdev/offb.c | 2 +- drivers/video/fbdev/omap/omapfb_main.c | 1 - drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 1 - drivers/video/fbdev/p9100.c | 1 - drivers/video/fbdev/platinumfb.c | 1 - drivers/video/fbdev/pm2fb.c | 3 +-- drivers/video/fbdev/pm3fb.c | 3 +-- drivers/video/fbdev/pmag-aa-fb.c | 1 - drivers/video/fbdev/pmag-ba-fb.c | 1 - drivers/video/fbdev/pmagb-b-fb.c | 1 - drivers/video/fbdev/ps3fb.c | 2 +- drivers/video/fbdev/pvr2fb.c | 2 +- drivers/video/fbdev/pxa168fb.c | 2 +- drivers/video/fbdev/pxafb.c | 2 -- drivers/video/fbdev/q40fb.c | 1 - drivers/video/fbdev/riva/fbdev.c | 3 +-- drivers/video/fbdev/s1d13xxxfb.c | 4 ++-- drivers/video/fbdev/s3c-fb.c | 1 - drivers/video/fbdev/sa1100fb.c | 1 - drivers/video/fbdev/savage/savagefb_driver.c | 3 +-- drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 -- drivers/video/fbdev/simplefb.c | 1 - drivers/video/fbdev/sis/sis_main.c | 5 +---- drivers/video/fbdev/skeletonfb.c | 2 +- drivers/video/fbdev/sm501fb.c | 2 +- drivers/video/fbdev/sm712fb.c | 1 - drivers/video/fbdev/smscufx.c | 2 +- drivers/video/fbdev/sstfb.c | 1 - drivers/video/fbdev/sunxvr1000.c | 1 - drivers/video/fbdev/sunxvr2500.c | 1 - drivers/video/fbdev/sunxvr500.c | 1 - drivers/video/fbdev/tcx.c | 1 - drivers/video/fbdev/tdfxfb.c | 2 +- drivers/video/fbdev/tgafb.c | 2 +- drivers/video/fbdev/tridentfb.c | 2 +- drivers/video/fbdev/udlfb.c | 2 +- drivers/video/fbdev/uvesafb.c | 3 +-- drivers/video/fbdev/valkyriefb.c | 1 - drivers/video/fbdev/vermilion/vermilion.c | 2 +- drivers/video/fbdev/vesafb.c | 2 +- drivers/video/fbdev/vfb.c | 1 - drivers/video/fbdev/vga16fb.c | 2 +- drivers/video/fbdev/via/viafbdev.c | 2 +- drivers/video/fbdev/vt8500lcdfb.c | 3 +-- drivers/video/fbdev/wm8505fb.c | 3 +-- drivers/video/fbdev/xen-fbfront.c | 2 +- drivers/video/fbdev/xilinxfb.c | 1 - include/linux/fb.h | 3 --- samples/vfio-mdev/mdpy-fb.c | 1 - 119 files changed, 65 insertions(+), 150 deletions(-)