[PATCH v1 0/2] Kconfig changes to enable Graphics Support for S390

[PATCH v1 0/2] Kconfig changes to enable Graphics Support for S390

[Farhan Ali]

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

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

[Farhan Ali]

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 <alifm@linux.vnet.ibm.com>
Tested-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
---
 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

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

[Thomas Huth]

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 <alifm@linux.vnet.ibm.com>
> Tested-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> ---
>  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

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

[Farhan Ali]

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 <alifm@linux.vnet.ibm.com>
---
 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

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

[Thomas Huth]

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 <alifm@linux.vnet.ibm.com>
> ---
>  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

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

[Tomi Valkeinen]

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 <alifm@linux.vnet.ibm.com>
>> ---
>>  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

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

[Farhan Ali]

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 <alifm@linux.vnet.ibm.com>
>>> ---
>>>   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

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

[Geert Uytterhoeven]

Hi Farhan,

On Thu, Jan 25, 2018 at 4:47 PM, Farhan Ali <alifm@linux.vnet.ibm.com> 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 -- 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

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

[Farhan Ali]

On 01/26/2018 08:41 AM, Geert Uytterhoeven wrote:
> Hi Farhan,
> 
> On Thu, Jan 25, 2018 at 4:47 PM, Farhan Ali <alifm@linux.vnet.ibm.com> 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 -- 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
> 

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

[Geert Uytterhoeven]

Hi Farhan,

On Fri, Jan 26, 2018 at 3:35 PM, Farhan Ali <alifm@linux.vnet.ibm.com> wrote:
> On 01/26/2018 08:41 AM, Geert Uytterhoeven wrote:
>> On Thu, Jan 25, 2018 at 4:47 PM, Farhan Ali <alifm@linux.vnet.ibm.com>
>> 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 -- 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

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

[Farhan Ali]

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

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

[Christian Borntraeger]

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