On Wed, Apr 17, 2019 at 3:31 PM Nanley Chery <nanleych...@gmail.com> wrote:
> On Wed, Apr 17, 2019 at 11:34:15AM -0700, Rafael Antognolli wrote: > > On Wed, Apr 17, 2019 at 09:04:09AM -0700, Kenneth Graunke wrote: > > > On Wednesday, April 17, 2019 7:16:28 AM PDT Topi Pohjolainen wrote: > > > > From: Rafael Antognolli <rafael.antogno...@intel.com> > > > > > > > > Fixes MCS fast clear gpu hangs with Vulkan CTS on ICL in CI. > > > > > > > > CC: Anuj Phogat <anuj.pho...@gmail.com> > > > > CC: Kenneth Graunke <kenn...@whitecape.org> > > > > Tested-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > > > > Signed-off-by: Rafael Antognolli <rafael.antogno...@intel.com> > > > > --- > > > > src/intel/isl/isl.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c > > > > index 6b9e6c9e0f0..acfed5119ba 100644 > > > > --- a/src/intel/isl/isl.c > > > > +++ b/src/intel/isl/isl.c > > > > @@ -122,7 +122,8 @@ isl_device_init(struct isl_device *dev, > > > > dev->ss.size = RENDER_SURFACE_STATE_length(info) * 4; > > > > dev->ss.align = isl_align(dev->ss.size, 32); > > > > > > > > - dev->ss.clear_color_state_size = CLEAR_COLOR_length(info) * 4; > > > > + dev->ss.clear_color_state_size = > > > > + isl_align(CLEAR_COLOR_length(info) * 4, 64); > > > > dev->ss.clear_color_state_offset = > > > > RENDER_SURFACE_STATE_ClearValueAddress_start(info) / 32 * 4; > > > > > > > > > > > > > > I'm not as familiar with Vulkan, but it looks like we're storing this > > > clear color data as part of the underlying image's BO, rather than as > > > a separate piece of data. I wonder if it has anything to do with that > > > BO being considered tiled, so something is trying to access an entire > > > cacheline around here. Or it's offsetting following data to not be > > > cacheline aligned... > > > > Hmmm... Yeah, we store it after the aux buffer, in the same BO as the > > image one. > > > > What I think it's the biggest issue in Vulkan is that we store some > > data (resolve type and tracking) right after the clear color data. And > > the data size is 32B, but the docs say it should be the lower 32B of a > > cacheline. For some reason I thought it was safe to write stuff into the > > higher 32B, but apparently it wasn't :-/ > > > > > I did notice that the clear address has to be 64B aligned. > > > > My understanding is that the image and aux surface were always 4K > > aligned, so this restriction would be met. I guess making an assert for > > it wouldn't hurt, though... > > This fix looks good to me. I'm a little confused about the title. I see > how this change pads the buffer, but I don't see how it aligns it. > I agree that this doesn't really "align" anything. We get the alignment because the size of each surface is a multiple of 4K. Here's an interesting bspec quote: The memory layout of the clear color pointed to by this address is a value stored in the lower-order bytes of a 64-byte cache-line. The fact that it refers to a cache line is important. It's possible (but I really don't know) something weird is happening with MI writes and caching. :-/ --Jason
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev