Re: [PATCH v2] drm/i915: Refine eDP aux backlight enable/disable sequence

From: "Lee, Shawn C" <shawn.c.lee@intel.com>
To: "Nikula, Jani" <jani.nikula@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Cc: "Chiou, Cooper" <cooper.chiou@intel.com>
Subject: Re: [PATCH v2] drm/i915: Refine eDP aux backlight enable/disable sequence
Date: Tue, 25 Jun 2019 15:29:21 +0000	[thread overview]
Message-ID: <D42A2A322A1FCA4089E30E9A9BA36AC65D61C808@PGSMSX111.gar.corp.intel.com> (raw)
In-Reply-To: <87pnn1ofyx.fsf@intel.com>

On Tue, 25 Jun 2019, Jani Nikula <jani.nikula@intel.com> wrote:
>On Tue, 25 Jun 2019, Lee Shawn C <shawn.c.lee@intel.com> wrote:
>> Modify aux backlight enable/disable sequence just like what we did for 
>> genernal eDP panel. Setup PWM freq and brightness level before enable 
>> display backlight.
>>
>> v2: According to spec, it is not necessary to set PWM to 0 if
>>     backlight enable already off. Remove intel_dp_aux_set_backlight to 0.
>
>Please think it through again. The same applies here. The brightness change before enable is ignored according to eDP spec.
>
>BR,
>Jani.
>

This is what the problem we see on a new eDP panel from customer. TCON did not turn backlight on successful.
After sawp the sequence, backlight works well as expected. 

In the other words. Looks like this TCON backlight enable flow violate eDP spec. What do you think?

>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Jose Roberto de Souza <jose.souza@intel.com>
>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>>
>> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c 
>> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> index 6b0b73479fb8..70d9b9c9586f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> @@ -216,8 +216,8 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st
>>  		}
>>  	}
>>  
>> -	set_aux_backlight_enable(intel_dp, true);
>>  	intel_dp_aux_set_backlight(conn_state, 
>> connector->panel.backlight.level);
>> +	set_aux_backlight_enable(intel_dp, true);
>>  }
>>  
>>  static void intel_dp_aux_disable_backlight(const struct 
>> drm_connector_state *old_conn_state)
>
>--
>Jani Nikula, Intel Open Source Graphics Center
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx