[PATCH v4 5/5] drm: simpledrm: honour remove_conflicting_framebuffers()

2016-08-23 Thread Noralf Trønnes 

Den 23.08.2016 20:01, skrev Daniel Vetter:
> On Tue, Aug 23, 2016 at 7:52 PM, Noralf Trønnes  
> wrote:
 +static int sdrm_fbdev_event_notify(struct notifier_block *self,
 +  unsigned long action, void *data)
 +{
 +   struct sdrm_device *sdrm;
 +   struct fb_event *event = data;
 +   struct fb_info *info = event->info;
 +   struct drm_fb_helper *fb_helper = info->par;
 +
 +   if (action != FB_EVENT_FB_UNREGISTERED)
 +   return NOTIFY_DONE;
 +
 +   if (!fb_helper || !fb_helper->dev || fb_helper->fbdev != info)
 +   return NOTIFY_DONE;
 +
 +   sdrm = fb_helper->dev->dev_private;
 +
 +   if (sdrm && sdrm->fb_helper == fb_helper)
 +
 platform_device_del(to_platform_device(fb_helper->dev->dev));
 +
 +   return NOTIFY_DONE;
 +}
>>> One problem this leaves behind is that registering of the new fbdev driver
>>> is too late - by that point we've already set up the entire driver,
>>> including modeset. If fbdev meanwhile does a dpms off or something like
>>> that all hell will break loose.
>>
>> I don't understand how fbdev registration comes into play here. Drivers call
>> remove_conflicting_framebuffers very early so simpledrm is gone by the time
>> they register anything.
>>
>> For simpledrm, fbdev doing blank/unblank is a no-op since fb_ops.fb_blank
>> is not implemented. So a fb_blank() just results in fbcon doing a
>> software blank.
> Maybe my scenario wasn't entirely clear:
> - prereq: fbdev emulation in drm is disabled
> 1. simpledrm loads and sets up the firmware fb
> 2. real driver loads, first calls
> drm_fb_helper_remove_conflicting_framebuffer. Nothing happens because
> CONFIG_FB=n.
> 3. real driver start loading, remapping the gart and what not else
> 4. something is drawn using fbcon, simplerdrm writes through the now
> invalid mapping
> -> BOOM

Yes CONFIG_FB=n is not covered, that's the drawback mentioned in the 
Kconfig.
However, who uses simpledrm without fbdev/fbcon when it shall be handed over
to a real hw-driver?
But yes, it's not a good stop gap solution, I like my other idea much 
better.

> You have code to listen to the framebuffer registration notifier, but
> I think even that happens way too late. Or at least I didn't spot any
> code in remove_conflicting_framebuffers which would call down into
> that notifier. Or maybe I entirely misunderstand your code ...

remove_conflicting_framebuffers unregisters the fbdev framebuffer.
sdrm_fbdev_event_notify detects that the framebuffer is being unregistered,
and deletes the platform device.

Some details:

