<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Mar 27, 2018 at 4:31 AM, Pohjolainen, Topi <span dir="ltr"><<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>></span> wrote:<br><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 Thu, Mar 08, 2018 at 08:48:58AM -0800, Rafael Antognolli wrote:<br>
> gen10 can emit the clear color by setting it on a buffer somewhere, and<br>
> then adding only the address to the surface state.<br>
><br>
> This commit add support for that on isl_surf_fill_state, and if that is<br>
> requested, skip setting the clear value itself.<br>
><br>
> v2: Add assert to make sure we are at least on gen10.<br>
><br>
> Signed-off-by: Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>><br>
> ---<br>
>  src/intel/isl/isl.h               |  9 +++++++++<br>
>  src/intel/isl/isl_surface_<wbr>state.c | 18 ++++++++++++++----<br>
>  2 files changed, 23 insertions(+), 4 deletions(-)<br>
><br>
> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h<br>
> index 2edf0522e32..c50b78d4701 100644<br>
> --- a/src/intel/isl/isl.h<br>
> +++ b/src/intel/isl/isl.h<br>
> @@ -1307,6 +1307,15 @@ struct isl_surf_fill_state_info {<br>
>      */<br>
>     union isl_color_value clear_color;<br>
><br>
> +   /**<br>
> +    * Send only the clear value address<br>
> +    *<br>
> +    * If set, we only pass the clear address to the GPU and it will fetch it<br>
> +    * from wherever it is.<br>
> +    */<br>
> +   bool use_clear_address;<br>
> +   uint64_t clear_address;<br>
> +<br>
>     /**<br>
>      * Surface write disables for gen4-5<br>
>      */<br>
> diff --git a/src/intel/isl/isl_surface_<wbr>state.c b/src/intel/isl/isl_surface_<wbr>state.c<br>
> index 32a5429f2bf..bff9693f02d 100644<br>
> --- a/src/intel/isl/isl_surface_<wbr>state.c<br>
> +++ b/src/intel/isl/isl_surface_<wbr>state.c<br>
> @@ -637,11 +637,21 @@ isl_genX(surf_fill_state_s)(<wbr>const struct isl_device *dev, void *state,<br>
>  #endif<br>
><br>
>     if (info->aux_usage != ISL_AUX_USAGE_NONE) {<br>
> +      if (info->use_clear_address) {<br>
> +#if GEN_GEN >= 10<br>
> +         s.ClearValueAddressEnable = true;<br>
<br>
</div></div>In order to make sampler working, I had to add:<br>
<br>
#if GEN_GEN >= 11<br>
      /* From the Bspec:<br>
       *<br>
       *  Enables Pixel backend hw to convert clear values into native format<br>
       *  and write back to clear address, so that display and sampler can use<br>
       *  the converted value for resolving fast cleared RTs.<br>
       */<br>
      s.ClearColorConversionEnable = s.ClearValueAddressEnable;<br></blockquote><div><br></div><div>Yeah... That's a fun bit...  I think we want to be careful with that bit since it causes a write to the CLEAR_COLOR state as part of the draw.  It *may* be safe to always set it but I'm not convinced.  We should probably add another bit to isl_surf_init_info and only set it from BLORP when doing a fast-clear.  Another option would be to make BLORP just do the conversion in software and write the clear value out manually.  In either case, yes, we need to do something for ICL.<br><br></div><div>Given that ICL fast-clears aren't working yet, I'd be a fan (if no one's opposed) to plumbing that through as a follow-on.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
#endif<br>
<br>
<br>
That along with another tweak in one of the later patches fixes at least these<br>
two on ICL:<br>
<br>
fbo-clearmipmap -auto -fbo<br>
fcc-read-after-clear sample tex -fbo -auto<br>
<div class="HOEnZb"><div class="h5"><br>
> +         s.ClearValueAddress = info->clear_address;<br>
> +#else<br>
> +         unreachable("Gen9 and earlier do not support indirect clear colors");<br>
> +#endif<br>
> +      }<br>
>  #if GEN_GEN >= 9<br>
> -      s.RedClearColor = info->clear_color.u32[0];<br>
> -      s.GreenClearColor = info->clear_color.u32[1];<br>
> -      s.BlueClearColor = info->clear_color.u32[2];<br>
> -      s.AlphaClearColor = info->clear_color.u32[3];<br>
> +      if (!info->use_clear_address) {<br>
> +         s.RedClearColor = info->clear_color.u32[0];<br>
> +         s.GreenClearColor = info->clear_color.u32[1];<br>
> +         s.BlueClearColor = info->clear_color.u32[2];<br>
> +         s.AlphaClearColor = info->clear_color.u32[3];<br>
> +      }<br>
>  #elif GEN_GEN >= 7<br>
>        /* Prior to Sky Lake, we only have one bit for the clear color which<br>
>         * gives us 0 or 1 in whatever the surface's format happens to be.<br>
> --<br>
> 2.14.3<br>
><br>
> ______________________________<wbr>_________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>