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(-)
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);
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);
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)
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 = {
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);
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)
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;
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.
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
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);
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); }
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
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
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; }
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) {-- 2.13.2
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;
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
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.14Please 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
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;
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 ||
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)
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.
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@lists.freedesktop.org
-
Daniel Vetter -
Dave Airlie -
Mathieu Larouche -
Paul Menzel -
Takashi Iwai