[PATCH 00/14] mgag200 fixes

[PATCH 00/14] mgag200 fixes

[Takashi Iwai]

Hi,

this is a summer cleanup sale, a patchset containing various fixes for
mgag200 driver taken from openSUSE / SUSE kernels.  They have been in
our kernels for ages, so at least they are supposed to be stable.

Most of patches came from Egbert, and one PM patch from me that is a
resubmission of the once-post-and-lost patch.


thanks,

Takashi

===

Egbert Eich (13):
  drm/mgag200: Add doublescan and interlace support
  drm/mgag200: Add additional limits for certain G200 variants
  drm/mgag200: Fix memleak in error path in mgag200_bo_create()
  drm/mgag200: Free container instead of member in
    mga_user_framebuffer_destroy()
  drm/mgag200: Initialize data needed to map fbdev memory
  drm/mgag200: Simplify function mgag200_ttm_placement()
  drm/mgag200: Add support for MATROX PCI device IDs 0x520 and 0x521
  drm/mgag200: Cleanup cursor BOs properly
  drm/mgag200: Add missing drm_connector_unregister()
  drm/mgag200: Don't use crtc_* parameters for validation
  drm/mgag200: Consolidate depth/bpp handling
  drm/mgag200: Add command line option to specify preferred depth
  drm/mgag200: Add mode validation debugging code

Takashi Iwai (1):
  drm/mgag200: Implement basic PM support

 drivers/gpu/drm/mgag200/mgag200_drv.c  |  54 +++++++++++
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  22 +++--
 drivers/gpu/drm/mgag200/mgag200_fb.c   |  14 +--
 drivers/gpu/drm/mgag200/mgag200_main.c | 146 +++++++++++++++++++++++++++--
 drivers/gpu/drm/mgag200/mgag200_mode.c | 166 ++++++++++++++++++++++++++-------
 drivers/gpu/drm/mgag200/mgag200_ttm.c  |  11 ++-
 6 files changed, 357 insertions(+), 56 deletions(-)

-- 
2.13.2

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

[PATCH 01/14] drm/mgag200: Add doublescan and interlace support

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

This code was ported from the xorg mga driver.
The doublescreen_allowed and interlace_allowed flags are set
unconditionally for all models for now.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index f4b53588e071..1b2b117a48d1 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1624,7 +1624,9 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 	struct drm_device *dev = connector->dev;
 	struct mga_device *mdev = (struct mga_device*)dev->dev_private;
 	int bpp = 32;
+	int lace;
 
