[PATCH v2 00/15] drm/mgag200: Convert to atomic modesetting

[PATCH v2 00/15] drm/mgag200: Convert to atomic modesetting

[Thomas Zimmermann]

This patchset converts mgag200 to atomic modesetting. It uses simple
KMS helpers and SHMEM.

Patch 1 removes cursor support. The HW cursor is not usable with the
way universal planes work.

Patches 2 to 11 untangle the existing modesetting code into smaller
functions. Specifically, mode setting and plane updates are being
separated from each other.

Patch 12 to 14 convert mgag200 to simple KMS helpers and enables atomic
mode setting.

Atomically switching plane framebuffers, requires either source or target
buffer to be located at a non-0 offet. As some HW revisions seem to require
a framebuffer offset of 0 within the video memory, they do not work with
atomic modesetting. To resolve this problem, patch 15 converts mgag200
from VRAM helpers to SHMEM helpers. During plane updates, the content of
the SHMEM BO is memcpy'd to VRAM. From my observation, performance is not
nuch different from the original code.

The patchset has been tested on MGA G200EH hardware.

v2:
	* rebase patchset
	* replace uint{8,32}_t with u{8,32} through-out patchset
	* define additional register constants
	* use helper functions around bpp-shift computations
	* split conversion patch
	* cleanups

Thomas Zimmermann (15):
  drm/mgag200: Remove HW cursor
  drm/mgag200: Clean up mga_set_start_address()
  drm/mgag200: Clean up mga_crtc_do_set_base()
  drm/mgag200: Move mode-setting code into separate helper function
  drm/mgag200: Split MISC register update into PLL selection, SYNC and
    I/O
  drm/mgag200: Update mode registers after plane registers
  drm/mgag200: Set pitch in a separate helper function
  drm/mgag200: Set primary plane's format in separate helper function
  drm/mgag200: Move TAGFIFO reset into separate function
  drm/mgag200: Move hiprilvl setting into separate functions
  drm/mgag200: Move register initialization into separate function
  drm/mgag200: Remove out-commented suspend/resume helpers
  drm/mgag200: Use simple-display data structures
  drm/mgag200: Convert to simple KMS helper
  drm/mgag200: Replace VRAM helpers with SHMEM helpers

 drivers/gpu/drm/mgag200/Kconfig        |   4 +-
 drivers/gpu/drm/mgag200/Makefile       |   2 +-
 drivers/gpu/drm/mgag200/mgag200_drv.c  |  51 +-
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  41 +-
 drivers/gpu/drm/mgag200/mgag200_main.c |   5 -
 drivers/gpu/drm/mgag200/mgag200_mode.c | 871 ++++++++++++++-----------
 drivers/gpu/drm/mgag200/mgag200_reg.h  |  11 +-
 drivers/gpu/drm/mgag200/mgag200_ttm.c  |  28 +-
 8 files changed, 528 insertions(+), 485 deletions(-)

--
2.26.2

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

[PATCH v2 01/15] drm/mgag200: Remove HW cursor

[Thomas Zimmermann]

The HW cursor of Matrox G200 cards only supports a 16-color palette
format. Univeral planes require at least ARGB or a similar component-
based format, so remove the HW cursor.

Alternatively, the driver could dither a cursor image from ARGB to
16 colors. But this does not produce pleasent-looking results in
general, so it's useless for modern compositors.

Without HW support, compositors will use software rendering.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/Makefile       |  2 +-
 drivers/gpu/drm/mgag200/mgag200_drv.h  | 13 -------------
 drivers/gpu/drm/mgag200/mgag200_main.c |  5 -----
 drivers/gpu/drm/mgag200/mgag200_mode.c |  2 --
 4 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/Makefile b/drivers/gpu/drm/mgag200/Makefile
index 04b281bcf6558..63403133638a3 100644
--- a/drivers/gpu/drm/mgag200/Makefile
+++ b/drivers/gpu/drm/mgag200/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
-mgag200-y   := mgag200_main.o mgag200_mode.o mgag200_cursor.o \
+mgag200-y   := mgag200_main.o mgag200_mode.o \
 	mgag200_drv.o mgag200_i2c.o mgag200_ttm.o
 
 obj-$(CONFIG_DRM_MGAG200) += mgag200.o
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index d9b7e96b214f8..bc372c2ec79e9 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -116,11 +116,6 @@ struct mga_connector {
 	struct mga_i2c_chan *i2c;
 };
 
-struct mga_cursor {
-	struct drm_gem_vram_object *gbo[2];
-	unsigned int next_index;
-};
-
 struct mga_mc {
 	resource_size_t			vram_size;
 	resource_size_t			vram_base;
@@ -156,8 +151,6 @@ struct mga_device {
 
 	struct mga_mc			mc;
 
-	struct mga_cursor cursor;
-
 	size_t vram_fb_available;
 
 	bool				suspended;
@@ -207,10 +200,4 @@ int mgag200_mm_init(struct mga_device *mdev);
 void mgag200_mm_fini(struct mga_device *mdev);
 int mgag200_mmap(struct file *filp, struct vm_area_struct *vma);
 
-int mgag200_cursor_init(struct mga_device *mdev);
-void mgag200_cursor_fini(struct mga_device *mdev);
-int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
-			    uint32_t handle, uint32_t width, uint32_t height);
-int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
-
 #endif				/* __MGAG200_DRV_H__ */
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 86df799fd38c5..3298eff7bd1b4 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -135,10 +135,6 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 		goto err_mgag200_mm_fini;
 	}
 
-	ret = mgag200_cursor_init(mdev);
-	if (ret)
-		drm_err(dev, "Could not initialize cursors. Not doing hardware cursors.\n");
-
 	return 0;
 
 err_mgag200_mm_fini:
@@ -154,7 +150,6 @@ void mgag200_driver_unload(struct drm_device *dev)
 
 	if (mdev == NULL)
 		return;
-	mgag200_cursor_fini(mdev);
 	mgag200_mm_fini(mdev);
 	dev->dev_private = NULL;
 }
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 5f4ac36a97760..c68ed8b6faf9b 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1412,8 +1412,6 @@ static void mga_crtc_disable(struct drm_crtc *crtc)
 
 /* These provide the minimum set of functions required to handle a CRTC */
 static const struct drm_crtc_funcs mga_crtc_funcs = {
-	.cursor_set = mgag200_crtc_cursor_set,
-	.cursor_move = mgag200_crtc_cursor_move,
 	.gamma_set = mga_crtc_gamma_set,
 	.set_config = drm_crtc_helper_set_config,
 	.destroy = mga_crtc_destroy,
-- 
2.26.2

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

[PATCH v2 02/15] drm/mgag200: Clean up mga_set_start_address()

[Thomas Zimmermann]

All register names and fields are now named according to the
MGA programming manuals. The function doesn't need the CRTC, so
callers pass in the device structure directly. The logging now
uses device-specific macros.

v2:
	* use to_mga_device()
	* use MiB instead of MB
	* replace empty while loop with do-while, fixes checkpatch warning
	* replace uint{8,32}_t with u{8,32}

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ++
 drivers/gpu/drm/mgag200/mgag200_mode.c | 82 +++++++++++++++-----------
 2 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index bc372c2ec79e9..1963876ef3b8c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -61,6 +61,11 @@
 		WREG8(MGAREG_CRTC_DATA, v);			\
 	} while (0)						\
 
+#define RREG_ECRT(reg, v)					\
+	do {							\
+		WREG8(MGAREG_CRTCEXT_INDEX, reg);		\
+		v = RREG8(MGAREG_CRTCEXT_DATA);			\
+	} while (0)						\
 
 #define WREG_ECRT(reg, v)					\
 	do {							\
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index c68ed8b6faf9b..80a3a805c0c4e 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -819,49 +819,53 @@ static void mga_g200wb_commit(struct drm_crtc *crtc)
 }
 
 /*
-   This is how the framebuffer base address is stored in g200 cards:
-   * Assume @offset is the gpu_addr variable of the framebuffer object
-   * Then addr is the number of _pixels_ (not bytes) from the start of
-     VRAM to the first pixel we want to display. (divided by 2 for 32bit
-     framebuffers)
-   * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers
-   addr<20> -> CRTCEXT0<6>
-   addr<19-16> -> CRTCEXT0<3-0>
-   addr<15-8> -> CRTCC<7-0>
-   addr<7-0> -> CRTCD<7-0>
-   CRTCEXT0 has to be programmed last to trigger an update and make the
-   new addr variable take effect.
+ * This is how the framebuffer base address is stored in g200 cards:
+ *   * Assume @offset is the gpu_addr variable of the framebuffer object
+ *   * Then addr is the number of _pixels_ (not bytes) from the start of
+ *     VRAM to the first pixel we want to display. (divided by 2 for 32bit
+ *     framebuffers)
+ *   * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers
+ *      addr<20> -> CRTCEXT0<6>
+ *      addr<19-16> -> CRTCEXT0<3-0>
+ *      addr<15-8> -> CRTCC<7-0>
+ *      addr<7-0> -> CRTCD<7-0>
+ *
+ *  CRTCEXT0 has to be programmed last to trigger an update and make the
+ *  new addr variable take effect.
  */
-static void mga_set_start_address(struct drm_crtc *crtc, unsigned offset)
+static void mgag200_set_startadd(struct mga_device *mdev,
+				 unsigned long offset)
 {
-	struct mga_device *mdev = to_mga_device(crtc->dev);
-	u32 addr;
-	int count;
-	u8 crtcext0;
+	struct drm_device *dev = mdev->dev;
+	u32 startadd;
+	u8 crtcc, crtcd, crtcext0;
 
-	while (RREG8(0x1fda) & 0x08);
-	while (!(RREG8(0x1fda) & 0x08));
+	startadd = offset / 8;
 
-	count = RREG8(MGAREG_VCOUNT) + 2;
-	while (RREG8(MGAREG_VCOUNT) < count);
-
-	WREG8(MGAREG_CRTCEXT_INDEX, 0);
-	crtcext0 = RREG8(MGAREG_CRTCEXT_DATA);
-	crtcext0 &= 0xB0;
-	addr = offset / 8;
-	/* Can't store addresses any higher than that...
-	   but we also don't have more than 16MB of memory, so it should be fine. */
-	WARN_ON(addr > 0x1fffff);
-	crtcext0 |= (!!(addr & (1<<20)))<<6;
-	WREG_CRT(0x0d, (u8)(addr & 0xff));
-	WREG_CRT(0x0c, (u8)(addr >> 8) & 0xff);
-	WREG_ECRT(0x0, ((u8)(addr >> 16) & 0xf) | crtcext0);
+	/*
+	 * Can't store addresses any higher than that, but we also
+	 * don't have more than 16 MiB of memory, so it should be fine.
+	 */
+	drm_WARN_ON(dev, startadd > 0x1fffff);
+
+	RREG_ECRT(0x00, crtcext0);
+
+	crtcc = (startadd >> 8) & 0xff;
+	crtcd = startadd & 0xff;
+	crtcext0 &= 0xb0;
+	crtcext0 |= ((startadd >> 14) & BIT(6)) |
+		    ((startadd >> 16) & 0x0f);
+
+	WREG_CRT(0x0c, crtcc);
+	WREG_CRT(0x0d, crtcd);
+	WREG_ECRT(0x00, crtcext0);
 }
 
 static int mga_crtc_do_set_base(struct drm_crtc *crtc,
 				struct drm_framebuffer *fb,
 				int x, int y, int atomic)
 {
+	struct mga_device *mdev = to_mga_device(crtc->dev);
 	struct drm_gem_vram_object *gbo;
 	int ret;
 	s64 gpu_addr;
@@ -882,7 +886,7 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc,
 		goto err_drm_gem_vram_unpin;
 	}
 
-	mga_set_start_address(crtc, (u32)gpu_addr);
+	mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
 
 	return 0;
 
@@ -894,6 +898,16 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc,
 static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 				  struct drm_framebuffer *old_fb)
 {
+	struct drm_device *dev = crtc->dev;
+	struct mga_device *mdev = dev->dev_private;
+	unsigned int count;
+
+	do { } while (RREG8(0x1fda) & 0x08);
+	do { } while (!(RREG8(0x1fda) & 0x08));
+
+	count = RREG8(MGAREG_VCOUNT) + 2;
+	do { } while (RREG8(MGAREG_VCOUNT) < count);
+
 	return mga_crtc_do_set_base(crtc, old_fb, x, y, 0);
 }
 
-- 
2.26.2

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

[PATCH v2 03/15] drm/mgag200: Clean up mga_crtc_do_set_base()

[Thomas Zimmermann]

The function now only takes the device structure, and the old and new
framebuffers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 80a3a805c0c4e..9aa6addbbb895 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -861,21 +861,20 @@ static void mgag200_set_startadd(struct mga_device *mdev,
 	WREG_ECRT(0x00, crtcext0);
 }
 
-static int mga_crtc_do_set_base(struct drm_crtc *crtc,
-				struct drm_framebuffer *fb,
-				int x, int y, int atomic)
+static int mga_crtc_do_set_base(struct mga_device *mdev,
+				const struct drm_framebuffer *fb,
+				const struct drm_framebuffer *old_fb)
 {
-	struct mga_device *mdev = to_mga_device(crtc->dev);
 	struct drm_gem_vram_object *gbo;
 	int ret;
 	s64 gpu_addr;
 
-	if (!atomic && fb) {
-		gbo = drm_gem_vram_of_gem(fb->obj[0]);
+	if (old_fb) {
+		gbo = drm_gem_vram_of_gem(old_fb->obj[0]);
 		drm_gem_vram_unpin(gbo);
 	}
 
-	gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]);
+	gbo = drm_gem_vram_of_gem(fb->obj[0]);
 
 	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
 	if (ret)
