Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt a écrit : But it's not specific to X11; I've applied the patch you posted and the same symptoms occur for pure tty switching as well, the delay has decreased a bit (it's hard to measure, but around a second), but it's still rather annoying to work with. Is it distinguishable which M6 models are buggy? I'm using my X31 for about a year now and have probably made some tens of thousands of switches without lockups, so presumably not all models cause lockups. ATI hasn't been very precise about that unfortunately... In case your interested, I tried your last patch (http://lkml.org/lkml/2005/4/7/308) on top of 2.6.12-rc2-mm2 on my Radeon Mobility M6 LY. It works great. Switching was at least one second long with plain mm2. Now it's almost immediat as in Linus' kernel. No more dirty colors during the switch from X to fbcon. Not annoying at all, here. Thanks, Brice - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
> But it's not specific to X11; I've applied the patch you posted and the > same symptoms occur for pure tty switching as well, the delay has decreased > a bit (it's hard to measure, but around a second), but it's still rather > annoying to work with. > > Is it distinguishable which M6 models are buggy? I'm using my X31 for about > a year now and have probably made some tens of thousands of switches without > lockups, so presumably not all models cause lockups. ATI hasn't been very precise about that unfortunately... Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt wrote: >> radeonfb_setcolreg: INPLL >> radeonfb_setcolreg: OUTPLL >> radeonfb_setcolreg: OUTPLL >> ... last three lines repeated 63 times > > Hrm... the last (serie of 64 setcolreg) are probably X beeing extremely > dumb, and calling the ioctl 64 times to set each palette entry instead > of doing a single call for the whole palette... > > Anyway. Except for maybe the double set-par on switch from X to console, > there isn't much more we can do here. We might be able to improve X but > there is a significant lag between a fix done to X.org HEAD appears in > any distro. The fact is, according to ATI, there is a HW bug on M6 taht > can cause lockups of the chip, and this 5ms workaround is necessary to > avoid it... But it's not specific to X11; I've applied the patch you posted and the same symptoms occur for pure tty switching as well, the delay has decreased a bit (it's hard to measure, but around a second), but it's still rather annoying to work with. Is it distinguishable which M6 models are buggy? I'm using my X31 for about a year now and have probably made some tens of thousands of switches without lockups, so presumably not all models cause lockups. Cheers, Moritz - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt wrote: radeonfb_setcolreg: INPLL radeonfb_setcolreg: OUTPLL radeonfb_setcolreg: OUTPLL ... last three lines repeated 63 times Hrm... the last (serie of 64 setcolreg) are probably X beeing extremely dumb, and calling the ioctl 64 times to set each palette entry instead of doing a single call for the whole palette... Anyway. Except for maybe the double set-par on switch from X to console, there isn't much more we can do here. We might be able to improve X but there is a significant lag between a fix done to X.org HEAD appears in any distro. The fact is, according to ATI, there is a HW bug on M6 taht can cause lockups of the chip, and this 5ms workaround is necessary to avoid it... But it's not specific to X11; I've applied the patch you posted and the same symptoms occur for pure tty switching as well, the delay has decreased a bit (it's hard to measure, but around a second), but it's still rather annoying to work with. Is it distinguishable which M6 models are buggy? I'm using my X31 for about a year now and have probably made some tens of thousands of switches without lockups, so presumably not all models cause lockups. Cheers, Moritz - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
But it's not specific to X11; I've applied the patch you posted and the same symptoms occur for pure tty switching as well, the delay has decreased a bit (it's hard to measure, but around a second), but it's still rather annoying to work with. Is it distinguishable which M6 models are buggy? I'm using my X31 for about a year now and have probably made some tens of thousands of switches without lockups, so presumably not all models cause lockups. ATI hasn't been very precise about that unfortunately... Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt a écrit : But it's not specific to X11; I've applied the patch you posted and the same symptoms occur for pure tty switching as well, the delay has decreased a bit (it's hard to measure, but around a second), but it's still rather annoying to work with. Is it distinguishable which M6 models are buggy? I'm using my X31 for about a year now and have probably made some tens of thousands of switches without lockups, so presumably not all models cause lockups. ATI hasn't been very precise about that unfortunately... In case your interested, I tried your last patch (http://lkml.org/lkml/2005/4/7/308) on top of 2.6.12-rc2-mm2 on my Radeon Mobility M6 LY. It works great. Switching was at least one second long with plain mm2. Now it's almost immediat as in Linus' kernel. No more dirty colors during the switch from X to fbcon. Not annoying at all, here. Thanks, Brice - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Sat, 2005-04-09 at 18:24 +0200, Andreas Schwab wrote: > Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > > > Can you redo the counting of the workarounds with the patch ? > > Switching from X to console: > > radeon_write_pll_regs: INPLL > radeon_write_pll_regs: INPLL > radeon_write_mode: OUTPLL > radeonfb_engine_reset: INPLL > radeonfb_engine_reset: OUTPLL > radeonfb_engine_reset: OUTPLL > radeonfb_setcmap: INPLL > radeonfb_setcmap: OUTPLL > radeonfb_setcmap: OUTPLL > radeon_write_pll_regs: INPLL > radeon_write_pll_regs: INPLL > radeon_write_mode: OUTPLL > radeonfb_engine_reset: INPLL > radeonfb_engine_reset: OUTPLL > radeonfb_engine_reset: OUTPLL > radeonfb_setcmap: INPLL > radeonfb_setcmap: OUTPLL > radeonfb_setcmap: OUTPLL > radeonfb_setcmap: INPLL > radeonfb_setcmap: OUTPLL > radeonfb_setcmap: OUTPLL > radeonfb_setcmap: INPLL > radeonfb_setcmap: OUTPLL > radeonfb_setcmap: OUTPLL > radeonfb_setcmap: INPLL > radeonfb_setcmap: OUTPLL Ok, so the above is interesting, something is callign setcmap way too much here I would say... Besides, it looks like set_par is called twice, which is wrong too. > Switching from console to X: > > radeonfb_setcmap: OUTPLL > radeon_write_pll_regs: INPLL > radeon_write_pll_regs: INPLL > radeon_write_mode: OUTPLL > radeonfb_engine_reset: INPLL > radeonfb_engine_reset: OUTPLL > radeonfb_engine_reset: OUTPLL > radeonfb_setcmap: INPLL > radeonfb_setcmap: OUTPLL > radeonfb_setcmap: OUTPLL > agpgart: Putting AGP V2 device at :00:0b.0 into 1x mode > agpgart: Putting AGP V2 device at :00:10.0 into 1x mode > radeonfb_setcolreg: INPLL > radeonfb_setcolreg: OUTPLL > radeonfb_setcolreg: OUTPLL > ... last three lines repeated 63 times Hrm... the last (serie of 64 setcolreg) are probably X beeing extremely dumb, and calling the ioctl 64 times to set each palette entry instead of doing a single call for the whole palette... Anyway. Except for maybe the double set-par on switch from X to console, there isn't much more we can do here. We might be able to improve X but there is a significant lag between a fix done to X.org HEAD appears in any distro. The fact is, according to ATI, there is a HW bug on M6 taht can cause lockups of the chip, and this 5ms workaround is necessary to avoid it... Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > Can you redo the counting of the workarounds with the patch ? Switching from X to console: radeon_write_pll_regs: INPLL radeon_write_pll_regs: INPLL radeon_write_mode: OUTPLL radeonfb_engine_reset: INPLL radeonfb_engine_reset: OUTPLL radeonfb_engine_reset: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeon_write_pll_regs: INPLL radeon_write_pll_regs: INPLL radeon_write_mode: OUTPLL radeonfb_engine_reset: INPLL radeonfb_engine_reset: OUTPLL radeonfb_engine_reset: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL Switching from console to X: radeonfb_setcmap: OUTPLL radeon_write_pll_regs: INPLL radeon_write_pll_regs: INPLL radeon_write_mode: OUTPLL radeonfb_engine_reset: INPLL radeonfb_engine_reset: OUTPLL radeonfb_engine_reset: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL agpgart: Putting AGP V2 device at :00:0b.0 into 1x mode agpgart: Putting AGP V2 device at :00:10.0 into 1x mode radeonfb_setcolreg: INPLL radeonfb_setcolreg: OUTPLL radeonfb_setcolreg: OUTPLL ... last three lines repeated 63 times Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Can you redo the counting of the workarounds with the patch ? Switching from X to console: radeon_write_pll_regs: INPLL radeon_write_pll_regs: INPLL radeon_write_mode: OUTPLL radeonfb_engine_reset: INPLL radeonfb_engine_reset: OUTPLL radeonfb_engine_reset: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeon_write_pll_regs: INPLL radeon_write_pll_regs: INPLL radeon_write_mode: OUTPLL radeonfb_engine_reset: INPLL radeonfb_engine_reset: OUTPLL radeonfb_engine_reset: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL Switching from console to X: radeonfb_setcmap: OUTPLL radeon_write_pll_regs: INPLL radeon_write_pll_regs: INPLL radeon_write_mode: OUTPLL radeonfb_engine_reset: INPLL radeonfb_engine_reset: OUTPLL radeonfb_engine_reset: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL agpgart: Putting AGP V2 device at :00:0b.0 into 1x mode agpgart: Putting AGP V2 device at :00:10.0 into 1x mode radeonfb_setcolreg: INPLL radeonfb_setcolreg: OUTPLL radeonfb_setcolreg: OUTPLL ... last three lines repeated 63 times Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Sat, 2005-04-09 at 18:24 +0200, Andreas Schwab wrote: Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Can you redo the counting of the workarounds with the patch ? Switching from X to console: radeon_write_pll_regs: INPLL radeon_write_pll_regs: INPLL radeon_write_mode: OUTPLL radeonfb_engine_reset: INPLL radeonfb_engine_reset: OUTPLL radeonfb_engine_reset: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeon_write_pll_regs: INPLL radeon_write_pll_regs: INPLL radeon_write_mode: OUTPLL radeonfb_engine_reset: INPLL radeonfb_engine_reset: OUTPLL radeonfb_engine_reset: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL Ok, so the above is interesting, something is callign setcmap way too much here I would say... Besides, it looks like set_par is called twice, which is wrong too. Switching from console to X: radeonfb_setcmap: OUTPLL radeon_write_pll_regs: INPLL radeon_write_pll_regs: INPLL radeon_write_mode: OUTPLL radeonfb_engine_reset: INPLL radeonfb_engine_reset: OUTPLL radeonfb_engine_reset: OUTPLL radeonfb_setcmap: INPLL radeonfb_setcmap: OUTPLL radeonfb_setcmap: OUTPLL agpgart: Putting AGP V2 device at :00:0b.0 into 1x mode agpgart: Putting AGP V2 device at :00:10.0 into 1x mode radeonfb_setcolreg: INPLL radeonfb_setcolreg: OUTPLL radeonfb_setcolreg: OUTPLL ... last three lines repeated 63 times Hrm... the last (serie of 64 setcolreg) are probably X beeing extremely dumb, and calling the ioctl 64 times to set each palette entry instead of doing a single call for the whole palette... Anyway. Except for maybe the double set-par on switch from X to console, there isn't much more we can do here. We might be able to improve X but there is a significant lag between a fix done to X.org HEAD appears in any distro. The fact is, according to ATI, there is a HW bug on M6 taht can cause lockups of the chip, and this 5ms workaround is necessary to avoid it... Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
> > This patch adds to the fbdev interface a set_cmap callback that allow > > the driver to "batch" palette changes. This is useful for drivers like > > radeonfb which might require lenghtly workarounds on palette accesses, > > thus allowing to factor out those workarounds efficiently. > > This makes it better. But there is still a delay of half a second, and > there is an annoying flicker when switching from X to the console. Can you redo the counting of the workarounds with the patch ? Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > On Fri, 2005-04-08 at 01:58 +0200, Andreas Schwab wrote: >> Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: >> >> > Yes, that's very extreme, I suspect somebody is banging on set_par or >> > something like that. >> >> fb_setcolreg is it. > > Ok, what about that patch: > > --- > > This patch adds to the fbdev interface a set_cmap callback that allow > the driver to "batch" palette changes. This is useful for drivers like > radeonfb which might require lenghtly workarounds on palette accesses, > thus allowing to factor out those workarounds efficiently. This makes it better. But there is still a delay of half a second, and there is an annoying flicker when switching from X to the console. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt [EMAIL PROTECTED] writes: On Fri, 2005-04-08 at 01:58 +0200, Andreas Schwab wrote: Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Yes, that's very extreme, I suspect somebody is banging on set_par or something like that. fb_setcolreg is it. Ok, what about that patch: --- This patch adds to the fbdev interface a set_cmap callback that allow the driver to batch palette changes. This is useful for drivers like radeonfb which might require lenghtly workarounds on palette accesses, thus allowing to factor out those workarounds efficiently. This makes it better. But there is still a delay of half a second, and there is an annoying flicker when switching from X to the console. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
This patch adds to the fbdev interface a set_cmap callback that allow the driver to batch palette changes. This is useful for drivers like radeonfb which might require lenghtly workarounds on palette accesses, thus allowing to factor out those workarounds efficiently. This makes it better. But there is still a delay of half a second, and there is an annoying flicker when switching from X to the console. Can you redo the counting of the workarounds with the patch ? Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-04-08 at 01:58 +0200, Andreas Schwab wrote: > Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > > > Yes, that's very extreme, I suspect somebody is banging on set_par or > > something like that. > > fb_setcolreg is it. Ok, what about that patch: --- This patch adds to the fbdev interface a set_cmap callback that allow the driver to "batch" palette changes. This is useful for drivers like radeonfb which might require lenghtly workarounds on palette accesses, thus allowing to factor out those workarounds efficiently. Signed-off-by: Benjamin Herrenschmidt <[EMAIL PROTECTED]> Index: linux-work/include/linux/fb.h === --- linux-work.orig/include/linux/fb.h 2005-04-01 09:04:19.0 +1000 +++ linux-work/include/linux/fb.h 2005-04-08 10:24:56.0 +1000 @@ -563,6 +563,9 @@ int (*fb_setcolreg)(unsigned regno, unsigned red, unsigned green, unsigned blue, unsigned transp, struct fb_info *info); + /* set color registers in batch */ + int (*fb_setcmap)(struct fb_cmap *cmap, struct fb_info *info); + /* blank display */ int (*fb_blank)(int blank, struct fb_info *info); Index: linux-work/drivers/video/aty/radeon_base.c === --- linux-work.orig/drivers/video/aty/radeon_base.c 2005-04-01 09:04:18.0 +1000 +++ linux-work/drivers/video/aty/radeon_base.c 2005-04-08 11:15:13.0 +1000 @@ -1057,13 +1057,14 @@ return radeon_screen_blank(rinfo, blank, 0); } -static int radeonfb_setcolreg (unsigned regno, unsigned red, unsigned green, - unsigned blue, unsigned transp, struct fb_info *info) +static int radeon_setcolreg (unsigned regno, unsigned red, unsigned green, + unsigned blue, unsigned transp, +struct radeonfb_info *rinfo) { -struct radeonfb_info *rinfo = info->par; u32 pindex; unsigned int i; - + + if (regno > 255) return 1; @@ -1078,20 +1079,7 @@ pindex = regno; if (!rinfo->asleep) { - u32 dac_cntl2, vclk_cntl = 0; - radeon_fifo_wait(9); - if (rinfo->is_mobility) { - vclk_cntl = INPLL(VCLK_ECP_CNTL); - OUTPLL(VCLK_ECP_CNTL, vclk_cntl & ~PIXCLK_DAC_ALWAYS_ONb); - } - - /* Make sure we are on first palette */ - if (rinfo->has_CRTC2) { - dac_cntl2 = INREG(DAC_CNTL2); - dac_cntl2 &= ~DAC2_PALETTE_ACCESS_CNTL; - OUTREG(DAC_CNTL2, dac_cntl2); - } if (rinfo->bpp == 16) { pindex = regno * 8; @@ -1101,24 +1089,27 @@ if (rinfo->depth == 15 && regno > 31) return 1; - /* For 565, the green component is mixed one order below */ + /* For 565, the green component is mixed one order +* below +*/ if (rinfo->depth == 16) { OUTREG(PALETTE_INDEX, pindex>>1); - OUTREG(PALETTE_DATA, (rinfo->palette[regno>>1].red << 16) | - (green << 8) | (rinfo->palette[regno>>1].blue)); + OUTREG(PALETTE_DATA, + (rinfo->palette[regno>>1].red << 16) | + (green << 8) | + (rinfo->palette[regno>>1].blue)); green = rinfo->palette[regno<<1].green; } } if (rinfo->depth != 16 || regno < 32) { OUTREG(PALETTE_INDEX, pindex); - OUTREG(PALETTE_DATA, (red << 16) | (green << 8) | blue); + OUTREG(PALETTE_DATA, (red << 16) | + (green << 8) | blue); } - if (rinfo->is_mobility) - OUTPLL(VCLK_ECP_CNTL, vclk_cntl); } if (regno < 16) { - u32 *pal = info->pseudo_palette; + u32 *pal = rinfo->info->pseudo_palette; switch (rinfo->depth) { case 15: pal[regno] = (regno << 10) | (regno << 5) | regno; @@ -1138,6 +1129,84 @@ return 0; } +static int radeonfb_setcolreg (unsigned regno, unsigned red, unsigned green, + unsigned blue, unsigned transp, + struct fb_info *info) +{ +struct radeonfb_info *rinfo = info->par; + u32 dac_cntl2, vclk_cntl = 0; + int rc; + +if
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-04-08 at 01:58 +0200, Andreas Schwab wrote: > Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > > > Yes, that's very extreme, I suspect somebody is banging on set_par or > > something like that. > > fb_setcolreg is it. Ahhh... interesting. I'll see if I can find a way to work around that one. Best would be to "cache" the current PLL register index in fact, but I'm afraid that may not work terribly well with userland apps hacking it ... Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > Yes, that's very extreme, I suspect somebody is banging on set_par or > something like that. fb_setcolreg is it. kernel: radeonfb_set_par kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_mode: radeon_pll_errata_after_data kernel: radeonfb_engine_reset: radeon_pll_errata_after_data last message repeated 2 times kernel: radeonfb_setcolreg: radeon_pll_errata_after_data last message repeated 767 times kernel: radeonfb_set_par kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_mode: radeon_pll_errata_after_data kernel: radeonfb_engine_reset: radeon_pll_errata_after_data last message repeated 2 times kernel: radeonfb_setcolreg: radeon_pll_errata_after_data last message repeated 911 times kernel: radeonfb_set_par kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_mode: radeon_pll_errata_after_data kernel: radeonfb_engine_reset: radeon_pll_errata_after_data last message repeated 2 times kernel: radeonfb_setcolreg: radeon_pll_errata_after_data last message repeated 193 times kernel: agpgart: Putting AGP V2 device at :00:0b.0 into 1x mode kernel: agpgart: Putting AGP V2 device at :00:10.0 into 1x mode kernel: radeonfb_setcolreg: radeon_pll_errata_after_data last message repeated 191 times Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-04-08 at 01:13 +0200, Andreas Schwab wrote: > Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > > > Can you cound how many times radeonfb_set_par is called and dump your > > "counter" at the beginning and end of each of these calls ? > > Switch from X to console: > > kernel: radeonfb_set_par > kernel: radeon_pll_errata_after_data > last message repeated 774 times > kernel: radeonfb_set_par > kernel: radeon_pll_errata_after_data > last message repeated 918 times Ok, so somebody is calling set_par twice ... I suppose I know why but it's not a very nice thing to do. Still, it doesn't explain why there are so many calls to the errata. Please read my other email and try to figure out where those big numbers come from... Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > Can you cound how many times radeonfb_set_par is called and dump your > "counter" at the beginning and end of each of these calls ? Switch from X to console: kernel: radeonfb_set_par kernel: radeon_pll_errata_after_data last message repeated 774 times kernel: radeonfb_set_par kernel: radeon_pll_errata_after_data last message repeated 918 times Switch from console to X: kernel: radeonfb_set_par kernel: radeon_pll_errata_after_data last message repeated 200 times kernel: agpgart: Putting AGP V2 device at :00:0b.0 into 1x mode kernel: agpgart: Putting AGP V2 device at :00:10.0 into 1x mode kernel: radeon_pll_errata_after_data last message repeated 191 times Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-04-08 at 07:22 +1000, Dave Airlie wrote: > > There are 1694 calls to radeon_pll_errata_after_data during a switch from > > X to the console and 393 calls the other way. > > Wow... Ben that seems a bit extreme... there's not even close to 393 plls :-) Yes, that's very extreme, I suspect somebody is banging on set_par or something like that. Let me count a normal set_par operation: - blank. That can do INPLL, OUTPLLP and OUTPLL on MT_LCD, that is 4 calls to the errata (OUTPLLP has two). - write_pll_regs which does * on mobility chips: 2x INPLL to test the PLL value, and if it matches, just writes the PLL selector with a call to the errata, that is only 3 calls to the errata. Can you check we actually get in that case ? Normally, on the internal LCD, we should never change the PLL registers, or only one (they should stay the same all the time after that) and thus we should get into this case. If not (CRT), indeed, we end up doing more accesses: * OUTPLLP (2), OUTPLLP (2), manual errata (1), OUTPLLP (2), OUTPLLP (2), OUTPLLP (2), an INPLL loop (hrm...), OUTPLLP (2), another INPLL loop, OUTPLL (1), OUTPLLP (2), OUTPLLP (2). That is 18 calls to the errata plus the 2 INPLL loops. It would be useful to instrument those loops and see what happens there, but I don't see why they would have any impact unless something wrong is going on there with the PLL locking... * One last call to OUTPLLP (2). - reset the engine, that is 3 calls to the errata So that means that one call to raeonfb_set_par() should be in the normal internal flat panel case about 12 calls to the errata, and in the case where we actually write the PLL registers, about 29, plus the ones called by the INPLL loops waiting for the PLL to lock. As you can see, we are far below the measured counts. So I would need more instrumentation of the driver to figure out what's going on there. Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Thu, 2005-04-07 at 22:21 +0200, Andreas Schwab wrote: > Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > > > It is weird tho. Could you try adding a printk or something to figure > > out how much this is called during a typical swich ? > > There are 1694 calls to radeon_pll_errata_after_data during a switch from > X to the console and 393 calls the other way. Wow ! That is plain wrong ! Can you cound how many times radeonfb_set_par is called and dump your "counter" at the beginning and end of each of these calls ? Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
> There are 1694 calls to radeon_pll_errata_after_data during a switch from > X to the console and 393 calls the other way. Wow... Ben that seems a bit extreme... there's not even close to 393 plls :-) Dave. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > It is weird tho. Could you try adding a printk or something to figure > out how much this is called during a typical swich ? There are 1694 calls to radeon_pll_errata_after_data during a switch from X to the console and 393 calls the other way. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt [EMAIL PROTECTED] writes: It is weird tho. Could you try adding a printk or something to figure out how much this is called during a typical swich ? There are 1694 calls to radeon_pll_errata_after_data during a switch from X to the console and 393 calls the other way. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
There are 1694 calls to radeon_pll_errata_after_data during a switch from X to the console and 393 calls the other way. Wow... Ben that seems a bit extreme... there's not even close to 393 plls :-) Dave. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Thu, 2005-04-07 at 22:21 +0200, Andreas Schwab wrote: Benjamin Herrenschmidt [EMAIL PROTECTED] writes: It is weird tho. Could you try adding a printk or something to figure out how much this is called during a typical swich ? There are 1694 calls to radeon_pll_errata_after_data during a switch from X to the console and 393 calls the other way. Wow ! That is plain wrong ! Can you cound how many times radeonfb_set_par is called and dump your counter at the beginning and end of each of these calls ? Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-04-08 at 07:22 +1000, Dave Airlie wrote: There are 1694 calls to radeon_pll_errata_after_data during a switch from X to the console and 393 calls the other way. Wow... Ben that seems a bit extreme... there's not even close to 393 plls :-) Yes, that's very extreme, I suspect somebody is banging on set_par or something like that. Let me count a normal set_par operation: - blank. That can do INPLL, OUTPLLP and OUTPLL on MT_LCD, that is 4 calls to the errata (OUTPLLP has two). - write_pll_regs which does * on mobility chips: 2x INPLL to test the PLL value, and if it matches, just writes the PLL selector with a call to the errata, that is only 3 calls to the errata. Can you check we actually get in that case ? Normally, on the internal LCD, we should never change the PLL registers, or only one (they should stay the same all the time after that) and thus we should get into this case. If not (CRT), indeed, we end up doing more accesses: * OUTPLLP (2), OUTPLLP (2), manual errata (1), OUTPLLP (2), OUTPLLP (2), OUTPLLP (2), an INPLL loop (hrm...), OUTPLLP (2), another INPLL loop, OUTPLL (1), OUTPLLP (2), OUTPLLP (2). That is 18 calls to the errata plus the 2 INPLL loops. It would be useful to instrument those loops and see what happens there, but I don't see why they would have any impact unless something wrong is going on there with the PLL locking... * One last call to OUTPLLP (2). - reset the engine, that is 3 calls to the errata So that means that one call to raeonfb_set_par() should be in the normal internal flat panel case about 12 calls to the errata, and in the case where we actually write the PLL registers, about 29, plus the ones called by the INPLL loops waiting for the PLL to lock. As you can see, we are far below the measured counts. So I would need more instrumentation of the driver to figure out what's going on there. Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Can you cound how many times radeonfb_set_par is called and dump your counter at the beginning and end of each of these calls ? Switch from X to console: kernel: radeonfb_set_par kernel: radeon_pll_errata_after_data last message repeated 774 times kernel: radeonfb_set_par kernel: radeon_pll_errata_after_data last message repeated 918 times Switch from console to X: kernel: radeonfb_set_par kernel: radeon_pll_errata_after_data last message repeated 200 times kernel: agpgart: Putting AGP V2 device at :00:0b.0 into 1x mode kernel: agpgart: Putting AGP V2 device at :00:10.0 into 1x mode kernel: radeon_pll_errata_after_data last message repeated 191 times Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-04-08 at 01:13 +0200, Andreas Schwab wrote: Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Can you cound how many times radeonfb_set_par is called and dump your counter at the beginning and end of each of these calls ? Switch from X to console: kernel: radeonfb_set_par kernel: radeon_pll_errata_after_data last message repeated 774 times kernel: radeonfb_set_par kernel: radeon_pll_errata_after_data last message repeated 918 times Ok, so somebody is calling set_par twice ... I suppose I know why but it's not a very nice thing to do. Still, it doesn't explain why there are so many calls to the errata. Please read my other email and try to figure out where those big numbers come from... Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Yes, that's very extreme, I suspect somebody is banging on set_par or something like that. fb_setcolreg is it. kernel: radeonfb_set_par kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_mode: radeon_pll_errata_after_data kernel: radeonfb_engine_reset: radeon_pll_errata_after_data last message repeated 2 times kernel: radeonfb_setcolreg: radeon_pll_errata_after_data last message repeated 767 times kernel: radeonfb_set_par kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_mode: radeon_pll_errata_after_data kernel: radeonfb_engine_reset: radeon_pll_errata_after_data last message repeated 2 times kernel: radeonfb_setcolreg: radeon_pll_errata_after_data last message repeated 911 times kernel: radeonfb_set_par kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_pll_regs: radeon_pll_errata_after_data kernel: radeon_write_mode: radeon_pll_errata_after_data kernel: radeonfb_engine_reset: radeon_pll_errata_after_data last message repeated 2 times kernel: radeonfb_setcolreg: radeon_pll_errata_after_data last message repeated 193 times kernel: agpgart: Putting AGP V2 device at :00:0b.0 into 1x mode kernel: agpgart: Putting AGP V2 device at :00:10.0 into 1x mode kernel: radeonfb_setcolreg: radeon_pll_errata_after_data last message repeated 191 times Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-04-08 at 01:58 +0200, Andreas Schwab wrote: Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Yes, that's very extreme, I suspect somebody is banging on set_par or something like that. fb_setcolreg is it. Ahhh... interesting. I'll see if I can find a way to work around that one. Best would be to cache the current PLL register index in fact, but I'm afraid that may not work terribly well with userland apps hacking it ... Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-04-08 at 01:58 +0200, Andreas Schwab wrote: Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Yes, that's very extreme, I suspect somebody is banging on set_par or something like that. fb_setcolreg is it. Ok, what about that patch: --- This patch adds to the fbdev interface a set_cmap callback that allow the driver to batch palette changes. This is useful for drivers like radeonfb which might require lenghtly workarounds on palette accesses, thus allowing to factor out those workarounds efficiently. Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED] Index: linux-work/include/linux/fb.h === --- linux-work.orig/include/linux/fb.h 2005-04-01 09:04:19.0 +1000 +++ linux-work/include/linux/fb.h 2005-04-08 10:24:56.0 +1000 @@ -563,6 +563,9 @@ int (*fb_setcolreg)(unsigned regno, unsigned red, unsigned green, unsigned blue, unsigned transp, struct fb_info *info); + /* set color registers in batch */ + int (*fb_setcmap)(struct fb_cmap *cmap, struct fb_info *info); + /* blank display */ int (*fb_blank)(int blank, struct fb_info *info); Index: linux-work/drivers/video/aty/radeon_base.c === --- linux-work.orig/drivers/video/aty/radeon_base.c 2005-04-01 09:04:18.0 +1000 +++ linux-work/drivers/video/aty/radeon_base.c 2005-04-08 11:15:13.0 +1000 @@ -1057,13 +1057,14 @@ return radeon_screen_blank(rinfo, blank, 0); } -static int radeonfb_setcolreg (unsigned regno, unsigned red, unsigned green, - unsigned blue, unsigned transp, struct fb_info *info) +static int radeon_setcolreg (unsigned regno, unsigned red, unsigned green, + unsigned blue, unsigned transp, +struct radeonfb_info *rinfo) { -struct radeonfb_info *rinfo = info-par; u32 pindex; unsigned int i; - + + if (regno 255) return 1; @@ -1078,20 +1079,7 @@ pindex = regno; if (!rinfo-asleep) { - u32 dac_cntl2, vclk_cntl = 0; - radeon_fifo_wait(9); - if (rinfo-is_mobility) { - vclk_cntl = INPLL(VCLK_ECP_CNTL); - OUTPLL(VCLK_ECP_CNTL, vclk_cntl ~PIXCLK_DAC_ALWAYS_ONb); - } - - /* Make sure we are on first palette */ - if (rinfo-has_CRTC2) { - dac_cntl2 = INREG(DAC_CNTL2); - dac_cntl2 = ~DAC2_PALETTE_ACCESS_CNTL; - OUTREG(DAC_CNTL2, dac_cntl2); - } if (rinfo-bpp == 16) { pindex = regno * 8; @@ -1101,24 +1089,27 @@ if (rinfo-depth == 15 regno 31) return 1; - /* For 565, the green component is mixed one order below */ + /* For 565, the green component is mixed one order +* below +*/ if (rinfo-depth == 16) { OUTREG(PALETTE_INDEX, pindex1); - OUTREG(PALETTE_DATA, (rinfo-palette[regno1].red 16) | - (green 8) | (rinfo-palette[regno1].blue)); + OUTREG(PALETTE_DATA, + (rinfo-palette[regno1].red 16) | + (green 8) | + (rinfo-palette[regno1].blue)); green = rinfo-palette[regno1].green; } } if (rinfo-depth != 16 || regno 32) { OUTREG(PALETTE_INDEX, pindex); - OUTREG(PALETTE_DATA, (red 16) | (green 8) | blue); + OUTREG(PALETTE_DATA, (red 16) | + (green 8) | blue); } - if (rinfo-is_mobility) - OUTPLL(VCLK_ECP_CNTL, vclk_cntl); } if (regno 16) { - u32 *pal = info-pseudo_palette; + u32 *pal = rinfo-info-pseudo_palette; switch (rinfo-depth) { case 15: pal[regno] = (regno 10) | (regno 5) | regno; @@ -1138,6 +1129,84 @@ return 0; } +static int radeonfb_setcolreg (unsigned regno, unsigned red, unsigned green, + unsigned blue, unsigned transp, + struct fb_info *info) +{ +struct radeonfb_info *rinfo = info-par; + u32 dac_cntl2, vclk_cntl = 0; + int rc; + +if (!rinfo-asleep) { + if (rinfo-is_mobility) { +
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Thu, 2005-04-07 at 00:35 +0200, Andreas Schwab wrote: > Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > > > Hrm... it should only add a few ms, maybe about 20 ms to the mode > > switching... If you remove the radeon_msleep(5) call from the > > radeon_pll_errata_after_data() routine in radeonfb.h, does it make a > > difference ? > > Yes, it does. Without the sleep, switching is as fast as before. It is weird tho. Could you try adding a printk or something to figure out how much this is called during a typical swich ? It will seriously spam your console though ... I would expect only 5 or 6 calls during a normal switch (since the PLL value shouldn't change for a flat panel), that is no more than maybe 50ms, while it seems you are having a much bigger delay. Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > Hrm... it should only add a few ms, maybe about 20 ms to the mode > switching... If you remove the radeon_msleep(5) call from the > radeon_pll_errata_after_data() routine in radeonfb.h, does it make a > difference ? Yes, it does. Without the sleep, switching is as fast as before. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Hrm... it should only add a few ms, maybe about 20 ms to the mode switching... If you remove the radeon_msleep(5) call from the radeon_pll_errata_after_data() routine in radeonfb.h, does it make a difference ? Yes, it does. Without the sleep, switching is as fast as before. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Thu, 2005-04-07 at 00:35 +0200, Andreas Schwab wrote: Benjamin Herrenschmidt [EMAIL PROTECTED] writes: Hrm... it should only add a few ms, maybe about 20 ms to the mode switching... If you remove the radeon_msleep(5) call from the radeon_pll_errata_after_data() routine in radeonfb.h, does it make a difference ? Yes, it does. Without the sleep, switching is as fast as before. It is weird tho. Could you try adding a printk or something to figure out how much this is called during a typical swich ? It will seriously spam your console though ... I would expect only 5 or 6 calls during a normal switch (since the PLL value shouldn't change for a flat panel), that is no more than maybe 50ms, while it seems you are having a much bigger delay. Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Tue, 2005-04-05 at 23:44 +0200, Andreas Schwab wrote: > Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > > > After discussion with ATIs, it seems that the workarounds they initially > > gave me were not completely correct. > > > > This patch implements the proper ones, which includes sleeping in PLL > > accesses, and thus requires the previous patch to make sure we do not > > call unblank at interrupt time (unless oops_in_progress is set, in which > > case I use an mdelay). > > > > It also removes obsolete code that used to disable some power management > > features in the accel init code. > > This patch does no good on Radeon M6 (iBook G3). It makes mode switching > to take an extraordinary amount of time, ie. when switching away from X it > takes about 2-3 seconds until the console is restored. Hrm... it should only add a few ms, maybe about 20 ms to the mode switching... If you remove the radeon_msleep(5) call from the radeon_pll_errata_after_data() routine in radeonfb.h, does it make a difference ? Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt <[EMAIL PROTECTED]> writes: > After discussion with ATIs, it seems that the workarounds they initially > gave me were not completely correct. > > This patch implements the proper ones, which includes sleeping in PLL > accesses, and thus requires the previous patch to make sure we do not > call unblank at interrupt time (unless oops_in_progress is set, in which > case I use an mdelay). > > It also removes obsolete code that used to disable some power management > features in the accel init code. This patch does no good on Radeon M6 (iBook G3). It makes mode switching to take an extraordinary amount of time, ie. when switching away from X it takes about 2-3 seconds until the console is restored. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
Benjamin Herrenschmidt [EMAIL PROTECTED] writes: After discussion with ATIs, it seems that the workarounds they initially gave me were not completely correct. This patch implements the proper ones, which includes sleeping in PLL accesses, and thus requires the previous patch to make sure we do not call unblank at interrupt time (unless oops_in_progress is set, in which case I use an mdelay). It also removes obsolete code that used to disable some power management features in the accel init code. This patch does no good on Radeon M6 (iBook G3). It makes mode switching to take an extraordinary amount of time, ie. when switching away from X it takes about 2-3 seconds until the console is restored. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Tue, 2005-04-05 at 23:44 +0200, Andreas Schwab wrote: Benjamin Herrenschmidt [EMAIL PROTECTED] writes: After discussion with ATIs, it seems that the workarounds they initially gave me were not completely correct. This patch implements the proper ones, which includes sleeping in PLL accesses, and thus requires the previous patch to make sure we do not call unblank at interrupt time (unless oops_in_progress is set, in which case I use an mdelay). It also removes obsolete code that used to disable some power management features in the accel init code. This patch does no good on Radeon M6 (iBook G3). It makes mode switching to take an extraordinary amount of time, ie. when switching away from X it takes about 2-3 seconds until the console is restored. Hrm... it should only add a few ms, maybe about 20 ms to the mode switching... If you remove the radeon_msleep(5) call from the radeon_pll_errata_after_data() routine in radeonfb.h, does it make a difference ? Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-03-11 at 16:42 +1100, Benjamin Herrenschmidt wrote: > Hi ! > > After discussion with ATIs, it seems that the workarounds they initially > gave me were not completely correct. > > This patch implements the proper ones, which includes sleeping in PLL > accesses, and thus requires the previous patch to make sure we do not > call unblank at interrupt time (unless oops_in_progress is set, in which > case I use an mdelay). > > It also removes obsolete code that used to disable some power management > features in the accel init code. > > Signed-off-by: Benjamin Herrenschmidt <[EMAIL PROTECTED]> And without a stupidity where I would use a register before ioremap'ing them on some cards: --- After discussion with ATIs, it seems that the workarounds they initially gave me were not completely correct. This patch implements the proper ones, which includes sleeping in PLL accesses, and thus requires the previous patch to make sure we do not call unblank at interrupt time (unless oops_in_progress is set, in which case I use an mdelay). It also removes obsolete code that used to disable some power management features in the accel init code. Signed-off-by: Benjamin Herrenschmidt <[EMAIL PROTECTED]> Index: linux-work/drivers/video/aty/radeonfb.h === --- linux-work.orig/drivers/video/aty/radeonfb.h2005-03-11 11:08:32.0 +1100 +++ linux-work/drivers/video/aty/radeonfb.h 2005-03-13 11:09:30.0 +1100 @@ -77,6 +77,15 @@ CHIP_HAS_CRTC2 = 0x0004UL, }; +/* + * Errata workarounds + */ +enum radeon_errata { + CHIP_ERRATA_R300_CG = 0x0001, + CHIP_ERRATA_PLL_DUMMYREADS = 0x0002, + CHIP_ERRATA_PLL_DELAY = 0x0004, +}; + /* * Monitor types @@ -295,6 +304,7 @@ int chipset; u8 family; u8 rev; + unsigned interrata; unsigned long video_ram; unsigned long mapped_vram; int vram_width; @@ -305,7 +315,6 @@ int has_CRTC2; int is_mobility; int is_IGP; - int R300_cg_workaround; int reversed_DAC; int reversed_TMDS; struct panel_info panel_info; @@ -369,6 +378,21 @@ * IO macros */ +/* Note about this function: we have some rare cases where we must not schedule, + * this typically happen with our special "wake up early" hook which allows us to + * wake up the graphic chip (and thus get the console back) before everything else + * on some machines that support that mecanism. At this point, interrupts are off + * and scheduling is not permitted + */ +static inline void _radeon_msleep(struct radeonfb_info *rinfo, unsigned long ms) +{ + if (rinfo->no_schedule || oops_in_progress) + mdelay(ms); + else + msleep(ms); +} + + #define INREG8(addr) readb((rinfo->mmio_base)+addr) #define OUTREG8(addr,val) writeb(val, (rinfo->mmio_base)+addr) #define INREG(addr)readl((rinfo->mmio_base)+addr) @@ -390,107 +414,85 @@ #define OUTREGP(addr,val,mask) _OUTREGP(rinfo, addr, val,mask) -/* This function is required to workaround a hardware bug in some (all?) - * revisions of the R300. This workaround should be called after every - * CLOCK_CNTL_INDEX register access. If not, register reads afterward - * may not be correct. - */ -static inline void R300_cg_workardound(struct radeonfb_info *rinfo) -{ - u32 save, tmp; - save = INREG(CLOCK_CNTL_INDEX); - tmp = save & ~(0x3f | PLL_WR_EN); - OUTREG(CLOCK_CNTL_INDEX, tmp); - tmp = INREG(CLOCK_CNTL_DATA); - OUTREG(CLOCK_CNTL_INDEX, save); -} - /* - * PLL accesses suffer from various HW issues on the different chip - * families. Some R300's need the above workaround, rv200 & friends - * need a couple of dummy reads after any write of CLOCK_CNTL_INDEX, - * and some RS100/200 need a dummy read before writing to - * CLOCK_CNTL_INDEX as well. Instead of testing every chip revision, - * we just unconditionally do the workarounds at once since PLL - * accesses are far from beeing performance critical. Except for R300 - * one which stays separate for now + * Note about PLL register accesses: + * + * I have removed the spinlock on them on purpose. The driver now + * expects that it will only manipulate the PLL registers in normal + * task environment, where radeon_msleep() will be called, protected + * by a semaphore (currently the console semaphore) so that no conflict + * will happen on the PLL register index. + * + * With the latest changes to the VT layer, this is guaranteed for all + * calls except the actual drawing/blits which aren't supposed to use + * the PLL
[PATCH] radeonfb: (#2) Implement proper workarounds for PLL accesses
On Fri, 2005-03-11 at 16:42 +1100, Benjamin Herrenschmidt wrote: Hi ! After discussion with ATIs, it seems that the workarounds they initially gave me were not completely correct. This patch implements the proper ones, which includes sleeping in PLL accesses, and thus requires the previous patch to make sure we do not call unblank at interrupt time (unless oops_in_progress is set, in which case I use an mdelay). It also removes obsolete code that used to disable some power management features in the accel init code. Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED] And without a stupidity where I would use a register before ioremap'ing them on some cards: --- After discussion with ATIs, it seems that the workarounds they initially gave me were not completely correct. This patch implements the proper ones, which includes sleeping in PLL accesses, and thus requires the previous patch to make sure we do not call unblank at interrupt time (unless oops_in_progress is set, in which case I use an mdelay). It also removes obsolete code that used to disable some power management features in the accel init code. Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED] Index: linux-work/drivers/video/aty/radeonfb.h === --- linux-work.orig/drivers/video/aty/radeonfb.h2005-03-11 11:08:32.0 +1100 +++ linux-work/drivers/video/aty/radeonfb.h 2005-03-13 11:09:30.0 +1100 @@ -77,6 +77,15 @@ CHIP_HAS_CRTC2 = 0x0004UL, }; +/* + * Errata workarounds + */ +enum radeon_errata { + CHIP_ERRATA_R300_CG = 0x0001, + CHIP_ERRATA_PLL_DUMMYREADS = 0x0002, + CHIP_ERRATA_PLL_DELAY = 0x0004, +}; + /* * Monitor types @@ -295,6 +304,7 @@ int chipset; u8 family; u8 rev; + unsigned interrata; unsigned long video_ram; unsigned long mapped_vram; int vram_width; @@ -305,7 +315,6 @@ int has_CRTC2; int is_mobility; int is_IGP; - int R300_cg_workaround; int reversed_DAC; int reversed_TMDS; struct panel_info panel_info; @@ -369,6 +378,21 @@ * IO macros */ +/* Note about this function: we have some rare cases where we must not schedule, + * this typically happen with our special wake up early hook which allows us to + * wake up the graphic chip (and thus get the console back) before everything else + * on some machines that support that mecanism. At this point, interrupts are off + * and scheduling is not permitted + */ +static inline void _radeon_msleep(struct radeonfb_info *rinfo, unsigned long ms) +{ + if (rinfo-no_schedule || oops_in_progress) + mdelay(ms); + else + msleep(ms); +} + + #define INREG8(addr) readb((rinfo-mmio_base)+addr) #define OUTREG8(addr,val) writeb(val, (rinfo-mmio_base)+addr) #define INREG(addr)readl((rinfo-mmio_base)+addr) @@ -390,107 +414,85 @@ #define OUTREGP(addr,val,mask) _OUTREGP(rinfo, addr, val,mask) -/* This function is required to workaround a hardware bug in some (all?) - * revisions of the R300. This workaround should be called after every - * CLOCK_CNTL_INDEX register access. If not, register reads afterward - * may not be correct. - */ -static inline void R300_cg_workardound(struct radeonfb_info *rinfo) -{ - u32 save, tmp; - save = INREG(CLOCK_CNTL_INDEX); - tmp = save ~(0x3f | PLL_WR_EN); - OUTREG(CLOCK_CNTL_INDEX, tmp); - tmp = INREG(CLOCK_CNTL_DATA); - OUTREG(CLOCK_CNTL_INDEX, save); -} - /* - * PLL accesses suffer from various HW issues on the different chip - * families. Some R300's need the above workaround, rv200 friends - * need a couple of dummy reads after any write of CLOCK_CNTL_INDEX, - * and some RS100/200 need a dummy read before writing to - * CLOCK_CNTL_INDEX as well. Instead of testing every chip revision, - * we just unconditionally do the workarounds at once since PLL - * accesses are far from beeing performance critical. Except for R300 - * one which stays separate for now + * Note about PLL register accesses: + * + * I have removed the spinlock on them on purpose. The driver now + * expects that it will only manipulate the PLL registers in normal + * task environment, where radeon_msleep() will be called, protected + * by a semaphore (currently the console semaphore) so that no conflict + * will happen on the PLL register index. + * + * With the latest changes to the VT layer, this is guaranteed for all + * calls except the actual drawing/blits which aren't supposed to use + * the PLL registers anyway + * + * This is