| Message ID | 20170308213053.194062-5-puthik@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 08 Mar 2017, Puthikorn Voravootivat <puthik@chromium.org> wrote: > TCON tend to have better brightness scaling with lower > PWM frequency. This patch set the divider to highest > value to lower the PWM frequency. Just setting 0xff to DP_EDP_BACKLIGHT_FREQ_SET is way too arbitrary and panel dependent. Going to a too low frequency may lead to flicker, and you have no idea how low you're going because you ignore DP_EDP_PWMGEN_BIT_COUNT completely. I think it's possible to land in audible frequencies too, depending on the TCON hardware. (*) I think the way to go is to check the VBT for OEM specified backlight frequency, tuned for the specific hardware, and do the math to set the registers right. This is what we use (well, fall back to) for the PWM pin frequency setting in intel_panel.c. BR, Jani. (*) Admittedly there's a certain charm in the idea of getting a bug report, "I can hear my backlight". ;) > > Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> > --- > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > index 643b604be2de..32b426006a6a 100644 > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > @@ -116,6 +116,7 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) > uint8_t dpcd_buf = 0; > uint8_t new_dpcd_buf = 0; > uint8_t edp_backlight_mode = 0; > + bool freq_cap = false; > > set_aux_backlight_enable(intel_dp, true); > > @@ -146,10 +147,20 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) > intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100); > } > > + /* Use highest frequency divider if supported */ > + freq_cap = intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP; > + if (freq_cap) > + new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; > + > if (new_dpcd_buf != dpcd_buf) { > drm_dp_dpcd_writeb(&intel_dp->aux, > DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf); > } > + > + if (freq_cap) { > + drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_FREQ_SET, > + 0xff); > + } > } > > static void intel_dp_aux_disable_backlight(struct intel_connector *connector)
Agree that your suggestion is better. I will drop this patch in the next version of the set. Thanks On Thu, Mar 9, 2017 at 2:40 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Wed, 08 Mar 2017, Puthikorn Voravootivat <puthik@chromium.org> wrote: >> TCON tend to have better brightness scaling with lower >> PWM frequency. This patch set the divider to highest >> value to lower the PWM frequency. > > Just setting 0xff to DP_EDP_BACKLIGHT_FREQ_SET is way too arbitrary and > panel dependent. Going to a too low frequency may lead to flicker, and > you have no idea how low you're going because you ignore > DP_EDP_PWMGEN_BIT_COUNT completely. I think it's possible to land in > audible frequencies too, depending on the TCON hardware. (*) > > I think the way to go is to check the VBT for OEM specified backlight > frequency, tuned for the specific hardware, and do the math to set the > registers right. This is what we use (well, fall back to) for the PWM > pin frequency setting in intel_panel.c. > > BR, > Jani. > > > (*) Admittedly there's a certain charm in the idea of getting a bug > report, "I can hear my backlight". ;) > > >> >> Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> >> --- >> drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> index 643b604be2de..32b426006a6a 100644 >> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c >> @@ -116,6 +116,7 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) >> uint8_t dpcd_buf = 0; >> uint8_t new_dpcd_buf = 0; >> uint8_t edp_backlight_mode = 0; >> + bool freq_cap = false; >> >> set_aux_backlight_enable(intel_dp, true); >> >> @@ -146,10 +147,20 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) >> intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100); >> } >> >> + /* Use highest frequency divider if supported */ >> + freq_cap = intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP; >> + if (freq_cap) >> + new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; >> + >> if (new_dpcd_buf != dpcd_buf) { >> drm_dp_dpcd_writeb(&intel_dp->aux, >> DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf); >> } >> + >> + if (freq_cap) { >> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_FREQ_SET, >> + 0xff); >> + } >> } >> >> static void intel_dp_aux_disable_backlight(struct intel_connector *connector) > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 643b604be2de..32b426006a6a 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -116,6 +116,7 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) uint8_t dpcd_buf = 0; uint8_t new_dpcd_buf = 0; uint8_t edp_backlight_mode = 0; + bool freq_cap = false; set_aux_backlight_enable(intel_dp, true); @@ -146,10 +147,20 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100); } + /* Use highest frequency divider if supported */ + freq_cap = intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP; + if (freq_cap) + new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; + if (new_dpcd_buf != dpcd_buf) { drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf); } + + if (freq_cap) { + drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_FREQ_SET, + 0xff); + } } static void intel_dp_aux_disable_backlight(struct intel_connector *connector)
TCON tend to have better brightness scaling with lower PWM frequency. This patch set the divider to highest value to lower the PWM frequency. Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org> --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 11 +++++++++++ 1 file changed, 11 insertions(+)