Bug 19016 - [regression GEM] oglconform spop.c slow (glBitmap fallback with stencil buffer)
Summary: [regression GEM] oglconform spop.c slow (glBitmap fallback with stencil buffer)
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i915 (show other bugs)
Version: unspecified
Hardware: Other Linux (All)
: medium normal
Assignee: Eric Anholt
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-10 21:12 UTC by liuhaien
Modified: 2009-10-29 20:54 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
xorg.0.log (43.79 KB, text/plain)
2008-12-10 21:12 UTC, liuhaien
Details
xorg conf file (3.67 KB, text/plain)
2008-12-10 21:12 UTC, liuhaien
Details

Note You need to log in before you can comment on or make changes to this bug.
Description liuhaien 2008-12-10 21:12:03 UTC
Created attachment 21037 [details]
xorg.0.log

System Environment:
--------------------------
Host:           x-q965
Arch:           i386
Libdrm:		(master)c99566fb810c9d8cae5e9cd39d1772b55e2f514c
Mesa_stable:		(intel-2008-q4)154a9e5317f890618932cea0129ef887e16baf84
Xserver_stable:	(server-1.6-branch)523aae1fa6d8002e55e85aee49f113b7eb9a6df3
Xf86_video_intel_stable:(xf86-video-intel-2.6-branch) 
                         6ca0d7e6ff05bff2bb88bfae64c2d79ac115bd38
Bug detailed description:
-------------------------
startx, then run oglconform/spop.c, it will take more than 40 minutes. but when run it with software rendering ,it can work rapidly(about 2 minutes).
Reproduce steps:
----------------
1.xinit&
2. run oglconform/spop.c
Comment 1 liuhaien 2008-12-10 21:12:44 UTC
Created attachment 21038 [details]
xorg conf file
Comment 2 liuhaien 2008-12-10 21:14:17 UTC
GEM_kernel:       (for-airlied)6c76409370d7ee18208a7adfa5f8dabf8a42a274
Comment 3 Gordon Jin 2008-12-11 00:54:27 UTC
Eric, please take this as high priority. It impacts QA's testing as we set timeout for 20mins. For now, we are temporarily disabling the case.
Comment 4 Ian Romanick 2008-12-11 09:06:43 UTC
Some additional information is going to be required.  I also run this test, and it takes less than 1 minute.  I'm running for-airlied kernel and master of all other trees.  spop.c has completed in less than 20 minutes on my system since at least WW42.  My weekly test results e-mail contains commit IDs as far back as WW43.

This week's bits are:

Mesa:             759000ff4fd1716edee4734a023ffcb61b443764
xf86-video-intel: 80e2d90139dd99f50beb4f9353599608624b777d
libdrm:           9583c099b4a08b49e03f7b461c344b6d277fd262
xserver:          119d9c46e841f5fa35610f557e6fa1ec58587c24
kernel:           8b1fae4e4200388b64dd88065639413cb3f1051c

I'm running a 64-bit kernel, and my X.org log says:

(--) PCI:*(0@0:2:0) Intel Corporation 82G965 Integrated Graphics Controller rev 2, Mem @ 0xd0200000/1048576, 0xc0000000/268435456, I/O @ 0x00002140/8
Comment 5 Eric Anholt 2008-12-11 18:29:53 UTC
Also, a bug that affects an oglconform testcase completion (not even locking up the hardware!) doesn't seem appropriate to be high priority, compared to numerous other bugs we have reported that relate to hanging the chip with user code.
Comment 6 Gordon Jin 2008-12-11 20:24:44 UTC
Ian, thanks for sharing your result. So the difference is you are using driver master branch while Haien is using release branch. Haien, please try master also.

