Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Pandiyan, Dhinakaran]

On Tue, 2017-05-16 at 17:39 -0700, Puthikorn Voravootivat wrote:
> 
> 
> On Tue, May 16, 2017 at 2:21 PM, Pandiyan, Dhinakaran
>  wrote:
> On Tue, 2017-05-16 at 13:56 -0700, Puthikorn Voravootivat
> wrote:
> >
> >
> > On Tue, May 16, 2017 at 1:29 PM, Pandiyan, Dhinakaran
> >  wrote:
> > On Tue, 2017-05-16 at 11:07 -0700, Puthikorn
> Voravootivat
> > wrote:
> > >
> > >
> > > On Mon, May 15, 2017 at 11:21 PM, Pandiyan,
> Dhinakaran
> > >  wrote:
> > > On Mon, 2017-05-15 at 17:43 -0700,
> Puthikorn
> > Voravootivat
> > > wrote:
> > > >
> > > >
> > > > On Mon, May 15, 2017 at 4:07 PM,
> Pandiyan,
> > Dhinakaran
> > > >  wrote:
> > > > On Fri, 2017-05-12 at 17:31
> -0700,
> > Puthikorn
> > > Voravootivat
> > > > wrote:
> > > > >
> > > > >
> > > > >
> > > > > On Fri, May 12, 2017 at 5:12
> PM,
> > Pandiyan,
> > > Dhinakaran
> > > > >
>  wrote:
> > > > > On Thu, 2017-05-11 at
> 16:02
> > -0700,
> > > Puthikorn
> > > > Voravootivat
> > > > > wrote:
> > > > > > Read desired PWM
> frequency
> > from panel
> > > vbt and
> > > > calculate the
> > > > > > value for divider in
> DPCD
> > address 0x724
> > > and 0x728
> > > > to have
> > > > > > as many bits as
> possible for
> > PWM duty
> > > cyle for
> > > > granularity
> > > > > of
> > > > > > brightness
> adjustment while
> > the
> > > frequency is still
> > > > within
> > > > > 25%
> > > > > > of the desired
> frequency.
> > > > >
> > > > > I read a few eDP panel
> data
> > sheets, the
> > > PWM
> > > > frequencies all
> > > > > start from
> > > > > ~200Hz. If the VBT
> chooses this
> > lowest
> > > value to
> > > > allow for more
> > > > > brightness control,
> and then
> > this patch
> > > lowers the
> > > > value by
> > > > > another 25%,
> > > > > we'll end up below the
> panel
> > allowed PWM
> > > frequency.
> > > > >
> > > > > In fact, one of the
> systems I
> > checked had
> > > PWM
> > > > frequency as
> > > > > 200Hz in VBT
> > > > > and the panel
> datasheet also had
> > PWM
> > > frequency range
> > > > starting
> > > > > from
> > > > > 200Hz. Have you
> considered this
> > case?
> > > > >
> > > > > The spec said "A given LCD
> panel
> > typically has a
> > > limited
> > > > range of
> > > > > backlight frequency
> capability.
> >  

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Puthikorn Voravootivat]

On Tue, May 16, 2017 at 2:21 PM, Pandiyan, Dhinakaran <
dhinakaran.pandi...@intel.com> wrote:

> On Tue, 2017-05-16 at 13:56 -0700, Puthikorn Voravootivat wrote:
> >
> >
> > On Tue, May 16, 2017 at 1:29 PM, Pandiyan, Dhinakaran
> >  wrote:
> > On Tue, 2017-05-16 at 11:07 -0700, Puthikorn Voravootivat
> > wrote:
> > >
> > >
> > > On Mon, May 15, 2017 at 11:21 PM, Pandiyan, Dhinakaran
> > >  wrote:
> > > On Mon, 2017-05-15 at 17:43 -0700, Puthikorn
> > Voravootivat
> > > wrote:
> > > >
> > > >
> > > > On Mon, May 15, 2017 at 4:07 PM, Pandiyan,
> > Dhinakaran
> > > >  wrote:
> > > > On Fri, 2017-05-12 at 17:31 -0700,
> > Puthikorn
> > > Voravootivat
> > > > wrote:
> > > > >
> > > > >
> > > > >
> > > > > On Fri, May 12, 2017 at 5:12 PM,
> > Pandiyan,
> > > Dhinakaran
> > > > >  wrote:
> > > > > On Thu, 2017-05-11 at 16:02
> > -0700,
> > > Puthikorn
> > > > Voravootivat
> > > > > wrote:
> > > > > > Read desired PWM frequency
> > from panel
> > > vbt and
> > > > calculate the
> > > > > > value for divider in DPCD
> > address 0x724
> > > and 0x728
> > > > to have
> > > > > > as many bits as possible for
> > PWM duty
> > > cyle for
> > > > granularity
> > > > > of
> > > > > > brightness adjustment while
> > the
> > > frequency is still
> > > > within
> > > > > 25%
> > > > > > of the desired frequency.
> > > > >
> > > > > I read a few eDP panel data
> > sheets, the
> > > PWM
> > > > frequencies all
> > > > > start from
> > > > > ~200Hz. If the VBT chooses this
> > lowest
> > > value to
> > > > allow for more
> > > > > brightness control, and then
> > this patch
> > > lowers the
> > > > value by
> > > > > another 25%,
> > > > > we'll end up below the panel
> > allowed PWM
> > > frequency.
> > > > >
> > > > > In fact, one of the systems I
> > checked had
> > > PWM
> > > > frequency as
> > > > > 200Hz in VBT
> > > > > and the panel datasheet also had
> > PWM
> > > frequency range
> > > > starting
> > > > > from
> > > > > 200Hz. Have you considered this
> > case?
> > > > >
> > > > > The spec said "A given LCD panel
> > typically has a
> > > limited
> > > > range of
> > > > > backlight frequency capability.
> > > > > To limit the programmable frequency
> > range,
> > > limitations are
> > > > placed on
> > > > > the allowable total divider ratio with
> > the Sink
> > > device"
> > > > >  So I think it should be auto cap to
> > 200Hz in this
> > > case.
> > > > >
> > > > > -DK
> > > > > >
> > > > > > Signed-off-by: Puthikorn
> > Voravootivat
> > > > 
> > > > > > ---
> > > > > >
> > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c |
> > > > 81
> > > > > +++
> > > > > >  1 file changed, 81
> > insertions(+)

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Pandiyan, Dhinakaran]

