On Fri, Apr 20, 2018 at 01:35:39PM -0700, Rafael Antognolli wrote: > On Wed, Apr 11, 2018 at 01:42:24PM -0700, Nanley Chery wrote: > > This getter allows CNL to sample from fast-cleared sRGB textures correctly. > > I think it might be worth mentioning in the commit message that the > helper both returns the clear color for inline use, and updates the > pointer to the indirect clear color buffer. >
Sure, here's what I'm planning to update it to: It returns the both the inline clear color and a clear address which points to the indirect clear color buffer (or NULL if unused/non-existent). This getter allows CNL to sample from fast-cleared sRGB textures correctly by doing the needed sRGB-decode on the clear color (inline) and making the indirect clear color buffer unused. > > --- > > src/mesa/drivers/dri/i965/brw_blorp.c | 13 ++++------- > > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 7 +++--- > > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 29 > > ++++++++++++++++++++++++ > > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 +++++++ > > 4 files changed, 46 insertions(+), 11 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c > > b/src/mesa/drivers/dri/i965/brw_blorp.c > > index a1882abb7cb..48022bb1c4f 100644 > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > > @@ -166,7 +166,11 @@ blorp_surf_for_miptree(struct brw_context *brw, > > /* We only really need a clear color if we also have an auxiliary > > * surface. Without one, it does nothing. > > */ > > - surf->clear_color = mt->fast_clear_color; > > + surf->clear_color = > > + intel_miptree_get_clear_color(devinfo, mt, mt->surf.format, > > + !is_render_target, (struct brw_bo > > **) > > + &surf->clear_color_addr.buffer, > > + &surf->clear_color_addr.offset); > > > > struct intel_miptree_aux_buffer *aux_buf = > > intel_miptree_get_aux_buffer(mt); > > @@ -178,13 +182,6 @@ blorp_surf_for_miptree(struct brw_context *brw, > > > > surf->aux_addr.buffer = aux_buf->bo; > > surf->aux_addr.offset = aux_buf->offset; > > - > > - if (devinfo->gen >= 10) { > > - surf->clear_color_addr = (struct blorp_address) { > > - .buffer = aux_buf->clear_color_bo, > > - .offset = aux_buf->clear_color_offset, > > - }; > > - } > > } else { > > surf->aux_addr = (struct blorp_address) { > > .buffer = NULL, > > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > index 3c70dbcc110..fb8e5942a11 100644 > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > @@ -167,9 +167,10 @@ brw_emit_surface_state(struct brw_context *brw, > > /* We only really need a clear color if we also have an auxiliary > > * surface. Without one, it does nothing. > > */ > > - clear_bo = aux_buf->clear_color_bo; > > - clear_offset = aux_buf->clear_color_offset; > > - clear_color = mt->fast_clear_color; > > + clear_color = > > + intel_miptree_get_clear_color(devinfo, mt, view.format, > > + view.usage & > > ISL_SURF_USAGE_TEXTURE_BIT, > > + &clear_bo, &clear_offset); > > } > > > > void *state = brw_state_batch(brw, > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > index c5791835409..88468399e1b 100644 > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > @@ -46,6 +46,9 @@ > > #include "main/texcompress_etc.h" > > #include "main/teximage.h" > > #include "main/streaming-load-memcpy.h" > > + > > +#include "util/format_srgb.h" > > + > > #include "x86/common_x86_asm.h" > > > > #define FILE_DEBUG_FLAG DEBUG_MIPTREE > > @@ -3802,3 +3805,29 @@ intel_miptree_set_depth_clear_value(struct > > brw_context *brw, > > } > > return false; > > } > > + > > +union isl_color_value > > +intel_miptree_get_clear_color(const struct gen_device_info *devinfo, > > + const struct intel_mipmap_tree *mt, > > + enum isl_format view_format, bool sampling, > > + struct brw_bo **clear_color_bo, > > + uint32_t *clear_color_offset) > > +{ > > + assert(mt->aux_buf); > > + > > + /* The gen10 sampler doesn't gamma-correct the clear color. */ > > + if (devinfo->gen == 10 && isl_format_is_srgb(view_format) && sampling) { > > + union isl_color_value srgb_decoded_value = mt->fast_clear_color; > > + for (unsigned i = 0; i < 3; i++) { > > Maybe this is a dumb question, but don't we have a alpha component on > srgb? Or it doesn't require conversion? > Not a dumb question at all. I had to look into this when learning about sRGB myself. As far as I understand it, the alpha channel is generally kept linear to have more accurate blending. So here we just keep the original value when initializing srgb_decoded_value. -Nanley > > + srgb_decoded_value.f32[i] = > > + util_format_srgb_to_linear_float(mt->fast_clear_color.f32[i]); > > + } > > + *clear_color_bo = 0; > > + *clear_color_offset = 0; > > + return srgb_decoded_value; > > + } else { > > + *clear_color_bo = mt->aux_buf->clear_color_bo; > > + *clear_color_offset = mt->aux_buf->clear_color_offset; > > + return mt->fast_clear_color; > > + } > > +} > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > > index 643de962d31..bb059cf4e8f 100644 > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > > @@ -735,6 +735,14 @@ intel_miptree_set_clear_color(struct brw_context *brw, > > struct intel_mipmap_tree *mt, > > const union gl_color_union *color); > > > > +/* Get a clear color suitable for filling out an ISL surface state. */ > > +union isl_color_value > > +intel_miptree_get_clear_color(const struct gen_device_info *devinfo, > > + const struct intel_mipmap_tree *mt, > > + enum isl_format view_format, bool sampling, > > + struct brw_bo **clear_color_bo, > > + uint32_t *clear_color_offset); > > + > > bool > > intel_miptree_set_depth_clear_value(struct brw_context *brw, > > struct intel_mipmap_tree *mt, > > -- > > 2.16.2 > > > > _______________________________________________ > > 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