<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, May 30, 2017 at 9:18 PM, Pandiyan, Dhinakaran <<a href="mailto:dhinakaran.pandiyan@intel.com" target="_blank" class="cremed">dhinakaran.pandiyan@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, 2017-05-26 at 18:42 -0700, Puthikorn Voravootivat wrote: > This patch adds option to enable dynamic backlight for eDP > panel that supports this feature via DPCD register and > set minimum / maximum brightness to 0% and 100% of the > normal brightness. > > Signed-off-by: Puthikorn Voravootivat <<a href="mailto:puthik@chromium.org" class="cremed">puthik@chromium.org</a>> > --- > drivers/gpu/drm/i915/i915_params.c | 5 ++++ > drivers/gpu/drm/i915/i915_params.h | 3 +- > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 42 ++++++++++++++++++++++----- > 3 files changed, 41 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > index 3758ae1f11b4..ce033d58134e 100644 > --- a/drivers/gpu/drm/i915/i915_params.c > +++ b/drivers/gpu/drm/i915/i915_params.c > @@ -65,6 +65,7 @@ struct i915_params i915 __read_mostly = { > .inject_load_failure = 0, > .enable_dpcd_backlight = -1, > .enable_gvt = false, > + .enable_dbc = false, Based on Daniel's earlier comments on module parameters, shouldn't this be enabled by default too? </blockquote><div> </div><div>Yes. Will do.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Or even more importantly, is this the right approach to enable/disable dynamic back light control? The reason I recommended having some sort of control to disable/enable is that the eDP spec. says the feature can have user visible impact. </blockquote><div> </div><div>I don't think we should expect end user to set this correctly. For power user,</div><div>I think the i915_params is adequate. I don't want to add more complication</div><div>to the driver.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Table 10-3 Display Control Capabilities "While the DBC control bit’s function is defined in this Standard, DBC implementation specifics are not defined, including interaction with other DPCD register settings. The DBC implementation, visual performance, and power savings characteristics can differ between specific panels." -DK <div class="HOEnZb"><div class="h5"> > }; > > module_param_named(modeset, i915.modeset, int, 0400); > @@ -254,3 +255,7 @@ MODULE_PARM_DESC(enable_dpcd_backlight, > module_param_named(enable_gvt, i915.enable_gvt, bool, 0400); > MODULE_PARM_DESC(enable_gvt, > "Enable support for Intel GVT-g graphics virtualization host support(default:false)"); > + > +module_param_named_unsafe(enable_dbc, i915.enable_dbc, bool, 0600); > +MODULE_PARM_DESC(enable_dbc, > + "Enable support for dynamic backlight control (default:false)"); > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h > index ac02efce6e22..2de3e2850b54 100644 > --- a/drivers/gpu/drm/i915/i915_params.h > +++ b/drivers/gpu/drm/i915/i915_params.h > @@ -67,7 +67,8 @@ > func(bool, nuclear_pageflip); \ > func(bool, enable_dp_mst); \ > func(int, enable_dpcd_backlight); \ > - func(bool, enable_gvt) > + func(bool, enable_gvt); \ > + func(bool, enable_dbc) > > #define MEMBER(T, member) T member > struct i915_params { > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > index c89aae804659..f55af41ce3bd 100644 > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > @@ -100,11 +100,26 @@ intel_dp_aux_set_backlight(struct intel_connector *connector, u32 level) > } > } > > +/* > + * Set minimum / maximum dynamic brightness percentage. This value is expressed > + * as the percentage of normal brightness in 5% increments. > + */ > +static void > +intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp, > + u32 min, u32 max) > +{ > + u8 dbc[] = { DIV_ROUND_CLOSEST(min, 5), DIV_ROUND_CLOSEST(max, 5) }; > + > + if (drm_dp_dpcd_write(&intel_dp->aux, DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET, > + dbc, sizeof(dbc)) < 0) { > + DRM_DEBUG_KMS("Failed to write aux DBC brightness level\n"); > + } > +} > + > static void intel_dp_aux_enable_backlight(struct intel_connector *connector) > { > struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); > - uint8_t dpcd_buf = 0; > - uint8_t edp_backlight_mode = 0; > + uint8_t dpcd_buf, new_dpcd_buf, edp_backlight_mode; > > if (drm_dp_dpcd_readb(&intel_dp->aux, > DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { > @@ -113,18 +128,15 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) > return; > } > > + new_dpcd_buf = dpcd_buf; > edp_backlight_mode = dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK; > > switch (edp_backlight_mode) { > case DP_EDP_BACKLIGHT_CONTROL_MODE_PWM: > case DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET: > case DP_EDP_BACKLIGHT_CONTROL_MODE_PRODUCT: > - dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK; > - dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD; > - if (drm_dp_dpcd_writeb(&intel_dp->aux, > - DP_EDP_BACKLIGHT_MODE_SET_REGISTER, dpcd_buf) < 0) { > - DRM_DEBUG_KMS("Failed to write aux backlight mode\n"); > - } > + new_dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK; > + new_dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD; > break; > > /* Do nothing when it is already DPCD mode */ > @@ -133,6 +145,20 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) > break; > } > > + if (i915.enable_dbc && > + (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP)) { > + new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE; > + intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100); > + DRM_DEBUG_KMS("Enable dynamic brightness.\n"); > + } > + > + if (new_dpcd_buf != dpcd_buf) { > + if (drm_dp_dpcd_writeb(&intel_dp->aux, > + DP_EDP_BACKLIGHT_MODE_SET_REGISTER, new_dpcd_buf) < 0) { > + DRM_DEBUG_KMS("Failed to write aux backlight mode\n"); > + } > + } > + > set_aux_backlight_enable(intel_dp, true); > intel_dp_aux_set_backlight(connector, connector->panel.backlight.level); > } </div></div></blockquote></div> </div></div>