[U-Boot] TI:OMAP: [PATCH 6/7] Enable DSS driver for Beagle

[U-Boot] TI:OMAP: [PATCH 6/7] Enable DSS driver for Beagle

[Khasim Syed Mohammed]

[U-Boot] TI:OMAP: [PATCH 6/7] Enable DSS driver for Beagle

[Khasim Syed Mohammed]

[U-Boot] TI:OMAP: [PATCH 6/7] Enable DSS driver for Beagle

[Tom]

Khasim Syed Mohammed wrote:
> From fe824eb530338182503dd75d466374ac2e96c063 Mon Sep 17 00:00:00 2001
> From: Syed Mohammed Khasim <khasim@ti.com>
> Date: Mon, 18 Jan 2010 18:35:40 +0530
> Subject: [PATCH] Enable DSS driver for Beagle
> 
> Configures DSS to display color bar on Svideo
> Configures DSS to display background color on DVID
> 
> Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
> ---
>  board/ti/beagle/beagle.c       |   12 ++++++
>  board/ti/beagle/beagle.h       |   75 ++++++++++++++++++++++++++++++++++++++++
>  include/configs/omap3_beagle.h |    1 +
>  3 files changed, 88 insertions(+), 0 deletions(-)
> 
> diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
> index 23985ea..2bf0430 100644
> --- a/board/ti/beagle/beagle.c
> +++ b/board/ti/beagle/beagle.c
> @@ -114,6 +114,16 @@ void beagle_identify(void)
>  }
> 
>  /*
> + * Configure DSS to display background color on DVID
> + * Configure VENC to display color bar on S-Video
> + */
> +void display_init(void)

This should be static

> +{
> +	omap3_dss_venc_config(&venc_config_std_tv, VENC_HEIGHT, VENC_WIDTH);
> +	omap3_dss_panel_config(&dvid_cfg);
> +}
> +
> +/*
>   * Routine: misc_init_r
>   * Description: Configure board specific parts
>   */
> @@ -124,6 +134,7 @@ int misc_init_r(void)
> 
>  	twl4030_power_init();
>  	twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON);
> +	display_init();

Should be if-defed to CONFIG_VIDEO_OMAP3

> 
>  	/* Configure GPIOs to output */
>  	writel(~(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1), &gpio6_base->oe);
> @@ -139,6 +150,7 @@ int misc_init_r(void)
>  	beagle_identify();
> 
>  	dieid_num_r();
> +	omap3_dss_enable();

Should be if-defed to CONFIG_VIDEO_OMAP3

> 
>  	return 0;
>  }
> diff --git a/board/ti/beagle/beagle.h b/board/ti/beagle/beagle.h
> index d1d5d27..3605427 100644
> --- a/board/ti/beagle/beagle.h
> +++ b/board/ti/beagle/beagle.h
> @@ -23,6 +23,8 @@
>  #ifndef _BEAGLE_H_
>  #define _BEAGLE_H_
> 
> +#include <asm/arch/dss.h>
> +
>  const omap3_sysinfo sysinfo = {
>  	DDR_STACKED,
>  	"OMAP3 Beagle board",
> @@ -385,4 +387,77 @@ const omap3_sysinfo sysinfo = {
>  	MUX_VAL(CP(UART2_RTS),		(IDIS | PTD | DIS | M0)) /*UART2_RTS*/\
>  	MUX_VAL(CP(UART2_TX),		(IDIS | PTD | DIS | M0)) /*UART2_TX*/
> 

This section is dss.
This should move to dss.c or dss.h

> +/*
> + * Display Configuration
> + */
> +
> +#define DVI_BEAGLE_ORANGE_COL		0x00FF8000
> +#define VENC_HEIGHT			0x00ef
> +#define VENC_WIDTH			0x027f
> +
> +/*
> + * Configure VENC in DSS for Beagle to generate Color Bar
> + *
> + * Kindly refer to OMAP TRM for definition of these values.
> + */

No good enough.
If you are going to refer people to TRM.
Give them some idea how to find the data.
The name of TRM, the section and maybe quote to help them find
the location..


15.5.8.4 Video Encoder Register Settings

For video encoder programming, see Table 15-67.
This table lists the register values to use in standard
applications. These values are validated programming values only
for the TV display support (NTSC 601 and PAL 601 standards).
Table 15-67. Video Encoder Register Programming Values
Register Name        NTSC 601                            PAL 601
VENC_F_CONTROL       0x00000000                          0x00000000
VENC_VIDOUT_CTRL     0x00000001                          0x00000001
VENC_SYNC_CTRL       0x00008040                          0x00000040
VENC_LLEN            0x00000359                          0x0000035F
VENC_FLENS

You need to qualify std_tv as either ntsc or pal.

Include both setting.
If it is impossible to test PAL, make a note with a big warning


> +static const struct venc_regs venc_config_std_tv = {
> +	.status					= 0x0000001B,
> +	.f_control				= 0x00000040,
> +	.vidout_ctrl				= 0x00000000,

?? not 1

> +	.sync_ctrl				= 0x00008000,

?? not 8040 or 40


> +	.llen					= 0x00008359,

?? not 359 or 35F

Likely you are using a different TRM than I am.
Please check if there isn't some dependency on dss silicon
revision.

Tom

[U-Boot] Fwd: TI:OMAP: [PATCH 6/7] Enable DSS driver for Beagle

[Khasim Syed Mohammed]

On Sun, Jan 24, 2010 at 5:54 AM, Tom <Tom.Rix@windriver.com> wrote:
> Khasim Syed Mohammed wrote:
>>
>> From fe824eb530338182503dd75d466374ac2e96c063 Mon Sep 17 00:00:00 2001
>> From: Syed Mohammed Khasim <khasim@ti.com>
>> Date: Mon, 18 Jan 2010 18:35:40 +0530
>> Subject: [PATCH] Enable DSS driver for Beagle
>>
>> Configures DSS to display color bar on Svideo
>> Configures DSS to display background color on DVID
>>
>> Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
>> ---
>> ?board/ti/beagle/beagle.c ? ? ? | ? 12 ++++++
>> ?board/ti/beagle/beagle.h ? ? ? | ? 75
>> ++++++++++++++++++++++++++++++++++++++++
>> ?include/configs/omap3_beagle.h | ? ?1 +
>> ?3 files changed, 88 insertions(+), 0 deletions(-)
>>
>> diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
>> index 23985ea..2bf0430 100644
>> --- a/board/ti/beagle/beagle.c
>> +++ b/board/ti/beagle/beagle.c
>> @@ -114,6 +114,16 @@ void beagle_identify(void)
>> ?}
>>
>> ?/*
>> + * Configure DSS to display background color on DVID
>> + * Configure VENC to display color bar on S-Video
>> + */
>> +void display_init(void)
>
> This should be static
ok
>
>> +{
>> + ? ? ? omap3_dss_venc_config(&venc_config_std_tv, VENC_HEIGHT,
>> VENC_WIDTH);
>> + ? ? ? omap3_dss_panel_config(&dvid_cfg);
>> +}
>> +
>> +/*
>> ?* Routine: misc_init_r
>> ?* Description: Configure board specific parts
>> ?*/
>> @@ -124,6 +134,7 @@ int misc_init_r(void)
>>
>> ? ? ? ?twl4030_power_init();
>> ? ? ? ?twl4030_led_init(TWL4030_LED_LEDEN_LEDAON |
>> TWL4030_LED_LEDEN_LEDBON);
>> + ? ? ? display_init();
>
> Should be if-defed to CONFIG_VIDEO_OMAP3
>
ok
>>
>> ? ? ? ?/* Configure GPIOs to output */
>> ? ? ? ?writel(~(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1),
>> &gpio6_base->oe);
>> @@ -139,6 +150,7 @@ int misc_init_r(void)
>> ? ? ? ?beagle_identify();
>>
>> ? ? ? ?dieid_num_r();
>> + ? ? ? omap3_dss_enable();
>
> Should be if-defed to CONFIG_VIDEO_OMAP3
>
ok

>>
>> ? ? ? ?return 0;
>> ?}
>> diff --git a/board/ti/beagle/beagle.h b/board/ti/beagle/beagle.h
>> index d1d5d27..3605427 100644
>> --- a/board/ti/beagle/beagle.h
>> +++ b/board/ti/beagle/beagle.h
>> @@ -23,6 +23,8 @@
>> ?#ifndef _BEAGLE_H_
>> ?#define _BEAGLE_H_
>>
>> +#include <asm/arch/dss.h>
>> +
>> ?const omap3_sysinfo sysinfo = {
>> ? ? ? ?DDR_STACKED,
>> ? ? ? ?"OMAP3 Beagle board",
>> @@ -385,4 +387,77 @@ const omap3_sysinfo sysinfo = {
>> ? ? ? ?MUX_VAL(CP(UART2_RTS), ? ? ? ? ?(IDIS | PTD | DIS | M0))
>> /*UART2_RTS*/\
>> ? ? ? ?MUX_VAL(CP(UART2_TX), ? ? ? ? ? (IDIS | PTD | DIS | M0))
>> /*UART2_TX*/
>>
>
> This section is dss.
> This should move to dss.c or dss.h
>
>> +/*
>> + * Display Configuration
>> + */
>> +
>> +#define DVI_BEAGLE_ORANGE_COL ? ? ? ? ?0x00FF8000
>> +#define VENC_HEIGHT ? ? ? ? ? ? ? ? ? ?0x00ef
>> +#define VENC_WIDTH ? ? ? ? ? ? ? ? ? ? 0x027f
No, they are board specific, so should be in board file. They don't
belong to DSS.

>> +
>> +/*
>> + * Configure VENC in DSS for Beagle to generate Color Bar
>> + *
>> + * Kindly refer to OMAP TRM for definition of these values.
>> + */
>
> No good enough.
> If you are going to refer people to TRM.
> Give them some idea how to find the data.
> The name of TRM, the section and maybe quote to help them find
> the location..
>
Sure.

>
> 15.5.8.4 Video Encoder Register Settings
>
> For video encoder programming, see Table 15-67.
> This table lists the register values to use in standard
> applications. These values are validated programming values only
> for the TV display support (NTSC 601 and PAL 601 standards).
> Table 15-67. Video Encoder Register Programming Values
> Register Name ? ? ? ?NTSC 601 ? ? ? ? ? ? ? ? ? ? ? ? ? ?PAL 601
> VENC_F_CONTROL ? ? ? 0x00000000 ? ? ? ? ? ? ? ? ? ? ? ? ?0x00000000
> VENC_VIDOUT_CTRL ? ? 0x00000001 ? ? ? ? ? ? ? ? ? ? ? ? ?0x00000001
> VENC_SYNC_CTRL ? ? ? 0x00008040 ? ? ? ? ? ? ? ? ? ? ? ? ?0x00000040
> VENC_LLEN ? ? ? ? ? ?0x00000359 ? ? ? ? ? ? ? ? ? ? ? ? ?0x0000035F
> VENC_FLENS
>
HUH .... :) :) :)

