[PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)

[PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)

[Marcelo Tosatti]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 411 bytes --]


Ani,

I'm sending your patch to lkml CC benh.

All changes should be reviewed publically.


---------- Forwarded message ----------
Date: Tue, 15 Jul 2003 13:27:00 -0500 (CDT)
From: ajoshi@kernel.crashing.org
To: marcelo@conectiva.com.br
Subject: [PATCH] radeonfb 0.1.9 against 2.4.21pre2


Marcelo,

Well now that the dust seems to have settled, attatched is a patch against
2.4.21p2 for radeonfb 0.1.9.


ani

[-- Attachment #2: Type: APPLICATION/OCTET-STREAM, Size: 24979 bytes --]

Re: [PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)

[Benjamin Herrenschmidt]

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
memory

 - 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
radeonfb_blank

 - 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... 

Ben.

Re: [PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)

[ajoshi]

Many of these have already been addressed in 0.1.9, though I added the
usage of the native clock, assertion for it, the dvi blanking, and the
nolcd passthrough.  For things like the updated PM code, a patch would be
helpful.


ani



On 16 Jul 2003, Benjamin Herrenschmidt wrote:

> 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
> memory
> 
>  - 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
> radeonfb_blank
> 
>  - 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... 
> 
> Ben.
> 

Re: [PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)

[Marcelo Tosatti]

On Wed, 16 Jul 2003 ajoshi@kernel.crashing.org wrote:

>
>
> Many of these have already been addressed in 0.1.9,

He is talking about 0.1.9. The issues he complained about are present in
0.1.9. I havent read one line of code, but I know him enough to know he's
has no reason to lie or come up with non-existant bugs.

Please resend me a patch when you really have addressed the issues
Benjamin pointed out (0.1.10 or whatever).

For now I'll stick to 0.1.8 + his fixes.

> though I added the usage of the native clock, assertion for it, the dvi
> blanking, and the nolcd passthrough.  For things like the updated PM
> code, a patch would be helpful.

Re: [PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)

[ajoshi]

Haha, ok this is geting ridiculous, it seems the childish games are still
being played.  I don't have time for this, Ben you can find a maintainer
for the driver or do it yourself.


On Wed, 16 Jul 2003, Marcelo Tosatti wrote:

> 
> On Wed, 16 Jul 2003 ajoshi@kernel.crashing.org wrote:
> 
> >
> >
> > Many of these have already been addressed in 0.1.9,
> 
> He is talking about 0.1.9. The issues he complained about are present in
> 0.1.9. I havent read one line of code, but I know him enough to know he's
> has no reason to lie or come up with non-existant bugs.
> 
> Please resend me a patch when you really have addressed the issues
> Benjamin pointed out (0.1.10 or whatever).
> 
> For now I'll stick to 0.1.8 + his fixes.
> 
> > though I added the usage of the native clock, assertion for it, the dvi
> > blanking, and the nolcd passthrough.  For things like the updated PM
> > code, a patch would be helpful.
> 

Re: [PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)

[Marcelo Tosatti]

On Wed, 16 Jul 2003 ajoshi@kernel.crashing.org wrote:

>
>
> Haha, ok this is geting ridiculous, it seems the childish games are still
> being played.  I don't have time for this, Ben you can find a maintainer
> for the driver or do it yourself.

Fine.

Ben are you willing to maintain radeonfb ?

Re: [PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)

[Benjamin Herrenschmidt]

On Wed, 2003-07-16 at 20:10, ajoshi@kernel.crashing.org wrote:
> Many of these have already been addressed in 0.1.9, though I added the
> usage of the native clock, assertion for it, the dvi blanking, and the
> nolcd passthrough.  For things like the updated PM code, a patch would be
> helpful.

Ok, I don't know what's up here, maybe you send the wrong patch
to Marcelo or whatever, but what I have here labelled 0.1.9 doesn't
contain these. Native clock for LCD is definitely not here, DVI
blanking neither, though nolcd passthrough is there.

Ben.

Re: [PATCH] radeonfb 0.1.9 against 2.4.21pre2 (fwd)

[Benjamin Herrenschmidt]

On Wed, 2003-07-16 at 20:28, ajoshi@kernel.crashing.org wrote:
> Haha, ok this is geting ridiculous, it seems the childish games are still
> being played.  I don't have time for this, Ben you can find a maintainer
> for the driver or do it yourself.

if you say so...

Marcelo, I can take over for now, I'd prefer someone with more time
as the 2.6 version really need a total rewrite that I don't have time
to do for now, so if there's any candidate, please speak up.

Ben.