Re: [PATCH] drm/i915/cnp: Backlight support for CNP.

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

On Thu, 2017-05-04 at 12:24 +0300, Jani Nikula wrote:
> On Wed, 03 May 2017, Anusha Srivatsa <anusha.srivatsa@intel.com> wrote:
> > From: Rodrigo Vivi <rodrigo.vivi@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@intel.com>
> 
> Yup. What's the plan for merging the series, incl. this patch?

I will:
1. rebase all patches here with cnp - cfl - cnl
2. fix that patch with proper gmbus pin table.
3. provide the workaround on vbt parsing as ville had suggested.
4. re-test on the platforms here.
5. Resubmit only first patches that would be ready for merging already
and start merging those, and continue subminting in small chunks. 

So, Jani, could you please review already this
one:https://patchwork.freedesktop.org/patch/155177/

Thanks a lot,
Rodrigo.

> BR,
> Jani.
> 
> 
> > Suggested-by: Jani Nikula <jani.nikula@intel.com>
> > Suggested-by: Ville Syrjala <ville.syrjala@intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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;
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx