[U-Boot] [PATCH 1/3] video: mb862xx: add option CONFIG_VIDEO_MB862xx_ACCEL for X888RGB mode

Wolfgang Grandegger wg at grandegger.com
Mon Oct 19 14:46:22 CEST 2009


Wolfgang Denk wrote:
> Dear Wolfgang,
> 
> In message <4ADC5661.7050508 at grandegger.com> you wrote:
>>>> The new IPEK01 board uses the X888RGB mode for the Lime graphics
>>>> controller. For this mode video accelaration does not work. This patch
>>>> makes the accelaration configurable via CONFIG_VIDEO_MB862xx_ACCEL,
>>>> which is enabled for the lwmon5 and the socrates board for backward
>>>> compatibility.
>>> Why would you want to disable it for IPEK01? Accelaration seems to be
>>> a good thing you don't give up if you don't have to, but I
>>> cannot think of reasons why you would have to do without it?
>> Because acceleration does work with 16 bpp but *not* with 32 bpp. That's
>> the reason why we made it configurable. Well, this patch could be
>> dropped, because the BSP for the IPEK01 posted here uses now 16 bpp as well.
> 
> Then please either mention this fact in the commit message (the
> current one does not say anything about 16 versus 32 bit mode), or
> realy drop the patch.

Well, X888RGB mode is a 32 bpp mode. I leave it up to Anatolij to accept
this patch or not (he is actually the original author).

>>>> --- u-boot-mainline.orig/drivers/video/cfb_console.c	2009-10-19 13:17:14.582303087 +0200
>>>> +++ u-boot-mainline/drivers/video/cfb_console.c	2009-10-19 13:17:29.406303158 +0200
>>> Please use git-format-patch to create patches.
>> Why? Do you have any problems to apply these patches? I personally
>> (still) prefer using quilt for patch stack management.
> 
> git-format-patch provides index information, which allows for
> intelligent merges (i. e. the merge code can then find the patch base
> and do a rebase internally). With your patches this is impossible.
> 
> Fell free to use quilt or any other tools for your own purposes, but
> for patch submission please prepare the patches using
> git-format-patch

OK.

>>>> +#else
>>>> +	unsigned int i, *p;
>>>> +
>>>> +	i = dev->winSizeX * dev->winSizeY;
>>>> +	p = (unsigned int *)dev->frameAdrs;
>>>> +	while (i--)
>>>> +		*p++ = 0;
>>>> +#endif
>>> Why don't you use memset() here?
>> Maybe to ensure that D32 accesses are performed. Anatolij might know?
> 
> How should Anatolij know? It is you who added this code, right?

No, this patch is from Anatolij and he has added his signed-off-by. My
signed-off-by is not correct, strictly speaking. I should have just
added an acked-by or tested-by line. Will change.

Wolfgang.


More information about the U-Boot mailing list