diff mbox

[3/4] drm/i915/dp: make backlight bl_power control power sequencer backlight

Message ID ede4679618e8968fc0e1199461d9cb7bc1f1a60f.1407849872.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jani Nikula Aug. 12, 2014, 2:11 p.m. UTC
This lets the userspace switch off the backlight using the backlight
class sysfs bl_power file. The switch is done using the power sequencer;
the backlight PWM, and everything else, remains enabled. The display
backlight won't draw power, but for maximum power savings the encoder
needs to be switched off.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Taylor, Clinton A Aug. 18, 2014, 5:44 p.m. UTC | #1
On 08/12/2014 07:11 AM, Jani Nikula wrote:
> This lets the userspace switch off the backlight using the backlight
> class sysfs bl_power file. The switch is done using the power sequencer;
> the backlight PWM, and everything else, remains enabled. The display
> backlight won't draw power, but for maximum power savings the encoder
> needs to be switched off.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d8baf60ff3fd..f6e3e9a906b0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1453,6 +1453,27 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
>   	intel_panel_disable_backlight(intel_dp->attached_connector);
>   }
>
> +/*
> + * Hook for controlling the panel power control backlight through the bl_power
> + * sysfs attribute. Take care to handle multiple calls.
> + */
> +static void intel_edp_backlight_power(struct intel_connector *connector,
> +				      bool enable)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
> +	bool is_enabled = ironlake_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
> +
> +	if (is_enabled == enable)
> +		return;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	if (enable)
> +		_intel_edp_backlight_on(intel_dp);
> +	else
> +		_intel_edp_backlight_off(intel_dp);

Is there a good reason why we leave the PWM enabled? There is a small 
power requirement to leave the PWM enabled.

> +}
> +
>   static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
>   {
>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -4579,6 +4600,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>   	}
>
>   	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> +	intel_connector->panel.backlight_power = intel_edp_backlight_power;
>   	intel_panel_setup_backlight(connector);
>
>   	return true;
>
Jani Nikula Aug. 19, 2014, 5:36 a.m. UTC | #2
On Mon, 18 Aug 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> On 08/12/2014 07:11 AM, Jani Nikula wrote:
>> This lets the userspace switch off the backlight using the backlight
>> class sysfs bl_power file. The switch is done using the power sequencer;
>> the backlight PWM, and everything else, remains enabled. The display
>> backlight won't draw power, but for maximum power savings the encoder
>> needs to be switched off.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index d8baf60ff3fd..f6e3e9a906b0 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1453,6 +1453,27 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
>>   	intel_panel_disable_backlight(intel_dp->attached_connector);
>>   }
>>
>> +/*
>> + * Hook for controlling the panel power control backlight through the bl_power
>> + * sysfs attribute. Take care to handle multiple calls.
>> + */
>> +static void intel_edp_backlight_power(struct intel_connector *connector,
>> +				      bool enable)
>> +{
>> +	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
>> +	bool is_enabled = ironlake_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
>> +
>> +	if (is_enabled == enable)
>> +		return;
>> +
>> +	DRM_DEBUG_KMS("\n");
>> +
>> +	if (enable)
>> +		_intel_edp_backlight_on(intel_dp);
>> +	else
>> +		_intel_edp_backlight_off(intel_dp);
>
> Is there a good reason why we leave the PWM enabled? There is a small 
> power requirement to leave the PWM enabled.

Implementation simplicity. If you want to go for max power savings, you
are supposed to switch everything off using KMS, not just backlight.

BR,
Jani.


>
>> +}
>> +
>>   static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
>>   {
>>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> @@ -4579,6 +4600,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>   	}
>>
>>   	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
>> +	intel_connector->panel.backlight_power = intel_edp_backlight_power;
>>   	intel_panel_setup_backlight(connector);
>>
>>   	return true;
>>
>
Taylor, Clinton A Aug. 19, 2014, 11:03 p.m. UTC | #3
On 08/12/2014 07:11 AM, Jani Nikula wrote:
> This lets the userspace switch off the backlight using the backlight
> class sysfs bl_power file. The switch is done using the power sequencer;
> the backlight PWM, and everything else, remains enabled. The display
> backlight won't draw power, but for maximum power savings the encoder
> needs to be switched off.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d8baf60ff3fd..f6e3e9a906b0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1453,6 +1453,27 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
>   	intel_panel_disable_backlight(intel_dp->attached_connector);
>   }
>
> +/*
> + * Hook for controlling the panel power control backlight through the bl_power
> + * sysfs attribute. Take care to handle multiple calls.
> + */
> +static void intel_edp_backlight_power(struct intel_connector *connector,
> +				      bool enable)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
> +	bool is_enabled = ironlake_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
> +
> +	if (is_enabled == enable)
> +		return;
> +
> +	DRM_DEBUG_KMS("\n");
> +
> +	if (enable)
> +		_intel_edp_backlight_on(intel_dp);
> +	else
> +		_intel_edp_backlight_off(intel_dp);
> +}
> +
>   static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
>   {
>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -4579,6 +4600,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>   	}
>
>   	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> +	intel_connector->panel.backlight_power = intel_edp_backlight_power;
>   	intel_panel_setup_backlight(connector);
>
>   	return true;
>
Reviewed_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
Tested_by: Clinton Taylor <Clinton.A.Taylor@intel.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d8baf60ff3fd..f6e3e9a906b0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1453,6 +1453,27 @@  void intel_edp_backlight_off(struct intel_dp *intel_dp)
 	intel_panel_disable_backlight(intel_dp->attached_connector);
 }
 
+/*
+ * Hook for controlling the panel power control backlight through the bl_power
+ * sysfs attribute. Take care to handle multiple calls.
+ */
+static void intel_edp_backlight_power(struct intel_connector *connector,
+				      bool enable)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
+	bool is_enabled = ironlake_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
+
+	if (is_enabled == enable)
+		return;
+
+	DRM_DEBUG_KMS("\n");
+
+	if (enable)
+		_intel_edp_backlight_on(intel_dp);
+	else
+		_intel_edp_backlight_off(intel_dp);
+}
+
 static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -4579,6 +4600,7 @@  static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	}
 
 	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+	intel_connector->panel.backlight_power = intel_edp_backlight_power;
 	intel_panel_setup_backlight(connector);
 
 	return true;