Hi,
On 9/14/2015 3:35 PM, Dave Airlie wrote:
(this time with correct email address).
On 14 September 2015 at 20:04, Dave Airlie airlied@gmail.com wrote:
If drm_fb_helper_alloc_fbi() fails then we were directly returning without freeing sysram. Also if drm_fb_helper_alloc_fbi() succeeds but mgag200_framebuffer_init() fails then we were not releasing sysram and we were not releasing fbi helper also.
Signed-off-by: Sudip Mukherjee sudip@vectorindia.org
drivers/gpu/drm/mgag200/mgag200_fb.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c index 87de15e..5fe476a 100644 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c @@ -189,14 +189,16 @@ static int mgag200fb_create(struct drm_fb_helper *helper, return -ENOMEM;
info = drm_fb_helper_alloc_fbi(helper);
if (IS_ERR(info))return PTR_ERR(info);
if (IS_ERR(info)) {ret = PTR_ERR(info);goto err_alloc_fbi;} info->par = mfbdev; ret = mgag200_framebuffer_init(dev, &mfbdev->mfb, &mode_cmd, gobj); if (ret)
return ret;
goto err_framebuffer_init; mfbdev->sysram = sysram; mfbdev->size = size;@@ -226,6 +228,13 @@ static int mgag200fb_create(struct drm_fb_helper *helper, DRM_DEBUG_KMS("allocated %dx%d\n", fb->width, fb->height); return 0;
+err_framebuffer_init:
drm_fb_helper_release_fbi(helper);+err_alloc_fbi:
vfree(sysram);return ret;}
static int mga_fbdev_destroy(struct drm_device *dev,
There's a new regression: v4.3-rc1 crashes on bootup on non-supported hardware, if CONFIG_DRM_MGAG200=y (built into the kernel).
Archit, I'm guessing this is some fallout from the fbdev changes.
There is no reason we should need CONFIG_FB_LITTLE_ENDIAN I don't think.
It looks like the mgag200 driver load fails and we crash in the error handling path. drm_fb_helper_fini ends up being called twice. The second call tries to free resources that were already free'd.
The erroneous path above should have existed even without the recent fbdev helper changes. I'm not sure what's causing the driver load to fail in the first place. It looks like it's failing at register_framebuffer. Is it possible that we never tried running this driver before with Big Endian set?
The patch below fixes the problem with the error path mentioned above. Could we try this?
From: Archit Taneja architt@codeaurora.org Date: Mon, 14 Sep 2015 20:11:43 +0530 Subject: [PATCH] drm/mgag200: Prevent calling drm_fb_helper_fini twice
mgag200_fbdev_init's error handling path calls drm_fb_helper_fini before bailing out. The error handling path of mgag200_driver_load also ends up calling drm_fb_helper_fini.
This results in drm_fb_helper_fini being called twice if the driver load drm op fails somewhere in between.
Make only mgag200_driver_unload call drm_fb_helper_fini, remove the call from mgag200_fbdev_init.
Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/mgag200/mgag200_fb.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c index 87de15e..6259b0a 100644 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c @@ -280,20 +280,16 @@ int mgag200_fbdev_init(struct mga_device *mdev)
ret = drm_fb_helper_single_add_all_connectors(&mfbdev->helper); if (ret) - goto fini; + return ret;
/* disable all the possible outputs/crtcs before entering KMS mode */ drm_helper_disable_unused_functions(mdev->dev);
ret = drm_fb_helper_initial_config(&mfbdev->helper, bpp_sel); if (ret) - goto fini; + return ret;
return 0; - -fini: - drm_fb_helper_fini(&mfbdev->helper); - return ret; }
void mgag200_fbdev_fini(struct mga_device *mdev)