This patchset converts mgag200 to atomic modesetting. It uses simple KMS helpers and SHMEM.
Patch 1 removes cursor support. The HW cursor is not usable with the way universal planes work.
Patches 2 to 11 untangle the existing modesetting code into smaller functions. Specifically, mode setting and plane updates are being separated from each other.
Patch 12 to 14 convert mgag200 to simple KMS helpers and enables atomic mode setting.
Atomically switching plane framebuffers, requires either source or target buffer to be located at a non-0 offet. As some HW revisions seem to require a framebuffer offset of 0 within the video memory, they do not work with atomic modesetting. To resolve this problem, patch 15 converts mgag200 from VRAM helpers to SHMEM helpers. During plane updates, the content of the SHMEM BO is memcpy'd to VRAM. From my observation, performance is not nuch different from the original code.
The patchset has been tested on MGA G200EH hardware.
v2: * rebase patchset * replace uint{8,32}_t with u{8,32} through-out patchset * define additional register constants * use helper functions around bpp-shift computations * split conversion patch * cleanups
Thomas Zimmermann (15): drm/mgag200: Remove HW cursor drm/mgag200: Clean up mga_set_start_address() drm/mgag200: Clean up mga_crtc_do_set_base() drm/mgag200: Move mode-setting code into separate helper function drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O drm/mgag200: Update mode registers after plane registers drm/mgag200: Set pitch in a separate helper function drm/mgag200: Set primary plane's format in separate helper function drm/mgag200: Move TAGFIFO reset into separate function drm/mgag200: Move hiprilvl setting into separate functions drm/mgag200: Move register initialization into separate function drm/mgag200: Remove out-commented suspend/resume helpers drm/mgag200: Use simple-display data structures drm/mgag200: Convert to simple KMS helper drm/mgag200: Replace VRAM helpers with SHMEM helpers
drivers/gpu/drm/mgag200/Kconfig | 4 +- drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 51 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 41 +- drivers/gpu/drm/mgag200/mgag200_main.c | 5 - drivers/gpu/drm/mgag200/mgag200_mode.c | 871 ++++++++++++++----------- drivers/gpu/drm/mgag200/mgag200_reg.h | 11 +- drivers/gpu/drm/mgag200/mgag200_ttm.c | 28 +- 8 files changed, 528 insertions(+), 485 deletions(-)
-- 2.26.2
The HW cursor of Matrox G200 cards only supports a 16-color palette format. Univeral planes require at least ARGB or a similar component- based format, so remove the HW cursor.
Alternatively, the driver could dither a cursor image from ARGB to 16 colors. But this does not produce pleasent-looking results in general, so it's useless for modern compositors.
Without HW support, compositors will use software rendering.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 13 ------------- drivers/gpu/drm/mgag200/mgag200_main.c | 5 ----- drivers/gpu/drm/mgag200/mgag200_mode.c | 2 -- 4 files changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/Makefile b/drivers/gpu/drm/mgag200/Makefile index 04b281bcf6558..63403133638a3 100644 --- a/drivers/gpu/drm/mgag200/Makefile +++ b/drivers/gpu/drm/mgag200/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only -mgag200-y := mgag200_main.o mgag200_mode.o mgag200_cursor.o \ +mgag200-y := mgag200_main.o mgag200_mode.o \ mgag200_drv.o mgag200_i2c.o mgag200_ttm.o
obj-$(CONFIG_DRM_MGAG200) += mgag200.o diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index d9b7e96b214f8..bc372c2ec79e9 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -116,11 +116,6 @@ struct mga_connector { struct mga_i2c_chan *i2c; };
-struct mga_cursor { - struct drm_gem_vram_object *gbo[2]; - unsigned int next_index; -}; - struct mga_mc { resource_size_t vram_size; resource_size_t vram_base; @@ -156,8 +151,6 @@ struct mga_device {
struct mga_mc mc;
- struct mga_cursor cursor; - size_t vram_fb_available;
bool suspended; @@ -207,10 +200,4 @@ int mgag200_mm_init(struct mga_device *mdev); void mgag200_mm_fini(struct mga_device *mdev); int mgag200_mmap(struct file *filp, struct vm_area_struct *vma);
-int mgag200_cursor_init(struct mga_device *mdev); -void mgag200_cursor_fini(struct mga_device *mdev); -int mgag200_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv, - uint32_t handle, uint32_t width, uint32_t height); -int mgag200_crtc_cursor_move(struct drm_crtc *crtc, int x, int y); - #endif /* __MGAG200_DRV_H__ */ diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 86df799fd38c5..3298eff7bd1b4 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -135,10 +135,6 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags) goto err_mgag200_mm_fini; }
- ret = mgag200_cursor_init(mdev); - if (ret) - drm_err(dev, "Could not initialize cursors. Not doing hardware cursors.\n"); - return 0;
err_mgag200_mm_fini: @@ -154,7 +150,6 @@ void mgag200_driver_unload(struct drm_device *dev)
if (mdev == NULL) return; - mgag200_cursor_fini(mdev); mgag200_mm_fini(mdev); dev->dev_private = NULL; } diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 5f4ac36a97760..c68ed8b6faf9b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1412,8 +1412,6 @@ static void mga_crtc_disable(struct drm_crtc *crtc)
/* These provide the minimum set of functions required to handle a CRTC */ static const struct drm_crtc_funcs mga_crtc_funcs = { - .cursor_set = mgag200_crtc_cursor_set, - .cursor_move = mgag200_crtc_cursor_move, .gamma_set = mga_crtc_gamma_set, .set_config = drm_crtc_helper_set_config, .destroy = mga_crtc_destroy,
All register names and fields are now named according to the MGA programming manuals. The function doesn't need the CRTC, so callers pass in the device structure directly. The logging now uses device-specific macros.
v2: * use to_mga_device() * use MiB instead of MB * replace empty while loop with do-while, fixes checkpatch warning * replace uint{8,32}_t with u{8,32}
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com --- drivers/gpu/drm/mgag200/mgag200_drv.h | 5 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 82 +++++++++++++++----------- 2 files changed, 53 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index bc372c2ec79e9..1963876ef3b8c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -61,6 +61,11 @@ WREG8(MGAREG_CRTC_DATA, v); \ } while (0) \
+#define RREG_ECRT(reg, v) \ + do { \ + WREG8(MGAREG_CRTCEXT_INDEX, reg); \ + v = RREG8(MGAREG_CRTCEXT_DATA); \ + } while (0) \
#define WREG_ECRT(reg, v) \ do { \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index c68ed8b6faf9b..80a3a805c0c4e 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -819,49 +819,53 @@ static void mga_g200wb_commit(struct drm_crtc *crtc) }
/* - This is how the framebuffer base address is stored in g200 cards: - * Assume @offset is the gpu_addr variable of the framebuffer object - * Then addr is the number of _pixels_ (not bytes) from the start of - VRAM to the first pixel we want to display. (divided by 2 for 32bit - framebuffers) - * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers - addr<20> -> CRTCEXT0<6> - addr<19-16> -> CRTCEXT0<3-0> - addr<15-8> -> CRTCC<7-0> - addr<7-0> -> CRTCD<7-0> - CRTCEXT0 has to be programmed last to trigger an update and make the - new addr variable take effect. + * This is how the framebuffer base address is stored in g200 cards: + * * Assume @offset is the gpu_addr variable of the framebuffer object + * * Then addr is the number of _pixels_ (not bytes) from the start of + * VRAM to the first pixel we want to display. (divided by 2 for 32bit + * framebuffers) + * * addr is stored in the CRTCEXT0, CRTCC and CRTCD registers + * addr<20> -> CRTCEXT0<6> + * addr<19-16> -> CRTCEXT0<3-0> + * addr<15-8> -> CRTCC<7-0> + * addr<7-0> -> CRTCD<7-0> + * + * CRTCEXT0 has to be programmed last to trigger an update and make the + * new addr variable take effect. */ -static void mga_set_start_address(struct drm_crtc *crtc, unsigned offset) +static void mgag200_set_startadd(struct mga_device *mdev, + unsigned long offset) { - struct mga_device *mdev = to_mga_device(crtc->dev); - u32 addr; - int count; - u8 crtcext0; + struct drm_device *dev = mdev->dev; + u32 startadd; + u8 crtcc, crtcd, crtcext0;
- while (RREG8(0x1fda) & 0x08); - while (!(RREG8(0x1fda) & 0x08)); + startadd = offset / 8;
- count = RREG8(MGAREG_VCOUNT) + 2; - while (RREG8(MGAREG_VCOUNT) < count); - - WREG8(MGAREG_CRTCEXT_INDEX, 0); - crtcext0 = RREG8(MGAREG_CRTCEXT_DATA); - crtcext0 &= 0xB0; - addr = offset / 8; - /* Can't store addresses any higher than that... - but we also don't have more than 16MB of memory, so it should be fine. */ - WARN_ON(addr > 0x1fffff); - crtcext0 |= (!!(addr & (1<<20)))<<6; - WREG_CRT(0x0d, (u8)(addr & 0xff)); - WREG_CRT(0x0c, (u8)(addr >> 8) & 0xff); - WREG_ECRT(0x0, ((u8)(addr >> 16) & 0xf) | crtcext0); + /* + * Can't store addresses any higher than that, but we also + * don't have more than 16 MiB of memory, so it should be fine. + */ + drm_WARN_ON(dev, startadd > 0x1fffff); + + RREG_ECRT(0x00, crtcext0); + + crtcc = (startadd >> 8) & 0xff; + crtcd = startadd & 0xff; + crtcext0 &= 0xb0; + crtcext0 |= ((startadd >> 14) & BIT(6)) | + ((startadd >> 16) & 0x0f); + + WREG_CRT(0x0c, crtcc); + WREG_CRT(0x0d, crtcd); + WREG_ECRT(0x00, crtcext0); }
static int mga_crtc_do_set_base(struct drm_crtc *crtc, struct drm_framebuffer *fb, int x, int y, int atomic) { + struct mga_device *mdev = to_mga_device(crtc->dev); struct drm_gem_vram_object *gbo; int ret; s64 gpu_addr; @@ -882,7 +886,7 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, goto err_drm_gem_vram_unpin; }
- mga_set_start_address(crtc, (u32)gpu_addr); + mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
return 0;
@@ -894,6 +898,16 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb) { + struct drm_device *dev = crtc->dev; + struct mga_device *mdev = dev->dev_private; + unsigned int count; + + do { } while (RREG8(0x1fda) & 0x08); + do { } while (!(RREG8(0x1fda) & 0x08)); + + count = RREG8(MGAREG_VCOUNT) + 2; + do { } while (RREG8(MGAREG_VCOUNT) < count); + return mga_crtc_do_set_base(crtc, old_fb, x, y, 0); }
Hi Thomas.
On Tue, May 12, 2020 at 10:42:45AM +0200, Thomas Zimmermann wrote:
All register names and fields are now named according to the MGA programming manuals. The function doesn't need the CRTC, so callers pass in the device structure directly. The logging now uses device-specific macros.
v2:
- use to_mga_device()
- use MiB instead of MB
- replace empty while loop with do-while, fixes checkpatch warning
- replace uint{8,32}_t with u{8,32}
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com
With the comment below addressed: Acked-by: Sam Ravnborg sam@ravnborg.org
drivers/gpu/drm/mgag200/mgag200_drv.h | 5 ++ drivers/gpu/drm/mgag200/mgag200_mode.c | 82 +++++++++++++++----------- 2 files changed, 53 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index bc372c2ec79e9..1963876ef3b8c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -61,6 +61,11 @@ WREG8(MGAREG_CRTC_DATA, v); \ } while (0) \
+#define RREG_ECRT(reg, v) \
- do { \
WREG8(MGAREG_CRTCEXT_INDEX, reg); \v = RREG8(MGAREG_CRTCEXT_DATA); \- } while (0) \
#define WREG_ECRT(reg, v) \ do { \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index c68ed8b6faf9b..80a3a805c0c4e 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -819,49 +819,53 @@ static void mga_g200wb_commit(struct drm_crtc *crtc) }
/*
- This is how the framebuffer base address is stored in g200 cards:
- Assume @offset is the gpu_addr variable of the framebuffer object
- Then addr is the number of _pixels_ (not bytes) from the start of
VRAM to the first pixel we want to display. (divided by 2 for 32bitframebuffers)
- addr is stored in the CRTCEXT0, CRTCC and CRTCD registers
- addr<20> -> CRTCEXT0<6>
- addr<19-16> -> CRTCEXT0<3-0>
- addr<15-8> -> CRTCC<7-0>
- addr<7-0> -> CRTCD<7-0>
- CRTCEXT0 has to be programmed last to trigger an update and make the
- new addr variable take effect.
- This is how the framebuffer base address is stored in g200 cards:
- Assume @offset is the gpu_addr variable of the framebuffer object
- Then addr is the number of _pixels_ (not bytes) from the start of
VRAM to the first pixel we want to display. (divided by 2 for 32bit
framebuffers)
- addr is stored in the CRTCEXT0, CRTCC and CRTCD registers
addr<20> -> CRTCEXT0<6>
addr<19-16> -> CRTCEXT0<3-0>
addr<15-8> -> CRTCC<7-0>
addr<7-0> -> CRTCD<7-0>
- CRTCEXT0 has to be programmed last to trigger an update and make the
*/
- new addr variable take effect.
-static void mga_set_start_address(struct drm_crtc *crtc, unsigned offset) +static void mgag200_set_startadd(struct mga_device *mdev,
unsigned long offset){
- struct mga_device *mdev = to_mga_device(crtc->dev);
- u32 addr;
- int count;
- u8 crtcext0;
- struct drm_device *dev = mdev->dev;
- u32 startadd;
- u8 crtcc, crtcd, crtcext0;
- while (RREG8(0x1fda) & 0x08);
- while (!(RREG8(0x1fda) & 0x08));
- startadd = offset / 8;
- count = RREG8(MGAREG_VCOUNT) + 2;
- while (RREG8(MGAREG_VCOUNT) < count);
- WREG8(MGAREG_CRTCEXT_INDEX, 0);
- crtcext0 = RREG8(MGAREG_CRTCEXT_DATA);
- crtcext0 &= 0xB0;
- addr = offset / 8;
- /* Can't store addresses any higher than that...
but we also don't have more than 16MB of memory, so it should be fine. */- WARN_ON(addr > 0x1fffff);
- crtcext0 |= (!!(addr & (1<<20)))<<6;
- WREG_CRT(0x0d, (u8)(addr & 0xff));
- WREG_CRT(0x0c, (u8)(addr >> 8) & 0xff);
- WREG_ECRT(0x0, ((u8)(addr >> 16) & 0xf) | crtcext0);
- /*
* Can't store addresses any higher than that, but we also* don't have more than 16 MiB of memory, so it should be fine.*/- drm_WARN_ON(dev, startadd > 0x1fffff);
- RREG_ECRT(0x00, crtcext0);
- crtcc = (startadd >> 8) & 0xff;
- crtcd = startadd & 0xff;
- crtcext0 &= 0xb0;
- crtcext0 |= ((startadd >> 14) & BIT(6)) |
((startadd >> 16) & 0x0f);- WREG_CRT(0x0c, crtcc);
- WREG_CRT(0x0d, crtcd);
- WREG_ECRT(0x00, crtcext0);
}
static int mga_crtc_do_set_base(struct drm_crtc *crtc, struct drm_framebuffer *fb, int x, int y, int atomic) {
- struct mga_device *mdev = to_mga_device(crtc->dev); struct drm_gem_vram_object *gbo; int ret; s64 gpu_addr;
@@ -882,7 +886,7 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, goto err_drm_gem_vram_unpin; }
- mga_set_start_address(crtc, (u32)gpu_addr);
mgag200_set_startadd(mdev, (unsigned long)gpu_addr);
return 0;
@@ -894,6 +898,16 @@ static int mga_crtc_do_set_base(struct drm_crtc *crtc, static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, struct drm_framebuffer *old_fb) {
- struct drm_device *dev = crtc->dev;
- struct mga_device *mdev = dev->dev_private;
- unsigned int count;
- do { } while (RREG8(0x1fda) & 0x08);
- do { } while (!(RREG8(0x1fda) & 0x08));
- count = RREG8(MGAREG_VCOUNT) + 2;
- do { } while (RREG8(MGAREG_VCOUNT) < count);
- return mga_crtc_do_set_base(crtc, old_fb, x, y, 0);
}
The changelog still does not explain why this code is moved here.
-- 2.26.2
The function now only takes the device structure, and the old and new framebuffers.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/mgag200_mode.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 80a3a805c0c4e..9aa6addbbb895 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -861,21 +861,20 @@ static void mgag200_set_startadd(struct mga_device *mdev, WREG_ECRT(0x00, crtcext0); }
-static int mga_crtc_do_set_base(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int x, int y, int atomic) +static int mga_crtc_do_set_base(struct mga_device *mdev, + const struct drm_framebuffer *fb, + const struct drm_framebuffer *old_fb) { - struct mga_device *mdev = to_mga_device(crtc->dev); struct drm_gem_vram_object *gbo; int ret; s64 gpu_addr;
- if (!atomic && fb) { - gbo = drm_gem_vram_of_gem(fb->obj[0]); + if (old_fb) { + gbo = drm_gem_vram_of_gem(old_fb->obj[0]); drm_gem_vram_unpin(gbo); }
- gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]); + gbo = drm_gem_vram_of_gem(fb->obj[0]);
ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM); if (ret) @@ -900,6 +899,7 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, { struct drm_device *dev = crtc->dev; struct mga_device *mdev = dev->dev_private; + struct drm_framebuffer *fb = crtc->primary->fb; unsigned int count;
do { } while (RREG8(0x1fda) & 0x08); @@ -908,7 +908,7 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, count = RREG8(MGAREG_VCOUNT) + 2; do { } while (RREG8(MGAREG_VCOUNT) < count);
- return mga_crtc_do_set_base(crtc, old_fb, x, y, 0); + return mga_crtc_do_set_base(mdev, fb, old_fb); }
static int mga_crtc_mode_set(struct drm_crtc *crtc, @@ -1150,7 +1150,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
WREG8(MGA_MISC_OUT, misc);
- mga_crtc_do_set_base(crtc, old_fb, x, y, 0); + mga_crtc_do_set_base(mdev, fb, old_fb);
/* reset tagfifo */ if (mdev->type == G200_ER) {
The mode-setting code is now located in mgag200_set_mode_regs(), sans a few flags that will be moved in a later patch for clarity.
v2: * replace uint8_t with u8
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/mgag200_mode.c | 140 ++++++++++++++----------- 1 file changed, 78 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 9aa6addbbb895..7c41bd43f79e0 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -911,6 +911,79 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, return mga_crtc_do_set_base(mdev, fb, old_fb); }
+static void mgag200_set_mode_regs(struct mga_device *mdev, + const struct drm_display_mode *mode) +{ + unsigned int hdisplay, hsyncstart, hsyncend, htotal; + unsigned int vdisplay, vsyncstart, vsyncend, vtotal; + u8 misc = 0; + u8 crtcext1, crtcext2, crtcext5; + + hdisplay = mode->hdisplay / 8 - 1; + hsyncstart = mode->hsync_start / 8 - 1; + hsyncend = mode->hsync_end / 8 - 1; + htotal = mode->htotal / 8 - 1; + + /* Work around hardware quirk */ + if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04) + htotal++; + + vdisplay = mode->vdisplay - 1; + vsyncstart = mode->vsync_start - 1; + vsyncend = mode->vsync_end - 1; + vtotal = mode->vtotal - 2; + + if (mode->flags & DRM_MODE_FLAG_NHSYNC) + misc |= 0x40; + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + misc |= 0x80; + + crtcext1 = (((htotal - 4) & 0x100) >> 8) | + ((hdisplay & 0x100) >> 7) | + ((hsyncstart & 0x100) >> 6) | + (htotal & 0x40); + if (mdev->type == G200_WB || mdev->type == G200_EW3) + crtcext1 |= BIT(7) | /* vrsten */ + BIT(3); /* hrsten */ + + crtcext2 = ((vtotal & 0xc00) >> 10) | + ((vdisplay & 0x400) >> 8) | + ((vdisplay & 0xc00) >> 7) | + ((vsyncstart & 0xc00) >> 5) | + ((vdisplay & 0x400) >> 3); + crtcext5 = 0x00; + + WREG_CRT(0, htotal - 4); + WREG_CRT(1, hdisplay); + WREG_CRT(2, hdisplay); + WREG_CRT(3, (htotal & 0x1F) | 0x80); + WREG_CRT(4, hsyncstart); + WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F)); + WREG_CRT(6, vtotal & 0xFF); + WREG_CRT(7, ((vtotal & 0x100) >> 8) | + ((vdisplay & 0x100) >> 7) | + ((vsyncstart & 0x100) >> 6) | + ((vdisplay & 0x100) >> 5) | + ((vdisplay & 0x100) >> 4) | /* linecomp */ + ((vtotal & 0x200) >> 4) | + ((vdisplay & 0x200) >> 3) | + ((vsyncstart & 0x200) >> 2)); + WREG_CRT(9, ((vdisplay & 0x200) >> 4) | + ((vdisplay & 0x200) >> 3)); + WREG_CRT(16, vsyncstart & 0xFF); + WREG_CRT(17, (vsyncend & 0x0F) | 0x20); + WREG_CRT(18, vdisplay & 0xFF); + WREG_CRT(20, 0); + WREG_CRT(21, vdisplay & 0xFF); + WREG_CRT(22, (vtotal + 1) & 0xFF); + WREG_CRT(23, 0xc3); + WREG_CRT(24, vdisplay & 0xFF); + + WREG_ECRT(0x01, crtcext1); + WREG_ECRT(0x02, crtcext2); + WREG_ECRT(0x05, crtcext5); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -919,8 +992,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); const struct drm_framebuffer *fb = crtc->primary->fb; - int hdisplay, hsyncstart, hsyncend, htotal; - int vdisplay, vsyncstart, vsyncend, vtotal; int pitch; int option = 0, option2 = 0; int i; @@ -999,12 +1070,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, break; }
- if (mode->flags & DRM_MODE_FLAG_NHSYNC) - misc |= 0x40; - if (mode->flags & DRM_MODE_FLAG_NVSYNC) - misc |= 0x80; - - for (i = 0; i < sizeof(dacvalue); i++) { if ((i <= 0x17) || (i == 0x1b) || @@ -1044,20 +1109,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, else pitch = pitch >> (4 - bppshift);
- hdisplay = mode->hdisplay / 8 - 1; - hsyncstart = mode->hsync_start / 8 - 1; - hsyncend = mode->hsync_end / 8 - 1; - htotal = mode->htotal / 8 - 1; - - /* Work around hardware quirk */ - if ((htotal & 0x07) == 0x06 || (htotal & 0x07) == 0x04) - htotal++; - - vdisplay = mode->vdisplay - 1; - vsyncstart = mode->vsync_start - 1; - vsyncend = mode->vsync_end - 1; - vtotal = mode->vtotal - 2; - WREG_GFX(0, 0); WREG_GFX(1, 0); WREG_GFX(2, 0); @@ -1068,61 +1119,26 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_GFX(7, 0xf); WREG_GFX(8, 0xf);
- WREG_CRT(0, htotal - 4); - WREG_CRT(1, hdisplay); - WREG_CRT(2, hdisplay); - WREG_CRT(3, (htotal & 0x1F) | 0x80); - WREG_CRT(4, hsyncstart); - WREG_CRT(5, ((htotal & 0x20) << 2) | (hsyncend & 0x1F)); - WREG_CRT(6, vtotal & 0xFF); - WREG_CRT(7, ((vtotal & 0x100) >> 8) | - ((vdisplay & 0x100) >> 7) | - ((vsyncstart & 0x100) >> 6) | - ((vdisplay & 0x100) >> 5) | - ((vdisplay & 0x100) >> 4) | /* linecomp */ - ((vtotal & 0x200) >> 4)| - ((vdisplay & 0x200) >> 3) | - ((vsyncstart & 0x200) >> 2)); - WREG_CRT(9, ((vdisplay & 0x200) >> 4) | - ((vdisplay & 0x200) >> 3)); WREG_CRT(10, 0); WREG_CRT(11, 0); WREG_CRT(12, 0); WREG_CRT(13, 0); WREG_CRT(14, 0); WREG_CRT(15, 0); - WREG_CRT(16, vsyncstart & 0xFF); - WREG_CRT(17, (vsyncend & 0x0F) | 0x20); - WREG_CRT(18, vdisplay & 0xFF); WREG_CRT(19, pitch & 0xFF); - WREG_CRT(20, 0); - WREG_CRT(21, vdisplay & 0xFF); - WREG_CRT(22, (vtotal + 1) & 0xFF); - WREG_CRT(23, 0xc3); - WREG_CRT(24, vdisplay & 0xFF); + + mgag200_set_mode_regs(mdev, mode);
ext_vga[0] = 0; - ext_vga[5] = 0;
/* TODO interlace */
ext_vga[0] |= (pitch & 0x300) >> 4; - ext_vga[1] = (((htotal - 4) & 0x100) >> 8) | - ((hdisplay & 0x100) >> 7) | - ((hsyncstart & 0x100) >> 6) | - (htotal & 0x40); - ext_vga[2] = ((vtotal & 0xc00) >> 10) | - ((vdisplay & 0x400) >> 8) | - ((vdisplay & 0xc00) >> 7) | - ((vsyncstart & 0xc00) >> 5) | - ((vdisplay & 0x400) >> 3); if (fb->format->cpp[0] * 8 == 24) ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80; else ext_vga[3] = ((1 << bppshift) - 1) | 0x80; ext_vga[4] = 0; - if (mdev->type == G200_WB || mdev->type == G200_EW3) - ext_vga[1] |= 0x88;
/* Set pixel clocks */ misc = 0x2d; @@ -1130,9 +1146,9 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
mga_crtc_set_plls(mdev, mode->clock);
- for (i = 0; i < 6; i++) { - WREG_ECRT(i, ext_vga[i]); - } + WREG_ECRT(0, ext_vga[0]); + WREG_ECRT(3, ext_vga[3]); + WREG_ECRT(4, ext_vga[4]);
if (mdev->type == G200_ER) WREG_ECRT(0x24, 0x5);
Set different fields in MISC in their rsp location in the code. This patch also fixes a bug in the original code where the mode's SYNC flags were never written into the MISC register.
v2: * use u8 instead of uint8_t * define MGAREG_MISC_CLK_SEL_MASK
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 38 ++++++++++++++++++-------- drivers/gpu/drm/mgag200/mgag200_reg.h | 6 +++- 2 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 7c41bd43f79e0..2007d7a4754ac 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -704,6 +704,8 @@ static int mga_g200er_set_plls(struct mga_device *mdev, long clock)
static int mga_crtc_set_plls(struct mga_device *mdev, long clock) { + u8 misc; + switch(mdev->type) { case G200_SE_A: case G200_SE_B: @@ -724,6 +726,12 @@ static int mga_crtc_set_plls(struct mga_device *mdev, long clock) return mga_g200er_set_plls(mdev, clock); break; } + + misc = RREG8(MGA_MISC_IN); + misc &= ~MGAREG_MISC_CLK_SEL_MASK; + misc |= MGAREG_MISC_CLK_SEL_MGA_MSK; + WREG8(MGA_MISC_OUT, misc); + return 0; }
@@ -916,8 +924,7 @@ static void mgag200_set_mode_regs(struct mga_device *mdev, { unsigned int hdisplay, hsyncstart, hsyncend, htotal; unsigned int vdisplay, vsyncstart, vsyncend, vtotal; - u8 misc = 0; - u8 crtcext1, crtcext2, crtcext5; + u8 misc, crtcext1, crtcext2, crtcext5;
hdisplay = mode->hdisplay / 8 - 1; hsyncstart = mode->hsync_start / 8 - 1; @@ -933,10 +940,17 @@ static void mgag200_set_mode_regs(struct mga_device *mdev, vsyncend = mode->vsync_end - 1; vtotal = mode->vtotal - 2;
+ misc = RREG8(MGA_MISC_IN); + if (mode->flags & DRM_MODE_FLAG_NHSYNC) - misc |= 0x40; + misc |= MGAREG_MISC_HSYNCPOL; + else + misc &= ~MGAREG_MISC_HSYNCPOL; + if (mode->flags & DRM_MODE_FLAG_NVSYNC) - misc |= 0x80; + misc |= MGAREG_MISC_VSYNCPOL; + else + misc &= ~MGAREG_MISC_VSYNCPOL;
crtcext1 = (((htotal - 4) & 0x100) >> 8) | ((hdisplay & 0x100) >> 7) | @@ -982,6 +996,10 @@ static void mgag200_set_mode_regs(struct mga_device *mdev, WREG_ECRT(0x01, crtcext1); WREG_ECRT(0x02, crtcext2); WREG_ECRT(0x05, crtcext5); + + WREG8(MGA_MISC_OUT, misc); + + mga_crtc_set_plls(mdev, mode->clock); }
static int mga_crtc_mode_set(struct drm_crtc *crtc, @@ -1140,12 +1158,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, ext_vga[3] = ((1 << bppshift) - 1) | 0x80; ext_vga[4] = 0;
- /* Set pixel clocks */ - misc = 0x2d; - WREG8(MGA_MISC_OUT, misc); - - mga_crtc_set_plls(mdev, mode->clock); - WREG_ECRT(0, ext_vga[0]); WREG_ECRT(3, ext_vga[3]); WREG_ECRT(4, ext_vga[4]); @@ -1161,9 +1173,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, }
WREG_ECRT(0, ext_vga[0]); - /* Enable mga pixel clock */ - misc = 0x2d;
+ misc = RREG8(MGA_MISC_IN); + misc |= MGAREG_MISC_IOADSEL | + MGAREG_MISC_RAMMAPEN | + MGAREG_MISC_HIGH_PG_SEL; WREG8(MGA_MISC_OUT, misc);
mga_crtc_do_set_base(mdev, fb, old_fb); diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index c096a9d6bcbc1..0ba6e15e97106 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -16,10 +16,11 @@ * MGA1064SG Mystique register file */
- #ifndef _MGA_REG_H_ #define _MGA_REG_H_
+#include <linux/bits.h> + #define MGAREG_DWGCTL 0x1c00 #define MGAREG_MACCESS 0x1c04 /* the following is a mystique only register */ @@ -221,12 +222,15 @@
#define MGAREG_MISC_IOADSEL (0x1 << 0) #define MGAREG_MISC_RAMMAPEN (0x1 << 1) +#define MGAREG_MISC_CLK_SEL_MASK GENMASK(3, 2) #define MGAREG_MISC_CLK_SEL_VGA25 (0x0 << 2) #define MGAREG_MISC_CLK_SEL_VGA28 (0x1 << 2) #define MGAREG_MISC_CLK_SEL_MGA_PIX (0x2 << 2) #define MGAREG_MISC_CLK_SEL_MGA_MSK (0x3 << 2) #define MGAREG_MISC_VIDEO_DIS (0x1 << 4) #define MGAREG_MISC_HIGH_PG_SEL (0x1 << 5) +#define MGAREG_MISC_HSYNCPOL BIT(6) +#define MGAREG_MISC_VSYNCPOL BIT(7)
/* MMIO VGA registers */ #define MGAREG_SEQ_INDEX 0x1fc4
Setting the plane registers first and the mode registers afterwards reproduces the sequence used by atomic helpers. Done in preparation of switching to simple KMS helpers.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/mgag200_mode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 2007d7a4754ac..4dba0a379c263 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1145,8 +1145,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_CRT(15, 0); WREG_CRT(19, pitch & 0xFF);
- mgag200_set_mode_regs(mdev, mode); - ext_vga[0] = 0;
/* TODO interlace */ @@ -1182,6 +1180,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
mga_crtc_do_set_base(mdev, fb, old_fb);
+ mgag200_set_mode_regs(mdev, mode); + /* reset tagfifo */ if (mdev->type == G200_ER) { u32 mem_ctl = RREG32(MGAREG_MEMCTL);
The framebuffer's pitch is now set in mgag200_set_offset().
v2: * move offset and bpp-shift calculation into helper functions * use u8 instead of uint8_t * add MGAREG_CRTCEXT0_OFFSET_MASK
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 57 +++++++++++++++++++------- drivers/gpu/drm/mgag200/mgag200_reg.h | 2 + 2 files changed, 45 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 4dba0a379c263..dee7838d7d368 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1002,6 +1002,48 @@ static void mgag200_set_mode_regs(struct mga_device *mdev, mga_crtc_set_plls(mdev, mode->clock); }
+static u8 mgag200_get_bpp_shift(struct mga_device *mdev, + const struct drm_format_info *format) +{ + return mdev->bpp_shifts[format->cpp[0] - 1]; +} + +/* + * Calculates the HW offset value from the framebuffer's pitch. The + * offset is a multiple of the pixel size and depends on the display + * format. + */ +static u32 mgag200_calculate_offset(struct mga_device *mdev, + const struct drm_framebuffer *fb) +{ + u32 offset = fb->pitches[0] / fb->format->cpp[0]; + u8 bppshift = mgag200_get_bpp_shift(mdev, fb->format); + + if (fb->format->cpp[0] * 8 == 24) + offset = (offset * 3) >> (4 - bppshift); + else + offset = offset >> (4 - bppshift); + + return offset; +} + +static void mgag200_set_offset(struct mga_device *mdev, + const struct drm_framebuffer *fb) +{ + u8 crtc13, crtcext0; + u32 offset = mgag200_calculate_offset(mdev, fb); + + RREG_ECRT(0, crtcext0); + + crtc13 = offset & 0xff; + + crtcext0 &= ~MGAREG_CRTCEXT0_OFFSET_MASK; + crtcext0 |= (offset >> 4) & MGAREG_CRTCEXT0_OFFSET_MASK; + + WREG_CRT(0x13, crtc13); + WREG_ECRT(0x00, crtcext0); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -1010,7 +1052,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); const struct drm_framebuffer *fb = crtc->primary->fb; - int pitch; int option = 0, option2 = 0; int i; unsigned char misc = 0; @@ -1121,12 +1162,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_SEQ(3, 0); WREG_SEQ(4, 0xe);
- pitch = fb->pitches[0] / fb->format->cpp[0]; - if (fb->format->cpp[0] * 8 == 24) - pitch = (pitch * 3) >> (4 - bppshift); - else - pitch = pitch >> (4 - bppshift); - WREG_GFX(0, 0); WREG_GFX(1, 0); WREG_GFX(2, 0); @@ -1143,20 +1178,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_CRT(13, 0); WREG_CRT(14, 0); WREG_CRT(15, 0); - WREG_CRT(19, pitch & 0xFF); - - ext_vga[0] = 0;
/* TODO interlace */
- ext_vga[0] |= (pitch & 0x300) >> 4; if (fb->format->cpp[0] * 8 == 24) ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80; else ext_vga[3] = ((1 << bppshift) - 1) | 0x80; ext_vga[4] = 0;
- WREG_ECRT(0, ext_vga[0]); WREG_ECRT(3, ext_vga[3]); WREG_ECRT(4, ext_vga[4]);
@@ -1170,8 +1200,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_ECRT(6, 0); }
- WREG_ECRT(0, ext_vga[0]); - misc = RREG8(MGA_MISC_IN); misc |= MGAREG_MISC_IOADSEL | MGAREG_MISC_RAMMAPEN | @@ -1179,6 +1207,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG8(MGA_MISC_OUT, misc);
mga_crtc_do_set_base(mdev, fb, old_fb); + mgag200_set_offset(mdev, fb);
mgag200_set_mode_regs(mdev, mode);
diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index 0ba6e15e97106..cd08dee29b06f 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -240,6 +240,8 @@ #define MGAREG_CRTCEXT_INDEX 0x1fde #define MGAREG_CRTCEXT_DATA 0x1fdf
+#define MGAREG_CRTCEXT0_OFFSET_MASK GENMASK(5, 4) + /* Cursor X and Y position */ #define MGA_CURPOSXL 0x3c0c #define MGA_CURPOSXH 0x3c0d
Hi Thomas,
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
@@ -1143,20 +1178,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_CRT(13, 0); WREG_CRT(14, 0); WREG_CRT(15, 0);
WREG_CRT(19, pitch & 0xFF);
This write has disappeared without an equivalent or a comment. Is that intentional?
-Emil
Hi Emil
Am 12.05.20 um 12:27 schrieb Emil Velikov:
Hi Thomas,
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
@@ -1143,20 +1178,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_CRT(13, 0); WREG_CRT(14, 0); WREG_CRT(15, 0);
WREG_CRT(19, pitch & 0xFF);This write has disappeared without an equivalent or a comment. Is that intentional?
It's still there. It's now located in mgag200_set_offset() and 19 is written as 0x13. :) I try to follow the naming style of the MGA programming manual, which prefers hexadecimal numbers.
Best regards Thomas
-Emil
On Tue, 12 May 2020 at 19:34, Thomas Zimmermann tzimmermann@suse.de wrote:
Hi Emil
Am 12.05.20 um 12:27 schrieb Emil Velikov:
Hi Thomas,
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
@@ -1143,20 +1178,15 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_CRT(13, 0); WREG_CRT(14, 0); WREG_CRT(15, 0);
WREG_CRT(19, pitch & 0xFF);This write has disappeared without an equivalent or a comment. Is that intentional?
It's still there. It's now located in mgag200_set_offset() and 19 is written as 0x13. :) I try to follow the naming style of the MGA programming manual, which prefers hexadecimal numbers.
D'oh, didn't register the dec->hex change. Thanks
-Emil
The primary plane's format registers are now updated in a mgag200_set_format_regs().
v2: * get bpp shift from helper function * replace uint8_t with u8
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 109 ++++++++++++++++--------- 1 file changed, 69 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index dee7838d7d368..38556f57ad218 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1044,6 +1044,68 @@ static void mgag200_set_offset(struct mga_device *mdev, WREG_ECRT(0x00, crtcext0); }
+static void mgag200_set_format_regs(struct mga_device *mdev, + const struct drm_framebuffer *fb) +{ + struct drm_device *dev = mdev->dev; + const struct drm_format_info *format = fb->format; + unsigned int bpp, bppshift, scale; + u8 crtcext3, xmulctrl; + + bpp = format->cpp[0] * 8; + + bppshift = mgag200_get_bpp_shift(mdev, format); + switch (bpp) { + case 24: + scale = ((1 << bppshift) * 3) - 1; + break; + default: + scale = (1 << bppshift) - 1; + break; + } + + RREG_ECRT(3, crtcext3); + + switch (bpp) { + case 8: + xmulctrl = MGA1064_MUL_CTL_8bits; + break; + case 16: + if (format->depth == 15) + xmulctrl = MGA1064_MUL_CTL_15bits; + else + xmulctrl = MGA1064_MUL_CTL_16bits; + break; + case 24: + xmulctrl = MGA1064_MUL_CTL_24bits; + break; + case 32: + xmulctrl = MGA1064_MUL_CTL_32_24bits; + break; + default: + /* BUG: We should have caught this problem already. */ + drm_WARN_ON(dev, "invalid format depth\n"); + return; + } + + crtcext3 &= ~GENMASK(2, 0); + crtcext3 |= scale; + + WREG_DAC(MGA1064_MUL_CTL, xmulctrl); + + WREG_GFX(0, 0x00); + WREG_GFX(1, 0x00); + WREG_GFX(2, 0x00); + WREG_GFX(3, 0x00); + WREG_GFX(4, 0x00); + WREG_GFX(5, 0x40); + WREG_GFX(6, 0x05); + WREG_GFX(7, 0x0f); + WREG_GFX(8, 0x0f); + + WREG_ECRT(3, crtcext3); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -1055,8 +1117,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, int option = 0, option2 = 0; int i; unsigned char misc = 0; - unsigned char ext_vga[6]; - u8 bppshift; + u8 crtcext3, crtcext4;
static unsigned char dacvalue[] = { /* 0x00: */ 0, 0, 0, 0, 0, 0, 0x00, 0, @@ -1071,8 +1132,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0 };
- bppshift = mdev->bpp_shifts[fb->format->cpp[0] - 1]; - switch (mdev->type) { case G200_SE_A: case G200_SE_B: @@ -1111,24 +1170,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, break; }
- switch (fb->format->cpp[0] * 8) { - case 8: - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_8bits; - break; - case 16: - if (fb->format->depth == 15) - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_15bits; - else - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_16bits; - break; - case 24: - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_24bits; - break; - case 32: - dacvalue[MGA1064_MUL_CTL] = MGA1064_MUL_CTL_32_24bits; - break; - } - for (i = 0; i < sizeof(dacvalue); i++) { if ((i <= 0x17) || (i == 0x1b) || @@ -1162,16 +1203,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_SEQ(3, 0); WREG_SEQ(4, 0xe);
- WREG_GFX(0, 0); - WREG_GFX(1, 0); - WREG_GFX(2, 0); - WREG_GFX(3, 0); - WREG_GFX(4, 0); - WREG_GFX(5, 0x40); - WREG_GFX(6, 0x5); - WREG_GFX(7, 0xf); - WREG_GFX(8, 0xf); - WREG_CRT(10, 0); WREG_CRT(11, 0); WREG_CRT(12, 0); @@ -1179,16 +1210,13 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, WREG_CRT(14, 0); WREG_CRT(15, 0);
- /* TODO interlace */ + RREG_ECRT(0x03, crtcext3);
- if (fb->format->cpp[0] * 8 == 24) - ext_vga[3] = (((1 << bppshift) * 3) - 1) | 0x80; - else - ext_vga[3] = ((1 << bppshift) - 1) | 0x80; - ext_vga[4] = 0; + crtcext3 |= BIT(7); /* enable MGA mode */ + crtcext4 = 0x00;
- WREG_ECRT(3, ext_vga[3]); - WREG_ECRT(4, ext_vga[4]); + WREG_ECRT(0x03, crtcext3); + WREG_ECRT(0x04, crtcext4);
if (mdev->type == G200_ER) WREG_ECRT(0x24, 0x5); @@ -1206,6 +1234,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, MGAREG_MISC_HIGH_PG_SEL; WREG8(MGA_MISC_OUT, misc);
+ mgag200_set_format_regs(mdev, fb); mga_crtc_do_set_base(mdev, fb, old_fb); mgag200_set_offset(mdev, fb);
The TAGFIFO state is now reset in mgag200_g200er_reset_tagfifo().
v2: * define MGAREG_SEQ1_SCROFF
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/mgag200_drv.h | 6 ++++ drivers/gpu/drm/mgag200/mgag200_mode.c | 45 +++++++++++++++++--------- drivers/gpu/drm/mgag200/mgag200_reg.h | 3 ++ 3 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 1963876ef3b8c..cf71a4ec84158 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -49,6 +49,12 @@ WREG8(ATTR_DATA, v); \ } while (0) \
+#define RREG_SEQ(reg, v) \ + do { \ + WREG8(MGAREG_SEQ_INDEX, reg); \ + v = RREG8(MGAREG_SEQ_DATA); \ + } while (0) \ + #define WREG_SEQ(reg, v) \ do { \ WREG8(MGAREG_SEQ_INDEX, reg); \ diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 38556f57ad218..68ae604926757 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1106,6 +1106,33 @@ static void mgag200_set_format_regs(struct mga_device *mdev, WREG_ECRT(3, crtcext3); }
+static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev) +{ + static uint32_t RESET_FLAG = 0x00200000; /* undocumented magic value */ + u8 seq1; + u32 memctl; + + /* screen off */ + RREG_SEQ(0x01, seq1); + seq1 |= MGAREG_SEQ1_SCROFF; + WREG_SEQ(0x01, seq1); + + memctl = RREG32(MGAREG_MEMCTL); + + memctl |= RESET_FLAG; + WREG32(MGAREG_MEMCTL, memctl); + + udelay(1000); + + memctl &= ~RESET_FLAG; + WREG32(MGAREG_MEMCTL, memctl); + + /* screen on */ + RREG_SEQ(0x01, seq1); + seq1 &= ~MGAREG_SEQ1_SCROFF; + WREG_SEQ(0x01, seq1); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -1240,22 +1267,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
mgag200_set_mode_regs(mdev, mode);
- /* reset tagfifo */ - if (mdev->type == G200_ER) { - u32 mem_ctl = RREG32(MGAREG_MEMCTL); - u8 seq1; - - /* screen off */ - WREG8(MGAREG_SEQ_INDEX, 0x01); - seq1 = RREG8(MGAREG_SEQ_DATA) | 0x20; - WREG8(MGAREG_SEQ_DATA, seq1); - - WREG32(MGAREG_MEMCTL, mem_ctl | 0x00200000); - udelay(1000); - WREG32(MGAREG_MEMCTL, mem_ctl & ~0x00200000); - - WREG8(MGAREG_SEQ_DATA, seq1 & ~0x20); - } + if (mdev->type == G200_ER) + mgag200_g200er_reset_tagfifo(mdev);
if (IS_G200_SE(mdev)) { diff --git a/drivers/gpu/drm/mgag200/mgag200_reg.h b/drivers/gpu/drm/mgag200/mgag200_reg.h index cd08dee29b06f..29f7194faadc0 100644 --- a/drivers/gpu/drm/mgag200/mgag200_reg.h +++ b/drivers/gpu/drm/mgag200/mgag200_reg.h @@ -235,6 +235,9 @@ /* MMIO VGA registers */ #define MGAREG_SEQ_INDEX 0x1fc4 #define MGAREG_SEQ_DATA 0x1fc5 + +#define MGAREG_SEQ1_SCROFF BIT(5) + #define MGAREG_CRTC_INDEX 0x1fd4 #define MGAREG_CRTC_DATA 0x1fd5 #define MGAREG_CRTCEXT_INDEX 0x1fde
The hiprivlvl settings are now updated in mgag200_g200se_set_hiprilvl() and mgag200_g200ev_set_hiprilvl().
v2: * replace uint8_t with u8
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/mgag200_mode.c | 98 ++++++++++++++------------ 1 file changed, 54 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 68ae604926757..46122fa319889 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1133,6 +1133,56 @@ static void mgag200_g200er_reset_tagfifo(struct mga_device *mdev) WREG_SEQ(0x01, seq1); }
+static void mgag200_g200se_set_hiprilvl(struct mga_device *mdev, + const struct drm_display_mode *mode, + const struct drm_framebuffer *fb) +{ + unsigned int hiprilvl; + u8 crtcext6; + + if (mdev->unique_rev_id >= 0x04) { + hiprilvl = 0; + } else if (mdev->unique_rev_id >= 0x02) { + unsigned int bpp; + unsigned long mb; + + if (fb->format->cpp[0] * 8 > 16) + bpp = 32; + else if (fb->format->cpp[0] * 8 > 8) + bpp = 16; + else + bpp = 8; + + mb = (mode->clock * bpp) / 1000; + if (mb > 3100) + hiprilvl = 0; + else if (mb > 2600) + hiprilvl = 1; + else if (mb > 1900) + hiprilvl = 2; + else if (mb > 1160) + hiprilvl = 3; + else if (mb > 440) + hiprilvl = 4; + else + hiprilvl = 5; + + } else if (mdev->unique_rev_id >= 0x01) { + hiprilvl = 3; + } else { + hiprilvl = 4; + } + + crtcext6 = hiprilvl; /* implicitly sets maxhipri to 0 */ + + WREG_ECRT(0x06, crtcext6); +} + +static void mgag200_g200ev_set_hiprilvl(struct mga_device *mdev) +{ + WREG_ECRT(0x06, 0x00); +} + static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, @@ -1251,10 +1301,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, if (mdev->type == G200_EW3) WREG_ECRT(0x34, 0x5);
- if (mdev->type == G200_EV) { - WREG_ECRT(6, 0); - } - misc = RREG8(MGA_MISC_IN); misc |= MGAREG_MISC_IOADSEL | MGAREG_MISC_RAMMAPEN | @@ -1270,47 +1316,11 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, if (mdev->type == G200_ER) mgag200_g200er_reset_tagfifo(mdev);
+ if (IS_G200_SE(mdev)) + mgag200_g200se_set_hiprilvl(mdev, mode, fb); + else if (mdev->type == G200_EV) + mgag200_g200ev_set_hiprilvl(mdev);
- if (IS_G200_SE(mdev)) { - if (mdev->unique_rev_id >= 0x04) { - WREG8(MGAREG_CRTCEXT_INDEX, 0x06); - WREG8(MGAREG_CRTCEXT_DATA, 0); - } else if (mdev->unique_rev_id >= 0x02) { - u8 hi_pri_lvl; - u32 bpp; - u32 mb; - - if (fb->format->cpp[0] * 8 > 16) - bpp = 32; - else if (fb->format->cpp[0] * 8 > 8) - bpp = 16; - else - bpp = 8; - - mb = (mode->clock * bpp) / 1000; - if (mb > 3100) - hi_pri_lvl = 0; - else if (mb > 2600) - hi_pri_lvl = 1; - else if (mb > 1900) - hi_pri_lvl = 2; - else if (mb > 1160) - hi_pri_lvl = 3; - else if (mb > 440) - hi_pri_lvl = 4; - else - hi_pri_lvl = 5; - - WREG8(MGAREG_CRTCEXT_INDEX, 0x06); - WREG8(MGAREG_CRTCEXT_DATA, hi_pri_lvl); - } else { - WREG8(MGAREG_CRTCEXT_INDEX, 0x06); - if (mdev->unique_rev_id >= 0x01) - WREG8(MGAREG_CRTCEXT_DATA, 0x03); - else - WREG8(MGAREG_CRTCEXT_DATA, 0x04); - } - } return 0; }
Registers are initialized with constants. This is now done in mgag200_init_regs(), mgag200_set_dac_regs() and mgag200_set_pci_regs(). Later patches should move these calls from mode setting to device initialization.
v2: * replace uint8_t with u8
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/mgag200_mode.c | 261 ++++++++++++++----------- 1 file changed, 147 insertions(+), 114 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 46122fa319889..199ae08976e16 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -919,6 +919,152 @@ static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, return mga_crtc_do_set_base(mdev, fb, old_fb); }
+static void mgag200_set_pci_regs(struct mga_device *mdev) +{ + uint32_t option = 0, option2 = 0; + struct drm_device *dev = mdev->dev; + + switch (mdev->type) { + case G200_SE_A: + case G200_SE_B: + if (mdev->has_sdram) + option = 0x40049120; + else + option = 0x4004d120; + option2 = 0x00008000; + break; + case G200_WB: + case G200_EW3: + option = 0x41049120; + option2 = 0x0000b000; + break; + case G200_EV: + option = 0x00000120; + option2 = 0x0000b000; + break; + case G200_EH: + case G200_EH3: + option = 0x00000120; + option2 = 0x0000b000; + break; + case G200_ER: + break; + } + + if (option) + pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option); + + if (option2) + pci_write_config_dword(dev->pdev, PCI_MGA_OPTION2, option2); +} + +static void mgag200_set_dac_regs(struct mga_device *mdev) +{ + size_t i; + u8 dacvalue[] = { + /* 0x00: */ 0, 0, 0, 0, 0, 0, 0x00, 0, + /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0, + /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0, + /* 0x18: */ 0x00, 0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20, + /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40, + /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83, + /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A, + /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0, + /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0 + }; + + switch (mdev->type) { + case G200_SE_A: + case G200_SE_B: + dacvalue[MGA1064_VREF_CTL] = 0x03; + dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL; + dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_DAC_EN | + MGA1064_MISC_CTL_VGA8 | + MGA1064_MISC_CTL_DAC_RAM_CS; + break; + case G200_WB: + case G200_EW3: + dacvalue[MGA1064_VREF_CTL] = 0x07; + break; + case G200_EV: + dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL; + dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 | + MGA1064_MISC_CTL_DAC_RAM_CS; + break; + case G200_EH: + case G200_EH3: + dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 | + MGA1064_MISC_CTL_DAC_RAM_CS; + break; + case G200_ER: + break; + } + + for (i = 0; i < ARRAY_SIZE(dacvalue); i++) { + if ((i <= 0x17) || + (i == 0x1b) || + (i == 0x1c) || + ((i >= 0x1f) && (i <= 0x29)) || + ((i >= 0x30) && (i <= 0x37))) + continue; + if (IS_G200_SE(mdev) && + ((i == 0x2c) || (i == 0x2d) || (i == 0x2e))) + continue; + if ((mdev->type == G200_EV || + mdev->type == G200_WB || + mdev->type == G200_EH || + mdev->type == G200_EW3 || + mdev->type == G200_EH3) && + (i >= 0x44) && (i <= 0x4e)) + continue; + + WREG_DAC(i, dacvalue[i]); + } + + if (mdev->type == G200_ER) + WREG_DAC(0x90, 0); +} + +static void mgag200_init_regs(struct mga_device *mdev) +{ + u8 crtcext3, crtcext4, misc; + + mgag200_set_pci_regs(mdev); + mgag200_set_dac_regs(mdev); + + WREG_SEQ(2, 0x0f); + WREG_SEQ(3, 0x00); + WREG_SEQ(4, 0x0e); + + WREG_CRT(10, 0); + WREG_CRT(11, 0); + WREG_CRT(12, 0); + WREG_CRT(13, 0); + WREG_CRT(14, 0); + WREG_CRT(15, 0); + + RREG_ECRT(0x03, crtcext3); + + crtcext3 |= BIT(7); /* enable MGA mode */ + crtcext4 = 0x00; + + WREG_ECRT(0x03, crtcext3); + WREG_ECRT(0x04, crtcext4); + + if (mdev->type == G200_ER) + WREG_ECRT(0x24, 0x5); + + if (mdev->type == G200_EW3) + WREG_ECRT(0x34, 0x5); + + misc = RREG8(MGA_MISC_IN); + misc |= MGAREG_MISC_IOADSEL | + MGAREG_MISC_RAMMAPEN | + MGAREG_MISC_HIGH_PG_SEL; + WREG8(MGA_MISC_OUT, misc); +} + static void mgag200_set_mode_regs(struct mga_device *mdev, const struct drm_display_mode *mode) { @@ -1191,121 +1337,8 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); const struct drm_framebuffer *fb = crtc->primary->fb; - int option = 0, option2 = 0; - int i; - unsigned char misc = 0; - u8 crtcext3, crtcext4; - - static unsigned char dacvalue[] = { - /* 0x00: */ 0, 0, 0, 0, 0, 0, 0x00, 0, - /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0, - /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0, - /* 0x18: */ 0x00, 0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20, - /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40, - /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83, - /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A, - /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0, - /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0 - };
- switch (mdev->type) { - case G200_SE_A: - case G200_SE_B: - dacvalue[MGA1064_VREF_CTL] = 0x03; - dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL; - dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_DAC_EN | - MGA1064_MISC_CTL_VGA8 | - MGA1064_MISC_CTL_DAC_RAM_CS; - if (mdev->has_sdram) - option = 0x40049120; - else - option = 0x4004d120; - option2 = 0x00008000; - break; - case G200_WB: - case G200_EW3: - dacvalue[MGA1064_VREF_CTL] = 0x07; - option = 0x41049120; - option2 = 0x0000b000; - break; - case G200_EV: - dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL; - dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 | - MGA1064_MISC_CTL_DAC_RAM_CS; - option = 0x00000120; - option2 = 0x0000b000; - break; - case G200_EH: - case G200_EH3: - dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 | - MGA1064_MISC_CTL_DAC_RAM_CS; - option = 0x00000120; - option2 = 0x0000b000; - break; - case G200_ER: - break; - } - - for (i = 0; i < sizeof(dacvalue); i++) { - if ((i <= 0x17) || - (i == 0x1b) || - (i == 0x1c) || - ((i >= 0x1f) && (i <= 0x29)) || - ((i >= 0x30) && (i <= 0x37))) - continue; - if (IS_G200_SE(mdev) && - ((i == 0x2c) || (i == 0x2d) || (i == 0x2e))) - continue; - if ((mdev->type == G200_EV || - mdev->type == G200_WB || - mdev->type == G200_EH || - mdev->type == G200_EW3 || - mdev->type == G200_EH3) && - (i >= 0x44) && (i <= 0x4e)) - continue; - - WREG_DAC(i, dacvalue[i]); - } - - if (mdev->type == G200_ER) - WREG_DAC(0x90, 0); - - if (option) - pci_write_config_dword(dev->pdev, PCI_MGA_OPTION, option); - if (option2) - pci_write_config_dword(dev->pdev, PCI_MGA_OPTION2, option2); - - WREG_SEQ(2, 0xf); - WREG_SEQ(3, 0); - WREG_SEQ(4, 0xe); - - WREG_CRT(10, 0); - WREG_CRT(11, 0); - WREG_CRT(12, 0); - WREG_CRT(13, 0); - WREG_CRT(14, 0); - WREG_CRT(15, 0); - - RREG_ECRT(0x03, crtcext3); - - crtcext3 |= BIT(7); /* enable MGA mode */ - crtcext4 = 0x00; - - WREG_ECRT(0x03, crtcext3); - WREG_ECRT(0x04, crtcext4); - - if (mdev->type == G200_ER) - WREG_ECRT(0x24, 0x5); - - if (mdev->type == G200_EW3) - WREG_ECRT(0x34, 0x5); - - misc = RREG8(MGA_MISC_IN); - misc |= MGAREG_MISC_IOADSEL | - MGAREG_MISC_RAMMAPEN | - MGAREG_MISC_HIGH_PG_SEL; - WREG8(MGA_MISC_OUT, misc); + mgag200_init_regs(mdev);
mgag200_set_format_regs(mdev, fb); mga_crtc_do_set_base(mdev, fb, old_fb);
The suspend/resume helpers are unused. Also remove associated state from struct mga_device.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/mgag200_drv.h | 1 - drivers/gpu/drm/mgag200/mgag200_mode.c | 71 -------------------------- 2 files changed, 72 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index cf71a4ec84158..0cf498d1e900c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -164,7 +164,6 @@ struct mga_device {
size_t vram_fb_available;
- bool suspended; enum mga_type type; int has_sdram;
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 199ae08976e16..d6f9763a4a450 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1357,65 +1357,6 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc, return 0; }
-#if 0 /* code from mjg to attempt D3 on crtc dpms off - revisit later */ -static int mga_suspend(struct drm_crtc *crtc) -{ - struct mga_crtc *mga_crtc = to_mga_crtc(crtc); - struct drm_device *dev = crtc->dev; - struct mga_device *mdev = dev->dev_private; - struct pci_dev *pdev = dev->pdev; - int option; - - if (mdev->suspended) - return 0; - - WREG_SEQ(1, 0x20); - WREG_ECRT(1, 0x30); - /* Disable the pixel clock */ - WREG_DAC(0x1a, 0x05); - /* Power down the DAC */ - WREG_DAC(0x1e, 0x18); - /* Power down the pixel PLL */ - WREG_DAC(0x1a, 0x0d); - - /* Disable PLLs and clocks */ - pci_read_config_dword(pdev, PCI_MGA_OPTION, &option); - option &= ~(0x1F8024); - pci_write_config_dword(pdev, PCI_MGA_OPTION, option); - pci_set_power_state(pdev, PCI_D3hot); - pci_disable_device(pdev); - - mdev->suspended = true; - - return 0; -} - -static int mga_resume(struct drm_crtc *crtc) -{ - struct mga_crtc *mga_crtc = to_mga_crtc(crtc); - struct drm_device *dev = crtc->dev; - struct mga_device *mdev = dev->dev_private; - struct pci_dev *pdev = dev->pdev; - int option; - - if (!mdev->suspended) - return 0; - - pci_set_power_state(pdev, PCI_D0); - pci_enable_device(pdev); - - /* Disable sysclk */ - pci_read_config_dword(pdev, PCI_MGA_OPTION, &option); - option &= ~(0x4); - pci_write_config_dword(pdev, PCI_MGA_OPTION, option); - - mdev->suspended = false; - - return 0; -} - -#endif - static void mga_crtc_dpms(struct drm_crtc *crtc, int mode) { struct drm_device *dev = crtc->dev; @@ -1442,11 +1383,6 @@ static void mga_crtc_dpms(struct drm_crtc *crtc, int mode) break; }
-#if 0 - if (mode == DRM_MODE_DPMS_OFF) { - mga_suspend(crtc); - } -#endif WREG8(MGAREG_SEQ_INDEX, 0x01); seq1 |= RREG8(MGAREG_SEQ_DATA) & ~0x20; mga_wait_vsync(mdev); @@ -1456,13 +1392,6 @@ static void mga_crtc_dpms(struct drm_crtc *crtc, int mode) WREG8(MGAREG_CRTCEXT_INDEX, 0x01); crtcext1 |= RREG8(MGAREG_CRTCEXT_DATA) & ~0x30; WREG8(MGAREG_CRTCEXT_DATA, crtcext1); - -#if 0 - if (mode == DRM_MODE_DPMS_ON && mdev->suspended == true) { - mga_resume(crtc); - drm_helper_resume_force_mode(dev); - } -#endif }
/*
Hi Thomas,
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
The suspend/resume helpers are unused. Also remove associated state from struct mga_device.
Although DPMS in it's traditional sense is no longer a thing, would it make sense to keep this around for documentation purposes? In particular, the pci magic and associated PLL/DAC/pixel clock could be used for dynamic PM.
-Emil
Hi
Am 12.05.20 um 12:14 schrieb Emil Velikov:
Hi Thomas,
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
The suspend/resume helpers are unused. Also remove associated state from struct mga_device.
Although DPMS in it's traditional sense is no longer a thing, would it make sense to keep this around for documentation purposes? In particular, the pci magic and associated PLL/DAC/pixel clock could be used for dynamic PM.
That patch is not about DPMS. The DPMS code is still there. The suspend/resume helpers were outcommented and I don't know if they ever worked. Let's remove them.
Best regards Thomas
-Emil
On Tue, 12 May 2020 at 19:47, Thomas Zimmermann tzimmermann@suse.de wrote:
Hi
Am 12.05.20 um 12:14 schrieb Emil Velikov:
Hi Thomas,
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
The suspend/resume helpers are unused. Also remove associated state from struct mga_device.
Although DPMS in it's traditional sense is no longer a thing, would it make sense to keep this around for documentation purposes? In particular, the pci magic and associated PLL/DAC/pixel clock could be used for dynamic PM.
That patch is not about DPMS. The DPMS code is still there. The suspend/resume helpers were outcommented and I don't know if they ever worked. Let's remove them.
Seems like the idea is to suspend/resume the device on DPMS off/on. A rather moot point IMHO. As the DPMS semantics and the whole modeset, got more subtle with atomic modeset, the idea gets even more moot.
If the documentation has that process - sure nuke it. Although for dynPM, this code is essential. Considering a) one has interest in dynPM and b) the code is (close to) working.
The last two are very big ifs so I'll leave it there.
-Emil
On Wed, May 13, 2020 at 10:15:25AM +0100, Emil Velikov wrote:
On Tue, 12 May 2020 at 19:47, Thomas Zimmermann tzimmermann@suse.de wrote:
Hi
Am 12.05.20 um 12:14 schrieb Emil Velikov:
Hi Thomas,
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
The suspend/resume helpers are unused. Also remove associated state from struct mga_device.
Although DPMS in it's traditional sense is no longer a thing, would it make sense to keep this around for documentation purposes? In particular, the pci magic and associated PLL/DAC/pixel clock could be used for dynamic PM.
That patch is not about DPMS. The DPMS code is still there. The suspend/resume helpers were outcommented and I don't know if they ever worked. Let's remove them.
Seems like the idea is to suspend/resume the device on DPMS off/on. A rather moot point IMHO. As the DPMS semantics and the whole modeset, got more subtle with atomic modeset, the idea gets even more moot.
With atomic it's actually a lot easier to do runtime pm in your modesetting code, since the flow is a lot more structured. There's even a helper function for that with drm_atomic_helper_commit_tail_rpm, since the default is optimized for max compatability with old legacy helpers. Maybe we should switch that around actually. -Daniel
If the documentation has that process - sure nuke it. Although for dynPM, this code is essential. Considering a) one has interest in dynPM and b) the code is (close to) working.
The last two are very big ifs so I'll leave it there.
-Emil
The MGA CRTC data structure struct mga_crtc contains unused additional fields; so it can removed. The standard DRM CRTC and encoder structures are embedded now in struct drm_simple_display_pipe. Done in preparation of converting mgag200 to simple KMS helpers.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/mgag200_drv.h | 11 ++-------- drivers/gpu/drm/mgag200/mgag200_mode.c | 28 ++++++-------------------- 2 files changed, 8 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 0cf498d1e900c..2392baff618aa 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -19,6 +19,7 @@ #include <drm/drm_fb_helper.h> #include <drm/drm_gem.h> #include <drm/drm_gem_vram_helper.h> +#include <drm/drm_simple_kms_helper.h>
#include "mgag200_reg.h"
@@ -105,16 +106,8 @@
#define MATROX_DPMS_CLEARED (-1)
-#define to_mga_crtc(x) container_of(x, struct mga_crtc, base) #define to_mga_connector(x) container_of(x, struct mga_connector, base)
-struct mga_crtc { - struct drm_crtc base; - u8 lut_r[256], lut_g[256], lut_b[256]; - int last_dpms; - bool enabled; -}; - struct mga_i2c_chan { struct i2c_adapter adapter; struct drm_device *dev; @@ -175,7 +168,7 @@ struct mga_device { u32 unique_rev_id;
struct mga_connector connector; - struct drm_encoder encoder; + struct drm_simple_display_pipe display_pipe; };
static inline struct mga_device *to_mga_device(struct drm_device *dev) diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index d6f9763a4a450..00bbc1f9b7db3 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1475,15 +1475,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, return 0; }
-/* Simple cleanup function */ -static void mga_crtc_destroy(struct drm_crtc *crtc) -{ - struct mga_crtc *mga_crtc = to_mga_crtc(crtc); - - drm_crtc_cleanup(crtc); - kfree(mga_crtc); -} - static void mga_crtc_disable(struct drm_crtc *crtc) { DRM_DEBUG_KMS("\n"); @@ -1501,7 +1492,7 @@ static void mga_crtc_disable(struct drm_crtc *crtc) static const struct drm_crtc_funcs mga_crtc_funcs = { .gamma_set = mga_crtc_gamma_set, .set_config = drm_crtc_helper_set_config, - .destroy = mga_crtc_destroy, + .destroy = drm_crtc_cleanup, };
static const struct drm_crtc_helper_funcs mga_helper_funcs = { @@ -1517,20 +1508,13 @@ static const struct drm_crtc_helper_funcs mga_helper_funcs = { static void mga_crtc_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev; - struct mga_crtc *mga_crtc; - - mga_crtc = kzalloc(sizeof(struct mga_crtc) + - (MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)), - GFP_KERNEL); - - if (mga_crtc == NULL) - return; + struct drm_crtc *crtc = &mdev->display_pipe.crtc;
- drm_crtc_init(dev, &mga_crtc->base, &mga_crtc_funcs); + drm_crtc_init(dev, crtc, &mga_crtc_funcs);
- drm_mode_crtc_set_gamma_size(&mga_crtc->base, MGAG200_LUT_SIZE); + drm_mode_crtc_set_gamma_size(crtc, MGAG200_LUT_SIZE);
- drm_crtc_helper_add(&mga_crtc->base, &mga_helper_funcs); + drm_crtc_helper_add(crtc, &mga_helper_funcs); }
/* @@ -1718,7 +1702,7 @@ static unsigned int mgag200_preferred_depth(struct mga_device *mdev) int mgag200_modeset_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev; - struct drm_encoder *encoder = &mdev->encoder; + struct drm_encoder *encoder = &mdev->display_pipe.encoder; struct drm_connector *connector = &mdev->connector.base; int ret;
Hi Thomas,
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
static void mga_crtc_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev;
struct mga_crtc *mga_crtc;mga_crtc = kzalloc(sizeof(struct mga_crtc) +(MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)),
The #define MGAG200FB_CONN_LIMIT in mgag200_drv.h is no longer used, correct?
-Emil
Hi
Am 12.05.20 um 12:16 schrieb Emil Velikov:
Hi Thomas,
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
static void mga_crtc_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev;
struct mga_crtc *mga_crtc;mga_crtc = kzalloc(sizeof(struct mga_crtc) +(MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)),The #define MGAG200FB_CONN_LIMIT in mgag200_drv.h is no longer used, correct?
Good point. I'll remove the define.
Best regards Thomas
-Emil _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
The mgag200 supports a single pipeline with only a primary plane. It can be converted to simple KMS helpers. This also adds support for atomic modesetting. Wayland compositors, which use pageflip ioctls, can now be used with mgag200.
v2: * prepare encoder and CRTC in a separate patch * remove suspend/resume code in a separate patch * don't call set_format_regs() in pipe_update()
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 314 +++++++++++++------------ 2 files changed, 164 insertions(+), 152 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index c2f0e4b40b052..a06ce4198adea 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -140,7 +140,7 @@ int mgag200_driver_dumb_create(struct drm_file *file, }
static struct drm_driver driver = { - .driver_features = DRIVER_GEM | DRIVER_MODESET, + .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, .fops = &mgag200_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 00bbc1f9b7db3..b50c1beb7b7b9 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -11,10 +11,13 @@ #include <linux/delay.h> #include <linux/pci.h>
+#include <drm/drm_atomic_helper.h> +#include <drm/drm_atomic_state_helper.h> #include <drm/drm_crtc_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_plane_helper.h> +#include <drm/drm_print.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h>
@@ -30,13 +33,18 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); - struct drm_framebuffer *fb = crtc->primary->fb; + struct drm_framebuffer *fb; u16 *r_ptr, *g_ptr, *b_ptr; int i;
if (!crtc->enabled) return;
+ if (!mdev->display_pipe.plane.state) + return; + + fb = mdev->display_pipe.plane.state->fb; + r_ptr = crtc->gamma_store; g_ptr = r_ptr + crtc->gamma_size; b_ptr = g_ptr + crtc->gamma_size; @@ -869,56 +877,6 @@ static void mgag200_set_startadd(struct mga_device *mdev, WREG_ECRT(0x00, crtcext0); }
-static int mga_crtc_do_set_base(struct mga_device *mdev, - const struct drm_framebuffer *fb, - const struct drm_framebuffer *old_fb) -{ - struct drm_gem_vram_object *gbo; - int ret; - s64 gpu_addr; - - if (old_fb) { - gbo = drm_gem_vram_of_gem(old_fb->obj[0]); - drm_gem_vram_unpin(gbo); - } - - gbo = drm_gem_vram_of_gem(fb->obj[0]); - - ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM); - if (ret) - return ret; - gpu_addr = drm_gem_vram_offset(gbo); - if (gpu_addr < 0) { - ret = (int)gpu_addr; - goto err_drm_gem_vram_unpin; - } - - mgag200_set_startadd(mdev, (unsigned long)gpu_addr); - - return 0; - -err_drm_gem_vram_unpin: - drm_gem_vram_unpin(gbo); - return ret; -} - -static int mga_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, - struct drm_framebuffer *old_fb) -{ - struct drm_device *dev = crtc->dev; - struct mga_device *mdev = dev->dev_private; - struct drm_framebuffer *fb = crtc->primary->fb; - unsigned int count; - - do { } while (RREG8(0x1fda) & 0x08); - do { } while (!(RREG8(0x1fda) & 0x08)); - - count = RREG8(MGAREG_VCOUNT) + 2; - do { } while (RREG8(MGAREG_VCOUNT) < count); - - return mga_crtc_do_set_base(mdev, fb, old_fb); -} - static void mgag200_set_pci_regs(struct mga_device *mdev) { uint32_t option = 0, option2 = 0; @@ -1329,34 +1287,6 @@ static void mgag200_g200ev_set_hiprilvl(struct mga_device *mdev) WREG_ECRT(0x06, 0x00); }
-static int mga_crtc_mode_set(struct drm_crtc *crtc, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode, - int x, int y, struct drm_framebuffer *old_fb) -{ - struct drm_device *dev = crtc->dev; - struct mga_device *mdev = to_mga_device(dev); - const struct drm_framebuffer *fb = crtc->primary->fb; - - mgag200_init_regs(mdev); - - mgag200_set_format_regs(mdev, fb); - mga_crtc_do_set_base(mdev, fb, old_fb); - mgag200_set_offset(mdev, fb); - - mgag200_set_mode_regs(mdev, mode); - - if (mdev->type == G200_ER) - mgag200_g200er_reset_tagfifo(mdev); - - if (IS_G200_SE(mdev)) - mgag200_g200se_set_hiprilvl(mdev, mode, fb); - else if (mdev->type == G200_EV) - mgag200_g200ev_set_hiprilvl(mdev); - - return 0; -} - static void mga_crtc_dpms(struct drm_crtc *crtc, int mode) { struct drm_device *dev = crtc->dev; @@ -1439,7 +1369,6 @@ static void mga_crtc_commit(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct mga_device *mdev = to_mga_device(dev); - const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; u8 tmp;
if (mdev->type == G200_WB || mdev->type == G200_EW3) @@ -1458,63 +1387,7 @@ static void mga_crtc_commit(struct drm_crtc *crtc) WREG_SEQ(0x1, tmp); WREG_SEQ(0, 3); } - crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON); -} - -/* - * The core can pass us a set of gamma values to program. We actually only - * use this for 8-bit mode so can't perform smooth fades on deeper modes, - * but it's a requirement that we provide the function - */ -static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, - u16 *blue, uint32_t size, - struct drm_modeset_acquire_ctx *ctx) -{ - mga_crtc_load_lut(crtc); - - return 0; -} - -static void mga_crtc_disable(struct drm_crtc *crtc) -{ - DRM_DEBUG_KMS("\n"); - mga_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); - if (crtc->primary->fb) { - struct drm_framebuffer *fb = crtc->primary->fb; - struct drm_gem_vram_object *gbo = - drm_gem_vram_of_gem(fb->obj[0]); - drm_gem_vram_unpin(gbo); - } - crtc->primary->fb = NULL; -} - -/* These provide the minimum set of functions required to handle a CRTC */ -static const struct drm_crtc_funcs mga_crtc_funcs = { - .gamma_set = mga_crtc_gamma_set, - .set_config = drm_crtc_helper_set_config, - .destroy = drm_crtc_cleanup, -}; - -static const struct drm_crtc_helper_funcs mga_helper_funcs = { - .disable = mga_crtc_disable, - .dpms = mga_crtc_dpms, - .mode_set = mga_crtc_mode_set, - .mode_set_base = mga_crtc_mode_set_base, - .prepare = mga_crtc_prepare, - .commit = mga_crtc_commit, -}; - -/* CRTC setup */ -static void mga_crtc_init(struct mga_device *mdev) -{ - struct drm_device *dev = mdev->dev; - struct drm_crtc *crtc = &mdev->display_pipe.crtc; - - drm_crtc_init(dev, crtc, &mga_crtc_funcs); - - drm_mode_crtc_set_gamma_size(crtc, MGAG200_LUT_SIZE); - - drm_crtc_helper_add(crtc, &mga_helper_funcs); + mga_crtc_dpms(crtc, DRM_MODE_DPMS_ON); }
/* @@ -1648,14 +1521,16 @@ static void mga_connector_destroy(struct drm_connector *connector) }
static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = { - .get_modes = mga_vga_get_modes, + .get_modes = mga_vga_get_modes, .mode_valid = mga_vga_mode_valid, };
static const struct drm_connector_funcs mga_vga_connector_funcs = { - .dpms = drm_helper_connector_dpms, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = mga_connector_destroy, + .reset = drm_atomic_helper_connector_reset, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = mga_connector_destroy, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, };
static int mgag200_vga_connector_init(struct mga_device *mdev) @@ -1687,8 +1562,137 @@ static int mgag200_vga_connector_init(struct mga_device *mdev) return ret; }
+/* + * Simple Display Pipe + */ + +static enum drm_mode_status +mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe *pipe, + const struct drm_display_mode *mode) +{ + return MODE_OK; +} + +static void +mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) +{ + struct drm_crtc *crtc = &pipe->crtc; + struct drm_device *dev = crtc->dev; + struct mga_device *mdev = to_mga_device(dev); + struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; + struct drm_framebuffer *fb = plane_state->fb; + struct drm_gem_vram_object *gbo; + s64 gpu_addr; + + gbo = drm_gem_vram_of_gem(fb->obj[0]); + + gpu_addr = drm_gem_vram_offset(gbo); + if (drm_WARN_ON_ONCE(dev, gpu_addr < 0)) + return; /* BUG: BO should have been pinned to VRAM. */ + + mga_crtc_prepare(crtc); + + mgag200_set_format_regs(mdev, fb); + mgag200_set_mode_regs(mdev, adjusted_mode); + + if (mdev->type == G200_ER) + mgag200_g200er_reset_tagfifo(mdev); + + if (IS_G200_SE(mdev)) + mgag200_g200se_set_hiprilvl(mdev, adjusted_mode, fb); + else if (mdev->type == G200_EV) + mgag200_g200ev_set_hiprilvl(mdev); + + mga_crtc_commit(crtc); +} + +static void +mgag200_simple_display_pipe_disable(struct drm_simple_display_pipe *pipe) +{ + struct drm_crtc *crtc = &pipe->crtc; + + mga_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); +} + +static int +mgag200_simple_display_pipe_check(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *plane_state, + struct drm_crtc_state *crtc_state) +{ + struct drm_plane *plane = plane_state->plane; + struct drm_framebuffer *new_fb = plane_state->fb; + struct drm_framebuffer *fb = NULL; + + if (!new_fb) + return 0; + + if (plane->state) + fb = plane->state->fb; + + if (!fb || (fb->format != new_fb->format)) + crtc_state->mode_changed = true; /* update PLL settings */ + + return 0; +} + +static void +mgag200_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, + struct drm_plane_state *old_state) +{ + struct drm_plane *plane = &pipe->plane; + struct drm_device *dev = plane->dev; + struct mga_device *mdev = to_mga_device(dev); + struct drm_plane_state *state = plane->state; + struct drm_framebuffer *fb = state->fb; + struct drm_gem_vram_object *gbo; + s64 gpu_addr; + + if (!fb) + return; + + gbo = drm_gem_vram_of_gem(fb->obj[0]); + + gpu_addr = drm_gem_vram_offset(gbo); + if (drm_WARN_ON_ONCE(dev, gpu_addr < 0)) + return; /* BUG: BO should have been pinned to VRAM. */ + + mgag200_set_startadd(mdev, (unsigned long)gpu_addr); + mgag200_set_offset(mdev, fb); +} + +static const struct drm_simple_display_pipe_funcs +mgag200_simple_display_pipe_funcs = { + .mode_valid = mgag200_simple_display_pipe_mode_valid, + .enable = mgag200_simple_display_pipe_enable, + .disable = mgag200_simple_display_pipe_disable, + .check = mgag200_simple_display_pipe_check, + .update = mgag200_simple_display_pipe_update, + .prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb, + .cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb, +}; + +static const uint32_t mgag200_simple_display_pipe_formats[] = { + DRM_FORMAT_XRGB8888, + DRM_FORMAT_RGB565, + DRM_FORMAT_RGB888, +}; + +static const uint64_t mgag200_simple_display_pipe_fmtmods[] = { + DRM_FORMAT_MOD_LINEAR, + DRM_FORMAT_MOD_INVALID +}; + +/* + * Mode config + */ + static const struct drm_mode_config_funcs mgag200_mode_config_funcs = { - .fb_create = drm_gem_fb_create + .fb_create = drm_gem_fb_create, + .mode_valid = drm_vram_helper_mode_valid, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, };
static unsigned int mgag200_preferred_depth(struct mga_device *mdev) @@ -1702,8 +1706,9 @@ static unsigned int mgag200_preferred_depth(struct mga_device *mdev) int mgag200_modeset_init(struct mga_device *mdev) { struct drm_device *dev = mdev->dev; - struct drm_encoder *encoder = &mdev->display_pipe.encoder; struct drm_connector *connector = &mdev->connector.base; + struct drm_simple_display_pipe *pipe = &mdev->display_pipe; + size_t format_count = ARRAY_SIZE(mgag200_simple_display_pipe_formats); int ret;
mdev->bpp_shifts[0] = 0; @@ -1711,6 +1716,8 @@ int mgag200_modeset_init(struct mga_device *mdev) mdev->bpp_shifts[2] = 0; mdev->bpp_shifts[3] = 2;
+ mgag200_init_regs(mdev); + ret = drmm_mode_config_init(dev); if (ret) { drm_err(dev, "drmm_mode_config_init() failed, error %d\n", @@ -1728,26 +1735,31 @@ int mgag200_modeset_init(struct mga_device *mdev)
dev->mode_config.funcs = &mgag200_mode_config_funcs;
- mga_crtc_init(mdev); - - ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_DAC); + ret = mgag200_vga_connector_init(mdev); if (ret) { drm_err(dev, - "drm_simple_encoder_init() failed, error %d\n", + "mgag200_vga_connector_init() failed, error %d\n", ret); return ret; } - encoder->possible_crtcs = 0x1;
- ret = mgag200_vga_connector_init(mdev); + ret = drm_simple_display_pipe_init(dev, pipe, + &mgag200_simple_display_pipe_funcs, + mgag200_simple_display_pipe_formats, + format_count, + mgag200_simple_display_pipe_fmtmods, + connector); if (ret) { drm_err(dev, - "mgag200_vga_connector_init() failed, error %d\n", + "drm_simple_display_pipe_init() failed, error %d\n", ret); return ret; }
- drm_connector_attach_encoder(connector, encoder); + /* FIXME: legacy gamma tables; convert to CRTC state */ + drm_mode_crtc_set_gamma_size(&pipe->crtc, MGAG200_LUT_SIZE); + + drm_mode_config_reset(dev);
return 0; }
The VRAM helpers managed the framebuffer memory for mgag200. This came with several problems, as some MGA device require the scanout address to be located at VRAM offset 0. It's incompatible with the page-flip semantics of DRM's atomic modesettting. With atomic modesetting, old and new framebuffers have to be located in VRAM at the same time. So at least one of them has to reside at a non-0 offset.
This patch replaces VRAM helpers with SHMEM helpers. GEM SHMEM buffers reside in system memory, and are shadow-copied into VRAM during page flips. The shadow copy always starts at VRAM offset 0.
v2: * revert dev->pdev changes
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Tested-by: John Donnelly John.p.donnelly@oracle.com --- drivers/gpu/drm/mgag200/Kconfig | 4 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 49 +--------------------- drivers/gpu/drm/mgag200/mgag200_drv.h | 5 ++- drivers/gpu/drm/mgag200/mgag200_mode.c | 58 ++++++++++++++++---------- drivers/gpu/drm/mgag200/mgag200_ttm.c | 28 +++++++------ 5 files changed, 56 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/Kconfig b/drivers/gpu/drm/mgag200/Kconfig index d60aa4b9ccd47..93be766715c9b 100644 --- a/drivers/gpu/drm/mgag200/Kconfig +++ b/drivers/gpu/drm/mgag200/Kconfig @@ -2,10 +2,8 @@ config DRM_MGAG200 tristate "Kernel modesetting driver for MGA G200 server engines" depends on DRM && PCI && MMU + select DRM_GEM_SHMEM_HELPER select DRM_KMS_HELPER - select DRM_VRAM_HELPER - select DRM_TTM - select DRM_TTM_HELPER help This is a KMS driver for the MGA G200 server chips, it does not support the original MGA G200 or any of the desktop diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index a06ce4198adea..00ddea7d7d270 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -22,15 +22,11 @@ * which then performs further device association and calls our graphics init * functions */ -int mgag200_modeset = -1;
+int mgag200_modeset = -1; MODULE_PARM_DESC(modeset, "Disable/Enable modesetting"); module_param_named(modeset, mgag200_modeset, int, 0400);
-int mgag200_hw_bug_no_startadd = -1; -MODULE_PARM_DESC(modeset, "HW does not interpret scanout-buffer start address correctly"); -module_param_named(hw_bug_no_startadd, mgag200_hw_bug_no_startadd, int, 0400); - static struct drm_driver driver;
static const struct pci_device_id pciidlist[] = { @@ -101,44 +97,6 @@ static void mga_pci_remove(struct pci_dev *pdev)
DEFINE_DRM_GEM_FOPS(mgag200_driver_fops);
-static bool mgag200_pin_bo_at_0(const struct mga_device *mdev) -{ - if (mgag200_hw_bug_no_startadd > 0) { - DRM_WARN_ONCE("Option hw_bug_no_startradd is enabled. Please " - "report the output of 'lspci -vvnn' to " - "dri-devel@lists.freedesktop.org if this " - "option is required to make mgag200 work " - "correctly on your system.\n"); - return true; - } else if (!mgag200_hw_bug_no_startadd) { - return false; - } - return mdev->flags & MGAG200_FLAG_HW_BUG_NO_STARTADD; -} - -int mgag200_driver_dumb_create(struct drm_file *file, - struct drm_device *dev, - struct drm_mode_create_dumb *args) -{ - struct mga_device *mdev = to_mga_device(dev); - unsigned long pg_align; - - if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized")) - return -EINVAL; - - pg_align = 0ul; - - /* - * Aligning scanout buffers to the size of the video ram forces - * placement at offset 0. Works around a bug where HW does not - * respect 'startadd' field. - */ - if (mgag200_pin_bo_at_0(mdev)) - pg_align = PFN_UP(mdev->mc.vram_size); - - return drm_gem_vram_fill_create_dumb(file, dev, pg_align, 0, args); -} - static struct drm_driver driver = { .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, .fops = &mgag200_driver_fops, @@ -148,10 +106,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, - .debugfs_init = drm_vram_mm_debugfs_init, - .dumb_create = mgag200_driver_dumb_create, - .dumb_map_offset = drm_gem_vram_driver_dumb_mmap_offset, - .gem_prime_mmap = drm_gem_prime_mmap, + DRM_GEM_SHMEM_DRIVER_OPS, };
static struct pci_driver mgag200_pci_driver = { diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 2392baff618aa..d530df25c822d 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -18,7 +18,7 @@ #include <drm/drm_encoder.h> #include <drm/drm_fb_helper.h> #include <drm/drm_gem.h> -#include <drm/drm_gem_vram_helper.h> +#include <drm/drm_gem_shmem_helper.h> #include <drm/drm_simple_kms_helper.h>
#include "mgag200_reg.h" @@ -155,7 +155,8 @@ struct mga_device {
struct mga_mc mc;
- size_t vram_fb_available; + void __iomem *vram; + size_t vram_fb_available;
enum mga_type type; int has_sdram; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index b50c1beb7b7b9..0155d4eb5fa6b 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -14,6 +14,8 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_atomic_state_helper.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_damage_helper.h> +#include <drm/drm_format_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_plane_helper.h> @@ -1573,6 +1575,26 @@ mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe *pipe, return MODE_OK; }
+static void +mgag200_handle_damage(struct mga_device *mdev, struct drm_framebuffer *fb, + struct drm_rect *clip) +{ + struct drm_device *dev = mdev->dev; + void *vmap; + + vmap = drm_gem_shmem_vmap(fb->obj[0]); + if (drm_WARN_ON(dev, !vmap)) + return; /* BUG: SHMEM BO should always be vmapped */ + + drm_fb_memcpy_dstclip(mdev->vram, vmap, fb, clip); + + drm_gem_shmem_vunmap(fb->obj[0], vmap); + + /* Always scanout image at VRAM offset 0 */ + mgag200_set_startadd(mdev, (u32)0); + mgag200_set_offset(mdev, fb); +} + static void mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_crtc_state *crtc_state, @@ -1583,14 +1605,12 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, struct mga_device *mdev = to_mga_device(dev); struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode; struct drm_framebuffer *fb = plane_state->fb; - struct drm_gem_vram_object *gbo; - s64 gpu_addr; - - gbo = drm_gem_vram_of_gem(fb->obj[0]); - - gpu_addr = drm_gem_vram_offset(gbo); - if (drm_WARN_ON_ONCE(dev, gpu_addr < 0)) - return; /* BUG: BO should have been pinned to VRAM. */ + struct drm_rect fullscreen = { + .x1 = 0, + .x2 = fb->width, + .y1 = 0, + .y2 = fb->height, + };
mga_crtc_prepare(crtc);
@@ -1606,6 +1626,8 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, mgag200_g200ev_set_hiprilvl(mdev);
mga_crtc_commit(crtc); + + mgag200_handle_damage(mdev, fb, &fullscreen); }
static void @@ -1646,20 +1668,13 @@ mgag200_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, struct mga_device *mdev = to_mga_device(dev); struct drm_plane_state *state = plane->state; struct drm_framebuffer *fb = state->fb; - struct drm_gem_vram_object *gbo; - s64 gpu_addr; + struct drm_rect damage;
if (!fb) return;
- gbo = drm_gem_vram_of_gem(fb->obj[0]); - - gpu_addr = drm_gem_vram_offset(gbo); - if (drm_WARN_ON_ONCE(dev, gpu_addr < 0)) - return; /* BUG: BO should have been pinned to VRAM. */ - - mgag200_set_startadd(mdev, (unsigned long)gpu_addr); - mgag200_set_offset(mdev, fb); + if (drm_atomic_helper_damage_merged(old_state, state, &damage)) + mgag200_handle_damage(mdev, fb, &damage); }
static const struct drm_simple_display_pipe_funcs @@ -1669,8 +1684,7 @@ mgag200_simple_display_pipe_funcs = { .disable = mgag200_simple_display_pipe_disable, .check = mgag200_simple_display_pipe_check, .update = mgag200_simple_display_pipe_update, - .prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb, - .cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb, + .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, };
static const uint32_t mgag200_simple_display_pipe_formats[] = { @@ -1689,8 +1703,7 @@ static const uint64_t mgag200_simple_display_pipe_fmtmods[] = { */
static const struct drm_mode_config_funcs mgag200_mode_config_funcs = { - .fb_create = drm_gem_fb_create, - .mode_valid = drm_vram_helper_mode_valid, + .fb_create = drm_gem_fb_create_with_dirty, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, }; @@ -1729,7 +1742,6 @@ int mgag200_modeset_init(struct mga_device *mdev) dev->mode_config.max_height = MGAG200_MAX_FB_HEIGHT;
dev->mode_config.preferred_depth = mgag200_preferred_depth(mdev); - dev->mode_config.prefer_shadow = 1;
dev->mode_config.fb_base = mdev->mc.vram_base;
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c index e89657630ea71..a683642fe4682 100644 --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c @@ -32,17 +32,8 @@
int mgag200_mm_init(struct mga_device *mdev) { - struct drm_vram_mm *vmm; - int ret; struct drm_device *dev = mdev->dev; - - vmm = drm_vram_helper_alloc_mm(dev, pci_resource_start(dev->pdev, 0), - mdev->mc.vram_size); - if (IS_ERR(vmm)) { - ret = PTR_ERR(vmm); - DRM_ERROR("Error initializing VRAM MM; %d\n", ret); - return ret; - } + int ret;
arch_io_reserve_memtype_wc(pci_resource_start(dev->pdev, 0), pci_resource_len(dev->pdev, 0)); @@ -50,9 +41,22 @@ int mgag200_mm_init(struct mga_device *mdev) mdev->fb_mtrr = arch_phys_wc_add(pci_resource_start(dev->pdev, 0), pci_resource_len(dev->pdev, 0));
+ mdev->vram = ioremap(pci_resource_start(dev->pdev, 0), + pci_resource_len(dev->pdev, 0)); + if (!mdev->vram) { + ret = -ENOMEM; + goto err_arch_phys_wc_del; + } + mdev->vram_fb_available = mdev->mc.vram_size;
return 0; + +err_arch_phys_wc_del: + arch_phys_wc_del(mdev->fb_mtrr); + arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0), + pci_resource_len(dev->pdev, 0)); + return ret; }
void mgag200_mm_fini(struct mga_device *mdev) @@ -60,9 +64,7 @@ void mgag200_mm_fini(struct mga_device *mdev) struct drm_device *dev = mdev->dev;
mdev->vram_fb_available = 0; - - drm_vram_helper_release_mm(dev); - + iounmap(mdev->vram); arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0), pci_resource_len(dev->pdev, 0)); arch_phys_wc_del(mdev->fb_mtrr);
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
The VRAM helpers managed the framebuffer memory for mgag200. This came with several problems, as some MGA device require the scanout address to be located at VRAM offset 0. It's incompatible with the page-flip semantics of DRM's atomic modesettting. With atomic modesetting, old and new framebuffers have to be located in VRAM at the same time. So at least one of them has to reside at a non-0 offset.
This patch replaces VRAM helpers with SHMEM helpers. GEM SHMEM buffers reside in system memory, and are shadow-copied into VRAM during page flips. The shadow copy always starts at VRAM offset 0.
I suspect MGAG200_FLAG_HW_BUG_NO_STARTADD is left around for documentation purposes?
I've made a few small comments, but regardless the series is: Acked-by: Emil Velikov emil.velikov@collabora.com
-Emil
Hi
Am 12.05.20 um 12:30 schrieb Emil Velikov:
On Tue, 12 May 2020 at 09:43, Thomas Zimmermann tzimmermann@suse.de wrote:
The VRAM helpers managed the framebuffer memory for mgag200. This came with several problems, as some MGA device require the scanout address to be located at VRAM offset 0. It's incompatible with the page-flip semantics of DRM's atomic modesettting. With atomic modesetting, old and new framebuffers have to be located in VRAM at the same time. So at least one of them has to reside at a non-0 offset.
This patch replaces VRAM helpers with SHMEM helpers. GEM SHMEM buffers reside in system memory, and are shadow-copied into VRAM during page flips. The shadow copy always starts at VRAM offset 0.
I suspect MGAG200_FLAG_HW_BUG_NO_STARTADD is left around for documentation purposes?
Yes. For now, that flag seems to be the best documentation of the HW issue.
Besides, there's another known bug in several MGAs: scanlines may not cross the 4 MiB boundary. If we ever have to work around that problem, NO_STARTADD might become useful again. One can either scanout from 0, or align FB scanlines around the 4 MiB boundary; but not both.
I've made a few small comments, but regardless the series is: Acked-by: Emil Velikov emil.velikov@collabora.com
Thanks!
Best regards Thomas
-Emil
Hi Thomas.
On Tue, May 12, 2020 at 10:42:43AM +0200, Thomas Zimmermann wrote:
This patchset converts mgag200 to atomic modesetting. It uses simple KMS helpers and SHMEM.
Patch 1 removes cursor support. The HW cursor is not usable with the way universal planes work.
Patches 2 to 11 untangle the existing modesetting code into smaller functions. Specifically, mode setting and plane updates are being separated from each other.
Patch 12 to 14 convert mgag200 to simple KMS helpers and enables atomic mode setting.
Atomically switching plane framebuffers, requires either source or target buffer to be located at a non-0 offet. As some HW revisions seem to require a framebuffer offset of 0 within the video memory, they do not work with atomic modesetting. To resolve this problem, patch 15 converts mgag200 from VRAM helpers to SHMEM helpers. During plane updates, the content of the SHMEM BO is memcpy'd to VRAM. From my observation, performance is not nuch different from the original code.
The patchset has been tested on MGA G200EH hardware.
v2:
- rebase patchset
- replace uint{8,32}_t with u{8,32} through-out patchset
- define additional register constants
- use helper functions around bpp-shift computations
- split conversion patch
- cleanups
With the one comment addressed patch 1-14 are now all: Acked-by: Sam Ravnborg sam@ravnborg.org
I did not look at the last patch - all the memory stuff is still beyond me.
Nice to see this driver gettting so much love and care. The end result is a much nicer driver implmentation.
Sam
Thomas Zimmermann (15): drm/mgag200: Remove HW cursor drm/mgag200: Clean up mga_set_start_address() drm/mgag200: Clean up mga_crtc_do_set_base() drm/mgag200: Move mode-setting code into separate helper function drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O drm/mgag200: Update mode registers after plane registers drm/mgag200: Set pitch in a separate helper function drm/mgag200: Set primary plane's format in separate helper function drm/mgag200: Move TAGFIFO reset into separate function drm/mgag200: Move hiprilvl setting into separate functions drm/mgag200: Move register initialization into separate function drm/mgag200: Remove out-commented suspend/resume helpers drm/mgag200: Use simple-display data structures drm/mgag200: Convert to simple KMS helper drm/mgag200: Replace VRAM helpers with SHMEM helpers
drivers/gpu/drm/mgag200/Kconfig | 4 +- drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 51 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 41 +- drivers/gpu/drm/mgag200/mgag200_main.c | 5 - drivers/gpu/drm/mgag200/mgag200_mode.c | 871 ++++++++++++++----------- drivers/gpu/drm/mgag200/mgag200_reg.h | 11 +- drivers/gpu/drm/mgag200/mgag200_ttm.c | 28 +- 8 files changed, 528 insertions(+), 485 deletions(-)
-- 2.26.2
Hi
Am 12.05.20 um 20:56 schrieb Sam Ravnborg:
Hi Thomas.
On Tue, May 12, 2020 at 10:42:43AM +0200, Thomas Zimmermann wrote:
This patchset converts mgag200 to atomic modesetting. It uses simple KMS helpers and SHMEM.
Patch 1 removes cursor support. The HW cursor is not usable with the way universal planes work.
Patches 2 to 11 untangle the existing modesetting code into smaller functions. Specifically, mode setting and plane updates are being separated from each other.
Patch 12 to 14 convert mgag200 to simple KMS helpers and enables atomic mode setting.
Atomically switching plane framebuffers, requires either source or target buffer to be located at a non-0 offet. As some HW revisions seem to require a framebuffer offset of 0 within the video memory, they do not work with atomic modesetting. To resolve this problem, patch 15 converts mgag200 from VRAM helpers to SHMEM helpers. During plane updates, the content of the SHMEM BO is memcpy'd to VRAM. From my observation, performance is not nuch different from the original code.
The patchset has been tested on MGA G200EH hardware.
v2:
- rebase patchset
- replace uint{8,32}_t with u{8,32} through-out patchset
- define additional register constants
- use helper functions around bpp-shift computations
- split conversion patch
- cleanups
With the one comment addressed patch 1-14 are now all: Acked-by: Sam Ravnborg sam@ravnborg.org
Sure, I'll add that comment. Thanks for the timely reviews.
Best regards Thomas
I did not look at the last patch - all the memory stuff is still beyond me.
Nice to see this driver gettting so much love and care. The end result is a much nicer driver implmentation.
Sam
Thomas Zimmermann (15): drm/mgag200: Remove HW cursor drm/mgag200: Clean up mga_set_start_address() drm/mgag200: Clean up mga_crtc_do_set_base() drm/mgag200: Move mode-setting code into separate helper function drm/mgag200: Split MISC register update into PLL selection, SYNC and I/O drm/mgag200: Update mode registers after plane registers drm/mgag200: Set pitch in a separate helper function drm/mgag200: Set primary plane's format in separate helper function drm/mgag200: Move TAGFIFO reset into separate function drm/mgag200: Move hiprilvl setting into separate functions drm/mgag200: Move register initialization into separate function drm/mgag200: Remove out-commented suspend/resume helpers drm/mgag200: Use simple-display data structures drm/mgag200: Convert to simple KMS helper drm/mgag200: Replace VRAM helpers with SHMEM helpers
drivers/gpu/drm/mgag200/Kconfig | 4 +- drivers/gpu/drm/mgag200/Makefile | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 51 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 41 +- drivers/gpu/drm/mgag200/mgag200_main.c | 5 - drivers/gpu/drm/mgag200/mgag200_mode.c | 871 ++++++++++++++----------- drivers/gpu/drm/mgag200/mgag200_reg.h | 11 +- drivers/gpu/drm/mgag200/mgag200_ttm.c | 28 +- 8 files changed, 528 insertions(+), 485 deletions(-)
-- 2.26.2
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Emil Velikov -
Sam Ravnborg -
Thomas Zimmermann