[Intel-gfx] [PATCH 4/5] drm/i915: Use highest frequency divider for PWM

[Puthikorn Voravootivat]

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 at linux.intel.com> wrote:
> On Wed, 08 Mar 2017, Puthikorn Voravootivat <puthik at 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 at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx