This patchset converts mgag200 to atomic modesetting. It uses simple KMS helpers and SHMEM. I'm posting v3 mostly for reference. These patches will land soon if no one objects.
Patch 1 removes cursor support. The HW cursor is not usable with the way universal planes work.
Patches 2 to 11 untangle the existing modesetting code into smaller functions. Specifically, mode setting and plane updates are being separated from each other.
Patch 12 to 14 convert mgag200 to simple KMS helpers and enables atomic mode setting.
Atomically switching plane framebuffers, requires either source or target buffer to be located at a non-0 offet. As some HW revisions seem to require a framebuffer offset of 0 within the video memory, they do not work with atomic modesetting. To resolve this problem, patch 15 converts mgag200 from VRAM helpers to SHMEM helpers. During plane updates, the content of the SHMEM BO is memcpy'd to VRAM. From my observation, performance is not nuch different from the original code.
The patchset has been tested on MGA G200EH hardware.
v3: * update commit messages * remove unused defines v2: * rebase patchset * replace uint{8,32}_t with u{8,32} through-out patchset * define additional register constants * use helper functions around bpp-shift computations * split conversion patch * keep busy-waiting in DPMS code * cleanups
Thomas Zimmermann (15): drm/mgag200: Remove HW cursor drm/mgag200: Clean up mga_set_start_address() drm/mgag200: Clean up mga_crtc_do_set_base() drm/mgag200: Move mode-setting code into separate helper function drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O drm/mgag200: Update mode registers after plane registers drm/mgag200: Set pitch in a separate helper function drm/mgag200: Set primary plane's format in separate helper function drm/mgag200: Move TAGFIFO reset into separate function drm/mgag200: Move hiprilvl setting into separate functions drm/mgag200: Move register initialization into separate function drm/mgag200: Remove out-commented suspend/resume helpers drm/mgag200: Use simple-display data structures drm/mgag200: Convert to simple KMS helper drm/mgag200: Replace VRAM helpers with SHMEM helpers
drivers/gpu/drm/mgag200/Kconfig | 4 +- drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 51 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 45 +- drivers/gpu/drm/mgag200/mgag200_main.c | 5 - drivers/gpu/drm/mgag200/mgag200_mode.c | 871 ++++++++++++++----------- drivers/gpu/drm/mgag200/mgag200_reg.h | 11 +- drivers/gpu/drm/mgag200/mgag200_ttm.c | 28 +- 8 files changed, 528 insertions(+), 489 deletions(-)
-- 2.26.2
The HW cursor of Matrox G200 cards only supports a 16-color palette format. Univeral planes require at least ARGB or a similar component- based format, so remove the HW cursor.
Alternatively, the driver could dither a cursor image from ARGB to 16 colors. But this does not produce pleasent-looking results in general, so it's useless for modern compositors.
Without HW support, compositors will use software rendering.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 13 ------------- drivers/gpu/drm/mgag200/mgag200_main.c | 5 ----- drivers/gpu/drm/mgag200/mgag200_mode.c | 2 -- 4 files changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/Makefile b/drivers/gpu/drm/mgag200/Makefile index 04b281bcf6558..63403133638a3 100644 --- a/drivers/gpu/drm/mgag200/Makefile +++ b/drivers/gpu/drm/mgag200/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -mgag200-y := mgag200_main.o mgag200_mode.o mgag200_cursor.o \ +mgag200-y := mgag200_main.o mgag200_mode.o \ mgag200_drv.o mgag200_i2c.o mgag200_ttm.o
obj-$(CONFIG_DRM_MGAG200) += mgag200.o diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index d9b7e96b214f8..bc372c2ec79e9 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -116,11 +116,6 @@ struct mga_connector { struct mga_i2c_chan *i2c; };
-struct mga_cursor { - struct drm_gem_vram_object *gbo[2]; - unsigned int next_index; -}; - struct mga_mc { resource_size_t vram_size; resource_size_t vram_base; @@ -156,8 +151,6 @@ struct mga_device {
struct mga_mc mc;
- struct mga_cursor cursor; - size_t vram_fb_available;
bool suspended; @@ -207,10 +200,4 @@ int mgag200_mm_init(struct mga_device *mdev); void mgag200_mm_fini(struct mga_device *mdev); int mgag200_mmap(struct file *filp, struct vm_area_struct *vma);
-int mgag200_cursor_init(struct mga_device *mdev); -void mgag200_cursor_fini(struct mga_device *mdev); -int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, - uint32_t handle, uint32_t width, uint32_t height); -int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y); - #endif /* __MGAG200_DRV_H__ */ diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 86df799fd38c5..3298eff7bd1b4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -135,10 +135,6 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags) goto err_mgag200_mm_fini; }
- ret = mgag200_cursor_init(mdev); - if (ret) - drm_err(dev, "Could not initialize cursors. Not doing hardware cursors.\n"); - return 0;
err_mgag200_mm_fini: @@ -154,7 +150,6 @@ void mgag200_driver_unload(struct drm_device *dev)
if (mdev == NULL) return; - mgag200_cursor_fini(mdev); mgag200_mm_fini(mdev); dev->dev_private = NULL; } diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 5f4ac36a97760..c68ed8b6faf9b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1412,8 +1412,6 @@ static void mga_crtc_disable(struct drm_crtc *crtc)
/* These provide the minimum set of functions required to handle a CRTC */ static const struct drm_crtc_funcs mga_crtc_funcs = { - .cursor_set = mgag200_crtc_cursor_set, - .cursor_move = mgag200_crtc_cursor_move, .gamma_set = mga_crtc_gamma_set, .set_config = drm_crtc_helper_set_config, .destroy = mga_crtc_destroy,
All register names and fields are now named according to the MGA programming manuals. The function doesn't need the CRTC, so callers pass in the device structure directly. The logging now uses device-specific macros.
The original implementation busy-waited for the VSYNC flag to go up, to synchronize the page flip with the display's vblank. This code has been moved to mga_crtc_mode_set_base(). It's still present in the non-atomic code paths, but won't be used in atomic commits. With atomic, we should use interrupts to synchronize with vblanks.
v3: * clarify commit message wrt. vblank busy-waiting v2: * use to_mga_device() * use MiB instead of MB * replace empty while loop with do-while, fixes checkpatch warning * replace uint{8,32}_t with u{8,32}
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_drv.h | 5 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 82 +++++++++++++++----------- 2 files changed, 53 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index bc372c2ec79e9..1963876ef3b8c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -61,6 +61,11 @@ WREG8(MGAREG_CRTC_DATA, v); \ } while (0) \
+#define RREG_ECRT(reg, v) \ + do { \ + WREG8(MGAREG_CRTCEXT_INDEX, reg); \ + v = RREG8(MGAREG_CRTCEXT_DATA); \ + } while (0) \
#define WREG_ECRT(reg, v) \ do { \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index c68ed8b6faf9b..80a3a805c0c4e 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -819,49 +819,53 @@ static void mga_g200wb_commit(struct drm_crtc *crtc) }
/* - This is how the framebuffer base address is stored in g200 cards: - * Assume @offset is the gpu_addr variable of the framebuffer object - * Then addr is the number of _pixels_ (not bytes) from the start of - VRAM to the first pixel we want to display. (divided by 2 for 32bit - framebuffers) - * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers - addr<20> -> CRTCEXT0<6> - addr<19-16> -> CRTCEXT0<3-0> - addr<15-8> -> CRTCC<7-0> - addr<7-0> -> CRTCD<7-0> - CRTCEXT0 has to be programmed last to trigger an update and make the - new addr variable take effect. + * This is how the framebuffer base address is stored in g200 cards: + * * Assume @offset is the gpu_addr variable of the framebuffer object + * * Then addr is the number of _pixels_ (not bytes) from the start of + * VRAM to the first pixel we want to display. (divided by 2 for 32bit + * framebuffers) + * * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers + * addr<20> -> CRTCEXT0<6> + * addr<19-16> -> CRTCEXT0<3-0> + * addr<15-8> -> CRTCC<7-0> + * addr<7-0> -> CRTCD<7-0> + * + * CRTCEXT0 has to be programmed last to trigger an update and make the + * new addr variable take effect. */ -static void mga_set_start_address(struct drm_crtc *crtc, unsigned offset) +static void mgag200_set_startadd(struct mga_device *mdev, + unsigned long offset) { - struct mga_device *mdev = to_mga_device(crtc->dev); - u32 addr; - int count; - u8 crtcext0; + struct drm_device *dev = mdev->dev; + u32 startadd; + u8 crtcc, crtcd, crtcext0;
- while (RREG8(0x1fda) & 0x08); - while (!(RREG8(0x1fda) & 0x08)); + startadd = offset / 8;
- count = RREG8(MGAREG_VCOUNT) + 2; - while (RREG8(MGAREG_VCOUNT) < count); - - WREG8(MGAREG_CRTCEXT_INDEX, 0); - crtcext0 = RREG8(MGAREG_CRTCEXT_DATA); - crtcext0 &= 0xB0; - addr = offset / 8; - /* Can't store addresses any higher than that... - but we also don't have more than 16MB of memory, so it should be fine. */ - WARN_ON(addr > 0x1fffff); - crtcext0 |= (!!(addr & (1<<20)))<<6; - WREG_CRT(0x0d, (u8)(addr & 0xff)); - WREG_CRT(0x0c, (u8)(addr >> 8) & 0xff); - WREG_ECRT(0x0, ((u8)(addr >> 16) & 0xf) | crtcext0); + /* + * Can't store addresses any higher than that, but we also + * don't have more than 16 MiB of memory, so it should be fine. + */ + drm_WARN_ON(dev, startadd > 0x1fffff); + + RREG_ECRT(0x00, crtcext0); + + crtcc = (startadd >> 8) & 0xff; + crtcd = startadd & 0xff; + crtcext0 &= 0xb0; + crtcext0 |= ((startadd >> 14) & BIT(6)) | + ((startadd >> 16) & 0x0f); + + WREG_CRT(0x0c, crtcc); + WREG_CRT(0x0d, crtcd); + WREG_ECRT(0x00, crtcext0); }
static int mga_crtc_do_set_base(struct drm_crtc *crtc, struct drm_framebuffer *fb, int x, int y, int atomic) { + struct mga_device *mdev = to_mga_device(crtc->dev); struct drm_gem_vram_object *gbo; int ret; s64 gpu_addr; @@ -882,7 +886,7 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, goto err_drm_gem_vram_unpin; }
- mga_set_start_address(crtc, (u32)gpu_addr); + mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
return 0;
@@ -894,6 +898,16 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb) { + struct drm_device *dev = crtc->dev; + struct mga_device *mdev = dev->dev_private; + unsigned int count; + + do { } while (RREG8(0x1fda) & 0x08); + do { } while (!(RREG8(0x1fda) & 0x08)); + + count = RREG8(MGAREG_VCOUNT) + 2; + do { } while (RREG8(MGAREG_VCOUNT) < count); + return mga_crtc_do_set_base(crtc, old_fb, x, y, 0); }
The function now only takes the device structure, and the old and new framebuffers.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 80a3a805c0c4e..9aa6addbbb895 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -861,21 +861,20 @@ static void mgag200_set_startadd(struct mga_device *mdev, WREG_ECRT(0x00, crtcext0); }
-static int mga_crtc_do_set_base(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int x, int y, int atomic) +static int mga_crtc_do_set_base(struct mga_device *mdev, + const struct drm_framebuffer *fb, + const struct drm_framebuffer *old_fb) { - struct mga_device *mdev = to_mga_device(crtc->dev); struct drm_gem_vram_object *gbo; int ret; s64 gpu_addr;
- if (!atomic && fb) { - gbo = drm_gem_vram_of_gem(fb->obj[0]); + if (old_fb) { + gbo = drm_gem_vram_of_gem(old_fb->obj[0]); drm_gem_vram_unpin(gbo); }
- gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]); + gbo = drm_gem_vram_of_gem(fb->obj[0]);
ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM); if (ret) @@ -900,6 +899,7 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, { struct drm_device *dev = crtc->dev; struct mga_device *mdev = dev->dev_private; + struct drm_framebuffer *fb = crtc->primary->fb; unsigned int count;
do { } while (RREG8(0x1fda) & 0x08); @@ -908,7 +908,7 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, count = RREG8(MGAREG_VCOUNT) + 2; do { } while (RREG8(MGAREG_VCOUNT) < count);
- return mga_crtc_do_set_base(crtc, old_fb, x, y, 0); + return mga_crtc_do_set_base(mdev, fb, old_fb); }
static int mga_crtc_mode_set(struct drm_crtc *crtc, @@ -1150,7 +1150,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
WREG8(MGA_MISC_OUT, misc);
- mga_crtc_do_set_base(crtc, old_fb, x, y, 0); + mga_crtc_do_set_base(mdev, fb, old_fb);
/* reset tagfifo */ if (mdev->type == G200_ER) {
The mode-setting code is now located in mgag200_set_mode_regs(), sans a few flags that will be moved in a later patch for clarity.
v2: * replace uint8_t with u8
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 140 ++++++++++++++----------- 1 file changed, 78 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 9aa6addbbb895..7c41bd43f79e0 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -911,6 +911,79 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, return mga_crtc_do_set_base(mdev, fb, old_fb); }
+static void mgag200_set_mode_regs(struct mga_device *mdev, + const struct drm_display_mode *mode) +{ + unsigned int hdisplay, hsyncstart, hsyncend, htotal; + unsigned int vdisplay, vsyncstart, vsyncend, vtotal; + u8 misc = 0; + u8 crtcext1, crtcext2, crtcext5; + + hdisplay = mode->hdisplay / 8 - 1; + hsyncstart = mode->hsync_start / 8 - 1; + hsyncend = mode->hsync_end / 8 - 1; + htotal = mode->htotal / 8 - 1; + + /* Work around hardware quirk */ + if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04) + htotal++; + + vdisplay = mode->vdisplay - 1; + vsyncstart = mode->vsync_start - 1; + vsyncend = mode->vsync_end - 1; + vtotal = mode->vtotal - 2; + + if (mode->flags & DRM_MODE_FLAG_NHSYNC) + misc |= 0x40; + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + misc |= 0x80; + + crtcext1 = (((htotal - 4) & 0x100) >> 8) | + ((hdisplay & 0x100) >> 7) | + ((hsyncstart & 0x100) >> 6) | + (htotal & 0x40); + if (mdev->type == G200_WB || mdev->type == G200_EW3) + crtcext1 |= BIT(7) | /* vrsten */ + BIT(3); /* hrsten */ + + crtcext2 = ((vtotal & 0xc00) >> 10) | + ((vdisplay & 0x400) >> 8) | + ((vdisplay & 0xc00) >> 7) | + ((vsyncstart & 0xc00) >> 5) | + ((vdisplay & 0x400) >> 3); + crtcext5 = 0x00; + + WREG_CRT(0, htotal - 4); + WREG_CRT(1, hdisplay); + WREG_CRT(2, hdisplay); + WREG_CRT(3, (htotal & 0x1F) | 0x80); + WREG_CRT(4, hsyncstart); + WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F)); + WREG_CRT(6, vtotal & 0xFF); + WREG_CRT(7, ((vtotal & 0x100) >> 8) | + ((vdisplay & 0x100) >> 7) | + ((vsyncstart & 0x100) >> 6) | + ((vdisplay & 0x100) >> 5) | + ((vdisplay & 0x100) >> 4) | /* linecomp */ + ((vtotal & 0x200) >> 4) | + ((vdisplay & 0x200) >> 3) | + ((vsyncstart & 0x200) >> 2)); + WREG_CRT(9, ((vdisplay & 0x200) >> 4) | + ((vdisplay & 0x200) >> 3)); + WREG_CRT(16, vsyncstart & 0xFF); + WREG_CRT(17, (vsyncend & 0x0F) | 0x20); + WREG_CRT(18, vdisplay & 0xFF); + WREG_CRT(20, 0); + WREG_CRT(21, vdisplay & 0xFF); + WREG_CRT(22, (vtotal + 1) & 0xFF); + WREG_CRT(23, 0xc3); + WREG_CRT(24, vdisplay & 0xFF); + + WREG_ECRT(0x01, crtcext1); + WREG_ECRT(0x02, crtcext2); + WREG_ECRT(0x05, crtcext5); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -919,8 +992,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); const struct drm_framebuffer *fb = crtc->primary->fb; - int hdisplay, hsyncstart, hsyncend, htotal; - int vdisplay, vsyncstart, vsyncend, vtotal; int pitch; int option = 0, option2 = 0; int i; @@ -999,12 +1070,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, break; }
- if (mode->flags & DRM_MODE_FLAG_NHSYNC) - misc |= 0x40; - if (mode->flags & DRM_MODE_FLAG_NVSYNC) - misc |= 0x80; - - for (i = 0; i < sizeof(dacvalue); i++) { if ((i <= 0x17) || (i == 0x1b) || @@ -1044,20 +1109,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, else pitch = pitch >> (4 - bppshift);
- hdisplay = mode->hdisplay / 8 - 1; - hsyncstart = mode->hsync_start / 8 - 1; - hsyncend = mode->hsync_end / 8 - 1; - htotal = mode->htotal / 8 - 1; - - /* Work around hardware quirk */ - if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04) - htotal++; - - vdisplay = mode->vdisplay - 1; - vsyncstart = mode->vsync_start - 1; - vsyncend = mode->vsync_end - 1; - vtotal = mode->vtotal - 2; - WREG_GFX(0, 0); WREG_GFX(1, 0); WREG_GFX(2, 0); @@ -1068,61 +1119,26 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_GFX(7, 0xf); WREG_GFX(8, 0xf);
- WREG_CRT(0, htotal - 4); - WREG_CRT(1, hdisplay); - WREG_CRT(2, hdisplay); - WREG_CRT(3, (htotal & 0x1F) | 0x80); - WREG_CRT(4, hsyncstart); - WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F)); - WREG_CRT(6, vtotal & 0xFF); - WREG_CRT(7, ((vtotal & 0x100) >> 8) | - ((vdisplay & 0x100) >> 7) | - ((vsyncstart & 0x100) >> 6) | - ((vdisplay & 0x100) >> 5) | - ((vdisplay & 0x100) >> 4) | /* linecomp */ - ((vtotal & 0x200) >> 4)| - ((vdisplay & 0x200) >> 3) | - ((vsyncstart & 0x200) >> 2)); - WREG_CRT(9, ((vdisplay & 0x200) >> 4) | - ((vdisplay & 0x200) >> 3)); WREG_CRT(10, 0); WREG_CRT(11, 0); WREG_CRT(12, 0); WREG_CRT(13, 0); WREG_CRT(14, 0); WREG_CRT(15, 0); - WREG_CRT(16, vsyncstart & 0xFF); - WREG_CRT(17, (vsyncend & 0x0F) | 0x20); - WREG_CRT(18, vdisplay & 0xFF); WREG_CRT(19, pitch & 0xFF); - WREG_CRT(20, 0); - WREG_CRT(21, vdisplay & 0xFF); - WREG_CRT(22, (vtotal + 1) & 0xFF); - WREG_CRT(23, 0xc3); - WREG_CRT(24, vdisplay & 0xFF); + + mgag200_set_mode_regs(mdev, mode);
ext_vga[0] = 0; - ext_vga[5] = 0;
/* TODO interlace */
ext_vga[0] |= (pitch & 0x300) >> 4; - ext_vga[1] = (((htotal - 4) & 0x100) >> 8) | - ((hdisplay & 0x100) >> 7) | - ((hsyncstart & 0x100) >> 6) | - (htotal & 0x40); - ext_vga[2] = ((vtotal & 0xc00) >> 10) | - ((vdisplay & 0x400) >> 8) | - ((vdisplay & 0xc00) >> 7) | - ((vsyncstart & 0xc00) >> 5) | - ((vdisplay & 0x400) >> 3); if (fb->format->cpp[0] * 8 == 24) ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80; else ext_vga[3] = ((1 << bppshift) - 1) | 0x80; ext_vga[4] = 0; - if (mdev->type == G200_WB || mdev->type == G200_EW3) - ext_vga[1] |= 0x88;
/* Set pixel clocks */ misc = 0x2d; @@ -1130,9 +1146,9 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
mga_crtc_set_plls(mdev, mode->clock);
- for (i = 0; i < 6; i++) { - WREG_ECRT(i, ext_vga[i]); - } + WREG_ECRT(0, ext_vga[0]); + WREG_ECRT(3, ext_vga[3]); + WREG_ECRT(4, ext_vga[4]);
if (mdev->type == G200_ER) WREG_ECRT(0x24, 0x5);
Set different fields in MISC in their rsp location in the code. This patch also fixes a bug in the original code where the mode's SYNC flags were never written into the MISC register.
v2: * use u8 instead of uint8_t * define MGAREG_MISC_CLK_SEL_MASK
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 38 ++++++++++++++++++-------- drivers/gpu/drm/mgag200/mgag200_reg.h | 6 +++- 2 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 7c41bd43f79e0..2007d7a4754ac 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -704,6 +704,8 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock)
static int mga_crtc_set_plls(struct mga_device *mdev, long clock) { + u8 misc; + switch(mdev->type) { case G200_SE_A: case G200_SE_B: @@ -724,6 +726,12 @@ static int mga_crtc_set_plls(struct mga_device *mdev, long clock) return mga_g200er_set_plls(mdev, clock); break; } + + misc = RREG8(MGA_MISC_IN); + misc &= ~MGAREG_MISC_CLK_SEL_MASK; + misc |= MGAREG_MISC_CLK_SEL_MGA_MSK; + WREG8(MGA_MISC_OUT, misc); + return 0; }
@@ -916,8 +924,7 @@ static void mgag200_set_mode_regs(struct mga_device *mdev, { unsigned int hdisplay, hsyncstart, hsyncend, htotal; unsigned int vdisplay, vsyncstart, vsyncend, vtotal; - u8 misc = 0; - u8 crtcext1, crtcext2, crtcext5; + u8 misc, crtcext1, crtcext2, crtcext5;
hdisplay = mode->hdisplay / 8 - 1; hsyncstart = mode->hsync_start / 8 - 1; @@ -933,10 +940,17 @@ static void mgag200_set_mode_regs(struct mga_device *mdev, vsyncend = mode->vsync_end - 1; vtotal = mode->vtotal - 2;
+ misc = RREG8(MGA_MISC_IN); + if (mode->flags & DRM_MODE_FLAG_NHSYNC) - misc |= 0x40; + misc |= MGAREG_MISC_HSYNCPOL; + else + misc &= ~MGAREG_MISC_HSYNCPOL; + if (mode->flags & DRM_MODE_FLAG_NVSYNC) - misc |= 0x80; + misc |= MGAREG_MISC_VSYNCPOL; + else + misc &= ~MGAREG_MISC_VSYNCPOL;
crtcext1 = (((htotal - 4) & 0x100) >> 8) | ((hdisplay & 0x100) >> 7) | @@ -982,6 +996,10 @@ static void mgag200_set_mode_regs(struct mga_device *mdev, WREG_ECRT(0x01, crtcext1); WREG_ECRT(0x02, crtcext2); WREG_ECRT(0x05, crtcext5); + + WREG8(MGA_MISC_OUT, misc); + + mga_crtc_set_plls(mdev, mode->clock); }
static int mga_crtc_mode_set(struct drm_crtc *crtc, @@ -1140,12 +1158,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, ext_vga[3] = ((1 << bppshift) - 1) | 0x80; ext_vga[4] = 0;
- /* Set pixel clocks */ - misc = 0x2d; - WREG8(MGA_MISC_OUT, misc); - - mga_crtc_set_plls(mdev, mode->clock); - WREG_ECRT(0, ext_vga[0]); WREG_ECRT(3, ext_vga[3]); WREG_ECRT(4, ext_vga[4]); @@ -1161,9 +1173,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, }
WREG_ECRT(0, ext_vga[0]); - /* Enable mga pixel clock */ - misc = 0x2d;
+ misc = RREG8(MGA_MISC_IN); + misc |= MGAREG_MISC_IOADSEL | + MGAREG_MISC_RAMMAPEN | + MGAREG_MISC_HIGH_PG_SEL; WREG8(MGA_MISC_OUT, misc);
mga_crtc_do_set_base(mdev, fb, old_fb); diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index c096a9d6bcbc1..0ba6e15e97106 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -16,10 +16,11 @@ * MGA1064SG Mystique register file */
- #ifndef _MGA_REG_H_ #define _MGA_REG_H_
+#include <linux/bits.h> + #define MGAREG_DWGCTL 0x1c00 #define MGAREG_MACCESS 0x1c04 /* the following is a mystique only register */ @@ -221,12 +222,15 @@
#define MGAREG_MISC_IOADSEL (0x1 << 0) #define MGAREG_MISC_RAMMAPEN (0x1 << 1) +#define MGAREG_MISC_CLK_SEL_MASK GENMASK(3, 2) #define MGAREG_MISC_CLK_SEL_VGA25 (0x0 << 2) #define MGAREG_MISC_CLK_SEL_VGA28 (0x1 << 2) #define MGAREG_MISC_CLK_SEL_MGA_PIX (0x2 << 2) #define MGAREG_MISC_CLK_SEL_MGA_MSK (0x3 << 2) #define MGAREG_MISC_VIDEO_DIS (0x1 << 4) #define MGAREG_MISC_HIGH_PG_SEL (0x1 << 5) +#define MGAREG_MISC_HSYNCPOL BIT(6) +#define MGAREG_MISC_VSYNCPOL BIT(7)
/* MMIO VGA registers */ #define MGAREG_SEQ_INDEX 0x1fc4
Greeting,
FYI, we noticed a -64.9% regression of phoronix-test-suite.glmark2.800x600.score due to commit:
commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Conve... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
in testcase: phoronix-test-suite on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with 48G memory with following parameters:
need_x: true test: glmark2-1.1.0 cpufreq_governor: performance ucode: 0x1d
test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added. test-url: http://www.phoronix-test-suite.com/
If you fix the issue, kindly add following tag Reported-by: kernel test robot rong.a.chen@intel.com
Details are as below: -------------------------------------------------------------------------------------------------->
To reproduce:
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp install job.yaml # job file is attached in this email bin/lkp run job.yaml
========================================================================================= compiler/cpufreq_governor/kconfig/need_x/rootfs/tbox_group/test/testcase/ucode: gcc-7/performance/x86_64-rhel-7.6/true/debian-x86_64-phoronix/lkp-nhm-2ep1/glmark2-1.1.0/phoronix-test-suite/0x1d
commit: bef2303526 ("drm/mgag200: Move mode-setting code into separate helper function") e44e907dd8 ("drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O")
bef2303526adb575 e44e907dd8f937313d35615d799 ---------------- --------------------------- %stddev %change %stddev \ | \ 66.00 -51.1% 32.25 phoronix-test-suite.glmark2.1024x768.score 97.00 -64.9% 34.00 phoronix-test-suite.glmark2.800x600.score
phoronix-test-suite.glmark2.800x600.score
100 +---------------------------------------------------------------------+ 90 |-+ + + +.+ + + + + + : | | : : : : : : : : : : : | 80 |-: : : : : : : : : : : | 70 |-:: : :: : : : :: :: : :: : | |: : : : : : : : : : : : : : : : : : : | 60 |:+: : : : : : : : : : : : : : : : : : | 50 |:+: : : : : : : : : : : : : : : : : : | 40 |:+ : : : : : : : : : : : : : : : : : : | |: : : : : : : : : : : : : : : : : : :O O O O O | 30 |:+ : : : : : : : : : : : : : : : : : : | 20 |-+ :: : : : : : :: : : :: : : O : | | : : : : : : : : : : : : : | 10 |-+ : : : : : : : : : : : : : | 0 +---------------------------------------------------------------------+
phoronix-test-suite.glmark2.1024x768.score
70 +----------------------------------------------------------------------+ | + + + +..+ + + + + + +.+ | 60 |-: : : : : : : : : : : | | : : : : : : : : : : : | 50 |-:: : :: : : :: : : :: :: : | |: : : : : : : : : : : : : : : : : : : | 40 |:+: : : : : : : : : : : : : : : : : : | |: : : : : : : : : : : : : : : : : : : O | 30 |:+ : : : : : : : : : : : : : : : : : :O O O O | |: : : : : : : : : : : : : : : : : : : | 20 |:+ : : : : : : : : : : : : : : : : : : | | :: : : : : : :: : : :: : : O :: | 10 |-+ : : : : : : : : : : : : : | | : : : O : O : : O : : : O : : : : | 0 +----------------------------------------------------------------------+
[*] bisect-good sample [O] bisect-bad sample
Disclaimer: Results have been estimated based on internal Intel analysis and are provided for informational purposes only. Any difference in system hardware or software design or configuration may affect actual performance.
Thanks, Rong Chen
Hi all,
On Thu, 4 Jun 2020 at 08:11, kernel test robot rong.a.chen@intel.com wrote:
Greeting,
FYI, we noticed a -64.9% regression of phoronix-test-suite.glmark2.800x600.score due to commit:
On one hand, I'm really happy to see performance testing happening although this report is missing various crucial pieces of information.
commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Conve... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
in testcase: phoronix-test-suite on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with 48G memory with following parameters:
need_x: true
Replace "need_x" with the Xorg version as seen in `Xorg -version'.
test: glmark2-1.1.0 cpufreq_governor: performance ucode: 0x1d
test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added. test-url: http://www.phoronix-test-suite.com/
Please remove the test description and url. They don't add any value.
Mention which Mesa version is used as well as on what GPU. The output of lspci and glxinfo will help here.
For this particular test - there is no Mesa/upstream driver for this GPU, so I imagine one of the swrast drivers was used. Which one - swrast (classic, softpipe, llvmpipe, swr) or kms_swrast. The output of `LD_DEBUG=libs glxinfo |& grep _dri.so` will help here.
commit: bef2303526 ("drm/mgag200: Move mode-setting code into separate helper function") e44e907dd8 ("drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O")
Actually the offending commit has a subtle change of behaviour - it adds an extra MGAREG_MISC_RAMMAPEN. That is not documented and I've failed to spot it during review.
Thomas - shall we revert that line in itself or at least add an inline comment why it is needed?
100 +---------------------------------------------------------------------+ 90 |-+ + + +.+ + + + + + : | | : : : : : : : : : : : | 80 |-: : : : : : : : : : : | 70 |-:: : :: : : : :: :: : :: : | |: : : : : : : : : : : : : : : : : : : | 60 |:+: : : : : : : : : : : : : : : : : : | 50 |:+: : : : : : : : : : : : : : : : : : | 40 |:+ : : : : : : : : : : : : : : : : : : | |: : : : : : : : : : : : : : : : : : :O O O O O | 30 |:+ : : : : : : : : : : : : : : : : : : | 20 |-+ :: : : : : : :: : : :: : : O : | | : : : : : : : : : : : : : | 10 |-+ : : : : : : : : : : : : : | 0 +---------------------------------------------------------------------+
phoronix-test-suite.glmark2.1024x768.score
70 +----------------------------------------------------------------------+ | + + + +..+ + + + + + +.+ | 60 |-: : : : : : : : : : : | | : : : : : : : : : : : | 50 |-:: : :: : : :: : : :: :: : | |: : : : : : : : : : : : : : : : : : : | 40 |:+: : : : : : : : : : : : : : : : : : | |: : : : : : : : : : : : : : : : : : : O | 30 |:+ : : : : : : : : : : : : : : : : : :O O O O | |: : : : : : : : : : : : : : : : : : : | 20 |:+ : : : : : : : : : : : : : : : : : : | | :: : : : : : :: : : :: : : O :: | 10 |-+ : : : : : : : : : : : : : | | : : : O : O : : O : : : O : : : : | 0 +----------------------------------------------------------------------+
[*] bisect-good sample [O] bisect-bad sample
Hmm I must be going blind - there isn't even a single * in either of the graphs. Or perhaps my eyesight is fine and the legend or the graphs need fixing.
HTH -Emil
On 6/16/20 4:58 AM, Emil Velikov wrote:
Hi all,
On Thu, 4 Jun 2020 at 08:11, kernel test robot rong.a.chen@intel.com wrote:
Greeting,
FYI, we noticed a -64.9% regression of phoronix-test-suite.glmark2.800x600.score due to commit:
On one hand, I'm really happy to see performance testing happening although this report is missing various crucial pieces of information.
commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Conve... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
in testcase: phoronix-test-suite on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with 48G memory with following parameters:
need_x: true
Replace "need_x" with the Xorg version as seen in `Xorg -version'.
# Xorg -version /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
X.Org X Server 1.20.4 X Protocol Version 11, Revision 0 Build Operating System: Linux 4.9.0-8-amd64 x86_64 Debian Current Operating System: Linux lkp-nhm-2ep1 5.7.0-rc5-01428-ge44e907dd8f937 #1 SMP Tue Jun 2 19:51:38 CST 2020 x86_64 Kernel command line: ip=::::lkp-nhm-2ep1::dhcp root=/dev/disk/by-id/wwn-0x55cd2e4123123127-part2 rootflags=subvol=debian-x86_64-phoronix remote_rootfs=internal-lkp-server:/osimage/debian/debian-x86_64-phoronix user=lkp job=/lkp/jobs/scheduled/lkp-nhm-2ep1/phoronix-test-suite-performance-true-glmark2-1.1.0-ucode=0x1d-debian-x86_64-phoronix-e44e907dd8f937313d35615d799d54162c56d173-20200616-56456-1kgmjzm-0.yaml ARCH=x86_64 kconfig=x86_64-rhel-7.6 branch=linux-devel/devel-hourly-2020051600 commit=e44e907dd8f937313d35615d799d54162c56d173 BOOT_IMAGE=/pkg/linux/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/vmlinuz-5.7.0-rc5-01428-ge44e907dd8f937 console=ttyS1,115200 console=tty0 max_uptime=3600 RESULT_ROOT=/result/phoronix-test-suite/performance-true-glmark2-1.1.0-ucode=0x1d/lkp-nhm-2ep1/debian-x86_64-phoronix/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/4 LKP_SERVER=inn nokaslr selinux=0 debug apic=debug sysrq_always_enabled rcupdate.rcu_cpu_stall_timeout=100 net.ifnames=0 printk.devkmsg=on panic=-1 softlockup_panic=1 nmi_watchdog=panic oops=panic load_ramdisk=2 prompt_ramdisk=0 drbd.minor_count=8 systemd.log_level=err ignore_loglevel console=tty0 earlyprintk=ttyS0,115200 console=ttyS0,115200 vga=normal rw Build Date: 05 March 2019 08:11:12PM xorg-server 2:1.20.4-1 (https://www.debian.org/support) Current version of pixman: 0.36.0        Before reporting problems, check http://wiki.x.org        to make sure that you have the latest version.
test: glmark2-1.1.0 cpufreq_governor: performance ucode: 0x1d
test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added. test-url: http://www.phoronix-test-suite.com/
Please remove the test description and url. They don't add any value.
Mention which Mesa version is used as well as on what GPU. The output of lspci and glxinfo will help here.
Attached please find the outputs of lspci and glxinfo
For this particular test - there is no Mesa/upstream driver for this GPU, so I imagine one of the swrast drivers was used. Which one - swrast (classic, softpipe, llvmpipe, swr) or kms_swrast. The output of `LD_DEBUG=libs glxinfo |& grep _dri.so` will help here.
# LD_DEBUG=libs glxinfo |& grep _dri.so      2132:    calling init: /usr/lib/i386-linux-gnu/dri/swrast_dri.so      2132:    calling fini: /usr/lib/i386-linux-gnu/dri/swrast_dri.so [0]
Best Regards, Rong Chen
commit: bef2303526 ("drm/mgag200: Move mode-setting code into separate helper function") e44e907dd8 ("drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O")
Actually the offending commit has a subtle change of behaviour - it adds an extra MGAREG_MISC_RAMMAPEN. That is not documented and I've failed to spot it during review.
Thomas - shall we revert that line in itself or at least add an inline comment why it is needed?
100 +---------------------------------------------------------------------+ 90 |-+ + + +.+ + + + + + : | | : : : : : : : : : : : | 80 |-: : : : : : : : : : : | 70 |-:: : :: : : : :: :: : :: : | |: : : : : : : : : : : : : : : : : : : | 60 |:+: : : : : : : : : : : : : : : : : : | 50 |:+: : : : : : : : : : : : : : : : : : | 40 |:+ : : : : : : : : : : : : : : : : : : | |: : : : : : : : : : : : : : : : : : :O O O O O | 30 |:+ : : : : : : : : : : : : : : : : : : | 20 |-+ :: : : : : : :: : : :: : : O : | | : : : : : : : : : : : : : | 10 |-+ : : : : : : : : : : : : : | 0 +---------------------------------------------------------------------+
phoronix-test-suite.glmark2.1024x768.score
70 +----------------------------------------------------------------------+ | + + + +..+ + + + + + +.+ | 60 |-: : : : : : : : : : : | | : : : : : : : : : : : | 50 |-:: : :: : : :: : : :: :: : | |: : : : : : : : : : : : : : : : : : : | 40 |:+: : : : : : : : : : : : : : : : : : | |: : : : : : : : : : : : : : : : : : : O | 30 |:+ : : : : : : : : : : : : : : : : : :O O O O | |: : : : : : : : : : : : : : : : : : : | 20 |:+ : : : : : : : : : : : : : : : : : : | | :: : : : : : :: : : :: : : O :: | 10 |-+ : : : : : : : : : : : : : | | : : : O : O : : O : : : O : : : : | 0 +----------------------------------------------------------------------+
[*] bisect-good sample [O] bisect-bad sample
Hmm I must be going blind - there isn't even a single * in either of the graphs. Or perhaps my eyesight is fine and the legend or the graphs need fixing.
HTH -Emil
On 6/16/20 11:10 AM, Rong Chen wrote:
On 6/16/20 4:58 AM, Emil Velikov wrote:
Hi all,
On Thu, 4 Jun 2020 at 08:11, kernel test robot rong.a.chen@intel.com wrote:
Greeting,
FYI, we noticed a -64.9% regression of phoronix-test-suite.glmark2.800x600.score due to commit:
On one hand, I'm really happy to see performance testing happening although this report is missing various crucial pieces of information.
commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Conve... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
in testcase: phoronix-test-suite on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with 48G memory with following parameters:
need_x: true
Replace "need_x" with the Xorg version as seen in `Xorg -version'.
# Xorg -version /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
X.Org X Server 1.20.4 X Protocol Version 11, Revision 0 Build Operating System: Linux 4.9.0-8-amd64 x86_64 Debian Current Operating System: Linux lkp-nhm-2ep1 5.7.0-rc5-01428-ge44e907dd8f937 #1 SMP Tue Jun 2 19:51:38 CST 2020 x86_64 Kernel command line: ip=::::lkp-nhm-2ep1::dhcp root=/dev/disk/by-id/wwn-0x55cd2e4123123127-part2 rootflags=subvol=debian-x86_64-phoronix remote_rootfs=internal-lkp-server:/osimage/debian/debian-x86_64-phoronix user=lkp job=/lkp/jobs/scheduled/lkp-nhm-2ep1/phoronix-test-suite-performance-true-glmark2-1.1.0-ucode=0x1d-debian-x86_64-phoronix-e44e907dd8f937313d35615d799d54162c56d173-20200616-56456-1kgmjzm-0.yaml ARCH=x86_64 kconfig=x86_64-rhel-7.6 branch=linux-devel/devel-hourly-2020051600 commit=e44e907dd8f937313d35615d799d54162c56d173 BOOT_IMAGE=/pkg/linux/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/vmlinuz-5.7.0-rc5-01428-ge44e907dd8f937 console=ttyS1,115200 console=tty0 max_uptime=3600 RESULT_ROOT=/result/phoronix-test-suite/performance-true-glmark2-1.1.0-ucode=0x1d/lkp-nhm-2ep1/debian-x86_64-phoronix/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/4 LKP_SERVER=inn nokaslr selinux=0 debug apic=debug sysrq_always_enabled rcupdate.rcu_cpu_stall_timeout=100 net.ifnames=0 printk.devkmsg=on panic=-1 softlockup_panic=1 nmi_watchdog=panic oops=panic load_ramdisk=2 prompt_ramdisk=0 drbd.minor_count=8 systemd.log_level=err ignore_loglevel console=tty0 earlyprintk=ttyS0,115200 console=ttyS0,115200 vga=normal rw Build Date: 05 March 2019 08:11:12PM xorg-server 2:1.20.4-1 (https://www.debian.org/support) Current version of pixman: 0.36.0        Before reporting problems, check http://wiki.x.org        to make sure that you have the latest version.
test: glmark2-1.1.0         cpufreq_governor: performance         ucode: 0x1d
test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added. test-url: http://www.phoronix-test-suite.com/
Please remove the test description and url. They don't add any value.
Mention which Mesa version is used as well as on what GPU. The output of lspci and glxinfo will help here.
Attached please find the outputs of lspci and glxinfo
Sorry, the previous lspci is not correct, please find it in this attachment.
Best Regards, Rong Chen
For this particular test - there is no Mesa/upstream driver for this GPU, so I imagine one of the swrast drivers was used. Which one - swrast (classic, softpipe, llvmpipe, swr) or kms_swrast. The output of `LD_DEBUG=libs glxinfo |& grep _dri.so` will help here.
# LD_DEBUG=libs glxinfo |& grep _dri.so      2132:    calling init: /usr/lib/i386-linux-gnu/dri/swrast_dri.so      2132:    calling fini: /usr/lib/i386-linux-gnu/dri/swrast_dri.so [0]
Best Regards, Rong Chen
commit: Â Â bef2303526 ("drm/mgag200: Move mode-setting code into separate helper function") Â Â e44e907dd8 ("drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O")
Actually the offending commit has a subtle change of behaviour - it adds an extra MGAREG_MISC_RAMMAPEN. That is not documented and I've failed to spot it during review.
Thomas - shall we revert that line in itself or at least add an inline comment why it is needed?
100 +---------------------------------------------------------------------+ Â Â Â 90 |-+Â Â Â +Â Â Â Â Â +Â Â +.+Â Â Â Â Â +Â Â Â +Â Â Â Â +Â Â Â +Â Â Â Â Â + :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â | :Â Â Â :Â Â Â Â Â :Â Â : :Â Â Â Â Â :Â Â Â :Â Â Â Â :Â Â Â :Â Â Â Â Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 80 |-:Â Â Â :Â Â Â Â Â :Â Â : :Â Â Â Â Â :Â Â Â :Â Â Â Â :Â Â Â :Â Â Â Â Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 70 |-::Â Â :Â Â Â Â ::Â Â :Â :Â Â Â Â :Â Â ::Â Â Â Â ::Â Â :Â Â Â Â :: :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â |: :Â : :Â Â Â : : :Â Â :Â Â Â : :Â : :Â Â : :Â : :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 60 |:+:Â : :Â Â Â : : :Â Â :Â Â Â : :Â : :Â Â : :Â : :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 50 |:+:Â : :Â Â Â : : :Â Â :Â Â Â : :Â : :Â Â : :Â : :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 40 |:+ : : :Â Â :Â : :Â Â Â :Â Â : : :Â :Â Â :Â : : :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â |:Â : : :Â Â :Â : :Â Â Â :Â Â : : :Â :Â Â :Â : : :Â Â :Â : :OÂ Â Â Â Â O O OÂ O | Â Â Â 30 |:+ : : :Â Â :Â : :Â Â Â :Â Â : : :Â :Â Â :Â : : :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 20 |-+ ::Â Â :Â :Â Â :Â Â Â Â :Â :Â Â ::Â Â : :Â Â ::Â Â :Â : O :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â |Â Â Â :Â Â : :Â Â Â :Â Â Â Â Â : :Â Â :Â Â Â : :Â Â Â :Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 10 |-+Â :Â Â : :Â Â Â :Â Â Â Â Â : :Â Â :Â Â Â : :Â Â Â :Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â 0 +---------------------------------------------------------------------+
phoronix-test-suite.glmark2.1024x768.score
70 +----------------------------------------------------------------------+
| +Â Â Â +Â Â Â Â Â +Â Â +..+Â Â Â Â +Â Â Â +Â Â Â Â Â +Â Â Â +Â Â Â Â + +.+Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 60 |-:Â Â Â :Â Â Â Â Â :Â Â :Â :Â Â Â Â :Â Â Â :Â Â Â Â Â :Â Â Â :Â Â Â Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â | :Â Â Â :Â Â Â Â Â :Â Â :Â :Â Â Â Â :Â Â Â :Â Â Â Â Â :Â Â Â :Â Â Â Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 50 |-::Â Â :Â Â Â Â ::Â Â :Â :Â Â Â Â ::Â Â :Â Â Â Â Â :Â Â ::Â Â Â Â :: :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |: :Â : :Â Â Â : : :Â Â Â :Â Â : :Â : :Â Â Â : :Â : :Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 40 |:+:Â : :Â Â Â : : :Â Â Â :Â Â : :Â : :Â Â Â : :Â : :Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |: :Â : :Â Â Â : : :Â Â Â :Â Â : :Â : :Â Â Â : :Â : :Â Â : : :Â Â Â Â Â Â Â Â OÂ Â Â Â Â | Â Â 30 |:+ : : :Â Â :Â : :Â Â Â :Â Â :Â : : :Â Â Â : : :Â :Â Â :Â : :OÂ Â Â Â Â OÂ Â OÂ O | Â Â Â Â Â |:Â : : :Â Â :Â : :Â Â Â :Â Â :Â : : :Â Â Â : : :Â :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 20 |:+ : : :Â Â :Â : :Â Â Â :Â Â :Â : : :Â Â Â : : :Â :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |Â Â ::Â Â :Â :Â Â :Â Â Â Â Â : :Â Â ::Â Â :Â :Â Â ::Â Â : : O ::Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 10 |-+Â :Â Â : :Â Â Â :Â Â Â Â Â : :Â Â Â :Â Â :Â :Â Â :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |Â Â Â :Â Â : :Â O :Â Â Â O : : OÂ :Â Â :Â : O :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 0 +----------------------------------------------------------------------+
[*] bisect-good sample [O] bisect-bad sample
Hmm I must be going blind - there isn't even a single * in either of the graphs. Or perhaps my eyesight is fine and the legend or the graphs need fixing.
HTH -Emil
LKP mailing list -- lkp@lists.01.org To unsubscribe send an email to lkp-leave@lists.01.org
Hi Rong,
Thanks for the prompt reply and information. Can I suggest including the suggested information in future reports. I've included a command for each one, to aid automating things.
Namely: Xorg: 1.20.4 (or None) $ which Xorg 2>/dev/null || echo None && Xorg -version |& grep -o "X Server.*" | sed "s/X Server//"
Mesa: 20.0.7 $ grep -E -o "Mesa [1-9]+.*" | head -n1 | sed "s/Mesa//"
Mesa module: swrast_dri.so $ basename `LD_DEBUG=libs glxinfo |& grep _dri.so | head -n1 | cut -f3 -d:`
Mesa device: llvmpipe (LLVM 10.0.0, 128 bits) (0xffffffff) $ glxinfo | grep -i device | cut -f2 -d:
GPU: Matrox Electronics ... $ lspci -nn | grep -E "VGA|Display|3D" | cut -f2- -d:
Thanks -Emil
Hi Emil,
Thanks for the guidance, we'll add these information in future reports.
Best Regards, Rong Chen
On 6/16/20 9:49 PM, Emil Velikov wrote:
Hi Rong,
Thanks for the prompt reply and information. Can I suggest including the suggested information in future reports. I've included a command for each one, to aid automating things.
Namely: Xorg: 1.20.4 (or None) $ which Xorg 2>/dev/null || echo None && Xorg -version |& grep -o "X Server.*" | sed "s/X Server//"
Mesa: 20.0.7 $ grep -E -o "Mesa [1-9]+.*" | head -n1 | sed "s/Mesa//"
Mesa module: swrast_dri.so $ basename `LD_DEBUG=libs glxinfo |& grep _dri.so | head -n1 | cut -f3 -d:`
Mesa device: llvmpipe (LLVM 10.0.0, 128 bits) (0xffffffff) $ glxinfo | grep -i device | cut -f2 -d:
GPU: Matrox Electronics ... $ lspci -nn | grep -E "VGA|Display|3D" | cut -f2- -d:
Thanks -Emil
Hi
Am 16.06.20 um 05:10 schrieb Rong Chen:
On 6/16/20 4:58 AM, Emil Velikov wrote:
Hi all,
On Thu, 4 Jun 2020 at 08:11, kernel test robot rong.a.chen@intel.com wrote:
Greeting,
FYI, we noticed a -64.9% regression of phoronix-test-suite.glmark2.800x600.score due to commit:
On one hand, I'm really happy to see performance testing happening although this report is missing various crucial pieces of information.
commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Conve...
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
in testcase: phoronix-test-suite on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with 48G memory with following parameters:
need_x: true
Replace "need_x" with the Xorg version as seen in `Xorg -version'.
# Xorg -version /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
X.Org X Server 1.20.4 X Protocol Version 11, Revision 0 Build Operating System: Linux 4.9.0-8-amd64 x86_64 Debian Current Operating System: Linux lkp-nhm-2ep1 5.7.0-rc5-01428-ge44e907dd8f937 #1 SMP Tue Jun 2 19:51:38 CST 2020 x86_64 Kernel command line: ip=::::lkp-nhm-2ep1::dhcp root=/dev/disk/by-id/wwn-0x55cd2e4123123127-part2 rootflags=subvol=debian-x86_64-phoronix remote_rootfs=internal-lkp-server:/osimage/debian/debian-x86_64-phoronix user=lkp job=/lkp/jobs/scheduled/lkp-nhm-2ep1/phoronix-test-suite-performance-true-glmark2-1.1.0-ucode=0x1d-debian-x86_64-phoronix-e44e907dd8f937313d35615d799d54162c56d173-20200616-56456-1kgmjzm-0.yaml ARCH=x86_64 kconfig=x86_64-rhel-7.6 branch=linux-devel/devel-hourly-2020051600 commit=e44e907dd8f937313d35615d799d54162c56d173 BOOT_IMAGE=/pkg/linux/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/vmlinuz-5.7.0-rc5-01428-ge44e907dd8f937 console=ttyS1,115200 console=tty0 max_uptime=3600 RESULT_ROOT=/result/phoronix-test-suite/performance-true-glmark2-1.1.0-ucode=0x1d/lkp-nhm-2ep1/debian-x86_64-phoronix/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/4 LKP_SERVER=inn nokaslr selinux=0 debug apic=debug sysrq_always_enabled rcupdate.rcu_cpu_stall_timeout=100 net.ifnames=0 printk.devkmsg=on panic=-1 softlockup_panic=1 nmi_watchdog=panic oops=panic load_ramdisk=2 prompt_ramdisk=0 drbd.minor_count=8 systemd.log_level=err ignore_loglevel console=tty0 earlyprintk=ttyS0,115200 console=ttyS0,115200 vga=normal rw Build Date: 05 March 2019 08:11:12PM xorg-server 2:1.20.4-1 (https://www.debian.org/support) Current version of pixman: 0.36.0        Before reporting problems, check http://wiki.x.org        to make sure that you have the latest version.
test: glmark2-1.1.0         cpufreq_governor: performance         ucode: 0x1d
test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added. test-url: http://www.phoronix-test-suite.com/
Please remove the test description and url. They don't add any value.
Mention which Mesa version is used as well as on what GPU. The output of lspci and glxinfo will help here.
Attached please find the outputs of lspci and glxinfo
For this particular test - there is no Mesa/upstream driver for this GPU, so I imagine one of the swrast drivers was used. Which one - swrast (classic, softpipe, llvmpipe, swr) or kms_swrast. The output of `LD_DEBUG=libs glxinfo |& grep _dri.so` will help here.
# LD_DEBUG=libs glxinfo |& grep _dri.so      2132:    calling init: /usr/lib/i386-linux-gnu/dri/swrast_dri.so      2132:    calling fini: /usr/lib/i386-linux-gnu/dri/swrast_dri.so [0]
Best Regards, Rong Chen
Thanks for testing. If I send out a patch, could you try it?
Best regards Thomas
commit: Â Â bef2303526 ("drm/mgag200: Move mode-setting code into separate helper function") Â Â e44e907dd8 ("drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O")
Actually the offending commit has a subtle change of behaviour - it adds an extra MGAREG_MISC_RAMMAPEN. That is not documented and I've failed to spot it during review.
Thomas - shall we revert that line in itself or at least add an inline comment why it is needed?
100 +---------------------------------------------------------------------+ Â Â Â 90 |-+Â Â Â +Â Â Â Â Â +Â Â +.+Â Â Â Â Â +Â Â Â +Â Â Â Â +Â Â Â +Â Â Â Â Â +Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â | :Â Â Â :Â Â Â Â Â :Â Â : :Â Â Â Â Â :Â Â Â :Â Â Â Â :Â Â Â :Â Â Â Â Â :Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 80 |-:Â Â Â :Â Â Â Â Â :Â Â : :Â Â Â Â Â :Â Â Â :Â Â Â Â :Â Â Â :Â Â Â Â Â :Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 70 |-::Â Â :Â Â Â Â ::Â Â :Â :Â Â Â Â :Â Â ::Â Â Â Â ::Â Â :Â Â Â Â ::Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â |: :Â : :Â Â Â : : :Â Â :Â Â Â : :Â : :Â Â : :Â : :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 60 |:+:Â : :Â Â Â : : :Â Â :Â Â Â : :Â : :Â Â : :Â : :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 50 |:+:Â : :Â Â Â : : :Â Â :Â Â Â : :Â : :Â Â : :Â : :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 40 |:+ : : :Â Â :Â : :Â Â Â :Â Â : : :Â :Â Â :Â : : :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â |:Â : : :Â Â :Â : :Â Â Â :Â Â : : :Â :Â Â :Â : : :Â Â :Â : :OÂ Â Â Â Â O O OÂ O | Â Â Â 30 |:+ : : :Â Â :Â : :Â Â Â :Â Â : : :Â :Â Â :Â : : :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 20 |-+ ::Â Â :Â :Â Â :Â Â Â Â :Â :Â Â ::Â Â : :Â Â ::Â Â :Â : O :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â |Â Â Â :Â Â : :Â Â Â :Â Â Â Â Â : :Â Â :Â Â Â : :Â Â Â :Â Â : :Â Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 10 |-+Â :Â Â : :Â Â Â :Â Â Â Â Â : :Â Â :Â Â Â : :Â Â Â :Â Â : :Â Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â 0 +---------------------------------------------------------------------+
phoronix-test-suite.glmark2.1024x768.score
70 +----------------------------------------------------------------------+ Â Â Â Â Â | +Â Â Â +Â Â Â Â Â +Â Â +..+Â Â Â Â +Â Â Â +Â Â Â Â Â +Â Â Â +Â Â Â Â +Â Â Â +.+Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 60 |-:Â Â Â :Â Â Â Â Â :Â Â :Â :Â Â Â Â :Â Â Â :Â Â Â Â Â :Â Â Â :Â Â Â Â :Â Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â | :Â Â Â :Â Â Â Â Â :Â Â :Â :Â Â Â Â :Â Â Â :Â Â Â Â Â :Â Â Â :Â Â Â Â :Â Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 50 |-::Â Â :Â Â Â Â ::Â Â :Â :Â Â Â Â ::Â Â :Â Â Â Â Â :Â Â ::Â Â Â Â ::Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |: :Â : :Â Â Â : : :Â Â Â :Â Â : :Â : :Â Â Â : :Â : :Â Â : :Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 40 |:+:Â : :Â Â Â : : :Â Â Â :Â Â : :Â : :Â Â Â : :Â : :Â Â : :Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |: :Â : :Â Â Â : : :Â Â Â :Â Â : :Â : :Â Â Â : :Â : :Â Â : :Â :Â Â Â Â Â Â Â Â OÂ Â Â Â Â | Â Â 30 |:+ : : :Â Â :Â : :Â Â Â :Â Â :Â : : :Â Â Â : : :Â :Â Â :Â : :OÂ Â Â Â Â OÂ Â OÂ O | Â Â Â Â Â |:Â : : :Â Â :Â : :Â Â Â :Â Â :Â : : :Â Â Â : : :Â :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 20 |:+ : : :Â Â :Â : :Â Â Â :Â Â :Â : : :Â Â Â : : :Â :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |Â Â ::Â Â :Â :Â Â :Â Â Â Â Â : :Â Â ::Â Â :Â :Â Â ::Â Â : : O ::Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 10 |-+Â :Â Â : :Â Â Â :Â Â Â Â Â : :Â Â Â :Â Â :Â :Â Â :Â Â Â : :Â Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |Â Â Â :Â Â : :Â O :Â Â Â O : : OÂ :Â Â :Â : O :Â Â Â : :Â Â Â :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 0 +----------------------------------------------------------------------+
[*] bisect-good sample [O] bisect-bad sample
Hmm I must be going blind - there isn't even a single * in either of the graphs. Or perhaps my eyesight is fine and the legend or the graphs need fixing.
HTH -Emil
On 6/16/20 9:49 PM, Thomas Zimmermann wrote:
Hi
Am 16.06.20 um 05:10 schrieb Rong Chen:
On 6/16/20 4:58 AM, Emil Velikov wrote:
Hi all,
On Thu, 4 Jun 2020 at 08:11, kernel test robot rong.a.chen@intel.com wrote:
Greeting,
FYI, we noticed a -64.9% regression of phoronix-test-suite.glmark2.800x600.score due to commit:
On one hand, I'm really happy to see performance testing happening although this report is missing various crucial pieces of information.
commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Conve...
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
in testcase: phoronix-test-suite on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with 48G memory with following parameters:
need_x: true
Replace "need_x" with the Xorg version as seen in `Xorg -version'.
# Xorg -version /bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
X.Org X Server 1.20.4 X Protocol Version 11, Revision 0 Build Operating System: Linux 4.9.0-8-amd64 x86_64 Debian Current Operating System: Linux lkp-nhm-2ep1 5.7.0-rc5-01428-ge44e907dd8f937 #1 SMP Tue Jun 2 19:51:38 CST 2020 x86_64 Kernel command line: ip=::::lkp-nhm-2ep1::dhcp root=/dev/disk/by-id/wwn-0x55cd2e4123123127-part2 rootflags=subvol=debian-x86_64-phoronix remote_rootfs=internal-lkp-server:/osimage/debian/debian-x86_64-phoronix user=lkp job=/lkp/jobs/scheduled/lkp-nhm-2ep1/phoronix-test-suite-performance-true-glmark2-1.1.0-ucode=0x1d-debian-x86_64-phoronix-e44e907dd8f937313d35615d799d54162c56d173-20200616-56456-1kgmjzm-0.yaml ARCH=x86_64 kconfig=x86_64-rhel-7.6 branch=linux-devel/devel-hourly-2020051600 commit=e44e907dd8f937313d35615d799d54162c56d173 BOOT_IMAGE=/pkg/linux/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/vmlinuz-5.7.0-rc5-01428-ge44e907dd8f937 console=ttyS1,115200 console=tty0 max_uptime=3600 RESULT_ROOT=/result/phoronix-test-suite/performance-true-glmark2-1.1.0-ucode=0x1d/lkp-nhm-2ep1/debian-x86_64-phoronix/x86_64-rhel-7.6/gcc-7/e44e907dd8f937313d35615d799d54162c56d173/4 LKP_SERVER=inn nokaslr selinux=0 debug apic=debug sysrq_always_enabled rcupdate.rcu_cpu_stall_timeout=100 net.ifnames=0 printk.devkmsg=on panic=-1 softlockup_panic=1 nmi_watchdog=panic oops=panic load_ramdisk=2 prompt_ramdisk=0 drbd.minor_count=8 systemd.log_level=err ignore_loglevel console=tty0 earlyprintk=ttyS0,115200 console=ttyS0,115200 vga=normal rw Build Date: 05 March 2019 08:11:12PM xorg-server 2:1.20.4-1 (https://www.debian.org/support) Current version of pixman: 0.36.0        Before reporting problems, check http://wiki.x.org        to make sure that you have the latest version.
test: glmark2-1.1.0         cpufreq_governor: performance         ucode: 0x1d
test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added. test-url: http://www.phoronix-test-suite.com/
Please remove the test description and url. They don't add any value.
Mention which Mesa version is used as well as on what GPU. The output of lspci and glxinfo will help here.
Attached please find the outputs of lspci and glxinfo
For this particular test - there is no Mesa/upstream driver for this GPU, so I imagine one of the swrast drivers was used. Which one - swrast (classic, softpipe, llvmpipe, swr) or kms_swrast. The output of `LD_DEBUG=libs glxinfo |& grep _dri.so` will help here.
# LD_DEBUG=libs glxinfo |& grep _dri.so      2132:    calling init: /usr/lib/i386-linux-gnu/dri/swrast_dri.so      2132:    calling fini: /usr/lib/i386-linux-gnu/dri/swrast_dri.so [0]
Best Regards, Rong Chen
Thanks for testing. If I send out a patch, could you try it?
Yes, we can test the new patch if still needed.
Best Regards, Rong Chen
Best regards Thomas
commit: Â Â bef2303526 ("drm/mgag200: Move mode-setting code into separate helper function") Â Â e44e907dd8 ("drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O")
Actually the offending commit has a subtle change of behaviour - it adds an extra MGAREG_MISC_RAMMAPEN. That is not documented and I've failed to spot it during review.
Thomas - shall we revert that line in itself or at least add an inline comment why it is needed?
100 +---------------------------------------------------------------------+ Â Â Â 90 |-+Â Â Â +Â Â Â Â Â +Â Â +.+Â Â Â Â Â +Â Â Â +Â Â Â Â +Â Â Â +Â Â Â Â Â + :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â | :Â Â Â :Â Â Â Â Â :Â Â : :Â Â Â Â Â :Â Â Â :Â Â Â Â :Â Â Â :Â Â Â Â Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 80 |-:Â Â Â :Â Â Â Â Â :Â Â : :Â Â Â Â Â :Â Â Â :Â Â Â Â :Â Â Â :Â Â Â Â Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 70 |-::Â Â :Â Â Â Â ::Â Â :Â :Â Â Â Â :Â Â ::Â Â Â Â ::Â Â :Â Â Â Â :: :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â |: :Â : :Â Â Â : : :Â Â :Â Â Â : :Â : :Â Â : :Â : :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 60 |:+:Â : :Â Â Â : : :Â Â :Â Â Â : :Â : :Â Â : :Â : :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 50 |:+:Â : :Â Â Â : : :Â Â :Â Â Â : :Â : :Â Â : :Â : :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 40 |:+ : : :Â Â :Â : :Â Â Â :Â Â : : :Â :Â Â :Â : : :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â |:Â : : :Â Â :Â : :Â Â Â :Â Â : : :Â :Â Â :Â : : :Â Â :Â : :OÂ Â Â Â Â O O OÂ O | Â Â Â 30 |:+ : : :Â Â :Â : :Â Â Â :Â Â : : :Â :Â Â :Â : : :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 20 |-+ ::Â Â :Â :Â Â :Â Â Â Â :Â :Â Â ::Â Â : :Â Â ::Â Â :Â : O :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â Â |Â Â Â :Â Â : :Â Â Â :Â Â Â Â Â : :Â Â :Â Â Â : :Â Â Â :Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 10 |-+Â :Â Â : :Â Â Â :Â Â Â Â Â : :Â Â :Â Â Â : :Â Â Â :Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â 0 +---------------------------------------------------------------------+
phoronix-test-suite.glmark2.1024x768.score
70 +----------------------------------------------------------------------+ Â Â Â Â Â | +Â Â Â +Â Â Â Â Â +Â Â +..+Â Â Â Â +Â Â Â +Â Â Â Â Â +Â Â Â +Â Â Â Â + +.+Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 60 |-:Â Â Â :Â Â Â Â Â :Â Â :Â :Â Â Â Â :Â Â Â :Â Â Â Â Â :Â Â Â :Â Â Â Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â | :Â Â Â :Â Â Â Â Â :Â Â :Â :Â Â Â Â :Â Â Â :Â Â Â Â Â :Â Â Â :Â Â Â Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 50 |-::Â Â :Â Â Â Â ::Â Â :Â :Â Â Â Â ::Â Â :Â Â Â Â Â :Â Â ::Â Â Â Â :: :Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |: :Â : :Â Â Â : : :Â Â Â :Â Â : :Â : :Â Â Â : :Â : :Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 40 |:+:Â : :Â Â Â : : :Â Â Â :Â Â : :Â : :Â Â Â : :Â : :Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |: :Â : :Â Â Â : : :Â Â Â :Â Â : :Â : :Â Â Â : :Â : :Â Â : :Â : OÂ Â Â Â Â | Â Â 30 |:+ : : :Â Â :Â : :Â Â Â :Â Â :Â : : :Â Â Â : : :Â :Â Â :Â : :O OÂ Â OÂ O | Â Â Â Â Â |:Â : : :Â Â :Â : :Â Â Â :Â Â :Â : : :Â Â Â : : :Â :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 20 |:+ : : :Â Â :Â : :Â Â Â :Â Â :Â : : :Â Â Â : : :Â :Â Â :Â : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |Â Â ::Â Â :Â :Â Â :Â Â Â Â Â : :Â Â ::Â Â :Â :Â Â ::Â Â : : O ::Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â 10 |-+Â :Â Â : :Â Â Â :Â Â Â Â Â : :Â Â Â :Â Â :Â :Â Â :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â Â Â |Â Â Â :Â Â : :Â O :Â Â Â O : : OÂ :Â Â :Â : O :Â Â Â : : :Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â | Â Â Â 0 +----------------------------------------------------------------------+
[*] bisect-good sample [O] bisect-bad sample
Hmm I must be going blind - there isn't even a single * in either of the graphs. Or perhaps my eyesight is fine and the legend or the graphs need fixing.
HTH -Emil
Hi
Am 15.06.20 um 22:58 schrieb Emil Velikov:
Hi all,
On Thu, 4 Jun 2020 at 08:11, kernel test robot rong.a.chen@intel.com wrote:
Greeting,
FYI, we noticed a -64.9% regression of phoronix-test-suite.glmark2.800x600.score due to commit:
On one hand, I'm really happy to see performance testing happening although this report is missing various crucial pieces of information.
commit: e44e907dd8f937313d35615d799d54162c56d173 ("[PATCH v3 05/15] drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O") url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-mgag200-Conve... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
in testcase: phoronix-test-suite on test machine: 16 threads Intel(R) Xeon(R) CPU X5570 @ 2.93GHz with 48G memory with following parameters:
need_x: true
Replace "need_x" with the Xorg version as seen in `Xorg -version'.
test: glmark2-1.1.0 cpufreq_governor: performance ucode: 0x1d
test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added. test-url: http://www.phoronix-test-suite.com/
Please remove the test description and url. They don't add any value.
Mention which Mesa version is used as well as on what GPU. The output of lspci and glxinfo will help here.
For this particular test - there is no Mesa/upstream driver for this GPU, so I imagine one of the swrast drivers was used. Which one - swrast (classic, softpipe, llvmpipe, swr) or kms_swrast. The output of `LD_DEBUG=libs glxinfo |& grep _dri.so` will help here.
commit: bef2303526 ("drm/mgag200: Move mode-setting code into separate helper function") e44e907dd8 ("drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O")
Actually the offending commit has a subtle change of behaviour - it adds an extra MGAREG_MISC_RAMMAPEN. That is not documented and I've failed to spot it during review.
Thomas - shall we revert that line in itself or at least add an inline comment why it is needed?
Oh, well spotted. I'll send out a patch to not set the bit. Hopefully this will resolve the problem.
100 +---------------------------------------------------------------------+ 90 |-+ + + +.+ + + + + + : | | : : : : : : : : : : : | 80 |-: : : : : : : : : : : | 70 |-:: : :: : : : :: :: : :: : | |: : : : : : : : : : : : : : : : : : : | 60 |:+: : : : : : : : : : : : : : : : : : | 50 |:+: : : : : : : : : : : : : : : : : : | 40 |:+ : : : : : : : : : : : : : : : : : : | |: : : : : : : : : : : : : : : : : : :O O O O O | 30 |:+ : : : : : : : : : : : : : : : : : : | 20 |-+ :: : : : : : :: : : :: : : O : | | : : : : : : : : : : : : : | 10 |-+ : : : : : : : : : : : : : | 0 +---------------------------------------------------------------------+
phoronix-test-suite.glmark2.1024x768.score
70 +----------------------------------------------------------------------+ | + + + +..+ + + + + + +.+ | 60 |-: : : : : : : : : : : | | : : : : : : : : : : : | 50 |-:: : :: : : :: : : :: :: : | |: : : : : : : : : : : : : : : : : : : | 40 |:+: : : : : : : : : : : : : : : : : : | |: : : : : : : : : : : : : : : : : : : O | 30 |:+ : : : : : : : : : : : : : : : : : :O O O O | |: : : : : : : : : : : : : : : : : : : | 20 |:+ : : : : : : : : : : : : : : : : : : | | :: : : : : : :: : : :: : : O :: | 10 |-+ : : : : : : : : : : : : : | | : : : O : O : : O : : : O : : : : | 0 +----------------------------------------------------------------------+
[*] bisect-good sample [O] bisect-bad sample
Hmm I must be going blind - there isn't even a single * in either of the graphs. Or perhaps my eyesight is fine and the legend or the graphs need fixing.
I cannot make sense of these graphs. The axis' should have descriptive labels.
Best regards Thomas
HTH -Emil
Setting the plane registers first and the mode registers afterwards reproduces the sequence used by atomic helpers. Done in preparation of switching to simple KMS helpers.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 2007d7a4754ac..4dba0a379c263 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1145,8 +1145,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_CRT(15, 0); WREG_CRT(19, pitch & 0xFF);
- mgag200_set_mode_regs(mdev, mode); - ext_vga[0] = 0;
/* TODO interlace */ @@ -1182,6 +1180,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
mga_crtc_do_set_base(mdev, fb, old_fb);
+ mgag200_set_mode_regs(mdev, mode); + /* reset tagfifo */ if (mdev->type == G200_ER) { u32 mem_ctl = RREG32(MGAREG_MEMCTL);
The framebuffer's pitch is now set in mgag200_set_offset().
v2: * move offset and bpp-shift calculation into helper functions * use u8 instead of uint8_t * add MGAREG_CRTCEXT0_OFFSET_MASK
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 57 +++++++++++++++++++------- drivers/gpu/drm/mgag200/mgag200_reg.h | 2 + 2 files changed, 45 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 4dba0a379c263..dee7838d7d368 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1002,6 +1002,48 @@ static void mgag200_set_mode_regs(struct mga_device *mdev, mga_crtc_set_plls(mdev, mode->clock); }
+static u8 mgag200_get_bpp_shift(struct mga_device *mdev, + const struct drm_format_info *format) +{ + return mdev->bpp_shifts[format->cpp[0] - 1]; +} + +/* + * Calculates the HW offset value from the framebuffer's pitch. The + * offset is a multiple of the pixel size and depends on the display + * format. + */ +static u32 mgag200_calculate_offset(struct mga_device *mdev, + const struct drm_framebuffer *fb) +{ + u32 offset = fb->pitches[0] / fb->format->cpp[0]; + u8 bppshift = mgag200_get_bpp_shift(mdev, fb->format); + + if (fb->format->cpp[0] * 8 == 24) + offset = (offset * 3) >> (4 - bppshift); + else + offset = offset >> (4 - bppshift); + + return offset; +} + +static void mgag200_set_offset(struct mga_device *mdev, + const struct drm_framebuffer *fb) +{ + u8 crtc13, crtcext0; + u32 offset = mgag200_calculate_offset(mdev, fb); + + RREG_ECRT(0, crtcext0); + + crtc13 = offset & 0xff; + + crtcext0 &= ~MGAREG_CRTCEXT0_OFFSET_MASK; + crtcext0 |= (offset >> 4) & MGAREG_CRTCEXT0_OFFSET_MASK; + + WREG_CRT(0x13, crtc13); + WREG_ECRT(0x00, crtcext0); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -1010,7 +1052,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); const struct drm_framebuffer *fb = crtc->primary->fb; - int pitch; int option = 0, option2 = 0; int i; unsigned char misc = 0; @@ -1121,12 +1162,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_SEQ(3, 0); WREG_SEQ(4, 0xe);
- pitch = fb->pitches[0] / fb->format->cpp[0]; - if (fb->format->cpp[0] * 8 == 24) - pitch = (pitch * 3) >> (4 - bppshift); - else - pitch = pitch >> (4 - bppshift); - WREG_GFX(0, 0); WREG_GFX(1, 0); WREG_GFX(2, 0); @@ -1143,20 +1178,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_CRT(13, 0); WREG_CRT(14, 0); WREG_CRT(15, 0); - WREG_CRT(19, pitch & 0xFF); - - ext_vga[0] = 0;
/* TODO interlace */
- ext_vga[0] |= (pitch & 0x300) >> 4; if (fb->format->cpp[0] * 8 == 24) ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80; else ext_vga[3] = ((1 << bppshift) - 1) | 0x80; ext_vga[4] = 0;
- WREG_ECRT(0, ext_vga[0]); WREG_ECRT(3, ext_vga[3]); WREG_ECRT(4, ext_vga[4]);
@@ -1170,8 +1200,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_ECRT(6, 0); }
- WREG_ECRT(0, ext_vga[0]); - misc = RREG8(MGA_MISC_IN); misc |= MGAREG_MISC_IOADSEL | MGAREG_MISC_RAMMAPEN | @@ -1179,6 +1207,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG8(MGA_MISC_OUT, misc);
mga_crtc_do_set_base(mdev, fb, old_fb); + mgag200_set_offset(mdev, fb);
mgag200_set_mode_regs(mdev, mode);
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index 0ba6e15e97106..cd08dee29b06f 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -240,6 +240,8 @@ #define MGAREG_CRTCEXT_INDEX 0x1fde #define MGAREG_CRTCEXT_DATA 0x1fdf
+#define MGAREG_CRTCEXT0_OFFSET_MASK GENMASK(5, 4) + /* Cursor X and Y position */ #define MGA_CURPOSXL 0x3c0c #define MGA_CURPOSXH 0x3c0d
The primary plane's format registers are now updated in a mgag200_set_format_regs().
v2: * get bpp shift from helper function * replace uint8_t with u8
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 109 ++++++++++++++++--------- 1 file changed, 69 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index dee7838d7d368..38556f57ad218 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1044,6 +1044,68 @@ static void mgag200_set_offset(struct mga_device *mdev, WREG_ECRT(0x00, crtcext0); }
+static void mgag200_set_format_regs(struct mga_device *mdev, + const struct drm_framebuffer *fb) +{ + struct drm_device *dev = mdev->dev; + const struct drm_format_info *format = fb->format; + unsigned int bpp, bppshift, scale; + u8 crtcext3, xmulctrl; + + bpp = format->cpp[0] * 8; + + bppshift = mgag200_get_bpp_shift(mdev, format); + switch (bpp) { + case 24: + scale = ((1 << bppshift) * 3) - 1; + break; + default: + scale = (1 << bppshift) - 1; + break; + } + + RREG_ECRT(3, crtcext3); + + switch (bpp) { + case 8: + xmulctrl = MGA1064_MUL_CTL_8bits; + break; + case 16: + if (format->depth == 15) + xmulctrl = MGA1064_MUL_CTL_15bits; + else + xmulctrl = MGA1064_MUL_CTL_16bits; + break; + case 24: + xmulctrl = MGA1064_MUL_CTL_24bits; + break; + case 32: + xmulctrl = MGA1064_MUL_CTL_32_24bits; + break; + default: + /* BUG: We should have caught this problem already. */ + drm_WARN_ON(dev, "invalid format depth\n"); + return; + } + + crtcext3 &= ~GENMASK(2, 0); + crtcext3 |= scale; + + WREG_DAC(MGA1064_MUL_CTL, xmulctrl); + + WREG_GFX(0, 0x00); + WREG_GFX(1, 0x00); + WREG_GFX(2, 0x00); + WREG_GFX(3, 0x00); + WREG_GFX(4, 0x00); + WREG_GFX(5, 0x40); + WREG_GFX(6, 0x05); + WREG_GFX(7, 0x0f); + WREG_GFX(8, 0x0f); + + WREG_ECRT(3, crtcext3); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -1055,8 +1117,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, int option = 0, option2 = 0; int i; unsigned char misc = 0; - unsigned char ext_vga[6]; - u8 bppshift; + u8 crtcext3, crtcext4;
static unsigned char dacvalue[] = { /* 0x00: */ 0, 0, 0, 0, 0, 0, 0x00, 0, @@ -1071,8 +1132,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0 };
- bppshift = mdev->bpp_shifts[fb->format->cpp[0] - 1]; - switch (mdev->type) { case G200_SE_A: case G200_SE_B: @@ -1111,24 +1170,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, break; }
- switch (fb->format->cpp[0] * 8) { - case 8: - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_8bits; - break; - case 16: - if (fb->format->depth == 15) - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_15bits; - else - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_16bits; - break; - case 24: - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_24bits; - break; - case 32: - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_32_24bits; - break; - } - for (i = 0; i < sizeof(dacvalue); i++) { if ((i <= 0x17) || (i == 0x1b) || @@ -1162,16 +1203,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_SEQ(3, 0); WREG_SEQ(4, 0xe);
- WREG_GFX(0, 0); - WREG_GFX(1, 0); - WREG_GFX(2, 0); - WREG_GFX(3, 0); - WREG_GFX(4, 0); - WREG_GFX(5, 0x40); - WREG_GFX(6, 0x5); - WREG_GFX(7, 0xf); - WREG_GFX(8, 0xf); - WREG_CRT(10, 0); WREG_CRT(11, 0); WREG_CRT(12, 0); @@ -1179,16 +1210,13 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_CRT(14, 0); WREG_CRT(15, 0);
- /* TODO interlace */ + RREG_ECRT(0x03, crtcext3);
- if (fb->format->cpp[0] * 8 == 24) - ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80; - else - ext_vga[3] = ((1 << bppshift) - 1) | 0x80; - ext_vga[4] = 0; + crtcext3 |= BIT(7); /* enable MGA mode */ + crtcext4 = 0x00;
- WREG_ECRT(3, ext_vga[3]); - WREG_ECRT(4, ext_vga[4]); + WREG_ECRT(0x03, crtcext3); + WREG_ECRT(0x04, crtcext4);
if (mdev->type == G200_ER) WREG_ECRT(0x24, 0x5); @@ -1206,6 +1234,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, MGAREG_MISC_HIGH_PG_SEL; WREG8(MGA_MISC_OUT, misc);
+ mgag200_set_format_regs(mdev, fb); mga_crtc_do_set_base(mdev, fb, old_fb); mgag200_set_offset(mdev, fb);
The TAGFIFO state is now reset in mgag200_g200er_reset_tagfifo().
v2: * define MGAREG_SEQ1_SCROFF
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_drv.h | 6 ++++ drivers/gpu/drm/mgag200/mgag200_mode.c | 45 +++++++++++++++++--------- drivers/gpu/drm/mgag200/mgag200_reg.h | 3 ++ 3 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 1963876ef3b8c..cf71a4ec84158 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -49,6 +49,12 @@ WREG8(ATTR_DATA, v); \ } while (0) \
+#define RREG_SEQ(reg, v) \ + do { \ + WREG8(MGAREG_SEQ_INDEX, reg); \ + v = RREG8(MGAREG_SEQ_DATA); \ + } while (0) \ + #define WREG_SEQ(reg, v) \ do { \ WREG8(MGAREG_SEQ_INDEX, reg); \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 38556f57ad218..68ae604926757 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1106,6 +1106,33 @@ static void mgag200_set_format_regs(struct mga_device *mdev, WREG_ECRT(3, crtcext3); }
+static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev) +{ + static uint32_t RESET_FLAG = 0x00200000; /* undocumented magic value */ + u8 seq1; + u32 memctl; + + /* screen off */ + RREG_SEQ(0x01, seq1); + seq1 |= MGAREG_SEQ1_SCROFF; + WREG_SEQ(0x01, seq1); + + memctl = RREG32(MGAREG_MEMCTL); + + memctl |= RESET_FLAG; + WREG32(MGAREG_MEMCTL, memctl); + + udelay(1000); + + memctl &= ~RESET_FLAG; + WREG32(MGAREG_MEMCTL, memctl); + + /* screen on */ + RREG_SEQ(0x01, seq1); + seq1 &= ~MGAREG_SEQ1_SCROFF; + WREG_SEQ(0x01, seq1); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -1240,22 +1267,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
mgag200_set_mode_regs(mdev, mode);
- /* reset tagfifo */ - if (mdev->type == G200_ER) { - u32 mem_ctl = RREG32(MGAREG_MEMCTL); - u8 seq1; - - /* screen off */ - WREG8(MGAREG_SEQ_INDEX, 0x01); - seq1 = RREG8(MGAREG_SEQ_DATA) | 0x20; - WREG8(MGAREG_SEQ_DATA, seq1); - - WREG32(MGAREG_MEMCTL, mem_ctl | 0x00200000); - udelay(1000); - WREG32(MGAREG_MEMCTL, mem_ctl & ~0x00200000); - - WREG8(MGAREG_SEQ_DATA, seq1 & ~0x20); - } + if (mdev->type == G200_ER) + mgag200_g200er_reset_tagfifo(mdev);
if (IS_G200_SE(mdev)) { diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index cd08dee29b06f..29f7194faadc0 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -235,6 +235,9 @@ /* MMIO VGA registers */ #define MGAREG_SEQ_INDEX 0x1fc4 #define MGAREG_SEQ_DATA 0x1fc5 + +#define MGAREG_SEQ1_SCROFF BIT(5) + #define MGAREG_CRTC_INDEX 0x1fd4 #define MGAREG_CRTC_DATA 0x1fd5 #define MGAREG_CRTCEXT_INDEX 0x1fde
The hiprivlvl settings are now updated in mgag200_g200se_set_hiprilvl() and mgag200_g200ev_set_hiprilvl().
v2: * replace uint8_t with u8
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 98 ++++++++++++++------------ 1 file changed, 54 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 68ae604926757..46122fa319889 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1133,6 +1133,56 @@ static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev) WREG_SEQ(0x01, seq1); }
+static void mgag200_g200se_set_hiprilvl(struct mga_device *mdev, + const struct drm_display_mode *mode, + const struct drm_framebuffer *fb) +{ + unsigned int hiprilvl; + u8 crtcext6; + + if (mdev->unique_rev_id >= 0x04) { + hiprilvl = 0; + } else if (mdev->unique_rev_id >= 0x02) { + unsigned int bpp; + unsigned long mb; + + if (fb->format->cpp[0] * 8 > 16) + bpp = 32; + else if (fb->format->cpp[0] * 8 > 8) + bpp = 16; + else + bpp = 8; + + mb = (mode->clock * bpp) / 1000; + if (mb > 3100) + hiprilvl = 0; + else if (mb > 2600) + hiprilvl = 1; + else if (mb > 1900) + hiprilvl = 2; + else if (mb > 1160) + hiprilvl = 3; + else if (mb > 440) + hiprilvl = 4; + else + hiprilvl = 5; + + } else if (mdev->unique_rev_id >= 0x01) { + hiprilvl = 3; + } else { + hiprilvl = 4; + } + + crtcext6 = hiprilvl; /* implicitly sets maxhipri to 0 */ + + WREG_ECRT(0x06, crtcext6); +} + +static void mgag200_g200ev_set_hiprilvl(struct mga_device *mdev) +{ + WREG_ECRT(0x06, 0x00); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -1251,10 +1301,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, if (mdev->type == G200_EW3) WREG_ECRT(0x34, 0x5);
- if (mdev->type == G200_EV) { - WREG_ECRT(6, 0); - } - misc = RREG8(MGA_MISC_IN); misc |= MGAREG_MISC_IOADSEL | MGAREG_MISC_RAMMAPEN | @@ -1270,47 +1316,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, if (mdev->type == G200_ER) mgag200_g200er_reset_tagfifo(mdev);
+ if (IS_G200_SE(mdev)) + mgag200_g200se_set_hiprilvl(mdev, mode, fb); + else if (mdev->type == G200_EV) + mgag200_g200ev_set_hiprilvl(mdev);
- if (IS_G200_SE(mdev)) { - if (mdev->unique_rev_id >= 0x04) { - WREG8(MGAREG_CRTCEXT_INDEX, 0x06); - WREG8(MGAREG_CRTCEXT_DATA, 0); - } else if (mdev->unique_rev_id >= 0x02) { - u8 hi_pri_lvl; - u32 bpp; - u32 mb; - - if (fb->format->cpp[0] * 8 > 16) - bpp = 32; - else if (fb->format->cpp[0] * 8 > 8) - bpp = 16; - else - bpp = 8; - - mb = (mode->clock * bpp) / 1000; - if (mb > 3100) - hi_pri_lvl = 0; - else if (mb > 2600) - hi_pri_lvl = 1; - else if (mb > 1900) - hi_pri_lvl = 2; - else if (mb > 1160) - hi_pri_lvl = 3; - else if (mb > 440) - hi_pri_lvl = 4; - else - hi_pri_lvl = 5; - - WREG8(MGAREG_CRTCEXT_INDEX, 0x06); - WREG8(MGAREG_CRTCEXT_DATA, hi_pri_lvl); - } else { - WREG8(MGAREG_CRTCEXT_INDEX, 0x06); - if (mdev->unique_rev_id >= 0x01) - WREG8(MGAREG_CRTCEXT_DATA, 0x03); - else - WREG8(MGAREG_CRTCEXT_DATA, 0x04); - } - } return 0; }
Registers are initialized with constants. This is now done in mgag200_init_regs(), mgag200_set_dac_regs() and mgag200_set_pci_regs(). Later patches should move these calls from mode setting to device initialization.
v2: * replace uint8_t with u8
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 261 ++++++++++++++----------- 1 file changed, 147 insertions(+), 114 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 46122fa319889..199ae08976e16 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -919,6 +919,152 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, return mga_crtc_do_set_base(mdev, fb, old_fb); }
+static void mgag200_set_pci_regs(struct mga_device *mdev) +{ + uint32_t option = 0, option2 = 0; + struct drm_device *dev = mdev->dev; + + switch (mdev->type) { + case G200_SE_A: + case G200_SE_B: + if (mdev->has_sdram) + option = 0x40049120; + else + option = 0x4004d120; + option2 = 0x00008000; + break; + case G200_WB: + case G200_EW3: + option = 0x41049120; + option2 = 0x0000b000; + break; + case G200_EV: + option = 0x00000120; + option2 = 0x0000b000; + break; + case G200_EH: + case G200_EH3: + option = 0x00000120; + option2 = 0x0000b000; + break; + case G200_ER: + break; + } + + if (option) + pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option); + + if (option2) + pci_write_config_dword(dev->pdev, PCI_MGA_OPTION2, option2); +} + +static void mgag200_set_dac_regs(struct mga_device *mdev) +{ + size_t i; + u8 dacvalue[] = { + /* 0x00: */ 0, 0, 0, 0, 0, 0, 0x00, 0, + /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0, + /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0, + /* 0x18: */ 0x00, 0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20, + /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40, + /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83, + /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A, + /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0, + /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0 + }; + + switch (mdev->type) { + case G200_SE_A: + case G200_SE_B: + dacvalue[MGA1064_VREF_CTL] = 0x03; + dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL; + dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_DAC_EN | + MGA1064_MISC_CTL_VGA8 | + MGA1064_MISC_CTL_DAC_RAM_CS; + break; + case G200_WB: + case G200_EW3: + dacvalue[MGA1064_VREF_CTL] = 0x07; + break; + case G200_EV: + dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL; + dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 | + MGA1064_MISC_CTL_DAC_RAM_CS; + break; + case G200_EH: + case G200_EH3: + dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 | + MGA1064_MISC_CTL_DAC_RAM_CS; + break; + case G200_ER: + break; + } + + for (i = 0; i < ARRAY_SIZE(dacvalue); i++) { + if ((i <= 0x17) || + (i == 0x1b) || + (i == 0x1c) || + ((i >= 0x1f) && (i <= 0x29)) || + ((i >= 0x30) && (i <= 0x37))) + continue; + if (IS_G200_SE(mdev) && + ((i == 0x2c) || (i == 0x2d) || (i == 0x2e))) + continue; + if ((mdev->type == G200_EV || + mdev->type == G200_WB || + mdev->type == G200_EH || + mdev->type == G200_EW3 || + mdev->type == G200_EH3) && + (i >= 0x44) && (i <= 0x4e)) + continue; + + WREG_DAC(i, dacvalue[i]); + } + + if (mdev->type == G200_ER) + WREG_DAC(0x90, 0); +} + +static void mgag200_init_regs(struct mga_device *mdev) +{ + u8 crtcext3, crtcext4, misc; + + mgag200_set_pci_regs(mdev); + mgag200_set_dac_regs(mdev); + + WREG_SEQ(2, 0x0f); + WREG_SEQ(3, 0x00); + WREG_SEQ(4, 0x0e); + + WREG_CRT(10, 0); + WREG_CRT(11, 0); + WREG_CRT(12, 0); + WREG_CRT(13, 0); + WREG_CRT(14, 0); + WREG_CRT(15, 0); + + RREG_ECRT(0x03, crtcext3); + + crtcext3 |= BIT(7); /* enable MGA mode */ + crtcext4 = 0x00; + + WREG_ECRT(0x03, crtcext3); + WREG_ECRT(0x04, crtcext4); + + if (mdev->type == G200_ER) + WREG_ECRT(0x24, 0x5); + + if (mdev->type == G200_EW3) + WREG_ECRT(0x34, 0x5); + + misc = RREG8(MGA_MISC_IN); + misc |= MGAREG_MISC_IOADSEL | + MGAREG_MISC_RAMMAPEN | + MGAREG_MISC_HIGH_PG_SEL; + WREG8(MGA_MISC_OUT, misc); +} + static void mgag200_set_mode_regs(struct mga_device *mdev, const struct drm_display_mode *mode) { @@ -1191,121 +1337,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); const struct drm_framebuffer *fb = crtc->primary->fb; - int option = 0, option2 = 0; - int i; - unsigned char misc = 0; - u8 crtcext3, crtcext4; - - static unsigned char dacvalue[] = { - /* 0x00: */ 0, 0, 0, 0, 0, 0, 0x00, 0, - /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0, - /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0, - /* 0x18: */ 0x00, 0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20, - /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40, - /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83, - /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A, - /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0, - /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0 - };
- switch (mdev->type) { - case G200_SE_A: - case G200_SE_B: - dacvalue[MGA1064_VREF_CTL] = 0x03; - dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL; - dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_DAC_EN | - MGA1064_MISC_CTL_VGA8 | - MGA1064_MISC_CTL_DAC_RAM_CS; - if (mdev->has_sdram) - option = 0x40049120; - else - option = 0x4004d120; - option2 = 0x00008000; - break; - case G200_WB: - case G200_EW3: - dacvalue[MGA1064_VREF_CTL] = 0x07; - option = 0x41049120; - option2 = 0x0000b000; - break; - case G200_EV: - dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL; - dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 | - MGA1064_MISC_CTL_DAC_RAM_CS; - option = 0x00000120; - option2 = 0x0000b000; - break; - case G200_EH: - case G200_EH3: - dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 | - MGA1064_MISC_CTL_DAC_RAM_CS; - option = 0x00000120; - option2 = 0x0000b000; - break; - case G200_ER: - break; - } - - for (i = 0; i < sizeof(dacvalue); i++) { - if ((i <= 0x17) || - (i == 0x1b) || - (i == 0x1c) || - ((i >= 0x1f) && (i <= 0x29)) || - ((i >= 0x30) && (i <= 0x37))) - continue; - if (IS_G200_SE(mdev) && - ((i == 0x2c) || (i == 0x2d) || (i == 0x2e))) - continue; - if ((mdev->type == G200_EV || - mdev->type == G200_WB || - mdev->type == G200_EH || - mdev->type == G200_EW3 || - mdev->type == G200_EH3) && - (i >= 0x44) && (i <= 0x4e)) - continue; - - WREG_DAC(i, dacvalue[i]); - } - - if (mdev->type == G200_ER) - WREG_DAC(0x90, 0); - - if (option) - pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option); - if (option2) - pci_write_config_dword(dev->pdev, PCI_MGA_OPTION2, option2); - - WREG_SEQ(2, 0xf); - WREG_SEQ(3, 0); - WREG_SEQ(4, 0xe); - - WREG_CRT(10, 0); - WREG_CRT(11, 0); - WREG_CRT(12, 0); - WREG_CRT(13, 0); - WREG_CRT(14, 0); - WREG_CRT(15, 0); - - RREG_ECRT(0x03, crtcext3); - - crtcext3 |= BIT(7); /* enable MGA mode */ - crtcext4 = 0x00; - - WREG_ECRT(0x03, crtcext3); - WREG_ECRT(0x04, crtcext4); - - if (mdev->type == G200_ER) - WREG_ECRT(0x24, 0x5); - - if (mdev->type == G200_EW3) - WREG_ECRT(0x34, 0x5); - - misc = RREG8(MGA_MISC_IN); - misc |= MGAREG_MISC_IOADSEL | - MGAREG_MISC_RAMMAPEN | - MGAREG_MISC_HIGH_PG_SEL; - WREG8(MGA_MISC_OUT, misc); + mgag200_init_regs(mdev);
mgag200_set_format_regs(mdev, fb); mga_crtc_do_set_base(mdev, fb, old_fb);
The suspend/resume helpers are unused. Also remove associated state from struct mga_device.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_drv.h | 1 - drivers/gpu/drm/mgag200/mgag200_mode.c | 71 -------------------------- 2 files changed, 72 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index cf71a4ec84158..0cf498d1e900c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -164,7 +164,6 @@ struct mga_device {
size_t vram_fb_available;
- bool suspended; enum mga_type type; int has_sdram;
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 199ae08976e16..d6f9763a4a450 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1357,65 +1357,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, return 0; }
-#if 0 /* code from mjg to attempt D3 on crtc dpms off - revisit later */ -static int mga_suspend(struct drm_crtc *crtc) -{ - struct mga_crtc *mga_crtc = to_mga_crtc(crtc); - struct drm_device *dev = crtc->dev; - struct mga_device *mdev = dev->dev_private; - struct pci_dev *pdev = dev->pdev; - int option; - - if (mdev->suspended) - return 0; - - WREG_SEQ(1, 0x20); - WREG_ECRT(1, 0x30); - /* Disable the pixel clock */ - WREG_DAC(0x1a, 0x05); - /* Power down the DAC */ - WREG_DAC(0x1e, 0x18); - /* Power down the pixel PLL */ - WREG_DAC(0x1a, 0x0d); - - /* Disable PLLs and clocks */ - pci_read_config_dword(pdev, PCI_MGA_OPTION, &option); - option &= ~(0x1F8024); - pci_write_config_dword(pdev, PCI_MGA_OPTION, option); - pci_set_power_state(pdev, PCI_D3hot); - pci_disable_device(pdev); - - mdev->suspended = true; - - return 0; -} - -static int mga_resume(struct drm_crtc *crtc) -{ - struct mga_crtc *mga_crtc = to_mga_crtc(crtc); - struct drm_device *dev = crtc->dev; - struct mga_device *mdev = dev->dev_private; - struct pci_dev *pdev = dev->pdev; - int option; - - if (!mdev->suspended) - return 0; - - pci_set_power_state(pdev, PCI_D0); - pci_enable_device(pdev); - - /* Disable sysclk */ - pci_read_config_dword(pdev, PCI_MGA_OPTION, &option); - option &= ~(0x4); - pci_write_config_dword(pdev, PCI_MGA_OPTION, option); - - mdev->suspended = false; - - return 0; -} - -#endif - static void mga_crtc_dpms(struct drm_crtc *crtc, int mode) { struct drm_device *dev = crtc->dev; @@ -1442,11 +1383,6 @@ static void mga_crtc_dpms(struct drm_crtc *crtc, int mode) break; }
-#if 0 - if (mode == DRM_MODE_DPMS_OFF) { - mga_suspend(crtc); - } -#endif WREG8(MGAREG_SEQ_INDEX, 0x01); seq1 |= RREG8(MGAREG_SEQ_DATA) & ~0x20; mga_wait_vsync(mdev); @@ -1456,13 +1392,6 @@ static void mga_crtc_dpms(struct drm_crtc *crtc, int mode) WREG8(MGAREG_CRTCEXT_INDEX, 0x01); crtcext1 |= RREG8(MGAREG_CRTCEXT_DATA) & ~0x30; WREG8(MGAREG_CRTCEXT_DATA, crtcext1); - -#if 0 - if (mode == DRM_MODE_DPMS_ON && mdev->suspended == true) { - mga_resume(crtc); - drm_helper_resume_force_mode(dev); - } -#endif }
/*
The MGA CRTC data structure struct mga_crtc contains unused additional fields; so it can removed. The standard DRM CRTC and encoder structures are embedded now in struct drm_simple_display_pipe. Done in preparation of converting mgag200 to simple KMS helpers.
v3: * remove now-unused define MGAG200FB_CONN_LIMIT * remove unused define MATROX_DPMS_CLEARED
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_drv.h | 15 ++------------ drivers/gpu/drm/mgag200/mgag200_mode.c | 28 ++++++-------------------- 2 files changed, 8 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 0cf498d1e900c..d929ca3a767a3 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -19,6 +19,7 @@ #include <drm/drm_fb_helper.h> #include <drm/drm_gem.h> #include <drm/drm_gem_vram_helper.h> +#include <drm/drm_simple_kms_helper.h>
#include "mgag200_reg.h"
@@ -32,8 +33,6 @@ #define DRIVER_MINOR 0 #define DRIVER_PATCHLEVEL 0
-#define MGAG200FB_CONN_LIMIT 1 - #define RREG8(reg) ioread8(((void __iomem *)mdev->rmmio) + (reg)) #define WREG8(reg, v) iowrite8(v, ((void __iomem *)mdev->rmmio) + (reg)) #define RREG32(reg) ioread32(((void __iomem *)mdev->rmmio) + (reg)) @@ -103,18 +102,8 @@ #define MGAG200_MAX_FB_HEIGHT 4096 #define MGAG200_MAX_FB_WIDTH 4096
-#define MATROX_DPMS_CLEARED (-1) - -#define to_mga_crtc(x) container_of(x, struct mga_crtc, base) #define to_mga_connector(x) container_of(x, struct mga_connector, base)
-struct mga_crtc { - struct drm_crtc base; - u8 lut_r[256], lut_g[256], lut_b[256]; - int last_dpms; - bool enabled; -}; - struct mga_i2c_chan { struct i2c_adapter adapter; struct drm_device *dev; @@ -175,7 +164,7 @@ struct mga_device { u32 unique_rev_id;
struct mga_connector connector; - struct drm_encoder encoder; + struct drm_simple_display_pipe display_pipe; };
static inline struct mga_device *to_mga_device(struct drm_device *dev) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index d6f9763a4a450..00bbc1f9b7db3 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1475,15 +1475,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, return 0; }
-/* Simple cleanup function */ -static void mga_crtc_destroy(struct drm_crtc *crtc) -{ - struct mga_crtc *mga_crtc = to_mga_crtc(crtc); - - drm_crtc_cleanup(crtc); - kfree(mga_crtc); -} - static void mga_crtc_disable(struct drm_crtc *crtc) { DRM_DEBUG_KMS("\n"); @@ -1501,7 +1492,7 @@ static void mga_crtc_disable(struct drm_crtc *crtc) static const struct drm_crtc_funcs mga_crtc_funcs = { .gamma_set = mga_crtc_gamma_set, .set_config = drm_crtc_helper_set_config, - .destroy = mga_crtc_destroy, + .destroy = drm_crtc_cleanup, };
static const struct drm_crtc_helper_funcs mga_helper_funcs = { @@ -1517,20 +1508,13 @@ static const struct drm_crtc_helper_funcs mga_helper_funcs = { static void mga_crtc_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev; - struct mga_crtc *mga_crtc; - - mga_crtc = kzalloc(sizeof(struct mga_crtc) + - (MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)), - GFP_KERNEL); - - if (mga_crtc == NULL) - return; + struct drm_crtc *crtc = &mdev->display_pipe.crtc;
- drm_crtc_init(dev, &mga_crtc->base, &mga_crtc_funcs); + drm_crtc_init(dev, crtc, &mga_crtc_funcs);
- drm_mode_crtc_set_gamma_size(&mga_crtc->base, MGAG200_LUT_SIZE); + drm_mode_crtc_set_gamma_size(crtc, MGAG200_LUT_SIZE);
- drm_crtc_helper_add(&mga_crtc->base, &mga_helper_funcs); + drm_crtc_helper_add(crtc, &mga_helper_funcs); }
/* @@ -1718,7 +1702,7 @@ static unsigned int mgag200_preferred_depth(struct mga_device *mdev) int mgag200_modeset_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev; - struct drm_encoder *encoder = &mdev->encoder; + struct drm_encoder *encoder = &mdev->display_pipe.encoder; struct drm_connector *connector = &mdev->connector.base; int ret;
The mgag200 supports a single pipeline with only a primary plane. It can be converted to simple KMS helpers. This also adds support for atomic modesetting. Wayland compositors, which use pageflip ioctls, can now be used with mgag200.
v2: * prepare encoder and CRTC in a separate patch * remove suspend/resume code in a separate patch * don't call set_format_regs() in pipe_update()
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 314 +++++++++++++------------ 2 files changed, 164 insertions(+), 152 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index c2f0e4b40b052..a06ce4198adea 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -140,7 +140,7 @@ int mgag200_driver_dumb_create(struct drm_file *file, }
static struct drm_driver driver = { - .driver_features = DRIVER_GEM | DRIVER_MODESET, + .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, .fops = &mgag200_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 00bbc1f9b7db3..b50c1beb7b7b9 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -11,10 +11,13 @@ #include <linux/delay.h> #include <linux/pci.h>
+#include <drm/drm_atomic_helper.h> +#include <drm/drm_atomic_state_helper.h> #include <drm/drm_crtc_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_plane_helper.h> +#include <drm/drm_print.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h>
@@ -30,13 +33,18 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); - struct drm_framebuffer *fb = crtc->primary->fb; + struct drm_framebuffer *fb; u16 *r_ptr, *g_ptr, *b_ptr; int i;
if (!crtc->enabled) return;
+ if (!mdev->display_pipe.plane.state) + return; + + fb = mdev->display_pipe.plane.state->fb; + r_ptr = crtc->gamma_store; g_ptr = r_ptr + crtc->gamma_size; b_ptr = g_ptr + crtc->gamma_size; @@ -869,56 +877,6 @@ static void mgag200_set_startadd(struct mga_device *mdev, WREG_ECRT(0x00, crtcext0); }
-static int mga_crtc_do_set_base(struct mga_device *mdev, - const struct drm_framebuffer *fb, - const struct drm_framebuffer *old_fb) -{ - struct drm_gem_vram_object *gbo; - int ret; - s64 gpu_addr; - - if (old_fb) { - gbo = drm_gem_vram_of_gem(old_fb->obj[0]); - drm_gem_vram_unpin(gbo); - } - - gbo = drm_gem_vram_of_gem(fb->obj[0]); - - ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM); - if (ret) - return ret; - gpu_addr = drm_gem_vram_offset(gbo); - if (gpu_addr < 0) { - ret = (int)gpu_addr; - goto err_drm_gem_vram_unpin; - } - - mgag200_set_startadd(mdev, (unsigned long)gpu_addr); - - return 0; - -err_drm_gem_vram_unpin: - drm_gem_vram_unpin(gbo); - return ret; -} - -static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, - struct drm_framebuffer *old_fb) -{ - struct drm_device *dev = crtc->dev; - struct mga_device *mdev = dev->dev_private; - struct drm_framebuffer *fb = crtc->primary->fb; - unsigned int count; - - do { } while (RREG8(0x1fda) & 0x08); - do { } while (!(RREG8(0x1fda) & 0x08)); - - count = RREG8(MGAREG_VCOUNT) + 2; - do { } while (RREG8(MGAREG_VCOUNT) < count); - - return mga_crtc_do_set_base(mdev, fb, old_fb); -} - static void mgag200_set_pci_regs(struct mga_device *mdev) { uint32_t option = 0, option2 = 0; @@ -1329,34 +1287,6 @@ static void mgag200_g200ev_set_hiprilvl(struct mga_device *mdev) WREG_ECRT(0x06, 0x00); }
-static int mga_crtc_mode_set(struct drm_crtc *crtc, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode, - int x, int y, struct drm_framebuffer *old_fb) -{ - struct drm_device *dev = crtc->dev; - struct mga_device *mdev = to_mga_device(dev); - const struct drm_framebuffer *fb = crtc->primary->fb; - - mgag200_init_regs(mdev); - - mgag200_set_format_regs(mdev, fb); - mga_crtc_do_set_base(mdev, fb, old_fb); - mgag200_set_offset(mdev, fb); - - mgag200_set_mode_regs(mdev, mode); - - if (mdev->type == G200_ER) - mgag200_g200er_reset_tagfifo(mdev); - - if (IS_G200_SE(mdev)) - mgag200_g200se_set_hiprilvl(mdev, mode, fb); - else if (mdev->type == G200_EV) - mgag200_g200ev_set_hiprilvl(mdev); - - return 0; -} - static void mga_crtc_dpms(struct drm_crtc *crtc, int mode) { struct drm_device *dev = crtc->dev; @@ -1439,7 +1369,6 @@ static void mga_crtc_commit(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); - const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; u8 tmp;
if (mdev->type == G200_WB || mdev->type == G200_EW3) @@ -1458,63 +1387,7 @@ static void mga_crtc_commit(struct drm_crtc *crtc) WREG_SEQ(0x1, tmp); WREG_SEQ(0, 3); } - crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON); -} - -/* - * The core can pass us a set of gamma values to program. We actually only - * use this for 8-bit mode so can't perform smooth fades on deeper modes, - * but it's a requirement that we provide the function - */ -static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, - u16 *blue, uint32_t size, - struct drm_modeset_acquire_ctx *ctx) -{ - mga_crtc_load_lut(crtc); - - return 0; -} - -static void mga_crtc_disable(struct drm_crtc *crtc) -{ - DRM_DEBUG_KMS("\n"); - mga_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); - if (crtc->primary->fb) { - struct drm_framebuffer *fb = crtc->primary->fb; - struct drm_gem_vram_object *gbo = - drm_gem_vram_of_gem(fb->obj[0]); - drm_gem_vram_unpin(gbo); - } - crtc->primary->fb = NULL; -} - -/* These provide the minimum set of functions required to handle a CRTC */ -static const struct drm_crtc_funcs mga_crtc_funcs = { - .gamma_set = mga_crtc_gamma_set, - .set_config = drm_crtc_helper_set_config, - .destroy = drm_crtc_cleanup, -}; - -static const struct drm_crtc_helper_funcs mga_helper_funcs = { - .disable = mga_crtc_disable, - .dpms = mga_crtc_dpms, - .mode_set = mga_crtc_mode_set, - .mode_set_base = mga_crtc_mode_set_base, - .prepare = mga_crtc_prepare, - .commit = mga_crtc_commit, -}; - -/* CRTC setup */ -static void mga_crtc_init(struct mga_device *mdev) -{ - struct drm_device *dev = mdev->dev; - struct drm_crtc *crtc = &mdev->display_pipe.crtc; - - drm_crtc_init(dev, crtc, &mga_crtc_funcs); - - drm_mode_crtc_set_gamma_size(crtc, MGAG200_LUT_SIZE); - - drm_crtc_helper_add(crtc, &mga_helper_funcs); + mga_crtc_dpms(crtc, DRM_MODE_DPMS_ON); }
/* @@ -1648,14 +1521,16 @@ static void mga_connector_destroy(struct drm_connector *connector) }
static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = { - .get_modes = mga_vga_get_modes, + .get_modes = mga_vga_get_modes, .mode_valid = mga_vga_mode_valid, };
static const struct drm_connector_funcs mga_vga_connector_funcs = { - .dpms = drm_helper_connector_dpms, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = mga_connector_destroy, + .reset = drm_atomic_helper_connector_reset, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = mga_connector_destroy, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, };
static int mgag200_vga_connector_init(struct mga_device *mdev) @@ -1687,8 +1562,137 @@ static int mgag200_vga_connector_init(struct mga_device *mdev) return ret; }
+/* + * Simple Display Pipe + */ + +static enum drm_mode_status +mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe *pipe, + const struct drm_display_mode *mode) +{ + return MODE_OK; +} + +static void +mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) +{ + struct drm_crtc *crtc = &pipe->crtc; + struct drm_device *dev = crtc->dev; + struct mga_device *mdev = to_mga_device(dev); + struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; + struct drm_framebuffer *fb = plane_state->fb; + struct drm_gem_vram_object *gbo; + s64 gpu_addr; + + gbo = drm_gem_vram_of_gem(fb->obj[0]); + + gpu_addr = drm_gem_vram_offset(gbo); + if (drm_WARN_ON_ONCE(dev, gpu_addr < 0)) + return; /* BUG: BO should have been pinned to VRAM. */ + + mga_crtc_prepare(crtc); + + mgag200_set_format_regs(mdev, fb); + mgag200_set_mode_regs(mdev, adjusted_mode); + + if (mdev->type == G200_ER) + mgag200_g200er_reset_tagfifo(mdev); + + if (IS_G200_SE(mdev)) + mgag200_g200se_set_hiprilvl(mdev, adjusted_mode, fb); + else if (mdev->type == G200_EV) + mgag200_g200ev_set_hiprilvl(mdev); + + mga_crtc_commit(crtc); +} + +static void +mgag200_simple_display_pipe_disable(struct drm_simple_display_pipe *pipe) +{ + struct drm_crtc *crtc = &pipe->crtc; + + mga_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); +} + +static int +mgag200_simple_display_pipe_check(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *plane_state, + struct drm_crtc_state *crtc_state) +{ + struct drm_plane *plane = plane_state->plane; + struct drm_framebuffer *new_fb = plane_state->fb; + struct drm_framebuffer *fb = NULL; + + if (!new_fb) + return 0; + + if (plane->state) + fb = plane->state->fb; + + if (!fb || (fb->format != new_fb->format)) + crtc_state->mode_changed = true; /* update PLL settings */ + + return 0; +} + +static void +mgag200_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *old_state) +{ + struct drm_plane *plane = &pipe->plane; + struct drm_device *dev = plane->dev; + struct mga_device *mdev = to_mga_device(dev); + struct drm_plane_state *state = plane->state; + struct drm_framebuffer *fb = state->fb; + struct drm_gem_vram_object *gbo; + s64 gpu_addr; + + if (!fb) + return; + + gbo = drm_gem_vram_of_gem(fb->obj[0]); + + gpu_addr = drm_gem_vram_offset(gbo); + if (drm_WARN_ON_ONCE(dev, gpu_addr < 0)) + return; /* BUG: BO should have been pinned to VRAM. */ + + mgag200_set_startadd(mdev, (unsigned long)gpu_addr); + mgag200_set_offset(mdev, fb); +} + +static const struct drm_simple_display_pipe_funcs +mgag200_simple_display_pipe_funcs = { + .mode_valid = mgag200_simple_display_pipe_mode_valid, + .enable = mgag200_simple_display_pipe_enable, + .disable = mgag200_simple_display_pipe_disable, + .check = mgag200_simple_display_pipe_check, + .update = mgag200_simple_display_pipe_update, + .prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb, + .cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb, +}; + +static const uint32_t mgag200_simple_display_pipe_formats[] = { + DRM_FORMAT_XRGB8888, + DRM_FORMAT_RGB565, + DRM_FORMAT_RGB888, +}; + +static const uint64_t mgag200_simple_display_pipe_fmtmods[] = { + DRM_FORMAT_MOD_LINEAR, + DRM_FORMAT_MOD_INVALID +}; + +/* + * Mode config + */ + static const struct drm_mode_config_funcs mgag200_mode_config_funcs = { - .fb_create = drm_gem_fb_create + .fb_create = drm_gem_fb_create, + .mode_valid = drm_vram_helper_mode_valid, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, };
static unsigned int mgag200_preferred_depth(struct mga_device *mdev) @@ -1702,8 +1706,9 @@ static unsigned int mgag200_preferred_depth(struct mga_device *mdev) int mgag200_modeset_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev; - struct drm_encoder *encoder = &mdev->display_pipe.encoder; struct drm_connector *connector = &mdev->connector.base; + struct drm_simple_display_pipe *pipe = &mdev->display_pipe; + size_t format_count = ARRAY_SIZE(mgag200_simple_display_pipe_formats); int ret;
mdev->bpp_shifts[0] = 0; @@ -1711,6 +1716,8 @@ int mgag200_modeset_init(struct mga_device *mdev) mdev->bpp_shifts[2] = 0; mdev->bpp_shifts[3] = 2;
+ mgag200_init_regs(mdev); + ret = drmm_mode_config_init(dev); if (ret) { drm_err(dev, "drmm_mode_config_init() failed, error %d\n", @@ -1728,26 +1735,31 @@ int mgag200_modeset_init(struct mga_device *mdev)
dev->mode_config.funcs = &mgag200_mode_config_funcs;
- mga_crtc_init(mdev); - - ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_DAC); + ret = mgag200_vga_connector_init(mdev); if (ret) { drm_err(dev, - "drm_simple_encoder_init() failed, error %d\n", + "mgag200_vga_connector_init() failed, error %d\n", ret); return ret; } - encoder->possible_crtcs = 0x1;
- ret = mgag200_vga_connector_init(mdev); + ret = drm_simple_display_pipe_init(dev, pipe, + &mgag200_simple_display_pipe_funcs, + mgag200_simple_display_pipe_formats, + format_count, + mgag200_simple_display_pipe_fmtmods, + connector); if (ret) { drm_err(dev, - "mgag200_vga_connector_init() failed, error %d\n", + "drm_simple_display_pipe_init() failed, error %d\n", ret); return ret; }
- drm_connector_attach_encoder(connector, encoder); + /* FIXME: legacy gamma tables; convert to CRTC state */ + drm_mode_crtc_set_gamma_size(&pipe->crtc, MGAG200_LUT_SIZE); + + drm_mode_config_reset(dev);
return 0; }
The VRAM helpers managed the framebuffer memory for mgag200. This came with several problems, as some MGA device require the scanout address to be located at VRAM offset 0. It's incompatible with the page-flip semantics of DRM's atomic modesettting. With atomic modesetting, old and new framebuffers have to be located in VRAM at the same time. So at least one of them has to reside at a non-0 offset.
This patch replaces VRAM helpers with SHMEM helpers. GEM SHMEM buffers reside in system memory, and are shadow-copied into VRAM during page flips. The shadow copy always starts at VRAM offset 0.
v2: * revert dev->pdev changes
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Emil Velikov emil.velikov@collabora.com --- drivers/gpu/drm/mgag200/Kconfig | 4 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 49 +--------------------- drivers/gpu/drm/mgag200/mgag200_drv.h | 5 ++- drivers/gpu/drm/mgag200/mgag200_mode.c | 58 ++++++++++++++++---------- drivers/gpu/drm/mgag200/mgag200_ttm.c | 28 +++++++------ 5 files changed, 56 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index d60aa4b9ccd47..93be766715c9b 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -2,10 +2,8 @@ config DRM_MGAG200 tristate "Kernel modesetting driver for MGA G200 server engines" depends on DRM && PCI && MMU + select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER - select DRM_VRAM_HELPER - select DRM_TTM - select DRM_TTM_HELPER help This is a KMS driver for the MGA G200 server chips, it does not support the original MGA G200 or any of the desktop diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index a06ce4198adea..00ddea7d7d270 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -22,15 +22,11 @@ * which then performs further device association and calls our graphics init * functions */ -int mgag200_modeset = -1;
+int mgag200_modeset = -1; MODULE_PARM_DESC(modeset, "Disable/Enable modesetting"); module_param_named(modeset, mgag200_modeset, int, 0400);
-int mgag200_hw_bug_no_startadd = -1; -MODULE_PARM_DESC(modeset, "HW does not interpret scanout-buffer start address correctly"); -module_param_named(hw_bug_no_startadd, mgag200_hw_bug_no_startadd, int, 0400); - static struct drm_driver driver;
static const struct pci_device_id pciidlist[] = { @@ -101,44 +97,6 @@ static void mga_pci_remove(struct pci_dev *pdev)
DEFINE_DRM_GEM_FOPS(mgag200_driver_fops);
-static bool mgag200_pin_bo_at_0(const struct mga_device *mdev) -{ - if (mgag200_hw_bug_no_startadd > 0) { - DRM_WARN_ONCE("Option hw_bug_no_startradd is enabled. Please " - "report the output of 'lspci -vvnn' to " - "dri-devel@lists.freedesktop.org if this " - "option is required to make mgag200 work " - "correctly on your system.\n"); - return true; - } else if (!mgag200_hw_bug_no_startadd) { - return false; - } - return mdev->flags & MGAG200_FLAG_HW_BUG_NO_STARTADD; -} - -int mgag200_driver_dumb_create(struct drm_file *file, - struct drm_device *dev, - struct drm_mode_create_dumb *args) -{ - struct mga_device *mdev = to_mga_device(dev); - unsigned long pg_align; - - if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized")) - return -EINVAL; - - pg_align = 0ul; - - /* - * Aligning scanout buffers to the size of the video ram forces - * placement at offset 0. Works around a bug where HW does not - * respect 'startadd' field. - */ - if (mgag200_pin_bo_at_0(mdev)) - pg_align = PFN_UP(mdev->mc.vram_size); - - return drm_gem_vram_fill_create_dumb(file, dev, pg_align, 0, args); -} - static struct drm_driver driver = { .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, .fops = &mgag200_driver_fops, @@ -148,10 +106,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, - .debugfs_init = drm_vram_mm_debugfs_init, - .dumb_create = mgag200_driver_dumb_create, - .dumb_map_offset = drm_gem_vram_driver_dumb_mmap_offset, - .gem_prime_mmap = drm_gem_prime_mmap, + DRM_GEM_SHMEM_DRIVER_OPS, };
static struct pci_driver mgag200_pci_driver = { diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index d929ca3a767a3..47df62b1ad290 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -18,7 +18,7 @@ #include <drm/drm_encoder.h> #include <drm/drm_fb_helper.h> #include <drm/drm_gem.h> -#include <drm/drm_gem_vram_helper.h> +#include <drm/drm_gem_shmem_helper.h> #include <drm/drm_simple_kms_helper.h>
#include "mgag200_reg.h" @@ -151,7 +151,8 @@ struct mga_device {
struct mga_mc mc;
- size_t vram_fb_available; + void __iomem *vram; + size_t vram_fb_available;
enum mga_type type; int has_sdram; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index b50c1beb7b7b9..0155d4eb5fa6b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -14,6 +14,8 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_atomic_state_helper.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_damage_helper.h> +#include <drm/drm_format_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_plane_helper.h> @@ -1573,6 +1575,26 @@ mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe *pipe, return MODE_OK; }
+static void +mgag200_handle_damage(struct mga_device *mdev, struct drm_framebuffer *fb, + struct drm_rect *clip) +{ + struct drm_device *dev = mdev->dev; + void *vmap; + + vmap = drm_gem_shmem_vmap(fb->obj[0]); + if (drm_WARN_ON(dev, !vmap)) + return; /* BUG: SHMEM BO should always be vmapped */ + + drm_fb_memcpy_dstclip(mdev->vram, vmap, fb, clip); + + drm_gem_shmem_vunmap(fb->obj[0], vmap); + + /* Always scanout image at VRAM offset 0 */ + mgag200_set_startadd(mdev, (u32)0); + mgag200_set_offset(mdev, fb); +} + static void mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_crtc_state *crtc_state, @@ -1583,14 +1605,12 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, struct mga_device *mdev = to_mga_device(dev); struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; struct drm_framebuffer *fb = plane_state->fb; - struct drm_gem_vram_object *gbo; - s64 gpu_addr; - - gbo = drm_gem_vram_of_gem(fb->obj[0]); - - gpu_addr = drm_gem_vram_offset(gbo); - if (drm_WARN_ON_ONCE(dev, gpu_addr < 0)) - return; /* BUG: BO should have been pinned to VRAM. */ + struct drm_rect fullscreen = { + .x1 = 0, + .x2 = fb->width, + .y1 = 0, + .y2 = fb->height, + };
mga_crtc_prepare(crtc);
@@ -1606,6 +1626,8 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, mgag200_g200ev_set_hiprilvl(mdev);
mga_crtc_commit(crtc); + + mgag200_handle_damage(mdev, fb, &fullscreen); }
static void @@ -1646,20 +1668,13 @@ mgag200_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, struct mga_device *mdev = to_mga_device(dev); struct drm_plane_state *state = plane->state; struct drm_framebuffer *fb = state->fb; - struct drm_gem_vram_object *gbo; - s64 gpu_addr; + struct drm_rect damage;
if (!fb) return;
- gbo = drm_gem_vram_of_gem(fb->obj[0]); - - gpu_addr = drm_gem_vram_offset(gbo); - if (drm_WARN_ON_ONCE(dev, gpu_addr < 0)) - return; /* BUG: BO should have been pinned to VRAM. */ - - mgag200_set_startadd(mdev, (unsigned long)gpu_addr); - mgag200_set_offset(mdev, fb); + if (drm_atomic_helper_damage_merged(old_state, state, &damage)) + mgag200_handle_damage(mdev, fb, &damage); }
static const struct drm_simple_display_pipe_funcs @@ -1669,8 +1684,7 @@ mgag200_simple_display_pipe_funcs = { .disable = mgag200_simple_display_pipe_disable, .check = mgag200_simple_display_pipe_check, .update = mgag200_simple_display_pipe_update, - .prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb, - .cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb, + .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, };
static const uint32_t mgag200_simple_display_pipe_formats[] = { @@ -1689,8 +1703,7 @@ static const uint64_t mgag200_simple_display_pipe_fmtmods[] = { */
static const struct drm_mode_config_funcs mgag200_mode_config_funcs = { - .fb_create = drm_gem_fb_create, - .mode_valid = drm_vram_helper_mode_valid, + .fb_create = drm_gem_fb_create_with_dirty, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, }; @@ -1729,7 +1742,6 @@ int mgag200_modeset_init(struct mga_device *mdev) dev->mode_config.max_height = MGAG200_MAX_FB_HEIGHT;
dev->mode_config.preferred_depth = mgag200_preferred_depth(mdev); - dev->mode_config.prefer_shadow = 1;
dev->mode_config.fb_base = mdev->mc.vram_base;
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c index e89657630ea71..a683642fe4682 100644 --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c @@ -32,17 +32,8 @@
int mgag200_mm_init(struct mga_device *mdev) { - struct drm_vram_mm *vmm; - int ret; struct drm_device *dev = mdev->dev; - - vmm = drm_vram_helper_alloc_mm(dev, pci_resource_start(dev->pdev, 0), - mdev->mc.vram_size); - if (IS_ERR(vmm)) { - ret = PTR_ERR(vmm); - DRM_ERROR("Error initializing VRAM MM; %d\n", ret); - return ret; - } + int ret;
arch_io_reserve_memtype_wc(pci_resource_start(dev->pdev, 0), pci_resource_len(dev->pdev, 0)); @@ -50,9 +41,22 @@ int mgag200_mm_init(struct mga_device *mdev) mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0), pci_resource_len(dev->pdev, 0));
+ mdev->vram = ioremap(pci_resource_start(dev->pdev, 0), + pci_resource_len(dev->pdev, 0)); + if (!mdev->vram) { + ret = -ENOMEM; + goto err_arch_phys_wc_del; + } + mdev->vram_fb_available = mdev->mc.vram_size;
return 0; + +err_arch_phys_wc_del: + arch_phys_wc_del(mdev->fb_mtrr); + arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0), + pci_resource_len(dev->pdev, 0)); + return ret; }
void mgag200_mm_fini(struct mga_device *mdev) @@ -60,9 +64,7 @@ void mgag200_mm_fini(struct mga_device *mdev) struct drm_device *dev = mdev->dev;
mdev->vram_fb_available = 0; - - drm_vram_helper_release_mm(dev); - + iounmap(mdev->vram); arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0), pci_resource_len(dev->pdev, 0)); arch_phys_wc_del(mdev->fb_mtrr);
dri-devel@lists.freedesktop.org