Bug 5766 - Video mode is too high for the ES1000 video ASIC
Summary: Video mode is too high for the ES1000 video ASIC
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/Radeon (show other bugs)
Version: unspecified
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Alex Deucher
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
: 9192 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-01-31 04:51 UTC by Anatoli Antonovitch
Modified: 2008-11-24 10:21 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Limit pixel clock to model memory bandwidth limits (1.82 KB, patch)
2006-02-17 05:40 UTC, Michel Dänzer
no flags Details | Splinter Review
This patch can limit max pixel clock within ES1000's capacity, also it resolves video corruption problem (2.50 KB, patch)
2007-01-28 06:36 UTC, LisaWu
no flags Details | Splinter Review

Description Anatoli Antonovitch 2006-01-31 04:51:03 UTC
Hello to everybody,

This problem was reported by Dell for Linux Distributor.  It was also
reproduced at ATI using SLES 9 SP2.

When using a monitor that can handle very large resolutions it is possible to
configure the video output beyond what the ES1000 can handle.

Specifically we are seeing resolution 1600x1200@85Hz which will display with
corruption.  This mode is beyond what the ES1000 can support when using 16bit
memory at 200MHz.
Comment 1 Michel Dänzer 2006-02-17 04:50:41 UTC
I'm going to attach WIP patches.
Comment 2 Michel Dänzer 2006-02-17 05:40:11 UTC
Created attachment 4636 [details] [review]
Limit pixel clock to model memory bandwidth limits

This patch models the memory bandwidth limits by calculating the maximum pixel
clock based on the memory clock, bpp and memory bus width (the detection of
which is also fixed for RN50).

It also adds the knowledge that RN50 only has a single CRTC, and uses that to
distinguish it from RV100.
Comment 3 Michel Dänzer 2006-03-10 02:44:22 UTC
Patch committed to xf86-video-ati CVS:

CVSROOT:        /cvs/xorg
Module name:    driver
Changes by:     daenzer@kemper.freedesktop.org  06/03/09 15:41:16

Log message:
  RN50: Skip modes that exceed memory bandwidth.
  
        * src/radeon_driver.c: (RADEONGetClockInfo), (RADEONGetVRamType),
        (RADEONPreInitConfig):
        Bugzilla #5766 <https://bugs.freedesktop.org/show_bug.cgi?id=5766>
        Patch #4636  <https://bugs.freedesktop.org/attachment.cgi?id=4636>
        - Acknowledge that RN50 only has one CRTC, and use this to distinguish
          it from RV100.
        - Fix detection of RN50 memory type and bus width.
        - Model RN50 memory bandwidth limits by capping the pixel clock range
          based on memory clock, bpp and memory bus width.
        (ATI Technologies Inc.)

Modified files:
      driver/xf86-video-ati/:
        ChangeLog 
      driver/xf86-video-ati/src/:
        radeon_driver.c 
  
  Revision      Changes    Path
  1.30          +13 -0     driver/xf86-video-ati/ChangeLog
  http://cvs.freedesktop.org/xorg/driver/xf86-video-ati/ChangeLog
  1.94          +17 -3     driver/xf86-video-ati/src/radeon_driver.c
  http://cvs.freedesktop.org/xorg/driver/xf86-video-ati/src/radeon_driver.c
Comment 4 LisaWu 2007-01-26 01:47:03 UTC
video corrupts at 1024x768@75Hz as a side-effect of the solution
Ajax from Red Hat has worked out a new solution to limit pixel clock within ES1000's capacity as well as to solve the video corruption problem
New patch is attached, this patch is based on radeon source code from git master branch.
Comment 5 LisaWu 2007-01-28 06:36:02 UTC
Created attachment 8521 [details] [review]
This patch can limit max pixel clock within ES1000's capacity, also it resolves video corruption problem
Comment 6 Michel Dänzer 2007-01-29 00:40:59 UTC
Is this really correct for all RV100 models, memory clocks and widths?
Comment 7 LisaWu 2007-01-29 17:26:52 UTC
This is only a patch for RN50 chips. RV100 don't need such pixel clock limit. RN50 has only one CRTC, use this to distinguish it from RV100 
Comment 8 Michel Dänzer 2007-01-30 04:49:41 UTC
(In reply to comment #7)
> This is only a patch for RN50 chips. RV100 don't need such pixel clock limit.
> RN50 has only one CRTC, use this to distinguish it from RV100 

Right, I missed the !pRADEONEnt->HasCRTC2 because it's not in the first if () condition (why?), sorry. However, AFAIK RN50s can have different memory widths and a wide range of clocks, which should have a significant impact on the sustainable pixel clock and are taken into account by the current code.

I assume the motivation for this patch is that there can be issues with limiting pll->max_pll_freq? If so, how about moving the code for clockRanges->maxClock but keeping the formula?
Comment 9 Adam Jackson 2007-01-30 08:38:47 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > This is only a patch for RN50 chips. RV100 don't need such pixel clock limit.
> > RN50 has only one CRTC, use this to distinguish it from RV100 
> 
> Right, I missed the !pRADEONEnt->HasCRTC2 because it's not in the first if ()
> condition (why?), sorry. However, AFAIK RN50s can have different memory widths
> and a wide range of clocks, which should have a significant impact on the
> sustainable pixel clock and are taken into account by the current code.

When I did that patch I was working from the expected resolution limits of then-current RN50 cards.  It's probably correct to take memory type into account when computing the maximum pixel clock, to future-proof against new RN50 models.

> I assume the motivation for this patch is that there can be issues with
> limiting pll->max_pll_freq? If so, how about moving the code for
> clockRanges->maxClock but keeping the formula?

Right.  If you constrain max_pll_freq you force the PLL code to pick timings it otherwise wouldn't.  In the worst case you can end up with max_pll_freq < min_pll_freq, which is catastrophic, but even when the range is well-formed it can have a negative impact on picture quality or cause the monitor to lose sync.

And now that I think about it, even limiting ->maxClock is kind of wrong, because you really need to filter on pixel bandwidth, which isn't linearly related to maxClock given things like reduced-blanking modes.  (My patch above would allow 1400x1050 RB at 24bpp, which is too much for the memory bandwidth.)  It should probably go in RADEONValidMode() instead, where we can reject modes directly based on the product of HActive and VActive.
Comment 10 Daniel Stone 2007-02-27 01:30:13 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 11 Alex Deucher 2007-08-31 07:58:48 UTC
*** Bug 9192 has been marked as a duplicate of this bug. ***
Comment 12 Stefan Dirsch 2007-11-04 09:24:34 UTC
Not sure, if this isn't already committed in a similar way.
Comment 13 Berk Akinci 2007-12-17 12:52:23 UTC
(In reply to comment #9)

I would like to see Ajax's change (or some variant of it) implemented soon.  The current implementation limiting max_pll_freq is breaking the code that configures the PLL, forcing it to default to PLL post-divider=1.  This causes the PLL to run very far outside of its specifications.  See these messages from Xorg.0.log:
-------------------------------
1024x768       65.00  1024 1048 1184 1344   768  771  777  806 (24,32) -H -V
(**) RADEON(0): Pitch = 14155992 bytes (virtualX = 1024, displayWidth = 1728)
(**) RADEON(0): dc=6500, of=6500, fd=29, pd=1
-------------------------------
Note that the VCO output frequency is set to 65MHz even though the VBIOS specifies PLL VCO range to be 200MHz-500MHz.

Even though the PLL can limp along in most cases, it's going to break in some.

Ajax was concerned about limiting the max clock not being sufficient.  I think limiting max dot clock is a reasonable thing to do.  I doubt that the RN50 is 'cache'ing the framebuffer, so it's still going to have instantaneous bandwidth demands on memory that correlates linearly with MaxClock.  (I don't know the guts of the chip, but I'm guessing from an EE point of view.)

In addition, there is the 2D acceleration engine that will use up memory bandwidth too.  I don't think that's something we can quantify easily, so we may have to just leave some headroom when (if?) 2D acceleration is used.  I believe the effect is visible when moving a large window around quickly as lines trailing to the right of the window that's being moved.  These visible artifacts I observed were transient, so no big harm.

I would like to see the max_pll_freq limiting logic removed.  In its place, I would like Ajax's patch (or a variant) applied.
Comment 14 Stefan Dirsch 2008-11-22 14:14:43 UTC
Alex, Cooper Yuan (AMD) is currently working on ES1000 support for xf86-video-ati 6.9.0.
Comment 15 Berk Akinci 2008-11-24 08:00:01 UTC
(In reply to comment #14)
> Alex, Cooper Yuan (AMD) is currently working on ES1000 support for
> xf86-video-ati 6.9.0.

Someone please add Cooper Yuan in the CC list of this bug.

This bug needs attention.  I believe the work is done; it's just a matter of cleaning up and committing.  Currently, I believe every ES1000 chip out in the world running Xorg is running very far out of its PLL spec.  Some are (as we have seen) failing.  See comment #13.

Thanks,
Berk
Comment 16 Alex Deucher 2008-11-24 10:18:43 UTC
I believe this was fixed last year in:
fb7a4e24f2da3561ef81371ca4013a4f13806e91


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.