[PATCH 0/7] drm/mgag200: Inline prepare/commit helpers

[PATCH 0/7] drm/mgag200: Inline prepare/commit helpers

[Thomas Zimmermann]

Mgag200's modesetting still utilizes the prepare and commit functions
that were part of the non-atomic interface. This patchset integrates
them into the simple-display's enable function.

Patch 1 disables CRTC write protection once when initializing the
registers. Before, CRTC registers were temporarily write-protected for
no apparent reason.

The screen is toggled on and off multiple times while setting a mode.
Patch 3 removes any intermediate screen on/off changes.

In patch 4, DPMS functionality is reduced to on an off. The DPMS helper
function implemements on, off, suspend and standby. In atomic modesetting,
only on and off is required.

Tested on G200SE HW with Xorg, Weston and fbdev.

Thomas Zimmermann (7):
  drm/mgag200: Don't write-protect CRTC 0-7 while in mga_crtc_prepare()
  drm/mgag200: Move PLL setup out of mode-setting function
  drm/mgag200: Don't set or clear <scroff> field during modeset
  drm/mgag200: Split DPMS function into helpers
  drm/mgag200: Set/clear <syncrst> field in display enable/disable
    helpers
  drm/mgag200: Rename G200WB prepare/commit function
  drm/mgag200: Inline mga_crtc_{prepare,commit}() into enable function

 drivers/gpu/drm/mgag200/mgag200_drv.h  |   6 +
 drivers/gpu/drm/mgag200/mgag200_mode.c | 168 +++++++++----------------
 drivers/gpu/drm/mgag200/mgag200_reg.h  |  11 ++
 3 files changed, 76 insertions(+), 109 deletions(-)

--
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/7] drm/mgag200: Don't write-protect CRTC 0-7 while in mga_crtc_prepare()

[Thomas Zimmermann]

The prepare function write-protects several registers that it doesn't
even touch. Removed the related code.

The code for unprotecting registers also clears VINT interrupts. Both
is now done once during initialization.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  6 ++++++
 drivers/gpu/drm/mgag200/mgag200_mode.c | 16 +++++++---------
 drivers/gpu/drm/mgag200/mgag200_reg.h  |  5 +++++
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 270c2f9a6766..3817520bfefc 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -60,6 +60,12 @@
 		WREG8(MGAREG_SEQ_DATA, v);			\
 	} while (0)						\
 
+#define RREG_CRT(reg, v)					\
+	do {							\
+		WREG8(MGAREG_CRTC_INDEX, reg);			\
+		v = RREG8(MGAREG_CRTC_DATA);			\
+	} while (0)						\
+
 #define WREG_CRT(reg, v)					\
 	do {							\
 		WREG8(MGAREG_CRTC_INDEX, reg);			\
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index f16bd278ab7e..9037057d3b3a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -988,7 +988,7 @@ static void mgag200_set_dac_regs(struct mga_device *mdev)
 
 static void mgag200_init_regs(struct mga_device *mdev)
 {
-	u8 crtcext3, crtcext4, misc;
+	u8 crtc11, crtcext3, crtcext4, misc;
 
 	mgag200_set_pci_regs(mdev);
 	mgag200_set_dac_regs(mdev);
@@ -1012,6 +1012,12 @@ static void mgag200_init_regs(struct mga_device *mdev)
 	WREG_ECRT(0x03, crtcext3);
 	WREG_ECRT(0x04, crtcext4);
 
+	RREG_CRT(0x11, crtc11);
+	crtc11 &= ~(MGAREG_CRTC11_CRTCPROTECT |
+		    MGAREG_CRTC11_VINTEN |
+		    MGAREG_CRTC11_VINTCLR);
+	WREG_CRT(0x11, crtc11);
+
 	if (mdev->type == G200_ER)
 		WREG_ECRT(0x24, 0x5);
 
@@ -1337,12 +1343,6 @@ static void mga_crtc_prepare(struct drm_crtc *crtc)
 	struct mga_device *mdev = to_mga_device(dev);
 	u8 tmp;
 
-	/*	mga_resume(crtc);*/
-
-	WREG8(MGAREG_CRTC_INDEX, 0x11);
-	tmp = RREG8(MGAREG_CRTC_DATA);
-	WREG_CRT(0x11, tmp | 0x80);
-
 	if (mdev->type == G200_SE_A || mdev->type == G200_SE_B) {
 		WREG_SEQ(0, 1);
 		msleep(50);
@@ -1359,8 +1359,6 @@ static void mga_crtc_prepare(struct drm_crtc *crtc)
 
 	if (mdev->type == G200_WB || mdev->type == G200_EW3)
 		mga_g200wb_prepare(crtc);
-
-	WREG_CRT(17, 0);
 }
 
 /*
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h
index 29f7194faadc..fb5da410804c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_reg.h
+++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
@@ -240,6 +240,11 @@
 
 #define MGAREG_CRTC_INDEX	0x1fd4
 #define MGAREG_CRTC_DATA	0x1fd5
+
+#define MGAREG_CRTC11_VINTCLR		BIT(4)
+#define MGAREG_CRTC11_VINTEN		BIT(5)
+#define MGAREG_CRTC11_CRTCPROTECT	BIT(7)
+
 #define MGAREG_CRTCEXT_INDEX	0x1fde
 #define MGAREG_CRTCEXT_DATA	0x1fdf
 
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/7] drm/mgag200: Move PLL setup out of mode-setting function

[Thomas Zimmermann]

Makes the code slightly more flexible. No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 9037057d3b3a..d9612f531e52 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -712,7 +712,7 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock)
 	return 0;
 }
 
-static int mga_crtc_set_plls(struct mga_device *mdev, long clock)
+static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
 {
 	u8 misc;
 
@@ -1110,8 +1110,6 @@ static void mgag200_set_mode_regs(struct mga_device *mdev,
 	WREG_ECRT(0x05, crtcext5);
 
 	WREG8(MGA_MISC_OUT, misc);
-
-	mga_crtc_set_plls(mdev, mode->clock);
 }
 
 static u8 mgag200_get_bpp_shift(struct mga_device *mdev,
@@ -1614,6 +1612,7 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	mgag200_set_format_regs(mdev, fb);
 	mgag200_set_mode_regs(mdev, adjusted_mode);
+	mgag200_crtc_set_plls(mdev, adjusted_mode->clock);
 
 	if (mdev->type == G200_ER)
 		mgag200_g200er_reset_tagfifo(mdev);
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/7] drm/mgag200: Don't set or clear <scroff> field during modeset

[Thomas Zimmermann]

The simple pipe's disable function disables the screen by calling
mgag200_disable_screen(). The simple pipe's enable function enables the
screen by calling mgag200_enable_display().

During modeset operations the screen is off and remains off. It's only
enabled after the modeset has been completed. Therefore remove all code
that sets or clears the <scroff> field while in modeset.

The related code also modifies the <syncrst> field in SEQ0. For now, keep
this code in place.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 42 ++------------------------
 1 file changed, 3 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index d9612f531e52..f0f8b7258f8c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1219,14 +1219,8 @@ static void mgag200_set_format_regs(struct mga_device *mdev,
 static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev)
 {
 	static uint32_t RESET_FLAG = 0x00200000; /* undocumented magic value */
-	u8 seq1;
 	u32 memctl;
 
-	/* screen off */
-	RREG_SEQ(0x01, seq1);
-	seq1 |= MGAREG_SEQ1_SCROFF;
-	WREG_SEQ(0x01, seq1);
-
 	memctl = RREG32(MGAREG_MEMCTL);
 
 	memctl |= RESET_FLAG;
@@ -1236,11 +1230,6 @@ static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev)
 
 	memctl &= ~RESET_FLAG;
 	WREG32(MGAREG_MEMCTL, memctl);
-
-	/* screen on */
-	RREG_SEQ(0x01, seq1);
-	seq1 &= ~MGAREG_SEQ1_SCROFF;
-	WREG_SEQ(0x01, seq1);
 }
 
 static void mgag200_g200se_set_hiprilvl(struct mga_device *mdev,
@@ -1339,21 +1328,9 @@ static void mga_crtc_prepare(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = to_mga_device(dev);
-	u8 tmp;
-
-	if (mdev->type == G200_SE_A || mdev->type == G200_SE_B) {
-		WREG_SEQ(0, 1);
-		msleep(50);
-		WREG_SEQ(1, 0x20);
-		msleep(20);
-	} else {
-		WREG8(MGAREG_SEQ_INDEX, 0x1);
-		tmp = RREG8(MGAREG_SEQ_DATA);
 
-		/* start sync reset */
-		WREG_SEQ(0, 1);
-		WREG_SEQ(1, tmp | 0x20);
-	}
+	/* start sync reset */
+	WREG_SEQ(0, 1);
 
 	if (mdev->type == G200_WB || mdev->type == G200_EW3)
 		mga_g200wb_prepare(crtc);
@@ -1367,24 +1344,11 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = to_mga_device(dev);
-	u8 tmp;
 
 	if (mdev->type == G200_WB || mdev->type == G200_EW3)
 		mga_g200wb_commit(crtc);
 
-	if (mdev->type == G200_SE_A || mdev->type == G200_SE_B) {
-		msleep(50);
-		WREG_SEQ(1, 0x0);
-		msleep(20);
-		WREG_SEQ(0, 0x3);
-	} else {
-		WREG8(MGAREG_SEQ_INDEX, 0x1);
-		tmp = RREG8(MGAREG_SEQ_DATA);
-
-		tmp &= ~0x20;
-		WREG_SEQ(0x1, tmp);
-		WREG_SEQ(0, 3);
-	}
+	WREG_SEQ(0, 0x3);
 	mga_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
 }
 
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/7] drm/mgag200: Split DPMS function into helpers

