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

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

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;
>>
>

-- 
Jani Nikula, Intel Open Source Technology Center