Bug in software cursor flash code, when align!=1?

Bug in software cursor flash code, when align!=1?

[Thomas S. Iversen]

Hi Again

Being puzzled by the fact that my software cursor did not work when
everything else worked with my accelerated routines I tried to investigate
where the problem was located. This being when scan_align!=1 which
my accel routines need.

I suspected the flash code in the software cursor routines, so I
changed the flash frequence to 1 per second. This showed me that
the first image (== combination of the cursor and the underlying char)
is good enough. After a second the cursor flash code gets called
and I get garbage where the cursor are. When I move the cursor to another
position I once more get one good image and garbage after that.

To rule out that my accell functions caused this I put in a

	for(i=0;i<64;i++)
		info->cursor.image.data[i]=1<<((i>>2) &0x07);

between

	info->cursor.image.data = dst;
        
and
        info->fbops->fb_imageblit(info, &info->cursor.image);

in softcursor.c. This gave me the expected / image all the time.
                                           /

I did not have the time to investigate this very much further, but I
suspect that the problem lies with the xor/copy functions in
softcursor.c not taking alignment into account on subsequent calls.

Just for information. 

Regards Thomas, Denmark


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

RE: Bug in software cursor flash code, when align!=1?

[Tony]

> Being puzzled by the fact that my software cursor did not work when
> everything else worked with my accelerated routines I tried
> to investigate
> where the problem was located. This being when scan_align!=1 which
> my accel routines need.
>

There are two main problems with the software cursor code in
drivers/video/softcursor.c:

1.  First, because it uses info->fbops->fb_imageblit(), this means that
the source data must be in info->pixmap structure, and secondly, it must
use info->pixmap.{in|out}buf methods when moving data from one location
to another.

2. Secondly, and you assessed it correctly, is that info->cursor.image
is always byte aligned, and so will not work with accelerated imageblit
routines which has scanline alignment requirements other than 1.

Below is updated code that will transfer (and pad) the final cursor
image data from info->cursor.image to info->pixmap.  It is of course
less optimal than current code, but it should be consistent with current
usage of fb_imageblit(). I'm typing the code on the fly, so no
guarantees though that it will compile, much less, work.  But the main
idea is there.

Tony

int soft_cursor(struct fb_info *info, struct fb_cursor *cursor)
{
	unsigned int scan_align = info->pixmap.scan_align - 1;
	unsigned int buf_align = info->pixmap.buf_align - 1;
	u8 *dst = (u8 *) info->cursor.image.data;
	u8 *s, *d;
	unsigned int i, size, p, pitch;

	/* p is the source pitch which is always byte aligned */
	p = (info->cursor.image.width + 7) >> 3;

	/* pitch is the destination data pitch, which is padded
	   to hardware requirements */
	pitch = p + scan_align;
	pitch &= ~scan_align;
	size = pitch * info->cursor.image.height + buf_align;
	size &= ~buf_align;

	if (cursor->set & FB_CUR_SETSIZE) {
 		info->cursor.image.height = cursor->image.height;
 		info->cursor.image.width = cursor->image.width;
 	}

 	if (cursor->set & FB_CUR_SETPOS) {
	info->cursor.image.dx = cursor->image.dx;
 		info->cursor.image.dy = cursor->image.dy;
 	}

 	if (cursor->set & FB_CUR_SETHOT)
 		info->cursor.hot = cursor->hot;

 	if (cursor->set & FB_CUR_SETCMAP) {
 		if (cursor->image.depth == 1) {
 			info->cursor.image.bg_color = cursor->image.bg_color;
 			info->cursor.image.fg_color = cursor->image.fg_color;
 		} else {
 			if (cursor->image.cmap.len)
 				fb_copy_cmap(&cursor->image.cmap, &info->cursor.image.cmap, 0);
		}
		info->cursor.image.depth = cursor->image.depth;
 	}

 	if (cursor->set & FB_CUR_SETSHAPE) {
 		switch (info->cursor.rop) {
 		case ROP_XOR:
 			for (i = 0; i < size; i++)
 				dst[i] ^= info->cursor.mask[i];
			break;
 		case ROP_COPY:
 		default:
 			for (i = 0; i < size; i++)
 				dst[i] &= info->cursor.mask[i];
			break;
 		}
 	}

 	if (!info->cursor.enable) {
 		for (i = 0; i < size; i++)
 			dst[i] ^= info->cursor.mask[i];
 	}

	/* Once cursor image processing is finished, start transferring
	   data to pixmap */

	/* First, get a chunk from scratch memory... */
	d = fb_get_buffer_offset(info, &info->pixmap, size);
	s = dst;

	/* ... then move data to scratch memory ... */
	for (i = info->cursor.image.height; i--; ) {
		info->pixmap.outbuf(info, d, s, p)
		d += pitch;
		s += p;
	}

	/* ... draw the cursor ... */
 	if (info->cursor.image.data)
 		info->fbops->fb_imageblit(info, d);

	/* ... tell fb we're done. */
	atomic_dec(info->pixmap.count);
	smb_mb__after_atomic_dec();

 	return 0;
}





