[PATCH v2 1/4] break kconfig dependency loop

[Mauro Carvalho Chehab]

Em Wed,  1 Apr 2015 15:15:27 +0200
Gerd Hoffmann  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 

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 
> ---
>  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.

[PATCH v2 1/4] break kconfig dependency loop

[Paul Bolle]

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

[PATCH v2 1/4] break kconfig dependency loop

[Mauro Carvalho Chehab]

Em Mon, 06 Apr 2015 11:50:21 +0200
Paul Bolle  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 

[PATCH v2 1/4] break kconfig dependency loop

[John Hunter]

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  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 
> ---
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>



-- 
Best regards
Junwang Zhao
Microprocessor Research and Develop Center
Department of Computer Science 
Peking University
Beijing, 100871, PRC
-- next part --
An HTML attachment was scrubbed...
URL: 

[PATCH v2 1/4] break kconfig dependency loop

[Gerd Hoffmann]

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

[PATCH v2 1/4] break kconfig dependency loop

[Michael S. Tsirkin]

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.

-- 
MST

[PATCH v2 1/4] break kconfig dependency loop

[Jani Nikula]

On Wed, 01 Apr 2015, Gerd Hoffmann  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 
> ---
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH v2 1/4] break kconfig dependency loop

[Gerd Hoffmann]

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 
---
 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