do_remove_conflicting_framebuffers():
 for (i = 0 ; i < FB_MAX; i++) {
[...]
 printk(KERN_INFO "fb: switching to %s from %s\n",
name, registered_fb[i]->fix.id);
 ret = do_unregister_framebuffer(registered_fb[i]);

do_unregister_framebuffer(): short version
{
 console_lock();

 event.info = fb_info;
 ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
 unlock_fb_info(fb_info);
 console_unlock();

 pm_vt_switch_unregister(fb_info->dev);

 unlink_framebuffer(fb_info);

 registered_fb[i] = NULL;
 num_registered_fb--;

 event.info = fb_info;
 console_lock();
 fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
/* at this point simpledrm has been deleted */
 console_unlock();

 /* this may free fb info */
 put_fb_info(fb_info);
 return 0;
}

> Wrt fixing: Just adding it to the recently added stub is of course
> also a working solution.

I actually like this better, because it's so straightforward and easy
to understand. The notifier solution is very convoluted and easy to
mess up. And it runs with the console lock held...

> -Daniel
>
> PS: Can you pls review the 2 patches I submitted with you on cc? I
> won't merge my own patches without proper review, so without that done
> they're stuck.

Ok.

I'll test the simple-helper plane patch tomorrow.
In simpledrm I first tried to send the vblank event only in pipe.update(),
but I had to do it in enable() and disable() as well to make that vblank
timeout go away.


Noralf.

[PATCH v4 5/5] drm: simpledrm: honour remove_conflicting_framebuffers()

2016-08-23 Thread Daniel Vetter 
On Tue, Aug 23, 2016 at 7:52 PM, Noralf Trønnes  wrote:
>>> +static int sdrm_fbdev_event_notify(struct notifier_block *self,
>>> +  unsigned long action, void *data)
>>> +{
>>> +   struct sdrm_device *sdrm;
>>> +   struct fb_event *event = data;
>>> +   struct fb_info *info = event->info;
>>> +   struct drm_fb_helper *fb_helper = info->par;
>>> +
>>> +   if (action != FB_EVENT_FB_UNREGISTERED)
>>> +   return NOTIFY_DONE;
>>> +
>>> +   if (!fb_helper || !fb_helper->dev || fb_helper->fbdev != info)
>>> +   return NOTIFY_DONE;
>>> +
>>> +   sdrm = fb_helper->dev->dev_private;
>>> +
>>> +   if (sdrm && sdrm->fb_helper == fb_helper)
>>> +
>>> platform_device_del(to_platform_device(fb_helper->dev->dev));
>>> +
>>> +   return NOTIFY_DONE;
>>> +}
>>
>> One problem this leaves behind is that registering of the new fbdev driver
>> is too late - by that point we've already set up the entire driver,
>> including modeset. If fbdev meanwhile does a dpms off or something like
>> that all hell will break loose.
>
>
> I don't understand how fbdev registration comes into play here. Drivers call
> remove_conflicting_framebuffers very early so simpledrm is gone by the time
> they register anything.
>
> For simpledrm, fbdev doing blank/unblank is a no-op since fb_ops.fb_blank
> is not implemented. So a fb_blank() just results in fbcon doing a
> software blank.

Maybe my scenario wasn't entirely clear:
- prereq: fbdev emulation in drm is disabled
1. simpledrm loads and sets up the firmware fb
2. real driver loads, first calls
drm_fb_helper_remove_conflicting_framebuffer. Nothing happens because
CONFIG_FB=n.
3. real driver start loading, remapping the gart and what not else
4. something is drawn using fbcon, simplerdrm writes through the now
invalid mapping
-> BOOM

You have code to listen to the framebuffer registration notifier, but
I think even that happens way too late. Or at least I didn't spot any
code in remove_conflicting_framebuffers which would call down into
that notifier. Or maybe I entirely misunderstand your code ...

Wrt fixing: Just adding it to the recently added stub is of course
also a working solution.
-Daniel

PS: Can you pls review the 2 patches I submitted with you on cc? I
won't merge my own patches without proper review, so without that done
they're stuck.
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v4 5/5] drm: simpledrm: honour remove_conflicting_framebuffers()

2016-08-23 Thread Noralf Trønnes 

Den 23.08.2016 14:41, skrev Daniel Vetter:
> On Mon, Aug 22, 2016 at 10:25:25PM +0200, Noralf Trønnes wrote:
>> There is currently no non-fbdev mechanism in place to kick out
>> simpledrm when the real hw-driver is probed. As a stop gap until
>> that is in place, honour remove_conflicting_framebuffers() and
>> delete the simple-framebuffer platform device when it's called.
>>
>> Signed-off-by: Noralf Trønnes 
>> ---



>> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c 
>> b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
>> index c6596ad..7c6db2c 100644
>> --- a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
>> +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
>> @@ -44,6 +44,20 @@ static int sdrm_fb_mmap(struct fb_info *info, struct 
>> vm_area_struct *vma)
>>  return -ENODEV;
>>   }
>>
>> +/*
>> + * Releasing has to be done outside the notifier callchain when we're
>> + * kicked out, since do_unregister_framebuffer() calls put_fb_info()
>> + * after the notifier has run.
>> + * Open code drm_fb_helper_release_fbi() since fb_helper is freed at
>> + * this point when kicked out.
>> + */
>> +static void sdrm_fbdev_fb_destroy(struct fb_info *info)
>> +{
>> +if (info->cmap.len)
>> +fb_dealloc_cmap(&info->cmap);
>> +framebuffer_release(info);
>> +}
>> +
>>   static struct fb_ops sdrm_fbdev_ops = {
>>  .owner  = THIS_MODULE,
>>  .fb_fillrect= drm_fb_helper_sys_fillrect,
>> @@ -53,6 +67,7 @@ static struct fb_ops sdrm_fbdev_ops = {
>>  .fb_set_par = drm_fb_helper_set_par,
>>  .fb_setcmap = drm_fb_helper_setcmap,
>>  .fb_mmap= sdrm_fb_mmap,
>> +.fb_destroy = sdrm_fbdev_fb_destroy,
>>   };
>>
>>   static int sdrm_fbdev_create(struct drm_fb_helper *helper,
>> @@ -110,6 +125,9 @@ static int sdrm_fbdev_create(struct drm_fb_helper 
>> *helper,
>>  fbi->screen_base = obj->vmapping;
>>  fbi->fix.smem_len = sdrm->fb_size;
>>
>> +fbi->apertures->ranges[0].base = sdrm->fb_base;
>> +fbi->apertures->ranges[0].size = sdrm->fb_size;
>> +
>>  return 0;
>>
>>   err_fbi_release:
>> @@ -188,9 +206,13 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
>>  sdrm->fb_helper = NULL;
>>  fbdev = to_sdrm_fbdev(fb_helper);
>>
>> -drm_fb_helper_unregister_fbi(fb_helper);
>> +/* it might have been kicked out */
>> +if (registered_fb[fbdev->fb_helper.fbdev->node])
>> +drm_fb_helper_unregister_fbi(fb_helper);
>> +
>> +/* freeing fb_info is done in fb_ops.fb_destroy() */
>> +
>>  cancel_work_sync(&fb_helper->dirty_work);
>> -drm_fb_helper_release_fbi(fb_helper);
>>
>>  drm_framebuffer_unregister_private(fb_helper->fb);
>>  drm_framebuffer_cleanup(fb_helper->fb);
>> @@ -199,3 +221,39 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
>>  drm_fb_helper_fini(fb_helper);
>>  kfree(fbdev);
>>   }
>> +
>> +static int sdrm_fbdev_event_notify(struct notifier_block *self,
>> +   unsigned long action, void *data)
>> +{
>> +struct sdrm_device *sdrm;
>> +struct fb_event *event = data;
>> +struct fb_info *info = event->info;
>> +struct drm_fb_helper *fb_helper = info->par;
>> +
>> +if (action != FB_EVENT_FB_UNREGISTERED)
>> +return NOTIFY_DONE;
>> +
>> +if (!fb_helper || !fb_helper->dev || fb_helper->fbdev != info)
>> +return NOTIFY_DONE;
>> +
>> +sdrm = fb_helper->dev->dev_private;
>> +
>> +if (sdrm && sdrm->fb_helper == fb_helper)
>> +platform_device_del(to_platform_device(fb_helper->dev->dev));
>> +
>> +return NOTIFY_DONE;
>> +}
> One problem this leaves behind is that registering of the new fbdev driver
> is too late - by that point we've already set up the entire driver,
> including modeset. If fbdev meanwhile does a dpms off or something like
> that all hell will break loose.

I don't understand how fbdev registration comes into play here. Drivers call
remove_conflicting_framebuffers very early so simpledrm is gone by the time
they register anything.

For simpledrm, fbdev doing blank/unblank is a no-op since fb_ops.fb_blank
is not implemented. So a fb_blank() just results in fbcon doing a
software blank.

> I think the only option is to add a new notifier chain for fbdev removal,
> called from remove_conflicting_framebuffers (even for CONFIG_FB=n, so need
> a fallback in core/fb_notify.c like with the other notifier I think). That
> would at least keep things working if fbdev is entirely disabled. Or have
> I entirely misunderstood how this works?

How about extending the new wrapper you just added. Something like this:

static int find_simpledrm_cb(struct device *dev, void *data)
{
 struct platform_device *pdev = to_platform_device(dev);
 char *name = data;

 DRM_INFO("Switching to %s from simpledrm\n", name);
 platform_device_del(pdev);

 return 0;
}

/* not sure where this function should go */
void drm_remove_simpledrm(const char *name)
{
 struct device_drive

[PATCH v4 5/5] drm: simpledrm: honour remove_conflicting_framebuffers()

2016-08-23 Thread Daniel Vetter 
On Mon, Aug 22, 2016 at 10:25:25PM +0200, Noralf Trønnes wrote:
> There is currently no non-fbdev mechanism in place to kick out
> simpledrm when the real hw-driver is probed. As a stop gap until
> that is in place, honour remove_conflicting_framebuffers() and
> delete the simple-framebuffer platform device when it's called.
> 
> Signed-off-by: Noralf Trønnes 
> ---
> 
> Changes from version 3:
> - drm_device.platformdev is deprecated, use to_platform_device(ddev->dev).
> - fb_helper might have been released in sdrm_fbdev_fb_destroy(),
>   so open code drm_fb_helper_release_fbi()
> - Strengthen the test in sdrm_fbdev_event_notify() that we're the one.
> 
> Changes from version 2:
> - Don't forget to free fb_info when kicked out.
> 
>  drivers/gpu/drm/simpledrm/Kconfig   |  5 +++
>  drivers/gpu/drm/simpledrm/simpledrm.h   |  2 +
>  drivers/gpu/drm/simpledrm/simpledrm_drv.c   |  3 ++
>  drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 62 
> -
>  4 files changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/simpledrm/Kconfig 
> b/drivers/gpu/drm/simpledrm/Kconfig
> index 3257590..4275d13 100644
> --- a/drivers/gpu/drm/simpledrm/Kconfig
> +++ b/drivers/gpu/drm/simpledrm/Kconfig
> @@ -16,6 +16,11 @@ config DRM_SIMPLEDRM
> If fbdev support is enabled, this driver will also provide an fbdev
> compatibility layer that supports fbcon, mmap is not supported.
> 
> +   WARNING
> +   fbdev must be enabled for simpledrm to disable itself when a real
> +   hw-driver is probed. It relies on remove_conflicting_framebuffers()
> +   to be called by the hw-driver.
> +
> If unsure, say Y.
> 
> To compile this driver as a module, choose M here: the
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h 
> b/drivers/gpu/drm/simpledrm/simpledrm.h
> index d4eb52c..3cca196 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm.h
> +++ b/drivers/gpu/drm/simpledrm/simpledrm.h
> @@ -87,5 +87,7 @@ int sdrm_fb_init(struct drm_device *ddev, struct 
> sdrm_framebuffer *fb,
>struct sdrm_gem_object *obj);
>  void sdrm_fbdev_init(struct sdrm_device *sdrm);
>  void sdrm_fbdev_cleanup(struct sdrm_device *sdrm);
> +void sdrm_fbdev_kickout_init(void);
> +void sdrm_fbdev_kickout_exit(void);
> 
>  #endif /* SDRM_DRV_H */
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c 
> b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> index fe752c6..0750652 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> @@ -531,12 +531,15 @@ static int __init sdrm_init(void)
>   }
>   }
> 
> + sdrm_fbdev_kickout_init();
> +
>   return 0;
>  }
>  module_init(sdrm_init);
> 
>  static void __exit sdrm_exit(void)
>  {
> + sdrm_fbdev_kickout_exit();
>   platform_driver_unregister(&sdrm_simplefb_driver);
>  }
>  module_exit(sdrm_exit);
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c 
> b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> index c6596ad..7c6db2c 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> @@ -44,6 +44,20 @@ static int sdrm_fb_mmap(struct fb_info *info, struct 
> vm_area_struct *vma)
>   return -ENODEV;
>  }
> 
> +/*
> + * Releasing has to be done outside the notifier callchain when we're
> + * kicked out, since do_unregister_framebuffer() calls put_fb_info()
> + * after the notifier has run.
> + * Open code drm_fb_helper_release_fbi() since fb_helper is freed at
> + * this point when kicked out.
> + */
> +static void sdrm_fbdev_fb_destroy(struct fb_info *info)
> +{
> + if (info->cmap.len)
> + fb_dealloc_cmap(&info->cmap);
> + framebuffer_release(info);
> +}
> +
>  static struct fb_ops sdrm_fbdev_ops = {
>   .owner  = THIS_MODULE,
>   .fb_fillrect= drm_fb_helper_sys_fillrect,
> @@ -53,6 +67,7 @@ static struct fb_ops sdrm_fbdev_ops = {
>   .fb_set_par = drm_fb_helper_set_par,
>   .fb_setcmap = drm_fb_helper_setcmap,
>   .fb_mmap= sdrm_fb_mmap,
> + .fb_destroy = sdrm_fbdev_fb_destroy,
>  };
> 
>  static int sdrm_fbdev_create(struct drm_fb_helper *helper,
> @@ -110,6 +125,9 @@ static int sdrm_fbdev_create(struct drm_fb_helper *helper,
>   fbi->screen_base = obj->vmapping;
>   fbi->fix.smem_len = sdrm->fb_size;
> 
> + fbi->apertures->ranges[0].base = sdrm->fb_base;
> + fbi->apertures->ranges[0].size = sdrm->fb_size;
> +
>   return 0;
> 
>  err_fbi_release:
> @@ -188,9 +206,13 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
>   sdrm->fb_helper = NULL;
>   fbdev = to_sdrm_fbdev(fb_helper);
> 
> - drm_fb_helper_unregister_fbi(fb_helper);
> + /* it might have been kicked out */
> + if (registered_fb[fbdev->fb_helper.fbdev->node])
> + drm_fb_helper_unregister_fbi(fb_helper);
> +
> + /* freeing fb_info is done in fb_ops.fb_destroy() */
> +
>   cancel_

[PATCH v4 5/5] drm: simpledrm: honour remove_conflicting_framebuffers()

2016-08-22 Thread Noralf Trønnes 
There is currently no non-fbdev mechanism in place to kick out
simpledrm when the real hw-driver is probed. As a stop gap until
that is in place, honour remove_conflicting_framebuffers() and
delete the simple-framebuffer platform device when it's called.

Signed-off-by: Noralf Trønnes 
---

Changes from version 3:
- drm_device.platformdev is deprecated, use to_platform_device(ddev->dev).
- fb_helper might have been released in sdrm_fbdev_fb_destroy(),
  so open code drm_fb_helper_release_fbi()
- Strengthen the test in sdrm_fbdev_event_notify() that we're the one.

Changes from version 2:
- Don't forget to free fb_info when kicked out.

 drivers/gpu/drm/simpledrm/Kconfig   |  5 +++
 drivers/gpu/drm/simpledrm/simpledrm.h   |  2 +
 drivers/gpu/drm/simpledrm/simpledrm_drv.c   |  3 ++
 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 62 -
 4 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/simpledrm/Kconfig 
b/drivers/gpu/drm/simpledrm/Kconfig
index 3257590..4275d13 100644
--- a/drivers/gpu/drm/simpledrm/Kconfig
+++ b/drivers/gpu/drm/simpledrm/Kconfig
@@ -16,6 +16,11 @@ config DRM_SIMPLEDRM
  If fbdev support is enabled, this driver will also provide an fbdev
  compatibility layer that supports fbcon, mmap is not supported.

+ WARNING
+ fbdev must be enabled for simpledrm to disable itself when a real
+ hw-driver is probed. It relies on remove_conflicting_framebuffers()
+ to be called by the hw-driver.
+
  If unsure, say Y.

  To compile this driver as a module, choose M here: the
diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h 
b/drivers/gpu/drm/simpledrm/simpledrm.h
index d4eb52c..3cca196 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm.h
+++ b/drivers/gpu/drm/simpledrm/simpledrm.h
@@ -87,5 +87,7 @@ int sdrm_fb_init(struct drm_device *ddev, struct 
sdrm_framebuffer *fb,
 struct sdrm_gem_object *obj);
 void sdrm_fbdev_init(struct sdrm_device *sdrm);
 void sdrm_fbdev_cleanup(struct sdrm_device *sdrm);
+void sdrm_fbdev_kickout_init(void);
+void sdrm_fbdev_kickout_exit(void);

 #endif /* SDRM_DRV_H */
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c 
b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
index fe752c6..0750652 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm_drv.c
+++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
@@ -531,12 +531,15 @@ static int __init sdrm_init(void)
}
}

+   sdrm_fbdev_kickout_init();
+
return 0;
 }
 module_init(sdrm_init);

 static void __exit sdrm_exit(void)
 {
+   sdrm_fbdev_kickout_exit();
platform_driver_unregister(&sdrm_simplefb_driver);
 }
 module_exit(sdrm_exit);
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c 
b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
index c6596ad..7c6db2c 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
+++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
@@ -44,6 +44,20 @@ static int sdrm_fb_mmap(struct fb_info *info, struct 
vm_area_struct *vma)
return -ENODEV;
 }

+/*
+ * Releasing has to be done outside the notifier callchain when we're
+ * kicked out, since do_unregister_framebuffer() calls put_fb_info()
+ * after the notifier has run.
+ * Open code drm_fb_helper_release_fbi() since fb_helper is freed at
+ * this point when kicked out.
+ */
+static void sdrm_fbdev_fb_destroy(struct fb_info *info)
+{
+   if (info->cmap.len)
+   fb_dealloc_cmap(&info->cmap);
+   framebuffer_release(info);
+}
+
 static struct fb_ops sdrm_fbdev_ops = {
.owner  = THIS_MODULE,
.fb_fillrect= drm_fb_helper_sys_fillrect,
@@ -53,6 +67,7 @@ static struct fb_ops sdrm_fbdev_ops = {
.fb_set_par = drm_fb_helper_set_par,
.fb_setcmap = drm_fb_helper_setcmap,
.fb_mmap= sdrm_fb_mmap,
+   .fb_destroy = sdrm_fbdev_fb_destroy,
 };

 static int sdrm_fbdev_create(struct drm_fb_helper *helper,
@@ -110,6 +125,9 @@ static int sdrm_fbdev_create(struct drm_fb_helper *helper,
fbi->screen_base = obj->vmapping;
fbi->fix.smem_len = sdrm->fb_size;

+   fbi->apertures->ranges[0].base = sdrm->fb_base;
+   fbi->apertures->ranges[0].size = sdrm->fb_size;
+
return 0;

 err_fbi_release:
@@ -188,9 +206,13 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
sdrm->fb_helper = NULL;
fbdev = to_sdrm_fbdev(fb_helper);

-   drm_fb_helper_unregister_fbi(fb_helper);
+   /* it might have been kicked out */
+   if (registered_fb[fbdev->fb_helper.fbdev->node])
+   drm_fb_helper_unregister_fbi(fb_helper);
+
+   /* freeing fb_info is done in fb_ops.fb_destroy() */
+
cancel_work_sync(&fb_helper->dirty_work);
-   drm_fb_helper_release_fbi(fb_helper);

drm_framebuffer_unregister_private(fb_helper->fb);
drm_framebuffer_cleanup(fb_helper->fb);
@@ -199,3 +221,39 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sd