-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

RE: Bug in software cursor flash code, when align!=1?

[Tony]

> 
> Hi Again
> 
> Being puzzled by the fact that my software cursor did not work when
> everything else worked with my accelerated routines I tried 
> to investigate
> where the problem was located. This being when scan_align!=1 which
> my accel routines need.
>

Oops, sorry, try this instead.

Tony

int soft_cursor(struct fb_info *info, struct fb_cursor *cursor)
{
	struct fb_image image;
	unsigned int scan_align = info->pixmap.scan_align - 1;
	unsigned int buf_align = info->pixmap.buf_align - 1;
	u8 *dst = (u8 *) info->cursor.image.data;
	u8 *s, *d;
	unsigned int i, size, p, pitch;
	
	/* p is the source pitch which is always byte aligned */
	p = (info->cursor.image.width + 7) >> 3;

	/* pitch is the destination data pitch, which is padded
	   to hardware requirements */
	pitch = p + scan_align;
	pitch &= ~scan_align;
	size = pitch * info->cursor.image.height + buf_align;
	size &= ~buf_align;
 	
	if (cursor->set & FB_CUR_SETSIZE) {
 		info->cursor.image.height = cursor->image.height;
 		info->cursor.image.width = cursor->image.width;
 	}
 
 	if (cursor->set & FB_CUR_SETPOS) {
	info->cursor.image.dx = cursor->image.dx;
 		info->cursor.image.dy = cursor->image.dy;
 	}

 	if (cursor->set & FB_CUR_SETHOT)
 		info->cursor.hot = cursor->hot;
 	
 	if (cursor->set & FB_CUR_SETCMAP) {
 		if (cursor->image.depth == 1) {
 			info->cursor.image.bg_color = cursor->image.bg_color;
 			info->cursor.image.fg_color = cursor->image.fg_color;
 		} else {
 			if (cursor->image.cmap.len)
 				fb_copy_cmap(&cursor->image.cmap, &info->cursor.image.cmap, 0);
		}
		info->cursor.image.depth = cursor->image.depth;
 	}	
 
 	if (cursor->set & FB_CUR_SETSHAPE) {
 		switch (info->cursor.rop) {
 		case ROP_XOR:
 			for (i = 0; i < size; i++)
 				dst[i] ^= info->cursor.mask[i]; 
			break;
 		case ROP_COPY:
 		default:
 			for (i = 0; i < size; i++)
 				dst[i] &= info->cursor.mask[i];
			break;
 		}
 	}
 	
 	if (!info->cursor.enable) {
 		for (i = 0; i < size; i++)
 			dst[i] ^= info->cursor.mask[i];
 	}
	
	/* Once cursor image processing is finished, start transferring
	   data to pixmap */

	/* First, get a chunk from scratch memory... */
	image = info->cursor.image;
	image.data = d = fb_get_buffer_offset(info, &info->pixmap, size);
	s = dst;

	/* ... then move data to scratch memory ... */
	if (info->cursor.image.data) {
		for (i = info->cursor.image.height; i--; ) {
			info->pixmap.outbuf(info, d, s, p)	
			d += pitch;
			s += p;
		}
	}
		
	/* ... draw the cursor ... */
 	info->fbops->fb_imageblit(info, &image);

	/* ... tell fb we're done. */
	atomic_dec(info->pixmap.count);
	smb_mb__after_atomic_dec();

 	return 0;
}



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf