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

[Pandiyan, Dhinakaran]

On Thu, 2017-07-20 at 18:35 +0300, David Weinehall wrote:
> On 2017-07-20 12:33, Jani Nikula wrote:
> > On Wed, 19 Jul 2017, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan at intel.com> wrote:
> >> On Wed, 2017-07-19 at 15:59 +0530, Jenny TC wrote:
> >>> With older panels, AUX pin for backlight doesn't work.
> > What evidence do you have to back up that claim?
> Chapter 10.1 does, at least according to my reading, state that AUX CH 
> backlight control
> requires eDP v1.4.
> 


Yeah, I read that too and it is very misleading. There are several
references to AUX CH backlight control being added in eDP v1.2 if you
read further. 

"Table 10-1 summarizes the display brightness control modes available
with eDP v1.2 (and higher)."


"Displays that report eDP v1.1 (or lower; indicated by a value of 00h)
have 00h values in DPCD Addresses 00701h through 007FFh a because
enhanced display control capability is not supported. For eDP v1.2 (and
higher), the Source device reads the eDP capability registers starting
at DPCD Address 00701h. Through the use of read/write registers located
within the range of DPCD Addresses 00720h through 007FFh, the Source
device is able to control certain display aspects based on the
capabilities reported h in DPCD Addresses 00701h through 0071Fh."



> >>> On some panels, this causes backlight issues on S3 resume.
> >> What is it that we are missing in the resume path?
> I suspect this isn't so much a resume issue, as a matter of some devices 
> by sheer luck (read by using the backlight set already on bootup) 
> keeping the right backlight level until after a suspend/resume cycle.

Makes sense.

> >>
> >>>   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) {
> 
> ---------------------------------------------------------------------
> Intel Finland Oy
> Registered Address: PL 281, 00181 Helsinki 
> Business Identity Code: 0357606 - 4 
> Domiciled in Helsinki 
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx