[PATCH] radeonfb: PLL access workaround

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).  
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

drivers/video/aty/radeon_base.c

@@ -530,7 +530,11 @@ static int __devinit radeon_probe_pll_params(struct radeonfb_info *rinfo)
         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);
@@ -1169,7 +1173,11 @@ static void radeon_save_state (struct radeonfb_info *rinfo,
 	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);
 }
@@ -1196,9 +1204,13 @@ static void radeon_write_pll_regs(struct radeonfb_info *rinfo, struct radeon_reg
 			/* 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;
 		}
 	}
@@ -1212,9 +1224,13 @@ static void radeon_write_pll_regs(struct radeonfb_info *rinfo, struct radeon_reg
 		~(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 ||
@@ -2360,6 +2376,15 @@ static int radeonfb_pci_register (struct pci_dev *pdev,
 	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 */
@@ -2370,13 +2395,6 @@ static int radeonfb_pci_register (struct pci_dev *pdev,
 		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,

drivers/video/aty/radeon_monitor.c

@@ -655,8 +655,14 @@ static void radeon_fixup_panel_info(struct radeonfb_info *rinfo)
 	 */
 	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;

drivers/video/aty/radeon_pm.c

@@ -1372,8 +1372,12 @@ static void radeon_pm_start_mclk_sclk(struct radeonfb_info *rinfo)
 
 	/* 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 @@ static void radeon_pm_start_mclk_sclk(struct radeonfb_info *rinfo)
 
 	/* 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 @@ static void radeon_pm_restore_pixel_pll(struct radeonfb_info *rinfo)
 {
 	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 @@ static void radeon_pm_restore_pixel_pll(struct radeonfb_info *rinfo)
 	 * 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 @@ static void radeon_pm_restore_pixel_pll(struct radeonfb_info *rinfo)
 	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 @@ void radeonfb_pm_init(struct radeonfb_info *rinfo, int dynclk)
 		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.

drivers/video/aty/radeonfb.h

@@ -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 *
  ***************************************************************/
@@ -311,7 +306,6 @@ struct radeonfb_info {
 	int			is_mobility;
 	int			is_IGP;
 	int			R300_cg_workaround;
-	int			m9p_workaround;
 	int			reversed_DAC;
 	int			reversed_TMDS;
 	struct panel_info	panel_info;
@@ -396,6 +390,11 @@ static inline void _OUTREGP(struct radeonfb_info *rinfo, u32 addr,
 
 #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;
@@ -406,35 +405,36 @@ static inline void R300_cg_workardound(struct radeonfb_info *rinfo)
 	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;
@@ -454,32 +454,16 @@ static inline u32 _INPLL(struct radeonfb_info *rinfo, u32 addr)
 #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)