[PATCH/RFC] atafb line length

[PATCH/RFC] atafb line length

[Geert Uytterhoeven]

Make sure fix->line_length is always set, as some applications need it because
they don't have fallback code if line_length is zero.

Works on ARAnyM (Falcon emulation), but par->next_line is not set on any other
Atari variant?

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/video/atafb.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/drivers/video/atafb.c
+++ b/drivers/video/atafb.c
@@ -613,7 +613,7 @@ static int tt_encode_fix(struct fb_fix_s
 	fix->xpanstep = 0;
 	fix->ypanstep = 1;
 	fix->ywrapstep = 0;
-	fix->line_length = 0;
+	fix->line_length = par->next_line;
 	fix->accel = FB_ACCEL_ATARIBLITT;
 	return 0;
 }
@@ -917,7 +917,8 @@ static int falcon_encode_fix(struct fb_f
 		fix->visual = FB_VISUAL_TRUECOLOR;
 		fix->xpanstep = 2;
 	}
-	fix->line_length = 0;
+	fix->line_length =
+		(par->hw.falcon.line_width + par->hw.falcon.line_offset) * 2;
 	fix->accel = FB_ACCEL_ATARIBLITT;
 	return 0;
 }
@@ -1844,7 +1845,7 @@ static int stste_encode_fix(struct fb_fi
 		fix->ypanstep = 0;
 	}
 	fix->ywrapstep = 0;
-	fix->line_length = 0;
+	fix->line_length = par->next_line;
 	fix->accel = FB_ACCEL_ATARIBLITT;
 	return 0;
 }
@@ -2161,7 +2162,7 @@ static int ext_encode_fix(struct fb_fix_
 	fix->xpanstep = 0;
 	fix->ypanstep = 0;
 	fix->ywrapstep = 0;
-	fix->line_length = 0;
+	fix->line_length = par->next_line;
 	return 0;
 }
 

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH/RFC] atafb line length

[Michael Schmitz]

Hi Geert,

> Make sure fix->line_length is always set, as some applications need it because
> they don't have fallback code if line_length is zero.

I did set line_length to zero for a reason while hacking on the driver, but my 
recollection of the reasons is quite fuzzy. I'll give it a test run on the real 
hardware. 

As with all other Atari bugs recently introduced, it may equally well be sloppy 
coding on my part. 
 
> Works on ARAnyM (Falcon emulation), but par->next_line is not set on any other
> Atari variant?

fix->line_length may not be used by anything other than the VIDEL. I'll check 
that in the hardware specific code. 

Thanks for adopting atafb :-)

	Michael

Re: [PATCH/RFC] atafb line length

[Michael Schmitz]

Hi Geert,

> Make sure fix->line_length is always set, as some applications need it because
> they don't have fallback code if line_length is zero.
> 
> Works on ARAnyM (Falcon emulation), but par->next_line is not set on any other
> Atari variant?

You're right - while testing on the Falcon framebuffer I noticed 
fix->line_length needs to be set, and kludged around to get it set to something 
more or less sane, but I could not test the other code paths. 

It looks like par->next_line should be set to linelen in all other decode_var 
variants. The quilt tree here does have fix->line_length = par->next_line 
already, what is your patch based on? 

I had 

fix->line_length = (par->hw.falcon.line_width + par->hw.falcon.line_offset) *
		par->hw.falcon.bpp / 8;

originally - I suppose this had to change with your cfb fixes? 

	Michael

Re: [PATCH/RFC] atafb line length

[Geert Uytterhoeven]

On Wed, 19 Nov 2008, Michael Schmitz wrote:
> > Make sure fix->line_length is always set, as some applications need it because
> > they don't have fallback code if line_length is zero.
> > 
> > Works on ARAnyM (Falcon emulation), but par->next_line is not set on any other
> > Atari variant?
> 
> You're right - while testing on the Falcon framebuffer I noticed 
> fix->line_length needs to be set, and kludged around to get it set to something 
> more or less sane, but I could not test the other code paths. 
> 
> It looks like par->next_line should be set to linelen in all other decode_var 
> variants. The quilt tree here does have fix->line_length = par->next_line 
> already, what is your patch based on? 