@@ -900,6 +899,7 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 {
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = dev->dev_private;
+	struct drm_framebuffer *fb = crtc->primary->fb;
 	unsigned int count;
 
 	do { } while (RREG8(0x1fda) & 0x08);
@@ -908,7 +908,7 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	count = RREG8(MGAREG_VCOUNT) + 2;
 	do { } while (RREG8(MGAREG_VCOUNT) < count);
 
-	return mga_crtc_do_set_base(crtc, old_fb, x, y, 0);
+	return mga_crtc_do_set_base(mdev, fb, old_fb);
 }
 
 static int mga_crtc_mode_set(struct drm_crtc *crtc,
@@ -1150,7 +1150,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 
 	WREG8(MGA_MISC_OUT, misc);
 
-	mga_crtc_do_set_base(crtc, old_fb, x, y, 0);
+	mga_crtc_do_set_base(mdev, fb, old_fb);
 
 	/* reset tagfifo */
 	if (mdev->type == G200_ER) {
-- 
2.26.2

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

[PATCH v2 04/15] drm/mgag200: Move mode-setting code into separate helper function

[Thomas Zimmermann]

The mode-setting code is now located in mgag200_set_mode_regs(), sans
a few flags that will be moved in a later patch for clarity.

v2:
	* replace uint8_t with u8

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 140 ++++++++++++++-----------
 1 file changed, 78 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 9aa6addbbb895..7c41bd43f79e0 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -911,6 +911,79 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return mga_crtc_do_set_base(mdev, fb, old_fb);
 }
 
+static void mgag200_set_mode_regs(struct mga_device *mdev,
+				  const struct drm_display_mode *mode)
+{
+	unsigned int hdisplay, hsyncstart, hsyncend, htotal;
+	unsigned int vdisplay, vsyncstart, vsyncend, vtotal;
+	u8 misc = 0;
+	u8 crtcext1, crtcext2, crtcext5;
+
+	hdisplay = mode->hdisplay / 8 - 1;
+	hsyncstart = mode->hsync_start / 8 - 1;
+	hsyncend = mode->hsync_end / 8 - 1;
+	htotal = mode->htotal / 8 - 1;
+
+	/* Work around hardware quirk */
+	if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04)
+		htotal++;
+
+	vdisplay = mode->vdisplay - 1;
+	vsyncstart = mode->vsync_start - 1;
+	vsyncend = mode->vsync_end - 1;
+	vtotal = mode->vtotal - 2;
+
+	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
+		misc |= 0x40;
+	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+		misc |= 0x80;
+
+	crtcext1 = (((htotal - 4) & 0x100) >> 8) |
+		   ((hdisplay & 0x100) >> 7) |
+		   ((hsyncstart & 0x100) >> 6) |
+		    (htotal & 0x40);
+	if (mdev->type == G200_WB || mdev->type == G200_EW3)
+		crtcext1 |= BIT(7) | /* vrsten */
+			    BIT(3); /* hrsten */
+
+	crtcext2 = ((vtotal & 0xc00) >> 10) |
+		   ((vdisplay & 0x400) >> 8) |
+		   ((vdisplay & 0xc00) >> 7) |
+		   ((vsyncstart & 0xc00) >> 5) |
+		   ((vdisplay & 0x400) >> 3);
+	crtcext5 = 0x00;
+
+	WREG_CRT(0, htotal - 4);
+	WREG_CRT(1, hdisplay);
+	WREG_CRT(2, hdisplay);
+	WREG_CRT(3, (htotal & 0x1F) | 0x80);
+	WREG_CRT(4, hsyncstart);
+	WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F));
+	WREG_CRT(6, vtotal & 0xFF);
+	WREG_CRT(7, ((vtotal & 0x100) >> 8) |
+		 ((vdisplay & 0x100) >> 7) |
+		 ((vsyncstart & 0x100) >> 6) |
+		 ((vdisplay & 0x100) >> 5) |
+		 ((vdisplay & 0x100) >> 4) | /* linecomp */
+		 ((vtotal & 0x200) >> 4) |
+		 ((vdisplay & 0x200) >> 3) |
+		 ((vsyncstart & 0x200) >> 2));
+	WREG_CRT(9, ((vdisplay & 0x200) >> 4) |
+		 ((vdisplay & 0x200) >> 3));
+	WREG_CRT(16, vsyncstart & 0xFF);
+	WREG_CRT(17, (vsyncend & 0x0F) | 0x20);
+	WREG_CRT(18, vdisplay & 0xFF);
+	WREG_CRT(20, 0);
+	WREG_CRT(21, vdisplay & 0xFF);
+	WREG_CRT(22, (vtotal + 1) & 0xFF);
+	WREG_CRT(23, 0xc3);
+	WREG_CRT(24, vdisplay & 0xFF);
+
+	WREG_ECRT(0x01, crtcext1);
+	WREG_ECRT(0x02, crtcext2);
+	WREG_ECRT(0x05, crtcext5);
+}
+
 static int mga_crtc_mode_set(struct drm_crtc *crtc,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode,
@@ -919,8 +992,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = to_mga_device(dev);
 	const struct drm_framebuffer *fb = crtc->primary->fb;
-	int hdisplay, hsyncstart, hsyncend, htotal;
-	int vdisplay, vsyncstart, vsyncend, vtotal;
 	int pitch;
 	int option = 0, option2 = 0;
 	int i;
@@ -999,12 +1070,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		break;
 	}
 
-	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
-		misc |= 0x40;
-	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
-		misc |= 0x80;
-
-
 	for (i = 0; i < sizeof(dacvalue); i++) {
 		if ((i <= 0x17) ||
 		    (i == 0x1b) ||
@@ -1044,20 +1109,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	else
 		pitch = pitch >> (4 - bppshift);
 
-	hdisplay = mode->hdisplay / 8 - 1;
-	hsyncstart = mode->hsync_start / 8 - 1;
-	hsyncend = mode->hsync_end / 8 - 1;
-	htotal = mode->htotal / 8 - 1;
-
-	/* Work around hardware quirk */
-	if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04)
-		htotal++;
-
-	vdisplay = mode->vdisplay - 1;
-	vsyncstart = mode->vsync_start - 1;
-	vsyncend = mode->vsync_end - 1;
-	vtotal = mode->vtotal - 2;
-
 	WREG_GFX(0, 0);
 	WREG_GFX(1, 0);
 	WREG_GFX(2, 0);
@@ -1068,61 +1119,26 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	WREG_GFX(7, 0xf);
 	WREG_GFX(8, 0xf);
 
-	WREG_CRT(0, htotal - 4);
-	WREG_CRT(1, hdisplay);
-	WREG_CRT(2, hdisplay);
-	WREG_CRT(3, (htotal & 0x1F) | 0x80);
-	WREG_CRT(4, hsyncstart);
-	WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F));
-	WREG_CRT(6, vtotal & 0xFF);
-	WREG_CRT(7, ((vtotal & 0x100) >> 8) |
-		 ((vdisplay & 0x100) >> 7) |
-		 ((vsyncstart & 0x100) >> 6) |
-		 ((vdisplay & 0x100) >> 5) |
-		 ((vdisplay & 0x100) >> 4) | /* linecomp */
-		 ((vtotal & 0x200) >> 4)|
-		 ((vdisplay & 0x200) >> 3) |
-		 ((vsyncstart & 0x200) >> 2));
-	WREG_CRT(9, ((vdisplay & 0x200) >> 4) |
-		 ((vdisplay & 0x200) >> 3));
 	WREG_CRT(10, 0);
 	WREG_CRT(11, 0);
 	WREG_CRT(12, 0);
 	WREG_CRT(13, 0);
 	WREG_CRT(14, 0);
 	WREG_CRT(15, 0);
-	WREG_CRT(16, vsyncstart & 0xFF);
-	WREG_CRT(17, (vsyncend & 0x0F) | 0x20);
-	WREG_CRT(18, vdisplay & 0xFF);
 	WREG_CRT(19, pitch & 0xFF);
-	WREG_CRT(20, 0);
-	WREG_CRT(21, vdisplay & 0xFF);
-	WREG_CRT(22, (vtotal + 1) & 0xFF);
-	WREG_CRT(23, 0xc3);
-	WREG_CRT(24, vdisplay & 0xFF);
+
+	mgag200_set_mode_regs(mdev, mode);
 
 	ext_vga[0] = 0;
-	ext_vga[5] = 0;
 
 	/* TODO interlace */
 
 	ext_vga[0] |= (pitch & 0x300) >> 4;
-	ext_vga[1] = (((htotal - 4) & 0x100) >> 8) |
-		((hdisplay & 0x100) >> 7) |
-		((hsyncstart & 0x100) >> 6) |
-		(htotal & 0x40);
-	ext_vga[2] = ((vtotal & 0xc00) >> 10) |
-		((vdisplay & 0x400) >> 8) |
-		((vdisplay & 0xc00) >> 7) |
-		((vsyncstart & 0xc00) >> 5) |
-		((vdisplay & 0x400) >> 3);
 	if (fb->format->cpp[0] * 8 == 24)
 		ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80;
 	else
 		ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
 	ext_vga[4] = 0;
-	if (mdev->type == G200_WB || mdev->type == G200_EW3)
-		ext_vga[1] |= 0x88;
 
 	/* Set pixel clocks */
 	misc = 0x2d;
@@ -1130,9 +1146,9 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 
 	mga_crtc_set_plls(mdev, mode->clock);
 
-	for (i = 0; i < 6; i++) {
-		WREG_ECRT(i, ext_vga[i]);
-	}
+	WREG_ECRT(0, ext_vga[0]);
+	WREG_ECRT(3, ext_vga[3]);
+	WREG_ECRT(4, ext_vga[4]);
 
 	if (mdev->type == G200_ER)
 		WREG_ECRT(0x24, 0x5);
-- 
2.26.2

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

[PATCH v2 05/15] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O

[Thomas Zimmermann]

Set different fields in MISC in their rsp location in the code. This
patch also fixes a bug in the original code where the mode's SYNC flags
were never written into the MISC register.

v2:
	* use u8 instead of uint8_t
	* define MGAREG_MISC_CLK_SEL_MASK

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 38 ++++++++++++++++++--------
 drivers/gpu/drm/mgag200/mgag200_reg.h  |  6 +++-
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 7c41bd43f79e0..2007d7a4754ac 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -704,6 +704,8 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock)
 
 static int mga_crtc_set_plls(struct mga_device *mdev, long clock)
 {
+	u8 misc;
+
 	switch(mdev->type) {
 	case G200_SE_A:
 	case G200_SE_B:
@@ -724,6 +726,12 @@ static int mga_crtc_set_plls(struct mga_device *mdev, long clock)
 		return mga_g200er_set_plls(mdev, clock);
 		break;
 	}
+
+	misc = RREG8(MGA_MISC_IN);
+	misc &= ~MGAREG_MISC_CLK_SEL_MASK;
+	misc |= MGAREG_MISC_CLK_SEL_MGA_MSK;
+	WREG8(MGA_MISC_OUT, misc);
+
 	return 0;
 }
 
@@ -916,8 +924,7 @@ static void mgag200_set_mode_regs(struct mga_device *mdev,
 {
 	unsigned int hdisplay, hsyncstart, hsyncend, htotal;
 	unsigned int vdisplay, vsyncstart, vsyncend, vtotal;
-	u8 misc = 0;
-	u8 crtcext1, crtcext2, crtcext5;
+	u8 misc, crtcext1, crtcext2, crtcext5;
 
 	hdisplay = mode->hdisplay / 8 - 1;
 	hsyncstart = mode->hsync_start / 8 - 1;
@@ -933,10 +940,17 @@ static void mgag200_set_mode_regs(struct mga_device *mdev,
 	vsyncend = mode->vsync_end - 1;
 	vtotal = mode->vtotal - 2;
 
+	misc = RREG8(MGA_MISC_IN);
+
 	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
-		misc |= 0x40;
+		misc |= MGAREG_MISC_HSYNCPOL;
+	else
+		misc &= ~MGAREG_MISC_HSYNCPOL;
+
 	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
-		misc |= 0x80;
+		misc |= MGAREG_MISC_VSYNCPOL;
+	else
+		misc &= ~MGAREG_MISC_VSYNCPOL;
 
 	crtcext1 = (((htotal - 4) & 0x100) >> 8) |
 		   ((hdisplay & 0x100) >> 7) |
@@ -982,6 +996,10 @@ static void mgag200_set_mode_regs(struct mga_device *mdev,
 	WREG_ECRT(0x01, crtcext1);
 	WREG_ECRT(0x02, crtcext2);
 	WREG_ECRT(0x05, crtcext5);
+
+	WREG8(MGA_MISC_OUT, misc);
+
+	mga_crtc_set_plls(mdev, mode->clock);
 }
 
 static int mga_crtc_mode_set(struct drm_crtc *crtc,
@@ -1140,12 +1158,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
 	ext_vga[4] = 0;
 
-	/* Set pixel clocks */
-	misc = 0x2d;
-	WREG8(MGA_MISC_OUT, misc);
-
-	mga_crtc_set_plls(mdev, mode->clock);
-
 	WREG_ECRT(0, ext_vga[0]);
 	WREG_ECRT(3, ext_vga[3]);
 	WREG_ECRT(4, ext_vga[4]);
@@ -1161,9 +1173,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	}
 
 	WREG_ECRT(0, ext_vga[0]);
-	/* Enable mga pixel clock */
-	misc = 0x2d;
 
+	misc = RREG8(MGA_MISC_IN);
+	misc |= MGAREG_MISC_IOADSEL |
+		MGAREG_MISC_RAMMAPEN |
+		MGAREG_MISC_HIGH_PG_SEL;
 	WREG8(MGA_MISC_OUT, misc);
 
 	mga_crtc_do_set_base(mdev, fb, old_fb);
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h
index c096a9d6bcbc1..0ba6e15e97106 100644
--- a/drivers/gpu/drm/mgag200/mgag200_reg.h
+++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
@@ -16,10 +16,11 @@
  *		MGA1064SG Mystique register file
  */
 
-
 #ifndef _MGA_REG_H_
 #define _MGA_REG_H_
 
+#include <linux/bits.h>
+
 #define	MGAREG_DWGCTL		0x1c00
 #define	MGAREG_MACCESS		0x1c04
 /* the following is a mystique only register */
@@ -221,12 +222,15 @@
 
 #define MGAREG_MISC_IOADSEL	(0x1 << 0)
 #define MGAREG_MISC_RAMMAPEN	(0x1 << 1)
+#define MGAREG_MISC_CLK_SEL_MASK	GENMASK(3, 2)
 #define MGAREG_MISC_CLK_SEL_VGA25	(0x0 << 2)
 #define MGAREG_MISC_CLK_SEL_VGA28	(0x1 << 2)
 #define MGAREG_MISC_CLK_SEL_MGA_PIX	(0x2 << 2)
 #define MGAREG_MISC_CLK_SEL_MGA_MSK	(0x3 << 2)
 #define MGAREG_MISC_VIDEO_DIS	(0x1 << 4)
 #define MGAREG_MISC_HIGH_PG_SEL	(0x1 << 5)
+#define MGAREG_MISC_HSYNCPOL		BIT(6)
+#define MGAREG_MISC_VSYNCPOL		BIT(7)
 
 /* MMIO VGA registers */
 #define MGAREG_SEQ_INDEX	0x1fc4
-- 
2.26.2

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

[PATCH v2 06/15] drm/mgag200: Update mode registers after plane registers

[Thomas Zimmermann]

Setting the plane registers first and the mode registers afterwards
reproduces the sequence used by atomic helpers. Done in preparation
of switching to simple KMS helpers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 2007d7a4754ac..4dba0a379c263 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1145,8 +1145,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	WREG_CRT(15, 0);
 	WREG_CRT(19, pitch & 0xFF);
 
-	mgag200_set_mode_regs(mdev, mode);
-
 	ext_vga[0] = 0;
 
 	/* TODO interlace */
@@ -1182,6 +1180,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 
 	mga_crtc_do_set_base(mdev, fb, old_fb);
 
+	mgag200_set_mode_regs(mdev, mode);
+
 	/* reset tagfifo */
 	if (mdev->type == G200_ER) {
 		u32 mem_ctl = RREG32(MGAREG_MEMCTL);
-- 
2.26.2

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

[PATCH v2 07/15] drm/mgag200: Set pitch in a separate helper function

[Thomas Zimmermann]

The framebuffer's pitch is now set in mgag200_set_offset().

v2:
	* move offset and bpp-shift calculation into helper functions
	* use u8 instead of uint8_t
	* add MGAREG_CRTCEXT0_OFFSET_MASK

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 57 +++++++++++++++++++-------
 drivers/gpu/drm/mgag200/mgag200_reg.h  |  2 +
 2 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 4dba0a379c263..dee7838d7d368 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1002,6 +1002,48 @@ static void mgag200_set_mode_regs(struct mga_device *mdev,
 	mga_crtc_set_plls(mdev, mode->clock);
 }
 
+static u8 mgag200_get_bpp_shift(struct mga_device *mdev,
+				const struct drm_format_info *format)
+{
+	return mdev->bpp_shifts[format->cpp[0] - 1];
+}
+
+/*
+ * Calculates the HW offset value from the framebuffer's pitch. The
+ * offset is a multiple of the pixel size and depends on the display
+ * format.
+ */
+static u32 mgag200_calculate_offset(struct mga_device *mdev,
+				    const struct drm_framebuffer *fb)
+{
+	u32 offset = fb->pitches[0] / fb->format->cpp[0];
+	u8 bppshift = mgag200_get_bpp_shift(mdev, fb->format);
+
+	if (fb->format->cpp[0] * 8 == 24)
+		offset = (offset * 3) >> (4 - bppshift);
+	else
+		offset = offset >> (4 - bppshift);
+
+	return offset;
+}
+
+static void mgag200_set_offset(struct mga_device *mdev,
+			       const struct drm_framebuffer *fb)
+{
+	u8 crtc13, crtcext0;
+	u32 offset = mgag200_calculate_offset(mdev, fb);
+
+	RREG_ECRT(0, crtcext0);
+
+	crtc13 = offset & 0xff;
+
+	crtcext0 &= ~MGAREG_CRTCEXT0_OFFSET_MASK;
+	crtcext0 |= (offset >> 4) & MGAREG_CRTCEXT0_OFFSET_MASK;
+
+	WREG_CRT(0x13, crtc13);
+	WREG_ECRT(0x00, crtcext0);
+}
+
 static int mga_crtc_mode_set(struct drm_crtc *crtc,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode,
@@ -1010,7 +1052,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = to_mga_device(dev);
 	const struct drm_framebuffer *fb = crtc->primary->fb;
-	int pitch;
 	int option = 0, option2 = 0;
 	int i;
 	unsigned char misc = 0;
@@ -1121,12 +1162,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	WREG_SEQ(3, 0);
 	WREG_SEQ(4, 0xe);
 
-	pitch = fb->pitches[0] / fb->format->cpp[0];
-	if (fb->format->cpp[0] * 8 == 24)
-		pitch = (pitch * 3) >> (4 - bppshift);
-	else
-		pitch = pitch >> (4 - bppshift);
-
 	WREG_GFX(0, 0);
 	WREG_GFX(1, 0);
 	WREG_GFX(2, 0);
@@ -1143,20 +1178,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	WREG_CRT(13, 0);
 	WREG_CRT(14, 0);
 	WREG_CRT(15, 0);
-	WREG_CRT(19, pitch & 0xFF);
-
-	ext_vga[0] = 0;
 
 	/* TODO interlace */
 
-	ext_vga[0] |= (pitch & 0x300) >> 4;
 	if (fb->format->cpp[0] * 8 == 24)
 		ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80;
 	else
 		ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
 	ext_vga[4] = 0;
 
-	WREG_ECRT(0, ext_vga[0]);
 	WREG_ECRT(3, ext_vga[3]);
 	WREG_ECRT(4, ext_vga[4]);
 
@@ -1170,8 +1200,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		WREG_ECRT(6, 0);
 	}
 
-	WREG_ECRT(0, ext_vga[0]);
-
 	misc = RREG8(MGA_MISC_IN);
 	misc |= MGAREG_MISC_IOADSEL |
 		MGAREG_MISC_RAMMAPEN |
@@ -1179,6 +1207,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	WREG8(MGA_MISC_OUT, misc);
 
 	mga_crtc_do_set_base(mdev, fb, old_fb);
+	mgag200_set_offset(mdev, fb);
 
 	mgag200_set_mode_regs(mdev, mode);
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h
index 0ba6e15e97106..cd08dee29b06f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_reg.h
+++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
@@ -240,6 +240,8 @@
 #define MGAREG_CRTCEXT_INDEX	0x1fde
 #define MGAREG_CRTCEXT_DATA	0x1fdf
 
+#define MGAREG_CRTCEXT0_OFFSET_MASK	GENMASK(5, 4)
+
 /* Cursor X and Y position */
 #define MGA_CURPOSXL 0x3c0c
 #define MGA_CURPOSXH 0x3c0d
-- 
2.26.2

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

[PATCH v2 08/15] drm/mgag200: Set primary plane's format in separate helper function

[Thomas Zimmermann]

The primary plane's format registers are now updated in a
mgag200_set_format_regs().

v2:
	* get bpp shift from helper function
	* replace uint8_t with u8

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 109 ++++++++++++++++---------
 1 file changed, 69 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index dee7838d7d368..38556f57ad218 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1044,6 +1044,68 @@ static void mgag200_set_offset(struct mga_device *mdev,
 	WREG_ECRT(0x00, crtcext0);
 }
 
+static void mgag200_set_format_regs(struct mga_device *mdev,
+				    const struct drm_framebuffer *fb)
+{
+	struct drm_device *dev = mdev->dev;
+	const struct drm_format_info *format = fb->format;
+	unsigned int bpp, bppshift, scale;
+	u8 crtcext3, xmulctrl;
+
+	bpp = format->cpp[0] * 8;
+
+	bppshift = mgag200_get_bpp_shift(mdev, format);
+	switch (bpp) {
+	case 24:
+		scale = ((1 << bppshift) * 3) - 1;
+		break;
+	default:
+		scale = (1 << bppshift) - 1;
+		break;
+	}
+
+	RREG_ECRT(3, crtcext3);
+
+	switch (bpp) {
+	case 8:
+		xmulctrl = MGA1064_MUL_CTL_8bits;
+		break;
+	case 16:
+		if (format->depth == 15)
+			xmulctrl = MGA1064_MUL_CTL_15bits;
+		else
+			xmulctrl = MGA1064_MUL_CTL_16bits;
+		break;
+	case 24:
+		xmulctrl = MGA1064_MUL_CTL_24bits;
+		break;
+	case 32:
+		xmulctrl = MGA1064_MUL_CTL_32_24bits;
+		break;
+	default:
+		/* BUG: We should have caught this problem already. */
+		drm_WARN_ON(dev, "invalid format depth\n");
+		return;
+	}
+
+	crtcext3 &= ~GENMASK(2, 0);
+	crtcext3 |= scale;
+
+	WREG_DAC(MGA1064_MUL_CTL, xmulctrl);
+
+	WREG_GFX(0, 0x00);
+	WREG_GFX(1, 0x00);
+	WREG_GFX(2, 0x00);
+	WREG_GFX(3, 0x00);
+	WREG_GFX(4, 0x00);
+	WREG_GFX(5, 0x40);
+	WREG_GFX(6, 0x05);
+	WREG_GFX(7, 0x0f);
+	WREG_GFX(8, 0x0f);
+
+	WREG_ECRT(3, crtcext3);
+}
+
 static int mga_crtc_mode_set(struct drm_crtc *crtc,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode,
@@ -1055,8 +1117,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	int option = 0, option2 = 0;
 	int i;
 	unsigned char misc = 0;
-	unsigned char ext_vga[6];
-	u8 bppshift;
+	u8 crtcext3, crtcext4;
 
 	static unsigned char dacvalue[] = {
 		/* 0x00: */        0,    0,    0,    0,    0,    0, 0x00,    0,
@@ -1071,8 +1132,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		/* 0x48: */        0,    0,    0,    0,    0,    0,    0,    0
 	};
 
-	bppshift = mdev->bpp_shifts[fb->format->cpp[0] - 1];
-
 	switch (mdev->type) {
 	case G200_SE_A:
 	case G200_SE_B:
@@ -1111,24 +1170,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		break;
 	}
 
-	switch (fb->format->cpp[0] * 8) {
-	case 8:
-		dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_8bits;
-		break;
-	case 16:
-		if (fb->format->depth == 15)
-			dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_15bits;
-		else
-			dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_16bits;
-		break;
-	case 24:
-		dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_24bits;
-		break;
-	case 32:
-		dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_32_24bits;
-		break;
-	}
-
 	for (i = 0; i < sizeof(dacvalue); i++) {
 		if ((i <= 0x17) ||
 		    (i == 0x1b) ||
@@ -1162,16 +1203,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	WREG_SEQ(3, 0);
 	WREG_SEQ(4, 0xe);
 
-	WREG_GFX(0, 0);
-	WREG_GFX(1, 0);
-	WREG_GFX(2, 0);
-	WREG_GFX(3, 0);
-	WREG_GFX(4, 0);
-	WREG_GFX(5, 0x40);
-	WREG_GFX(6, 0x5);
-	WREG_GFX(7, 0xf);
-	WREG_GFX(8, 0xf);
-
 	WREG_CRT(10, 0);
 	WREG_CRT(11, 0);
 	WREG_CRT(12, 0);
@@ -1179,16 +1210,13 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	WREG_CRT(14, 0);
 	WREG_CRT(15, 0);
 
-	/* TODO interlace */
+	RREG_ECRT(0x03, crtcext3);
 
-	if (fb->format->cpp[0] * 8 == 24)
-		ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80;
-	else
-		ext_vga[3] = ((1 << bppshift) - 1) | 0x80;
-	ext_vga[4] = 0;
+	crtcext3 |= BIT(7); /* enable MGA mode */
+	crtcext4 = 0x00;
 
-	WREG_ECRT(3, ext_vga[3]);
-	WREG_ECRT(4, ext_vga[4]);
+	WREG_ECRT(0x03, crtcext3);
+	WREG_ECRT(0x04, crtcext4);
 
 	if (mdev->type == G200_ER)
 		WREG_ECRT(0x24, 0x5);
@@ -1206,6 +1234,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 		MGAREG_MISC_HIGH_PG_SEL;
 	WREG8(MGA_MISC_OUT, misc);
 
+	mgag200_set_format_regs(mdev, fb);
 	mga_crtc_do_set_base(mdev, fb, old_fb);
 	mgag200_set_offset(mdev, fb);
 
-- 
2.26.2

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

[PATCH v2 09/15] drm/mgag200: Move TAGFIFO reset into separate function

[Thomas Zimmermann]

The TAGFIFO state is now reset in mgag200_g200er_reset_tagfifo().

v2:
	* define MGAREG_SEQ1_SCROFF

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  6 ++++
 drivers/gpu/drm/mgag200/mgag200_mode.c | 45 +++++++++++++++++---------
 drivers/gpu/drm/mgag200/mgag200_reg.h  |  3 ++
 3 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 1963876ef3b8c..cf71a4ec84158 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -49,6 +49,12 @@
 		WREG8(ATTR_DATA, v);				\
 	} while (0)						\
 
+#define RREG_SEQ(reg, v)					\
+	do {							\
+		WREG8(MGAREG_SEQ_INDEX, reg);			\
+		v = RREG8(MGAREG_SEQ_DATA);			\
+	} while (0)						\
+
 #define WREG_SEQ(reg, v)					\
 	do {							\
 		WREG8(MGAREG_SEQ_INDEX, reg);			\
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 38556f57ad218..68ae604926757 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1106,6 +1106,33 @@ static void mgag200_set_format_regs(struct mga_device *mdev,
 	WREG_ECRT(3, crtcext3);
 }
 
+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;
+	WREG32(MGAREG_MEMCTL, memctl);
+
+	udelay(1000);
+
+	memctl &= ~RESET_FLAG;
+	WREG32(MGAREG_MEMCTL, memctl);
+
+	/* screen on */
+	RREG_SEQ(0x01, seq1);
+	seq1 &= ~MGAREG_SEQ1_SCROFF;
+	WREG_SEQ(0x01, seq1);
+}
+
 static int mga_crtc_mode_set(struct drm_crtc *crtc,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode,
@@ -1240,22 +1267,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 
 	mgag200_set_mode_regs(mdev, mode);
 
-	/* reset tagfifo */
-	if (mdev->type == G200_ER) {
-		u32 mem_ctl = RREG32(MGAREG_MEMCTL);
-		u8 seq1;
-
-		/* screen off */
-		WREG8(MGAREG_SEQ_INDEX, 0x01);
-		seq1 = RREG8(MGAREG_SEQ_DATA) | 0x20;
-		WREG8(MGAREG_SEQ_DATA, seq1);
-
-		WREG32(MGAREG_MEMCTL, mem_ctl | 0x00200000);
-		udelay(1000);
-		WREG32(MGAREG_MEMCTL, mem_ctl & ~0x00200000);
-
-		WREG8(MGAREG_SEQ_DATA, seq1 & ~0x20);
-	}
+	if (mdev->type == G200_ER)
+		mgag200_g200er_reset_tagfifo(mdev);
 
 
 	if (IS_G200_SE(mdev)) {
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h
index cd08dee29b06f..29f7194faadc0 100644
--- a/drivers/gpu/drm/mgag200/mgag200_reg.h
+++ b/drivers/gpu/drm/mgag200/mgag200_reg.h
@@ -235,6 +235,9 @@
 /* MMIO VGA registers */
 #define MGAREG_SEQ_INDEX	0x1fc4
 #define MGAREG_SEQ_DATA		0x1fc5
+
+#define MGAREG_SEQ1_SCROFF	BIT(5)
+
 #define MGAREG_CRTC_INDEX	0x1fd4
 #define MGAREG_CRTC_DATA	0x1fd5
 #define MGAREG_CRTCEXT_INDEX	0x1fde
-- 
2.26.2

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

[PATCH v2 10/15] drm/mgag200: Move hiprilvl setting into separate functions

[Thomas Zimmermann]

The hiprivlvl settings are now updated in mgag200_g200se_set_hiprilvl()
and mgag200_g200ev_set_hiprilvl().

v2:
	* replace uint8_t with u8

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 98 ++++++++++++++------------
 1 file changed, 54 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 68ae604926757..46122fa319889 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1133,6 +1133,56 @@ static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev)
 	WREG_SEQ(0x01, seq1);
 }
 
+static void mgag200_g200se_set_hiprilvl(struct mga_device *mdev,
+					const struct drm_display_mode *mode,
+					const struct drm_framebuffer *fb)
+{
+	unsigned int hiprilvl;
+	u8 crtcext6;
+
+	if  (mdev->unique_rev_id >= 0x04) {
+		hiprilvl = 0;
+	} else if (mdev->unique_rev_id >= 0x02) {
+		unsigned int bpp;
+		unsigned long mb;
+
+		if (fb->format->cpp[0] * 8 > 16)
+			bpp = 32;
+		else if (fb->format->cpp[0] * 8 > 8)
+			bpp = 16;
+		else
+			bpp = 8;
+
+		mb = (mode->clock * bpp) / 1000;
+		if (mb > 3100)
+			hiprilvl = 0;
+		else if (mb > 2600)
+			hiprilvl = 1;
+		else if (mb > 1900)
+			hiprilvl = 2;
+		else if (mb > 1160)
+			hiprilvl = 3;
+		else if (mb > 440)
+			hiprilvl = 4;
+		else
+			hiprilvl = 5;
+
+	} else if (mdev->unique_rev_id >= 0x01) {
+		hiprilvl = 3;
+	} else {
+		hiprilvl = 4;
+	}
+
+	crtcext6 = hiprilvl; /* implicitly sets maxhipri to 0 */
+
+	WREG_ECRT(0x06, crtcext6);
+}
+
+static void mgag200_g200ev_set_hiprilvl(struct mga_device *mdev)
+{
+	WREG_ECRT(0x06, 0x00);
+}
+
 static int mga_crtc_mode_set(struct drm_crtc *crtc,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode,
@@ -1251,10 +1301,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	if (mdev->type == G200_EW3)
 		WREG_ECRT(0x34, 0x5);
 
-	if (mdev->type == G200_EV) {
-		WREG_ECRT(6, 0);
-	}
-
 	misc = RREG8(MGA_MISC_IN);
 	misc |= MGAREG_MISC_IOADSEL |
 		MGAREG_MISC_RAMMAPEN |
@@ -1270,47 +1316,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	if (mdev->type == G200_ER)
 		mgag200_g200er_reset_tagfifo(mdev);
 
+	if (IS_G200_SE(mdev))
+		mgag200_g200se_set_hiprilvl(mdev, mode, fb);
+	else if (mdev->type == G200_EV)
+		mgag200_g200ev_set_hiprilvl(mdev);
 
-	if (IS_G200_SE(mdev)) {
-		if  (mdev->unique_rev_id >= 0x04) {
-			WREG8(MGAREG_CRTCEXT_INDEX, 0x06);
-			WREG8(MGAREG_CRTCEXT_DATA, 0);
-		} else if (mdev->unique_rev_id >= 0x02) {
-			u8 hi_pri_lvl;
-			u32 bpp;
-			u32 mb;
-
-			if (fb->format->cpp[0] * 8 > 16)
-				bpp = 32;
-			else if (fb->format->cpp[0] * 8 > 8)
-				bpp = 16;
-			else
-				bpp = 8;
-
-			mb = (mode->clock * bpp) / 1000;
-			if (mb > 3100)
-				hi_pri_lvl = 0;
-			else if (mb > 2600)
-				hi_pri_lvl = 1;
-			else if (mb > 1900)
-				hi_pri_lvl = 2;
-			else if (mb > 1160)
-				hi_pri_lvl = 3;
-			else if (mb > 440)
-				hi_pri_lvl = 4;
-			else
-				hi_pri_lvl = 5;
-
-			WREG8(MGAREG_CRTCEXT_INDEX, 0x06);
-			WREG8(MGAREG_CRTCEXT_DATA, hi_pri_lvl);
-		} else {
-			WREG8(MGAREG_CRTCEXT_INDEX, 0x06);
-			if (mdev->unique_rev_id >= 0x01)
-				WREG8(MGAREG_CRTCEXT_DATA, 0x03);
-			else
-				WREG8(MGAREG_CRTCEXT_DATA, 0x04);
-		}
-	}
 	return 0;
 }
 
-- 
2.26.2

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

[PATCH v2 11/15] drm/mgag200: Move register initialization into separate function

[Thomas Zimmermann]

Registers are initialized with constants. This is now done in
mgag200_init_regs(), mgag200_set_dac_regs() and mgag200_set_pci_regs().
Later patches should move these calls from mode setting to device
initialization.

v2:
	* replace uint8_t with u8

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 261 ++++++++++++++-----------
 1 file changed, 147 insertions(+), 114 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 46122fa319889..199ae08976e16 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -919,6 +919,152 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return mga_crtc_do_set_base(mdev, fb, old_fb);
 }
 
+static void mgag200_set_pci_regs(struct mga_device *mdev)
+{
+	uint32_t option = 0, option2 = 0;
+	struct drm_device *dev = mdev->dev;
+
+	switch (mdev->type) {
+	case G200_SE_A:
+	case G200_SE_B:
+		if (mdev->has_sdram)
+			option = 0x40049120;
+		else
+			option = 0x4004d120;
+		option2 = 0x00008000;
+		break;
+	case G200_WB:
+	case G200_EW3:
+		option = 0x41049120;
+		option2 = 0x0000b000;
+		break;
+	case G200_EV:
+		option = 0x00000120;
+		option2 = 0x0000b000;
+		break;
+	case G200_EH:
+	case G200_EH3:
+		option = 0x00000120;
+		option2 = 0x0000b000;
+		break;
+	case G200_ER:
+		break;
+	}
+
+	if (option)
+		pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option);
+
+	if (option2)
+		pci_write_config_dword(dev->pdev, PCI_MGA_OPTION2, option2);
+}
+
+static void mgag200_set_dac_regs(struct mga_device *mdev)
+{
+	size_t i;
+	u8 dacvalue[] = {
+		/* 0x00: */        0,    0,    0,    0,    0,    0, 0x00,    0,
+		/* 0x08: */        0,    0,    0,    0,    0,    0,    0,    0,
+		/* 0x10: */        0,    0,    0,    0,    0,    0,    0,    0,
+		/* 0x18: */     0x00,    0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20,
+		/* 0x20: */     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		/* 0x28: */     0x00, 0x00, 0x00, 0x00,    0,    0,    0, 0x40,
+		/* 0x30: */     0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
+		/* 0x38: */     0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
+		/* 0x40: */        0,    0,    0,    0,    0,    0,    0,    0,
+		/* 0x48: */        0,    0,    0,    0,    0,    0,    0,    0
+	};
+
+	switch (mdev->type) {
+	case G200_SE_A:
+	case G200_SE_B:
+		dacvalue[MGA1064_VREF_CTL] = 0x03;
+		dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL;
+		dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_DAC_EN |
+					     MGA1064_MISC_CTL_VGA8 |
+					     MGA1064_MISC_CTL_DAC_RAM_CS;
+		break;
+	case G200_WB:
+	case G200_EW3:
+		dacvalue[MGA1064_VREF_CTL] = 0x07;
+		break;
+	case G200_EV:
+		dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL;
+		dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 |
+					     MGA1064_MISC_CTL_DAC_RAM_CS;
+		break;
+	case G200_EH:
+	case G200_EH3:
+		dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 |
+					     MGA1064_MISC_CTL_DAC_RAM_CS;
+		break;
+	case G200_ER:
+		break;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(dacvalue); i++) {
+		if ((i <= 0x17) ||
+		    (i == 0x1b) ||
+		    (i == 0x1c) ||
+		    ((i >= 0x1f) && (i <= 0x29)) ||
+		    ((i >= 0x30) && (i <= 0x37)))
+			continue;
+		if (IS_G200_SE(mdev) &&
+		    ((i == 0x2c) || (i == 0x2d) || (i == 0x2e)))
+			continue;
+		if ((mdev->type == G200_EV ||
+		    mdev->type == G200_WB ||
+		    mdev->type == G200_EH ||
+		    mdev->type == G200_EW3 ||
+		    mdev->type == G200_EH3) &&
+		    (i >= 0x44) && (i <= 0x4e))
+			continue;
+
+		WREG_DAC(i, dacvalue[i]);
+	}
+
+	if (mdev->type == G200_ER)
+		WREG_DAC(0x90, 0);
+}
+
+static void mgag200_init_regs(struct mga_device *mdev)
+{
+	u8 crtcext3, crtcext4, misc;
+
+	mgag200_set_pci_regs(mdev);
+	mgag200_set_dac_regs(mdev);
+
+	WREG_SEQ(2, 0x0f);
+	WREG_SEQ(3, 0x00);
+	WREG_SEQ(4, 0x0e);
+
+	WREG_CRT(10, 0);
+	WREG_CRT(11, 0);
+	WREG_CRT(12, 0);
+	WREG_CRT(13, 0);
+	WREG_CRT(14, 0);
+	WREG_CRT(15, 0);
+
+	RREG_ECRT(0x03, crtcext3);
+
+	crtcext3 |= BIT(7); /* enable MGA mode */
+	crtcext4 = 0x00;
+
+	WREG_ECRT(0x03, crtcext3);
+	WREG_ECRT(0x04, crtcext4);
+
+	if (mdev->type == G200_ER)
+		WREG_ECRT(0x24, 0x5);
+
+	if (mdev->type == G200_EW3)
+		WREG_ECRT(0x34, 0x5);
+
+	misc = RREG8(MGA_MISC_IN);
+	misc |= MGAREG_MISC_IOADSEL |
+		MGAREG_MISC_RAMMAPEN |
+		MGAREG_MISC_HIGH_PG_SEL;
+	WREG8(MGA_MISC_OUT, misc);
+}
+
 static void mgag200_set_mode_regs(struct mga_device *mdev,
 				  const struct drm_display_mode *mode)
 {
@@ -1191,121 +1337,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = to_mga_device(dev);
 	const struct drm_framebuffer *fb = crtc->primary->fb;
-	int option = 0, option2 = 0;
-	int i;
-	unsigned char misc = 0;
-	u8 crtcext3, crtcext4;
-
-	static unsigned char dacvalue[] = {
-		/* 0x00: */        0,    0,    0,    0,    0,    0, 0x00,    0,
-		/* 0x08: */        0,    0,    0,    0,    0,    0,    0,    0,
-		/* 0x10: */        0,    0,    0,    0,    0,    0,    0,    0,
-		/* 0x18: */     0x00,    0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20,
-		/* 0x20: */     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		/* 0x28: */     0x00, 0x00, 0x00, 0x00,    0,    0,    0, 0x40,
-		/* 0x30: */     0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
-		/* 0x38: */     0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
-		/* 0x40: */        0,    0,    0,    0,    0,    0,    0,    0,
-		/* 0x48: */        0,    0,    0,    0,    0,    0,    0,    0
-	};
 
-	switch (mdev->type) {
-	case G200_SE_A:
-	case G200_SE_B:
-		dacvalue[MGA1064_VREF_CTL] = 0x03;
-		dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL;
-		dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_DAC_EN |
-					     MGA1064_MISC_CTL_VGA8 |
-					     MGA1064_MISC_CTL_DAC_RAM_CS;
-		if (mdev->has_sdram)
-			option = 0x40049120;
-		else
-			option = 0x4004d120;
-		option2 = 0x00008000;
-		break;
-	case G200_WB:
-	case G200_EW3:
-		dacvalue[MGA1064_VREF_CTL] = 0x07;
-		option = 0x41049120;
-		option2 = 0x0000b000;
-		break;
-	case G200_EV:
-		dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL;
-		dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 |
-					     MGA1064_MISC_CTL_DAC_RAM_CS;
-		option = 0x00000120;
-		option2 = 0x0000b000;
-		break;
-	case G200_EH:
-	case G200_EH3:
-		dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 |
-					     MGA1064_MISC_CTL_DAC_RAM_CS;
-		option = 0x00000120;
-		option2 = 0x0000b000;
-		break;
-	case G200_ER:
-		break;
-	}
-
-	for (i = 0; i < sizeof(dacvalue); i++) {
-		if ((i <= 0x17) ||
-		    (i == 0x1b) ||
-		    (i == 0x1c) ||
-		    ((i >= 0x1f) && (i <= 0x29)) ||
-		    ((i >= 0x30) && (i <= 0x37)))
-			continue;
-		if (IS_G200_SE(mdev) &&
-		    ((i == 0x2c) || (i == 0x2d) || (i == 0x2e)))
-			continue;
-		if ((mdev->type == G200_EV ||
-		    mdev->type == G200_WB ||
-		    mdev->type == G200_EH ||
-		    mdev->type == G200_EW3 ||
-		    mdev->type == G200_EH3) &&
-		    (i >= 0x44) && (i <= 0x4e))
-			continue;
-
-		WREG_DAC(i, dacvalue[i]);
-	}
-
-	if (mdev->type == G200_ER)
-		WREG_DAC(0x90, 0);
-
-	if (option)
-		pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option);
-	if (option2)
-		pci_write_config_dword(dev->pdev, PCI_MGA_OPTION2, option2);
-
-	WREG_SEQ(2, 0xf);
-	WREG_SEQ(3, 0);
-	WREG_SEQ(4, 0xe);
-
-	WREG_CRT(10, 0);
-	WREG_CRT(11, 0);
-	WREG_CRT(12, 0);
-	WREG_CRT(13, 0);
-	WREG_CRT(14, 0);
-	WREG_CRT(15, 0);
-
-	RREG_ECRT(0x03, crtcext3);
-
-	crtcext3 |= BIT(7); /* enable MGA mode */
-	crtcext4 = 0x00;
-
-	WREG_ECRT(0x03, crtcext3);
-	WREG_ECRT(0x04, crtcext4);
-
-	if (mdev->type == G200_ER)
-		WREG_ECRT(0x24, 0x5);
-
-	if (mdev->type == G200_EW3)
-		WREG_ECRT(0x34, 0x5);
-
-	misc = RREG8(MGA_MISC_IN);
-	misc |= MGAREG_MISC_IOADSEL |
-		MGAREG_MISC_RAMMAPEN |
-		MGAREG_MISC_HIGH_PG_SEL;
-	WREG8(MGA_MISC_OUT, misc);
+	mgag200_init_regs(mdev);
 
 	mgag200_set_format_regs(mdev, fb);
 	mga_crtc_do_set_base(mdev, fb, old_fb);
-- 
2.26.2

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

[PATCH v2 12/15] drm/mgag200: Remove out-commented suspend/resume helpers

[Thomas Zimmermann]

The suspend/resume helpers are unused. Also remove associated state
from struct mga_device.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  1 -
 drivers/gpu/drm/mgag200/mgag200_mode.c | 71 --------------------------
 2 files changed, 72 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index cf71a4ec84158..0cf498d1e900c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -164,7 +164,6 @@ struct mga_device {
 
 	size_t vram_fb_available;
 
-	bool				suspended;
 	enum mga_type			type;
 	int				has_sdram;
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 199ae08976e16..d6f9763a4a450 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1357,65 +1357,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	return 0;
 }
 
-#if 0 /* code from mjg to attempt D3 on crtc dpms off - revisit later */
-static int mga_suspend(struct drm_crtc *crtc)
-{
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	struct mga_device *mdev = dev->dev_private;
-	struct pci_dev *pdev = dev->pdev;
-	int option;
-
-	if (mdev->suspended)
-		return 0;
-
-	WREG_SEQ(1, 0x20);
-	WREG_ECRT(1, 0x30);
-	/* Disable the pixel clock */
-	WREG_DAC(0x1a, 0x05);
-	/* Power down the DAC */
-	WREG_DAC(0x1e, 0x18);
-	/* Power down the pixel PLL */
-	WREG_DAC(0x1a, 0x0d);
-
-	/* Disable PLLs and clocks */
-	pci_read_config_dword(pdev, PCI_MGA_OPTION, &option);
-	option &= ~(0x1F8024);
-	pci_write_config_dword(pdev, PCI_MGA_OPTION, option);
-	pci_set_power_state(pdev, PCI_D3hot);
-	pci_disable_device(pdev);
-
-	mdev->suspended = true;
-
-	return 0;
-}
-
-static int mga_resume(struct drm_crtc *crtc)
-{
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	struct mga_device *mdev = dev->dev_private;
-	struct pci_dev *pdev = dev->pdev;
-	int option;
-
-	if (!mdev->suspended)
-		return 0;
-
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_device(pdev);
-
-	/* Disable sysclk */
-	pci_read_config_dword(pdev, PCI_MGA_OPTION, &option);
-	option &= ~(0x4);
-	pci_write_config_dword(pdev, PCI_MGA_OPTION, option);
-
-	mdev->suspended = false;
-
-	return 0;
-}
-
-#endif
-
 static void mga_crtc_dpms(struct drm_crtc *crtc, int mode)
 {
 	struct drm_device *dev = crtc->dev;
@@ -1442,11 +1383,6 @@ static void mga_crtc_dpms(struct drm_crtc *crtc, int mode)
 		break;
 	}
 
-#if 0
-	if (mode == DRM_MODE_DPMS_OFF) {
-		mga_suspend(crtc);
-	}
-#endif
 	WREG8(MGAREG_SEQ_INDEX, 0x01);
 	seq1 |= RREG8(MGAREG_SEQ_DATA) & ~0x20;
 	mga_wait_vsync(mdev);
@@ -1456,13 +1392,6 @@ static void mga_crtc_dpms(struct drm_crtc *crtc, int mode)
 	WREG8(MGAREG_CRTCEXT_INDEX, 0x01);
 	crtcext1 |= RREG8(MGAREG_CRTCEXT_DATA) & ~0x30;
 	WREG8(MGAREG_CRTCEXT_DATA, crtcext1);
-
-#if 0
-	if (mode == DRM_MODE_DPMS_ON && mdev->suspended == true) {
-		mga_resume(crtc);
-		drm_helper_resume_force_mode(dev);
-	}
-#endif
 }
 
 /*
-- 
2.26.2

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

[PATCH v2 13/15] drm/mgag200: Use simple-display data structures

[Thomas Zimmermann]

The MGA CRTC data structure struct mga_crtc contains unused additional
fields; so it can removed. The standard DRM CRTC and encoder structures
are embedded now in struct drm_simple_display_pipe. Done in preparation
of converting mgag200 to simple KMS helpers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  | 11 ++--------
 drivers/gpu/drm/mgag200/mgag200_mode.c | 28 ++++++--------------------
 2 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 0cf498d1e900c..2392baff618aa 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -19,6 +19,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_gem_vram_helper.h>
+#include <drm/drm_simple_kms_helper.h>
 
 #include "mgag200_reg.h"
 
@@ -105,16 +106,8 @@
 
 #define MATROX_DPMS_CLEARED (-1)
 
-#define to_mga_crtc(x) container_of(x, struct mga_crtc, base)
 #define to_mga_connector(x) container_of(x, struct mga_connector, base)
 
-struct mga_crtc {
-	struct drm_crtc base;
-	u8 lut_r[256], lut_g[256], lut_b[256];
-	int last_dpms;
-	bool enabled;
-};
-
 struct mga_i2c_chan {
 	struct i2c_adapter adapter;
 	struct drm_device *dev;
@@ -175,7 +168,7 @@ struct mga_device {
 	u32 unique_rev_id;
 
 	struct mga_connector connector;
-	struct drm_encoder encoder;
+	struct drm_simple_display_pipe display_pipe;
 };
 
 static inline struct mga_device *to_mga_device(struct drm_device *dev)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index d6f9763a4a450..00bbc1f9b7db3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1475,15 +1475,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 	return 0;
 }
 
-/* Simple cleanup function */
-static void mga_crtc_destroy(struct drm_crtc *crtc)
-{
-	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-
-	drm_crtc_cleanup(crtc);
-	kfree(mga_crtc);
-}
-
 static void mga_crtc_disable(struct drm_crtc *crtc)
 {
 	DRM_DEBUG_KMS("\n");
@@ -1501,7 +1492,7 @@ static void mga_crtc_disable(struct drm_crtc *crtc)
 static const struct drm_crtc_funcs mga_crtc_funcs = {
 	.gamma_set = mga_crtc_gamma_set,
 	.set_config = drm_crtc_helper_set_config,
-	.destroy = mga_crtc_destroy,
+	.destroy = drm_crtc_cleanup,
 };
 
 static const struct drm_crtc_helper_funcs mga_helper_funcs = {
@@ -1517,20 +1508,13 @@ static const struct drm_crtc_helper_funcs mga_helper_funcs = {
 static void mga_crtc_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
-	struct mga_crtc *mga_crtc;
-
-	mga_crtc = kzalloc(sizeof(struct mga_crtc) +
-			      (MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)),
-			      GFP_KERNEL);
-
-	if (mga_crtc == NULL)
-		return;
+	struct drm_crtc *crtc = &mdev->display_pipe.crtc;
 
-	drm_crtc_init(dev, &mga_crtc->base, &mga_crtc_funcs);
+	drm_crtc_init(dev, crtc, &mga_crtc_funcs);
 
-	drm_mode_crtc_set_gamma_size(&mga_crtc->base, MGAG200_LUT_SIZE);
+	drm_mode_crtc_set_gamma_size(crtc, MGAG200_LUT_SIZE);
 
-	drm_crtc_helper_add(&mga_crtc->base, &mga_helper_funcs);
+	drm_crtc_helper_add(crtc, &mga_helper_funcs);
 }
 
 /*
@@ -1718,7 +1702,7 @@ static unsigned int mgag200_preferred_depth(struct mga_device *mdev)
 int mgag200_modeset_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
-	struct drm_encoder *encoder = &mdev->encoder;
+	struct drm_encoder *encoder = &mdev->display_pipe.encoder;
 	struct drm_connector *connector = &mdev->connector.base;
 	int ret;
 
-- 
2.26.2

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

[PATCH v2 14/15] drm/mgag200: Convert to simple KMS helper

[Thomas Zimmermann]

The mgag200 supports a single pipeline with only a primary plane. It can
be converted to simple KMS helpers. This also adds support for atomic
modesetting. Wayland compositors, which use pageflip ioctls, can now be
used with mgag200.

v2:
	* prepare encoder and CRTC in a separate patch
	* remove suspend/resume code in a separate patch
	* don't call set_format_regs() in pipe_update()

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/mgag200/mgag200_drv.c  |   2 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c | 314 +++++++++++++------------
 2 files changed, 164 insertions(+), 152 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index c2f0e4b40b052..a06ce4198adea 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -140,7 +140,7 @@ int mgag200_driver_dumb_create(struct drm_file *file,
 }
 
 static struct drm_driver driver = {
-	.driver_features = DRIVER_GEM | DRIVER_MODESET,
+	.driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
 	.fops = &mgag200_driver_fops,
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 00bbc1f9b7db3..b50c1beb7b7b9 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -11,10 +11,13 @@
 #include <linux/delay.h>
 #include <linux/pci.h>
 
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_plane_helper.h>
+#include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
 
@@ -30,13 +33,18 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = to_mga_device(dev);
-	struct drm_framebuffer *fb = crtc->primary->fb;
+	struct drm_framebuffer *fb;
 	u16 *r_ptr, *g_ptr, *b_ptr;
 	int i;
 
 	if (!crtc->enabled)
 		return;
 
+	if (!mdev->display_pipe.plane.state)
+		return;
+
+	fb = mdev->display_pipe.plane.state->fb;
+
 	r_ptr = crtc->gamma_store;
 	g_ptr = r_ptr + crtc->gamma_size;
 	b_ptr = g_ptr + crtc->gamma_size;
@@ -869,56 +877,6 @@ static void mgag200_set_startadd(struct mga_device *mdev,
 	WREG_ECRT(0x00, crtcext0);
 }
 
-static int mga_crtc_do_set_base(struct mga_device *mdev,
-				const struct drm_framebuffer *fb,
-				const struct drm_framebuffer *old_fb)
-{
-	struct drm_gem_vram_object *gbo;
-	int ret;
-	s64 gpu_addr;
-
-	if (old_fb) {
-		gbo = drm_gem_vram_of_gem(old_fb->obj[0]);
-		drm_gem_vram_unpin(gbo);
-	}
-
-	gbo = drm_gem_vram_of_gem(fb->obj[0]);
-
-	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
-	if (ret)
-		return ret;
-	gpu_addr = drm_gem_vram_offset(gbo);
-	if (gpu_addr < 0) {
-		ret = (int)gpu_addr;
-		goto err_drm_gem_vram_unpin;
-	}
-
-	mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
-
-	return 0;
-
-err_drm_gem_vram_unpin:
-	drm_gem_vram_unpin(gbo);
-	return ret;
-}
-
-static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-				  struct drm_framebuffer *old_fb)
-{
-	struct drm_device *dev = crtc->dev;
-	struct mga_device *mdev = dev->dev_private;
-	struct drm_framebuffer *fb = crtc->primary->fb;
-	unsigned int count;
-
-	do { } while (RREG8(0x1fda) & 0x08);
-	do { } while (!(RREG8(0x1fda) & 0x08));
-
-	count = RREG8(MGAREG_VCOUNT) + 2;
-	do { } while (RREG8(MGAREG_VCOUNT) < count);
-
-	return mga_crtc_do_set_base(mdev, fb, old_fb);
-}
-
 static void mgag200_set_pci_regs(struct mga_device *mdev)
 {
 	uint32_t option = 0, option2 = 0;
@@ -1329,34 +1287,6 @@ static void mgag200_g200ev_set_hiprilvl(struct mga_device *mdev)
 	WREG_ECRT(0x06, 0x00);
 }
 
-static int mga_crtc_mode_set(struct drm_crtc *crtc,
-				struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode,
-				int x, int y, struct drm_framebuffer *old_fb)
-{
-	struct drm_device *dev = crtc->dev;
-	struct mga_device *mdev = to_mga_device(dev);
-	const struct drm_framebuffer *fb = crtc->primary->fb;
-
-	mgag200_init_regs(mdev);
-
-	mgag200_set_format_regs(mdev, fb);
-	mga_crtc_do_set_base(mdev, fb, old_fb);
-	mgag200_set_offset(mdev, fb);
-
-	mgag200_set_mode_regs(mdev, mode);
-
-	if (mdev->type == G200_ER)
-		mgag200_g200er_reset_tagfifo(mdev);
-
-	if (IS_G200_SE(mdev))
-		mgag200_g200se_set_hiprilvl(mdev, mode, fb);
-	else if (mdev->type == G200_EV)
-		mgag200_g200ev_set_hiprilvl(mdev);
-
-	return 0;
-}
-
 static void mga_crtc_dpms(struct drm_crtc *crtc, int mode)
 {
 	struct drm_device *dev = crtc->dev;
@@ -1439,7 +1369,6 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct mga_device *mdev = to_mga_device(dev);
-	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
 	u8 tmp;
 
 	if (mdev->type == G200_WB || mdev->type == G200_EW3)
@@ -1458,63 +1387,7 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
 		WREG_SEQ(0x1, tmp);
 		WREG_SEQ(0, 3);
 	}
-	crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON);
-}
-
-/*
- * The core can pass us a set of gamma values to program. We actually only
- * use this for 8-bit mode so can't perform smooth fades on deeper modes,
- * but it's a requirement that we provide the function
- */
-static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, uint32_t size,
-			      struct drm_modeset_acquire_ctx *ctx)
-{
-	mga_crtc_load_lut(crtc);
-
-	return 0;
-}
-
-static void mga_crtc_disable(struct drm_crtc *crtc)
-{
-	DRM_DEBUG_KMS("\n");
-	mga_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
-	if (crtc->primary->fb) {
-		struct drm_framebuffer *fb = crtc->primary->fb;
-		struct drm_gem_vram_object *gbo =
-			drm_gem_vram_of_gem(fb->obj[0]);
-		drm_gem_vram_unpin(gbo);
-	}
-	crtc->primary->fb = NULL;
-}
-
-/* These provide the minimum set of functions required to handle a CRTC */
-static const struct drm_crtc_funcs mga_crtc_funcs = {
-	.gamma_set = mga_crtc_gamma_set,
-	.set_config = drm_crtc_helper_set_config,
-	.destroy = drm_crtc_cleanup,
-};
-
-static const struct drm_crtc_helper_funcs mga_helper_funcs = {
-	.disable = mga_crtc_disable,
-	.dpms = mga_crtc_dpms,
-	.mode_set = mga_crtc_mode_set,
-	.mode_set_base = mga_crtc_mode_set_base,
-	.prepare = mga_crtc_prepare,
-	.commit = mga_crtc_commit,
-};
-
-/* CRTC setup */
-static void mga_crtc_init(struct mga_device *mdev)
-{
-	struct drm_device *dev = mdev->dev;
-	struct drm_crtc *crtc = &mdev->display_pipe.crtc;
-
-	drm_crtc_init(dev, crtc, &mga_crtc_funcs);
-
-	drm_mode_crtc_set_gamma_size(crtc, MGAG200_LUT_SIZE);
-
-	drm_crtc_helper_add(crtc, &mga_helper_funcs);
+	mga_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
 }
 
 /*
@@ -1648,14 +1521,16 @@ static void mga_connector_destroy(struct drm_connector *connector)
 }
 
 static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = {
-	.get_modes = mga_vga_get_modes,
+	.get_modes  = mga_vga_get_modes,
 	.mode_valid = mga_vga_mode_valid,
 };
 
 static const struct drm_connector_funcs mga_vga_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = mga_connector_destroy,
+	.reset                  = drm_atomic_helper_connector_reset,
+	.fill_modes             = drm_helper_probe_single_connector_modes,
+	.destroy                = mga_connector_destroy,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state   = drm_atomic_helper_connector_destroy_state,
 };
 
 static int mgag200_vga_connector_init(struct mga_device *mdev)
@@ -1687,8 +1562,137 @@ static int mgag200_vga_connector_init(struct mga_device *mdev)
 	return ret;
 }
 
+/*
+ * Simple Display Pipe
+ */
+
+static enum drm_mode_status
+mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
+				       const struct drm_display_mode *mode)
+{
+	return MODE_OK;
+}
+
+static void
+mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
+				   struct drm_crtc_state *crtc_state,
+				   struct drm_plane_state *plane_state)
+{
+	struct drm_crtc *crtc = &pipe->crtc;
+	struct drm_device *dev = crtc->dev;
+	struct mga_device *mdev = to_mga_device(dev);
+	struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_gem_vram_object *gbo;
+	s64 gpu_addr;
+
+	gbo = drm_gem_vram_of_gem(fb->obj[0]);
+
+	gpu_addr = drm_gem_vram_offset(gbo);
+	if (drm_WARN_ON_ONCE(dev, gpu_addr < 0))
+		return; /* BUG: BO should have been pinned to VRAM. */
+
+	mga_crtc_prepare(crtc);
+
+	mgag200_set_format_regs(mdev, fb);
+	mgag200_set_mode_regs(mdev, adjusted_mode);
+
+	if (mdev->type == G200_ER)
+		mgag200_g200er_reset_tagfifo(mdev);
+
+	if (IS_G200_SE(mdev))
+		mgag200_g200se_set_hiprilvl(mdev, adjusted_mode, fb);
+	else if (mdev->type == G200_EV)
+		mgag200_g200ev_set_hiprilvl(mdev);
+
+	mga_crtc_commit(crtc);
+}
+
+static void
+mgag200_simple_display_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct drm_crtc *crtc = &pipe->crtc;
+
+	mga_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
+}
+
+static int
+mgag200_simple_display_pipe_check(struct drm_simple_display_pipe *pipe,
+				  struct drm_plane_state *plane_state,
+				  struct drm_crtc_state *crtc_state)
+{
+	struct drm_plane *plane = plane_state->plane;
+	struct drm_framebuffer *new_fb = plane_state->fb;
+	struct drm_framebuffer *fb = NULL;
+
+	if (!new_fb)
+		return 0;
+
+	if (plane->state)
+		fb = plane->state->fb;
+
+	if (!fb || (fb->format != new_fb->format))
+		crtc_state->mode_changed = true; /* update PLL settings */
+
+	return 0;
+}
+
+static void
+mgag200_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
+				   struct drm_plane_state *old_state)
+{
+	struct drm_plane *plane = &pipe->plane;
+	struct drm_device *dev = plane->dev;
+	struct mga_device *mdev = to_mga_device(dev);
+	struct drm_plane_state *state = plane->state;
+	struct drm_framebuffer *fb = state->fb;
+	struct drm_gem_vram_object *gbo;
+	s64 gpu_addr;
+
+	if (!fb)
+		return;
+
+	gbo = drm_gem_vram_of_gem(fb->obj[0]);
+
+	gpu_addr = drm_gem_vram_offset(gbo);
+	if (drm_WARN_ON_ONCE(dev, gpu_addr < 0))
+		return; /* BUG: BO should have been pinned to VRAM. */
+
+	mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
+	mgag200_set_offset(mdev, fb);
+}
+
+static const struct drm_simple_display_pipe_funcs
+mgag200_simple_display_pipe_funcs = {
+	.mode_valid = mgag200_simple_display_pipe_mode_valid,
+	.enable	    = mgag200_simple_display_pipe_enable,
+	.disable    = mgag200_simple_display_pipe_disable,
+	.check	    = mgag200_simple_display_pipe_check,
+	.update	    = mgag200_simple_display_pipe_update,
+	.prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb,
+	.cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb,
+};
+
+static const uint32_t mgag200_simple_display_pipe_formats[] = {
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_RGB888,
+};
+
+static const uint64_t mgag200_simple_display_pipe_fmtmods[] = {
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
+
+/*
+ * Mode config
+ */
+
 static const struct drm_mode_config_funcs mgag200_mode_config_funcs = {
-	.fb_create = drm_gem_fb_create
+	.fb_create     = drm_gem_fb_create,
+	.mode_valid    = drm_vram_helper_mode_valid,
+	.atomic_check  = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
 };
 
 static unsigned int mgag200_preferred_depth(struct mga_device *mdev)
@@ -1702,8 +1706,9 @@ static unsigned int mgag200_preferred_depth(struct mga_device *mdev)
 int mgag200_modeset_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
-	struct drm_encoder *encoder = &mdev->display_pipe.encoder;
 	struct drm_connector *connector = &mdev->connector.base;
+	struct drm_simple_display_pipe *pipe = &mdev->display_pipe;
+	size_t format_count = ARRAY_SIZE(mgag200_simple_display_pipe_formats);
 	int ret;
 
 	mdev->bpp_shifts[0] = 0;
@@ -1711,6 +1716,8 @@ int mgag200_modeset_init(struct mga_device *mdev)
 	mdev->bpp_shifts[2] = 0;
 	mdev->bpp_shifts[3] = 2;
 
+	mgag200_init_regs(mdev);
+
 	ret = drmm_mode_config_init(dev);
 	if (ret) {
 		drm_err(dev, "drmm_mode_config_init() failed, error %d\n",
@@ -1728,26 +1735,31 @@ int mgag200_modeset_init(struct mga_device *mdev)
 
 	dev->mode_config.funcs = &mgag200_mode_config_funcs;
 
-	mga_crtc_init(mdev);
-
-	ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_DAC);
+	ret = mgag200_vga_connector_init(mdev);
 	if (ret) {
 		drm_err(dev,
-			"drm_simple_encoder_init() failed, error %d\n",
+			"mgag200_vga_connector_init() failed, error %d\n",
 			ret);
 		return ret;
 	}
-	encoder->possible_crtcs = 0x1;
 
-	ret = mgag200_vga_connector_init(mdev);
+	ret = drm_simple_display_pipe_init(dev, pipe,
+					   &mgag200_simple_display_pipe_funcs,
+					   mgag200_simple_display_pipe_formats,
+					   format_count,
+					   mgag200_simple_display_pipe_fmtmods,
+					   connector);
 	if (ret) {
 		drm_err(dev,
-			"mgag200_vga_connector_init() failed, error %d\n",
+			"drm_simple_display_pipe_init() failed, error %d\n",
 			ret);
 		return ret;
 	}
 
-	drm_connector_attach_encoder(connector, encoder);
+	/* FIXME: legacy gamma tables; convert to CRTC state */
+	drm_mode_crtc_set_gamma_size(&pipe->crtc, MGAG200_LUT_SIZE);
+
+	drm_mode_config_reset(dev);
 
 	return 0;
 }
-- 
2.26.2

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

[PATCH v2 15/15] drm/mgag200: Replace VRAM helpers with SHMEM helpers

[Thomas Zimmermann]

The VRAM helpers managed the framebuffer memory for mgag200. This came
with several problems, as some MGA device require the scanout address
to be located at VRAM offset 0. It's incompatible with the page-flip
semantics of DRM's atomic modesettting. With atomic modesetting, old and
new framebuffers have to be located in VRAM at the same time. So at least
one of them has to reside at a non-0 offset.

This patch replaces VRAM helpers with SHMEM helpers. GEM SHMEM buffers
reside in system memory, and are shadow-copied into VRAM during page
flips. The shadow copy always starts at VRAM offset 0.

v2:
	* revert dev->pdev changes

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: John Donnelly <John.p.donnelly@oracle.com>
---
 drivers/gpu/drm/mgag200/Kconfig        |  4 +-
 drivers/gpu/drm/mgag200/mgag200_drv.c  | 49 +---------------------
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ++-
 drivers/gpu/drm/mgag200/mgag200_mode.c | 58 ++++++++++++++++----------
 drivers/gpu/drm/mgag200/mgag200_ttm.c  | 28 +++++++------
 5 files changed, 56 insertions(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig
index d60aa4b9ccd47..93be766715c9b 100644
--- a/drivers/gpu/drm/mgag200/Kconfig
+++ b/drivers/gpu/drm/mgag200/Kconfig
@@ -2,10 +2,8 @@
 config DRM_MGAG200
 	tristate "Kernel modesetting driver for MGA G200 server engines"
 	depends on DRM && PCI && MMU
+	select DRM_GEM_SHMEM_HELPER
 	select DRM_KMS_HELPER
-	select DRM_VRAM_HELPER
-	select DRM_TTM
-	select DRM_TTM_HELPER
 	help
 	 This is a KMS driver for the MGA G200 server chips, it
 	 does not support the original MGA G200 or any of the desktop
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index a06ce4198adea..00ddea7d7d270 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -22,15 +22,11 @@
  * which then performs further device association and calls our graphics init
  * functions
  */
-int mgag200_modeset = -1;
 
+int mgag200_modeset = -1;
 MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
 module_param_named(modeset, mgag200_modeset, int, 0400);
 
-int mgag200_hw_bug_no_startadd = -1;
-MODULE_PARM_DESC(modeset, "HW does not interpret scanout-buffer start address correctly");
-module_param_named(hw_bug_no_startadd, mgag200_hw_bug_no_startadd, int, 0400);
-
 static struct drm_driver driver;
 
 static const struct pci_device_id pciidlist[] = {
@@ -101,44 +97,6 @@ static void mga_pci_remove(struct pci_dev *pdev)
 
 DEFINE_DRM_GEM_FOPS(mgag200_driver_fops);
 
-static bool mgag200_pin_bo_at_0(const struct mga_device *mdev)
-{
-	if (mgag200_hw_bug_no_startadd > 0) {
-		DRM_WARN_ONCE("Option hw_bug_no_startradd is enabled. Please "
-			      "report the output of 'lspci -vvnn' to "
-			      "<dri-devel@lists.freedesktop.org> if this "
-			      "option is required to make mgag200 work "
-			      "correctly on your system.\n");
-		return true;
-	} else if (!mgag200_hw_bug_no_startadd) {
-		return false;
-	}
-	return mdev->flags & MGAG200_FLAG_HW_BUG_NO_STARTADD;
-}
-
-int mgag200_driver_dumb_create(struct drm_file *file,
-			       struct drm_device *dev,
-			       struct drm_mode_create_dumb *args)
-{
-	struct mga_device *mdev = to_mga_device(dev);
-	unsigned long pg_align;
-
-	if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized"))
-		return -EINVAL;
-
-	pg_align = 0ul;
-
-	/*
-	 * Aligning scanout buffers to the size of the video ram forces
-	 * placement at offset 0. Works around a bug where HW does not
-	 * respect 'startadd' field.
-	 */
-	if (mgag200_pin_bo_at_0(mdev))
-		pg_align = PFN_UP(mdev->mc.vram_size);
-
-	return drm_gem_vram_fill_create_dumb(file, dev, pg_align, 0, args);
-}
-
 static struct drm_driver driver = {
 	.driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
 	.fops = &mgag200_driver_fops,
@@ -148,10 +106,7 @@ static struct drm_driver driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
-	.debugfs_init = drm_vram_mm_debugfs_init,
-	.dumb_create = mgag200_driver_dumb_create,
-	.dumb_map_offset = drm_gem_vram_driver_dumb_mmap_offset,
-	.gem_prime_mmap = drm_gem_prime_mmap,
+	DRM_GEM_SHMEM_DRIVER_OPS,
 };
 
 static struct pci_driver mgag200_pci_driver = {
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 2392baff618aa..d530df25c822d 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -18,7 +18,7 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem.h>
-#include <drm/drm_gem_vram_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/drm_simple_kms_helper.h>
 
 #include "mgag200_reg.h"
@@ -155,7 +155,8 @@ struct mga_device {
 
 	struct mga_mc			mc;
 
-	size_t vram_fb_available;
+	void __iomem			*vram;
+	size_t				vram_fb_available;
 
 	enum mga_type			type;
 	int				has_sdram;
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index b50c1beb7b7b9..0155d4eb5fa6b 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -14,6 +14,8 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_format_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_plane_helper.h>
@@ -1573,6 +1575,26 @@ mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
 	return MODE_OK;
 }
 
+static void
+mgag200_handle_damage(struct mga_device *mdev, struct drm_framebuffer *fb,
+		      struct drm_rect *clip)
+{
+	struct drm_device *dev = mdev->dev;
+	void *vmap;
+
+	vmap = drm_gem_shmem_vmap(fb->obj[0]);
+	if (drm_WARN_ON(dev, !vmap))
+		return; /* BUG: SHMEM BO should always be vmapped */
+
+	drm_fb_memcpy_dstclip(mdev->vram, vmap, fb, clip);
+
+	drm_gem_shmem_vunmap(fb->obj[0], vmap);
+
+	/* Always scanout image at VRAM offset 0 */
+	mgag200_set_startadd(mdev, (u32)0);
+	mgag200_set_offset(mdev, fb);
+}
+
 static void
 mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 				   struct drm_crtc_state *crtc_state,
@@ -1583,14 +1605,12 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 	struct mga_device *mdev = to_mga_device(dev);
 	struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
 	struct drm_framebuffer *fb = plane_state->fb;
-	struct drm_gem_vram_object *gbo;
-	s64 gpu_addr;
-
-	gbo = drm_gem_vram_of_gem(fb->obj[0]);
-
-	gpu_addr = drm_gem_vram_offset(gbo);
-	if (drm_WARN_ON_ONCE(dev, gpu_addr < 0))
-		return; /* BUG: BO should have been pinned to VRAM. */
+	struct drm_rect fullscreen = {
+		.x1 = 0,
+		.x2 = fb->width,
+		.y1 = 0,
+		.y2 = fb->height,
+	};
 
 	mga_crtc_prepare(crtc);
 
@@ -1606,6 +1626,8 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe,
 		mgag200_g200ev_set_hiprilvl(mdev);
 
 	mga_crtc_commit(crtc);
+
+	mgag200_handle_damage(mdev, fb, &fullscreen);
 }
 
 static void
@@ -1646,20 +1668,13 @@ mgag200_simple_display_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct mga_device *mdev = to_mga_device(dev);
 	struct drm_plane_state *state = plane->state;
 	struct drm_framebuffer *fb = state->fb;
-	struct drm_gem_vram_object *gbo;
-	s64 gpu_addr;
+	struct drm_rect damage;
 
 	if (!fb)
 		return;
 
-	gbo = drm_gem_vram_of_gem(fb->obj[0]);
-
-	gpu_addr = drm_gem_vram_offset(gbo);
-	if (drm_WARN_ON_ONCE(dev, gpu_addr < 0))
-		return; /* BUG: BO should have been pinned to VRAM. */
-
-	mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
-	mgag200_set_offset(mdev, fb);
+	if (drm_atomic_helper_damage_merged(old_state, state, &damage))
+		mgag200_handle_damage(mdev, fb, &damage);
 }
 
 static const struct drm_simple_display_pipe_funcs
@@ -1669,8 +1684,7 @@ mgag200_simple_display_pipe_funcs = {
 	.disable    = mgag200_simple_display_pipe_disable,
 	.check	    = mgag200_simple_display_pipe_check,
 	.update	    = mgag200_simple_display_pipe_update,
-	.prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb,
-	.cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb,
+	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
 static const uint32_t mgag200_simple_display_pipe_formats[] = {
@@ -1689,8 +1703,7 @@ static const uint64_t mgag200_simple_display_pipe_fmtmods[] = {
  */
 
 static const struct drm_mode_config_funcs mgag200_mode_config_funcs = {
-	.fb_create     = drm_gem_fb_create,
-	.mode_valid    = drm_vram_helper_mode_valid,
+	.fb_create     = drm_gem_fb_create_with_dirty,
 	.atomic_check  = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -1729,7 +1742,6 @@ int mgag200_modeset_init(struct mga_device *mdev)
 	dev->mode_config.max_height = MGAG200_MAX_FB_HEIGHT;
 
 	dev->mode_config.preferred_depth = mgag200_preferred_depth(mdev);
-	dev->mode_config.prefer_shadow = 1;
 
 	dev->mode_config.fb_base = mdev->mc.vram_base;
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index e89657630ea71..a683642fe4682 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -32,17 +32,8 @@
 
 int mgag200_mm_init(struct mga_device *mdev)
 {
-	struct drm_vram_mm *vmm;
-	int ret;
 	struct drm_device *dev = mdev->dev;
-
-	vmm = drm_vram_helper_alloc_mm(dev, pci_resource_start(dev->pdev, 0),
-				       mdev->mc.vram_size);
-	if (IS_ERR(vmm)) {
-		ret = PTR_ERR(vmm);
-		DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
-		return ret;
-	}
+	int ret;
 
 	arch_io_reserve_memtype_wc(pci_resource_start(dev->pdev, 0),
 				   pci_resource_len(dev->pdev, 0));
@@ -50,9 +41,22 @@ int mgag200_mm_init(struct mga_device *mdev)
 	mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0),
 					 pci_resource_len(dev->pdev, 0));
 
+	mdev->vram = ioremap(pci_resource_start(dev->pdev, 0),
+			     pci_resource_len(dev->pdev, 0));
+	if (!mdev->vram) {
+		ret = -ENOMEM;
+		goto err_arch_phys_wc_del;
+	}
+
 	mdev->vram_fb_available = mdev->mc.vram_size;
 
 	return 0;
+
+err_arch_phys_wc_del:
+	arch_phys_wc_del(mdev->fb_mtrr);
+	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
+				pci_resource_len(dev->pdev, 0));
+	return ret;
 }
 
 void mgag200_mm_fini(struct mga_device *mdev)
@@ -60,9 +64,7 @@ void mgag200_mm_fini(struct mga_device *mdev)
 	struct drm_device *dev = mdev->dev;
 
 	mdev->vram_fb_available = 0;
-
-	drm_vram_helper_release_mm(dev);
-
+	iounmap(mdev->vram);
 	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
 				pci_resource_len(dev->pdev, 0));
 	arch_phys_wc_del(mdev->fb_mtrr);
-- 
2.26.2

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

Re: [PATCH v2 12/15] drm/mgag200: Remove out-commented suspend/resume helpers

[Emil Velikov]

Hi Thomas,

On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> The suspend/resume helpers are unused. Also remove associated state
> from struct mga_device.
>
Although DPMS in it's traditional sense is no longer a thing, would it
make sense to keep this around for documentation purposes?
In particular, the pci magic and associated PLL/DAC/pixel clock could
be used for dynamic PM.

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

Re: [PATCH v2 13/15] drm/mgag200: Use simple-display data structures

[Emil Velikov]

Hi Thomas,

On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:

>  static void mga_crtc_init(struct mga_device *mdev)
>  {
>         struct drm_device *dev = mdev->dev;
> -       struct mga_crtc *mga_crtc;
> -
> -       mga_crtc = kzalloc(sizeof(struct mga_crtc) +
> -                             (MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)),

The #define MGAG200FB_CONN_LIMIT in mgag200_drv.h is no longer used, correct?

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

Re: [PATCH v2 07/15] drm/mgag200: Set pitch in a separate helper function

[Emil Velikov]

Hi Thomas,

On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:

> @@ -1143,20 +1178,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>         WREG_CRT(13, 0);
>         WREG_CRT(14, 0);
>         WREG_CRT(15, 0);
> -       WREG_CRT(19, pitch & 0xFF);
> -
This write has disappeared without an equivalent or a comment.
Is that intentional?


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

Re: [PATCH v2 15/15] drm/mgag200: Replace VRAM helpers with SHMEM helpers

[Emil Velikov]

On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> The VRAM helpers managed the framebuffer memory for mgag200. This came
> with several problems, as some MGA device require the scanout address
> to be located at VRAM offset 0. It's incompatible with the page-flip
> semantics of DRM's atomic modesettting. With atomic modesetting, old and
> new framebuffers have to be located in VRAM at the same time. So at least
> one of them has to reside at a non-0 offset.
>
> This patch replaces VRAM helpers with SHMEM helpers. GEM SHMEM buffers
> reside in system memory, and are shadow-copied into VRAM during page
> flips. The shadow copy always starts at VRAM offset 0.
>
I suspect MGAG200_FLAG_HW_BUG_NO_STARTADD is left around for
documentation purposes?

I've made a few small comments, but regardless the series is:
Acked-by: Emil Velikov <emil.velikov@collabora.com>

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

Re: [PATCH v2 07/15] drm/mgag200: Set pitch in a separate helper function

[Thomas Zimmermann]

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

Hi Emil

Am 12.05.20 um 12:27 schrieb Emil Velikov:
> Hi Thomas,
> 
> On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
>> @@ -1143,20 +1178,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
>>         WREG_CRT(13, 0);
>>         WREG_CRT(14, 0);
>>         WREG_CRT(15, 0);
>> -       WREG_CRT(19, pitch & 0xFF);
>> -
> This write has disappeared without an equivalent or a comment.
> Is that intentional?

It's still there. It's now located in mgag200_set_offset() and 19 is
written as 0x13. :) I try to follow the naming style of the MGA
programming manual, which prefers hexadecimal numbers.

Best regards
Thomas

> 
> 
> -Emil
> 

-- 
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: 488 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

Re: [PATCH v2 12/15] drm/mgag200: Remove out-commented suspend/resume helpers

[Thomas Zimmermann]

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

Hi

Am 12.05.20 um 12:14 schrieb Emil Velikov:
> Hi Thomas,
> 
> On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> The suspend/resume helpers are unused. Also remove associated state
>> from struct mga_device.
>>
> Although DPMS in it's traditional sense is no longer a thing, would it
> make sense to keep this around for documentation purposes?
> In particular, the pci magic and associated PLL/DAC/pixel clock could
> be used for dynamic PM.

That patch is not about DPMS. The DPMS code is still there. The
suspend/resume helpers were outcommented and I don't know if they ever
worked. Let's remove them.

Best regards
Thomas

> 
> -Emil
> 

-- 
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: 488 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

Re: [PATCH v2 13/15] drm/mgag200: Use simple-display data structures

[Thomas Zimmermann]

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

Hi

Am 12.05.20 um 12:16 schrieb Emil Velikov:
> Hi Thomas,
> 
> On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
>>  static void mga_crtc_init(struct mga_device *mdev)
>>  {
>>         struct drm_device *dev = mdev->dev;
>> -       struct mga_crtc *mga_crtc;
>> -
>> -       mga_crtc = kzalloc(sizeof(struct mga_crtc) +
>> -                             (MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)),
> 
> The #define MGAG200FB_CONN_LIMIT in mgag200_drv.h is no longer used, correct?

Good point. I'll remove the define.

Best regards
Thomas

> 
> -Emil
> _______________________________________________
> 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: 488 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

Re: [PATCH v2 02/15] drm/mgag200: Clean up mga_set_start_address()

[Sam Ravnborg]

Hi Thomas.

On Tue, May 12, 2020 at 10:42:45AM +0200, Thomas Zimmermann wrote:
> All register names and fields are now named according to the
> MGA programming manuals. The function doesn't need the CRTC, so
> callers pass in the device structure directly. The logging now
> uses device-specific macros.
> 
> v2:
> 	* use to_mga_device()
> 	* use MiB instead of MB
> 	* replace empty while loop with do-while, fixes checkpatch warning
> 	* replace uint{8,32}_t with u{8,32}
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Tested-by: John Donnelly <John.p.donnelly@oracle.com>

With the comment below addressed:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  5 ++
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 82 +++++++++++++++-----------
>  2 files changed, 53 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index bc372c2ec79e9..1963876ef3b8c 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -61,6 +61,11 @@
>  		WREG8(MGAREG_CRTC_DATA, v);			\
>  	} while (0)						\
>  
> +#define RREG_ECRT(reg, v)					\
> +	do {							\
> +		WREG8(MGAREG_CRTCEXT_INDEX, reg);		\
> +		v = RREG8(MGAREG_CRTCEXT_DATA);			\
> +	} while (0)						\
>  
>  #define WREG_ECRT(reg, v)					\
>  	do {							\
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index c68ed8b6faf9b..80a3a805c0c4e 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -819,49 +819,53 @@ static void mga_g200wb_commit(struct drm_crtc *crtc)
>  }
>  
>  /*
> -   This is how the framebuffer base address is stored in g200 cards:
> -   * Assume @offset is the gpu_addr variable of the framebuffer object
> -   * Then addr is the number of _pixels_ (not bytes) from the start of
> -     VRAM to the first pixel we want to display. (divided by 2 for 32bit
> -     framebuffers)
> -   * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers
> -   addr<20> -> CRTCEXT0<6>
> -   addr<19-16> -> CRTCEXT0<3-0>
> -   addr<15-8> -> CRTCC<7-0>
> -   addr<7-0> -> CRTCD<7-0>
> -   CRTCEXT0 has to be programmed last to trigger an update and make the
> -   new addr variable take effect.
> + * This is how the framebuffer base address is stored in g200 cards:
> + *   * Assume @offset is the gpu_addr variable of the framebuffer object
> + *   * Then addr is the number of _pixels_ (not bytes) from the start of
> + *     VRAM to the first pixel we want to display. (divided by 2 for 32bit
> + *     framebuffers)
> + *   * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers
> + *      addr<20> -> CRTCEXT0<6>
> + *      addr<19-16> -> CRTCEXT0<3-0>
> + *      addr<15-8> -> CRTCC<7-0>
> + *      addr<7-0> -> CRTCD<7-0>
> + *
> + *  CRTCEXT0 has to be programmed last to trigger an update and make the
> + *  new addr variable take effect.
>   */
> -static void mga_set_start_address(struct drm_crtc *crtc, unsigned offset)
> +static void mgag200_set_startadd(struct mga_device *mdev,
> +				 unsigned long offset)
>  {
> -	struct mga_device *mdev = to_mga_device(crtc->dev);
> -	u32 addr;
> -	int count;
> -	u8 crtcext0;
> +	struct drm_device *dev = mdev->dev;
> +	u32 startadd;
> +	u8 crtcc, crtcd, crtcext0;
>  
> -	while (RREG8(0x1fda) & 0x08);
> -	while (!(RREG8(0x1fda) & 0x08));
> +	startadd = offset / 8;
>  
> -	count = RREG8(MGAREG_VCOUNT) + 2;
> -	while (RREG8(MGAREG_VCOUNT) < count);
> -
> -	WREG8(MGAREG_CRTCEXT_INDEX, 0);
> -	crtcext0 = RREG8(MGAREG_CRTCEXT_DATA);
> -	crtcext0 &= 0xB0;
> -	addr = offset / 8;
> -	/* Can't store addresses any higher than that...
> -	   but we also don't have more than 16MB of memory, so it should be fine. */
> -	WARN_ON(addr > 0x1fffff);
> -	crtcext0 |= (!!(addr & (1<<20)))<<6;
> -	WREG_CRT(0x0d, (u8)(addr & 0xff));
> -	WREG_CRT(0x0c, (u8)(addr >> 8) & 0xff);
> -	WREG_ECRT(0x0, ((u8)(addr >> 16) & 0xf) | crtcext0);
> +	/*
> +	 * Can't store addresses any higher than that, but we also
> +	 * don't have more than 16 MiB of memory, so it should be fine.
> +	 */
> +	drm_WARN_ON(dev, startadd > 0x1fffff);
> +
> +	RREG_ECRT(0x00, crtcext0);
> +
> +	crtcc = (startadd >> 8) & 0xff;
> +	crtcd = startadd & 0xff;
> +	crtcext0 &= 0xb0;
> +	crtcext0 |= ((startadd >> 14) & BIT(6)) |
> +		    ((startadd >> 16) & 0x0f);
> +
> +	WREG_CRT(0x0c, crtcc);
> +	WREG_CRT(0x0d, crtcd);
> +	WREG_ECRT(0x00, crtcext0);
>  }
>  
>  static int mga_crtc_do_set_base(struct drm_crtc *crtc,
>  				struct drm_framebuffer *fb,
>  				int x, int y, int atomic)
>  {
> +	struct mga_device *mdev = to_mga_device(crtc->dev);
>  	struct drm_gem_vram_object *gbo;
>  	int ret;
>  	s64 gpu_addr;
> @@ -882,7 +886,7 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc,
>  		goto err_drm_gem_vram_unpin;
>  	}
>  
> -	mga_set_start_address(crtc, (u32)gpu_addr);
> +	mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
>  
>  	return 0;
>  
> @@ -894,6 +898,16 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc,
>  static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  				  struct drm_framebuffer *old_fb)
>  {
> +	struct drm_device *dev = crtc->dev;
> +	struct mga_device *mdev = dev->dev_private;
> +	unsigned int count;
> +
> +	do { } while (RREG8(0x1fda) & 0x08);
> +	do { } while (!(RREG8(0x1fda) & 0x08));
> +
> +	count = RREG8(MGAREG_VCOUNT) + 2;
> +	do { } while (RREG8(MGAREG_VCOUNT) < count);
> +
>  	return mga_crtc_do_set_base(crtc, old_fb, x, y, 0);
>  }
The changelog still does not explain why this code is moved here.



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

Re: [PATCH v2 15/15] drm/mgag200: Replace VRAM helpers with SHMEM helpers

[Thomas Zimmermann]

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

Hi

Am 12.05.20 um 12:30 schrieb Emil Velikov:
> On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> The VRAM helpers managed the framebuffer memory for mgag200. This came
>> with several problems, as some MGA device require the scanout address
>> to be located at VRAM offset 0. It's incompatible with the page-flip
>> semantics of DRM's atomic modesettting. With atomic modesetting, old and
>> new framebuffers have to be located in VRAM at the same time. So at least
>> one of them has to reside at a non-0 offset.
>>
>> This patch replaces VRAM helpers with SHMEM helpers. GEM SHMEM buffers
>> reside in system memory, and are shadow-copied into VRAM during page
>> flips. The shadow copy always starts at VRAM offset 0.
>>
> I suspect MGAG200_FLAG_HW_BUG_NO_STARTADD is left around for
> documentation purposes?

Yes. For now, that flag seems to be the best documentation of the HW issue.

Besides, there's another known bug in several MGAs: scanlines may not
cross the 4 MiB boundary. If we ever have to work around that problem,
NO_STARTADD might become useful again. One can either scanout from 0, or
align FB scanlines around the 4 MiB boundary; but not both.

> 
> I've made a few small comments, but regardless the series is:
> Acked-by: Emil Velikov <emil.velikov@collabora.com>

Thanks!

Best regards
Thomas

> 
> -Emil
> 

-- 
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: 488 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

Re: [PATCH v2 00/15] drm/mgag200: Convert to atomic modesetting

[Sam Ravnborg]

Hi Thomas.

On Tue, May 12, 2020 at 10:42:43AM +0200, Thomas Zimmermann wrote:
> This patchset converts mgag200 to atomic modesetting. It uses simple
> KMS helpers and SHMEM.
> 
> Patch 1 removes cursor support. The HW cursor is not usable with the
> way universal planes work.
> 
> Patches 2 to 11 untangle the existing modesetting code into smaller
> functions. Specifically, mode setting and plane updates are being
> separated from each other.
> 
> Patch 12 to 14 convert mgag200 to simple KMS helpers and enables atomic
> mode setting.
> 
> Atomically switching plane framebuffers, requires either source or target
> buffer to be located at a non-0 offet. As some HW revisions seem to require
> a framebuffer offset of 0 within the video memory, they do not work with
> atomic modesetting. To resolve this problem, patch 15 converts mgag200
> from VRAM helpers to SHMEM helpers. During plane updates, the content of
> the SHMEM BO is memcpy'd to VRAM. From my observation, performance is not
> nuch different from the original code.
> 
> The patchset has been tested on MGA G200EH hardware.
> 
> v2:
> 	* rebase patchset
> 	* replace uint{8,32}_t with u{8,32} through-out patchset
> 	* define additional register constants
> 	* use helper functions around bpp-shift computations
> 	* split conversion patch
> 	* cleanups
With the one comment addressed patch 1-14 are now all:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

I did not look at the last patch - all the memory stuff is still beyond
me.

Nice to see this driver gettting so much love and care.
The end result is a much nicer driver implmentation.

	Sam

> 
> Thomas Zimmermann (15):
>   drm/mgag200: Remove HW cursor
>   drm/mgag200: Clean up mga_set_start_address()
>   drm/mgag200: Clean up mga_crtc_do_set_base()
>   drm/mgag200: Move mode-setting code into separate helper function
>   drm/mgag200: Split MISC register update into PLL selection, SYNC and
>     I/O
>   drm/mgag200: Update mode registers after plane registers
>   drm/mgag200: Set pitch in a separate helper function
>   drm/mgag200: Set primary plane's format in separate helper function
>   drm/mgag200: Move TAGFIFO reset into separate function
>   drm/mgag200: Move hiprilvl setting into separate functions
>   drm/mgag200: Move register initialization into separate function
>   drm/mgag200: Remove out-commented suspend/resume helpers
>   drm/mgag200: Use simple-display data structures
>   drm/mgag200: Convert to simple KMS helper
>   drm/mgag200: Replace VRAM helpers with SHMEM helpers
> 
>  drivers/gpu/drm/mgag200/Kconfig        |   4 +-
>  drivers/gpu/drm/mgag200/Makefile       |   2 +-
>  drivers/gpu/drm/mgag200/mgag200_drv.c  |  51 +-
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  41 +-
>  drivers/gpu/drm/mgag200/mgag200_main.c |   5 -
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 871 ++++++++++++++-----------
>  drivers/gpu/drm/mgag200/mgag200_reg.h  |  11 +-
>  drivers/gpu/drm/mgag200/mgag200_ttm.c  |  28 +-
>  8 files changed, 528 insertions(+), 485 deletions(-)
> 
> --
> 2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 07/15] drm/mgag200: Set pitch in a separate helper function

[Emil Velikov]

On Tue, 12 May 2020 at 19:34, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi Emil
>
> Am 12.05.20 um 12:27 schrieb Emil Velikov:
> > Hi Thomas,
> >
> > On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> >> @@ -1143,20 +1178,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
> >>         WREG_CRT(13, 0);
> >>         WREG_CRT(14, 0);
> >>         WREG_CRT(15, 0);
> >> -       WREG_CRT(19, pitch & 0xFF);
> >> -
> > This write has disappeared without an equivalent or a comment.
> > Is that intentional?
>
> It's still there. It's now located in mgag200_set_offset() and 19 is
> written as 0x13. :) I try to follow the naming style of the MGA
> programming manual, which prefers hexadecimal numbers.
>
D'oh, didn't register the dec->hex change. Thanks

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

Re: [PATCH v2 12/15] drm/mgag200: Remove out-commented suspend/resume helpers

[Emil Velikov]

On Tue, 12 May 2020 at 19:47, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 12.05.20 um 12:14 schrieb Emil Velikov:
> > Hi Thomas,
> >
> > On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> The suspend/resume helpers are unused. Also remove associated state
> >> from struct mga_device.
> >>
> > Although DPMS in it's traditional sense is no longer a thing, would it
> > make sense to keep this around for documentation purposes?
> > In particular, the pci magic and associated PLL/DAC/pixel clock could
> > be used for dynamic PM.
>
> That patch is not about DPMS. The DPMS code is still there. The
> suspend/resume helpers were outcommented and I don't know if they ever
> worked. Let's remove them.
>
Seems like the idea is to suspend/resume the device on DPMS off/on. A
rather moot point IMHO.
As the DPMS semantics and the whole modeset, got more subtle with
atomic modeset, the idea gets even more moot.

