Den 23.08.2016 20:01, skrev Daniel Vetter:
On Tue, Aug 23, 2016 at 7:52 PM, Noralf Trønnes <noralf@xxxxxxxxxxx> 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.
--
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
