[Intel-gfx] [PATCH] drm/fb-helper: Fix hpd vs. initial config races

[Thierry Reding]

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 at ffwll.ch>
> 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 at nvidia.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  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 at nvidia.com>

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.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-tegra-Implement-race-free-hotplug-detection.patch
Type: text/x-diff
Size: 6077 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20140417/74e1a373/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20140417/74e1a373/attachment.sig>