This patch set contain 3 patches. Another 6 patches in previous version was already merged in v7 and v9. - First patch sets the PWM freqency to match data in panel vbt. - Next patch adds heuristic to determine whether we should use AUX or PWM pin to adjust panel backlight brightness. - Last patch adds support for dynamic brightness.
Change log: v11: - Reorder patches in v10 to make the last patch come first - Fix nits
v10: - Add heuristic in patch #1 - Add _unsafe mod option in patch #1, #2 - handle frequency set error in patch #3
v9: - Fix nits in v8
v8: - Drop 4 patches that was already merged - Move DP_EDP_BACKLIGHT_AUX_ENABLE_CAP check to patch #2 to avoid behavior change if only apply patch #1 - Add TODO to warn about enable backlight twice in patch #2 - Use DIV_ROUND_CLOSEST instead of just "/" in patch #5 - Fix bug calculate pn in patch #5 - Clarify commit message / code comment in patch #5
v7: - Add check in intel_dp_pwm_pin_display_control_capable in patch #4 - Add option in patch #6 to enable DPCD or not - Change definition in patch #8 and implementation in #9 to use Khz - Fix compiler warning from build bot in patch #9
v6: - Address review from Dhinakaran - Make PWM frequency to have highest value of Pn that make the frequency still within 25% of desired frequency.
v5: - Split first patch in v4 to 3 patches - Bump priority for "Correctly enable backlight brightness adjustment via DPCD" - Make logic clearer for the case that both PWM pin and AUX are supported - Add more log when write to register fail - Add log when enable DBC
v4: - Rebase / minor typo fix.
v3: - Add new implementation of PWM frequency patch
v2: - Drop PWM frequency patch - Address suggestion from Jani Nikula
Puthikorn Voravootivat (3): drm/i915: Set PWM divider to match desired frequency in vbt drm/i915: Add heuristic to determine better way to adjust brightness drm/i915: Add option to support dynamic backlight via DPCD
drivers/gpu/drm/i915/i915_params.c | 12 +- drivers/gpu/drm/i915/i915_params.h | 5 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 185 ++++++++++++++++++++++++-- 3 files changed, 186 insertions(+), 16 deletions(-)
Read desired PWM frequency from panel vbt and calculate the value for divider in DPCD address 0x724 and 0x728 to have as many bits as possible for PWM duty cyle for granularity of brightness adjustment while the frequency divisor is still within 25% of the desired value.
Change-Id: I96221608e1288ffc03a0fd9c4a658809acda4aca Signed-off-by: Puthikorn Voravootivat puthik@chromium.org --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 98 ++++++++++++++++++++++++--- 1 file changed, 90 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index a0995c00fc84..6c64e1f75c4e 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -97,11 +97,85 @@ intel_dp_aux_set_backlight(struct intel_connector *connector, u32 level) } }
+/* + * Set PWM Frequency divider to match desired frequency in vbt. + * The PWM Frequency is calculated as 27Mhz / (F x P). + * - Where F = PWM Frequency Pre-Divider value programmed by field 7:0 of the + * EDP_BACKLIGHT_FREQ_SET register (DPCD Address 00728h) + * - Where P = 2^Pn, where Pn is the value programmed by field 4:0 of the + * EDP_PWMGEN_BIT_COUNT register (DPCD Address 00724h) + */ +static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector) +{ + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); + int freq, fxp, fxp_min, fxp_max, fxp_actual, f = 1; + u8 pn, pn_min, pn_max; + + /* Find desired value of (F x P) + * Note that, if F x P is out of supported range, the maximum value or + * minimum value will applied automatically. So no need to check that. + */ + freq = dev_priv->vbt.backlight.pwm_freq_hz; + DRM_DEBUG_KMS("VBT defined backlight frequency %u Hz\n", freq); + if (!freq) { + DRM_DEBUG_KMS("Use panel default backlight frequency\n"); + return false; + } + + fxp = DIV_ROUND_CLOSEST(KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ), freq); + + /* Use highest possible value of Pn for more granularity of brightness + * adjustment while satifying the conditions below. + * - Pn is in the range of Pn_min and Pn_max + * - F is in the range of 1 and 255 + * - FxP is within 25% of desired value. + * Note: 25% is arbitrary value and may need some tweak. + */ + if (drm_dp_dpcd_readb(&intel_dp->aux, + DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN, &pn_min) != 1) { + DRM_DEBUG_KMS("Failed to read pwmgen bit count cap min\n"); + return false; + } + if (drm_dp_dpcd_readb(&intel_dp->aux, + DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX, &pn_max) != 1) { + DRM_DEBUG_KMS("Failed to read pwmgen bit count cap max\n"); + return false; + } + pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK; + pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK; + + fxp_min = DIV_ROUND_CLOSEST(fxp * 3, 4); + fxp_max = DIV_ROUND_CLOSEST(fxp * 5, 4); + if (fxp_min < (1 << pn_min) || (255 << pn_max) < fxp_max) { + DRM_DEBUG_KMS("VBT defined backlight frequency out of range\n"); + return false; + } + + for (pn = pn_max; pn >= pn_min; pn--) { + f = clamp(DIV_ROUND_CLOSEST(fxp, 1 << pn), 1, 255); + fxp_actual = f << pn; + if (fxp_min <= fxp_actual && fxp_actual <= fxp_max) + break; + } + + if (drm_dp_dpcd_writeb(&intel_dp->aux, + DP_EDP_PWMGEN_BIT_COUNT, pn) < 0) { + DRM_DEBUG_KMS("Failed to write aux pwmgen bit count\n"); + return false; + } + if (drm_dp_dpcd_writeb(&intel_dp->aux, + DP_EDP_BACKLIGHT_FREQ_SET, (u8) f) < 0) { + DRM_DEBUG_KMS("Failed to write aux backlight freq\n"); + return false; + } + return true; +} + 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) { @@ -110,18 +184,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 */ @@ -130,6 +201,17 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) break; }
+ if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP) + if (intel_dp_aux_set_pwm_freq(connector)) + new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; + + 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); }
Add heuristic to decide that AUX or PWM pin should use for backlight brightness adjustment and modify i915 param description to have auto, force disable, and force enable.
The heuristic to determine that using AUX pin is better than using PWM pin is that the panel support any of the feature list here. - Regional backlight brightness adjustment - Backlight PWM frequency set - More than 8 bits resolution of brightness level - Backlight enablement via AUX and not by BL_ENABLE pin
Change-Id: If8376b09fbf83398c40037a04ddba1035c6b71fc Signed-off-by: Puthikorn Voravootivat puthik@chromium.org --- drivers/gpu/drm/i915/i915_params.c | 7 +-- drivers/gpu/drm/i915/i915_params.h | 2 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 61 +++++++++++++++++++++++++-- 3 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index b6a7e363d076..3758ae1f11b4 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -63,7 +63,7 @@ struct i915_params i915 __read_mostly = { .huc_firmware_path = NULL, .enable_dp_mst = true, .inject_load_failure = 0, - .enable_dpcd_backlight = false, + .enable_dpcd_backlight = -1, .enable_gvt = false, };
@@ -246,9 +246,10 @@ MODULE_PARM_DESC(enable_dp_mst, module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, 0400); MODULE_PARM_DESC(inject_load_failure, "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)"); -module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600); +module_param_named_unsafe(enable_dpcd_backlight, i915.enable_dpcd_backlight, int, 0600); MODULE_PARM_DESC(enable_dpcd_backlight, - "Enable support for DPCD backlight control (default:false)"); + "Enable support for DPCD backlight control " + "(-1:auto (default), 0:force disable, 1:force enabled if supported");
module_param_named(enable_gvt, i915.enable_gvt, bool, 0400); MODULE_PARM_DESC(enable_gvt, diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 34148cc8637c..643dfaf41c1f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -52,6 +52,7 @@ func(int, mmio_debug); \ func(int, edp_vswing); \ func(unsigned int, inject_load_failure); \ + func(int, enable_dpcd_backlight); \ /* leave bools at the end to not create holes */ \ func(bool, alpha_support); \ func(bool, enable_cmd_parser); \ @@ -66,7 +67,6 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \ - func(bool, enable_dpcd_backlight); \ func(bool, enable_gvt)
#define MEMBER(T, member) T member diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 6c64e1f75c4e..b73b3d431a82 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -250,15 +250,66 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) /* Check the eDP Display control capabilities registers to determine if * the panel can support backlight control over the aux channel */ - if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP && - (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) && - !(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) { + if ((intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && + (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); return true; } return false; }
+/* + * Heuristic function whether we should use AUX for backlight adjustment or not. + * + * We should use AUX for backlight brightness adjustment if panel doesn't this + * via PWM pin or using AUX is better than using PWM pin. + * + * The heuristic to determine that using AUX pin is better than using PWM pin is + * that the panel support any of the feature list here. + * - Regional backlight brightness adjustment + * - Backlight PWM frequency set + * - More than 8 bits resolution of brightness level + * - Backlight enablement via AUX and not by BL_ENABLE pin + * + * If all above are not true, assume that using PWM pin is better. + */ +static bool +intel_dp_aux_display_control_heuristic(struct intel_connector *connector) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); + uint8_t reg_val; + + /* Panel doesn't support adjusting backlight brightness via PWN pin */ + if (!(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) + return true; + + /* Panel supports regional backlight brightness adjustment */ + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_GENERAL_CAP_3, + ®_val) != 1) { + DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", + DP_EDP_GENERAL_CAP_3); + return false; + } + if (reg_val > 0) + return true; + + /* Panel supports backlight PWM frequency set */ + if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP) + return true; + + /* Panel supports more than 8 bits resolution of brightness level */ + if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT) + return true; + + /* Panel supports enabling backlight via AUX but not by BL_ENABLE pin */ + if ((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && + !(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP)) + return true; + + return false; + +} + int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) { struct intel_panel *panel = &intel_connector->panel; @@ -269,6 +320,10 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) if (!intel_dp_aux_display_control_capable(intel_connector)) return -ENODEV;
+ if (i915.enable_dpcd_backlight == -1 && + !intel_dp_aux_display_control_heuristic(intel_connector)) + return -ENODEV; + panel->backlight.setup = intel_dp_aux_setup_backlight; panel->backlight.enable = intel_dp_aux_enable_backlight; panel->backlight.disable = intel_dp_aux_disable_backlight;
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.
Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b 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 | 26 ++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 3758ae1f11b4..d84042ddf1fc 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 = true, };
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:true)"); diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 643dfaf41c1f..99f68d853c18 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -67,7 +67,8 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \ - 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 b73b3d431a82..b2f4cc975a3e 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -172,6 +172,24 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector) return true; }
+/* +* Set minimum / maximum dynamic brightness percentage. This value is expressed +* as the percentage of normal brightness in 5% increments. +*/ +static bool +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"); + return false; + } + return true; +} + static void intel_dp_aux_enable_backlight(struct intel_connector *connector) { struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); @@ -205,6 +223,14 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) if (intel_dp_aux_set_pwm_freq(connector)) new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
+ if (i915.enable_dbc && + (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP)) { + if(intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100)) { + new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE; + 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) {
On Fri, 2017-06-02 at 19:04 -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.
Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b 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 | 26 ++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 3758ae1f11b4..d84042ddf1fc 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 = true,
};
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:true)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 643dfaf41c1f..99f68d853c18 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -67,7 +67,8 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \
- 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 b73b3d431a82..b2f4cc975a3e 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -172,6 +172,24 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector) return true; }
+/* +* Set minimum / maximum dynamic brightness percentage. This value is expressed +* as the percentage of normal brightness in 5% increments. +*/ +static bool +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");return false;- }
- return true;
+}
static void intel_dp_aux_enable_backlight(struct intel_connector *connector) { struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); @@ -205,6 +223,14 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) if (intel_dp_aux_set_pwm_freq(connector)) new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
- if (i915.enable_dbc &&
(intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP)) {if(intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100)) {new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE;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) {
1) The Gerrit Change-ID's you've added in this version *may* be a problem, haven't seen any patches here with those before. 2) The PWM v/s AUX heuristics function looks likes a reasonable start that can be adjusted later. We should be taking into account the number of brightness levels b/w PWM pin and AUX options IMO. 3) The DPCD r/w needs optimization.
However things lgtm for now. So, Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com for this series.
Sorry forgot to remove Gerrit Change-ID before sending the email.
Do you want me to send the same patches again with your reviewed-by tag and the Gerrit change-Id removed?
On Mon, Jun 5, 2017 at 11:49 AM, Pandiyan, Dhinakaran < dhinakaran.pandiyan@intel.com> wrote:
On Fri, 2017-06-02 at 19:04 -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.
Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b 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 | 26
++++++++++++++++++++++++++
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index 3758ae1f11b4..d84042ddf1fc 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 = true,};
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:true)");diff --git a/drivers/gpu/drm/i915/i915_params.h
b/drivers/gpu/drm/i915/i915_params.h
index 643dfaf41c1f..99f68d853c18 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -67,7 +67,8 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \
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 b73b3d431a82..b2f4cc975a3e 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -172,6 +172,24 @@ static bool intel_dp_aux_set_pwm_freq(struct
intel_connector *connector)
return true;}
+/* +* Set minimum / maximum dynamic brightness percentage. This value is
expressed
+* as the percentage of normal brightness in 5% increments. +*/ +static bool +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 brightnesslevel\n");
return false;}return true;+}
static void intel_dp_aux_enable_backlight(struct intel_connector
*connector)
{ struct intel_dp *intel_dp = enc_to_intel_dp(&connector->
encoder->base);
@@ -205,6 +223,14 @@ static void intel_dp_aux_enable_backlight(struct
intel_connector *connector)
if (intel_dp_aux_set_pwm_freq(connector)) new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
if (i915.enable_dbc &&(intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP)) {if(intel_dp_aux_set_dynamic_backlight_percent(intel_dp,0, 100)) {
new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE;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) {
- The Gerrit Change-ID's you've added in this version *may* be a
problem, haven't seen any patches here with those before. 2) The PWM v/s AUX heuristics function looks likes a reasonable start that can be adjusted later. We should be taking into account the number of brightness levels b/w PWM pin and AUX options IMO. 3) The DPCD r/w needs optimization.
However things lgtm for now. So, Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com for this series.
On Mon, 2017-06-05 at 12:43 -0700, Puthikorn Voravootivat wrote:
Sorry forgot to remove Gerrit Change-ID before sending the email.
Do you want me to send the same patches again with your reviewed-by tag and the Gerrit change-Id removed?
Thanks, please do.
-DK
On Mon, Jun 5, 2017 at 11:49 AM, Pandiyan, Dhinakaran dhinakaran.pandiyan@intel.com wrote: On Fri, 2017-06-02 at 19:04 -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. > > Change-Id: I52f04b814bb4cd9df570ab59094ae974b9baec5b > 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 | 26 ++++++++++++++++++++++++++ > 3 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c > index 3758ae1f11b4..d84042ddf1fc 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 = true, > }; > > 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:true)"); > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h > index 643dfaf41c1f..99f68d853c18 100644 > --- a/drivers/gpu/drm/i915/i915_params.h > +++ b/drivers/gpu/drm/i915/i915_params.h > @@ -67,7 +67,8 @@ > func(bool, verbose_state_checks); \ > func(bool, nuclear_pageflip); \ > func(bool, enable_dp_mst); \ > - 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 b73b3d431a82..b2f4cc975a3e 100644 > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > @@ -172,6 +172,24 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector) > return true; > } > > +/* > +* Set minimum / maximum dynamic brightness percentage. This value is expressed > +* as the percentage of normal brightness in 5% increments. > +*/ > +static bool > +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"); > + return false; > + } > + return true; > +} > + > static void intel_dp_aux_enable_backlight(struct intel_connector *connector) > { > struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); > @@ -205,6 +223,14 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) > if (intel_dp_aux_set_pwm_freq(connector)) > new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; > > + if (i915.enable_dbc && > + (intel_dp->edp_dpcd[2] & DP_EDP_DYNAMIC_BACKLIGHT_CAP)) { > + if(intel_dp_aux_set_dynamic_backlight_percent(intel_dp, 0, 100)) { > + new_dpcd_buf |= DP_EDP_DYNAMIC_BACKLIGHT_ENABLE; > + 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) {
1) The Gerrit Change-ID's you've added in this version *may* be a problem, haven't seen any patches here with those before. 2) The PWM v/s AUX heuristics function looks likes a reasonable start that can be adjusted later. We should be taking into account the number of brightness levels b/w PWM pin and AUX options IMO. 3) The DPCD r/w needs optimization. However things lgtm for now. So, Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> for this series.
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
dri-devel@lists.freedesktop.org
-
Pandiyan, Dhinakaran -
Puthikorn Voravootivat