[Intel-gfx] [PATCH] drm/i915: Refine eDP aux backlight enable/disable sequence

[Jani Nikula]

On Tue, 25 Jun 2019, "Lee, Shawn C" <shawn.c.lee at intel.com> wrote:
> On Tue, 25 Jun 2019, Jani Nikula <jani.nikula at intel.com> wrote:
>>On Thu, 20 Jun 2019, Lee Shawn C <shawn.c.lee at intel.com> wrote:
>>> Modify aux backlight enable/disable sequence just like what we did for 
>>> genernal eDP panel.
>>> 1. Setup PWM freq and brightness level before enable display backlight.
>>> 2. Set PWM to 0 after backlight enable was off.
>>
>>It took me a while to realize why we do this in this order.
>>
>>eDP v1.4b says about DPCD addresses 00722h-00723h i.e. EDP_BACKLIGHT_BRIGHTNESS MSB and LSB:
>>
>>"""
>>Writes are ignored if:
>>
>>[...]
>>
>>- BACKLIGHT_ENABLE bit in the EDP_DISPLAY_CONTROL register (DPCD Address
>>  00720h, bit 0) is cleared to 0"
>>"""
>>
>>i.e. per the spec this change makes the brightness updates no-ops.
>>
>>Do you have a panel where this patch makes a difference?
>>
>>BR,
>>Jani.
>>
>
> What you say is correct. According to spec, write EDP_BACKLIGHT_BRIGHTNESS does not affect
> if BACKLIGHT_ENABLE already be off.
>
> Backlight enable always turn off before stop PWM output when used general eDP backlight (from SOC) control.
> But AUX backlight control is a little different. I will update this patch and remove "Set PWM to 0".

AFAICT the only thing you can do, really, is calling
intel_dp_aux_set_backlight(old_conn_state, 0); *before*
set_aux_backlight_enable(enc_to_intel_dp(old_conn_state->best_encoder),
false); in intel_dp_aux_disable_backlight().

BR,
Jani.



>
>>>
>>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> Cc: Jani Nikula <jani.nikula at intel.com>
>>> Cc: Jose Roberto de Souza <jose.souza at intel.com>
>>> Cc: Cooper Chiou <cooper.chiou at intel.com>
>>>
>>> Signed-off-by: Lee Shawn C <shawn.c.lee at intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 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..bbc579734238 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>>> @@ -216,13 +216,14 @@ 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)  {
>>>  	
>>> set_aux_backlight_enable(enc_to_intel_dp(old_conn_state->best_encoder)
>>> , false);
>>> +	intel_dp_aux_set_backlight(old_conn_state, 0);
>>>  }
>>>  
>>>  static int intel_dp_aux_setup_backlight(struct intel_connector 
>>> *connector,
>>
>>--
>>Jani Nikula, Intel Open Source Graphics Center
>>

-- 
Jani Nikula, Intel Open Source Graphics Center