drm/mgag200: Split PLL setup into compute and update functions

The _set_plls() functions compute a pixel clock's PLL values
and program the hardware accordingly. This happens during atomic
commits.

For atomic modesetting, it's better to separate computation and
programming from each other. This will allow to compute the PLL
value during atomic checks and catch unsupported modes early.

Split the PLL setup into a compute and an update functions, and
call them one after the other. Computed PLL values are store in
struct mgag200_pll_values. There are four parameters for the PLL,
m, n, p and s. Every compute function stores a value for each
of these parameters, and the rsp update function makes the register
bits from them. The values stored by the compute function are
either plain values or register bits. An additional change is
required to always store plain values.

No functional changes.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20210714142240.21979-5-tzimmermann@suse.de

drivers/gpu/drm/mgag200/mgag200_drv.h

@@ -126,6 +126,23 @@
 #define MGAG200_MAX_FB_HEIGHT 4096
 #define MGAG200_MAX_FB_WIDTH 4096
 
+/*
+ * Stores parameters for programming the PLLs
+ *
+ * Fref: reference frequency (A: 25.175 Mhz, B: 28.361, C: XX Mhz)
+ * Fo: output frequency
+ * Fvco = Fref * (N / M)
+ * Fo = Fvco / P
+ *
+ * S = [0..3]
+ */
+struct mgag200_pll_values {
+	unsigned int m;
+	unsigned int n;
+	unsigned int p;
+	unsigned int s;
+};
+
 #define to_mga_connector(x) container_of(x, struct mga_connector, base)
 
 struct mga_i2c_chan {

drivers/gpu/drm/mgag200/mgag200_mode.c

@@ -114,7 +114,8 @@ static inline void mga_wait_busy(struct mga_device *mdev)
  * PLL setup
  */
 
-static int mgag200_g200_set_plls(struct mga_device *mdev, long clock)
+static int mgag200_compute_pixpll_values_g200(struct mga_device *mdev, long clock,
+					      struct mgag200_pll_values *pixpllc)
 {
 	struct drm_device *dev = &mdev->base;
 	const int post_div_max = 7;
@@ -174,16 +175,32 @@ static int mgag200_g200_set_plls(struct mga_device *mdev, long clock)
 	drm_dbg_kms(dev, "clock: %ld vco: %ld m: %d n: %d p: %d s: %d\n",
 		    clock, f_vco, m, n, p, s);
 
-	WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK);
-
-	WREG_DAC(MGA1064_PIX_PLLC_M, m);
-	WREG_DAC(MGA1064_PIX_PLLC_N, n);
-	WREG_DAC(MGA1064_PIX_PLLC_P, (p | (s << 3)));
+	pixpllc->m = m;
+	pixpllc->n = n;
+	pixpllc->p = p;
+	pixpllc->s = s;
 
 	return 0;
 }
 
-static int mga_g200se_set_plls(struct mga_device *mdev, long clock)
+static void mgag200_set_pixpll_g200(struct mga_device *mdev,
+				    const struct mgag200_pll_values *pixpllc)
+{
+	u8 xpixpllcm, xpixpllcn, xpixpllcp;
+
+	xpixpllcm = pixpllc->m;
+	xpixpllcn = pixpllc->n;
+	xpixpllcp = pixpllc->p | (pixpllc->s << 3);
+
+	WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK);
+
+	WREG_DAC(MGA1064_PIX_PLLC_M, xpixpllcm);
+	WREG_DAC(MGA1064_PIX_PLLC_N, xpixpllcn);
+	WREG_DAC(MGA1064_PIX_PLLC_P, xpixpllcp);
+}
+
+static int mgag200_compute_pixpll_values_g200se(struct mga_device *mdev, long clock,
+						struct mgag200_pll_values *pixpllc)
 {
 	static const unsigned int pvalues_e4[] = {16, 14, 12, 10, 8, 6, 4, 2, 1};
 
@@ -290,32 +307,45 @@ static int mga_g200se_set_plls(struct mga_device *mdev, long clock)
 		return -EINVAL;
 	}
 
+	pixpllc->m = m;
+	pixpllc->n = n;
+	pixpllc->p = p;
+	pixpllc->s = 0;
+
+	return 0;
+}
+
+static void mgag200_set_pixpll_g200se(struct mga_device *mdev,
+				      const struct mgag200_pll_values *pixpllc)
+{
+	u32 unique_rev_id = mdev->model.g200se.unique_rev_id;
+	u8 xpixpllcm, xpixpllcn, xpixpllcp;
+
+	xpixpllcm = pixpllc->m;
+	xpixpllcn = pixpllc->n;
+	xpixpllcp = pixpllc->p | (pixpllc->s << 3);
+
 	WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK);
 
-	WREG_DAC(MGA1064_PIX_PLLC_M, m);
-	WREG_DAC(MGA1064_PIX_PLLC_N, n);
-	WREG_DAC(MGA1064_PIX_PLLC_P, p);
+	WREG_DAC(MGA1064_PIX_PLLC_M, xpixpllcm);
+	WREG_DAC(MGA1064_PIX_PLLC_N, xpixpllcn);
+	WREG_DAC(MGA1064_PIX_PLLC_P, xpixpllcp);
 
 	if (unique_rev_id >= 0x04) {
 		WREG_DAC(0x1a, 0x09);
 		msleep(20);
 		WREG_DAC(0x1a, 0x01);
-
 	}
-
-	return 0;
 }
 
-static int mga_g200wb_set_plls(struct mga_device *mdev, long clock)
+static int mgag200_compute_pixpll_values_g200wb(struct mga_device *mdev, long clock,
+						struct mgag200_pll_values *pixpllc)
 {
 	unsigned int vcomax, vcomin, pllreffreq;
 	unsigned int delta, tmpdelta;
 	unsigned int testp, testm, testn, testp2;
 	unsigned int p, m, n;
 	unsigned int computed;
-	int i, j, tmpcount, vcount;
-	bool pll_locked = false;
-	u8 tmp;
 
 	m = n = p = 0;
 
@@ -388,6 +418,25 @@ static int mga_g200wb_set_plls(struct mga_device *mdev, long clock)
 		}
 	}
 
+	pixpllc->m = m;
+	pixpllc->n = n;
+	pixpllc->p = p;
+	pixpllc->s = 0;
+
+	return 0;
+}
+
+static void mgag200_set_pixpll_g200wb(struct mga_device *mdev,
+				      const struct mgag200_pll_values *pixpllc)
+{
+	u8 xpixpllcm, xpixpllcn, xpixpllcp, tmp;
+	int i, j, tmpcount, vcount;
+	bool pll_locked = false;
+
+	xpixpllcm = pixpllc->m;
+	xpixpllcn = pixpllc->n;
+	xpixpllcp = pixpllc->p | (pixpllc->s << 3);
+
 	WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK);
 
 	for (i = 0; i <= 32 && pll_locked == false; i++) {
@@ -430,9 +479,9 @@ static int mga_g200wb_set_plls(struct mga_device *mdev, long clock)
 		udelay(50);
 
 		/* program pixel pll register */
-		WREG_DAC(MGA1064_WB_PIX_PLLC_N, n);
-		WREG_DAC(MGA1064_WB_PIX_PLLC_M, m);
-		WREG_DAC(MGA1064_WB_PIX_PLLC_P, p);
+		WREG_DAC(MGA1064_PIX_PLLC_N, xpixpllcn);
+		WREG_DAC(MGA1064_PIX_PLLC_M, xpixpllcm);
+		WREG_DAC(MGA1064_PIX_PLLC_P, xpixpllcp);
 
 		udelay(50);
 
@@ -480,21 +529,21 @@ static int mga_g200wb_set_plls(struct mga_device *mdev, long clock)
 				udelay(5);
 		}
 	}
+
 	WREG8(DAC_INDEX, MGA1064_REMHEADCTL);
 	tmp = RREG8(DAC_DATA);
 	tmp &= ~MGA1064_REMHEADCTL_CLKDIS;
 	WREG_DAC(MGA1064_REMHEADCTL, tmp);
-	return 0;
 }
 
-static int mga_g200ev_set_plls(struct mga_device *mdev, long clock)
+static int mgag200_compute_pixpll_values_g200ev(struct mga_device *mdev, long clock,
+						struct mgag200_pll_values *pixpllc)
 {
 	unsigned int vcomax, vcomin, pllreffreq;
 	unsigned int delta, tmpdelta;
 	unsigned int testp, testm, testn;
 	unsigned int p, m, n;
 	unsigned int computed;
-	u8 tmp;
 
 	m = n = p = 0;
 	vcomax = 550000;
@@ -527,6 +576,23 @@ static int mga_g200ev_set_plls(struct mga_device *mdev, long clock)
 		}
 	}
 
