2016-03-03 10:26:15

by Paul Bolle

[permalink] [raw]
Subject: [PATCH] drm/vmwgfx: use *_32_bits() macros

Use the upper_32_bits() macro instead of the four line equivalent that
triggers a GCC warning on 32 bits x86:
drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c: In function 'vmw_cmdbuf_header_submit':
drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c:297:25: warning: right shift count >= width of type [-Wshift-count-overflow]
val = (header->handle >> 32);
^

And use the lower_32_bits() macro instead of and-ing with a 32 bits
mask.

Signed-off-by: Paul Bolle <[email protected]>
---
Note: compile tested only (I don't use any of vmware's products).

drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
index 67cebb23c940..aa04fb0159a7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
@@ -293,13 +293,10 @@ static int vmw_cmdbuf_header_submit(struct vmw_cmdbuf_header *header)
struct vmw_cmdbuf_man *man = header->man;
u32 val;

- if (sizeof(header->handle) > 4)
- val = (header->handle >> 32);
- else
- val = 0;
+ val = upper_32_bits(header->handle);
vmw_write(man->dev_priv, SVGA_REG_COMMAND_HIGH, val);

- val = (header->handle & 0xFFFFFFFFULL);
+ val = lower_32_bits(header->handle);
val |= header->cb_context & SVGA_CB_CONTEXT_MASK;
vmw_write(man->dev_priv, SVGA_REG_COMMAND_LOW, val);

--
2.4.3


2016-04-14 11:32:27

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] drm/vmwgfx: use *_32_bits() macros

On do, 2016-03-03 at 11:26 +0100, Paul Bolle wrote:
> Use the upper_32_bits() macro instead of the four line equivalent that
> triggers a GCC warning on 32 bits x86:
> drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c: In function
> 'vmw_cmdbuf_header_submit':
> drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c:297:25: warning: right
> shift count >= width of type [-Wshift-count-overflow]
> val = (header->handle >> 32);
> ^
>
> And use the lower_32_bits() macro instead of and-ing with a 32 bits
> mask.
>
> Signed-off-by: Paul Bolle <[email protected]>
> ---
> Note: compile tested only (I don't use any of vmware's products).

The warning can still be seen on v4.6-rc3 for 32 bits x86. This patch
applies cleanly to that rc.

Has anyone had a chance to look at this patch, and perhaps even test it?

Thanks,


Paul Bolle

> drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
> index 67cebb23c940..aa04fb0159a7 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
> @@ -293,13 +293,10 @@ static int vmw_cmdbuf_header_submit(struct
> vmw_cmdbuf_header *header)
> struct vmw_cmdbuf_man *man = header->man;
> u32 val;
>
> - if (sizeof(header->handle) > 4)
> - val = (header->handle >> 32);
> - else
> - val = 0;
> + val = upper_32_bits(header->handle);
> vmw_write(man->dev_priv, SVGA_REG_COMMAND_HIGH, val);
>
> - val = (header->handle & 0xFFFFFFFFULL);
> + val = lower_32_bits(header->handle);
> val |= header->cb_context & SVGA_CB_CONTEXT_MASK;
> vmw_write(man->dev_priv, SVGA_REG_COMMAND_LOW, val);
>

2016-04-14 14:34:16

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] drm/vmwgfx: use *_32_bits() macros

On Thu, 2016-04-14 at 13:32 +0200, Paul Bolle wrote:
> On do, 2016-03-03 at 11:26 +0100, Paul Bolle wrote:
> >
> > Use the upper_32_bits() macro instead of the four line equivalent that
> > triggers a GCC warning on 32 bits x86:
> > ????drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c: In function
> > 'vmw_cmdbuf_header_submit':
> > ????drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c:297:25: warning: right
> > shift count >= width of type [-Wshift-count-overflow]
> > ???????val = (header->handle >> 32);
> > ?????????????????????????????^
> >
> > And use the lower_32_bits() macro instead of and-ing with a 32 bits
> > mask.
> >
> > Signed-off-by: Paul Bolle <[email protected]>
> > ---
> > Note: compile tested only (I don't use any of vmware's products).
> The warning can still be seen on v4.6-rc3 for 32 bits x86. This patch
> applies cleanly to that rc.
>
> Has anyone had a chance to look at this patch, and perhaps even test it?

Test? ?Nope. ?Seems obviously correct.

It might be clearer removing the temporary though:

Maybe:
---
?drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c | 15 +++++----------
?1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
index 67cebb2..330794f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
@@ -291,17 +291,12 @@ void vmw_cmdbuf_header_free(struct vmw_cmdbuf_header *header)
?static int vmw_cmdbuf_header_submit(struct vmw_cmdbuf_header *header)
?{
? struct vmw_cmdbuf_man *man = header->man;
- u32 val;
?
- if (sizeof(header->handle) > 4)
- val = (header->handle >> 32);
- else
- val = 0;
- vmw_write(man->dev_priv, SVGA_REG_COMMAND_HIGH, val);
-
- val = (header->handle & 0xFFFFFFFFULL);
- val |= header->cb_context & SVGA_CB_CONTEXT_MASK;
- vmw_write(man->dev_priv, SVGA_REG_COMMAND_LOW, val);
+ vmw_write(man->dev_priv, SVGA_REG_COMMAND_HIGH,
+ ??upper_32_bits(header->handle));
+ vmw_write(man->dev_priv, SVGA_REG_COMMAND_LOW,
+ ??lower_32_bits(header->handle) |
+ ??(header->cb_context & SVGA_CB_CONTEXT_MASK));
?
? return header->cb_header->status;
?}