<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Feb 26, 2018 at 1:14 PM, Jordan Justen <span dir="ltr"><<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.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 2018-02-21 13:45:15, 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 f1b38efed44..bab0ad3d544 100644<br>
> --- a/src/intel/isl/isl.h<br>
> +++ b/src/intel/isl/isl.h<br>
> @@ -1306,6 +1306,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></div></div></blockquote><div><br></div><div>I would recommend a slightly different comment:<br><br>/**<br></div><div>If set, the Clear Value Address field is set to point to a CLEAR_COLOR state and the inline clear color fields are left empty.<br>*/<br></div><div><br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
> +    */<br>
> +   bool use_clear_address;<br>
<br>
</div></div>I'm still wondering about this field. I think at the end we can just a<br>
assume that if gen >= 10 and aux_usage != ISL_AUX_USAGE_NONE, then<br>
we'll use the address.<br></blockquote><div><br></div><div>That's not going to work if we want to turn this on for blorp, anv, and i965 separately.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I think you mentioned that it could be tough implement the support in<br>
steps if we had an all or nothing enaling of the address usage. But,<br>
does that mean that at the end of your series you could add a patch to<br>
remove this `use_clear_address` field?<br>
<br>
Maybe as a test in jenkins, you could add a patch that asserts that if<br>
gen >= 10 and there is an aux_buffer, then use_clear_address==true in<br>
your current series.<br>
<span class="HOEnZb"><font color="#888888"><br>
-Jordan<br>
</font></span><div class="HOEnZb"><div class="h5"><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 bfb27fa4a44..e7c489df912 100644<br>
> --- a/src/intel/isl/isl_surface_<wbr>state.c<br>
> +++ b/src/intel/isl/isl_surface_<wbr>state.c<br>
> @@ -635,11 +635,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>
> +         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>