<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 8, 2018 at 8:48 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. [<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/genxml/gen10.xml | 8 ++++++++ src/intel/genxml/gen11.xml | 10 ++++++++++ src/intel/isl/isl.c | 23 +++++++++++++++-------- 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 | 22 +++++++++++----------- 8 files changed, 63 insertions(+), 23 deletions(-) diff --git a/src/intel/blorp/blorp_genX_exec.h b/src/intel/blorp/blorp_genX_exec.h index 1348659233c..7f5a4539e8d 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/genxml/gen10.xml b/src/intel/genxml/gen10.xml index 6302497a5e9..fdb12bace03 100644 --- a/src/intel/genxml/gen10.xml +++ b/src/intel/genxml/gen10.xml @@ -584,6 +584,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"/> +  + </struct> </blockquote><div> </div><div>genxml changes should probably go in their own commit. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + <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/genxml/gen11.xml b/src/intel/genxml/gen11.xml index 023b56f83f1..28b06b8b2e2 100644 --- a/src/intel/genxml/gen11.xml +++ b/src/intel/genxml/gen11.xml @@ -586,6 +586,16 @@ <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"/> +  + <field name="Converted Clear Value Hi/Low" start="128" end="191" type="uint"/> +  + </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 1a32c028183..3566bd3f0dd 100644 --- a/src/intel/isl/isl.c +++ b/src/intel/isl/isl.c @@ -73,14 +73,21 @@ 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_value_size = - isl_align(RENDER_SURFACE_STATE_RedClearColor_bits(info) + - RENDER_SURFACE_STATE_GreenClearColor_bits(info) + - RENDER_SURFACE_STATE_BlueClearColor_bits(info) + - RENDER_SURFACE_STATE_AlphaClearColor_bits(info), 32) / 8; - - dev->ss.clear_value_offset = - RENDER_SURFACE_STATE_RedClearColor_start(info) / 32 * 4; + if (ISL_DEV_GEN(dev) >= 10) { + 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; + } + + if (ISL_DEV_GEN(dev) <= 10) { + dev->ss.clear_value_size = + isl_align(RENDER_SURFACE_STATE_RedClearColor_bits(info) + + RENDER_SURFACE_STATE_GreenClearColor_bits(info) + + RENDER_SURFACE_STATE_BlueClearColor_bits(info) + + RENDER_SURFACE_STATE_AlphaClearColor_bits(info), 32) / 8; + dev->ss.clear_value_offset = + RENDER_SURFACE_STATE_RedClearColor_start(info) / 32 * 4; + } </blockquote><div> </div><div>You don't need the gen checks here the _bits and _start functions will return 0 on older platforms. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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 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 ee533581ab4..3af81e16025 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..53d095a6ee2 100644 --- a/src/intel/vulkan/genX_cmd_buffer.c +++ b/src/intel/vulkan/genX_cmd_buffer.c @@ -826,35 +826,35 @@ 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; /* MCS buffers on SKL+ can only have 1/0 clear colors. */ assert(image->samples > 1); sdi.ImmediateData = 0; - } else if (GEN_VERSIONx10 >= 75) { + } + } + } else { + anv_batch_emit(&cmd_buffer->batch, GENX(MI_STORE_DATA_IMM), sdi) { + sdi.Address = addr; + if (GEN_VERSIONx10 >= 75) { /* 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) { /* 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; </blockquote><div> </div><div>This needs to be part of your gen >= 9 loop somehow. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> } } -- 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>