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@xxxxxxxxxxxxxxx> wrote:
> On Wed, 08 Mar 2017, Puthikorn Voravootivat <puthik@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
