[Mesa-dev] [PATCH 1/3] i965: solve cubemap negative x/y/z faces buffer offset issue in dEQP.

Wed Oct 5 17:58:27 UTC 2016

Randy,

I hadn't realized that we could get images in from EGL where we have a
non-zero tile_x and tile_y offset for layer 0 mip 0.  That explains
things.  In that case, I believe this is the correct patch.  That said, I
would like to see an "assert(tile_x == 0 && tile_y == 0)" right before we
do the intel_miptree_get_tile_offset() in the case below.  I don't think
those can ever happen at the same time, but if they do, I want to know.

--Jason

On Tue, Oct 4, 2016 at 5:13 PM, Xu, Randy <randy.xu at intel.com> wrote:

> Hi, Jason & Tapani
>
>
>
> Thanks for your review, let me introduce the dEQP failure first.
>
>
>
> In dEQP-EGL.functional.image.create.gles2_cubemap_negative_*_texture, 2D
> textures are generated from all 6 faces of a Cubemap texture (64x64), and
> then rendered through glDrawXXX.
>
> In brw_miptree_get_vertical_slice_pitch, the mt->qpitch is counted as 144.
>
>       return h0 + h1 + (brw->gen >= 7 ? 12 : 11) * mt->valign;
>                             // 64+32+12*4 = 144
>
>
>
> Take the face negative_x for example, the total offset in bo is
> 144(y)*64(x)*4(bpp) = 36864.
>
> It’s TILING_Y buffer, as the y (144) is not 32 aligned (mask_y = 31 from
> intel_region_get_tile_masks), the total bo offset is divided into two
> parts: 36864 =  32768 (offset 128*64*4) + 16(tile_y)*64*4
>
>    case I915_TILING_Y:
>
>       *mask_x = 128 / cpp - 1;
>
>       *mask_y = 31;
>
>
>
>
>
> Both the tile_y and offset are passed to texture2D in
> create_mt_for_dri_image, while the tile_y is not used to count the total
> offset in rendering path, that’s why I add this patch.
>
> Please check and comment more.
>
>
>
> Thanks,
>
> Randy
>
>
>
>
>
>
>
>
>
> *From:* Jason Ekstrand [mailto:jason at jlekstrand.net]
> *Sent:* Tuesday, October 4, 2016 11:59 PM
> *To:* Palli, Tapani <tapani.palli at intel.com>
> *Cc:* Xu, Randy <randy.xu at intel.com>; mesa-dev at lists.freedesktop.org;
> Xu at freedesktop.org
> *Subject:* Re: [Mesa-dev] [PATCH 1/3] i965: solve cubemap negative x/y/z
> faces buffer offset issue in dEQP.
>
>
>
> On Tue, Oct 4, 2016 at 8:55 AM, Tapani Pälli <tapani.palli at intel.com>
> wrote:
>
> On 10/04/2016 06:09 PM, Jason Ekstrand wrote:
>
> On Thu, Sep 29, 2016 at 11:27 PM, Xu,Randy <randy.xu at intel.com> wrote:
>
> Add the miptree level/slice x/y_offset when count the surface offset
> in brw_emit_surface_state. The surface offset has two parts, one is
> from mt->offset, which should be 32 aligned in width/height for tiled
> buffer; another is from mt->level[current_level].slice[current_slice].
> x/y_offset.
>
> This fix will solve 12 deqp failure
> dEQP-EGL.functional.image.create.gles2_cubemap_negative_*_texture
>
> Signed-off-by: Xu,Randy <randy.xu at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> 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 61a4b94..3a5c573 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -85,7 +85,8 @@ brw_emit_surface_state(struct brw_context *brw,
>                         unsigned read_domains, unsigned write_domains)
>  {
>     const struct surface_state_info ss_info =
> surface_state_infos[brw->gen];
> -   uint32_t tile_x = 0, tile_y = 0;
> +   uint32_t tile_x = mt->level[0].slice[0].x_offset;
> +   uint32_t tile_y = mt->level[0].slice[0].y_offset;
>
>
>
> This isn't correct.  First off, there are some fairly strict restrictions
> on what we can do with tile_x and tile_y and we can't just shove x/y_offset
> in there.  We need to use intel_miptree_get_tile_offsets to get both a byte
> offset and an intratile offset.  Second, we should already be taking slices
> into account for cube maps via base_array_layer where needed.
>
> Unfortunately, I'm not 100% sure what the correct patch is without a bit
> more information about what the test is doing that causes a problem.
>
>
>
> I did take a brief look and when running the set mentioned above (for
> example with ./deqp-egl --deqp-case=*EGL.functional.
> image.create.gles2_cubemap_negative_*_texture) what happens is that we
> never end up to the part of code calling intel_miptree_get_tile_offsets in
> that function (because surf.dim_layout != dim_layout condition does not
> trigger). This is just what I observed, should we just call
> intel_miptree_get_tile_offsets() unconditionally then?
>
>
>
> No.  Very much no.  The intel_miptree_get_tile_offsets() stuff is a hack
> that lets us convert non-2D things to 2D things and it comes with piles of
> restrictions.
>
>
>
>
>
> --Jason
>
>
>
>     uint32_t offset = mt->offset;
>
>     struct isl_surf surf;
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
>
>
> _______________________________________________
>
> mesa-dev mailing list
>
> mesa-dev at lists.freedesktop.org
>
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161005/f4a60d88/attachment-0001.html>