| Message ID | 1427894130-14228-2-git-send-email-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 01 Apr 2015, Gerd Hoffmann <kraxel@redhat.com> wrote: > After adding virtio-gpu I get this funky kconfig dependency loop. > > scripts/kconfig/conf --oldconfig Kconfig > drivers/video/fbdev/Kconfig:5:error: recursive dependency detected! > drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER > drivers/gpu/drm/Kconfig:34: symbol DRM_KMS_FB_HELPER is selected by DRM_VIRTIO_GPU > drivers/gpu/drm/virtio/Kconfig:1: symbol DRM_VIRTIO_GPU depends on VIRTIO > drivers/virtio/Kconfig:1: symbol VIRTIO is selected by REMOTEPROC > drivers/remoteproc/Kconfig:4: symbol REMOTEPROC is selected by OMAP_REMOTEPROC > drivers/remoteproc/Kconfig:12: symbol OMAP_REMOTEPROC depends on OMAP_IOMMU > drivers/iommu/Kconfig:141: symbol OMAP_IOMMU is selected by VIDEO_OMAP3 > drivers/media/platform/Kconfig:96: symbol VIDEO_OMAP3 depends on VIDEO_V4L2 > drivers/media/v4l2-core/Kconfig:6: symbol VIDEO_V4L2 depends on I2C > drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC > drivers/video/fbdev/Kconfig:59: symbol FB_DDC is selected by FB_CYBER2000_DDC > drivers/video/fbdev/Kconfig:374: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 > drivers/video/fbdev/Kconfig:362: symbol FB_CYBER2000 depends on FB > > Making VIDEO_OMAP3 depend on OMAP_IOMMU instead of selecting it breaks the > loop, which looks like the best way to handle it to me. I'm open to better > suggestions though. I think part of the problem is that "select" is often used not as documented [1] but rather as "show my config in menuconfig for convenience even if my dependency is not met, and select the dependency even though I know it can screw up the dependency chain". In light of the documentation, your patch seems to DTRT. (Disclaimer: I don't work with the drivers in question, hence no Reviewed-by.) In the big picture, it feels like menuconfig needs a way to display items whose dependencies are not met, and a way to recursively enable said items and all their dependencies when told. This would reduce the resistance to sticking with "select" when clearly "depends" is what's meant. BR, Jani. [1] Documentation/kbuild/kconfig-language.txt: "In general use select only for non-visible symbols (no prompts anywhere) and for symbols with no dependencies. That will limit the usefulness but on the other hand avoid the illegal configurations all over." > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > drivers/media/platform/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index d9b872b..fc21734 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -87,8 +87,8 @@ config VIDEO_OMAP3 > tristate "OMAP 3 Camera support" > depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 > depends on HAS_DMA > + depends on OMAP_IOMMU > select ARM_DMA_USE_IOMMU > - select OMAP_IOMMU > select VIDEOBUF2_DMA_CONTIG > ---help--- > Driver for an OMAP 3 camera controller. > -- > 1.8.3.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Gerd, I've read the patches about the virtio-gpu, it's a nice design. As far as I know, there are two other drivers used by qemu, CIRRUS and BOCHS. I have a question about the relationship of these three drivers, is that the virtio-gpu designed to replace the other two drivers? I mean are the CIRRUS and BOCHS going to be deprecated in the future? Would you please kindly explain this a little bit? Actually, this is a problem by *Martin Peres* who is the GSoC xorg administor. My proposal is "Convert the BOCHS and CIRRUS drivers to atomic mode-setting". Martin wonder if the two drivers are going to be deprecated, there is no need for me to do the job. Best regards, John On Wed, Apr 1, 2015 at 9:15 PM, Gerd Hoffmann <kraxel@redhat.com> wrote: > After adding virtio-gpu I get this funky kconfig dependency loop. > > scripts/kconfig/conf --oldconfig Kconfig > drivers/video/fbdev/Kconfig:5:error: recursive dependency detected! > drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER > drivers/gpu/drm/Kconfig:34: symbol DRM_KMS_FB_HELPER is selected by > DRM_VIRTIO_GPU > drivers/gpu/drm/virtio/Kconfig:1: symbol DRM_VIRTIO_GPU depends on > VIRTIO > drivers/virtio/Kconfig:1: symbol VIRTIO is selected by REMOTEPROC > drivers/remoteproc/Kconfig:4: symbol REMOTEPROC is selected by > OMAP_REMOTEPROC > drivers/remoteproc/Kconfig:12: symbol OMAP_REMOTEPROC depends on > OMAP_IOMMU > drivers/iommu/Kconfig:141: symbol OMAP_IOMMU is selected by > VIDEO_OMAP3 > drivers/media/platform/Kconfig:96: symbol VIDEO_OMAP3 depends on > VIDEO_V4L2 > drivers/media/v4l2-core/Kconfig:6: symbol VIDEO_V4L2 depends on I2C > drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC > drivers/video/fbdev/Kconfig:59: symbol FB_DDC is selected by > FB_CYBER2000_DDC > drivers/video/fbdev/Kconfig:374: symbol FB_CYBER2000_DDC depends on > FB_CYBER2000 > drivers/video/fbdev/Kconfig:362: symbol FB_CYBER2000 depends on FB > > Making VIDEO_OMAP3 depend on OMAP_IOMMU instead of selecting it breaks the > loop, which looks like the best way to handle it to me. I'm open to better > suggestions though. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > drivers/media/platform/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/Kconfig > b/drivers/media/platform/Kconfig > index d9b872b..fc21734 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -87,8 +87,8 @@ config VIDEO_OMAP3 > tristate "OMAP 3 Camera support" > depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 > depends on HAS_DMA > + depends on OMAP_IOMMU > select ARM_DMA_USE_IOMMU > - select OMAP_IOMMU > select VIDEOBUF2_DMA_CONTIG > ---help--- > Driver for an OMAP 3 camera controller. > -- > 1.8.3.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
On Wed, Apr 01, 2015 at 10:55:01PM +0800, John Hunter wrote: > Hi Gerd, > I've read the patches about the virtio-gpu, it's a nice design. > As far as I know, there are two other drivers used by qemu, CIRRUS and BOCHS. > I have a question about the relationship of these three drivers, is that the > virtio-gpu > designed to replace the other two drivers? I mean are the CIRRUS and BOCHS > going to be deprecated in the future? > > Would you please kindly explain this a little bit? > > Actually, this is a problem by Martin Peres who is the GSoC xorg administor. > My proposal is "Convert the BOCHS and CIRRUS drivers to atomic mode-setting". > Martin wonder if the two drivers are going to be deprecated, there is no need > for > me to do the job. > > Best regards, > John Hypervisors are going to support BOCHS and CIRRUS for years to come.
On Mi, 2015-04-01 at 22:55 +0800, John Hunter wrote: > Hi Gerd, > I've read the patches about the virtio-gpu, it's a nice design. > As far as I know, there are two other drivers used by qemu, CIRRUS and > BOCHS. > I have a question about the relationship of these three drivers, is > that the virtio-gpu > designed to replace the other two drivers? I mean are the CIRRUS and > BOCHS > going to be deprecated in the future? qemu has a bunch of different virtual graphics cards, and these are the drivers for them. cirrus used to be the default gfx card until recently (qemu older then version 2.2). stdvga (bochs driver) is the current default. So expect them to be around for a while. virtio-gpu will not replace them. > Actually, this is a problem by Martin Peres who is the GSoC xorg > administor. > My proposal is "Convert the BOCHS and CIRRUS drivers to atomic > mode-setting". Surely makes sense for bochs and you shouldn't find major blockers. Not sure this is a reasonable task size for gsoc given it took me only a few days to convert virtio-gpu to atomic modesetting. But maybe fine if you are new to drm kernel hacking and therefore the task includes learning alot new stuff. I have my doubts it'll work out for cirrus though, due to the small amount of video memory it has (and other limitations, because we mimic hardware from the 90ies here). Current code is already swapping framebuffers in and out of video ram because of that. So atomic modesetting, page flip, running wayland on that beast all is going to be problematic I expect. See also: https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/ HTH, Gerd
On Wed, 2015-04-01 at 16:47 +0300, Jani Nikula wrote: > I think part of the problem is that "select" is often used not as > documented [1] but rather as "show my config in menuconfig for > convenience even if my dependency is not met, and select the dependency > even though I know it can screw up the dependency chain". Perhaps people use select because it offers, given the problem they face, a reasonable way to make the kconfig tools generate a sensible .config. It helps them to spend less time fiddling with Kconfig files. And they expect that it helps others to configure their build more easily, as it might save those others some work. > In the big picture, it feels like menuconfig needs a way to display > items whose dependencies are not met, and a way to recursively enable > said items and all their dependencies when told. How could that work its way through (multiple levels of) things like: depends on FOO || (BAZ && BAR) > This would reduce the > resistance to sticking with "select" when clearly "depends" is what's > meant. I had drafted a rather verbose response to this. But I think I'm not really sure what you're saying here, probably because "select" and "depends on" are rather different. How would you know that the actual intention was to use "depends on"? Paul Bolle
Em Mon, 06 Apr 2015 11:50:21 +0200 Paul Bolle <pebolle@tiscali.nl> escreveu: > On Wed, 2015-04-01 at 16:47 +0300, Jani Nikula wrote: > > I think part of the problem is that "select" is often used not as > > documented [1] but rather as "show my config in menuconfig for > > convenience even if my dependency is not met, and select the dependency > > even though I know it can screw up the dependency chain". > > Perhaps people use select because it offers, given the problem they > face, a reasonable way to make the kconfig tools generate a > sensible .config. It helps them to spend less time fiddling with Kconfig > files. And they expect that it helps others to configure their build > more easily, as it might save those others some work. At least on media, the main usage of select is to handle complex hardware where lots of drivers are needed in order to have support for all boards supported by a given board. On a practical example, let's take a look at em28xx driver. This is a bridge driver for media boards. It supports ~100 different boards. Each board may have different tuners, different analog TV demods, different digital TV demods, eeproms attached into one of its I2C buses. As this is an USB stick, the normal user will never know what are the other components of the board without some hard work (by opening the USB stick by seeking for the information at the Internet until he discovers all the components). Even after knowing the hardware components, he has to figure out what drivers implement support for each component. So, for the normal user, we offer a way for the user to select all possible combinations via MEDIA_SUBDRV_AUTOSELECT. This way, he'll know for sure that all boards supported by em28xx will be available. For those that would embedded em28xx on some hardware (like a TV PVR box), he can disable such option and manually select the specific components his hardware uses. For obvious reasons, we recommend distros to always enable MEDIA_SUBDRV_AUTOSELECT. So, yes, select saves a lot of work to configure the build what they want on an easy and sane way. That's said, while it would be possible to convert select into depends on, the result would be really ugly, and very hard to be maintained, as I2C drivers that don't actually depend on the bridge drivers would need a fake depends on list: config DVB_MB86A20S tristate "Fujitsu mb86a20s" # real dependency chain depends on DVB_CORE && I2C # fake dependency chain depends on (!MEDIA_SUBDRV_AUTOSELECT || CONFIG_VIDEO_CX23885_DVB || CONFIG_VIDEO_CX231XX_DVB || CONFIG_VIDEO_EM28xx_DVB) I got here an easy example of an ISDB-T driver, with is not used on many boards. There are other I2C drivers that are used by almost all media drivers, with would result on a very complex fake dependency chain. The worse thing with such ugly fake dependency chain is that people adding a new driver (or support for a new board) would need to remember to dig into drivers that weren't touched, adding new stuff on their Kconfig. > > In the big picture, it feels like menuconfig needs a way to display > > items whose dependencies are not met, and a way to recursively enable > > said items and all their dependencies when told. I believe so. > How could that work its way through (multiple levels of) things like: > depends on FOO || (BAZ && BAR) At least for our typical use case, the dependency chain should not have things like the above, as we use this mainly for I2C drivers, and the bridge drivers also depend on I2C. I would say that, if Kconfig adds a recursive select implementation, such implementation should: - stop recursion if the depends condition was met; - if the depends condition is unmet and has two or more possible options to satisfy (like on your above example), it should prompt the user about what option it would want. If it can't do it (for example, for a "silent" type of config, where it shouldn't be expected any userspace interaction), it should print a warning and not enable the driver. Granted, this is easier said than done. We have this problem mapped for a long time, but none was brave enough (or had enough time) to actually try to implement something like that. So, what we generally do, as a workaround, is to try to simplify the Kconfig stuff to avoid complex use cases. We do that by adding fake dependencies to drivers that might require another driver for some board to work. For example, several USB media drivers now depends on I2C_MUX just because a few of the possible drivers it may need should select I2C_MUX. Btw, I2C_MUX is an optional feature of the I2C core. There's no way for a normal user to know if a driver would need such feature or not, as it basically depends on how the device was internally wired and if the Kernel driver would use it (older drivers have their own solution without using the I2C_MUX new way). There are a few other such options at the I2C sub-system. All of them depends on I2C only (with is a mandatory option for any I2C driver), so select works fine for such features. > > > This would reduce the > > resistance to sticking with "select" when clearly "depends" is what's > > meant. > > I had drafted a rather verbose response to this. But I think I'm not > really sure what you're saying here, probably because "select" and > "depends on" are rather different. How would you know that the actual > intention was to use "depends on"? > > > Paul Bolle Regards, Mauro
Em Wed, 1 Apr 2015 15:15:27 +0200 Gerd Hoffmann <kraxel@redhat.com> escreveu: > After adding virtio-gpu I get this funky kconfig dependency loop. > > scripts/kconfig/conf --oldconfig Kconfig > drivers/video/fbdev/Kconfig:5:error: recursive dependency detected! > drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER > drivers/gpu/drm/Kconfig:34: symbol DRM_KMS_FB_HELPER is selected by DRM_VIRTIO_GPU > drivers/gpu/drm/virtio/Kconfig:1: symbol DRM_VIRTIO_GPU depends on VIRTIO > drivers/virtio/Kconfig:1: symbol VIRTIO is selected by REMOTEPROC > drivers/remoteproc/Kconfig:4: symbol REMOTEPROC is selected by OMAP_REMOTEPROC > drivers/remoteproc/Kconfig:12: symbol OMAP_REMOTEPROC depends on OMAP_IOMMU > drivers/iommu/Kconfig:141: symbol OMAP_IOMMU is selected by VIDEO_OMAP3 > drivers/media/platform/Kconfig:96: symbol VIDEO_OMAP3 depends on VIDEO_V4L2 > drivers/media/v4l2-core/Kconfig:6: symbol VIDEO_V4L2 depends on I2C > drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC > drivers/video/fbdev/Kconfig:59: symbol FB_DDC is selected by FB_CYBER2000_DDC > drivers/video/fbdev/Kconfig:374: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 > drivers/video/fbdev/Kconfig:362: symbol FB_CYBER2000 depends on FB > > Making VIDEO_OMAP3 depend on OMAP_IOMMU instead of selecting it breaks the > loop, which looks like the best way to handle it to me. I'm open to better > suggestions though. Gerd, It seems that I never actually acked or nacked about this patch. I'm ok with such change. So: Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> Btw, it would make sense to add some help for config OMAP_IOMMU and say that this is required in order to compile the OMAP3 media platform drivers at drivers/iommu/Kconfig. Regards, Mauro > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > drivers/media/platform/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index d9b872b..fc21734 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -87,8 +87,8 @@ config VIDEO_OMAP3 > tristate "OMAP 3 Camera support" > depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 > depends on HAS_DMA > + depends on OMAP_IOMMU > select ARM_DMA_USE_IOMMU > - select OMAP_IOMMU > select VIDEOBUF2_DMA_CONTIG > ---help--- > Driver for an OMAP 3 camera controller.
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index d9b872b..fc21734 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -87,8 +87,8 @@ config VIDEO_OMAP3 tristate "OMAP 3 Camera support" depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 depends on HAS_DMA + depends on OMAP_IOMMU select ARM_DMA_USE_IOMMU - select OMAP_IOMMU select VIDEOBUF2_DMA_CONTIG ---help--- Driver for an OMAP 3 camera controller.
After adding virtio-gpu I get this funky kconfig dependency loop. scripts/kconfig/conf --oldconfig Kconfig drivers/video/fbdev/Kconfig:5:error: recursive dependency detected! drivers/video/fbdev/Kconfig:5: symbol FB is selected by DRM_KMS_FB_HELPER drivers/gpu/drm/Kconfig:34: symbol DRM_KMS_FB_HELPER is selected by DRM_VIRTIO_GPU drivers/gpu/drm/virtio/Kconfig:1: symbol DRM_VIRTIO_GPU depends on VIRTIO drivers/virtio/Kconfig:1: symbol VIRTIO is selected by REMOTEPROC drivers/remoteproc/Kconfig:4: symbol REMOTEPROC is selected by OMAP_REMOTEPROC drivers/remoteproc/Kconfig:12: symbol OMAP_REMOTEPROC depends on OMAP_IOMMU drivers/iommu/Kconfig:141: symbol OMAP_IOMMU is selected by VIDEO_OMAP3 drivers/media/platform/Kconfig:96: symbol VIDEO_OMAP3 depends on VIDEO_V4L2 drivers/media/v4l2-core/Kconfig:6: symbol VIDEO_V4L2 depends on I2C drivers/i2c/Kconfig:7: symbol I2C is selected by FB_DDC drivers/video/fbdev/Kconfig:59: symbol FB_DDC is selected by FB_CYBER2000_DDC drivers/video/fbdev/Kconfig:374: symbol FB_CYBER2000_DDC depends on FB_CYBER2000 drivers/video/fbdev/Kconfig:362: symbol FB_CYBER2000 depends on FB Making VIDEO_OMAP3 depend on OMAP_IOMMU instead of selecting it breaks the loop, which looks like the best way to handle it to me. I'm open to better suggestions though. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- drivers/media/platform/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)