It was based on mainline. Yes, a part of my patch had been in my quilt series
for a while, but I wanted to post the full thing for review.

> I had 
> 
> fix->line_length = (par->hw.falcon.line_width + par->hw.falcon.line_offset) *
> 		par->hw.falcon.bpp / 8;
> 
> originally - I suppose this had to change with your cfb fixes? 

That should be OK. Strange, it's not what I got from you, cfr. atafb in
mainline.

Cfb16 needed
  1. fix->line_length to be valid (it was zero)
  2. info->pseudo_palette

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH/RFC] atafb line length

[Michael Schmitz]

Hi,

> > It looks like par->next_line should be set to linelen in all other decode_var 
> > variants. The quilt tree here does have fix->line_length = par->next_line 
> > already, what is your patch based on? 
> 
> It was based on mainline. Yes, a part of my patch had been in my quilt series
> for a while, but I wanted to post the full thing for review.

OK, I see. 
 
> > I had 
> > 
> > fix->line_length = (par->hw.falcon.line_width + par->hw.falcon.line_offset) *
> > 		par->hw.falcon.bpp / 8;
> > 
> > originally - I suppose this had to change with your cfb fixes? 
> 
> That should be OK. Strange, it's not what I got from you, cfr. atafb in
> mainline.

I concede this may be something I changed later on. atafb had been worked on my 
Roman Zippel as well after my initial patches so I'll double check this isn't 
obsolete (I was looking at my current working tree though).
 
> Cfb16 needed
>   1. fix->line_length to be valid (it was zero)
>   2. info->pseudo_palette

OK, so the above fix->line_length may be independent from your fixes.

I'll test that (just not right away, home improvement work has taken a heavy 
toll on my time lately).

	Michael

Re: [PATCH/RFC] atafb line length

[Geert Uytterhoeven]

On Sun, 16 Nov 2008, Geert Uytterhoeven wrote:
> Make sure fix->line_length is always set, as some applications need it because
> they don't have fallback code if line_length is zero.
> 
> Works on ARAnyM (Falcon emulation), but par->next_line is not set on any other
> Atari variant?

And this should fix the remaining ones. I cannot test it, though.

Without this, atafb is completely broken on anything but Falcon, as all the
text console drawing operations need a valid par->next_line.

On (emulated) Falcon, we can just use par->next_line, too.

I plan to clean up the atafb/c2p patch series, and get it into 2.6.29.

diff --git a/drivers/video/atafb.c b/drivers/video/atafb.c
index 8f70296..477ce55 100644
--- a/drivers/video/atafb.c
+++ b/drivers/video/atafb.c
@@ -691,6 +691,7 @@ static int tt_decode_var(struct fb_var_screeninfo *var, struct atafb_par *par)
 		return -EINVAL;
 	par->yres_virtual = yres_virtual;
 	par->screen_base = screen_base + var->yoffset * linelen;
+	par->next_line = linelen;
 	return 0;
 }
 
@@ -914,8 +915,7 @@ static int falcon_encode_fix(struct fb_fix_screeninfo *fix,
 		fix->visual = FB_VISUAL_TRUECOLOR;
 		fix->xpanstep = 2;
 	}
-	fix->line_length =
-		(par->hw.falcon.line_width + par->hw.falcon.line_offset) * 2;
+	fix->line_length = par->next_line;
 	fix->accel = FB_ACCEL_ATARIBLITT;
 	return 0;
 }
@@ -1900,6 +1900,7 @@ static int stste_decode_var(struct fb_var_screeninfo *var,
 		return -EINVAL;
 	par->yres_virtual = yres_virtual;
 	par->screen_base = screen_base + var->yoffset * linelen;
+	par->next_line = linelen;
 	return 0;
 }
 
@@ -2174,6 +2175,8 @@ static int ext_decode_var(struct fb_var_screeninfo *var, struct atafb_par *par)
 	    var->xoffset > 0 ||
 	    var->yoffset > 0)
 		return -EINVAL;
+
+	par->next_line = external_xres_virtual * external_depth / 8;
 	return 0;
 }
 

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds