On Mon, Jul 01, 2002 at 10:48:41AM -0700, James Simmons wrote:
Here are the latest updates to the framebuffer layer. Please test it outso
I can push it as soon as possible to Linus.
The following comments are from a first review of sa1100fb.[ch] changes.
1. It would appear to break sa1100 inverse colourmap stuff. There are LCD
panels out there where it is a rather fundamental requirement to write
the palette with "inverted" colourmap values.
2. I've no idea why you moved "lccr0" and "lccr3" in sa1100fb.h - this
looks like noise to me.
3. I think you replaced "fbi" too many times:
+/* Fake monspecs to fill in infonfo structure */
4. You're also merging in cpufreq changes from my tree in this patch;
however I was going to send these to Linus along with the cpufreq
submission so no problem.
5. I strongly disagree with your apparant decision to make the cpufreq
part of the generic framebuffer core (by apparantly adding the notifier
block to the core fb_info structure). Firstly, you've broken sa1100fb.c
by not including the relevant definition in fb_info (ok, so cpufreq
stuff isn't in Linus' tree yet). Secondly, it isn't something that all
framebuffers require; its only required on SoC devices where the hardware
designers have been stingy. As such, we should NOT penalise the x86
people by adding random useless garbage to structures that they're never
going to use.
--
Russell King (rmk@arm.linux.org.uk) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
No, I will not fix your computer.
http://thinkgeek.com/sf
1. It would appear to break sa1100 inverse colourmap stuff. There are LCD
panels out there where it is a rather fundamental requirement to write
the palette with "inverted" colourmap values.
Okay. Fixed.
2. I've no idea why you moved "lccr0" and "lccr3" in sa1100fb.h - this
looks like noise to me.
Ah the idea of a par and using generic struct fb_info. A few reasons. One
I noticed people having fields inside their struct xxxfb_info that was
already there in the generic struct fb_info. By making a strick rule I
hope to avoid that ugly mess. The second reason is eventually I like to
combine DRI and the fbdev layer. This was both interfaces could use the
same struct xxx_par. That is a 2.7 thing but I like to prepare now for
this. For the SA1100 this is not really needed but I still like to enforce
this rule and we still can take advantage of the nice generic functions in
fbgen.c.
3. I think you replaced "fbi" too many times:
+/* Fake monspecs to fill in infonfo structure */
Fixed.
4. You're also merging in cpufreq changes from my tree in this patch;
however I was going to send these to Linus along with the cpufreq
submission so no problem.
Just what was in sa1100fb.c. I grabbed that version of the driver from
your tree sinc ethe stuff in Linus tree didn't compile at the time.
5. I strongly disagree with your apparant decision to make the cpufreq
part of the generic framebuffer core (by apparantly adding the notifier
block to the core fb_info structure). Firstly, you've broken sa1100fb.c
by not including the relevant definition in fb_info (ok, so cpufreq
stuff isn't in Linus' tree yet). Secondly, it isn't something that all
framebuffers require; its only required on SoC devices where the hardware
designers have been stingy. As such, we should NOT penalise the x86
people by adding random useless garbage to structures that they're never
going to use.
I completely agree. I'm going to remove that. Originally I thought about
making it generic for everyone because I have seen other platforms
(Mips Au1000 with Epson 1385 framebuffers) do something similar. Then I
realized not everyone needs it and also it is possible that devices with
more than one framebuffer might use the same pixclock frequency. Dumb but
I have seen alot of cards share the accel engine and/or CRTC registers
between two different framebuffers on the same piece of hardware. So it
belongs in par.
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Caffeinated soap. No kidding.
http://thinkgeek.com/sf