If the documentation has that process - sure nuke it. Although for
dynPM, this code is essential.
Considering a) one has interest in dynPM and b) the code is (close to) working.

The last two are very big ifs so I'll leave it there.

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

Re: [PATCH v2 00/15] drm/mgag200: Convert to atomic modesetting

[Thomas Zimmermann]

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

Hi

Am 12.05.20 um 20:56 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Tue, May 12, 2020 at 10:42:43AM +0200, Thomas Zimmermann wrote:
>> This patchset converts mgag200 to atomic modesetting. It uses simple
>> KMS helpers and SHMEM.
>>
>> Patch 1 removes cursor support. The HW cursor is not usable with the
>> way universal planes work.
>>
>> Patches 2 to 11 untangle the existing modesetting code into smaller
>> functions. Specifically, mode setting and plane updates are being
>> separated from each other.
>>
>> Patch 12 to 14 convert mgag200 to simple KMS helpers and enables atomic
>> mode setting.
>>
>> Atomically switching plane framebuffers, requires either source or target
>> buffer to be located at a non-0 offet. As some HW revisions seem to require
>> a framebuffer offset of 0 within the video memory, they do not work with
>> atomic modesetting. To resolve this problem, patch 15 converts mgag200
>> from VRAM helpers to SHMEM helpers. During plane updates, the content of
>> the SHMEM BO is memcpy'd to VRAM. From my observation, performance is not
>> nuch different from the original code.
>>
>> The patchset has been tested on MGA G200EH hardware.
>>
>> v2:
>> 	* rebase patchset
>> 	* replace uint{8,32}_t with u{8,32} through-out patchset
>> 	* define additional register constants
>> 	* use helper functions around bpp-shift computations
>> 	* split conversion patch
>> 	* cleanups
> With the one comment addressed patch 1-14 are now all:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Sure, I'll add that comment. Thanks for the timely reviews.

