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

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

[Tushar Behera]

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

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

[Tushar Behera]

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 <tony@atomide.com>
Cc: Samreen <samreen@ti.com>
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
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

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

[Premi, Sanjeev]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Tushar Behera
> Sent: Monday, June 20, 2011 4:16 PM
> To: linux-kernel@vger.kernel.org; 
> linux-arm-kernel@lists.infradead.org; 
> linux-fbdev@vger.kernel.org; linux-omap@vger.kernel.org
> Cc: linux@arm.linux.org.uk; tony@atomide.com; 
> lethal@linux-sh.org; linaro-dev@lists.linaro.org; 
> patches@linaro.org; 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 <tony@atomide.com>
> Cc: Samreen <samreen@ti.com>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Russell King - ARM Linux]

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.

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

[Premi, Sanjeev]

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] 
> Sent: Monday, June 20, 2011 5:51 PM
> To: Premi, Sanjeev
> Cc: Tushar Behera; linux-kernel@vger.kernel.org; 
> linux-arm-kernel@lists.infradead.org; 
> linux-fbdev@vger.kernel.org; linux-omap@vger.kernel.org; 
> tony@atomide.com; lethal@linux-sh.org; 
> linaro-dev@lists.linaro.org; patches@linaro.org; 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

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

[Tomi Valkeinen]

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 <tony@atomide.com>
> Cc: Samreen <samreen@ti.com>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
> 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

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

[Tushar Behera]

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<tony@atomide.com>
>> Cc: Samreen<samreen@ti.com>
>> Signed-off-by: Tushar Behera<tushar.behera@linaro.org>
>> ---
>> 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

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

[Tushar Behera]

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 <lethal@linux-sh.org>
Cc: Tomi Valkeinen <tomi.valkeinen@nokia.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 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

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

[Tomi Valkeinen]

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

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

[Tushar Behera]

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

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

[Tomi Valkeinen]

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