2022-07-11 15:59:02

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 00/10] video: fbdev: atari: Miscellaneous fixes and cleanups

Hi all,

This patch series contains miscellaneous fixes and cleanups for the
Atari frame buffer device driver, which were identified while working on
the Atari DRM driver.

Most of them have been tested on ARAnyM, and should be safe to apply,
except perhaps for the last one, which is marked RFC.

Thanks for your comments!

Geert Uytterhoeven (10):
video: fbdev: atari: Simplify atafb_pan_display()
video: fbdev: atari: Remove bogus FB_VMODE_YWRAP flags
video: fbdev: atari: Fix inverse handling
video: fbdev: atari: Fix ext_setcolreg()
video: fbdev: atari: Remove unneeded casts from void *
video: fbdev: atari: Remove unneeded casts to void *
video: fbdev: atari: Fix TT High video mode vertical refresh
video: fbdev: atari: Fix VGA modes
video: fbdev: atari: Remove unused definitions and variables
[RFC] video: fbdev: atari: Remove backward bug-compatibility

Documentation/m68k/kernel-options.rst | 4 +-
drivers/video/fbdev/atafb.c | 101 +++++++-------------------
2 files changed, 29 insertions(+), 76 deletions(-)

--
2.25.1

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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


2022-07-11 15:59:02

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 10/10] [RFC] video: fbdev: atari: Remove backward bug-compatibility

As of v2.1.0, falcon_decode_var() contains a quirk to fix a rounding
error, as explained by Günther Kelleter on Fri, 30 Aug 1996:

This diff removes the now obsolete Falcon video option "pwrsave", and
fixes a rounding error that is triggered by the resolution switching X
server (those who use the pixel clock value 39722 in their /etc/fb.modes
should change it to 39721).

However, this causes the modified video mode returned by
falcon_decode_var() to not match the video mode returned by
falcon_encode_var(). Fix this by dropping the quirk.

Unfortunately /etc/fb.modes in fbset was never updated, so the
"640x480-60" mode still contains the wrong pixclock.
Hence this change may introduce a regression.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Any comments?
---
drivers/video/fbdev/atafb.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index e8b178e732e2c785..2bc4089865e60ac2 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -1008,10 +1008,6 @@ static int falcon_decode_var(struct fb_var_screeninfo *var,
else if (yres_virtual < yres)
yres_virtual = yres;

- /* backward bug-compatibility */
- if (var->pixclock > 1)
- var->pixclock -= 1;
-
par->hw.falcon.line_width = bpp * xres / 16;
par->hw.falcon.line_offset = bpp * (xres_virtual - xres) / 16;

--
2.25.1

2022-07-11 16:06:59

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 06/10] video: fbdev: atari: Remove unneeded casts to void *

Arbitrary pointers can be passed to functions accepting "void *" without
casting.

