Bug 53311

Summary: [Bisected IVB]Oglc transform_feedback(advanced.transformFeedback.points) Invalid argument
Product: Mesa Reporter: lu hua <huax.lu>
Component: Drivers/DRI/i965Assignee: Kenneth Graunke <kenneth>
Status: VERIFIED FIXED QA Contact:
Severity: major    
Priority: high CC: idr, xunx.fang
Version: git   
Hardware: All   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:

Description lu hua 2012-08-10 01:51:58 UTC
System Environment:
--------------------------
Arch:             x86_64
Platform:         Ivybridge
Libdrm:(master)libdrm-2.4.37-26-g93fef04b1e3a83e2f884880ed1c3395f67b038ab
Mesa:(master)34665381713249c29b7da5028396222dfea477c2
Xserver:(master)xorg-server-1.12.99.904
Xf86_video_intel:(master)2.20.3-18-g22306144030b5d37df6d46321555bced6e33c50c
Libva:	(staging)f12f80371fb534e6bbf248586b3c17c298a31f4e
Libva_intel_driver:(staging)82fa52510a37ab645daaa3bb7091ff5096a20d0b
Kernel:	(drm-intel-next-queued) 65bccb5c708bd9f00d24f041f4f7c45130359448

Bug detailed description:
-----------------------------
It happens on ivybridge with mesa master branch.It doesn't happen on mesa 8.0 branch.
Following cases also have this issue,and same bisect commit:
transform_feedback(advanced.transformFeedback.points)
transform_feedback(advanced.transformFeedback.lines)
transform_feedback(advanced.transformFeedback.lineStrip)
transform_feedback(advanced.transformFeedback.lineLoop)
transform_feedback(advanced.transformFeedback.triangles)
transform_feedback(advanced.transformFeedback.triangleStrip)
transform_feedback(advanced.transformFeedback.triangleFan)
transform_feedback(advanced.transformFeedback.quads)
transform_feedback(advanced.transformFeedback.quadStrip)
transform_feedback(advanced.transformFeedback.polygon)
transform_feedback(advanced.buffer.usage)

Bisect shows: e45a9ce474c3562f16c8a773260752d77a4fed5c is the first bad commit.
commit e45a9ce474c3562f16c8a773260752d77a4fed5c
Author:     Kenneth Graunke <kenneth@whitecape.org>
AuthorDate: Tue Aug 7 10:17:04 2012 -0700
Commit:     Kenneth Graunke <kenneth@whitecape.org>
CommitDate: Wed Aug 8 09:24:23 2012 -0700

    i965: Use 64-bit writes for occlusion queries.

    The hardware seems to use the length of the PIPE_CONTROL command to
    indicate whether the write is 64-bits or 32-bits.  Which makes sense
    for immediate writes.

    Daniel discovered this by writing a pattern into the query object bo
    and noticing that the high 32-bits were left intact, even on those
    pipe control writes that seemingly worked.

    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Reviewed-by: Eric Anholt <eric@anholt.net>


Visual Report for ID 148 (32 bits).
ID      |ACCELERA|DB      |REND_T  |SURF_T  |C_BUF_T |BUF_S   |RED_S   |
     148|       1|       1|      gl|  wipbpx|    rgba|      32|       8|

GREEN_S |BLUE_S  |ALPHA_S |DEPTH_S |STENC_S |ACCUM_S |SPL_BUF |SAMPLES |
       8|       8|       8|      24|       8|      64|       0|       0|

SRGB    |TEX_RGB |TEX_RGBA|CAVEAT  |SWAP    |M_PBUF_W|M_PBUF_H|M_PBUF_P
      -1|       0|       0|    slow|   undef|       0|       0|       0

OpenGL Report.
    Vendor - 'Intel Open Source Technology Center'
    Renderer - 'Mesa DRI Intel(R) Ivybridge Desktop '
    Version - '3.0 Mesa 8.1-devel (git-3466538)' (3.0)
    GLSL Version - '1.30'
    Context Flags - None

>> Transform feedback (transform_feedback)  test:
--> 2.1.1 - advanced.transformFeedback.points subcase:
intel_do_flush_locked failed: Invalid argument

Reproduce steps:
----------------------------
1. start x
2. ./oglconform -z -suite all -v 2 -D 148 -test transform_feedback \ advanced.transformFeedback.points
Comment 1 Kenneth Graunke 2012-08-10 17:27:30 UTC
Yow!  This was actually a nasty bug that's existed for ages and ages, and just happened to get tripped up by this change.

Patches on mailing list, awaiting review:
http://lists.freedesktop.org/archives/mesa-dev/2012-August/025538.html
http://lists.freedesktop.org/archives/mesa-dev/2012-August/025539.html

These should also fix some occlusion query issues.
Comment 2 Kenneth Graunke 2012-08-13 03:18:49 UTC
Fixed by the following commit:

commit 9da50667f490ba2c6240f4c91c9707e3f181adae
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Fri Aug 10 10:26:03 2012 -0700

    intel: Move finish_batch() call before MI_BATCH_BUFFER_END and padding.
    
    On Gen4+, brw_finish_batch() calls brw_emit_query_end(), which emits
    some extra PIPE_CONTROLs to capture the current occlusion query data.
    Unfortunately, it was being called *after* _intel_batchbuffer_flush
    added the MI_BATCH_BUFFER_END, meaning those PIPE_CONTROLs didn't get
    inside the batch.
    
    Not only does this likely cause bogus occlusion query values, it can
    also cause crashes: with the recent change to use 64-bit depth count
    writes on Gen6+, we started emitting an odd-length PIPE_CONTROL, which
    happened after the MI_NOOP padding.  This resulted in an odd-length
    batch buffer, which resulted in execbuf2 returning -EINVAL and the
    application dying with an intel_do_flush_locked failure.
    
    On older generations, finish_batch() doesn't emit any state, so this
    change shouldn't have any effect.
    
    Huge thanks to Chris Wilson for helping me figure this out.
    
    NOTE: This is a candidate for stable release branches.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=53311
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
    Reviewed-by: Eric Anholt <eric@anholt.net>

The commit immediately following that one also should help fix related issues:

commit 4e087de51ad0e7ba4a7199d3664e1d096f8dc510
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Fri Aug 10 10:26:04 2012 -0700

    intel: Reserve enough space to finish occlusion queries on Gen6.
    
    After realizing that brw_finish_batch emitted some final PIPE_CONTROLs
    to record occlusion queries, Chris noted that we probably hadn't
    reserved enough space to actually emit them.
    
    Reserving a full 60 bytes seems a bit harsh, since we only need that
    much if occlusion queries are actually active.  Plus, 28 bytes would be
    sufficient for Gen7, and 24 for Gen4-5.
    
    We could optimize this in the future, but it doesn't seem too critical.
    
    NOTE: This is a candidate for stable release branches.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=53311
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
    Reviewed-by: Eric Anholt <eric@anholt.net>
Comment 3 lu hua 2012-08-14 01:44:27 UTC
Verified.Fixed on commit master 4e087de51ad0e7ba4a7199d3664e1d096f8dc510.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.