Bug 21561 - UMS: Audio curves display corrupted in Audacity with radeon and EXA on RV670
UMS: Audio curves display corrupted in Audacity with radeon and EXA on RV670
Status: NEW
Product: xorg
Classification: Unclassified
Component: Driver/Radeon
7.4 (2008.09)
Other All
: medium normal
Assigned To: xf86-video-ati maintainers
Xorg Project Team
:
: 21855 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-05 04:27 UTC by Alain Perrot
Modified: 2011-01-12 13:18 UTC (History)
3 users (show)

See Also:


Attachments
Xorg log with radeon driver, EXA enabled (51.68 KB, text/plain)
2009-05-05 04:27 UTC, Alain Perrot
no flags Details
Audacity audio editor, with corrupted audio curves (155.86 KB, image/png)
2009-05-05 04:29 UTC, Alain Perrot
no flags Details
fallback for all exa accel hooks (1.26 KB, patch)
2009-05-06 11:37 UTC, Alex Deucher
no flags Details | Splinter Review
fallback for all exa accel hooks (1.26 KB, patch)
2009-05-06 11:38 UTC, Alex Deucher
no flags Details | Splinter Review
disable EXA copy (406 bytes, patch)
2009-05-06 14:21 UTC, Alex Deucher
no flags Details | Splinter Review
disable EXA solid (397 bytes, patch)
2009-05-06 14:22 UTC, Alex Deucher
no flags Details | Splinter Review
fallback for all solid rops except gxcopy (413 bytes, patch)
2009-05-07 15:09 UTC, Alex Deucher
no flags Details | Splinter Review
Audio file with short parts correctly displayed, and longer parts corrupted (59.58 KB, image/png)
2009-10-08 12:40 UTC, Alain Perrot
no flags Details
properly check planemasks (2.88 KB, patch)
2009-10-08 14:20 UTC, Alex Deucher
no flags Details | Splinter Review
untested patch (10.87 KB, patch)
2010-04-13 11:38 UTC, Alex Deucher
no flags Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alain Perrot 2009-05-05 04:27:57 UTC
Created attachment 25458 [details]
Xorg log with radeon driver, EXA enabled

Hi,

Using the Audacity audio editor version 1.3.7 on my Kubuntu 9.04 desktop system, I noticed that the audio curves display is corrupted when using the radeon (xf68-driver-ati) driver with EXA acceleration enabled on my Radeon HD 3870 (RV670) card.

The radeonhd driver also shows this issue with EXA acceleration enabled.

The display is correct with both drivers when EXA acceleration is disabled.

I am using the Kubuntu 9.04 kernel 2.6.28 with up-to-date drm modules build from the r6xx-r7xx-support branch of the git repository, and both radeon and radeonhd drivers build from the master branch of their git repository.
Comment 1 Alain Perrot 2009-05-05 04:29:45 UTC
Created attachment 25459 [details]
Audacity audio editor, with corrupted audio curves
Comment 2 Alex Deucher 2009-05-06 09:29:58 UTC
Do any of these options fix the problem:

Option "EXANoComposite"
Option "EXANoUploadToScreen"
Option "EXANoDownloadFromScreen"
Comment 3 Alain Perrot 2009-05-06 11:07:25 UTC
I have just tried with the three options set, but this does not fix the issue.

I have not tried each option alone, but I will do if you think it might be useful.
Comment 4 Rafael Monica 2009-05-06 12:59:01 UTC
Hi Alex, I have also this bug with audacity on my HD 3200 IGP. The options you mentioned did not have any effect. However using

Option "MigrationHeuristic" "greedy"

does seem to fix the problem.
Comment 5 Alex Deucher 2009-05-06 13:14:45 UTC
Does the patch I attached help?
Comment 6 Alex Deucher 2009-05-06 13:16:23 UTC
This might be an issue with the EXA core code.  The patch sets all the accel hooks to fall back to software.
Comment 7 Rafael Monica 2009-05-06 13:58:38 UTC
Yes, the attached patch works. However the software fall back is unbearably slow.
Comment 8 Alain Perrot 2009-05-06 14:16:38 UTC
I can confirm that the patch fix the corruption at the cost of performance.
Comment 9 Alex Deucher 2009-05-06 14:19:05 UTC
(In reply to comment #8)
> I can confirm that the patch fix the corruption at the cost of performance.
> 

the performance will be slow as it disables acceleration.  I just wanted to see if it was an EXA bug or a driver bug.
Comment 10 Alex Deucher 2009-05-06 14:21:24 UTC
Created attachment 25570 [details] [review]
disable EXA copy

Try these two patches separately to see whether it's EXA copy or solid that is causing the problem.
Comment 11 Alex Deucher 2009-05-06 14:22:06 UTC
Created attachment 25571 [details] [review]
disable EXA solid
Comment 12 Alain Perrot 2009-05-06 14:38:23 UTC
Disable EXA copy: issue is still there
Disable EXA solid: issue fixed
Comment 13 Rafael Monica 2009-05-06 14:51:40 UTC
(In reply to comment #12)
> Disable EXA copy: issue is still there
> Disable EXA solid: issue fixed
> 

Yep, same here.
Comment 14 Michel Dänzer 2009-05-07 03:09:31 UTC
Can't seem to reproduce this on an RV350, so maybe it's a bug in the R600 Solid code.
Comment 15 Alex Deucher 2009-05-07 15:09:07 UTC
Created attachment 25613 [details] [review]
fallback for all solid rops except gxcopy

Does this patch help?
Comment 16 Rafael Monica 2009-05-07 15:34:16 UTC
Nope. But changing the if (alu != 3) to if (alu == 3) does work,
unsurprisingly  :-)
Comment 17 Michel Dänzer 2009-05-08 00:15:34 UTC
Not sure this is the problem, but one thing I notice is that the R600Prepare{Solid,Copy} hooks don't seem to handle the planemask correctly - they just seem to assume it's a 32 bit ARGB component mask. They need to take the pixmap depth into account and fall back if the mask doesn't correspond to a set of components.
Comment 18 Nicos Gollan 2009-08-02 07:26:24 UTC
A quick "me too": I am experiencing the issue on an HD4870 (RV770) with what Debian ships as version 6.12.2 of the "radeon" driver.
Comment 19 Alex Deucher 2009-10-08 09:11:06 UTC
*** Bug 21855 has been marked as a duplicate of this bug. ***
Comment 20 Alex Deucher 2009-10-08 09:13:57 UTC
Is there a specific audio file or spreadsheet that exhibits this?  I can't reproduce this on any r6xx or r7xx hardware I have.
Comment 21 Yang Zhao 2009-10-08 09:58:59 UTC
(In reply to comment #20)
> Is there a specific audio file or spreadsheet that exhibits this?  I can't
> reproduce this on any r6xx or r7xx hardware I have.

Seems to only apply to a very dense waveform.  Try a sample that's at least several seconds long.
Comment 22 Michel Dänzer 2009-10-08 10:08:34 UTC
Alex, addressing the problem described in comment #17 might be a good start. :)
Comment 23 Alain Perrot 2009-10-08 12:40:52 UTC
Created attachment 30183 [details]
Audio file with short parts correctly displayed, and longer parts corrupted
Comment 24 Alain Perrot 2009-10-08 12:41:24 UTC
(In reply to comment #20)
> Is there a specific audio file or spreadsheet that exhibits this?  I can't
> reproduce this on any r6xx or r7xx hardware I have.
> 

All of the audio files I tried show some corruptions. These files are vinyl disc rips.

One thing I didn't noticed before however with one of these files is that some parts of the curves are correctly diplayed while others are corrupted. The parts that are correctly displayed are less than 1 minute long, while the corrupted ones are longer. I attached a screenshot of Audacity diplaying this file.
Comment 25 Alex Deucher 2009-10-08 14:20:40 UTC
Created attachment 30189 [details] [review]
properly check planemasks

Does this patch help?
Comment 26 Rafał Miłecki 2009-10-08 14:39:53 UTC
(In reply to comment #25)
> Created an attachment (id=30189) [details]
> properly check planemasks
> 
> Does this patch help?

Did not help on my RV620 (M82). I've added some debugging to this patch. R600ValidPM is called very often, but:

>     if (!R600ValidPM(pm, pPix->drawable.bitsPerPixel))
>        RADEON_FALLBACK(("invalid planemask\n"));

never happens in any: Solid or Copy. I've changed that in both places to:

>     if (!R600ValidPM(pm, pPix->drawable.bitsPerPixel)) {
>         xf86DrvMsg(pScrn->scrnIndex, X_INFO, "[ZAJEC] invalid");
>         RADEON_FALLBACK(("invalid planemask\n"));
>     }

and nothing in my log.
Comment 27 Michel Dänzer 2009-10-09 05:19:05 UTC
The patch basically looks good, so that rules out the planemask...

The screenshots look like maybe some of the solid fills are dropped on the floor. Has the code below in R600Solid() been tested to work correctly? Maybe it would need to e.g. do some more of the things R600PrepareSolid() does?

    if (((accel_state->vb_index + 3) * 8) > accel_state->vb_total) {
        R600DoneSolid(pPix);
	r600_cp_start(pScrn);
    }
Comment 28 Rafał Miłecki 2009-10-09 05:46:08 UTC
(In reply to comment #25)
> Created an attachment (id=30189) [details]
> properly check planemasks
> 
> Does this patch help?

I've also added printing a, r, g and b in ValidPM function (with ZAJEC prefix).

> grep ZAJEC /var/log/Xorg.0.log | wc -l
26239
> grep ZAJEC /var/log/Xorg.0.log | uniq
(II) RADEON(0): [ZAJEC] a:ff ; r:ff ; g:ff ; b:ff

Btw. this patch is not going to driver, is it? Because I feel you missed "break" in case 8.
Comment 29 Alex Deucher 2009-10-09 08:09:04 UTC
does de55995e82c3875f70b6394fff440d695d062113 help?
Comment 30 Rafał Miłecki 2009-10-09 08:27:45 UTC
(In reply to comment #29)
> does de55995e82c3875f70b6394fff440d695d062113 help?

I would like MrCooper to be wrong. Unfortunately he is not :(

<MrCooper> agd5f: from Zajec's debugging it looks like there are only solid masks, so it probably doesn't help
Comment 31 Alain Perrot 2009-10-09 12:57:36 UTC
(In reply to comment #29)
> does de55995e82c3875f70b6394fff440d695d062113 help?
> 

I've just checked with an updated radeon driver, including commit de55995e82c3875f70b6394fff440d695d062113. Corruption is still there.
Comment 32 Alain Perrot 2009-11-16 11:03:14 UTC
Hi,

Some news on this issue.

I am currently testing kernel mode setting on my Radeon HD 3870 using Kubuntu 9.10, Linux 2.6.32-rc7 from Ubuntu kernel PPA and libdrm/radeon/mesa from Xorg-edgers PPA.

When KMS is disabled, the audio curves are still corrupted in Audacity, but when KMS is enabled, the audio curves are correctly displayed.

It also looks like other minor graphical glitches that I used to have with KDE (mostly in the panel) and in NetBeans 6.7 IDE splash screen completely disappear when KMS is enabled.

I will switch back to UMS for now because of some performance issues (mostly scrolling in Firefox), but KMS really impressed me so far.
Comment 33 Alex Deucher 2009-11-16 16:29:16 UTC
Do the latest changes in git master make any difference?
Comment 34 Rafał Miłecki 2009-11-17 00:48:58 UTC
(In reply to comment #33)
> Do the latest changes in git master make any difference?

Tried current git (commit ce8299962003de572122561a6f7f61eaccf633b2) still broken.
Comment 35 udo 2010-04-12 07:30:20 UTC
A 'me too' here.
Can easily reproduce the issue in Audacity.
Patch 'disable EXA solid' works around the issue.
Can test more if needed. Please supply patches etc.
Comment 36 Rafael Monica 2010-04-12 11:34:17 UTC
This bug is gone since I switched to kms.
Comment 37 udo 2010-04-13 07:12:10 UTC
(In reply to comment #35)
> A 'me too' here.

I see the error with radeon (F12 latest) *AND* radeonhd (git, also on 1.2.5).

Can easily reproduce the issue in Audacity.
Can test more if needed. Please supply patches etc.
Comment 38 udo 2010-04-13 09:19:24 UTC
After undoing the patch at  https://bugs.freedesktop.org/attachment.cgi?id=25571 amd applying this patch:

      --- a/src/r600_exa.c
      +++ b/src/r600_exa.c
      @@ -267,6 +267,7 @@ R600Solid(PixmapPtr pPix, int x1, int y1, int x2, int y2)
       
           if (((accel_state->vb_index + 3) * 8) > (accel_state->ib->total / 2)) {
              R600DoneSolid(pPix);
      +        ErrorF("udo: bla\n");
              accel_state->vb_index = 0;
              accel_state->ib = RHDDRMCPBuffer(pScrn->scrnIndex);
           }

And staring Audacity and generating some DTMF we saw some `udo: bla` lines logged.
Conclusion:

<agd5f> udovdh: ok.  well, if it's really dropping rendering due to missing state, the ddx really needs to be re-organized like mesa to set and emit state functions
Comment 39 Alex Deucher 2010-04-13 11:38:37 UTC
Created attachment 34974 [details] [review]
untested patch

If the bug is indeed related to state, this hack might help, but the real solution is probably to rewrite the EXA and Xv code to track/emit state like mesa does.  Unfortunately, that means a fairly major rewrite of the current code.
Comment 40 Dave Airlie 2010-04-15 23:29:23 UTC
Just to add my analysis to this for posterity.

This is a UMS bug only, it could happen with KMS, its just extremely unlikely.

When we emit the solid vertices in KMS, we don't have to flush the whole indirect buffer when we run out of VBO space, we just create a new VBO and continue from there, adding a new VBO to the current IB involves writing 7+7+5+11+10 = 40 dwords to the IB. We currently leave a buffer of 1024 dwords in the command stream, so that is quite a lot of VBOs.