On Tue, 2017-05-16 at 13:56 -0700, Puthikorn Voravootivat wrote:
> 
> 
> On Tue, May 16, 2017 at 1:29 PM, Pandiyan, Dhinakaran
>  wrote:
> On Tue, 2017-05-16 at 11:07 -0700, Puthikorn Voravootivat
> wrote:
> >
> >
> > On Mon, May 15, 2017 at 11:21 PM, Pandiyan, Dhinakaran
> >  wrote:
> > On Mon, 2017-05-15 at 17:43 -0700, Puthikorn
> Voravootivat
> > wrote:
> > >
> > >
> > > On Mon, May 15, 2017 at 4:07 PM, Pandiyan,
> Dhinakaran
> > >  wrote:
> > > On Fri, 2017-05-12 at 17:31 -0700,
> Puthikorn
> > Voravootivat
> > > wrote:
> > > >
> > > >
> > > >
> > > > On Fri, May 12, 2017 at 5:12 PM,
> Pandiyan,
> > Dhinakaran
> > > >  wrote:
> > > > On Thu, 2017-05-11 at 16:02
> -0700,
> > Puthikorn
> > > Voravootivat
> > > > wrote:
> > > > > Read desired PWM frequency
> from panel
> > vbt and
> > > calculate the
> > > > > value for divider in DPCD
> address 0x724
> > and 0x728
> > > to have
> > > > > as many bits as possible for
> PWM duty
> > cyle for
> > > granularity
> > > > of
> > > > > brightness adjustment while
> the
> > frequency is still
> > > within
> > > > 25%
> > > > > of the desired frequency.
> > > >
> > > > I read a few eDP panel data
> sheets, the
> > PWM
> > > frequencies all
> > > > start from
> > > > ~200Hz. If the VBT chooses this
> lowest
> > value to
> > > allow for more
> > > > brightness control, and then
> this patch
> > lowers the
> > > value by
> > > > another 25%,
> > > > we'll end up below the panel
> allowed PWM
> > frequency.
> > > >
> > > > In fact, one of the systems I
> checked had
> > PWM
> > > frequency as
> > > > 200Hz in VBT
> > > > and the panel datasheet also had
> PWM
> > frequency range
> > > starting
> > > > from
> > > > 200Hz. Have you considered this
> case?
> > > >
> > > > The spec said "A given LCD panel
> typically has a
> > limited
> > > range of
> > > > backlight frequency capability.
> > > > To limit the programmable frequency
> range,
> > limitations are
> > > placed on
> > > > the allowable total divider ratio with
> the Sink
> > device"
> > > >  So I think it should be auto cap to
> 200Hz in this
> > case.
> > > >
> > > > -DK
> > > > >
> > > > > Signed-off-by: Puthikorn
> Voravootivat
> > > 
> > > > > ---
> > > > >
> > drivers/gpu/drm/i915/intel_dp_aux_backlight.c |
> > > 81
> > > > +++
> > > > >  1 file changed, 81
> insertions(+)
> > > > >
> > > > > diff --git
> > >
>  a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > >
> >  b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > > 

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Puthikorn Voravootivat]

On Tue, May 16, 2017 at 1:29 PM, Pandiyan, Dhinakaran <
dhinakaran.pandi...@intel.com> wrote:

