| Message ID | 20170527014209.186331-3-puthik@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <puthik@chromium.org> > --- > 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? 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. 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 > }; > > 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); > }
On Tue, May 30, 2017 at 9:18 PM, Pandiyan, Dhinakaran < dhinakaran.pandiyan@intel.com> wrote: > 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 <puthik@chromium.org> > > --- > > 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? > Yes. Will do. > > 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. > I don't think we should expect end user to set this correctly. For power user, I think the i915_params is adequate. I don't want to add more complication to the driver. > > 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 > > > }; > > > > 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); > > } > >
On Wed, 2017-05-31 at 14:37 -0700, Puthikorn Voravootivat wrote: > > > On Tue, May 30, 2017 at 9:18 PM, Pandiyan, Dhinakaran > <dhinakaran.pandiyan@intel.com> wrote: > 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 <puthik@chromium.org> > > --- > > 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? > > > Yes. Will do. > > > 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. > > > I don't think we should expect end user to set this correctly. For > power user, > I think the i915_params is adequate. I don't want to add more > complication > to the driver. > Sure, I am not requesting any changes here :) I meant to address the question to Daniel in case he had any feedback. -DK > > 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 > > > }; > > > > 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); > > } > > > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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 <puthik@chromium.org> > --- > 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, > }; > > 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"); > + } > + Just curious, does DBC require DP AUX brightness control? -DK > + 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); > }
On Fri, Jun 2, 2017 at 11:25 AM, Pandiyan, Dhinakaran < dhinakaran.pandiyan@intel.com> wrote: > 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 <puthik@chromium.org> > > --- > > 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, > > }; > > > > 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"); > > + } > > + > > > Just curious, does DBC require DP AUX brightness control? > > No. Here is quote from the spec. "The Dynamic Backlight Control (DBC) and color engine modes are not listed in Table 10-1, because when they are available, they are available for all supported modes." > -DK > > > + 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); > > } > >
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, }; 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); }
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 <puthik@chromium.org> --- 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(-)