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

2014-08-19 Thread Clint Taylor

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 
---
  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 
Tested_by: Clinton Taylor 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2014-08-18 Thread Jani Nikula
On Mon, 18 Aug 2014, Clint Taylor  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 
>> ---
>>   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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2014-08-18 Thread Clint Taylor

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



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2014-08-12 Thread Jani Nikula
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 
---
 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;
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx