Re: [PATCH 4/7] drm/i915: clean up backlight conditional build

[Joe Konno <joe.konno@linux.intel.com>]

On 10/31/2013 09:55 AM, Jani Nikula wrote:
> I've always felt the backlight device conditional build has been all
> backwards. Make it feel right.
>
> Gently move things towards connector based stuff while at it.
>
> There should be no functional changes, except for a slight
> reordering/interleaving of connector backlight and sysfs destroy calls.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c |    9 +++--
>   drivers/gpu/drm/i915/intel_drv.h     |    2 +-
>   drivers/gpu/drm/i915/intel_panel.c   |   65 +++++++++++++++++++---------------
>   3 files changed, 42 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 606a594..774407d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11007,12 +11007,11 @@ 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);
> -
> -	/* destroy the sysfs files before encoders/connectors */
> -	list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> +	/* destroy the backlight and sysfs files before encoders/connectors */
> +	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> +		intel_panel_destroy_backlight(connector);
>   		drm_sysfs_connector_remove(connector);
> +	}
>
>   	drm_mode_config_cleanup(dev);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1e49aa8..5548180 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -808,7 +808,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
>   int intel_panel_setup_backlight(struct drm_connector *connector);
>   void intel_panel_enable_backlight(struct intel_connector *connector);
>   void intel_panel_disable_backlight(struct intel_connector *connector);
> -void intel_panel_destroy_backlight(struct drm_device *dev);
> +void intel_panel_destroy_backlight(struct drm_connector *connector);
>   enum drm_connector_status intel_panel_detect(struct drm_device *dev);
>
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index f161ac0..a0d13d3 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -709,16 +709,6 @@ static void intel_panel_init_backlight_regs(struct drm_device *dev)
>   	}
>   }
>
> -static void intel_panel_init_backlight(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	intel_panel_init_backlight_regs(dev);
> -
> -	dev_priv->backlight.level = intel_panel_get_backlight(dev, 0);
> -	dev_priv->backlight.enabled = dev_priv->backlight.level != 0;
> -}
> -
>   enum drm_connector_status
>   intel_panel_detect(struct drm_device *dev)
>   {
> @@ -742,7 +732,7 @@ intel_panel_detect(struct drm_device *dev)
>   }
>
>   #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> -static int intel_panel_update_status(struct backlight_device *bd)
> +static int intel_backlight_device_update_status(struct backlight_device *bd)
>   {
>   	struct intel_connector *connector = bl_get_data(bd);
>   	struct drm_device *dev = connector->base.dev;
> @@ -756,7 +746,7 @@ static int intel_panel_update_status(struct backlight_device *bd)
>   	return 0;
>   }
>
> -static int intel_panel_get_brightness(struct backlight_device *bd)
> +static int intel_backlight_device_get_brightness(struct backlight_device *bd)
>   {
>   	struct intel_connector *connector = bl_get_data(bd);
>   	struct drm_device *dev = connector->base.dev;
> @@ -771,20 +761,18 @@ static int intel_panel_get_brightness(struct backlight_device *bd)
>   	return intel_panel_get_backlight(connector->base.dev, pipe);
>   }
>
> -static const struct backlight_ops intel_panel_bl_ops = {
> -	.update_status = intel_panel_update_status,
> -	.get_brightness = intel_panel_get_brightness,
> +static const struct backlight_ops intel_backlight_device_ops = {
> +	.update_status = intel_backlight_device_update_status,
> +	.get_brightness = intel_backlight_device_get_brightness,
>   };
>
> -int intel_panel_setup_backlight(struct drm_connector *connector)
> +static int intel_backlight_device_register(struct intel_connector *connector)
>   {
> -	struct drm_device *dev = connector->dev;
> +	struct drm_device *dev = connector->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct backlight_properties props;
>   	unsigned long flags;
>
> -	intel_panel_init_backlight(dev);
> -
>   	if (WARN_ON(dev_priv->backlight.device))
>   		return -ENODEV;
>
> @@ -802,9 +790,9 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
>   	}
>   	dev_priv->backlight.device =
>   		backlight_device_register("intel_backlight",
> -					  connector->kdev,
> -					  to_intel_connector(connector),
> -					  &intel_panel_bl_ops, &props);
> +					  connector->base.kdev,
> +					  connector,
> +					  &intel_backlight_device_ops, &props);
>
>   	if (IS_ERR(dev_priv->backlight.device)) {
>   		DRM_ERROR("Failed to register backlight: %ld\n",
> @@ -815,26 +803,47 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
>   	return 0;
>   }
>
> -void intel_panel_destroy_backlight(struct drm_device *dev)
> +static void intel_backlight_device_unregister(struct intel_connector *connector)
>   {
> +	struct drm_device *dev = connector->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	if (dev_priv->backlight.device) {
>   		backlight_device_unregister(dev_priv->backlight.device);
>   		dev_priv->backlight.device = NULL;
>   	}
>   }
> -#else
> +#else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
> +static int intel_backlight_device_register(struct intel_connector *connector)
> +{
> +	return 0;
> +}
> +static void intel_backlight_device_unregister(struct intel_connector *connector)
> +{
> +}
> +#endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
> +
>   int intel_panel_setup_backlight(struct drm_connector *connector)
>   {
> -	intel_panel_init_backlight(connector->dev);
> +	struct drm_device *dev = connector->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +
> +	intel_panel_init_backlight_regs(dev);
> +
> +	dev_priv->backlight.level = intel_panel_get_backlight(dev, 0);
> +	dev_priv->backlight.enabled = dev_priv->backlight.level != 0;
> +
> +	intel_backlight_device_register(intel_connector);
> +
>   	return 0;
>   }
>
> -void intel_panel_destroy_backlight(struct drm_device *dev)
> +void intel_panel_destroy_backlight(struct drm_connector *connector)
>   {
> -	return;
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +
> +	intel_backlight_device_unregister(intel_connector);
>   }
> -#endif
>
>   int intel_panel_init(struct intel_panel *panel,
>   		     struct drm_display_mode *fixed_mode)
>

Tested-by: Joe Konno <joe.konno@intel.com>