On Thu, Jul 04, 2002 at 09:05:40PM -0700, James Simmons wrote:
quoted
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.
First, I detest the idea of "fix", "var", "par" and "info". Specifically
the "par" crap. Intensely. "par" and "info" should be combined IMO,
which my framebuffer drivers do.
Secondly, I think you're completely confused above. lccr0 and lccr3 have
nothing to do with some "generic struct fb_info". They hold the base
register values for two of the SA1100 control registers.
Thirdly, you didn't delete them. You _moved_ them within the structure.
They therefore served zero functional purpose.
Fourthly, nothing but the sa1100fb driver has any business accessing the
elements around these two both before and after the move.
In total, the change serves ZERO purpose and is therefore noise.
quoted
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.
Ehh? That's got nothing to do with cpufreq. The reason we have the
cpufreq interface in sa1100fb is that the base clock rate for the LCD
controller on this chip is derived from the CPU core clock. You change
the core clock, you have to reprogram the pixel clock divisor.
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.
Again, I think this is another misplaced reason; that's irrelevant to
cpufreq.
--
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
Bringing you mounds of caffeinated joy.
http://thinkgeek.com/sf
On Fri, 5 Jul 2002, Russell King wrote:
On Thu, Jul 04, 2002 at 09:05:40PM -0700, James Simmons wrote:
quoted
quoted
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.
First, I detest the idea of "fix", "var", "par" and "info". Specifically
the "par" crap. Intensely. "par" and "info" should be combined IMO,
which my framebuffer drivers do.
If you have an `asymmetric' dual-head chipset (both heads are not independent,
and/or have different capabilities), you'll have 2 info structures, with one
shared par. So info and par cannot be combined.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Bringing you mounds of caffeinated joy.
http://thinkgeek.com/sf
First, I detest the idea of "fix", "var", "par" and "info". Specifically
the "par" crap. Intensely.
Why? Should each driver have their own structs that are completely
different then? In this case you would be better off doing something like
newport_con.c.
We pretty much have this today. Each driver has so much excess code
because they want to create their own special structs. Lost of bloat for
no reason! I will NOT put up with that anymore!!!! Sorry!
"par" and "info" should be combined IMO,
which my framebuffer drivers do.
No!!!! This is one of the reasons we have the mess we have!! Look at how
much code that could be removed from the standard drivers into fbmem.c and
fbcon.c.
We should have
focus on
struct vc_data struct fb_info struct xxx_par
[ fbcon.c] [ fbmem.c] [ fbdev driver ]
This makes for a nice modular system. It allows for fbdev to exist without
fbcon. Ideally fbmem could even be modular. Insmod the core driver.
Insmod fbmem.o and you have the fbdev interface. You could then rmmod
fbmem.o and do dri.o. Now with the same core driver you have the DRI
interface. Or you could do both drivers at the same time. Eventually I
will combine both interfaces but that will take some time.
Secondly, I think you're completely confused above. lccr0 and lccr3 have
nothing to do with some "generic struct fb_info". They hold the base
register values for two of the SA1100 control registers.
Thirdly, you didn't delete them. You _moved_ them within the structure.
They therefore served zero functional purpose.
If you could send me a patch I would be happy.
Fourthly, nothing but the sa1100fb driver has any business accessing the
elements around these two both before and after the move.
True which is why things like that go into par.
Ehh? That's got nothing to do with cpufreq. The reason we have the
cpufreq interface in sa1100fb is that the base clock rate for the LCD
controller on this chip is derived from the CPU core clock. You change
the core clock, you have to reprogram the pixel clock divisor.
Other devices besides the SA1100 does this as well. I knew this and this
reason lead me to originally place it into struct fb_info. Later I
realized it was a bad idea.
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Bringing you mounds of caffeinated joy.
http://thinkgeek.com/sf