On Fri, 07 Nov 2014, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Currently we register the backlight device as soon as we register the
> connector. That means we can get backlight requests from userspace
> already before reading out the current modeset hardware state.
>
> That means we don't yet know the current crtc->encoder->connector mapping,
> which causes problems for VLV/CHV which need to know the current pipe in
> order to figure out which BLC registers to poke. Currently we just
> ignore such requests fairly deep in the backlight code which means the
> backlight device brightness property will get out of sync with our
> backlight.level and the actual hardware state.
>
> Fix the problem by delaying the backlight device registration until the
> entire modeset init has been performed. And we also move the
> backlight unregisteration to happen as the first thing during the
> modeset cleanup so that we also won't be bothered with userspace
> backlight requested during teardown.
>
> This is a real world problem on machines using systemd, because systemd,
> for some reason, wants to restore the backlight to the level it used last
> time. And that happens as soon as it sees the backlight device appearing
> in the system. Sometimes the userspace access makes it through before
> the modeset init, sometimes not.
>
> v2: Do not lie to the user in the debug prints (Jani)
> Include connector name in the prints (Jani)
> Fix a typo in the commit message (Jani)
>
> Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Yup.
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 ++++
> drivers/gpu/drm/i915/intel_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_panel.c | 33 ++++++++++++++++++++++++++-------
> 3 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b9529d0..158d65b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13378,6 +13378,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
> }
> }
> mutex_unlock(&dev->struct_mutex);
> +
> + intel_backlight_register(dev);
> }
>
> void intel_connector_unregister(struct intel_connector *intel_connector)
> @@ -13393,6 +13395,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_connector *connector;
>
> + intel_backlight_unregister(dev);
> +
> /*
> * Interrupts and polling as the first thing to avoid creating havoc.
> * Too much stuff here (turning of rps, connectors, ...) would
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0ff011e..2a1f790 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1113,6 +1113,9 @@ extern struct drm_display_mode *intel_find_panel_downclock(
> struct drm_device *dev,
> struct drm_display_mode *fixed_mode,
> struct drm_connector *connector);
> +void intel_backlight_register(struct drm_device *dev);
> +void intel_backlight_unregister(struct drm_device *dev);
> +
>
> /* intel_runtime_pm.c */
> int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 69bbfba..708642a 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1041,6 +1041,9 @@ static int intel_backlight_device_register(struct intel_connector *connector)
> if (WARN_ON(panel->backlight.device))
> return -ENODEV;
>
> + if (!panel->backlight.present)
> + return 0;
> +
> WARN_ON(panel->backlight.max == 0);
>
> memset(&props, 0, sizeof(props));
> @@ -1076,6 +1079,10 @@ static int intel_backlight_device_register(struct intel_connector *connector)
> panel->backlight.device = NULL;
> return -ENODEV;
> }
> +
> + DRM_DEBUG_KMS("Connector %s backlight sysfs interface registered\n",
> + connector->base.name);
> +
> return 0;
> }
>
> @@ -1302,15 +1309,12 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
> return ret;
> }
>
> - intel_backlight_device_register(intel_connector);
> -
> panel->backlight.present = true;
>
> - DRM_DEBUG_KMS("backlight initialized, %s, brightness %u/%u, "
> - "sysfs interface %sregistered\n",
> + DRM_DEBUG_KMS("Connector %s backlight initialized, %s, brightness %u/%u\n",
> + connector->name,
> panel->backlight.enabled ? "enabled" : "disabled",
> - panel->backlight.level, panel->backlight.max,
> - panel->backlight.device ? "" : "not ");
> + panel->backlight.level, panel->backlight.max);
>
> return 0;
> }
> @@ -1321,7 +1325,6 @@ void intel_panel_destroy_backlight(struct drm_connector *connector)
> struct intel_panel *panel = &intel_connector->panel;
>
> panel->backlight.present = false;
> - intel_backlight_device_unregister(intel_connector);
> }
>
> /* Set up chip specific backlight functions */
> @@ -1384,3 +1387,19 @@ void intel_panel_fini(struct intel_panel *panel)
> drm_mode_destroy(intel_connector->base.dev,
> panel->downclock_mode);
> }
> +
> +void intel_backlight_register(struct drm_device *dev)
> +{
> + struct intel_connector *connector;
> +
> + list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
> + intel_backlight_device_register(connector);
> +}
> +
> +void intel_backlight_unregister(struct drm_device *dev)
> +{
> + struct intel_connector *connector;
> +
> + list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
> + intel_backlight_device_unregister(connector);
> +}
> --
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
