[PATCH] viafb: Use proper register for bgcolor when doing MONO blits

[PATCH] viafb: Use proper register for bgcolor when doing MONO blits

[Daniel Drake]

From: Jon Nettleton <jon.nettleton@gmail.com>

Seemingly random colours were being painted as the background on the
terminal. This fixes MONO color blits.

Fixes http://dev.laptop.org/ticket/9323

Signed-off-by: Daniel Drake <dsd@laptop.org>

---
 drivers/video/via/accel.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/via/accel.c b/drivers/video/via/accel.c
index e44893e..8d9819c 100644
--- a/drivers/video/via/accel.c
+++ b/drivers/video/via/accel.c
@@ -287,7 +287,7 @@ static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
 		writel(fg_color, engine + 0x4C);
 
 	if (op = VIA_BITBLT_MONO)
-		writel(bg_color, engine + 0x50);
+		writel(bg_color, engine + 0x58);
 
 	if (op = VIA_BITBLT_FILL)
 		ge_cmd |= fill_rop << 24 | 0x00002000 | 0x00000001;
-- 
1.7.2.2

Re: [PATCH] viafb: Use proper register for bgcolor when doing MONO

[Florian Tobias Schandinat]

Daniel Drake schrieb:
> From: Jon Nettleton <jon.nettleton@gmail.com>
> 
> Seemingly random colours were being painted as the background on the
> terminal. This fixes MONO color blits.

Not really. For color blits it looks like we're doing the right thing here, at 
least it works on VX800 as I'd expect it (when I change the bg_color to red I 
get the characters with a red background, after applying your patch I get them 
with a black background). To me it looks like we should set this register for 
fillrect operations (as after your patch is applied I get a red backround for 
the space where no characters are).


Thanks,

Florian Tobias Schandinat

> 
> Fixes http://dev.laptop.org/ticket/9323
> 
> Signed-off-by: Daniel Drake <dsd@laptop.org>
> 
> ---
>  drivers/video/via/accel.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/via/accel.c b/drivers/video/via/accel.c
> index e44893e..8d9819c 100644
> --- a/drivers/video/via/accel.c
> +++ b/drivers/video/via/accel.c
> @@ -287,7 +287,7 @@ static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
>  		writel(fg_color, engine + 0x4C);
>  
>  	if (op = VIA_BITBLT_MONO)
> -		writel(bg_color, engine + 0x50);
> +		writel(bg_color, engine + 0x58);
>  
>  	if (op = VIA_BITBLT_FILL)
>  		ge_cmd |= fill_rop << 24 | 0x00002000 | 0x00000001;

Re: [PATCH] viafb: Use proper register for bgcolor when doing MONO blits

[Daniel Drake]

On 21 September 2010 18:00, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> Not really. For color blits it looks like we're doing the right thing here,
> at least it works on VX800 as I'd expect it (when I change the bg_color to
> red I get the characters with a red background, after applying your patch I
> get them with a black background). To me it looks like we should set this
> register for fillrect operations (as after your patch is applied I get a red
> backround for the space where no characters are).

Odd. That's exactly what we get on VX855 before the patch.

Jon, any comments?

Daniel

Re: [PATCH] viafb: Use proper register for bgcolor when doing MONO blits

[Jon Nettleton]

On Tue, Sep 21, 2010 at 10:07 AM, Daniel Drake <dsd@laptop.org> wrote:
> On 21 September 2010 18:00, Florian Tobias Schandinat
> <FlorianSchandinat@gmx.de> wrote:
>> Not really. For color blits it looks like we're doing the right thing here,
Yes you are but the code says.

 if (op = VIA_BITBLT_MONO)

*snip*
>
> Odd. That's exactly what we get on VX855 before the patch.
>

I must admit I didn't look at the code thoroughly because I am more
interested in getting my KMS code finished.  The proper fix is to have
color blits use fgcolor 0x4C and bgcolor use 0x50 and mono blits use
fgcolor 0x58 and bgcolor 0x5C.

My patch would have broken color blits, but is technically correct.
Hope that gives you the information you need to properly fix the
problem.

Jon

