Re: [PATCH v3 3/3] drm: simpledrm: honour remove_conflicting_framebuffers()

Daniel Vetter
2016-08-15T07:12:57Z

On Sun, Aug 14, 2016 at 06:52:06PM +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>

Pure coincidence, but I just merge a patch to stub out
remove_conflicting_framebuffers with a drm_fb_helper.c function. I guess
we could lift that one to core (probably needs an extended signature) and
extend it. But this should get the job done for now I think.
-Daniel

> ---
>
> Changes from version 2:
> - Don't forget to free fb_info when kicked out.
>
> drivers/gpu/drm/simpledrm/Kconfig | 5 +++
> drivers/gpu/drm/simpledrm/Makefile | 2 +-
> drivers/gpu/drm/simpledrm/simpledrm.h | 11 +++++-
> drivers/gpu/drm/simpledrm/simpledrm_drv.c | 3 ++
> drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 55 +++++++++++++++++++++++++++--
> 5 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig
> index 9454536..6205b17 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.
>
> + 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/Makefile b/drivers/gpu/drm/simpledrm/Makefile
> index 7087245..4b4bcdd 100644
> --- a/drivers/gpu/drm/simpledrm/Makefile
> +++ b/drivers/gpu/drm/simpledrm/Makefile
> @@ -1,5 +1,5 @@
> simpledrm-y := simpledrm_drv.o simpledrm_kms.o simpledrm_gem.o \
> simpledrm_damage.o
> -simpledrm-$(CONFIG_DRM_FBDEV_EMULATION) += simpledrm_fbdev.o
> +simpledrm-$(CONFIG_FB) += simpledrm_fbdev.o
>
> obj-$(CONFIG_DRM_SIMPLEDRM) := simpledrm.o
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h b/drivers/gpu/drm/simpledrm/simpledrm.h
> index f01b75d..7bc1292 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm.h
> +++ b/drivers/gpu/drm/simpledrm/simpledrm.h
> @@ -88,13 +88,15 @@ struct sdrm_framebuffer {
>
> #define to_sdrm_fb(x) container_of(x, struct sdrm_framebuffer, base)
>
> -#ifdef CONFIG_DRM_FBDEV_EMULATION
> +#ifdef CONFIG_FB
>
> void sdrm_fbdev_init(struct sdrm_device *sdrm);
> void sdrm_fbdev_cleanup(struct sdrm_device *sdrm);
> void sdrm_fbdev_display_pipe_update(struct sdrm_device *sdrm,
> struct drm_framebuffer *fb);
> void sdrm_fbdev_restore_mode(struct sdrm_device *sdrm);
> +void sdrm_fbdev_kickout_init(void);
> +void sdrm_fbdev_kickout_exit(void);
>
> #else
>
> @@ -115,6 +117,13 @@ static inline void sdrm_fbdev_restore_mode(struct sdrm_device *sdrm)
> {
> }
>
> +static inline void sdrm_fbdev_kickout_init(void)
> +{
> +}
> +
> +static inline void sdrm_fbdev_kickout_exit(void)
> +{
> +}
> #endif
>
> #endif /* SDRM_DRV_H */
> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> index a4e6566..26956c3 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
> @@ -527,12 +527,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 4038dd9..daf5943 100644
> --- a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
> @@ -28,6 +28,16 @@ static inline struct sdrm_fbdev *to_sdrm_fbdev(struct drm_fb_helper *helper)
> return container_of(helper, struct sdrm_fbdev, fb_helper);
> }
>
> +/*
> + * 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.
> + */
> +static void sdrm_fbdev_fb_destroy(struct fb_info *info)
> +{
> + drm_fb_helper_release_fbi(info->par);
> +}
> +
> static struct fb_ops sdrm_fbdev_ops = {
> .owner = THIS_MODULE,
> .fb_fillrect = drm_fb_helper_cfb_fillrect,
> @@ -36,6 +46,7 @@ static struct fb_ops sdrm_fbdev_ops = {
> .fb_check_var = drm_fb_helper_check_var,
> .fb_set_par = drm_fb_helper_set_par,
> .fb_setcmap = drm_fb_helper_setcmap,
> + .fb_destroy = sdrm_fbdev_fb_destroy,
> };
>
> static struct drm_framebuffer_funcs sdrm_fb_funcs = {
> @@ -85,6 +96,9 @@ static int sdrm_fbdev_create(struct drm_fb_helper *helper,
> fbi->fix.smem_len = sdrm->fb_size;
> fbi->screen_base = sdrm->fb_map;
>
> + fbi->apertures->ranges[0].base = sdrm->fb_base;
> + fbi->apertures->ranges[0].size = sdrm->fb_size;
> +
> return 0;
>
> err_fb_info_destroy:
> @@ -154,8 +168,11 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
> sdrm->fbdev = NULL;
> fb_helper = &fbdev->fb_helper;
>
> - drm_fb_helper_unregister_fbi(fb_helper);
> - drm_fb_helper_release_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() */
>
> drm_framebuffer_unregister_private(&fbdev->fb);
> drm_framebuffer_cleanup(&fbdev->fb);
> @@ -199,3 +216,37 @@ void sdrm_fbdev_restore_mode(struct sdrm_device *sdrm)
> if (fbdev->fb_helper.fbdev->state != FBINFO_STATE_RUNNING)
> sdrm_fbdev_set_suspend(fbdev->fb_helper.fbdev, 0);
> }
> +
> +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 || !fb_helper ||
> + !fb_helper->dev || fb_helper->fbdev != info)
> + return NOTIFY_DONE;
> +
> + sdrm = fb_helper->dev->dev_private;
> +
> + if (sdrm && sdrm->fbdev)
> + platform_device_del(sdrm->ddev->platformdev);
> +
> + return NOTIFY_DONE;
> +}
> +
> +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

--
Daniel Vetter
Software Engineer, Intel Corporation