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

[Vivi, Rodrigo]

On Thu, 2017-05-04 at 12:24 +0300, Jani Nikula wrote:
> On Wed, 03 May 2017, Anusha Srivatsa  wrote:
> > From: Rodrigo Vivi 
> >
> > 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 
> 
> 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 
> > Suggested-by: Ville Syrjala 
> > Cc: Ville Syrjala 
> > Cc: Jani Nikula 
> > Signed-off-by: Anusha Srivatsa 
> > Signed-off-by: Rodrigo Vivi 
> > ---
> >  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 = >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 = >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 = >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 = >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 

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

[Srivatsa, Anusha]

>-Original Message-
>From: Pandiyan, Dhinakaran
>Sent: Friday, May 5, 2017 1:35 PM
>To: Nikula, Jani <jani.nik...@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo <rodrigo.v...@intel.com>;
>Taylor, Clinton A <clinton.a.tay...@intel.com>; Srivatsa, Anusha
><anusha.sriva...@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.sriva...@intel.com> wrote:
>> >>-Original Message-
>> >>From: Nikula, Jani
>> >>Sent: Thursday, May 4, 2017 2:25 AM
>> >>To: Srivatsa, Anusha <anusha.sriva...@intel.com>; intel-
>> >>g...@lists.freedesktop.org
>> >>Cc: Vivi, Rodrigo <rodrigo.v...@intel.com>; Ville Syrjala
>> >><ville.syrj...@linux.intel.com>; Srivatsa, Anusha
>> >><anusha.sriva...@intel.com>
>> >>Subject: Re: [PATCH] drm/i915/cnp: Backlight support for CNP.
>> >>
>> >>On Wed, 03 May 2017, Anusha Srivatsa <anusha.sriva...@intel.com> wrote:
>> >>> From: Rodrigo Vivi <rodrigo.v...@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.nik...@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.nik...@intel.com>
>> >>> Suggested-by: Ville Syrjala <ville.syrj...@intel.com>
>> >>> Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
>> >>> Cc: Jani Nikula <jani.nik...@intel.com>
>> >>> Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com>
>> >>> Signed-off-by: Rodrigo Vivi <rodrigo.v...@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_i91

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

[Pandiyan, Dhinakaran]

On Fri, 2017-05-05 at 19:50 +0300, Jani Nikula wrote:
> On Fri, 05 May 2017, "Srivatsa, Anusha"  wrote:
> >>-Original Message-
> >>From: Nikula, Jani
> >>Sent: Thursday, May 4, 2017 2:25 AM
> >>To: Srivatsa, Anusha ; intel-
> >>g...@lists.freedesktop.org
> >>Cc: Vivi, Rodrigo ; Ville Syrjala
> >>; Srivatsa, Anusha 
> >>
> >>Subject: Re: [PATCH] drm/i915/cnp: Backlight support for CNP.
> >>
> >>On Wed, 03 May 2017, Anusha Srivatsa  wrote:
> >>> From: Rodrigo Vivi 
> >>>
> >>> 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 
> >>
> >>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) ? 


-DK


> 
> >
> > Anusha
> >>BR,
> >>Jani.
> >>
> >>
> >>> Suggested-by: Jani Nikula 
> >>> Suggested-by: Ville Syrjala 
> >>> Cc: Ville Syrjala 
> >>> Cc: Jani Nikula 
> >>> Signed-off-by: Anusha Srivatsa 
> >>> Signed-off-by: Rodrigo Vivi 
> >>> ---
> >>>  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 = >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 = >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 = >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);
> >>> + 

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

[Jani Nikula]

On Fri, 05 May 2017, "Srivatsa, Anusha"  wrote:
>>-Original Message-
>>From: Nikula, Jani
>>Sent: Thursday, May 4, 2017 2:25 AM
>>To: Srivatsa, Anusha ; intel-
>>g...@lists.freedesktop.org
>>Cc: Vivi, Rodrigo ; Ville Syrjala
>>; Srivatsa, Anusha 
>>Subject: Re: [PATCH] drm/i915/cnp: Backlight support for CNP.
>>
>>On Wed, 03 May 2017, Anusha Srivatsa  wrote:
>>> From: Rodrigo Vivi 
>>>
>>> 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 
>>
>>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.


>
> Anusha
>>BR,
>>Jani.
>>
>>
>>> Suggested-by: Jani Nikula 
>>> Suggested-by: Ville Syrjala 
>>> Cc: Ville Syrjala 
>>> Cc: Jani Nikula 
>>> Signed-off-by: Anusha Srivatsa 
>>> Signed-off-by: Rodrigo Vivi 
>>> ---
>>>  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 = >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 = >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 = >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 = >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 = 

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

[Srivatsa, Anusha]

>-Original Message-
>From: Nikula, Jani
>Sent: Thursday, May 4, 2017 2:25 AM
>To: Srivatsa, Anusha ; intel-
>g...@lists.freedesktop.org
>Cc: Vivi, Rodrigo ; Ville Syrjala
>; Srivatsa, Anusha 
>Subject: Re: [PATCH] drm/i915/cnp: Backlight support for CNP.
>
>On Wed, 03 May 2017, Anusha Srivatsa  wrote:
>> From: Rodrigo Vivi 
>>
>> 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 
>
>Yup. What's the plan for merging the series, incl. this patch?

Yes. This will be merged as part of CNP series.

Anusha
>BR,
>Jani.
>
>
>> Suggested-by: Jani Nikula 
>> Suggested-by: Ville Syrjala 
>> Cc: Ville Syrjala 
>> Cc: Jani Nikula 
>> Signed-off-by: Anusha Srivatsa 
>> Signed-off-by: Rodrigo Vivi 
>> ---
>>  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 = >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 = >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 = >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 = >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 = >panel;
>> +u32 pwm_ctl, val;
>> +
>> +panel->backlight.controller = dev_priv->vbt.backlight.controller;
>> +
>> +pwm_ctl = 

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

[Jani Nikula]

On Wed, 03 May 2017, Anusha Srivatsa  wrote:
> From: Rodrigo Vivi 
>
> 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 

Yup. What's the plan for merging the series, incl. this patch?

BR,
Jani.


> Suggested-by: Jani Nikula 
> Suggested-by: Ville Syrjala 
> Cc: Ville Syrjala 
> Cc: Jani Nikula 
> Signed-off-by: Anusha Srivatsa 
> Signed-off-by: Rodrigo Vivi 
> ---
>  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 = >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 = >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 = >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 = >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 = >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);
> + 

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

[Anusha Srivatsa]

From: Rodrigo Vivi 

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 
Suggested-by: Jani Nikula 
Suggested-by: Ville Syrjala 
Cc: Ville Syrjala 
Cc: Jani Nikula 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Rodrigo Vivi 
---
 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 = >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 = >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 = >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 = >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 = >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,
  

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

[Anusha Srivatsa]

From: Rodrigo Vivi 

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).

Suggested-by: Jani Nikula 
Suggested-by: Ville Syrjala 
Cc: Ville Syrjala 
Cc: Jani Nikula 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Rodrigo Vivi 
---
 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 = >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 = >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 = >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 = >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 = >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 @@