> On Tue, 2017-05-16 at 11:07 -0700, Puthikorn Voravootivat wrote:
> >
> >
> > On Mon, May 15, 2017 at 11:21 PM, Pandiyan, Dhinakaran
> >  wrote:
> > On Mon, 2017-05-15 at 17:43 -0700, Puthikorn Voravootivat
> > wrote:
> > >
> > >
> > > On Mon, May 15, 2017 at 4:07 PM, Pandiyan, Dhinakaran
> > >  wrote:
> > > On Fri, 2017-05-12 at 17:31 -0700, Puthikorn
> > Voravootivat
> > > wrote:
> > > >
> > > >
> > > >
> > > > On Fri, May 12, 2017 at 5:12 PM, Pandiyan,
> > Dhinakaran
> > > >  wrote:
> > > > On Thu, 2017-05-11 at 16:02 -0700,
> > Puthikorn
> > > Voravootivat
> > > > wrote:
> > > > > Read desired PWM frequency from panel
> > vbt and
> > > calculate the
> > > > > value for divider in DPCD address 0x724
> > and 0x728
> > > to have
> > > > > as many bits as possible for PWM duty
> > cyle for
> > > granularity
> > > > of
> > > > > brightness adjustment while the
> > frequency is still
> > > within
> > > > 25%
> > > > > of the desired frequency.
> > > >
> > > > I read a few eDP panel data sheets, the
> > PWM
> > > frequencies all
> > > > start from
> > > > ~200Hz. If the VBT chooses this lowest
> > value to
> > > allow for more
> > > > brightness control, and then this patch
> > lowers the
> > > value by
> > > > another 25%,
> > > > we'll end up below the panel allowed PWM
> > frequency.
> > > >
> > > > In fact, one of the systems I checked had
> > PWM
> > > frequency as
> > > > 200Hz in VBT
> > > > and the panel datasheet also had PWM
> > frequency range
> > > starting
> > > > from
> > > > 200Hz. Have you considered this case?
> > > >
> > > > The spec said "A given LCD panel typically has a
> > limited
> > > range of
> > > > backlight frequency capability.
> > > > To limit the programmable frequency range,
> > limitations are
> > > placed on
> > > > the allowable total divider ratio with the Sink
> > device"
> > > >  So I think it should be auto cap to 200Hz in this
> > case.
> > > >
> > > > -DK
> > > > >
> > > > > Signed-off-by: Puthikorn Voravootivat
> > > 
> > > > > ---
> > > > >
> > drivers/gpu/drm/i915/intel_dp_aux_backlight.c |
> > > 81
> > > > +++
> > > > >  1 file changed, 81 insertions(+)
> > > > >
> > > > > diff --git
> > > a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > >
> >  b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > > index 0b48851013cc..6f10a2f1ab76 100644
> > > > > ---
> > > a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > > +++
> > > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > > @@ -113,12 +113,86 @@
> > > >
> >  intel_dp_aux_set_dynamic_backlight_percent(struct
> > > intel_dp
> > > > *intel_dp,
> > > > >   }
> > > > >  }
> > > > >
> > > > > +/*
> > > > > + * Set PWM Frequency divider to match
> > desired
> > > frequency in
> > > > vbt.
> > > > > + * The PWM Frequency is calculated as
> > 27Mhz / (F
> > > x P).
> > > > > + * - Where F = PWM Frequency
> > Pre-Divider value
> > > programmed
> >  

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Pandiyan, Dhinakaran]

On Tue, 2017-05-16 at 11:07 -0700, Puthikorn Voravootivat wrote:
> 
> 
> On Mon, May 15, 2017 at 11:21 PM, Pandiyan, Dhinakaran
>  wrote:
> On Mon, 2017-05-15 at 17:43 -0700, Puthikorn Voravootivat
> wrote:
> >
> >
> > On Mon, May 15, 2017 at 4:07 PM, Pandiyan, Dhinakaran
> >  wrote:
> > On Fri, 2017-05-12 at 17:31 -0700, Puthikorn
> Voravootivat
> > wrote:
> > >
> > >
> > >
> > > On Fri, May 12, 2017 at 5:12 PM, Pandiyan,
> Dhinakaran
> > >  wrote:
> > > On Thu, 2017-05-11 at 16:02 -0700,
> Puthikorn
> > Voravootivat
> > > wrote:
> > > > Read desired PWM frequency from panel
> vbt and
> > calculate the
> > > > value for divider in DPCD address 0x724
> and 0x728
> > to have
> > > > as many bits as possible for PWM duty
> cyle for
> > granularity
> > > of
> > > > brightness adjustment while the
> frequency is still
> > within
> > > 25%
> > > > of the desired frequency.
> > >
> > > I read a few eDP panel data sheets, the
> PWM
> > frequencies all
> > > start from
> > > ~200Hz. If the VBT chooses this lowest
> value to
> > allow for more
> > > brightness control, and then this patch
> lowers the
> > value by
> > > another 25%,
> > > we'll end up below the panel allowed PWM
> frequency.
> > >
> > > In fact, one of the systems I checked had
> PWM
> > frequency as
> > > 200Hz in VBT
> > > and the panel datasheet also had PWM
> frequency range
> > starting
> > > from
> > > 200Hz. Have you considered this case?
> > >
> > > The spec said "A given LCD panel typically has a
> limited
> > range of
> > > backlight frequency capability.
> > > To limit the programmable frequency range,
> limitations are
> > placed on
> > > the allowable total divider ratio with the Sink
> device"
> > >  So I think it should be auto cap to 200Hz in this
> case.
> > >
> > > -DK
> > > >
> > > > Signed-off-by: Puthikorn Voravootivat
> > 
> > > > ---
> > > >
> drivers/gpu/drm/i915/intel_dp_aux_backlight.c |
> > 81
> > > +++
> > > >  1 file changed, 81 insertions(+)
> > > >
> > > > diff --git
> > a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > >
>  b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > index 0b48851013cc..6f10a2f1ab76 100644
> > > > ---
> > a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > +++
> > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > @@ -113,12 +113,86 @@
> > >
>  intel_dp_aux_set_dynamic_backlight_percent(struct
> > intel_dp
> > > *intel_dp,
> > > >   }
> > > >  }
> > > >
> > > > +/*
> > > > + * Set PWM Frequency divider to match
> desired
> > frequency in
> > > vbt.
> > > > + * The PWM Frequency is calculated as
> 27Mhz / (F
> > x P).
> > > > + * - Where F = PWM Frequency
> Pre-Divider value
> > programmed
> > > by field 7:0 of the
> > > > + * EDP_BACKLIGHT_FREQ_SET
> register
> > (DPCD
> > > Address 00728h)
> > > > + * - Where P = 2^Pn, where Pn is the
> value
> > 

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Puthikorn Voravootivat]

On Mon, May 15, 2017 at 11:21 PM, Pandiyan, Dhinakaran <
dhinakaran.pandi...@intel.com> wrote:

> On Mon, 2017-05-15 at 17:43 -0700, Puthikorn Voravootivat wrote:
> >
> >
> > On Mon, May 15, 2017 at 4:07 PM, Pandiyan, Dhinakaran
> >  wrote:
> > On Fri, 2017-05-12 at 17:31 -0700, Puthikorn Voravootivat
> > wrote:
> > >
> > >
> > >
> > > On Fri, May 12, 2017 at 5:12 PM, Pandiyan, Dhinakaran
> > >  wrote:
> > > On Thu, 2017-05-11 at 16:02 -0700, Puthikorn
> > Voravootivat
> > > wrote:
> > > > Read desired PWM frequency from panel vbt and
> > calculate the
> > > > value for divider in DPCD address 0x724 and 0x728
> > to have
> > > > as many bits as possible for PWM duty cyle for
> > granularity
> > > of
> > > > brightness adjustment while the frequency is still
> > within
> > > 25%
> > > > of the desired frequency.
> > >
> > > I read a few eDP panel data sheets, the PWM
> > frequencies all
> > > start from
> > > ~200Hz. If the VBT chooses this lowest value to
> > allow for more
> > > brightness control, and then this patch lowers the
> > value by
> > > another 25%,
> > > we'll end up below the panel allowed PWM frequency.
> > >
> > > In fact, one of the systems I checked had PWM
> > frequency as
> > > 200Hz in VBT
> > > and the panel datasheet also had PWM frequency range
> > starting
> > > from
> > > 200Hz. Have you considered this case?
> > >
> > > The spec said "A given LCD panel typically has a limited
> > range of
> > > backlight frequency capability.
> > > To limit the programmable frequency range, limitations are
> > placed on
> > > the allowable total divider ratio with the Sink device"
> > >  So I think it should be auto cap to 200Hz in this case.
> > >
> > > -DK
> > > >
> > > > Signed-off-by: Puthikorn Voravootivat
> > 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c |
> > 81
> > > +++
> > > >  1 file changed, 81 insertions(+)
> > > >
> > > > diff --git
> > a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > index 0b48851013cc..6f10a2f1ab76 100644
> > > > ---
> > a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > +++
> > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > > @@ -113,12 +113,86 @@
> > > intel_dp_aux_set_dynamic_backlight_percent(struct
> > intel_dp
> > > *intel_dp,
> > > >   }
> > > >  }
> > > >
> > > > +/*
> > > > + * Set PWM Frequency divider to match desired
> > frequency in
> > > vbt.
> > > > + * The PWM Frequency is calculated as 27Mhz / (F
> > x P).
> > > > + * - Where F = PWM Frequency Pre-Divider value
> > programmed
> > > by field 7:0 of the
> > > > + * EDP_BACKLIGHT_FREQ_SET register
> > (DPCD
> > > Address 00728h)
> > > > + * - Where P = 2^Pn, where Pn is the value
> > programmed by
> > > field 4:0 of the
> > > > + * EDP_PWMGEN_BIT_COUNT register
> > (DPCD Address
> > > 00724h)
> > > > + */
> > > > +static void intel_dp_aux_set_pwm_freq(struct
> > > intel_connector *connector)
> > > > +{
> > > > + struct drm_i915_private *dev_priv =
> > > to_i915(connector->base.dev);
> > > > + struct intel_dp *intel_dp =
> > > enc_to_intel_dp(>encoder->base);
> > > > + int freq, fxp, fxp_min, fxp_max, fxp_actual,
> > f = 1;
> > > > + u8 pn, pn_min, pn_max;
> > > > +
> > > > + /* Find desired value of (F x P)
> > > > +  * Note that, if F x P is out of supported
> > range, the
> > > maximum value or
> > > > +  * minimum value will applied automatically.
> > So no
> > > 

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Pandiyan, Dhinakaran]

On Mon, 2017-05-15 at 17:43 -0700, Puthikorn Voravootivat wrote:
> 
> 
> On Mon, May 15, 2017 at 4:07 PM, Pandiyan, Dhinakaran
>  wrote:
> On Fri, 2017-05-12 at 17:31 -0700, Puthikorn Voravootivat
> wrote:
> >
> >
> >
> > On Fri, May 12, 2017 at 5:12 PM, Pandiyan, Dhinakaran
> >  wrote:
> > On Thu, 2017-05-11 at 16:02 -0700, Puthikorn
> Voravootivat
> > wrote:
> > > Read desired PWM frequency from panel vbt and
> calculate the
> > > value for divider in DPCD address 0x724 and 0x728
> to have
> > > as many bits as possible for PWM duty cyle for
> granularity
> > of
> > > brightness adjustment while the frequency is still
> within
> > 25%
> > > of the desired frequency.
> >
> > I read a few eDP panel data sheets, the PWM
> frequencies all
> > start from
> > ~200Hz. If the VBT chooses this lowest value to
> allow for more
> > brightness control, and then this patch lowers the
> value by
> > another 25%,
> > we'll end up below the panel allowed PWM frequency.
> >
> > In fact, one of the systems I checked had PWM
> frequency as
> > 200Hz in VBT
> > and the panel datasheet also had PWM frequency range
> starting
> > from
> > 200Hz. Have you considered this case?
> >
> > The spec said "A given LCD panel typically has a limited
> range of
> > backlight frequency capability.
> > To limit the programmable frequency range, limitations are
> placed on
> > the allowable total divider ratio with the Sink device"
> >  So I think it should be auto cap to 200Hz in this case.
> >
> > -DK
> > >
> > > Signed-off-by: Puthikorn Voravootivat
> 
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c |
> 81
> > +++
> > >  1 file changed, 81 insertions(+)
> > >
> > > diff --git
> a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > index 0b48851013cc..6f10a2f1ab76 100644
> > > ---
> a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > +++
> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > @@ -113,12 +113,86 @@
> > intel_dp_aux_set_dynamic_backlight_percent(struct
> intel_dp
> > *intel_dp,
> > >   }
> > >  }
> > >
> > > +/*
> > > + * Set PWM Frequency divider to match desired
> frequency in
> > vbt.
> > > + * The PWM Frequency is calculated as 27Mhz / (F
> x P).
> > > + * - Where F = PWM Frequency Pre-Divider value
> programmed
> > by field 7:0 of the
> > > + * EDP_BACKLIGHT_FREQ_SET register
> (DPCD
> > Address 00728h)
> > > + * - Where P = 2^Pn, where Pn is the value
> programmed by
> > field 4:0 of the
> > > + * EDP_PWMGEN_BIT_COUNT register
> (DPCD Address
> > 00724h)
> > > + */
> > > +static void intel_dp_aux_set_pwm_freq(struct
> > intel_connector *connector)
> > > +{
> > > + struct drm_i915_private *dev_priv =
> > to_i915(connector->base.dev);
> > > + struct intel_dp *intel_dp =
> > enc_to_intel_dp(>encoder->base);
> > > + int freq, fxp, fxp_min, fxp_max, fxp_actual,
> f = 1;
> > > + u8 pn, pn_min, pn_max;
> > > +
> > > + /* Find desired value of (F x P)
> > > +  * Note that, if F x P is out of supported
> range, the
> > maximum value or
> > > +  * minimum value will applied automatically.
> So no
> > need to check that.
> > > +  */
> > > + freq = dev_priv->vbt.backlight.pwm_freq_hz;
> > > + DRM_DEBUG_KMS("VBT defined backlight
> frequency %u Hz
> > \n", freq);
> > > + if (!freq) {
> > > + 

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Puthikorn Voravootivat]

On Mon, May 15, 2017 at 4:07 PM, Pandiyan, Dhinakaran <
dhinakaran.pandi...@intel.com> wrote:

> On Fri, 2017-05-12 at 17:31 -0700, Puthikorn Voravootivat wrote:
> >
> >
> >
> > On Fri, May 12, 2017 at 5:12 PM, Pandiyan, Dhinakaran
> >  wrote:
> > On Thu, 2017-05-11 at 16:02 -0700, Puthikorn Voravootivat
> > wrote:
> > > Read desired PWM frequency from panel vbt and calculate the
> > > value for divider in DPCD address 0x724 and 0x728 to have
> > > as many bits as possible for PWM duty cyle for granularity
> > of
> > > brightness adjustment while the frequency is still within
> > 25%
> > > of the desired frequency.
> >
> > I read a few eDP panel data sheets, the PWM frequencies all
> > start from
> > ~200Hz. If the VBT chooses this lowest value to allow for more
> > brightness control, and then this patch lowers the value by
> > another 25%,
> > we'll end up below the panel allowed PWM frequency.
> >
> > In fact, one of the systems I checked had PWM frequency as
> > 200Hz in VBT
> > and the panel datasheet also had PWM frequency range starting
> > from
> > 200Hz. Have you considered this case?
> >
> > The spec said "A given LCD panel typically has a limited range of
> > backlight frequency capability.
> > To limit the programmable frequency range, limitations are placed on
> > the allowable total divider ratio with the Sink device"
> >  So I think it should be auto cap to 200Hz in this case.
> >
> > -DK
> > >
> > > Signed-off-by: Puthikorn Voravootivat 
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 81
> > +++
> > >  1 file changed, 81 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > index 0b48851013cc..6f10a2f1ab76 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > > @@ -113,12 +113,86 @@
> > intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp
> > *intel_dp,
> > >   }
> > >  }
> > >
> > > +/*
> > > + * Set PWM Frequency divider to match desired frequency in
> > vbt.
> > > + * The PWM Frequency is calculated as 27Mhz / (F x P).
> > > + * - Where F = PWM Frequency Pre-Divider value programmed
> > by field 7:0 of the
> > > + * EDP_BACKLIGHT_FREQ_SET register (DPCD
> > Address 00728h)
> > > + * - Where P = 2^Pn, where Pn is the value programmed by
> > field 4:0 of the
> > > + * EDP_PWMGEN_BIT_COUNT register (DPCD Address
> > 00724h)
> > > + */
> > > +static void intel_dp_aux_set_pwm_freq(struct
> > intel_connector *connector)
> > > +{
> > > + struct drm_i915_private *dev_priv =
> > to_i915(connector->base.dev);
> > > + struct intel_dp *intel_dp =
> > enc_to_intel_dp(>encoder->base);
> > > + int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1;
> > > + u8 pn, pn_min, pn_max;
> > > +
> > > + /* Find desired value of (F x P)
> > > +  * Note that, if F x P is out of supported range, the
> > maximum value or
> > > +  * minimum value will applied automatically. So no
> > need to check that.
> > > +  */
> > > + freq = dev_priv->vbt.backlight.pwm_freq_hz;
> > > + DRM_DEBUG_KMS("VBT defined backlight frequency %u Hz
> > \n", freq);
> > > + if (!freq) {
> > > + DRM_DEBUG_KMS("Use panel default backlight
> > frequency\n");
> > > + return;
> > > + }
> > > +
> > > + fxp = KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ) / freq;
> > > +
> > > + /* Use highest possible value of Pn for more
> > granularity of brightness
> > > +  * adjustment while satifying the conditions below.
> > > +  * - Pn is in the range of Pn_min and Pn_max
> > > +  * - F is in the range of 1 and 255
> > > +  * - Effective frequency is within 25% of desired
> > frequency.
> > > +  */
> > > + if (drm_dp_dpcd_readb(_dp->aux,
> > > +
> > DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, _min) != 1) {
> > > + DRM_DEBUG_KMS("Failed to read pwmgen bit count
> > cap min\n");
> > > + return;
> > > + }
> > > + if (drm_dp_dpcd_readb(_dp->aux,
> > > +
> > DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, _max) != 1) {
> > 

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Pandiyan, Dhinakaran]

On Fri, 2017-05-12 at 17:31 -0700, Puthikorn Voravootivat wrote:
> 
> 
> 
> On Fri, May 12, 2017 at 5:12 PM, Pandiyan, Dhinakaran
>  wrote:
> On Thu, 2017-05-11 at 16:02 -0700, Puthikorn Voravootivat
> wrote:
> > Read desired PWM frequency from panel vbt and calculate the
> > value for divider in DPCD address 0x724 and 0x728 to have
> > as many bits as possible for PWM duty cyle for granularity
> of
> > brightness adjustment while the frequency is still within
> 25%
> > of the desired frequency.
> 
> I read a few eDP panel data sheets, the PWM frequencies all
> start from
> ~200Hz. If the VBT chooses this lowest value to allow for more
> brightness control, and then this patch lowers the value by
> another 25%,
> we'll end up below the panel allowed PWM frequency.
> 
> In fact, one of the systems I checked had PWM frequency as
> 200Hz in VBT
> and the panel datasheet also had PWM frequency range starting
> from
> 200Hz. Have you considered this case?
> 
> The spec said "A given LCD panel typically has a limited range of
> backlight frequency capability. 
> To limit the programmable frequency range, limitations are placed on
> the allowable total divider ratio with the Sink device"
>  So I think it should be auto cap to 200Hz in this case.
>  
> -DK
> >
> > Signed-off-by: Puthikorn Voravootivat 
> > ---
> >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 81
> +++
> >  1 file changed, 81 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > index 0b48851013cc..6f10a2f1ab76 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > @@ -113,12 +113,86 @@
> intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp
> *intel_dp,
> >   }
> >  }
> >
> > +/*
> > + * Set PWM Frequency divider to match desired frequency in
> vbt.
> > + * The PWM Frequency is calculated as 27Mhz / (F x P).
> > + * - Where F = PWM Frequency Pre-Divider value programmed
> by field 7:0 of the
> > + * EDP_BACKLIGHT_FREQ_SET register (DPCD
> Address 00728h)
> > + * - Where P = 2^Pn, where Pn is the value programmed by
> field 4:0 of the
> > + * EDP_PWMGEN_BIT_COUNT register (DPCD Address
> 00724h)
> > + */
> > +static void intel_dp_aux_set_pwm_freq(struct
> intel_connector *connector)
> > +{
> > + struct drm_i915_private *dev_priv =
> to_i915(connector->base.dev);
> > + struct intel_dp *intel_dp =
> enc_to_intel_dp(>encoder->base);
> > + int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1;
> > + u8 pn, pn_min, pn_max;
> > +
> > + /* Find desired value of (F x P)
> > +  * Note that, if F x P is out of supported range, the
> maximum value or
> > +  * minimum value will applied automatically. So no
> need to check that.
> > +  */
> > + freq = dev_priv->vbt.backlight.pwm_freq_hz;
> > + DRM_DEBUG_KMS("VBT defined backlight frequency %u Hz
> \n", freq);
> > + if (!freq) {
> > + DRM_DEBUG_KMS("Use panel default backlight
> frequency\n");
> > + return;
> > + }
> > +
> > + fxp = KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ) / freq;
> > +
> > + /* Use highest possible value of Pn for more
> granularity of brightness
> > +  * adjustment while satifying the conditions below.
> > +  * - Pn is in the range of Pn_min and Pn_max
> > +  * - F is in the range of 1 and 255
> > +  * - Effective frequency is within 25% of desired
> frequency.
> > +  */
> > + if (drm_dp_dpcd_readb(_dp->aux,
> > +
> DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, _min) != 1) {
> > + DRM_DEBUG_KMS("Failed to read pwmgen bit count
> cap min\n");
> > + return;
> > + }
> > + if (drm_dp_dpcd_readb(_dp->aux,
> > +
> DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, _max) != 1) {
> > + DRM_DEBUG_KMS("Failed to read pwmgen bit count
> cap max\n");
> > + return;
> > + }
> > + pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
> > + pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
> > +
> 

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Puthikorn Voravootivat]

On Fri, May 12, 2017 at 5:12 PM, Pandiyan, Dhinakaran <
dhinakaran.pandi...@intel.com> wrote:

> On Thu, 2017-05-11 at 16:02 -0700, Puthikorn Voravootivat wrote:
> > Read desired PWM frequency from panel vbt and calculate the
> > value for divider in DPCD address 0x724 and 0x728 to have
> > as many bits as possible for PWM duty cyle for granularity of
> > brightness adjustment while the frequency is still within 25%
> > of the desired frequency.
>
> I read a few eDP panel data sheets, the PWM frequencies all start from
> ~200Hz. If the VBT chooses this lowest value to allow for more
> brightness control, and then this patch lowers the value by another 25%,
> we'll end up below the panel allowed PWM frequency.
>
> In fact, one of the systems I checked had PWM frequency as 200Hz in VBT
> and the panel datasheet also had PWM frequency range starting from
> 200Hz. Have you considered this case?
>
> The spec said "A given LCD panel typically has a limited range of
backlight frequency capability.
To limit the programmable frequency range, limitations are placed on the
allowable total divider ratio with the Sink device"
 So I think it should be auto cap to 200Hz in this case.


> -DK
> >
> > Signed-off-by: Puthikorn Voravootivat 
> > ---
> >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 81
> +++
> >  1 file changed, 81 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > index 0b48851013cc..6f10a2f1ab76 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > @@ -113,12 +113,86 @@ intel_dp_aux_set_dynamic_backlight_percent(struct
> intel_dp *intel_dp,
> >   }
> >  }
> >
> > +/*
> > + * Set PWM Frequency divider to match desired frequency in vbt.
> > + * The PWM Frequency is calculated as 27Mhz / (F x P).
> > + * - Where F = PWM Frequency Pre-Divider value programmed by field 7:0
> of the
> > + * EDP_BACKLIGHT_FREQ_SET register (DPCD Address 00728h)
> > + * - Where P = 2^Pn, where Pn is the value programmed by field 4:0 of
> the
> > + * EDP_PWMGEN_BIT_COUNT register (DPCD Address 00724h)
> > + */
> > +static void intel_dp_aux_set_pwm_freq(struct intel_connector
> *connector)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > + struct intel_dp *intel_dp = enc_to_intel_dp(>
> encoder->base);
> > + int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1;
> > + u8 pn, pn_min, pn_max;
> > +
> > + /* Find desired value of (F x P)
> > +  * Note that, if F x P is out of supported range, the maximum
> value or
> > +  * minimum value will applied automatically. So no need to check
> that.
> > +  */
> > + freq = dev_priv->vbt.backlight.pwm_freq_hz;
> > + DRM_DEBUG_KMS("VBT defined backlight frequency %u Hz\n", freq);
> > + if (!freq) {
> > + DRM_DEBUG_KMS("Use panel default backlight frequency\n");
> > + return;
> > + }
> > +
> > + fxp = KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ) / freq;
> > +
> > + /* Use highest possible value of Pn for more granularity of
> brightness
> > +  * adjustment while satifying the conditions below.
> > +  * - Pn is in the range of Pn_min and Pn_max
> > +  * - F is in the range of 1 and 255
> > +  * - Effective frequency is within 25% of desired frequency.
> > +  */
> > + if (drm_dp_dpcd_readb(_dp->aux,
> > +DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, _min)
> != 1) {
> > + DRM_DEBUG_KMS("Failed to read pwmgen bit count cap min\n");
> > + return;
> > + }
> > + if (drm_dp_dpcd_readb(_dp->aux,
> > +DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, _max)
> != 1) {
> > + DRM_DEBUG_KMS("Failed to read pwmgen bit count cap max\n");
> > + return;
> > + }
> > + pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
> > + pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
> > +
> > + fxp_min = fxp * 3 / 4;
> > + fxp_max = fxp * 5 / 4;
>
> You are allowing fxp between +/- 25% of the actual. This isn't same as
> the "Effective frequency is within 25% of desired frequency." right? The
> frequency can vary between -20% and +33%.
>
> You are right.
You want me to change commit message to reflect this or change the code to
match the commit message?

>
> > + if (fxp_min < (1 << pn_min) || (255 << pn_max) < fxp_max) {
> > + DRM_DEBUG_KMS("VBT defined backlight frequency out of
> range\n");
> > + return;
> > + }
> > +
> > + for (pn = pn_max; pn > pn_min; pn--) {
> > + f = clamp(fxp >> pn, 1, 255);
> > + fxp_actual = f << pn;
> > + if (fxp_min <= fxp_actual && fxp_actual <= fxp_max)
> > + break;
> > + }
> > +
> > + if (drm_dp_dpcd_writeb(_dp->aux,
> > +

Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt

[Pandiyan, Dhinakaran]

On Thu, 2017-05-11 at 16:02 -0700, Puthikorn Voravootivat wrote:
> Read desired PWM frequency from panel vbt and calculate the
> value for divider in DPCD address 0x724 and 0x728 to have
> as many bits as possible for PWM duty cyle for granularity of
> brightness adjustment while the frequency is still within 25%
> of the desired frequency.

I read a few eDP panel data sheets, the PWM frequencies all start from
~200Hz. If the VBT chooses this lowest value to allow for more
brightness control, and then this patch lowers the value by another 25%,
we'll end up below the panel allowed PWM frequency. 

In fact, one of the systems I checked had PWM frequency as 200Hz in VBT
and the panel datasheet also had PWM frequency range starting from
200Hz. Have you considered this case?

-DK
> 
> Signed-off-by: Puthikorn Voravootivat 
> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 81 
> +++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c 
> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index 0b48851013cc..6f10a2f1ab76 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -113,12 +113,86 @@ intel_dp_aux_set_dynamic_backlight_percent(struct 
> intel_dp *intel_dp,
>   }
>  }
>  
> +/*
> + * Set PWM Frequency divider to match desired frequency in vbt.
> + * The PWM Frequency is calculated as 27Mhz / (F x P).
> + * - Where F = PWM Frequency Pre-Divider value programmed by field 7:0 of the
> + * EDP_BACKLIGHT_FREQ_SET register (DPCD Address 00728h)
> + * - Where P = 2^Pn, where Pn is the value programmed by field 4:0 of the
> + * EDP_PWMGEN_BIT_COUNT register (DPCD Address 00724h)
> + */
> +static void intel_dp_aux_set_pwm_freq(struct intel_connector *connector)
> +{
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct intel_dp *intel_dp = enc_to_intel_dp(>encoder->base);
> + int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1;
> + u8 pn, pn_min, pn_max;
> +
> + /* Find desired value of (F x P)
> +  * Note that, if F x P is out of supported range, the maximum value or
> +  * minimum value will applied automatically. So no need to check that.
> +  */
> + freq = dev_priv->vbt.backlight.pwm_freq_hz;
> + DRM_DEBUG_KMS("VBT defined backlight frequency %u Hz\n", freq);
> + if (!freq) {
> + DRM_DEBUG_KMS("Use panel default backlight frequency\n");
> + return;
> + }
> +
> + fxp = KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ) / freq;
> +
> + /* Use highest possible value of Pn for more granularity of brightness
> +  * adjustment while satifying the conditions below.
> +  * - Pn is in the range of Pn_min and Pn_max
> +  * - F is in the range of 1 and 255
> +  * - Effective frequency is within 25% of desired frequency.
> +  */
> + if (drm_dp_dpcd_readb(_dp->aux,
> +DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, _min) != 1) {
> + DRM_DEBUG_KMS("Failed to read pwmgen bit count cap min\n");
> + return;
> + }
> + if (drm_dp_dpcd_readb(_dp->aux,
> +DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, _max) != 1) {
> + DRM_DEBUG_KMS("Failed to read pwmgen bit count cap max\n");
> + return;
> + }
> + pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
> + pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK;
> +
> + fxp_min = fxp * 3 / 4;
> + fxp_max = fxp * 5 / 4;

You are allowing fxp between +/- 25% of the actual. This isn't same as
the "Effective frequency is within 25% of desired frequency." right? The
frequency can vary between -20% and +33%.


> + if (fxp_min < (1 << pn_min) || (255 << pn_max) < fxp_max) {
> + DRM_DEBUG_KMS("VBT defined backlight frequency out of range\n");
> + return;
> + }
> +
> + for (pn = pn_max; pn > pn_min; pn--) {
> + f = clamp(fxp >> pn, 1, 255);
> + fxp_actual = f << pn;
> + if (fxp_min <= fxp_actual && fxp_actual <= fxp_max)
> + break;
> + }
> +
> + if (drm_dp_dpcd_writeb(_dp->aux,
> +DP_EDP_PWMGEN_BIT_COUNT, pn) < 0) {
> + DRM_DEBUG_KMS("Failed to write aux pwmgen bit count\n");
> + return;
> + }
> + if (drm_dp_dpcd_writeb(_dp->aux,
> +DP_EDP_BACKLIGHT_FREQ_SET, (u8) f) < 0) {
> + DRM_DEBUG_KMS("Failed to write aux backlight freq\n");
> + return;
> + }
> +}
> +
>  static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
>  {
>   struct intel_dp *intel_dp = enc_to_intel_dp(>encoder->base);
>   uint8_t dpcd_buf = 0;
>   uint8_t new_dpcd_buf = 0;
>   uint8_t edp_backlight_mode = 0;
> + bool freq_cap;
>  
>   if (drm_dp_dpcd_readb(_dp->aux,
>