<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 29, 2018 at 10:58 AM, Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com" target="_blank">rafael.antognolli@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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. v4: - Add struct to gen11 too (Jason, Jordan) - Add field for Converted Clear Color to gen11 (Jason) - Add clear_color_state_offset to differentiate from clear_value_offset. - Fix all the places where clear_value_size was used. v5 (Jason): - Split genxml changes to another commit. - Remove unnecessary gen checks. - Bring back missing offset increment to init_fast_clear_color(). [<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>: isl_device_init changes] Signed-off-by: Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>> Signed-off-by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> --- src/intel/blorp/blorp_genX_exec.h | 5 +++-- src/intel/isl/isl.c | 4 ++++ src/intel/isl/isl.h | 6 ++++++ src/intel/vulkan/anv_image.c | 6 +++++- src/intel/vulkan/anv_private.h | 6 +++++- src/intel/vulkan/genX_cmd_buffer.c | 23 ++++++++++++----------- 6 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/intel/blorp/blorp_genX_exec.h b/src/intel/blorp/blorp_genX_exec.h index 992bc9959a1..f3a96fbd58c 100644 --- a/src/intel/blorp/blorp_genX_exec.h +++ b/src/intel/blorp/blorp_genX_exec.h @@ -310,10 +310,11 @@ blorp_emit_vertex_buffers(struct blorp_batch *batch, uint32_t num_vbs = 2; if (params->dst_clear_color_as_input) { + const unsigned clear_color_size = + GEN_GEN < 10 ? batch->blorp->isl_dev->ss.clear_value_size : 4 * 4; blorp_fill_vertex_buffer_state(batch, vb, num_vbs++, params->dst.clear_color_addr, - batch->blorp->isl_dev->ss.clear_value_size, - 0); + clear_color_size, 0); } const unsigned num_dwords = 1 + num_vbs * GENX(VERTEX_BUFFER_STATE_length); diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c index 1a32c028183..875c691b43e 100644 --- a/src/intel/isl/isl.c +++ b/src/intel/isl/isl.c @@ -73,6 +73,10 @@ 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_offset = + RENDER_SURFACE_STATE_ClearValueAddress_start(info) / 32 * 4; + dev->ss.clear_value_size = isl_align(RENDER_SURFACE_STATE_RedClearColor_bits(info) + RENDER_SURFACE_STATE_GreenClearColor_bits(info) + diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h index 0da6abb71d4..2edf0522e32 100644 --- a/src/intel/isl/isl.h +++ b/src/intel/isl/isl.h @@ -963,6 +963,12 @@ 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 color + extra + * additional space used by the hardware */ + uint8_t clear_color_state_size; + uint8_t clear_color_state_offset; + /* 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 b20d791751d..d9b5d266020 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -267,8 +267,12 @@ add_aux_state_tracking_buffer(struct anv_image *image, (image->planes[plane].offset + image->planes[plane].size)); } + const unsigned clear_color_state_size = device->info.gen >= 10 ? + device->isl_dev.ss.clear_color_state_size : + device->isl_dev.ss.clear_value_size; + /* Clear color and fast clear type */ - unsigned state_size = device->isl_dev.ss.clear_value_size + 4; + unsigned state_size = clear_color_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 3c803178c41..08e4362b028 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -2606,7 +2606,11 @@ 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; + + const unsigned clear_color_state_size = device->info.gen >= 10 ? + device->isl_dev.ss.clear_color_state_size : + device->isl_dev.ss.clear_value_size; + addr.offset += clear_color_state_size; return addr; } diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c index b5741fb8dc1..9411854b7e5 100644 --- a/src/intel/vulkan/genX_cmd_buffer.c +++ b/src/intel/vulkan/genX_cmd_buffer.c @@ -826,35 +826,36 @@ init_fast_clear_color(struct anv_cmd_buffer *cmd_buffer, */ struct anv_address addr = anv_image_get_clear_color_addr(cmd_buffer->device, image, aspect); - unsigned i = 0; - for (; i < cmd_buffer->device->isl_dev.ss.clear_value_size; i += 4) { - anv_batch_emit(&cmd_buffer->batch, GENX(MI_STORE_DATA_IMM), sdi) { - sdi.Address = addr; - if (GEN_GEN >= 9) { + if (GEN_GEN >= 9) { + for (unsigned i = 0; i < 4; i++) { + anv_batch_emit(&cmd_buffer->batch, GENX(MI_STORE_DATA_IMM), sdi) { + sdi.Address = addr; </blockquote><div> </div><div>How about removing "addr.offset += 4" from below and just add </div><div>sdi.Address.offset += i * 4; </div><div>here? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> /* MCS buffers on SKL+ can only have 1/0 clear colors. */ assert(image->samples > 1); sdi.ImmediateData = 0; - } else if (GEN_VERSIONx10 >= 75) { + addr.offset += 4; + } + } + } else { + anv_batch_emit(&cmd_buffer->batch, GENX(MI_STORE_DATA_IMM), sdi) { + sdi.Address = addr; + if (GEN_VERSIONx10 >= 75) { </blockquote><div> </div><div>GEN_GEN >= 8 || GEN_IS_HASWELL </div><div>We try to avoid using the GEN_VERSIONx10 macros directly as we may want to do something else in the future besides multiplying by 10. This also better matches the kind of gen check you would see if you were using devinfo. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> /* Pre-SKL, the dword containing the clear values also contains * other fields, so we need to initialize those fields to match the * values that would be in a color attachment. */ - assert(i == 0); sdi.ImmediateData = ISL_CHANNEL_SELECT_RED << 25 | ISL_CHANNEL_SELECT_GREEN << 22 | ISL_CHANNEL_SELECT_BLUE << 19 | ISL_CHANNEL_SELECT_ALPHA << 16; - } else if (GEN_VERSIONx10 == 70) { + } else if (GEN_VERSIONx10 == 70) { </blockquote><div> </div><div>} else { </div><div>With those three changes made, 4 and 5 are </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> /* On IVB, the dword containing the clear values also contains * other fields that must be zero or can be zero. */ - assert(i == 0); sdi.ImmediateData = 0; } } - - addr.offset += 4; } } -- 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> </blockquote></div> </div></div>