lkml.org 
[lkml]   [2016]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 5/5] drm: simpledrm: honour remove_conflicting_framebuffers()
From
Date

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 <noralf@tronnes.org>
>> ---

<snip>

>> 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_driver *drv;

drv = driver_find("simpledrm", &platform_bus_type);
if (drv)
driver_for_each_device(drv, NULL, name, find_simpledrm_cb);

/* or we can just unregister the driver instead */
if (drv) {
struct platform_driver *pdrv = to_platform_driver(drv);

DRM_INFO("simpledrm is being removed by %s\n", name);
platform_driver_unregister(pdrv);
}
}

static inline int
drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
const char *name, bool primary)
{
int ret;

ret = remove_conflicting_framebuffers(a, name, primary);
drm_remove_simpledrm(name);

return ret;
}
#else
static inline int
drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
const char *name, bool primary)
{
drm_remove_simpledrm(name);

return 0;
}

This removes simpledrm unconditionally without looking at apertures_struct
though. Not sure if that is needed?


Noralf.

> Oh and another one, which we learned the hard way with i915: We probably
> need to get rid of vgacon too. And again we need to do that _before_ we
> start touching the hardware. I think simpledrm probably needs that too.
> But since simpledrm doesn't know on which pci device it sits it probably
> won't know if the same device is decoding vga signals, so no one knows how
> that is supposed to work :( One thing at the time probably.
> -Daniel
>
>> +
>> +static struct notifier_block sdrm_fbdev_event_notifier = {
>> + .notifier_call = sdrm_fbdev_event_notify,
>> +};
>> +
>> +void sdrm_fbdev_kickout_init(void)
>> +{
>> + fb_register_client(&sdrm_fbdev_event_notifier);
>> +}
>> +
>> +void sdrm_fbdev_kickout_exit(void)
>> +{
>> + fb_unregister_client(&sdrm_fbdev_event_notifier);
>> +}
>> --
>> 2.8.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

\
 
 \ /
  Last update: 2016-09-17 09:57    [W:0.093 / U:1.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site