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

[Pandiyan, Dhinakaran]

On Tue, 2017-05-30 at 15:42 -0700, Rodrigo Vivi wrote:
> 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).
> 
> v9: Remove unused enum pipe. (by CI)
> 
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
> 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 | 93 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index c8103f8..7e34a1b 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;
> +
> +	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;
> @@ -1086,6 +1099,35 @@ 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;
> +	u32 pwm_ctl;
> +
> +	pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));

Shouldn't this be BLC_PWM_PCH_CTL1?

I think reusing CPU register definitions for PCH is confusing. Even more
so, when we already have separate definitions for PCH.

BSpec specifically refers to these registers as SBLC_PWM_CTL1,
SBLC_PWM_FREQ and SBLC_PWM_DUTY.




> +	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;
> @@ -1250,6 +1292,18 @@ void intel_backlight_device_unregister(struct intel_connector *connector)
>  #endif /* CONFIG_BACKLIGHT_CLASS_DEVICE */
>  
>  /*
> + * CNP: PWM clock frequency is 19.2 MHz or 24 MHz.
> + *      Value is found in SFUSE_STRAP.
> + *      PWM increment = 1
> + */
> +static u32 cnp_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +
> +	return DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), pwm_freq_hz);
> +}
> +
> +/*
>   * BXT: PWM clock frequency = 19.2 MHz.
>   */
>  static u32 bxt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> @@ -1644,6 +1698,37 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe
>  	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;

Are there are two controllers in CNP? I did not find any references in
BSpec, can you please confirm?


> +
> +	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)
>  {
> @@ -1760,6 +1845,14 @@ void intel_panel_destroy_backlight(struct drm_connector *connector)
>  		panel->backlight.set = bxt_set_backlight;
>  		panel->backlight.get = bxt_get_backlight;
>  		panel->backlight.hz_to_pwm = bxt_hz_to_pwm;
> +		panel->backlight.hz_to_pwm = bxt_hz_to_pwm;
^Spurious line.

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