This patchset puts device initialization in the correct order and adds support for G200 Desktop chips (PCI ids 0x520 and 0x521).
The first 7 patches prepare the driver. Desktop chips would probably work without them, but since we're at it we can also do it right.
Patch 1 enables cached page mappings GEM buffers. SHMEM supports this well now and the MGA device does not access the buffer memory directly. So now corrupt display output is to be expected.
Patches 2 to 6 fix the initialization of device registers. Several fundamental registers were only done late during device initialization. This was probably not a problem in practice, as the VGA BIOS does the setup iduring POST anyway. These patches move the code to the beginning of the device initialization. If we ever have to POST a MGA device from the driver, the corect order of operations counts.
G200SEs store a unique id in the device structure. Patch 7 moves the value to model-specific data area. This will be helpful for patch 8.
Patch 8 adds support for desktop chips' PCI ids. all the memory and modesetting code continues to work as before. The PLL setup code gets an additional helper for the new HW. PCI and DAC regsiters get a few new default values. Most significantly, the driver parses the VGA BIOS for clock settings. It's all separate from the server code, so no regressions are to be expected.
The new HW support is based on an earlier patch the was posted in July 2017. [1]
Tested on G200EW and G200 AGP hardware by running the fbdev console, Weston and Gnome on Xorg.
[1] https://lists.freedesktop.org/archives/dri-devel/2017-July/147647.html
Thomas Zimmermann (8): drm/mgag200: Enable caching for SHMEM pages drm/mgag200: Move register initialization into helper function drm/mgag200: Initialize PCI registers early during device setup drm/mgag200: Enable MGA mode during device register initialization drm/mgag200: Set MISC memory flags in mm init code drm/mgag200: Clear <page> field during MM init drm/mgag200: Move G200SE's unique id into model-specific data drm/mgag200: Add support for G200 desktop cards
MAINTAINERS | 2 +- drivers/gpu/drm/mgag200/Kconfig | 12 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 213 +++++++++++++++++++++++-- drivers/gpu/drm/mgag200/mgag200_drv.h | 19 ++- drivers/gpu/drm/mgag200/mgag200_mm.c | 8 + drivers/gpu/drm/mgag200/mgag200_mode.c | 153 +++++++++++------- drivers/gpu/drm/mgag200/mgag200_reg.h | 4 + 7 files changed, 328 insertions(+), 83 deletions(-)
-- 2.27.0
SHMEM pages use write-combine caching by default, but can also use the platform's default page caching. Doing so may improve the performance of I/O on the framebuffer.
Mgag200's hardware does not access framebuffer pages directly (i.e., via DMA), so enabling caching does not have an effect on consistency of the framebuffer memory or the displayed data.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_drv.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index e19660f4a637..7189c7745baf 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -36,6 +36,7 @@ static struct drm_driver mgag200_driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .gem_create_object = drm_gem_shmem_create_object_cached, DRM_GEM_SHMEM_DRIVER_OPS, };
The mgag200 driver maps registers into the address space. Move the code into a separate helper function. No functional changes.
One small difference is in the handling of SDRAM/SGRAM. MGA devices can come with either SDRAM or SGRAM. So far, the driver checked for SDRAM, which is the common case. The patch moves this code into a separate helper and checks for SGRAM, which is the special case. The test itself is the same as before.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_drv.c | 37 ++++++++++++++++++++++----- drivers/gpu/drm/mgag200/mgag200_reg.h | 2 ++ 2 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 7189c7745baf..e50c682c4702 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -44,18 +44,26 @@ static struct drm_driver mgag200_driver = { * DRM device */
-static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) +static bool mgag200_has_sgram(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; - int ret, option; + u32 option; + int ret;
- mdev->flags = mgag200_flags_from_driver_data(flags); - mdev->type = mgag200_type_from_driver_data(flags); + ret = pci_read_config_dword(dev->pdev, PCI_MGA_OPTION, &option); + if (drm_WARN(dev, ret, "failed to read PCI config dword: %d\n", ret)) + return false; + + return !!(option & PCI_MGA_OPTION_HARDPWMSK); +}
- pci_read_config_dword(dev->pdev, PCI_MGA_OPTION, &option); - mdev->has_sdram = !(option & (1 << 14)); +static int mgag200_regs_init(struct mga_device *mdev) +{ + struct drm_device *dev = &mdev->base; + + mdev->has_sdram = !mgag200_has_sgram(mdev);
- /* BAR 0 is the framebuffer, BAR 1 contains registers */ + /* BAR 1 contains registers */ mdev->rmmio_base = pci_resource_start(dev->pdev, 1); mdev->rmmio_size = pci_resource_len(dev->pdev, 1);
@@ -69,6 +77,21 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) if (mdev->rmmio == NULL) return -ENOMEM;
+ return 0; +} + +static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) +{ + struct drm_device *dev = &mdev->base; + int ret; + + mdev->flags = mgag200_flags_from_driver_data(flags); + mdev->type = mgag200_type_from_driver_data(flags); + + ret = mgag200_regs_init(mdev); + if (ret) + return ret; + /* stash G200 SE model number for later use */ if (IS_G200_SE(mdev)) { mdev->unique_rev_id = RREG32(0x1e24); diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index c3b7bcad52ed..a44c08bf4074 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -282,6 +282,8 @@ #define PCI_MGA_OPTION2 0x50 #define PCI_MGA_OPTION3 0x54
+#define PCI_MGA_OPTION_HARDPWMSK BIT(14) + #define RAMDAC_OFFSET 0x3c00
/* TVP3026 direct registers */
So far, PCI option registers were initialized as part of modesetting, which is late in the process. As these registers control fundamental operation, they should be set early.
The patch moves the PCI option handling into device register setup, before even the device MMIO memory is being mapped. No functional changes made.
Moving the PCI code next to the device-register setup also allows to remove the has_sdram field from struct mga_device. The state is now local to the init helper.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_drv.c | 32 +++++++++++++++++++- drivers/gpu/drm/mgag200/mgag200_drv.h | 1 - drivers/gpu/drm/mgag200/mgag200_mode.c | 41 -------------------------- 3 files changed, 31 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index e50c682c4702..3dbb00045c24 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -60,8 +60,38 @@ static bool mgag200_has_sgram(struct mga_device *mdev) static int mgag200_regs_init(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; + u32 option, option2; + + switch (mdev->type) { + case G200_SE_A: + case G200_SE_B: + if (mgag200_has_sgram(mdev)) + option |= PCI_MGA_OPTION_HARDPWMSK; + option2 = 0x00008000; + break; + case G200_WB: + case G200_EW3: + option = 0x41049120; + option2 = 0x0000b000; + break; + case G200_EV: + option = 0x00000120; + option2 = 0x0000b000; + break; + case G200_EH: + case G200_EH3: + option = 0x00000120; + option2 = 0x0000b000; + break; + default: + option = 0; + option2 = 0; + }
- mdev->has_sdram = !mgag200_has_sgram(mdev); + if (option) + pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option); + if (option2) + pci_write_config_dword(dev->pdev, PCI_MGA_OPTION2, option2);
/* BAR 1 contains registers */ mdev->rmmio_base = pci_resource_start(dev->pdev, 1); diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 3817520bfefc..819c03cc626b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -161,7 +161,6 @@ struct mga_device { size_t vram_fb_available;
enum mga_type type; - int has_sdram;
int bpp_shifts[4];
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index e0d037a7413c..3aa078e69a5a 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -9,7 +9,6 @@ */
#include <linux/delay.h> -#include <linux/pci.h>
#include <drm/drm_atomic_helper.h> #include <drm/drm_atomic_state_helper.h> @@ -877,45 +876,6 @@ static void mgag200_set_startadd(struct mga_device *mdev, WREG_ECRT(0x00, crtcext0); }
-static void mgag200_set_pci_regs(struct mga_device *mdev) -{ - uint32_t option = 0, option2 = 0; - struct drm_device *dev = &mdev->base; - - switch (mdev->type) { - case G200_SE_A: - case G200_SE_B: - if (mdev->has_sdram) - option = 0x40049120; - else - option = 0x4004d120; - option2 = 0x00008000; - break; - case G200_WB: - case G200_EW3: - option = 0x41049120; - option2 = 0x0000b000; - break; - case G200_EV: - option = 0x00000120; - option2 = 0x0000b000; - break; - case G200_EH: - case G200_EH3: - option = 0x00000120; - option2 = 0x0000b000; - break; - case G200_ER: - break; - } - - if (option) - pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option); - - if (option2) - pci_write_config_dword(dev->pdev, PCI_MGA_OPTION2, option2); -} - static void mgag200_set_dac_regs(struct mga_device *mdev) { size_t i; @@ -988,7 +948,6 @@ static void mgag200_init_regs(struct mga_device *mdev) { u8 crtc11, crtcext3, crtcext4, misc;
- mgag200_set_pci_regs(mdev); mgag200_set_dac_regs(mdev);
WREG_SEQ(2, 0x0f);
MGA cards can run in traditional VGA mode or an enhanced MGA mode; with the latter being required for KMS. So far, MGA mode was enabled during modesetting. As it's fundamental for device operation, the patch moves it next to the device register setup.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_drv.c | 5 +++++ drivers/gpu/drm/mgag200/mgag200_mode.c | 6 +----- drivers/gpu/drm/mgag200/mgag200_reg.h | 2 ++ 3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 3dbb00045c24..ac9ac5b6d587 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -61,6 +61,7 @@ static int mgag200_regs_init(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; u32 option, option2; + u8 crtcext3;
switch (mdev->type) { case G200_SE_A: @@ -107,6 +108,10 @@ static int mgag200_regs_init(struct mga_device *mdev) if (mdev->rmmio == NULL) return -ENOMEM;
+ RREG_ECRT(0x03, crtcext3); + crtcext3 |= MGAREG_CRTCEXT3_MGAMODE; + WREG_ECRT(0x03, crtcext3); + return 0; }
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 3aa078e69a5a..7161b1651aa0 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -946,7 +946,7 @@ static void mgag200_set_dac_regs(struct mga_device *mdev)
static void mgag200_init_regs(struct mga_device *mdev) { - u8 crtc11, crtcext3, crtcext4, misc; + u8 crtc11, crtcext4, misc;
mgag200_set_dac_regs(mdev);
@@ -961,12 +961,8 @@ static void mgag200_init_regs(struct mga_device *mdev) WREG_CRT(14, 0); WREG_CRT(15, 0);
- RREG_ECRT(0x03, crtcext3); - - crtcext3 |= BIT(7); /* enable MGA mode */ crtcext4 = 0x00;
- WREG_ECRT(0x03, crtcext3); WREG_ECRT(0x04, crtcext4);
RREG_CRT(0x11, crtc11); diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index a44c08bf4074..977be0565c06 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -256,6 +256,8 @@ #define MGAREG_CRTCEXT1_VSYNCOFF BIT(5) #define MGAREG_CRTCEXT1_HSYNCOFF BIT(4)
+#define MGAREG_CRTCEXT3_MGAMODE BIT(7) + /* Cursor X and Y position */ #define MGA_CURPOSXL 0x3c0c #define MGA_CURPOSXH 0x3c0d
The modesetting code initialized several memory-related flags in the MISC register. Move this code to MM initialization.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_mm.c | 6 ++++++ drivers/gpu/drm/mgag200/mgag200_mode.c | 4 +--- 2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c index 7b69392bcb89..1b1918839e1e 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mm.c +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c @@ -90,9 +90,15 @@ static void mgag200_mm_release(struct drm_device *dev, void *ptr) int mgag200_mm_init(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; + u8 misc; resource_size_t start, len; int ret;
+ misc = RREG8(MGA_MISC_IN); + misc |= MGAREG_MISC_RAMMAPEN | + MGAREG_MISC_HIGH_PG_SEL; + WREG8(MGA_MISC_OUT, misc); + /* BAR 0 is VRAM */ start = pci_resource_start(dev->pdev, 0); len = pci_resource_len(dev->pdev, 0); diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 7161b1651aa0..66818ee10694 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -978,9 +978,7 @@ static void mgag200_init_regs(struct mga_device *mdev) WREG_ECRT(0x34, 0x5);
misc = RREG8(MGA_MISC_IN); - misc |= MGAREG_MISC_IOADSEL | - MGAREG_MISC_RAMMAPEN | - MGAREG_MISC_HIGH_PG_SEL; + misc |= MGAREG_MISC_IOADSEL; WREG8(MGA_MISC_OUT, misc); }
The modesetting code initialized the memory-related register CRTCEXT4. Move this code to MM initialization.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_mm.c | 2 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 6 +----- 2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c index 1b1918839e1e..641f1aa992be 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mm.c +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c @@ -94,6 +94,8 @@ int mgag200_mm_init(struct mga_device *mdev) resource_size_t start, len; int ret;
+ WREG_ECRT(0x04, 0x00); + misc = RREG8(MGA_MISC_IN); misc |= MGAREG_MISC_RAMMAPEN | MGAREG_MISC_HIGH_PG_SEL; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 66818ee10694..4fa64cf884cb 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -946,7 +946,7 @@ static void mgag200_set_dac_regs(struct mga_device *mdev)
static void mgag200_init_regs(struct mga_device *mdev) { - u8 crtc11, crtcext4, misc; + u8 crtc11, misc;
mgag200_set_dac_regs(mdev);
@@ -961,10 +961,6 @@ static void mgag200_init_regs(struct mga_device *mdev) WREG_CRT(14, 0); WREG_CRT(15, 0);
- crtcext4 = 0x00; - - WREG_ECRT(0x04, crtcext4); - RREG_CRT(0x11, crtc11); crtc11 &= ~(MGAREG_CRTC11_CRTCPROTECT | MGAREG_CRTC11_VINTEN |
The unique revision id is only useful for G200SE devices. Store the value in model-specific data within struct mga_device. While at it, the patch also adds an init helper for the value.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/mgag200/mgag200_drv.c | 19 +++++++++++++------ drivers/gpu/drm/mgag200/mgag200_drv.h | 8 ++++++-- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 +++++++++++------- 3 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index ac9ac5b6d587..f7652e16365c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -115,6 +115,17 @@ static int mgag200_regs_init(struct mga_device *mdev) return 0; }
+static void mgag200_g200se_init_unique_id(struct mga_device *mdev) +{ + struct drm_device *dev = &mdev->base; + + /* stash G200 SE model number for later use */ + mdev->model.g200se.unique_rev_id = RREG32(0x1e24); + + drm_dbg(dev, "G200 SE unique revision id is 0x%x\n", + mdev->model.g200se.unique_rev_id); +} + static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) { struct drm_device *dev = &mdev->base; @@ -127,12 +138,8 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) if (ret) return ret;
- /* stash G200 SE model number for later use */ - if (IS_G200_SE(mdev)) { - mdev->unique_rev_id = RREG32(0x1e24); - drm_dbg(dev, "G200 SE unique revision id is 0x%x\n", - mdev->unique_rev_id); - } + if (IS_G200_SE(mdev)) + mgag200_g200se_init_unique_id(mdev);
ret = mgag200_mm_init(mdev); if (ret) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 819c03cc626b..048efe635aff 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -166,8 +166,12 @@ struct mga_device {
int fb_mtrr;
- /* SE model number stored in reg 0x1e24 */ - u32 unique_rev_id; + union { + struct { + /* SE model number stored in reg 0x1e24 */ + u32 unique_rev_id; + } g200se; + } model;
struct mga_connector connector; struct drm_simple_display_pipe display_pipe; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 4fa64cf884cb..752409c7f326 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -112,6 +112,7 @@ static inline void mga_wait_busy(struct mga_device *mdev)
static int mga_g200se_set_plls(struct mga_device *mdev, long clock) { + u32 unique_rev_id = mdev->model.g200se.unique_rev_id; unsigned int vcomax, vcomin, pllreffreq; unsigned int delta, tmpdelta, permitteddelta; unsigned int testp, testm, testn; @@ -121,7 +122,7 @@ static int mga_g200se_set_plls(struct mga_device *mdev, long clock) unsigned int fvv; unsigned int i;
- if (mdev->unique_rev_id <= 0x03) { + if (unique_rev_id <= 0x03) {
m = n = p = 0; vcomax = 320000; @@ -219,7 +220,7 @@ static int mga_g200se_set_plls(struct mga_device *mdev, long clock) WREG_DAC(MGA1064_PIX_PLLC_N, n); WREG_DAC(MGA1064_PIX_PLLC_P, p);
- if (mdev->unique_rev_id >= 0x04) { + if (unique_rev_id >= 0x04) { WREG_DAC(0x1a, 0x09); msleep(20); WREG_DAC(0x1a, 0x01); @@ -1183,12 +1184,13 @@ static void mgag200_g200se_set_hiprilvl(struct mga_device *mdev, const struct drm_display_mode *mode, const struct drm_framebuffer *fb) { + u32 unique_rev_id = mdev->model.g200se.unique_rev_id; unsigned int hiprilvl; u8 crtcext6;
- if (mdev->unique_rev_id >= 0x04) { + if (unique_rev_id >= 0x04) { hiprilvl = 0; - } else if (mdev->unique_rev_id >= 0x02) { + } else if (unique_rev_id >= 0x02) { unsigned int bpp; unsigned long mb;
@@ -1213,7 +1215,7 @@ static void mgag200_g200se_set_hiprilvl(struct mga_device *mdev, else hiprilvl = 5;
- } else if (mdev->unique_rev_id >= 0x01) { + } else if (unique_rev_id >= 0x01) { hiprilvl = 3; } else { hiprilvl = 4; @@ -1337,7 +1339,9 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector, int bpp = 32;
if (IS_G200_SE(mdev)) { - if (mdev->unique_rev_id == 0x01) { + u32 unique_rev_id = mdev->model.g200se.unique_rev_id; + + if (unique_rev_id == 0x01) { if (mode->hdisplay > 1600) return MODE_VIRTUAL_X; if (mode->vdisplay > 1200) @@ -1345,7 +1349,7 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector, if (mga_vga_calculate_mode_bandwidth(mode, bpp) > (24400 * 1024)) return MODE_BANDWIDTH; - } else if (mdev->unique_rev_id == 0x02) { + } else if (unique_rev_id == 0x02) { if (mode->hdisplay > 1920) return MODE_VIRTUAL_X; if (mode->vdisplay > 1200)
This patch adds support for G200 desktop cards. We can reuse the whole memory and modesetting code. A few PCI and DAC register values have to be updated accordingly.
The most significant change is in the PLL setup. The get the clock limits and reference clocks, parses the device's BIOS. With no BIOS found, safe defaults are being used.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Co-developed-by: Egbert Eich eich@suse.com Signed-off-by: Egbert Eich eich@suse.com Co-developed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Takashi Iwai tiwai@suse.de --- MAINTAINERS | 2 +- drivers/gpu/drm/mgag200/Kconfig | 12 +-- drivers/gpu/drm/mgag200/mgag200_drv.c | 125 ++++++++++++++++++++++++- drivers/gpu/drm/mgag200/mgag200_drv.h | 10 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 80 ++++++++++++++++ 5 files changed, 220 insertions(+), 9 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 415954a98934..4c6f96e2b79b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5406,7 +5406,7 @@ S: Orphan / Obsolete F: drivers/gpu/drm/mga/ F: include/uapi/drm/mga_drm.h
-DRM DRIVER FOR MGA G200 SERVER GRAPHICS CHIPS +DRM DRIVER FOR MGA G200 GRAPHICS CHIPS M: Dave Airlie airlied@redhat.com S: Odd Fixes F: drivers/gpu/drm/mgag200/ diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 93be766715c9..eec59658a938 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -1,13 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_MGAG200 - tristate "Kernel modesetting driver for MGA G200 server engines" + tristate "Matrox G200" depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER help - This is a KMS driver for the MGA G200 server chips, it - does not support the original MGA G200 or any of the desktop - chips. It requires 0.3.0 of the modesetting userspace driver, - and a version of mga driver that will fail on KMS enabled - devices. - + This is a KMS driver for Matrox G200 chips. It supports the original + MGA G200 desktop chips and the server variants. It requires 0.3.0 + of the modesetting userspace driver, and a version of mga driver + that will fail on KMS enabled devices. diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index f7652e16365c..419817d6e2cd 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -64,6 +64,14 @@ static int mgag200_regs_init(struct mga_device *mdev) u8 crtcext3;
switch (mdev->type) { + case G200_PCI: + case G200_AGP: + if (mgag200_has_sgram(mdev)) + option = 0x4049cd21; + else + option = 0x40499121; + option2 = 0x00008000; + break; case G200_SE_A: case G200_SE_B: if (mgag200_has_sgram(mdev)) @@ -115,6 +123,117 @@ static int mgag200_regs_init(struct mga_device *mdev) return 0; }
+static void mgag200_g200_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 + }; + + struct drm_device *dev = &mdev->base; + 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(dev, "Unknown BIOS PInS version: %d\n", version); + return; + } + if (pins_len != expected_length[version]) { + drm_warn(dev, "Unexpected BIOS PInS size: %d expeced: %d\n", + pins_len, expected_length[version]); + return; + } + + if (offset + pins_len > size) + return; + + drm_dbg_kms(dev, "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->model.g200.pclk_max = tmp * 10; + break; + case 2: + if (pins[41] != 0xff) + mdev->model.g200.pclk_max = (pins[41] + 100) * 1000; + break; + case 3: + if (pins[36] != 0xff) + mdev->model.g200.pclk_max = (pins[36] + 100) * 1000; + if (pins[52] & 0x20) + mdev->model.g200.ref_clk = 14318; + break; + case 4: + if (pins[39] != 0xff) + mdev->model.g200.pclk_max = pins[39] * 4 * 1000; + if (pins[92] & 0x01) + mdev->model.g200.ref_clk = 14318; + break; + case 5: + tmp = pins[4] ? 8000 : 6000; + if (pins[123] != 0xff) + mdev->model.g200.pclk_min = pins[123] * tmp; + if (pins[38] != 0xff) + mdev->model.g200.pclk_max = pins[38] * tmp; + if (pins[110] & 0x01) + mdev->model.g200.ref_clk = 14318; + break; + default: + break; + } +} + +static void mgag200_g200_init_refclk(struct mga_device *mdev) +{ + struct drm_device *dev = &mdev->base; + unsigned char __iomem *bios; + size_t size; + + mdev->model.g200.pclk_min = 50000; + mdev->model.g200.pclk_max = 230000; + mdev->model.g200.ref_clk = 27050; + + bios = pci_map_rom(dev->pdev, &size); + if (!bios) + return; + + if (size != 0 && bios[0] == 0x55 && bios[1] == 0xaa) + mgag200_g200_interpret_bios(mdev, bios, size); + + pci_unmap_rom(dev->pdev, bios); + + drm_dbg_kms(dev, "pclk_min: %ld pclk_max: %ld ref_clk: %ld\n", + mdev->model.g200.pclk_min, mdev->model.g200.pclk_max, + mdev->model.g200.ref_clk); +} + static void mgag200_g200se_init_unique_id(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; @@ -138,7 +257,9 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) if (ret) return ret;
- if (IS_G200_SE(mdev)) + if (mdev->type == G200_PCI || mdev->type == G200_AGP) + mgag200_g200_init_refclk(mdev); + else if (IS_G200_SE(mdev)) mgag200_g200se_init_unique_id(mdev);
ret = mgag200_mm_init(mdev); @@ -182,6 +303,8 @@ mgag200_device_create(struct pci_dev *pdev, unsigned long flags) */
static const struct pci_device_id mgag200_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_AGP }, { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B }, diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 048efe635aff..54061a61e9ca 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -38,6 +38,8 @@ #define RREG32(reg) ioread32(((void __iomem *)mdev->rmmio) + (reg)) #define WREG32(reg, v) iowrite32(v, ((void __iomem *)mdev->rmmio) + (reg))
+#define MGA_BIOS_OFFSET 0x7ffc + #define ATTR_INDEX 0x1fc0 #define ATTR_DATA 0x1fc1
@@ -129,6 +131,8 @@ struct mga_mc { };
enum mga_type { + G200_PCI, + G200_AGP, G200_SE_A, G200_SE_B, G200_WB, @@ -167,12 +171,18 @@ struct mga_device { int fb_mtrr;
union { + struct { + long ref_clk; + long pclk_min; + long pclk_max; + } g200; struct { /* SE model number stored in reg 0x1e24 */ u32 unique_rev_id; } g200se; } model;
+ struct mga_connector connector; struct drm_simple_display_pipe display_pipe; }; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 752409c7f326..bc11552415f5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -108,6 +108,77 @@ static inline void mga_wait_busy(struct mga_device *mdev) } while ((status & 0x01) && time_before(jiffies, timeout)); }
+/* + * PLL setup + */ + +static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) +{ + struct drm_device *dev = &mdev->base; + 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->model.g200.ref_clk; + long p_clk_min = mdev->model.g200.pclk_min; + long p_clk_max = mdev->model.g200.pclk_max; + + if (clock > p_clk_max) { + drm_err(dev, "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_dbg_kms(dev, "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) @@ -717,6 +788,9 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock) u8 misc;
switch(mdev->type) { + case G200_PCI: + case G200_AGP: + return mgag200_g200_set_plls(mdev, clock); case G200_SE_A: case G200_SE_B: return mga_g200se_set_plls(mdev, clock); @@ -894,6 +968,12 @@ static void mgag200_set_dac_regs(struct mga_device *mdev) };
switch (mdev->type) { + case G200_PCI: + case G200_AGP: + dacvalue[MGA1064_SYS_PLL_M] = 0x04; + dacvalue[MGA1064_SYS_PLL_N] = 0x2D; + dacvalue[MGA1064_SYS_PLL_P] = 0x19; + break; case G200_SE_A: case G200_SE_B: dacvalue[MGA1064_VREF_CTL] = 0x03;
On Wed, 2020-07-15 at 16:59 +0200, Thomas Zimmermann wrote:
This patch adds support for G200 desktop cards. We can reuse the whole memory and modesetting code. A few PCI and DAC register values have to be updated accordingly.
The most significant change is in the PLL setup. The get the clock limits and reference clocks, parses the device's BIOS. With no BIOS found, safe defaults are being used.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Co-developed-by: Egbert Eich eich@suse.com Signed-off-by: Egbert Eich eich@suse.com Co-developed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Takashi Iwai tiwai@suse.de
MAINTAINERS | 2 +- drivers/gpu/drm/mgag200/Kconfig | 12 +-- drivers/gpu/drm/mgag200/mgag200_drv.c | 125 ++++++++++++++++++++++++- drivers/gpu/drm/mgag200/mgag200_drv.h | 10 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 80 ++++++++++++++++ 5 files changed, 220 insertions(+), 9 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 415954a98934..4c6f96e2b79b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5406,7 +5406,7 @@ S: Orphan / Obsolete F: drivers/gpu/drm/mga/ F: include/uapi/drm/mga_drm.h
-DRM DRIVER FOR MGA G200 SERVER GRAPHICS CHIPS +DRM DRIVER FOR MGA G200 GRAPHICS CHIPS M: Dave Airlie airlied@redhat.com S: Odd Fixes F: drivers/gpu/drm/mgag200/ diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 93be766715c9..eec59658a938 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -1,13 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_MGAG200
- tristate "Kernel modesetting driver for MGA G200 server engines"
- tristate "Matrox G200" depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER help
This is a KMS driver for the MGA G200 server chips, itdoes not support the original MGA G200 or any of the desktopchips. It requires 0.3.0 of the modesetting userspace driver,and a version of mga driver that will fail on KMS enableddevices.
This is a KMS driver for Matrox G200 chips. It supports the originalMGA G200 desktop chips and the server variants. It requires 0.3.0of the modesetting userspace driver, and a version of mga driverthat will fail on KMS enabled devices.diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index f7652e16365c..419817d6e2cd 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -64,6 +64,14 @@ static int mgag200_regs_init(struct mga_device *mdev) u8 crtcext3;
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
if (mgag200_has_sgram(mdev))option = 0x4049cd21;elseoption = 0x40499121;option2 = 0x00008000;break;@@ -115,6 +123,117 @@ static int mgag200_regs_init(struct mga_device *mdev) return 0; }
+static void mgag200_g200_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- };
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *pins;
huh, never realized you could write directly to unsigned char __iomem pointers!
- 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(dev, "Unknown BIOS PInS version: %d\n", version);
Did you maybe mean pins or PINS here? or is PInS some weird abbreviation matrox uses?
return;- }
- if (pins_len != expected_length[version]) {
drm_warn(dev, "Unexpected BIOS PInS size: %d expeced: %d\n",pins_len, expected_length[version]);return;- }
- if (offset + pins_len > size)
return;- drm_dbg_kms(dev, "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->model.g200.pclk_max = tmp * 10;break;- case 2:
if (pins[41] != 0xff)mdev->model.g200.pclk_max = (pins[41] + 100) * 1000;break;- case 3:
if (pins[36] != 0xff)mdev->model.g200.pclk_max = (pins[36] + 100) * 1000;if (pins[52] & 0x20)mdev->model.g200.ref_clk = 14318;break;- case 4:
if (pins[39] != 0xff)mdev->model.g200.pclk_max = pins[39] * 4 * 1000;if (pins[92] & 0x01)mdev->model.g200.ref_clk = 14318;break;- case 5:
tmp = pins[4] ? 8000 : 6000;if (pins[123] != 0xff)mdev->model.g200.pclk_min = pins[123] * tmp;if (pins[38] != 0xff)mdev->model.g200.pclk_max = pins[38] * tmp;if (pins[110] & 0x01)mdev->model.g200.ref_clk = 14318;break;- default:
break;- }
+}
+static void mgag200_g200_init_refclk(struct mga_device *mdev) +{
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *bios;
- size_t size;
- mdev->model.g200.pclk_min = 50000;
- mdev->model.g200.pclk_max = 230000;
- mdev->model.g200.ref_clk = 27050;
- bios = pci_map_rom(dev->pdev, &size);
- if (!bios)
return;- if (size != 0 && bios[0] == 0x55 && bios[1] == 0xaa)
mgag200_g200_interpret_bios(mdev, bios, size);- pci_unmap_rom(dev->pdev, bios);
- drm_dbg_kms(dev, "pclk_min: %ld pclk_max: %ld ref_clk: %ld\n",
mdev->model.g200.pclk_min, mdev->model.g200.pclk_max,mdev->model.g200.ref_clk);+}
static void mgag200_g200se_init_unique_id(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; @@ -138,7 +257,9 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) if (ret) return ret;
- if (IS_G200_SE(mdev))
if (mdev->type == G200_PCI || mdev->type == G200_AGP)
mgag200_g200_init_refclk(mdev);else if (IS_G200_SE(mdev)) mgag200_g200se_init_unique_id(mdev);
ret = mgag200_mm_init(mdev);
@@ -182,6 +303,8 @@ mgag200_device_create(struct pci_dev *pdev, unsigned long flags) */
static const struct pci_device_id mgag200_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_AGP }, { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B
}, diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 048efe635aff..54061a61e9ca 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -38,6 +38,8 @@ #define RREG32(reg) ioread32(((void __iomem *)mdev->rmmio) + (reg)) #define WREG32(reg, v) iowrite32(v, ((void __iomem *)mdev->rmmio) + (reg))
+#define MGA_BIOS_OFFSET 0x7ffc
#define ATTR_INDEX 0x1fc0 #define ATTR_DATA 0x1fc1
@@ -129,6 +131,8 @@ struct mga_mc { };
enum mga_type {
- G200_PCI,
- G200_AGP, G200_SE_A, G200_SE_B, G200_WB,
@@ -167,12 +171,18 @@ struct mga_device { int fb_mtrr;
union {
struct {long ref_clk;long pclk_min;long pclk_max;} g200;struct { /* SE model number stored in reg 0x1e24 */ u32 unique_rev_id; } g200se; } model;
struct mga_connector connector; struct drm_simple_display_pipe display_pipe;
}; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 752409c7f326..bc11552415f5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -108,6 +108,77 @@ static inline void mga_wait_busy(struct mga_device *mdev) } while ((status & 0x01) && time_before(jiffies, timeout)); }
+/*
- PLL setup
- */
+static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) +{
- struct drm_device *dev = &mdev->base;
- 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->model.g200.ref_clk;
- long p_clk_min = mdev->model.g200.pclk_min;
- long p_clk_max = mdev->model.g200.pclk_max;
- if (clock > p_clk_max) {
drm_err(dev, "Pixel Clock %ld too high\n", clock);return 1;- }
- if (clock < p_clk_min >> 3)
Looks like there's a stray space after the <. You could also just use max() here, but I'll leave that up to you
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;elsetmp_delta = computed - f_vco;
Another stray space before the =
With those nitpicks addressed, this series is:
Reviewed-by: Lyude Paul lyude@redhat.com
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_dbg_kms(dev, "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) @@ -717,6 +788,9 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock) u8 misc;
switch(mdev->type) {
- case G200_PCI:
- case G200_AGP:
return mgag200_g200_set_plls(mdev, clock);@@ -894,6 +968,12 @@ static void mgag200_set_dac_regs(struct mga_device *mdev) };
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
dacvalue[MGA1064_SYS_PLL_M] = 0x04;dacvalue[MGA1064_SYS_PLL_N] = 0x2D;dacvalue[MGA1064_SYS_PLL_P] = 0x19;break;
Hi Lyude.
On Thu, Jul 16, 2020 at 06:43:40PM -0400, Lyude Paul wrote:
On Wed, 2020-07-15 at 16:59 +0200, Thomas Zimmermann wrote:
This patch adds support for G200 desktop cards. We can reuse the whole memory and modesetting code. A few PCI and DAC register values have to be updated accordingly.
The most significant change is in the PLL setup. The get the clock limits and reference clocks, parses the device's BIOS. With no BIOS found, safe defaults are being used.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Co-developed-by: Egbert Eich eich@suse.com Signed-off-by: Egbert Eich eich@suse.com Co-developed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Takashi Iwai tiwai@suse.de
MAINTAINERS | 2 +- drivers/gpu/drm/mgag200/Kconfig | 12 +-- drivers/gpu/drm/mgag200/mgag200_drv.c | 125 ++++++++++++++++++++++++- drivers/gpu/drm/mgag200/mgag200_drv.h | 10 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 80 ++++++++++++++++ 5 files changed, 220 insertions(+), 9 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 415954a98934..4c6f96e2b79b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5406,7 +5406,7 @@ S: Orphan / Obsolete F: drivers/gpu/drm/mga/ F: include/uapi/drm/mga_drm.h
-DRM DRIVER FOR MGA G200 SERVER GRAPHICS CHIPS +DRM DRIVER FOR MGA G200 GRAPHICS CHIPS M: Dave Airlie airlied@redhat.com S: Odd Fixes F: drivers/gpu/drm/mgag200/ diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 93be766715c9..eec59658a938 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -1,13 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_MGAG200
- tristate "Kernel modesetting driver for MGA G200 server engines"
- tristate "Matrox G200" depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER help
This is a KMS driver for the MGA G200 server chips, itdoes not support the original MGA G200 or any of the desktopchips. It requires 0.3.0 of the modesetting userspace driver,and a version of mga driver that will fail on KMS enableddevices.
This is a KMS driver for Matrox G200 chips. It supports the originalMGA G200 desktop chips and the server variants. It requires 0.3.0of the modesetting userspace driver, and a version of mga driverthat will fail on KMS enabled devices.diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index f7652e16365c..419817d6e2cd 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -64,6 +64,14 @@ static int mgag200_regs_init(struct mga_device *mdev) u8 crtcext3;
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
if (mgag200_has_sgram(mdev))option = 0x4049cd21;elseoption = 0x40499121;option2 = 0x00008000;break;@@ -115,6 +123,117 @@ static int mgag200_regs_init(struct mga_device *mdev) return 0; }
+static void mgag200_g200_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- };
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *pins;
huh, never realized you could write directly to unsigned char __iomem pointers!
You cannot :-)
It works for some architectures but not for all. On sparc64, for instance, this will fail. The right thing is to use the accessors in io.h
sparse will help you finding such illegal uses of __iomem *. The good thing is that the pointers are annotated __iomem here.
Thomas: Please run sparse and fix the warnings using the right accessors for this patch.
Sam
- 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(dev, "Unknown BIOS PInS version: %d\n", version);Did you maybe mean pins or PINS here? or is PInS some weird abbreviation matrox uses?
return;- }
- if (pins_len != expected_length[version]) {
drm_warn(dev, "Unexpected BIOS PInS size: %d expeced: %d\n",pins_len, expected_length[version]);return;- }
- if (offset + pins_len > size)
return;- drm_dbg_kms(dev, "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->model.g200.pclk_max = tmp * 10;break;- case 2:
if (pins[41] != 0xff)mdev->model.g200.pclk_max = (pins[41] + 100) * 1000;break;- case 3:
if (pins[36] != 0xff)mdev->model.g200.pclk_max = (pins[36] + 100) * 1000;if (pins[52] & 0x20)mdev->model.g200.ref_clk = 14318;break;- case 4:
if (pins[39] != 0xff)mdev->model.g200.pclk_max = pins[39] * 4 * 1000;if (pins[92] & 0x01)mdev->model.g200.ref_clk = 14318;break;- case 5:
tmp = pins[4] ? 8000 : 6000;if (pins[123] != 0xff)mdev->model.g200.pclk_min = pins[123] * tmp;if (pins[38] != 0xff)mdev->model.g200.pclk_max = pins[38] * tmp;if (pins[110] & 0x01)mdev->model.g200.ref_clk = 14318;break;- default:
break;- }
+}
+static void mgag200_g200_init_refclk(struct mga_device *mdev) +{
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *bios;
- size_t size;
- mdev->model.g200.pclk_min = 50000;
- mdev->model.g200.pclk_max = 230000;
- mdev->model.g200.ref_clk = 27050;
- bios = pci_map_rom(dev->pdev, &size);
- if (!bios)
return;- if (size != 0 && bios[0] == 0x55 && bios[1] == 0xaa)
mgag200_g200_interpret_bios(mdev, bios, size);- pci_unmap_rom(dev->pdev, bios);
- drm_dbg_kms(dev, "pclk_min: %ld pclk_max: %ld ref_clk: %ld\n",
mdev->model.g200.pclk_min, mdev->model.g200.pclk_max,mdev->model.g200.ref_clk);+}
static void mgag200_g200se_init_unique_id(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; @@ -138,7 +257,9 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) if (ret) return ret;
- if (IS_G200_SE(mdev))
if (mdev->type == G200_PCI || mdev->type == G200_AGP)
mgag200_g200_init_refclk(mdev);else if (IS_G200_SE(mdev)) mgag200_g200se_init_unique_id(mdev);
ret = mgag200_mm_init(mdev);
@@ -182,6 +303,8 @@ mgag200_device_create(struct pci_dev *pdev, unsigned long flags) */
static const struct pci_device_id mgag200_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_AGP }, { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B
}, diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 048efe635aff..54061a61e9ca 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -38,6 +38,8 @@ #define RREG32(reg) ioread32(((void __iomem *)mdev->rmmio) + (reg)) #define WREG32(reg, v) iowrite32(v, ((void __iomem *)mdev->rmmio) + (reg))
+#define MGA_BIOS_OFFSET 0x7ffc
#define ATTR_INDEX 0x1fc0 #define ATTR_DATA 0x1fc1
@@ -129,6 +131,8 @@ struct mga_mc { };
enum mga_type {
- G200_PCI,
- G200_AGP, G200_SE_A, G200_SE_B, G200_WB,
@@ -167,12 +171,18 @@ struct mga_device { int fb_mtrr;
union {
struct {long ref_clk;long pclk_min;long pclk_max;} g200;struct { /* SE model number stored in reg 0x1e24 */ u32 unique_rev_id; } g200se; } model;
struct mga_connector connector; struct drm_simple_display_pipe display_pipe;
}; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 752409c7f326..bc11552415f5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -108,6 +108,77 @@ static inline void mga_wait_busy(struct mga_device *mdev) } while ((status & 0x01) && time_before(jiffies, timeout)); }
+/*
- PLL setup
- */
+static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) +{
- struct drm_device *dev = &mdev->base;
- 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->model.g200.ref_clk;
- long p_clk_min = mdev->model.g200.pclk_min;
- long p_clk_max = mdev->model.g200.pclk_max;
- if (clock > p_clk_max) {
drm_err(dev, "Pixel Clock %ld too high\n", clock);return 1;- }
- if (clock < p_clk_min >> 3)
Looks like there's a stray space after the <. You could also just use max() here, but I'll leave that up to you
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;elsetmp_delta = computed - f_vco;Another stray space before the =
With those nitpicks addressed, this series is:
Reviewed-by: Lyude Paul lyude@redhat.com
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_dbg_kms(dev, "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) @@ -717,6 +788,9 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock) u8 misc;
switch(mdev->type) {
- case G200_PCI:
- case G200_AGP:
return mgag200_g200_set_plls(mdev, clock);@@ -894,6 +968,12 @@ static void mgag200_set_dac_regs(struct mga_device *mdev) };
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
dacvalue[MGA1064_SYS_PLL_M] = 0x04;dacvalue[MGA1064_SYS_PLL_N] = 0x2D;dacvalue[MGA1064_SYS_PLL_P] = 0x19;break;
Hi
Am 17.07.20 um 00:43 schrieb Lyude Paul:
On Wed, 2020-07-15 at 16:59 +0200, Thomas Zimmermann wrote:
This patch adds support for G200 desktop cards. We can reuse the whole memory and modesetting code. A few PCI and DAC register values have to be updated accordingly.
The most significant change is in the PLL setup. The get the clock limits and reference clocks, parses the device's BIOS. With no BIOS found, safe defaults are being used.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Co-developed-by: Egbert Eich eich@suse.com Signed-off-by: Egbert Eich eich@suse.com Co-developed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Takashi Iwai tiwai@suse.de
MAINTAINERS | 2 +- drivers/gpu/drm/mgag200/Kconfig | 12 +-- drivers/gpu/drm/mgag200/mgag200_drv.c | 125 ++++++++++++++++++++++++- drivers/gpu/drm/mgag200/mgag200_drv.h | 10 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 80 ++++++++++++++++ 5 files changed, 220 insertions(+), 9 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 415954a98934..4c6f96e2b79b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5406,7 +5406,7 @@ S: Orphan / Obsolete F: drivers/gpu/drm/mga/ F: include/uapi/drm/mga_drm.h
-DRM DRIVER FOR MGA G200 SERVER GRAPHICS CHIPS +DRM DRIVER FOR MGA G200 GRAPHICS CHIPS M: Dave Airlie airlied@redhat.com S: Odd Fixes F: drivers/gpu/drm/mgag200/ diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 93be766715c9..eec59658a938 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -1,13 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_MGAG200
- tristate "Kernel modesetting driver for MGA G200 server engines"
- tristate "Matrox G200" depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER help
This is a KMS driver for the MGA G200 server chips, itdoes not support the original MGA G200 or any of the desktopchips. It requires 0.3.0 of the modesetting userspace driver,and a version of mga driver that will fail on KMS enableddevices.
This is a KMS driver for Matrox G200 chips. It supports the originalMGA G200 desktop chips and the server variants. It requires 0.3.0of the modesetting userspace driver, and a version of mga driverthat will fail on KMS enabled devices.diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index f7652e16365c..419817d6e2cd 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -64,6 +64,14 @@ static int mgag200_regs_init(struct mga_device *mdev) u8 crtcext3;
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
if (mgag200_has_sgram(mdev))option = 0x4049cd21;elseoption = 0x40499121;option2 = 0x00008000;break;@@ -115,6 +123,117 @@ static int mgag200_regs_init(struct mga_device *mdev) return 0; }
+static void mgag200_g200_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- };
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *pins;
huh, never realized you could write directly to unsigned char __iomem pointers!
I took the patch as-is, but this probably wouldn't work on all architectures.
- 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(dev, "Unknown BIOS PInS version: %d\n", version);Did you maybe mean pins or PINS here? or is PInS some weird abbreviation matrox uses?
It's the name of a data structure
https://gitlab.freedesktop.org/xorg/driver/xf86-video-mga/-/blob/master/mga_...
I have no idea what it stands for.
return;- }
- if (pins_len != expected_length[version]) {
drm_warn(dev, "Unexpected BIOS PInS size: %d expeced: %d\n",pins_len, expected_length[version]);return;- }
- if (offset + pins_len > size)
return;- drm_dbg_kms(dev, "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->model.g200.pclk_max = tmp * 10;break;- case 2:
if (pins[41] != 0xff)mdev->model.g200.pclk_max = (pins[41] + 100) * 1000;break;- case 3:
if (pins[36] != 0xff)mdev->model.g200.pclk_max = (pins[36] + 100) * 1000;if (pins[52] & 0x20)mdev->model.g200.ref_clk = 14318;break;- case 4:
if (pins[39] != 0xff)mdev->model.g200.pclk_max = pins[39] * 4 * 1000;if (pins[92] & 0x01)mdev->model.g200.ref_clk = 14318;break;- case 5:
tmp = pins[4] ? 8000 : 6000;if (pins[123] != 0xff)mdev->model.g200.pclk_min = pins[123] * tmp;if (pins[38] != 0xff)mdev->model.g200.pclk_max = pins[38] * tmp;if (pins[110] & 0x01)mdev->model.g200.ref_clk = 14318;break;- default:
break;- }
+}
+static void mgag200_g200_init_refclk(struct mga_device *mdev) +{
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *bios;
- size_t size;
- mdev->model.g200.pclk_min = 50000;
- mdev->model.g200.pclk_max = 230000;
- mdev->model.g200.ref_clk = 27050;
- bios = pci_map_rom(dev->pdev, &size);
- if (!bios)
return;- if (size != 0 && bios[0] == 0x55 && bios[1] == 0xaa)
mgag200_g200_interpret_bios(mdev, bios, size);- pci_unmap_rom(dev->pdev, bios);
- drm_dbg_kms(dev, "pclk_min: %ld pclk_max: %ld ref_clk: %ld\n",
mdev->model.g200.pclk_min, mdev->model.g200.pclk_max,mdev->model.g200.ref_clk);+}
static void mgag200_g200se_init_unique_id(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; @@ -138,7 +257,9 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) if (ret) return ret;
- if (IS_G200_SE(mdev))
if (mdev->type == G200_PCI || mdev->type == G200_AGP)
mgag200_g200_init_refclk(mdev);else if (IS_G200_SE(mdev)) mgag200_g200se_init_unique_id(mdev);
ret = mgag200_mm_init(mdev);
@@ -182,6 +303,8 @@ mgag200_device_create(struct pci_dev *pdev, unsigned long flags) */
static const struct pci_device_id mgag200_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_AGP }, { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B
}, diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 048efe635aff..54061a61e9ca 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -38,6 +38,8 @@ #define RREG32(reg) ioread32(((void __iomem *)mdev->rmmio) + (reg)) #define WREG32(reg, v) iowrite32(v, ((void __iomem *)mdev->rmmio) + (reg))
+#define MGA_BIOS_OFFSET 0x7ffc
#define ATTR_INDEX 0x1fc0 #define ATTR_DATA 0x1fc1
@@ -129,6 +131,8 @@ struct mga_mc { };
enum mga_type {
- G200_PCI,
- G200_AGP, G200_SE_A, G200_SE_B, G200_WB,
@@ -167,12 +171,18 @@ struct mga_device { int fb_mtrr;
union {
struct {long ref_clk;long pclk_min;long pclk_max;} g200;struct { /* SE model number stored in reg 0x1e24 */ u32 unique_rev_id; } g200se; } model;
struct mga_connector connector; struct drm_simple_display_pipe display_pipe;
}; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 752409c7f326..bc11552415f5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -108,6 +108,77 @@ static inline void mga_wait_busy(struct mga_device *mdev) } while ((status & 0x01) && time_before(jiffies, timeout)); }
+/*
- PLL setup
- */
+static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) +{
- struct drm_device *dev = &mdev->base;
- 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->model.g200.ref_clk;
- long p_clk_min = mdev->model.g200.pclk_min;
- long p_clk_max = mdev->model.g200.pclk_max;
- if (clock > p_clk_max) {
drm_err(dev, "Pixel Clock %ld too high\n", clock);return 1;- }
- if (clock < p_clk_min >> 3)
Looks like there's a stray space after the <. You could also just use max() here, but I'll leave that up to you
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;elsetmp_delta = computed - f_vco;Another stray space before the =
With those nitpicks addressed, this series is:
Reviewed-by: Lyude Paul lyude@redhat.com
Thanks a lot.
The other patches are probably uncontoversial. Let's see what happens to this one. :)
Best regards Thomas
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_dbg_kms(dev, "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) @@ -717,6 +788,9 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock) u8 misc;
switch(mdev->type) {
- case G200_PCI:
- case G200_AGP:
return mgag200_g200_set_plls(mdev, clock);@@ -894,6 +968,12 @@ static void mgag200_set_dac_regs(struct mga_device *mdev) };
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
dacvalue[MGA1064_SYS_PLL_M] = 0x04;dacvalue[MGA1064_SYS_PLL_N] = 0x2D;dacvalue[MGA1064_SYS_PLL_P] = 0x19;break;
On Mon, 2020-07-20 at 09:04 +0200, Thomas Zimmermann wrote:
Hi
Am 17.07.20 um 00:43 schrieb Lyude Paul:
On Wed, 2020-07-15 at 16:59 +0200, Thomas Zimmermann wrote:
This patch adds support for G200 desktop cards. We can reuse the whole memory and modesetting code. A few PCI and DAC register values have to be updated accordingly.
The most significant change is in the PLL setup. The get the clock limits and reference clocks, parses the device's BIOS. With no BIOS found, safe defaults are being used.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Co-developed-by: Egbert Eich eich@suse.com Signed-off-by: Egbert Eich eich@suse.com Co-developed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Takashi Iwai tiwai@suse.de
MAINTAINERS | 2 +- drivers/gpu/drm/mgag200/Kconfig | 12 +-- drivers/gpu/drm/mgag200/mgag200_drv.c | 125 ++++++++++++++++++++++++- drivers/gpu/drm/mgag200/mgag200_drv.h | 10 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 80 ++++++++++++++++ 5 files changed, 220 insertions(+), 9 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 415954a98934..4c6f96e2b79b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5406,7 +5406,7 @@ S: Orphan / Obsolete F: drivers/gpu/drm/mga/ F: include/uapi/drm/mga_drm.h
-DRM DRIVER FOR MGA G200 SERVER GRAPHICS CHIPS +DRM DRIVER FOR MGA G200 GRAPHICS CHIPS M: Dave Airlie airlied@redhat.com S: Odd Fixes F: drivers/gpu/drm/mgag200/ diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 93be766715c9..eec59658a938 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -1,13 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_MGAG200
- tristate "Kernel modesetting driver for MGA G200 server engines"
- tristate "Matrox G200" depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER help
This is a KMS driver for the MGA G200 server chips, itdoes not support the original MGA G200 or any of the desktopchips. It requires 0.3.0 of the modesetting userspace driver,and a version of mga driver that will fail on KMS enableddevices.
This is a KMS driver for Matrox G200 chips. It supports the originalMGA G200 desktop chips and the server variants. It requires 0.3.0of the modesetting userspace driver, and a version of mga driverthat will fail on KMS enabled devices.diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index f7652e16365c..419817d6e2cd 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -64,6 +64,14 @@ static int mgag200_regs_init(struct mga_device *mdev) u8 crtcext3;
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
if (mgag200_has_sgram(mdev))option = 0x4049cd21;elseoption = 0x40499121;option2 = 0x00008000;break;@@ -115,6 +123,117 @@ static int mgag200_regs_init(struct mga_device *mdev) return 0; }
+static void mgag200_g200_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- };
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *pins;
huh, never realized you could write directly to unsigned char __iomem pointers!
I took the patch as-is, but this probably wouldn't work on all architectures.
Something occurred to me just now - do we actually care? I don't think I've ever seen mgag200 on anything other then x86_64 systems
- 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(dev, "Unknown BIOS PInS version: %d\n", version);Did you maybe mean pins or PINS here? or is PInS some weird abbreviation matrox uses?
It's the name of a data structure
https://gitlab.freedesktop.org/xorg/driver/xf86-video-mga/-/blob/master/mga_...
I have no idea what it stands for.
return;- }
- if (pins_len != expected_length[version]) {
drm_warn(dev, "Unexpected BIOS PInS size: %d expeced: %d\n",pins_len, expected_length[version]);return;- }
- if (offset + pins_len > size)
return;- drm_dbg_kms(dev, "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->model.g200.pclk_max = tmp * 10;break;- case 2:
if (pins[41] != 0xff)mdev->model.g200.pclk_max = (pins[41] + 100) * 1000;break;- case 3:
if (pins[36] != 0xff)mdev->model.g200.pclk_max = (pins[36] + 100) * 1000;if (pins[52] & 0x20)mdev->model.g200.ref_clk = 14318;break;- case 4:
if (pins[39] != 0xff)mdev->model.g200.pclk_max = pins[39] * 4 * 1000;if (pins[92] & 0x01)mdev->model.g200.ref_clk = 14318;break;- case 5:
tmp = pins[4] ? 8000 : 6000;if (pins[123] != 0xff)mdev->model.g200.pclk_min = pins[123] * tmp;if (pins[38] != 0xff)mdev->model.g200.pclk_max = pins[38] * tmp;if (pins[110] & 0x01)mdev->model.g200.ref_clk = 14318;break;- default:
break;- }
+}
+static void mgag200_g200_init_refclk(struct mga_device *mdev) +{
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *bios;
- size_t size;
- mdev->model.g200.pclk_min = 50000;
- mdev->model.g200.pclk_max = 230000;
- mdev->model.g200.ref_clk = 27050;
- bios = pci_map_rom(dev->pdev, &size);
- if (!bios)
return;- if (size != 0 && bios[0] == 0x55 && bios[1] == 0xaa)
mgag200_g200_interpret_bios(mdev, bios, size);- pci_unmap_rom(dev->pdev, bios);
- drm_dbg_kms(dev, "pclk_min: %ld pclk_max: %ld ref_clk: %ld\n",
mdev->model.g200.pclk_min, mdev->model.g200.pclk_max,mdev->model.g200.ref_clk);+}
static void mgag200_g200se_init_unique_id(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; @@ -138,7 +257,9 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) if (ret) return ret;
- if (IS_G200_SE(mdev))
if (mdev->type == G200_PCI || mdev->type == G200_AGP)
mgag200_g200_init_refclk(mdev);else if (IS_G200_SE(mdev)) mgag200_g200se_init_unique_id(mdev);
ret = mgag200_mm_init(mdev);
@@ -182,6 +303,8 @@ mgag200_device_create(struct pci_dev *pdev, unsigned long flags) */
static const struct pci_device_id mgag200_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_AGP
}, { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B }, diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 048efe635aff..54061a61e9ca 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -38,6 +38,8 @@ #define RREG32(reg) ioread32(((void __iomem *)mdev->rmmio) + (reg)) #define WREG32(reg, v) iowrite32(v, ((void __iomem *)mdev->rmmio) + (reg))
+#define MGA_BIOS_OFFSET 0x7ffc
#define ATTR_INDEX 0x1fc0 #define ATTR_DATA 0x1fc1
@@ -129,6 +131,8 @@ struct mga_mc { };
enum mga_type {
- G200_PCI,
- G200_AGP, G200_SE_A, G200_SE_B, G200_WB,
@@ -167,12 +171,18 @@ struct mga_device { int fb_mtrr;
union {
struct {long ref_clk;long pclk_min;long pclk_max;} g200;struct { /* SE model number stored in reg 0x1e24 */ u32 unique_rev_id; } g200se; } model;
struct mga_connector connector; struct drm_simple_display_pipe display_pipe;
}; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 752409c7f326..bc11552415f5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -108,6 +108,77 @@ static inline void mga_wait_busy(struct mga_device *mdev) } while ((status & 0x01) && time_before(jiffies, timeout)); }
+/*
- PLL setup
- */
+static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) +{
- struct drm_device *dev = &mdev->base;
- 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->model.g200.ref_clk;
- long p_clk_min = mdev->model.g200.pclk_min;
- long p_clk_max = mdev->model.g200.pclk_max;
- if (clock > p_clk_max) {
drm_err(dev, "Pixel Clock %ld too high\n", clock);return 1;- }
- if (clock < p_clk_min >> 3)
Looks like there's a stray space after the <. You could also just use max() here, but I'll leave that up to you
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;elsetmp_delta = computed - f_vco;Another stray space before the =
With those nitpicks addressed, this series is:
Reviewed-by: Lyude Paul lyude@redhat.com
Thanks a lot.
The other patches are probably uncontoversial. Let's see what happens to this one. :)
Best regards Thomas
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_dbg_kms(dev, "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) @@ -717,6 +788,9 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock) u8 misc;
switch(mdev->type) {
- case G200_PCI:
- case G200_AGP:
return mgag200_g200_set_plls(mdev, clock);@@ -894,6 +968,12 @@ static void mgag200_set_dac_regs(struct mga_device *mdev) };
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
dacvalue[MGA1064_SYS_PLL_M] = 0x04;dacvalue[MGA1064_SYS_PLL_N] = 0x2D;dacvalue[MGA1064_SYS_PLL_P] = 0x19;break;
On Mon, Jul 20, 2020 at 03:18:56PM -0400, Lyude Paul wrote:
On Mon, 2020-07-20 at 09:04 +0200, Thomas Zimmermann wrote:
Hi
Am 17.07.20 um 00:43 schrieb Lyude Paul:
On Wed, 2020-07-15 at 16:59 +0200, Thomas Zimmermann wrote:
This patch adds support for G200 desktop cards. We can reuse the whole memory and modesetting code. A few PCI and DAC register values have to be updated accordingly.
The most significant change is in the PLL setup. The get the clock limits and reference clocks, parses the device's BIOS. With no BIOS found, safe defaults are being used.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Co-developed-by: Egbert Eich eich@suse.com Signed-off-by: Egbert Eich eich@suse.com Co-developed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Takashi Iwai tiwai@suse.de
MAINTAINERS | 2 +- drivers/gpu/drm/mgag200/Kconfig | 12 +-- drivers/gpu/drm/mgag200/mgag200_drv.c | 125 ++++++++++++++++++++++++- drivers/gpu/drm/mgag200/mgag200_drv.h | 10 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 80 ++++++++++++++++ 5 files changed, 220 insertions(+), 9 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 415954a98934..4c6f96e2b79b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5406,7 +5406,7 @@ S: Orphan / Obsolete F: drivers/gpu/drm/mga/ F: include/uapi/drm/mga_drm.h
-DRM DRIVER FOR MGA G200 SERVER GRAPHICS CHIPS +DRM DRIVER FOR MGA G200 GRAPHICS CHIPS M: Dave Airlie airlied@redhat.com S: Odd Fixes F: drivers/gpu/drm/mgag200/ diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 93be766715c9..eec59658a938 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -1,13 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_MGAG200
- tristate "Kernel modesetting driver for MGA G200 server engines"
- tristate "Matrox G200" depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER help
This is a KMS driver for the MGA G200 server chips, itdoes not support the original MGA G200 or any of the desktopchips. It requires 0.3.0 of the modesetting userspace driver,and a version of mga driver that will fail on KMS enableddevices.
This is a KMS driver for Matrox G200 chips. It supports the originalMGA G200 desktop chips and the server variants. It requires 0.3.0of the modesetting userspace driver, and a version of mga driverthat will fail on KMS enabled devices.diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index f7652e16365c..419817d6e2cd 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -64,6 +64,14 @@ static int mgag200_regs_init(struct mga_device *mdev) u8 crtcext3;
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
if (mgag200_has_sgram(mdev))option = 0x4049cd21;elseoption = 0x40499121;option2 = 0x00008000;break;@@ -115,6 +123,117 @@ static int mgag200_regs_init(struct mga_device *mdev) return 0; }
+static void mgag200_g200_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- };
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *pins;
huh, never realized you could write directly to unsigned char __iomem pointers!
I took the patch as-is, but this probably wouldn't work on all architectures.
Something occurred to me just now - do we actually care? I don't think I've ever seen mgag200 on anything other then x86_64 systems
For starters to silence the sparse warnings. Also other parts of the driver uses the io{read,write} functions so to be consistent this code shoudl also do it. And to avoid having bad code floating around.
It is not as it is a big deal to fix it neither does it hurt performance. But then it is up to Thomas in the end.
Sam
- 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(dev, "Unknown BIOS PInS version: %d\n", version);Did you maybe mean pins or PINS here? or is PInS some weird abbreviation matrox uses?
It's the name of a data structure
https://gitlab.freedesktop.org/xorg/driver/xf86-video-mga/-/blob/master/mga_...
I have no idea what it stands for.
return;- }
- if (pins_len != expected_length[version]) {
drm_warn(dev, "Unexpected BIOS PInS size: %d expeced: %d\n",pins_len, expected_length[version]);return;- }
- if (offset + pins_len > size)
return;- drm_dbg_kms(dev, "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->model.g200.pclk_max = tmp * 10;break;- case 2:
if (pins[41] != 0xff)mdev->model.g200.pclk_max = (pins[41] + 100) * 1000;break;- case 3:
if (pins[36] != 0xff)mdev->model.g200.pclk_max = (pins[36] + 100) * 1000;if (pins[52] & 0x20)mdev->model.g200.ref_clk = 14318;break;- case 4:
if (pins[39] != 0xff)mdev->model.g200.pclk_max = pins[39] * 4 * 1000;if (pins[92] & 0x01)mdev->model.g200.ref_clk = 14318;break;- case 5:
tmp = pins[4] ? 8000 : 6000;if (pins[123] != 0xff)mdev->model.g200.pclk_min = pins[123] * tmp;if (pins[38] != 0xff)mdev->model.g200.pclk_max = pins[38] * tmp;if (pins[110] & 0x01)mdev->model.g200.ref_clk = 14318;break;- default:
break;- }
+}
+static void mgag200_g200_init_refclk(struct mga_device *mdev) +{
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *bios;
- size_t size;
- mdev->model.g200.pclk_min = 50000;
- mdev->model.g200.pclk_max = 230000;
- mdev->model.g200.ref_clk = 27050;
- bios = pci_map_rom(dev->pdev, &size);
- if (!bios)
return;- if (size != 0 && bios[0] == 0x55 && bios[1] == 0xaa)
mgag200_g200_interpret_bios(mdev, bios, size);- pci_unmap_rom(dev->pdev, bios);
- drm_dbg_kms(dev, "pclk_min: %ld pclk_max: %ld ref_clk: %ld\n",
mdev->model.g200.pclk_min, mdev->model.g200.pclk_max,mdev->model.g200.ref_clk);+}
static void mgag200_g200se_init_unique_id(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; @@ -138,7 +257,9 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) if (ret) return ret;
- if (IS_G200_SE(mdev))
if (mdev->type == G200_PCI || mdev->type == G200_AGP)
mgag200_g200_init_refclk(mdev);else if (IS_G200_SE(mdev)) mgag200_g200se_init_unique_id(mdev);
ret = mgag200_mm_init(mdev);
@@ -182,6 +303,8 @@ mgag200_device_create(struct pci_dev *pdev, unsigned long flags) */
static const struct pci_device_id mgag200_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_AGP
}, { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B }, diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 048efe635aff..54061a61e9ca 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -38,6 +38,8 @@ #define RREG32(reg) ioread32(((void __iomem *)mdev->rmmio) + (reg)) #define WREG32(reg, v) iowrite32(v, ((void __iomem *)mdev->rmmio) + (reg))
+#define MGA_BIOS_OFFSET 0x7ffc
#define ATTR_INDEX 0x1fc0 #define ATTR_DATA 0x1fc1
@@ -129,6 +131,8 @@ struct mga_mc { };
enum mga_type {
- G200_PCI,
- G200_AGP, G200_SE_A, G200_SE_B, G200_WB,
@@ -167,12 +171,18 @@ struct mga_device { int fb_mtrr;
union {
struct {long ref_clk;long pclk_min;long pclk_max;} g200;struct { /* SE model number stored in reg 0x1e24 */ u32 unique_rev_id; } g200se; } model;
struct mga_connector connector; struct drm_simple_display_pipe display_pipe;
}; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 752409c7f326..bc11552415f5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -108,6 +108,77 @@ static inline void mga_wait_busy(struct mga_device *mdev) } while ((status & 0x01) && time_before(jiffies, timeout)); }
+/*
- PLL setup
- */
+static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) +{
- struct drm_device *dev = &mdev->base;
- 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->model.g200.ref_clk;
- long p_clk_min = mdev->model.g200.pclk_min;
- long p_clk_max = mdev->model.g200.pclk_max;
- if (clock > p_clk_max) {
drm_err(dev, "Pixel Clock %ld too high\n", clock);return 1;- }
- if (clock < p_clk_min >> 3)
Looks like there's a stray space after the <. You could also just use max() here, but I'll leave that up to you
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;elsetmp_delta = computed - f_vco;Another stray space before the =
With those nitpicks addressed, this series is:
Reviewed-by: Lyude Paul lyude@redhat.com
Thanks a lot.
The other patches are probably uncontoversial. Let's see what happens to this one. :)
Best regards Thomas
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_dbg_kms(dev, "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) @@ -717,6 +788,9 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock) u8 misc;
switch(mdev->type) {
- case G200_PCI:
- case G200_AGP:
return mgag200_g200_set_plls(mdev, clock);@@ -894,6 +968,12 @@ static void mgag200_set_dac_regs(struct mga_device *mdev) };
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
dacvalue[MGA1064_SYS_PLL_M] = 0x04;dacvalue[MGA1064_SYS_PLL_N] = 0x2D;dacvalue[MGA1064_SYS_PLL_P] = 0x19;break;-- Cheers, Lyude Paul (she/her) Associate Software Engineer at Red Hat
Hi
Am 20.07.20 um 21:18 schrieb Lyude Paul:
On Mon, 2020-07-20 at 09:04 +0200, Thomas Zimmermann wrote:
Hi
Am 17.07.20 um 00:43 schrieb Lyude Paul:
On Wed, 2020-07-15 at 16:59 +0200, Thomas Zimmermann wrote:
This patch adds support for G200 desktop cards. We can reuse the whole memory and modesetting code. A few PCI and DAC register values have to be updated accordingly.
The most significant change is in the PLL setup. The get the clock limits and reference clocks, parses the device's BIOS. With no BIOS found, safe defaults are being used.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Co-developed-by: Egbert Eich eich@suse.com Signed-off-by: Egbert Eich eich@suse.com Co-developed-by: Takashi Iwai tiwai@suse.de Signed-off-by: Takashi Iwai tiwai@suse.de
MAINTAINERS | 2 +- drivers/gpu/drm/mgag200/Kconfig | 12 +-- drivers/gpu/drm/mgag200/mgag200_drv.c | 125 ++++++++++++++++++++++++- drivers/gpu/drm/mgag200/mgag200_drv.h | 10 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 80 ++++++++++++++++ 5 files changed, 220 insertions(+), 9 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 415954a98934..4c6f96e2b79b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5406,7 +5406,7 @@ S: Orphan / Obsolete F: drivers/gpu/drm/mga/ F: include/uapi/drm/mga_drm.h
-DRM DRIVER FOR MGA G200 SERVER GRAPHICS CHIPS +DRM DRIVER FOR MGA G200 GRAPHICS CHIPS M: Dave Airlie airlied@redhat.com S: Odd Fixes F: drivers/gpu/drm/mgag200/ diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index 93be766715c9..eec59658a938 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -1,13 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only config DRM_MGAG200
- tristate "Kernel modesetting driver for MGA G200 server engines"
- tristate "Matrox G200" depends on DRM && PCI && MMU select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER help
This is a KMS driver for the MGA G200 server chips, itdoes not support the original MGA G200 or any of the desktopchips. It requires 0.3.0 of the modesetting userspace driver,and a version of mga driver that will fail on KMS enableddevices.
This is a KMS driver for Matrox G200 chips. It supports the originalMGA G200 desktop chips and the server variants. It requires 0.3.0of the modesetting userspace driver, and a version of mga driverthat will fail on KMS enabled devices.diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index f7652e16365c..419817d6e2cd 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -64,6 +64,14 @@ static int mgag200_regs_init(struct mga_device *mdev) u8 crtcext3;
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
if (mgag200_has_sgram(mdev))option = 0x4049cd21;elseoption = 0x40499121;option2 = 0x00008000;break;@@ -115,6 +123,117 @@ static int mgag200_regs_init(struct mga_device *mdev) return 0; }
+static void mgag200_g200_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- };
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *pins;
huh, never realized you could write directly to unsigned char __iomem pointers!
I took the patch as-is, but this probably wouldn't work on all architectures.
Something occurred to me just now - do we actually care? I don't think I've ever seen mgag200 on anything other then x86_64 systems
That's no big deal. As Sam mentioned, it fixes some warnings and does not cause overhead. Besides, we have a bug in fbdev that is caused by direct I/O within framebuffer memory. So I'll better do it right here.
Wrt architecture, the MGAs have a PowerPC mode where they interpret certain I/O as big endian IIRC. They where standard on some old Macs. (?) I guess it's not really relevant any longer.
Best regards Thomas
- 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(dev, "Unknown BIOS PInS version: %d\n", version);Did you maybe mean pins or PINS here? or is PInS some weird abbreviation matrox uses?
It's the name of a data structure
https://gitlab.freedesktop.org/xorg/driver/xf86-video-mga/-/blob/master/mga_...
I have no idea what it stands for.
return;- }
- if (pins_len != expected_length[version]) {
drm_warn(dev, "Unexpected BIOS PInS size: %d expeced: %d\n",pins_len, expected_length[version]);return;- }
- if (offset + pins_len > size)
return;- drm_dbg_kms(dev, "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->model.g200.pclk_max = tmp * 10;break;- case 2:
if (pins[41] != 0xff)mdev->model.g200.pclk_max = (pins[41] + 100) * 1000;break;- case 3:
if (pins[36] != 0xff)mdev->model.g200.pclk_max = (pins[36] + 100) * 1000;if (pins[52] & 0x20)mdev->model.g200.ref_clk = 14318;break;- case 4:
if (pins[39] != 0xff)mdev->model.g200.pclk_max = pins[39] * 4 * 1000;if (pins[92] & 0x01)mdev->model.g200.ref_clk = 14318;break;- case 5:
tmp = pins[4] ? 8000 : 6000;if (pins[123] != 0xff)mdev->model.g200.pclk_min = pins[123] * tmp;if (pins[38] != 0xff)mdev->model.g200.pclk_max = pins[38] * tmp;if (pins[110] & 0x01)mdev->model.g200.ref_clk = 14318;break;- default:
break;- }
+}
+static void mgag200_g200_init_refclk(struct mga_device *mdev) +{
- struct drm_device *dev = &mdev->base;
- unsigned char __iomem *bios;
- size_t size;
- mdev->model.g200.pclk_min = 50000;
- mdev->model.g200.pclk_max = 230000;
- mdev->model.g200.ref_clk = 27050;
- bios = pci_map_rom(dev->pdev, &size);
- if (!bios)
return;- if (size != 0 && bios[0] == 0x55 && bios[1] == 0xaa)
mgag200_g200_interpret_bios(mdev, bios, size);- pci_unmap_rom(dev->pdev, bios);
- drm_dbg_kms(dev, "pclk_min: %ld pclk_max: %ld ref_clk: %ld\n",
mdev->model.g200.pclk_min, mdev->model.g200.pclk_max,mdev->model.g200.ref_clk);+}
static void mgag200_g200se_init_unique_id(struct mga_device *mdev) { struct drm_device *dev = &mdev->base; @@ -138,7 +257,9 @@ static int mgag200_device_init(struct mga_device *mdev, unsigned long flags) if (ret) return ret;
- if (IS_G200_SE(mdev))
if (mdev->type == G200_PCI || mdev->type == G200_AGP)
mgag200_g200_init_refclk(mdev);else if (IS_G200_SE(mdev)) mgag200_g200se_init_unique_id(mdev);
ret = mgag200_mm_init(mdev);
@@ -182,6 +303,8 @@ mgag200_device_create(struct pci_dev *pdev, unsigned long flags) */
static const struct pci_device_id mgag200_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_AGP
}, { PCI_VENDOR_ID_MATROX, 0x522, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_A | MGAG200_FLAG_HW_BUG_NO_STARTADD}, { PCI_VENDOR_ID_MATROX, 0x524, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_SE_B }, diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 048efe635aff..54061a61e9ca 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -38,6 +38,8 @@ #define RREG32(reg) ioread32(((void __iomem *)mdev->rmmio) + (reg)) #define WREG32(reg, v) iowrite32(v, ((void __iomem *)mdev->rmmio) + (reg))
+#define MGA_BIOS_OFFSET 0x7ffc
#define ATTR_INDEX 0x1fc0 #define ATTR_DATA 0x1fc1
@@ -129,6 +131,8 @@ struct mga_mc { };
enum mga_type {
- G200_PCI,
- G200_AGP, G200_SE_A, G200_SE_B, G200_WB,
@@ -167,12 +171,18 @@ struct mga_device { int fb_mtrr;
union {
struct {long ref_clk;long pclk_min;long pclk_max;} g200;struct { /* SE model number stored in reg 0x1e24 */ u32 unique_rev_id; } g200se; } model;
struct mga_connector connector; struct drm_simple_display_pipe display_pipe;
}; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 752409c7f326..bc11552415f5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -108,6 +108,77 @@ static inline void mga_wait_busy(struct mga_device *mdev) } while ((status & 0x01) && time_before(jiffies, timeout)); }
+/*
- PLL setup
- */
+static int mgag200_g200_set_plls(struct mga_device *mdev, long clock) +{
- struct drm_device *dev = &mdev->base;
- 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->model.g200.ref_clk;
- long p_clk_min = mdev->model.g200.pclk_min;
- long p_clk_max = mdev->model.g200.pclk_max;
- if (clock > p_clk_max) {
drm_err(dev, "Pixel Clock %ld too high\n", clock);return 1;- }
- if (clock < p_clk_min >> 3)
Looks like there's a stray space after the <. You could also just use max() here, but I'll leave that up to you
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;elsetmp_delta = computed - f_vco;Another stray space before the =
With those nitpicks addressed, this series is:
Reviewed-by: Lyude Paul lyude@redhat.com
Thanks a lot.
The other patches are probably uncontoversial. Let's see what happens to this one. :)
Best regards Thomas
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_dbg_kms(dev, "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) @@ -717,6 +788,9 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock) u8 misc;
switch(mdev->type) {
- case G200_PCI:
- case G200_AGP:
return mgag200_g200_set_plls(mdev, clock);@@ -894,6 +968,12 @@ static void mgag200_set_dac_regs(struct mga_device *mdev) };
switch (mdev->type) {
- case G200_PCI:
- case G200_AGP:
dacvalue[MGA1064_SYS_PLL_M] = 0x04;dacvalue[MGA1064_SYS_PLL_N] = 0x2D;dacvalue[MGA1064_SYS_PLL_P] = 0x19;break;
On Thu, 16 Jul 2020 at 00:59, Thomas Zimmermann tzimmermann@suse.de wrote:
This patchset puts device initialization in the correct order and adds support for G200 Desktop chips (PCI ids 0x520 and 0x521).
why? :-)
I'm pretty sure I NAKed the previous version because the userspace experience for these old cards was probably better with xorg-x11-drv-mga, but hey maybe it isn't anymore and we should go ahead. I know SuSE use these for testing, but apart from that do we really think we have any users for this?
Dave.
On Wed, Jul 15, 2020 at 9:56 PM Dave Airlie airlied@gmail.com wrote:
On Thu, 16 Jul 2020 at 00:59, Thomas Zimmermann tzimmermann@suse.de wrote:
This patchset puts device initialization in the correct order and adds support for G200 Desktop chips (PCI ids 0x520 and 0x521).
why? :-)
I'm pretty sure I NAKed the previous version because the userspace experience for these old cards was probably better with xorg-x11-drv-mga, but hey maybe it isn't anymore and we should go ahead. I know SuSE use these for testing, but apart from that do we really think we have any users for this?
I'm more of the "why not" kind ... if you don't want this driver, don't enable it. Maybe worst case the physical card driver ids should be a Kconfig option or so. But if the goal is to stomp fbdev into the ground I think we should be ok with having drivers for anything. Even if it's kinda horrible :-)
Of course you're not going to get any kind of acceleration, but then modern desktops don't accelerate if you have anything less than maybe gles2 anyway, and that entire idea of a reasonable 2d api that's actually generally useful died a hundred times already. -Daniel
Hi
Am 15.07.20 um 21:56 schrieb Dave Airlie:
On Thu, 16 Jul 2020 at 00:59, Thomas Zimmermann tzimmermann@suse.de wrote:
This patchset puts device initialization in the correct order and adds support for G200 Desktop chips (PCI ids 0x520 and 0x521).
why? :-)
With G200 support in place, we can add also support for the newer cards in the G-Series up to the G550. Believe it or not, the G550 for PCIe is still being actively marketed and manufactured by Matrox. [1] Even the predecessor chips G450 was only EOLed in Oct 2016. [2] So while the chips might be 20yrs old, the devices are still current.
Best regards Thomas
[1] https://matrox.com/graphics/en/products/graphics_cards/g_series/g550pcie/?pr... [2] https://www.matrox.com/graphics/en/products/legacy/g_series/g450pci/
I'm pretty sure I NAKed the previous version because the userspace experience for these old cards was probably better with xorg-x11-drv-mga, but hey maybe it isn't anymore and we should go ahead. I know SuSE use these for testing, but apart from that do we really think we have any users for this?
Dave. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Am 16.07.20 um 07:44 schrieb Thomas Zimmermann:
Hi
Am 15.07.20 um 21:56 schrieb Dave Airlie:
On Thu, 16 Jul 2020 at 00:59, Thomas Zimmermann tzimmermann@suse.de wrote:
This patchset puts device initialization in the correct order and adds support for G200 Desktop chips (PCI ids 0x520 and 0x521).
why? :-)
With G200 support in place, we can add also support for the newer cards in the G-Series up to the G550. Believe it or not, the G550 for PCIe is still being actively marketed and manufactured by Matrox. [1] Even the predecessor chips G450 was only EOLed in Oct 2016. [2] So while the chips might be 20yrs old, the devices are still current.
Best regards Thomas
[1] https://matrox.com/graphics/en/products/graphics_cards/g_series/g550pcie/?pr... [2] https://www.matrox.com/graphics/en/products/legacy/g_series/g450pci/
I'm pretty sure I NAKed the previous version because the userspace experience for these old cards was probably better with xorg-x11-drv-mga, but hey maybe it isn't anymore and we should go ahead. I know SuSE use these for testing, but apart from that do we really think we have any users for this?
Well, I got at least one email from someone thanking me for this patch. :)
Dave. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Will try to look over this tomorrow, jfyi
On Wed, 2020-07-15 at 16:58 +0200, Thomas Zimmermann wrote:
This patchset puts device initialization in the correct order and adds support for G200 Desktop chips (PCI ids 0x520 and 0x521).
The first 7 patches prepare the driver. Desktop chips would probably work without them, but since we're at it we can also do it right.
Patch 1 enables cached page mappings GEM buffers. SHMEM supports this well now and the MGA device does not access the buffer memory directly. So now corrupt display output is to be expected.
Patches 2 to 6 fix the initialization of device registers. Several fundamental registers were only done late during device initialization. This was probably not a problem in practice, as the VGA BIOS does the setup iduring POST anyway. These patches move the code to the beginning of the device initialization. If we ever have to POST a MGA device from the driver, the corect order of operations counts.
G200SEs store a unique id in the device structure. Patch 7 moves the value to model-specific data area. This will be helpful for patch 8.
Patch 8 adds support for desktop chips' PCI ids. all the memory and modesetting code continues to work as before. The PLL setup code gets an additional helper for the new HW. PCI and DAC regsiters get a few new default values. Most significantly, the driver parses the VGA BIOS for clock settings. It's all separate from the server code, so no regressions are to be expected.
The new HW support is based on an earlier patch the was posted in July 2017. [1]
Tested on G200EW and G200 AGP hardware by running the fbdev console, Weston and Gnome on Xorg.
[1] https://lists.freedesktop.org/archives/dri-devel/2017-July/147647.html
Thomas Zimmermann (8): drm/mgag200: Enable caching for SHMEM pages drm/mgag200: Move register initialization into helper function drm/mgag200: Initialize PCI registers early during device setup drm/mgag200: Enable MGA mode during device register initialization drm/mgag200: Set MISC memory flags in mm init code drm/mgag200: Clear <page> field during MM init drm/mgag200: Move G200SE's unique id into model-specific data drm/mgag200: Add support for G200 desktop cards
MAINTAINERS | 2 +- drivers/gpu/drm/mgag200/Kconfig | 12 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 213 +++++++++++++++++++++++-- drivers/gpu/drm/mgag200/mgag200_drv.h | 19 ++- drivers/gpu/drm/mgag200/mgag200_mm.c | 8 + drivers/gpu/drm/mgag200/mgag200_mode.c | 153 +++++++++++------- drivers/gpu/drm/mgag200/mgag200_reg.h | 4 + 7 files changed, 328 insertions(+), 83 deletions(-)
-- 2.27.0
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Dave Airlie -
Lyude Paul -
Sam Ravnborg -
Thomas Zimmermann