On Fri, Aug 1, 2014 at 2:44 AM, Pohjolainen, Topi
<topi.pohjolai...@intel.com> wrote:
> On Fri, Aug 01, 2014 at 12:53:40AM -0700, Jordan Justen wrote:
>> (a23cfb8 for gen6)
>>
>> In layered rendering this will be 0. Otherwise it will be the
>> selected slice.
>>
>> Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
>> ---
>> src/mesa/drivers/dri/i965/gen6_blorp.cpp | 3 +++
>> src/mesa/drivers/dri/i965/gen6_depth_state.c | 10 ++++++++++
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
>> b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
>> index 131c4aa..ff1732d 100644
>> --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
>> +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
>> @@ -793,6 +793,7 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context
>> *brw,
>> uint32_t tile_mask_x, tile_mask_y;
>> uint32_t surftype;
>> unsigned int depth = MAX2(params->depth.mt->logical_depth0, 1);
>> + unsigned int min_array_element;
>> GLenum gl_target = params->depth.mt->target;
>> unsigned int lod;
>>
>> @@ -818,6 +819,8 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context
>> *brw,
>> NULL,
>> &tile_mask_x, &tile_mask_y);
>>
>> + min_array_element = params->depth.layer;
>
> This could be slightly simpler:
>
> const unsigned min_array_element = params->depth.layer;
>
>> +
>> lod = params->depth.level - params->depth.mt->first_level;
>>
>> /* 3DSTATE_DEPTH_BUFFER */
>> diff --git a/src/mesa/drivers/dri/i965/gen6_depth_state.c
>> b/src/mesa/drivers/dri/i965/gen6_depth_state.c
>> index 1f36ed8..5e3981c 100644
>> --- a/src/mesa/drivers/dri/i965/gen6_depth_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen6_depth_state.c
>> @@ -48,6 +48,7 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
>> struct gl_framebuffer *fb = ctx->DrawBuffer;
>> uint32_t surftype;
>> unsigned int depth = 1;
>> + unsigned int min_array_element;
>> GLenum gl_target = GL_TEXTURE_2D;
>> unsigned int lod;
>> const struct intel_renderbuffer *irb = NULL;
>> @@ -100,6 +101,15 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
>> break;
>> }
>>
>> + if (fb->MaxNumLayers > 0 || !irb) {
>> + min_array_element = 0;
>> + } else if (irb->mt->num_samples > 1) {
>> + /* Convert physical layer to logical layer. */
>> + min_array_element = irb->mt_layer / irb->mt->num_samples;
>
> I still don't understand this. This would be needed only for surfaces were
> samples indices are in their own layers. On gen6 this is never the case as
> all the surfaces are interleaved (i.e., samples are in the same physical
> layer). Moreover, this is true also on gen7 for depth surfaces - and hence
> I'm also questioning the existing logic in the gen7 specific path.
I looked again, and I think gen7/gen8 does not divide by num_samples
for depth state. (Chris changed this in 77d55ef4.) It does in
gen7_wm_surface_state.c.
A piglit run verifies that this works for gen6:
const unsigned min_array_element = irb ? irb->mt_layer : 0;
This matches gen7 & gen8 for depth state, so I'll change to this.
Would that get an r-b from you for this patch?
But delving further into the "divide by num_samples" question,
git grep -C5 "mt_layer =" src/mesa/drivers/dri/i965
turned up some interesting results.
intel_fbo.c:
irb->mt_layer = layer_multiplier * layer;
(For gen7+ UMS/CMS layer_multiplier = mt->num_samples)
intel_fbo.h:
* Note: for a 2D multisample array texture on Gen7+ using
* INTEL_MSAA_LAYOUT_UMS or INTEL_MSAA_LAYOUT_CMS, mt_layer is the physical
* layer holding sample 0. So, for example, if mt->num_samples == 4, then
* logical layer n corresponds to mt_layer == 4*n.
intel_mipmap_tree.c:compute_msaa_layout always returns IMS for depth,
so that explains why in all cases we don't divide by num_samples for
depth.
It might be better to make a helper routine that adjusts layer based
on the msaa layout type. Basically if UMS/CMS, divide by num_samples.
Right now there is a lot of code path knowledge in this calculation.
For instance:
* on depth paths we are using IMS, so don't divide
* on gen6 we only use IMS for color, so don't divide
* on gen7/8 color, we are using UMS/CMS, so divide
-Jordan
>> + } else {
>> + min_array_element = irb->mt_layer;
>> + }
>> +
>> lod = irb ? irb->mt_level - irb->mt->first_level : 0;
>>
>> BEGIN_BATCH(7);
>> --
>> 2.0.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
