2011-06-20 10:46:35

by Tushar Behera

[permalink] [raw]
Subject: [PATCH 0/2] video: Make omap2 support conditional

Currently drivers/video/omap2 is built by default with every configuration
of the kernel. Current patchset makes the omap2 video support conditional
on appropriate config option.

[Patch 2/2] was first generated, but that resulted in a build error.
[Patch 1/2] was created to fix that build error.

These patches have only been tested for build consistency and have not been
tested on any related hardware.

Tushar Behera (2):
config: omap2+: force fb and dss support as built-in
video: omap2: Compile omap2 support only when needed

arch/arm/configs/omap2plus_defconfig | 4 ++--
drivers/video/Makefile | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

--
1.7.4.1


2011-06-20 10:46:41

by Tushar Behera

[permalink] [raw]
Subject: [PATCH 1/2] config: omap2+: force fb and dss support as built-in

In certain board files, there are references to vram related functions
which are defined in drivers/video/omap2/vram.c. Because of this direct
dependency, CONFIG_FB_OMAP2 should be a built-in feature.

As per the current architecture, CONFIG_FB_OMAP2 is dependent on
CONFIG_OMAP2_DSS. Hence CONFIG_OMAP2_DSS support should also be selected
by default.

Cc: Tony Lindgren <[email protected]>
Cc: Samreen <[email protected]>
Signed-off-by: Tushar Behera <[email protected]>
---
arch/arm/mach-omap2/built-in.o: In function `rx51_video_mem_init':
linux-linaro-2.6.39/arch/arm/mach-omap2/board-rx51-video.c:97: undefined reference to `omap_vram_set_sdram_vram'
arch/arm/plat-omap/built-in.o: In function `omap_reserve':
linux-linaro-2.6.39/arch/arm/plat-omap/common.c:66: undefined reference to `omap_vram_reserve_sdram_memblock'
arch/arm/plat-omap/built-in.o: In function `omap_detect_sram':
linux-linaro-2.6.39/arch/arm/plat-omap/sram.c:179: undefined reference to `omap_vram_reserve_sram'
make[1]: *** [.tmp_vmlinux1] Error 1
make: *** [sub-make] Error 2

arch/arm/configs/omap2plus_defconfig | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index d5f00d7..c3ffff2 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -138,11 +138,11 @@ CONFIG_FIRMWARE_EDID=y
CONFIG_FB_MODE_HELPERS=y
CONFIG_FB_TILEBLITTING=y
CONFIG_FB_OMAP_LCD_VGA=y
-CONFIG_OMAP2_DSS=m
+CONFIG_OMAP2_DSS=y
CONFIG_OMAP2_DSS_RFBI=y
CONFIG_OMAP2_DSS_SDI=y
CONFIG_OMAP2_DSS_DSI=y
-CONFIG_FB_OMAP2=m
+CONFIG_FB_OMAP2=y
CONFIG_PANEL_GENERIC_DPI=m
CONFIG_PANEL_SHARP_LS037V7DW01=m
CONFIG_PANEL_NEC_NL8048HL11_01B=m
--
1.7.4.1

2011-06-20 10:46:47

by Tushar Behera

[permalink] [raw]
Subject: [PATCH 2/2] video: omap2: Compile omap2 support only when needed

Currently display support for omap2 is selected by default and
it gets built for all the configurations.

Instead of it being a built-in feature, it's compilation should
depend on the config option CONFIG_FB_OMAP2.

Cc: Paul Mundt <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
Cc: Tony Lindgren <[email protected]>
Signed-off-by: Tushar Behera <[email protected]>
---
drivers/video/Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 8b83129..a19e44e 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -133,7 +133,7 @@ obj-$(CONFIG_FB_SH_MOBILE_HDMI) += sh_mobile_hdmi.o
obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o
obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o
obj-$(CONFIG_FB_OMAP) += omap/
-obj-y += omap2/
+obj-$(CONFIG_FB_OMAP2) += omap2/
obj-$(CONFIG_XEN_FBDEV_FRONTEND) += xen-fbfront.o
obj-$(CONFIG_FB_CARMINE) += carminefb.o
obj-$(CONFIG_FB_MB862XX) += mb862xx/
--
1.7.4.1

