[Intel-gfx] [PATCH] drm/i915/cnp: Backlight support for CNP.

Srivatsa, Anusha anusha.srivatsa at intel.com
Fri May 5 20:36:04 UTC 2017



>-----Original Message-----
>From: Pandiyan, Dhinakaran
>Sent: Friday, May 5, 2017 1:35 PM
>To: Nikula, Jani <jani.nikula at intel.com>
>Cc: intel-gfx at lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi at intel.com>;
>Taylor, Clinton A <clinton.a.taylor at intel.com>; Srivatsa, Anusha
><anusha.srivatsa at intel.com>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915/cnp: Backlight support for CNP.
>
>On Fri, 2017-05-05 at 19:50 +0300, Jani Nikula wrote:
>> On Fri, 05 May 2017, "Srivatsa, Anusha" <anusha.srivatsa at intel.com> wrote:
>> >>-----Original Message-----
>> >>From: Nikula, Jani
>> >>Sent: Thursday, May 4, 2017 2:25 AM
>> >>To: Srivatsa, Anusha <anusha.srivatsa at intel.com>; intel-
>> >>gfx at lists.freedesktop.org
>> >>Cc: Vivi, Rodrigo <rodrigo.vivi at intel.com>; Ville Syrjala
>> >><ville.syrjala at linux.intel.com>; Srivatsa, Anusha
>> >><anusha.srivatsa at intel.com>
>> >>Subject: Re: [PATCH] drm/i915/cnp: Backlight support for CNP.
>> >>
>> >>On Wed, 03 May 2017, Anusha Srivatsa <anusha.srivatsa at intel.com> wrote:
>> >>> From: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> >>>
>> >>> Split out BXT and CNP's setup_backlight(),enable_backlight(),
>> >>> disable_backlight() and hz_to_pwm() into two separate functions
>> >>> instead of reusing BXT function.
>> >>>
>> >>> Reuse set_backlight() and get_backlight() since they have no
>> >>> reference to the utility pin.
>> >>>
>> >>> v2: Reuse BXT functions with controller 0 instead of
>> >>>     redefining it. (Jani).
>> >>>     Use dev_priv->rawclk_freq instead of getting the value
>> >>>     from SFUSE_STRAP.
>> >>> v3: Avoid setup backligh controller along with hooks and
>> >>>     fully reuse hooks setup as suggested by Jani.
>> >>> v4: Clean up commit message.
>> >>> v5: Implement per PCH instead per platform.
>> >>>
>> >>> v6: Introduce a new function for CNP.(Jani and Ville)
>> >>>
>> >>> v7: Squash the all CNP Backlight support patches into a single patch.
>> >>> (Jani)
>> >>>
>> >>> v8: Correct indentation, remove unneeded blank lines and correct
>> >>> mail address (Jani).
>> >>>
>> >>> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
>> >>
>> >>Yup. What's the plan for merging the series, incl. this patch?
>> >
>> > Yes. This will be merged as part of CNP series.
>>
>> Of course, but what's the plan for merging the series?
>>
>> BR,
>> Jani.
>>
>
>This patch is 4/67 in Rodrigo's series. It makes sense to merge the top six (CNP
>PCH) patches in Rodrigo's series and then focus on the rest after that. Out of the
>top six, 6/67 still needs a R-B.
>
>Anusha, can you please rebase and submit Rodrigo's first six patches (replacing
>4/67 with this) ?
>
Will do.

Anusha
>-DK
>
>
>>
>> >
>> > Anusha
>> >>BR,
>> >>Jani.
>> >>
>> >>
>> >>> Suggested-by: Jani Nikula <jani.nikula at intel.com>
>> >>> Suggested-by: Ville Syrjala <ville.syrjala at intel.com>
>> >>> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
>> >>> Cc: Jani Nikula <jani.nikula at intel.com>
>> >>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> >>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> >>> ---
>> >>>  drivers/gpu/drm/i915/intel_panel.c | 88
>> >>> +++++++++++++++++++++++++++++++++++---
>> >>>  1 file changed, 83 insertions(+), 5 deletions(-)
>> >>>
>> >>> diff --git a/drivers/gpu/drm/i915/intel_panel.c
>> >>> b/drivers/gpu/drm/i915/intel_panel.c
>> >>> index 1978bec..8ee61c1 100644
>> >>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> >>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> >>> @@ -796,6 +796,19 @@ static void bxt_disable_backlight(struct
>> >>intel_connector *connector)
>> >>>  	}
>> >>>  }
>> >>>
>> >>> +static void cnp_disable_backlight(struct intel_connector
>> >>> +*connector) {
>> >>> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> >>> +	struct intel_panel *panel = &connector->panel;
>> >>> +	u32 tmp, val;
>> >>> +
>> >>> +	intel_panel_actually_set_backlight(connector, 0);
>> >>> +
>> >>> +	tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>> >>> +	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
>> >>> +		   tmp & ~BXT_BLC_PWM_ENABLE);
>> >>> +}
>> >>> +
>> >>>  static void pwm_disable_backlight(struct intel_connector
>> >>> *connector) {
>> >>>  	struct intel_panel *panel = &connector->panel; @@ -1076,6
>> >>> +1089,36 @@ static void bxt_enable_backlight(struct intel_connector
>*connector)
>> >>>  			pwm_ctl | BXT_BLC_PWM_ENABLE);  }
>> >>>
>> >>> +static void cnp_enable_backlight(struct intel_connector *connector) {
>> >>> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> >>> +	struct intel_panel *panel = &connector->panel;
>> >>> +	enum pipe pipe = intel_get_pipe_from_connector(connector);
>> >>> +	u32 pwm_ctl, val;
>> >>> +
>> >>> +	pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>> >>> +	if (pwm_ctl & BXT_BLC_PWM_ENABLE) {
>> >>> +		DRM_DEBUG_KMS("backlight already enabled\n");
>> >>> +		pwm_ctl &= ~BXT_BLC_PWM_ENABLE;
>> >>> +		I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
>> >>> +			   pwm_ctl);
>> >>> +	}
>> >>> +
>> >>> +	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
>> >>> +		   panel->backlight.max);
>> >>> +
>> >>> +	intel_panel_actually_set_backlight(connector,
>> >>> +panel->backlight.level);
>> >>> +
>> >>> +	pwm_ctl = 0;
>> >>> +	if (panel->backlight.active_low_pwm)
>> >>> +		pwm_ctl |= BXT_BLC_PWM_POLARITY;
>> >>> +
>> >>> +	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), pwm_ctl);
>> >>> +	POSTING_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>> >>> +	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
>> >>> +		   pwm_ctl | BXT_BLC_PWM_ENABLE); }
>> >>> +
>> >>>  static void pwm_enable_backlight(struct intel_connector
>> >>> *connector) {
>> >>>  	struct intel_panel *panel = &connector->panel; @@ -1645,6
>> >>> +1688,37 @@ bxt_setup_backlight(struct intel_connector *connector,
>> >>> enum pipe
>> >>unused)
>> >>>  	return 0;
>> >>>  }
>> >>>
>> >>> +static int
>> >>> +cnp_setup_backlight(struct intel_connector *connector, enum pipe
>> >>> +unused) {
>> >>> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> >>> +	struct intel_panel *panel = &connector->panel;
>> >>> +	u32 pwm_ctl, val;
>> >>> +
>> >>> +	panel->backlight.controller =
>> >>> +dev_priv->vbt.backlight.controller;
>> >>> +
>> >>> +	pwm_ctl =
>> >>> +I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>> >>> +
>> >>> +	panel->backlight.active_low_pwm = pwm_ctl &
>> >>BXT_BLC_PWM_POLARITY;
>> >>> +	panel->backlight.max =
>> >>> +		I915_READ(BXT_BLC_PWM_FREQ(panel->backlight.controller));
>> >>> +
>> >>> +	if (!panel->backlight.max)
>> >>> +		panel->backlight.max = get_backlight_max_vbt(connector);
>> >>> +
>> >>> +	if (!panel->backlight.max)
>> >>> +		return -ENODEV;
>> >>> +
>> >>> +	val = bxt_get_backlight(connector);
>> >>> +	val = intel_panel_compute_brightness(connector, val);
>> >>> +	panel->backlight.level = clamp(val, panel->backlight.min,
>> >>> +				       panel->backlight.max);
>> >>> +
>> >>> +	panel->backlight.enabled = pwm_ctl & BXT_BLC_PWM_ENABLE;
>> >>> +
>> >>> +	return 0;
>> >>> +}
>> >>> +
>> >>>  static int pwm_setup_backlight(struct intel_connector *connector,
>> >>>  			       enum pipe pipe)
>> >>>  {
>> >>> @@ -1754,16 +1828,20 @@ intel_panel_init_backlight_funcs(struct
>> >>> intel_panel
>> >>*panel)
>> >>>  	    intel_dsi_dcs_init_backlight_funcs(connector) == 0)
>> >>>  		return;
>> >>>
>> >>> -	if (IS_GEN9_LP(dev_priv) || HAS_PCH_CNP(dev_priv)) {
>> >>> +	if (IS_GEN9_LP(dev_priv)) {
>> >>>  		panel->backlight.setup = bxt_setup_backlight;
>> >>>  		panel->backlight.enable = bxt_enable_backlight;
>> >>>  		panel->backlight.disable = bxt_disable_backlight;
>> >>>  		panel->backlight.set = bxt_set_backlight;
>> >>>  		panel->backlight.get = bxt_get_backlight;
>> >>> -		if (IS_GEN9_LP(dev_priv))
>> >>> -			panel->backlight.hz_to_pwm = bxt_hz_to_pwm;
>> >>> -		else
>> >>> -			panel->backlight.hz_to_pwm = cnp_hz_to_pwm;
>> >>> +		panel->backlight.hz_to_pwm = bxt_hz_to_pwm;
>> >>> +	} else if (HAS_PCH_CNP(dev_priv)) {
>> >>> +		panel->backlight.setup = cnp_setup_backlight;
>> >>> +		panel->backlight.enable = cnp_enable_backlight;
>> >>> +		panel->backlight.disable = cnp_disable_backlight;
>> >>> +		panel->backlight.set = bxt_set_backlight;
>> >>> +		panel->backlight.get = bxt_get_backlight;
>> >>> +		panel->backlight.hz_to_pwm = cnp_hz_to_pwm;
>> >>>  	} else if (HAS_PCH_LPT(dev_priv) || HAS_PCH_SPT(dev_priv) ||
>> >>>  		   HAS_PCH_KBP(dev_priv)) {
>> >>>  		panel->backlight.setup = lpt_setup_backlight;
>> >>
>> >>--
>> >>Jani Nikula, Intel Open Source Technology Center
>>



More information about the Intel-gfx mailing list