+	pixpllc->m = m;
+	pixpllc->n = n;
+	pixpllc->p = p;
+	pixpllc->s = 0;
+
+	return 0;
+}
+
+static void mgag200_set_pixpll_g200ev(struct mga_device *mdev,
+				      const struct mgag200_pll_values *pixpllc)
+{
+	u8 xpixpllcm, xpixpllcn, xpixpllcp, tmp;
+
+	xpixpllcm = pixpllc->m;
+	xpixpllcn = pixpllc->n;
+	xpixpllcp = pixpllc->p | (pixpllc->s << 3);
+
 	WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK);
 
 	WREG8(DAC_INDEX, MGA1064_PIX_CLK_CTL);
@@ -547,9 +613,9 @@ static int mga_g200ev_set_plls(struct mga_device *mdev, long clock)
 	tmp |= MGA1064_PIX_CLK_CTL_CLK_POW_DOWN;
 	WREG8(DAC_DATA, tmp);
 
-	WREG_DAC(MGA1064_EV_PIX_PLLC_M, m);
-	WREG_DAC(MGA1064_EV_PIX_PLLC_N, n);
-	WREG_DAC(MGA1064_EV_PIX_PLLC_P, p);
+	WREG_DAC(MGA1064_EV_PIX_PLLC_M, xpixpllcm);
+	WREG_DAC(MGA1064_EV_PIX_PLLC_N, xpixpllcn);
+	WREG_DAC(MGA1064_EV_PIX_PLLC_P, xpixpllcp);
 
 	udelay(50);
 
@@ -578,20 +644,16 @@ static int mga_g200ev_set_plls(struct mga_device *mdev, long clock)
 	tmp = RREG8(DAC_DATA);
 	tmp &= ~MGA1064_PIX_CLK_CTL_CLK_DIS;
 	WREG8(DAC_DATA, tmp);
-
-	return 0;
 }
 
-static int mga_g200eh_set_plls(struct mga_device *mdev, long clock)
+static int mgag200_compute_pixpll_values_g200eh(struct mga_device *mdev, long clock,
+						struct mgag200_pll_values *pixpllc)
 {
 	unsigned int vcomax, vcomin, pllreffreq;
 	unsigned int delta, tmpdelta;
 	unsigned int testp, testm, testn;
 	unsigned int p, m, n;
 	unsigned int computed;
-	int i, j, tmpcount, vcount;
-	u8 tmp;
-	bool pll_locked = false;
 
 	m = n = p = 0;
 
@@ -662,6 +724,25 @@ static int mga_g200eh_set_plls(struct mga_device *mdev, long clock)
 		}
 	}
 
+	pixpllc->m = m;
+	pixpllc->n = n;
+	pixpllc->p = p;
+	pixpllc->s = 0;
+
+	return 0;
+}
+
+static void mgag200_set_pixpll_g200eh(struct mga_device *mdev,
+				      const struct mgag200_pll_values *pixpllc)
+{
+	u8 xpixpllcm, xpixpllcn, xpixpllcp, tmp;
+	int i, j, tmpcount, vcount;
+	bool pll_locked = false;
+
+	xpixpllcm = pixpllc->m;
+	xpixpllcn = pixpllc->n;
+	xpixpllcp = pixpllc->p | (pixpllc->s << 3);
+
 	WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK);
 
 	for (i = 0; i <= 32 && pll_locked == false; i++) {
@@ -681,9 +762,9 @@ static int mga_g200eh_set_plls(struct mga_device *mdev, long clock)
 
 		udelay(500);
 
-		WREG_DAC(MGA1064_EH_PIX_PLLC_M, m);
-		WREG_DAC(MGA1064_EH_PIX_PLLC_N, n);
-		WREG_DAC(MGA1064_EH_PIX_PLLC_P, p);
+		WREG_DAC(MGA1064_EH_PIX_PLLC_M, xpixpllcm);
+		WREG_DAC(MGA1064_EH_PIX_PLLC_N, xpixpllcn);
+		WREG_DAC(MGA1064_EH_PIX_PLLC_P, xpixpllcp);
 
 		udelay(500);
 
@@ -711,11 +792,10 @@ static int mga_g200eh_set_plls(struct mga_device *mdev, long clock)
 				udelay(5);
 		}
 	}
-
-	return 0;
 }
 
-static int mga_g200er_set_plls(struct mga_device *mdev, long clock)
+static int mgag200_compute_pixpll_values_g200er(struct mga_device *mdev, long clock,
+						struct mgag200_pll_values *pixpllc)
 {
 	static const unsigned int m_div_val[] = { 1, 2, 4, 8 };
 	unsigned int vcomax, vcomin, pllreffreq;
@@ -723,7 +803,6 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock)
 	int testr, testn, testm, testo;
 	unsigned int p, m, n;
 	unsigned int computed, vco;
-	int tmp;
 
 	m = n = p = 0;
 	vcomax = 1488000;
@@ -764,6 +843,23 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock)
 		}
 	}
 
+	pixpllc->m = m;
+	pixpllc->n = n;
+	pixpllc->p = p;
+	pixpllc->s = 0;
+
+	return 0;
+}
+
+static void mgag200_set_pixpll_g200er(struct mga_device *mdev,
+				      const struct mgag200_pll_values *pixpllc)
+{
+	u8 xpixpllcm, xpixpllcn, xpixpllcp, tmp;
+
+	xpixpllcm = pixpllc->m;
+	xpixpllcn = pixpllc->n;
+	xpixpllcp = pixpllc->p | (pixpllc->s << 3);
+
 	WREG_MISC_MASKED(MGAREG_MISC_CLKSEL_MGA, MGAREG_MISC_CLKSEL_MASK);
 
 	WREG8(DAC_INDEX, MGA1064_PIX_CLK_CTL);
@@ -788,37 +884,70 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock)
 
 	udelay(500);
 
-	WREG_DAC(MGA1064_ER_PIX_PLLC_N, n);
-	WREG_DAC(MGA1064_ER_PIX_PLLC_M, m);
-	WREG_DAC(MGA1064_ER_PIX_PLLC_P, p);
+	WREG_DAC(MGA1064_ER_PIX_PLLC_N, xpixpllcn);
+	WREG_DAC(MGA1064_ER_PIX_PLLC_M, xpixpllcm);
+	WREG_DAC(MGA1064_ER_PIX_PLLC_P, xpixpllcp);
 
 	udelay(50);
-
-	return 0;
 }
 
-static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
+static void mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
 {
+	struct mgag200_pll_values pixpll;
+	int ret;
+
 	switch(mdev->type) {
 	case G200_PCI:
 	case G200_AGP:
-		return mgag200_g200_set_plls(mdev, clock);
+		ret = mgag200_compute_pixpll_values_g200(mdev, clock, &pixpll);
+		break;
 	case G200_SE_A:
 	case G200_SE_B:
-		return mga_g200se_set_plls(mdev, clock);
+		ret = mgag200_compute_pixpll_values_g200se(mdev, clock, &pixpll);
+		break;
 	case G200_WB:
 	case G200_EW3:
-		return mga_g200wb_set_plls(mdev, clock);
+		ret = mgag200_compute_pixpll_values_g200wb(mdev, clock, &pixpll);
+		break;
 	case G200_EV:
-		return mga_g200ev_set_plls(mdev, clock);
+		ret = mgag200_compute_pixpll_values_g200ev(mdev, clock, &pixpll);
+		break;
 	case G200_EH:
 	case G200_EH3:
-		return mga_g200eh_set_plls(mdev, clock);
+		ret = mgag200_compute_pixpll_values_g200eh(mdev, clock, &pixpll);
+		break;
 	case G200_ER:
-		return mga_g200er_set_plls(mdev, clock);
+		ret = mgag200_compute_pixpll_values_g200er(mdev, clock, &pixpll);
+		break;
 	}
 
-	return 0;
+	if (ret)
+		return;
+
+	switch (mdev->type) {
+	case G200_PCI:
+	case G200_AGP:
+		mgag200_set_pixpll_g200(mdev, &pixpll);
+		break;
+	case G200_SE_A:
+	case G200_SE_B:
+		mgag200_set_pixpll_g200se(mdev, &pixpll);
+		break;
+	case G200_WB:
+	case G200_EW3:
+		mgag200_set_pixpll_g200wb(mdev, &pixpll);
+		break;
+	case G200_EV:
+		mgag200_set_pixpll_g200ev(mdev, &pixpll);
+		break;
+	case G200_EH:
+	case G200_EH3:
+		mgag200_set_pixpll_g200eh(mdev, &pixpll);
+		break;
+	case G200_ER:
+		mgag200_set_pixpll_g200er(mdev, &pixpll);
+		break;
+	}
 }
 
 static void mgag200_g200wb_hold_bmc(struct mga_device *mdev)