I was under an impression that people who really code, read TRM and
they don't need this level of details. I will give the section details
for sure.

But I feel you are asking some thing un necessarily more.... ?There
are tons of programming, if every one does this then u-boot code will
become a document portal. :)

> You need to qualify std_tv as either ntsc or pal.
>
I was not interested in confusing others on the ntsc or pal. But any
way I test NTSC, I can change the name accordingly. I will not add the
structure for PAL, will wait for some one to send out patch for PAL if
interested.

> Include both setting.
> If it is impossible to test PAL, make a note with a big warning
>
Why warning when I am saying that it is NTSC, can we not understand
that NTSC is not PAL :)

>
>> +static const struct venc_regs venc_config_std_tv = {
>> + ? ? ? .status ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? = 0x0000001B,
>> + ? ? ? .f_control ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?= 0x00000040,
>> + ? ? ? .vidout_ctrl ? ? ? ? ? ? ? ? ? ? ? ? ? ?= 0x00000000,
>
> ?? not 1
>
>> + ? ? ? .sync_ctrl ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?= 0x00008000,
>
> ?? not 8040 or 40
>
>
>> + ? ? ? .llen ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? = 0x00008359,
>
> ?? not 359 or 35F
>
> Likely you are using a different TRM than I am.
> Please check if there isn't some dependency on dss silicon
> revision.
There might be some differences, I am taking values that were working
before for me from the kernel. If some one needs they can do the TRM
comparison, PLEASE UNDERSTAND this is not for building your PRODUCT
around this, it is for TESTING THE BOARD FOR TV OUT And nothing BEYOND
THIS. Hope you understand the need for this driver in u-boot.

You always have access to TRM and register definition, they can see
what is getting changed and why.

Regards,
Khasim

[U-Boot] Fwd: TI:OMAP: [PATCH 6/7] Enable DSS driver for Beagle

[Tom]

Khasim Syed Mohammed wrote:
> On Sun, Jan 24, 2010 at 5:54 AM, Tom <Tom.Rix@windriver.com> wrote:
>> Khasim Syed Mohammed wrote:
>>> From fe824eb530338182503dd75d466374ac2e96c063 Mon Sep 17 00:00:00 2001
>>> From: Syed Mohammed Khasim <khasim@ti.com>
>>> Date: Mon, 18 Jan 2010 18:35:40 +0530
>>> Subject: [PATCH] Enable DSS driver for Beagle
>>>
>>> Configures DSS to display color bar on Svideo
>>> Configures DSS to display background color on DVID
>>>
>>> Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>

<snip>

>>> /*UART2_TX*/
>>>
>> This section is dss.
>> This should move to dss.c or dss.h
>>
>>> +/*
>>> + * Display Configuration
>>> + */
>>> +
>>> +#define DVI_BEAGLE_ORANGE_COL          0x00FF8000
>>> +#define VENC_HEIGHT                    0x00ef
>>> +#define VENC_WIDTH                     0x027f
> No, they are board specific, so should be in board file. They don't
> belong to DSS.

Ok

> 
>>> +
>>> +/*
>>> + * Configure VENC in DSS for Beagle to generate Color Bar
>>> + *
>>> + * Kindly refer to OMAP TRM for definition of these values.
>>> + */
>> No good enough.
>> If you are going to refer people to TRM.
>> Give them some idea how to find the data.
>> The name of TRM, the section and maybe quote to help them find
>> the location..
>>
> Sure.
> 
>> 15.5.8.4 Video Encoder Register Settings
>>
>> For video encoder programming, see Table 15-67.
>> This table lists the register values to use in standard
>> applications. These values are validated programming values only
>> for the TV display support (NTSC 601 and PAL 601 standards).
>> Table 15-67. Video Encoder Register Programming Values
>> Register Name        NTSC 601                            PAL 601
>> VENC_F_CONTROL       0x00000000                          0x00000000
>> VENC_VIDOUT_CTRL     0x00000001                          0x00000001
>> VENC_SYNC_CTRL       0x00008040                          0x00000040
>> VENC_LLEN            0x00000359                          0x0000035F
>> VENC_FLENS
>>
> HUH .... :) :) :)
> 
> I was under an impression that people who really code, read TRM and
> they don't need this level of details. I will give the section details
> for sure.

I don't mean for you to quote large sections of TRM like this
just provide enough information, title, version of doc, section
that will help the next person find where these values came from

> 
> But I feel you are asking some thing un necessarily more....  There
> are tons of programming, if every one does this then u-boot code will
> become a document portal. :)
> 
>> You need to qualify std_tv as either ntsc or pal.
>>
> I was not interested in confusing others on the ntsc or pal. But any
> way I test NTSC, I can change the name accordingly. I will not add the
> structure for PAL, will wait for some one to send out patch for PAL if
> interested.

Ok just ntsc

There should at least be a comment somewhere that this is ntsc.
I realize PAL is hard to test without a monitor.

> 
>> Include both setting.
>> If it is impossible to test PAL, make a note with a big warning
>>
> Why warning when I am saying that it is NTSC, can we not understand
> that NTSC is not PAL :)
> 
>>> +static const struct venc_regs venc_config_std_tv = {
>>> +       .status                                 = 0x0000001B,
>>> +       .f_control                              = 0x00000040,
>>> +       .vidout_ctrl                            = 0x00000000,
>> ?? not 1
>>
>>> +       .sync_ctrl                              = 0x00008000,
>> ?? not 8040 or 40
>>
>>
>>> +       .llen                                   = 0x00008359,
>> ?? not 359 or 35F
>>
>> Likely you are using a different TRM than I am.
>> Please check if there isn't some dependency on dss silicon
>> revision.
> There might be some differences, I am taking values that were working
> before for me from the kernel. If some one needs they can do the TRM
> comparison, PLEASE UNDERSTAND this is not for building your PRODUCT
> around this, it is for TESTING THE BOARD FOR TV OUT And nothing BEYOND
> THIS. Hope you understand the need for this driver in u-boot.
> 
> You always have access to TRM and register definition, they can see
> what is getting changed and why.

I was pointing out that without a version number of the TRM or a kernel
commit id, it is difficult to verify or change the values given.

All this needs is better comment on where the values came from.

Tom

> 
> Regards,
> Khasim