[PATCH v2 0/2] DSS2:Allow FB to build without VRFB

[PATCH v2 0/2] DSS2:Allow FB to build without VRFB

[Senthilvadivu Guruswamy]

From: Senthilvadivu Guruswamy  <svadivu@ti.com>

Hi all,

This patch series replaces the patch 
"DSS2 Include VRFB into omap2-3build only"
Thanks for the review comments. 

The intent of this series is to split the patch into 2 logical
patches and also to incorporate the comments on multi-omap build.

In this series, Kconfig is changed to have 
OMAP2_VRFB depend on ARCH_OMAP2 and ARCH_OMAP3.
This change takes care of the multi-omap builds. 

This patch would allow successful build of omap_4430sdp_defconfig 
when OMAP2_DSS and FB_OMAP2 is enabled from menuconfig.

For verification: Generated the .config on omap3_defconfig with DSS
and FB enabled. Generated .config is same with and without the patch.

List of Changed Files:
arch/arm/plat-omap/include/plat/vrfb.h
drivers/video/omap2/Kconfig
drivers/video/omap2/omapfb/Kconfig

Thanks,
Senthil

[PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB

[Senthilvadivu Guruswamy]

FB_OMAP2 can work without VRFB, but currently does not build. Fix this.

Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
---
 arch/arm/plat-omap/include/plat/vrfb.h |   16 ++++++++++++++++
 1 file changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/vrfb.h b/arch/arm/plat-omap/include/plat/vrfb.h
index d8a03ce..3792bde 100644
--- a/arch/arm/plat-omap/include/plat/vrfb.h
+++ b/arch/arm/plat-omap/include/plat/vrfb.h
@@ -35,6 +35,7 @@ struct vrfb {
 	bool yuv_mode;
 };
 
+#ifdef CONFIG_OMAP2_VRFB
 extern int omap_vrfb_request_ctx(struct vrfb *vrfb);
 extern void omap_vrfb_release_ctx(struct vrfb *vrfb);
 extern void omap_vrfb_adjust_size(u16 *width, u16 *height,
@@ -47,4 +48,19 @@ extern void omap_vrfb_setup(struct vrfb *vrfb, unsigned long paddr,
 extern int omap_vrfb_map_angle(struct vrfb *vrfb, u16 height, u8 rot);
 extern void omap_vrfb_restore_context(void);
 
+#else
+static inline int omap_vrfb_request_ctx(struct vrfb *vrfb) { return 0; }
+static inline void omap_vrfb_release_ctx(struct vrfb *vrfb) {}
+static inline void omap_vrfb_adjust_size(u16 *width, u16 *height,
+		u8 bytespp) {}
+static inline u32 omap_vrfb_min_phys_size(u16 width, u16 height, u8 bytespp)
+		{ return 0; }
+static inline u16 omap_vrfb_max_height(u32 phys_size, u16 width, u8 bytespp)
+		{ return 0; }
+static inline void omap_vrfb_setup(struct vrfb *vrfb, unsigned long paddr,
+		u16 width, u16 height, unsigned bytespp, bool yuv_mode) {}
+static inline int omap_vrfb_map_angle(struct vrfb *vrfb, u16 height, u8 rot)
+		{ return 0; }
+static inline void omap_vrfb_restore_context(void) {}
+#endif
 #endif /* __VRFB_H */
-- 
1.5.4.7

Re: [PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB

[Nishanth Menon]

Senthilvadivu Guruswamy had written, on 05/13/2010 10:20 AM, the following:
> FB_OMAP2 can work without VRFB, but currently does not build. Fix this.
> 
> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
> ---
>  arch/arm/plat-omap/include/plat/vrfb.h |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/vrfb.h b/arch/arm/plat-omap/include/plat/vrfb.h
> index d8a03ce..3792bde 100644
> --- a/arch/arm/plat-omap/include/plat/vrfb.h
> +++ b/arch/arm/plat-omap/include/plat/vrfb.h
> @@ -35,6 +35,7 @@ struct vrfb {
>  	bool yuv_mode;
>  };
>  
> +#ifdef CONFIG_OMAP2_VRFB
>  extern int omap_vrfb_request_ctx(struct vrfb *vrfb);
>  extern void omap_vrfb_release_ctx(struct vrfb *vrfb);
>  extern void omap_vrfb_adjust_size(u16 *width, u16 *height,
> @@ -47,4 +48,19 @@ extern void omap_vrfb_setup(struct vrfb *vrfb, unsigned long paddr,
>  extern int omap_vrfb_map_angle(struct vrfb *vrfb, u16 height, u8 rot);
>  extern void omap_vrfb_restore_context(void);
>  
> +#else
> +static inline int omap_vrfb_request_ctx(struct vrfb *vrfb) { return 0; }
> +static inline void omap_vrfb_release_ctx(struct vrfb *vrfb) {}
> +static inline void omap_vrfb_adjust_size(u16 *width, u16 *height,
> +		u8 bytespp) {}
> +static inline u32 omap_vrfb_min_phys_size(u16 width, u16 height, u8 bytespp)
> +		{ return 0; }
> +static inline u16 omap_vrfb_max_height(u32 phys_size, u16 width, u8 bytespp)
> +		{ return 0; }
> +static inline void omap_vrfb_setup(struct vrfb *vrfb, unsigned long paddr,
> +		u16 width, u16 height, unsigned bytespp, bool yuv_mode) {}
> +static inline int omap_vrfb_map_angle(struct vrfb *vrfb, u16 height, u8 rot)
> +		{ return 0; }
> +static inline void omap_vrfb_restore_context(void) {}
> +#endif
>  #endif /* __VRFB_H */

the core of the problem not solved: How do we handle the same kernel 
bootup on OMAP3(vrfb) and OMAP4(tiler) if it is compile time decided?

-- 
Regards,
Nishanth Menon

RE: [PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB

[Guruswamy, Senthilvadivu]

 

> -----Original Message-----
> From: Menon, Nishanth 
> Sent: Thursday, May 13, 2010 9:00 PM
> To: Guruswamy, Senthilvadivu
> Cc: linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org; 
> tony@atomide.com; tomi.valkeinen@nokia.com; Hiremath, Vaibhav
> Subject: Re: [PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB
> 
> Senthilvadivu Guruswamy had written, on 05/13/2010 10:20 AM, 
> the following:
> > FB_OMAP2 can work without VRFB, but currently does not 
> build. Fix this.
> > 
> > Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
> >  
> > +#ifdef CONFIG_OMAP2_VRFB
> >  extern int omap_vrfb_request_ctx(struct vrfb *vrfb);
> > +#else
> > +static inline int omap_vrfb_request_ctx(struct vrfb *vrfb) 
> { return 0; }
> > +static inline void omap_vrfb_release_ctx(struct vrfb *vrfb) {}
> 
> the core of the problem not solved: How do we handle the same kernel 
> bootup on OMAP3(vrfb) and OMAP4(tiler) if it is compile time decided?
> 
[Senthil] Compile time decision would come into picture only for 
build with omap_4430sdp_defconfig.  Kernel is built with 
omap3_defconfig will have CONFIG_OMAP2_VRFB=y in it. 
In runtime,  VRFB APIs will not get called in OMAP4 since these 
calls are already within runtime check "if(rotation.type = VRFB)".

This patch is for omap_4430sdp_defconfig to build.
Reason:
VRFB functions make calls to "sms_..." functions in sdrc.c
which is applicable to omap2-3-common and gets compiled only with 
ARCH_OMAP2, ARCH_OMAP3. omap_4430sdp_defconfig has only ARCH_OMAP4 
defined in it, so sdrc.c is not included in the build leading 
to unresolved symbols "sms...".  So empty the VRFB functions for
non omap2-3 builds.



Regards,
Senthil

[PATCH v2 2/2] DSS2: make VRFB depends on OMAP2,3

[Senthilvadivu Guruswamy]

config VRFB should depend on ARCH_OMAP2 or ARCH_OMAP3.

Changes from v1:
	- Addressed multi-omap build issue

Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
---
 drivers/video/omap2/Kconfig            |    4 ++++
 drivers/video/omap2/omapfb/Kconfig     |    1 -
 2 files changed,  4 insertions(+), 1 deletions(-)

diff --git a/drivers/video/omap2/Kconfig b/drivers/video/omap2/Kconfig
index d877c36..18bb835 100644
--- a/drivers/video/omap2/Kconfig
+++ b/drivers/video/omap2/Kconfig
@@ -3,6 +3,10 @@ config OMAP2_VRAM
 
 config OMAP2_VRFB
 	bool
+	depends on ARCH_OMAP2 || ARCH_OMAP3
+	default y if FB_OMAP2
+	help
+	  OMAP VRFB buffer support is efficient for rotation
 
 source "drivers/video/omap2/dss/Kconfig"
 source "drivers/video/omap2/omapfb/Kconfig"
diff --git a/drivers/video/omap2/omapfb/Kconfig b/drivers/video/omap2/omapfb/Kconfig
index a3ed15c..f186c2b 100644
--- a/drivers/video/omap2/omapfb/Kconfig
+++ b/drivers/video/omap2/omapfb/Kconfig
@@ -3,7 +3,6 @@ menuconfig FB_OMAP2
         depends on FB && OMAP2_DSS
 
 	select OMAP2_VRAM
-	select OMAP2_VRFB
         select FB_CFB_FILLRECT
         select FB_CFB_COPYAREA
         select FB_CFB_IMAGEBLIT
-- 
1.5.4.7

Re: [PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB

[Koen Kooi]

Op 13 mei 2010, om 17:20 heeft Senthilvadivu Guruswamy het volgende geschreven:

> FB_OMAP2 can work without VRFB, but currently does not build. Fix this.
> 
> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
> ---
> arch/arm/plat-omap/include/plat/vrfb.h |   16 ++++++++++++++++
> 1 file changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/vrfb.h b/arch/arm/plat-omap/include/plat/vrfb.h
> index d8a03ce..3792bde 100644
> --- a/arch/arm/plat-omap/include/plat/vrfb.h
> +++ b/arch/arm/plat-omap/include/plat/vrfb.h
> @@ -35,6 +35,7 @@ struct vrfb {
> 	bool yuv_mode;
> };
> 
> +#ifdef CONFIG_OMAP2_VRFB

That is still a compiletime option, not a runtime check. You need something like if(is_omap3()), not #ifdef 

regards,

Koen

Re: [PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB

[Nishanth Menon]

Koen Kooi had written, on 05/13/2010 11:00 AM, the following:
> Op 13 mei 2010, om 17:20 heeft Senthilvadivu Guruswamy het volgende geschreven:
> 
>> FB_OMAP2 can work without VRFB, but currently does not build. Fix this.
>>
>> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
>> ---
>> arch/arm/plat-omap/include/plat/vrfb.h |   16 ++++++++++++++++
>> 1 file changed, 16 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/plat/vrfb.h b/arch/arm/plat-omap/include/plat/vrfb.h
>> index d8a03ce..3792bde 100644
>> --- a/arch/arm/plat-omap/include/plat/vrfb.h
>> +++ b/arch/arm/plat-omap/include/plat/vrfb.h
>> @@ -35,6 +35,7 @@ struct vrfb {
>> 	bool yuv_mode;
>> };
>>
>> +#ifdef CONFIG_OMAP2_VRFB
> 
> That is still a compiletime option, not a runtime check. You need something like if(is_omap3()), not #ifdef 
> 
having VRFB or tiler is a SOC feature - ideal detection should be in 
id.c using the FEATURES framework.

and the actual rotation handling should be handled with function 
pointers to use VRFB apis OR use tiler APIs (once it is available) to 
runtime use the right rotation/other features functions runtime..

-- 
Regards,
Nishanth Menon

RE: [PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB

[Guruswamy, Senthilvadivu]

 

> -----Original Message-----
> From: Menon, Nishanth 
> Sent: Thursday, May 13, 2010 9:45 PM
> To: Koen Kooi
> Cc: Guruswamy, Senthilvadivu; linux-omap@vger.kernel.org; 
> linux-fbdev@vger.kernel.org; tony@atomide.com; 
> tomi.valkeinen@nokia.com; Hiremath, Vaibhav
> Subject: Re: [PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB
> 
> Koen Kooi had written, on 05/13/2010 11:00 AM, the following:
> > Op 13 mei 2010, om 17:20 heeft Senthilvadivu Guruswamy het 
> volgende geschreven:
> > 
> >> FB_OMAP2 can work without VRFB, but currently does not 
> build. Fix this.
> >>
> >> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
> >> ---
> >>
> >> +#ifdef CONFIG_OMAP2_VRFB
> > 
> > That is still a compiletime option, not a runtime check. 
> You need something like if(is_omap3()), not #ifdef 
> > 
> having VRFB or tiler is a SOC feature - ideal detection should be in 
> id.c using the FEATURES framework.
> 
> and the actual rotation handling should be handled with function 
> pointers to use VRFB apis OR use tiler APIs (once it is available) to 
> runtime use the right rotation/other features functions runtime..
[Senthil] Yes, its good to have function pointers to call VRFB/Tiler APIs.  
We will consider FnPtrs for VRFB/Tiler when we plugin Tiler APIs in FB driver.
ie "if (rotation.type = VRFB)" could be replaced with FnPtrs later.

Even to introduce FnPtrs for VRFB, compiletime dependency of sdrc.c-vrfb.c
has to be resolved first (this patch address this) in case of omap 
platforms without ARCH_OMAP2 and ARCH_OMAP3 
(ie omap_4430sdp_defconfig, omap_panda_defconfig).
Currently these omap4_defconfig is not compilable since sdrc.c is included
as  $(omap2-3-common) = sdrc.o.

Regards,
Senthil

RE: [PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB

[Guruswamy, Senthilvadivu]

 

> -----Original Message-----
> From: Koen Kooi [mailto:koen@dominion.thruhere.net] 
> Sent: Thursday, May 13, 2010 9:30 PM
> To: Guruswamy, Senthilvadivu
> Cc: linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org; 
> tony@atomide.com; tomi.valkeinen@nokia.com; Hiremath, Vaibhav
> Subject: Re: [PATCH v2 1/2] DSS2: Allow FB_OMAP2 to build without VRFB
> 
> 
> Op 13 mei 2010, om 17:20 heeft Senthilvadivu Guruswamy het 
> volgende geschreven:
> 
> > FB_OMAP2 can work without VRFB, but currently does not 
> build. Fix this.
> > 
> > Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
> > ---
> > arch/arm/plat-omap/include/plat/vrfb.h |   16 ++++++++++++++++
> > 1 file changed, 16 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/plat-omap/include/plat/vrfb.h 
> b/arch/arm/plat-omap/include/plat/vrfb.h
> > index d8a03ce..3792bde 100644
> > --- a/arch/arm/plat-omap/include/plat/vrfb.h
> > +++ b/arch/arm/plat-omap/include/plat/vrfb.h
> > @@ -35,6 +35,7 @@ struct vrfb {
> > 	bool yuv_mode;
> > };
> > 
> > +#ifdef CONFIG_OMAP2_VRFB
> 
> That is still a compiletime option, not a runtime check. You 
> need something like if(is_omap3()), not #ifdef 
> 
[Senthil] Runtime check for calling VRFB functions are already
taken care in FB driver omapfb-main.c with 
"if(rotation.type = VRFB).  This compile time option is only for
omap_4430sdp_defconfig to build, where sdrc functions are called
from VRFB.c.  Sdrc.c is not included in omap_4430sdp_defconfig build.

> regards,
> 
> Koen

Re: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB

[Tomi Valkeinen]

Hi,

On Thu, 2010-05-13 at 17:20 +0200, ext Senthilvadivu Guruswamy wrote:
> From: Senthilvadivu Guruswamy  <svadivu@ti.com>
> 
> Hi all,
> 
> This patch series replaces the patch 
> "DSS2 Include VRFB into omap2-3build only"
> Thanks for the review comments. 
> 
> The intent of this series is to split the patch into 2 logical
> patches and also to incorporate the comments on multi-omap build.
> 
> In this series, Kconfig is changed to have 
> OMAP2_VRFB depend on ARCH_OMAP2 and ARCH_OMAP3.
> This change takes care of the multi-omap builds. 
> 
> This patch would allow successful build of omap_4430sdp_defconfig 
> when OMAP2_DSS and FB_OMAP2 is enabled from menuconfig.
> 
> For verification: Generated the .config on omap3_defconfig with DSS
> and FB enabled. Generated .config is same with and without the patch.
> 
> List of Changed Files:
> arch/arm/plat-omap/include/plat/vrfb.h
> drivers/video/omap2/Kconfig
> drivers/video/omap2/omapfb/Kconfig

The patch set makes VRFB optional. What happens if VRFB is turned off,
and the user uses VRFB for a framebuffer?

 Tomi

RE: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB

[Guruswamy, Senthilvadivu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 2354 bytes --]

 

> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com] 
> Sent: Friday, May 14, 2010 12:54 PM
> To: Guruswamy, Senthilvadivu
> Cc: linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org; 
> tony@atomide.com; Hiremath, Vaibhav
> Subject: Re: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB
> 
> Hi,
> 
> On Thu, 2010-05-13 at 17:20 +0200, ext Senthilvadivu Guruswamy wrote:
> > From: Senthilvadivu Guruswamy  <svadivu@ti.com>
> > 
> > Hi all,
> > 
> > This patch series replaces the patch 
> > "DSS2 Include VRFB into omap2-3build only"
> > Thanks for the review comments. 
> > 
> > The intent of this series is to split the patch into 2 logical
> > patches and also to incorporate the comments on multi-omap build.
> > 
> > In this series, Kconfig is changed to have 
> > OMAP2_VRFB depend on ARCH_OMAP2 and ARCH_OMAP3.
> > This change takes care of the multi-omap builds. 
> > 
> > This patch would allow successful build of omap_4430sdp_defconfig 
> > when OMAP2_DSS and FB_OMAP2 is enabled from menuconfig.
> > 
> > For verification: Generated the .config on omap3_defconfig with DSS
> > and FB enabled. Generated .config is same with and without 
> the patch.
> > 
> > List of Changed Files:
> > arch/arm/plat-omap/include/plat/vrfb.h
> > drivers/video/omap2/Kconfig
> > drivers/video/omap2/omapfb/Kconfig
> 
> The patch set makes VRFB optional. What happens if VRFB is turned off,
> and the user uses VRFB for a framebuffer?
[Senthil] This patch keeps VRFB=y for ARCH_OMAP2 and ARCH_OMAP3.
User would have got an option to turn it OFF if it had appeared in 
the menuconfig selections.  I did not give that option in menuconfig 
explicitly. 
ie  config OMAP2_VRFB
	bool <No name given here>

Suppose on a build the user deliberately gives "CONFIG_OMAP2_VRFB not set",
then VRFB functions are made as empty functions by the compiler.

This is fine as long as the user does not say omapfb.vrfb=1.

But if the user sets omapfb.vrfb=1, then it is a wrong usage of the bootargs
as he has already deliberately changed the defconfig to say "VRFB not set".

The result of his experiment: No bootup on the board as the vaddr of VRFB 
is populated nor of the normal RAM buffer.

>  Tomi
> 
> 
> ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±ýöÝzÿâžØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~†­†Ûiÿÿïêÿ‘êçz_è®\x0fæj:+v‰¨þ)ߣøm

Re: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB

[Koen Kooi]

Op 14 mei 2010, om 12:03 heeft Guruswamy, Senthilvadivu het volgende geschreven:

> 
> 
>> -----Original Message-----
>> From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com] 
>> Sent: Friday, May 14, 2010 12:54 PM
>> To: Guruswamy, Senthilvadivu
>> Cc: linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org; 
>> tony@atomide.com; Hiremath, Vaibhav
>> Subject: Re: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB
>> 
>> Hi,
>> 
>> On Thu, 2010-05-13 at 17:20 +0200, ext Senthilvadivu Guruswamy wrote:
>>> From: Senthilvadivu Guruswamy  <svadivu@ti.com>
>>> 
>>> Hi all,
>>> 
>>> This patch series replaces the patch 
>>> "DSS2 Include VRFB into omap2-3build only"
>>> Thanks for the review comments. 
>>> 
>>> The intent of this series is to split the patch into 2 logical
>>> patches and also to incorporate the comments on multi-omap build.
>>> 
>>> In this series, Kconfig is changed to have 
>>> OMAP2_VRFB depend on ARCH_OMAP2 and ARCH_OMAP3.
>>> This change takes care of the multi-omap builds. 
>>> 
>>> This patch would allow successful build of omap_4430sdp_defconfig 
>>> when OMAP2_DSS and FB_OMAP2 is enabled from menuconfig.
>>> 
>>> For verification: Generated the .config on omap3_defconfig with DSS
>>> and FB enabled. Generated .config is same with and without 
>> the patch.
>>> 
>>> List of Changed Files:
>>> arch/arm/plat-omap/include/plat/vrfb.h
>>> drivers/video/omap2/Kconfig
>>> drivers/video/omap2/omapfb/Kconfig
>> 
>> The patch set makes VRFB optional. What happens if VRFB is turned off,
>> and the user uses VRFB for a framebuffer?
> [Senthil] This patch keeps VRFB=y for ARCH_OMAP2 and ARCH_OMAP3.
> User would have got an option to turn it OFF if it had appeared in 
> the menuconfig selections.  I did not give that option in menuconfig 
> explicitly. 
> ie  config OMAP2_VRFB
> 	bool <No name given here>
> 
> Suppose on a build the user deliberately gives "CONFIG_OMAP2_VRFB not set",
> then VRFB functions are made as empty functions by the compiler.
> 
> This is fine as long as the user does not say omapfb.vrfb=1.
> 
> But if the user sets omapfb.vrfb=1, then it is a wrong usage of the bootargs
> as he has already deliberately changed the defconfig to say "VRFB not set".
> 
> The result of his experiment: No bootup on the board as the vaddr of VRFB 
> is populated nor of the normal RAM buffer.

And that is unacceptable when working with customers (or users in the open source world).  Instead of the kernel hacker spending an hour or 2 on a proper solution we now need to waste a whole lot more time supporting customers who pass vrfb in bootargs without knowing that it's turned off in the kernel.

I suspect my viewpoint is skewed since I work in the field with customers, instead of in the factory doing kernel work (to use TI parlance).

regards,

Koen

RE: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB

[Guruswamy, Senthilvadivu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 2486 bytes --]

 

> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com] 
> Sent: Friday, May 14, 2010 6:32 PM
> To: Guruswamy, Senthilvadivu
> Cc: linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org; 
> tony@atomide.com; Hiremath, Vaibhav
> Subject: RE: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB
> 
> On Fri, 2010-05-14 at 12:03 +0200, ext Guruswamy, Senthilvadivu wrote:
> > 
> > > -----Original Message-----
> > > From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com] 
> > > Sent: Friday, May 14, 2010 12:54 PM
> > > To: Guruswamy, Senthilvadivu
> > > Cc: linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org; 
> > > tony@atomide.com; Hiremath, Vaibhav
> > > Subject: Re: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB
> 
> <snip>
> 
> > > The patch set makes VRFB optional. What happens if VRFB 
> is turned off,
> > > and the user uses VRFB for a framebuffer?
> > [Senthil] This patch keeps VRFB=y for ARCH_OMAP2 and ARCH_OMAP3.
> > User would have got an option to turn it OFF if it had appeared in 
> > the menuconfig selections.  I did not give that option in 
> menuconfig 
> > explicitly. 
> > ie  config OMAP2_VRFB
> > 	bool <No name given here>
> > 
> > Suppose on a build the user deliberately gives 
> "CONFIG_OMAP2_VRFB not set",
> > then VRFB functions are made as empty functions by the compiler.
> > 
> > This is fine as long as the user does not say omapfb.vrfb=1.
> > 
> > But if the user sets omapfb.vrfb=1, then it is a wrong 
> usage of the bootargs
> > as he has already deliberately changed the defconfig to say 
> "VRFB not set".
> > 
> > The result of his experiment: No bootup on the board as the 
> vaddr of VRFB 
> > is populated nor of the normal RAM buffer.
> 
> The kernel should be able to cope with that. While giving wrong boot
> arguments to the kernel causing it to not boot is bad, it could be
> somewhat acceptable. But if the user changes the rotation 
> type via sysfs
> file, and the kernel crashes (which is what I fear will happen), it's
> totally unacceptable.
> 
> If it's possible to turn VRFB off, then the code should 
> handle the case
> where VRFB is not there. Meaning, returning error values or 
> somehow else
> failing gracefully, and informing the user of wrong arguments.
>
[Senthil] Yes, I could provide a check in the driver for wrong arguments.
>  Tomi
> 
> 
> ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±ýöÝzÿâžØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~†­†Ûiÿÿïêÿ‘êçz_è®\x0fæj:+v‰¨þ)ߣøm

RE: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB

[Guruswamy, Senthilvadivu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 2245 bytes --]

Tomi,
 

> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org 
> [mailto:linux-fbdev-owner@vger.kernel.org] On Behalf Of 
> Guruswamy, Senthilvadivu
> Sent: Monday, May 17, 2010 11:20 AM
> To: Tomi Valkeinen
> Cc: linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org; 
> tony@atomide.com; Hiremath, Vaibhav
> Subject: RE: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB
> 
>  
> 
> > -----Original Message-----
> > From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com] 
> > Sent: Friday, May 14, 2010 6:32 PM
> > To: Guruswamy, Senthilvadivu
> > Cc: linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org; 
> > tony@atomide.com; Hiremath, Vaibhav
> > Subject: RE: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB
> > 
> > On Fri, 2010-05-14 at 12:03 +0200, ext Guruswamy, 
> Senthilvadivu wrote:
> > > 
> > > > -----Original Message-----
> > > > From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com] 
> > > > Sent: Friday, May 14, 2010 12:54 PM
> > > > To: Guruswamy, Senthilvadivu
> > > > Cc: linux-omap@vger.kernel.org; linux-fbdev@vger.kernel.org; 
> > > > tony@atomide.com; Hiremath, Vaibhav
> > > > Subject: Re: [PATCH v2 0/2] DSS2:Allow FB to build without VRFB
> > 
> > <snip>
> > 
> > 
> > If it's possible to turn VRFB off, then the code should 
> > handle the case
> > where VRFB is not there. Meaning, returning error values or 
> > somehow else
> > failing gracefully, and informing the user of wrong arguments.
> >
> [Senthil] Yes, I could provide a check in the driver for 
> wrong arguments.
[Senthil]  I have 2 options to make this check.  I am now evaluating
the OMAP_HAS_FEATURE patch from Nishant to see if it could be used 
to extend for VRFB before I send this patch.

Option 1:
In omapfb-main.c - in omapfb_probe()

/* Patches required on top of Nishant Menon's HAS_FEATURE */
if (def_vrfb && !OMAP_HAS_FEATURE(VRFB)) { 
	def_vrfb = 0;
	printk(warning);
}
And OMAP_HAS_FEATURE(VRFB) in id.c

Option 2:
/*TODO : Replace cpu check with OMAP_HAS_FEATURE */
if ( def_vrfb && ! (cpu_is_omap24xx || cpu_is_omap34xx) ) { 
	def_vrfb = 0;
	printk(warning);
}

Regards,
Senthilÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±ýöÝzÿâžØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~†­†Ûiÿÿïêÿ‘êçz_è®\x0fæj:+v‰¨þ)ߣøm