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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxxx>
---

<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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux