This set of patches contains several fixes and enhancements for the MGAG200 KMS driver.
Egbert Eich (15): drm/mgag200: Fix memleaks in error path in mgag200_fb_create() drm/mgag200: Fix memleak in error path in mgag200_bo_create() drm/mgag200: Free container instead of member in mga_user_framebuffer_destroy() drm/mgag200: Don't unreference when handle creation failed drm/mgag200: Copy fb name string before using it in mgag200_fb_create() drm/mgag200: Fix logic in mgag200_bo_pin() drm/mgag200: Make local function mgag200_gem_init_object() static drm/mgag200: Simplify function mgag200_bo_unpin() drm/mgag200: Add an crtc_disable callback to the crtc helper funcs drm/mgag200: Invalidate page tables when pinning a BO drm/mgag200: Initialize data needed to map fbdev memory drm/mgag200: Add sysfs support for connectors drm/mgag200: Fix LUT programming for 16bpp drm/mgag200: Reject modes when h-parameters are no multiple of 8 drm/mgag200: Add doublescan and interlace support
Takashi Iwai (1): drm/mgag200: Fix framebuffer pitch calculation
drivers/gpu/drm/mgag200/mgag200_fb.c | 27 +++++++++++---- drivers/gpu/drm/mgag200/mgag200_main.c | 7 ++-- drivers/gpu/drm/mgag200/mgag200_mode.c | 60 ++++++++++++++++++++++++++++++++-- drivers/gpu/drm/mgag200/mgag200_ttm.c | 42 +++++++++++------------- 4 files changed, 100 insertions(+), 36 deletions(-)
Some mmemory allocated in mgag200fb_create() was not properly freed before the function returned with an error. This patch takes care of this.
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_fb.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c index 964f58c..6453e4c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c @@ -162,13 +162,13 @@ static int mgag200fb_create(struct drm_fb_helper *helper, struct drm_device *dev = mfbdev->helper.dev; struct drm_mode_fb_cmd2 mode_cmd; struct mga_device *mdev = dev->dev_private; - struct fb_info *info; + struct fb_info *info = NULL; struct drm_framebuffer *fb; struct drm_gem_object *gobj = NULL; struct device *device = &dev->pdev->dev; struct mgag200_bo *bo; int ret; - void *sysram; + void *sysram = NULL; int size;
mode_cmd.width = sizes->surface_width; @@ -191,14 +191,16 @@ static int mgag200fb_create(struct drm_fb_helper *helper, return -ENOMEM;
info = framebuffer_alloc(0, device); - if (info == NULL) - return -ENOMEM; + if (info == NULL) { + ret = -ENOMEM; + goto out; + }
info->par = mfbdev;
ret = mgag200_framebuffer_init(dev, &mfbdev->mfb, &mode_cmd, gobj); if (ret) - return ret; + goto out;
mfbdev->sysram = sysram; mfbdev->size = size; @@ -242,6 +244,15 @@ static int mgag200fb_create(struct drm_fb_helper *helper, fb->width, fb->height); return 0; out: + mfbdev->mfb.obj = NULL; + vfree(sysram); + mfbdev->sysram = NULL; + if (info && info->cmap.len) + fb_dealloc_cmap(&info->cmap); + framebuffer_release(info); + mfbdev->helper.fb = NULL; + mfbdev->helper.fbdev = NULL; + return ret; }
The allocated struct mgag200_bo was not freed in all error paths. This patch consolidates error handling and fixes this.
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_ttm.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c index 3acb2b0..6461fd2 100644 --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c @@ -316,10 +316,8 @@ int mgag200_bo_create(struct drm_device *dev, int size, int align, return -ENOMEM;
ret = drm_gem_object_init(dev, &mgabo->gem, size); - if (ret) { - kfree(mgabo); - return ret; - } + if (ret) + goto err;
mgabo->gem.driver_private = NULL; mgabo->bo.bdev = &mdev->ttm.bdev; @@ -334,10 +332,13 @@ int mgag200_bo_create(struct drm_device *dev, int size, int align, align >> PAGE_SHIFT, false, NULL, acc_size, NULL, mgag200_bo_ttm_destroy); if (ret) - return ret; + goto err;
*pmgabo = mgabo; return 0; +err: + kfree(mgabo); + return ret; }
static inline u64 mgag200_bo_gpu_offset(struct mgag200_bo *bo)
Technically freeing mga_fb->base is the same as freeing mga_fb as 'base' the first member of the data structure. Still this makes it cleaner.
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 9fa5685..43ebf8d 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -20,7 +20,7 @@ static void mga_user_framebuffer_destroy(struct drm_framebuffer *fb) if (mga_fb->obj) drm_gem_object_unreference_unlocked(mga_fb->obj); drm_framebuffer_cleanup(fb); - kfree(fb); + kfree(mga_fb); }
static const struct drm_framebuffer_funcs mga_fb_funcs = {
drm_gem_handle_create() should not have referenced an object when it's creation has failed for some reason.
Signed-off-by: Egbert Eich eich@suse.com --- drivers/gpu/drm/mgag200/mgag200_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index 43ebf8d..d51096c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -302,10 +302,11 @@ int mgag200_dumb_create(struct drm_file *file, return ret;
ret = drm_gem_handle_create(file, gobj, &handle); - drm_gem_object_unreference_unlocked(gobj); if (ret) return ret;
+ drm_gem_object_unreference_unlocked(gobj); + args->handle = handle; return 0; }
On Wed, Jul 17, 2013 at 03:07:17PM +0200, Egbert Eich wrote:
drm_gem_handle_create() should not have referenced an object when it's creation has failed for some reason.
Signed-off-by: Egbert Eich eich@suse.com
Nak. The unreference here is to free the locally created resource before returning the error to the user. In the case of success, the only reference to the object that then remains is through the new handle. -Chris
The fb id string was used in an error message right before it was set. Reversing the order of the code fixes this.
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_fb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c index 6453e4c..371f3eb 100644 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c @@ -211,6 +211,8 @@ static int mgag200fb_create(struct drm_fb_helper *helper, mfbdev->helper.fb = fb; mfbdev->helper.fbdev = info;
+ strcpy(info->fix.id, "mgadrmfb"); + ret = fb_alloc_cmap(&info->cmap, 256, 0); if (ret) { DRM_ERROR("%s: can't allocate color map\n", info->fix.id); @@ -218,8 +220,6 @@ static int mgag200fb_create(struct drm_fb_helper *helper, goto out; }
- strcpy(info->fix.id, "mgadrmfb"); - info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT; info->fbops = &mgag200fb_ops;
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_ttm.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c index 6461fd2..2606031 100644 --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c @@ -350,20 +350,16 @@ int mgag200_bo_pin(struct mgag200_bo *bo, u32 pl_flag, u64 *gpu_addr) { int i, ret;
- if (bo->pin_count) { - bo->pin_count++; - if (gpu_addr) - *gpu_addr = mgag200_bo_gpu_offset(bo); - } - - mgag200_ttm_placement(bo, pl_flag); - for (i = 0; i < bo->placement.num_placement; i++) - bo->placements[i] |= TTM_PL_FLAG_NO_EVICT; - ret = ttm_bo_validate(&bo->bo, &bo->placement, false, false); - if (ret) - return ret;
- bo->pin_count = 1; + if (!bo->pin_count) { + mgag200_ttm_placement(bo, pl_flag); + for (i = 0; i < bo->placement.num_placement; i++) + bo->placements[i] |= TTM_PL_FLAG_NO_EVICT; + ret = ttm_bo_validate(&bo->bo, &bo->placement, false, false); + if (ret) + return ret; + } + bo->pin_count++; if (gpu_addr) *gpu_addr = mgag200_bo_gpu_offset(bo); return 0;
On Wed, Jul 17, 2013 at 11:07 PM, Egbert Eich eich@suse.com wrote:
Signed-off-by: Egbert Eich eich@suse.de
Is this just missing a return 0; ? in the bo->pin_count > 0 case? seems like a simpler patch.
Dave.
drivers/gpu/drm/mgag200/mgag200_ttm.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c index 6461fd2..2606031 100644 --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c @@ -350,20 +350,16 @@ int mgag200_bo_pin(struct mgag200_bo *bo, u32 pl_flag, u64 *gpu_addr) { int i, ret;
if (bo->pin_count) {bo->pin_count++;if (gpu_addr)*gpu_addr = mgag200_bo_gpu_offset(bo);}mgag200_ttm_placement(bo, pl_flag);for (i = 0; i < bo->placement.num_placement; i++)bo->placements[i] |= TTM_PL_FLAG_NO_EVICT;ret = ttm_bo_validate(&bo->bo, &bo->placement, false, false);if (ret)return ret;bo->pin_count = 1;
if (!bo->pin_count) {mgag200_ttm_placement(bo, pl_flag);for (i = 0; i < bo->placement.num_placement; i++)bo->placements[i] |= TTM_PL_FLAG_NO_EVICT;ret = ttm_bo_validate(&bo->bo, &bo->placement, false, false);if (ret)return ret;}bo->pin_count++; if (gpu_addr) *gpu_addr = mgag200_bo_gpu_offset(bo); return 0;-- 1.8.1.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c index d51096c..fe8ed66 100644 --- a/drivers/gpu/drm/mgag200/mgag200_main.c +++ b/drivers/gpu/drm/mgag200/mgag200_main.c @@ -324,7 +324,7 @@ int mgag200_gem_init_object(struct drm_gem_object *obj) return 0; }
-void mgag200_bo_unref(struct mgag200_bo **bo) +static void mgag200_bo_unref(struct mgag200_bo **bo) { struct ttm_buffer_object *tbo;
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_ttm.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c index 2606031..3a2e5e2 100644 --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c @@ -367,7 +367,7 @@ int mgag200_bo_pin(struct mgag200_bo *bo, u32 pl_flag, u64 *gpu_addr)
int mgag200_bo_unpin(struct mgag200_bo *bo) { - int i, ret; + int i; if (!bo->pin_count) { DRM_ERROR("unpin bad %p\n", bo); return 0; @@ -378,11 +378,8 @@ int mgag200_bo_unpin(struct mgag200_bo *bo)
for (i = 0; i < bo->placement.num_placement ; i++) bo->placements[i] &= ~TTM_PL_FLAG_NO_EVICT; - ret = ttm_bo_validate(&bo->bo, &bo->placement, false, false); - if (ret) - return ret;
- return 0; + return ttm_bo_validate(&bo->bo, &bo->placement, false, false); }
int mgag200_bo_push_sysram(struct mgag200_bo *bo)
Signed-off-by: Egbert Eich eich@suse.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 251784a..2fe1f64 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1251,6 +1251,24 @@ static void mga_crtc_destroy(struct drm_crtc *crtc) kfree(mga_crtc); }
+static void mga_crtc_disable(struct drm_crtc *crtc) +{ + int ret; + DRM_DEBUG_KMS("\n"); + mga_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); + if (crtc->fb) { + struct mga_framebuffer *mga_fb = to_mga_framebuffer(crtc->fb); + struct drm_gem_object *obj = mga_fb->obj; + struct mgag200_bo *bo = gem_to_mga_bo(obj); + ret = mgag200_bo_reserve(bo, false); + if (ret) + return; + mgag200_bo_push_sysram(bo); + mgag200_bo_unreserve(bo); + } + crtc->fb = NULL; +} + /* These provide the minimum set of functions required to handle a CRTC */ static const struct drm_crtc_funcs mga_crtc_funcs = { .cursor_set = mga_crtc_cursor_set, @@ -1261,6 +1279,7 @@ static const struct drm_crtc_funcs mga_crtc_funcs = { };
static const struct drm_crtc_helper_funcs mga_helper_funcs = { + .disable = mga_crtc_disable, .dpms = mga_crtc_dpms, .mode_fixup = mga_crtc_mode_fixup, .mode_set = mga_crtc_mode_set,
When a BO gets pinned the placement may get changed. If the memory is mapped into user space and user space has already accessed the mapped range the page tables are set up but now point to the wrong memory. A call to ttm_bo_unmap_virtual() will invalidate all page tables of all mappings of this BO. When user space accesses this memory again we will receive a page fault and are able to set up the page tables to point to the correct physical memory.
Signed-off-by: Egbert Eich eich@suse.com --- drivers/gpu/drm/mgag200/mgag200_ttm.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c index 3a2e5e2..8ceeb0c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c @@ -321,6 +321,7 @@ int mgag200_bo_create(struct drm_device *dev, int size, int align,
mgabo->gem.driver_private = NULL; mgabo->bo.bdev = &mdev->ttm.bdev; + mgabo->bo.bdev->dev_mapping = dev->dev_mapping;
mgag200_ttm_placement(mgabo, TTM_PL_FLAG_VRAM | TTM_PL_FLAG_SYSTEM);
@@ -359,6 +360,7 @@ int mgag200_bo_pin(struct mgag200_bo *bo, u32 pl_flag, u64 *gpu_addr) if (ret) return ret; } + ttm_bo_unmap_virtual(&bo->bo); bo->pin_count++; if (gpu_addr) *gpu_addr = mgag200_bo_gpu_offset(bo);
Due to a missing initialization there was no way to map fbdev memory. Thus for example using the Xserver with the fbdev driver failed. This fix adds initialization for fix.smem_start and fix.smem_len in the fb_info structure, which fixes this problem.
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_fb.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c index 371f3eb..94c64de 100644 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c @@ -231,6 +231,8 @@ static int mgag200fb_create(struct drm_fb_helper *helper, } info->apertures->ranges[0].base = mdev->dev->mode_config.fb_base; info->apertures->ranges[0].size = mdev->mc.vram_size; + info->fix.smem_start = mdev->dev->mode_config.fb_base; + info->fix.smem_len = size;
drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth); drm_fb_helper_fill_var(info, &mfbdev->helper, sizes->fb_width,
On Wed, Jul 17, 2013 at 11:07 PM, Egbert Eich eich@suse.com wrote:
Due to a missing initialization there was no way to map fbdev memory. Thus for example using the Xserver with the fbdev driver failed. This fix adds initialization for fix.smem_start and fix.smem_len in the fb_info structure, which fixes this problem.
I don't think this is correct, since it'll give userspace a direct phy addr for the base of the video ram, though I'm not in a position to say exactly what should be set here, guess I need to get an mgag200 machine up and running.
Dave.
Signed-off-by: Egbert Eich eich@suse.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 2fe1f64..020a623 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1600,6 +1600,8 @@ static struct drm_connector *mga_vga_init(struct drm_device *dev)
drm_connector_helper_add(connector, &mga_vga_connector_helper_funcs);
+ drm_sysfs_connector_add(connector); + mga_connector->i2c = mgag200_i2c_create(dev); if (!mga_connector->i2c) DRM_ERROR("failed to add ddc bus\n");
From: Takashi Iwai tiwai@suse.com
The framebuffer pitch calculation needs to be done differently for bpp == 24 - check xf86-video-mga for reference.
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_mode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 020a623..c8983f9 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -877,7 +877,7 @@ static int mga_crtc_mode_set(struct drm_crtc *crtc,
pitch = crtc->fb->pitches[0] / (crtc->fb->bits_per_pixel / 8); if (crtc->fb->bits_per_pixel == 24) - pitch = pitch >> (4 - bppshift); + pitch = (pitch * 3) >> (4 - bppshift); else pitch = pitch >> (4 - bppshift);
Since there are only 32 (64) distinct color values for each color in 16bpp Matrox hardware expects those in a 'dense' manner, ie in the first 32 (64) entries of the respective color.
Signed-off-by: Egbert Eich eich@suse.de --- drivers/gpu/drm/mgag200/mgag200_mode.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index c8983f9..503a414 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -29,6 +29,7 @@ static void mga_crtc_load_lut(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 drm_framebuffer *fb = crtc->fb; int i;
if (!crtc->enabled) @@ -36,6 +37,28 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
WREG8(DAC_INDEX + MGA1064_INDEX, 0);
+ if (fb && fb->bits_per_pixel == 16) { + int inc = (fb->depth == 15) ? 8 : 4; + u8 r, b; + for (i = 0; i < MGAG200_LUT_SIZE; i += inc) { + if (fb->depth == 16) { + if (i > (MGAG200_LUT_SIZE >> 1)) { + r = b = 0; + } else { + r = mga_crtc->lut_r[i << 1]; + b = mga_crtc->lut_b[i << 1]; + } + } else { + r = mga_crtc->lut_r[i]; + b = mga_crtc->lut_b[i]; + } + /* VGA registers */ + WREG8(DAC_INDEX + MGA1064_COL_PAL, r); + WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]); + WREG8(DAC_INDEX + MGA1064_COL_PAL, b); + } + return; + } for (i = 0; i < MGAG200_LUT_SIZE; i++) { /* VGA registers */ WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_r[i]);
Matrox hardware only supports modes whose horizontal parameters are multiples of 8. This rules out a mode like 1366x768 for example.
Signed-off-by: Egbert Eich eich@suse.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 503a414..0bb0e1e 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1491,6 +1491,10 @@ static int mga_vga_mode_valid(struct drm_connector *connector, int bpp = 32; int i = 0;
+ if (mode->hdisplay % 8 || mode->hsync_start % 8 || + mode->hsync_end % 8 || mode->htotal % 8) + return MODE_H_ILLEGAL; + if (IS_G200_SE(mdev)) { if (mdev->unique_rev_id == 0x01) { if (mode->hdisplay > 1600)
On Wed, Jul 17, 2013 at 11:07 PM, Egbert Eich eich@suse.com wrote:
Matrox hardware only supports modes whose horizontal parameters are multiples of 8. This rules out a mode like 1366x768 for example.
Signed-off-by: Egbert Eich eich@suse.com
I'd like to get a second opinion from ajax for where this should be dealt with, though this is probably okay.
Dave.
On Tue, 2013-07-23 at 15:46 +1000, Dave Airlie wrote:
On Wed, Jul 17, 2013 at 11:07 PM, Egbert Eich eich@suse.com wrote:
Matrox hardware only supports modes whose horizontal parameters are multiples of 8. This rules out a mode like 1366x768 for example.
Signed-off-by: Egbert Eich eich@suse.com
I'd like to get a second opinion from ajax for where this should be dealt with, though this is probably okay.
Patch seems like it's in the right place to me.
- ajax
On Tue, Jul 23, 2013 at 5:30 PM, Adam Jackson ajax@redhat.com wrote:
On Tue, 2013-07-23 at 15:46 +1000, Dave Airlie wrote:
On Wed, Jul 17, 2013 at 11:07 PM, Egbert Eich eich@suse.com wrote:
Matrox hardware only supports modes whose horizontal parameters are multiples of 8. This rules out a mode like 1366x768 for example.
Signed-off-by: Egbert Eich eich@suse.com
I'd like to get a second opinion from ajax for where this should be dealt with, though this is probably okay.
Patch seems like it's in the right place to me.
->mode_valid is only used when parsing the list of modes obtained from the EDID. Userspace can still add new modes of its own and force the kernel to use them. To plug that gap mgag200 also needs to reject any modes in the crtc_helper->mode_fixup callback.
I've pondered whether we should unify these too callers, e.g. by using the ->mode_fixup function from ->mode_valid with a fake adjusted_mode. But I never got around to that ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
This code was ported from the xorg mga driver.
Signed-off-by: Egbert Eich eich@suse.com --- drivers/gpu/drm/mgag200/mgag200_mode.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 0bb0e1e..3faacbe 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -1488,6 +1488,7 @@ static int mga_vga_mode_valid(struct drm_connector *connector, struct mga_fbdev *mfbdev = mdev->mfbdev; struct drm_fb_helper *fb_helper = &mfbdev->helper; struct drm_fb_helper_connector *fb_helper_conn = NULL; + int lace = 1 + ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0); int bpp = 32; int i = 0;
@@ -1537,8 +1538,10 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
if (mode->crtc_hdisplay > 2048 || mode->crtc_hsync_start > 4096 || mode->crtc_hsync_end > 4096 || mode->crtc_htotal > 4096 || - mode->crtc_vdisplay > 2048 || mode->crtc_vsync_start > 4096 || - mode->crtc_vsync_end > 4096 || mode->crtc_vtotal > 4096) { + mode->crtc_vdisplay > 2048 * lace || + mode->crtc_vsync_start > 4096 * lace || + mode->crtc_vsync_end > 4096 * lace || + mode->crtc_vtotal > 4096 * lace) { return MODE_BAD; }
@@ -1622,6 +1625,9 @@ static struct drm_connector *mga_vga_init(struct drm_device *dev)
connector = &mga_connector->base;
+ connector->interlace_allowed = true; + connector->doublescan_allowed = true; + drm_connector_init(dev, connector, &mga_vga_connector_funcs, DRM_MODE_CONNECTOR_VGA);
On Wed, Jul 17, 2013 at 11:07 PM, Egbert Eich eich@suse.com wrote:
This set of patches contains several fixes and enhancements for the MGAG200 KMS driver.
Hi Egbert,
I've pushed some of these into a branch in my repo, mgag200-queue,
I'll try and review the rest soon, I'll probably split these up into obvious fixes for Linus, and leave the cleanups/features for -next if they aren't fixing any actual problems.
Dave.
Egbert Eich (15): drm/mgag200: Fix memleaks in error path in mgag200_fb_create() drm/mgag200: Fix memleak in error path in mgag200_bo_create() drm/mgag200: Free container instead of member in mga_user_framebuffer_destroy() drm/mgag200: Don't unreference when handle creation failed drm/mgag200: Copy fb name string before using it in mgag200_fb_create() drm/mgag200: Fix logic in mgag200_bo_pin() drm/mgag200: Make local function mgag200_gem_init_object() static drm/mgag200: Simplify function mgag200_bo_unpin() drm/mgag200: Add an crtc_disable callback to the crtc helper funcs drm/mgag200: Invalidate page tables when pinning a BO drm/mgag200: Initialize data needed to map fbdev memory drm/mgag200: Add sysfs support for connectors drm/mgag200: Fix LUT programming for 16bpp drm/mgag200: Reject modes when h-parameters are no multiple of 8 drm/mgag200: Add doublescan and interlace support
Takashi Iwai (1): drm/mgag200: Fix framebuffer pitch calculation
drivers/gpu/drm/mgag200/mgag200_fb.c | 27 +++++++++++---- drivers/gpu/drm/mgag200/mgag200_main.c | 7 ++-- drivers/gpu/drm/mgag200/mgag200_mode.c | 60 ++++++++++++++++++++++++++++++++-- drivers/gpu/drm/mgag200/mgag200_ttm.c | 42 +++++++++++------------- 4 files changed, 100 insertions(+), 36 deletions(-)
-- 1.8.1.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Adam Jackson -
Chris Wilson -
Daniel Vetter -
Dave Airlie -
Egbert Eich