Skip to contentSkip to conversation

Moving mx3fb.c to CONFIG_VIDEO

U-Boot New Helmut Raiger 4 participants 36 messages Aug 22, 2011 – Oct 14, 2011 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

The following patch partly rewrites mx3fb.c to be used along with cfb_console.c. It was using CONFIG_LCD and now needs CONFIG_VIDEO. The defines for some displays have been removed from the driver, instead one may specify displays using the videmode env variable.

For what I could see, it breaks support for qong and the phycore board. I'd appreciate, if someone (Stefano?) could test on these boards (using the correct videomode setting).

The phycore board specifies 8bit colors in the setup, the driver was only tested for a 565 set-up. Either the board setup is changed to 16bits or the driver needs some extra care.

--
Scanned by MailScanner.

#1 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

---
 arch/arm/cpu/arm1136/mx31/generic.c    |   14 ++++++++++++++
 arch/arm/include/asm/arch-mx31/clock.h |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)
arch/arm/cpu/arm1136/mx31/generic.c +14 -0
index 248431b..b077d29 100644
@@ -72,6 +72,18 @@ static u32 mx31_get_ipg_clk(void)
 	return freq;
 }
 
+/* hsp is the clock for the ipu */
+static u32 mx31_get_hsp_clk(void)
+{
+	u32 freq = mx31_get_mcu_main_clk();
+	u32 pdr0 = __REG(CCM_PDR0);
+
+	/* divided by HSP_PODF in PDR0 */
+	freq /= ((pdr0 >> 11) & 0x7) + 1;
+
+	return freq;
+}
+
 void mx31_dump_clocks(void)
 {
 	u32 cpufreq = mx31_get_mcu_main_clk();
@@ -88,6 +100,8 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
 	case MXC_CSPI_CLK:
 	case MXC_UART_CLK:
 		return mx31_get_ipg_clk();
+	case MXC_IPU_CLK:
+		return mx31_get_hsp_clk();
 	}
 	return -1;
 }
arch/arm/include/asm/arch-mx31/clock.h +1 -0
index fb035c4..b0aa1ce 100644
@@ -29,6 +29,7 @@ enum mxc_clock {
 	MXC_IPG_CLK,
 	MXC_CSPI_CLK,
 	MXC_UART_CLK,
+	MXC_IPU_CLK
 };
 
 unsigned int mxc_get_clock(enum mxc_clock clk);
Signature · 6 lines
1.7.4.4



--
Scanned by MailScanner.
#2 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

Quoted 11 lines · Helmut Raiger
---
 arch/arm/cpu/arm1136/mx31/generic.c    |   14 ++++++++++++++
 arch/arm/include/asm/arch-mx31/clock.h |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)
(quoted diff) +0 -0

b/arch/arm/cpu/arm1136/mx31/generic.c index 248431b..b077d29 100644

arch/arm/cpu/arm1136/mx31/generic.c static u32 mx31_get_ipg_clk(void) · line 79

Quoted 8 lines
arch/arm/cpu/arm1136/mx31/generic.c +5 -0
@@ -72,6 +72,18 @@ static u32 mx31_get_ipg_clk(void)
 	return freq;
 }
 
+/* hsp is the clock for the ipu */
+static u32 mx31_get_hsp_clk(void)
+{
+	u32 freq = mx31_get_mcu_main_clk();
+	u32 pdr0 = __REG(CCM_PDR0);

Can't we do readl() here?

arch/arm/cpu/arm1136/mx31/generic.c static u32 mx31_get_ipg_clk(void) · line 82

Quoted 3 lines
arch/arm/cpu/arm1136/mx31/generic.c +3 -0
+
+	/* divided by HSP_PODF in PDR0 */
+	freq /= ((pdr0 >> 11) & 0x7) + 1;

Um, don't we have defined constants for those magic numbers already?

Quoted 21 lines
arch/arm/cpu/arm1136/mx31/generic.c +6 -0
+
+	return freq;
+}
+
 void mx31_dump_clocks(void)
 {
 	u32 cpufreq = mx31_get_mcu_main_clk();
@@ -88,6 +100,8 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
 	case MXC_CSPI_CLK:
 	case MXC_UART_CLK:
 		return mx31_get_ipg_clk();
+	case MXC_IPU_CLK:
+		return mx31_get_hsp_clk();
 	}
 	return -1;
 }
(quoted diff) +0 -0

b/arch/arm/include/asm/arch-mx31/clock.h index fb035c4..b0aa1ce 100644

arch/arm/include/asm/arch-mx31/clock.h enum mxc_clock { · line 35

Quoted 7 lines
arch/arm/include/asm/arch-mx31/clock.h +1 -0
@@ -29,6 +29,7 @@ enum mxc_clock {
 	MXC_IPG_CLK,
 	MXC_CSPI_CLK,
 	MXC_UART_CLK,
+	MXC_IPU_CLK
 };
 
 unsigned int mxc_get_clock(enum mxc_clock clk);

Cheers

#3 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

Quoted 7 lines · Marek Vasut
arch/arm/cpu/arm1136/mx31/generic.c +1 -0
+	u32 pdr0 = __REG(CCM_PDR0);

Can't we do readl() here?

arch/arm/cpu/arm1136/mx31/generic.c static u32 mx31_get_ipg_clk(void)

Quoted 3 lines
arch/arm/cpu/arm1136/mx31/generic.c +3 -0
+
+	/* divided by HSP_PODF in PDR0 */
+	freq /= ((pdr0>>  11)&  0x7) + 1;

Um, don't we have defined constants for those magic numbers already?

Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the code, which was in the file. The generic.c file for MX35 however is a different story.

To keep things simple, I suggest we leave it at that for now.

Helmut

--
Scanned by MailScanner.

#4 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

Quoted 10 lines · Helmut Raiger
Quoted 8 lines · Marek Vasut
arch/arm/cpu/arm1136/mx31/generic.c +1 -0
+	u32 pdr0 = __REG(CCM_PDR0);

Can't we do readl() here?

arch/arm/cpu/arm1136/mx31/generic.c static u32 mx31_get_ipg_clk(void)

Quoted 3 lines
arch/arm/cpu/arm1136/mx31/generic.c +3 -0
+
+	/* divided by HSP_PODF in PDR0 */
+	freq /= ((pdr0>>  11)&  0x7) + 1;

Um, don't we have defined constants for those magic numbers already?

Quoted 5 lines

Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the code, which was in the file. The generic.c file for MX35 however is a different story.

To keep things simple, I suggest we leave it at that for now.

And noone's gonna fix it later. No please, define the proper constants please.

Thanks in advance

Quoted 9 lines

Helmut

--
Scanned by MailScanner.

#5 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

Quoted 8 lines · Marek Vasut
Quoted 6 lines

Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the code, which was in the file. The generic.c file for MX35 however is a different story.

To keep things simple, I suggest we leave it at that for now.

And noone's gonna fix it later. No please, define the proper constants please.

Do you really expect me to fix code in all the arch/arm/cpu/arm1136/mx31 files (2000+ lines) that use that kind of access, because I add 15 lines to one of them? There are some 50 other drivers and boards etc. that depend on imx-regs.h, which probably use these access methods as-well!

I'm not prepared to take this effort, sorry. After all this is _COSMETIC_, while this new mx3fb driver really works nicely ;-) If anyone out there modifies these files, I'll definitely rebase the driver to it.

Helmut

--
Scanned by MailScanner.

#6 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

Quoted 7 lines · Helmut Raiger
Quoted 6 lines · Marek Vasut
Quoted 5 lines

Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the code, which was in the file. The generic.c file for MX35 however is a different story.

To keep things simple, I suggest we leave it at that for now.

Quoted 7 lines
Quoted 2 lines

And noone's gonna fix it later. No please, define the proper constants please.

Do you really expect me to fix code in all the arch/arm/cpu/arm1136/mx31 files (2000+ lines) that use that kind of access, because I add 15 lines to one of them? There are some 50 other drivers and boards etc. that depend on imx-regs.h, which probably use these access methods as-well!

Hey, calm down, I didn't say anything about fixing gazilion of crap (even though that'd be nice of you !).

Actually, I did a quick look and see arch/arm/include/arch-mx35/crm_regs.h containing PDR0 definitions. Though they don't match your constants. It's either bug in the crm_regs.h or your code.

Quoted 3 lines

I'm not prepared to take this effort, sorry. After all this is _COSMETIC_, while this new mx3fb driver really works nicely ;-)

Oh my sense for metrosexuality of code really kicks in here ;-)

Quoted 2 lines

If anyone out there modifies these files, I'll definitely rebase the driver to it.

It has nothing to do with your driver, the problem I have is with these few magic numbers you use in 1/2 (make HSP clock ...). You can use constants from crm_regs.h, see above.

Quoted 2 lines

Helmut

Cheers!

Quoted 4 lines

--
Scanned by MailScanner.

#7 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

Quoted 16 lines · Marek Vasut
Quoted 7 lines · Helmut Raiger
Quoted 6 lines · Marek Vasut
Quoted 5 lines

Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the code, which was in the file. The generic.c file for MX35 however is a different story.

To keep things simple, I suggest we leave it at that for now.

Quoted 7 lines
Quoted 2 lines

And noone's gonna fix it later. No please, define the proper constants please.

Do you really expect me to fix code in all the arch/arm/cpu/arm1136/mx31 files (2000+ lines) that use that kind of access, because I add 15 lines to one of them? There are some 50 other drivers and boards etc. that depend on imx-regs.h, which probably use these access methods as-well!

Quoted 6 lines

Hey, calm down, I didn't say anything about fixing gazilion of crap (even though that'd be nice of you !).

Actually, I did a quick look and see arch/arm/include/arch-mx35/crm_regs.h containing PDR0 definitions. Though they don't match your constants. It's either bug in the crm_regs.h or your code.

Ah sorry, it's mx31. Right, the defines are missing. Do what Stefano said then. And really ... noone wants you to modify gazilions of lines, just your few :)

Cheers and sorry for misguiding you about mx35.

#8 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

In reply to Marek Vasut
Quoted 2 lines · Marek Vasut

... _COSMETIC_, while this new mx3fb driver really works nicely ;-) Oh my sense for metrosexuality of code really kicks in here ;-)

Arousal was not my first objective, but deliberately endorsed :-P

I'll do what can be done easily concerning the bit accesses. Helmut

--
Scanned by MailScanner.

#9 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

Quoted 5 lines · Helmut Raiger
Quoted 2 lines · Marek Vasut

... _COSMETIC_, while this new mx3fb driver really works nicely ;-) Oh my sense for metrosexuality of code really kicks in here ;-)

Arousal was not my first objective, but deliberately endorsed :-P

Hehe :-D

Quoted 3 lines

I'll do what can be done easily concerning the bit accesses. Helmut

Thanks!

Quoted 3 lines

--
Scanned by MailScanner.

#10 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

In reply to Helmut Raiger
Quoted 9 lines · Helmut Raiger
Quoted 8 lines · Marek Vasut
Quoted 6 lines

Masks are not defined for i.MX31 (imx-regs.h), so I simply followed the code, which was in the file. The generic.c file for MX35 however is a different story.

To keep things simple, I suggest we leave it at that for now.

And noone's gonna fix it later. No please, define the proper constants please.

Hi Helmut,

Quoted 3 lines

Do you really expect me to fix code in all the arch/arm/cpu/arm1136/mx31 files (2000+ lines) that use that kind of access, because I add 15 lines to one of them?

No, nobody thinks you should do this. However, should be not better to fix it for the 15 lines of code you added ?

Agree that a lot of things in imx-regs.h for i.MX31 are missing. The i.MX31 was the first i.MX processor added to u-boot, and there are still some parts that are different from the rest of IMX code.

However, can you add a simple macro to get the HSP_PODF field ? I agree it does not change the behavior, but it helps a bit to understand the code without running to the i.MX31 manual...

Macro to set the PODR register are already defined in imx-regs.h. As you pointed out, there is no macro to get values from the register.

Quoted 2 lines

There are some 50 other drivers and boards etc. that
depend on imx-regs.h, which probably use these access methods as-well!

Yes, I know. I am trying to uniform the behavior between i.MX processors.

Quoted 1 line

I'm not prepared to take this effort, sorry.

Do not do it, it is not asked. We make always a step-by-step approach. Add defines only for the part you added (as I can see, it is only related to get the HSP_PODF field in podr register), and let the rest of the 2000+ lines untouched.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#11 lore raw

PATCH 2/2 mx3fb: move to CONFIG_VIDEO to support videomodes

In reply to Helmut Raiger

mx3fb.c was based on CONFIG_LCD and is moved by this patch to CONFIG_VIDEO, which has greater freedom in selecting videomodes even at runtime.

This renders the accumulating list of display defines (CONFIG_DISPLAY_VBEST..., CONFIG_DISPLAY_C057...) obsolete as these may be setup through env variables:

setenv mydisplay "video=ctfb:x:240,y:320,depth:16,mode:0,pclk:185925,

	le:9,ri:17,up:7,lo:10,hs:1,vs:1,sync:100663296,vmode:0"

setenv videomode mydisplay

---
 drivers/video/Makefile      |    2 +-
 drivers/video/cfb_console.c |    7 +
 drivers/video/mx3fb.c       |  353 +++++++++++++++++++++++++------------------
 3 files changed, 213 insertions(+), 149 deletions(-)
drivers/video/Makefile +1 -1
index 086dc05..2c0e500 100644
@@ -34,7 +34,7 @@ COBJS-$(CONFIG_VIDEO_AMBA) += amba.o
 COBJS-$(CONFIG_VIDEO_CT69000) += ct69000.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MB862xx) += mb862xx.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MB86R0xGDC) += mb86r0xgdc.o videomodes.o
-COBJS-$(CONFIG_VIDEO_MX3) += mx3fb.o
+COBJS-$(CONFIG_VIDEO_MX3) += mx3fb.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MX5) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
 COBJS-$(CONFIG_VIDEO_SED13806) += sed13806.o
 COBJS-$(CONFIG_SED156X) += sed156x.o
drivers/video/cfb_console.c +7 -0
index 3a93b64..4e653b8 100644
@@ -161,6 +161,13 @@
 #endif
 
 /*
+ * Defines for the i.MX31 driver (mx3fb.c)
+ */
+#ifdef CONFIG_VIDEO_MX3
+#define VIDEO_FB_16BPP_WORD_SWAP
+#endif
+
+/*
  * Include video_fb.h after definitions of VIDEO_HW_RECTFILL etc.
  */
 #include <video_fb.h>
drivers/video/mx3fb.c +205 -148

494 lines not on this page · raw message

Signature · 6 lines
1.7.4.4



--
Scanned by MailScanner.
#12 lore raw

PATCH 2/2 mx3fb: move to CONFIG_VIDEO to support videomodes

On 08/22/2011 03:41 PM, Helmut Raiger wrote:

Hi Helmut,

Quoted 11 lines

mx3fb.c was based on CONFIG_LCD and is moved by this patch to CONFIG_VIDEO, which has greater freedom in selecting videomodes even at runtime.

This renders the accumulating list of display defines (CONFIG_DISPLAY_VBEST..., CONFIG_DISPLAY_C057...) obsolete as these may be setup through env variables:

setenv mydisplay "video=ctfb:x:240,y:320,depth:16,mode:0,pclk:185925,

	le:9,ri:17,up:7,lo:10,hs:1,vs:1,sync:100663296,vmode:0"

setenv videomode mydisplay

Really betters as hard coded in driver...

I see you updated the code synchronizing it with the linux driver. Add to the commit message the kernel version (better: the commit id) of the kernel you used as base for your changes, so that everybody in future can know where it comes from.

drivers/video/mx3fb.c union chan_param_mem { · line 369

Quoted 8 lines
drivers/video/mx3fb.c +8 -0
+DECLARE_GLOBAL_DATA_PTR;
+
+/* graphics setup */
+static GraphicDevice panel;
+static struct ctfb_res_modes *mode;
+static struct ctfb_res_modes var_mode;
+
+

One newline should be enough.

drivers/video/mx3fb.c static inline void reg_write(u32 value, unsigned long reg) · line 455

Quoted 8 lines

static inline u32 reg_read(unsigned long reg)
{

drivers/video/mx3fb.c +0 -1
 	return __REG(reg);
@@ -452,28 +381,39 @@ static inline void reg_write(u32 value, unsigned long reg)
  * sdc_init_panel() - initialize a synchronous LCD panel.
  * @width:		width of panel in pixels.
  * @height:		height of panel in pixels.
- * @pixel_fmt:		pixel format of buffer as FOURCC ASCII code.

pixel_fmt is still in the parameter list, and di_panel should be added to the description.

Quoted 9 lines
drivers/video/mx3fb.c +3 -2
  * @return:		0 on success or negative error code on failure.
  */
-static int sdc_init_panel(u16 width, u16 height, enum pixel_fmt pixel_fmt)
+static int sdc_init_panel(u16 width, u16 height,
+		enum pixel_fmt di_setup, enum ipu_panel di_panel)
 {
-	u32 reg;
+	u32 reg, div;
 	uint32_t old_conf;

drivers/video/mx3fb.c static inline void reg_write(u32 value, unsigned long reg) · line 398

Quoted 10 lines
drivers/video/mx3fb.c +6 -2
 
+	debug("%s(width=%d, height=%d)\n", __func__, width, height);
+
 	/* Init panel size and blanking periods */
-	reg = ((H_SYNC_WIDTH - 1) << 26) |
-		((u32)(width + H_START_WIDTH + H_END_WIDTH - 1) << 16);
+	reg = width + mode->left_margin + mode->right_margin - 1;
+	if (reg > 1023) {
+		debug("display width too large, coerced to 1023!");
+		reg = 1023;

I do not if it is better to try to adapt the values if the caller pass to the function wrong parameters. Probably it does not work. I think in these case it is better to output an error (print instead of debug) and return without with an error. What do you think ?

drivers/video/mx3fb.c static inline void reg_write(u32 value, unsigned long reg) · line 416

Quoted 7 lines
drivers/video/mx3fb.c +2 -1
-	switch (PANEL_TYPE) {
+	switch (di_panel) {
 	case IPU_PANEL_SHARP_TFT:
 		reg_write(0x00FD0102L, SDC_SHARP_CONF_1);
 		reg_write(0x00F500F4L, SDC_SHARP_CONF_2);
 		reg_write(SDC_COM_SHARP | SDC_COM_TFT_COLOR, SDC_COM_CONF);
+		/* TODO: probably IF_CONF must be adapted (see below)! */

I do not understand this comment.

drivers/video/mx3fb.c static int sdc_init_panel(u16 width, u16 height, enum pixel_fmt pixel_fmt) · line 428

Quoted 8 lines
drivers/video/mx3fb.c +2 -4
 	/* Init clocking */
 
-	/*
-	 * Calculate divider: fractional part is 4 bits so simply multiple by
-	 * 2^4 to get fractional part, as long as we stay under ~250MHz and on
-	 * i.MX31 it (HSP_CLK) is <= 178MHz. Currently 128.267MHz
+	/* Calculate divider: the IPU receives its clock from the hsp divder */
+	/* fractional part is 4 bits so simply multiple by 2^4 to get it

Multiline comments, you should use the same style as in the removed lines.

drivers/video/mx3fb.c static int sdc_init_panel(u16 width, u16 height, enum pixel_fmt pixel_fmt) · line 432

Quoted 1 line
drivers/video/mx3fb.c +1 -0
+	div = ((mxc_get_clock(MXC_IPU_CLK)/1024)*(mode->pixclock/128))/476837;

I try to understand this line. pixclock is in ps, as in kernel. I am missing something. I am expecting to have the same formula as in kernel, where I can read:

div = clk_get_rate(ipu_clk) * 16 / pixel_clk;

Where does 476837 come from ?

drivers/video/mx3fb.c static int sdc_init_panel(u16 width, u16 height, enum pixel_fmt pixel_fmt) · line 443

Quoted 11 lines
drivers/video/mx3fb.c +11 -0
+	debug("hsp_clk is %d, div=%d\n", mxc_get_clock(MXC_IPU_CLK), div);
+	/* coerce to not less than 4.0, not more than 255.9375 */
+	if (div < 0x40)
+		div = 0x40;
+	else if (div > 0xFFF)
+		div = 0xFFF;
+	/* DISP3_IF_CLK_DOWN_WR is half the divider value and 2 less
+	 * fraction bits. Subtract 1 extra from DISP3_IF_CLK_DOWN_WR
+	 * based on timing debug DISP3_IF_CLK_UP_WR is 0
+	 */
+	reg_write((((div / 8) - 1) << 22) | div, DI_DISP3_TIME_CONF);

Right. This code is now with the linux driver synchronized. If I could understand how div is computed and because it is different from the linux driver...

drivers/video/mx3fb.c void lcd_enable(void) · line 804

Quoted 10 lines
drivers/video/mx3fb.c +10 -0
+/*
+ * The current implementation is only tested for GDF_16BIT_565RGB!
+ * It was switched from the original CONFIG_LCD setup to CONFIG_VIDEO,
+ * because the lcd code seemed loaded with color table stuff, that
+ * does not relate to most modern TFTs. cfb_console.c looks more
+ * straight forward.
+ * This is the environment setting for the original setup
+     "unknown=video=ctfb:x:240,y:320,depth:16,mode:0,pclk:185925,le:9,ri:17,
+		up:7,lo:10,hs:1,vs:1,sync:100663296,vmode:0"
+     "videomode=unknown"

Multiline comment. As "original setup" you mean the setup if not CONFIG_DISPLAY_* was set, right ?

drivers/video/mx3fb.c void lcd_enable(void) · line 815

Quoted 12 lines
drivers/video/mx3fb.c +9 -2
+void *video_hw_init(void)
 {
-	return ((panel_info.vl_col * panel_info.vl_row *
-		NBITS(panel_info.vl_bpix)) / 8) + PAGE_SIZE;
+	char *penv;
+	u32 memsize;
+	unsigned long t1, hsynch, vsynch;
+	int bits_per_pixel, i, tmp, vesa_idx = 0, videomode;
+
+	tmp = 0;
+
+	videomode = CONFIG_SYS_DEFAULT_VIDEO_MODE;

Ok. This is a way to fix qong/phycore after merging these patches, and avoid that they do not work anymore if the videomode variable is not set. I write it down...

Quoted 21 lines
drivers/video/mx3fb.c +21 -0
+	/* get video mode via environment */
+	penv = getenv("videomode");
+	if (penv) {
+		/* decide if it is a string */
+		if (penv[0] <= '9') {
+			videomode = (int) simple_strtoul(penv, NULL, 16);
+			tmp = 1;
+		}
+	} else {
+		tmp = 1;
+	}
+	if (tmp) {
+		/* parameter are vesa modes */
+		/* search params */
+		for (i = 0; i < VESA_MODES_COUNT; i++) {
+			if (vesa_modes[i].vesanr == videomode)
+				break;
+		}
+		if (i == VESA_MODES_COUNT) {
+			printf("no VESA Mode found, switching to mode 0x%x ",
+					CONFIG_SYS_DEFAULT_VIDEO_MODE);

drivers/video/mx3fb.c void lcd_enable(void) · line 846

Quoted 10 lines
drivers/video/mx3fb.c +10 -0
+			i = 0;
+		}
+		mode = (struct ctfb_res_modes *)
+				&res_mode_init[vesa_modes[i].resindex];
+		bits_per_pixel = vesa_modes[i].bits_per_pixel;
+		vesa_idx = vesa_modes[i].resindex;
+	} else {
+		mode = (struct ctfb_res_modes *) &var_mode;
+		bits_per_pixel = video_get_params(mode, penv);
+	}

Anatolij, should be this code not general ? It is not strictly related to the i.MX. Should we put it in another place ?

Quoted 8 lines
drivers/video/mx3fb.c +8 -0
+
+	/* calculate hsynch and vsynch freq (info only) */
+	t1 = (mode->left_margin + mode->xres +
+	      mode->right_margin + mode->hsync_len) / 8;
+	t1 *= 8;
+	t1 *= mode->pixclock;
+	t1 /= 1000;
+	hsynch = 1000000000L / t1;

drivers/video/mx3fb.c void lcd_enable(void) · line 864

Quoted 10 lines
drivers/video/mx3fb.c +10 -0
+	t1 *= (mode->upper_margin + mode->yres +
+	       mode->lower_margin + mode->vsync_len);
+	t1 /= 1000;
+	vsynch = 1000000000L / t1;
+
+	/* fill in Graphic device struct */
+	sprintf(panel.modeIdent, "%dx%dx%d %ldkHz %ldHz",
+			mode->xres, mode->yres,
+			bits_per_pixel, (hsynch / 1000), (vsynch / 1000));
+	printf("%s\n", panel.modeIdent);

Should we not use "debug" instead ?

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#13 lore raw

PATCH 2/2 mx3fb: move to CONFIG_VIDEO to support videomodes

Quoted 4 lines · Stefano Babic

I see you updated the code synchronizing it with the linux driver. Add to the commit message the kernel version (better: the commit id) of the kernel you used as base for your changes, so that everybody in future can know where it comes from.

Ok.

Quoted 5 lines

drivers/video/mx3fb.c union chan_param_mem { · line 369

Quoted 4 lines
drivers/video/mx3fb.c +4 -0
+static struct ctfb_res_modes *mode;
+static struct ctfb_res_modes var_mode;
+
+

One newline should be enough.

Sure.

Quoted 3 lines
Quoted 1 line

- * @pixel_fmt: pixel format of buffer as FOURCC ASCII code.

pixel_fmt is still in the parameter list, and di_panel should be added to the description.

I'll update it.

Quoted 9 lines

drivers/video/mx3fb.c static inline void reg_write(u32 value, unsigned long reg) · line 398

Quoted 4 lines
drivers/video/mx3fb.c +4 -0
+	reg = width + mode->left_margin + mode->right_margin - 1;
+	if (reg>  1023) {
+		debug("display width too large, coerced to 1023!");
+		reg = 1023;

I do not if it is better to try to adapt the values if the caller pass to the function wrong parameters. Probably it does not work. I think in these case it is better to output an error (print instead of debug) and return without with an error. What do you think ?

I put that code in as I tried to adjust the porches (left and right margin) for our display.
If it is coerced the way I did, you'll never overwrite other bits in the same register field, so
you can still see the effect it has. I preferred it during display configuration, so I left it in.
We could output an error (not only during debug builds) but write the registers anyway.

Quoted 8 lines

drivers/video/mx3fb.c static inline void reg_write(u32 value, unsigned long reg) · line 416

Quoted 7 lines
drivers/video/mx3fb.c +2 -1
-	switch (PANEL_TYPE) {
+	switch (di_panel) {
  	case IPU_PANEL_SHARP_TFT:
  		reg_write(0x00FD0102L, SDC_SHARP_CONF_1);
  		reg_write(0x00F500F4L, SDC_SHARP_CONF_2);
  		reg_write(SDC_COM_SHARP | SDC_COM_TFT_COLOR, SDC_COM_CONF);
+		/* TODO: probably IF_CONF must be adapted (see below)! */

I do not understand this comment.

Each display specific define found an equivalent in the ctfb_res_modes struct.
IF_CONF however is currently always 0, but might need adaption for SHARP TFT panels, which
I could not test.

Quoted 9 lines

drivers/video/mx3fb.c static int sdc_init_panel(u16 width, u16 height, enum pixel_fmt pixel_fmt) · line 428

Quoted 8 lines

/* Init clocking */

drivers/video/mx3fb.c +2 -4
-	/*
-	 * Calculate divider: fractional part is 4 bits so simply multiple by
-	 * 2^4 to get fractional part, as long as we stay under ~250MHz and on
-	 * i.MX31 it (HSP_CLK) is<= 178MHz. Currently 128.267MHz
+	/* Calculate divider: the IPU receives its clock from the hsp divder */
+	/* fractional part is 4 bits so simply multiple by 2^4 to get it

Multiline comments, you should use the same style as in the removed lines.

Ok.

Quoted 8 lines

drivers/video/mx3fb.c static int sdc_init_panel(u16 width, u16 height, enum pixel_fmt pixel_fmt) · line 432

Quoted 1 line
drivers/video/mx3fb.c +1 -0
+	div = ((mxc_get_clock(MXC_IPU_CLK)/1024)*(mode->pixclock/128))/476837;

I try to understand this line. pixclock is in ps, as in kernel. I am missing something. I am expecting to have the same formula as in kernel, where I can read:

div = clk_get_rate(ipu_clk) * 16 / pixel_clk;

Where does 476837 come from ?

Well I already thought that might need another line of comment. In the kernel the pixel_clk really
is a clock value, while it is a time (in pico seconds) in this driver. I could have calculated the
pixel clock from the pixel time value first:

pixel_clk = 1e12 / mode->pixclock;
div = ipu_clk * 16 / pixel_clk;

I simply put that into one calculation:

div = ipu_clk * 16 / (1e12 / mode->pixclock)

or

div = ipu_clk * mode->pixclock / ~60e6;

but this would provoke an overflow problem during the multiplication, so I split the division to
1024, 128 and 476837 which exactly gives 1e12 / 16 (~60e6). So I have 2 shifts a multiplication and a division.

Probably I simply put the 2 code lines above into a comment. The name 'pixel_clk' is actually misleading, but it sat there already. We could use 'pixel_ps' in ctfb_res_modes instead?

Quoted 4 lines
Quoted 4 lines

+/*
+ * The current implementation is only tested for GDF_16BIT_565RGB!
+ * It was switched from the original CONFIG_LCD setup to CONFIG_VIDEO,
+ * because the lcd code seemed loaded with color table stuff, that

Quoted 8 lines

drivers/video/mx3fb.c void lcd_enable(void) · line 804

Quoted 6 lines

+ * does not relate to most modern TFTs. cfb_console.c looks more
+ * straight forward.
+ * This is the environment setting for the original setup

drivers/video/mx3fb.c +3 -0
+     "unknown=video=ctfb:x:240,y:320,depth:16,mode:0,pclk:185925,le:9,ri:17,
+		up:7,lo:10,hs:1,vs:1,sync:100663296,vmode:0"
+     "videomode=unknown"

Multiline comment. As "original setup" you mean the setup if not CONFIG_DISPLAY_* was set, right ?

I'll fix the comment and yes you're right.

Quoted 7 lines
Quoted 7 lines
drivers/video/mx3fb.c +4 -2
+void *video_hw_init(void)
  {
-	return ((panel_info.vl_col * panel_info.vl_row *
-		NBITS(panel_info.vl_bpix)) / 8) + PAGE_SIZE;
+	char *penv;
+	u32 memsize;
+	unsigned long t1, hsynch, vsynch;
Quoted 9 lines

drivers/video/mx3fb.c void lcd_enable(void) · line 815

Quoted 5 lines
drivers/video/mx3fb.c +5 -0
+	int bits_per_pixel, i, tmp, vesa_idx = 0, videomode;
+
+	tmp = 0;
+
+	videomode = CONFIG_SYS_DEFAULT_VIDEO_MODE;

Ok. This is a way to fix qong/phycore after merging these patches, and avoid that they do not work anymore if the videomode variable is not set. I write it down...

Perfect. I could have done that already, but lacking hardware to test with ...

Quoted 3 lines

Anatolij, should be this code not general ? It is not strictly related to the i.MX. Should we put it in another place ?

I thought of that as-well.

Quoted 7 lines

drivers/video/mx3fb.c void lcd_enable(void) · line 864

Quoted 6 lines
drivers/video/mx3fb.c +6 -0
+
+	/* fill in Graphic device struct */
+	sprintf(panel.modeIdent, "%dx%dx%d %ldkHz %ldHz",
+			mode->xres, mode->yres,
+			bits_per_pixel, (hsynch / 1000), (vsynch / 1000));
+	printf("%s\n", panel.modeIdent);

Should we not use "debug" instead ?

The kernel driver also outputs this during probe, but debug is fine with me.

Thanks for the review, I'll give it a few more days to settle and deliver a V2 then.
Helmut

--
Scanned by MailScanner.

#14 lore raw

PATCH 2/2 mx3fb: move to CONFIG_VIDEO to support videomodes

I just found that the mx3fb driver uses:

static inline u32 reg_read(unsigned long reg)

{
        return __REG(reg);
}

static inline void reg_write(u32 value, unsigned long reg)

{
        __REG(reg) = value;
}

I am about to change this to readl() and writel(), should I do it in a separate 'cosmetic only' patch before the really thing?
As it is more or less a re-write anyway (350 of 900 lines changed), it think its probably ok to do it in one shot.

Helmut

--
Scanned by MailScanner.

#15 lore raw

PATCH 2/2 mx3fb: move to CONFIG_VIDEO to support videomodes

Quoted 4 lines · Helmut Raiger

I just found that the mx3fb driver uses:

static inline u32 reg_read(unsigned long reg)

Quoted 9 lines
{
        return __REG(reg);
}

static inline void reg_write(u32 value, unsigned long reg)

{
        __REG(reg) = value;
}

i.MX31 is quite old, and it was pushed before general ARM accessors were introduced in u-boot. This is the reason. I appreciate you will fix this point.

Quoted 4 lines

I am about to change this to readl() and writel(), should I do it in a separate 'cosmetic only' patch before the really thing?
As it is more or less a re-write anyway (350 of 900 lines changed), it think its probably ok to do it in one shot.

IMHO you can do in one shot - you can mention this change in the commit message.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

A message that is not in the archive (sent off-list or not yet received): [email protected]

#17 lore raw

PATCH 2/2 mx3fb: move to CONFIG_VIDEO to support videomodes

Please ignore the 2 messages sent at 8:30, obviously the save and send buttons changed position.
Helmut

--
Scanned by MailScanner.

#18 lore raw
In reply to Helmut Raiger
Quoted 4 lines · Helmut Raiger

The following patch partly rewrites mx3fb.c to be used along with cfb_console.c. It was using CONFIG_LCD and now needs CONFIG_VIDEO. The defines for some displays have been removed from the driver, instead one may specify displays using the videmode env variable.

Hi Helmut,

thanks to fix this point. It was also on my todo list, but really not on the top, an I was waiting the occasion when I

The long list of hardcoded display configurations is very nasty, I am happy to get rid of it.

Please add also Anatolij in CC, as maintainer of the video subsystem.

Quoted 3 lines

For what I could see, it breaks support for qong and the phycore board. I'd appreciate, if someone (Stefano?) could test on these boards (using the correct videomode setting).

I am not sure I could test (I do not have now the board), but I will try. Maybe in next days.

Quoted 3 lines

The phycore board specifies 8bit colors in the setup, the driver was only tested for a 565 set-up. Either the board setup is changed to 16bits or the driver needs some extra care.

No chance to test the phycore on my side, maybe someone else....

Regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#19 lore raw

[U-Boot] Version 2 of 'Moving mx3fb to CONFIG_VIDEO'

In reply to Helmut Raiger

For details of version 2, please consult the patch history of the individual patches.

--
Scanned by MailScanner.

#20 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

This additionally updates mx31/generic.c by
- replacing __REG() macro accesses with readl() and writel()
- providing macros for PDR0 and PLL bit accesses
It also fixes a warning about the prototype of imx_get_uartclk(void)

---
V2: uses macros and readl(), writel(), see commit message

 arch/arm/cpu/arm1136/mx31/generic.c       |   40 +++++++++++++++++++---------
 arch/arm/include/asm/arch-mx31/clock.h    |    3 +-
 arch/arm/include/asm/arch-mx31/imx-regs.h |   14 ++++++++++
 3 files changed, 43 insertions(+), 14 deletions(-)
arch/arm/cpu/arm1136/mx31/generic.c +27 -13
index 248431b..9a7612b 100644
@@ -28,10 +28,10 @@
 
 static u32 mx31_decode_pll(u32 reg, u32 infreq)
 {
-	u32 mfi = (reg >> 10) & 0xf;
-	u32 mfn = reg & 0x3ff;
-	u32 mfd = (reg >> 16) & 0x3ff;
-	u32 pd =  (reg >> 26) & 0xf;
+	u32 mfi = GET_PLL_MFI(reg);
+	u32 mfn = GET_PLL_MFN(reg);
+	u32 mfd = GET_PLL_MFD(reg);
+	u32 pd =  GET_PLL_PD(reg);
 
 	mfi = mfi <= 5 ? 5 : mfi;
 	mfd += 1;
@@ -45,12 +45,12 @@ static u32 mx31_get_mpl_dpdgck_clk(void)
 {
 	u32 infreq;
 
-	if ((__REG(CCM_CCMR) & CCMR_PRCS_MASK) == CCMR_FPM)
+	if ((readl(CCM_CCMR) & CCMR_PRCS_MASK) == CCMR_FPM)
 		infreq = CONFIG_MX31_CLK32 * 1024;
 	else
 		infreq = CONFIG_MX31_HCLK_FREQ;
 
-	return mx31_decode_pll(__REG(CCM_MPCTL), infreq);
+	return mx31_decode_pll(readl(CCM_MPCTL), infreq);
 }
 
 static u32 mx31_get_mcu_main_clk(void)
@@ -64,10 +64,21 @@ static u32 mx31_get_mcu_main_clk(void)
 static u32 mx31_get_ipg_clk(void)
 {
 	u32 freq = mx31_get_mcu_main_clk();
-	u32 pdr0 = __REG(CCM_PDR0);
+	u32 pdr0 = readl(CCM_PDR0);
 
-	freq /= ((pdr0 >> 3) & 0x7) + 1;
-	freq /= ((pdr0 >> 6) & 0x3) + 1;
+	freq /= GET_PDR0_MAX_PODF(pdr0) + 1;
+	freq /= GET_PDR0_IPG_PODF(pdr0) + 1;
+
+	return freq;
+}
+
+/* hsp is the clock for the ipu */
+static u32 mx31_get_hsp_clk(void)
+{
+	u32 freq = mx31_get_mcu_main_clk();
+	u32 pdr0 = readl(CCM_PDR0);
+
+	freq /= GET_PDR0_HSP_PODF(pdr0) + 1;
 
 	return freq;
 }
@@ -77,6 +88,7 @@ void mx31_dump_clocks(void)
 	u32 cpufreq = mx31_get_mcu_main_clk();
 	printf("mx31 cpu clock: %dMHz\n",cpufreq / 1000000);
 	printf("ipg clock     : %dHz\n", mx31_get_ipg_clk());
+	printf("hsp clock     : %dHz\n", mx31_get_hsp_clk());
 }
 
 unsigned int mxc_get_clock(enum mxc_clock clk)
@@ -88,6 +100,8 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
 	case MXC_CSPI_CLK:
 	case MXC_UART_CLK:
 		return mx31_get_ipg_clk();
+	case MXC_IPU_CLK:
+		return mx31_get_hsp_clk();
 	}
 	return -1;
 }
@@ -104,10 +118,10 @@ void mx31_gpio_mux(unsigned long mode)
 	reg = IOMUXC_BASE + (mode & 0x1fc);
 	shift = (~mode & 0x3) * 8;
 
-	tmp = __REG(reg);
+	tmp = readl(reg);
 	tmp &= ~(0xff << shift);
 	tmp |= ((mode >> IOMUX_MODE_POS) & 0xff) << shift;
-	__REG(reg) = tmp;
+	writel(tmp, reg);
 }
 
 void mx31_set_pad(enum iomux_pins pin, u32 config)
@@ -118,10 +132,10 @@ void mx31_set_pad(enum iomux_pins pin, u32 config)
 	reg = (IOMUXC_BASE + 0x154) + (pin + 2) / 3 * 4;
 	field = (pin + 2) % 3;
 
-	l = __REG(reg);
+	l = readl(reg);
 	l &= ~(0x1ff << (field * 10));
 	l |= config << (field * 10);
-	__REG(reg) = l;
+	writel(l, reg);
 
 }
 
arch/arm/include/asm/arch-mx31/clock.h +2 -1
index fb035c4..1ea5a47 100644
@@ -29,10 +29,11 @@ enum mxc_clock {
 	MXC_IPG_CLK,
 	MXC_CSPI_CLK,
 	MXC_UART_CLK,
+	MXC_IPU_CLK
 };
 
 unsigned int mxc_get_clock(enum mxc_clock clk);
-extern u32 imx_get_uartclk();
+extern u32 imx_get_uartclk(void);
 extern void mx31_gpio_mux(unsigned long mode);
 extern void mx31_set_pad(enum iomux_pins pin, u32 config);
 
arch/arm/include/asm/arch-mx31/imx-regs.h +14 -0
index 3c8d607..c24dae2 100644
@@ -518,6 +518,20 @@ enum iomux_pins {
 #define PLL_MFI(x)		(((x) & 0xf) << 10)
 #define PLL_MFN(x)		(((x) & 0x3ff) << 0)
 
+#define GET_PDR0_CSI_PODF(x)	(((x) >> 23) & 0x1ff)
+#define GET_PDR0_PER_PODF(x)	(((x) >> 16) & 0x1f)
+#define GET_PDR0_HSP_PODF(x)	(((x) >> 11) & 0x7)
+#define GET_PDR0_NFC_PODF(x)	(((x) >> 8) & 0x7)
+#define GET_PDR0_IPG_PODF(x)	(((x) >> 6) & 0x3)
+#define GET_PDR0_MAX_PODF(x)	(((x) >> 3) & 0x7)
+#define GET_PDR0_MCU_PODF(x)	((x) & 0x7)
+
+#define GET_PLL_PD(x)		(((x) >> 26) & 0xf)
+#define GET_PLL_MFD(x)		(((x) >> 16) & 0x3ff)
+#define GET_PLL_MFI(x)		(((x) >> 10) & 0xf)
+#define GET_PLL_MFN(x)		(((x) >> 0) & 0x3ff)
+
+
 #define WEIM_ESDCTL0	0xB8001000
 #define WEIM_ESDCFG0	0xB8001004
 #define WEIM_ESDCTL1	0xB8001008
Signature · 6 lines
1.7.4.4



--
Scanned by MailScanner.
#21 lore raw

PATCH 1/2 mx31: make HSP clock for mx3fb driver available

Quoted 8 lines · Helmut Raiger

This additionally updates mx31/generic.c by
- replacing __REG() macro accesses with readl() and writel()
- providing macros for PDR0 and PLL bit accesses
It also fixes a warning about the prototype of imx_get_uartclk(void)

---
V2: uses macros and readl(), writel(), see commit message

This looks awesome. Please, when submitting V2, also add V2 to [PATCH 1/2] ... like [PATCH V2 1/2]. Minor thing though.

#22 lore raw

PATCH 2/2 Moving mx3fb.c to CONFIG_VIDEO

In reply to Helmut Raiger

mx3fb.c was based on CONFIG_LCD and is moved by this patch to CONFIG_VIDEO, which has greater freedom in selecting videomodes even at runtime.

This renders the accumulating list of display defines (CONFIG_DISPLAY_VBEST..., CONFIG_DISPLAY_C057...) obsolete as these may be setup through env variables:

setenv mydisplay
"video=ctfb:x:240,y:320,depth:16,mode:0,pclk:185925,

        le:9,ri:17,up:7,lo:10,hs:1,vs:1,sync:100663296,vmode:0"

setenv videomode mydisplay

---
V2:
- replace __REG() in mx3fb.c by readl()/writel()
- cosmetic changes (newlines, multiline comments)
- clock init and divider setup clarified
- printf() instead of debug() for width and height coerces

 drivers/video/Makefile      |    2 +-
 drivers/video/cfb_console.c |    7 +
 drivers/video/mx3fb.c       |  449 +++++++++++++++++++++++++------------------
 3 files changed, 265 insertions(+), 193 deletions(-)
drivers/video/Makefile +1 -1
index 086dc05..2c0e500 100644
@@ -34,7 +34,7 @@ COBJS-$(CONFIG_VIDEO_AMBA) += amba.o
 COBJS-$(CONFIG_VIDEO_CT69000) += ct69000.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MB862xx) += mb862xx.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MB86R0xGDC) += mb86r0xgdc.o videomodes.o
-COBJS-$(CONFIG_VIDEO_MX3) += mx3fb.o
+COBJS-$(CONFIG_VIDEO_MX3) += mx3fb.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MX5) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
 COBJS-$(CONFIG_VIDEO_SED13806) += sed13806.o
 COBJS-$(CONFIG_SED156X) += sed156x.o
drivers/video/cfb_console.c +7 -0
index 3a93b64..4e653b8 100644
@@ -161,6 +161,13 @@
 #endif
 
 /*
+ * Defines for the i.MX31 driver (mx3fb.c)
+ */
+#ifdef CONFIG_VIDEO_MX3
+#define VIDEO_FB_16BPP_WORD_SWAP
+#endif
+
+/*
  * Include video_fb.h after definitions of VIDEO_HW_RECTFILL etc.
  */
 #include <video_fb.h>
drivers/video/mx3fb.c +257 -192

674 lines not on this page · raw message

Signature · 6 lines
1.7.4.4



--
Scanned by MailScanner.
#23 lore raw

PATCH 2/2 Moving mx3fb.c to CONFIG_VIDEO

Quoted 4 lines · Helmut Raiger

mx3fb.c was based on CONFIG_LCD and is moved by this patch to CONFIG_VIDEO, which has greater freedom in selecting videomodes even at runtime.

Hi Anatolji,

do you have some comments about this patch ? Is it ok for you if I put it in my IMX queue ? The driver is for iMX-only, and I would check if everything remains consistent.

Thanks,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#24 lore raw

PATCH 2/2 Moving mx3fb.c to CONFIG_VIDEO

Hi Stefano, Helmut,

Quoted 11 lines · On Tue, 20 Sep 2011 13:44:33 +0200 Stefano Babic <[email protected]> wrote:
Quoted 4 lines · Helmut Raiger

mx3fb.c was based on CONFIG_LCD and is moved by this patch to CONFIG_VIDEO, which has greater freedom in selecting videomodes even at runtime.

Hi Anatolji,

do you have some comments about this patch ? Is it ok for you if I put it in my IMX queue ? The driver is for iMX-only, and I would check if everything remains consistent.

I'm not really ok with it as is. Currently we have video support for two boards in the tree using the mx3fb driver. Applying this patch as is will break the display support for qong and imx31_phycore. I'd prefer a patch fixing this, also e.g. fixing the defines in the affected board config files and adding the display configuration environment variables to CONFIG_EXTRA_ENV_SETTINGS.

Currently I do not have access to the imx31 boards to test/fix this. Stefano, can you test this patch on qong board to ensure that at least on one board the display support is working with changed mx3fb driver?

Best regards,
Anatolij

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de

#25 lore raw

PATCH 2/2 Moving mx3fb.c to CONFIG_VIDEO

Quoted 2 lines · Anatolij Gustschin

Hi Stefano, Helmut,

Hi Anatolij,

Quoted 11 lines

I'm not really ok with it as is. Currently we have video support for two boards in the tree using the mx3fb driver. Applying this patch as is will break the display support for qong and imx31_phycore. I'd prefer a patch fixing this, also e.g. fixing the defines in the affected board config files and adding the display configuration environment variables to CONFIG_EXTRA_ENV_SETTINGS.

Currently I do not have access to the imx31 boards to test/fix this. Stefano, can you test this patch on qong board to ensure that at least on one board the display support is working with changed mx3fb driver?

I tried, I cannot. We have not the LCD anymore for the QONG board ;-(( I have patched the QONG for fixing the build, but I cannot test it.

Best regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#26 lore raw

PATCH 2/2 Moving mx3fb.c to CONFIG_VIDEO

Quoted 5 lines · Stefano Babic

I tried, I cannot. We have not the LCD anymore for the QONG board ;-(( I have patched the QONG for fixing the build, but I cannot test it.

Best regards,
Stefano

I don't really know how to address this, but what if we simply insert a compile time warning into these board supports, like: "warning: Display setup defined, but settings not tested!".

Helmut

--
Scanned by MailScanner.

#27 lore raw

PATCH 2/2 Moving mx3fb.c to CONFIG_VIDEO

Quoted 11 lines · Helmut Raiger
Quoted 5 lines · Stefano Babic

I tried, I cannot. We have not the LCD anymore for the QONG board ;-(( I have patched the QONG for fixing the build, but I cannot test it.

Best regards,
Stefano

I don't really know how to address this, but what if we simply insert a compile time warning into these board supports, like: "warning: Display setup defined, but settings not tested!".

I have nothing against this...it seems at the moment your board is the only one doing something with the framebuffer. We will not get a QONG with display, and the other one (imx31_phycore) has no maintainer anymore and is one of the orphaned boards.

Best regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#28 lore raw

PATCH 2/2 Moving mx3fb.c to CONFIG_VIDEO

In reply to Stefano Babic
Quoted 1 line · On Tue, 20 Sep 2011 15:09:19 +0200 Stefano Babic <[email protected]> wrote:

Hi Anatolij,

Hi Stefano,

Quoted 7 lines
Quoted 7 lines

I'm not really ok with it as is. Currently we have video support for two boards in the tree using the mx3fb driver. Applying this patch as is will break the display support for qong and imx31_phycore. I'd prefer a patch fixing this, also e.g. fixing the defines in the affected board config files and adding the display configuration environment variables to CONFIG_EXTRA_ENV_SETTINGS.

Quoted 7 lines
Quoted 4 lines

Currently I do not have access to the imx31 boards to test/fix this. Stefano, can you test this patch on qong board to ensure that at least on one board the display support is working with changed mx3fb driver?

I tried, I cannot. We have not the LCD anymore for the QONG board ;-(( I have patched the QONG for fixing the build, but I cannot test it.

now I see that we have a 240x320 display for pcm037 board in the lab. The display works in Linux, but doesn't seem to work in u-boot, still do not know were the issue is, I've just tested it with v2011.09-rc2. I'll try to test the patch on the imx31_phycore board with our display this week.

Thanks,
Anatolij

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de

#29 lore raw

PATCH v3 1/2 mx31: make HSP clock for mx3fb driver available

In reply to Helmut Raiger

From: Helmut Raiger <[email protected]>

This additionally updates mx31/generic.c by
- replacing __REG() macro accesses with readl() and writel()
- providing macros for PDR0 and PLL bit accesses

---
Changes since v2:
- rebased to apply on current top of tree

 arch/arm/cpu/arm1136/mx31/generic.c       |   40 +++++++++++++++++++---------
 arch/arm/include/asm/arch-mx31/clock.h    |    1 +
 arch/arm/include/asm/arch-mx31/imx-regs.h |   14 ++++++++++
 3 files changed, 42 insertions(+), 13 deletions(-)
arch/arm/cpu/arm1136/mx31/generic.c +27 -13
index c6def5d..5524b23 100644
@@ -28,10 +28,10 @@
 
 static u32 mx31_decode_pll(u32 reg, u32 infreq)
 {
-	u32 mfi = (reg >> 10) & 0xf;
-	u32 mfn = reg & 0x3ff;
-	u32 mfd = (reg >> 16) & 0x3ff;
-	u32 pd =  (reg >> 26) & 0xf;
+	u32 mfi = GET_PLL_MFI(reg);
+	u32 mfn = GET_PLL_MFN(reg);
+	u32 mfd = GET_PLL_MFD(reg);
+	u32 pd =  GET_PLL_PD(reg);
 
 	mfi = mfi <= 5 ? 5 : mfi;
 	mfd += 1;
@@ -45,12 +45,12 @@ static u32 mx31_get_mpl_dpdgck_clk(void)
 {
 	u32 infreq;
 
-	if ((__REG(CCM_CCMR) & CCMR_PRCS_MASK) == CCMR_FPM)
+	if ((readl(CCM_CCMR) & CCMR_PRCS_MASK) == CCMR_FPM)
 		infreq = CONFIG_MX31_CLK32 * 1024;
 	else
 		infreq = CONFIG_MX31_HCLK_FREQ;
 
-	return mx31_decode_pll(__REG(CCM_MPCTL), infreq);
+	return mx31_decode_pll(readl(CCM_MPCTL), infreq);
 }
 
 static u32 mx31_get_mcu_main_clk(void)
@@ -64,10 +64,21 @@ static u32 mx31_get_mcu_main_clk(void)
 static u32 mx31_get_ipg_clk(void)
 {
 	u32 freq = mx31_get_mcu_main_clk();
-	u32 pdr0 = __REG(CCM_PDR0);
+	u32 pdr0 = readl(CCM_PDR0);
 
-	freq /= ((pdr0 >> 3) & 0x7) + 1;
-	freq /= ((pdr0 >> 6) & 0x3) + 1;
+	freq /= GET_PDR0_MAX_PODF(pdr0) + 1;
+	freq /= GET_PDR0_IPG_PODF(pdr0) + 1;
+
+	return freq;
+}
+
+/* hsp is the clock for the ipu */
+static u32 mx31_get_hsp_clk(void)
+{
+	u32 freq = mx31_get_mcu_main_clk();
+	u32 pdr0 = readl(CCM_PDR0);
+
+	freq /= GET_PDR0_HSP_PODF(pdr0) + 1;
 
 	return freq;
 }
@@ -77,6 +88,7 @@ void mx31_dump_clocks(void)
 	u32 cpufreq = mx31_get_mcu_main_clk();
 	printf("mx31 cpu clock: %dMHz\n",cpufreq / 1000000);
 	printf("ipg clock     : %dHz\n", mx31_get_ipg_clk());
+	printf("hsp clock     : %dHz\n", mx31_get_hsp_clk());
 }
 
 unsigned int mxc_get_clock(enum mxc_clock clk)
@@ -89,6 +101,8 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
 	case MXC_CSPI_CLK:
 	case MXC_UART_CLK:
 		return mx31_get_ipg_clk();
+	case MXC_IPU_CLK:
+		return mx31_get_hsp_clk();
 	}
 	return -1;
 }
@@ -105,10 +119,10 @@ void mx31_gpio_mux(unsigned long mode)
 	reg = IOMUXC_BASE + (mode & 0x1fc);
 	shift = (~mode & 0x3) * 8;
 
-	tmp = __REG(reg);
+	tmp = readl(reg);
 	tmp &= ~(0xff << shift);
 	tmp |= ((mode >> IOMUX_MODE_POS) & 0xff) << shift;
-	__REG(reg) = tmp;
+	writel(tmp, reg);
 }
 
 void mx31_set_pad(enum iomux_pins pin, u32 config)
@@ -119,10 +133,10 @@ void mx31_set_pad(enum iomux_pins pin, u32 config)
 	reg = (IOMUXC_BASE + 0x154) + (pin + 2) / 3 * 4;
 	field = (pin + 2) % 3;
 
-	l = __REG(reg);
+	l = readl(reg);
 	l &= ~(0x1ff << (field * 10));
 	l |= config << (field * 10);
-	__REG(reg) = l;
+	writel(l, reg);
 
 }
 
arch/arm/include/asm/arch-mx31/clock.h +1 -0
index cc99a75..2e3bce2 100644
@@ -30,6 +30,7 @@ enum mxc_clock {
 	MXC_IPG_PERCLK,
 	MXC_CSPI_CLK,
 	MXC_UART_CLK,
+	MXC_IPU_CLK
 };
 
 unsigned int mxc_get_clock(enum mxc_clock clk);
arch/arm/include/asm/arch-mx31/imx-regs.h +14 -0
index 0dcd9fe..f263e6b 100644
@@ -513,6 +513,20 @@ enum iomux_pins {
 #define PLL_MFI(x)		(((x) & 0xf) << 10)
 #define PLL_MFN(x)		(((x) & 0x3ff) << 0)
 
+#define GET_PDR0_CSI_PODF(x)	(((x) >> 23) & 0x1ff)
+#define GET_PDR0_PER_PODF(x)	(((x) >> 16) & 0x1f)
+#define GET_PDR0_HSP_PODF(x)	(((x) >> 11) & 0x7)
+#define GET_PDR0_NFC_PODF(x)	(((x) >> 8) & 0x7)
+#define GET_PDR0_IPG_PODF(x)	(((x) >> 6) & 0x3)
+#define GET_PDR0_MAX_PODF(x)	(((x) >> 3) & 0x7)
+#define GET_PDR0_MCU_PODF(x)	((x) & 0x7)
+
+#define GET_PLL_PD(x)		(((x) >> 26) & 0xf)
+#define GET_PLL_MFD(x)		(((x) >> 16) & 0x3ff)
+#define GET_PLL_MFI(x)		(((x) >> 10) & 0xf)
+#define GET_PLL_MFN(x)		(((x) >> 0) & 0x3ff)
+
+
 #define WEIM_ESDCTL0	0xB8001000
 #define WEIM_ESDCFG0	0xB8001004
 #define WEIM_ESDCTL1	0xB8001008
Signature · 1 line
1.7.1
#30 lore raw

PATCH v3 1/2 mx31: make HSP clock for mx3fb driver available

Hi Stefano,

Quoted 10 lines · On Thu, 13 Oct 2011 11:16:28 +0200 Anatolij Gustschin <[email protected]> wrote:

From: Helmut Raiger <[email protected]>

This additionally updates mx31/generic.c by
- replacing __REG() macro accesses with readl() and writel()
- providing macros for PDR0 and PLL bit accesses

Quoted 8 lines

---
Changes since v2:
- rebased to apply on current top of tree

 arch/arm/cpu/arm1136/mx31/generic.c       |   40 +++++++++++++++++++---------
 arch/arm/include/asm/arch-mx31/clock.h    |    1 +
 arch/arm/include/asm/arch-mx31/imx-regs.h |   14 ++++++++++
 3 files changed, 42 insertions(+), 13 deletions(-)

Are you okay with this patch? Should I queue it with another (video: Moving mx3fb.c to CONFIG_VIDEO) patch in my video tree?

Thanks,
Anatolij
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de

#31 lore raw

PATCH v3 1/2 mx31: make HSP clock for mx3fb driver available

Quoted 19 lines · Anatolij Gustschin

Hi Stefano,

Quoted 14 lines · On Thu, 13 Oct 2011 11:16:28 +0200 Anatolij Gustschin <[email protected]> wrote:

From: Helmut Raiger <[email protected]>

This additionally updates mx31/generic.c by
- replacing __REG() macro accesses with readl() and writel()
- providing macros for PDR0 and PLL bit accesses

---
Changes since v2:
- rebased to apply on current top of tree

Quoted 6 lines
Quoted 4 lines
 arch/arm/cpu/arm1136/mx31/generic.c       |   40 +++++++++++++++++++---------
 arch/arm/include/asm/arch-mx31/clock.h    |    1 +
 arch/arm/include/asm/arch-mx31/imx-regs.h |   14 ++++++++++
 3 files changed, 42 insertions(+), 13 deletions(-)

Are you okay with this patch?

Yes, they are ok - if not yet sent, here my:

Quoted 2 lines

Should I queue it with another
(video: Moving mx3fb.c to CONFIG_VIDEO) patch in my video tree?

Yes, they belong together - we should assure that all three goes to mainline in the same shot.

Thanks,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#32 lore raw

PATCH v3 1/2 mx31: make HSP clock for mx3fb driver available

In reply to Anatolij Gustschin
Quoted 10 lines · On Thu, 13 Oct 2011 11:16:28 +0200 Anatolij Gustschin <[email protected]> wrote:

From: Helmut Raiger <[email protected]>

This additionally updates mx31/generic.c by
- replacing __REG() macro accesses with readl() and writel()
- providing macros for PDR0 and PLL bit accesses

Quoted 8 lines

---
Changes since v2:
- rebased to apply on current top of tree

 arch/arm/cpu/arm1136/mx31/generic.c       |   40 +++++++++++++++++++---------
 arch/arm/include/asm/arch-mx31/clock.h    |    1 +
 arch/arm/include/asm/arch-mx31/imx-regs.h |   14 ++++++++++
 3 files changed, 42 insertions(+), 13 deletions(-)

Applied to u-boot-video master.

Anatolij

#33 lore raw

PATCH v3 2/2 video: Moving mx3fb.c to CONFIG_VIDEO

In reply to Helmut Raiger

From: Helmut Raiger <[email protected]>

mx3fb.c was based on CONFIG_LCD and is moved by this patch to CONFIG_VIDEO, which has greater freedom in selecting videomodes even at runtime.

This renders the accumulating list of display defines (CONFIG_DISPLAY_VBEST..., CONFIG_DISPLAY_C057...) obsolete as these may be setup through env variables:

uboot> setenv mydisplay 'video=ctfb:x:240,y:320,depth:16,mode:0,pclk:185925,

	le:9,ri:17,up:7,lo:10,hs:1,vs:1,sync:100663296,vmode:0'

uboot> setenv videomode ${mydisplay}

This commit also fixes the board config files for qong and imx31_phycore boards as needed. The videomode settings of previously supported displays are added to CONFIG_EXTRA_ENV_SETTINGS now. CONFIG_SYS_MALLOC_LEN for imx31_phycore board is increased to make the frame buffer allocation working with the changed driver.

---
Changes since v2:
- fixed commit message
- added display info for removed display timings of previously supported displays (in a comment)
- fixed compiler warning showed when compiling with debug enabled
- added "Video: " prefix before display info output (as we usually do it in U-Boot)
- fixed board config files for 'qong' and 'imx31_phycore' boards so that this commit doesn't break compiling (bisectable now)
- added videomode settings for previously supported displays on 'qong' and 'imx31_phycore' to CONFIG_EXTRA_ENV_SETTINGS
- increased CONFIG_SYS_MALLOC_LEN for 'imx31_phycore' so that the frame buffer can be allocated now (this is required to make the display support working with the changed mx3fb driver)

 drivers/video/Makefile          |    2 +-
 drivers/video/cfb_console.c     |    7 +
 drivers/video/mx3fb.c           |  459 +++++++++++++++++++++++----------------
 include/configs/imx31_phycore.h |   24 +-
 include/configs/qong.h          |   14 +-
 5 files changed, 297 insertions(+), 209 deletions(-)
drivers/video/Makefile +1 -1
index 3790f91..ecc1896 100644
@@ -35,7 +35,7 @@ COBJS-$(CONFIG_VIDEO_AMBA) += amba.o
 COBJS-$(CONFIG_VIDEO_CT69000) += ct69000.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MB862xx) += mb862xx.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MB86R0xGDC) += mb86r0xgdc.o videomodes.o
-COBJS-$(CONFIG_VIDEO_MX3) += mx3fb.o
+COBJS-$(CONFIG_VIDEO_MX3) += mx3fb.o videomodes.o
 COBJS-$(CONFIG_VIDEO_MX5) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
 COBJS-$(CONFIG_VIDEO_OMAP3) += omap3_dss.o videomodes.o
 COBJS-$(CONFIG_VIDEO_SED13806) += sed13806.o
drivers/video/cfb_console.c +7 -0
index 3a93b64..4e653b8 100644
@@ -161,6 +161,13 @@
 #endif
 
 /*
+ * Defines for the i.MX31 driver (mx3fb.c)
+ */
+#ifdef CONFIG_VIDEO_MX3
+#define VIDEO_FB_16BPP_WORD_SWAP
+#endif
+
+/*
  * Include video_fb.h after definitions of VIDEO_HW_RECTFILL etc.
  */
 #include <video_fb.h>
drivers/video/mx3fb.c +267 -192

684 lines not on this page · raw message

include/configs/imx31_phycore.h +13 -11
index 61f0534..48bd50b 100644
@@ -53,7 +53,7 @@
 /*
  * Size of malloc() pool
  */
-#define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + 128 * 1024)
+#define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + 512 * 1024)
 
 /*
  * Hardware drivers
@@ -104,7 +104,8 @@
 	"mtdparts=" MTDPARTS_DEFAULT "\0"										\
 	"prg_uboot=tftpboot 0x80000000 $(uboot); protect off 0xa0000000 +0x20000; erase 0xa0000000 +0x20000; cp.b 0x80000000 0xa0000000 $(filesize)\0" \
 	"prg_kernel=tftpboot 0x80000000 $(uimage); erase 0xa0040000 +0x180000; cp.b 0x80000000 0xa0040000 $(filesize)\0"	\
-	"prg_jffs2=tftpboot 0x80000000 $(jffs2); erase 0xa01c0000 0xa1ffffff; cp.b 0x80000000 0xa01c0000 $(filesize)\0"
+	"prg_jffs2=tftpboot 0x80000000 $(jffs2); erase 0xa01c0000 0xa1ffffff; cp.b 0x80000000 0xa01c0000 $(filesize)\0"	\
+	"videomode=video=ctfb:x:240,y:320,depth:16,mode:0,pclk:185925,le:9,ri:17,up:7,lo:10,hs:1,vs:1,sync:1241513985,vmode:0\0"
 
 
 #define CONFIG_SMC911X		1
@@ -201,15 +202,16 @@
 
 #define CONFIG_S6E63D6				1
 
-#define CONFIG_LCD				1
-#define CONFIG_VIDEO_MX3			1
-#define CONFIG_SYS_WHITE_ON_BLACK		1
-#define LCD_BPP					LCD_COLOR8
-#define	CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE	1
-#define CONFIG_SYS_CONSOLE_IS_IN_ENV		1
-
-#define	CONFIG_SPLASH_SCREEN			1
-#define CONFIG_CMD_BMP				1
+#define CONFIG_VIDEO
+#define CONFIG_CFB_CONSOLE
+#define CONFIG_VIDEO_MX3
+#define CONFIG_VIDEO_LOGO
+#define CONFIG_VIDEO_SW_CURSOR
+#define CONFIG_VGA_AS_SINGLE_DEVICE
+#define CONFIG_SYS_CONSOLE_IS_IN_ENV
+#define CONFIG_SPLASH_SCREEN
+#define CONFIG_CMD_BMP
+#define CONFIG_BMP_16BPP
 #endif
 
 #endif /* __CONFIG_H */
include/configs/qong.h +9 -5
index e4bc1d0..b1fc80c 100644
@@ -79,15 +79,16 @@
 #define CONFIG_DNET_BASE	(CS1_BASE + QONG_FPGA_PERIPH_SIZE)
 
 /* Framebuffer and LCD */
-#define CONFIG_LCD
+#define CONFIG_VIDEO
+#define CONFIG_CFB_CONSOLE
 #define CONFIG_VIDEO_MX3
-#define	CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE
+#define CONFIG_VIDEO_LOGO
+#define CONFIG_VIDEO_SW_CURSOR
+#define CONFIG_VGA_AS_SINGLE_DEVICE
 #define CONFIG_SYS_CONSOLE_IS_IN_ENV
-#define LCD_BPP		LCD_COLOR16
-#define	CONFIG_SPLASH_SCREEN
+#define CONFIG_SPLASH_SCREEN
 #define CONFIG_CMD_BMP
 #define CONFIG_BMP_16BPP
-#define CONFIG_DISPLAY_COM57H5M10XRC
 
 /* USB */
 #define CONFIG_CMD_USB
@@ -177,6 +178,9 @@
 		" +${filesize};cp.b ${fileaddr} "			\
 		xstr(CONFIG_SYS_MONITOR_BASE) " ${filesize}\0"		\
 	"upd=run load update\0"						\
+	"videomode=video=ctfb:x:640,y:480,depth:16,mode:0,pclk:40000,"	\
+		"le:120,ri:40,up:35,lo:10,hs:30,vs:3,sync:100663296,"	\
+		"vmode:0\0"						\
 
 /*
  * Miscellaneous configurable options
Signature · 1 line
1.7.1
#34 lore raw

PATCH v3 2/2 video: Moving mx3fb.c to CONFIG_VIDEO

Quoted 24 lines · Anatolij Gustschin

From: Helmut Raiger<[email protected]>

mx3fb.c was based on CONFIG_LCD and is moved by this patch to CONFIG_VIDEO, which has greater freedom in selecting videomodes even at runtime.

This renders the accumulating list of display defines (CONFIG_DISPLAY_VBEST..., CONFIG_DISPLAY_C057...) obsolete as these may be setup through env variables:

uboot> setenv mydisplay 'video=ctfb:x:240,y:320,depth:16,mode:0,pclk:185925,

	le:9,ri:17,up:7,lo:10,hs:1,vs:1,sync:100663296,vmode:0'

uboot> setenv videomode ${mydisplay}

This commit also fixes the board config files for qong and imx31_phycore boards as needed. The videomode settings of previously supported displays are added to CONFIG_EXTRA_ENV_SETTINGS now. CONFIG_SYS_MALLOC_LEN for imx31_phycore board is increased to make the frame buffer allocation working with the changed driver.

Quoted 17 lines

---
Changes since v2:
- fixed commit message
- added display info for removed display timings of previously supported displays (in a comment)
- fixed compiler warning showed when compiling with debug enabled
- added "Video: " prefix before display info output (as we usually do it in U-Boot)
- fixed board config files for 'qong' and 'imx31_phycore' boards so that this commit doesn't break compiling (bisectable now)
- added videomode settings for previously supported displays on 'qong' and 'imx31_phycore' to CONFIG_EXTRA_ENV_SETTINGS
- increased CONFIG_SYS_MALLOC_LEN for 'imx31_phycore' so that the frame buffer can be allocated now (this is required to make the display support working with the changed mx3fb driver)

Hi Anatolij,

I'm fine with your changes. It still works on our board. Thanks for
testing and improving.

Helmut

--
Scanned by MailScanner.

#35 lore raw

PATCH v3 2/2 video: Moving mx3fb.c to CONFIG_VIDEO

Hi Helmut,

On Fri, 14 Oct 2011 08:37:48 +0200
Helmut Raiger <[email protected]> wrote:
...

Quoted 4 lines

Hi Anatolij,

I'm fine with your changes. It still works on our board. Thanks for
testing and improving.

Great, thanks for the feedback and testing!

Anatolij

#36 lore raw

PATCH v3 2/2 video: Moving mx3fb.c to CONFIG_VIDEO

In reply to Anatolij Gustschin
Quoted 41 lines · On Thu, 13 Oct 2011 11:16:29 +0200 Anatolij Gustschin <[email protected]> wrote:

From: Helmut Raiger <[email protected]>

mx3fb.c was based on CONFIG_LCD and is moved by this patch to CONFIG_VIDEO, which has greater freedom in selecting videomodes even at runtime.

This renders the accumulating list of display defines (CONFIG_DISPLAY_VBEST..., CONFIG_DISPLAY_C057...) obsolete as these may be setup through env variables:

uboot> setenv mydisplay 'video=ctfb:x:240,y:320,depth:16,mode:0,pclk:185925,

	le:9,ri:17,up:7,lo:10,hs:1,vs:1,sync:100663296,vmode:0'

uboot> setenv videomode ${mydisplay}

This commit also fixes the board config files for qong and imx31_phycore boards as needed. The videomode settings of previously supported displays are added to CONFIG_EXTRA_ENV_SETTINGS now. CONFIG_SYS_MALLOC_LEN for imx31_phycore board is increased to make the frame buffer allocation working with the changed driver.

---
Changes since v2:
- fixed commit message
- added display info for removed display timings of previously supported displays (in a comment)
- fixed compiler warning showed when compiling with debug enabled
- added "Video: " prefix before display info output (as we usually do it in U-Boot)
- fixed board config files for 'qong' and 'imx31_phycore' boards so that this commit doesn't break compiling (bisectable now)
- added videomode settings for previously supported displays on 'qong' and 'imx31_phycore' to CONFIG_EXTRA_ENV_SETTINGS
- increased CONFIG_SYS_MALLOC_LEN for 'imx31_phycore' so that the frame buffer can be allocated now (this is required to make the display support working with the changed mx3fb driver)

Quoted 6 lines
 drivers/video/Makefile          |    2 +-
 drivers/video/cfb_console.c     |    7 +
 drivers/video/mx3fb.c           |  459 +++++++++++++++++++++++----------------
 include/configs/imx31_phycore.h |   24 +-
 include/configs/qong.h          |   14 +-
 5 files changed, 297 insertions(+), 209 deletions(-)

Applied to u-boot-video master.

Anatolij