On Mon, Feb 26, 2018 at 05:04:37PM -0800, Jason Ekstrand wrote:
> On Wed, Feb 21, 2018 at 1:45 PM, Rafael Antognolli
> <rafael.antogno...@intel.com
> > wrote:
>
> The size of the clear color struct (expected by the hardware) is 8
> dwords (isl_dev.ss.clear_value_state_size here). But we still need to
> track the size of the clear color, used when memcopying it to/from the
> state buffer. For that we keep isl_dev.ss.clear_value_size.
>
> Signed-off-by: Rafael Antognolli <rafael.antogno...@intel.com>
> ---
> src/intel/genxml/gen10.xml | 8 ++++++++
> src/intel/isl/isl.c | 4 +++-
> src/intel/isl/isl.h | 5 +++++
> src/intel/vulkan/anv_image.c | 2 +-
> src/intel/vulkan/anv_private.h | 2 +-
> 5 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/src/intel/genxml/gen10.xml b/src/intel/genxml/gen10.xml
> index b434d1b0f66..58b83954c4c 100644
> --- a/src/intel/genxml/gen10.xml
> +++ b/src/intel/genxml/gen10.xml
>
>
> We need gen11 as well
>
>
> @@ -809,6 +809,14 @@
> <field name="Alpha Clear Color" start="480" end="511" type="int"/>
> </struct>
>
> + <struct name="CLEAR_COLOR" length="8">
> + <field name="Raw Clear Color Red" start="0" end="31" type="int"/>
> + <field name="Raw Clear Color Green" start="32" end="63" type="int"/>
> + <field name="Raw Clear Color Blue" start="64" end="95" type="int"/>
> + <field name="Raw Clear Color Alpha" start="96" end="127" type="int"/>
>
>
> Might be good to put Converted Clear Value Hi/Low in here as well.
>
>
> + <!-- Reserved - MBZ -->
> + </struct>
> +
> <struct name="SAMPLER_INDIRECT_STATE_BORDER_COLOR" length="4">
> <field name="Border Color Red As S31" start="0" end="31" type="int"/>
> <field name="Border Color Red As U32" start="0" end="31"
> type="uint"/>
> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> index 77641a89f86..e94470362e2 100644
> --- a/src/intel/isl/isl.c
> +++ b/src/intel/isl/isl.c
> @@ -79,9 +79,10 @@ isl_device_init(struct isl_device *dev,
> * - 2 dwords that can be used by the hardware for converted clear
> color
> * + some extra bits.
> */
> - dev->ss.clear_value_size = 8 * 4;
> + dev->ss.clear_value_size = 4 * 4;
> dev->ss.clear_value_offset =
> RENDER_SURFACE_STATE_ClearValueAddress_start(info) / 32 * 4;
> + dev->ss.clear_value_state_size = CLEAR_COLOR_length(info) * 4;
> } else {
> dev->ss.clear_value_size =
> isl_align(RENDER_SURFACE_STATE_RedClearColor_bits(info) +
> @@ -90,6 +91,7 @@ isl_device_init(struct isl_device *dev,
> RENDER_SURFACE_STATE_AlphaClearColor_bits(info), 32) /
> 8;
> dev->ss.clear_value_offset =
> RENDER_SURFACE_STATE_RedClearColor_start(info) / 32 * 4;
> + dev->ss.clear_value_state_size = dev->ss.clear_value_size;
>
>
> Ugh... Let's just make these two separate things. clear_value_size will be 4
> *
> 4 on gen9-10 and clear_color_state_size will be 8*4 on gen10+
I'm not sure I understand/agree with you here. clear_value_size should
be 4 * 4 everywhere, since we use this to memcpy the 4 dwords of the
clear color. So, are you suggesting we remove clear_value_state_size
from gen9?
> I think this means dropping the previous patch entirely and just making this
> patch add a size field.
Ack.
> }
> assert(RENDER_SURFACE_STATE_SurfaceBaseAddress_start(info) % 8 == 0);
> dev->ss.addr_offset =
> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> index 209769a9a99..f1b38efed44 100644
> --- a/src/intel/isl/isl.h
> +++ b/src/intel/isl/isl.h
> @@ -963,6 +963,11 @@ struct isl_device {
> uint8_t aux_addr_offset;
>
> /* Rounded up to the nearest dword to simplify GPU memcpy
> operations. */
> +
> + /* size of the state buffer used to store the clear value + extra
> + * additional space used by the hardware */
> + uint8_t clear_value_state_size;
>
>
> Maybe call this clear_color_state_size since it is the size of the CLEAR_COLOR
> state.
Ack.
>
> + /* size of the clear color itself - used to copy it to/from a BO */
> uint8_t clear_value_size;
> uint8_t clear_value_offset;
> } ss;
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index a0aee43bd21..0dafe03442d 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -264,7 +264,7 @@ add_aux_state_tracking_buffer(struct anv_image *image,
> }
>
> /* Clear color and fast clear type */
> - unsigned state_size = device->isl_dev.ss.clear_value_size + 4;
> + unsigned state_size = device->isl_dev.ss.clear_value_state_size + 4;
>
> /* We only need to track compression on CCS_E surfaces. */
> if (image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E) {
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private
> .h
> index 37e63f56aa0..b8c381d2665 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -2566,7 +2566,7 @@ anv_image_get_fast_clear_type_addr(const struct
> anv_device *device,
> {
> struct anv_address addr =
> anv_image_get_clear_color_addr(device, image, aspect);
> - addr.offset += device->isl_dev.ss.clear_value_size;
> + addr.offset += device->isl_dev.ss.clear_value_state_size;
> return addr;
> }
>
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