Best regards
Thomas

> 
> I did not look at the last patch - all the memory stuff is still beyond
> me.
> 
> Nice to see this driver gettting so much love and care.
> The end result is a much nicer driver implmentation.
> 
> 	Sam
> 
>>
>> Thomas Zimmermann (15):
>>   drm/mgag200: Remove HW cursor
>>   drm/mgag200: Clean up mga_set_start_address()
>>   drm/mgag200: Clean up mga_crtc_do_set_base()
>>   drm/mgag200: Move mode-setting code into separate helper function
>>   drm/mgag200: Split MISC register update into PLL selection, SYNC and
>>     I/O
>>   drm/mgag200: Update mode registers after plane registers
>>   drm/mgag200: Set pitch in a separate helper function
>>   drm/mgag200: Set primary plane's format in separate helper function
>>   drm/mgag200: Move TAGFIFO reset into separate function
>>   drm/mgag200: Move hiprilvl setting into separate functions
>>   drm/mgag200: Move register initialization into separate function
>>   drm/mgag200: Remove out-commented suspend/resume helpers
>>   drm/mgag200: Use simple-display data structures
>>   drm/mgag200: Convert to simple KMS helper
>>   drm/mgag200: Replace VRAM helpers with SHMEM helpers
>>
>>  drivers/gpu/drm/mgag200/Kconfig        |   4 +-
>>  drivers/gpu/drm/mgag200/Makefile       |   2 +-
>>  drivers/gpu/drm/mgag200/mgag200_drv.c  |  51 +-
>>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  41 +-
>>  drivers/gpu/drm/mgag200/mgag200_main.c |   5 -
>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 871 ++++++++++++++-----------
>>  drivers/gpu/drm/mgag200/mgag200_reg.h  |  11 +-
>>  drivers/gpu/drm/mgag200/mgag200_ttm.c  |  28 +-
>>  8 files changed, 528 insertions(+), 485 deletions(-)
>>
>> --
>> 2.26.2

-- 
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: 488 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

Re: [PATCH v2 12/15] drm/mgag200: Remove out-commented suspend/resume helpers

[Daniel Vetter]

On Wed, May 13, 2020 at 10:15:25AM +0100, Emil Velikov wrote:
> On Tue, 12 May 2020 at 19:47, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> > Hi
> >
> > Am 12.05.20 um 12:14 schrieb Emil Velikov:
> > > Hi Thomas,
> > >
> > > On Tue, 12 May 2020 at 09:43, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> > >>
> > >> The suspend/resume helpers are unused. Also remove associated state
> > >> from struct mga_device.
> > >>
> > > Although DPMS in it's traditional sense is no longer a thing, would it
> > > make sense to keep this around for documentation purposes?
> > > In particular, the pci magic and associated PLL/DAC/pixel clock could
> > > be used for dynamic PM.
> >
> > That patch is not about DPMS. The DPMS code is still there. The
> > suspend/resume helpers were outcommented and I don't know if they ever
> > worked. Let's remove them.
> >
> Seems like the idea is to suspend/resume the device on DPMS off/on. A
> rather moot point IMHO.
> As the DPMS semantics and the whole modeset, got more subtle with
> atomic modeset, the idea gets even more moot.

With atomic it's actually a lot easier to do runtime pm in your
modesetting code, since the flow is a lot more structured. There's even a
helper function for that with drm_atomic_helper_commit_tail_rpm, since the
default is optimized for max compatability with old legacy helpers. Maybe
we should switch that around actually.
-Daniel

> If the documentation has that process - sure nuke it. Although for
> dynPM, this code is essential.
> Considering a) one has interest in dynPM and b) the code is (close to) working.
> 
> The last two are very big ifs so I'll leave it there.
> 
> -Emil

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel