[U-Boot] [PATCH] Add suport for the graphic controller included in the JADE SOC

[Anatolij Gustschin <agust@denx.de>]

Matthias Weisser wrote:
>>>> Anatolij Gustschin <agust@denx.de> schrieb am 01.07.2009 um 15:40:
>> Dear Matthias,
>>
>> Thanks for this patch! Please see some comments/issues below which
> we
>> should resolve before committing the driver.
> 
> First of all thanks for your detailed comments. I fix them all before 
> resending the patch. Please see some additional comments at some
> points.
> 
>>> +		if(0 == bpp){
>> if (bpp == 0) {
> 
> The spaces are OK but why turn around the comparison? I always use 
> this style to prevent from unwanted assignments. Well, GCC warns 
> about assignments in ifs and as u-boot is GCC only I can do that.

it is a matter of personal preference. "if (0 == bpp) {" is OK for
me, too.

>> We also should use macros for register offsets, I think. Can we
>> coordinate efforts for fixing this in mb862xx driver also?
> 
> As the graphic controller in the jade soc is only a slight evolution
> of
> the coral graphic chip (additional display output/video input) I think
> 
> we may even merge the two drivers into one with some #ifdefs to deal
> with the differences.
> 
> When I started to implement the driver I decided against that idea
> because I don't have any hardware here to test the "non-jade" case.

if you want to use the existing driver, it shouldn't be too complicated.
To use the mb862xx driver you can implement "board_video_init()" in your
board code in which you initialize both display controllers, set
"GraphicDevice mb862xx" structure fields winSizeX, etc. and return
the video RAM base. Then additionally implement "board_get_regs()" in
your board code, e.g.

#include <mb862xx.h>

static const gdc_regs init_regs [] =
{
	{0, 0}
}

const gdc_regs *board_get_regs (void)
{
        return init_regs;
}

and define CONFIG_VIDEO_MB862xx in the board config file.

see e.g. the appropriate code in "board/lwmon5/lwmon5.c".
It does the display controller initialization differently and
therefore my suggestion to do the display controller init in
custom "board_video_init()".

We have to fix some register access macros in the mb862xx driver
for JADE and it should work. I can try to provide a patch for
this fix in mb862xx.c.

> But a common include file with the register offsets seems to be a
> good idea to me.

I'will prepare a list of register offset defines for "include/mb862xx.h"
and send a patch so that you can use them in your code.

Thanks,
Anatolij