Re: [PATCH v2 2/4] drm/i915: add some framework for backlight bl_power support

[Clint Taylor <clinton.a.taylor@intel.com>]

On 08/13/2014 02:10 AM, Jani Nikula wrote:
> Make backlight class sysfs bl_power a sub-state of backlight enabled, if
> a backlight power connector callback is defined. It's up to the
> connector callback to handle the sub-state, typically in a way that
> respects panel power sequencing.
>
> v2: Post the version that does not oops. *facepalm*.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_drv.h   |  2 ++
>   drivers/gpu/drm/i915/intel_panel.c | 26 ++++++++++++++++++++++++++
>   2 files changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1b3d1d7e466e..43b7b6609f0e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -173,6 +173,8 @@ struct intel_panel {
>   		bool active_low_pwm;
>   		struct backlight_device *device;
>   	} backlight;
> +
> +	void (*backlight_power)(struct intel_connector *, bool enable);
>   };
>
>   struct intel_connector {
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 59b028f0b1e8..af5435634929 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -751,6 +751,8 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
>
>   	spin_lock_irqsave(&dev_priv->backlight_lock, flags);
>
> +	if (panel->backlight.device)
> +		panel->backlight.device->props.power = FB_BLANK_POWERDOWN;
>   	panel->backlight.enabled = false;
>   	dev_priv->display.disable_backlight(connector);
>
> @@ -957,6 +959,8 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
>
>   	dev_priv->display.enable_backlight(connector);
>   	panel->backlight.enabled = true;
> +	if (panel->backlight.device)
> +		panel->backlight.device->props.power = FB_BLANK_UNBLANK;
>
>   	spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
>   }
> @@ -965,6 +969,7 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
>   static int intel_backlight_device_update_status(struct backlight_device *bd)
>   {
>   	struct intel_connector *connector = bl_get_data(bd);
> +	struct intel_panel *panel = &connector->panel;
>   	struct drm_device *dev = connector->base.dev;
>
>   	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> @@ -972,6 +977,22 @@ static int intel_backlight_device_update_status(struct backlight_device *bd)
>   		      bd->props.brightness, bd->props.max_brightness);
>   	intel_panel_set_backlight(connector, bd->props.brightness,
>   				  bd->props.max_brightness);
> +
> +	/*
> +	 * Allow flipping bl_power as a sub-state of enabled. Sadly the
> +	 * backlight class device does not make it easy to to differentiate
> +	 * between callbacks for brightness and bl_power, so our backlight_power
> +	 * callback needs to take this into account.
> +	 */
> +	if (panel->backlight.enabled) {
> +		if (panel->backlight_power) {
> +			bool enable = bd->props.power == FB_BLANK_UNBLANK;
> +			panel->backlight_power(connector, enable);
> +		}
> +	} else {
> +		bd->props.power = FB_BLANK_POWERDOWN;
> +	}
> +
>   	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>   	return 0;
>   }
> @@ -1023,6 +1044,11 @@ static int intel_backlight_device_register(struct intel_connector *connector)
>   					    panel->backlight.level,
>   					    props.max_brightness);
>
> +	if (panel->backlight.enabled)
> +		props.power = FB_BLANK_UNBLANK;
> +	else
> +		props.power = FB_BLANK_POWERDOWN;
> +
>   	/*
>   	 * Note: using the same name independent of the connector prevents
>   	 * registration of multiple backlight devices in the driver.
>
Reviewed_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Tested_by: Clinton Taylor <Clinton.A.Taylor@intel.com>