2011-06-20 12:07:34

by Premi, Sanjeev

[permalink] [raw]
Subject: RE: [PATCH 1/2] config: omap2+: force fb and dss support as built-in

> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of Tushar Behera
> Sent: Monday, June 20, 2011 4:16 PM
> To: [email protected];
> [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Nilofer, Samreen
> Subject: [PATCH 1/2] config: omap2+: force fb and dss support
> as built-in
>
> In certain board files, there are references to vram related functions
> which are defined in drivers/video/omap2/vram.c. Because of
> this direct
> dependency, CONFIG_FB_OMAP2 should be a built-in feature.
>
> As per the current architecture, CONFIG_FB_OMAP2 is dependent on
> CONFIG_OMAP2_DSS. Hence CONFIG_OMAP2_DSS support should also
> be selected
> by default.
>
> Cc: Tony Lindgren <[email protected]>
> Cc: Samreen <[email protected]>
> Signed-off-by: Tushar Behera <[email protected]>
> ---
> arch/arm/mach-omap2/built-in.o: In function `rx51_video_mem_init':
> linux-linaro-2.6.39/arch/arm/mach-omap2/board-rx51-video.c:97:
> undefined reference to `omap_vram_set_sdram_vram'
> arch/arm/plat-omap/built-in.o: In function `omap_reserve':
> linux-linaro-2.6.39/arch/arm/plat-omap/common.c:66: undefined
> reference to `omap_vram_reserve_sdram_memblock'
> arch/arm/plat-omap/built-in.o: In function `omap_detect_sram':
> linux-linaro-2.6.39/arch/arm/plat-omap/sram.c:179: undefined
> reference to `omap_vram_reserve_sram'
> make[1]: *** [.tmp_vmlinux1] Error 1
> make: *** [sub-make] Error 2
>
> arch/arm/configs/omap2plus_defconfig | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/configs/omap2plus_defconfig
> b/arch/arm/configs/omap2plus_defconfig
> index d5f00d7..c3ffff2 100644
> --- a/arch/arm/configs/omap2plus_defconfig
> +++ b/arch/arm/configs/omap2plus_defconfig
> @@ -138,11 +138,11 @@ CONFIG_FIRMWARE_EDID=y
> CONFIG_FB_MODE_HELPERS=y
> CONFIG_FB_TILEBLITTING=y
> CONFIG_FB_OMAP_LCD_VGA=y
> -CONFIG_OMAP2_DSS=m
> +CONFIG_OMAP2_DSS=y
> CONFIG_OMAP2_DSS_RFBI=y
> CONFIG_OMAP2_DSS_SDI=y
> CONFIG_OMAP2_DSS_DSI=y
> -CONFIG_FB_OMAP2=m
> +CONFIG_FB_OMAP2=y
> CONFIG_PANEL_GENERIC_DPI=m
> CONFIG_PANEL_SHARP_LS037V7DW01=m
> CONFIG_PANEL_NEC_NL8048HL11_01B=m


[sp] Instead of changing the omap2plus_defconfig, shouldn't the
board specific file be fixed instead?

~sanjeev

> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-omap" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> -

2011-06-20 12:20:50

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 1/2] config: omap2+: force fb and dss support as built-in

On Mon, Jun 20, 2011 at 05:37:07PM +0530, Premi, Sanjeev wrote:
> [sp] Instead of changing the omap2plus_defconfig, shouldn't the
> board specific file be fixed instead?

The board specific configuration files in the mainline kernel are
deprecated and are gradually being removed.

2011-06-20 12:24:59

by Premi, Sanjeev

