On Tue, Apr 22, 2014 at 10:26:37AM +0200, Daniel Vetter wrote: > On Thu, Apr 17, 2014 at 10:38:17AM +0200, Thierry Reding wrote: > > On Wed, Apr 16, 2014 at 04:45:21PM +0200, Daniel Vetter wrote: > > > Some drivers need to be able to have a perfect race-free fbcon setup. > > > Current drivers only enable hotplug processing after the call to > > > drm_fb_helper_initial_config which leaves a tiny but important race. > > > > > > This race is especially noticable on embedded platforms where the > > > driver itself enables the voltage for the hdmi output, since only then > > > will monitors (after a bit of delay, as usual) respond by asserting > > > the hpd pin. > > > > > > Most of the infrastructure is already there with the split-out > > > drm_fb_helper_init. And drm_fb_helper_initial_config already has all > > > the required locking to handle concurrent hpd events since > > > > > > commit 53f1904bced78d7c00f5d874c662ec3ac85d0f9f > > > Author: Daniel Vetter <daniel.vetter@xxxxxxxx> > > > Date: Thu Mar 20 14:26:35 2014 +0100 > > > > > > drm/fb-helper: improve drm_fb_helper_initial_config locking > > > > > > The only missing bit is making drm_fb_helper_hotplug_event save > > > against concurrent calls of drm_fb_helper_initial_config. The only > > > unprotected bit is the check for fb_helper->fb. > > > > > > With that drivers can first initialize the fb helper, then enabel > > > hotplug processing and then set up the initial config all in a > > > completely race-free manner. Update kerneldoc and convert i915 as a > > > proof of concept. > > > > > > Feature requested by Thierry since his tegra driver atm reliably boots > > > slowly enough to misses the hotplug event for an external hdmi screen, > > > but also reliably boots to quickly for the hpd pin to be asserted when > > > the fb helper calls into the hdmi ->detect function. > > > > > > Cc: Thierry Reding <treding@xxxxxxxxxx> > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > > --- > > > drivers/gpu/drm/drm_fb_helper.c | 11 +++++------ > > > drivers/gpu/drm/i915/i915_dma.c | 3 --- > > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > > drivers/gpu/drm/i915/i915_drv.h | 1 - > > > drivers/gpu/drm/i915/i915_irq.c | 4 ---- > > > 5 files changed, 5 insertions(+), 16 deletions(-) > > > > The FB helper parts: > > > > Tested-by: Thierry Reding <treding@xxxxxxxxxx> > > > > But I have one comment below... > > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > [...] > > > - * Note that the driver must ensure that this is only called _after_ the fb has > > > - * been fully set up, i.e. after the call to drm_fb_helper_initial_config. > > > + * Note that drivers may call this even before calling > > > + * drm_fb_helper_initial_config but only aftert drm_fb_helper_init. This allows > > > > I don't think the requirement is that strict. To elaborate: on Tegra we > > cannot call drm_fb_helper_init() because the number of CRTCs and > > connectors isn't known this early. We determine that dynamically after > > all sub-devices have been initialized. So instead of calling > > drm_fb_helper_init() before drm_kms_helper_poll_init(), I did something > > more minimal (see attached patch). It's kind of difficult to tell > > because of the context, but tegra_drm_fb_prepare() sets up the mode > > config and functions and allocate memory for the FB helper and sets the > > FB helper functions. > > > > This may not be enough for all drivers, but on Tegra the implementation > > of .output_poll_changed() simply calls drm_fb_helper_hotplug_event(), > > which will work fine with just that rudimentary initialization. > > Hm yeah I think this should be sufficient, too. It would be good to > extract this minimal initialization into a new drm_fb_helper_prepare > function and update the kerneldoc a bit more. Maybe as a patch on top of > mine? > > Then we could merge this all as an early tegra-next pull to Dave. Sounds like a good idea. I'll go prepare a patch. Thierry
Attachment:
pgpNfGBjKlXb0.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