Eric, I admit it's wrong to set high priority. Thanks for pointing this out.
Comment 7 liuhaien 2008-12-14 23:16:19 UTC
(In reply to comment #6)
> Ian, thanks for sharing your result. So the difference is you are using driver
> master branch while Haien is using release branch. Haien, please try master
> also.
> 
> Eric, I admit it's wrong to set high priority. Thanks for pointing this out.
> 

same issues exits with master on our 965, we tried it with below commits:
Libdrm: master c86d431fe6174b1c2de531929213ea7dbd92326d
Mesa:master 1126aa86bf9ca223218695eec1f41c6523068961
xserver:master f1c9b5ab230cbb4124d8d476ae4886d05022adcb
Xf86-video-intel :master ea2b6b405e4c8b1bfb4bc568d0453a39a9194a8f
Comment 8 Ian Romanick 2008-12-15 00:17:49 UTC
(In reply to comment #4)
> Some additional information is going to be required.  I also run this test, and
> it takes less than 1 minute.  I'm running for-airlied kernel and master of all
> other trees.  spop.c has completed in less than 20 minutes on my system since
> at least WW42.  My weekly test results e-mail contains commit IDs as far back
> as WW43.
> 
> This week's bits are:
> 
> Mesa:             759000ff4fd1716edee4734a023ffcb61b443764
> xf86-video-intel: 80e2d90139dd99f50beb4f9353599608624b777d
> libdrm:           9583c099b4a08b49e03f7b461c344b6d277fd262
> xserver:          119d9c46e841f5fa35610f557e6fa1ec58587c24
> kernel:           8b1fae4e4200388b64dd88065639413cb3f1051c

Small update.  Moving from WW49 (kernel 66647dc60d16fae9f6963fd98b6d9baa1a8dac69) to WW50 (kernel 8b1fae4e4200388b64dd88065639413cb3f1051c) I am now able to reproduce this bug.  When I posted the above my system was still set to boot from the older kernel. :(

With the two-way kernel merging (from the various DRM trees to Linus and back), the older SHA1 no longer exists.  I believe this corresponded to the commit "drm/i915: Disable the GM965 MSI errata workaround."  Instead of having all the patches from DRM grouped and all the upstream kernel changes grouped, this particular change is in the middle of a sea of non-DRM changes.  I have the feeling this is going to be git-bisect hell.
Comment 9 Ian Romanick 2008-12-15 00:20:22 UTC
(In reply to comment #8)

> Small update.  Moving from WW49 (kernel
> 66647dc60d16fae9f6963fd98b6d9baa1a8dac69) to WW50 (kernel
> 8b1fae4e4200388b64dd88065639413cb3f1051c) I am now able to reproduce this bug.

I hit commit too soon.  My test times have gone from <1 minute to ~15 minutes on this test.

I probably shouldn't post bug updates late on Sunday night. :P
Comment 10 Eric Anholt 2008-12-31 00:11:10 UTC
Did another review of my changes for glBitmap metaops, and found the two bugs I was looking for.  Changes pushed -- spop.c now completes in a couple minutes for me (with DRI2, which punishes us for calling glViewport so much).

commit e1a92175542c6645c23cc78f2a4fcd36dd0235e6
Author: Eric Anholt <eric@anholt.net>
Date:   Wed Dec 31 00:02:43 2008 -0800

    intel: Add support for glBitmap as metaops using GL calls.
    
    This lets us avoid software fallbacks when clients forget to turn some state
    off (engine demo) or just do crazy things to test conformance (OGLC).
    
    This should probably be brought into mesa generic code so other drivers can
    make use of it.
    
    Bug #19016.
Comment 11 liuhaien 2009-01-03 18:39:11 UTC
(In reply to comment #10)
> Did another review of my changes for glBitmap metaops, and found the two bugs I
> was looking for.  Changes pushed -- spop.c now completes in a couple minutes
> for me (with DRI2, which punishes us for calling glViewport so much).
> 
> commit e1a92175542c6645c23cc78f2a4fcd36dd0235e6
> Author: Eric Anholt <eric@anholt.net>
> Date:   Wed Dec 31 00:02:43 2008 -0800
> 
>     intel: Add support for glBitmap as metaops using GL calls.
> 
>     This lets us avoid software fallbacks when clients forget to turn some
> state
>     off (engine demo) or just do crazy things to test conformance (OGLC).
> 
>     This should probably be brought into mesa generic code so other drivers can
>     make use of it.
> 
>     Bug #19016.
> 

thanks,it works for me against master branch, we won't verify this bug util this commit is cherry-picked to intel-2008-q4 branch.
Comment 12 liuhaien 2009-01-11 19:25:35 UTC
verified against:
Libdrm:		(master)badc63464cbd64606c6dff9ea561a787d072fd5f
Mesa:		(intel-2008-q4)eef0dcc298f65158dc750a09f80317ded1101dc7
Xorg:		7.2
Xserver:	(server-1.6-branch)1ffd9ec7606606570980490b91b780e02bc29d7d
Xf86_video_intel:		(xf86-video-intel-2.6-branch)
                            4447973345a2a7af20ba1d6cb18c5f1ed8949d00
GEM_kernel:       (drm-intel-2.6.28)e1a6fcee467556a7e955fe1f7ccc134dd2f974e7

Comment 13 liuhaien 2009-02-05 19:05:16 UTC
  spop.c run slow(more then 20 minutes) recently,I try to bisect and find below commit cause this regression:

72ee0e247d799c85612c72bbd2257648e11fa583 is first bad commit
commit 72ee0e247d799c85612c72bbd2257648e11fa583
Author: Brian Paul <brianp@vmware.com>
Date:   Mon Jan 26 14:22:30 2009 -0700

    intel: check if stencil test is enabled in intel_stencil_drawpixels()

:040000 040000 6da523034e04d6b3599138a817e9734e63b75a10 b33df7a1e4812d5854ad48753dd4daa5cd8efe46 M      src
Comment 14 Eric Anholt 2009-02-25 10:45:34 UTC
A conformance test doing stupid things and being slow is *certainly* not a release blocker.
Comment 15 Gordon Jin 2009-02-25 17:08:32 UTC
Eric,
If you think it's a case issue, please explain offline to us. We need fix the case.
Otherwise I assume it indicates a driver performance regression, which certainly possible to impact the release. I didn't mark it as high priority major bug to be a true release blocker.
btw, I thought contributing a bisect result will get it as high priority for the bug owner. With time going farther, I'm afraid the bisect result will be less useful.
Comment 16 Ian Romanick 2009-02-25 20:18:55 UTC
(In reply to comment #15)
> Eric,
> If you think it's a case issue, please explain offline to us. We need fix the
> case.
> Otherwise I assume it indicates a driver performance regression, which
> certainly possible to impact the release. I didn't mark it as high priority
> major bug to be a true release blocker.
> btw, I thought contributing a bisect result will get it as high priority for
> the bug owner. With time going farther, I'm afraid the bisect result will be
> less useful.


The particular case is one of the "upstream" conformance tests, so we can't change it.  Previously we had a work around in the driver to ignore stencil test in the glBitmap case.  However, this is not correct.  Fixing the correctness regressed the performance.  The only way to fix it is to write new glBitmap acceleration code.  Real applications should not do this, so that makes this a very low priority issue.
Comment 17 Gordon Jin 2009-02-26 00:40:05 UTC
(In reply to comment #16)
> The particular case is one of the "upstream" conformance tests, so we can't
> change it.  Previously we had a work around in the driver to ignore stencil
> test in the glBitmap case.  However, this is not correct.  Fixing the
> correctness regressed the performance.  The only way to fix it is to write new
> glBitmap acceleration code.  Real applications should not do this, so that
> makes this a very low priority issue.

Thanks for the explanation. Sounds reasonable to be low priority.

Comment 18 Ian Romanick 2009-10-26 10:21:03 UTC
Is this still a problem?  On my i965 system this test only requires ~2 minutes using Mesa master.
Comment 19 Shuang He 2009-10-27 02:15:48 UTC
(In reply to comment #18)
> Is this still a problem?  On my i965 system this test only requires ~2 minutes
> using Mesa master.
> 

Yes, this case now run only for 2 minutes on G45, I'd try to enable this case in our nightly testing to see if it's really gone.
Comment 20 Shuang He 2009-10-29 20:49:45 UTC
This bug is gone
Comment 21 Shuang He 2009-10-29 20:50:39 UTC
verified on 945GM and G45 with:
Kernel 	(drm-intel-next)a83a4400415893d2599a256f6842ac4d871dffd7
Libdrm 	(master)0d7ad7e43ca212b1e9f16cd18f36493cab455e61
Mesa 	(master)59b29516af2d4d8f83723559921a3709eb77a7d2
Xserver 	(master)fab74d1081270fb8f1d231e6e10d10aa33e164da
Xf86_video_intel 	(master)10946118dd3a63f1375a1bfde0b2f0542a93c1c2


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.