Message ID | 20230710130113.14563-1-tzimmermann@suse.de |
---|---|
Headers | show |
Series | fbdev: Remove FBINFO_DEFAULT and FBINFO_FLAG_DEFAULT flags | expand |
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. >