Re: [PATCH 3/4] drm/i915: ensure single initialization and cleanup of backlight device

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 12 Apr 2013, Jani Nikula <jani.nikula@intel.com> wrote:
> Backlight cleanup in the eDP connector destroy callback caused the
> backlight device to be removed on some systems that first initialized LVDS
> and then attempted to initialize eDP. Prevent multiple backlight
> initializations, and ensure backlight cleanup is only done once by moving
> it to modeset cleanup.
>
> A small wrinkle is the introduced asymmetry in backlight
> setup/cleanup. This could be solved by adding refcounting, but it seems
> overkill considering that there should only ever be one backlight device.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=55701

Via bugzilla,

Tested-by: Peter Verthez <peter.verthez@skynet.be>

CC: Stable?

> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    3 +++
>  drivers/gpu/drm/i915/intel_dp.c      |    5 +----
>  drivers/gpu/drm/i915/intel_lvds.c    |    1 -
>  drivers/gpu/drm/i915/intel_panel.c   |    7 ++++++-
>  4 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 457a0a0..712b0af 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9419,6 +9419,9 @@ void intel_modeset_cleanup(struct drm_device *dev)
>  	/* flush any delayed tasks or pending work */
>  	flush_scheduled_work();
>  
> +	/* destroy backlight, if any, before the connectors */
> +	intel_panel_destroy_backlight(dev);
> +
>  	drm_mode_config_cleanup(dev);
>  
>  	intel_cleanup_overlay(dev);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 173add1..8845e82 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2474,17 +2474,14 @@ done:
>  static void
>  intel_dp_destroy(struct drm_connector *connector)
>  {
> -	struct drm_device *dev = connector->dev;
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  
>  	if (!IS_ERR_OR_NULL(intel_connector->edid))
>  		kfree(intel_connector->edid);
>  
> -	if (is_edp(intel_dp)) {
> -		intel_panel_destroy_backlight(dev);
> +	if (is_edp(intel_dp))
>  		intel_panel_fini(&intel_connector->panel);
> -	}
>  
>  	drm_sysfs_connector_remove(connector);
>  	drm_connector_cleanup(connector);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index ca2d903..f36f1ba 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -631,7 +631,6 @@ static void intel_lvds_destroy(struct drm_connector *connector)
>  	if (!IS_ERR_OR_NULL(lvds_connector->base.edid))
>  		kfree(lvds_connector->base.edid);
>  
> -	intel_panel_destroy_backlight(connector->dev);
>  	intel_panel_fini(&lvds_connector->base.panel);
>  
>  	drm_sysfs_connector_remove(connector);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 464c3d7..5d3e9d7 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -459,6 +459,9 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
>  
>  	intel_panel_init_backlight(dev);
>  
> +	if (WARN_ON(dev_priv->backlight.device))
> +		return -ENODEV;
> +
>  	memset(&props, 0, sizeof(props));
>  	props.type = BACKLIGHT_RAW;
>  	props.brightness = dev_priv->backlight.level;
> @@ -488,8 +491,10 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
>  void intel_panel_destroy_backlight(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	if (dev_priv->backlight.device)
> +	if (dev_priv->backlight.device) {
>  		backlight_device_unregister(dev_priv->backlight.device);
> +		dev_priv->backlight.device = NULL;
> +	}
>  }
>  #else
>  int intel_panel_setup_backlight(struct drm_connector *connector)
> -- 
> 1.7.9.5