All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] vfb: fix video mode and line_length being set when loaded
@ 2017-12-20  0:24 Pieter "PoroCYon" Sluys
  2017-12-20  8:26   ` Geert Uytterhoeven
  0 siblings, 1 reply; 6+ messages in thread
From: Pieter "PoroCYon" Sluys @ 2017-12-20  0:24 UTC (permalink / raw)
  To: linux-fbdev, linux-kernel; +Cc: Bartlomiej Zolnierkiewicz, Geert Uytterhoeven

[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]

Currently, when loading the vfb module, the newly created fbdev
has a line_length of 0, and its video mode would be PSEUDOCOLOR
regardless of color depth. (The former could be worked around by
calling the FBIOPUT_VSCREENINFO ioctl with having the FBACTIVIATE_FORCE
flag set.) This patch automatically sets the line_length correctly,
and the video mode is derived from the bit depth now as well.

Thanks to Geert Uytterhoeven for confirming the bug and helping me with
the patch.

Output of `fbset -i' before the patch:
mode "1366x768-60"
    # D: 72.432 MHz, H: 47.403 kHz, V: 60.004 Hz
    geometry 1366 768 1366 768 32
    timings 13806 120 10 14 3 32 5
    rgba 8/0,8/8,8/16,8/24
endmode

Frame buffer device information:
    Name        : Virtual FB
    Address     : 0xffffaa1405d85000
    Size        : 4196352
    Type        : PACKED PIXELS
    Visual      : PSEUDOCOLOR
    XPanStep    : 1
    YPanStep    : 1
    YWrapStep   : 1
    LineLength  : 0                    <-- note this
    Accelerator : No

After:
mode "1366x768-60"
    # D: 72.432 MHz, H: 47.403 kHz, V: 60.004 Hz
    geometry 1366 768 1366 768 32
    timings 13806 120 10 14 3 32 5
    rgba 8/0,8/8,8/16,8/24
endmode

Frame buffer device information:
    Name        : Virtual FB
    Address     : 0xffffaa1405d85000
    Size        : 4196352
    Type        : PACKED PIXELS
    Visual      : TRUECOLOR
    XPanStep    : 1
    YPanStep    : 1
    YWrapStep   : 1
    LineLength  : 5464
    Accelerator : No

Thanks for your consideration,
Pieter "PoroCYon" Sluys

---
 drivers/video/fbdev/vfb.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/video/fbdev/vfb.c b/drivers/video/fbdev/vfb.c
index da653a080394..6e866fbaab8a 100644
--- a/drivers/video/fbdev/vfb.c
+++ b/drivers/video/fbdev/vfb.c
@@ -239,8 +239,25 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
  */
 static int vfb_set_par(struct fb_info *info)
 {
+	switch (info->var.bits_per_pixel) {
+		case 1:
+			info->fix.visual = FB_VISUAL_MONO01;
+			break;
+		case 2:
+		case 4:
+		case 8:
+			info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
+			break;
+		case 16:
+		case 24:
+		case 32:
+			info->fix.visual = FB_VISUAL_TRUECOLOR;
+			break;
+	}
+
 	info->fix.line_length = get_line_length(info->var.xres_virtual,
 						info->var.bits_per_pixel);
+
 	return 0;
 }

@@ -450,6 +467,8 @@ static int vfb_probe(struct platform_device *dev)
 		goto err2;
 	platform_set_drvdata(dev, info);

+	vfb_set_par(info);
+
 	fb_info(info, "Virtual frame buffer device, using %ldK of video memory\n",
 		videomemorysize >> 10);
 	return 0;
--
2.15.1

[-- Attachment #2: README --]
[-- Type: text/plain, Size: 166 bytes --]

Fix vfb not setting the video mode and line_length when loaded.
Copyright 2017 Pieter "PoroCYon" Sluys <pcy@national.shitposting.agency>
under the terms of the GPLv2

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: vfb.c.patch --]
[-- Type: text/x-patch; name="vfb.c.patch", Size: 1115 bytes --]

---
 drivers/video/fbdev/vfb.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/video/fbdev/vfb.c b/drivers/video/fbdev/vfb.c
index da653a080394..6e866fbaab8a 100644
--- a/drivers/video/fbdev/vfb.c
+++ b/drivers/video/fbdev/vfb.c
@@ -239,8 +239,25 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
  */
 static int vfb_set_par(struct fb_info *info)
 {
+	switch (info->var.bits_per_pixel) {
+		case 1:
+			info->fix.visual = FB_VISUAL_MONO01;
+			break;
+		case 2:
+		case 4:
+		case 8:
+			info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
+			break;
+		case 16:
+		case 24:
+		case 32:
+			info->fix.visual = FB_VISUAL_TRUECOLOR;
+			break;
+	}
+
 	info->fix.line_length = get_line_length(info->var.xres_virtual,
 						info->var.bits_per_pixel);
+
 	return 0;
 }
 
@@ -450,6 +467,8 @@ static int vfb_probe(struct platform_device *dev)
 		goto err2;
 	platform_set_drvdata(dev, info);
 
+	vfb_set_par(info);
+
 	fb_info(info, "Virtual frame buffer device, using %ldK of video memory\n",
 		videomemorysize >> 10);
 	return 0;
--
2.15.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] vfb: fix video mode and line_length being set when loaded
  2017-12-20  0:24 [PATCH] vfb: fix video mode and line_length being set when loaded Pieter "PoroCYon" Sluys
@ 2017-12-20  8:26   ` Geert Uytterhoeven
  0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2017-12-20  8:26 UTC (permalink / raw)
  To: Pieter PoroCYon Sluys
  Cc: Linux Fbdev development list, Linux Kernel Mailing List,
	Bartlomiej Zolnierkiewicz

Hi Pieter,

On Wed, Dec 20, 2017 at 1:24 AM, Pieter "PoroCYon" Sluys
<pcy@national.shitposting.agency> wrote:
> Currently, when loading the vfb module, the newly created fbdev
> has a line_length of 0, and its video mode would be PSEUDOCOLOR
> regardless of color depth. (The former could be worked around by
> calling the FBIOPUT_VSCREENINFO ioctl with having the FBACTIVIATE_FORCE
> flag set.) This patch automatically sets the line_length correctly,
> and the video mode is derived from the bit depth now as well.

Thanks for your patch!
This bug was introduced in 2002, when converting the driver to the new
internal API.

Please add your "Signed-off-by", cfr.
Documentation/process/submitting-patches.rst.

> --- a/drivers/video/fbdev/vfb.c
> +++ b/drivers/video/fbdev/vfb.c
> @@ -239,8 +239,25 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
>   */
>  static int vfb_set_par(struct fb_info *info)
>  {
> +       switch (info->var.bits_per_pixel) {
> +               case 1:
> +                       info->fix.visual = FB_VISUAL_MONO01;
> +                       break;
> +               case 2:
> +               case 4:

You can drop the tests for 2 and 4, as these are no longer used by the driver.

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vfb: fix video mode and line_length being set when loaded
@ 2017-12-20  8:26   ` Geert Uytterhoeven
  0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2017-12-20  8:26 UTC (permalink / raw)
  To: Pieter PoroCYon Sluys
  Cc: Linux Fbdev development list, Linux Kernel Mailing List,
	Bartlomiej Zolnierkiewicz

Hi Pieter,

On Wed, Dec 20, 2017 at 1:24 AM, Pieter "PoroCYon" Sluys
<pcy@national.shitposting.agency> wrote:
> Currently, when loading the vfb module, the newly created fbdev
> has a line_length of 0, and its video mode would be PSEUDOCOLOR
> regardless of color depth. (The former could be worked around by
> calling the FBIOPUT_VSCREENINFO ioctl with having the FBACTIVIATE_FORCE
> flag set.) This patch automatically sets the line_length correctly,
> and the video mode is derived from the bit depth now as well.

Thanks for your patch!
This bug was introduced in 2002, when converting the driver to the new
internal API.

Please add your "Signed-off-by", cfr.
Documentation/process/submitting-patches.rst.

> --- a/drivers/video/fbdev/vfb.c
> +++ b/drivers/video/fbdev/vfb.c
> @@ -239,8 +239,25 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
>   */
>  static int vfb_set_par(struct fb_info *info)
>  {
> +       switch (info->var.bits_per_pixel) {
> +               case 1:
> +                       info->fix.visual = FB_VISUAL_MONO01;
> +                       break;
> +               case 2:
> +               case 4:

You can drop the tests for 2 and 4, as these are no longer used by the driver.

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vfb: fix video mode and line_length being set when loaded
  2017-12-20  8:26   ` Geert Uytterhoeven
  (?)
@ 2017-12-20 14:53   ` Pieter "PoroCYon" Sluys
  2018-01-04 15:05       ` Bartlomiej Zolnierkiewicz
  -1 siblings, 1 reply; 6+ messages in thread
From: Pieter "PoroCYon" Sluys @ 2017-12-20 14:53 UTC (permalink / raw)
  To: linux-fbdev, linux-kernel; +Cc: Bartlomiej Zolnierkiewicz, Geert Uytterhoeven

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

Geert Uytterhoeven:
> Please add your "Signed-off-by", cfr.
> Documentation/process/submitting-patches.rst.

Sorry I knew I would've messed up *something*.

> You can drop the tests for 2 and 4, as these are no longer used by the driver.

Done that.

Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Pieter "PoroCYon" Sluys <pcy@national.shitposting.agency>

---
 drivers/video/fbdev/vfb.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/video/fbdev/vfb.c b/drivers/video/fbdev/vfb.c
index da653a080394..5fb360283c6b 100644
--- a/drivers/video/fbdev/vfb.c
+++ b/drivers/video/fbdev/vfb.c
@@ -239,8 +239,23 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
  */
 static int vfb_set_par(struct fb_info *info)
 {
+	switch (info->var.bits_per_pixel) {
+		case 1:
+			info->fix.visual = FB_VISUAL_MONO01;
+			break;
+		case 8:
+			info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
+			break;
+		case 16:
+		case 24:
+		case 32:
+			info->fix.visual = FB_VISUAL_TRUECOLOR;
+			break;
+	}
+
 	info->fix.line_length = get_line_length(info->var.xres_virtual,
 						info->var.bits_per_pixel);
+
 	return 0;
 }

@@ -450,6 +465,8 @@ static int vfb_probe(struct platform_device *dev)
 		goto err2;
 	platform_set_drvdata(dev, info);

+	vfb_set_par(info);
+
 	fb_info(info, "Virtual frame buffer device, using %ldK of video memory\n",
 		videomemorysize >> 10);
 	return 0;
--
2.15.1


[-- Attachment #2: README --]
[-- Type: text/plain, Size: 166 bytes --]

Fix vfb not setting the video mode and line_length when loaded.
Copyright 2017 Pieter "PoroCYon" Sluys <pcy@national.shitposting.agency>
under the terms of the GPLv2

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: vfb.c.patch --]
[-- Type: text/x-patch; name="vfb.c.patch", Size: 1089 bytes --]

---
 drivers/video/fbdev/vfb.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/video/fbdev/vfb.c b/drivers/video/fbdev/vfb.c
index da653a080394..5fb360283c6b 100644
--- a/drivers/video/fbdev/vfb.c
+++ b/drivers/video/fbdev/vfb.c
@@ -239,8 +239,23 @@ static int vfb_check_var(struct fb_var_screeninfo *var,
  */
 static int vfb_set_par(struct fb_info *info)
 {
+	switch (info->var.bits_per_pixel) {
+		case 1:
+			info->fix.visual = FB_VISUAL_MONO01;
+			break;
+		case 8:
+			info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
+			break;
+		case 16:
+		case 24:
+		case 32:
+			info->fix.visual = FB_VISUAL_TRUECOLOR;
+			break;
+	}
+
 	info->fix.line_length = get_line_length(info->var.xres_virtual,
 						info->var.bits_per_pixel);
+
 	return 0;
 }
 
@@ -450,6 +465,8 @@ static int vfb_probe(struct platform_device *dev)
 		goto err2;
 	platform_set_drvdata(dev, info);
 
+	vfb_set_par(info);
+
 	fb_info(info, "Virtual frame buffer device, using %ldK of video memory\n",
 		videomemorysize >> 10);
 	return 0;
--
2.15.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] vfb: fix video mode and line_length being set when loaded
  2017-12-20 14:53   ` Pieter "PoroCYon" Sluys
@ 2018-01-04 15:05       ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-01-04 15:05 UTC (permalink / raw)
  To: Pieter "PoroCYon" Sluys
  Cc: linux-fbdev, linux-kernel, Geert Uytterhoeven

On Wednesday, December 20, 2017 02:53:00 PM Pieter PoroCYon Sluys wrote:
> Geert Uytterhoeven:
> > Please add your "Signed-off-by", cfr.
> > Documentation/process/submitting-patches.rst.
> 
> Sorry I knew I would've messed up *something*.
> 
> > You can drop the tests for 2 and 4, as these are no longer used by the driver.
> 
> Done that.
> 
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Pieter "PoroCYon" Sluys <pcy@national.shitposting.agency>

I've queued your patch for 4.16 kernel (after fixing few minor
issues [*]), thanks!

[*] next time please remember to:

* always include full patch description in the later patch revisions

* put things that don't belong to patch description after "---" so
  they will not be included in the final commit

* in the later patch revisions include ChangeLog after "---" and also
  put patch version in the patch summary line / mail subject
  (i.e. "[PATCH v2] ...")

* add tags (i.e. "Reviewed-by: ...") from patch reviewers / testers
  into the patch description

* check your patch with scripts/checkpatch.pl tool and fix reported
  issues, i.e.:

ERROR: switch and case should be at the same indent
#69: FILE: drivers/video/fbdev/vfb.c:242:
+       switch (info->var.bits_per_pixel) {
+               case 1:
[...]
+               case 8:
[...]
+               case 16:
+               case 24:
+               case 32:

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vfb: fix video mode and line_length being set when loaded
@ 2018-01-04 15:05       ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2018-01-04 15:05 UTC (permalink / raw)
  To: Pieter "PoroCYon" Sluys
  Cc: linux-fbdev, linux-kernel, Geert Uytterhoeven

On Wednesday, December 20, 2017 02:53:00 PM Pieter PoroCYon Sluys wrote:
> Geert Uytterhoeven:
> > Please add your "Signed-off-by", cfr.
> > Documentation/process/submitting-patches.rst.
> 
> Sorry I knew I would've messed up *something*.
> 
> > You can drop the tests for 2 and 4, as these are no longer used by the driver.
> 
> Done that.
> 
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Pieter "PoroCYon" Sluys <pcy@national.shitposting.agency>

I've queued your patch for 4.16 kernel (after fixing few minor
issues [*]), thanks!

[*] next time please remember to:

* always include full patch description in the later patch revisions

* put things that don't belong to patch description after "---" so
  they will not be included in the final commit

* in the later patch revisions include ChangeLog after "---" and also
  put patch version in the patch summary line / mail subject
  (i.e. "[PATCH v2] ...")

* add tags (i.e. "Reviewed-by: ...") from patch reviewers / testers
  into the patch description

* check your patch with scripts/checkpatch.pl tool and fix reported
  issues, i.e.:

ERROR: switch and case should be at the same indent
#69: FILE: drivers/video/fbdev/vfb.c:242:
+       switch (info->var.bits_per_pixel) {
+               case 1:
[...]
+               case 8:
[...]
+               case 16:
+               case 24:
+               case 32:

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-01-04 15:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-20  0:24 [PATCH] vfb: fix video mode and line_length being set when loaded Pieter "PoroCYon" Sluys
2017-12-20  8:26 ` Geert Uytterhoeven
2017-12-20  8:26   ` Geert Uytterhoeven
2017-12-20 14:53   ` Pieter "PoroCYon" Sluys
2018-01-04 15:05     ` Bartlomiej Zolnierkiewicz
2018-01-04 15:05       ` Bartlomiej Zolnierkiewicz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.