<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Apr 3, 2018 at 9:40 PM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Apr 03, 2018 at 09:38:52PM -0700, Jason Ekstrand wrote: > On Tue, Apr 3, 2018 at 8:23 PM, Pohjolainen, Topi < > <a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> wrote: > > > On Tue, Apr 03, 2018 at 02:55:31PM -0700, Jason Ekstrand wrote: > > > On Tue, Apr 3, 2018 at 8:05 AM, Pohjolainen, Topi < > > > <a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> wrote: > > > > > > > On Thu, Mar 29, 2018 at 10:58:40AM -0700, Rafael Antognolli wrote: > > > > > gen10 can emit the clear color by setting it on a buffer somewhere, > > and > > > > > then adding only the address to the surface state. > > > > > > > > > > This commit add support for that on isl_surf_fill_state, and if that > > is > > > > > requested, skip setting the clear value itself. > > > > > > > > > > v2: Add assert to make sure we are at least on gen10. > > > > > > > > > > Signed-off-by: Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>> > > > > > Reviewed-by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> > > > > > --- > > > > > src/intel/isl/isl.h | 9 +++++++++ > > > > > src/intel/isl/isl_surface_state.c | 18 ++++++++++++++---- > > > > > 2 files changed, 23 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h > > > > > index 2edf0522e32..c50b78d4701 100644 > > > > > --- a/src/intel/isl/isl.h > > > > > +++ b/src/intel/isl/isl.h > > > > > @@ -1307,6 +1307,15 @@ struct isl_surf_fill_state_info { > > > > > */ > > > > > union isl_color_value clear_color; > > > > > > > > > > + /** > > > > > + * Send only the clear value address > > > > > + * > > > > > + * If set, we only pass the clear address to the GPU and it will > > > > fetch it > > > > > + * from wherever it is. > > > > > + */ > > > > > + bool use_clear_address; > > > > > + uint64_t clear_address; > > > > > + > > > > > /** > > > > > * Surface write disables for gen4-5 > > > > > */ > > > > > diff --git a/src/intel/isl/isl_surface_state.c > > > > b/src/intel/isl/isl_surface_state.c > > > > > index 32a5429f2bf..bff9693f02d 100644 > > > > > --- a/src/intel/isl/isl_surface_state.c > > > > > +++ b/src/intel/isl/isl_surface_state.c > > > > > @@ -637,11 +637,21 @@ isl_genX(surf_fill_state_s)(const struct > > > > isl_device *dev, void *state, > > > > > #endif > > > > > > > > > > if (info->aux_usage != ISL_AUX_USAGE_NONE) { > > > > > + if (info->use_clear_address) { > > > > > +#if GEN_GEN >= 10 > > > > > + s.ClearValueAddressEnable = true; > > > > > > > > This will set it for multisampled also and upset piglit tests. We need > > > > something of this sort: > > > > > > > > s.ClearValueAddressEnable = info->aux_usage != > > > > ISL_AUX_USAGE_MCS; > > > > > > > > > > Can we assert instead? If the caller asks for a clear address to be set > > > they should get it and not have it magically disabled when they ask for > > MCS. > > > > Right, here an assert would be just fine. I should have made the comment > > in patch in 19 where it belongs. There we start setting the flag > > unconditionally for "info->aux_usage != ISL_AUX_USAGE_NONE". If we don't, > > then on ICL we start failing multisampling tests. > > > > Is this a known hardware restriction or is there just something weird going > on with MCS? </div></div>Bspec says the clear address is only for CCS_D and CCS_E. And setting it for MCS seems to upset both the simulator and hardware. At least with </blockquote><div> </div><div>Ok, that's unfortunate. :-( I guess we need to not use it for MCS which means an assert. Unfortunately, it also means that Rafiel's plan of just always turning it on for gen10+ is also not going to work. :-( This is getting unfortunately complicated. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> ext_framebuffer_multisample-accuracy 2 color <div class="HOEnZb"><div class="h5"> > > > > > > > > > > > > > + s.ClearValueAddress = info->clear_address; > > > > > +#else > > > > > + unreachable("Gen9 and earlier do not support indirect clear > > > > colors"); > > > > > +#endif > > > > > + } > > > > > #if GEN_GEN >= 9 > > > > > - s.RedClearColor = info->clear_color.u32[0]; > > > > > - s.GreenClearColor = info->clear_color.u32[1]; > > > > > - s.BlueClearColor = info->clear_color.u32[2]; > > > > > - s.AlphaClearColor = info->clear_color.u32[3]; > > > > > + if (!info->use_clear_address) { > > > > > + s.RedClearColor = info->clear_color.u32[0]; > > > > > + s.GreenClearColor = info->clear_color.u32[1]; > > > > > + s.BlueClearColor = info->clear_color.u32[2]; > > > > > + s.AlphaClearColor = info->clear_color.u32[3]; > > > > > + } > > > > > #elif GEN_GEN >= 7 > > > > > /* Prior to Sky Lake, we only have one bit for the clear color > > > > which > > > > > * gives us 0 or 1 in whatever the surface's format happens > > to be. > > > > > -- > > > > > 2.14.3 > > > > > > > > > > _______________________________________________ > > > > > mesa-dev mailing list > > > > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > > > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > > > _______________________________________________ > > > > mesa-dev mailing list > > > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > > > > > </div></div></blockquote></div> </div></div>