[Mesa-dev] [PATCH v2 08/27] i965/blorp: Stop using the miptree in state setup for tex/rt surfaces
Jason Ekstrand
jason at jlekstrand.net
Fri Jul 29 16:34:11 UTC 2016
On Jul 29, 2016 9:09 AM, "Jason Ekstrand" <jason at jlekstrand.net> wrote:
>
> On Jul 29, 2016 1:47 AM, "Pohjolainen, Topi" <topi.pohjolainen at intel.com>
wrote:
> >
> > On Thu, Jul 28, 2016 at 11:41:01AM -0700, Jason Ekstrand wrote:
> > > On Jul 28, 2016 12:05 PM, "Pohjolainen, Topi"
> > > <[1]topi.pohjolainen at intel.com> wrote:
> > > >
> > > > On Tue, Jul 26, 2016 at 03:11:12PM -0700, Jason Ekstrand wrote:
> > > > > Instead, we add a bo and offset field to
brw_blorp_surface_info and
> > > use
> > > > > those in the backend.
> > > > > ---
> > > > > src/mesa/drivers/dri/i965/brw_blorp.c | 10 +++++---
> > > > > src/mesa/drivers/dri/i965/brw_blorp.h | 3 ++-
> > > > > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 4 +++-
> > > > > src/mesa/drivers/dri/i965/gen6_blorp.c | 33
> > > +++++++++++---------------
> > > > > src/mesa/drivers/dri/i965/gen7_blorp.c | 35
> > > ++++++++++++----------------
> > > > > src/mesa/drivers/dri/i965/gen8_blorp.c | 10 ++++----
> > > > > 6 files changed, 45 insertions(+), 50 deletions(-)
> > > > >
> > > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > > index 87d8929..cf1615f 100644
> > > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > > @@ -126,8 +126,12 @@ brw_blorp_surface_info_init(struct
brw_context
> > > *brw,
> > > > > intel_miptree_check_level_layer(mt, level, layer);
> > > > >
> > > > > info->mt = mt;
> > > > > + if (is_render_target)
> > > > > + intel_miptree_used_for_rendering(mt);
> > > > >
> > > > > intel_miptree_get_isl_surf(brw, mt, &info->surf);
> > > > > + info->bo = mt->bo;
> > > > > + info->offset = mt->offset;
> > > > >
> > > > > if (mt->mcs_mt) {
> > > > > intel_miptree_get_aux_isl_surf(brw, mt, &info->aux_surf,
> > > > > @@ -360,7 +364,7 @@ brw_blorp_emit_surface_state(struct
brw_context
> > > *brw,
> > > > > const uint32_t mocs = is_render_target ? ss_info.rb_mocs :
> > > ss_info.tex_mocs;
> > > > >
> > > > > isl_surf_fill_state(&brw->isl_dev, dw, .surf = &surf,
.view =
> > > &surface->view,
> > > > > - .address = surface->mt->bo->offset64 +
> > > surface->bo_offset,
> > > > > + .address = surface->bo->offset64 +
> > > surface->offset,
> > > > > .aux_surf = aux_surf, .aux_usage =
> > > surface->aux_usage,
> > > > > .aux_address = aux_offset,
> > > > > .mocs = mocs, .clear_color =
clear_color,
> > > > > @@ -370,8 +374,8 @@ brw_blorp_emit_surface_state(struct
brw_context
> > > *brw,
> > > > > /* Emit relocation to surface contents */
> > > > > drm_intel_bo_emit_reloc(brw->[2]batch.bo,
> > > > > surf_offset + ss_info.reloc_dw * 4,
> > > > > - surface->mt->bo,
> > > > > - dw[ss_info.reloc_dw] -
> > > surface->mt->bo->offset64,
> > > > > + surface->bo,
> > > > > + dw[ss_info.reloc_dw] -
> > > surface->bo->offset64,
> > > > > read_domains, write_domain);
> > > > >
> > > > > if (aux_surf) {
> > > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
> > > b/src/mesa/drivers/dri/i965/brw_blorp.h
> > > > > index 076d26d..98a9436 100644
> > > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> > > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> > > > > @@ -72,6 +72,8 @@ struct brw_blorp_surface_info
> > > > > struct intel_mipmap_tree *mt;
> > > > >
> > > > > struct isl_surf surf;
> > > > > + drm_intel_bo *bo;
> > > > > + uint32_t offset;
> > > > >
> > > > > struct isl_surf aux_surf;
> > > > > enum isl_aux_usage aux_usage;
> > > > > @@ -81,7 +83,6 @@ struct brw_blorp_surface_info
> > > > > /* Z offset into a 3-D texture or slice of a 2-D array
texture.
> > > */
> > > > > uint32_t z_offset;
> > > > >
> > > > > - uint32_t bo_offset;
> > > >
> > > > So effectively you are renaming bo_offset to offset, right? To
me the
> > > older is
> > > > somewhat more informative but I'm okay either way.
> > >
> > > Sure but aux_go_offset starts getting long. I also wanted to
manually
> > > look at every use of it (see below)
> > >
> > > > > uint32_t tile_x_sa, tile_y_sa;
> > > > > };
> > > > >
> > > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > > b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > > > > index ed68734..ee34a70 100644
> > > > > --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> > > > > @@ -1604,11 +1604,13 @@ surf_convert_to_single_slice(struct
> > > brw_context *brw,
> > > > > info->view.base_array_layer,
> > > > > &x_offset_sa, &y_offset_sa);
> > > > >
> > > > > + uint32_t byte_offset;
> > > > > isl_tiling_get_intratile_offset_sa(&brw->isl_dev,
> > > info->surf.tiling,
> > > > > info->view.format,
> > > info->surf.row_pitch,
> > > > > x_offset_sa,
y_offset_sa,
> > > > > - &info->bo_offset,
> > > > > + &byte_offset,
> > > >
> > > > Is there something else going on preventing simple:
> > > >
> > > > &info->offset,
> > >
> > > Yes. The offset used to mean offset into the surface defined by
the
> > > miptree but now means full offset into the BO. Before, you had to
add
> > > offset and mt->offset in order to get the final offset. Now there
is
> > > just one so we need += here.
> >
> > Hmm, reading again and I still can't see where you drop the old addition
> > calling for the addition here. To me it just looks that the name of the
member
> > changes. Can you help me a bit?
>
> I thought for sure we had it somewhere. In any case offsets that aren't
zero need to be supported on all surfaces for Vulkan. As I've been going
through I've been trying to make sure they do. I can put that in the
commit message.
Maybe this is a better way of putting it: We're moving from a miptree model
to a surf+bo+offset model. In the GL driver, miptrees are almost always at
the start of the bo so the offset is zero but we don't want to always make
that assumption. In the sort term, gen6 stencil and HiZ will be at an
offset but, in the long term, any Vulkan surface is liable to be at a
non-zero offset.
> > >
> > > > > &info->tile_x_sa,
> > > &info->tile_y_sa);
> > > > > + info->offset += byte_offset;
> > > > >
> > > > > /* TODO: Once this file gets converted to C, we shouls
just use
> > > designated
> > > > > * initializers.
> > > > > diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.c
> > > b/src/mesa/drivers/dri/i965/gen6_blorp.c
> > > > > index 9e08374..6f3073b 100644
> > > > > --- a/src/mesa/drivers/dri/i965/gen6_blorp.c
> > > > > +++ b/src/mesa/drivers/dri/i965/gen6_blorp.c
> > > > > @@ -634,7 +634,7 @@ gen6_blorp_emit_wm_config(struct
brw_context
> > > *brw,
> > > > > dw5 |= GEN6_WM_16_DISPATCH_ENABLE;
> > > > > }
> > > > >
> > > > > - if (params->[3]src.mt) {
> > > > > + if (params->[4]src.bo) {
> > > > > dw5 |= GEN6_WM_KILL_ENABLE; /* TODO: temporarily smash
on */
> > > > > dw2 |= 1 << GEN6_WM_SAMPLER_COUNT_SHIFT; /* Up to 4
samplers
> > > */
> > > > > }
> > > > > @@ -700,20 +700,16 @@
gen6_blorp_emit_depth_stencil_config(struct
> > > brw_context *brw,
> > > > > const struct
brw_blorp_params
> > > *params)
> > > > > {
> > > > > uint32_t surftype;
> > > > > - GLenum gl_target = params->depth.mt->target;
> > > > > -
> > > > > - switch (gl_target) {
> > > > > - case GL_TEXTURE_CUBE_MAP_ARRAY:
> > > > > - case GL_TEXTURE_CUBE_MAP:
> > > > > - /* The PRM claims that we should use BRW_SURFACE_CUBE
for
> > > this
> > > > > - * situation, but experiments show that gl_Layer doesn't
> > > work when we do
> > > > > - * this. So we use BRW_SURFACE_2D, since for rendering
> > > purposes this is
> > > > > - * equivalent.
> > > > > - */
> > > > > +
> > > > > + switch (params->depth.surf.dim) {
> > > > > + case ISL_SURF_DIM_1D:
> > > > > + surftype = BRW_SURFACE_1D;
> > > > > + break;
> > > > > + case ISL_SURF_DIM_2D:
> > > > > surftype = BRW_SURFACE_2D;
> > > > > break;
> > > > > - default:
> > > > > - surftype = translate_tex_target(gl_target);
> > > > > + case ISL_SURF_DIM_3D:
> > > > > + surftype = BRW_SURFACE_3D;
> > > > > break;
> > > > > }
> > > > >
> > > > > @@ -738,9 +734,9 @@ gen6_blorp_emit_depth_stencil_config(struct
> > > brw_context *brw,
> > > > > surftype << 29);
> > > > >
> > > > > /* 3DSTATE_DEPTH_BUFFER dw2 */
> > > > > - OUT_RELOC(params->depth.mt->bo,
> > > > > + OUT_RELOC(params->[5]depth.bo,
> > > > > I915_GEM_DOMAIN_RENDER,
I915_GEM_DOMAIN_RENDER,
> > > > > - 0);
> > > > > + params->depth.offset);
> > > > >
> > > > > /* 3DSTATE_DEPTH_BUFFER dw3 */
> > > > > OUT_BATCH(BRW_SURFACE_MIPMAPLAYOUT_BELOW << 1 |
> > > > > @@ -940,12 +936,11 @@ gen6_blorp_exec(struct brw_context *brw,
> > > > > uint32_t wm_surf_offset_renderbuffer;
> > > > > uint32_t wm_surf_offset_texture = 0;
> > > > >
> > > > > - intel_miptree_used_for_rendering(params->[6]dst.mt);
> > > > > wm_surf_offset_renderbuffer =
> > > > > brw_blorp_emit_surface_state(brw, ¶ms->dst,
> > > > > I915_GEM_DOMAIN_RENDER,
> > > > > I915_GEM_DOMAIN_RENDER,
> > > true);
> > > > > - if (params->[7]src.mt) {
> > > > > + if (params->[8]src.bo) {
> > > > > wm_surf_offset_texture =
> > > > > brw_blorp_emit_surface_state(brw, ¶ms->src,
> > > > >
I915_GEM_DOMAIN_SAMPLER,
> > > 0, false);
> > > > > @@ -956,7 +951,7 @@ gen6_blorp_exec(struct brw_context *brw,
> > > > >
wm_surf_offset_texture);
> > > > > }
> > > > >
> > > > > - if (params->[9]src.mt) {
> > > > > + if (params->[10]src.bo) {
> > > > > const uint32_t sampler_offset =
> > > > > gen6_blorp_emit_sampler_state(brw,
BRW_MAPFILTER_LINEAR,
> > > 0, true);
> > > > > gen6_blorp_emit_sampler_state_pointers(brw,
sampler_offset);
> > > > > @@ -971,7 +966,7 @@ gen6_blorp_exec(struct brw_context *brw,
> > > > > gen6_blorp_emit_binding_table_pointers(brw,
> > > wm_bind_bo_offset);
> > > > > gen6_blorp_emit_viewport_state(brw, params);
> > > > >
> > > > > - if (params->[11]depth.mt)
> > > > > + if (params->[12]depth.bo)
> > > > > gen6_blorp_emit_depth_stencil_config(brw, params);
> > > > > else
> > > > > gen6_blorp_emit_depth_disable(brw, params);
> > > > > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.c
> > > b/src/mesa/drivers/dri/i965/gen7_blorp.c
> > > > > index 420a285..0ca1a7b 100644
> > > > > --- a/src/mesa/drivers/dri/i965/gen7_blorp.c
> > > > > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.c
> > > > > @@ -374,7 +374,7 @@ gen7_blorp_emit_wm_config(struct
brw_context
> > > *brw,
> > > > > if (params->wm_prog_data)
> > > > > dw1 |= GEN7_WM_DISPATCH_ENABLE; /* We are rendering */
> > > > >
> > > > > - if (params->[13]src.mt)
> > > > > + if (params->[14]src.bo)
> > > > > dw1 |= GEN7_WM_KILL_ENABLE; /* TODO: temporarily smash
on */
> > > > >
> > > > > if (params->dst.surf.samples > 1) {
> > > > > @@ -441,7 +441,7 @@ gen7_blorp_emit_ps_config(struct
brw_context
> > > *brw,
> > > > > dw4 |= GEN7_PS_16_DISPATCH_ENABLE;
> > > > > }
> > > > >
> > > > > - if (params->[15]src.mt)
> > > > > + if (params->[16]src.bo)
> > > > > dw2 |= 1 << GEN7_PS_SAMPLER_COUNT_SHIFT; /* Up to 4
samplers
> > > */
> > > > >
> > > > > dw4 |= params->fast_clear_op;
> > > > > @@ -486,20 +486,16 @@
gen7_blorp_emit_depth_stencil_config(struct
> > > brw_context *brw,
> > > > > {
> > > > > const uint8_t mocs = GEN7_MOCS_L3;
> > > > > uint32_t surftype;
> > > > > - GLenum gl_target = params->depth.mt->target;
> > > > > -
> > > > > - switch (gl_target) {
> > > > > - case GL_TEXTURE_CUBE_MAP_ARRAY:
> > > > > - case GL_TEXTURE_CUBE_MAP:
> > > > > - /* The PRM claims that we should use BRW_SURFACE_CUBE
for
> > > this
> > > > > - * situation, but experiments show that gl_Layer doesn't
> > > work when we do
> > > > > - * this. So we use BRW_SURFACE_2D, since for rendering
> > > purposes this is
> > > > > - * equivalent.
> > > > > - */
> > > > > +
> > > > > + switch (params->depth.surf.dim) {
> > > > > + case ISL_SURF_DIM_1D:
> > > > > + surftype = BRW_SURFACE_1D;
> > > > > + break;
> > > > > + case ISL_SURF_DIM_2D:
> > > > > surftype = BRW_SURFACE_2D;
> > > > > break;
> > > > > - default:
> > > > > - surftype = translate_tex_target(gl_target);
> > > > > + case ISL_SURF_DIM_3D:
> > > > > + surftype = BRW_SURFACE_3D;
> > > > > break;
> > > > > }
> > > > >
> > > > > @@ -517,9 +513,9 @@ gen7_blorp_emit_depth_stencil_config(struct
> > > brw_context *brw,
> > > > > 1 << 22 | /* hiz enable */
> > > > > 1 << 28 | /* depth write */
> > > > > surftype << 29);
> > > > > - OUT_RELOC(params->depth.mt->bo,
> > > > > + OUT_RELOC(params->[17]depth.bo,
> > > > > I915_GEM_DOMAIN_RENDER,
I915_GEM_DOMAIN_RENDER,
> > > > > - 0);
> > > > > + params->depth.offset);
> > > > > OUT_BATCH((params->depth.surf.logical_level0_px.width -
1)
> > > << 4 |
> > > > > (params->depth.surf.logical_level0_px.height
- 1)
> > > << 18 |
> > > > > params->depth.view.base_level);
> > > > > @@ -667,13 +663,12 @@ gen7_blorp_exec(struct brw_context *brw,
> > > > > uint32_t wm_surf_offset_renderbuffer;
> > > > > uint32_t wm_surf_offset_texture = 0;
> > > > >
> > > > > - intel_miptree_used_for_rendering(params->[18]dst.mt);
> > > > > wm_surf_offset_renderbuffer =
> > > > > brw_blorp_emit_surface_state(brw, ¶ms->dst,
> > > > > I915_GEM_DOMAIN_RENDER,
> > > > > I915_GEM_DOMAIN_RENDER,
> > > > > true /* is_render_target
> > > */);
> > > > > - if (params->[19]src.mt) {
> > > > > + if (params->[20]src.bo) {
> > > > > wm_surf_offset_texture =
> > > > > brw_blorp_emit_surface_state(brw, ¶ms->src,
> > > > >
I915_GEM_DOMAIN_SAMPLER,
> > > 0,
> > > > > @@ -696,7 +691,7 @@ gen7_blorp_exec(struct brw_context *brw,
> > > > > if (params->wm_prog_data)
> > > > > gen7_blorp_emit_binding_table_pointers_ps(brw,
> > > wm_bind_bo_offset);
> > > > >
> > > > > - if (params->[21]src.mt) {
> > > > > + if (params->[22]src.bo) {
> > > > > const uint32_t sampler_offset =
> > > > > gen6_blorp_emit_sampler_state(brw,
BRW_MAPFILTER_LINEAR,
> > > 0, true);
> > > > > gen7_blorp_emit_sampler_state_pointers_ps(brw,
> > > sampler_offset);
> > > > > @@ -705,7 +700,7 @@ gen7_blorp_exec(struct brw_context *brw,
> > > > > gen7_blorp_emit_ps_config(brw, params);
> > > > > gen7_blorp_emit_cc_viewport(brw);
> > > > >
> > > > > - if (params->[23]depth.mt)
> > > > > + if (params->[24]depth.bo)
> > > > > gen7_blorp_emit_depth_stencil_config(brw, params);
> > > > > else
> > > > > gen7_blorp_emit_depth_disable(brw);
> > > > > diff --git a/src/mesa/drivers/dri/i965/gen8_blorp.c
> > > b/src/mesa/drivers/dri/i965/gen8_blorp.c
> > > > > index 1756b29..eac3414 100644
> > > > > --- a/src/mesa/drivers/dri/i965/gen8_blorp.c
> > > > > +++ b/src/mesa/drivers/dri/i965/gen8_blorp.c
> > > > > @@ -297,7 +297,7 @@ gen8_blorp_emit_ps_config(struct
brw_context
> > > *brw,
> > > > > dw3 = dw5 = dw6 = dw7 = ksp0 = ksp2 = 0;
> > > > > dw3 |= GEN7_PS_VECTOR_MASK_ENABLE;
> > > > >
> > > > > - if (params->[25]src.mt) {
> > > > > + if (params->[26]src.bo) {
> > > > > dw3 |= 1 << GEN7_PS_SAMPLER_COUNT_SHIFT; /* Up to 4
samplers
> > > */
> > > > > dw3 |= 2 << GEN7_PS_BINDING_TABLE_ENTRY_COUNT_SHIFT; /*
Two
> > > surfaces */
> > > > > } else {
> > > > > @@ -362,7 +362,7 @@ gen8_blorp_emit_ps_extra(struct brw_context
> > > *brw,
> > > > >
> > > > > dw1 |= GEN8_PSX_PIXEL_SHADER_VALID;
> > > > >
> > > > > - if (params->[27]src.mt)
> > > > > + if (params->[28]src.bo)
> > > > > dw1 |= GEN8_PSX_KILL_ENABLE;
> > > > >
> > > > > if (params->wm_prog_data->num_varying_inputs)
> > > > > @@ -482,14 +482,12 @@ gen8_blorp_emit_surface_states(struct
> > > brw_context *brw,
> > > > > uint32_t wm_surf_offset_renderbuffer;
> > > > > uint32_t wm_surf_offset_texture = 0;
> > > > >
> > > > > - intel_miptree_used_for_rendering(params->[29]dst.mt);
> > > > > -
> > > > > wm_surf_offset_renderbuffer =
> > > > > brw_blorp_emit_surface_state(brw, ¶ms->dst,
> > > > > I915_GEM_DOMAIN_RENDER,
> > > > > I915_GEM_DOMAIN_RENDER,
> > > > > true /* is_render_target
*/);
> > > > > - if (params->[30]src.mt) {
> > > > > + if (params->[31]src.bo) {
> > > > > wm_surf_offset_texture =
> > > > > brw_blorp_emit_surface_state(brw, ¶ms->src,
> > > > >
I915_GEM_DOMAIN_SAMPLER, 0,
> > > > > @@ -533,7 +531,7 @@ gen8_blorp_exec(struct brw_context *brw,
const
> > > struct brw_blorp_params *params)
> > > > >
> > > > > gen7_blorp_emit_binding_table_pointers_ps(brw,
> > > wm_bind_bo_offset);
> > > > >
> > > > > - if (params->[32]src.mt) {
> > > > > + if (params->[33]src.bo) {
> > > > > const uint32_t sampler_offset =
> > > > > gen6_blorp_emit_sampler_state(brw,
BRW_MAPFILTER_LINEAR,
> > > 0, true);
> > > > > gen7_blorp_emit_sampler_state_pointers_ps(brw,
> > > sampler_offset);
> > > > > --
> > > > > 2.5.0.400.gff86faf
> > > > >
> > > > > _______________________________________________
> > > > > mesa-dev mailing list
> > > > > [34]mesa-dev at lists.freedesktop.org
> > > > > [35]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > >
> > > References
> > >
> > > 1. mailto:topi.pohjolainen at intel.com
> > > 2. http://batch.bo/
> > > 3. http://src.mt/
> > > 4. http://src.bo/
> > > 5. http://depth.bo/
> > > 6. http://dst.mt/
> > > 7. http://src.mt/
> > > 8. http://src.bo/
> > > 9. http://src.mt/
> > > 10. http://src.bo/
> > > 11. http://depth.mt/
> > > 12. http://depth.bo/
> > > 13. http://src.mt/
> > > 14. http://src.bo/
> > > 15. http://src.mt/
> > > 16. http://src.bo/
> > > 17. http://depth.bo/
> > > 18. http://dst.mt/
> > > 19. http://src.mt/
> > > 20. http://src.bo/
> > > 21. http://src.mt/
> > > 22. http://src.bo/
> > > 23. http://depth.mt/
> > > 24. http://depth.bo/
> > > 25. http://src.mt/
> > > 26. http://src.bo/
> > > 27. http://src.mt/
> > > 28. http://src.bo/
> > > 29. http://dst.mt/
> > > 30. http://src.mt/
> > > 31. http://src.bo/
> > > 32. http://src.mt/
> > > 33. http://src.bo/
> > > 34. mailto:mesa-dev at lists.freedesktop.org
> > > 35. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160729/1079c3ef/attachment-0001.html>
More information about the mesa-dev
mailing list