On Fri, Jun 03, 2016 at 06:11:16PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> FB helper code falls back to a 1024x768 mode if no outputs are connected
> or don't report back any modes upon initialization. This can be annoying
> because outputs that are added to FB helper later on can't be used with
> FB helper if they don't support a matching mode.
>
> The fallback is in place because VGA connectors can happen to report an
> unknown connection status even when they are in fact connected.
>
> Some drivers have custom solutions in place to defer FB helper setup
> until at least one output is connected. But the logic behind these
> solutions is always the same and there is nothing driver-specific about
> it, so a better alterative is to fix the FB helper core and add support
> for all drivers automatically.
>
> This patch adds support for deferred FB helper setup. It checks all the
> connectors for their connection status, and if all of them report to be
> disconnected marks the FB helper as needing deferred setup. Whet setup
> is deferred, the FB helper core will automatically retry setup after a
> hotplug event, and it will keep trying until it succeeds.
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 36 ++++++++++++++++++++++++++++++++++++
> include/drm/drm_fb_helper.h | 21 +++++++++++++++++++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 7c2eb75db60f..4f334f9cab28 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -442,6 +442,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> if (!drm_fbdev_emulation)
> return -ENODEV;
>
> + if (fb_helper->deferred_setup)
> + return 0;
> +
> drm_modeset_lock_all(dev);
> ret = restore_fbdev_mode(fb_helper);
>
> @@ -1398,6 +1401,23 @@ unlock:
> }
> EXPORT_SYMBOL(drm_fb_helper_pan_display);
>
> +static bool drm_fb_helper_maybe_connected(struct drm_fb_helper *helper)
> +{
> + bool connected = false;
> + unsigned int i;
> +
> + for (i = 0; i < helper->connector_count; i++) {
> + struct drm_fb_helper_connector *fb = helper->connector_info[i];
> +
> + if (fb->connector->status != connector_status_disconnected) {
> + connected = true;
> + break;
> + }
> + }
> +
> + return connected;
> +}
> +
> /*
> * Allocates the backing storage and sets up the fbdev info structure through
> * the ->fb_probe callback and then registers the fbdev and sets up the panic
> @@ -1499,6 +1519,17 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
> }
>
> + /*
> + * If everything's disconnected, there's no use in attempting to set
> + * up fbdev.
> + */
> + if (!drm_fb_helper_maybe_connected(fb_helper)) {
> + DRM_INFO("No outputs connected, deferring setup\n");
> + fb_helper->preferred_bpp = preferred_bpp;
> + fb_helper->deferred_setup = true;
> + return 0;
> + }
> +
> if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
> /* hmm everyone went away - assume VGA cable just fell out
> and will come back later. */
> @@ -1539,6 +1570,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>
> list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);
>
> + fb_helper->deferred_setup = false;
> return 0;
> }
>
> @@ -2237,6 +2269,10 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> if (!drm_fbdev_emulation)
> return 0;
>
> + if (fb_helper->deferred_setup)
> + return drm_fb_helper_initial_config(fb_helper,
> + fb_helper->preferred_bpp);
Logic looks all nice, but we need more locking. Extending our abuse of
dev->mode_config.mutex for fb_helper internals won't work since
initial_config already takes some locks at various times, we need a real
fb_helper->mutex now. We want that anyway, to be able to push a bunch of
the modeset locking out of the fbdev code, or at least down a lot.
To make it work we need to make it a top-level lock, surrounding all kms
locks I think. And minimally it needs to wrapt hotplug_event, restore_mode
and the single_add/remove_connector functions.
-Daniel
> +
> mutex_lock(&fb_helper->dev->mode_config.mutex);
> if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
> fb_helper->delayed_hotplug = true;
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 5b4aa35026a3..25446690b2c1 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -214,6 +214,27 @@ struct drm_fb_helper {
> bool delayed_hotplug;
>
> /**
> + * @deferred_setup:
> + *
> + * If no outputs are connected (disconnected or unknown) the FB helper
> + * code will defer setup until at least one of the outputs shows up.
> + * This field keeps track of the status so that setup can be retried
> + * at every hotplug event until it succeeds eventually.
> + */
> + bool deferred_setup;
> +
> + /**
> + * @preferred_bpp:
> + *
> + * Temporary storage for the driver's preferred BPP setting passed to
> + * FB helper initialization. This needs to be tracked so that deferred
> + * FB helper setup can pass this on.
> + *
> + * See also: @deferred_setup
> + */
> + int preferred_bpp;
> +
> + /**
> * @atomic:
> *
> * Use atomic updates for restore_fbdev_mode(), etc. This defaults to
> --
> 2.8.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
