From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH 3/4] drm/i915/dp: make backlight bl_power control power sequencer backlight Date: Tue, 19 Aug 2014 08:36:28 +0300 Message-ID: <87mwb1hwgz.fsf@intel.com> References: <53F23B70.4080008@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id DD66889BB0 for ; Mon, 18 Aug 2014 22:36:42 -0700 (PDT) In-Reply-To: <53F23B70.4080008@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Clint Taylor , intel-gfx@lists.freedesktop.org Cc: rodrigo.vivi@intel.com, kevin.strasser@intel.com, jesse.barnes@intel.com List-Id: intel-gfx@lists.freedesktop.org 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