archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <>
To: Marcelo Tosatti <>
Cc: lkml <>,
Subject: Re: [PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)
Date: 16 Jul 2003 16:18:06 +0200	[thread overview]
Message-ID: <1058365086.515.89.camel@gaston> (raw)
In-Reply-To: <Pine.LNX.4.55L.0307161003250.31785@freak.distro.conectiva>

On Wed, 2003-07-16 at 15:05, Marcelo Tosatti wrote:
> Ani,
> I'm sending your patch to lkml CC benh.
> All changes should be reviewed publically.

hrm... ok, here are my complaints against 0.1.9:

 - rinfo->name shall not be a pointer to an entry stored in a
__devinitdata array, or get_fix() would possibly access freed

 - you didn't take my code for obtaining the DFP info from the
BIOS. The current code works only with some flat panels (laptops
mostly in my understanding). Also, I defaulted to syncs active
high when using the "old" code, that fixed some laptops and is
what XFree does

 - On LCDs you should use the LCD native pixel clock when using
the RMX engine, not the mode specified one. Without this, scaled
mode in XFree won't work properly among others on a lot of panels

 - The workaround when setting PLL registers helped some laptop
users (not writing them when they don't change)

 - I added some sanity checking to the clocks we obtain from
the BIOS pll block, especially since we now have relaxed algorithm
to find the BIOS, that makes sense

 - If you fail to retreive BIOS infos, rinfo->clock can be 0 and
you divide by 0 when setting the mode

 - After much discussions with some other hackers, I decided to
keep the pitch to match exactly xres when accel is not enabled for
various old apps that can't deal with a line lenght beeing different,
you didn't take that code, so 0.1.9 won't work with those apps.

 - I added some code that helps blanking on some DVI panels in

 - When force_nolcd is set, I want to still call getmoninfo,
not break (in radeonfb_pci_register())

 - There are more cases in radeonfb_switch where you want to
reprogram the mode. Typically, accel flag changes, 15/16 bits
changes, etc... Also, I worked around some XFree bugs by
always re-initing the engine. It's not perfect yet, hopefully
X will be fixed sooner or later, though I had no time to
investigate that much yet.

 - I don't like the "workaround" disabling dynamic clocks in
XFree, the code ATI gave me for M6, M7 and M9 on Apple HW enabled
dynamic clocks, so I suspect those work properly, and I want the
power saving. I suppose we shall discuss that with ATI to find out
a better workaround

 - The "PM" code isn't up to date (cleanups & small fixes I did)

 - Not important, but inconsistent use of tab/space (you basically
replace all tabs by spaces in code you touched) can be painful ;)

I may have missed one or two though... 


  reply	other threads:[~2003-07-16 14:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-07-16 13:05 [PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd) Marcelo Tosatti
2003-07-16 14:18 ` Benjamin Herrenschmidt [this message]
2003-07-16 18:10   ` ajoshi
2003-07-16 18:17     ` Marcelo Tosatti
2003-07-16 18:28       ` ajoshi
2003-07-16 18:31         ` Marcelo Tosatti
2003-07-16 20:18         ` Benjamin Herrenschmidt
2003-07-16 20:17     ` Benjamin Herrenschmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1058365086.515.89.camel@gaston \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).