Remove the casts, as they make it impossible to validate types.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/video/fbdev/atafb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index f20535ea3e549384..fbc333d5615df5d5 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -2599,14 +2599,14 @@ atafb_ioctl(struct fb_info *info, unsigned int cmd, unsigned long arg)
switch (cmd) {
#ifdef FBCMD_GET_CURRENTPAR
case FBCMD_GET_CURRENTPAR:
- if (copy_to_user((void *)arg, (void *)&current_par,
+ if (copy_to_user((void *)arg, &current_par,
sizeof(struct atafb_par)))
return -EFAULT;
return 0;
#endif
#ifdef FBCMD_SET_CURRENTPAR
case FBCMD_SET_CURRENTPAR:
- if (copy_from_user((void *)&current_par, (void *)arg,
+ if (copy_from_user(&current_par, (void *)arg,
sizeof(struct atafb_par)))
return -EFAULT;
ata_set_par(&current_par);
--
2.25.1

2022-07-11 16:17:56

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 03/10] video: fbdev: atari: Fix inverse handling

Currently, the "inverse" option does not do anything, as it just sets a
flag, which is further unused.

Fix this by calling fb_invert_cmaps() instead, like other drivers do.
As this only affects the console colormap, this does not affect X.
Update the documentation to match the actual behavior.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Documentation/m68k/kernel-options.rst | 4 ++--
drivers/video/fbdev/atafb.c | 4 +---
2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/m68k/kernel-options.rst b/Documentation/m68k/kernel-options.rst
index cabd9419740d5ada..2008a20b43295bd5 100644
--- a/Documentation/m68k/kernel-options.rst
+++ b/Documentation/m68k/kernel-options.rst
@@ -367,8 +367,8 @@ activated by a "external:" sub-option.
4.1.2) inverse
--------------

-Invert the display. This affects both, text (consoles) and graphics
-(X) display. Usually, the background is chosen to be black. With this
+Invert the display. This affects only text consoles.
+Usually, the background is chosen to be black. With this
option, you can make the background white.

4.1.3) font
diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index 172ef547ff6f4883..39c3b860a797d4bc 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -236,8 +236,6 @@ static int *MV300_reg = MV300_reg_8bit;
#endif /* ATAFB_EXT */


-static int inverse;
-
/*
* struct fb_ops {
* * open/release and usage marking
@@ -2971,7 +2969,7 @@ static int __init atafb_setup(char *options)
default_par = temp;
mode_option = this_opt;
} else if (!strcmp(this_opt, "inverse"))
- inverse = 1;
+ fb_invert_cmaps();
else if (!strncmp(this_opt, "hwscroll_", 9)) {
hwscroll = simple_strtoul(this_opt + 9, NULL, 10);
if (hwscroll < 0)
--
2.25.1

2022-07-11 16:23:33

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 04/10] video: fbdev: atari: Fix ext_setcolreg()

The red, green, and blue color values are 16-bit, while the external
graphics hardware registers are 8-bit.
Add the missing conversion from 16-bit to 8-bit.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Untested due to lack of hardware.
---
drivers/video/fbdev/atafb.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index 39c3b860a797d4bc..a36cd8f1f4200dd5 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -2206,6 +2206,10 @@ static int ext_setcolreg(unsigned int regno, unsigned int red,
if (regno > 255)
return 1;

+ red >>= 8;
+ green >>= 8;
+ blue >>= 8;
+
switch (external_card_type) {
case IS_VGA:
OUTB(0x3c8, regno);
--
2.25.1

2022-07-11 16:24:32

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 07/10] video: fbdev: atari: Fix TT High video mode vertical refresh

The vertical refresh rate for the TT High video mode (1280x960) is
wrong. Fortunately this field is not really used.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/video/fbdev/atafb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index fbc333d5615df5d5..528478f6f30857ef 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -484,7 +484,7 @@ static struct fb_videomode atafb_modedb[] __initdata = {
0, FB_VMODE_NONINTERLACED
}, {
/* 1280x960, 72 kHz, 72 Hz (TT high) */
- "tt-high", 57, 1280, 960, 7760, 260, 60, 36, 4, 192, 4,
+ "tt-high", 72, 1280, 960, 7760, 260, 60, 36, 4, 192, 4,
0, FB_VMODE_NONINTERLACED
},

--
2.25.1

2022-07-11 16:25:23

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 02/10] video: fbdev: atari: Remove bogus FB_VMODE_YWRAP flags

Vertical wrap is not supported (fb_fix_screeninfo.ywrapstep = 0), hence
there is no point in setting the FB_VMODE_YWRAP flag in video modes.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/video/fbdev/atafb.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index c0683d2a4efaf1e8..172ef547ff6f4883 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -467,27 +467,27 @@ static struct fb_videomode atafb_modedb[] __initdata = {
{
/* 320x200, 15 kHz, 60 Hz (ST low) */
"st-low", 60, 320, 200, 32000, 32, 16, 31, 14, 96, 4,
- 0, FB_VMODE_NONINTERLACED | FB_VMODE_YWRAP
+ 0, FB_VMODE_NONINTERLACED
}, {
/* 640x200, 15 kHz, 60 Hz (ST medium) */
"st-mid", 60, 640, 200, 32000, 32, 16, 31, 14, 96, 4,
- 0, FB_VMODE_NONINTERLACED | FB_VMODE_YWRAP
+ 0, FB_VMODE_NONINTERLACED
}, {
/* 640x400, 30.25 kHz, 63.5 Hz (ST high) */
"st-high", 63, 640, 400, 32000, 128, 0, 40, 14, 128, 4,
- 0, FB_VMODE_NONINTERLACED | FB_VMODE_YWRAP
+ 0, FB_VMODE_NONINTERLACED
}, {
/* 320x480, 15 kHz, 60 Hz (TT low) */
"tt-low", 60, 320, 480, 31041, 120, 100, 8, 16, 140, 30,
- 0, FB_VMODE_NONINTERLACED | FB_VMODE_YWRAP
+ 0, FB_VMODE_NONINTERLACED
}, {
/* 640x480, 29 kHz, 57 Hz (TT medium) */
"tt-mid", 60, 640, 480, 31041, 120, 100, 8, 16, 140, 30,
- 0, FB_VMODE_NONINTERLACED | FB_VMODE_YWRAP
+ 0, FB_VMODE_NONINTERLACED
}, {
/* 1280x960, 72 kHz, 72 Hz (TT high) */
"tt-high", 57, 1280, 960, 7760, 260, 60, 36, 4, 192, 4,
- 0, FB_VMODE_NONINTERLACED | FB_VMODE_YWRAP
+ 0, FB_VMODE_NONINTERLACED
},

/*
@@ -497,11 +497,11 @@ static struct fb_videomode atafb_modedb[] __initdata = {
{
/* 640x480, 31 kHz, 60 Hz (VGA) */
"vga", 63.5, 640, 480, 32000, 18, 42, 31, 11, 96, 3,
- 0, FB_VMODE_NONINTERLACED | FB_VMODE_YWRAP
+ 0, FB_VMODE_NONINTERLACED
}, {
/* 640x400, 31 kHz, 70 Hz (VGA) */
"vga70", 70, 640, 400, 32000, 18, 42, 31, 11, 96, 3,
- FB_SYNC_VERT_HIGH_ACT | FB_SYNC_COMP_HIGH_ACT, FB_VMODE_NONINTERLACED | FB_VMODE_YWRAP
+ FB_SYNC_VERT_HIGH_ACT | FB_SYNC_COMP_HIGH_ACT, FB_VMODE_NONINTERLACED
},

/*
@@ -511,7 +511,7 @@ static struct fb_videomode atafb_modedb[] __initdata = {
{
/* 896x608, 31 kHz, 60 Hz (Falcon High) */
"falh", 60, 896, 608, 32000, 18, 42, 31, 1, 96,3,
- 0, FB_VMODE_NONINTERLACED | FB_VMODE_YWRAP
+ 0, FB_VMODE_NONINTERLACED
},
};

--
2.25.1

2022-07-11 16:25:31

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 01/10] video: fbdev: atari: Simplify atafb_pan_display()

The fb_pan_display() function in the core already takes care of
validating the panning parameters before calling the driver's
.fb_pan_display() callback, and of updating the panning state
afterwards, so there is no need to repeat that in the driver.

Remove the duplicate code.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/video/fbdev/atafb.c | 29 ++---------------------------
1 file changed, 2 insertions(+), 27 deletions(-)

diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index 102c727cedc0f005..c0683d2a4efaf1e8 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -2407,35 +2407,10 @@ static void atafb_set_disp(struct fb_info *info)
static int
atafb_pan_display(struct fb_var_screeninfo *var, struct fb_info *info)
{
- int xoffset = var->xoffset;
- int yoffset = var->yoffset;
- int err;
-
- if (var->vmode & FB_VMODE_YWRAP) {
- if (yoffset < 0 || yoffset >= info->var.yres_virtual || xoffset)
- return -EINVAL;
- } else {
- if (xoffset + info->var.xres > info->var.xres_virtual ||
- yoffset + info->var.yres > info->var.yres_virtual)
- return -EINVAL;
- }
-
- if (fbhw->pan_display) {
- err = fbhw->pan_display(var, info);
- if (err)
- return err;
- } else
+ if (!fbhw->pan_display)
return -EINVAL;

- info->var.xoffset = xoffset;
- info->var.yoffset = yoffset;
-
- if (var->vmode & FB_VMODE_YWRAP)
- info->var.vmode |= FB_VMODE_YWRAP;
- else
- info->var.vmode &= ~FB_VMODE_YWRAP;
-
- return 0;
+ return fbhw->pan_display(var, info);
}

/*
--
2.25.1

2022-07-11 16:27:13

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 09/10] video: fbdev: atari: Remove unused definitions and variables

Several definitions and variables are unused.
Some variables are set but further unused.

Remove them.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/video/fbdev/atafb.c | 20 --------------------
1 file changed, 20 deletions(-)

diff --git a/drivers/video/fbdev/atafb.c b/drivers/video/fbdev/atafb.c
index 46a00e0ad5e785ac..e8b178e732e2c785 100644
--- a/drivers/video/fbdev/atafb.c
+++ b/drivers/video/fbdev/atafb.c
@@ -1070,8 +1070,6 @@ static int falcon_decode_var(struct fb_var_screeninfo *var,
xstretch = 2; /* Double pixel width only for hicolor */
/* Default values are used for vert./hor. timing if no pixelclock given. */
if (var->pixclock == 0) {
- int linesize;
-
/* Choose master pixelclock depending on hor. timing */
plen = 1 * xstretch;
if ((plen * xres + f25.right + f25.hsync + f25.left) *
@@ -1090,7 +1088,6 @@ static int falcon_decode_var(struct fb_var_screeninfo *var,
left_margin = pclock->left / plen;
right_margin = pclock->right / plen;
hsync_len = pclock->hsync / plen;
- linesize = left_margin + xres + right_margin + hsync_len;
upper_margin = 31;
lower_margin = 11;
vsync_len = 3;
@@ -2419,17 +2416,6 @@ atafb_pan_display(struct fb_var_screeninfo *var, struct fb_info *info)
* generic drawing routines; imageblit needs updating for image depth > 1
*/

-#if BITS_PER_LONG == 32
-#define BYTES_PER_LONG 4
-#define SHIFT_PER_LONG 5
-#elif BITS_PER_LONG == 64
-#define BYTES_PER_LONG 8
-#define SHIFT_PER_LONG 6
-#else
-#define Please update me
-#endif
-
-
static void atafb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
{
struct atafb_par *par = info->par;
@@ -2531,8 +2517,6 @@ static void atafb_imageblit(struct fb_info *info, const struct fb_image *image)
{
struct atafb_par *par = info->par;
int x2, y2;
- unsigned long *dst;
- int dst_idx;
const char *src;
u32 dx, dy, width, height, pitch;

@@ -2559,10 +2543,6 @@ static void atafb_imageblit(struct fb_info *info, const struct fb_image *image)

if (image->depth == 1) {
// used for font data
- dst = (unsigned long *)
- ((unsigned long)info->screen_base & ~(BYTES_PER_LONG - 1));
- dst_idx = ((unsigned long)info->screen_base & (BYTES_PER_LONG - 1)) * 8;
- dst_idx += dy * par->next_line * 8 + dx;
src = image->data;
pitch = (image->width + 7) / 8;
while (height--) {
--
2.25.1

2022-07-12 08:25:56

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH 00/10] video: fbdev: atari: Miscellaneous fixes and cleanups

On 7/11/22 17:50, Geert Uytterhoeven wrote:
> Hi all,
>
> This patch series contains miscellaneous fixes and cleanups for the
> Atari frame buffer device driver, which were identified while working on
> the Atari DRM driver.
>
> Most of them have been tested on ARAnyM, and should be safe to apply,

I've applied patches 1-9 to fbdev for-next git tree....

> except perhaps for the last one, which is marked RFC.

... and would like to see some thoughts/feedback from others on this one ...

Thanks,
Helge

>
> Thanks for your comments!
>
> Geert Uytterhoeven (10):
> video: fbdev: atari: Simplify atafb_pan_display()
> video: fbdev: atari: Remove bogus FB_VMODE_YWRAP flags
> video: fbdev: atari: Fix inverse handling
> video: fbdev: atari: Fix ext_setcolreg()
> video: fbdev: atari: Remove unneeded casts from void *
> video: fbdev: atari: Remove unneeded casts to void *
> video: fbdev: atari: Fix TT High video mode vertical refresh
> video: fbdev: atari: Fix VGA modes
> video: fbdev: atari: Remove unused definitions and variables
> [RFC] video: fbdev: atari: Remove backward bug-compatibility
>
> Documentation/m68k/kernel-options.rst | 4 +-
> drivers/video/fbdev/atafb.c | 101 +++++++-------------------
> 2 files changed, 29 insertions(+), 76 deletions(-)
>

2022-07-15 05:11:09

by Michael Schmitz

[permalink] [raw]
Subject: Re: [PATCH 00/10] video: fbdev: atari: Miscellaneous fixes and cleanups

Hi Geert,

somehow this series slipped into my spam folder ... only saw it now.

Am 12.07.2022 um 03:50 schrieb Geert Uytterhoeven:
> Hi all,
>
> This patch series contains miscellaneous fixes and cleanups for the
> Atari frame buffer device driver, which were identified while working on
> the Atari DRM driver.
>
> Most of them have been tested on ARAnyM, and should be safe to apply,
> except perhaps for the last one, which is marked RFC.
>
> Thanks for your comments!
>
> Geert Uytterhoeven (10):
> video: fbdev: atari: Simplify atafb_pan_display()
> video: fbdev: atari: Remove bogus FB_VMODE_YWRAP flags
> video: fbdev: atari: Fix inverse handling
> video: fbdev: atari: Fix ext_setcolreg()
> video: fbdev: atari: Remove unneeded casts from void *
> video: fbdev: atari: Remove unneeded casts to void *
> video: fbdev: atari: Fix TT High video mode vertical refresh
> video: fbdev: atari: Fix VGA modes
> video: fbdev: atari: Remove unused definitions and variables
> [RFC] video: fbdev: atari: Remove backward bug-compatibility
>
> Documentation/m68k/kernel-options.rst | 4 +-
> drivers/video/fbdev/atafb.c | 101 +++++++-------------------
> 2 files changed, 29 insertions(+), 76 deletions(-)
>

Looks good to me. (I'll still try to test it on hardware this weekend.)

I'd suggest the last one be applied as well - if the regression can only
be triggered by a X server resolution switch, I doubt it'll matter in
practice.

Cheers,

Michael

2022-07-17 23:21:37

by Michael Schmitz

[permalink] [raw]
Subject: Re: [PATCH 00/10] video: fbdev: atari: Miscellaneous fixes and cleanups

Hi Geert,

On 12/07/22 03:50, Geert Uytterhoeven wrote:
> Hi all,
>
> This patch series contains miscellaneous fixes and cleanups for the
> Atari frame buffer device driver, which were identified while working on
> the Atari DRM driver.
>
> Most of them have been tested on ARAnyM, and should be sa<fe to apply,
> except perhaps for the last one, which is marked RFC.
>
> Thanks for your comments!
>
> Geert Uytterhoeven (10):
> video: fbdev: atari: Simplify atafb_pan_display()
> video: fbdev: atari: Remove bogus FB_VMODE_YWRAP flags
> video: fbdev: atari: Fix inverse handling
> video: fbdev: atari: Fix ext_setcolreg()
> video: fbdev: atari: Remove unneeded casts from void *
> video: fbdev: atari: Remove unneeded casts to void *
> video: fbdev: atari: Fix TT High video mode vertical refresh
> video: fbdev: atari: Fix VGA modes
> video: fbdev: atari: Remove unused definitions and variables
> [RFC] video: fbdev: atari: Remove backward bug-compatibility
>
> Documentation/m68k/kernel-options.rst | 4 +-
> drivers/video/fbdev/atafb.c | 101 +++++++-------------------
> 2 files changed, 29 insertions(+), 76 deletions(-)
>
Works OK on my Falcon (except for the falh* modes, but that is not a
regression - might be something I miscoded in these modes, ages ago).

Tested-by: Michael Schmitz <[email protected]>