[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 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

[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

[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

[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

[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

[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

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

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

[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

[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: 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 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

[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]

 

> -----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
> 
> 
> 

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

[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

[Nishanth Menon]

Koen Kooi had written, on 05/14/2010 05:39 AM, the following:
> 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.
But why use bootargs? We introduced FEATURES to detect if the SOC is 
capable of having VRFB or not. Use the same dynamically -> the series 
helps making VRFB optional - makes sense if there is an SOC without 
VRFB. if user space decides to use or not use rotation etc should be 
through the normal interface from userspace.. NOT through board 
file(unless ofcourse your screen is mounted 90degree rotated by default, 
and you turn around and ironically thank the board designer who mounted 
the connector wrongly) and definitely not thru bootargs..


> 
> 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,
Nishanth Menon

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

[Koen Kooi]

Op 14 mei 2010, om 12:58 heeft Nishanth Menon het volgende geschreven:

> Koen Kooi had written, on 05/14/2010 05:39 AM, the following:
>> 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.

> But why use bootargs?

Because (for some unknown reason) you can't toggle vrfb at runtime. If you realize you need rotation you need to reboot. I guess it's because the memory layout for vrfb is completely different, but again, I'm not a kernel hacker :)

regards,

Koen

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

[Nishanth Menon]

Koen Kooi had written, on 05/14/2010 06:18 AM, the following:
> Op 14 mei 2010, om 12:58 heeft Nishanth Menon het volgende geschreven:
> 
>> Koen Kooi had written, on 05/14/2010 05:39 AM, the following:
>>> 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.
> 
>> But why use bootargs?
> 
> Because (for some unknown reason) you can't toggle vrfb at runtime. If you realize
 > you need rotation you need to reboot. I guess it's because the memory 
layout for
 >vrfb is completely different, but again, I'm not a kernel hacker :)
Sorry, I am no DSS hacker myself, but looking at mainline 2.6.34-rc6 
kernel drivers/video/omap2/omapfb/omapfb-main.c, as part of late_init, 
omapfb_init gets called -> which causes probe to be called if there is a 
display around, which calls create_framebuffers and based on the 
rotation flag DMA or VRFB bootargs call to allocate goes to 
drivers/video/omap2/vrfb.c, i see request_mem_region and 
release_mem_region from a physical address mapped - so ok.. we loose 
some of our memory map..

Since this entire fuss is about being able to select the type of 
rotation... how about changing vrfb to rotation_type which defaults to 
best possible for the silicon as indicated by FEATURES as I have been 
mentioning so far..


just an idea about what I am talking about, here is an illustration patch:

diff --git a/Documentation/arm/OMAP/DSS b/Documentation/arm/OMAP/DSS
index 0af0e9e..b3df400 100644
--- a/Documentation/arm/OMAP/DSS
+++ b/Documentation/arm/OMAP/DSS
@@ -279,8 +279,11 @@ omapfb.test=<y|n>
  	- Draw test pattern to framebuffer whenever framebuffer settings change.
  	  You need to have OMAPFB debug support enabled in kernel config.

-omapfb.vrfb=<y|n>
-	- Use VRFB rotation for all framebuffers.
+omapfb.rotation_type= 0|1|2
+	 - Select rotation type on hardware
+	   0 - Default: Use Optimal rotation style for the silicon
+	   1 - Force use VRFB rotation for all framebuffers if available
+	   2 - Use DSS to rotate all framebuffers

  omapfb.rotate=<angle>
  	- Default rotation applied to all framebuffers.

diff --git a/arch/arm/plat-omap/include/plat/cpu.h 
b/arch/arm/plat-omap/include/plat/cpu.h
index 7514174..52aebae 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -444,6 +444,7 @@ extern u32 omap3_features;
  #define OMAP3_HAS_NEON			BIT(3)
  #define OMAP3_HAS_ISP			BIT(4)
  #define OMAP3_HAS_192MHZ_CLK		BIT(5)
+#define OMAP3_HAS_VRFB			BIT(6)

  #define OMAP3_HAS_FEATURE(feat,flag)			\
  static inline unsigned int omap3_has_ ##feat(void)	\
@@ -457,5 +458,6 @@ OMAP3_HAS_FEATURE(iva, IVA)
  OMAP3_HAS_FEATURE(neon, NEON)
  OMAP3_HAS_FEATURE(isp, ISP)
  OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK)
+OMAP3_HAS_FEATURE(vrfb, VRFB)

  #endif
diff --git a/drivers/video/omap2/omapfb/omapfb-main.c 
b/drivers/video/omap2/omapfb/omapfb-main.c
index 4b4506d..b0cc251 100644
--- a/drivers/video/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/omap2/omapfb/omapfb-main.c
@@ -33,6 +33,7 @@
  #include <plat/display.h>
  #include <plat/vram.h>
  #include <plat/vrfb.h>
+#include <plat/cpu.h>

  #include "omapfb.h"

@@ -43,7 +44,7 @@

  static char *def_mode;
  static char *def_vram;
-static int def_vrfb;
+static int def_rotate_type;
  static int def_rotate;
  static int def_mirror;

@@ -1872,8 +1873,14 @@ static int omapfb_create_framebuffers(struct 
omapfb2_device *fbdev)
  		ofbi->id = i;

  		/* assign these early, so that fb alloc can use them */
-		ofbi->rotation_type = def_vrfb ? OMAP_DSS_ROT_VRFB :
-			OMAP_DSS_ROT_DMA;
+
+		/* Unless forced to use otherwise, use vrfb */
+		if (omap3_has_vrfb())
+			ofbi->rotation_type = (def_rot_style == 2) ?
+				OMAP_DSS_ROT_DMA : OMAP_DSS_ROT_VRFB;
+		else
+			ofbi->rotation_type = OMAP_DSS_ROT_DMA;
+
  		ofbi->mirror = def_mirror;

  		fbdev->num_fbs++;
@@ -2272,7 +2279,7 @@ static void __exit omapfb_exit(void)
  module_param_named(mode, def_mode, charp, 0);
  module_param_named(vram, def_vram, charp, 0);
  module_param_named(rotate, def_rotate, int, 0);
-module_param_named(vrfb, def_vrfb, bool, 0);
+module_param_named(rotation_type, def_rot_style, int, 0);
  module_param_named(mirror, def_mirror, bool, 0);

  /* late_initcall to let panel/ctrl drivers loaded first.
-- 
Regards,
Nishanth Menon

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

[Tomi Valkeinen]

On Fri, 2010-05-14 at 13:18 +0200, ext Koen Kooi wrote:
> Op 14 mei 2010, om 12:58 heeft Nishanth Menon het volgende geschreven:

> > But why use bootargs?
> 
> Because (for some unknown reason) you can't toggle vrfb at runtime. If you realize you need rotation you need to reboot. I guess it's because the memory layout for vrfb is completely different, but again, I'm not a kernel hacker :)
> 

You can. There's rotate_type in the framebuffer sysfs directory. But I
think you need to release memories etc before that, and reallocate after
changing the rotation type.

 Tomi

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

[Guruswamy, Senthilvadivu]

 

> -----Original Message-----
> From: Koen Kooi [mailto:koen@dominion.thruhere.net] 
> Sent: Friday, May 14, 2010 4:09 PM
> To: Guruswamy, Senthilvadivu
> Cc: Tomi Valkeinen; 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
> 
> 
> 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>
> >>> 
> >>> 
> >>> 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.
> >>> 
> >>> 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).
> 
[Senthil] This error condition could occur only when the user does 
the settings wrong twice.  One time he is deliberately modifying the 
defconfig as "VRFB not set" even though menuconfig option is not given.  
Second he is contradicting himself by providing omapfb.vrfb=1.

If a build has to take care of this contradicting error scenarios
then my suggestion would be to add sdrc.o in arch/arm/mach-omap2,
even though it does not make sense to have an omap2-3 feature in the
omap4 and future omap defconfig builds.

Could you please share if you have thought of any other suggestion 
for this?

Regards,
Senthil

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

[Tomi Valkeinen]

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.

 Tomi

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

[Guruswamy, Senthilvadivu]

 

> -----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
> 
> 
> 

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]

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

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

[RFC] DSS: Movement of base addr, silicon specific data from driver to platform_device

[Guruswamy, Senthilvadivu]

Hi,

This RFC is for making DSS drivers not aware of omap versions and 
omap2,3 specific data like base addr, and irqs. 
DSS base address, irqs, and silicon specific data could be placed in platform_device.
This avoids the base address changes in the dss specific drivers like rfbi, dsi, sdi.
Board specific data shall be continued to be maintained in platform_data.
More details are inlined in the patch with signature [RFC].
Please provide your comments on this.

Files tentatively to be modified are:
 arch/arm/mach-omap2/board-3430sdp.c and all omap board files.
 arch/arm/mach-omap2/devices.c            
 arch/arm/plat-omap/include/plat/display.h
 drivers/video/omap2/dss/core.c           
 drivers/video/omap2/dss/dispc.c         
 drivers/video/omap2/dss/dsi.c and all other dss driver files.
 drivers/video/omap2/dss/dss.c           
 drivers/video/omap2/dss/dss.h           

Regards,
Senthil

diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index 540d28f..bfdc5f0
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -536,16 +536,7 @@ static struct omap_dss_board_info sdp3430_dss_data = {
 	.default_device	=	&sdp3430_lcd_device,
 };
 
-static struct platform_device sdp3430_dss_device = {
-	.name	=	"omapdss",
-	.id	=	-1,
-	.dev	= {
-		.platform_data = &sdp4430_dss_data,
-	},
-};
-
 static struct platform_device *sdp3430_devices[] __initdata = {
-	&sdp3430_dss_device,
 	&sdp3430_keypad_device,
 };
@@ -905,6 +896,7 @@ static void __init omap_3430sdp_init(void)
 	platform_add_devices(sdp3430_devices, ARRAY_SIZE(sdp3430_devices));
 	omap_serial_init();
+	display_init(&sdp3430_dss_data);
 	
[RFC]  Platform device shall be moved to devices.c for definition and registration
of the dss device in the platform bus.

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 83bd3d6..e481f63
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -26,6 +26,7 @@
 #include <plat/mux.h>
 #include <mach/gpio.h>
 #include <plat/mmc.h>
+#include <plat/display.h>
 
 #include "mux.h"
 
@@ -790,7 +791,103 @@ static inline void omap_hdq_init(void)
 #else
 static inline void omap_hdq_init(void) {}
 #endif
+/*---------------------------------------------------------------------------*/
+#ifdef CONFIG_OMAP2_DSS
+
+#define OMAP4_DISPC_BASE		0x58001000
+#define OMAP2_DISPC_BASE		0x48050400
+#define OMAP3_DSI_BASE			0x4804FC00
+#define OMAP4_DSI_BASE			0x58004000
+#define OMAP4_DSI2_BASE		0x58005000
+#define OMAP2_DSS_BASE			0x48050000
+#define OMAP4_DSS_BASE			0x58000000
+
+
[RFC]  Move the Base Address macros from the dss driver files to devices.c where
the platform_device is defined.  These macros get into resource structure.  The
resource strucutre would be a part of platform_device.


+
+static struct platform_device omap_display_dev = {
+	.name		= "omapdss",
+	.id		= 1,
+	.dev		= {
+		.platform_data = NULL, // rename as omapboard_dss_data
+	},
+	.num_resources	= 0,
+	.resource	= NULL,
+};
+
+
+static struct resource omap2_dss_resources[] = {
+	{
+		.start		= OMAP2_DISPC_BASE,
+		.name			= "dispc",
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP2_DSS_BASE,
+		.name			= "dss",
+		.flags		= IORESOURCE_MEM,
+	},
+};
+static struct resource omap3_dss_resources[] = {
+	{
+		.start		= OMAP2_DISPC_BASE,
+		.name			= "dispc",
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP2_DSS_BASE,
+		.name			= "dss",
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP2_DSI_BASE,
+		.name			= "dsi",
+		.flags		= IORESOURCE_MEM,
+	},
+};
+static struct resource omap4_dss_resources[] = {
+	{
+		.start		= OMAP4_DISPC_BASE,
+		.name			= "dispc",
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP4_DSS_BASE,
+		.name			= "dss",
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP4_DSI_BASE,
+		.name			= "dsi",
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP4_DSI2_BASE,
+		.name			= "dsi2",
+		.flags		= IORESOURCE_MEM,
+	},
+};
+
 
+void __init display_init(struct omap_dss_board_info *board_data)
+{
+	if (cpu_is_omap24xx()) {
+		omap_display_dev.resource = omap2_dss_resources;
+		omap_display_dev.num_resources = ARRAY_SIZE(omap2_dss_resources);
+	} else if (cpu_is_omap34xx()) {
+		omap_display_dev.resource = omap3_dss_resources;
+		omap_display_dev.num_resources = ARRAY_SIZE(omap3_dss_resources);
+	} else if (cpu_is_omap44xx()) {
+		omap_display_dev.resource = omap4_dss_resources;
+		omap_display_dev.num_resources = ARRAY_SIZE(omap4_dss_resources);
+	}
+	omap_display_dev.dev.platform_data = board_data;
+	if (platform_device_register(&omap_display_dev) < 0)
+		printk(KERN_ERR "Unable to register Display device\n");
+}
+
+#else
+void __init display_init(struct omap_dss_board_info *board_data)
+{
+}
+#endif
 /*---------------------------------------------------------------------------*/
 
 #if defined(CONFIG_VIDEO_OMAP2_VOUT) || \
diff --git a/arch/arm/plat-omap/include/plat/display.h b/arch/arm/plat-omap/include/plat/display.h
index 5ea4229..80bb135 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -24,6 +24,7 @@
 #include <linux/kobject.h>
 #include <linux/device.h>
 #include <asm/atomic.h>
+#include <linux/platform_device.h>
 
 #define DISPC_IRQ_FRAMEDONE		(1 << 0)
 #define DISPC_IRQ_VSYNC			(1 << 1)
@@ -334,6 +335,7 @@ struct omap_dss_board_info {
 	struct omap_dss_device **devices;
 	struct omap_dss_device *default_device;
 };
+extern void display_init(struct omap_dss_board_info *board_data);
 
 struct omap_video_timings {
 	/* Unit: pixels */
[RFC]  Inclusion of platform_device.h in display.h so that all the display drivers
could access it.

diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index ed9f769..10bb592 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c

@@ -526,14 +526,14 @@ static int omap_dss_probe(struct platform_device *pdev)
 		skip_init = 1;
 #endif
 
-	r = dss_init(skip_init);
+	r = dss_init(pdev,skip_init);
 	if (r) {
 		DSSERR("Failed to initialize DSS\n");
 		goto fail0;
 	}
 
 #ifdef CONFIG_OMAP2_DSS_RFBI
-	r = rfbi_init();
+	r = rfbi_init(pdev);
 	if (r) {
 		DSSERR("Failed to initialize rfbi\n");
 		goto fail0;
@@ -548,7 +548,7 @@ static int omap_dss_probe(struct platform_device *pdev)
 	}
 #endif
 
-	r = dispc_init();
+	r = dispc_init(pdev);
 	if (r) {
 		DSSERR("Failed to initialize dispc\n");
 		goto fail0;
@@ -562,7 +562,7 @@ static int omap_dss_probe(struct platform_device *pdev)
 #endif
 	if (cpu_is_omap34xx()) {
 #ifdef CONFIG_OMAP2_DSS_SDI
-		r = sdi_init(skip_init);
+		r = sdi_init(pdev,skip_init);
 		if (r) {
 			DSSERR("Failed to initialize SDI\n");

 			goto fail0;
[RFC]  Each of the driver initialisation take the platform device structure in it.
The corresponding base address, irq info could be extracted based on the module name
from the platform_device structure.

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index fc35ffb..35fab95 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -40,12 +40,6 @@
 #include "dss.h"

 
-#define DISPC_BASE		0x48050400

 #define DISPC_SZ_REGS			SZ_1K
 
 struct dispc_reg { u16 idx; };
@@ -4038,9 +4032,12 @@ static void _omap_dispc_initial_config(void)
 	dispc_read_plane_fifo_sizes();
 }
 
-int dispc_init(void)
+int dispc_init(struct platform_device *pdev)
 {
 	u32 rev;
+	struct resource *dispc_res;
+
+	dispc_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dispc");
 
 	spin_lock_init(&dispc.irq_lock);
 
@@ -4051,7 +4048,7 @@ int dispc_init(void)
 
 	INIT_WORK(&dispc.error_work, dispc_error_worker);
 
-	dispc_base = dispc.base = ioremap(DISPC_BASE, DISPC_SZ_REGS);
+	dispc_base = dispc.base = ioremap(dispc_res->start, DISPC_SZ_REGS);
 	if (!dispc.base) {
 		DSSERR("can't ioremap DISPC\n");
 		return -ENOMEM;
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 178f0fd..95a49c7 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -42,19 +42,13 @@
 /*#define VERBOSE_IRQ*/
 #define DSI_CATCH_MISSING_TE
 
-#define DSI_BASE		0x58004000

#define DSI_SZ_REGS			SZ_1K
 
 struct dsi_reg { u16 idx; };
 
 #define DSI_REG(idx)		((const struct dsi_reg) { idx })
 
-#define DSI_SZ_REGS		SZ_1K
+
 /* DSI Protocol Engine */
 
 #define DSI_REVISION			DSI_REG(0x0000)
@@ -3701,6 +3695,9 @@ int dsi_init(struct platform_device *pdev)
 	u32 rev;
 	int r;
 	enum omap_dsi_index ix = DSI1;
+	struct resource *dsi_res;
+
+	dsi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dsi");
 
 	spin_lock_init(&dsi.errors_lock);
 	dsi.errors = 0;
@@ -3730,7 +3727,7 @@ int dsi_init(struct platform_device *pdev)
 	dsi.te_timer.function = dsi_te_timeout;
 	dsi.te_timer.data = 0;
 #endif
-	dsi.base = ioremap(DSI_BASE, DSI_SZ_REGS);
+	dsi.base = ioremap(dsi_res->start, DSI_SZ_REGS);
 	if (!dsi.base) {
 		DSSERR("can't ioremap DSI\n");
 		r = -ENOMEM;
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index db8bc71..958e96b 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -33,12 +33,6 @@
 #include <plat/display.h>
 #include "dss.h"
 
-#define DSS_BASE			0x48050000
 #define DSS_SZ_REGS			SZ_512
 
 struct dss_reg {
 
-int dss_init(bool skip_init)
+int dss_init(struct platform_device *pdev, bool skip_init)
 {
 	int r;
 	u32 rev;
+	struct resource *dss_res;
+
+	dss_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dss");
 
-	dss.base = ioremap(DSS_BASE, DSS_SZ_REGS);
+	dss.base = ioremap(dss_res->start, DSS_SZ_REGS);
 	if (!dss.base) {
 		DSSERR("can't ioremap DSS\n");
 		r = -ENOMEM;
[RFC]  Example of how the base address could be applied on dispc, dsi, sdi. 

diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 02cd5ae..636c08a 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h

 /* DSS */
-int dss_init(bool skip_init);
+int dss_init(struct platform_device *pdev, bool skip_init);
 void dss_exit(void);
 
 /* SDI */
-int sdi_init(bool skip_init);
+int sdi_init(struct platform_device *pdev, bool skip_init);
 void sdi_exit(void);
 int sdi_init_display(struct omap_dss_device *display);
 
 /* DISPC */
-int dispc_init(void);
+int dispc_init(struct platform_device *pdev);
 void dispc_exit(void);
 void dispc_dump_clocks(struct seq_file *s);
 void dispc_dump_irqs(struct seq_file *s);
 
 /* RFBI */
-int rfbi_init(void);
+int rfbi_init(struct platform_device *pdev);
 void rfbi_exit(void);
 void rfbi_dump_regs(struct seq_file *s);
 

RE: [RFC] DSS: Movement of base addr, silicon specific data from driver to platform_device

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Guruswamy, Senthilvadivu
> Sent: Thursday, June 10, 2010 10:49 AM
> To: linux-omap@vger.kernel.org; tony@atomide.com; tomi.valkeinen@nokia.com;
> Hiremath, Vaibhav
> Subject: [RFC] DSS: Movement of base addr, silicon specific data from driver
> to platform_device
> 
> Hi,
> 
> This RFC is for making DSS drivers not aware of omap versions and
> omap2,3 specific data like base addr, and irqs.
> DSS base address, irqs, and silicon specific data could be placed in
> platform_device.
> This avoids the base address changes in the dss specific drivers like rfbi,
> dsi, sdi.
> Board specific data shall be continued to be maintained in platform_data.
> More details are inlined in the patch with signature [RFC].
> Please provide your comments on this.
> 
[Hiremath, Vaibhav] Ideally, this is correct way of sharing the resource. I do agree that we should take all the platform/device specific resources from platform_data.


> Files tentatively to be modified are:
>  arch/arm/mach-omap2/board-3430sdp.c and all omap board files.
>  arch/arm/mach-omap2/devices.c
>  arch/arm/plat-omap/include/plat/display.h
>  drivers/video/omap2/dss/core.c
>  drivers/video/omap2/dss/dispc.c
>  drivers/video/omap2/dss/dsi.c and all other dss driver files.
>  drivers/video/omap2/dss/dss.c
>  drivers/video/omap2/dss/dss.h
> 
> Regards,
> Senthil
> 
> diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-
> omap2/board-3430sdp.c
> index 540d28f..bfdc5f0
> --- a/arch/arm/mach-omap2/board-3430sdp.c
> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> @@ -536,16 +536,7 @@ static struct omap_dss_board_info sdp3430_dss_data = {
>  	.default_device	=	&sdp3430_lcd_device,
>  };
> 
> -static struct platform_device sdp3430_dss_device = {
> -	.name	=	"omapdss",
> -	.id	=	-1,
> -	.dev	= {
> -		.platform_data = &sdp4430_dss_data,
> -	},
> -};
> -
>  static struct platform_device *sdp3430_devices[] __initdata = {
> -	&sdp3430_dss_device,
>  	&sdp3430_keypad_device,
>  };
> @@ -905,6 +896,7 @@ static void __init omap_3430sdp_init(void)
>  	platform_add_devices(sdp3430_devices, ARRAY_SIZE(sdp3430_devices));
>  	omap_serial_init();
> +	display_init(&sdp3430_dss_data);
> 
> [RFC]  Platform device shall be moved to devices.c for definition and
> registration
> of the dss device in the platform bus.
> 
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 83bd3d6..e481f63
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -26,6 +26,7 @@
>  #include <plat/mux.h>
>  #include <mach/gpio.h>
>  #include <plat/mmc.h>
> +#include <plat/display.h>
> 
>  #include "mux.h"
> 
> @@ -790,7 +791,103 @@ static inline void omap_hdq_init(void)
>  #else
>  static inline void omap_hdq_init(void) {}
>  #endif
> +/*-------------------------------------------------------------------------
> --*/
> +#ifdef CONFIG_OMAP2_DSS
> +
> +#define OMAP4_DISPC_BASE		0x58001000
> +#define OMAP2_DISPC_BASE		0x48050400
> +#define OMAP3_DSI_BASE			0x4804FC00
> +#define OMAP4_DSI_BASE			0x58004000
> +#define OMAP4_DSI2_BASE		0x58005000
> +#define OMAP2_DSS_BASE			0x48050000
> +#define OMAP4_DSS_BASE			0x58000000
> +
> +
> [RFC]  Move the Base Address macros from the dss driver files to devices.c
> where
> the platform_device is defined.  These macros get into resource structure.
> The
> resource strucutre would be a part of platform_device.
> 
[Hiremath, Vaibhav] Don't you think this should be part of either omap24xx.h, omap34xx.h or omap44xx.h.

> 
> +
> +static struct platform_device omap_display_dev = {
> +	.name		= "omapdss",
> +	.id		= 1,
> +	.dev		= {
> +		.platform_data = NULL, // rename as omapboard_dss_data
> +	},
> +	.num_resources	= 0,
> +	.resource	= NULL,
> +};
> +
> +
> +static struct resource omap2_dss_resources[] = {
> +	{
> +		.start		= OMAP2_DISPC_BASE,
> +		.name			= "dispc",
> +		.flags		= IORESOURCE_MEM,
> +	},
> +	{
> +		.start		= OMAP2_DSS_BASE,
> +		.name			= "dss",
> +		.flags		= IORESOURCE_MEM,
> +	},
> +};
> +static struct resource omap3_dss_resources[] = {
> +	{
> +		.start		= OMAP2_DISPC_BASE,
> +		.name			= "dispc",
> +		.flags		= IORESOURCE_MEM,
> +	},
> +	{
> +		.start		= OMAP2_DSS_BASE,
> +		.name			= "dss",
> +		.flags		= IORESOURCE_MEM,
> +	},
> +	{
> +		.start		= OMAP2_DSI_BASE,
> +		.name			= "dsi",
> +		.flags		= IORESOURCE_MEM,
> +	},
> +};
> +static struct resource omap4_dss_resources[] = {
> +	{
> +		.start		= OMAP4_DISPC_BASE,
> +		.name			= "dispc",
> +		.flags		= IORESOURCE_MEM,
> +	},
> +	{
> +		.start		= OMAP4_DSS_BASE,
> +		.name			= "dss",
> +		.flags		= IORESOURCE_MEM,
> +	},
> +	{
> +		.start		= OMAP4_DSI_BASE,
> +		.name			= "dsi",
> +		.flags		= IORESOURCE_MEM,
> +	},
> +	{
> +		.start		= OMAP4_DSI2_BASE,
> +		.name			= "dsi2",
> +		.flags		= IORESOURCE_MEM,
> +	},
> +};
> +
> 
[Hiremath, Vaibhav] Specify .end to all of these resources so that we do not need to hardcode the size while mapping.

> +void __init display_init(struct omap_dss_board_info *board_data)
> +{
> +	if (cpu_is_omap24xx()) {
> +		omap_display_dev.resource = omap2_dss_resources;
> +		omap_display_dev.num_resources =
> ARRAY_SIZE(omap2_dss_resources);
> +	} else if (cpu_is_omap34xx()) {
> +		omap_display_dev.resource = omap3_dss_resources;
> +		omap_display_dev.num_resources =
> ARRAY_SIZE(omap3_dss_resources);
> +	} else if (cpu_is_omap44xx()) {
> +		omap_display_dev.resource = omap4_dss_resources;
> +		omap_display_dev.num_resources =
> ARRAY_SIZE(omap4_dss_resources);
> +	}
> +	omap_display_dev.dev.platform_data = board_data;
> +	if (platform_device_register(&omap_display_dev) < 0)
> +		printk(KERN_ERR "Unable to register Display device\n");
> +}
> +
> +#else
> +void __init display_init(struct omap_dss_board_info *board_data)
> +{
> +}
> +#endif
>  /*-------------------------------------------------------------------------
> --*/
> 
>  #if defined(CONFIG_VIDEO_OMAP2_VOUT) || \
> diff --git a/arch/arm/plat-omap/include/plat/display.h b/arch/arm/plat-
> omap/include/plat/display.h
> index 5ea4229..80bb135 100644
> --- a/arch/arm/plat-omap/include/plat/display.h
> +++ b/arch/arm/plat-omap/include/plat/display.h
> @@ -24,6 +24,7 @@
>  #include <linux/kobject.h>
>  #include <linux/device.h>
>  #include <asm/atomic.h>
> +#include <linux/platform_device.h>
> 
>  #define DISPC_IRQ_FRAMEDONE		(1 << 0)
>  #define DISPC_IRQ_VSYNC			(1 << 1)
> @@ -334,6 +335,7 @@ struct omap_dss_board_info {
>  	struct omap_dss_device **devices;
>  	struct omap_dss_device *default_device;
>  };
> +extern void display_init(struct omap_dss_board_info *board_data);
> 
[Hiremath, Vaibhav] Why extern?

I suggest you to submit the actual patches to the list for review.

Thanks,
Vaibhav
>  struct omap_video_timings {
>  	/* Unit: pixels */
> [RFC]  Inclusion of platform_device.h in display.h so that all the display
> drivers
> could access it.
> 
> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
> index ed9f769..10bb592 100644
> --- a/drivers/video/omap2/dss/core.c
> +++ b/drivers/video/omap2/dss/core.c
> 
> @@ -526,14 +526,14 @@ static int omap_dss_probe(struct platform_device
> *pdev)
>  		skip_init = 1;
>  #endif
> 
> -	r = dss_init(skip_init);
> +	r = dss_init(pdev,skip_init);
>  	if (r) {
>  		DSSERR("Failed to initialize DSS\n");
>  		goto fail0;
>  	}
> 
>  #ifdef CONFIG_OMAP2_DSS_RFBI
> -	r = rfbi_init();
> +	r = rfbi_init(pdev);
>  	if (r) {
>  		DSSERR("Failed to initialize rfbi\n");
>  		goto fail0;
> @@ -548,7 +548,7 @@ static int omap_dss_probe(struct platform_device *pdev)
>  	}
>  #endif
> 
> -	r = dispc_init();
> +	r = dispc_init(pdev);
>  	if (r) {
>  		DSSERR("Failed to initialize dispc\n");
>  		goto fail0;
> @@ -562,7 +562,7 @@ static int omap_dss_probe(struct platform_device *pdev)
>  #endif
>  	if (cpu_is_omap34xx()) {
>  #ifdef CONFIG_OMAP2_DSS_SDI
> -		r = sdi_init(skip_init);
> +		r = sdi_init(pdev,skip_init);
>  		if (r) {
>  			DSSERR("Failed to initialize SDI\n");
> 
>  			goto fail0;
> [RFC]  Each of the driver initialisation take the platform device structure
> in it.
> The corresponding base address, irq info could be extracted based on the
> module name
> from the platform_device structure.
> 
> diff --git a/drivers/video/omap2/dss/dispc.c
> b/drivers/video/omap2/dss/dispc.c
> index fc35ffb..35fab95 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -40,12 +40,6 @@
>  #include "dss.h"
> 
> 
> -#define DISPC_BASE		0x48050400
> 
>  #define DISPC_SZ_REGS			SZ_1K
> 
>  struct dispc_reg { u16 idx; };
> @@ -4038,9 +4032,12 @@ static void _omap_dispc_initial_config(void)
>  	dispc_read_plane_fifo_sizes();
>  }
> 
> -int dispc_init(void)
> +int dispc_init(struct platform_device *pdev)
>  {
>  	u32 rev;
> +	struct resource *dispc_res;
> +
> +	dispc_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "dispc");
> 
>  	spin_lock_init(&dispc.irq_lock);
> 
> @@ -4051,7 +4048,7 @@ int dispc_init(void)
> 
>  	INIT_WORK(&dispc.error_work, dispc_error_worker);
> 
> -	dispc_base = dispc.base = ioremap(DISPC_BASE, DISPC_SZ_REGS);
> +	dispc_base = dispc.base = ioremap(dispc_res->start, DISPC_SZ_REGS);
>  	if (!dispc.base) {
>  		DSSERR("can't ioremap DISPC\n");
>  		return -ENOMEM;
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 178f0fd..95a49c7 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -42,19 +42,13 @@
>  /*#define VERBOSE_IRQ*/
>  #define DSI_CATCH_MISSING_TE
> 
> -#define DSI_BASE		0x58004000
> 
> #define DSI_SZ_REGS			SZ_1K
> 
>  struct dsi_reg { u16 idx; };
> 
>  #define DSI_REG(idx)		((const struct dsi_reg) { idx })
> 
> -#define DSI_SZ_REGS		SZ_1K
> +
>  /* DSI Protocol Engine */
> 
>  #define DSI_REVISION			DSI_REG(0x0000)
> @@ -3701,6 +3695,9 @@ int dsi_init(struct platform_device *pdev)
>  	u32 rev;
>  	int r;
>  	enum omap_dsi_index ix = DSI1;
> +	struct resource *dsi_res;
> +
> +	dsi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dsi");
> 
>  	spin_lock_init(&dsi.errors_lock);
>  	dsi.errors = 0;
> @@ -3730,7 +3727,7 @@ int dsi_init(struct platform_device *pdev)
>  	dsi.te_timer.function = dsi_te_timeout;
>  	dsi.te_timer.data = 0;
>  #endif
> -	dsi.base = ioremap(DSI_BASE, DSI_SZ_REGS);
> +	dsi.base = ioremap(dsi_res->start, DSI_SZ_REGS);
>  	if (!dsi.base) {
>  		DSSERR("can't ioremap DSI\n");
>  		r = -ENOMEM;
> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> index db8bc71..958e96b 100644
> --- a/drivers/video/omap2/dss/dss.c
> +++ b/drivers/video/omap2/dss/dss.c
> @@ -33,12 +33,6 @@
>  #include <plat/display.h>
>  #include "dss.h"
> 
> -#define DSS_BASE			0x48050000
>  #define DSS_SZ_REGS			SZ_512
> 
>  struct dss_reg {
> 
> -int dss_init(bool skip_init)
> +int dss_init(struct platform_device *pdev, bool skip_init)
>  {
>  	int r;
>  	u32 rev;
> +	struct resource *dss_res;
> +
> +	dss_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dss");
> 
> -	dss.base = ioremap(DSS_BASE, DSS_SZ_REGS);
> +	dss.base = ioremap(dss_res->start, DSS_SZ_REGS);
>  	if (!dss.base) {
>  		DSSERR("can't ioremap DSS\n");
>  		r = -ENOMEM;
> [RFC]  Example of how the base address could be applied on dispc, dsi, sdi.
> 
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index 02cd5ae..636c08a 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> 
>  /* DSS */
> -int dss_init(bool skip_init);
> +int dss_init(struct platform_device *pdev, bool skip_init);
>  void dss_exit(void);
> 
>  /* SDI */
> -int sdi_init(bool skip_init);
> +int sdi_init(struct platform_device *pdev, bool skip_init);
>  void sdi_exit(void);
>  int sdi_init_display(struct omap_dss_device *display);
> 
>  /* DISPC */
> -int dispc_init(void);
> +int dispc_init(struct platform_device *pdev);
>  void dispc_exit(void);
>  void dispc_dump_clocks(struct seq_file *s);
>  void dispc_dump_irqs(struct seq_file *s);
> 
>  /* RFBI */
> -int rfbi_init(void);
> +int rfbi_init(struct platform_device *pdev);
>  void rfbi_exit(void);
>  void rfbi_dump_regs(struct seq_file *s);
> 

[RFC] DSS2: Need to make dsi, sdi, rfbi as platform drivers instead of a library in omapdss driver

[Guruswamy, Senthilvadivu]

Hi,

When I started with RFC patch "DSS: Movement of base addr, silicon specific 
data from driver to platform_device",  I see a lot of encapsulation of OMAP
DSS IPs into the "omapdss" driver.

The previous RFC patch would only be good enough to handle base address and
IRQ for multi-omap.  But it won't address the PRCM power management handled
in HWMOD APIs.

The encapsulation of all the DSS IPs like RFBI, DSI, SDI, VENC into omapdss
driver would prevent these drivers being handled from the platform database.

For Eg:  
HWMOD adaptation to DSS would not be possible unless these libraries are made 
as platform drivers.
	HWMOD would provide dynamic usage of multi-omap data like base addr,irq.
	HWMOD would also manage the PRCM clocks needed for the dss.

I would make an RFC patch by introducing these IPs as platform drivers which
would comply and ease for HWMOD adpatation. 

Before I go ahead make the changes in the code, I would like to get opinions
on this proposed changes.

Current dss driver:
-------------------
1.  Omapdss platform driver
	- initialises necessary Ips dss, dispc.
	- also initialises Ips like sdi, dsi, venc, rfbi
	- creates a custom bus and registers the display devices/drivers
	connected on the board to the custom bus.

2.  Suspend/resume of omapdss
	- in turn sends suspend/resume calls for each of the display devices
	registered to it.

	
Proposed change:
----------------
Omapdss platform driver
 	- initialises necessary Ips dss, dispc.
	- continues to have a custom bus and registers the display devices 
	and drivers connected on the board to the custom bus.
	- continues to handle suspend/resume of the display devices registerd
	to the custom bus.

Dsi platform driver
	- initialises DSI IP alone
	- handles suspend/resume as a platform driver.  
		The implementation gets tricky on how to know that the panels 
		connected to it is also in suspended/resumed state.

SDI platform driver
	- initialises SDI IP alone
	- handles suspend/resume as a platform driver.  
		The implementation gets tricky on how to know that the panels 
		connected to it is also in suspended/resumed state.

RFBI platform driver
	- initialises RFBI IP alone
	- handles suspend/resume as a platform driver.  
		The implementation gets tricky on how to know that the panels 
		connected to it is also in suspended/resumed state.	

DPI platform driver
	- initialises DPI IP
	- should also take care of dsi pll inits from the dsi platform driver.
	- handles suspend/resume as a platform driver.  
		The implementation gets tricky on how to know that the panels 
		connected to it is also in suspended/resumed state.

Regards,
Senthil

Re: [RFC] DSS2: Need to make dsi, sdi, rfbi as platform drivers instead of a library in omapdss driver

[Tomi Valkeinen]

On Wed, 2010-06-16 at 12:52 +0200, ext Guruswamy, Senthilvadivu wrote:
> Hi,
> 
> When I started with RFC patch "DSS: Movement of base addr, silicon specific 
> data from driver to platform_device",  I see a lot of encapsulation of OMAP
> DSS IPs into the "omapdss" driver.
> 
> The previous RFC patch would only be good enough to handle base address and
> IRQ for multi-omap.  But it won't address the PRCM power management handled
> in HWMOD APIs.
> 
> The encapsulation of all the DSS IPs like RFBI, DSI, SDI, VENC into omapdss
> driver would prevent these drivers being handled from the platform database.
> 
> For Eg:  
> HWMOD adaptation to DSS would not be possible unless these libraries are made 
> as platform drivers.
> 	HWMOD would provide dynamic usage of multi-omap data like base addr,irq.
> 	HWMOD would also manage the PRCM clocks needed for the dss.
> 
> I would make an RFC patch by introducing these IPs as platform drivers which
> would comply and ease for HWMOD adpatation. 
> 
> Before I go ahead make the changes in the code, I would like to get opinions
> on this proposed changes.

I haven't looked at how HWMODs work, but I don't see any downsides to
having the DSS HW modules as platform devices, if that is required.

But the interface modules (DSI/DPI/RFBI/SDI) will still be tightly
connected to the core DSS module, even if they are separate drivers. I'm
not sure what complexities this may bring.

This also raises the question that should the interface modules each
have a bus of their own? This is something I've pondered for a long
time, and sounds to me to be a cleaner architecture, but it may be a
bigger change.

 Tomi