From: Daniel Vetter <email@example.com> To: Liviu Dudau <Liviu.Dudau@arm.com> Cc: Daniel Vetter <firstname.lastname@example.org>, Intel Graphics Development <email@example.com>, DRI Development <firstname.lastname@example.org>, John Stultz <email@example.com>, Thierry Reding <firstname.lastname@example.org> Subject: Re: [PATCH] drm/fb-helper: Restore first connection behaviour on deferred setup Date: Mon, 3 Jul 2017 18:33:19 +0200 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <20170703084450.GH1103@e110455-lin.cambridge.arm.com> On Mon, Jul 03, 2017 at 09:44:50AM +0100, Liviu Dudau wrote: > On Fri, Jun 30, 2017 at 08:13:58PM +0200, Daniel Vetter wrote: > > On Fri, Jun 30, 2017 at 6:51 PM, Liviu Dudau <Liviu.Dudau@arm.com> wrote: > > > Prior to commit b0aa06e9a7fd ("drm/fb-helper: Support deferred setup"), > > > if no output is connected at framebuffer setup time, we get a default > > > 1024x768 mode that is going to be used when we first connect a monitor. > > > After the commit, on first connection after deferred setup, we probe > > > the monitor and get the preferred resolution, but no mode get set > > > because the drm_fb_helper_hotplug_event() function returns early > > > when the setup has been deferred. That is different from what happens > > > on a second re-connect of the monitor, when the native mode get set. > > > > > > Create a more consistent behaviour by checking in the > > > drm_fb_helper_hotplug_event() function if the deferred setup is still > > > active. If not, that means we now have a valid framebuffer that can be > > > used for setting the correct mode. > > > > > > Fixes: b0aa06e9a7fd ("drm/fb-helper: Support deferred setup") > > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > > > Cc: Daniel Vetter <firstname.lastname@example.org> > > > > I thought the analysis over irc was that fbcon picked a different > > driver as it's console, and that's why nothing shows up on the malidp > > output in the deferred case? That's mildly annoying, but iirc fbcon > > has always been rather erratic in multi-gpu setups. Although I thought > > that it would by default bind all fbdev drivers as consoles (and then > > you need to rebind the right console driver, if the right Kconfig is > > enabled, through sysfs). > > That's right, fbcon picks a different driver and binds to it. But > before the patch, on the first connection with a monitor that happened > after setup (i.e. when the fbdev emulation mode set the default 1024x768 > mode), a full atomic modeset was trigger for the driver that had the > connection and the "card" output was enabled. Now with your patch that > does not happen on the first connection, but it does after I turn off and > back on the monitor. I know that is how most computer problems are solved, > but still ... :) > > > > > Either way if the register_framebuffer() call in initial_config isn't > > good enough, then we need to add the set_par in initial_config > > unconditionally, not just in the deferred probe case. Just disable > > fbcon entirely for testing, in that case even without deferred probing > > nothing will show up. > > Actually the un-ballanced call to set_par happens with your patch (it > gets called for all hotplug events that are *not* deferred. I was trying > to balance things and call set_par also for deferred setups where the > __drm_fb_helper_initial_config() call succeeded. And the fact that we do > call set_par in the hotplug path even when the setup is not deferred > tells me that register_framebuffer() is probably not enough. But then you end up doing it twice if that's the fbdev fbcon will bind too. And of course we must call set_par to enable the newly hotplugged screen, it'd be black otherwise. It's just that with just 1 gpu, the register_framebuffer takes care of the initial config stuff (if you have fbcon enabled). If you boot with just 1 gpu, and hotplug that one, it will work correctly. Multi-gpu + fbcon just don't work in a predictable fashion. > > I'd say if this is still needed in the single gpu case then we need to > > investigate more, but for multi-gpu it is what it is (aka fbcon is not > > great). > > I think single gpu case hides the problem because fbcon forces the right > sequence of calls. And I think the old behaviour was correct, as I was > getting output of the second "card" on hotplug, now I have to turn the > monitor off and on again to get some signal sync. So the problem is that we rely upon register_framebuffer->fbcon to create that initial set_par. We could add an unconditional set_par to fix this bug (the condition really is pre-existing, if you'd boot with 2 gpus and both connected at start-up the 2nd one would still be off). But that comes at the cost of forcing everyone to do a 2nd, unecessary modeset right after boot-up, potentially destroying neat tricks like fastboot. So we can't just do that either, because that would regress boot-up time. Now if you have an idea how to fix this without rewriting fbcon+fbdev, we could try, but I'm not sure that's feasible really ... After all if you entirely disable fbdev emulation, your screen also stays off. And maybe you event _want_ it to stay off, even with fbdev emulation enabled. So really no idea what exactly we should do here, but adding an unconditional set_par in the initial_config path isn't the solution I think. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intelemail@example.com https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-07-03 16:33 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-06-27 14:59 [PATCH 00/13] fbdev locking rework and deferred setup, take 2 Daniel Vetter 2017-06-27 14:59 ` [PATCH 01/13] drm/fb-helper: Push down modeset lock into FB helpers Daniel Vetter 2017-06-29 9:10 ` Maarten Lankhorst 2017-06-29 9:23 ` Daniel Vetter 2017-06-29 9:33 ` Maarten Lankhorst 2017-06-29 9:44 ` Daniel Vetter 2017-06-29 11:13 ` Maarten Lankhorst 2017-06-29 12:38 ` Daniel Vetter 2017-06-27 14:59 ` [PATCH 02/13] drm/i915: Drop FBDEV #ifdev in mst code Daniel Vetter 2017-06-27 14:59 ` [PATCH 03/13] drm/fb-helper: Add top-level lock Daniel Vetter 2017-06-27 14:59 ` [PATCH 04/13] drm/fb-helper: Push locking in fb_is_bound Daniel Vetter 2017-06-27 14:59 ` [PATCH 05/13] drm/fb-helper: Drop locking from the vsync wait ioctl code Daniel Vetter 2017-06-27 14:59 ` [PATCH 06/13] drm/fb-helper: Push locking into pan_display_atomic|legacy Daniel Vetter 2017-06-27 14:59 ` [PATCH 07/13] drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy Daniel Vetter 2017-06-27 14:59 ` [PATCH 08/13] drm/fb-helper: Stop using mode_config.mutex for internals Daniel Vetter 2017-06-27 14:59 ` [PATCH 09/13] drm/fb-helper: Split dpms handling into legacy and atomic paths Daniel Vetter 2017-06-29 10:22 ` Maarten Lankhorst 2017-06-29 10:31 ` Daniel Vetter 2017-06-29 10:58 ` Maarten Lankhorst 2017-06-29 11:00 ` Daniel Vetter 2017-06-29 11:23 ` Maarten Lankhorst 2017-06-27 14:59 ` [PATCH 10/13] drm/fb-helper: Support deferred setup Daniel Vetter 2017-06-28 11:32 ` [PATCH] " Daniel Vetter 2017-06-28 16:24 ` Liviu Dudau 2017-06-29 10:59 ` Maarten Lankhorst 2017-06-29 12:36 ` Daniel Vetter 2017-06-30 16:51 ` [PATCH] drm/fb-helper: Restore first connection behaviour on " Liviu Dudau 2017-06-30 18:13 ` Daniel Vetter 2017-07-03 8:44 ` Liviu Dudau 2017-07-03 16:33 ` Daniel Vetter [this message] 2017-06-27 14:59 ` [PATCH 11/13] drm/exynos: Remove custom FB helper " Daniel Vetter 2017-06-27 14:59 ` [PATCH 12/13] drm/hisilicon: " Daniel Vetter 2017-06-28 9:08 ` [PATCH] " Daniel Vetter 2017-06-27 14:59 ` [PATCH 13/13] drm/atomic-helper: Realign function parameters Daniel Vetter 2017-06-27 15:01 ` Deucher, Alexander 2017-06-27 15:42 ` Harry Wentland 2017-07-04 15:16 ` Daniel Vetter 2017-06-27 15:30 ` ✓ Fi.CI.BAT: success for fbdev locking rework and deferred setup, take 2 Patchwork 2017-06-27 23:02 ` [PATCH 00/13] " John Stultz 2017-06-28 7:36 ` Daniel Vetter 2017-06-28 9:28 ` ✓ Fi.CI.BAT: success for fbdev locking rework and deferred setup, take 2 (rev2) Patchwork 2017-06-28 12:28 ` ✓ Fi.CI.BAT: success for fbdev locking rework and deferred setup, take 2 (rev3) Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org \ --email@example.com \ --cc=Liviu.Dudau@arm.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH] drm/fb-helper: Restore first connection behaviour on deferred setup' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.