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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 19 Jul 2017, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@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?

>> 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@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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx