All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvidia/noveau: Fix color mask
@ 2015-06-17 17:05 ` Michael Büsch
  0 siblings, 0 replies; 8+ messages in thread
From: Michael Büsch @ 2015-06-17 17:05 UTC (permalink / raw)
  To: dri-devel, David Airlie, Antonino Daplas, linux-fbdev

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

The expression (~0 >> x) will always yield all-ones, because the right
shift is an arithmetic right shift that will always shift ones in.
Accordingly ~(~0 >> x) will always be zero.
Hence 'mask' will always be zero in this case.

Fix this by forcing a logical right shift instead of an arithmetic
right shift by using an unsigned int constant.

Signed-off-by: Michael Buesch <m@bues.ch>

---

This patch is untested, because I do not have the hardware.


Index: linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
===================================================================
--- linux.orig/drivers/gpu/drm/nouveau/nv50_fbcon.c
+++ linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
@@ -96,7 +96,7 @@ nv50_fbcon_imageblit(struct fb_info *inf
 	struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
 	struct nouveau_channel *chan = drm->channel;
 	uint32_t width, dwords, *data = (uint32_t *)image->data;
-	uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	uint32_t *palette = info->pseudo_palette;
 	int ret;
 
Index: linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
===================================================================
--- linux.orig/drivers/gpu/drm/nouveau/nvc0_fbcon.c
+++ linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
@@ -96,7 +96,7 @@ nvc0_fbcon_imageblit(struct fb_info *inf
 	struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
 	struct nouveau_channel *chan = drm->channel;
 	uint32_t width, dwords, *data = (uint32_t *)image->data;
-	uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	uint32_t *palette = info->pseudo_palette;
 	int ret;
 
Index: linux/drivers/video/fbdev/nvidia/nv_accel.c
===================================================================
--- linux.orig/drivers/video/fbdev/nvidia/nv_accel.c
+++ linux/drivers/video/fbdev/nvidia/nv_accel.c
@@ -351,7 +351,7 @@ static void nvidiafb_mono_color_expand(s
 				       const struct fb_image *image)
 {
 	struct nvidia_par *par = info->par;
-	u32 fg, bg, mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	u32 fg, bg, mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	u32 dsize, width, *data = (u32 *) image->data, tmp;
 	int j, k = 0;
 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* [PATCH] nvidia/noveau: Fix color mask
@ 2015-06-17 17:05 ` Michael Büsch
  0 siblings, 0 replies; 8+ messages in thread
From: Michael Büsch @ 2015-06-17 17:05 UTC (permalink / raw)
  To: dri-devel, David Airlie, Antonino Daplas, linux-fbdev


[-- Attachment #1.1: Type: text/plain, Size: 2300 bytes --]

The expression (~0 >> x) will always yield all-ones, because the right
shift is an arithmetic right shift that will always shift ones in.
Accordingly ~(~0 >> x) will always be zero.
Hence 'mask' will always be zero in this case.

Fix this by forcing a logical right shift instead of an arithmetic
right shift by using an unsigned int constant.

Signed-off-by: Michael Buesch <m@bues.ch>

---

This patch is untested, because I do not have the hardware.


Index: linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
===================================================================
--- linux.orig/drivers/gpu/drm/nouveau/nv50_fbcon.c
+++ linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
@@ -96,7 +96,7 @@ nv50_fbcon_imageblit(struct fb_info *inf
 	struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
 	struct nouveau_channel *chan = drm->channel;
 	uint32_t width, dwords, *data = (uint32_t *)image->data;
-	uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	uint32_t *palette = info->pseudo_palette;
 	int ret;
 
Index: linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
===================================================================
--- linux.orig/drivers/gpu/drm/nouveau/nvc0_fbcon.c
+++ linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
@@ -96,7 +96,7 @@ nvc0_fbcon_imageblit(struct fb_info *inf
 	struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
 	struct nouveau_channel *chan = drm->channel;
 	uint32_t width, dwords, *data = (uint32_t *)image->data;
-	uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	uint32_t *palette = info->pseudo_palette;
 	int ret;
 
Index: linux/drivers/video/fbdev/nvidia/nv_accel.c
===================================================================
--- linux.orig/drivers/video/fbdev/nvidia/nv_accel.c
+++ linux/drivers/video/fbdev/nvidia/nv_accel.c
@@ -351,7 +351,7 @@ static void nvidiafb_mono_color_expand(s
 				       const struct fb_image *image)
 {
 	struct nvidia_par *par = info->par;
-	u32 fg, bg, mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	u32 fg, bg, mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	u32 dsize, width, *data = (u32 *) image->data, tmp;
 	int j, k = 0;
 

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH] nvidia/noveau: Fix color mask
  2015-06-17 17:05 ` Michael Büsch
@ 2015-06-18  0:47   ` Ilia Mirkin
  -1 siblings, 0 replies; 8+ messages in thread
From: Ilia Mirkin @ 2015-06-18  0:47 UTC (permalink / raw)
  To: Michael Büsch
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Antonino Daplas,
	David Airlie, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs

On Wed, Jun 17, 2015 at 1:05 PM, Michael Büsch <m@bues.ch> wrote:
> The expression (~0 >> x) will always yield all-ones, because the right
> shift is an arithmetic right shift that will always shift ones in.
> Accordingly ~(~0 >> x) will always be zero.
> Hence 'mask' will always be zero in this case.
>
> Fix this by forcing a logical right shift instead of an arithmetic
> right shift by using an unsigned int constant.
>
> Signed-off-by: Michael Buesch <m@bues.ch>

Confirmed that this does indeed happen with

#include <stdio.h>
int main(int argc, char *argv[]) {
  unsigned mask = ~(~0 >> (32 - (argv[1][0] - '0')));
  printf("%08x\n", mask);
}

I guess fbdev/nvidia/nv_accel.c was the source of all this, as the
code is identical, and it probably came first.

FWIW this is

Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>


>
> ---
>
> This patch is untested, because I do not have the hardware.
>
>
> Index: linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
> =================================> --- linux.orig/drivers/gpu/drm/nouveau/nv50_fbcon.c
> +++ linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
> @@ -96,7 +96,7 @@ nv50_fbcon_imageblit(struct fb_info *inf
>         struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
>         struct nouveau_channel *chan = drm->channel;
>         uint32_t width, dwords, *data = (uint32_t *)image->data;
> -       uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
> +       uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
>         uint32_t *palette = info->pseudo_palette;
>         int ret;
>
> Index: linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
> =================================> --- linux.orig/drivers/gpu/drm/nouveau/nvc0_fbcon.c
> +++ linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
> @@ -96,7 +96,7 @@ nvc0_fbcon_imageblit(struct fb_info *inf
>         struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
>         struct nouveau_channel *chan = drm->channel;
>         uint32_t width, dwords, *data = (uint32_t *)image->data;
> -       uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
> +       uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
>         uint32_t *palette = info->pseudo_palette;
>         int ret;
>
> Index: linux/drivers/video/fbdev/nvidia/nv_accel.c
> =================================> --- linux.orig/drivers/video/fbdev/nvidia/nv_accel.c
> +++ linux/drivers/video/fbdev/nvidia/nv_accel.c
> @@ -351,7 +351,7 @@ static void nvidiafb_mono_color_expand(s
>                                        const struct fb_image *image)
>  {
>         struct nvidia_par *par = info->par;
> -       u32 fg, bg, mask = ~(~0 >> (32 - info->var.bits_per_pixel));
> +       u32 fg, bg, mask = ~(~0U >> (32 - info->var.bits_per_pixel));
>         u32 dsize, width, *data = (u32 *) image->data, tmp;
>         int j, k = 0;
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

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

* Re: [PATCH] nvidia/noveau: Fix color mask
@ 2015-06-18  0:47   ` Ilia Mirkin
  0 siblings, 0 replies; 8+ messages in thread
From: Ilia Mirkin @ 2015-06-18  0:47 UTC (permalink / raw)
  To: Michael Büsch
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Antonino Daplas,
	David Airlie, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs

On Wed, Jun 17, 2015 at 1:05 PM, Michael Büsch <m@bues.ch> wrote:
> The expression (~0 >> x) will always yield all-ones, because the right
> shift is an arithmetic right shift that will always shift ones in.
> Accordingly ~(~0 >> x) will always be zero.
> Hence 'mask' will always be zero in this case.
>
> Fix this by forcing a logical right shift instead of an arithmetic
> right shift by using an unsigned int constant.
>
> Signed-off-by: Michael Buesch <m@bues.ch>

Confirmed that this does indeed happen with

#include <stdio.h>
int main(int argc, char *argv[]) {
  unsigned mask = ~(~0 >> (32 - (argv[1][0] - '0')));
  printf("%08x\n", mask);
}

I guess fbdev/nvidia/nv_accel.c was the source of all this, as the
code is identical, and it probably came first.

FWIW this is

Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>


>
> ---
>
> This patch is untested, because I do not have the hardware.
>
>
> Index: linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/nouveau/nv50_fbcon.c
> +++ linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
> @@ -96,7 +96,7 @@ nv50_fbcon_imageblit(struct fb_info *inf
>         struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
>         struct nouveau_channel *chan = drm->channel;
>         uint32_t width, dwords, *data = (uint32_t *)image->data;
> -       uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
> +       uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
>         uint32_t *palette = info->pseudo_palette;
>         int ret;
>
> Index: linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/nouveau/nvc0_fbcon.c
> +++ linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
> @@ -96,7 +96,7 @@ nvc0_fbcon_imageblit(struct fb_info *inf
>         struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
>         struct nouveau_channel *chan = drm->channel;
>         uint32_t width, dwords, *data = (uint32_t *)image->data;
> -       uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
> +       uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
>         uint32_t *palette = info->pseudo_palette;
>         int ret;
>
> Index: linux/drivers/video/fbdev/nvidia/nv_accel.c
> ===================================================================
> --- linux.orig/drivers/video/fbdev/nvidia/nv_accel.c
> +++ linux/drivers/video/fbdev/nvidia/nv_accel.c
> @@ -351,7 +351,7 @@ static void nvidiafb_mono_color_expand(s
>                                        const struct fb_image *image)
>  {
>         struct nvidia_par *par = info->par;
> -       u32 fg, bg, mask = ~(~0 >> (32 - info->var.bits_per_pixel));
> +       u32 fg, bg, mask = ~(~0U >> (32 - info->var.bits_per_pixel));
>         u32 dsize, width, *data = (u32 *) image->data, tmp;
>         int j, k = 0;
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

* Re: [PATCH] nvidia/noveau: Fix color mask
  2015-06-18  0:47   ` Ilia Mirkin
@ 2015-06-18 15:31     ` Michael Büsch
  -1 siblings, 0 replies; 8+ messages in thread
From: Michael Büsch @ 2015-06-18 15:31 UTC (permalink / raw)
  To: Ilia Mirkin; +Cc: linux-fbdev, nouveau, dri-devel, Ben Skeggs

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

On Wed, 17 Jun 2015 20:47:17 -0400
Ilia Mirkin <imirkin@alum.mit.edu> wrote:

> On Wed, Jun 17, 2015 at 1:05 PM, Michael Büsch <m@bues.ch> wrote:
> > The expression (~0 >> x) will always yield all-ones, because the right
> > shift is an arithmetic right shift that will always shift ones in.
> > Accordingly ~(~0 >> x) will always be zero.
> > Hence 'mask' will always be zero in this case.
> >
> > Fix this by forcing a logical right shift instead of an arithmetic
> > right shift by using an unsigned int constant.
> >
> > Signed-off-by: Michael Buesch <m@bues.ch>
> 
> Confirmed that this does indeed happen with
> 
> #include <stdio.h>
> int main(int argc, char *argv[]) {
>   unsigned mask = ~(~0 >> (32 - (argv[1][0] - '0')));
>   printf("%08x\n", mask);
> }
> 
> I guess fbdev/nvidia/nv_accel.c was the source of all this, as the
> code is identical, and it probably came first.


If anybody is able to help me in creating a working semantic patch
(coccinelle) for this, that would be great.

I found this using a very hacky and incorrect spatch (some
version is attached). It throws many false positives, doesn't find all
such bugs and does not create correct patch output (especially the
#define related rule is just meant as a hint).

Some basic thoughts that come to mind that could possibly be statically
checked somehow are:

- right shift of promoted variables. That is stuff like this:
  u8 x, y = 0x0F;
  x = ~y >> 1;
  /* x is 0xF8, not 0x78 as someone might expect. */
- Also check this for typedef'ed types where promotion takes place (that
  are smaller than int)?
- right shift of signed constants (like in this case). That probably is
  wrong in most cases.
  How to check signedness of constants in spatch? (123 vs 123U)
  Is that even possible?
- Also detect this stuff, if variables/constants are hidden via #define
  or such:
  #define REGVAL	0x0F
  writereg(REGISTER, ~REGVAL >> 1);

Probably more stuff could be checked. Ideas are welcome. :)

-- 
Michael



@@
u8 e;
expression s;
@@
- ~e >> s
+ (u8)~e >> s

@@
s8 e;
expression s;
@@
- ~e >> s
+ (s8)~e >> s

@@
u16 e;
expression s;
@@
- ~e >> s
+ (u16)~e >> s

@@
s16 e;
expression s;
@@
- ~e >> s
+ (s16)~e >> s

@@
char e;
expression s;
@@
- ~e >> s
+ (char)~e >> s

@@
unsigned char e;
expression s;
@@
- ~e >> s
+ (unsigned char)~e >> s

@@
short e;
expression s;
@@
- ~e >> s
+ (short)~e >> s

@@
unsigned short e;
expression s;
@@
- ~e >> s
+ (unsigned short)~e >> s




@@
constant c;
expression s;
@@
- ~c >> s
+ (unsigned int)~c >> s





@sh expression@
identifier val;
expression shift;
@@
val >> shift


@@
expression e;
identifier sh.val;
@@
- #define val ~e
+ #define val e


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH] nvidia/noveau: Fix color mask
@ 2015-06-18 15:31     ` Michael Büsch
  0 siblings, 0 replies; 8+ messages in thread
From: Michael Büsch @ 2015-06-18 15:31 UTC (permalink / raw)
  To: Ilia Mirkin; +Cc: linux-fbdev, nouveau, dri-devel, Ben Skeggs


[-- Attachment #1.1: Type: text/plain, Size: 2814 bytes --]

On Wed, 17 Jun 2015 20:47:17 -0400
Ilia Mirkin <imirkin@alum.mit.edu> wrote:

> On Wed, Jun 17, 2015 at 1:05 PM, Michael Büsch <m@bues.ch> wrote:
> > The expression (~0 >> x) will always yield all-ones, because the right
> > shift is an arithmetic right shift that will always shift ones in.
> > Accordingly ~(~0 >> x) will always be zero.
> > Hence 'mask' will always be zero in this case.
> >
> > Fix this by forcing a logical right shift instead of an arithmetic
> > right shift by using an unsigned int constant.
> >
> > Signed-off-by: Michael Buesch <m@bues.ch>
> 
> Confirmed that this does indeed happen with
> 
> #include <stdio.h>
> int main(int argc, char *argv[]) {
>   unsigned mask = ~(~0 >> (32 - (argv[1][0] - '0')));
>   printf("%08x\n", mask);
> }
> 
> I guess fbdev/nvidia/nv_accel.c was the source of all this, as the
> code is identical, and it probably came first.


If anybody is able to help me in creating a working semantic patch
(coccinelle) for this, that would be great.

I found this using a very hacky and incorrect spatch (some
version is attached). It throws many false positives, doesn't find all
such bugs and does not create correct patch output (especially the
#define related rule is just meant as a hint).

Some basic thoughts that come to mind that could possibly be statically
checked somehow are:

- right shift of promoted variables. That is stuff like this:
  u8 x, y = 0x0F;
  x = ~y >> 1;
  /* x is 0xF8, not 0x78 as someone might expect. */
- Also check this for typedef'ed types where promotion takes place (that
  are smaller than int)?
- right shift of signed constants (like in this case). That probably is
  wrong in most cases.
  How to check signedness of constants in spatch? (123 vs 123U)
  Is that even possible?
- Also detect this stuff, if variables/constants are hidden via #define
  or such:
  #define REGVAL	0x0F
  writereg(REGISTER, ~REGVAL >> 1);

Probably more stuff could be checked. Ideas are welcome. :)

-- 
Michael



@@
u8 e;
expression s;
@@
- ~e >> s
+ (u8)~e >> s

@@
s8 e;
expression s;
@@
- ~e >> s
+ (s8)~e >> s

@@
u16 e;
expression s;
@@
- ~e >> s
+ (u16)~e >> s

@@
s16 e;
expression s;
@@
- ~e >> s
+ (s16)~e >> s

@@
char e;
expression s;
@@
- ~e >> s
+ (char)~e >> s

@@
unsigned char e;
expression s;
@@
- ~e >> s
+ (unsigned char)~e >> s

@@
short e;
expression s;
@@
- ~e >> s
+ (short)~e >> s

@@
unsigned short e;
expression s;
@@
- ~e >> s
+ (unsigned short)~e >> s




@@
constant c;
expression s;
@@
- ~c >> s
+ (unsigned int)~c >> s





@sh expression@
identifier val;
expression shift;
@@
val >> shift


@@
expression e;
identifier sh.val;
@@
- #define val ~e
+ #define val e


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH] nvidia/noveau: Fix color mask
  2015-06-17 17:05 ` Michael Büsch
@ 2015-11-19 20:10 ` Michael Büsch
  -1 siblings, 0 replies; 8+ messages in thread
From: Michael Büsch @ 2015-11-19 20:10 UTC (permalink / raw)
  To: dri-devel, David Airlie, Antonino Daplas, linux-fbdev,
	linux-kernel, Andrew Morton

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

The expression (~0 >> x) will always yield all-ones, because the right
shift is an arithmetic right shift that will always shift ones in.
Accordingly ~(~0 >> x) will always be zero.
Hence 'mask' will always be zero in this case.

Fix this by forcing a logical right shift instead of an arithmetic
right shift by using an unsigned int constant.

Signed-off-by: Michael Buesch <m@bues.ch>

---

This patch is untested, because I do not have the hardware.

Resend: Patch was originally sent on Wed, 17 Jun 2015.


Index: linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
===================================================================
--- linux.orig/drivers/gpu/drm/nouveau/nv50_fbcon.c
+++ linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
@@ -96,7 +96,7 @@ nv50_fbcon_imageblit(struct fb_info *inf
 	struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
 	struct nouveau_channel *chan = drm->channel;
 	uint32_t width, dwords, *data = (uint32_t *)image->data;
-	uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	uint32_t *palette = info->pseudo_palette;
 	int ret;
 
Index: linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
===================================================================
--- linux.orig/drivers/gpu/drm/nouveau/nvc0_fbcon.c
+++ linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
@@ -96,7 +96,7 @@ nvc0_fbcon_imageblit(struct fb_info *inf
 	struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
 	struct nouveau_channel *chan = drm->channel;
 	uint32_t width, dwords, *data = (uint32_t *)image->data;
-	uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	uint32_t *palette = info->pseudo_palette;
 	int ret;
 
Index: linux/drivers/video/fbdev/nvidia/nv_accel.c
===================================================================
--- linux.orig/drivers/video/fbdev/nvidia/nv_accel.c
+++ linux/drivers/video/fbdev/nvidia/nv_accel.c
@@ -351,7 +351,7 @@ static void nvidiafb_mono_color_expand(s
 				       const struct fb_image *image)
 {
 	struct nvidia_par *par = info->par;
-	u32 fg, bg, mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	u32 fg, bg, mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	u32 dsize, width, *data = (u32 *) image->data, tmp;
 	int j, k = 0;
 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* [PATCH] nvidia/noveau: Fix color mask
@ 2015-11-19 20:10 ` Michael Büsch
  0 siblings, 0 replies; 8+ messages in thread
From: Michael Büsch @ 2015-11-19 20:10 UTC (permalink / raw)
  To: dri-devel, David Airlie, Antonino Daplas, linux-fbdev,
	linux-kernel, Andrew Morton

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

The expression (~0 >> x) will always yield all-ones, because the right
shift is an arithmetic right shift that will always shift ones in.
Accordingly ~(~0 >> x) will always be zero.
Hence 'mask' will always be zero in this case.

Fix this by forcing a logical right shift instead of an arithmetic
right shift by using an unsigned int constant.

Signed-off-by: Michael Buesch <m@bues.ch>

---

This patch is untested, because I do not have the hardware.

Resend: Patch was originally sent on Wed, 17 Jun 2015.


Index: linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
===================================================================
--- linux.orig/drivers/gpu/drm/nouveau/nv50_fbcon.c
+++ linux/drivers/gpu/drm/nouveau/nv50_fbcon.c
@@ -96,7 +96,7 @@ nv50_fbcon_imageblit(struct fb_info *inf
 	struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
 	struct nouveau_channel *chan = drm->channel;
 	uint32_t width, dwords, *data = (uint32_t *)image->data;
-	uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	uint32_t *palette = info->pseudo_palette;
 	int ret;
 
Index: linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
===================================================================
--- linux.orig/drivers/gpu/drm/nouveau/nvc0_fbcon.c
+++ linux/drivers/gpu/drm/nouveau/nvc0_fbcon.c
@@ -96,7 +96,7 @@ nvc0_fbcon_imageblit(struct fb_info *inf
 	struct nouveau_drm *drm = nouveau_drm(nfbdev->dev);
 	struct nouveau_channel *chan = drm->channel;
 	uint32_t width, dwords, *data = (uint32_t *)image->data;
-	uint32_t mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	uint32_t mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	uint32_t *palette = info->pseudo_palette;
 	int ret;
 
Index: linux/drivers/video/fbdev/nvidia/nv_accel.c
===================================================================
--- linux.orig/drivers/video/fbdev/nvidia/nv_accel.c
+++ linux/drivers/video/fbdev/nvidia/nv_accel.c
@@ -351,7 +351,7 @@ static void nvidiafb_mono_color_expand(s
 				       const struct fb_image *image)
 {
 	struct nvidia_par *par = info->par;
-	u32 fg, bg, mask = ~(~0 >> (32 - info->var.bits_per_pixel));
+	u32 fg, bg, mask = ~(~0U >> (32 - info->var.bits_per_pixel));
 	u32 dsize, width, *data = (u32 *) image->data, tmp;
 	int j, k = 0;
 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

end of thread, other threads:[~2015-11-19 20:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17 17:05 [PATCH] nvidia/noveau: Fix color mask Michael Büsch
2015-06-17 17:05 ` Michael Büsch
2015-06-18  0:47 ` Ilia Mirkin
2015-06-18  0:47   ` Ilia Mirkin
2015-06-18 15:31   ` Michael Büsch
2015-06-18 15:31     ` Michael Büsch
2015-11-19 20:10 Michael Büsch
2015-11-19 20:10 ` Michael Büsch

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.