[permalink] [raw]
Subject: RE: [PATCH 1/2] config: omap2+: force fb and dss support as built-in

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:[email protected]]
> Sent: Monday, June 20, 2011 5:51 PM
> To: Premi, Sanjeev
> Cc: Tushar Behera; [email protected];
> [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Nilofer, Samreen
> Subject: Re: [PATCH 1/2] config: omap2+: force fb and dss
> support as built-in
>
> On Mon, Jun 20, 2011 at 05:37:07PM +0530, Premi, Sanjeev wrote:
> > [sp] Instead of changing the omap2plus_defconfig, shouldn't the
> > board specific file be fixed instead?
>
> The board specific configuration files in the mainline kernel are
> deprecated and are gradually being removed.
>
[sp] I didn't mean board specific config file, but:
linux-linaro-2.6.39/arch/arm/mach-omap2/board-rx51-video.c

~sanjeev-

2011-06-20 13:04:54

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH 1/2] config: omap2+: force fb and dss support as built-in

On Mon, 2011-06-20 at 16:16 +0530, Tushar Behera wrote:
> In certain board files, there are references to vram related functions
> which are defined in drivers/video/omap2/vram.c. Because of this direct
> dependency, CONFIG_FB_OMAP2 should be a built-in feature.

arch/arm/plat-omap/include/plat/vram.h defines dummy inline function in
case vram.c is not compiled in, so the board files should compile fine.

> As per the current architecture, CONFIG_FB_OMAP2 is dependent on
> CONFIG_OMAP2_DSS. Hence CONFIG_OMAP2_DSS support should also be selected
> by default.

The configuration is fine as it is. And anyway, if things do not compile
when something is configured as a module, the correct fix is hardly just
changing the feature to be compiled built-in =).

> Cc: Tony Lindgren <[email protected]>
> Cc: Samreen <[email protected]>
> Signed-off-by: Tushar Behera <[email protected]>
> ---
> arch/arm/mach-omap2/built-in.o: In function `rx51_video_mem_init':
> linux-linaro-2.6.39/arch/arm/mach-omap2/board-rx51-video.c:97: undefined reference to `omap_vram_set_sdram_vram'
> arch/arm/plat-omap/built-in.o: In function `omap_reserve':
> linux-linaro-2.6.39/arch/arm/plat-omap/common.c:66: undefined reference to `omap_vram_reserve_sdram_memblock'
> arch/arm/plat-omap/built-in.o: In function `omap_detect_sram':
> linux-linaro-2.6.39/arch/arm/plat-omap/sram.c:179: undefined reference to `omap_vram_reserve_sram'
> make[1]: *** [.tmp_vmlinux1] Error 1
> make: *** [sub-make] Error 2

Compiles fine for me. Perhaps you are using some old kernel?

Tomi

2011-06-20 13:06:50

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH 2/2] video: omap2: Compile omap2 support only when needed

On Mon, 2011-06-20 at 16:16 +0530, Tushar Behera wrote:
> Currently display support for omap2 is selected by default and
> it gets built for all the configurations.
>
> Instead of it being a built-in feature, it's compilation should
> depend on the config option CONFIG_FB_OMAP2.

No, I don't think so. omap2 directory contains vram, vrfb and omapdss,
all of which can be used without omapfb driver. vram and vrfb can be
even used without omapdss driver.

Is this patch fixing some real problem?

Tomi

2011-06-21 04:49:04

by Tushar Behera

[permalink] [raw]
Subject: Re: [PATCH 2/2] video: omap2: Compile omap2 support only when needed

Hi,

On Monday 20 June 2011 06:36 PM, Tomi Valkeinen wrote:
> On Mon, 2011-06-20 at 16:16 +0530, Tushar Behera wrote:
>> Currently display support for omap2 is selected by default and
>> it gets built for all the configurations.
>>
>> Instead of it being a built-in feature, it's compilation should
>> depend on the config option CONFIG_FB_OMAP2.
>
> No, I don't think so. omap2 directory contains vram, vrfb and omapdss,
> all of which can be used without omapfb driver. vram and vrfb can be
> even used without omapdss driver.
Even if I build the kernel with i386_defconfig, I get some compiled
files within drivers/video/omap2.

$ make ARCH=x86 i386_defconfig O=out_dir
$ make ARCH=x86 O=out_dir

$ ls out_dir/drivers/video/omap2
built-in.o displays modules.builtin modules.order

IMHO, drivers/video/omap2/ should not be compiled if the kernel is not
built for omap2.
>
> Is this patch fixing some real problem?
This patch does not fix any other problem.


>
> Tomi
>
>


--
Tushar Behera

2011-06-21 05:00:27

by Tushar Behera

[permalink] [raw]
Subject: Re: [PATCH 1/2] config: omap2+: force fb and dss support as built-in

On Monday 20 June 2011 06:34 PM, Tomi Valkeinen wrote:
> On Mon, 2011-06-20 at 16:16 +0530, Tushar Behera wrote:
>> In certain board files, there are references to vram related functions
>> which are defined in drivers/video/omap2/vram.c. Because of this direct
>> dependency, CONFIG_FB_OMAP2 should be a built-in feature.
>
> arch/arm/plat-omap/include/plat/vram.h defines dummy inline function in
> case vram.c is not compiled in, so the board files should compile fine.
>
>> As per the current architecture, CONFIG_FB_OMAP2 is dependent on
>> CONFIG_OMAP2_DSS. Hence CONFIG_OMAP2_DSS support should also be selected
>> by default.
>
> The configuration is fine as it is. And anyway, if things do not compile
> when something is configured as a module, the correct fix is hardly just
> changing the feature to be compiled built-in =).
Agreed. :)
I am not an expert in DSS. I just wanted to raise my concern.
>
>> Cc: Tony Lindgren<[email protected]>
>> Cc: Samreen<[email protected]>
>> Signed-off-by: Tushar Behera<[email protected]>
>> ---
>> arch/arm/mach-omap2/built-in.o: In function `rx51_video_mem_init':
>> linux-linaro-2.6.39/arch/arm/mach-omap2/board-rx51-video.c:97: undefined reference to `omap_vram_set_sdram_vram'
>> arch/arm/plat-omap/built-in.o: In function `omap_reserve':
>> linux-linaro-2.6.39/arch/arm/plat-omap/common.c:66: undefined reference to `omap_vram_reserve_sdram_memblock'
>> arch/arm/plat-omap/built-in.o: In function `omap_detect_sram':
>> linux-linaro-2.6.39/arch/arm/plat-omap/sram.c:179: undefined reference to `omap_vram_reserve_sram'
>> make[1]: *** [.tmp_vmlinux1] Error 1
>> make: *** [sub-make] Error 2
>
> Compiles fine for me. Perhaps you are using some old kernel?
As mentioned in the cover letter, we get this error only if 2/2 is
applied and 1/2 is not applied.

I tested this by applying 2/2 on v3.0-rc4 and using omap2plus_defconfig.
>
> Tomi
>
>


--
Tushar Behera

2011-06-21 05:25:00

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH 2/2] video: omap2: Compile omap2 support only when needed

On Tue, 2011-06-21 at 10:18 +0530, Tushar Behera wrote:
> Hi,
>
> On Monday 20 June 2011 06:36 PM, Tomi Valkeinen wrote:
> > On Mon, 2011-06-20 at 16:16 +0530, Tushar Behera wrote:
> >> Currently display support for omap2 is selected by default and
> >> it gets built for all the configurations.
> >>
> >> Instead of it being a built-in feature, it's compilation should
> >> depend on the config option CONFIG_FB_OMAP2.
> >
> > No, I don't think so. omap2 directory contains vram, vrfb and omapdss,
> > all of which can be used without omapfb driver. vram and vrfb can be
> > even used without omapdss driver.
> Even if I build the kernel with i386_defconfig, I get some compiled
> files within drivers/video/omap2.
>
> $ make ARCH=x86 i386_defconfig O=out_dir
> $ make ARCH=x86 O=out_dir
>
> $ ls out_dir/drivers/video/omap2
> built-in.o displays modules.builtin modules.order
>
> IMHO, drivers/video/omap2/ should not be compiled if the kernel is not
> built for omap2.

Ok. Yes, that's a known "problem". We could have a OMAPx check there,
but then again, the driver is a driver for the DSS HW, which could, at
least in theory, used in other SoCs.

I don't think any driver should normally depend on ARCH_something or
MACH_something.

I think this is more of a "problem" with the build system. The same
thing can be seen with, for example, drivers/video/backlight/ and
drivers/video/display/ directories. In practice the created files do not
affect the kernel in any way, as far as I see.

Tomi