* [PATCH] rendercopy/gen8: Remove a hole in struct gen8_blend_state
@ 2014-03-24 18:53 Damien Lespiau
2014-03-25 2:14 ` Ben Widawsky
2014-03-26 18:45 ` Barbalho, Rafael
0 siblings, 2 replies; 4+ messages in thread
From: Damien Lespiau @ 2014-03-24 18:53 UTC (permalink / raw
To: intel-gfx; +Cc: Ben Widawsky
Using uint64_t in that second member makes it aligned to 64bits, while
the first member is only 32bits. We then had a 32bits hole in there!
Found-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Rafael Barbalho <rafael.barbalho@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
lib/gen8_render.h | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/lib/gen8_render.h b/lib/gen8_render.h
index ca53d64..fffc100 100644
--- a/lib/gen8_render.h
+++ b/lib/gen8_render.h
@@ -273,25 +273,25 @@ struct gen8_blend_state {
} bs0;
struct {
- uint64_t write_disable_blue:1;
- uint64_t write_disable_green:1;
- uint64_t write_disable_red:1;
- uint64_t write_disable_alpha:1;
- uint64_t pad1:1;
- uint64_t alpha_blend_func:3;
- uint64_t dest_alpha_blend_factor:5;
- uint64_t source_alpha_blend_factor:5;
- uint64_t color_blend_func:3;
- uint64_t dest_blend_factor:5;
- uint64_t source_blend_factor:5;
- uint64_t color_buffer_blend:1;
- uint64_t post_blend_color_clamp:1;
- uint64_t pre_blend_color_clamp:1;
- uint64_t color_clamp_range:2;
- uint64_t pre_blend_source_only_clamp:1;
- uint64_t pad0:22;
- uint64_t logic_op_func:4;
- uint64_t logic_op_enable:1;
+ uint32_t write_disable_blue:1;
+ uint32_t write_disable_green:1;
+ uint32_t write_disable_red:1;
+ uint32_t write_disable_alpha:1;
+ uint32_t pad1:1;
+ uint32_t alpha_blend_func:3;
+ uint32_t dest_alpha_blend_factor:5;
+ uint32_t source_alpha_blend_factor:5;
+ uint32_t color_blend_func:3;
+ uint32_t dest_blend_factor:5;
+ uint32_t source_blend_factor:5;
+ uint32_t color_buffer_blend:1;
+ uint32_t post_blend_color_clamp:1;
+ uint32_t pre_blend_color_clamp:1;
+ uint32_t color_clamp_range:2;
+ uint32_t pre_blend_source_only_clamp:1;
+ uint32_t pad0:22;
+ uint32_t logic_op_func:4;
+ uint32_t logic_op_enable:1;
} bs[16];
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] rendercopy/gen8: Remove a hole in struct gen8_blend_state
2014-03-24 18:53 [PATCH] rendercopy/gen8: Remove a hole in struct gen8_blend_state Damien Lespiau
@ 2014-03-25 2:14 ` Ben Widawsky
2014-03-26 18:45 ` Barbalho, Rafael
1 sibling, 0 replies; 4+ messages in thread
From: Ben Widawsky @ 2014-03-25 2:14 UTC (permalink / raw
To: Damien Lespiau; +Cc: intel-gfx, Ben Widawsky
On Mon, Mar 24, 2014 at 06:53:47PM +0000, Damien Lespiau wrote:
> Using uint64_t in that second member makes it aligned to 64bits, while
> the first member is only 32bits. We then had a 32bits hole in there!
>
> Found-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Rafael Barbalho <rafael.barbalho@intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
I sent a mail 9 hours ago then never appeared to arrive. Hopefully it
won't show up later. Recapping:
lgtm - maybe a compile time assert on the struct would be a nice patch
on top of this?
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> lib/gen8_render.h | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/lib/gen8_render.h b/lib/gen8_render.h
> index ca53d64..fffc100 100644
> --- a/lib/gen8_render.h
> +++ b/lib/gen8_render.h
> @@ -273,25 +273,25 @@ struct gen8_blend_state {
> } bs0;
>
> struct {
> - uint64_t write_disable_blue:1;
> - uint64_t write_disable_green:1;
> - uint64_t write_disable_red:1;
> - uint64_t write_disable_alpha:1;
> - uint64_t pad1:1;
> - uint64_t alpha_blend_func:3;
> - uint64_t dest_alpha_blend_factor:5;
> - uint64_t source_alpha_blend_factor:5;
> - uint64_t color_blend_func:3;
> - uint64_t dest_blend_factor:5;
> - uint64_t source_blend_factor:5;
> - uint64_t color_buffer_blend:1;
> - uint64_t post_blend_color_clamp:1;
> - uint64_t pre_blend_color_clamp:1;
> - uint64_t color_clamp_range:2;
> - uint64_t pre_blend_source_only_clamp:1;
> - uint64_t pad0:22;
> - uint64_t logic_op_func:4;
> - uint64_t logic_op_enable:1;
> + uint32_t write_disable_blue:1;
> + uint32_t write_disable_green:1;
> + uint32_t write_disable_red:1;
> + uint32_t write_disable_alpha:1;
> + uint32_t pad1:1;
> + uint32_t alpha_blend_func:3;
> + uint32_t dest_alpha_blend_factor:5;
> + uint32_t source_alpha_blend_factor:5;
> + uint32_t color_blend_func:3;
> + uint32_t dest_blend_factor:5;
> + uint32_t source_blend_factor:5;
> + uint32_t color_buffer_blend:1;
> + uint32_t post_blend_color_clamp:1;
> + uint32_t pre_blend_color_clamp:1;
> + uint32_t color_clamp_range:2;
> + uint32_t pre_blend_source_only_clamp:1;
> + uint32_t pad0:22;
> + uint32_t logic_op_func:4;
> + uint32_t logic_op_enable:1;
> } bs[16];
> };
>
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rendercopy/gen8: Remove a hole in struct gen8_blend_state
2014-03-24 18:53 [PATCH] rendercopy/gen8: Remove a hole in struct gen8_blend_state Damien Lespiau
2014-03-25 2:14 ` Ben Widawsky
@ 2014-03-26 18:45 ` Barbalho, Rafael
2014-03-26 19:07 ` Damien Lespiau
1 sibling, 1 reply; 4+ messages in thread
From: Barbalho, Rafael @ 2014-03-26 18:45 UTC (permalink / raw
To: Lespiau, Damien, intel-gfx@lists.freedesktop.org; +Cc: Widawsky, Benjamin
> -----Original Message-----
> From: Lespiau, Damien
> Sent: Monday, March 24, 2014 11:54 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Widawsky, Benjamin; Barbalho, Rafael
> Subject: [PATCH] rendercopy/gen8: Remove a hole in struct
> gen8_blend_state
>
> Using uint64_t in that second member makes it aligned to 64bits, while the
> first member is only 32bits. We then had a 32bits hole in there!
>
This stopped my hangs but I still have failures in render copy. If I let android boot up to the home screen and stop everything rendercopy works. I haven't managed to debug the state of the pipeline yet.
Thanks,
Raf
> Found-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Rafael Barbalho <rafael.barbalho@intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
> lib/gen8_render.h | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/lib/gen8_render.h b/lib/gen8_render.h index ca53d64..fffc100
> 100644
> --- a/lib/gen8_render.h
> +++ b/lib/gen8_render.h
> @@ -273,25 +273,25 @@ struct gen8_blend_state {
> } bs0;
>
> struct {
> - uint64_t write_disable_blue:1;
> - uint64_t write_disable_green:1;
> - uint64_t write_disable_red:1;
> - uint64_t write_disable_alpha:1;
> - uint64_t pad1:1;
> - uint64_t alpha_blend_func:3;
> - uint64_t dest_alpha_blend_factor:5;
> - uint64_t source_alpha_blend_factor:5;
> - uint64_t color_blend_func:3;
> - uint64_t dest_blend_factor:5;
> - uint64_t source_blend_factor:5;
> - uint64_t color_buffer_blend:1;
> - uint64_t post_blend_color_clamp:1;
> - uint64_t pre_blend_color_clamp:1;
> - uint64_t color_clamp_range:2;
> - uint64_t pre_blend_source_only_clamp:1;
> - uint64_t pad0:22;
> - uint64_t logic_op_func:4;
> - uint64_t logic_op_enable:1;
> + uint32_t write_disable_blue:1;
> + uint32_t write_disable_green:1;
> + uint32_t write_disable_red:1;
> + uint32_t write_disable_alpha:1;
> + uint32_t pad1:1;
> + uint32_t alpha_blend_func:3;
> + uint32_t dest_alpha_blend_factor:5;
> + uint32_t source_alpha_blend_factor:5;
> + uint32_t color_blend_func:3;
> + uint32_t dest_blend_factor:5;
> + uint32_t source_blend_factor:5;
> + uint32_t color_buffer_blend:1;
> + uint32_t post_blend_color_clamp:1;
> + uint32_t pre_blend_color_clamp:1;
> + uint32_t color_clamp_range:2;
> + uint32_t pre_blend_source_only_clamp:1;
> + uint32_t pad0:22;
> + uint32_t logic_op_func:4;
> + uint32_t logic_op_enable:1;
> } bs[16];
> };
>
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rendercopy/gen8: Remove a hole in struct gen8_blend_state
2014-03-26 18:45 ` Barbalho, Rafael
@ 2014-03-26 19:07 ` Damien Lespiau
0 siblings, 0 replies; 4+ messages in thread
From: Damien Lespiau @ 2014-03-26 19:07 UTC (permalink / raw
To: Barbalho, Rafael; +Cc: intel-gfx@lists.freedesktop.org, Widawsky, Benjamin
On Wed, Mar 26, 2014 at 06:45:11PM +0000, Barbalho, Rafael wrote:
> > -----Original Message-----
> > From: Lespiau, Damien
> > Sent: Monday, March 24, 2014 11:54 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Widawsky, Benjamin; Barbalho, Rafael
> > Subject: [PATCH] rendercopy/gen8: Remove a hole in struct
> > gen8_blend_state
> >
> > Using uint64_t in that second member makes it aligned to 64bits, while the
> > first member is only 32bits. We then had a 32bits hole in there!
> >
>
> This stopped my hangs but I still have failures in render copy. If I
> let android boot up to the home screen and stop everything rendercopy
> works. I haven't managed to debug the state of the pipeline yet.
Thanks for testing it Rafael, pushed it then.
--
Damien
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-26 19:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-24 18:53 [PATCH] rendercopy/gen8: Remove a hole in struct gen8_blend_state Damien Lespiau
2014-03-25 2:14 ` Ben Widawsky
2014-03-26 18:45 ` Barbalho, Rafael
2014-03-26 19:07 ` Damien Lespiau
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.