On Tue, May 30, 2017 at 9:18 PM, Pandiyan, Dhinakaran <dhinakaran.pandiyan@xxxxxxxxx> 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@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_params.c | 5 ++++ Based on Daniel's earlier comments on module parameters, shouldn't this
> 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,
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);
> }
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
