| Message ID | 20200417155553.675905-1-arnd@arndb.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm, fbdev: rework dependencies | expand |
On Fri, Apr 17, 2020 at 05:55:45PM +0200, Arnd Bergmann wrote: > I tried to fix up some dependencies after the sii8620 "imply EXTCON" > statementn broke, trying a few things but in the backing out a > change that would completely reverse the LEDS_CLASS selects into > a 'depends on'. > > However, what I got now are multiple changes that remove gratious > "selects" that lead to circular dependencies for sii8620 and others: > > - Anything doing "select FB" is now gone, or becomes "depends on FB", > > - DDC support depends on I2C instead of selecting it > > - backlight class device support is never selected by framebuffer > drivers but has proper dependencies > > I have done thousands of randconfig build tests on this, but no > runtime tests. > > Some of the 'depends on FOO || !FOO' statements could be simplified > into a new 'uses FOO' syntax based on a patch from Saeed Mahameed, > but I would for the moment treat that as a cleanup that can be done > later. > > If we can agree on these changes, maybe someone can merge them > through the drm-misc tree. > > Please review Biggest concern I have is that usability of make menuconfig is horrible, and it's very hard to find options that are hidden by depends on. You can use the search interface, if you happen to know the option. Once you've surmounted that bar, the next one is trying to find what exactly you need to enable. Which again means endless of recursive screaming at Kconfig files, since make menuconfig doesn't help you at all. That's pretty much why we've never pushed this in, and instead done the selects. I'm vary applying all this, since after after there'll be all the screaming again and we have to back it out. I think to embrace this without regrets what we need is: - some way to list the hidden options - some way to browse the depedencies of those hidden options menuconfig cant do that, gconfig I cant build here (it's some old gtk2 thing, where do you even get the deps for that). xconfig also cant do this easily, dependencies aren't linked. So yeah not sure this is a good idea at all, until at least menuconfig can cope. -Daniel > > Arnd > > Arnd Bergmann (8): > fbdev: w100fb: clean up mach-pxa compile-time dependency > fbdev/ARM: pxa: avoid selecting CONFIG_FB > fbdev: rework FB_DDC dependencies > drm/rcar: stop using 'imply' for dependencies > drm/vmwgfx: make framebuffer support optional > drm: decouple from CONFIG_FB > fbdev: rework backlight dependencies > drm/bridge/sii8620: fix extcon dependency > > arch/arm/configs/pxa_defconfig | 3 ++ > arch/arm/mach-pxa/Kconfig | 7 --- > arch/arm/mach-pxa/eseries.c | 14 +---- > arch/arm/mach-pxa/saar.c | 2 +- > arch/arm/mach-pxa/tavorevb.c | 2 +- > drivers/auxdisplay/Kconfig | 1 + > drivers/gpu/drm/Kconfig | 5 +- > drivers/gpu/drm/bridge/Kconfig | 2 +- > drivers/gpu/drm/mxsfb/Kconfig | 1 - > drivers/gpu/drm/rcar-du/Kconfig | 23 +++++--- > drivers/gpu/drm/vmwgfx/Kconfig | 17 +++--- > drivers/gpu/drm/vmwgfx/Makefile | 4 +- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 35 +++++++----- > drivers/gpu/drm/zte/Kconfig | 1 - > drivers/macintosh/Kconfig | 1 + > drivers/staging/fbtft/Kconfig | 1 + > drivers/staging/olpc_dcon/Kconfig | 2 +- > drivers/video/fbdev/Kconfig | 31 ++++++++--- > drivers/video/fbdev/w100fb.c | 84 +++++------------------------ > include/video/w100fb.h | 6 +-- > 20 files changed, 101 insertions(+), 141 deletions(-) > > -- > 2.26.0 > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Nicolas Pitre <nico@fluxnic.net> > Cc: Andrzej Hajda <a.hajda@samsung.com> > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Cc: Saeed Mahameed <saeedm@mellanox.com> > Cc: <masahiroy@kernel.org> > Cc: <Laurent.pinchart@ideasonboard.com> > Cc: <linux-renesas-soc@vger.kernel.org>, > Cc: "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, > Cc: <kieran.bingham+renesas@ideasonboard.com>, > Cc: <airlied@linux.ie> > Cc: daniel@zonque.org > Cc: haojian.zhuang@gmail.com > Cc: robert.jarzmik@free.fr > Cc: daniel@ffwll.ch > Cc: marex@denx.de > Cc: stefan@agner.ch > Cc: linux-graphics-maintainer@vmware.com > Cc: thellstrom@vmware.com > Cc: jfrederich@gmail.com > Cc: dsd@laptop.org > Cc: dri-devel@lists.freedesktop.org > Cc: linux-fbdev@vger.kernel.org > Cc: geert@linux-m68k.org
On Fri, Apr 17, 2020 at 07:14:53PM +0200, Daniel Vetter wrote: > On Fri, Apr 17, 2020 at 05:55:45PM +0200, Arnd Bergmann wrote: > > I tried to fix up some dependencies after the sii8620 "imply EXTCON" > > statementn broke, trying a few things but in the backing out a > > change that would completely reverse the LEDS_CLASS selects into > > a 'depends on'. > > > > However, what I got now are multiple changes that remove gratious > > "selects" that lead to circular dependencies for sii8620 and others: > > > > - Anything doing "select FB" is now gone, or becomes "depends on FB", > > > > - DDC support depends on I2C instead of selecting it > > > > - backlight class device support is never selected by framebuffer > > drivers but has proper dependencies > > > > I have done thousands of randconfig build tests on this, but no > > runtime tests. > > > > Some of the 'depends on FOO || !FOO' statements could be simplified > > into a new 'uses FOO' syntax based on a patch from Saeed Mahameed, > > but I would for the moment treat that as a cleanup that can be done > > later. > > > > If we can agree on these changes, maybe someone can merge them > > through the drm-misc tree. > > > > Please review > > Biggest concern I have is that usability of make menuconfig is horrible, > and it's very hard to find options that are hidden by depends on. You can > use the search interface, if you happen to know the option. > > Once you've surmounted that bar, the next one is trying to find what > exactly you need to enable. Which again means endless of recursive > screaming at Kconfig files, since make menuconfig doesn't help you at all. +1 on this. But this is a general kconfig problem, and not unique to DRM, I've done this screaming for many different things now.. eg to turn on every single RDMA driver. I hackily delt with it by creating this rather insane script based on the python kconfiglib to try and sort things out mostly automatically: https://github.com/jgunthorpe/Kernel-Maintainer-Tools/blob/master/gj_tools/cmd_kconfig.py It would be great if menuconfig had a key to say 'hey, really, turn this on and everything it depends on, recursively' Jason
On Fri, 17 Apr 2020, Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Fri, Apr 17, 2020 at 07:14:53PM +0200, Daniel Vetter wrote: >> On Fri, Apr 17, 2020 at 05:55:45PM +0200, Arnd Bergmann wrote: >> > I tried to fix up some dependencies after the sii8620 "imply EXTCON" >> > statementn broke, trying a few things but in the backing out a >> > change that would completely reverse the LEDS_CLASS selects into >> > a 'depends on'. >> > >> > However, what I got now are multiple changes that remove gratious >> > "selects" that lead to circular dependencies for sii8620 and others: >> > >> > - Anything doing "select FB" is now gone, or becomes "depends on FB", >> > >> > - DDC support depends on I2C instead of selecting it >> > >> > - backlight class device support is never selected by framebuffer >> > drivers but has proper dependencies >> > >> > I have done thousands of randconfig build tests on this, but no >> > runtime tests. >> > >> > Some of the 'depends on FOO || !FOO' statements could be simplified >> > into a new 'uses FOO' syntax based on a patch from Saeed Mahameed, >> > but I would for the moment treat that as a cleanup that can be done >> > later. >> > >> > If we can agree on these changes, maybe someone can merge them >> > through the drm-misc tree. >> > >> > Please review >> >> Biggest concern I have is that usability of make menuconfig is horrible, >> and it's very hard to find options that are hidden by depends on. You can >> use the search interface, if you happen to know the option. >> >> Once you've surmounted that bar, the next one is trying to find what >> exactly you need to enable. Which again means endless of recursive >> screaming at Kconfig files, since make menuconfig doesn't help you at all. > > +1 on this. But this is a general kconfig problem, and not unique to > DRM, I've done this screaming for many different things now.. eg to > turn on every single RDMA driver. > > I hackily delt with it by creating this rather insane script based on > the python kconfiglib to try and sort things out mostly automatically: > > https://github.com/jgunthorpe/Kernel-Maintainer-Tools/blob/master/gj_tools/cmd_kconfig.py > > It would be great if menuconfig had a key to say 'hey, really, turn > this on and everything it depends on, recursively' I'm really all for switching to using depends when that is the semantically right thing to do. In many places using select is a hack to make the UI simpler, and that's just plain wrong. We'll be doomed to perpetual randconfig build failures and duct tape fixes. I'm pretty tired of this, and I regularly ignore those duct tape fixes to i915 backlight build issues on some bizarre configs that nobody will ever use, and would not exist if depends were used throughout. I'm fine with select but only when it's restricted to symbols that have no dependencies of their own and have no UI. This is in line with Documentation/kbuild/kconfig-language.rst. Not enforcing this is another Kconfig tool shortcoming. See also my reply to Sam [1]. BR, Jani. [1] https://lore.kernel.org/dri-devel/871roi37qe.fsf@intel.com/
On Mon, Apr 20, 2020 at 10:14 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Fri, 17 Apr 2020, Jason Gunthorpe <jgg@ziepe.ca> wrote: > > On Fri, Apr 17, 2020 at 07:14:53PM +0200, Daniel Vetter wrote: > >> On Fri, Apr 17, 2020 at 05:55:45PM +0200, Arnd Bergmann wrote: > >> > > >> > If we can agree on these changes, maybe someone can merge them > >> > through the drm-misc tree. > >> > > >> > Please review > >> > >> Biggest concern I have is that usability of make menuconfig is horrible, No doubt about that, but that seems to be unrelated to the cleanup. > >> and it's very hard to find options that are hidden by depends on. You can > >> use the search interface, if you happen to know the option. > >> > >> Once you've surmounted that bar, the next one is trying to find what > >> exactly you need to enable. Which again means endless of recursive > >> screaming at Kconfig files, since make menuconfig doesn't help you at all. The changes I'm doing are mostly for fbdev, which is currently the odd one out. Most kernel subsystems today follow the documented recommendations and only use 'depends on' for things they depend on. Having fbdev be the exception causes two problems: - It does not make kconfig any easier to use overall, just less consistent when it is the only thing that implicitly turns on dependencies and for everything else one still has to look up what the dependencies are. - Most of the problems with circular dependencies come from mixing the two methods, and most of the cases where they have caused problems in the past involve fbdev in some way. I also doubt switching lots of 'depends on' to 'select' all over Kconfig would improve the situation on a global level. It would simplify the problem of turning something on without understanding the what it does, but in turn it makes it harder to turn off something else. E.g. today it is hard to turn off fbdev because that is selected by a number of (partly unrelated) options, but there was a recent discussion about getting distros to stop enabling fbdev out of security concerns. > I'm really all for switching to using depends when that is the > semantically right thing to do. In many places using select is a hack to > make the UI simpler, and that's just plain wrong. We'll be doomed to > perpetual randconfig build failures and duct tape fixes. > > I'm pretty tired of this, and I regularly ignore those duct tape fixes > to i915 backlight build issues on some bizarre configs that nobody will > ever use, and would not exist if depends were used throughout. > > I'm fine with select but only when it's restricted to symbols that have > no dependencies of their own and have no UI. This is in line with > Documentation/kbuild/kconfig-language.rst. Not enforcing this is another > Kconfig tool shortcoming. Agreed, that is generally a good rule. Arnd
On Mon, Apr 20, 2020 at 04:03:23PM +0200, Arnd Bergmann wrote: > On Mon, Apr 20, 2020 at 10:14 AM Jani Nikula > <jani.nikula@linux.intel.com> wrote: > > On Fri, 17 Apr 2020, Jason Gunthorpe <jgg@ziepe.ca> wrote: > > > On Fri, Apr 17, 2020 at 07:14:53PM +0200, Daniel Vetter wrote: > > >> On Fri, Apr 17, 2020 at 05:55:45PM +0200, Arnd Bergmann wrote: > > >> > > > >> > If we can agree on these changes, maybe someone can merge them > > >> > through the drm-misc tree. > > >> > > > >> > Please review > > >> > > >> Biggest concern I have is that usability of make menuconfig is horrible, > > No doubt about that, but that seems to be unrelated to the cleanup. > > > >> and it's very hard to find options that are hidden by depends on. You can > > >> use the search interface, if you happen to know the option. > > >> > > >> Once you've surmounted that bar, the next one is trying to find what > > >> exactly you need to enable. Which again means endless of recursive > > >> screaming at Kconfig files, since make menuconfig doesn't help you at all. > > The changes I'm doing are mostly for fbdev, which is currently the > odd one out. Most kernel subsystems today follow the documented > recommendations and only use 'depends on' for things they > depend on. > > Having fbdev be the exception causes two problems: > > - It does not make kconfig any easier to use overall, just less consistent > when it is the only thing that implicitly turns on dependencies and > for everything else one still has to look up what the dependencies are. > > - Most of the problems with circular dependencies come from mixing > the two methods, and most of the cases where they have caused > problems in the past involve fbdev in some way. > > I also doubt switching lots of 'depends on' to 'select' all over Kconfig > would improve the situation on a global level. It would simplify the > problem of turning something on without understanding the what it > does, but in turn it makes it harder to turn off something else. > > E.g. today it is hard to turn off fbdev because that is selected by a > number of (partly unrelated) options, but there was a recent discussion > about getting distros to stop enabling fbdev out of security concerns. I've done some history digging, this is the patch that started this all: commit d2f59357700487a8b944f4f7777d1e97cf5ea2ed Author: Ingo Molnar <mingo@elte.hu> Date: Thu Feb 5 16:03:34 2009 +0100 drm/i915: select framebuffer support automatically I.e. driver gets disabled because a new config is added which isn't enabled. System doesn't boot, maintainer gets angry regression report, select hack gets added. Note on the specific example the code has been reworked enough that even if you'd upgrade the kernel all that would get disabled now is the fbdev emulation on top of drm drivers, not any of the drm drivers. The above says we should have an automatic system for at least oldconfig (but would be nice in menuconfig too), since "break user's kernel on upgrade" isn't an option. And without that select is going to come back somewhere and make a huge nasty mess: We're definitely not going to fix Kconfig when fixing a regression in -rc kernels. So in theory no need to convince me that select is terrible. Practice disagrees unfortunately. -Daniel > > > I'm really all for switching to using depends when that is the > > semantically right thing to do. In many places using select is a hack to > > make the UI simpler, and that's just plain wrong. We'll be doomed to > > perpetual randconfig build failures and duct tape fixes. > > > > I'm pretty tired of this, and I regularly ignore those duct tape fixes > > to i915 backlight build issues on some bizarre configs that nobody will > > ever use, and would not exist if depends were used throughout. > > > > I'm fine with select but only when it's restricted to symbols that have > > no dependencies of their own and have no UI. This is in line with > > Documentation/kbuild/kconfig-language.rst. Not enforcing this is another > > Kconfig tool shortcoming. > > Agreed, that is generally a good rule. > > Arnd
On Tue, 21 Apr 2020, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Apr 20, 2020 at 04:03:23PM +0200, Arnd Bergmann wrote: >> On Mon, Apr 20, 2020 at 10:14 AM Jani Nikula >> <jani.nikula@linux.intel.com> wrote: >> > On Fri, 17 Apr 2020, Jason Gunthorpe <jgg@ziepe.ca> wrote: >> > > On Fri, Apr 17, 2020 at 07:14:53PM +0200, Daniel Vetter wrote: >> > >> On Fri, Apr 17, 2020 at 05:55:45PM +0200, Arnd Bergmann wrote: >> > >> > >> > >> > If we can agree on these changes, maybe someone can merge them >> > >> > through the drm-misc tree. >> > >> > >> > >> > Please review >> > >> >> > >> Biggest concern I have is that usability of make menuconfig is horrible, >> >> No doubt about that, but that seems to be unrelated to the cleanup. >> >> > >> and it's very hard to find options that are hidden by depends on. You can >> > >> use the search interface, if you happen to know the option. >> > >> >> > >> Once you've surmounted that bar, the next one is trying to find what >> > >> exactly you need to enable. Which again means endless of recursive >> > >> screaming at Kconfig files, since make menuconfig doesn't help you at all. >> >> The changes I'm doing are mostly for fbdev, which is currently the >> odd one out. Most kernel subsystems today follow the documented >> recommendations and only use 'depends on' for things they >> depend on. >> >> Having fbdev be the exception causes two problems: >> >> - It does not make kconfig any easier to use overall, just less consistent >> when it is the only thing that implicitly turns on dependencies and >> for everything else one still has to look up what the dependencies are. >> >> - Most of the problems with circular dependencies come from mixing >> the two methods, and most of the cases where they have caused >> problems in the past involve fbdev in some way. >> >> I also doubt switching lots of 'depends on' to 'select' all over Kconfig >> would improve the situation on a global level. It would simplify the >> problem of turning something on without understanding the what it >> does, but in turn it makes it harder to turn off something else. >> >> E.g. today it is hard to turn off fbdev because that is selected by a >> number of (partly unrelated) options, but there was a recent discussion >> about getting distros to stop enabling fbdev out of security concerns. > > I've done some history digging, this is the patch that started this all: > > commit d2f59357700487a8b944f4f7777d1e97cf5ea2ed > Author: Ingo Molnar <mingo@elte.hu> > Date: Thu Feb 5 16:03:34 2009 +0100 > > drm/i915: select framebuffer support automatically > > I.e. driver gets disabled because a new config is added which isn't > enabled. System doesn't boot, maintainer gets angry regression report, > select hack gets added. Gotta love a good commit message from a decade ago. First, it says it's a migration helper. And that the problem specifically is that the user has a working config *without* FB enabled as a starting point. Now, if the starting point for a new config *now* is less than ten years old, and it had i915 enabled, it'll also have FB enabled. Because select. The migration part has done its job, and I think we should be good to make some progress. The commit message also notes the problems of select. BR, Jani. > Note on the specific example the code has been reworked enough that even > if you'd upgrade the kernel all that would get disabled now is the fbdev > emulation on top of drm drivers, not any of the drm drivers. > > The above says we should have an automatic system for at least oldconfig > (but would be nice in menuconfig too), since "break user's kernel on > upgrade" isn't an option. And without that select is going to come back > somewhere and make a huge nasty mess: We're definitely not going to > fix Kconfig when fixing a regression in -rc kernels. > > So in theory no need to convince me that select is terrible. Practice > disagrees unfortunately. > -Daniel > >> >> > I'm really all for switching to using depends when that is the >> > semantically right thing to do. In many places using select is a hack to >> > make the UI simpler, and that's just plain wrong. We'll be doomed to >> > perpetual randconfig build failures and duct tape fixes. >> > >> > I'm pretty tired of this, and I regularly ignore those duct tape fixes >> > to i915 backlight build issues on some bizarre configs that nobody will >> > ever use, and would not exist if depends were used throughout. >> > >> > I'm fine with select but only when it's restricted to symbols that have >> > no dependencies of their own and have no UI. This is in line with >> > Documentation/kbuild/kconfig-language.rst. Not enforcing this is another >> > Kconfig tool shortcoming. >> >> Agreed, that is generally a good rule. >> >> Arnd
Hi Jani, On Tue, Apr 21, 2020 at 2:58 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Tue, 21 Apr 2020, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Mon, Apr 20, 2020 at 04:03:23PM +0200, Arnd Bergmann wrote: > >> On Mon, Apr 20, 2020 at 10:14 AM Jani Nikula > >> <jani.nikula@linux.intel.com> wrote: > >> > On Fri, 17 Apr 2020, Jason Gunthorpe <jgg@ziepe.ca> wrote: > >> > > On Fri, Apr 17, 2020 at 07:14:53PM +0200, Daniel Vetter wrote: > >> > >> On Fri, Apr 17, 2020 at 05:55:45PM +0200, Arnd Bergmann wrote: > >> > >> > > >> > >> > If we can agree on these changes, maybe someone can merge them > >> > >> > through the drm-misc tree. > >> > >> > > >> > >> > Please review > >> > >> > >> > >> Biggest concern I have is that usability of make menuconfig is horrible, > >> > >> No doubt about that, but that seems to be unrelated to the cleanup. > >> > >> > >> and it's very hard to find options that are hidden by depends on. You can > >> > >> use the search interface, if you happen to know the option. > >> > >> > >> > >> Once you've surmounted that bar, the next one is trying to find what > >> > >> exactly you need to enable. Which again means endless of recursive > >> > >> screaming at Kconfig files, since make menuconfig doesn't help you at all. > >> > >> The changes I'm doing are mostly for fbdev, which is currently the > >> odd one out. Most kernel subsystems today follow the documented > >> recommendations and only use 'depends on' for things they > >> depend on. > >> > >> Having fbdev be the exception causes two problems: > >> > >> - It does not make kconfig any easier to use overall, just less consistent > >> when it is the only thing that implicitly turns on dependencies and > >> for everything else one still has to look up what the dependencies are. > >> > >> - Most of the problems with circular dependencies come from mixing > >> the two methods, and most of the cases where they have caused > >> problems in the past involve fbdev in some way. > >> > >> I also doubt switching lots of 'depends on' to 'select' all over Kconfig > >> would improve the situation on a global level. It would simplify the > >> problem of turning something on without understanding the what it > >> does, but in turn it makes it harder to turn off something else. > >> > >> E.g. today it is hard to turn off fbdev because that is selected by a > >> number of (partly unrelated) options, but there was a recent discussion > >> about getting distros to stop enabling fbdev out of security concerns. > > > > I've done some history digging, this is the patch that started this all: > > > > commit d2f59357700487a8b944f4f7777d1e97cf5ea2ed > > Author: Ingo Molnar <mingo@elte.hu> > > Date: Thu Feb 5 16:03:34 2009 +0100 > > > > drm/i915: select framebuffer support automatically > > > > I.e. driver gets disabled because a new config is added which isn't > > enabled. System doesn't boot, maintainer gets angry regression report, > > select hack gets added. > > Gotta love a good commit message from a decade ago. > > First, it says it's a migration helper. And that the problem > specifically is that the user has a working config *without* FB enabled > as a starting point. > > Now, if the starting point for a new config *now* is less than ten years > old, and it had i915 enabled, it'll also have FB enabled. Because > select. The migration part has done its job, and I think we should be > good to make some progress. It will indeed work with "make oldconfig", as an old config with CONFIG_DRM_I915 enabled will have CONFIG_FB set. However, that is not true when starting with a defconfig that has CONFIG_DRM_I915 enabled: such a defconfig will not have CONFIG_FB set, due to the trimming process when creating a minimal defconfig. Hence when making the change from "select FB" to "depends on FB", you have to make sure to update the affected defconfigs, too: $ git grep CONFIG_DRM_I915 -- "*defconfig*" arch/x86/configs/i386_defconfig:CONFIG_DRM_I915=y arch/x86/configs/x86_64_defconfig:CONFIG_DRM_I915=y Gr{oetje,eeting}s, Geert
On Tue, Apr 21, 2020 at 3:05 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Jani, > > On Tue, Apr 21, 2020 at 2:58 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Tue, 21 Apr 2020, Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Mon, Apr 20, 2020 at 04:03:23PM +0200, Arnd Bergmann wrote: > > >> On Mon, Apr 20, 2020 at 10:14 AM Jani Nikula > > >> <jani.nikula@linux.intel.com> wrote: > > >> > On Fri, 17 Apr 2020, Jason Gunthorpe <jgg@ziepe.ca> wrote: > > >> > > On Fri, Apr 17, 2020 at 07:14:53PM +0200, Daniel Vetter wrote: > > >> > >> On Fri, Apr 17, 2020 at 05:55:45PM +0200, Arnd Bergmann wrote: > > >> > >> > > > >> > >> > If we can agree on these changes, maybe someone can merge them > > >> > >> > through the drm-misc tree. > > >> > >> > > > >> > >> > Please review > > >> > >> > > >> > >> Biggest concern I have is that usability of make menuconfig is horrible, > > >> > > >> No doubt about that, but that seems to be unrelated to the cleanup. > > >> > > >> > >> and it's very hard to find options that are hidden by depends on. You can > > >> > >> use the search interface, if you happen to know the option. > > >> > >> > > >> > >> Once you've surmounted that bar, the next one is trying to find what > > >> > >> exactly you need to enable. Which again means endless of recursive > > >> > >> screaming at Kconfig files, since make menuconfig doesn't help you at all. > > >> > > >> The changes I'm doing are mostly for fbdev, which is currently the > > >> odd one out. Most kernel subsystems today follow the documented > > >> recommendations and only use 'depends on' for things they > > >> depend on. > > >> > > >> Having fbdev be the exception causes two problems: > > >> > > >> - It does not make kconfig any easier to use overall, just less consistent > > >> when it is the only thing that implicitly turns on dependencies and > > >> for everything else one still has to look up what the dependencies are. > > >> > > >> - Most of the problems with circular dependencies come from mixing > > >> the two methods, and most of the cases where they have caused > > >> problems in the past involve fbdev in some way. > > >> > > >> I also doubt switching lots of 'depends on' to 'select' all over Kconfig > > >> would improve the situation on a global level. It would simplify the > > >> problem of turning something on without understanding the what it > > >> does, but in turn it makes it harder to turn off something else. > > >> > > >> E.g. today it is hard to turn off fbdev because that is selected by a > > >> number of (partly unrelated) options, but there was a recent discussion > > >> about getting distros to stop enabling fbdev out of security concerns. > > > > > > I've done some history digging, this is the patch that started this all: > > > > > > commit d2f59357700487a8b944f4f7777d1e97cf5ea2ed > > > Author: Ingo Molnar <mingo@elte.hu> > > > Date: Thu Feb 5 16:03:34 2009 +0100 > > > > > > drm/i915: select framebuffer support automatically > > > > > > I.e. driver gets disabled because a new config is added which isn't > > > enabled. System doesn't boot, maintainer gets angry regression report, > > > select hack gets added. > > > > Gotta love a good commit message from a decade ago. > > > > First, it says it's a migration helper. And that the problem > > specifically is that the user has a working config *without* FB enabled > > as a starting point. > > > > Now, if the starting point for a new config *now* is less than ten years > > old, and it had i915 enabled, it'll also have FB enabled. Because > > select. The migration part has done its job, and I think we should be > > good to make some progress. > > It will indeed work with "make oldconfig", as an old config with > CONFIG_DRM_I915 enabled will have CONFIG_FB set. > > However, that is not true when starting with a defconfig that has > CONFIG_DRM_I915 enabled: such a defconfig will not have CONFIG_FB set, > due to the trimming process when creating a minimal defconfig. > > Hence when making the change from "select FB" to "depends on FB", you > have to make sure to update the affected defconfigs, too: > > $ git grep CONFIG_DRM_I915 -- "*defconfig*" > arch/x86/configs/i386_defconfig:CONFIG_DRM_I915=y > arch/x86/configs/x86_64_defconfig:CONFIG_DRM_I915=y To clarify what I was aiming for with my mail: I'm not worried about fbdev here, I'm just worried that this will come back, and we'll grow select somewhere else until it's become a big & totally horrible mess. I think a lot of the backlight selects have also grown because of this, so this isn't just a one-off I think. If Arnd is happy to play "Kconfig select" whack-a-mole ever once in a while (and deal with the intermediate compile horrors while everyone upgrades) I'm ok with this landing. Just not terribly happy if the underlying issue isn't fixed. -Daniel > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
On Tue, 21 Apr 2020, Daniel Vetter <daniel@ffwll.ch> wrote: > To clarify what I was aiming for with my mail: I'm not worried about > fbdev here, I'm just worried that this will come back, and we'll grow > select somewhere else until it's become a big & totally horrible mess. > I think a lot of the backlight selects have also grown because of > this, so this isn't just a one-off I think. > > If Arnd is happy to play "Kconfig select" whack-a-mole ever once in a > while (and deal with the intermediate compile horrors while everyone > upgrades) I'm ok with this landing. Just not terribly happy if the > underlying issue isn't fixed. And I'll keep ignoring the IS_REACHABLE() patches that make i915 build with a config that should not exist. ;) BR, Jani.
I tried to fix up some dependencies after the sii8620 "imply EXTCON" statementn broke, trying a few things but in the backing out a change that would completely reverse the LEDS_CLASS selects into a 'depends on'. However, what I got now are multiple changes that remove gratious "selects" that lead to circular dependencies for sii8620 and others: - Anything doing "select FB" is now gone, or becomes "depends on FB", - DDC support depends on I2C instead of selecting it - backlight class device support is never selected by framebuffer drivers but has proper dependencies I have done thousands of randconfig build tests on this, but no runtime tests. Some of the 'depends on FOO || !FOO' statements could be simplified into a new 'uses FOO' syntax based on a patch from Saeed Mahameed, but I would for the moment treat that as a cleanup that can be done later. If we can agree on these changes, maybe someone can merge them through the drm-misc tree. Please review Arnd Arnd Bergmann (8): fbdev: w100fb: clean up mach-pxa compile-time dependency fbdev/ARM: pxa: avoid selecting CONFIG_FB fbdev: rework FB_DDC dependencies drm/rcar: stop using 'imply' for dependencies drm/vmwgfx: make framebuffer support optional drm: decouple from CONFIG_FB fbdev: rework backlight dependencies drm/bridge/sii8620: fix extcon dependency arch/arm/configs/pxa_defconfig | 3 ++ arch/arm/mach-pxa/Kconfig | 7 --- arch/arm/mach-pxa/eseries.c | 14 +---- arch/arm/mach-pxa/saar.c | 2 +- arch/arm/mach-pxa/tavorevb.c | 2 +- drivers/auxdisplay/Kconfig | 1 + drivers/gpu/drm/Kconfig | 5 +- drivers/gpu/drm/bridge/Kconfig | 2 +- drivers/gpu/drm/mxsfb/Kconfig | 1 - drivers/gpu/drm/rcar-du/Kconfig | 23 +++++--- drivers/gpu/drm/vmwgfx/Kconfig | 17 +++--- drivers/gpu/drm/vmwgfx/Makefile | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 35 +++++++----- drivers/gpu/drm/zte/Kconfig | 1 - drivers/macintosh/Kconfig | 1 + drivers/staging/fbtft/Kconfig | 1 + drivers/staging/olpc_dcon/Kconfig | 2 +- drivers/video/fbdev/Kconfig | 31 ++++++++--- drivers/video/fbdev/w100fb.c | 84 +++++------------------------ include/video/w100fb.h | 6 +-- 20 files changed, 101 insertions(+), 141 deletions(-)