[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