[PATCH v5 2/9] drm/i915: Correctly enable backlight brightness adjustment via DPCD
intel_dp_aux_enable_backlight() assumed that the register BACKLIGHT_BRIGHTNESS_CONTROL_MODE can only has value 01 (DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET) when initialize. This patch fixed that by handling all cases of that register. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 33 ++- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 24a905d1a74b..ad8560c5f689 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -97,15 +97,36 @@ 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; set_aux_backlight_enable(intel_dp, true); - if ((drm_dp_dpcd_readb(&intel_dp->aux, - DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) == 1) && - ((dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) == -DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET)) - drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, - (dpcd_buf | DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD)); + if (drm_dp_dpcd_readb(&intel_dp->aux, + DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { + DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", + DP_EDP_BACKLIGHT_MODE_SET_REGISTER); + return; + } + + 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"); + } + break; + + /* Do nothing when it is already DPCD mode */ + case DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD: + default: + break; + } } static void intel_dp_aux_disable_backlight(struct intel_connector *connector) -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 5/9] drm/i915: Set backlight mode before enable backlight
We should set backlight mode register before set register to enable the backlight. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index e82f7cb9a7af..5ef3ade7c40e 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -103,8 +103,6 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) uint8_t dpcd_buf = 0; uint8_t edp_backlight_mode = 0; - set_aux_backlight_enable(intel_dp, true); - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", @@ -131,6 +129,8 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) default: break; } + + set_aux_backlight_enable(intel_dp, true); } static void intel_dp_aux_disable_backlight(struct intel_connector *connector) -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 3/9] drm/i915: Drop AUX backlight enable check for backlight control
There are some panel that (1) does not support display backlight enable via AUX (2) support display backlight adjustment via AUX (3) support display backlight enable via eDP BL_ENABLE pin The current driver required that (1) must be support to enable (2). This patch drops that requirement. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index ad8560c5f689..5b83c9737644 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) { uint8_t reg_val = 0; + /* Early return when display use other mechanism to enable backlight. */ + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) + return; + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, ®_val) < 0) { DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); return true; -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 1/9] drm/i915: Fix cap check for intel_dp_aux_backlight driver
intel_dp_aux_backlight driver should check for the DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP before enable the driver. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 6532e226db29..24a905d1a74b 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -142,10 +142,9 @@ 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 && + if ((intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && - !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || - (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) { + (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) { DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); return true; } -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 6/9] drm/i915: Support dynamic backlight via DPCD register
This patch enables dynamic backlight by default 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 --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 39 ++- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 5ef3ade7c40e..7d323af96636 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -97,10 +97,27 @@ 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 new_dpcd_buf = 0; uint8_t edp_backlight_mode = 0; if (drm_dp_dpcd_readb(&intel_dp->aux, @@ -110,18 +127,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 +144,19 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) break; } + if (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); } -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 4/9] drm/i915: Allow choosing how to adjust brightness if both supported
Add option to allow choosing how to adjust brightness if panel supports both PWM pin and AUX channel. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/i915_params.c| 6 -- drivers/gpu/drm/i915/i915_params.h| 2 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 17 - 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index b6a7e363d076..323d12badace 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, }; @@ -248,7 +248,9 @@ 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_PARM_DESC(enable_dpcd_backlight, - "Enable support for DPCD backlight control (default:false)"); + "Enable support for DPCD backlight control " + "(-1:disable (default), 0:Use PWM pin if both supported, " + "1:Use DPCD if both 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..ac02efce6e22 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -66,7 +66,7 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \ - func(bool, enable_dpcd_backlight); \ + func(int, 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 5b83c9737644..e82f7cb9a7af 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -175,11 +175,26 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) return false; } +static bool +intel_dp_pwm_pin_display_control_capable(struct intel_connector *connector) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); + + /* Check the eDP Display control capabilities registers to determine if +* the panel can support backlight control via BL_PWM_DIM eDP pin +*/ + return intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP; +} + int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) { struct intel_panel *panel = &intel_connector->panel; - if (!i915.enable_dpcd_backlight) + if (i915.enable_dpcd_backlight == -1) + return -ENODEV; + + if (i915.enable_dpcd_backlight == 0 && + intel_dp_pwm_pin_display_control_capable(intel_connector)) return -ENODEV; if (!intel_dp_aux_display_control_capable(intel_connector)) -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 0/9] Enhancement to intel_dp_aux_backlight driver
This patch set contain 9 patches. - First five patches fix bug in the driver and allow choosing which way to adjust brightness if both PWM pin and AUX are supported - Next patch adds enable DBC by default - Next patch makes the driver restore last brightness level after turning display off and on. - Last two patches set the PWM freqency to match data in panel vbt. Change log: v2: - Drop PWM frequency patch - Addess suggestion from Jani Nikula v3: - Add new implementation of PWM frequency patch v4: - Rebase / minor typo fix. 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 Puthikorn Voravootivat (9): drm/i915: Fix cap check for intel_dp_aux_backlight driver drm/i915: Correctly enable backlight brightness adjustment via DPCD drm/i915: Drop AUX backlight enable check for backlight control drm/i915: Allow choosing how to adjust brightness if both supported drm/i915: Set backlight mode before enable backlight drm/i915: Support dynamic backlight via DPCD register drm/i915: Restore brightness level in aux backlight driver drm: Add definition for eDP backlight frequency drm/i915: Set PWM divider to match desired frequency in vbt drivers/gpu/drm/i915/i915_params.c| 6 +- drivers/gpu/drm/i915/i915_params.h| 2 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 161 -- include/drm/drm_dp_helper.h | 2 + 4 files changed, 156 insertions(+), 15 deletions(-) -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
Read desired PWM frequency from panel vbt and calculate the value for divider in DPCD address 0x724 and 0x728 to match that frequency as close as possible. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 71 +++ 1 file changed, 71 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index fc26fea94fd4..441ad434a82b 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -113,12 +113,76 @@ intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp, } } +/* + * 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 void 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, f; + 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; + } + + fxp = DP_EDP_BACKLIGHT_FREQ_BASE / freq; + + /* Use lowest possible value of Pn to try to make F to be between 1 and +* 255 while still in the range Pn_min and Pn_max +*/ + 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; + } + 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; + } + pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK; + pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK; + + f = fxp >> pn_min; + + for (pn = pn_min; pn < pn_max && f > 255; pn++) + f >>= 1; + + f = clamp(f, 1, 255); + + 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; + } + 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; + } +} + 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 new_dpcd_buf = 0; uint8_t edp_backlight_mode = 0; + bool freq_cap; if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { @@ -150,6 +214,10 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) DRM_DEBUG_KMS("Enable dynamic brightness.\n"); } + freq_cap = intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP; + if (freq_cap) + 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) { @@ -157,6 +225,9 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) } } + if (freq_cap) + intel_dp_aux_set_pwm_freq(connector); + set_aux_backlight_enable(intel_dp, true); intel_dp_aux_set_backlight(connector, connector->panel.backlight.level); } -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 8/9] drm: Add definition for eDP backlight frequency
This patch adds the following definition - Bit mask for EDP_PWMGEN_BIT_COUNT and min/max cap register which only use bit 0:4 - Base frequency (27 MHz) for backlight PWM frequency generator. Signed-off-by: Puthikorn Voravootivat --- include/drm/drm_dp_helper.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index c0bd0d7651a9..810b7d5d9f2b 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -572,10 +572,12 @@ #define DP_EDP_PWMGEN_BIT_COUNT 0x724 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN 0x725 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX 0x726 +# define DP_EDP_PWMGEN_BIT_COUNT_MASK (0x1f << 0) #define DP_EDP_BACKLIGHT_CONTROL_STATUS 0x727 #define DP_EDP_BACKLIGHT_FREQ_SET 0x728 +# define DP_EDP_BACKLIGHT_FREQ_BASE 2700 #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB 0x72a #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID 0x72b -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 7/9] drm/i915: Restore brightness level in aux backlight driver
Some panel will default to zero brightness when turning the panel off and on again. This patch restores last brightness level back when panel is turning back on. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 7d323af96636..fc26fea94fd4 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -158,6 +158,7 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) } set_aux_backlight_enable(intel_dp, true); + intel_dp_aux_set_backlight(connector, connector->panel.backlight.level); } static void intel_dp_aux_disable_backlight(struct intel_connector *connector) -- 2.13.0.rc1.294.g07d810a77f-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
This is not related to brightness control. This calculation is used to set the PWM frequency. Frequency = 27 Mhz / (F * 2^ Pn) Lower Pn means higher value for F which mean more accuracy for this calculation. On Sat, May 6, 2017 at 1:35 AM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat wrote: > > Read desired PWM frequency from panel vbt and calculate the > > value for divider in DPCD address 0x724 and 0x728 to match > > that frequency as close as possible. > > > > Signed-off-by: Puthikorn Voravootivat > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 71 > +++ > > 1 file changed, 71 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index fc26fea94fd4..441ad434a82b 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -113,12 +113,76 @@ intel_dp_aux_set_dynamic_backlight_percent(struct > intel_dp *intel_dp, > > } > > } > > > > +/* > > + * 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 void 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, f; > > + 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; > > + } > > + > > + fxp = DP_EDP_BACKLIGHT_FREQ_BASE / freq; > > + > > + /* Use lowest possible value of Pn to try to make F to be between > 1 and > > What's the reason to use the lowest possible value of Pn? From what I > understand, choosing a higher value of Pn offers more steps for > brightness control. > > -DK > > > + * 255 while still in the range Pn_min and Pn_max > > + */ > > + 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; > > + } > > + 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; > > + } > > + pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK; > > + pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK; > > + > > + f = fxp >> pn_min; > > + > > + for (pn = pn_min; pn < pn_max && f > 255; pn++) > > + f >>= 1; > > + > > + f = clamp(f, 1, 255); > > + > > + 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; > > + } > > + 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; > > + } > > +} > > + > > 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
Re: [Intel-gfx] [PATCH v5 1/9] drm/i915: Fix cap check for intel_dp_aux_backlight driver
I added the option to choose to prioritized AUX or PWM before in last version of this patch set. But comment from Jani said that it is better to separate the patch. The option to prioritized the PWM in now in patch #4 On Mon, May 8, 2017 at 10:55 AM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat wrote: > > intel_dp_aux_backlight driver should check for the > > DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP before enable the driver. > > > > Signed-off-by: Puthikorn Voravootivat > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index 6532e226db29..24a905d1a74b 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -142,10 +142,9 @@ 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 > && > > + if ((intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) > && > > (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > > - !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || > > - (intel_dp->edp_dpcd[2] & > > DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) > { > > ^Were these two changes intended? The patch claims an additional check > for DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP is being added but also > makes this unrelated change. Aren't you changing how the driver > prioritizes AUX v/s PWM brightness control by removing these lines? > > -DK > > > > + (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) > { > > DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); > > return true; > > } > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
Actually you are right. Sorry it's my mistake. I was focusing on make the actual frequency match the desired frequency as much as possible. But more granularity in backlight adjustment probably more important than that. Will submit new version of this patch to fix this. On Mon, May 8, 2017 at 11:17 AM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Mon, 2017-05-08 at 10:49 -0700, Puthikorn Voravootivat wrote: > > This is not related to brightness control. This calculation is used to > > set the PWM frequency. > > Frequency = 27 Mhz / (F * 2^ Pn) > > > > Lower Pn means higher value for F which mean more accuracy for this > > calculation. > > I am not sure I follow this. Quoting from the spec - > "A larger Pn value (meaning more PWM generator control bits) provides a > finer backlight adjustment (increased granularity), but also limits > the maximum backlight frequency, as described in this section." > > and then again > > "Allowing Pn to be adjustable provides the flexibility of backlight > dimming granularity vs. maximum backlight frequency." > > > > > On Sat, May 6, 2017 at 1:35 AM, Pandiyan, Dhinakaran > > wrote: > > On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat > > wrote: > > > Read desired PWM frequency from panel vbt and calculate the > > > value for divider in DPCD address 0x724 and 0x728 to match > > > that frequency as close as possible. > > > > > > Signed-off-by: Puthikorn Voravootivat > > > --- > > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 71 > > +++ > > > 1 file changed, 71 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > index fc26fea94fd4..441ad434a82b 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > @@ -113,12 +113,76 @@ > > intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp > > *intel_dp, > > > } > > > } > > > > > > +/* > > > + * 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 void 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, f; > > > + 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; > > > + } > > > + > > > + fxp = DP_EDP_BACKLIGHT_FREQ_BASE / freq; > > > + > > > + /* Use lowest possible value of Pn to try to make F to > > be between 1 and > > > > > > What's the reason to use the lowest possible value of Pn? From > > what I > > understand, choosing a higher value of Pn offers more steps > >
Re: [PATCH 1/2] drm/dp: Add defines for latency in sink
On Mon, Sep 25, 2017 at 10:11 PM, Daniel Vetter wrote: > On Thu, Sep 21, 2017 at 07:42:07AM -0700, Rodrigo Vivi wrote: > > On Wed, Sep 20, 2017 at 02:32:34PM +, vathsala nagaraju wrote: > > > Add defines for dpcd register 2009 (synchronization latency > > > in sink). > > > > > > Cc: Rodrigo Vivi > > > CC: Puthikorn Voravootivat > > > Signed-off-by: Vathsala Nagaraju > > > > I keep forgetting to update my eDP spec 1.4 to this 1.4b... > > Maybe the patch should then make this clear, by annotating it with > /* eDP 1.4b */ That's missing, which isn't all that great really, since it > makes specs hunts like yours necessary. > > It's actually in eDP 1.4 spec, table 5-6 page 86 > Please fix up before applying. > -Daniel > > > > > > Reviewed-by: Rodrigo Vivi > > > > > > > > > --- > > > include/drm/drm_dp_helper.h | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > > > index 11c39f1..846004e6 100644 > > > --- a/include/drm/drm_dp_helper.h > > > +++ b/include/drm/drm_dp_helper.h > > > @@ -735,6 +735,9 @@ > > > # define DP_PSR_SINK_INTERNAL_ERROR 7 > > > # define DP_PSR_SINK_STATE_MASK 0x07 > > > > > > +#define DP_SINK_SYNCHRONIZATION_LATENCY0x2009 > > > +# define DP_MAX_RESYNC_FRAME_CNT_MASK 0xf > > > + > > > #define DP_RECEIVER_ALPM_STATUS0x200b /* eDP 1.4 */ > > > # define DP_ALPM_LOCK_TIMEOUT_ERROR(1 << 0) > > > > > > -- > > > 1.9.1 > > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/dp: Add defines for latency in sink
On Tue, Sep 26, 2017 at 10:37 AM, Puthikorn Voravootivat < put...@chromium.org> wrote: > > > On Mon, Sep 25, 2017 at 10:11 PM, Daniel Vetter wrote: > >> On Thu, Sep 21, 2017 at 07:42:07AM -0700, Rodrigo Vivi wrote: >> > On Wed, Sep 20, 2017 at 02:32:34PM +, vathsala nagaraju wrote: >> > > Add defines for dpcd register 2009 (synchronization latency >> > > in sink). >> > > >> > > Cc: Rodrigo Vivi >> > > CC: Puthikorn Voravootivat >> > > Signed-off-by: Vathsala Nagaraju >> > >> > I keep forgetting to update my eDP spec 1.4 to this 1.4b... >> >> Maybe the patch should then make this clear, by annotating it with >> /* eDP 1.4b */ That's missing, which isn't all that great really, since it >> makes specs hunts like yours necessary. >> >> It's actually in eDP 1.4 spec, table 5-6 page 86 > Copy and paste the wrong one. 0x2009 is actually in eDP 1.4 spec, Table 6-7: DPCD – Sink Device PSR Status Field page 124 > > >> Please fix up before applying. >> -Daniel >> > >> > >> > Reviewed-by: Rodrigo Vivi >> > >> > >> > >> > > --- >> > > include/drm/drm_dp_helper.h | 3 +++ >> > > 1 file changed, 3 insertions(+) >> > > >> > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> > > index 11c39f1..846004e6 100644 >> > > --- a/include/drm/drm_dp_helper.h >> > > +++ b/include/drm/drm_dp_helper.h >> > > @@ -735,6 +735,9 @@ >> > > # define DP_PSR_SINK_INTERNAL_ERROR 7 >> > > # define DP_PSR_SINK_STATE_MASK 0x07 >> > > >> > > +#define DP_SINK_SYNCHRONIZATION_LATENCY0x2009 >> > > +# define DP_MAX_RESYNC_FRAME_CNT_MASK 0xf >> > > + >> > > #define DP_RECEIVER_ALPM_STATUS0x200b /* eDP 1.4 */ >> > > # define DP_ALPM_LOCK_TIMEOUT_ERROR(1 << 0) >> > > >> > > -- >> > > 1.9.1 >> > > >> > ___ >> > dri-devel mailing list >> > dri-devel@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> http://blog.ffwll.ch >> > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 0/3] Enhancement to intel_dp_aux_backlight driver
This patch set contain 3 patches which are already reviewed by DK. 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: v12: - Rebase 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(-) -- 2.13.1.611.g7e3b11ae1-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 1/3] drm/i915: Set PWM divider to match desired frequency in vbt
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. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- 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 228ca06d9f0b..d2830ba3162e 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -98,13 +98,87 @@ intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32 lev } } +/* + * 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(const struct intel_crtc_state *crtc_state, const struct drm_connector_state *conn_state) { struct intel_connector *connector = to_intel_connector(conn_state->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 +187,15 @@ static void intel_
[PATCH v12 3/3] drm/i915: Add option to support dynamic backlight via DPCD
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 Reviewed-by: Dhinakaran Pandiyan --- 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 5b5ab15d191f..88b9d3e6713a 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 0d6cf9138dc4..057e203e6bda 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 fea161727c6e..b25cd88fc1c5 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -173,6 +173,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(const struct intel_crtc_state *crtc_state, const struct drm_connector_state *conn_state) { @@ -208,6 +226,14 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st 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) { -- 2.13.1.611.g7e3b11ae1-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 2/3] drm/i915: Add heuristic to determine better way to adjust brightness
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 Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- 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 14e2c2e57f96..5b5ab15d191f 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 febbfdbd30bd..0d6cf9138dc4 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -53,6 +53,7 @@ func(int, edp_vswing); \ func(int, reset); \ 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 d2830ba3162e..fea161727c6e 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -251,15 +251,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 */ +
[PATCH v6 8/9] drm: Add definition for eDP backlight frequency
This patch adds the following definition - Bit mask for EDP_PWMGEN_BIT_COUNT and min/max cap register which only use bit 0:4 - Base frequency (27 MHz) for backlight PWM frequency generator. Signed-off-by: Puthikorn Voravootivat --- include/drm/drm_dp_helper.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index c0bd0d7651a9..810b7d5d9f2b 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -572,10 +572,12 @@ #define DP_EDP_PWMGEN_BIT_COUNT 0x724 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN 0x725 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX 0x726 +# define DP_EDP_PWMGEN_BIT_COUNT_MASK (0x1f << 0) #define DP_EDP_BACKLIGHT_CONTROL_STATUS 0x727 #define DP_EDP_BACKLIGHT_FREQ_SET 0x728 +# define DP_EDP_BACKLIGHT_FREQ_BASE 2700 #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB 0x72a #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID 0x72b -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 6/9] drm/i915: Support dynamic backlight via DPCD register
This patch enables dynamic backlight by default 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 --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 39 ++- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 5ef3ade7c40e..7d323af96636 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -97,10 +97,27 @@ 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 new_dpcd_buf = 0; uint8_t edp_backlight_mode = 0; if (drm_dp_dpcd_readb(&intel_dp->aux, @@ -110,18 +127,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 +144,19 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) break; } + if (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); } -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 0/9] Enhancement to intel_dp_aux_backlight driver
This patch set contain 9 patches. - First five patches fix bug in the driver and allow choosing which way to adjust brightness if both PWM pin and AUX are supported - Next patch adds enable DBC by default - Next patch makes the driver restore last brightness level after turning display off and on. - Last two patches set the PWM freqency to match data in panel vbt. Change log: 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 (9): drm/i915: Fix cap check for intel_dp_aux_backlight driver drm/i915: Correctly enable backlight brightness adjustment via DPCD drm/i915: Drop AUX backlight enable check for backlight control drm/i915: Allow choosing how to adjust brightness if both supported drm/i915: Set backlight mode before enable backlight drm/i915: Support dynamic backlight via DPCD register drm/i915: Restore brightness level in aux backlight driver drm: Add definition for eDP backlight frequency drm/i915: Set PWM divider to match desired frequency in vbt drivers/gpu/drm/i915/i915_params.c| 8 +- drivers/gpu/drm/i915/i915_params.h| 2 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 171 -- include/drm/drm_dp_helper.h | 2 + 4 files changed, 167 insertions(+), 16 deletions(-) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 5/9] drm/i915: Set backlight mode before enable backlight
We should set backlight mode register before set register to enable the backlight. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index e82f7cb9a7af..5ef3ade7c40e 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -103,8 +103,6 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) uint8_t dpcd_buf = 0; uint8_t edp_backlight_mode = 0; - set_aux_backlight_enable(intel_dp, true); - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", @@ -131,6 +129,8 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) default: break; } + + set_aux_backlight_enable(intel_dp, true); } static void intel_dp_aux_disable_backlight(struct intel_connector *connector) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 4/9] drm/i915: Allow choosing how to adjust brightness if both supported
Add option to allow choosing how to adjust brightness if panel supports both PWM pin and AUX channel. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/i915_params.c| 8 +--- drivers/gpu/drm/i915/i915_params.h| 2 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 23 ++- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index b6a7e363d076..13cf3f1572ab 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,11 @@ 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(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:disable (default), 0:Use PWM pin if both supported, " + "1:Use DPCD if both 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..ac02efce6e22 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -66,7 +66,7 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \ - func(bool, enable_dpcd_backlight); \ + func(int, 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 c22712762957..e82f7cb9a7af 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -167,21 +167,34 @@ 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[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_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; } +static bool +intel_dp_pwm_pin_display_control_capable(struct intel_connector *connector) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); + + /* Check the eDP Display control capabilities registers to determine if +* the panel can support backlight control via BL_PWM_DIM eDP pin +*/ + return intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP; +} + int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) { struct intel_panel *panel = &intel_connector->panel; - if (!i915.enable_dpcd_backlight) + if (i915.enable_dpcd_backlight == -1) + return -ENODEV; + + if (i915.enable_dpcd_backlight == 0 && + intel_dp_pwm_pin_display_control_capable(intel_connector)) return -ENODEV; if (!intel_dp_aux_display_control_capable(intel_connector)) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 7/9] drm/i915: Restore brightness level in aux backlight driver
Some panel will default to zero brightness when turning the panel off and on again. This patch restores last brightness level back when panel is turning back on. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 7d323af96636..fc26fea94fd4 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -158,6 +158,7 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) } set_aux_backlight_enable(intel_dp, true); + intel_dp_aux_set_backlight(connector, connector->panel.backlight.level); } static void intel_dp_aux_disable_backlight(struct intel_connector *connector) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 3/9] drm/i915: Drop AUX backlight enable check for backlight control
> How is backlight enabled in this case? Using eDP BL_ENABLE pin On Sat, May 6, 2017 at 1:59 AM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat wrote: > > There are some panel that > > (1) does not support display backlight enable via AUX > > (2) support display backlight adjustment via AUX > > (3) support display backlight enable via eDP BL_ENABLE pin > > > > The current driver required that (1) must be support to enable (2). > > This patch drops that requirement. > > > > Signed-off-by: Puthikorn Voravootivat > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index ad8560c5f689..5b83c9737644 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp > *intel_dp, bool enable) > > { > > uint8_t reg_val = 0; > > > > + /* Early return when display use other mechanism to enable > backlight. */ > > + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) > > + return; > > + > > How is backlight enabled in this case? > > -DK > > > if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_ > REGISTER, > > ®_val) < 0) { > > DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", > > @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct > intel_connector *connector) > >* 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > > (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) > { > > DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); > > return true; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 2/9] drm/i915: Correctly enable backlight brightness adjustment via DPCD
intel_dp_aux_enable_backlight() assumed that the register BACKLIGHT_BRIGHTNESS_CONTROL_MODE can only has value 01 (DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET) when initialize. This patch fixed that by handling all cases of that register. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 33 ++- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 341bf2cb0c25..870c03fc0f3a 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -97,15 +97,36 @@ 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; set_aux_backlight_enable(intel_dp, true); - if ((drm_dp_dpcd_readb(&intel_dp->aux, - DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) == 1) && - ((dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) == -DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET)) - drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, - (dpcd_buf | DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD)); + if (drm_dp_dpcd_readb(&intel_dp->aux, + DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { + DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", + DP_EDP_BACKLIGHT_MODE_SET_REGISTER); + return; + } + + 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"); + } + break; + + /* Do nothing when it is already DPCD mode */ + case DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD: + default: + break; + } } static void intel_dp_aux_disable_backlight(struct intel_connector *connector) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 1/9] drm/i915: Fix cap check for intel_dp_aux_backlight driver
intel_dp_aux_backlight driver should check for the DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP before enable the driver. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 6532e226db29..341bf2cb0c25 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -144,6 +144,7 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) */ if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP && (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && + (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) && !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) { DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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 is still within 25% of the desired frequency. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 81 +++ 1 file changed, 81 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index fc26fea94fd4..0549ccb1bb09 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -113,12 +113,86 @@ intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp, } } +/* + * 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 void 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, f, fxp_min, fxp_max, fxp_actual; + 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; + } + + fxp = DP_EDP_BACKLIGHT_FREQ_BASE / 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 +* - Effective frequency is within 25% of desired frequency. +*/ + 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; + } + 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; + } + pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK; + pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK; + + fxp_min = fxp * 3 / 4; + fxp_max = 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; + } + + for (pn = pn_max; pn > pn_min; pn--) { + f = clamp(fxp >> 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; + } + 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; + } +} + 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 new_dpcd_buf = 0; uint8_t edp_backlight_mode = 0; + bool freq_cap; if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { @@ -150,6 +224,10 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) DRM_DEBUG_KMS("Enable dynamic brightness.\n"); } + freq_cap = intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP; + if (freq_cap) + new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; + if (new_dpcd_buf != dpcd_buf) { if (drm_dp_dpcd_writeb(&intel_dp->aux,
[PATCH v6 3/9] drm/i915: Drop AUX backlight enable check for backlight control
There are some panel that (1) does not support display backlight enable via AUX (2) support display backlight adjustment via AUX (3) support display backlight enable via eDP BL_ENABLE pin The current driver required that (1) must be support to enable (2). This patch drops that requirement. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 870c03fc0f3a..c22712762957 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) { uint8_t reg_val = 0; + /* Early return when display use other mechanism to enable backlight. */ + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) + return; + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, ®_val) < 0) { DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) && !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) { -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 2/9] drm/i915: Correctly enable backlight brightness adjustment via DPCD
intel_dp_aux_enable_backlight() assumed that the register BACKLIGHT_BRIGHTNESS_CONTROL_MODE can only has value 01 (DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET) when initialize. This patch fixed that by handling all cases of that register. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 33 ++- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 341bf2cb0c25..870c03fc0f3a 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -97,15 +97,36 @@ 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; set_aux_backlight_enable(intel_dp, true); - if ((drm_dp_dpcd_readb(&intel_dp->aux, - DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) == 1) && - ((dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) == -DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET)) - drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, - (dpcd_buf | DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD)); + if (drm_dp_dpcd_readb(&intel_dp->aux, + DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { + DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", + DP_EDP_BACKLIGHT_MODE_SET_REGISTER); + return; + } + + 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"); + } + break; + + /* Do nothing when it is already DPCD mode */ + case DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD: + default: + break; + } } static void intel_dp_aux_disable_backlight(struct intel_connector *connector) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 7/9] drm/i915: Restore brightness level in aux backlight driver
Some panel will default to zero brightness when turning the panel off and on again. This patch restores last brightness level back when panel is turning back on. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 1c5459bc20ae..0b48851013cc 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -159,6 +159,7 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) } set_aux_backlight_enable(intel_dp, true); + intel_dp_aux_set_backlight(connector, connector->panel.backlight.level); } static void intel_dp_aux_disable_backlight(struct intel_connector *connector) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 1/9] drm/i915: Fix cap check for intel_dp_aux_backlight driver
intel_dp_aux_backlight driver should check for the DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP before enable the driver. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 6532e226db29..341bf2cb0c25 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -144,6 +144,7 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) */ if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP && (intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && + (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) && !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) { DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 5/9] drm/i915: Set backlight mode before enable backlight
We should set backlight mode register before set register to enable the backlight. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 5ee1d90a3263..a72893da78d0 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -103,8 +103,6 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) uint8_t dpcd_buf = 0; uint8_t edp_backlight_mode = 0; - set_aux_backlight_enable(intel_dp, true); - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", @@ -131,6 +129,8 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) default: break; } + + set_aux_backlight_enable(intel_dp, true); } static void intel_dp_aux_disable_backlight(struct intel_connector *connector) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 6/9] drm/i915: Support dynamic backlight via DPCD register
Fair enough. Will add kernel switch in next version. On Wed, May 10, 2017 at 6:26 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Wed, 2017-05-03 at 17:28 -0700, Puthikorn Voravootivat wrote: > > This patch enables dynamic backlight by default for eDP > > panel that supports this feature via DPCD register and > > set minimum / maximum brightness to 0% and 100% of the > > normal brightness. > > > I read the link that you shared last time, should there be a switch for > a feature like this that can affect image quality? Should this be a > decision in the kernel with no provision to turn off/on? > > > -DK > > > > > Signed-off-by: Puthikorn Voravootivat > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 39 > ++- > > 1 file changed, 33 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index 5ef3ade7c40e..7d323af96636 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -97,10 +97,27 @@ 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 new_dpcd_buf = 0; > > uint8_t edp_backlight_mode = 0; > > > > if (drm_dp_dpcd_readb(&intel_dp->aux, > > @@ -110,18 +127,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 +144,19 @@ static void intel_dp_aux_enable_backlight(struct > intel_connector *connector) > > break; > > } > > > > + if (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); > > } > > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 8/9] drm: Add definition for eDP backlight frequency
This patch adds the following definition - Bit mask for EDP_PWMGEN_BIT_COUNT and min/max cap register which only use bit 0:4 - Base frequency (27 MHz) for backlight PWM frequency generator. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- include/drm/drm_dp_helper.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index c0bd0d7651a9..eaa307f6ae8c 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -572,10 +572,12 @@ #define DP_EDP_PWMGEN_BIT_COUNT 0x724 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN 0x725 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX 0x726 +# define DP_EDP_PWMGEN_BIT_COUNT_MASK (0x1f << 0) #define DP_EDP_BACKLIGHT_CONTROL_STATUS 0x727 #define DP_EDP_BACKLIGHT_FREQ_SET 0x728 +# define DP_EDP_BACKLIGHT_FREQ_BASE_KHZ 27000 #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB 0x72a #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID 0x72b -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
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 is still within 25% of the desired frequency. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 81 +++ 1 file changed, 81 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 0b48851013cc..6f10a2f1ab76 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -113,12 +113,86 @@ intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp, } } +/* + * 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 void 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; + } + + fxp = 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 +* - Effective frequency is within 25% of desired frequency. +*/ + 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; + } + 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; + } + pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK; + pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK; + + fxp_min = fxp * 3 / 4; + fxp_max = 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; + } + + for (pn = pn_max; pn > pn_min; pn--) { + f = clamp(fxp >> 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; + } + 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; + } +} + 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 new_dpcd_buf = 0; uint8_t edp_backlight_mode = 0; + bool freq_cap; if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { @@ -151,6 +225,10 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) DRM_DEBUG_KMS("Enable dynamic brightness.\n"); } + freq_cap = intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP; + if (freq_cap) + new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; + if (new_dpcd_buf != dpcd_buf) { if (drm_dp_dpcd_writeb(&intel_dp->aux,
[PATCH v7 0/9] Enhancement to intel_dp_aux_backlight driver
This patch set contain 9 patches. - First five patches fix bug in the driver and allow choosing which way to adjust brightness if both PWM pin and AUX are supported - Next patch adds enable DBC by default - Next patch makes the driver restore last brightness level after turning display off and on. - Last two patches set the PWM freqency to match data in panel vbt. This patch set is reviewed by Dhinakaran in patch #1, 2, 5, 7, 8 Change log: 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 (9): drm/i915: Fix cap check for intel_dp_aux_backlight driver drm/i915: Correctly enable backlight brightness adjustment via DPCD drm/i915: Drop AUX backlight enable check for backlight control drm/i915: Allow choosing how to adjust brightness if both supported drm/i915: Set backlight mode before enable backlight drm/i915: Add option to support dynamic backlight via DPCD drm/i915: Restore brightness level in aux backlight driver drm: Add definition for eDP backlight frequency drm/i915: Set PWM divider to match desired frequency in vbt drivers/gpu/drm/i915/i915_params.c| 13 +- drivers/gpu/drm/i915/i915_params.h| 5 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 173 -- include/drm/drm_dp_helper.h | 2 + 4 files changed, 176 insertions(+), 17 deletions(-) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 6/9] drm/i915: Add option to support dynamic backlight via DPCD
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 --- 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 | 40 +++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 13cf3f1572ab..6eaf660e74da 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); @@ -255,3 +256,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(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 a72893da78d0..1c5459bc20ae 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -97,10 +97,27 @@ 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 new_dpcd_buf = 0; uint8_t edp_backlight_mode = 0; if (drm_dp_dpcd_readb(&intel_dp->aux, @@ -110,18 +127,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 +144,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 m
Re: [Intel-gfx] [PATCH v6 3/9] drm/i915: Drop AUX backlight enable check for backlight control
On Wed, May 10, 2017 at 5:39 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Tue, 2017-05-09 at 16:40 -0700, Puthikorn Voravootivat wrote: > > There are some panel that > > (1) does not support display backlight enable via AUX > > (2) support display backlight adjustment via AUX > > (3) support display backlight enable via eDP BL_ENABLE pin > > > > The current driver required that (1) must be support to enable (2). > > This patch drops that requirement. > > > > Signed-off-by: Puthikorn Voravootivat > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index 870c03fc0f3a..c22712762957 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp > *intel_dp, bool enable) > > { > > uint8_t reg_val = 0; > > > > + /* Early return when display use other mechanism to enable > backlight. */ > > + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) > > + return; > > Won't DP_EDP_BACKLIGHT_AUX_ENABLE_CAP be 1 always? The code below, in > intel_dp_aux_display_control_capable(), makes sure > DP_EDP_BACKLIGHT_PIN_ENABLE_CAP=0. The spec says at least one of these > has to be 1. > > We will drop the DP_EDP_BACKLIGHT_PIN_ENABLE_CAP != 0 check in next patch set. This patch adds check here to prepare for that. > > "BACKLIGHT_AUX_ENABLE_CAPABLE > 1 = Indicates that the Sink device supports display backlight > enable through the BACKLIGHT_ENABLE bit in the > EDP_DISPLAY_CONTROL register (DPCD Address 00720h, bit 0). > Must be set to 1 if the BACKLIGHT_PIN_ENABLE_CAPABLE bit (bit 1) > is cleared to 0." > > -DK > > > + > > if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_ > REGISTER, > > ®_val) < 0) { > > DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", > > @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct > intel_connector *connector) > >* 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > > (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) > && > > !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || > > (intel_dp->edp_dpcd[2] & > > DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) > { > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 4/9] drm/i915: Allow choosing how to adjust brightness if both supported
Add option to allow choosing how to adjust brightness if panel supports both PWM pin and AUX channel. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/i915_params.c| 8 +--- drivers/gpu/drm/i915/i915_params.h| 2 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 24 +++- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index b6a7e363d076..13cf3f1572ab 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,11 @@ 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(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:disable (default), 0:Use PWM pin if both supported, " + "1:Use DPCD if both 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..ac02efce6e22 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -66,7 +66,7 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \ - func(bool, enable_dpcd_backlight); \ + func(int, 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 c22712762957..5ee1d90a3263 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -167,21 +167,35 @@ 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[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_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; } +static bool +intel_dp_pwm_pin_display_control_capable(struct intel_connector *connector) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); + + /* Check the eDP Display control capabilities registers to determine if +* the panel can support backlight control via BL_PWM_DIM eDP pin +*/ + return (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && + (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP); +} + int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) { struct intel_panel *panel = &intel_connector->panel; - if (!i915.enable_dpcd_backlight) + if (i915.enable_dpcd_backlight == -1) + return -ENODEV; + + if (i915.enable_dpcd_backlight == 0 && + intel_dp_pwm_pin_display_control_capable(intel_connector)) return -ENODEV; if (!intel_dp_aux_display_control_capable(intel_connector)) -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 3/9] drm/i915: Drop AUX backlight enable check for backlight control
There are some panel that (1) does not support display backlight enable via AUX (2) support display backlight adjustment via AUX (3) support display backlight enable via eDP BL_ENABLE pin The current driver required that (1) must be support to enable (2). This patch drops that requirement. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index 870c03fc0f3a..c22712762957 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) { uint8_t reg_val = 0; + /* Early return when display use other mechanism to enable backlight. */ + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) + return; + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, ®_val) < 0) { DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) && !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) { -- 2.13.0.rc2.291.g57267f2277-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 3/9] drm/i915: Drop AUX backlight enable check for backlight control
On Fri, May 12, 2017 at 6:14 AM, Jani Nikula wrote: > On Fri, 12 May 2017, "Pandiyan, Dhinakaran" > wrote: > > On Thu, 2017-05-11 at 16:02 -0700, Puthikorn Voravootivat wrote: > >> There are some panel that > >> (1) does not support display backlight enable via AUX > >> (2) support display backlight adjustment via AUX > >> (3) support display backlight enable via eDP BL_ENABLE pin > >> > >> The current driver required that (1) must be support to enable (2). > >> This patch drops that requirement. > >> > > > > You sent this version before I finished my follow-up questions, copying > > the conversation here for context. > > Puthikorn, please don't send new versions before the review is > addressed. > Sorry I thought I was explained it clear enough. > > Pushed patches 1, 2, 5, and 7. Thanks for the patches and review. > > BR, > Jani. > > > DK: Won't DP_EDP_BACKLIGHT_AUX_ENABLE_CAP be 1 always? The code below, > > in > > intel_dp_aux_display_control_capable(), makes sure > > DP_EDP_BACKLIGHT_PIN_ENABLE_CAP=0. The spec says at least one of these > > has to be 1. > > > > Puthikorn: We will drop the DP_EDP_BACKLIGHT_PIN_ENABLE_CAP != 0 check > > in next patch set. > > This patch adds check here to prepare for that. > > > > > > 1) So, this patch does not really fix what the commit message claims > > because it is dependent on the following patch. Does it make sense to > > remove this check in this patch? That way, this patch by itself is the > > fix that the commit message says. > > > > - !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) > > > Sure. I can remove this here and adds it in next patch instead. > > > 2) If a panel supports backlight enable via AUX and BL_ENABLE pin, this > > patch (along with the next) enables backlight twice, doesn't it? > > _intel_edp_backlight_on(intel_dp) in intel_dp.c is called > > unconditionally after intel_dp_aux_enable_backlight(). I don't know how > > likely this configuration is or if it's alright to enable via both AUX > > and BL_ENABLE pin. > > > The eDP spec did not mention this case explicitly. But it should not hurt to enable backlight twice as we want the backlight to be enabled anyway. > > > > > > > >> Signed-off-by: Puthikorn Voravootivat > >> --- > >> drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 5 - > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > >> index 870c03fc0f3a..c22712762957 100644 > >> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > >> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > >> @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct > intel_dp *intel_dp, bool enable) > >> { > >> uint8_t reg_val = 0; > >> > >> + /* Early return when display use other mechanism to enable > backlight. */ > >> +if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) > >> +return; > >> + > >> if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_ > REGISTER, > >>®_val) < 0) { > >> DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", > >> @@ -164,7 +168,6 @@ intel_dp_aux_display_control_capable(struct > intel_connector *connector) > >> * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > >> (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) > && > >> !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || > >>(intel_dp->edp_dpcd[2] & > >> DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) > { > > > > -- > Jani Nikula, Intel Open Source Technology Center > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
On Fri, May 12, 2017 at 5:12 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Thu, 2017-05-11 at 16:02 -0700, Puthikorn Voravootivat wrote: > > 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 is still within 25% > > of the desired frequency. > > I read a few eDP panel data sheets, the PWM frequencies all start from > ~200Hz. If the VBT chooses this lowest value to allow for more > brightness control, and then this patch lowers the value by another 25%, > we'll end up below the panel allowed PWM frequency. > > In fact, one of the systems I checked had PWM frequency as 200Hz in VBT > and the panel datasheet also had PWM frequency range starting from > 200Hz. Have you considered this case? > > The spec said "A given LCD panel typically has a limited range of backlight frequency capability. To limit the programmable frequency range, limitations are placed on the allowable total divider ratio with the Sink device" So I think it should be auto cap to 200Hz in this case. > -DK > > > > Signed-off-by: Puthikorn Voravootivat > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 81 > +++ > > 1 file changed, 81 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index 0b48851013cc..6f10a2f1ab76 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -113,12 +113,86 @@ intel_dp_aux_set_dynamic_backlight_percent(struct > intel_dp *intel_dp, > > } > > } > > > > +/* > > + * 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 void 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; > > + } > > + > > + fxp = 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 > > + * - Effective frequency is within 25% of desired frequency. > > + */ > > + 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; > > + } > > + 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; > > + } > > + pn_min &= DP_EDP_PWMGEN_BIT_COUNT_MASK; > > + pn_max &= DP_EDP_PWMGEN_BIT_COUNT_MASK; > > + > > + fxp_min = fxp * 3 / 4; > > + fxp_max = fxp * 5 / 4; > > You are allowing fxp between +/- 25% of the actual. This isn't same as > the "Effective frequency is within 25% of desired frequency." right? The > frequency can vary between -20% and +33%. > >
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
On Mon, May 15, 2017 at 4:07 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Fri, 2017-05-12 at 17:31 -0700, Puthikorn Voravootivat wrote: > > > > > > > > On Fri, May 12, 2017 at 5:12 PM, Pandiyan, Dhinakaran > > wrote: > > On Thu, 2017-05-11 at 16:02 -0700, Puthikorn Voravootivat > > wrote: > > > 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 is still within > > 25% > > > of the desired frequency. > > > > I read a few eDP panel data sheets, the PWM frequencies all > > start from > > ~200Hz. If the VBT chooses this lowest value to allow for more > > brightness control, and then this patch lowers the value by > > another 25%, > > we'll end up below the panel allowed PWM frequency. > > > > In fact, one of the systems I checked had PWM frequency as > > 200Hz in VBT > > and the panel datasheet also had PWM frequency range starting > > from > > 200Hz. Have you considered this case? > > > > The spec said "A given LCD panel typically has a limited range of > > backlight frequency capability. > > To limit the programmable frequency range, limitations are placed on > > the allowable total divider ratio with the Sink device" > > So I think it should be auto cap to 200Hz in this case. > > > > -DK > > > > > > Signed-off-by: Puthikorn Voravootivat > > > --- > > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 81 > > +++ > > > 1 file changed, 81 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > index 0b48851013cc..6f10a2f1ab76 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > @@ -113,12 +113,86 @@ > > intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp > > *intel_dp, > > > } > > > } > > > > > > +/* > > > + * 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 void 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; > > > + } > > > + > > > + fxp = KHz(DP_EDP_BACKLIGHT_FREQ_BASE_KHZ) / freq; > > > + > > > + /* Use highest possible value of Pn for more > &
[PATCH v8 5/5] drm/i915: Set PWM divider to match desired frequency in vbt
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. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 82 +++ 1 file changed, 82 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index c0eeb8fc2013..a01cbf3db1c2 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -116,12 +116,87 @@ intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp, } } +/* + * 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 void 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; + } + + 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; + } + 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; + } + 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; + } + + 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; + } + 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; + } +} + 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 new_dpcd_buf = 0; uint8_t edp_backlight_mode = 0; + bool freq_cap; if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { @@ -154,6 +229,10 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) DRM_DEBUG_KMS("Enable dynamic brightness.\n"); } + freq_cap = intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP; + if (freq_cap) + new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
On Tue, May 16, 2017 at 2:21 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Tue, 2017-05-16 at 13:56 -0700, Puthikorn Voravootivat wrote: > > > > > > On Tue, May 16, 2017 at 1:29 PM, Pandiyan, Dhinakaran > > wrote: > > On Tue, 2017-05-16 at 11:07 -0700, Puthikorn Voravootivat > > wrote: > > > > > > > > > On Mon, May 15, 2017 at 11:21 PM, Pandiyan, Dhinakaran > > > wrote: > > > On Mon, 2017-05-15 at 17:43 -0700, Puthikorn > > Voravootivat > > > wrote: > > > > > > > > > > > > On Mon, May 15, 2017 at 4:07 PM, Pandiyan, > > Dhinakaran > > > > wrote: > > > > On Fri, 2017-05-12 at 17:31 -0700, > > Puthikorn > > > Voravootivat > > > > wrote: > > > > > > > > > > > > > > > > > > > > On Fri, May 12, 2017 at 5:12 PM, > > Pandiyan, > > > Dhinakaran > > > > > wrote: > > > > > On Thu, 2017-05-11 at 16:02 > > -0700, > > > Puthikorn > > > > Voravootivat > > > > > wrote: > > > > > > 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 is still > > > > within > > > > > 25% > > > > > > of the desired frequency. > > > > > > > > > > I read a few eDP panel data > > sheets, the > > > PWM > > > > frequencies all > > > > > start from > > > > > ~200Hz. If the VBT chooses this > > lowest > > > value to > > > > allow for more > > > > > brightness control, and then > > this patch > > > lowers the > > > > value by > > > > > another 25%, > > > > > we'll end up below the panel > > allowed PWM > > > frequency. > > > > > > > > > > In fact, one of the systems I > > checked had > > > PWM > > > > frequency as > > > > > 200Hz in VBT > > > > > and the panel datasheet also had > > PWM > > > frequency range > > > > starting > > > > > from > > > > > 200Hz. Have you considered this > > case? > > > > > > > > > > The spec said "A given LCD panel > > typically has a > > > limited > > > > range of > > > > > backlight frequency capability. > > > > > To limit the programmable frequency > > range, > > > limitations are > >
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
On Tue, May 16, 2017 at 1:29 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Tue, 2017-05-16 at 11:07 -0700, Puthikorn Voravootivat wrote: > > > > > > On Mon, May 15, 2017 at 11:21 PM, Pandiyan, Dhinakaran > > wrote: > > On Mon, 2017-05-15 at 17:43 -0700, Puthikorn Voravootivat > > wrote: > > > > > > > > > On Mon, May 15, 2017 at 4:07 PM, Pandiyan, Dhinakaran > > > wrote: > > > On Fri, 2017-05-12 at 17:31 -0700, Puthikorn > > Voravootivat > > > wrote: > > > > > > > > > > > > > > > > On Fri, May 12, 2017 at 5:12 PM, Pandiyan, > > Dhinakaran > > > > wrote: > > > > On Thu, 2017-05-11 at 16:02 -0700, > > Puthikorn > > > Voravootivat > > > > wrote: > > > > > 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 is still > > > within > > > > 25% > > > > > of the desired frequency. > > > > > > > > I read a few eDP panel data sheets, the > > PWM > > > frequencies all > > > > start from > > > > ~200Hz. If the VBT chooses this lowest > > value to > > > allow for more > > > > brightness control, and then this patch > > lowers the > > > value by > > > > another 25%, > > > > we'll end up below the panel allowed PWM > > frequency. > > > > > > > > In fact, one of the systems I checked had > > PWM > > > frequency as > > > > 200Hz in VBT > > > > and the panel datasheet also had PWM > > frequency range > > > starting > > > > from > > > > 200Hz. Have you considered this case? > > > > > > > > The spec said "A given LCD panel typically has a > > limited > > > range of > > > > backlight frequency capability. > > > > To limit the programmable frequency range, > > limitations are > > > placed on > > > > the allowable total divider ratio with the Sink > > device" > > > > So I think it should be auto cap to 200Hz in this > > case. > > > > > > > > -DK > > > > > > > > > > Signed-off-by: Puthikorn Voravootivat > > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | > > > 81 > > > > +++ > > > > > 1 file changed, 81 insertions(+) > > > > > > > > > > diff --git > > > a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > > > > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > > > index 0b48851013cc..6f10a2f1ab76 100644 > > > > > --- > > > a/drivers/gpu/drm/i915/in
[PATCH v8 1/5] drm/i915: Drop AUX backlight enable check for backlight control
There are some panel that (1) does not support display backlight enable via AUX (2) support display backlight adjustment via AUX (3) support display backlight enable via eDP BL_ENABLE pin The current driver required that (1) must be support to enable (2). This patch drops that requirement. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index b87c5a381d6a..d32c06583e0b 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -165,7 +165,6 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) && !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) { -- 2.13.0.303.g4ebf302169-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v7 9/9] drm/i915: Set PWM divider to match desired frequency in vbt
On Mon, May 15, 2017 at 11:21 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Mon, 2017-05-15 at 17:43 -0700, Puthikorn Voravootivat wrote: > > > > > > On Mon, May 15, 2017 at 4:07 PM, Pandiyan, Dhinakaran > > wrote: > > On Fri, 2017-05-12 at 17:31 -0700, Puthikorn Voravootivat > > wrote: > > > > > > > > > > > > On Fri, May 12, 2017 at 5:12 PM, Pandiyan, Dhinakaran > > > wrote: > > > On Thu, 2017-05-11 at 16:02 -0700, Puthikorn > > Voravootivat > > > wrote: > > > > 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 is still > > within > > > 25% > > > > of the desired frequency. > > > > > > I read a few eDP panel data sheets, the PWM > > frequencies all > > > start from > > > ~200Hz. If the VBT chooses this lowest value to > > allow for more > > > brightness control, and then this patch lowers the > > value by > > > another 25%, > > > we'll end up below the panel allowed PWM frequency. > > > > > > In fact, one of the systems I checked had PWM > > frequency as > > > 200Hz in VBT > > > and the panel datasheet also had PWM frequency range > > starting > > > from > > > 200Hz. Have you considered this case? > > > > > > The spec said "A given LCD panel typically has a limited > > range of > > > backlight frequency capability. > > > To limit the programmable frequency range, limitations are > > placed on > > > the allowable total divider ratio with the Sink device" > > > So I think it should be auto cap to 200Hz in this case. > > > > > > -DK > > > > > > > > Signed-off-by: Puthikorn Voravootivat > > > > > > --- > > > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | > > 81 > > > +++ > > > > 1 file changed, 81 insertions(+) > > > > > > > > diff --git > > a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > > index 0b48851013cc..6f10a2f1ab76 100644 > > > > --- > > a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > > +++ > > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > > @@ -113,12 +113,86 @@ > > > intel_dp_aux_set_dynamic_backlight_percent(struct > > intel_dp > > > *intel_dp, > > > > } > > > > } > > > > > > > > +/* > > > > + * 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) > > > > + */ > >
[PATCH v8 4/5] drm: Add definition for eDP backlight frequency
This patch adds the following definition - Bit mask for EDP_PWMGEN_BIT_COUNT and min/max cap register which only use bit 0:4 - Base frequency (27 MHz) for backlight PWM frequency generator. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- include/drm/drm_dp_helper.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index c0bd0d7651a9..eaa307f6ae8c 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -572,10 +572,12 @@ #define DP_EDP_PWMGEN_BIT_COUNT 0x724 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN 0x725 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX 0x726 +# define DP_EDP_PWMGEN_BIT_COUNT_MASK (0x1f << 0) #define DP_EDP_BACKLIGHT_CONTROL_STATUS 0x727 #define DP_EDP_BACKLIGHT_FREQ_SET 0x728 +# define DP_EDP_BACKLIGHT_FREQ_BASE_KHZ 27000 #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB 0x72a #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID 0x72b -- 2.13.0.303.g4ebf302169-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 0/5] Enhancement to intel_dp_aux_backlight driver
This patch set contain 5 patches. Another 4 patches in previous version was already merged. - First two patches allow choosing which way to adjust brightness if both PWM pin and AUX are supported - Next patch adds support for dynamic brightness. - Last two patches set the PWM freqency to match data in panel vbt. Change log: 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 (5): drm/i915: Drop AUX backlight enable check for backlight control drm/i915: Allow choosing how to adjust brightness if both supported drm/i915: Add option to support dynamic backlight via DPCD drm: Add definition for eDP backlight frequency drm/i915: Set PWM divider to match desired frequency in vbt drivers/gpu/drm/i915/i915_params.c| 13 ++- drivers/gpu/drm/i915/i915_params.h| 5 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 154 -- include/drm/drm_dp_helper.h | 2 + 4 files changed, 157 insertions(+), 17 deletions(-) -- 2.13.0.303.g4ebf302169-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 1/5] drm/i915: Drop AUX backlight enable check for backlight control
On Wed, May 17, 2017 at 1:09 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > > ____ > From: Puthikorn Voravootivat [put...@google.com] on behalf of Puthikorn > Voravootivat [put...@chromium.org] > Sent: Tuesday, May 16, 2017 5:33 PM > To: intel-...@lists.freedesktop.org; Pandiyan, Dhinakaran > Cc: dri-devel@lists.freedesktop.org; Jani Nikula; Navare, Manasi D; > Stephane Marchesin; Puthikorn Voravootivat > Subject: [PATCH v8 1/5] drm/i915: Drop AUX backlight enable check for > backlight control > > > There are some panel that > (1) does not support display backlight enable via AUX > (2) support display backlight adjustment via AUX > (3) support display backlight enable via eDP BL_ENABLE pin > > The current driver required that (1) must be support to enable (2). > This patch drops that requirement. > > Signed-off-by: Puthikorn Voravootivat > --- > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > index b87c5a381d6a..d32c06583e0b 100644 > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > @@ -165,7 +165,6 @@ intel_dp_aux_display_control_capable(struct > intel_connector *connector) > * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) && > !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || > (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) { > -- > 2.13.0.303.g4ebf302169-goog > > > ^ still has the problem I mentioned last time. How about this? > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > index b87c5a3..7072bcf 100644 > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp > *intel_dp, bool enable) > { > uint8_t reg_val = 0; > > + /* Early return when display use other mechanism to enable > backlight. */ > + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) > + return; > + > if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_ > REGISTER, > ®_val) < 0) { > DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", > @@ -165,10 +169,8 @@ intel_dp_aux_display_control_capable(struct > intel_connector *connector) > * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && > (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) > && > - !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || > - (intel_dp->edp_dpcd[2] & > DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) > { > + !(intel_dp->edp_dpcd[2] & > DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) > { > DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); > return true; > } > > > This works too. I probably misunderstood your comment as I move the code to next patch in the set. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 3/5] drm/i915: Add option to support dynamic backlight via DPCD
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 --- 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 | 40 +++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 13cf3f1572ab..6eaf660e74da 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); @@ -255,3 +256,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(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 16ba1924308d..c0eeb8fc2013 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -100,10 +100,27 @@ 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 new_dpcd_buf = 0; uint8_t edp_backlight_mode = 0; if (drm_dp_dpcd_readb(&intel_dp->aux, @@ -113,18 +130,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 +147,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"); +
[PATCH v8 2/5] drm/i915: Allow choosing how to adjust brightness if both supported
Add option to allow choosing how to adjust brightness if panel supports both PWM pin and AUX channel. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/i915_params.c| 8 --- drivers/gpu/drm/i915/i915_params.h| 2 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 31 ++- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index b6a7e363d076..13cf3f1572ab 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,11 @@ 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(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:disable (default), 0:Use PWM pin if both supported, " + "1:Use DPCD if both 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..ac02efce6e22 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -66,7 +66,7 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \ - func(bool, enable_dpcd_backlight); \ + func(int, 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 d32c06583e0b..16ba1924308d 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) { uint8_t reg_val = 0; + /* Early return when display use other mechanism to enable backlight. */ + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) + return; + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, ®_val) < 0) { DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", @@ -39,6 +43,9 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) else reg_val &= ~(DP_EDP_BACKLIGHT_ENABLE); + /* TODO: If the panel also support enabling backlight via BL_ENABLE pin, +* the backlight will be enabled again in _intel_edp_backlight_on() +*/ if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, reg_val) != 1) { DRM_DEBUG_KMS("Failed to %s aux backlight\n", @@ -164,21 +171,35 @@ 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[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_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; } +static bool +intel_dp_pwm_pin_display_control_capable(struct intel_connector *connector) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); + + /* Check the eDP Display control capabilities registers to determine if +* the panel can support backlight control via BL_PWM_DIM eDP pin +*/ + return (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CA
Re: [Intel-gfx] [PATCH v8 3/5] drm/i915: Add option to support dynamic backlight via DPCD
Hi Dhinakaran, Quick question So what is the update about adding new option in i915_params? Is this patch good to go after fixing the 2 points you mentioned? Thanks On Wed, May 17, 2017 at 1:33 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Tue, 2017-05-16 at 17:34 -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 > > --- > > 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 | 40 > +++ > > 3 files changed, 41 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_params.c > b/drivers/gpu/drm/i915/i915_params.c > > index 13cf3f1572ab..6eaf660e74da 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); > > @@ -255,3 +256,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(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 16ba1924308d..c0eeb8fc2013 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -100,10 +100,27 @@ 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)) { > > Incorrect parentheses placement and return value check. > > > + 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 new_dpcd_buf = 0; > > nit: unnecessary initialization. > > > uint8_t edp_backlight_mode = 0; > > > > if (drm_dp_dpcd_readb(&intel_dp->aux, > > @@ -113,18 +130,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: > > - dpc
[PATCH v9 0/5] Enhancement to intel_dp_aux_backlight driver
This patch set contain 5 patches. Another 4 patches in previous version was already merged in v7. - First two patches allow choosing which way to adjust brightness if both PWM pin and AUX are supported - Next patch adds support for dynamic brightness. - Last two patches set the PWM freqency to match data in panel vbt. Change log: 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 (5): drm/i915: Drop AUX backlight enable check for backlight control drm/i915: Allow choosing how to adjust brightness if both supported drm/i915: Add option to support dynamic backlight via DPCD drm: Add definition for eDP backlight frequency drm/i915: Set PWM divider to match desired frequency in vbt drivers/gpu/drm/i915/i915_params.c| 13 ++- drivers/gpu/drm/i915/i915_params.h| 5 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 156 +++--- include/drm/drm_dp_helper.h | 2 + 4 files changed, 157 insertions(+), 19 deletions(-) -- 2.13.0.219.gdb65acc882-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 3/5] drm/i915: Add option to support dynamic backlight via DPCD
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 --- 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 13cf3f1572ab..6eaf660e74da 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); @@ -255,3 +256,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(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 16ba1924308d..f1b7855a2d2a 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_
[PATCH v9 1/5] drm/i915: Drop AUX backlight enable check for backlight control
There are some panel that (1) does not support display backlight enable via AUX (2) support display backlight adjustment via AUX (3) support display backlight enable via eDP BL_ENABLE pin The current driver required that (1) must be support to enable (2). This patch drops that requirement. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index b87c5a381d6a..a0995c00fc84 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -28,6 +28,10 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) { uint8_t reg_val = 0; + /* Early return when display use other mechanism to enable backlight. */ + if (!(intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP)) + return; + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, ®_val) < 0) { DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", @@ -165,10 +169,8 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) * 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[1] & DP_EDP_BACKLIGHT_AUX_ENABLE_CAP) && (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) && - !((intel_dp->edp_dpcd[1] & DP_EDP_BACKLIGHT_PIN_ENABLE_CAP) || - (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP))) { + !(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) { DRM_DEBUG_KMS("AUX Backlight Control Supported!\n"); return true; } -- 2.13.0.219.gdb65acc882-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 4/5] drm: Add definition for eDP backlight frequency
This patch adds the following definition - Bit mask for EDP_PWMGEN_BIT_COUNT and min/max cap register which only use bit 0:4 - Base frequency (27 MHz) for backlight PWM frequency generator. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- include/drm/drm_dp_helper.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index c0bd0d7651a9..eaa307f6ae8c 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -572,10 +572,12 @@ #define DP_EDP_PWMGEN_BIT_COUNT 0x724 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MIN 0x725 #define DP_EDP_PWMGEN_BIT_COUNT_CAP_MAX 0x726 +# define DP_EDP_PWMGEN_BIT_COUNT_MASK (0x1f << 0) #define DP_EDP_BACKLIGHT_CONTROL_STATUS 0x727 #define DP_EDP_BACKLIGHT_FREQ_SET 0x728 +# define DP_EDP_BACKLIGHT_FREQ_BASE_KHZ 27000 #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MSB 0x72a #define DP_EDP_BACKLIGHT_FREQ_CAP_MIN_MID 0x72b -- 2.13.0.219.gdb65acc882-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 2/5] drm/i915: Allow choosing how to adjust brightness if both supported
Add option to allow choosing how to adjust brightness if panel supports both PWM pin and AUX channel. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/i915_params.c| 8 +--- drivers/gpu/drm/i915/i915_params.h| 2 +- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 26 ++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index b6a7e363d076..13cf3f1572ab 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,11 @@ 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(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:disable (default), 0:Use PWM pin if both supported, " + "1:Use DPCD if both 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..ac02efce6e22 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -66,7 +66,7 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \ - func(bool, enable_dpcd_backlight); \ + func(int, 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 a0995c00fc84..16ba1924308d 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -43,6 +43,9 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) else reg_val &= ~(DP_EDP_BACKLIGHT_ENABLE); + /* TODO: If the panel also support enabling backlight via BL_ENABLE pin, +* the backlight will be enabled again in _intel_edp_backlight_on() +*/ if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, reg_val) != 1) { DRM_DEBUG_KMS("Failed to %s aux backlight\n", @@ -168,20 +171,35 @@ 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; } +static bool +intel_dp_pwm_pin_display_control_capable(struct intel_connector *connector) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); + + /* Check the eDP Display control capabilities registers to determine if +* the panel can support backlight control via BL_PWM_DIM eDP pin +*/ + return (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP) && + (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP); +} + int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) { struct intel_panel *panel = &intel_connector->panel; - if (!i915.enable_dpcd_backlight) + if (i915.enable_dpcd_backlight == -1) + return -ENODEV; + + if (i915.enable_dpcd_backlight == 0 && + intel_dp_pwm_pin_display_control_capable(intel_connector)) return -ENODEV; if (!intel_dp_aux_display_control_capable(intel_connector)) -- 2.13.0.219.gdb65acc882-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 5/5] drm/i915: Set PWM divider to match desired frequency in vbt
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. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 82 +++ 1 file changed, 82 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index f1b7855a2d2a..b7cd44550127 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -116,10 +116,85 @@ intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp, } } +/* + * 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 void 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; + } + + 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; + } + 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; + } + 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; + } + + 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; + } + 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; + } +} + 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, new_dpcd_buf, edp_backlight_mode; + bool freq_cap; if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { @@ -152,6 +227,10 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) DRM_DEBUG_KMS("Enable dynamic brightness.\n"); } + freq_cap = intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP; + if (freq_cap) + new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE; + if (new_dpcd_buf != dpcd_buf) {
[PATCH v10 3/3] drm/i915: Set PWM divider to match desired frequency in vbt
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. Signed-off-by: Puthikorn Voravootivat --- drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 79 +++ 1 file changed, 79 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c index f55af41ce3bd..a8d485a29f29 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -116,6 +116,81 @@ intel_dp_aux_set_dynamic_backlight_percent(struct intel_dp *intel_dp, } } +/* + * 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 int 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 -1; + } + + 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 -EIO; + } + 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 -EIO; + } + 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 -ERANGE; + } + + 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 -EIO; + } + 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 -EIO; + } + return 0; +} + static void intel_dp_aux_enable_backlight(struct intel_connector *connector) { struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base); @@ -152,6 +227,10 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector) DRM_DEBUG_KMS("Enable dynamic brightness.\n"); } + 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) { -- 2.13.0.219.gdb65acc882-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 0/3] Enhancement to intel_dp_aux_backlight driver
This patch set contain 3 patches. Another 6 patches in previous version was already merged in v7 and v9. - First patch adds heuristic to determine whether we should use AUX or PWM pin to adjust panel backlight brightness. - Next patch adds support for dynamic brightness. - Last patch sets the PWM freqency to match data in panel vbt. Change log: 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: Add heuristic to determine better way to adjust brightness drm/i915: Add option to support dynamic backlight via DPCD drm/i915: Set PWM divider to match desired frequency in vbt 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(-) -- 2.13.0.219.gdb65acc882-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 1/3] drm/i915: Add heuristic to determine better way to adjust brightness
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 Signed-off-by: Puthikorn Voravootivat --- 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 | 64 +-- 3 files changed, 66 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..ac02efce6e22 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -66,7 +66,7 @@ func(bool, verbose_state_checks); \ func(bool, nuclear_pageflip); \ func(bool, enable_dp_mst); \ - func(bool, enable_dpcd_backlight); \ + func(int, 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 a0995c00fc84..c89aae804659 100644 --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c @@ -43,6 +43,9 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) else reg_val &= ~(DP_EDP_BACKLIGHT_ENABLE); + /* TODO: If the panel also support enabling backlight via BL_ENABLE pin, +* the backlight will be enabled again in _intel_edp_backlight_on() +*/ if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER, reg_val) != 1) { DRM_DEBUG_KMS("Failed to %s aux backlight\n", @@ -168,15 +171,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
[PATCH v10 2/3] drm/i915: Add option to support dynamic backlight via DPCD
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 --- 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"); + } + + if (new_dpcd_buf != dpcd_buf) { + if (drm_dp_dpcd_writeb(&intel_dp->aux, + DP_EDP_
Re: [PATCH v9 5/5] drm/i915: Set PWM divider to match desired frequency in vbt
Good catch. It will use default frequency in this case. But it is better to not set DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP in this case. I will add return value to intel_dp_aux_set_pwm_freq() and set DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP based on that. On Fri, May 26, 2017 at 5:49 AM, Jani Nikula wrote: > On Tue, 23 May 2017, Puthikorn Voravootivat wrote: > > 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. > > IIUC this patch doesn't have a dependency on the more contentious > patches 2/5 and 3/5. This should probably be merged before them. > > I share DK's concern about doing a bunch of reads and writes and > calculations every time the backlight's enabled. But I guess that could > be optimized later. > > I haven't had time to check the changed algorithm here, but in the mean > time one comment below. > > BR, > Jani. > > > > > > Signed-off-by: Puthikorn Voravootivat > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 82 > +++ > > 1 file changed, 82 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index f1b7855a2d2a..b7cd44550127 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -116,10 +116,85 @@ intel_dp_aux_set_dynamic_backlight_percent(struct > intel_dp *intel_dp, > > } > > } > > > > +/* > > + * 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 void 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; > > + } > > + > > + 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; > > + } > > + 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; > > + } > > + 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; > > + } > > + > > +
Re: [Intel-gfx] [PATCH v10 2/3] drm/i915: Add option to support dynamic backlight via DPCD
On Tue, May 30, 2017 at 9:18 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@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 > > --- > > 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; > > -
Re: [Intel-gfx] [PATCH v10 3/3] drm/i915: Set PWM divider to match desired frequency in vbt
On Tue, May 30, 2017 at 8:40 PM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > The patch looks good overall, it would have been easier to merge if > you'd sent this as the first patch in this version. Some comments > inline. > > > Will re-order to make this the first patch in the next version. > > On Fri, 2017-05-26 at 18:42 -0700, Puthikorn Voravootivat wrote: > > 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. > > > > Signed-off-by: Puthikorn Voravootivat > > --- > > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 79 > +++ > > 1 file changed, 79 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > index f55af41ce3bd..a8d485a29f29 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > @@ -116,6 +116,81 @@ intel_dp_aux_set_dynamic_backlight_percent(struct > intel_dp *intel_dp, > > } > > } > > > > +/* > > + * 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 int 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; > > nit: unnecessary initialization for f? > No. We only set f in for (pn = pn_max; pn >= pn_min; pn--) but compiler wouldn't know that pn_max >= pn_min so f might be uninitialized. > > + 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 -1; > > + } > > + > > + 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 -EIO; > > The error numbers are not propagated outside, so I don't see a real need > for them. bool should suffice. > > Sure. > > > + } > > + 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 -EIO; > > + } > > + 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 -ERANGE; > > + } > > + > > + for (pn = pn_max; pn >= pn_min; pn--) { > >
Re: [Intel-gfx] [PATCH v10 1/3] drm/i915: Add heuristic to determine better way to adjust brightness
On Fri, Jun 2, 2017 at 10:56 AM, Pandiyan, Dhinakaran < dhinakaran.pandi...@intel.com> wrote: > On Fri, 2017-06-02 at 17:42 +, Pandiyan, Dhinakaran wrote: > > Somehow the CC's got removed in my previous reply, adding them back. See > one additional comment below. > > > > On Fri, 2017-05-26 at 18:42 -0700, Puthikorn Voravootivat wrote: > > > 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 > > > > > > Signed-off-by: Puthikorn Voravootivat > > > --- > > > 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 | 64 > +-- > > > 3 files changed, 66 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..ac02efce6e22 100644 > > > --- a/drivers/gpu/drm/i915/i915_params.h > > > +++ b/drivers/gpu/drm/i915/i915_params.h > > > @@ -66,7 +66,7 @@ > > > func(bool, verbose_state_checks); \ > > > func(bool, nuclear_pageflip); \ > > > func(bool, enable_dp_mst); \ > > > - func(bool, enable_dpcd_backlight); \ > > > + func(int, enable_dpcd_backlight); \ > > > Please move this above the bools, see comment in code that's a few lines > above. > > Will do > -DK > > > > 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 a0995c00fc84..c89aae804659 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c > > > @@ -43,6 +43,9 @@ static void set_aux_backlight_enable(struct > intel_dp *intel_dp, bool enable) > > > else > > > reg_val &= ~(DP_EDP_BACKLIGHT_ENABLE); > > > > > > + /* TODO: If the panel also support enabling backlight via > BL_ENABLE pin, > > > +* the backlight will be enabled again in _intel_edp_backlight_on() > > > +*/ > > > > Unrelated hunk, please remove. This should have been included in one of > > the previous patches. > Removed > > > > > if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_DISPLAY_CONTROL_ > REGISTER, > >
[PATCH v11 0/3] Enhancement to intel_dp_aux_backlight driver
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(-) -- 2.13.0.506.g27d5fe0cd-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v10 2/3] drm/i915: Add option to support dynamic backlight via DPCD
On Fri, Jun 2, 2017 at 11:25 AM, Pandiyan, Dhinakaran < dhinakaran.pandi...@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 > > --- > > 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_dp
[PATCH v11 1/3] drm/i915: Set PWM divider to match desired frequency in vbt
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 --- 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_
[PATCH v11 2/3] drm/i915: Add heuristic to determine better way to adjust brightness
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 --- 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 brig
[PATCH v11 3/3] drm/i915: Add option to support dynamic backlight via DPCD
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 --- 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) { -- 2.13.0.506.g27d5fe0cd-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RESEND v11 2/3] drm/i915: Add heuristic to determine better way to adjust brightness
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 Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- 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 */ +
Re: [Intel-gfx] [PATCH v11 3/3] drm/i915: Add option to support dynamic backlight via DPCD
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.pandi...@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 > > --- > > 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"); > > + } > > + }
[PATCH RESEND v11 1/3] drm/i915: Set PWM divider to match desired frequency in vbt
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. Signed-off-by: Puthikorn Voravootivat Reviewed-by: Dhinakaran Pandiyan --- 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_
[PATCH RESEND v11 0/3] Enhancement to intel_dp_aux_backlight driver
This patch set contain 3 patches which are already reviewed by DK. 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(-) -- 2.13.0.506.g27d5fe0cd-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RESEND v11 3/3] drm/i915: Add option to support dynamic backlight via DPCD
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 Reviewed-by: Dhinakaran Pandiyan --- 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) { -- 2.13.0.506.g27d5fe0cd-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RESEND v11 0/3] Enhancement to intel_dp_aux_backlight driver
Friendly ping. Is this patch set good to go now? On Mon, Jun 5, 2017 at 2:56 PM, Puthikorn Voravootivat wrote: > This patch set contain 3 patches which are already reviewed by DK. > 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(-) > > -- > 2.13.0.506.g27d5fe0cd-goog > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] i915: Introduce quirk for shifting eDP brightness.
The Lyude fde7266fb2f6 change is actually based on Chromium change (https://crrev.com/c/1650325) that fixes the brightness for Samsung Galaxy Chromebook. So currently we have 2 examples that use LSB interpretation and 1 that use MSB. On Thu, Sep 17, 2020 at 10:55 AM Kevin Chowski wrote: > > Apologies for being too vague. To be as precise I can be, here is the > specific code delta I tested: https://crrev.com/c/2406616 . To answer > your other question, the code I tested against is indeed including the > fde7266fb2f6 (despite ostensibly being called 5.4 in my commit > message): our current top-of-tree for our 5.4 branch includes the > intel_dp_aux_calc_max_backlight logic. Further, I'll note that change > is exactly the change which breaks my Pixelbook model: prior to the > change, the max_brightness was hard-coded to 0x and the math > worked out that it didn't matter that the hardware cared about the MSB > despite the driver code caring about the LSB. > > To answer Ville's question: the fde7266fb2f6 change which fixes one > laptop (I believe Thinkpad X1 extreme Gen 2, from some bug reports I > dug up) and breaks another (Pixelbook); so unfortunately I believe we > need a quirk at least for some laptop. Reading through the copy of the > datasheet I have, it wasn't clear to me which was the correct > interpretation. I'm cc'ing puthik@, who was leaning toward the current > kernel code (caring about LSB) being the correct interpretation. I > believe we have other chromebooks which do rely on LSB functionality, > so unless we can find more examples of laptops wanting MSB it > currently looks like Pixelbook is the outlier. > > On Thu, Sep 17, 2020 at 11:28 AM Jani Nikula > wrote: > > > > On Thu, 17 Sep 2020, Kevin Chowski wrote: > > > We have observed that Google Pixelbook's backlight hardware is > > > interpretting these backlight bits from the most-significant side of the > > > 16 bit word (if DP_EDP_PWMGEN_BIT_COUNT < 16), whereas the driver code > > > assumes the peripheral cares about the least-significant bits. > > > > > > Testing was done from within Chrome OS's build environment when the > > > patch is backported to 5.4 (the version we are newly targeting for the > > > Pixelbook); for the record: > > >$ emerge-eve-kernelnext sys-kernel/chromeos-kernel-5_4 && \ > > > ./update_kernel.sh --remote=$IP > > > > > > I used `/sys/kernel/debug/dri/0/eDP-1/i915_dpcd` on my laptop to verify > > > that the registers were being set according to what the actual hardware > > > expects; I also observe that the backlight is noticeably brighter with > > > this patch. > > > > It's unclear to me what kernel version this is against, and what you've > > actually tested. > > > > Have you tried v5.7 kernel with Lyude's fde7266fb2f6 ("drm/i915: Fix eDP > > DPCD aux max backlight calculations")? > > > > I just want to make sure you've tested with all the relevant fixes > > before adding quirks. > > > > BR, > > Jani. > > > > > > > > Signed-off-by: Kevin Chowski > > > --- > > > > > > .../drm/i915/display/intel_dp_aux_backlight.c | 34 +++ > > > drivers/gpu/drm/i915/display/intel_quirks.c | 13 +++ > > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > > 3 files changed, 48 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > > b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > > index acbd7eb66cbe3..99c98f217356d 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > > @@ -91,6 +91,23 @@ static u32 intel_dp_aux_get_backlight(struct > > > intel_connector *connector) > > > if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT) > > > level = (read_val[0] << 8 | read_val[1]); > > > > > > + if (i915->quirks & QUIRK_SHIFT_EDP_BACKLIGHT_BRIGHTNESS) { > > > + if (!drm_dp_dpcd_readb(&intel_dp->aux, > > > DP_EDP_PWMGEN_BIT_COUNT, > > > + &read_val[0])) { > > > + DRM_DEBUG_KMS("Failed to read DPCD register 0x%x\n", > > > + DP_EDP_PWMGEN_BIT_COUNT); > > > + return 0; > > > + } > > > + // Only bits 4:0 are used, 7:5 are reserved. > > > + read_val[0] = read_val[0] & 0x1F; > > > + if (read_val[0] > 16) { > > > + DRM_DEBUG_KMS("Invalid DP_EDP_PWNGEN_BIT_COUNT > > > 0x%X, expected at most 16\n", > > > + read_val[0]); > > > + return 0; > > > + } > > > + level >>= 16 - read_val[0]; > > > + } > > > + > > > return level; > > > } > > > > > > @@ -106,6 +123,23 @@ intel_dp_aux_set_backlight(const struct > > > drm_connector_state *conn_state, u32 lev > > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > u8 va
Re: [Intel-gfx] [PATCH] i915: Introduce quirk for shifting eDP brightness.
I'll defer to Ville & Lyude. I dug up more on the bug report and found that both Thinkpad and Galaxy Chromebook use the same Samsung OLED. So my 2 vs 1 argument is actually not valid. On Fri, Sep 18, 2020 at 10:59 AM Kevin Chowski wrote: > > Apologies once again, some of my emails were bouncing for some > addresses yesterday. Hopefully it was a temporary condition; I'll > continue trying to dig into it on my end if it happens again for this > email. > > Since there's evidence that some models want lsb and some (well, at > least one) want msb, my understanding is that we'll need a quirk one > way or the other (please correct if I'm mistaken). I unfortunately > don't have the ability to test anything other than the Pixelbook, so > if we decide the msb is the "right" way, then I will have to rely on > others to test (and find the OUI of) other models which require lsb. > > I am happy to make any changes requested, but I do not at all have > enough background here to make the decision on whether the msb > functionality deserves the quirk or if the lsb one does. How can I > help you all come to an agreement here? > > * It seems like Ville feels strongly about the msb being the correct > interpretation of the spec. > * It's unclear to me on which side of the fence Lyude falls, I > couldn't pick up a strong opinion in her clarifying question. > * Puthikorn seems to be on the side of lsb being correct, but maybe > was swayed by Ville's argument. > > If no one feels that Ville's argument is not strong in some way, and > we go with that, I will get to work on the requested changes. I am > concerned, though, about changing the default functionality without > testing it widely to find the set of laptops which require the lsb > quirk. I'd appreciate any advice people might have about making this > change safely. > > Thank you for your time, > Kevin > > On Thu, Sep 17, 2020 at 2:11 PM Ville Syrjälä > wrote: > > > > On Thu, Sep 17, 2020 at 09:25:35PM +0300, Ville Syrjälä wrote: > > > On Thu, Sep 17, 2020 at 11:14:43AM -0700, Puthikorn Voravootivat wrote: > > > > The Lyude fde7266fb2f6 change is actually based on Chromium change > > > > (https://crrev.com/c/1650325) that fixes the brightness for Samsung > > > > Galaxy Chromebook. So currently we have 2 examples that use LSB > > > > interpretation and 1 that use MSB. > > > > > > "If field 4:0 of the EDP_PWMGEN_BIT_COUNT register represents a value > > > of greater than 8 and the BACKLIGHT_BRIGHTNESS_BYTE_COUNT bit > > > is cleared to 0, only the 8 MSB of the brightness control value can be > > > controlled. > > > (See Note below.) > > > Assigned bits are allocated to the MSB of the enabled register > > > combination." > > > > > > I think that's pretty clear the assigned bits are supposed to be > > > msb aligned. > > > > I guess there's some email issues happening, but just to clarify: > > > > When the spec says MSB in caps here it clearly means > > "most significant-bit(s)" since otherwise "8 MSB" would not make > > any sense in the context of a 2 byte value. > > > > Granted the spec is crap here since "Table 1-1: Acronyms and > > Initialism" does claim "MSB" should be byte(s) and "msb" bit(s). > > > > Also I can't imagine anyone would allocate the bits starting > > from the lsb when the whole thing is clearly supposed to be a > > 16bit big endian integer. So with >8 assigned bits you'd end > > up with crazy stuff like this: > > > > [ 7 ... 0 ][7 ... 0] > > [ 8 MSB ][][N LSB] > > > > so you couldn't even treat the value as a regular big endian > > thing. Instead, if you squint a bit, it now looks like a funky > > little endian value. So we're deep into some mixed endian land > > where nothing makes sense anymore. > > > > Anyways I think the code should simply do this to match the spec: > > u16 value = brightness << (16 - num_assigned_bits); > > val[0] = value >> 8; > > val[1] = value & 0xff; > > > > > > > > > > > > > > > > > > > On Thu, Sep 17, 2020 at 10:55 AM Kevin Chowski > > > > wrote: > > > > > > > > > > Apologies for being too vague. To be as precise I can be, here is the > > > > > specific code delta I tested: https://crrev.com/c/2406616 . To answer > > > > > your other question, the code I tested against is indeed
Re: [Intel-gfx] [PATCH] i915: Introduce quirk for shifting eDP brightness.
+Lyude I notice that Lyude email was somehow dropped from the thread. Lyude was the person who submitted the patch for Thinkpad and should know the OUI of the panel. On Tue, Sep 22, 2020 at 11:47 AM Kevin Chowski wrote: > > Alrighty, I'll take everyone else's silence as tacit approval of > Ville's opinions. (I didn't receive any email bounces this time, so I > think my issue was transient.) I will start on inverting the quirk and > making the most-significant-alignment matter for these registers by > default. > > Who can help me gather a list of OUIs that we need to add to the > quirk? I can follow up with Puthikorn about the relevant Chromebooks, > but I don't know what other types of laptops are using this driver. > > Thanks for your time, > Kevin Chowski > > > On Fri, Sep 18, 2020 at 12:16 PM Puthikorn Voravootivat > wrote: > > > > I'll defer to Ville & Lyude. > > > > I dug up more on the bug report and found that both Thinkpad and > > Galaxy Chromebook use the same Samsung OLED. > > So my 2 vs 1 argument is actually not valid. > > > > On Fri, Sep 18, 2020 at 10:59 AM Kevin Chowski wrote: > > > > > > Apologies once again, some of my emails were bouncing for some > > > addresses yesterday. Hopefully it was a temporary condition; I'll > > > continue trying to dig into it on my end if it happens again for this > > > email. > > > > > > Since there's evidence that some models want lsb and some (well, at > > > least one) want msb, my understanding is that we'll need a quirk one > > > way or the other (please correct if I'm mistaken). I unfortunately > > > don't have the ability to test anything other than the Pixelbook, so > > > if we decide the msb is the "right" way, then I will have to rely on > > > others to test (and find the OUI of) other models which require lsb. > > > > > > I am happy to make any changes requested, but I do not at all have > > > enough background here to make the decision on whether the msb > > > functionality deserves the quirk or if the lsb one does. How can I > > > help you all come to an agreement here? > > > > > > * It seems like Ville feels strongly about the msb being the correct > > > interpretation of the spec. > > > * It's unclear to me on which side of the fence Lyude falls, I > > > couldn't pick up a strong opinion in her clarifying question. > > > * Puthikorn seems to be on the side of lsb being correct, but maybe > > > was swayed by Ville's argument. > > > > > > If no one feels that Ville's argument is not strong in some way, and > > > we go with that, I will get to work on the requested changes. I am > > > concerned, though, about changing the default functionality without > > > testing it widely to find the set of laptops which require the lsb > > > quirk. I'd appreciate any advice people might have about making this > > > change safely. > > > > > > Thank you for your time, > > > Kevin > > > > > > On Thu, Sep 17, 2020 at 2:11 PM Ville Syrjälä > > > wrote: > > > > > > > > On Thu, Sep 17, 2020 at 09:25:35PM +0300, Ville Syrjälä wrote: > > > > > On Thu, Sep 17, 2020 at 11:14:43AM -0700, Puthikorn Voravootivat > > > > > wrote: > > > > > > The Lyude fde7266fb2f6 change is actually based on Chromium change > > > > > > (https://crrev.com/c/1650325) that fixes the brightness for Samsung > > > > > > Galaxy Chromebook. So currently we have 2 examples that use LSB > > > > > > interpretation and 1 that use MSB. > > > > > > > > > > "If field 4:0 of the EDP_PWMGEN_BIT_COUNT register represents a value > > > > > of greater than 8 and the BACKLIGHT_BRIGHTNESS_BYTE_COUNT bit > > > > > is cleared to 0, only the 8 MSB of the brightness control value can be > > > > > controlled. > > > > > (See Note below.) > > > > > Assigned bits are allocated to the MSB of the enabled register > > > > > combination." > > > > > > > > > > I think that's pretty clear the assigned bits are supposed to be > > > > > msb aligned. > > > > > > > > I guess there's some email issues happening, but just to clarify: > > > > > > > > When the spec says MSB in caps here it clearly means > > > > "most significant-bit(s)"