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

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

On Fri, 14 Jun 2019, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> On Thu, 13 Jun 2019, Jani Nikula <jani.nikula@intel.com> wrote:
>>On Thu, 13 Jun 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>> On Wed, Jun 12, 2019 at 10:47:22PM -0700, Lee, Shawn C wrote:
>>>> Modify aux backlight enable sequence just like what we did for 
>>>> genernal eDP panel.
>>>> 1. Setup PWM freq and brightness level before enable display backlight.
>>>> 2. Add T8 (valid data to backlight enable) delay.
>>>
>>> If we respect the on_delay shouldn't we then respect the off_delay too?
>>>
>>> In which case I suggest two patches:
>>> 1. swaap the enable vs. set_brightness order 2. add the on/off delays
>>
>>The aux backlight functions are called via the same hooks as everything in intel_panel.c, which do not have delays (they're all on the calling side in intel_dp.c) so I'm confused why these would require the delays.
>>
>>BR,
>>Jani.
>
> Refer to intel_edp_backlight_on(). PWM was restored at the end of
> intel_panel_enable_backlight().  After that, driver would wait for
> some delay wait_backlight_on() before enable backlight.

You need to be more specific.

I repeat: When you have DPCD AUX backlight in use, it gets enabled via
intel_panel_enable_backlight(). All the delays for backlight (regardless
of whether it's DDI PWM or DPCD AUX) are already handled in intel_dp.c.

What is the exact problem you're trying to solve?

BR,
Jani.


>
> Thanks for Ville's recommendation. I will send separate for this change.
>
> Best regards,
> Shawn
>
>>>> 
>>>> 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/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 7ded95a334db..c112e0b930de 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>>>> @@ -216,8 +216,12 @@ 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);
>>>> +
>>>> +	wait_remaining_ms_from_jiffies(intel_dp->last_power_on,
>>>> +				       intel_dp->backlight_on_delay);
>>>> +
>>>> +	set_aux_backlight_enable(intel_dp, true);
>>>>  }
>>>>  
>>>>  static void intel_dp_aux_disable_backlight(const struct 
>>>> drm_connector_state *old_conn_state)
>>>> --
>>>> 2.7.4
>>>> 
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>--
>>Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx