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

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Aug 23, 2016 at 7:52 PM, Noralf Trønnes <noralf@tronnes.org> 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