<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 2, 2017 at 8:00 PM, Pandiyan, Dhinakaran <span dir="ltr"><<a href="mailto:dhinakaran.pandiyan@intel.com" target="_blank" class="gmail-cremed cremed">dhinakaran.pandiyan@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">On Tue, 2017-04-18 at 16:48 -0700, Puthikorn Voravootivat wrote:<br>
> This patch enables dynamic backlight by default for eDP<br>
> panel that supports this feature via DPCD register and<br>
> set minimum / maximum brightness to 0% and 100% of the<br>
> normal brightness.<br>
<br>
<br>
</span>What does dynamic backlight do? I am trying to understand what allowing<br>
0% minimum brightness means.<br>
<div><div class="gmail-h5"><br></div></div></blockquote><div>Display will make backlight dimmer when content shown on screen is darker.</div><div>See <a href="https://www.ecnmag.com/article/2010/04/content-adaptive-lcd-backlight-control">https://www.ecnmag.com/article/2010/04/content-adaptive-lcd-backlight-control</a></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-h5">
<br>
><br>
> Signed-off-by: Puthikorn Voravootivat <<a href="mailto:puthik@chromium.org" class="gmail-cremed cremed">puthik@chromium.org</a>><br>
> ---<br>
>  drivers/gpu/drm/i915/intel_dp_<wbr>aux_backlight.c | 31 +++++++++++++++++++++++----<br>
>  1 file changed, 27 insertions(+), 4 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/i915/intel_<wbr>dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_<wbr>dp_aux_backlight.c<br>
> index f06c8381c74e..ae1b6fe67feb 100644<br>
> --- a/drivers/gpu/drm/i915/intel_<wbr>dp_aux_backlight.c<br>
> +++ b/drivers/gpu/drm/i915/intel_<wbr>dp_aux_backlight.c<br>
> @@ -97,10 +97,24 @@ intel_dp_aux_set_backlight(<wbr>struct intel_connector *connector, u32 level)<br>
>       }<br>
>  }<br>
><br>
> +/*<br>
> + * Set minimum / maximum dynamic brightness percentage. This value is expressed<br>
> + * as the percentage of normal brightness in 5% increments.<br>
> + */<br>
> +static void<br>
> +intel_dp_aux_set_dynamic_<wbr>backlight_percent(struct intel_dp *intel_dp,<br>
> +                                        u32 min, u32 max)<br>
> +{<br>
> +     u8 dbc[] = { DIV_ROUND_CLOSEST(min, 5), DIV_ROUND_CLOSEST(max, 5) };<br>
> +     drm_dp_dpcd_write(&intel_dp-><wbr>aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_<wbr>SET,<br>
> +                       dbc, sizeof(dbc));<br>
> +}<br>
> +<br>
>  static void intel_dp_aux_enable_backlight(<wbr>struct intel_connector *connector)<br>
>  {<br>
>       struct intel_dp *intel_dp = enc_to_intel_dp(&connector-><wbr>encoder->base);<br>
>       uint8_t dpcd_buf = 0;<br>
> +     uint8_t new_dpcd_buf = 0;<br>
>       uint8_t edp_backlight_mode = 0;<br>
><br>
>       set_aux_backlight_enable(<wbr>intel_dp, true);<br>
> @@ -110,16 +124,15 @@ static void intel_dp_aux_enable_backlight(<wbr>struct intel_connector *connector)<br>
>               return;<br>
>       }<br>
><br>
> +     new_dpcd_buf = dpcd_buf;<br>
>       edp_backlight_mode = dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_<wbr>MASK;<br>
><br>
>       switch (edp_backlight_mode) {<br>
>       case DP_EDP_BACKLIGHT_CONTROL_MODE_<wbr>PWM:<br>
>       case DP_EDP_BACKLIGHT_CONTROL_MODE_<wbr>PRESET:<br>
>       case DP_EDP_BACKLIGHT_CONTROL_MODE_<wbr>PRODUCT:<br>
> -             dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_<wbr>MODE_MASK;<br>
> -             dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_<wbr>DPCD;<br>
> -             drm_dp_dpcd_writeb(&intel_dp-><wbr>aux,<br>
> -                     DP_EDP_BACKLIGHT_MODE_SET_<wbr>REGISTER, dpcd_buf);<br>
> +             new_dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_<wbr>MODE_MASK;<br>
> +             new_dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_<wbr>DPCD;<br>
>               break;<br>
><br>
>       /* Do nothing when it is already DPCD mode */<br>
> @@ -127,6 +140,16 @@ static void intel_dp_aux_enable_backlight(<wbr>struct intel_connector *connector)<br>
>       default:<br>
>               break;<br>
>       }<br>
<br>
<br>
</div></div>Still need a switch here? You are setting mode as<br>
DP_EDP_BACKLIGHT_CONTROL_MODE_<wbr>DPCD for all four values of mode.<br>
<span class="gmail-"><br>
<br>
<br>
> +<br>
> +     if (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP) {<br>
> +             new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_<wbr>ENABLE;<br>
> +             intel_dp_aux_set_dynamic_<wbr>backlight_percent(intel_dp, 0, 100);<br>
> +     }<br>
<br>
</span>Should enabling dynamic backlight control be logged in debug messages?<br>
Could be useful in case this turns out to be buggy.<br></blockquote><div>Sure </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
-DK<br>
<div class="gmail-HOEnZb"><div class="gmail-h5"><br>
> +<br>
> +     if (new_dpcd_buf != dpcd_buf) {<br>
> +             drm_dp_dpcd_writeb(&intel_dp-><wbr>aux,<br>
> +                     DP_EDP_BACKLIGHT_MODE_SET_<wbr>REGISTER, new_dpcd_buf);<br>
> +     }<br>
>  }<br>
><br>
>  static void intel_dp_aux_disable_<wbr>backlight(struct intel_connector *connector)<br>
<br>
</div></div></blockquote></div><br></div></div>