[Intel-gfx] [PATCH] drm/i915: Use AUX for backlight only if eDP 1.4 or later

[Puthikorn Voravootivat]

I saw a DP 1.3 panel that advertise AUX backlight brightness control
but not working properly. So it should work but not in real world.
I think that is good reason enough to add this as a heuristic.

On Thu, Jul 20, 2017 at 1:27 PM, Pandiyan, Dhinakaran
<dhinakaran.pandiyan at intel.com> wrote:
>
>
>
> On Thu, 2017-07-20 at 10:06 +0000, Tc, Jenny wrote:
>> > >> With older panels, AUX pin for backlight doesn't work.
>> >
>> > What evidence do you have to back up that claim?
>>
>> Debugging further it's found that the panel I am having doesn't support AUX Backlight.
>>
>> cat /sys/kernel/debug/dri/0/eDP-1/i915_dpcd
>> ...
>> 0701: bb ff 00 00
>> ....
>>
>> With below change its working for my panel. But doesn't address issue reported in
>> https://bugs.freedesktop.org/show_bug.cgi?id=101619 which seems to have a wrong DPCD data.
>> Since we don't have a proper fix for all panels, I agree for revert.
>>
>> intel_dp_aux_display_control_heuristic(struct intel_connector *connector)
>>         struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
>>         uint8_t reg_val;
>>
>> +       /* Panel dosn't support enabling AUX Backlight control */
>> +       if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) {
>> +               return false;
>> +       }
>
> AUX backlight brightness control should work even without AUX enable
> capability.
>
>
>>         /* Panel doesn't support adjusting backlight brightness via PWN pin */
>>         if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))
>>                 return true;
>>
>> > >> On some panels, this causes backlight issues on S3 resume.
>> > >
>> > > What is it that we are missing in the resume path?
>> > >
>> > >>  Enable the
>> > >> feature only for eDP1.4 or later.
>> > >
>> > > I can't find the eDP 1.4 requirement in the spec. Regional brightness
>> > > control was added in eDP 1.4, but we don't enable that. My concern is
>> > > we might be missing a real fix by ignoring < eDP 1.4 panels.
>> >
>> > Agreed.
>> >
>> > This has been going on too long now, with no real effort to get at the roots of
>> > the problem. The right approach is to revert the regressing commits now,
>> > and start over. Reverts posted [1].
>> >
>> > BR,
>> > Jani.
>> >
>> > [1] https://patchwork.freedesktop.org/series/27623/
>> >
>> > >
>> > >
>> > >>
>> > >> Fix-suggested-by: Puthikorn Voravootivat <puthik at chromium.org>
>> > >
>> > > 1. Please use the "Fixes" tag to refer to the commit that introduced
>> > > the code you are fixing.
>> > > 2. The "Suggested-by" tag is more common to give credits to the person
>> > > who suggested the fix.
>> > > 3. Please use the "Bugzilla" tag to point to the bug that David
>> > > reported.
>> > >
>> > >
>> > >> Signed-off-by: Jenny TC <jenny.tc at intel.com>
>> > >> ---
>> > >>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 6 +++++-
>> > >>  1 file changed, 5 insertions(+), 1 deletion(-)
>> > >>
>> > >> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>> > >> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>> > >> index b25cd88..421f31f 100644
>> > >> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>> > >> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>> > >> @@ -292,7 +292,7 @@ static int intel_dp_aux_setup_backlight(struct
>> > intel_connector *connector,
>> > >>   * via PWM pin or using AUX is better than using PWM pin.
>> > >>   *
>> > >>   * The heuristic to determine that using AUX pin is better than
>> > >> using PWM pin is
>> > >> - * that the panel support any of the feature list here.
>> > >> + * that the panel is eDP 1.4 or later and support any of the feature
>> > >> + list here
>> > >>   * - Regional backlight brightness adjustment
>> > >>   * - Backlight PWM frequency set
>> > >>   * - More than 8 bits resolution of brightness level @@ -310,6
>> > >> +310,10 @@ static int intel_dp_aux_setup_backlight(struct
>> > intel_connector *connector,
>> > >>          if (!(intel_dp->edp_dpcd[2] &
>> > DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))
>> > >>                  return true;
>> > >>
>> > >> +        /* Enable this for eDP 1.4 panel or later. */
>> > >> +        if (intel_dp->edp_dpcd[0] < DP_EDP_14)
>> > >> +                return false;
>> > >> +
>> > >>          /* Panel supports regional backlight brightness adjustment */
>> > >>          if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_GENERAL_CAP_3,
>> > >>                                &reg_val) != 1) {
>> >
>> > --
>> > 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