2018-01-25 15:49:07

by Farhan Ali

[permalink] [raw]
Subject: [PATCH v1 0/2] Kconfig changes to enable Graphics Support for S390

Hi,

This series of patches are in preparation for enabling an additional
tty and console for a S390 KVM guest using a virtio-gpu device[1].
One of the steps to do this would be to enable CONFIG_VT for S390,
and this would also require the dummy console (CONFIG_DUMMY_CONSOLE).

Patch 1 enables the "Graphics support" menu which is
needed to enable dummy console, since the VT layer needs it.

Patch 2 fixes a Kconfig dependency issue for opencores
framebuffer devices. This issue was exposed by the previous
patch.

Thanks
Farhan


[1] https://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg04184.html

Farhan Ali (2):
Kconfig : Remove HAS_IOMEM dependency for Graphics support
fbdev: Kconfig: Add HAS_IOMEM dependency for FB_OPENCORES

drivers/video/Kconfig | 1 -
drivers/video/fbdev/Kconfig | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)

--
2.7.4



2018-01-25 15:49:13

by Farhan Ali

[permalink] [raw]
Subject: [PATCH v1 2/2] fbdev: Kconfig: Add HAS_IOMEM dependency for FB_OPENCORES

The Opencores framebuffer device uses I/O memory and with
CONFIG_HAS_IOMEM disabled will lead to build errors:

ERROR: "devm_ioremap_resource" [drivers/video/fbdev/ocfb.ko] undefined!

Fix this by adding HAS_IOMEM dependency for FB_OPENCORES.

Signed-off-by: Farhan Ali <[email protected]>
---
drivers/video/fbdev/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 5e58f5e..8667e5d 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -976,7 +976,7 @@ config FB_PVR2

config FB_OPENCORES
tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
- depends on FB && HAS_DMA
+ depends on FB && HAS_DMA && HAS_IOMEM
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
--
2.7.4


2018-01-25 15:49:17

by Farhan Ali

[permalink] [raw]
Subject: [PATCH v1 1/2] Kconfig : Remove HAS_IOMEM dependency for Graphics support

The 'commit e25df1205f37 ("[S390] Kconfig: menus with depends on HAS_IOMEM.")'
added the HAS_IOMEM dependecy for "Graphics support". This disabled the
"Graphics support" menu for S390. But if we enable VT layer for S390,
we would also need to enable the dummy console. So let's remove the
HAS_IOMEM dependency.

Signed-off-by: Farhan Ali <[email protected]>
Tested-by: Dong Jia Shi <[email protected]>
---
drivers/video/Kconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 3c20af9..41e7ba9 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -3,7 +3,6 @@
#

menu "Graphics support"
- depends on HAS_IOMEM

config HAVE_FB_ATMEL
bool
--
2.7.4


2018-01-26 12:32:11

by Thomas Huth

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] Kconfig : Remove HAS_IOMEM dependency for Graphics support

On 25.01.2018 16:47, Farhan Ali wrote:
> The 'commit e25df1205f37 ("[S390] Kconfig: menus with depends on HAS_IOMEM.")'
> added the HAS_IOMEM dependecy for "Graphics support". This disabled the
> "Graphics support" menu for S390. But if we enable VT layer for S390,
> we would also need to enable the dummy console. So let's remove the
> HAS_IOMEM dependency.
>
> Signed-off-by: Farhan Ali <[email protected]>
> Tested-by: Dong Jia Shi <[email protected]>
> ---
> drivers/video/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 3c20af9..41e7ba9 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -3,7 +3,6 @@
> #
>
> menu "Graphics support"
> - depends on HAS_IOMEM

Generally a good idea, but should the removed line maybe rather be added
to the menu "Frame buffer Devices" now instead?

Thomas

2018-01-26 12:35:12

by Thomas Huth

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] fbdev: Kconfig: Add HAS_IOMEM dependency for FB_OPENCORES

On 25.01.2018 16:47, Farhan Ali wrote:
> The Opencores framebuffer device uses I/O memory and with
> CONFIG_HAS_IOMEM disabled will lead to build errors:
>
> ERROR: "devm_ioremap_resource" [drivers/video/fbdev/ocfb.ko] undefined!
>
> Fix this by adding HAS_IOMEM dependency for FB_OPENCORES.
>
> Signed-off-by: Farhan Ali <[email protected]>
> ---
> drivers/video/fbdev/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index 5e58f5e..8667e5d 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -976,7 +976,7 @@ config FB_PVR2
>
> config FB_OPENCORES
> tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
> - depends on FB && HAS_DMA
> + depends on FB && HAS_DMA && HAS_IOMEM
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT

I think it would be better if fbdevs in general would depend on
HAS_IOMEM ... or could there be any frame buffer devices without IOMEM ?

Thomas

2018-01-26 12:40:56

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] fbdev: Kconfig: Add HAS_IOMEM dependency for FB_OPENCORES

On 26/01/18 14:33, Thomas Huth wrote:
> On 25.01.2018 16:47, Farhan Ali wrote:
>> The Opencores framebuffer device uses I/O memory and with
>> CONFIG_HAS_IOMEM disabled will lead to build errors:
>>
>> ERROR: "devm_ioremap_resource" [drivers/video/fbdev/ocfb.ko] undefined!
>>
>> Fix this by adding HAS_IOMEM dependency for FB_OPENCORES.
>>
>> Signed-off-by: Farhan Ali <[email protected]>
>> ---
>> drivers/video/fbdev/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>> index 5e58f5e..8667e5d 100644
>> --- a/drivers/video/fbdev/Kconfig
>> +++ b/drivers/video/fbdev/Kconfig
>> @@ -976,7 +976,7 @@ config FB_PVR2
>>
>> config FB_OPENCORES
>> tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
>> - depends on FB && HAS_DMA
>> + depends on FB && HAS_DMA && HAS_IOMEM
>> select FB_CFB_FILLRECT
>> select FB_CFB_COPYAREA
>> select FB_CFB_IMAGEBLIT
>
> I think it would be better if fbdevs in general would depend on
> HAS_IOMEM ... or could there be any frame buffer devices without IOMEM ?

There are some small ones which are updated with, say, i2c
(ssd1307fb.c). I think those don't need iomem.

Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2018-01-26 13:41:51

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Kconfig changes to enable Graphics Support for S390

Hi Farhan,

On Thu, Jan 25, 2018 at 4:47 PM, Farhan Ali <[email protected]> wrote:
> This series of patches are in preparation for enabling an additional
> tty and console for a S390 KVM guest using a virtio-gpu device[1].
> One of the steps to do this would be to enable CONFIG_VT for S390,
> and this would also require the dummy console (CONFIG_DUMMY_CONSOLE).
>
> Patch 1 enables the "Graphics support" menu which is
> needed to enable dummy console, since the VT layer needs it.
>
> Patch 2 fixes a Kconfig dependency issue for opencores
> framebuffer devices. This issue was exposed by the previous
> patch.
>
> Thanks
> Farhan
>
>
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg04184.html
>
> Farhan Ali (2):
> Kconfig : Remove HAS_IOMEM dependency for Graphics support
> fbdev: Kconfig: Add HAS_IOMEM dependency for FB_OPENCORES
>
> drivers/video/Kconfig | 1 -
> drivers/video/fbdev/Kconfig | 2 +-
> 2 files changed, 1 insertion(+), 2 deletions(-)

Shouldn't the order of your two patches be inverted, to avoid patch 1
introducing
build breakage fixed by patch 2?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2018-01-26 14:34:14

by Farhan Ali

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] fbdev: Kconfig: Add HAS_IOMEM dependency for FB_OPENCORES



On 01/26/2018 07:38 AM, Tomi Valkeinen wrote:
> On 26/01/18 14:33, Thomas Huth wrote:
>> On 25.01.2018 16:47, Farhan Ali wrote:
>>> The Opencores framebuffer device uses I/O memory and with
>>> CONFIG_HAS_IOMEM disabled will lead to build errors:
>>>
>>> ERROR: "devm_ioremap_resource" [drivers/video/fbdev/ocfb.ko] undefined!
>>>
>>> Fix this by adding HAS_IOMEM dependency for FB_OPENCORES.
>>>
>>> Signed-off-by: Farhan Ali <[email protected]>
>>> ---
>>> drivers/video/fbdev/Kconfig | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>>> index 5e58f5e..8667e5d 100644
>>> --- a/drivers/video/fbdev/Kconfig
>>> +++ b/drivers/video/fbdev/Kconfig
>>> @@ -976,7 +976,7 @@ config FB_PVR2
>>>
>>> config FB_OPENCORES
>>> tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
>>> - depends on FB && HAS_DMA
>>> + depends on FB && HAS_DMA && HAS_IOMEM
>>> select FB_CFB_FILLRECT
>>> select FB_CFB_COPYAREA
>>> select FB_CFB_IMAGEBLIT
>>
>> I think it would be better if fbdevs in general would depend on
>> HAS_IOMEM ... or could there be any frame buffer devices without IOMEM ?
>
> There are some small ones which are updated with, say, i2c
> (ssd1307fb.c). I think those don't need iomem.
>
> Tomi
>

Most of the other framebuffer devices are fenced of by architecture
dependency or PCI dependency. So I am hesitant to introduce a blanket
dependency for all fbdevs.

Thank you guys for reviewing!

Thanks
Farhan


2018-01-26 14:36:28

by Farhan Ali

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Kconfig changes to enable Graphics Support for S390



On 01/26/2018 08:41 AM, Geert Uytterhoeven wrote:
> Hi Farhan,
>
> On Thu, Jan 25, 2018 at 4:47 PM, Farhan Ali <[email protected]> wrote:
>> This series of patches are in preparation for enabling an additional
>> tty and console for a S390 KVM guest using a virtio-gpu device[1].
>> One of the steps to do this would be to enable CONFIG_VT for S390,
>> and this would also require the dummy console (CONFIG_DUMMY_CONSOLE).
>>
>> Patch 1 enables the "Graphics support" menu which is
>> needed to enable dummy console, since the VT layer needs it.
>>
>> Patch 2 fixes a Kconfig dependency issue for opencores
>> framebuffer devices. This issue was exposed by the previous
>> patch.
>>
>> Thanks
>> Farhan
>>
>>
>> [1] https://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg04184.html
>>
>> Farhan Ali (2):
>> Kconfig : Remove HAS_IOMEM dependency for Graphics support
>> fbdev: Kconfig: Add HAS_IOMEM dependency for FB_OPENCORES
>>
>> drivers/video/Kconfig | 1 -
>> drivers/video/fbdev/Kconfig | 2 +-
>> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> Shouldn't the order of your two patches be inverted, to avoid patch 1
> introducing
> build breakage fixed by patch 2?
>
> Gr{oetje,eeting}s,
>
> Geert
>

Hi Geert,

I wasn't sure what would be the best ordering since we would never hit
the issue if patch 1 didn't exist. But if the preference is to invert
the ordering of patches, then I will change the ordering.

Thank you for reviewing.

Thanks
Farhan


> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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
>


2018-01-26 15:07:59

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Kconfig changes to enable Graphics Support for S390

Hi Farhan,

On Fri, Jan 26, 2018 at 3:35 PM, Farhan Ali <[email protected]> wrote:
> On 01/26/2018 08:41 AM, Geert Uytterhoeven wrote:
>> On Thu, Jan 25, 2018 at 4:47 PM, Farhan Ali <[email protected]>
>> wrote:
>>> This series of patches are in preparation for enabling an additional
>>> tty and console for a S390 KVM guest using a virtio-gpu device[1].
>>> One of the steps to do this would be to enable CONFIG_VT for S390,
>>> and this would also require the dummy console (CONFIG_DUMMY_CONSOLE).
>>>
>>> Patch 1 enables the "Graphics support" menu which is
>>> needed to enable dummy console, since the VT layer needs it.
>>>
>>> Patch 2 fixes a Kconfig dependency issue for opencores
>>> framebuffer devices. This issue was exposed by the previous
>>> patch.
>>>
>>> Thanks
>>> Farhan
>>>
>>>
>>> [1]
>>> https://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg04184.html
>>>
>>> Farhan Ali (2):
>>> Kconfig : Remove HAS_IOMEM dependency for Graphics support
>>> fbdev: Kconfig: Add HAS_IOMEM dependency for FB_OPENCORES
>>>
>>> drivers/video/Kconfig | 1 -
>>> drivers/video/fbdev/Kconfig | 2 +-
>>> 2 files changed, 1 insertion(+), 2 deletions(-)
>>
>>
>> Shouldn't the order of your two patches be inverted, to avoid patch 1
>> introducing
>> build breakage fixed by patch 2?
>>
>> Gr{oetje,eeting}s,
>>
>> Geert
>>
>
> Hi Geert,
>
> I wasn't sure what would be the best ordering since we would never hit the
> issue if patch 1 didn't exist. But if the preference is to invert the
> ordering of patches, then I will change the ordering.

Alternatively, you can combine two patches into a single patch, which
moves the dependency from the whole subsystem to the driver that needs
it (are there more?).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2018-01-26 16:28:36

by Farhan Ali

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Kconfig changes to enable Graphics Support for S390



On 01/26/2018 10:06 AM, Geert Uytterhoeven wrote:
>> Hi Geert,
>>
>> I wasn't sure what would be the best ordering since we would never hit the
>> issue if patch 1 didn't exist. But if the preference is to invert the
>> ordering of patches, then I will change the ordering.
> Alternatively, you can combine two patches into a single patch, which
> moves the dependency from the whole subsystem to the driver that needs
> it (are there more?).
>
> Gr{oetje,eeting}s,
>
> Geert


I like the idea of combining both patches into one.

There are other fbdev drivers that use iomem (found by grepping for
"devm_ioremap_resource"):

CONFIG_FB_S3C (s3c-fb.c)

CONFIG_FB_CLPS711X (clps711x-fb.c)

CONFIG_FB_JZ4740 (jz4740_fb.c)

CONFIG_FB_DA8XX (da8xx-fb.c)

CONFIG_FB_WM8505 (wm8505fb.c)

CONFIG_OMAP2_VRFB (omap2/omapfb/vrfb.c)

CONFIG_FB_OMAP2 (omap2/omapfb/dss/*

CONFIG_FB_MXS (mxsfb.c)

CONFIG_PXA3XX_GCU (pxa3xx-gcu.c)

CONFIG_FB_XILINX (xilinxfb.c)


All of these are already fenced off by architecture dependencies (which
I am assuming enables CONFIG_HAS_IOMEM by default). If we want to be
cautious I can add HAS_IOMEM dependency for all of them.

Thanks
Farhan


2018-01-30 13:23:59

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] Kconfig changes to enable Graphics Support for S390

Yes, merging sounds good.
Farhan, can you maybe resend t whole series together with your CONFIG_VT rework?



On 01/26/2018 05:26 PM, Farhan Ali wrote:
>
>
> On 01/26/2018 10:06 AM, Geert Uytterhoeven wrote:
>>> Hi Geert,
>>>
>>> I wasn't sure what would be the best ordering since we would never hit the
>>> issue if patch 1 didn't exist. But if the preference is to invert the
>>> ordering of patches, then I will change the ordering.
>> Alternatively, you can combine two patches into a single patch, which
>> moves the dependency from the whole subsystem to the driver that needs
>> it (are there more?).
>>
>> Gr{oetje,eeting}s,
>>
>>                          Geert
>
>
> I like the idea of combining both patches into one.
>
> There are other fbdev drivers that use iomem (found by grepping for "devm_ioremap_resource"):
>
> CONFIG_FB_S3C (s3c-fb.c)
>
> CONFIG_FB_CLPS711X (clps711x-fb.c)
>
> CONFIG_FB_JZ4740 (jz4740_fb.c)
>
> CONFIG_FB_DA8XX (da8xx-fb.c)
>
> CONFIG_FB_WM8505 (wm8505fb.c)
>
> CONFIG_OMAP2_VRFB (omap2/omapfb/vrfb.c)
>
> CONFIG_FB_OMAP2 (omap2/omapfb/dss/*
>
> CONFIG_FB_MXS (mxsfb.c)
>
> CONFIG_PXA3XX_GCU (pxa3xx-gcu.c)
>
> CONFIG_FB_XILINX (xilinxfb.c)
>
>
> All of these are already fenced off by architecture dependencies (which I am assuming enables CONFIG_HAS_IOMEM by default). If we want to be cautious I can add HAS_IOMEM dependency for all of them.
>
> Thanks
> Farhan