diff mbox series

mach64: fix console corruption in 24bpp mode

Message ID alpine.LRH.2.02.1808171514010.31883@file01.intranet.prod.int.rdu2.redhat.com (mailing list archive)
State New, archived
Headers show
Series mach64: fix console corruption in 24bpp mode | expand

Commit Message

Mikulas Patocka Aug. 17, 2018, 7:15 p.m. UTC
There's console font corruption when using the mach64 driver in 24bpp
mode.

In 24bpp mode, the mach64 accelerator is set up for 8-bpp mode (with
horizontal width and stride multiplied by 3). In this mode, the
accelerator can't even possibly support color expansion. Consquently, we
have to use an unaccelerated function cfb_imageblit for color expansion.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org

---
 drivers/video/fbdev/aty/mach64_accel.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Ville Syrjälä Aug. 19, 2018, 10:26 a.m. UTC | #1
On Fri, Aug 17, 2018 at 03:15:52PM -0400, Mikulas Patocka wrote:
> There's console font corruption when using the mach64 driver in 24bpp
> mode.
> 
> In 24bpp mode, the mach64 accelerator is set up for 8-bpp mode (with
> horizontal width and stride multiplied by 3). In this mode, the
> accelerator can't even possibly support color expansion. Consquently, we
> have to use an unaccelerated function cfb_imageblit for color expansion.

Hmm. I would think it should work just fine since we feed in each bit
three times and the hw 24bpp rotate thing should take care of selecting
the right component.

-               if (M64_HAS(HW_TRIPLE) && image->width % 8 == 0)
+               if (M64_HAS(HW_TRIPLE) && width % 8 == 0)
		                        pix_width |= DP_HOST_TRIPLE_EN;
perhaps?

> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: stable@vger.kernel.org
> 
> ---
>  drivers/video/fbdev/aty/mach64_accel.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c
> ===================================================================
> --- linux-stable.orig/drivers/video/fbdev/aty/mach64_accel.c	2018-04-20 18:11:01.000000000 +0200
> +++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c	2018-08-13 17:37:04.000000000 +0200
> @@ -291,7 +291,8 @@ void atyfb_imageblit(struct fb_info *inf
>  	if (!image->width || !image->height)
>  		return;
>  	if (!par->accel_flags ||
> -	    (image->depth != 1 && info->var.bits_per_pixel != image->depth)) {
> +	    (image->depth != 1 && info->var.bits_per_pixel != image->depth) ||
> +	    (image->depth == 1 && info->var.bits_per_pixel == 24)) {
>  		cfb_imageblit(info, image);
>  		return;
>  	}
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Mikulas Patocka Aug. 25, 2018, 7:48 p.m. UTC | #2
On Sun, 19 Aug 2018, Ville Syrjälä wrote:

> On Fri, Aug 17, 2018 at 03:15:52PM -0400, Mikulas Patocka wrote:
> > There's console font corruption when using the mach64 driver in 24bpp
> > mode.
> > 
> > In 24bpp mode, the mach64 accelerator is set up for 8-bpp mode (with
> > horizontal width and stride multiplied by 3). In this mode, the
> > accelerator can't even possibly support color expansion. Consquently, we
> > have to use an unaccelerated function cfb_imageblit for color expansion.
> 
> Hmm. I would think it should work just fine since we feed in each bit
> three times and the hw 24bpp rotate thing should take care of selecting
> the right component.
> 
> -               if (M64_HAS(HW_TRIPLE) && image->width % 8 == 0)
> +               if (M64_HAS(HW_TRIPLE) && width % 8 == 0)
> 		                        pix_width |= DP_HOST_TRIPLE_EN;
> perhaps?

This doesn't have any effect (width is image->width * 3 and multiplication 
by 3 doesn't affect divisibility by 8).

But I have looked at it deeper and I realized that we could fix the screen 
corruption and use the accelerator in 24bpp mode. I'll send the patches in 
following emails.

There's endianity problem in the code that does manual bit triple in 
atyfb_imageblit (I am using this driver on sparc64). After fixing this, 
some of the corruption is fixed, but not all of it (there's still 
corruption when the screen is scrolled).

Another bug is that the function atyfb_imageblit is reading the 
accelerator registers without waiting for the idle engine - so it reads 
some indetermediate value of some previous command. I reworked the 
accelerator functions so that they don't read the registers at all. After 
fixing these two bugs, I don't observe any corruption at all.

I also noticed that loading the bitmap is inefficient - for every word 
written, the driver reads the fifo status register and these reads are 
slow and cannot be queued. I improved the function wait_for_fifo, so that 
if there's more space than we need, it remembers the surplus and it 
doesn't have to read the register next time.

Mikulas
Mikulas Patocka Aug. 25, 2018, 7:55 p.m. UTC | #3
On Fri, 17 Aug 2018, Mikulas Patocka wrote:

> There's console font corruption when using the mach64 driver in 24bpp
> mode.
> 
> In 24bpp mode, the mach64 accelerator is set up for 8-bpp mode (with
> horizontal width and stride multiplied by 3). In this mode, the
> accelerator can't even possibly support color expansion. Consquently, we
> have to use an unaccelerated function cfb_imageblit for color expansion.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: stable@vger.kernel.org

Self-nack for this patch.

I've sent further patches that fix the display corruption without 
disabling the accelerator.

Mikulas

> ---
>  drivers/video/fbdev/aty/mach64_accel.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c
> ===================================================================
> --- linux-stable.orig/drivers/video/fbdev/aty/mach64_accel.c	2018-04-20 18:11:01.000000000 +0200
> +++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c	2018-08-13 17:37:04.000000000 +0200
> @@ -291,7 +291,8 @@ void atyfb_imageblit(struct fb_info *inf
>  	if (!image->width || !image->height)
>  		return;
>  	if (!par->accel_flags ||
> -	    (image->depth != 1 && info->var.bits_per_pixel != image->depth)) {
> +	    (image->depth != 1 && info->var.bits_per_pixel != image->depth) ||
> +	    (image->depth == 1 && info->var.bits_per_pixel == 24)) {
>  		cfb_imageblit(info, image);
>  		return;
>  	}
>
diff mbox series

Patch

Index: linux-stable/drivers/video/fbdev/aty/mach64_accel.c
===================================================================
--- linux-stable.orig/drivers/video/fbdev/aty/mach64_accel.c	2018-04-20 18:11:01.000000000 +0200
+++ linux-stable/drivers/video/fbdev/aty/mach64_accel.c	2018-08-13 17:37:04.000000000 +0200
@@ -291,7 +291,8 @@  void atyfb_imageblit(struct fb_info *inf
 	if (!image->width || !image->height)
 		return;
 	if (!par->accel_flags ||
-	    (image->depth != 1 && info->var.bits_per_pixel != image->depth)) {
+	    (image->depth != 1 && info->var.bits_per_pixel != image->depth) ||
+	    (image->depth == 1 && info->var.bits_per_pixel == 24)) {
 		cfb_imageblit(info, image);
 		return;
 	}