Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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(&connector->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(&intel_dp->aux, > +DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &pn_min) != 1) { > + DRM_DEBUG_KMS("Failed to read pwmgen bit count cap min\n"); > + return; > + } > + if (drm_dp_dpcd_readb(&intel_dp->aux, > +DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, &pn_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(&intel_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(&intel_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(&connector->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(&intel_d
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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(&connector-> > 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(&intel_dp->aux, > > +DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &pn_min) > != 1) { > > + DRM_DEBUG_KMS("Failed to read pwmgen bit count cap min\n"); > > + return; > > + } > > + if (drm_dp_dpcd_readb(&intel_dp->aux, > > +DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, &pn_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(&intel_dp->aux, > > +DP_EDP
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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(&connector->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(&intel_dp->aux, > > + > DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &pn_min) != 1) { > > + DRM_DEBUG_KMS("Failed to read pwmgen bit count > cap min\n"); > > + return; > > + } > > + if (drm_dp_dpcd_readb(&intel_dp->aux, > > + > DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, &pn_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 *
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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(&connector->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(&intel_dp->aux, > > > + > > DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &pn_min) != 1) { > > > + DRM_DEBUG_KMS("Failed to read pwmgen bit count > > cap min\n"); > > > + return; > > > + } > > > + if (drm_dp_dpcd_readb(&intel_dp->aux, > > > + > > DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, &pn_max) != 1) { > > > +
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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(&connector->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 > >
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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(+) > > > > > > > > > > > > diff --git > > > > > > a/drivers/
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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 > > > > by field 7:0 of the > > > > > + * EDP_BACKLIGHT_FRE
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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. > > > > > To limit the programmable > frequency > > range, > > > limitations are > >
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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(&connector->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. > > > > + */ > >
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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 > > > > > index > 0b48851013cc..6f10a2f1ab76 100644 > > > > > --- > > > > a/drivers/gpu/drm/i
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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 > > programmed by > > > field 4:0 of the > > > > + * EDP_PWMGEN_BIT_