On Thu 14 Jul 2016, Chad Versace wrote:
> On Wed 13 Jul 2016, Jason Ekstrand wrote:
> > Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> > ---
> > src/mesa/drivers/dri/i965/brw_state.h | 8 +++
> > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 79
> > ++++++++++++++++++++++++
> > 2 files changed, 87 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> > b/src/mesa/drivers/dri/i965/brw_state.h
> > index a16e876..91ebce9 100644
> > --- a/src/mesa/drivers/dri/i965/brw_state.h
> > +++ b/src/mesa/drivers/dri/i965/brw_state.h
> > @@ -274,6 +274,14 @@ GLuint translate_tex_format(struct brw_context *brw,
> > int brw_get_texture_swizzle(const struct gl_context *ctx,
> > const struct gl_texture_object *t);
> >
>
> Move the forward declaration below to the top of the header. As written,
> it looks like the function's return type.
> > +struct isl_view;
> > +void brw_emit_surface_state(struct brw_context *brw,
> > + struct intel_mipmap_tree *mt,
> > + const struct isl_view *view,
> > + uint32_t mocs, bool for_gather,
> > + uint32_t *surf_offset, int surf_index,
> > + unsigned read_domains, unsigned write_domains);
Ping! Unresolved issue^^^
Fix that, and this patch is
Reviewed-by: Chad Versace <chad.vers...@intel.com>
But keep reading...
> > + if (aux_surf) {
> > + /* On gen7 and prior, the bottom 12 bits of the MCS base address are
> > + * used to store other information. This should be ok, however,
> > because
> > + * surface buffer addresses are always 4K page alinged.
> > + */
>
> Could you please insert the original comment from gen7_wm_surface_state?
> Because...
>
> > + assert((aux_offset & 0xfff) == 0);
> > + drm_intel_bo_emit_reloc(brw->batch.bo,
> > + *surf_offset + 4 * ss_info.aux_reloc_dw,
> > + mt->mcs_mt->bo, dw[ss_info.aux_reloc_dw] &
> > 0xfff,
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> ...that ^^^ bit of code REALLY confused me until I found the original
> comment. What's happening is obvious in retrospect, yet terribly obscure
> without a fuller comment.
I prefer you insert the original comment, or at least some more
explanation. My r-b stands with or without a comment update.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