Re: [PATCH] viafb: Use proper register for bgcolor when doing MONO

[Florian Tobias Schandinat]

Hi,

Jon Nettleton schrieb:
> On Tue, Sep 21, 2010 at 10:07 AM, Daniel Drake <dsd@laptop.org> wrote:
>> On 21 September 2010 18:00, Florian Tobias Schandinat
>> <FlorianSchandinat@gmx.de> wrote:
>>> Not really. For color blits it looks like we're doing the right thing here,
> Yes you are but the code says.
> 
>  if (op = VIA_BITBLT_MONO)

I wrote a little test case:
 >>> colortest.sh
#!/bin/sh

echo -e "\033[30;47mblit: BLACK_ON_WHITE\033[0m"
echo -e "\033[37;40mblit: WHITE_ON_BLACK\033[0m"
echo -e "\033[31;44mbilt: RED_ON_BLUE\033[0m"
echo -e "fillrect: YELLOW\033[43m"
sleep 5
clear
echo -e "\033[0m"
<<<
The first three cases behaved on VX800 as well as XO 1.5 (with stock kernel as 
well as self-compiled) as expected, only the last went wrong (the cleared 
background was black instead yellow). Yes that is a bug but nut anything in the 
blitting code but in the VIA_BITBLT_FILL code where I admit that we write the 
colour to the wrong register. Will try to get a proper patch for that in the 
next few days (although I'm very, very busy at the moment).


Thanks,

Florian Tobias Schandinat

Re: [PATCH] viafb: Use proper register for bgcolor when doing MONO blits

[Jon Nettleton]

> #!/bin/sh
>
> echo -e "\033[30;47mblit: BLACK_ON_WHITE\033[0m"
> echo -e "\033[37;40mblit: WHITE_ON_BLACK\033[0m"
> echo -e "\033[31;44mbilt: RED_ON_BLUE\033[0m"
> echo -e "fillrect: YELLOW\033[43m"
> sleep 5
> clear
> echo -e "\033[0m"
> <<<
Ah that is a mono image blit not a mono pattern blit.  then that
register is correct.

Yes that is a bug but nut anything in
> the blitting code but in the VIA_BITBLT_FILL code where I admit that we
> write the colour to the wrong register. Will try to get a proper patch for
> that in the next few days

That code-block should look something like this then.


    if (op != VIA_BITBLT_FILL) {
        writel(fg_color, engine + 0x4C);
        if (op = VIA_BITBLT_MONO)
            writel(bg_color, engine + 0x50);
    } else {
        writel(fg_color, engine + 0x58);
    }

-Jon

[PATCH] viafb: use proper register for colour when doing fill ops

[Florian Tobias Schandinat]

The colour was written to a wrong register for fillrect operations.
This sometimes caused empty console space (for example after 'clear')
to have a different colour than desired. Fix this by writing to the
correct register.
Many thanks to Daniel Drake and Jon Nettleton for pointing out this
issue and pointing me in the right direction for the fix.

Fixes http://dev.laptop.org/ticket/9323

Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Joseph Chan <JosephChan@via.com.tw>
Cc: Daniel Drake <dsd@laptop.org>
Cc: Jon Nettleton <jon.nettleton@gmail.com>
---
 drivers/video/via/accel.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/video/via/accel.c b/drivers/video/via/accel.c
index e44893e..c2f4e6e 100644
--- a/drivers/video/via/accel.c
+++ b/drivers/video/via/accel.c
@@ -283,11 +283,12 @@ static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
 		writel(tmp, engine + 0x1C);
 	}
 
-	if (op != VIA_BITBLT_COLOR)
+	if (op = VIA_BITBLT_FILL) {
+		writel(fg_color, engine + 0x58);
+	} else if (op = VIA_BITBLT_MONO) {
 		writel(fg_color, engine + 0x4C);
-
-	if (op = VIA_BITBLT_MONO)
 		writel(bg_color, engine + 0x50);
+	}
 
 	if (op = VIA_BITBLT_FILL)
 		ge_cmd |= fill_rop << 24 | 0x00002000 | 0x00000001;
-- 
1.6.3.2