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

[Tc, Jenny]

> >> 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;
+       }
        /* 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