[PATCH] (#2) radeonfb: PLL access workaround

From: Benjamin Herrenschmidt
Date: Mon Mar 07 2005 - 23:03:35 EST


On Tue, 2005-03-08 at 11:59 +1100, Benjamin Herrenschmidt wrote:
> Hi !
>
> So you remember all those weird lockups at boot that happened in late
> 2.6.11-rc with radeonfb. I posted a "workaround" which just moved code
> around a bit and it appeared to work. I finally got some real infos
> about the problem from ATI, and it seems my "workaround" is not very
> safe and there are other potential issues realted to HW bugs when
> accessing the PLL registers.
>
> This patch implements all of these workarounds (and puts back the code
> where it was before my previous fix).
>
> Please test and let me know if it works. If it's fine, then it should go
> to -mm, and eventually after a while, to a 2.6.11.x update.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>

Here's a new one adding a missing bit..

Index: linux-work/drivers/video/aty/radeonfb.h
===================================================================
--- linux-work.orig/drivers/video/aty/radeonfb.h 2005-03-08 11:30:37.000000000 +1100
+++ linux-work/drivers/video/aty/radeonfb.h 2005-03-08 14:49:24.000000000 +1100
@@ -22,11 +22,6 @@

#include <video/radeon.h>

-/* Some weird black magic use by Apple driver that we don't use for
- * now --BenH
- */
-#undef HAS_PLL_M9_GPIO_MAGIC
-
/***************************************************************
* Most of the definitions here are adapted right from XFree86 *
***************************************************************/
@@ -312,7 +307,6 @@
int is_mobility;
int is_IGP;
int R300_cg_workaround;
- int m9p_workaround;
int reversed_DAC;
int reversed_TMDS;
struct panel_info panel_info;
@@ -397,6 +391,11 @@

#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;
@@ -407,35 +406,36 @@
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
+ */
+
+static inline void radeon_pll_workaround_before(struct radeonfb_info *rinfo)
+{
+ (void)INREG(CRTC_GEN_CNTL);
+}
+
+static inline void radeon_pll_workaround_after(struct radeonfb_info *rinfo)
+{
+ (void)INREG(CLOCK_CNTL_DATA);
+ (void)INREG(CRTC_GEN_CNTL);
+}

static inline u32 __INPLL(struct radeonfb_info *rinfo, u32 addr)
{
u32 data;
-#ifdef HAS_PLL_M9_GPIO_MAGIC
- u32 sv[3];
-
- if (rinfo->m9p_workaround) {
- sv[0] = INREG(0x19c);
- sv[1] = INREG(0x1a0);
- sv[2] = INREG(0x198);
- OUTREG(0x198, 0);
- OUTREG(0x1a0, 0);
- OUTREG(0x19c, 0);
- }
-#endif /* HAS_PLL_M9_GPIO_MAGIC */

+ radeon_pll_workaround_before(rinfo);
OUTREG8(CLOCK_CNTL_INDEX, addr & 0x0000003f);
- data = (INREG(CLOCK_CNTL_DATA));
-
-#ifdef HAS_PLL_M9_GPIO_MAGIC
- if (rinfo->m9p_workaround) {
- (void)INREG(CRTC_GEN_CNTL);
- data = INREG(CLOCK_CNTL_DATA);
- OUTREG(0x19c, sv[0]);
- OUTREG(0x1a0, sv[1]);
- OUTREG(0x198, sv[2]);
- }
-#endif /* HAS_PLL_M9_GPIO_MAGIC */
+ radeon_pll_workaround_after(rinfo);
+ data = INREG(CLOCK_CNTL_DATA);
if (rinfo->R300_cg_workaround)
R300_cg_workardound(rinfo);
return data;
@@ -455,32 +455,16 @@
#define INPLL(addr) _INPLL(rinfo, addr)


-static inline void __OUTPLL(struct radeonfb_info *rinfo, unsigned int index, u32 val)
+static inline void __OUTPLL(struct radeonfb_info *rinfo, unsigned int index,
+ u32 val)
{
-#ifdef HAS_PLL_M9_GPIO_MAGIC
- u32 sv[3];
-
- if (rinfo->m9p_workaround) {
- sv[0] = INREG(0x19c);
- sv[1] = INREG(0x1a0);
- sv[2] = INREG(0x198);
- OUTREG(0x198, 0);
- OUTREG(0x1a0, 0);
- OUTREG(0x19c, 0);
- mdelay(1);
- }
-#endif /* HAS_PLL_M9_GPIO_MAGIC */

+ radeon_pll_workaround_before(rinfo);
OUTREG8(CLOCK_CNTL_INDEX, (index & 0x0000003f) | 0x00000080);
+ radeon_pll_workaround_after(rinfo);
OUTREG(CLOCK_CNTL_DATA, val);
-
-#ifdef HAS_PLL_M9_GPIO_MAGIC
- if (rinfo->m9p_workaround) {
- OUTREG(0x19c, sv[0]);
- OUTREG(0x1a0, sv[1]);
- OUTREG(0x198, sv[2]);
- }
-#endif /* HAS_PLL_M9_GPIO_MAGIC */
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);
}

static inline void _OUTPLL(struct radeonfb_info *rinfo, unsigned int index, u32 val)
Index: linux-work/drivers/video/aty/radeon_base.c
===================================================================
--- linux-work.orig/drivers/video/aty/radeon_base.c 2005-03-08 11:30:37.000000000 +1100
+++ linux-work/drivers/video/aty/radeon_base.c 2005-03-08 14:49:24.000000000 +1100
@@ -530,7 +530,11 @@
break;
}

+ radeon_pll_workaround_before(rinfo);
ppll_div_sel = INREG8(CLOCK_CNTL_INDEX + 1) & 0x3;
+ radeon_pll_workaround_after(rinfo);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);

n = (INPLL(PPLL_DIV_0 + ppll_div_sel) & 0x7ff);
m = (INPLL(PPLL_REF_DIV) & 0x3ff);
@@ -1168,7 +1172,11 @@
save->vclk_ecp_cntl = INPLL(VCLK_ECP_CNTL);

/* PLL regs */
+ radeon_pll_workaround_before(rinfo);
save->clk_cntl_index = INREG(CLOCK_CNTL_INDEX) & ~0x3f;
+ radeon_pll_workaround_after(rinfo);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);
save->ppll_div_3 = INPLL(PPLL_DIV_3);
save->ppll_ref_div = INPLL(PPLL_REF_DIV);
}
@@ -1195,9 +1203,13 @@
/* We still have to force a switch to selected PPLL div thanks to
* an XFree86 driver bug which will switch it away in some cases
* even when using UseFDev */
+ radeon_pll_workaround_before(rinfo);
OUTREGP(CLOCK_CNTL_INDEX,
mode->clk_cntl_index & PPLL_DIV_SEL_MASK,
~PPLL_DIV_SEL_MASK);
+ radeon_pll_workaround_after(rinfo);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);
return;
}
}
@@ -1211,9 +1223,13 @@
~(PPLL_RESET | PPLL_ATOMIC_UPDATE_EN | PPLL_VGA_ATOMIC_UPDATE_EN));

/* Switch to selected PPLL divider */
+ radeon_pll_workaround_before(rinfo);
OUTREGP(CLOCK_CNTL_INDEX,
mode->clk_cntl_index & PPLL_DIV_SEL_MASK,
~PPLL_DIV_SEL_MASK);
+ radeon_pll_workaround_after(rinfo);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);

/* Set PPLL ref. div */
if (rinfo->family == CHIP_FAMILY_R300 ||
@@ -2359,6 +2375,15 @@
radeon_save_state (rinfo, &rinfo->init_state);
memcpy(&rinfo->state, &rinfo->init_state, sizeof(struct radeon_regs));

+ /* Setup Power Management capabilities */
+ if (default_dynclk < -1) {
+ /* -2 is special: means ON on mobility chips and do not
+ * change on others
+ */
+ radeonfb_pm_init(rinfo, rinfo->is_mobility ? 1 : -1);
+ } else
+ radeonfb_pm_init(rinfo, default_dynclk);
+
pci_set_drvdata(pdev, info);

/* Register with fbdev layer */
@@ -2369,13 +2394,6 @@
goto err_unmap_fb;
}

- /* Setup Power Management capabilities */
- if (default_dynclk < -1) {
- /* -2 is special: means ON on mobility chips and do not change on others */
- radeonfb_pm_init(rinfo, rinfo->is_mobility ? 1 : -1);
- } else
- radeonfb_pm_init(rinfo, default_dynclk);
-
#ifdef CONFIG_MTRR
rinfo->mtrr_hdl = nomtrr ? -1 : mtrr_add(rinfo->fb_base_phys,
rinfo->video_ram,
Index: linux-work/drivers/video/aty/radeon_pm.c
===================================================================
--- linux-work.orig/drivers/video/aty/radeon_pm.c 2005-03-04 17:22:13.000000000 +1100
+++ linux-work/drivers/video/aty/radeon_pm.c 2005-03-08 11:47:11.000000000 +1100
@@ -1372,8 +1372,12 @@

/* Reconfigure SPLL charge pump, VCO gain, duty cycle */
tmp = INPLL(pllSPLL_CNTL);
+ radeon_pll_workaround_before(rinfo);
OUTREG8(CLOCK_CNTL_INDEX, pllSPLL_CNTL + PLL_WR_EN);
+ radeon_pll_workaround_after(rinfo);
OUTREG8(CLOCK_CNTL_DATA + 1, (tmp >> 8) & 0xff);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);

/* Set SPLL feedback divider */
tmp = INPLL(pllM_SPLL_REF_FB_DIV);
@@ -1405,8 +1409,12 @@

/* Reconfigure MPLL charge pump, VCO gain, duty cycle */
tmp = INPLL(pllMPLL_CNTL);
+ radeon_pll_workaround_before(rinfo);
OUTREG8(CLOCK_CNTL_INDEX, pllMPLL_CNTL + PLL_WR_EN);
+ radeon_pll_workaround_after(rinfo);
OUTREG8(CLOCK_CNTL_DATA + 1, (tmp >> 8) & 0xff);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);

/* Set MPLL feedback divider */
tmp = INPLL(pllM_SPLL_REF_FB_DIV);
@@ -1524,8 +1532,12 @@
{
u32 tmp;

+ radeon_pll_workaround_before(rinfo);
OUTREG8(CLOCK_CNTL_INDEX, pllHTOTAL_CNTL + PLL_WR_EN);
+ radeon_pll_workaround_after(rinfo);
OUTREG8(CLOCK_CNTL_DATA, 0);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);

tmp = INPLL(pllVCLK_ECP_CNTL);
OUTPLL(pllVCLK_ECP_CNTL, tmp | 0x80);
@@ -1540,12 +1552,12 @@
* probably useless since we already did it ...
*/
tmp = INPLL(pllPPLL_CNTL);
+ radeon_pll_workaround_before(rinfo);
OUTREG8(CLOCK_CNTL_INDEX, pllSPLL_CNTL + PLL_WR_EN);
+ radeon_pll_workaround_after(rinfo);
OUTREG8(CLOCK_CNTL_DATA + 1, (tmp >> 8) & 0xff);
-
- /* Not sure what was intended here ... */
- tmp = INREG(CLOCK_CNTL_INDEX);
- OUTREG(CLOCK_CNTL_INDEX, tmp);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);

/* Restore our "reference" PPLL divider set by firmware
* according to proper spread spectrum calculations
@@ -1569,7 +1581,11 @@
mdelay(5);

/* Switch pixel clock to firmware default div 0 */
+ radeon_pll_workaround_before(rinfo);
OUTREG8(CLOCK_CNTL_INDEX+1, 0);
+ radeon_pll_workaround_after(rinfo);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);
}

static void radeon_pm_m10_reconfigure_mc(struct radeonfb_info *rinfo)
@@ -2752,8 +2768,6 @@
if (!strcmp(rinfo->of_node->name, "ATY,ViaParent")) {
rinfo->reinit_func = radeon_reinitialize_M9P;
rinfo->pm_mode |= radeon_pm_off;
- /* Workaround not used for now */
- rinfo->m9p_workaround = 1;
}

/* If any of the above is set, we assume the machine can sleep/resume.
Index: linux-work/drivers/video/aty/radeon_monitor.c
===================================================================
--- linux-work.orig/drivers/video/aty/radeon_monitor.c 2005-03-05 16:04:38.000000000 +1100
+++ linux-work/drivers/video/aty/radeon_monitor.c 2005-03-08 14:50:43.000000000 +1100
@@ -655,8 +655,14 @@
*/
if (!rinfo->panel_info.use_bios_dividers && rinfo->mon1_type == MT_LCD
&& rinfo->is_mobility) {
- int ppll_div_sel = INREG8(CLOCK_CNTL_INDEX + 1) & 0x3;
- u32 ppll_divn = INPLL(PPLL_DIV_0 + ppll_div_sel);
+ int ppll_div_sel;
+ u32 ppll_divn;
+ radeon_pll_workaround_before(rinfo);
+ ppll_div_sel = INREG8(CLOCK_CNTL_INDEX + 1) & 0x3;
+ radeon_pll_workaround_after(rinfo);
+ if (rinfo->R300_cg_workaround)
+ R300_cg_workardound(rinfo);
+ ppll_divn = INPLL(PPLL_DIV_0 + ppll_div_sel);
rinfo->panel_info.ref_divider = rinfo->pll.ref_div;
rinfo->panel_info.fbk_divider = ppll_divn & 0x7ff;
rinfo->panel_info.post_divider = (ppll_divn >> 16) & 0x7;


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/