Bug 53311 - [Bisected IVB]Oglc transform_feedback(advanced.transformFeedback.points) Invalid argument
[Bisected IVB]Oglc transform_feedback(advanced.transformFeedback.points) Inva...
Status: VERIFIED FIXED
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965
git
All Linux (All)
: high major
Assigned To: Kenneth Graunke
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-10 01:51 UTC by lu hua
Modified: 2012-08-14 01:44 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.