[Thomas Zimmermann]

Of the DPMS code, only ON and OFF states are used. Simplify mode setting
by moving both into separate functions and removing the rest.

The DPMS code also set the LUT before enabling the screen. The patch moves
this code into the simple-display pipe's enable function.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 73 +++++++++++++++-----------
 drivers/gpu/drm/mgag200/mgag200_reg.h  |  3 ++
 2 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index f0f8b7258f8c..05f8aa50b908 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1282,41 +1282,50 @@ static void mgag200_g200ev_set_hiprilvl(struct mga_device *mdev)
 	WREG_ECRT(0x06, 0x00);
 }
 
-static void mga_crtc_dpms(struct drm_crtc *crtc, int mode)
+static void mgag200_enable_display(struct mga_device *mdev)
 {
-	struct drm_device *dev = crtc->dev;
-	struct mga_device *mdev = to_mga_device(dev);
-	u8 seq1 = 0, crtcext1 = 0;
+	u8 seq1, crtcext1;
 
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
-		seq1 = 0;
-		crtcext1 = 0;
-		mga_crtc_load_lut(crtc);
-		break;
-	case DRM_MODE_DPMS_STANDBY:
-		seq1 = 0x20;
-		crtcext1 = 0x10;
-		break;
-	case DRM_MODE_DPMS_SUSPEND:
-		seq1 = 0x20;
-		crtcext1 = 0x20;
-		break;
-	case DRM_MODE_DPMS_OFF:
-		seq1 = 0x20;
-		crtcext1 = 0x30;
-		break;
-	}
+	/*
+	 * TODO: replace busy waiting with vblank IRQ; put
+	 *       msleep(50) before changing SCROFF
+	 */
+	mga_wait_vsync(mdev);
+	mga_wait_busy(mdev);
+
+	RREG_SEQ(0x01, seq1);
+	seq1 &= ~MGAREG_SEQ1_SCROFF;
+	WREG_SEQ(0x01, seq1);
+
+	msleep(20);
+
+	RREG_ECRT(0x01, crtcext1);
+	crtcext1 &= ~MGAREG_CRTCEXT1_VSYNCOFF;
+	crtcext1 &= ~MGAREG_CRTCEXT1_HSYNCOFF;
+	WREG_ECRT(0x01, crtcext1);
+}
 
-	WREG8(MGAREG_SEQ_INDEX, 0x01);
-	seq1 |= RREG8(MGAREG_SEQ_DATA) & ~0x20;
+static void mgag200_disable_display(struct mga_device *mdev)
+{
+	u8 seq1, crtcext1;
+
+	/*
+	 * TODO: replace busy waiting with vblank IRQ; put
+	 *       msleep(50) before changing SCROFF
+	 */
 	mga_wait_vsync(mdev);
 	mga_wait_busy(mdev);
-	WREG8(MGAREG_SEQ_DATA, seq1);
+
+	RREG_SEQ(0x01, seq1);
+	seq1 |= MGAREG_SEQ1_SCROFF;
+	WREG_SEQ(0x01, seq1);
+
 	msleep(20);
-	WREG8(MGAREG_CRTCEXT_INDEX, 0x01);
-	crtcext1 |= RREG8(MGAREG_CRTCEXT_DATA) & ~0x30;
-	WREG8(MGAREG_CRTCEXT_DATA, crtcext1);
+
+	RREG_ECRT(0x01, crtcext1);
+	crtcext1 |= MGAREG_CRTCEXT1_VSYNCOFF |
+		    MGAREG_CRTCEXT1_HSYNCOFF;
+	WREG_ECRT(0x01, crtcext1);
 }
 
 /*
@@ -1349,7 +1358,8 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
 		mga_g200wb_commit(crtc);
 
 	WREG_SEQ(0, 0x3);
-	mga_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
+	mga_crtc_load_lut(crtc);
+	mgag200_enable_display(mdev);
 }
 
 /*
@@ -1595,8 +1605,9 @@ static void
 mgag200_simple_display_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
 	struct drm_crtc *crtc = &pipe->crtc;
+	struct mga_device *mdev = to_mga_device(crtc->dev);
 
-	mga_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
+	mgag200_disable_display(mdev);
 }
 
 static int
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h
index fb5da410804c..9f0be1878854 100644
--- a/drivers/gpu/drm/mgag200/mgag200_reg.h
+++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
@@ -250,6 +250,9 @@
 
 #define MGAREG_CRTCEXT0_OFFSET_MASK	GENMASK(5, 4)
 
+#define MGAREG_CRTCEXT1_VSYNCOFF	BIT(5)
+#define MGAREG_CRTCEXT1_HSYNCOFF	BIT(4)
+
 /* Cursor X and Y position */
 #define MGA_CURPOSXL 0x3c0c
 #define MGA_CURPOSXH 0x3c0d
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/7] drm/mgag200: Set/clear <syncrst> field in display enable/disable helpers

[Thomas Zimmermann]

Modifying the <syncrst> field in mgag200_{enable,disable}_display()
makes the code more readable. Also clear the <asyncrst> field to enable
the display. The other bits in SEQ0 are unused, so no functional changes
are made.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 17 +++++++++++------
 drivers/gpu/drm/mgag200/mgag200_reg.h  |  3 +++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 05f8aa50b908..679eb5abe24d 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1284,7 +1284,12 @@ static void mgag200_g200ev_set_hiprilvl(struct mga_device *mdev)
 
 static void mgag200_enable_display(struct mga_device *mdev)
 {
-	u8 seq1, crtcext1;
+	u8 seq0, seq1, crtcext1;
+
+	RREG_SEQ(0x00, seq0);
+	seq0 |= MGAREG_SEQ0_SYNCRST |
+		MGAREG_SEQ0_ASYNCRST;
+	WREG_SEQ(0x00, seq0);
 
 	/*
 	 * TODO: replace busy waiting with vblank IRQ; put
@@ -1307,7 +1312,11 @@ static void mgag200_enable_display(struct mga_device *mdev)
 
 static void mgag200_disable_display(struct mga_device *mdev)
 {
-	u8 seq1, crtcext1;
+	u8 seq0, seq1, crtcext1;
+
+	RREG_SEQ(0x00, seq0);
+	seq0 &= ~MGAREG_SEQ0_SYNCRST;
+	WREG_SEQ(0x00, seq0);
 
 	/*
 	 * TODO: replace busy waiting with vblank IRQ; put
@@ -1338,9 +1347,6 @@ static void mga_crtc_prepare(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = to_mga_device(dev);
 
-	/* start sync reset */
-	WREG_SEQ(0, 1);
-
 	if (mdev->type == G200_WB || mdev->type == G200_EW3)
 		mga_g200wb_prepare(crtc);
 }
@@ -1357,7 +1363,6 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
 	if (mdev->type == G200_WB || mdev->type == G200_EW3)
 		mga_g200wb_commit(crtc);
 
-	WREG_SEQ(0, 0x3);
 	mga_crtc_load_lut(crtc);
 	mgag200_enable_display(mdev);
 }
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h
index 9f0be1878854..c3b7bcad52ed 100644
--- a/drivers/gpu/drm/mgag200/mgag200_reg.h
+++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
@@ -236,6 +236,9 @@
 #define MGAREG_SEQ_INDEX	0x1fc4
 #define MGAREG_SEQ_DATA		0x1fc5
 
+#define MGAREG_SEQ0_ASYNCRST	BIT(0)
+#define MGAREG_SEQ0_SYNCRST	BIT(1)
+
 #define MGAREG_SEQ1_SCROFF	BIT(5)
 
 #define MGAREG_CRTC_INDEX	0x1fd4
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/7] drm/mgag200: Rename G200WB prepare/commit function

[Thomas Zimmermann]

The prepare and commit helpers for G200WB devices control the BMC.
Rename them accordingly. While at it, also change the passed value's
type to struct mga_device and remove some type upcasting.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 679eb5abe24d..4f9007f1a554 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -745,9 +745,8 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
 	return 0;
 }
 
-static void mga_g200wb_prepare(struct drm_crtc *crtc)
+static void mgag200_g200wb_hold_bmc(struct mga_device *mdev)
 {
-	struct mga_device *mdev = to_mga_device(crtc->dev);
 	u8 tmp;
 	int iter_max;
 
@@ -799,10 +798,9 @@ static void mga_g200wb_prepare(struct drm_crtc *crtc)
 	}
 }
 
-static void mga_g200wb_commit(struct drm_crtc *crtc)
+static void mgag200_g200wb_release_bmc(struct mga_device *mdev)
 {
 	u8 tmp;
-	struct mga_device *mdev = to_mga_device(crtc->dev);
 
 	/* 1- The first step is to ensure that the vrsten and hrsten are set */
 	WREG8(MGAREG_CRTCEXT_INDEX, 1);
@@ -1348,7 +1346,7 @@ static void mga_crtc_prepare(struct drm_crtc *crtc)
 	struct mga_device *mdev = to_mga_device(dev);
 
 	if (mdev->type == G200_WB || mdev->type == G200_EW3)
-		mga_g200wb_prepare(crtc);
+		mgag200_g200wb_hold_bmc(mdev);
 }
 
 /*
@@ -1361,7 +1359,7 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
 	struct mga_device *mdev = to_mga_device(dev);
 
 	if (mdev->type == G200_WB || mdev->type == G200_EW3)
-		mga_g200wb_commit(crtc);
+		mgag200_g200wb_release_bmc(mdev);
 
 	mga_crtc_load_lut(crtc);
 	mgag200_enable_display(mdev);
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/7] drm/mgag200: Inline mga_crtc_{prepare, commit}() into enable function

[Thomas Zimmermann]

There's only trivial code left in mga_crtc_{prepare,commit}(). Merge the
functions into the simple pipe's enable function.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 39 +++++---------------------
 1 file changed, 7 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 4f9007f1a554..e0d037a7413c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1335,36 +1335,6 @@ static void mgag200_disable_display(struct mga_device *mdev)
 	WREG_ECRT(0x01, crtcext1);
 }
 
-/*
- * This is called before a mode is programmed. A typical use might be to
- * enable DPMS during the programming to avoid seeing intermediate stages,
- * but that's not relevant to us
- */
-static void mga_crtc_prepare(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct mga_device *mdev = to_mga_device(dev);
-
-	if (mdev->type == G200_WB || mdev->type == G200_EW3)
-		mgag200_g200wb_hold_bmc(mdev);
-}
-
-/*
- * This is called after a mode is programmed. It should reverse anything done
- * by the prepare function
- */
-static void mga_crtc_commit(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct mga_device *mdev = to_mga_device(dev);
-
-	if (mdev->type == G200_WB || mdev->type == G200_EW3)
-		mgag200_g200wb_release_bmc(mdev);
-
-	mga_crtc_load_lut(crtc);
-	mgag200_enable_display(mdev);
-}
-
 /*
  * Connector
  */
@@ -1585,7 +1555,8 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 		.y2 = fb->height,
 	};
 
-	mga_crtc_prepare(crtc);
+	if (mdev->type == G200_WB || mdev->type == G200_EW3)
+		mgag200_g200wb_hold_bmc(mdev);
 
 	mgag200_set_format_regs(mdev, fb);
 	mgag200_set_mode_regs(mdev, adjusted_mode);
@@ -1599,7 +1570,11 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	else if (mdev->type == G200_EV)
 		mgag200_g200ev_set_hiprilvl(mdev);
 