+	lace = (mode->flags & DRM_MODE_FLAG_INTERLACE) ? 2 : 1;
 	if (IS_G200_SE(mdev)) {
 		if (mdev->unique_rev_id == 0x01) {
 			if (mode->hdisplay > 1600)
@@ -1676,8 +1678,10 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 
 	if (mode->crtc_hdisplay > 2048 || mode->crtc_hsync_start > 4096 ||
 	    mode->crtc_hsync_end > 4096 || mode->crtc_htotal > 4096 ||
-	    mode->crtc_vdisplay > 2048 || mode->crtc_vsync_start > 4096 ||
-	    mode->crtc_vsync_end > 4096 || mode->crtc_vtotal > 4096) {
+	    mode->crtc_vdisplay > 2048 * lace ||
+	    mode->crtc_vsync_start > 4096 * lace ||
+	    mode->crtc_vsync_end > 4096 * lace ||
+	    mode->crtc_vtotal > 4096 * lace) {
 		return MODE_BAD;
 	}
 
@@ -1737,6 +1741,9 @@ static struct drm_connector *mga_vga_init(struct drm_device *dev)
 
 	connector = &mga_connector->base;
 
+	connector->interlace_allowed = true;
+	connector->doublescan_allowed = true;
+
 	drm_connector_init(dev, connector,
 			   &mga_vga_connector_funcs, DRM_MODE_CONNECTOR_VGA);
 
-- 
2.13.2

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

[PATCH 02/14] drm/mgag200: Add additional limits for certain G200 variants

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

According to the use UMS X.Org driver G200 WB chips don't support
doublescan

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 1b2b117a48d1..ca5a6df98fa3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1734,6 +1734,7 @@ static struct drm_connector *mga_vga_init(struct drm_device *dev)
 {
 	struct drm_connector *connector;
 	struct mga_connector *mga_connector;
+	struct mga_device *mdev = dev->dev_private;
 
 	mga_connector = kzalloc(sizeof(struct mga_connector), GFP_KERNEL);
 	if (!mga_connector)
@@ -1742,7 +1743,7 @@ static struct drm_connector *mga_vga_init(struct drm_device *dev)
 	connector = &mga_connector->base;
 
 	connector->interlace_allowed = true;
-	connector->doublescan_allowed = true;
+	connector->doublescan_allowed = (mdev->type == G200_WB) ? false : true;
 
 	drm_connector_init(dev, connector,
 			   &mga_vga_connector_funcs, DRM_MODE_CONNECTOR_VGA);
-- 
2.13.2

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

[PATCH 03/14] drm/mgag200: Fix memleak in error path in mgag200_bo_create()

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

The allocated struct mgag200_bo was not freed in all error paths.
This patch consolidates error handling and fixes this.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_ttm.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 3e7e1cd31395..4082eb275b1e 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -324,10 +324,8 @@ int mgag200_bo_create(struct drm_device *dev, int size, int align,
 		return -ENOMEM;
 
 	ret = drm_gem_object_init(dev, &mgabo->gem, size);
-	if (ret) {
-		kfree(mgabo);
-		return ret;
-	}
+	if (ret)
+		goto err;
 
 	mgabo->bo.bdev = &mdev->ttm.bdev;
 
@@ -341,10 +339,13 @@ int mgag200_bo_create(struct drm_device *dev, int size, int align,
 			  align >> PAGE_SHIFT, false, NULL, acc_size,
 			  NULL, NULL, mgag200_bo_ttm_destroy);
 	if (ret)
-		return ret;
+		goto err;
 
 	*pmgabo = mgabo;
 	return 0;
+err:
+	kfree(mgabo);
+	return ret;
 }
 
 static inline u64 mgag200_bo_gpu_offset(struct mgag200_bo *bo)
-- 
2.13.2

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

[PATCH 04/14] drm/mgag200: Free container instead of member in mga_user_framebuffer_destroy()

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

Technically freeing mga_fb->base is the same as freeing mga_fb as 'base'
the first member of the data structure.
Still this makes it cleaner.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index dce8a3eb5a10..cf3a602f453f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -20,7 +20,7 @@ static void mga_user_framebuffer_destroy(struct drm_framebuffer *fb)
 
 	drm_gem_object_unreference_unlocked(mga_fb->obj);
 	drm_framebuffer_cleanup(fb);
-	kfree(fb);
+	kfree(mga_fb);
 }
 
 static const struct drm_framebuffer_funcs mga_fb_funcs = {
-- 
2.13.2

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

[PATCH 05/14] drm/mgag200: Initialize data needed to map fbdev memory

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

Due to a missing initialization there was no way to map fbdev memory.
Thus for example using the Xserver with the fbdev driver failed.
This fix adds initialization for fix.smem_start and fix.smem_len
in the fb_info structure, which fixes this problem.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  | 1 +
 drivers/gpu/drm/mgag200/mgag200_fb.c   | 7 +++++++
 drivers/gpu/drm/mgag200/mgag200_mode.c | 3 ++-
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index c88b6ec88dd2..a724e1931e87 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -250,6 +250,7 @@ void mgag200_modeset_fini(struct mga_device *mdev);
 				/* mgag200_fb.c */
 int mgag200_fbdev_init(struct mga_device *mdev);
 void mgag200_fbdev_fini(struct mga_device *mdev);
+void mgag200_fbdev_set_base(struct mga_device *mdev, unsigned long gpu_addr);
 
 				/* mgag200_main.c */
 int mgag200_framebuffer_init(struct drm_device *dev,
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 5d3b1fac906f..bb9a4c409dc0 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -315,3 +315,10 @@ void mgag200_fbdev_fini(struct mga_device *mdev)
 
 	mga_fbdev_destroy(mdev->dev, mdev->mfbdev);
 }
+
+void mgag200_fbdev_set_base(struct mga_device *mdev, unsigned long gpu_addr)
+{
+	mdev->mfbdev->helper.fbdev->fix.smem_start =
+		mdev->mfbdev->helper.fbdev->apertures->ranges[0].base + gpu_addr;
+	mdev->mfbdev->helper.fbdev->fix.smem_len = mdev->mc.vram_size - gpu_addr;
+}
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index ca5a6df98fa3..afec8f190ee3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -895,7 +895,8 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc,
 		ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
 		if (ret)
 			DRM_ERROR("failed to kmap fbcon\n");
-
+		else
+			mgag200_fbdev_set_base(mdev, gpu_addr);
 	}
 	mgag200_bo_unreserve(bo);
 
-- 
2.13.2

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

[PATCH 06/14] drm/mgag200: Simplify function mgag200_ttm_placement()

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

Just a code refactoring, no functional change.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index a724e1931e87..2ba4060e3363 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -285,12 +285,10 @@ static inline int mgag200_bo_reserve(struct mgag200_bo *bo, bool no_wait)
 	int ret;
 
 	ret = ttm_bo_reserve(&bo->bo, true, no_wait, NULL);
-	if (ret) {
-		if (ret != -ERESTARTSYS && ret != -EBUSY)
-			DRM_ERROR("reserve failed %p\n", bo);
-		return ret;
-	}
-	return 0;
+	if (ret && ret != -ERESTARTSYS && ret != -EBUSY)
+		DRM_ERROR("reserve failed %p\n", bo);
+
+	return ret;
 }
 
 static inline void mgag200_bo_unreserve(struct mgag200_bo *bo)
-- 
2.13.2

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

[PATCH 07/14] drm/mgag200: Add support for MATROX PCI device IDs 0x520 and 0x521

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

Add two more models G200_PCI and G200 for PCI device IDs 0x520 and
0x521, respectively.  They need to retrieve the reference clock and
pclk min/max values from BIOS, and set up the PLLs accordingly.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.c  |   2 +
 drivers/gpu/drm/mgag200/mgag200_drv.h  |   7 ++
 drivers/gpu/drm/mgag200/mgag200_main.c | 113 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/mgag200/mgag200_mode.c |  81 +++++++++++++++++++++++
 4 files changed, 203 insertions(+)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 9ac007880328..03258e9fc6c0 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -29,6 +29,8 @@ module_param_named(modeset, mgag200_modeset, int, 0400);
 static struct drm_driver driver;
 
 static const struct pci_device_id pciidlist[] = {
+	{ PCI_VENDOR_ID_MATROX, 0x520, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_PCI },
+	{ PCI_VENDOR_ID_MATROX, 0x521, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200 },
 	{ PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A },
 	{ PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B },
 	{ PCI_VENDOR_ID_MATROX, 0x530, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EV },
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 2ba4060e3363..1ba559c93b8f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -175,6 +175,8 @@ struct mga_mc {
 };
 
 enum mga_type {
+	G200_PCI,
+	G200,
 	G200_SE_A,
 	G200_SE_B,
 	G200_WB,
@@ -219,6 +221,11 @@ struct mga_device {
 
 	/* SE model number stored in reg 0x1e24 */
 	u32 unique_rev_id;
+	struct  {
+		long ref_clk;
+		long pclk_min;
+		long pclk_max;
+	} bios;
 };
 
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index cf3a602f453f..87c5469d06a7 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -155,6 +155,116 @@ static int mga_vram_init(struct mga_device *mdev)
 	return 0;
 }
 
+#define MGA_BIOS_OFFSET 0x7ffc
+
+static void mgag200_interpret_bios(struct mga_device *mdev,
+				   unsigned char __iomem *bios,
+				   size_t size)
+{
+	static const unsigned int expected_length[6] = {
+		0, 64, 64, 64, 128, 128
+	};
+	unsigned char __iomem *pins;
+	unsigned int pins_len, version;
+	int offset;
+	int tmp;
+
+	if (size < MGA_BIOS_OFFSET + 1)
+		return;
+
+	if (bios[45] != 'M' || bios[46] != 'A' || bios[47] != 'T' ||
+	    bios[48] != 'R' || bios[49] != 'O' || bios[50] != 'X')
+		return;
+
+	offset = (bios[MGA_BIOS_OFFSET + 1] << 8) | bios[MGA_BIOS_OFFSET];
+
+	if (offset + 5 > size)
+		return;
+
+	pins = bios + offset;
+	if (pins[0] == 0x2e && pins[1] == 0x41) {
+		version = pins[5];
+		pins_len = pins[2];
+	} else {
+		version = 1;
+		pins_len = pins[0] + (pins[1] << 8);
+	}
+
+	if (version < 1 || version > 5) {
+		DRM_WARN("Unknown BIOS PInS version: %d\n", version);
+		return;
+	}
+	if (pins_len != expected_length[version]) {
+		DRM_WARN("Unexpected BIOS PInS size: %d expeced: %d\n",
+			 pins_len, expected_length[version]);
+		return;
+	}
+
+	if (offset + pins_len > size)
+		return;
+
+	DRM_DEBUG_KMS("MATROX BIOS PInS version %d size: %d found\n",
+		      version, pins_len);
+
+	switch (version) {
+	case 1:
+		tmp = pins[24] + (pins[25] << 8);
+		if (tmp)
+			mdev->bios.pclk_max = tmp * 10;
+		break;
+	case 2:
+		if (pins[41] != 0xff)
+			mdev->bios.pclk_max = (pins[41] + 100) * 1000;
+		break;
+	case 3:
+		if (pins[36] != 0xff)
+			mdev->bios.pclk_max = (pins[36] + 100) * 1000;
+		if (pins[52] & 0x20)
+			mdev->bios.ref_clk = 14318;
+		break;
+	case 4:
+		if (pins[39] != 0xff)
+			mdev->bios.pclk_max = pins[39] * 4 * 1000;
+		if (pins[92] & 0x01)
+			mdev->bios.ref_clk = 14318;
+		break;
+	case 5:
+		tmp = pins[4] ? 8000 : 6000;
+		if (pins[123] != 0xff)
+			mdev->bios.pclk_min = pins[123] * tmp;
+		if (pins[38] != 0xff)
+			mdev->bios.pclk_max = pins[38] * tmp;
+		if (pins[110] & 0x01)
+			mdev->bios.ref_clk = 14318;
+		break;
+	default:
+		break;
+	}
+}
+
+static void mgag200_probe_bios(struct mga_device *mdev)
+{
+	unsigned char __iomem *bios;
+	size_t size;
+
+	mdev->bios.pclk_min = 50000;
+	mdev->bios.pclk_max = 230000;
+	mdev->bios.ref_clk = 27050;
+
+	bios = pci_map_rom(mdev->dev->pdev, &size);
+	if (!bios)
+		return;
+
+	if (size != 0 && bios[0] == 0x55 && bios[1] == 0xaa)
+		mgag200_interpret_bios(mdev, bios, size);
+
+	pci_unmap_rom(mdev->dev->pdev, bios);
+
+	DRM_DEBUG_KMS("pclk_min: %ld pclk_max: %ld ref_clk: %ld\n",
+		      mdev->bios.pclk_min, mdev->bios.pclk_max,
+		      mdev->bios.ref_clk);
+}
+
 static int mgag200_device_init(struct drm_device *dev,
 			       uint32_t flags)
 {
@@ -187,6 +297,9 @@ static int mgag200_device_init(struct drm_device *dev,
 	if (IS_G200_SE(mdev))
 		mdev->unique_rev_id = RREG32(0x1e24);
 
+	if (mdev->type == G200_PCI || mdev->type == G200)
+		mgag200_probe_bios(mdev);
+
 	ret = mga_vram_init(mdev);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index afec8f190ee3..ba7ee93b57a9 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -92,6 +92,72 @@ static inline void mga_wait_busy(struct mga_device *mdev)
 	} while ((status & 0x01) && time_before(jiffies, timeout));
 }
 
+static int mga_set_plls(struct mga_device *mdev, long clock)
+{
+	const int post_div_max = 7;
+	const int in_div_min = 1;
+	const int in_div_max = 6;
+	const int feed_div_min = 7;
+	const int feed_div_max = 127;
+	u8 testm, testn;
+	u8 n = 0, m = 0, p, s;
+	long f_vco;
+	long computed;
+	long delta, tmp_delta;
+	long ref_clk = mdev->bios.ref_clk;
+	long p_clk_min = mdev->bios.pclk_min;
+	long p_clk_max =  mdev->bios.pclk_max;
+
+	if (clock > p_clk_max) {
+		DRM_WARN("Pixel Clock %ld too high\n", clock);
+		return 1;
+	}
+
+	if (clock <  p_clk_min >> 3)
+		clock = p_clk_min >> 3;
+
+	f_vco = clock;
+	for (p = 0;
+	     p <= post_div_max && f_vco < p_clk_min;
+	     p = (p << 1) + 1, f_vco <<= 1)
+		;
+
+	delta = clock;
+
+	for (testm = in_div_min; testm <= in_div_max; testm++) {
+		for (testn = feed_div_min; testn <= feed_div_max; testn++) {
+			computed = ref_clk * (testn + 1) / (testm + 1);
+			if (computed < f_vco)
+				tmp_delta = f_vco - computed;
+			else
+				tmp_delta  = computed - f_vco;
+			if (tmp_delta < delta) {
+				delta = tmp_delta;
+				m = testm;
+				n = testn;
+			}
+		}
+	}
+	f_vco = ref_clk * (n + 1) / (m + 1);
+	if (f_vco < 100000)
+		s = 0;
+	else if (f_vco < 140000)
+		s = 1;
+	else if (f_vco < 180000)
+		s = 2;
+	else
+		s = 3;
+
+	DRM_DEBUG_KMS("clock: %ld vco: %ld m: %d n: %d p: %d s: %d\n",
+		      clock, f_vco, m, n, p, s);
+
+	WREG_DAC(MGA1064_PIX_PLLC_M, m);
+	WREG_DAC(MGA1064_PIX_PLLC_N, n);
+	WREG_DAC(MGA1064_PIX_PLLC_P, (p | (s << 3)));
+
+	return 0;
+}
+
 #define P_ARRAY_SIZE 9
 
 static int mga_g200se_set_plls(struct mga_device *mdev, long clock)
@@ -698,6 +764,10 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock)
 static int mga_crtc_set_plls(struct mga_device *mdev, long clock)
 {
 	switch(mdev->type) {
+	case G200:
+	case G200_PCI:
+		return mga_set_plls(mdev, clock);
+		break;
 	case G200_SE_A:
 	case G200_SE_B:
 		return mga_g200se_set_plls(mdev, clock);
@@ -944,6 +1014,17 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
 	bppshift = mdev->bpp_shifts[fb->format->cpp[0] - 1];
 
 	switch (mdev->type) {
+	case G200:
+	case G200_PCI:
+		dacvalue[MGA1064_SYS_PLL_M] = 0x04;
+		dacvalue[MGA1064_SYS_PLL_N] = 0x2D;
+		dacvalue[MGA1064_SYS_PLL_P] = 0x19;
+		if (mdev->has_sdram)
+			option = 0x40499121;
+		else
+			option = 0x4049cd21;
+		option2 = 0x00008000;
+		break;
 	case G200_SE_A:
 	case G200_SE_B:
 		dacvalue[MGA1064_VREF_CTL] = 0x03;
-- 
2.13.2

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

Re: [PATCH 07/14] drm/mgag200: Add support for MATROX PCI device IDs 0x520 and 0x521

[Dave Airlie]

On 19 July 2017 at 00:43, Takashi Iwai <tiwai@suse.de> wrote:
> From: Egbert Eich <eich@suse.de>
>
> Add two more models G200_PCI and G200 for PCI device IDs 0x520 and
> 0x521, respectively.  They need to retrieve the reference clock and
> pclk min/max values from BIOS, and set up the PLLs accordingly.

Is there any advantage in supporting these GPUs?

The main idea of the mgag200 driver is for unaccelerated support for
server GPUs.

These look like ancient desktop GPUs, I can't imagine we see them
shipping in any modern server hw, and also I'm guessing they had basic
3D accel features that the old UMS mga driver would "support".

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

Re: [PATCH 07/14] drm/mgag200: Add support for MATROX PCI device IDs 0x520 and 0x521

[Takashi Iwai]

On Thu, 20 Jul 2017 06:17:58 +0200,
Dave Airlie wrote:
> 
> On 19 July 2017 at 00:43, Takashi Iwai <tiwai@suse.de> wrote:
> > From: Egbert Eich <eich@suse.de>
> >
> > Add two more models G200_PCI and G200 for PCI device IDs 0x520 and
> > 0x521, respectively.  They need to retrieve the reference clock and
> > pclk min/max values from BIOS, and set up the PLLs accordingly.
> 
> Is there any advantage in supporting these GPUs?

Heh, you are suggesting that KMS support has no merit? ;)

> The main idea of the mgag200 driver is for unaccelerated support for
> server GPUs.
> 
> These look like ancient desktop GPUs, I can't imagine we see them
> shipping in any modern server hw, and also I'm guessing they had basic
> 3D accel features that the old UMS mga driver would "support".

Right, but I thought we want to move on to support KMS.
If there is another big picture to glean such leftover devices, I'd
love to follow that.  But this patch itself is simple enough and it
can't regress the other existing mgag200 devices, so it's still worth
to consider, IMO.


thanks,

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

Re: [PATCH 07/14] drm/mgag200: Add support for MATROX PCI device IDs 0x520 and 0x521

[Egbert Eich]

Takashi Iwai writes:
 > On Thu, 20 Jul 2017 06:17:58 +0200,
 > Dave Airlie wrote:
 > > 
 > > On 19 July 2017 at 00:43, Takashi Iwai <tiwai@suse.de> wrote:
 > > > From: Egbert Eich <eich@suse.de>
 > > >
 > > > Add two more models G200_PCI and G200 for PCI device IDs 0x520 and
 > > > 0x521, respectively.  They need to retrieve the reference clock and
 > > > pclk min/max values from BIOS, and set up the PLLs accordingly.
 > > 
 > > Is there any advantage in supporting these GPUs?
 > 
 > Heh, you are suggesting that KMS support has no merit? ;)

The merit of this patch was that I could test a lot of functionality of
this  driver without having access to a machine with a built-in mgag200.
Such servers tend not to be home office friendly ;)
Instead I was able to pull a vintage g200 discrete PCI card from my 
collection and shove it into a test machine.

So there was some merit to have this patch and it was small enough to
preserve it.

Cheers,
	Egbert.

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

[PATCH 08/14] drm/mgag200: Cleanup cursor BOs properly

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

Cursor BOs should be cleaned up properly on error or when unloading
the driver.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_main.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 87c5469d06a7..73614154a5ef 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -354,9 +354,12 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 	/* Make small buffers to store a hardware cursor (double buffered icon updates) */
 	mgag200_bo_create(dev, roundup(48*64, PAGE_SIZE), 0, 0,
 					  &mdev->cursor.pixels_1);
-	mgag200_bo_create(dev, roundup(48*64, PAGE_SIZE), 0, 0,
-					  &mdev->cursor.pixels_2);
+	if (mdev->cursor.pixels_1)
+		mgag200_bo_create(dev, roundup(48*64, PAGE_SIZE), 0, 0,
+				  &mdev->cursor.pixels_2);
 	if (!mdev->cursor.pixels_2 || !mdev->cursor.pixels_1) {
+		if (mdev->cursor.pixels_1)
+			drm_gem_object_unreference_unlocked(&mdev->cursor.pixels_1->gem);
 		mdev->cursor.pixels_1 = NULL;
 		mdev->cursor.pixels_2 = NULL;
 		dev_warn(&dev->pdev->dev,
@@ -384,6 +387,10 @@ void mgag200_driver_unload(struct drm_device *dev)
 	if (mdev == NULL)
 		return;
 	mgag200_modeset_fini(mdev);
+	if (mdev->cursor.pixels_1)
+		drm_gem_object_unreference_unlocked(&mdev->cursor.pixels_1->gem);
+	if (mdev->cursor.pixels_2)
+		drm_gem_object_unreference_unlocked(&mdev->cursor.pixels_2->gem);
 	mgag200_fbdev_fini(mdev);
 	drm_mode_config_cleanup(dev);
 	mgag200_mm_fini(mdev);
-- 
2.13.2

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

[PATCH 09/14] drm/mgag200: Add missing drm_connector_unregister()

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

When destroying connector unregister it.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index ba7ee93b57a9..7c8e3c6ace0b 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1796,6 +1796,7 @@ static void mga_connector_destroy(struct drm_connector *connector)
 {
 	struct mga_connector *mga_connector = to_mga_connector(connector);
 	mgag200_i2c_destroy(mga_connector->i2c);
+	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 	kfree(connector);
 }
-- 
2.13.2

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

Re: [PATCH 09/14] drm/mgag200: Add missing drm_connector_unregister()

[Takashi Iwai]

On Tue, 18 Jul 2017 16:43:15 +0200,
Takashi Iwai wrote:
> 
> From: Egbert Eich <eich@suse.de>
> 
> When destroying connector unregister it.
> 
> Signed-off-by: Egbert Eich <eich@suse.de>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

This one should be also superfluous as Daniel pointed for the similar
patch for cirrus.  Please disregard this one.


BTW, I guess the patch was inspired from ast driver code, but then it
implies that the call in ast_mode.c is also superfluous.
Shall I cook up a patch?


thanks,

Takashi

> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index ba7ee93b57a9..7c8e3c6ace0b 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1796,6 +1796,7 @@ static void mga_connector_destroy(struct drm_connector *connector)
>  {
>  	struct mga_connector *mga_connector = to_mga_connector(connector);
>  	mgag200_i2c_destroy(mga_connector->i2c);
> +	drm_connector_unregister(connector);
>  	drm_connector_cleanup(connector);
>  	kfree(connector);
>  }
> -- 
> 2.13.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/14] drm/mgag200: Add missing drm_connector_unregister()

[Daniel Vetter]

On Wed, Jul 19, 2017 at 10:44:15AM +0200, Takashi Iwai wrote:
> On Tue, 18 Jul 2017 16:43:15 +0200,
> Takashi Iwai wrote:
> > 
> > From: Egbert Eich <eich@suse.de>
> > 
> > When destroying connector unregister it.
> > 
> > Signed-off-by: Egbert Eich <eich@suse.de>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> This one should be also superfluous as Daniel pointed for the similar
> patch for cirrus.  Please disregard this one.
> 
> 
> BTW, I guess the patch was inspired from ast driver code, but then it
> implies that the call in ast_mode.c is also superfluous.
> Shall I cook up a patch?

Would be good. The entire drm unload code paths are in fairly bad shape,
with lots of cargo-culted copypasta going on ...
-Daniel
-- 
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

[PATCH 10/14] drm/mgag200: Don't use crtc_* parameters for validation

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

The crtc_* are wrong references as the mode parameters to validate,
use the ones without crtc_ prefix instead.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 7c8e3c6ace0b..a07f67ed6e4a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1758,12 +1758,12 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 		return MODE_H_ILLEGAL;
 	}
 
-	if (mode->crtc_hdisplay > 2048 || mode->crtc_hsync_start > 4096 ||
-	    mode->crtc_hsync_end > 4096 || mode->crtc_htotal > 4096 ||
-	    mode->crtc_vdisplay > 2048 * lace ||
-	    mode->crtc_vsync_start > 4096 * lace ||
-	    mode->crtc_vsync_end > 4096 * lace ||
-	    mode->crtc_vtotal > 4096 * lace) {
+	if (mode->hdisplay > 2048 || mode->hsync_start > 4096 ||
+	    mode->hsync_end > 4096 || mode->htotal > 4096 ||
+	    mode->vdisplay > 2048 * lace ||
+	    mode->vsync_start > 4096 * lace ||
+	    mode->vsync_end > 4096 * lace ||
+	    mode->vtotal > 4096 * lace) {
 		return MODE_BAD;
 	}
 
-- 
2.13.2

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

Re: [PATCH 10/14] drm/mgag200: Don't use crtc_* parameters for validation

[Daniel Vetter]

On Tue, Jul 18, 2017 at 04:43:16PM +0200, Takashi Iwai wrote:
> From: Egbert Eich <eich@suse.de>
> 
> The crtc_* are wrong references as the mode parameters to validate,
> use the ones without crtc_ prefix instead.

They only differ for interlaced/double. You might want to set the
set_crtcinfo flags differently. crtc_ are meant to be the values
programmed into hw, and this looks like hw limits here.

Just fyi, I didn't read the driver code, so probably wrong :-)
-Daniel

> 
> Signed-off-by: Egbert Eich <eich@suse.de>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 7c8e3c6ace0b..a07f67ed6e4a 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1758,12 +1758,12 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
>  		return MODE_H_ILLEGAL;
>  	}
>  
> -	if (mode->crtc_hdisplay > 2048 || mode->crtc_hsync_start > 4096 ||
> -	    mode->crtc_hsync_end > 4096 || mode->crtc_htotal > 4096 ||
> -	    mode->crtc_vdisplay > 2048 * lace ||
> -	    mode->crtc_vsync_start > 4096 * lace ||
> -	    mode->crtc_vsync_end > 4096 * lace ||
> -	    mode->crtc_vtotal > 4096 * lace) {
> +	if (mode->hdisplay > 2048 || mode->hsync_start > 4096 ||
> +	    mode->hsync_end > 4096 || mode->htotal > 4096 ||
> +	    mode->vdisplay > 2048 * lace ||
> +	    mode->vsync_start > 4096 * lace ||
> +	    mode->vsync_end > 4096 * lace ||
> +	    mode->vtotal > 4096 * lace) {
>  		return MODE_BAD;
>  	}
>  
> -- 
> 2.13.2
> 

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

[PATCH 11/14] drm/mgag200: Consolidate depth/bpp handling

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

The depth/bpp handling for chips with limited memory in commit
918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to
16-bit") was incomplete: the bpp limits were applied to mode
validation.

This consolidates dpeth/bpp handling, adds it to mode validation
and moves the code which reads the command line specified depth
into the correct location.

Fixes: 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to 16-bit")
Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  2 ++
 drivers/gpu/drm/mgag200/mgag200_fb.c   |  7 +------
 drivers/gpu/drm/mgag200/mgag200_main.c |  9 ++++++---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 14 +++++++-------
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 1ba559c93b8f..9d7ae61ce915 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -209,6 +209,8 @@ struct mga_device {
 	int				has_sdram;
 	struct drm_display_mode		mode;
 
+	int preferred_bpp;
+
 	int bpp_shifts[4];
 
 	int fb_mtrr;
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index bb9a4c409dc0..173aed918183 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -267,11 +267,6 @@ int mgag200_fbdev_init(struct mga_device *mdev)
 {
 	struct mga_fbdev *mfbdev;
 	int ret;
-	int bpp_sel = 32;
-
-	/* prefer 16bpp on low end gpus with limited VRAM */
-	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024))
-		bpp_sel = 16;
 
 	mfbdev = devm_kzalloc(mdev->dev->dev, sizeof(struct mga_fbdev), GFP_KERNEL);
 	if (!mfbdev)
@@ -294,7 +289,7 @@ int mgag200_fbdev_init(struct mga_device *mdev)
 	/* disable all the possible outputs/crtcs before entering KMS mode */
 	drm_helper_disable_unused_functions(mdev->dev);
 
-	ret = drm_fb_helper_initial_config(&mfbdev->helper, bpp_sel);
+	ret = drm_fb_helper_initial_config(&mfbdev->helper, mdev->preferred_bpp);
 	if (ret)
 		goto err_fb_setup;
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 73614154a5ef..2ec76d6615a8 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -339,10 +339,13 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 
 	drm_mode_config_init(dev);
 	dev->mode_config.funcs = (void *)&mga_mode_funcs;
-	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024))
-		dev->mode_config.preferred_depth = 16;
-	else
+	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024)) {
+		/* prefer 16bpp on low end gpus with limited VRAM */
+		mdev->preferred_bpp = dev->mode_config.preferred_depth = 16;
+	} else {
+		mdev->preferred_bpp = 32;
 		dev->mode_config.preferred_depth = 24;
+	}
 	dev->mode_config.prefer_shadow = 1;
 
 	r = mgag200_modeset_init(mdev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index a07f67ed6e4a..77b8efcd3c65 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1705,9 +1705,15 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 {
 	struct drm_device *dev = connector->dev;
 	struct mga_device *mdev = (struct mga_device*)dev->dev_private;
-	int bpp = 32;
+	int bpp;
 	int lace;
 
+	bpp = mdev->preferred_bpp;
+	/* Validate the mode input by the user */
+	if (connector->cmdline_mode.specified &&
+	    connector->cmdline_mode.bpp_specified)
+		bpp = connector->cmdline_mode.bpp;
+
 	lace = (mode->flags & DRM_MODE_FLAG_INTERLACE) ? 2 : 1;
 	if (IS_G200_SE(mdev)) {
 		if (mdev->unique_rev_id == 0x01) {
@@ -1767,12 +1773,6 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 		return MODE_BAD;
 	}
 
-	/* Validate the mode input by the user */
-	if (connector->cmdline_mode.specified) {
-		if (connector->cmdline_mode.bpp_specified)
-			bpp = connector->cmdline_mode.bpp;
-	}
-
 	if ((mode->hdisplay * mode->vdisplay * (bpp/8)) > mdev->mc.vram_size) {
 		if (connector->cmdline_mode.specified)
 			connector->cmdline_mode.specified = false;
-- 
2.13.2

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

Re: [PATCH 11/14] drm/mgag200: Consolidate depth/bpp handling

[Paul Menzel]

[-- Attachment #1.1: Type: text/plain, Size: 1437 bytes --]

Dear Takashi,


Thank you for posting these patches for review.


Am Dienstag, den 18.07.2017, 16:43 +0200 schrieb Takashi Iwai:
> From: Egbert Eich <eich@suse.de>
> 
> The depth/bpp handling for chips with limited memory in commit
> 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to
> 16-bit") was incomplete: the bpp limits were applied to mode
> validation.
> 
> This consolidates dpeth/bpp handling, adds it to mode validation

depth

> and moves the code which reads the command line specified depth
> into the correct location.
> 
> Fixes: 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to 16-bit")

```
$ git tag --contains 918be888d613 | head -1
v3.14
```

Please mark this as stable then too?

Also, could the original commit author time-stamps be preserved if you
have them in your SUSE repositories? The `Date` line could be added
below the `From` line. That would help to know for how long the changes
have been tested.

> Signed-off-by: Egbert Eich <eich@suse.de>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  2 ++
>  drivers/gpu/drm/mgag200/mgag200_fb.c   |  7 +------
>  drivers/gpu/drm/mgag200/mgag200_main.c |  9 ++++++---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 14 +++++++-------
>  4 files changed, 16 insertions(+), 16 deletions(-)

[…]

Otherwise this looks fine.


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 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 11/14] drm/mgag200: Consolidate depth/bpp handling

[Takashi Iwai]

On Thu, 20 Jul 2017 13:58:14 +0200,
Paul Menzel wrote:
> 
> Dear Takashi,
> 
> 
> Thank you for posting these patches for review.
> 
> 
> Am Dienstag, den 18.07.2017, 16:43 +0200 schrieb Takashi Iwai:
> > From: Egbert Eich <eich@suse.de>
> > 
> > The depth/bpp handling for chips with limited memory in commit
> > 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to
> > 16-bit") was incomplete: the bpp limits were applied to mode
> > validation.
> > 
> > This consolidates dpeth/bpp handling, adds it to mode validation
> 
> depth
> 
> > and moves the code which reads the command line specified depth
> > into the correct location.
> > 
> > Fixes: 918be888d613 ("drm/mgag200: on cards with < 2MB VRAM default to 16-bit")
> 
> ```
> $ git tag --contains 918be888d613 | head -1
> v3.14
> ```
> 
> Please mark this as stable then too?

I guess it deserves for stable, yes.
I leave the decision to the maintainer.

> Also, could the original commit author time-stamps be preserved if you
> have them in your SUSE repositories? The `Date` line could be added
> below the `From` line. That would help to know for how long the changes
> have been tested.

Unfortunately, we don't know how old the original patch was.  The date
isn't preserved in the original patch, either, as it's stored in quilt
queue, not committed in a kernel git tree.

In general, the original date isn't that important, so let's skip that
part.

> 
> > Signed-off-by: Egbert Eich <eich@suse.de>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/gpu/drm/mgag200/mgag200_drv.h  |  2 ++
> >  drivers/gpu/drm/mgag200/mgag200_fb.c   |  7 +------
> >  drivers/gpu/drm/mgag200/mgag200_main.c |  9 ++++++---
> >  drivers/gpu/drm/mgag200/mgag200_mode.c | 14 +++++++-------
> >  4 files changed, 16 insertions(+), 16 deletions(-)
> 
> […]
> 
> Otherwise this looks fine.

Thanks for your review!


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

[PATCH 12/14] drm/mgag200: Add command line option to specify preferred depth

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

G200 is old hardware. When KMS was designed around 2007 none of the
chipsets current at this time had any restrictions to video modes
depending on the depth. Thus video modes are validated independent
of the depth which is purely a property of the scanout buffer.

The mgag200 driver however has some bandwidth limitations and the
bandwidth required depends on the bpp. Since the video mode is validated
before the depth/bpp is actually known we are missing an important piece of
information at validation time.
So far we assumed the highest possible bpp value (except for systems with
video memory <= 2MB). Unfortunately this will kill some badly needed higher
res modes at lower depths.

This patch adds a cludge which allows to set the desired depth for the
mgag200 driver with the boot option mgag200.preferreddepth=16|24.
This is a KLUDGE(!) to get around the shortcomings of the mode setting
model for older hardware.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.c  | 11 +++++++++++
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  2 ++
 drivers/gpu/drm/mgag200/mgag200_main.c | 17 +++++++++++++----
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 03258e9fc6c0..7a71ac729a81 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -22,9 +22,12 @@
  * functions
  */
 int mgag200_modeset = -1;
+int mgag200_preferred_depth __read_mostly;
 
 MODULE_PARM_DESC(modeset, "Disable/Enable modesetting");
 module_param_named(modeset, mgag200_modeset, int, 0400);
+MODULE_PARM_DESC(preferreddepth, "Set preferred bpp");
+module_param_named(preferreddepth, mgag200_preferred_depth, int, 0400);
 
 static struct drm_driver driver;
 
@@ -122,6 +125,14 @@ static int __init mgag200_init(void)
 
 	if (mgag200_modeset == 0)
 		return -EINVAL;
+	switch (mgag200_preferred_depth) {
+	case 0: /* driver default */
+	case 16:
+	case 24:
+		break;
+	default:
+		return -EINVAL;
+	}
 	return drm_pci_init(&driver, &mgag200_pci_driver);
 }
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 9d7ae61ce915..949bbcbf9d19 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -318,4 +318,6 @@ int mga_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 						uint32_t handle, uint32_t width, uint32_t height);
 int mga_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
 
+extern int mgag200_preferred_depth __read_mostly;
+
 #endif				/* __MGAG200_DRV_H__ */
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index 2ec76d6615a8..56970eced8d7 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -339,12 +339,21 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 
 	drm_mode_config_init(dev);
 	dev->mode_config.funcs = (void *)&mga_mode_funcs;
-	if (IS_G200_SE(mdev) && mdev->mc.vram_size < (2048*1024)) {
+	if (mgag200_preferred_depth == 0) {
 		/* prefer 16bpp on low end gpus with limited VRAM */
-		mdev->preferred_bpp = dev->mode_config.preferred_depth = 16;
+		if (IS_G200_SE(mdev) && mdev->mc.vram_size <= 2048 * 1024) {
+			mdev->preferred_bpp =
+				dev->mode_config.preferred_depth = 16;
+		} else {
+			mdev->preferred_bpp = 32;
+			dev->mode_config.preferred_depth = 24;
+		}
 	} else {
-		mdev->preferred_bpp = 32;
-		dev->mode_config.preferred_depth = 24;
+		if (mgag200_preferred_depth == 16)
+			mdev->preferred_bpp = 16;
+		else /* mgag200_preferred_depth == 24 */
+			mdev->preferred_bpp = 32;
+		dev->mode_config.preferred_depth = mgag200_preferred_depth;
 	}
 	dev->mode_config.prefer_shadow = 1;
 
-- 
2.13.2

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

[PATCH 13/14] drm/mgag200: Add mode validation debugging code

[Takashi Iwai]

From: Egbert Eich <eich@suse.de>

Give more verbose debug message at mode bandwidth checks.

Signed-off-by: Egbert Eich <eich@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 51 +++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 77b8efcd3c65..50efb10f86c5 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1698,6 +1698,21 @@ static uint32_t mga_vga_calculate_mode_bandwidth(struct drm_display_mode *mode,
 	return (uint32_t)(bandwidth);
 }
 
+static bool mga_vga_check_mode_bandwidth(struct drm_display_mode *mode,
+					 int bits_per_pixel,
+					 unsigned int max_bw)
+{
+	unsigned int bw;
+
+	bw = mga_vga_calculate_mode_bandwidth(mode, bits_per_pixel);
+	if (bw > max_bw) {
+		DRM_DEBUG_KMS("Mode %d:%s exceeds bandwidth: %d > %d",
+			      mode->base.id, mode->name, bw, max_bw);
+		return false;
+	}
+	return true;
+}
+
 #define MODE_BANDWIDTH	MODE_BAD
 
 static int mga_vga_mode_valid(struct drm_connector *connector,
@@ -1721,20 +1736,20 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 				return MODE_VIRTUAL_X;
 			if (mode->vdisplay > 1200)
 				return MODE_VIRTUAL_Y;
-			if (mga_vga_calculate_mode_bandwidth(mode, bpp)
-				> (24400 * 1024))
+			if (mga_vga_check_mode_bandwidth(mode, bpp,
+							 24400 * 1024))
 				return MODE_BANDWIDTH;
 		} else if (mdev->unique_rev_id == 0x02) {
 			if (mode->hdisplay > 1920)
 				return MODE_VIRTUAL_X;
 			if (mode->vdisplay > 1200)
 				return MODE_VIRTUAL_Y;
-			if (mga_vga_calculate_mode_bandwidth(mode, bpp)
-				> (30100 * 1024))
+			if (mga_vga_check_mode_bandwidth(mode, bpp,
+							 30100 * 1024))
 				return MODE_BANDWIDTH;
 		} else {
-			if (mga_vga_calculate_mode_bandwidth(mode, bpp)
-				> (55000 * 1024))
+			if (mga_vga_check_mode_bandwidth(mode, bpp,
+							 55000 * 1024))
 				return MODE_BANDWIDTH;
 		}
 	} else if (mdev->type == G200_WB) {
@@ -1742,21 +1757,17 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 			return MODE_VIRTUAL_X;
 		if (mode->vdisplay > 1024)
 			return MODE_VIRTUAL_Y;
-		if (mga_vga_calculate_mode_bandwidth(mode,
-			bpp > (31877 * 1024)))
+		if (mga_vga_check_mode_bandwidth(mode, bpp, 31877 * 1024))
+			return MODE_BANDWIDTH;
+	} else if (mdev->type == G200_EV) {
+		if (mga_vga_check_mode_bandwidth(mode, bpp, 32700 * 1024))
+			return MODE_BANDWIDTH;
+	} else if (mdev->type == G200_EH) {
+		if (mga_vga_check_mode_bandwidth(mode, bpp, 37500 * 1024))
+			return MODE_BANDWIDTH;
+	} else if (mdev->type == G200_ER) {
+		if (mga_vga_check_mode_bandwidth(mode, bpp, 55000 * 1024))
 			return MODE_BANDWIDTH;
-	} else if (mdev->type == G200_EV &&
-		(mga_vga_calculate_mode_bandwidth(mode, bpp)
-			> (32700 * 1024))) {
-		return MODE_BANDWIDTH;
-	} else if (mdev->type == G200_EH &&
-		(mga_vga_calculate_mode_bandwidth(mode, bpp)
-			> (37500 * 1024))) {
-		return MODE_BANDWIDTH;
-	} else if (mdev->type == G200_ER &&
-		(mga_vga_calculate_mode_bandwidth(mode,
-			bpp) > (55000 * 1024))) {
-		return MODE_BANDWIDTH;
 	}
 
 	if ((mode->hdisplay % 8) != 0 || (mode->hsync_start % 8) != 0 ||
-- 
2.13.2

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

[PATCH 14/14] drm/mgag200: Implement basic PM support

[Takashi Iwai]

Add a basic PM support to mgag200 driver.
As there is no hardware specific init codes for this device, the
callbacks are written in a fairly simple way.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.c | 41 +++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 7a71ac729a81..8ae36c1ab4d2 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/console.h>
 #include <drm/drmP.h>
+#include <drm/drm_crtc_helper.h>
 
 #include "mgag200_drv.h"
 
@@ -81,6 +82,45 @@ static void mga_pci_remove(struct pci_dev *pdev)
 	drm_put_dev(dev);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int mgag200_pm_suspend(struct device *dev)
+{
+	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct mga_device *mdev = drm_dev->dev_private;
+
+	drm_kms_helper_poll_disable(drm_dev);
+
+	if (mdev->mfbdev) {
+		console_lock();
+		drm_fb_helper_set_suspend(&mdev->mfbdev->helper, 1);
+		console_unlock();
+	}
+
+	return 0;
+}
+
+static int mgag200_pm_resume(struct device *dev)
+{
+	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct mga_device *mdev = drm_dev->dev_private;
+
+	drm_helper_resume_force_mode(drm_dev);
+
+	if (mdev->mfbdev) {
+		console_lock();
+		drm_fb_helper_set_suspend(&mdev->mfbdev->helper, 0);
+		console_unlock();
+	}
+
+	drm_kms_helper_poll_enable(drm_dev);
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops mgag200_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mgag200_pm_suspend, mgag200_pm_resume)
+};
+
 static const struct file_operations mgag200_driver_fops = {
 	.owner = THIS_MODULE,
 	.open = drm_open,
@@ -116,6 +156,7 @@ static struct pci_driver mgag200_pci_driver = {
 	.id_table = pciidlist,
 	.probe = mga_pci_probe,
 	.remove = mga_pci_remove,
+	.driver.pm = &mgag200_pm_ops,
 };
 
 static int __init mgag200_init(void)
-- 
2.13.2

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

Re: [PATCH 00/14] mgag200 fixes

[Mathieu Larouche]

On 18/07/2017 10:43 AM, Takashi Iwai wrote:
> Hi,
>
> this is a summer cleanup sale, a patchset containing various fixes for
> mgag200 driver taken from openSUSE / SUSE kernels.  They have been in
> our kernels for ages, so at least they are supposed to be stable.
>
> Most of patches came from Egbert, and one PM patch from me that is a
> resubmission of the once-post-and-lost patch.
>
>
> thanks,
>
> Takashi
>
> ===
>
> Egbert Eich (13):
>    drm/mgag200: Add doublescan and interlace support
>    drm/mgag200: Add additional limits for certain G200 variants
>    drm/mgag200: Fix memleak in error path in mgag200_bo_create()
>    drm/mgag200: Free container instead of member in
>      mga_user_framebuffer_destroy()
>    drm/mgag200: Initialize data needed to map fbdev memory
>    drm/mgag200: Simplify function mgag200_ttm_placement()
>    drm/mgag200: Add support for MATROX PCI device IDs 0x520 and 0x521
>    drm/mgag200: Cleanup cursor BOs properly
>    drm/mgag200: Add missing drm_connector_unregister()
>    drm/mgag200: Don't use crtc_* parameters for validation
>    drm/mgag200: Consolidate depth/bpp handling
>    drm/mgag200: Add command line option to specify preferred depth
>    drm/mgag200: Add mode validation debugging code
>
> Takashi Iwai (1):
>    drm/mgag200: Implement basic PM support
>
>   drivers/gpu/drm/mgag200/mgag200_drv.c  |  54 +++++++++++
>   drivers/gpu/drm/mgag200/mgag200_drv.h  |  22 +++--
>   drivers/gpu/drm/mgag200/mgag200_fb.c   |  14 +--
>   drivers/gpu/drm/mgag200/mgag200_main.c | 146 +++++++++++++++++++++++++++--
>   drivers/gpu/drm/mgag200/mgag200_mode.c | 166 ++++++++++++++++++++++++++-------
>   drivers/gpu/drm/mgag200/mgag200_ttm.c  |  11 ++-
>   6 files changed, 357 insertions(+), 56 deletions(-)
>

Patches were tested against G200eW3, G200e4 & G200eH3 and it's working 
fine and we haven't seen any issues.

There's one thing though, the patch "[PATCH 01/14] drm/mgag200: Add 
doublescan and interlace support" may cause problems as doublescan and 
interlace aren't tested and aren't officially supported on the G200 
server line products. So, I'm wondering if it shouldn't be kept disabled 
for them.

-- 
Mathieu Larouche Ing./Eng.
Software Designer
Matrox Graphics Inc.
Phone : 514 822-6000 x7905
Email : mathieu.larouche@matrox.com

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

Re: [PATCH 00/14] mgag200 fixes

[Takashi Iwai]

On Tue, 01 Aug 2017 21:25:11 +0200,
Mathieu Larouche wrote:
> 
> On 18/07/2017 10:43 AM, Takashi Iwai wrote:
> > Hi,
> >
> > this is a summer cleanup sale, a patchset containing various fixes for
> > mgag200 driver taken from openSUSE / SUSE kernels.  They have been in
> > our kernels for ages, so at least they are supposed to be stable.
> >
> > Most of patches came from Egbert, and one PM patch from me that is a
> > resubmission of the once-post-and-lost patch.
> >
> >
> > thanks,
> >
> > Takashi
> >
> > ===
> >
> > Egbert Eich (13):
> >    drm/mgag200: Add doublescan and interlace support
> >    drm/mgag200: Add additional limits for certain G200 variants
> >    drm/mgag200: Fix memleak in error path in mgag200_bo_create()
> >    drm/mgag200: Free container instead of member in
> >      mga_user_framebuffer_destroy()
> >    drm/mgag200: Initialize data needed to map fbdev memory
> >    drm/mgag200: Simplify function mgag200_ttm_placement()
> >    drm/mgag200: Add support for MATROX PCI device IDs 0x520 and 0x521
> >    drm/mgag200: Cleanup cursor BOs properly
> >    drm/mgag200: Add missing drm_connector_unregister()
> >    drm/mgag200: Don't use crtc_* parameters for validation
> >    drm/mgag200: Consolidate depth/bpp handling
> >    drm/mgag200: Add command line option to specify preferred depth
> >    drm/mgag200: Add mode validation debugging code
> >
> > Takashi Iwai (1):
> >    drm/mgag200: Implement basic PM support
> >
> >   drivers/gpu/drm/mgag200/mgag200_drv.c  |  54 +++++++++++
> >   drivers/gpu/drm/mgag200/mgag200_drv.h  |  22 +++--
> >   drivers/gpu/drm/mgag200/mgag200_fb.c   |  14 +--
> >   drivers/gpu/drm/mgag200/mgag200_main.c | 146 +++++++++++++++++++++++++++--
> >   drivers/gpu/drm/mgag200/mgag200_mode.c | 166 ++++++++++++++++++++++++++-------
> >   drivers/gpu/drm/mgag200/mgag200_ttm.c  |  11 ++-
> >   6 files changed, 357 insertions(+), 56 deletions(-)
> >
> 
> Patches were tested against G200eW3, G200e4 & G200eH3 and it's working
> fine and we haven't seen any issues.

Thanks for testing!

> There's one thing though, the patch "[PATCH 01/14] drm/mgag200: Add
> doublescan and interlace support" may cause problems as doublescan and
> interlace aren't tested and aren't officially supported on the G200
> server line products. So, I'm wondering if it shouldn't be kept
> disabled for them.

OK, that's good to know.  My understanding is that the patch was
brought only for adapting the UMS X driver quality, but not for
actually fixing the real bugs.  Let's drop that patch, then.


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