-	mga_crtc_commit(crtc);
+	if (mdev->type == G200_WB || mdev->type == G200_EW3)
+		mgag200_g200wb_release_bmc(mdev);
+
+	mga_crtc_load_lut(crtc);
+	mgag200_enable_display(mdev);
 
 	mgag200_handle_damage(mdev, fb, &fullscreen);
 }
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/7] drm/mgag200: Inline prepare/commit helpers

[Lyude Paul]

For context - I already reviewed this once, it just missed the dri-devel list by
accident

For the whole series:

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Tue, 2020-07-07 at 10:24 +0200, Thomas Zimmermann wrote:
> Mgag200's modesetting still utilizes the prepare and commit functions
> that were part of the non-atomic interface. This patchset integrates
> them into the simple-display's enable function.
> 
> Patch 1 disables CRTC write protection once when initializing the
> registers. Before, CRTC registers were temporarily write-protected for
> no apparent reason.
> 
> The screen is toggled on and off multiple times while setting a mode.
> Patch 3 removes any intermediate screen on/off changes.
> 
> In patch 4, DPMS functionality is reduced to on an off. The DPMS helper
> function implemements on, off, suspend and standby. In atomic modesetting,
> only on and off is required.
> 
> Tested on G200SE HW with Xorg, Weston and fbdev.
> 
> Thomas Zimmermann (7):
>   drm/mgag200: Don't write-protect CRTC 0-7 while in mga_crtc_prepare()
>   drm/mgag200: Move PLL setup out of mode-setting function
>   drm/mgag200: Don't set or clear <scroff> field during modeset
>   drm/mgag200: Split DPMS function into helpers
>   drm/mgag200: Set/clear <syncrst> field in display enable/disable
>     helpers
>   drm/mgag200: Rename G200WB prepare/commit function
>   drm/mgag200: Inline mga_crtc_{prepare,commit}() into enable function
> 
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |   6 +
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 168 +++++++++----------------
>  drivers/gpu/drm/mgag200/mgag200_reg.h  |  11 ++
>  3 files changed, 76 insertions(+), 109 deletions(-)
> 
> --
> 2.27.0
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/7] drm/mgag200: Inline prepare/commit helpers

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2366 bytes --]

Hi

Am 07.07.20 um 18:40 schrieb Lyude Paul:
> For context - I already reviewed this once, it just missed the dri-devel list by
> accident
> 
> For the whole series:
> 
> Reviewed-by: Lyude Paul <lyude@redhat.com>

I pushed the patches into drm-misc-next and also added a comment in the
commit message on the busy-waiting thing you asked about.

Best regards
Thomas

> 
> On Tue, 2020-07-07 at 10:24 +0200, Thomas Zimmermann wrote:
>> Mgag200's modesetting still utilizes the prepare and commit functions
>> that were part of the non-atomic interface. This patchset integrates
>> them into the simple-display's enable function.
>>
>> Patch 1 disables CRTC write protection once when initializing the
>> registers. Before, CRTC registers were temporarily write-protected for
>> no apparent reason.
>>
>> The screen is toggled on and off multiple times while setting a mode.
>> Patch 3 removes any intermediate screen on/off changes.
>>
>> In patch 4, DPMS functionality is reduced to on an off. The DPMS helper
>> function implemements on, off, suspend and standby. In atomic modesetting,
>> only on and off is required.
>>
>> Tested on G200SE HW with Xorg, Weston and fbdev.
>>
>> Thomas Zimmermann (7):
>>   drm/mgag200: Don't write-protect CRTC 0-7 while in mga_crtc_prepare()
>>   drm/mgag200: Move PLL setup out of mode-setting function
>>   drm/mgag200: Don't set or clear <scroff> field during modeset
>>   drm/mgag200: Split DPMS function into helpers
>>   drm/mgag200: Set/clear <syncrst> field in display enable/disable
>>     helpers
>>   drm/mgag200: Rename G200WB prepare/commit function
>>   drm/mgag200: Inline mga_crtc_{prepare,commit}() into enable function
>>
>>  drivers/gpu/drm/mgag200/mgag200_drv.h  |   6 +
>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 168 +++++++++----------------
>>  drivers/gpu/drm/mgag200/mgag200_reg.h  |  11 ++
>>  3 files changed, 76 insertions(+), 109 deletions(-)
>>
>> --
>> 2.27.0
>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel