A while ago we ran into issues while trying to enable the eDP backlight control interface as defined by VESA, in order to make the DPCD backlight controls on newer laptop panels work. The issue ended up being much more complicated however, as we also apparently needed to add support for an Intel-specific DPCD backlight control interface as the VESA interface is broken on many laptop panels. For lack of a better name, we just call this the Intel HDR backlight interface.
While this only adds support for the SDR backlight mode (I think), this will fix a lot of user's laptop panels that we weren't able to properly automatically detect DPCD backlight controls on previously.
Series-wide changes in v3: * Pass down brightness values to enable/disable backlight callbacks in a separate patch * Rebase
Lyude Paul (9): drm/i915/dp: Program source OUI on eDP panels drm/i915: Rename pwm_* backlight callbacks to ext_pwm_* drm/i915: Pass down brightness values to enable/disable backlight callbacks drm/i915: Keep track of pwm-related backlight hooks separately drm/i915/dp: Rename eDP VESA backlight interface functions drm/i915/dp: Add register definitions for Intel HDR backlight interface drm/i915/dp: Enable Intel's HDR backlight interface (only SDR for now) drm/i915/dp: Allow forcing specific interfaces through enable_dpcd_backlight drm/dp: Revert "drm/dp: Introduce EDID-based quirks"
drivers/gpu/drm/drm_dp_helper.c | 83 +--- drivers/gpu/drm/drm_dp_mst_topology.c | 3 +- .../drm/i915/display/intel_display_types.h | 18 +- drivers/gpu/drm/i915/display/intel_dp.c | 42 +- .../drm/i915/display/intel_dp_aux_backlight.c | 394 +++++++++++++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +- .../i915/display/intel_dsi_dcs_backlight.c | 7 +- drivers/gpu/drm/i915/display/intel_panel.c | 435 ++++++++++-------- drivers/gpu/drm/i915/display/intel_panel.h | 4 + drivers/gpu/drm/i915/display/intel_psr.c | 2 +- drivers/gpu/drm/i915/i915_params.c | 2 +- include/drm/drm_dp_helper.h | 21 +- 12 files changed, 655 insertions(+), 359 deletions(-)
Since we're about to start adding support for Intel's magic HDR backlight interface over DPCD, we need to ensure we're properly programming this field so that Intel specific sink services are exposed. Otherwise, 0x300-0x3ff will just read zeroes.
We also take care not to reprogram the source OUI if it already matches what we expect. This is just to be careful so that we don't accidentally take the panel out of any backlight control modes we found it in.
v2: * Add careful parameter to intel_edp_init_source_oui() to avoid re-writing the source OUI if it's already been set during driver initialization
Signed-off-by: Lyude Paul lyude@redhat.com Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com --- drivers/gpu/drm/i915/display/intel_dp.c | 33 +++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 2d4d5e95af84..4cb2bfee9c40 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3583,6 +3583,29 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp, enable ? "enable" : "disable"); }
+static void +intel_edp_init_source_oui(struct intel_dp *intel_dp, bool careful) +{ + struct drm_i915_private *i915 = dp_to_i915(intel_dp); + u8 oui[] = { 0x00, 0xaa, 0x01 }; + u8 buf[3] = { 0 }; + + /* + * During driver init, we want to be careful and avoid changing the source OUI if it's + * already set to what we want, so as to avoid clearing any state by accident + */ + if (careful) { + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SOURCE_OUI, buf, sizeof(buf)) < 0) + drm_err(&i915->drm, "Failed to read source OUI\n"); + + if (memcmp(oui, buf, sizeof(oui)) == 0) + return; + } + + if (drm_dp_dpcd_write(&intel_dp->aux, DP_SOURCE_OUI, oui, sizeof(oui)) < 0) + drm_err(&i915->drm, "Failed to write source OUI\n"); +} + /* If the device supports it, try to set the power state appropriately */ void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode) { @@ -3604,6 +3627,10 @@ void intel_dp_set_power(struct intel_dp *intel_dp, u8 mode)
lspcon_resume(dp_to_dig_port(intel_dp));
+ /* Write the source OUI as early as possible */ + if (intel_dp_is_edp(intel_dp)) + intel_edp_init_source_oui(intel_dp, false); + /* * When turning on, we need to retry for 1ms to give the sink * time to wake up. @@ -4869,6 +4896,12 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) intel_dp_get_dsc_sink_cap(intel_dp);
+ /* + * If needed, program our source OUI so we can make various Intel-specific AUX services + * available (such as HDR backlight controls) + */ + intel_edp_init_source_oui(intel_dp, true); + return true; }
Since we're going to need to add a set of lower-level PWM backlight control hooks to be shared by normal backlight controls and HDR backlight controls in SDR mode, let's add a prefix to the external PWM backlight functions so that the difference between them and the high level PWM-only backlight functions is a bit more obvious.
This introduces no functional changes.
Signed-off-by: Lyude Paul lyude@redhat.com Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com Reviewed-by: Jani Nikula jani.nikula@intel.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com --- drivers/gpu/drm/i915/display/intel_panel.c | 28 +++++++++++----------- 1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index 36b7693453ae..da8f7c12ae22 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -589,7 +589,7 @@ static u32 bxt_get_backlight(struct intel_connector *connector) BXT_BLC_PWM_DUTY(panel->backlight.controller)); }
-static u32 pwm_get_backlight(struct intel_connector *connector) +static u32 ext_pwm_get_backlight(struct intel_connector *connector) { struct intel_panel *panel = &connector->panel; struct pwm_state state; @@ -666,7 +666,7 @@ static void bxt_set_backlight(const struct drm_connector_state *conn_state, u32 BXT_BLC_PWM_DUTY(panel->backlight.controller), level); }
-static void pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level) +static void ext_pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level) { struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel;
@@ -835,7 +835,7 @@ static void cnp_disable_backlight(const struct drm_connector_state *old_conn_sta tmp & ~BXT_BLC_PWM_ENABLE); }
-static void pwm_disable_backlight(const struct drm_connector_state *old_conn_state) +static void ext_pwm_disable_backlight(const struct drm_connector_state *old_conn_state) { struct intel_connector *connector = to_intel_connector(old_conn_state->connector); struct intel_panel *panel = &connector->panel; @@ -1168,8 +1168,8 @@ static void cnp_enable_backlight(const struct intel_crtc_state *crtc_state, pwm_ctl | BXT_BLC_PWM_ENABLE); }
-static void pwm_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) +static void ext_pwm_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_panel *panel = &connector->panel; @@ -1890,8 +1890,8 @@ cnp_setup_backlight(struct intel_connector *connector, enum pipe unused) return 0; }
-static int pwm_setup_backlight(struct intel_connector *connector, - enum pipe pipe) +static int ext_pwm_setup_backlight(struct intel_connector *connector, + enum pipe pipe) { struct drm_device *dev = connector->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -2061,12 +2061,12 @@ static const struct intel_panel_bl_funcs pch_funcs = { .hz_to_pwm = pch_hz_to_pwm, };
-static const struct intel_panel_bl_funcs pwm_funcs = { - .setup = pwm_setup_backlight, - .enable = pwm_enable_backlight, - .disable = pwm_disable_backlight, - .set = pwm_set_backlight, - .get = pwm_get_backlight, +static const struct intel_panel_bl_funcs ext_pwm_funcs = { + .setup = ext_pwm_setup_backlight, + .enable = ext_pwm_enable_backlight, + .disable = ext_pwm_disable_backlight, + .set = ext_pwm_set_backlight, + .get = ext_pwm_get_backlight, };
static const struct intel_panel_bl_funcs vlv_funcs = { @@ -2125,7 +2125,7 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) panel->backlight.funcs = &pch_funcs; } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI) { - panel->backlight.funcs = &pwm_funcs; + panel->backlight.funcs = &ext_pwm_funcs; } else { panel->backlight.funcs = &vlv_funcs; }
Instead of using intel_panel->backlight.level, have the caller provide us with the current panel backlight value. We'll need this for when we separate PWM-related backlight callbacks from other means of backlight control (like DPCD backlight controls), as the caller of each PWM callback will be responsible for converting the current brightness value to it's respective PWM level.
Signed-off-by: Lyude Paul lyude@redhat.com --- .../drm/i915/display/intel_display_types.h | 4 +- .../drm/i915/display/intel_dp_aux_backlight.c | 8 +-- .../i915/display/intel_dsi_dcs_backlight.c | 7 +- drivers/gpu/drm/i915/display/intel_panel.c | 67 +++++++++---------- 4 files changed, 42 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 1fa0246b3a82..a70d1bf29aa5 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -230,9 +230,9 @@ struct intel_panel_bl_funcs { int (*setup)(struct intel_connector *connector, enum pipe pipe); u32 (*get)(struct intel_connector *connector); void (*set)(const struct drm_connector_state *conn_state, u32 level); - void (*disable)(const struct drm_connector_state *conn_state); + void (*disable)(const struct drm_connector_state *conn_state, u32 level); void (*enable)(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state); + const struct drm_connector_state *conn_state, u32 level); u32 (*hz_to_pwm)(struct intel_connector *connector, u32 hz); };
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 4fd536801b14..c76287e9e91e 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -174,7 +174,7 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector) }
static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct intel_dp *intel_dp = intel_attached_dp(connector); @@ -225,12 +225,12 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st } }
- intel_dp_aux_set_backlight(conn_state, - connector->panel.backlight.level); + intel_dp_aux_set_backlight(conn_state, level); set_aux_backlight_enable(intel_dp, true); }
-static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old_conn_state) +static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old_conn_state, + u32 level) { set_aux_backlight_enable(enc_to_intel_dp(to_intel_encoder(old_conn_state->best_encoder)), false); diff --git a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c index 5c508d51f526..88628764956d 100644 --- a/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dsi_dcs_backlight.c @@ -77,7 +77,7 @@ static void dcs_set_backlight(const struct drm_connector_state *conn_state, u32 } }
-static void dcs_disable_backlight(const struct drm_connector_state *conn_state) +static void dcs_disable_backlight(const struct drm_connector_state *conn_state, u32 level) { struct intel_dsi *intel_dsi = enc_to_intel_dsi(to_intel_encoder(conn_state->best_encoder)); struct mipi_dsi_device *dsi_device; @@ -111,10 +111,9 @@ static void dcs_disable_backlight(const struct drm_connector_state *conn_state) }
static void dcs_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_dsi *intel_dsi = enc_to_intel_dsi(to_intel_encoder(conn_state->best_encoder)); - struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel; struct mipi_dsi_device *dsi_device; enum port port;
@@ -142,7 +141,7 @@ static void dcs_enable_backlight(const struct intel_crtc_state *crtc_state, &cabc, sizeof(cabc)); }
- dcs_set_backlight(conn_state, panel->backlight.level); + dcs_set_backlight(conn_state, level); }
static int dcs_setup_backlight(struct intel_connector *connector, diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index da8f7c12ae22..67f81ae995c4 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -726,13 +726,13 @@ void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state mutex_unlock(&dev_priv->backlight_lock); }
-static void lpt_disable_backlight(const struct drm_connector_state *old_conn_state) +static void lpt_disable_backlight(const struct drm_connector_state *old_conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(old_conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, 0); + intel_panel_actually_set_backlight(old_conn_state, level);
/* * Although we don't support or enable CPU PWM with LPT/SPT based @@ -754,13 +754,13 @@ static void lpt_disable_backlight(const struct drm_connector_state *old_conn_sta intel_de_write(dev_priv, BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE); }
-static void pch_disable_backlight(const struct drm_connector_state *old_conn_state) +static void pch_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) { struct intel_connector *connector = to_intel_connector(old_conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, 0); + intel_panel_actually_set_backlight(old_conn_state, val);
tmp = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2); intel_de_write(dev_priv, BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE); @@ -769,44 +769,44 @@ static void pch_disable_backlight(const struct drm_connector_state *old_conn_sta intel_de_write(dev_priv, BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE); }
-static void i9xx_disable_backlight(const struct drm_connector_state *old_conn_state) +static void i9xx_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) { - intel_panel_actually_set_backlight(old_conn_state, 0); + intel_panel_actually_set_backlight(old_conn_state, val); }
-static void i965_disable_backlight(const struct drm_connector_state *old_conn_state) +static void i965_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) { struct drm_i915_private *dev_priv = to_i915(old_conn_state->connector->dev); u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, 0); + intel_panel_actually_set_backlight(old_conn_state, val);
tmp = intel_de_read(dev_priv, BLC_PWM_CTL2); intel_de_write(dev_priv, BLC_PWM_CTL2, tmp & ~BLM_PWM_ENABLE); }
-static void vlv_disable_backlight(const struct drm_connector_state *old_conn_state) +static void vlv_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) { struct intel_connector *connector = to_intel_connector(old_conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); enum pipe pipe = to_intel_crtc(old_conn_state->crtc)->pipe; u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, 0); + intel_panel_actually_set_backlight(old_conn_state, val);
tmp = intel_de_read(dev_priv, VLV_BLC_PWM_CTL2(pipe)); intel_de_write(dev_priv, VLV_BLC_PWM_CTL2(pipe), tmp & ~BLM_PWM_ENABLE); }
-static void bxt_disable_backlight(const struct drm_connector_state *old_conn_state) +static void bxt_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) { struct intel_connector *connector = to_intel_connector(old_conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - u32 tmp, val; + u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, 0); + intel_panel_actually_set_backlight(old_conn_state, val);
tmp = intel_de_read(dev_priv, BXT_BLC_PWM_CTL(panel->backlight.controller)); @@ -820,14 +820,14 @@ static void bxt_disable_backlight(const struct drm_connector_state *old_conn_sta } }
-static void cnp_disable_backlight(const struct drm_connector_state *old_conn_state) +static void cnp_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) { struct intel_connector *connector = to_intel_connector(old_conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, 0); + intel_panel_actually_set_backlight(old_conn_state, val);
tmp = intel_de_read(dev_priv, BXT_BLC_PWM_CTL(panel->backlight.controller)); @@ -835,7 +835,7 @@ static void cnp_disable_backlight(const struct drm_connector_state *old_conn_sta tmp & ~BXT_BLC_PWM_ENABLE); }
-static void ext_pwm_disable_backlight(const struct drm_connector_state *old_conn_state) +static void ext_pwm_disable_backlight(const struct drm_connector_state *old_conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(old_conn_state->connector); struct intel_panel *panel = &connector->panel; @@ -870,13 +870,13 @@ void intel_panel_disable_backlight(const struct drm_connector_state *old_conn_st if (panel->backlight.device) panel->backlight.device->props.power = FB_BLANK_POWERDOWN; panel->backlight.enabled = false; - panel->backlight.funcs->disable(old_conn_state); + panel->backlight.funcs->disable(old_conn_state, 0);
mutex_unlock(&dev_priv->backlight_lock); }
static void lpt_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -923,11 +923,11 @@ static void lpt_enable_backlight(const struct intel_crtc_state *crtc_state, pch_ctl1 | BLM_PCH_PWM_ENABLE);
/* This won't stick until the above enable. */ - intel_panel_actually_set_backlight(conn_state, panel->backlight.level); + intel_panel_actually_set_backlight(conn_state, level); }
static void pch_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -958,7 +958,7 @@ static void pch_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE);
/* This won't stick until the above enable. */ - intel_panel_actually_set_backlight(conn_state, panel->backlight.level); + intel_panel_actually_set_backlight(conn_state, level);
pch_ctl2 = panel->backlight.max << 16; intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2); @@ -974,7 +974,7 @@ static void pch_enable_backlight(const struct intel_crtc_state *crtc_state, }
static void i9xx_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -1001,7 +1001,7 @@ static void i9xx_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_posting_read(dev_priv, BLC_PWM_CTL);
/* XXX: combine this into above write? */ - intel_panel_actually_set_backlight(conn_state, panel->backlight.level); + intel_panel_actually_set_backlight(conn_state, level);
/* * Needed to enable backlight on some 855gm models. BLC_HIST_CTL is @@ -1013,7 +1013,7 @@ static void i9xx_enable_backlight(const struct intel_crtc_state *crtc_state, }
static void i965_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -1044,11 +1044,11 @@ static void i965_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_posting_read(dev_priv, BLC_PWM_CTL2); intel_de_write(dev_priv, BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE);
- intel_panel_actually_set_backlight(conn_state, panel->backlight.level); + intel_panel_actually_set_backlight(conn_state, level); }
static void vlv_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -1067,7 +1067,7 @@ static void vlv_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, VLV_BLC_PWM_CTL(pipe), ctl);
/* XXX: combine this into above write? */ - intel_panel_actually_set_backlight(conn_state, panel->backlight.level); + intel_panel_actually_set_backlight(conn_state, level);
ctl2 = 0; if (panel->backlight.active_low_pwm) @@ -1079,7 +1079,7 @@ static void vlv_enable_backlight(const struct intel_crtc_state *crtc_state, }
static void bxt_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -1118,7 +1118,7 @@ static void bxt_enable_backlight(const struct intel_crtc_state *crtc_state, BXT_BLC_PWM_FREQ(panel->backlight.controller), panel->backlight.max);
- intel_panel_actually_set_backlight(conn_state, panel->backlight.level); + intel_panel_actually_set_backlight(conn_state, level);
pwm_ctl = 0; if (panel->backlight.active_low_pwm) @@ -1133,7 +1133,7 @@ static void bxt_enable_backlight(const struct intel_crtc_state *crtc_state, }
static void cnp_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -1154,7 +1154,7 @@ static void cnp_enable_backlight(const struct intel_crtc_state *crtc_state, BXT_BLC_PWM_FREQ(panel->backlight.controller), panel->backlight.max);
- intel_panel_actually_set_backlight(conn_state, panel->backlight.level); + intel_panel_actually_set_backlight(conn_state, level);
pwm_ctl = 0; if (panel->backlight.active_low_pwm) @@ -1169,11 +1169,10 @@ static void cnp_enable_backlight(const struct intel_crtc_state *crtc_state, }
static void ext_pwm_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state) + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct intel_panel *panel = &connector->panel; - int level = panel->backlight.level;
level = intel_panel_compute_brightness(connector, level); pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100); @@ -1198,7 +1197,7 @@ static void __intel_panel_enable_backlight(const struct intel_crtc_state *crtc_s panel->backlight.device->props.max_brightness); }
- panel->backlight.funcs->enable(crtc_state, conn_state); + panel->backlight.funcs->enable(crtc_state, conn_state, panel->backlight.level); panel->backlight.enabled = true; if (panel->backlight.device) panel->backlight.device->props.power = FB_BLANK_UNBLANK;
Currently, every different type of backlight hook that i915 supports is pretty straight forward - you have a backlight, probably through PWM (but maybe DPCD), with a single set of platform-specific hooks that are used for controlling it.
HDR backlights, in particular VESA and Intel's HDR backlight implementations, can end up being more complicated. With Intel's proprietary interface, HDR backlight controls always run through the DPCD. When the backlight is in SDR backlight mode however, the driver may need to bypass the TCON and control the backlight directly through PWM.
So, in order to support this we'll need to split our backlight callbacks into two groups: a set of high-level backlight control callbacks in intel_panel, and an additional set of pwm-specific backlight control callbacks. This also implies a functional changes for how these callbacks are used:
* We now keep track of two separate backlight level ranges, one for the high-level backlight, and one for the pwm backlight range * We also keep track of backlight enablement and PWM backlight enablement separately * Since the currently set backlight level might not be the same as the currently programmed PWM backlight level, we stop setting panel->backlight.level with the currently programmed PWM backlight level in panel->backlight.pwm_funcs->setup(). Instead, we rely on the higher level backlight control functions to retrieve the current PWM backlight level (in this case, intel_pwm_get_backlight()). Note that there are still a few PWM backlight setup callbacks that do actually need to retrieve the current PWM backlight level, although we no longer save this value in panel->backlight.level like before.
Additionally, we drop the call to lpt_get_backlight() in lpt_setup_backlight(), and avoid unconditionally writing the PWM value that we get from it and only write it back if we're in PCH mode. The reason for this is because in the original codepath for this, it was expected that the intel_panel_bl_funcs->setup() hook would be responsible for fetching the initial backlight level. On lpt systems, the only time we could ever be in PCH backlight mode is during the initial driver load - meaning that outside of the setup() hook, lpt_get_backlight() will always be the callback used for retrieving the current backlight level. After this patch we still need to fetch and write-back the PCH backlight value if we're in PCH mode, but because intel_pwm_setup_backlight() will retrieve the backlight level after setup() using the get() hook, which always ends up being lpt_get_backlight(). Thus - an additional call to lpt_get_backlight() in lpt_setup_backlight() is made redundant.
v3: * Reuse intel_panel_bl_funcs() for pwm_funcs * Explain why we drop lpt_get_backlight()
Signed-off-by: Lyude Paul lyude@redhat.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com --- .../drm/i915/display/intel_display_types.h | 4 + drivers/gpu/drm/i915/display/intel_panel.c | 344 ++++++++++-------- 2 files changed, 194 insertions(+), 154 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index a70d1bf29aa5..47ee565c49a2 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -252,6 +252,9 @@ struct intel_panel { bool alternate_pwm_increment; /* lpt+ */
/* PWM chip */ + u32 pwm_min; + u32 pwm_max; + bool pwm_enabled; bool util_pin_active_low; /* bxt+ */ u8 controller; /* bxt+ only */ struct pwm_device *pwm; @@ -263,6 +266,7 @@ struct intel_panel { struct backlight_device *device;
const struct intel_panel_bl_funcs *funcs; + const struct intel_panel_bl_funcs *pwm_funcs; void (*power)(struct intel_connector *, bool enable); } backlight; }; diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index 67f81ae995c4..41f0d2b2c627 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -511,25 +511,34 @@ static u32 scale_hw_to_user(struct intel_connector *connector, 0, user_max); }
-static u32 intel_panel_compute_brightness(struct intel_connector *connector, - u32 val) +static u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 val) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel;
- drm_WARN_ON(&dev_priv->drm, panel->backlight.max == 0); + drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_max == 0);
if (dev_priv->params.invert_brightness < 0) return val;
if (dev_priv->params.invert_brightness > 0 || dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS) { - return panel->backlight.max - val + panel->backlight.min; + return panel->backlight.pwm_max - val + panel->backlight.pwm_min; }
return val; }
+static void intel_panel_set_pwm_level(const struct drm_connector_state *conn_state, u32 val) +{ + struct intel_connector *connector = to_intel_connector(conn_state->connector); + struct drm_i915_private *i915 = to_i915(connector->base.dev); + struct intel_panel *panel = &connector->panel; + + drm_dbg_kms(&i915->drm, "set backlight PWM = %d\n", val); + panel->backlight.pwm_funcs->set(conn_state, val); +} + static u32 lpt_get_backlight(struct intel_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -624,12 +633,12 @@ static void i9xx_set_backlight(const struct drm_connector_state *conn_state, u32 struct intel_panel *panel = &connector->panel; u32 tmp, mask;
- drm_WARN_ON(&dev_priv->drm, panel->backlight.max == 0); + drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_max == 0);
if (panel->backlight.combination_mode) { u8 lbpc;
- lbpc = level * 0xfe / panel->backlight.max + 1; + lbpc = level * 0xfe / panel->backlight.pwm_max + 1; level /= lbpc; pci_write_config_byte(dev_priv->drm.pdev, LBPC, lbpc); } @@ -681,9 +690,8 @@ intel_panel_actually_set_backlight(const struct drm_connector_state *conn_state, struct drm_i915_private *i915 = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel;
- drm_dbg_kms(&i915->drm, "set backlight PWM = %d\n", level); + drm_dbg_kms(&i915->drm, "set backlight level = %d\n", level);
- level = intel_panel_compute_brightness(connector, level); panel->backlight.funcs->set(conn_state, level); }
@@ -732,7 +740,7 @@ static void lpt_disable_backlight(const struct drm_connector_state *old_conn_sta struct drm_i915_private *dev_priv = to_i915(connector->base.dev); u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, level); + intel_panel_set_pwm_level(old_conn_state, level);
/* * Although we don't support or enable CPU PWM with LPT/SPT based @@ -760,7 +768,7 @@ static void pch_disable_backlight(const struct drm_connector_state *old_conn_sta struct drm_i915_private *dev_priv = to_i915(connector->base.dev); u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2); intel_de_write(dev_priv, BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE); @@ -771,7 +779,7 @@ static void pch_disable_backlight(const struct drm_connector_state *old_conn_sta
static void i9xx_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) { - intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val); }
static void i965_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) @@ -779,7 +787,7 @@ static void i965_disable_backlight(const struct drm_connector_state *old_conn_st struct drm_i915_private *dev_priv = to_i915(old_conn_state->connector->dev); u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, BLC_PWM_CTL2); intel_de_write(dev_priv, BLC_PWM_CTL2, tmp & ~BLM_PWM_ENABLE); @@ -792,7 +800,7 @@ static void vlv_disable_backlight(const struct drm_connector_state *old_conn_sta enum pipe pipe = to_intel_crtc(old_conn_state->crtc)->pipe; u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, VLV_BLC_PWM_CTL2(pipe)); intel_de_write(dev_priv, VLV_BLC_PWM_CTL2(pipe), @@ -806,7 +814,7 @@ static void bxt_disable_backlight(const struct drm_connector_state *old_conn_sta struct intel_panel *panel = &connector->panel; u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, BXT_BLC_PWM_CTL(panel->backlight.controller)); @@ -827,7 +835,7 @@ static void cnp_disable_backlight(const struct drm_connector_state *old_conn_sta struct intel_panel *panel = &connector->panel; u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, BXT_BLC_PWM_CTL(panel->backlight.controller)); @@ -906,7 +914,7 @@ static void lpt_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, SOUTH_CHICKEN1, schicken); }
- pch_ctl2 = panel->backlight.max << 16; + pch_ctl2 = panel->backlight.pwm_max << 16; intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2);
pch_ctl1 = 0; @@ -923,7 +931,7 @@ static void lpt_enable_backlight(const struct intel_crtc_state *crtc_state, pch_ctl1 | BLM_PCH_PWM_ENABLE);
/* This won't stick until the above enable. */ - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level); }
static void pch_enable_backlight(const struct intel_crtc_state *crtc_state, @@ -958,9 +966,9 @@ static void pch_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE);
/* This won't stick until the above enable. */ - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level);
- pch_ctl2 = panel->backlight.max << 16; + pch_ctl2 = panel->backlight.pwm_max << 16; intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2);
pch_ctl1 = 0; @@ -987,7 +995,7 @@ static void i9xx_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CTL, 0); }
- freq = panel->backlight.max; + freq = panel->backlight.pwm_max; if (panel->backlight.combination_mode) freq /= 0xff;
@@ -1001,7 +1009,7 @@ static void i9xx_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_posting_read(dev_priv, BLC_PWM_CTL);
/* XXX: combine this into above write? */ - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level);
/* * Needed to enable backlight on some 855gm models. BLC_HIST_CTL is @@ -1028,7 +1036,7 @@ static void i965_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CTL2, ctl2); }
- freq = panel->backlight.max; + freq = panel->backlight.pwm_max; if (panel->backlight.combination_mode) freq /= 0xff;
@@ -1044,7 +1052,7 @@ static void i965_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_posting_read(dev_priv, BLC_PWM_CTL2); intel_de_write(dev_priv, BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE);
- intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level); }
static void vlv_enable_backlight(const struct intel_crtc_state *crtc_state, @@ -1063,11 +1071,11 @@ static void vlv_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, VLV_BLC_PWM_CTL2(pipe), ctl2); }
- ctl = panel->backlight.max << 16; + ctl = panel->backlight.pwm_max << 16; intel_de_write(dev_priv, VLV_BLC_PWM_CTL(pipe), ctl);
/* XXX: combine this into above write? */ - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level);
ctl2 = 0; if (panel->backlight.active_low_pwm) @@ -1116,9 +1124,9 @@ static void bxt_enable_backlight(const struct intel_crtc_state *crtc_state,
intel_de_write(dev_priv, BXT_BLC_PWM_FREQ(panel->backlight.controller), - panel->backlight.max); + panel->backlight.pwm_max);
- intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level);
pwm_ctl = 0; if (panel->backlight.active_low_pwm) @@ -1152,9 +1160,9 @@ static void cnp_enable_backlight(const struct intel_crtc_state *crtc_state,
intel_de_write(dev_priv, BXT_BLC_PWM_FREQ(panel->backlight.controller), - panel->backlight.max); + panel->backlight.pwm_max);
- intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level);
pwm_ctl = 0; if (panel->backlight.active_low_pwm) @@ -1174,7 +1182,6 @@ static void ext_pwm_enable_backlight(const struct intel_crtc_state *crtc_state, struct intel_connector *connector = to_intel_connector(conn_state->connector); struct intel_panel *panel = &connector->panel;
- level = intel_panel_compute_brightness(connector, level); pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100); panel->backlight.pwm_state.enabled = true; pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state); @@ -1232,10 +1239,8 @@ static u32 intel_panel_get_backlight(struct intel_connector *connector)
mutex_lock(&dev_priv->backlight_lock);
- if (panel->backlight.enabled) { + if (panel->backlight.enabled) val = panel->backlight.funcs->get(connector); - val = intel_panel_compute_brightness(connector, val); - }
mutex_unlock(&dev_priv->backlight_lock);
@@ -1566,13 +1571,13 @@ static u32 get_backlight_max_vbt(struct intel_connector *connector) u16 pwm_freq_hz = get_vbt_pwm_freq(dev_priv); u32 pwm;
- if (!panel->backlight.funcs->hz_to_pwm) { + if (!panel->backlight.pwm_funcs->hz_to_pwm) { drm_dbg_kms(&dev_priv->drm, "backlight frequency conversion not supported\n"); return 0; }
- pwm = panel->backlight.funcs->hz_to_pwm(connector, pwm_freq_hz); + pwm = panel->backlight.pwm_funcs->hz_to_pwm(connector, pwm_freq_hz); if (!pwm) { drm_dbg_kms(&dev_priv->drm, "backlight frequency conversion failed\n"); @@ -1591,7 +1596,7 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector) struct intel_panel *panel = &connector->panel; int min;
- drm_WARN_ON(&dev_priv->drm, panel->backlight.max == 0); + drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_max == 0);
/* * XXX: If the vbt value is 255, it makes min equal to max, which leads @@ -1608,7 +1613,7 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector) }
/* vbt value is a coefficient in range [0..255] */ - return scale(min, 0, 255, 0, panel->backlight.max); + return scale(min, 0, 255, 0, panel->backlight.pwm_max); }
static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unused) @@ -1628,37 +1633,32 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
pch_ctl2 = intel_de_read(dev_priv, BLC_PWM_PCH_CTL2); - panel->backlight.max = pch_ctl2 >> 16; + panel->backlight.pwm_max = pch_ctl2 >> 16;
cpu_ctl2 = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2);
- if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE; + panel->backlight.pwm_enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
- cpu_mode = panel->backlight.enabled && HAS_PCH_LPT(dev_priv) && + cpu_mode = panel->backlight.pwm_enabled && HAS_PCH_LPT(dev_priv) && !(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE) && (cpu_ctl2 & BLM_PWM_ENABLE); - if (cpu_mode) - val = pch_get_backlight(connector); - else - val = lpt_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max);
if (cpu_mode) { + val = intel_panel_sanitize_pwm_level(connector, pch_get_backlight(connector)); + drm_dbg_kms(&dev_priv->drm, "CPU backlight register was enabled, switching to PCH override\n");
/* Write converted CPU PWM value to PCH override register */ - lpt_set_backlight(connector->base.state, panel->backlight.level); + lpt_set_backlight(connector->base.state, val); intel_de_write(dev_priv, BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE);
@@ -1673,29 +1673,24 @@ static int pch_setup_backlight(struct intel_connector *connector, enum pipe unus { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - u32 cpu_ctl2, pch_ctl1, pch_ctl2, val; + u32 cpu_ctl2, pch_ctl1, pch_ctl2;
pch_ctl1 = intel_de_read(dev_priv, BLC_PWM_PCH_CTL1); panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
pch_ctl2 = intel_de_read(dev_priv, BLC_PWM_PCH_CTL2); - panel->backlight.max = pch_ctl2 >> 16; + panel->backlight.pwm_max = pch_ctl2 >> 16;
- if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector); - - val = pch_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + panel->backlight.pwm_min = get_backlight_min_vbt(connector);
cpu_ctl2 = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2); - panel->backlight.enabled = (cpu_ctl2 & BLM_PWM_ENABLE) && + panel->backlight.pwm_enabled = (cpu_ctl2 & BLM_PWM_ENABLE) && (pch_ctl1 & BLM_PCH_PWM_ENABLE);
return 0; @@ -1715,27 +1710,26 @@ static int i9xx_setup_backlight(struct intel_connector *connector, enum pipe unu if (IS_PINEVIEW(dev_priv)) panel->backlight.active_low_pwm = ctl & BLM_POLARITY_PNV;
- panel->backlight.max = ctl >> 17; + panel->backlight.pwm_max = ctl >> 17;
- if (!panel->backlight.max) { - panel->backlight.max = get_backlight_max_vbt(connector); - panel->backlight.max >>= 1; + if (!panel->backlight.pwm_max) { + panel->backlight.pwm_max = get_backlight_max_vbt(connector); + panel->backlight.pwm_max >>= 1; }
- if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV;
if (panel->backlight.combination_mode) - panel->backlight.max *= 0xff; + panel->backlight.pwm_max *= 0xff;
- panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_min = get_backlight_min_vbt(connector);
val = i9xx_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + val = intel_panel_sanitize_pwm_level(connector, val); + val = clamp(val, panel->backlight.pwm_min, panel->backlight.pwm_max);
- panel->backlight.enabled = val != 0; + panel->backlight.pwm_enabled = val != 0;
return 0; } @@ -1744,32 +1738,27 @@ static int i965_setup_backlight(struct intel_connector *connector, enum pipe unu { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - u32 ctl, ctl2, val; + u32 ctl, ctl2;
ctl2 = intel_de_read(dev_priv, BLC_PWM_CTL2); panel->backlight.combination_mode = ctl2 & BLM_COMBINATION_MODE; panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965;
ctl = intel_de_read(dev_priv, BLC_PWM_CTL); - panel->backlight.max = ctl >> 16; + panel->backlight.pwm_max = ctl >> 16;
- if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV;
if (panel->backlight.combination_mode) - panel->backlight.max *= 0xff; - - panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_max *= 0xff;
- val = i9xx_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE; + panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;
return 0; } @@ -1778,7 +1767,7 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - u32 ctl, ctl2, val; + u32 ctl, ctl2;
if (drm_WARN_ON(&dev_priv->drm, pipe != PIPE_A && pipe != PIPE_B)) return -ENODEV; @@ -1787,22 +1776,17 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965;
ctl = intel_de_read(dev_priv, VLV_BLC_PWM_CTL(pipe)); - panel->backlight.max = ctl >> 16; + panel->backlight.pwm_max = ctl >> 16;
- if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector); - - val = _vlv_get_backlight(dev_priv, pipe); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE; + panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;
return 0; } @@ -1827,24 +1811,18 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused) }
panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY; - panel->backlight.max = - intel_de_read(dev_priv, - BXT_BLC_PWM_FREQ(panel->backlight.controller)); + panel->backlight.pwm_max = intel_de_read(dev_priv, + BXT_BLC_PWM_FREQ(panel->backlight.controller));
- if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- val = bxt_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); - - panel->backlight.enabled = pwm_ctl & BXT_BLC_PWM_ENABLE; + panel->backlight.pwm_enabled = pwm_ctl & BXT_BLC_PWM_ENABLE;
return 0; } @@ -1854,7 +1832,7 @@ cnp_setup_backlight(struct intel_connector *connector, enum pipe unused) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - u32 pwm_ctl, val; + u32 pwm_ctl;
/* * CNP has the BXT implementation of backlight, but with only one @@ -1867,24 +1845,18 @@ cnp_setup_backlight(struct intel_connector *connector, enum pipe unused) BXT_BLC_PWM_CTL(panel->backlight.controller));
panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY; - panel->backlight.max = - intel_de_read(dev_priv, - BXT_BLC_PWM_FREQ(panel->backlight.controller)); + panel->backlight.pwm_max = intel_de_read(dev_priv, + BXT_BLC_PWM_FREQ(panel->backlight.controller));
- if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector); - - val = bxt_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- panel->backlight.enabled = pwm_ctl & BXT_BLC_PWM_ENABLE; + panel->backlight.pwm_enabled = pwm_ctl & BXT_BLC_PWM_ENABLE;
return 0; } @@ -1914,8 +1886,8 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector, return -ENODEV; }
- panel->backlight.max = 100; /* 100% */ - panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_max = 100; /* 100% */ + panel->backlight.pwm_min = get_backlight_min_vbt(connector);
if (pwm_is_enabled(panel->backlight.pwm)) { /* PWM is already enabled, use existing settings */ @@ -1923,10 +1895,8 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector,
level = pwm_get_relative_duty_cycle(&panel->backlight.pwm_state, 100); - level = intel_panel_compute_brightness(connector, level); - panel->backlight.level = clamp(level, panel->backlight.min, - panel->backlight.max); - panel->backlight.enabled = true; + level = intel_panel_sanitize_pwm_level(connector, level); + panel->backlight.pwm_enabled = true;
drm_dbg_kms(&dev_priv->drm, "PWM already enabled at freq %ld, VBT freq %d, level %d\n", NSEC_PER_SEC / (unsigned long)panel->backlight.pwm_state.period, @@ -1942,6 +1912,61 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector, return 0; }
+static void intel_pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state->connector); + struct intel_panel *panel = &connector->panel; + + panel->backlight.pwm_funcs->set(conn_state, + intel_panel_sanitize_pwm_level(connector, level)); +} + +static u32 intel_pwm_get_backlight(struct intel_connector *connector) +{ + struct intel_panel *panel = &connector->panel; + + return intel_panel_sanitize_pwm_level(connector, + panel->backlight.pwm_funcs->get(connector)); +} + +static void intel_pwm_enable_backlight(const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state->connector); + struct intel_panel *panel = &connector->panel; + + panel->backlight.pwm_funcs->enable(crtc_state, conn_state, + intel_panel_sanitize_pwm_level(connector, level)); +} + +static void intel_pwm_disable_backlight(const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state->connector); + struct intel_panel *panel = &connector->panel; + + panel->backlight.pwm_funcs->disable(conn_state, + intel_panel_sanitize_pwm_level(connector, level)); +} + +/* The only hook PWM-only backlight setups need to override, the rest of the hooks are filled in + * from pwm_funcs + */ +static int intel_pwm_setup_backlight(struct intel_connector *connector, enum pipe pipe) +{ + struct intel_panel *panel = &connector->panel; + int ret = panel->backlight.pwm_funcs->setup(connector, pipe); + + if (ret < 0) + return ret; + + panel->backlight.min = panel->backlight.pwm_min; + panel->backlight.max = panel->backlight.pwm_max; + panel->backlight.level = panel->backlight.funcs->get(connector); + panel->backlight.enabled = panel->backlight.pwm_enabled; + + return 0; +} + void intel_panel_update_backlight(struct intel_atomic_state *state, struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state, @@ -2015,7 +2040,7 @@ static void intel_panel_destroy_backlight(struct intel_panel *panel) panel->backlight.present = false; }
-static const struct intel_panel_bl_funcs bxt_funcs = { +static const struct intel_panel_bl_funcs bxt_pwm_funcs = { .setup = bxt_setup_backlight, .enable = bxt_enable_backlight, .disable = bxt_disable_backlight, @@ -2024,7 +2049,7 @@ static const struct intel_panel_bl_funcs bxt_funcs = { .hz_to_pwm = bxt_hz_to_pwm, };
-static const struct intel_panel_bl_funcs cnp_funcs = { +static const struct intel_panel_bl_funcs cnp_pwm_funcs = { .setup = cnp_setup_backlight, .enable = cnp_enable_backlight, .disable = cnp_disable_backlight, @@ -2033,7 +2058,7 @@ static const struct intel_panel_bl_funcs cnp_funcs = { .hz_to_pwm = cnp_hz_to_pwm, };
-static const struct intel_panel_bl_funcs lpt_funcs = { +static const struct intel_panel_bl_funcs lpt_pwm_funcs = { .setup = lpt_setup_backlight, .enable = lpt_enable_backlight, .disable = lpt_disable_backlight, @@ -2042,7 +2067,7 @@ static const struct intel_panel_bl_funcs lpt_funcs = { .hz_to_pwm = lpt_hz_to_pwm, };
-static const struct intel_panel_bl_funcs spt_funcs = { +static const struct intel_panel_bl_funcs spt_pwm_funcs = { .setup = lpt_setup_backlight, .enable = lpt_enable_backlight, .disable = lpt_disable_backlight, @@ -2051,7 +2076,7 @@ static const struct intel_panel_bl_funcs spt_funcs = { .hz_to_pwm = spt_hz_to_pwm, };
-static const struct intel_panel_bl_funcs pch_funcs = { +static const struct intel_panel_bl_funcs pch_pwm_funcs = { .setup = pch_setup_backlight, .enable = pch_enable_backlight, .disable = pch_disable_backlight, @@ -2068,7 +2093,7 @@ static const struct intel_panel_bl_funcs ext_pwm_funcs = { .get = ext_pwm_get_backlight, };
-static const struct intel_panel_bl_funcs vlv_funcs = { +static const struct intel_panel_bl_funcs vlv_pwm_funcs = { .setup = vlv_setup_backlight, .enable = vlv_enable_backlight, .disable = vlv_disable_backlight, @@ -2077,7 +2102,7 @@ static const struct intel_panel_bl_funcs vlv_funcs = { .hz_to_pwm = vlv_hz_to_pwm, };
-static const struct intel_panel_bl_funcs i965_funcs = { +static const struct intel_panel_bl_funcs i965_pwm_funcs = { .setup = i965_setup_backlight, .enable = i965_enable_backlight, .disable = i965_disable_backlight, @@ -2086,7 +2111,7 @@ static const struct intel_panel_bl_funcs i965_funcs = { .hz_to_pwm = i965_hz_to_pwm, };
-static const struct intel_panel_bl_funcs i9xx_funcs = { +static const struct intel_panel_bl_funcs i9xx_pwm_funcs = { .setup = i9xx_setup_backlight, .enable = i9xx_enable_backlight, .disable = i9xx_disable_backlight, @@ -2095,6 +2120,14 @@ static const struct intel_panel_bl_funcs i9xx_funcs = { .hz_to_pwm = i9xx_hz_to_pwm, };
+static const struct intel_panel_bl_funcs pwm_bl_funcs = { + .setup = intel_pwm_setup_backlight, + .enable = intel_pwm_enable_backlight, + .disable = intel_pwm_disable_backlight, + .set = intel_pwm_set_backlight, + .get = intel_pwm_get_backlight, +}; + /* Set up chip specific backlight functions */ static void intel_panel_init_backlight_funcs(struct intel_panel *panel) @@ -2103,36 +2136,39 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) container_of(panel, struct intel_connector, panel); struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
- if (connector->base.connector_type == DRM_MODE_CONNECTOR_eDP && - intel_dp_aux_init_backlight_funcs(connector) == 0) - return; - if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI && intel_dsi_dcs_init_backlight_funcs(connector) == 0) return;
if (IS_GEN9_LP(dev_priv)) { - panel->backlight.funcs = &bxt_funcs; + panel->backlight.pwm_funcs = &bxt_pwm_funcs; } else if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) { - panel->backlight.funcs = &cnp_funcs; + panel->backlight.pwm_funcs = &cnp_pwm_funcs; } else if (INTEL_PCH_TYPE(dev_priv) >= PCH_LPT) { if (HAS_PCH_LPT(dev_priv)) - panel->backlight.funcs = &lpt_funcs; + panel->backlight.pwm_funcs = &lpt_pwm_funcs; else - panel->backlight.funcs = &spt_funcs; + panel->backlight.pwm_funcs = &spt_pwm_funcs; } else if (HAS_PCH_SPLIT(dev_priv)) { - panel->backlight.funcs = &pch_funcs; + panel->backlight.pwm_funcs = &pch_pwm_funcs; } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI) { - panel->backlight.funcs = &ext_pwm_funcs; + panel->backlight.pwm_funcs = &ext_pwm_funcs; } else { - panel->backlight.funcs = &vlv_funcs; + panel->backlight.pwm_funcs = &vlv_pwm_funcs; } } else if (IS_GEN(dev_priv, 4)) { - panel->backlight.funcs = &i965_funcs; + panel->backlight.pwm_funcs = &i965_pwm_funcs; } else { - panel->backlight.funcs = &i9xx_funcs; + panel->backlight.pwm_funcs = &i9xx_pwm_funcs; } + + if (connector->base.connector_type == DRM_MODE_CONNECTOR_eDP && + intel_dp_aux_init_backlight_funcs(connector) == 0) + return; + + /* We're using a standard PWM backlight interface */ + panel->backlight.funcs = &pwm_bl_funcs; }
enum drm_connector_status
On Fri, 04 Dec 2020, Lyude Paul lyude@redhat.com wrote:
Currently, every different type of backlight hook that i915 supports is pretty straight forward - you have a backlight, probably through PWM (but maybe DPCD), with a single set of platform-specific hooks that are used for controlling it.
HDR backlights, in particular VESA and Intel's HDR backlight implementations, can end up being more complicated. With Intel's proprietary interface, HDR backlight controls always run through the DPCD. When the backlight is in SDR backlight mode however, the driver may need to bypass the TCON and control the backlight directly through PWM.
So, in order to support this we'll need to split our backlight callbacks into two groups: a set of high-level backlight control callbacks in intel_panel, and an additional set of pwm-specific backlight control callbacks. This also implies a functional changes for how these callbacks are used:
- We now keep track of two separate backlight level ranges, one for the high-level backlight, and one for the pwm backlight range
- We also keep track of backlight enablement and PWM backlight enablement separately
- Since the currently set backlight level might not be the same as the currently programmed PWM backlight level, we stop setting panel->backlight.level with the currently programmed PWM backlight level in panel->backlight.pwm_funcs->setup(). Instead, we rely on the higher level backlight control functions to retrieve the current PWM backlight level (in this case, intel_pwm_get_backlight()). Note that there are still a few PWM backlight setup callbacks that do actually need to retrieve the current PWM backlight level, although we no longer save this value in panel->backlight.level like before.
Additionally, we drop the call to lpt_get_backlight() in lpt_setup_backlight(), and avoid unconditionally writing the PWM value that we get from it and only write it back if we're in PCH mode. The reason for
Should be something like, "...only write it back if we're in CPU mode, and switching to PCH mode."
this is because in the original codepath for this, it was expected that the intel_panel_bl_funcs->setup() hook would be responsible for fetching the initial backlight level. On lpt systems, the only time we could ever be in PCH backlight mode is during the initial driver load - meaning that outside of the setup() hook, lpt_get_backlight() will always be the callback used for retrieving the current backlight level. After this patch we still need to fetch and write-back the PCH backlight value if we're in PCH mode, but
Ditto here. We may probe at CPU mode, set up by BIOS/GOP, but we always switch to PCH override mode. (The pch_ prefixed function naming is misleading...)
Historically there was a CPU register for pwm control. Then the functionality was moved to PCH but the register remained. Then there was a transition to PCH register, but the CPU register remained and used some low level internal messaging from CPU to PCH, unless PCH override was set. Finally the messaging was removed. Anyway, on the CPU+PCH combos where we can choose, we prefer using the PCH register directly ("PCH mode") and not use the CPU register with messaging. Simple made complex(tm).
because intel_pwm_setup_backlight() will retrieve the backlight level after setup() using the get() hook, which always ends up being lpt_get_backlight(). Thus - an additional call to lpt_get_backlight() in lpt_setup_backlight() is made redundant.
v3:
- Reuse intel_panel_bl_funcs() for pwm_funcs
- Explain why we drop lpt_get_backlight()
Signed-off-by: Lyude Paul lyude@redhat.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com
.../drm/i915/display/intel_display_types.h | 4 + drivers/gpu/drm/i915/display/intel_panel.c | 344 ++++++++++-------- 2 files changed, 194 insertions(+), 154 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index a70d1bf29aa5..47ee565c49a2 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -252,6 +252,9 @@ struct intel_panel { bool alternate_pwm_increment; /* lpt+ */
/* PWM chip */
u32 pwm_min;u32 pwm_max;bool pwm_enabled;@@ -263,6 +266,7 @@ struct intel_panel { struct backlight_device *device;
const struct intel_panel_bl_funcs *funcs;
const struct intel_panel_bl_funcs *pwm_funcs;}; diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index 67f81ae995c4..41f0d2b2c627 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -511,25 +511,34 @@ static u32 scale_hw_to_user(struct intel_connector *connector, 0, user_max); }
-static u32 intel_panel_compute_brightness(struct intel_connector *connector,
u32 val)+static u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 val) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel;
- drm_WARN_ON(&dev_priv->drm, panel->backlight.max == 0);
drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_max == 0);
if (dev_priv->params.invert_brightness < 0) return val;
if (dev_priv->params.invert_brightness > 0 || dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS) {
return panel->backlight.max - val + panel->backlight.min;
return panel->backlight.pwm_max - val + panel->backlight.pwm_min;}
return val;
}
+static void intel_panel_set_pwm_level(const struct drm_connector_state *conn_state, u32 val) +{
- struct intel_connector *connector = to_intel_connector(conn_state->connector);
- struct drm_i915_private *i915 = to_i915(connector->base.dev);
- struct intel_panel *panel = &connector->panel;
- drm_dbg_kms(&i915->drm, "set backlight PWM = %d\n", val);
- panel->backlight.pwm_funcs->set(conn_state, val);
+}
static u32 lpt_get_backlight(struct intel_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -624,12 +633,12 @@ static void i9xx_set_backlight(const struct drm_connector_state *conn_state, u32 struct intel_panel *panel = &connector->panel; u32 tmp, mask;
- drm_WARN_ON(&dev_priv->drm, panel->backlight.max == 0);
drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_max == 0);
if (panel->backlight.combination_mode) { u8 lbpc;
lbpc = level * 0xfe / panel->backlight.max + 1;
lbpc = level * 0xfe / panel->backlight.pwm_max + 1;@@ -681,9 +690,8 @@ intel_panel_actually_set_backlight(const struct drm_connector_state *conn_state, struct drm_i915_private *i915 = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel;
- drm_dbg_kms(&i915->drm, "set backlight PWM = %d\n", level);
- drm_dbg_kms(&i915->drm, "set backlight level = %d\n", level);
- level = intel_panel_compute_brightness(connector, level); panel->backlight.funcs->set(conn_state, level);
}
@@ -732,7 +740,7 @@ static void lpt_disable_backlight(const struct drm_connector_state *old_conn_sta struct drm_i915_private *dev_priv = to_i915(connector->base.dev); u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, level);
intel_panel_set_pwm_level(old_conn_state, level);
/*
- Although we don't support or enable CPU PWM with LPT/SPT based
@@ -760,7 +768,7 @@ static void pch_disable_backlight(const struct drm_connector_state *old_conn_sta struct drm_i915_private *dev_priv = to_i915(connector->base.dev); u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val);
intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2); intel_de_write(dev_priv, BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
@@ -771,7 +779,7 @@ static void pch_disable_backlight(const struct drm_connector_state *old_conn_sta
static void i9xx_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) {
- intel_panel_actually_set_backlight(old_conn_state, val);
- intel_panel_set_pwm_level(old_conn_state, val);
}
static void i965_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) @@ -779,7 +787,7 @@ static void i965_disable_backlight(const struct drm_connector_state *old_conn_st struct drm_i915_private *dev_priv = to_i915(old_conn_state->connector->dev); u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val);
intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, BLC_PWM_CTL2); intel_de_write(dev_priv, BLC_PWM_CTL2, tmp & ~BLM_PWM_ENABLE);
@@ -792,7 +800,7 @@ static void vlv_disable_backlight(const struct drm_connector_state *old_conn_sta enum pipe pipe = to_intel_crtc(old_conn_state->crtc)->pipe; u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val);
intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, VLV_BLC_PWM_CTL2(pipe)); intel_de_write(dev_priv, VLV_BLC_PWM_CTL2(pipe),
@@ -806,7 +814,7 @@ static void bxt_disable_backlight(const struct drm_connector_state *old_conn_sta struct intel_panel *panel = &connector->panel; u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val);
intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, BXT_BLC_PWM_CTL(panel->backlight.controller));
@@ -827,7 +835,7 @@ static void cnp_disable_backlight(const struct drm_connector_state *old_conn_sta struct intel_panel *panel = &connector->panel; u32 tmp;
- intel_panel_actually_set_backlight(old_conn_state, val);
intel_panel_set_pwm_level(old_conn_state, val);
tmp = intel_de_read(dev_priv, BXT_BLC_PWM_CTL(panel->backlight.controller));
@@ -906,7 +914,7 @@ static void lpt_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, SOUTH_CHICKEN1, schicken); }
- pch_ctl2 = panel->backlight.max << 16;
pch_ctl2 = panel->backlight.pwm_max << 16; intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2);
pch_ctl1 = 0;
@@ -923,7 +931,7 @@ static void lpt_enable_backlight(const struct intel_crtc_state *crtc_state, pch_ctl1 | BLM_PCH_PWM_ENABLE);
/* This won't stick until the above enable. */
- intel_panel_actually_set_backlight(conn_state, level);
- intel_panel_set_pwm_level(conn_state, level);
}
static void pch_enable_backlight(const struct intel_crtc_state *crtc_state, @@ -958,9 +966,9 @@ static void pch_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE);
/* This won't stick until the above enable. */
- intel_panel_actually_set_backlight(conn_state, level);
- intel_panel_set_pwm_level(conn_state, level);
- pch_ctl2 = panel->backlight.max << 16;
pch_ctl2 = panel->backlight.pwm_max << 16; intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2);
pch_ctl1 = 0;
@@ -987,7 +995,7 @@ static void i9xx_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CTL, 0); }
- freq = panel->backlight.max;
- freq = panel->backlight.pwm_max; if (panel->backlight.combination_mode) freq /= 0xff;
@@ -1001,7 +1009,7 @@ static void i9xx_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_posting_read(dev_priv, BLC_PWM_CTL);
/* XXX: combine this into above write? */
- intel_panel_actually_set_backlight(conn_state, level);
intel_panel_set_pwm_level(conn_state, level);
/*
- Needed to enable backlight on some 855gm models. BLC_HIST_CTL is
@@ -1028,7 +1036,7 @@ static void i965_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CTL2, ctl2); }
- freq = panel->backlight.max;
- freq = panel->backlight.pwm_max; if (panel->backlight.combination_mode) freq /= 0xff;
@@ -1044,7 +1052,7 @@ static void i965_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_posting_read(dev_priv, BLC_PWM_CTL2); intel_de_write(dev_priv, BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE);
- intel_panel_actually_set_backlight(conn_state, level);
- intel_panel_set_pwm_level(conn_state, level);
}
static void vlv_enable_backlight(const struct intel_crtc_state *crtc_state, @@ -1063,11 +1071,11 @@ static void vlv_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, VLV_BLC_PWM_CTL2(pipe), ctl2); }
- ctl = panel->backlight.max << 16;
ctl = panel->backlight.pwm_max << 16; intel_de_write(dev_priv, VLV_BLC_PWM_CTL(pipe), ctl);
/* XXX: combine this into above write? */
- intel_panel_actually_set_backlight(conn_state, level);
intel_panel_set_pwm_level(conn_state, level);
ctl2 = 0; if (panel->backlight.active_low_pwm)
@@ -1116,9 +1124,9 @@ static void bxt_enable_backlight(const struct intel_crtc_state *crtc_state,
intel_de_write(dev_priv, BXT_BLC_PWM_FREQ(panel->backlight.controller),
panel->backlight.max);
panel->backlight.pwm_max);
- intel_panel_actually_set_backlight(conn_state, level);
intel_panel_set_pwm_level(conn_state, level);
pwm_ctl = 0; if (panel->backlight.active_low_pwm)
@@ -1152,9 +1160,9 @@ static void cnp_enable_backlight(const struct intel_crtc_state *crtc_state,
intel_de_write(dev_priv, BXT_BLC_PWM_FREQ(panel->backlight.controller),
panel->backlight.max);
panel->backlight.pwm_max);
- intel_panel_actually_set_backlight(conn_state, level);
intel_panel_set_pwm_level(conn_state, level);
pwm_ctl = 0; if (panel->backlight.active_low_pwm)
@@ -1174,7 +1182,6 @@ static void ext_pwm_enable_backlight(const struct intel_crtc_state *crtc_state, struct intel_connector *connector = to_intel_connector(conn_state->connector); struct intel_panel *panel = &connector->panel;
- level = intel_panel_compute_brightness(connector, level); pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100); panel->backlight.pwm_state.enabled = true; pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
@@ -1232,10 +1239,8 @@ static u32 intel_panel_get_backlight(struct intel_connector *connector)
mutex_lock(&dev_priv->backlight_lock);
- if (panel->backlight.enabled) {
- if (panel->backlight.enabled) val = panel->backlight.funcs->get(connector);
val = intel_panel_compute_brightness(connector, val);}
mutex_unlock(&dev_priv->backlight_lock);
@@ -1566,13 +1571,13 @@ static u32 get_backlight_max_vbt(struct intel_connector *connector) u16 pwm_freq_hz = get_vbt_pwm_freq(dev_priv); u32 pwm;
- if (!panel->backlight.funcs->hz_to_pwm) {
- if (!panel->backlight.pwm_funcs->hz_to_pwm) { drm_dbg_kms(&dev_priv->drm, "backlight frequency conversion not supported\n"); return 0; }
- pwm = panel->backlight.funcs->hz_to_pwm(connector, pwm_freq_hz);
- pwm = panel->backlight.pwm_funcs->hz_to_pwm(connector, pwm_freq_hz); if (!pwm) { drm_dbg_kms(&dev_priv->drm, "backlight frequency conversion failed\n");
@@ -1591,7 +1596,7 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector) struct intel_panel *panel = &connector->panel; int min;
- drm_WARN_ON(&dev_priv->drm, panel->backlight.max == 0);
drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_max == 0);
/*
- XXX: If the vbt value is 255, it makes min equal to max, which leads
@@ -1608,7 +1613,7 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector) }
/* vbt value is a coefficient in range [0..255] */
- return scale(min, 0, 255, 0, panel->backlight.max);
- return scale(min, 0, 255, 0, panel->backlight.pwm_max);
}
static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unused) @@ -1628,37 +1633,32 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
pch_ctl2 = intel_de_read(dev_priv, BLC_PWM_PCH_CTL2);
- panel->backlight.max = pch_ctl2 >> 16;
panel->backlight.pwm_max = pch_ctl2 >> 16;
cpu_ctl2 = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2);
- if (!panel->backlight.max)
panel->backlight.max = get_backlight_max_vbt(connector);
- if (!panel->backlight.pwm_max)
panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max)
- if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector);
- panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
- panel->backlight.pwm_enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
- cpu_mode = panel->backlight.enabled && HAS_PCH_LPT(dev_priv) &&
- cpu_mode = panel->backlight.pwm_enabled && HAS_PCH_LPT(dev_priv) && !(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE) && (cpu_ctl2 & BLM_PWM_ENABLE);
if (cpu_mode)
val = pch_get_backlight(connector);else
val = lpt_get_backlight(connector);val = intel_panel_compute_brightness(connector, val);
panel->backlight.level = clamp(val, panel->backlight.min,
panel->backlight.max);if (cpu_mode) {
val = intel_panel_sanitize_pwm_level(connector, pch_get_backlight(connector));drm_dbg_kms(&dev_priv->drm, "CPU backlight register was enabled, switching to PCH override\n");
/* Write converted CPU PWM value to PCH override register */
lpt_set_backlight(connector->base.state, panel->backlight.level);
lpt_set_backlight(connector->base.state, val);@@ -1673,29 +1673,24 @@ static int pch_setup_backlight(struct intel_connector *connector, enum pipe unus { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel;
- u32 cpu_ctl2, pch_ctl1, pch_ctl2, val;
u32 cpu_ctl2, pch_ctl1, pch_ctl2;
pch_ctl1 = intel_de_read(dev_priv, BLC_PWM_PCH_CTL1); panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
pch_ctl2 = intel_de_read(dev_priv, BLC_PWM_PCH_CTL2);
- panel->backlight.max = pch_ctl2 >> 16;
- panel->backlight.pwm_max = pch_ctl2 >> 16;
- if (!panel->backlight.max)
panel->backlight.max = get_backlight_max_vbt(connector);
- if (!panel->backlight.pwm_max)
panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max)
- if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector);
- val = pch_get_backlight(connector);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
panel->backlight.max);
panel->backlight.pwm_min = get_backlight_min_vbt(connector);
cpu_ctl2 = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2);
- panel->backlight.enabled = (cpu_ctl2 & BLM_PWM_ENABLE) &&
panel->backlight.pwm_enabled = (cpu_ctl2 & BLM_PWM_ENABLE) && (pch_ctl1 & BLM_PCH_PWM_ENABLE);
return 0;
@@ -1715,27 +1710,26 @@ static int i9xx_setup_backlight(struct intel_connector *connector, enum pipe unu if (IS_PINEVIEW(dev_priv)) panel->backlight.active_low_pwm = ctl & BLM_POLARITY_PNV;
- panel->backlight.max = ctl >> 17;
- panel->backlight.pwm_max = ctl >> 17;
- if (!panel->backlight.max) {
panel->backlight.max = get_backlight_max_vbt(connector);panel->backlight.max >>= 1;
- if (!panel->backlight.pwm_max) {
panel->backlight.pwm_max = get_backlight_max_vbt(connector);panel->backlight.pwm_max >>= 1;
- if (!panel->backlight.max)
if (!panel->backlight.pwm_max) return -ENODEV;
if (panel->backlight.combination_mode)
panel->backlight.max *= 0xff;
panel->backlight.pwm_max *= 0xff;
- panel->backlight.min = get_backlight_min_vbt(connector);
panel->backlight.pwm_min = get_backlight_min_vbt(connector);
val = i9xx_get_backlight(connector);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
panel->backlight.max);
- val = intel_panel_sanitize_pwm_level(connector, val);
- val = clamp(val, panel->backlight.pwm_min, panel->backlight.pwm_max);
- panel->backlight.enabled = val != 0;
panel->backlight.pwm_enabled = val != 0;
return 0;
} @@ -1744,32 +1738,27 @@ static int i965_setup_backlight(struct intel_connector *connector, enum pipe unu { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel;
- u32 ctl, ctl2, val;
u32 ctl, ctl2;
ctl2 = intel_de_read(dev_priv, BLC_PWM_CTL2); panel->backlight.combination_mode = ctl2 & BLM_COMBINATION_MODE; panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965;
ctl = intel_de_read(dev_priv, BLC_PWM_CTL);
- panel->backlight.max = ctl >> 16;
- panel->backlight.pwm_max = ctl >> 16;
- if (!panel->backlight.max)
panel->backlight.max = get_backlight_max_vbt(connector);
- if (!panel->backlight.pwm_max)
panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max)
if (!panel->backlight.pwm_max) return -ENODEV;
if (panel->backlight.combination_mode)
panel->backlight.max *= 0xff;- panel->backlight.min = get_backlight_min_vbt(connector);
panel->backlight.pwm_max *= 0xff;
- val = i9xx_get_backlight(connector);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
panel->backlight.max);
- panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE;
panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;
return 0;
} @@ -1778,7 +1767,7 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel;
- u32 ctl, ctl2, val;
u32 ctl, ctl2;
if (drm_WARN_ON(&dev_priv->drm, pipe != PIPE_A && pipe != PIPE_B)) return -ENODEV;
@@ -1787,22 +1776,17 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965;
ctl = intel_de_read(dev_priv, VLV_BLC_PWM_CTL(pipe));
- panel->backlight.max = ctl >> 16;
- panel->backlight.pwm_max = ctl >> 16;
- if (!panel->backlight.max)
panel->backlight.max = get_backlight_max_vbt(connector);
- if (!panel->backlight.pwm_max)
panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max)
- if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector);
- val = _vlv_get_backlight(dev_priv, pipe);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
panel->backlight.max);
- panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE;
panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;
return 0;
} @@ -1827,24 +1811,18 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused) }
panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
- panel->backlight.max =
intel_de_read(dev_priv,BXT_BLC_PWM_FREQ(panel->backlight.controller));
- panel->backlight.pwm_max = intel_de_read(dev_priv,
BXT_BLC_PWM_FREQ(panel->backlight.controller));
- if (!panel->backlight.max)
panel->backlight.max = get_backlight_max_vbt(connector);
- if (!panel->backlight.pwm_max)
panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max)
- if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector);
- panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- val = bxt_get_backlight(connector);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
panel->backlight.max);- panel->backlight.enabled = pwm_ctl & BXT_BLC_PWM_ENABLE;
panel->backlight.pwm_enabled = pwm_ctl & BXT_BLC_PWM_ENABLE;
return 0;
} @@ -1854,7 +1832,7 @@ cnp_setup_backlight(struct intel_connector *connector, enum pipe unused) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel;
- u32 pwm_ctl, val;
u32 pwm_ctl;
/*
- CNP has the BXT implementation of backlight, but with only one
@@ -1867,24 +1845,18 @@ cnp_setup_backlight(struct intel_connector *connector, enum pipe unused) BXT_BLC_PWM_CTL(panel->backlight.controller));
panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
- panel->backlight.max =
intel_de_read(dev_priv,BXT_BLC_PWM_FREQ(panel->backlight.controller));
- panel->backlight.pwm_max = intel_de_read(dev_priv,
BXT_BLC_PWM_FREQ(panel->backlight.controller));
- if (!panel->backlight.max)
panel->backlight.max = get_backlight_max_vbt(connector);
- if (!panel->backlight.pwm_max)
panel->backlight.pwm_max = get_backlight_max_vbt(connector);
- if (!panel->backlight.max)
- if (!panel->backlight.pwm_max) return -ENODEV;
- panel->backlight.min = get_backlight_min_vbt(connector);
- val = bxt_get_backlight(connector);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
panel->backlight.max);
- panel->backlight.pwm_min = get_backlight_min_vbt(connector);
- panel->backlight.enabled = pwm_ctl & BXT_BLC_PWM_ENABLE;
panel->backlight.pwm_enabled = pwm_ctl & BXT_BLC_PWM_ENABLE;
return 0;
} @@ -1914,8 +1886,8 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector, return -ENODEV; }
- panel->backlight.max = 100; /* 100% */
- panel->backlight.min = get_backlight_min_vbt(connector);
panel->backlight.pwm_max = 100; /* 100% */
panel->backlight.pwm_min = get_backlight_min_vbt(connector);
if (pwm_is_enabled(panel->backlight.pwm)) { /* PWM is already enabled, use existing settings */
@@ -1923,10 +1895,8 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector,
level = pwm_get_relative_duty_cycle(&panel->backlight.pwm_state, 100);
level = intel_panel_compute_brightness(connector, level);panel->backlight.level = clamp(level, panel->backlight.min,panel->backlight.max);panel->backlight.enabled = true;
level = intel_panel_sanitize_pwm_level(connector, level);panel->backlight.pwm_enabled = true;drm_dbg_kms(&dev_priv->drm, "PWM already enabled at freq %ld, VBT freq %d, level %d\n", NSEC_PER_SEC / (unsigned long)panel->backlight.pwm_state.period,
@@ -1942,6 +1912,61 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector, return 0; }
+static void intel_pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level) +{
- struct intel_connector *connector = to_intel_connector(conn_state->connector);
- struct intel_panel *panel = &connector->panel;
- panel->backlight.pwm_funcs->set(conn_state,
intel_panel_sanitize_pwm_level(connector, level));+}
+static u32 intel_pwm_get_backlight(struct intel_connector *connector) +{
- struct intel_panel *panel = &connector->panel;
- return intel_panel_sanitize_pwm_level(connector,
panel->backlight.pwm_funcs->get(connector));+}
+static void intel_pwm_enable_backlight(const struct intel_crtc_state *crtc_state,
const struct drm_connector_state *conn_state, u32 level)+{
- struct intel_connector *connector = to_intel_connector(conn_state->connector);
- struct intel_panel *panel = &connector->panel;
- panel->backlight.pwm_funcs->enable(crtc_state, conn_state,
intel_panel_sanitize_pwm_level(connector, level));+}
+static void intel_pwm_disable_backlight(const struct drm_connector_state *conn_state, u32 level) +{
- struct intel_connector *connector = to_intel_connector(conn_state->connector);
- struct intel_panel *panel = &connector->panel;
- panel->backlight.pwm_funcs->disable(conn_state,
intel_panel_sanitize_pwm_level(connector, level));+}
+/* The only hook PWM-only backlight setups need to override, the rest of the hooks are filled in
- from pwm_funcs
- */
Isn't this comment stale now?
+static int intel_pwm_setup_backlight(struct intel_connector *connector, enum pipe pipe) +{
- struct intel_panel *panel = &connector->panel;
- int ret = panel->backlight.pwm_funcs->setup(connector, pipe);
- if (ret < 0)
return ret;- panel->backlight.min = panel->backlight.pwm_min;
- panel->backlight.max = panel->backlight.pwm_max;
- panel->backlight.level = panel->backlight.funcs->get(connector);
Brain hurts with the two value "domains". I'm wondering if we should start to follow some naming convention, for example "pwm_val" or "pwm_level" for everything that is in the "pwm domain". It would be easier to follow.
Perhaps the level should be set with just direct intel_pwm_get_backlight() to avoid an unnecessary indirection here?
- panel->backlight.enabled = panel->backlight.pwm_enabled;
- return 0;
+}
void intel_panel_update_backlight(struct intel_atomic_state *state, struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state, @@ -2015,7 +2040,7 @@ static void intel_panel_destroy_backlight(struct intel_panel *panel) panel->backlight.present = false; }
-static const struct intel_panel_bl_funcs bxt_funcs = { +static const struct intel_panel_bl_funcs bxt_pwm_funcs = { .setup = bxt_setup_backlight, .enable = bxt_enable_backlight, .disable = bxt_disable_backlight, @@ -2024,7 +2049,7 @@ static const struct intel_panel_bl_funcs bxt_funcs = { .hz_to_pwm = bxt_hz_to_pwm, };
-static const struct intel_panel_bl_funcs cnp_funcs = { +static const struct intel_panel_bl_funcs cnp_pwm_funcs = { .setup = cnp_setup_backlight, .enable = cnp_enable_backlight, .disable = cnp_disable_backlight, @@ -2033,7 +2058,7 @@ static const struct intel_panel_bl_funcs cnp_funcs = { .hz_to_pwm = cnp_hz_to_pwm, };
-static const struct intel_panel_bl_funcs lpt_funcs = { +static const struct intel_panel_bl_funcs lpt_pwm_funcs = { .setup = lpt_setup_backlight, .enable = lpt_enable_backlight, .disable = lpt_disable_backlight, @@ -2042,7 +2067,7 @@ static const struct intel_panel_bl_funcs lpt_funcs = { .hz_to_pwm = lpt_hz_to_pwm, };
-static const struct intel_panel_bl_funcs spt_funcs = { +static const struct intel_panel_bl_funcs spt_pwm_funcs = { .setup = lpt_setup_backlight, .enable = lpt_enable_backlight, .disable = lpt_disable_backlight, @@ -2051,7 +2076,7 @@ static const struct intel_panel_bl_funcs spt_funcs = { .hz_to_pwm = spt_hz_to_pwm, };
-static const struct intel_panel_bl_funcs pch_funcs = { +static const struct intel_panel_bl_funcs pch_pwm_funcs = { .setup = pch_setup_backlight, .enable = pch_enable_backlight, .disable = pch_disable_backlight, @@ -2068,7 +2093,7 @@ static const struct intel_panel_bl_funcs ext_pwm_funcs = { .get = ext_pwm_get_backlight, };
-static const struct intel_panel_bl_funcs vlv_funcs = { +static const struct intel_panel_bl_funcs vlv_pwm_funcs = { .setup = vlv_setup_backlight, .enable = vlv_enable_backlight, .disable = vlv_disable_backlight, @@ -2077,7 +2102,7 @@ static const struct intel_panel_bl_funcs vlv_funcs = { .hz_to_pwm = vlv_hz_to_pwm, };
-static const struct intel_panel_bl_funcs i965_funcs = { +static const struct intel_panel_bl_funcs i965_pwm_funcs = { .setup = i965_setup_backlight, .enable = i965_enable_backlight, .disable = i965_disable_backlight, @@ -2086,7 +2111,7 @@ static const struct intel_panel_bl_funcs i965_funcs = { .hz_to_pwm = i965_hz_to_pwm, };
-static const struct intel_panel_bl_funcs i9xx_funcs = { +static const struct intel_panel_bl_funcs i9xx_pwm_funcs = { .setup = i9xx_setup_backlight, .enable = i9xx_enable_backlight, .disable = i9xx_disable_backlight, @@ -2095,6 +2120,14 @@ static const struct intel_panel_bl_funcs i9xx_funcs = { .hz_to_pwm = i9xx_hz_to_pwm, };
+static const struct intel_panel_bl_funcs pwm_bl_funcs = {
- .setup = intel_pwm_setup_backlight,
- .enable = intel_pwm_enable_backlight,
- .disable = intel_pwm_disable_backlight,
- .set = intel_pwm_set_backlight,
- .get = intel_pwm_get_backlight,
+};
/* Set up chip specific backlight functions */ static void intel_panel_init_backlight_funcs(struct intel_panel *panel) @@ -2103,36 +2136,39 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) container_of(panel, struct intel_connector, panel); struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
- if (connector->base.connector_type == DRM_MODE_CONNECTOR_eDP &&
intel_dp_aux_init_backlight_funcs(connector) == 0)return;
I think I'd keep this here in this patch. It helps with the interpretation of the change here, i.e. we're not starting to utilize the two levels just yet.
if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI && intel_dsi_dcs_init_backlight_funcs(connector) == 0) return;
if (IS_GEN9_LP(dev_priv)) {
panel->backlight.funcs = &bxt_funcs;
panel->backlight.pwm_funcs = &bxt_pwm_funcs;
panel->backlight.funcs = &cnp_funcs;
panel->backlight.pwm_funcs = &cnp_pwm_funcs;
panel->backlight.funcs = &lpt_funcs;
panel->backlight.pwm_funcs = &lpt_pwm_funcs;
panel->backlight.funcs = &spt_funcs;
panel->backlight.pwm_funcs = &spt_pwm_funcs;
panel->backlight.funcs = &pch_funcs;
panel->backlight.pwm_funcs = &pch_pwm_funcs;
panel->backlight.funcs = &ext_pwm_funcs;
panel->backlight.pwm_funcs = &ext_pwm_funcs;
panel->backlight.funcs = &vlv_funcs;
panel->backlight.pwm_funcs = &vlv_pwm_funcs;
panel->backlight.funcs = &i965_funcs;
panel->backlight.pwm_funcs = &i965_pwm_funcs;
panel->backlight.funcs = &i9xx_funcs;
panel->backlight.pwm_funcs = &i9xx_pwm_funcs;- if (connector->base.connector_type == DRM_MODE_CONNECTOR_eDP &&
intel_dp_aux_init_backlight_funcs(connector) == 0)return;- /* We're using a standard PWM backlight interface */
- panel->backlight.funcs = &pwm_bl_funcs;
}
enum drm_connector_status
On Wed, 2020-12-23 at 18:37 +0200, Jani Nikula wrote:
On Fri, 04 Dec 2020, Lyude Paul lyude@redhat.com wrote:
Currently, every different type of backlight hook that i915 supports is pretty straight forward - you have a backlight, probably through PWM (but maybe DPCD), with a single set of platform-specific hooks that are used for controlling it.
HDR backlights, in particular VESA and Intel's HDR backlight implementations, can end up being more complicated. With Intel's proprietary interface, HDR backlight controls always run through the DPCD. When the backlight is in SDR backlight mode however, the driver may need to bypass the TCON and control the backlight directly through PWM.
So, in order to support this we'll need to split our backlight callbacks into two groups: a set of high-level backlight control callbacks in intel_panel, and an additional set of pwm-specific backlight control callbacks. This also implies a functional changes for how these callbacks are used:
- We now keep track of two separate backlight level ranges, one for the
high-level backlight, and one for the pwm backlight range
- We also keep track of backlight enablement and PWM backlight
enablement separately
- Since the currently set backlight level might not be the same as the
currently programmed PWM backlight level, we stop setting panel->backlight.level with the currently programmed PWM backlight level in panel->backlight.pwm_funcs->setup(). Instead, we rely on the higher level backlight control functions to retrieve the current PWM backlight level (in this case, intel_pwm_get_backlight()). Note that there are still a few PWM backlight setup callbacks that do actually need to retrieve the current PWM backlight level, although we no longer save this value in panel->backlight.level like before.
Additionally, we drop the call to lpt_get_backlight() in lpt_setup_backlight(), and avoid unconditionally writing the PWM value that we get from it and only write it back if we're in PCH mode. The reason for
Should be something like, "...only write it back if we're in CPU mode, and switching to PCH mode."
this is because in the original codepath for this, it was expected that the intel_panel_bl_funcs->setup() hook would be responsible for fetching the initial backlight level. On lpt systems, the only time we could ever be in PCH backlight mode is during the initial driver load - meaning that outside of the setup() hook, lpt_get_backlight() will always be the callback used for retrieving the current backlight level. After this patch we still need to fetch and write-back the PCH backlight value if we're in PCH mode, but
Ditto here. We may probe at CPU mode, set up by BIOS/GOP, but we always switch to PCH override mode. (The pch_ prefixed function naming is misleading...)
Historically there was a CPU register for pwm control. Then the functionality was moved to PCH but the register remained. Then there was a transition to PCH register, but the CPU register remained and used some low level internal messaging from CPU to PCH, unless PCH override was set. Finally the messaging was removed. Anyway, on the CPU+PCH combos where we can choose, we prefer using the PCH register directly ("PCH mode") and not use the CPU register with messaging. Simple made complex(tm).
because intel_pwm_setup_backlight() will retrieve the backlight level after setup() using the get() hook, which always ends up being lpt_get_backlight(). Thus - an additional call to lpt_get_backlight() in lpt_setup_backlight() is made redundant.
v3:
- Reuse intel_panel_bl_funcs() for pwm_funcs
- Explain why we drop lpt_get_backlight()
Signed-off-by: Lyude Paul lyude@redhat.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com
.../drm/i915/display/intel_display_types.h | 4 + drivers/gpu/drm/i915/display/intel_panel.c | 344 ++++++++++-------- 2 files changed, 194 insertions(+), 154 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index a70d1bf29aa5..47ee565c49a2 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -252,6 +252,9 @@ struct intel_panel { bool alternate_pwm_increment; /* lpt+ */ /* PWM chip */ + u32 pwm_min; + u32 pwm_max; + bool pwm_enabled; bool util_pin_active_low; /* bxt+ */ u8 controller; /* bxt+ only */ struct pwm_device *pwm; @@ -263,6 +266,7 @@ struct intel_panel { struct backlight_device *device; const struct intel_panel_bl_funcs *funcs; + const struct intel_panel_bl_funcs *pwm_funcs; void (*power)(struct intel_connector *, bool enable); } backlight; }; diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index 67f81ae995c4..41f0d2b2c627 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -511,25 +511,34 @@ static u32 scale_hw_to_user(struct intel_connector *connector, 0, user_max); } -static u32 intel_panel_compute_brightness(struct intel_connector *connector, - u32 val) +static u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 val) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - drm_WARN_ON(&dev_priv->drm, panel->backlight.max == 0); + drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_max == 0); if (dev_priv->params.invert_brightness < 0) return val; if (dev_priv->params.invert_brightness > 0 || dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS) { - return panel->backlight.max - val + panel->backlight.min; + return panel->backlight.pwm_max - val + panel-
backlight.pwm_min;
} return val; } +static void intel_panel_set_pwm_level(const struct drm_connector_state *conn_state, u32 val) +{ + struct intel_connector *connector = to_intel_connector(conn_state-
connector);
+ struct drm_i915_private *i915 = to_i915(connector->base.dev); + struct intel_panel *panel = &connector->panel;
+ drm_dbg_kms(&i915->drm, "set backlight PWM = %d\n", val); + panel->backlight.pwm_funcs->set(conn_state, val); +}
static u32 lpt_get_backlight(struct intel_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); @@ -624,12 +633,12 @@ static void i9xx_set_backlight(const struct drm_connector_state *conn_state, u32 struct intel_panel *panel = &connector->panel; u32 tmp, mask; - drm_WARN_ON(&dev_priv->drm, panel->backlight.max == 0); + drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_max == 0); if (panel->backlight.combination_mode) { u8 lbpc; - lbpc = level * 0xfe / panel->backlight.max + 1; + lbpc = level * 0xfe / panel->backlight.pwm_max + 1; level /= lbpc; pci_write_config_byte(dev_priv->drm.pdev, LBPC, lbpc); } @@ -681,9 +690,8 @@ intel_panel_actually_set_backlight(const struct drm_connector_state *conn_state, struct drm_i915_private *i915 = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - drm_dbg_kms(&i915->drm, "set backlight PWM = %d\n", level); + drm_dbg_kms(&i915->drm, "set backlight level = %d\n", level); - level = intel_panel_compute_brightness(connector, level); panel->backlight.funcs->set(conn_state, level); } @@ -732,7 +740,7 @@ static void lpt_disable_backlight(const struct drm_connector_state *old_conn_sta struct drm_i915_private *dev_priv = to_i915(connector->base.dev); u32 tmp; - intel_panel_actually_set_backlight(old_conn_state, level); + intel_panel_set_pwm_level(old_conn_state, level); /* * Although we don't support or enable CPU PWM with LPT/SPT based @@ -760,7 +768,7 @@ static void pch_disable_backlight(const struct drm_connector_state *old_conn_sta struct drm_i915_private *dev_priv = to_i915(connector->base.dev); u32 tmp; - intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val); tmp = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2); intel_de_write(dev_priv, BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE); @@ -771,7 +779,7 @@ static void pch_disable_backlight(const struct drm_connector_state *old_conn_sta static void i9xx_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) { - intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val); } static void i965_disable_backlight(const struct drm_connector_state *old_conn_state, u32 val) @@ -779,7 +787,7 @@ static void i965_disable_backlight(const struct drm_connector_state *old_conn_st struct drm_i915_private *dev_priv = to_i915(old_conn_state-
connector->dev);
u32 tmp; - intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val); tmp = intel_de_read(dev_priv, BLC_PWM_CTL2); intel_de_write(dev_priv, BLC_PWM_CTL2, tmp & ~BLM_PWM_ENABLE); @@ -792,7 +800,7 @@ static void vlv_disable_backlight(const struct drm_connector_state *old_conn_sta enum pipe pipe = to_intel_crtc(old_conn_state->crtc)->pipe; u32 tmp; - intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val); tmp = intel_de_read(dev_priv, VLV_BLC_PWM_CTL2(pipe)); intel_de_write(dev_priv, VLV_BLC_PWM_CTL2(pipe), @@ -806,7 +814,7 @@ static void bxt_disable_backlight(const struct drm_connector_state *old_conn_sta struct intel_panel *panel = &connector->panel; u32 tmp; - intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val); tmp = intel_de_read(dev_priv, BXT_BLC_PWM_CTL(panel->backlight.controller)); @@ -827,7 +835,7 @@ static void cnp_disable_backlight(const struct drm_connector_state *old_conn_sta struct intel_panel *panel = &connector->panel; u32 tmp; - intel_panel_actually_set_backlight(old_conn_state, val); + intel_panel_set_pwm_level(old_conn_state, val); tmp = intel_de_read(dev_priv, BXT_BLC_PWM_CTL(panel->backlight.controller)); @@ -906,7 +914,7 @@ static void lpt_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, SOUTH_CHICKEN1, schicken); } - pch_ctl2 = panel->backlight.max << 16; + pch_ctl2 = panel->backlight.pwm_max << 16; intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2); pch_ctl1 = 0; @@ -923,7 +931,7 @@ static void lpt_enable_backlight(const struct intel_crtc_state *crtc_state, pch_ctl1 | BLM_PCH_PWM_ENABLE); /* This won't stick until the above enable. */ - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level); } static void pch_enable_backlight(const struct intel_crtc_state *crtc_state, @@ -958,9 +966,9 @@ static void pch_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE); /* This won't stick until the above enable. */ - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level); - pch_ctl2 = panel->backlight.max << 16; + pch_ctl2 = panel->backlight.pwm_max << 16; intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2); pch_ctl1 = 0; @@ -987,7 +995,7 @@ static void i9xx_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CTL, 0); } - freq = panel->backlight.max; + freq = panel->backlight.pwm_max; if (panel->backlight.combination_mode) freq /= 0xff; @@ -1001,7 +1009,7 @@ static void i9xx_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_posting_read(dev_priv, BLC_PWM_CTL); /* XXX: combine this into above write? */ - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level); /* * Needed to enable backlight on some 855gm models. BLC_HIST_CTL is @@ -1028,7 +1036,7 @@ static void i965_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BLC_PWM_CTL2, ctl2); } - freq = panel->backlight.max; + freq = panel->backlight.pwm_max; if (panel->backlight.combination_mode) freq /= 0xff; @@ -1044,7 +1052,7 @@ static void i965_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_posting_read(dev_priv, BLC_PWM_CTL2); intel_de_write(dev_priv, BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE); - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level); } static void vlv_enable_backlight(const struct intel_crtc_state *crtc_state, @@ -1063,11 +1071,11 @@ static void vlv_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, VLV_BLC_PWM_CTL2(pipe), ctl2); } - ctl = panel->backlight.max << 16; + ctl = panel->backlight.pwm_max << 16; intel_de_write(dev_priv, VLV_BLC_PWM_CTL(pipe), ctl); /* XXX: combine this into above write? */ - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level); ctl2 = 0; if (panel->backlight.active_low_pwm) @@ -1116,9 +1124,9 @@ static void bxt_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BXT_BLC_PWM_FREQ(panel->backlight.controller), - panel->backlight.max); + panel->backlight.pwm_max); - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level); pwm_ctl = 0; if (panel->backlight.active_low_pwm) @@ -1152,9 +1160,9 @@ static void cnp_enable_backlight(const struct intel_crtc_state *crtc_state, intel_de_write(dev_priv, BXT_BLC_PWM_FREQ(panel->backlight.controller), - panel->backlight.max); + panel->backlight.pwm_max); - intel_panel_actually_set_backlight(conn_state, level); + intel_panel_set_pwm_level(conn_state, level); pwm_ctl = 0; if (panel->backlight.active_low_pwm) @@ -1174,7 +1182,6 @@ static void ext_pwm_enable_backlight(const struct intel_crtc_state *crtc_state, struct intel_connector *connector = to_intel_connector(conn_state-
connector);
struct intel_panel *panel = &connector->panel; - level = intel_panel_compute_brightness(connector, level); pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100); panel->backlight.pwm_state.enabled = true; pwm_apply_state(panel->backlight.pwm, &panel-
backlight.pwm_state);
@@ -1232,10 +1239,8 @@ static u32 intel_panel_get_backlight(struct intel_connector *connector) mutex_lock(&dev_priv->backlight_lock); - if (panel->backlight.enabled) { + if (panel->backlight.enabled) val = panel->backlight.funcs->get(connector); - val = intel_panel_compute_brightness(connector, val); - } mutex_unlock(&dev_priv->backlight_lock); @@ -1566,13 +1571,13 @@ static u32 get_backlight_max_vbt(struct intel_connector *connector) u16 pwm_freq_hz = get_vbt_pwm_freq(dev_priv); u32 pwm; - if (!panel->backlight.funcs->hz_to_pwm) { + if (!panel->backlight.pwm_funcs->hz_to_pwm) { drm_dbg_kms(&dev_priv->drm, "backlight frequency conversion not supported\n"); return 0; } - pwm = panel->backlight.funcs->hz_to_pwm(connector, pwm_freq_hz); + pwm = panel->backlight.pwm_funcs->hz_to_pwm(connector, pwm_freq_hz); if (!pwm) { drm_dbg_kms(&dev_priv->drm, "backlight frequency conversion failed\n"); @@ -1591,7 +1596,7 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector) struct intel_panel *panel = &connector->panel; int min; - drm_WARN_ON(&dev_priv->drm, panel->backlight.max == 0); + drm_WARN_ON(&dev_priv->drm, panel->backlight.pwm_max == 0); /* * XXX: If the vbt value is 255, it makes min equal to max, which leads @@ -1608,7 +1613,7 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector) } /* vbt value is a coefficient in range [0..255] */ - return scale(min, 0, 255, 0, panel->backlight.max); + return scale(min, 0, 255, 0, panel->backlight.pwm_max); } static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unused) @@ -1628,37 +1633,32 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY; pch_ctl2 = intel_de_read(dev_priv, BLC_PWM_PCH_CTL2); - panel->backlight.max = pch_ctl2 >> 16; + panel->backlight.pwm_max = pch_ctl2 >> 16; cpu_ctl2 = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2); - if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector); - if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV; - panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_min = get_backlight_min_vbt(connector); - panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE; + panel->backlight.pwm_enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE; - cpu_mode = panel->backlight.enabled && HAS_PCH_LPT(dev_priv) && + cpu_mode = panel->backlight.pwm_enabled && HAS_PCH_LPT(dev_priv) && !(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE) && (cpu_ctl2 & BLM_PWM_ENABLE); - if (cpu_mode) - val = pch_get_backlight(connector); - else - val = lpt_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); if (cpu_mode) { + val = intel_panel_sanitize_pwm_level(connector, pch_get_backlight(connector));
drm_dbg_kms(&dev_priv->drm, "CPU backlight register was enabled, switching to PCH override\n"); /* Write converted CPU PWM value to PCH override register */ - lpt_set_backlight(connector->base.state, panel-
backlight.level);
+ lpt_set_backlight(connector->base.state, val); intel_de_write(dev_priv, BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE); @@ -1673,29 +1673,24 @@ static int pch_setup_backlight(struct intel_connector *connector, enum pipe unus { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - u32 cpu_ctl2, pch_ctl1, pch_ctl2, val; + u32 cpu_ctl2, pch_ctl1, pch_ctl2; pch_ctl1 = intel_de_read(dev_priv, BLC_PWM_PCH_CTL1); panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY; pch_ctl2 = intel_de_read(dev_priv, BLC_PWM_PCH_CTL2); - panel->backlight.max = pch_ctl2 >> 16; + panel->backlight.pwm_max = pch_ctl2 >> 16; - if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector); - if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV; - panel->backlight.min = get_backlight_min_vbt(connector);
- val = pch_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + panel->backlight.pwm_min = get_backlight_min_vbt(connector); cpu_ctl2 = intel_de_read(dev_priv, BLC_PWM_CPU_CTL2); - panel->backlight.enabled = (cpu_ctl2 & BLM_PWM_ENABLE) && + panel->backlight.pwm_enabled = (cpu_ctl2 & BLM_PWM_ENABLE) && (pch_ctl1 & BLM_PCH_PWM_ENABLE); return 0; @@ -1715,27 +1710,26 @@ static int i9xx_setup_backlight(struct intel_connector *connector, enum pipe unu if (IS_PINEVIEW(dev_priv)) panel->backlight.active_low_pwm = ctl & BLM_POLARITY_PNV; - panel->backlight.max = ctl >> 17; + panel->backlight.pwm_max = ctl >> 17; - if (!panel->backlight.max) { - panel->backlight.max = get_backlight_max_vbt(connector); - panel->backlight.max >>= 1; + if (!panel->backlight.pwm_max) { + panel->backlight.pwm_max = get_backlight_max_vbt(connector); + panel->backlight.pwm_max >>= 1; } - if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV; if (panel->backlight.combination_mode) - panel->backlight.max *= 0xff; + panel->backlight.pwm_max *= 0xff; - panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_min = get_backlight_min_vbt(connector); val = i9xx_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + val = intel_panel_sanitize_pwm_level(connector, val); + val = clamp(val, panel->backlight.pwm_min, panel-
backlight.pwm_max);
- panel->backlight.enabled = val != 0; + panel->backlight.pwm_enabled = val != 0; return 0; } @@ -1744,32 +1738,27 @@ static int i965_setup_backlight(struct intel_connector *connector, enum pipe unu { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - u32 ctl, ctl2, val; + u32 ctl, ctl2; ctl2 = intel_de_read(dev_priv, BLC_PWM_CTL2); panel->backlight.combination_mode = ctl2 & BLM_COMBINATION_MODE; panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965; ctl = intel_de_read(dev_priv, BLC_PWM_CTL); - panel->backlight.max = ctl >> 16; + panel->backlight.pwm_max = ctl >> 16; - if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector); - if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV; if (panel->backlight.combination_mode) - panel->backlight.max *= 0xff;
- panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_max *= 0xff; - val = i9xx_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + panel->backlight.pwm_min = get_backlight_min_vbt(connector); - panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE; + panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE; return 0; } @@ -1778,7 +1767,7 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - u32 ctl, ctl2, val; + u32 ctl, ctl2; if (drm_WARN_ON(&dev_priv->drm, pipe != PIPE_A && pipe != PIPE_B)) return -ENODEV; @@ -1787,22 +1776,17 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965; ctl = intel_de_read(dev_priv, VLV_BLC_PWM_CTL(pipe)); - panel->backlight.max = ctl >> 16; + panel->backlight.pwm_max = ctl >> 16; - if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector); - if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV; - panel->backlight.min = get_backlight_min_vbt(connector);
- val = _vlv_get_backlight(dev_priv, pipe); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + panel->backlight.pwm_min = get_backlight_min_vbt(connector); - panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE; + panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE; return 0; } @@ -1827,24 +1811,18 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused) } panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY; - panel->backlight.max = - intel_de_read(dev_priv, - BXT_BLC_PWM_FREQ(panel-
backlight.controller));
+ panel->backlight.pwm_max = intel_de_read(dev_priv, + BXT_BLC_PWM_FREQ(panel-
backlight.controller));
- if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector); - if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV; - panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_min = get_backlight_min_vbt(connector); - val = bxt_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max);
- panel->backlight.enabled = pwm_ctl & BXT_BLC_PWM_ENABLE; + panel->backlight.pwm_enabled = pwm_ctl & BXT_BLC_PWM_ENABLE; return 0; } @@ -1854,7 +1832,7 @@ cnp_setup_backlight(struct intel_connector *connector, enum pipe unused) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; - u32 pwm_ctl, val; + u32 pwm_ctl; /* * CNP has the BXT implementation of backlight, but with only one @@ -1867,24 +1845,18 @@ cnp_setup_backlight(struct intel_connector *connector, enum pipe unused) BXT_BLC_PWM_CTL(panel-
backlight.controller));
panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY; - panel->backlight.max = - intel_de_read(dev_priv, - BXT_BLC_PWM_FREQ(panel-
backlight.controller));
+ panel->backlight.pwm_max = intel_de_read(dev_priv, + BXT_BLC_PWM_FREQ(panel-
backlight.controller));
- if (!panel->backlight.max) - panel->backlight.max = get_backlight_max_vbt(connector); + if (!panel->backlight.pwm_max) + panel->backlight.pwm_max = get_backlight_max_vbt(connector); - if (!panel->backlight.max) + if (!panel->backlight.pwm_max) return -ENODEV; - panel->backlight.min = get_backlight_min_vbt(connector);
- val = bxt_get_backlight(connector); - val = intel_panel_compute_brightness(connector, val); - panel->backlight.level = clamp(val, panel->backlight.min, - panel->backlight.max); + panel->backlight.pwm_min = get_backlight_min_vbt(connector); - panel->backlight.enabled = pwm_ctl & BXT_BLC_PWM_ENABLE; + panel->backlight.pwm_enabled = pwm_ctl & BXT_BLC_PWM_ENABLE; return 0; } @@ -1914,8 +1886,8 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector, return -ENODEV; } - panel->backlight.max = 100; /* 100% */ - panel->backlight.min = get_backlight_min_vbt(connector); + panel->backlight.pwm_max = 100; /* 100% */ + panel->backlight.pwm_min = get_backlight_min_vbt(connector); if (pwm_is_enabled(panel->backlight.pwm)) { /* PWM is already enabled, use existing settings */ @@ -1923,10 +1895,8 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector, level = pwm_get_relative_duty_cycle(&panel-
backlight.pwm_state,
100); - level = intel_panel_compute_brightness(connector, level); - panel->backlight.level = clamp(level, panel-
backlight.min,
- panel->backlight.max); - panel->backlight.enabled = true; + level = intel_panel_sanitize_pwm_level(connector, level); + panel->backlight.pwm_enabled = true; drm_dbg_kms(&dev_priv->drm, "PWM already enabled at freq %ld, VBT freq %d, level %d\n", NSEC_PER_SEC / (unsigned long)panel-
backlight.pwm_state.period,
@@ -1942,6 +1912,61 @@ static int ext_pwm_setup_backlight(struct intel_connector *connector, return 0; } +static void intel_pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state-
connector);
+ struct intel_panel *panel = &connector->panel;
+ panel->backlight.pwm_funcs->set(conn_state, + intel_panel_sanitize_pwm_level(connector, level)); +}
+static u32 intel_pwm_get_backlight(struct intel_connector *connector) +{ + struct intel_panel *panel = &connector->panel;
+ return intel_panel_sanitize_pwm_level(connector, + panel->backlight.pwm_funcs-
get(connector));
+}
+static void intel_pwm_enable_backlight(const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state-
connector);
+ struct intel_panel *panel = &connector->panel;
+ panel->backlight.pwm_funcs->enable(crtc_state, conn_state, + intel_panel_sanitize_pwm_level(connector, level)); +}
+static void intel_pwm_disable_backlight(const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state-
connector);
+ struct intel_panel *panel = &connector->panel;
+ panel->backlight.pwm_funcs->disable(conn_state, + intel_panel_sanitize_pwm_level(connector, level)); +}
+/* The only hook PWM-only backlight setups need to override, the rest of the hooks are filled in
- from pwm_funcs
- */
Isn't this comment stale now?
+static int intel_pwm_setup_backlight(struct intel_connector *connector, enum pipe pipe) +{ + struct intel_panel *panel = &connector->panel; + int ret = panel->backlight.pwm_funcs->setup(connector, pipe);
+ if (ret < 0) + return ret;
+ panel->backlight.min = panel->backlight.pwm_min; + panel->backlight.max = panel->backlight.pwm_max; + panel->backlight.level = panel->backlight.funcs->get(connector);
Brain hurts with the two value "domains". I'm wondering if we should start to follow some naming convention, for example "pwm_val" or "pwm_level" for everything that is in the "pwm domain". It would be easier to follow.
I assume you mean just renaming pwm_min/pwm_max, will make sure this is done for the next respin
Perhaps the level should be set with just direct intel_pwm_get_backlight() to avoid an unnecessary indirection here?
+ panel->backlight.enabled = panel->backlight.pwm_enabled;
+ return 0; +}
void intel_panel_update_backlight(struct intel_atomic_state *state, struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state, @@ -2015,7 +2040,7 @@ static void intel_panel_destroy_backlight(struct intel_panel *panel) panel->backlight.present = false; } -static const struct intel_panel_bl_funcs bxt_funcs = { +static const struct intel_panel_bl_funcs bxt_pwm_funcs = { .setup = bxt_setup_backlight, .enable = bxt_enable_backlight, .disable = bxt_disable_backlight, @@ -2024,7 +2049,7 @@ static const struct intel_panel_bl_funcs bxt_funcs = { .hz_to_pwm = bxt_hz_to_pwm, }; -static const struct intel_panel_bl_funcs cnp_funcs = { +static const struct intel_panel_bl_funcs cnp_pwm_funcs = { .setup = cnp_setup_backlight, .enable = cnp_enable_backlight, .disable = cnp_disable_backlight, @@ -2033,7 +2058,7 @@ static const struct intel_panel_bl_funcs cnp_funcs = { .hz_to_pwm = cnp_hz_to_pwm, }; -static const struct intel_panel_bl_funcs lpt_funcs = { +static const struct intel_panel_bl_funcs lpt_pwm_funcs = { .setup = lpt_setup_backlight, .enable = lpt_enable_backlight, .disable = lpt_disable_backlight, @@ -2042,7 +2067,7 @@ static const struct intel_panel_bl_funcs lpt_funcs = { .hz_to_pwm = lpt_hz_to_pwm, }; -static const struct intel_panel_bl_funcs spt_funcs = { +static const struct intel_panel_bl_funcs spt_pwm_funcs = { .setup = lpt_setup_backlight, .enable = lpt_enable_backlight, .disable = lpt_disable_backlight, @@ -2051,7 +2076,7 @@ static const struct intel_panel_bl_funcs spt_funcs = { .hz_to_pwm = spt_hz_to_pwm, }; -static const struct intel_panel_bl_funcs pch_funcs = { +static const struct intel_panel_bl_funcs pch_pwm_funcs = { .setup = pch_setup_backlight, .enable = pch_enable_backlight, .disable = pch_disable_backlight, @@ -2068,7 +2093,7 @@ static const struct intel_panel_bl_funcs ext_pwm_funcs = { .get = ext_pwm_get_backlight, }; -static const struct intel_panel_bl_funcs vlv_funcs = { +static const struct intel_panel_bl_funcs vlv_pwm_funcs = { .setup = vlv_setup_backlight, .enable = vlv_enable_backlight, .disable = vlv_disable_backlight, @@ -2077,7 +2102,7 @@ static const struct intel_panel_bl_funcs vlv_funcs = { .hz_to_pwm = vlv_hz_to_pwm, }; -static const struct intel_panel_bl_funcs i965_funcs = { +static const struct intel_panel_bl_funcs i965_pwm_funcs = { .setup = i965_setup_backlight, .enable = i965_enable_backlight, .disable = i965_disable_backlight, @@ -2086,7 +2111,7 @@ static const struct intel_panel_bl_funcs i965_funcs = { .hz_to_pwm = i965_hz_to_pwm, }; -static const struct intel_panel_bl_funcs i9xx_funcs = { +static const struct intel_panel_bl_funcs i9xx_pwm_funcs = { .setup = i9xx_setup_backlight, .enable = i9xx_enable_backlight, .disable = i9xx_disable_backlight, @@ -2095,6 +2120,14 @@ static const struct intel_panel_bl_funcs i9xx_funcs = { .hz_to_pwm = i9xx_hz_to_pwm, }; +static const struct intel_panel_bl_funcs pwm_bl_funcs = { + .setup = intel_pwm_setup_backlight, + .enable = intel_pwm_enable_backlight, + .disable = intel_pwm_disable_backlight, + .set = intel_pwm_set_backlight, + .get = intel_pwm_get_backlight, +};
/* Set up chip specific backlight functions */ static void intel_panel_init_backlight_funcs(struct intel_panel *panel) @@ -2103,36 +2136,39 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) container_of(panel, struct intel_connector, panel); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); - if (connector->base.connector_type == DRM_MODE_CONNECTOR_eDP && - intel_dp_aux_init_backlight_funcs(connector) == 0) - return;
I think I'd keep this here in this patch. It helps with the interpretation of the change here, i.e. we're not starting to utilize the two levels just yet.
if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI && intel_dsi_dcs_init_backlight_funcs(connector) == 0) return; if (IS_GEN9_LP(dev_priv)) { - panel->backlight.funcs = &bxt_funcs; + panel->backlight.pwm_funcs = &bxt_pwm_funcs; } else if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP) { - panel->backlight.funcs = &cnp_funcs; + panel->backlight.pwm_funcs = &cnp_pwm_funcs; } else if (INTEL_PCH_TYPE(dev_priv) >= PCH_LPT) { if (HAS_PCH_LPT(dev_priv)) - panel->backlight.funcs = &lpt_funcs; + panel->backlight.pwm_funcs = &lpt_pwm_funcs; else - panel->backlight.funcs = &spt_funcs; + panel->backlight.pwm_funcs = &spt_pwm_funcs; } else if (HAS_PCH_SPLIT(dev_priv)) { - panel->backlight.funcs = &pch_funcs; + panel->backlight.pwm_funcs = &pch_pwm_funcs; } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI) { - panel->backlight.funcs = &ext_pwm_funcs; + panel->backlight.pwm_funcs = &ext_pwm_funcs; } else { - panel->backlight.funcs = &vlv_funcs; + panel->backlight.pwm_funcs = &vlv_pwm_funcs; } } else if (IS_GEN(dev_priv, 4)) { - panel->backlight.funcs = &i965_funcs; + panel->backlight.pwm_funcs = &i965_pwm_funcs; } else { - panel->backlight.funcs = &i9xx_funcs; + panel->backlight.pwm_funcs = &i9xx_pwm_funcs; }
+ if (connector->base.connector_type == DRM_MODE_CONNECTOR_eDP && + intel_dp_aux_init_backlight_funcs(connector) == 0) + return;
+ /* We're using a standard PWM backlight interface */ + panel->backlight.funcs = &pwm_bl_funcs; } enum drm_connector_status
Since we're about to add support for a second type of backlight control interface over DP AUX (specifically, Intel's proprietary HDR backlight controls) let's rename all of the current backlight hooks we have for vesa to make it clear that they're specific to the VESA interface and not Intel's.
v3: * Rebase
Signed-off-by: Lyude Paul lyude@redhat.com Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com --- .../drm/i915/display/intel_dp_aux_backlight.c | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-)
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 c76287e9e91e..b102692a659d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -25,7 +25,7 @@ #include "intel_display_types.h" #include "intel_dp_aux_backlight.h"
-static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) +static void set_vesa_backlight_enable(struct intel_dp *intel_dp, bool enable) { struct drm_i915_private *i915 = dp_to_i915(intel_dp); u8 reg_val = 0; @@ -52,7 +52,7 @@ static void set_aux_backlight_enable(struct intel_dp *intel_dp, bool enable) } }
-static bool intel_dp_aux_backlight_dpcd_mode(struct intel_connector *connector) +static bool intel_dp_aux_vesa_backlight_dpcd_mode(struct intel_connector *connector) { struct intel_dp *intel_dp = intel_attached_dp(connector); struct drm_i915_private *i915 = dp_to_i915(intel_dp); @@ -75,7 +75,7 @@ static bool intel_dp_aux_backlight_dpcd_mode(struct intel_connector *connector) * Read the current backlight value from DPCD register(s) based * on if 8-bit(MSB) or 16-bit(MSB and LSB) values are supported */ -static u32 intel_dp_aux_get_backlight(struct intel_connector *connector) +static u32 intel_dp_aux_vesa_get_backlight(struct intel_connector *connector) { struct intel_dp *intel_dp = intel_attached_dp(connector); struct drm_i915_private *i915 = dp_to_i915(intel_dp); @@ -86,7 +86,7 @@ static u32 intel_dp_aux_get_backlight(struct intel_connector *connector) * If we're not in DPCD control mode yet, the programmed brightness * value is meaningless and we should assume max brightness */ - if (!intel_dp_aux_backlight_dpcd_mode(connector)) + if (!intel_dp_aux_vesa_backlight_dpcd_mode(connector)) return connector->panel.backlight.max;
if (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB, @@ -107,7 +107,8 @@ static u32 intel_dp_aux_get_backlight(struct intel_connector *connector) * 8-bit or 16 bit value (MSB and LSB) */ static void -intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32 level) +intel_dp_aux_vesa_set_backlight(const struct drm_connector_state *conn_state, + u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct intel_dp *intel_dp = intel_attached_dp(connector); @@ -137,7 +138,7 @@ intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32 lev * - 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) +static bool intel_dp_aux_vesa_set_pwm_freq(struct intel_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_dp *intel_dp = intel_attached_dp(connector); @@ -173,8 +174,9 @@ static bool intel_dp_aux_set_pwm_freq(struct intel_connector *connector) return true; }
-static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_state, - const struct drm_connector_state *conn_state, u32 level) +static void +intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state, u32 level) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct intel_dp *intel_dp = intel_attached_dp(connector); @@ -214,7 +216,7 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st }
if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP) - if (intel_dp_aux_set_pwm_freq(connector)) + if (intel_dp_aux_vesa_set_pwm_freq(connector)) new_dpcd_buf |= DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE;
if (new_dpcd_buf != dpcd_buf) { @@ -225,18 +227,18 @@ static void intel_dp_aux_enable_backlight(const struct intel_crtc_state *crtc_st } }
- intel_dp_aux_set_backlight(conn_state, level); - set_aux_backlight_enable(intel_dp, true); + intel_dp_aux_vesa_set_backlight(conn_state, level); + set_vesa_backlight_enable(intel_dp, true); }
-static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old_conn_state, - u32 level) +static void intel_dp_aux_vesa_disable_backlight(const struct drm_connector_state *old_conn_state, + u32 level) { - set_aux_backlight_enable(enc_to_intel_dp(to_intel_encoder(old_conn_state->best_encoder)), - false); + set_vesa_backlight_enable(enc_to_intel_dp(to_intel_encoder(old_conn_state->best_encoder)), + false); }
-static u32 intel_dp_aux_calc_max_backlight(struct intel_connector *connector) +static u32 intel_dp_aux_vesa_calc_max_backlight(struct intel_connector *connector) { struct drm_i915_private *i915 = to_i915(connector->base.dev); struct intel_dp *intel_dp = intel_attached_dp(connector); @@ -316,25 +318,25 @@ static u32 intel_dp_aux_calc_max_backlight(struct intel_connector *connector) return max_backlight; }
-static int intel_dp_aux_setup_backlight(struct intel_connector *connector, - enum pipe pipe) +static int intel_dp_aux_vesa_setup_backlight(struct intel_connector *connector, + enum pipe pipe) { struct intel_panel *panel = &connector->panel;
- panel->backlight.max = intel_dp_aux_calc_max_backlight(connector); + panel->backlight.max = intel_dp_aux_vesa_calc_max_backlight(connector); if (!panel->backlight.max) return -ENODEV;
panel->backlight.min = 0; - panel->backlight.level = intel_dp_aux_get_backlight(connector); - panel->backlight.enabled = intel_dp_aux_backlight_dpcd_mode(connector) && + panel->backlight.level = intel_dp_aux_vesa_get_backlight(connector); + panel->backlight.enabled = intel_dp_aux_vesa_backlight_dpcd_mode(connector) && panel->backlight.level != 0;
return 0; }
static bool -intel_dp_aux_display_control_capable(struct intel_connector *connector) +intel_dp_aux_supports_vesa_backlight(struct intel_connector *connector) { struct intel_dp *intel_dp = intel_attached_dp(connector); struct drm_i915_private *i915 = dp_to_i915(intel_dp); @@ -350,12 +352,12 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector) return false; }
-static const struct intel_panel_bl_funcs intel_dp_bl_funcs = { - .setup = intel_dp_aux_setup_backlight, - .enable = intel_dp_aux_enable_backlight, - .disable = intel_dp_aux_disable_backlight, - .set = intel_dp_aux_set_backlight, - .get = intel_dp_aux_get_backlight, +static const struct intel_panel_bl_funcs intel_dp_vesa_bl_funcs = { + .setup = intel_dp_aux_vesa_setup_backlight, + .enable = intel_dp_aux_vesa_enable_backlight, + .disable = intel_dp_aux_vesa_disable_backlight, + .set = intel_dp_aux_vesa_set_backlight, + .get = intel_dp_aux_vesa_get_backlight, };
int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) @@ -365,7 +367,7 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) struct drm_i915_private *i915 = dp_to_i915(intel_dp);
if (i915->params.enable_dpcd_backlight == 0 || - !intel_dp_aux_display_control_capable(intel_connector)) + !intel_dp_aux_supports_vesa_backlight(intel_connector)) return -ENODEV;
/* @@ -387,7 +389,7 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) return -ENODEV; }
- panel->backlight.funcs = &intel_dp_bl_funcs; + panel->backlight.funcs = &intel_dp_vesa_bl_funcs;
return 0; }
No functional changes yet, this just adds definitions for all of the known DPCD registers used by Intel's HDR backlight interface. Since we'll only ever use this in i915, we just define them in intel_dp_aux_backlight.c
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: Lyude Paul lyude@redhat.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com --- .../drm/i915/display/intel_dp_aux_backlight.c | 53 +++++++++++++++++++ 1 file changed, 53 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 b102692a659d..9775f33d1aac 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -25,6 +25,59 @@ #include "intel_display_types.h" #include "intel_dp_aux_backlight.h"
+/* + * DP AUX registers for Intel's proprietary HDR backlight interface. We define + * them here since we'll likely be the only driver to ever use these. + */ +#define INTEL_EDP_HDR_TCON_CAP0 0x340 + +#define INTEL_EDP_HDR_TCON_CAP1 0x341 +# define INTEL_EDP_HDR_TCON_2084_DECODE_CAP BIT(0) +# define INTEL_EDP_HDR_TCON_2020_GAMUT_CAP BIT(1) +# define INTEL_EDP_HDR_TCON_TONE_MAPPING_CAP BIT(2) +# define INTEL_EDP_HDR_TCON_SEGMENTED_BACKLIGHT_CAP BIT(3) +# define INTEL_EDP_HDR_TCON_BRIGHTNESS_NITS_CAP BIT(4) +# define INTEL_EDP_HDR_TCON_OPTIMIZATION_CAP BIT(5) +# define INTEL_EDP_HDR_TCON_SDP_COLORIMETRY_CAP BIT(6) +# define INTEL_EDP_HDR_TCON_SRGB_TO_PANEL_GAMUT_CONVERSION_CAP BIT(7) + +#define INTEL_EDP_HDR_TCON_CAP2 0x342 +# define INTEL_EDP_SDR_TCON_BRIGHTNESS_AUX_CAP BIT(0) + +#define INTEL_EDP_HDR_TCON_CAP3 0x343 + +#define INTEL_EDP_HDR_GETSET_CTRL_PARAMS 0x344 +# define INTEL_EDP_HDR_TCON_2084_DECODE_ENABLE BIT(0) +# define INTEL_EDP_HDR_TCON_2020_GAMUT_ENABLE BIT(1) +# define INTEL_EDP_HDR_TCON_TONE_MAPPING_ENABLE BIT(2) /* Pre-TGL+ */ +# define INTEL_EDP_HDR_TCON_SEGMENTED_BACKLIGHT_ENABLE BIT(3) +# define INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE BIT(4) +# define INTEL_EDP_HDR_TCON_SRGB_TO_PANEL_GAMUT_ENABLE BIT(5) +/* Bit 6 is reserved */ +# define INTEL_EDP_HDR_TCON_SDP_COLORIMETRY_ENABLE BIT(7) + +#define INTEL_EDP_HDR_CONTENT_LUMINANCE 0x346 /* Pre-TGL+ */ +#define INTEL_EDP_HDR_PANEL_LUMINANCE_OVERRIDE 0x34A +#define INTEL_EDP_SDR_LUMINANCE_LEVEL 0x352 +#define INTEL_EDP_BRIGHTNESS_NITS_LSB 0x354 +#define INTEL_EDP_BRIGHTNESS_NITS_MSB 0x355 +#define INTEL_EDP_BRIGHTNESS_DELAY_FRAMES 0x356 +#define INTEL_EDP_BRIGHTNESS_PER_FRAME_STEPS 0x357 + +#define INTEL_EDP_BRIGHTNESS_OPTIMIZATION_0 0x358 +# define INTEL_EDP_TCON_USAGE_MASK GENMASK(0, 3) +# define INTEL_EDP_TCON_USAGE_UNKNOWN 0x0 +# define INTEL_EDP_TCON_USAGE_DESKTOP 0x1 +# define INTEL_EDP_TCON_USAGE_FULL_SCREEN_MEDIA 0x2 +# define INTEL_EDP_TCON_USAGE_FULL_SCREEN_GAMING 0x3 +# define INTEL_EDP_TCON_POWER_MASK BIT(4) +# define INTEL_EDP_TCON_POWER_DC (0 << 4) +# define INTEL_EDP_TCON_POWER_AC (1 << 4) +# define INTEL_EDP_TCON_OPTIMIZATION_STRENGTH_MASK GENMASK(5, 7) + +#define INTEL_EDP_BRIGHTNESS_OPTIMIZATION_1 0x359 + +/* VESA backlight callbacks */ static void set_vesa_backlight_enable(struct intel_dp *intel_dp, bool enable) { struct drm_i915_private *i915 = dp_to_i915(intel_dp);
So-recently a bunch of laptops on the market have started using DPCD backlight controls instead of the traditional DDI backlight controls. Originally we thought we had this handled by adding VESA backlight control support to i915, but the story ended up being a lot more complicated then that.
Simply put-there's two main backlight interfaces Intel can see in the wild. Intel's proprietary HDR backlight interface, and the standard VESA backlight interface. Note that many panels have been observed to report support for both backlight interfaces, but testing has shown far more panels work with the Intel HDR backlight interface at the moment. Additionally, the VBT appears to be capable of reporting support for the VESA backlight interface but not the Intel HDR interface which needs to be probed by setting the right magic OUI.
On top of that however, there's also actually two different variants of the Intel HDR backlight interface. The first uses the AUX channel for controlling the brightness of the screen in both SDR and HDR mode, and the second only uses the AUX channel for setting the brightness level in HDR mode - relying on PWM for setting the brightness level in SDR mode.
For the time being we've been using EDIDs to maintain a list of quirks for panels that safely do support the VESA backlight interface. Adding support for Intel's HDR backlight interface in addition however, should finally allow us to auto-detect eDP backlight controls properly so long as we probe like so:
* If the panel's VBT reports VESA backlight support, assume it really does support it * If the panel's VBT reports DDI backlight controls: * First probe for Intel's HDR backlight interface * If that fails, probe for VESA's backlight interface * If that fails, assume no DPCD backlight control * If the panel's VBT reports any other backlight type: just assume it doesn't have DPCD backlight controls
Signed-off-by: Lyude Paul lyude@redhat.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com --- .../drm/i915/display/intel_display_types.h | 9 +- .../drm/i915/display/intel_dp_aux_backlight.c | 244 ++++++++++++++++-- drivers/gpu/drm/i915/display/intel_panel.c | 34 ++- drivers/gpu/drm/i915/display/intel_panel.h | 4 + 4 files changed, 263 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 47ee565c49a2..889b6f9c1aa9 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -261,7 +261,14 @@ struct intel_panel { struct pwm_state pwm_state;
/* DPCD backlight */ - u8 pwmgen_bit_count; + union { + struct { + u8 pwmgen_bit_count; + } vesa; + struct { + bool sdr_uses_aux; + } intel; + } edp;
struct backlight_device *device;
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 9775f33d1aac..9a3ff3ffc158 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -22,8 +22,26 @@ * */
+/* + * Laptops with Intel GPUs which have panels that support controlling the + * backlight through DP AUX can actually use two different interfaces: Intel's + * proprietary DP AUX backlight interface, and the standard VESA backlight + * interface. Unfortunately, at the time of writing this a lot of laptops will + * advertise support for the standard VESA backlight interface when they + * don't properly support it. However, on these systems the Intel backlight + * interface generally does work properly. Additionally, these systems will + * usually just indicate that they use PWM backlight controls in their VBIOS + * for some reason. + */ + #include "intel_display_types.h" #include "intel_dp_aux_backlight.h" +#include "intel_panel.h" + +/* TODO: + * Implement HDR, right now we just implement the bare minimum to bring us back into SDR mode so we + * can make people's backlights work in the mean time + */
/* * DP AUX registers for Intel's proprietary HDR backlight interface. We define @@ -77,6 +95,175 @@
#define INTEL_EDP_BRIGHTNESS_OPTIMIZATION_1 0x359
+/* Intel EDP backlight callbacks */ +static bool +intel_dp_aux_supports_hdr_backlight(struct intel_connector *connector) +{ + struct drm_device *dev = connector->base.dev; + struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); + struct drm_dp_aux *aux = &intel_dp->aux; + struct intel_panel *panel = &connector->panel; + int ret; + u8 tcon_cap[4]; + + ret = drm_dp_dpcd_read(aux, INTEL_EDP_HDR_TCON_CAP0, tcon_cap, sizeof(tcon_cap)); + if (ret < 0) + return false; + + if (!(tcon_cap[1] & INTEL_EDP_HDR_TCON_BRIGHTNESS_NITS_CAP)) + return false; + + if (tcon_cap[0] >= 1) { + drm_dbg_kms(dev, "Detected Intel HDR backlight interface version %d\n", + tcon_cap[0]); + } else { + drm_dbg_kms(dev, "Detected unsupported HDR backlight interface version %d\n", + tcon_cap[0]); + return false; + } + + panel->backlight.edp.intel.sdr_uses_aux = + tcon_cap[2] & INTEL_EDP_SDR_TCON_BRIGHTNESS_AUX_CAP; + + return true; +} + +static u32 +intel_dp_aux_hdr_get_backlight(struct intel_connector *connector) +{ + struct drm_device *dev = connector->base.dev; + struct intel_panel *panel = &connector->panel; + struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); + u8 tmp; + u8 buf[2] = { 0 }; + + if (drm_dp_dpcd_readb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, &tmp) < 0) + drm_err(dev, "Failed to read current backlight mode from DPCD\n"); + + if (!(tmp & INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE)) { + if (panel->backlight.edp.intel.sdr_uses_aux) { + /* Assume 100% brightness if backlight controls aren't enabled yet */ + return panel->backlight.max; + } else { + u32 pwm_level = panel->backlight.pwm_funcs->get(connector); + + return intel_panel_backlight_level_from_pwm(connector, pwm_level); + } + } + + if (drm_dp_dpcd_read(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB, buf, sizeof(buf)) < 0) { + drm_err(dev, "Failed to read brightness from DPCD\n"); + return 0; + } + + return (buf[1] << 8 | buf[0]); +} + +static void +intel_dp_aux_hdr_set_aux_backlight(const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state->connector); + struct drm_device *dev = connector->base.dev; + struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); + uint8_t buf[4] = { 0 }; + + buf[0] = level & 0xFF; + buf[1] = (level & 0xFF00) >> 8; + + if (drm_dp_dpcd_write(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB, buf, 4) < 0) + drm_err(dev, "Failed to write brightness level to DPCD\n"); +} + +static void +intel_dp_aux_hdr_set_backlight(const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state->connector); + struct intel_panel *panel = &connector->panel; + + if (panel->backlight.edp.intel.sdr_uses_aux) { + intel_dp_aux_hdr_set_aux_backlight(conn_state, level); + } else { + const u32 pwm_level = intel_panel_backlight_level_to_pwm(connector, level); + intel_panel_set_pwm_level(conn_state, pwm_level); + } +} + +static void +intel_dp_aux_hdr_enable_backlight(const struct intel_crtc_state *crtc_state, + const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state->connector); + struct intel_panel *panel = &connector->panel; + struct drm_device *dev = connector->base.dev; + struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); + int ret; + u8 old_ctrl, ctrl; + + ret = drm_dp_dpcd_readb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, &old_ctrl); + if (ret < 0) { + drm_err(dev, "Failed to read current backlight control mode: %d\n", ret); + return; + } + + ctrl = old_ctrl; + if (panel->backlight.edp.intel.sdr_uses_aux) { + ctrl |= INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE; + intel_dp_aux_hdr_set_aux_backlight(conn_state, level); + } else { + u32 pwm_level = intel_panel_backlight_level_to_pwm(connector, level); + panel->backlight.pwm_funcs->enable(crtc_state, conn_state, pwm_level); + + ctrl &= ~(INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE); + } + + if (ctrl != old_ctrl) + if (drm_dp_dpcd_writeb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, ctrl) < 0) + drm_err(dev, "Failed to configure DPCD brightness controls\n"); +} + +static void +intel_dp_aux_hdr_disable_backlight(const struct drm_connector_state *conn_state, u32 level) +{ + struct intel_connector *connector = to_intel_connector(conn_state->connector); + struct intel_panel *panel = &connector->panel; + + /* Nothing to do for AUX based backlight controls */ + if (panel->backlight.edp.intel.sdr_uses_aux) + return; + + /* Note we want the actual pwm_level to be 0, regardless of pwm_min */ + panel->backlight.pwm_funcs->disable(conn_state, + intel_panel_sanitize_pwm_level(connector, 0)); +} + +static int +intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pipe) +{ + struct drm_device *dev = connector->base.dev; + struct intel_panel *panel = &connector->panel; + int ret; + + if (panel->backlight.edp.intel.sdr_uses_aux) { + drm_dbg_kms(dev, "SDR backlight is controlled through DPCD\n"); + } else { + drm_dbg_kms(dev, "SDR backlight is controlled through PWM\n"); + + ret = panel->backlight.pwm_funcs->setup(connector, pipe); + if (ret < 0) { + drm_err(dev, "Failed to setup SDR backlight controls through PWM: %d\n", + ret); + return ret; + } + } + + panel->backlight.max = 512; + panel->backlight.min = 0; + panel->backlight.level = intel_dp_aux_hdr_get_backlight(connector); + panel->backlight.enabled = panel->backlight.level != 0; + + return 0; +} + /* VESA backlight callbacks */ static void set_vesa_backlight_enable(struct intel_dp *intel_dp, bool enable) { @@ -195,7 +382,7 @@ static bool intel_dp_aux_vesa_set_pwm_freq(struct intel_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_dp *intel_dp = intel_attached_dp(connector); - const u8 pn = connector->panel.backlight.pwmgen_bit_count; + const u8 pn = connector->panel.backlight.edp.vesa.pwmgen_bit_count; int freq, fxp, f, fxp_actual, fxp_min, fxp_max;
freq = dev_priv->vbt.backlight.pwm_freq_hz; @@ -236,6 +423,7 @@ intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state, struct drm_i915_private *i915 = dp_to_i915(intel_dp); struct intel_panel *panel = &connector->panel; u8 dpcd_buf, new_dpcd_buf, edp_backlight_mode; + u8 pwmgen_bit_count = panel->backlight.edp.vesa.pwmgen_bit_count;
if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) { @@ -256,7 +444,7 @@ intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state,
if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_PWMGEN_BIT_COUNT, - panel->backlight.pwmgen_bit_count) < 0) + pwmgen_bit_count) < 0) drm_dbg_kms(&i915->drm, "Failed to write aux pwmgen bit count\n");
@@ -364,7 +552,7 @@ static u32 intel_dp_aux_vesa_calc_max_backlight(struct intel_connector *connecto "Failed to write aux pwmgen bit count\n"); return max_backlight; } - panel->backlight.pwmgen_bit_count = pn; + panel->backlight.edp.vesa.pwmgen_bit_count = pn;
max_backlight = (1 << pn) - 1;
@@ -405,6 +593,14 @@ intel_dp_aux_supports_vesa_backlight(struct intel_connector *connector) return false; }
+static const struct intel_panel_bl_funcs intel_dp_hdr_bl_funcs = { + .setup = intel_dp_aux_hdr_setup_backlight, + .enable = intel_dp_aux_hdr_enable_backlight, + .disable = intel_dp_aux_hdr_disable_backlight, + .set = intel_dp_aux_hdr_set_backlight, + .get = intel_dp_aux_hdr_get_backlight, +}; + static const struct intel_panel_bl_funcs intel_dp_vesa_bl_funcs = { .setup = intel_dp_aux_vesa_setup_backlight, .enable = intel_dp_aux_vesa_enable_backlight, @@ -413,36 +609,34 @@ static const struct intel_panel_bl_funcs intel_dp_vesa_bl_funcs = { .get = intel_dp_aux_vesa_get_backlight, };
-int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) +int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector) { - struct intel_panel *panel = &intel_connector->panel; - struct intel_dp *intel_dp = enc_to_intel_dp(intel_connector->encoder); + struct drm_device *dev = connector->base.dev; + struct intel_panel *panel = &connector->panel; + struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); struct drm_i915_private *i915 = dp_to_i915(intel_dp);
- if (i915->params.enable_dpcd_backlight == 0 || - !intel_dp_aux_supports_vesa_backlight(intel_connector)) + if (i915->params.enable_dpcd_backlight == 0) return -ENODEV;
/* - * There are a lot of machines that don't advertise the backlight - * control interface to use properly in their VBIOS, :\ + * A lot of eDP panels in the wild will report supporting both the + * Intel proprietary backlight control interface, and the VESA + * backlight control interface. Many of these panels are liars though, + * and will only work with the Intel interface. So, always probe for + * that first. */ - if (i915->vbt.backlight.type != - INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE && - i915->params.enable_dpcd_backlight != 1 && - !drm_dp_has_quirk(&intel_dp->desc, intel_dp->edid_quirks, - DP_QUIRK_FORCE_DPCD_BACKLIGHT)) { - drm_info(&i915->drm, - "Panel advertises DPCD backlight support, but " - "VBT disagrees. If your backlight controls " - "don't work try booting with " - "i915.enable_dpcd_backlight=1. If your machine " - "needs this, please file a _new_ bug report on " - "drm/i915, see " FDO_BUG_URL " for details.\n"); - return -ENODEV; + if (intel_dp_aux_supports_hdr_backlight(connector)) { + drm_dbg(dev, "Using Intel proprietary eDP backlight controls\n"); + panel->backlight.funcs = &intel_dp_hdr_bl_funcs; + return 0; }
- panel->backlight.funcs = &intel_dp_vesa_bl_funcs; + if (intel_dp_aux_supports_vesa_backlight(connector)) { + drm_dbg(dev, "Using VESA eDP backlight controls\n"); + panel->backlight.funcs = &intel_dp_vesa_bl_funcs; + return 0; + }
- return 0; + return -ENODEV; } diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index 41f0d2b2c627..c48aa13b0b90 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -511,7 +511,7 @@ static u32 scale_hw_to_user(struct intel_connector *connector, 0, user_max); }
-static u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 val) +u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 val) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; @@ -529,7 +529,7 @@ static u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 return val; }
-static void intel_panel_set_pwm_level(const struct drm_connector_state *conn_state, u32 val) +void intel_panel_set_pwm_level(const struct drm_connector_state *conn_state, u32 val) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_i915_private *i915 = to_i915(connector->base.dev); @@ -539,6 +539,36 @@ static void intel_panel_set_pwm_level(const struct drm_connector_state *conn_sta panel->backlight.pwm_funcs->set(conn_state, val); }
+u32 intel_panel_backlight_level_to_pwm(struct intel_connector *connector, u32 val) +{ + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct intel_panel *panel = &connector->panel; + + drm_WARN_ON_ONCE(&dev_priv->drm, + panel->backlight.max == 0 || panel->backlight.pwm_max == 0); + + val = scale(val, panel->backlight.min, panel->backlight.max, + panel->backlight.pwm_min, panel->backlight.pwm_max); + + return intel_panel_sanitize_pwm_level(connector, val); +} + +u32 intel_panel_backlight_level_from_pwm(struct intel_connector *connector, u32 val) +{ + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct intel_panel *panel = &connector->panel; + + drm_WARN_ON_ONCE(&dev_priv->drm, + panel->backlight.max == 0 || panel->backlight.pwm_max == 0); + + if (dev_priv->params.invert_brightness > 0 || + (dev_priv->params.invert_brightness == 0 && dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS)) + val = panel->backlight.pwm_max - (val - panel->backlight.pwm_min); + + return scale(val, panel->backlight.pwm_min, panel->backlight.pwm_max, + panel->backlight.min, panel->backlight.max); +} + static u32 lpt_get_backlight(struct intel_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h index 5b813fe90557..a548347a975f 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.h +++ b/drivers/gpu/drm/i915/display/intel_panel.h @@ -49,6 +49,10 @@ struct drm_display_mode * intel_panel_edid_fixed_mode(struct intel_connector *connector); struct drm_display_mode * intel_panel_vbt_fixed_mode(struct intel_connector *connector); +void intel_panel_set_pwm_level(const struct drm_connector_state *conn_state, u32 level); +u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 level); +u32 intel_panel_backlight_level_to_pwm(struct intel_connector *connector, u32 level); +u32 intel_panel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE) int intel_backlight_device_register(struct intel_connector *connector);
On Fri, 04 Dec 2020, Lyude Paul lyude@redhat.com wrote:
So-recently a bunch of laptops on the market have started using DPCD backlight controls instead of the traditional DDI backlight controls. Originally we thought we had this handled by adding VESA backlight control support to i915, but the story ended up being a lot more complicated then that.
Simply put-there's two main backlight interfaces Intel can see in the wild. Intel's proprietary HDR backlight interface, and the standard VESA backlight interface. Note that many panels have been observed to report support for both backlight interfaces, but testing has shown far more panels work with the Intel HDR backlight interface at the moment. Additionally, the VBT appears to be capable of reporting support for the VESA backlight interface but not the Intel HDR interface which needs to be probed by setting the right magic OUI.
On top of that however, there's also actually two different variants of the Intel HDR backlight interface. The first uses the AUX channel for controlling the brightness of the screen in both SDR and HDR mode, and the second only uses the AUX channel for setting the brightness level in HDR mode - relying on PWM for setting the brightness level in SDR mode.
For the time being we've been using EDIDs to maintain a list of quirks for panels that safely do support the VESA backlight interface. Adding support for Intel's HDR backlight interface in addition however, should finally allow us to auto-detect eDP backlight controls properly so long as we probe like so:
- If the panel's VBT reports VESA backlight support, assume it really does support it
- If the panel's VBT reports DDI backlight controls:
- First probe for Intel's HDR backlight interface
- If that fails, probe for VESA's backlight interface
- If that fails, assume no DPCD backlight control
- If the panel's VBT reports any other backlight type: just assume it doesn't have DPCD backlight controls
Signed-off-by: Lyude Paul lyude@redhat.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com
.../drm/i915/display/intel_display_types.h | 9 +- .../drm/i915/display/intel_dp_aux_backlight.c | 244 ++++++++++++++++-- drivers/gpu/drm/i915/display/intel_panel.c | 34 ++- drivers/gpu/drm/i915/display/intel_panel.h | 4 + 4 files changed, 263 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 47ee565c49a2..889b6f9c1aa9 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -261,7 +261,14 @@ struct intel_panel { struct pwm_state pwm_state;
/* DPCD backlight */
u8 pwmgen_bit_count;
union {struct {u8 pwmgen_bit_count;} vesa;struct {bool sdr_uses_aux;} intel;} edp;struct backlight_device *device;
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 9775f33d1aac..9a3ff3ffc158 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -22,8 +22,26 @@
*/
+/*
- Laptops with Intel GPUs which have panels that support controlling the
- backlight through DP AUX can actually use two different interfaces: Intel's
- proprietary DP AUX backlight interface, and the standard VESA backlight
- interface. Unfortunately, at the time of writing this a lot of laptops will
- advertise support for the standard VESA backlight interface when they
- don't properly support it. However, on these systems the Intel backlight
- interface generally does work properly. Additionally, these systems will
- usually just indicate that they use PWM backlight controls in their VBIOS
- for some reason.
- */
#include "intel_display_types.h" #include "intel_dp_aux_backlight.h" +#include "intel_panel.h"
+/* TODO:
- Implement HDR, right now we just implement the bare minimum to bring us back into SDR mode so we
- can make people's backlights work in the mean time
- */
/*
- DP AUX registers for Intel's proprietary HDR backlight interface. We define
@@ -77,6 +95,175 @@
#define INTEL_EDP_BRIGHTNESS_OPTIMIZATION_1 0x359
+/* Intel EDP backlight callbacks */ +static bool +intel_dp_aux_supports_hdr_backlight(struct intel_connector *connector) +{
- struct drm_device *dev = connector->base.dev;
Nitpick, as a convention we usually prefer having struct drm_i915_private *i915 around instead, and use &i915->drm when we need the drm device. Typically we'll need to access i915 for other reasons anyway, and only having one seems neater. Not a blocker here, just FYI.
- struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
- struct drm_dp_aux *aux = &intel_dp->aux;
- struct intel_panel *panel = &connector->panel;
- int ret;
- u8 tcon_cap[4];
- ret = drm_dp_dpcd_read(aux, INTEL_EDP_HDR_TCON_CAP0, tcon_cap, sizeof(tcon_cap));
- if (ret < 0)
return false;- if (!(tcon_cap[1] & INTEL_EDP_HDR_TCON_BRIGHTNESS_NITS_CAP))
return false;- if (tcon_cap[0] >= 1) {
drm_dbg_kms(dev, "Detected Intel HDR backlight interface version %d\n",tcon_cap[0]);- } else {
drm_dbg_kms(dev, "Detected unsupported HDR backlight interface version %d\n",tcon_cap[0]);return false;- }
- panel->backlight.edp.intel.sdr_uses_aux =
tcon_cap[2] & INTEL_EDP_SDR_TCON_BRIGHTNESS_AUX_CAP;- return true;
+}
+static u32 +intel_dp_aux_hdr_get_backlight(struct intel_connector *connector) +{
- struct drm_device *dev = connector->base.dev;
- struct intel_panel *panel = &connector->panel;
- struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
- u8 tmp;
- u8 buf[2] = { 0 };
- if (drm_dp_dpcd_readb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, &tmp) < 0)
drm_err(dev, "Failed to read current backlight mode from DPCD\n");
Now what, tmp could be uninitialized? Return?
- if (!(tmp & INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE)) {
if (panel->backlight.edp.intel.sdr_uses_aux) {/* Assume 100% brightness if backlight controls aren't enabled yet */return panel->backlight.max;} else {u32 pwm_level = panel->backlight.pwm_funcs->get(connector);
This is exactly what I meant with having pwm_ prefixes for levels in the "pwm domain" for clarity, in reply to another patch. +1
return intel_panel_backlight_level_from_pwm(connector, pwm_level);}- }
- if (drm_dp_dpcd_read(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB, buf, sizeof(buf)) < 0) {
drm_err(dev, "Failed to read brightness from DPCD\n");return 0;- }
- return (buf[1] << 8 | buf[0]);
+}
+static void +intel_dp_aux_hdr_set_aux_backlight(const struct drm_connector_state *conn_state, u32 level) +{
- struct intel_connector *connector = to_intel_connector(conn_state->connector);
- struct drm_device *dev = connector->base.dev;
- struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
- uint8_t buf[4] = { 0 };
Nitpick, u8.
- buf[0] = level & 0xFF;
- buf[1] = (level & 0xFF00) >> 8;
- if (drm_dp_dpcd_write(&intel_dp->aux, INTEL_EDP_BRIGHTNESS_NITS_LSB, buf, 4) < 0)
drm_err(dev, "Failed to write brightness level to DPCD\n");+}
+static void +intel_dp_aux_hdr_set_backlight(const struct drm_connector_state *conn_state, u32 level) +{
- struct intel_connector *connector = to_intel_connector(conn_state->connector);
- struct intel_panel *panel = &connector->panel;
- if (panel->backlight.edp.intel.sdr_uses_aux) {
intel_dp_aux_hdr_set_aux_backlight(conn_state, level);- } else {
const u32 pwm_level = intel_panel_backlight_level_to_pwm(connector, level);intel_panel_set_pwm_level(conn_state, pwm_level);- }
+}
+static void +intel_dp_aux_hdr_enable_backlight(const struct intel_crtc_state *crtc_state,
const struct drm_connector_state *conn_state, u32 level)+{
- struct intel_connector *connector = to_intel_connector(conn_state->connector);
- struct intel_panel *panel = &connector->panel;
- struct drm_device *dev = connector->base.dev;
- struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
- int ret;
- u8 old_ctrl, ctrl;
- ret = drm_dp_dpcd_readb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, &old_ctrl);
- if (ret < 0) {
drm_err(dev, "Failed to read current backlight control mode: %d\n", ret);return;- }
- ctrl = old_ctrl;
- if (panel->backlight.edp.intel.sdr_uses_aux) {
ctrl |= INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE;intel_dp_aux_hdr_set_aux_backlight(conn_state, level);- } else {
u32 pwm_level = intel_panel_backlight_level_to_pwm(connector, level);panel->backlight.pwm_funcs->enable(crtc_state, conn_state, pwm_level);ctrl &= ~(INTEL_EDP_HDR_TCON_BRIGHTNESS_AUX_ENABLE);
Nitpick, unnecessary braces.
- }
- if (ctrl != old_ctrl)
if (drm_dp_dpcd_writeb(&intel_dp->aux, INTEL_EDP_HDR_GETSET_CTRL_PARAMS, ctrl) < 0)drm_err(dev, "Failed to configure DPCD brightness controls\n");+}
+static void +intel_dp_aux_hdr_disable_backlight(const struct drm_connector_state *conn_state, u32 level) +{
- struct intel_connector *connector = to_intel_connector(conn_state->connector);
- struct intel_panel *panel = &connector->panel;
- /* Nothing to do for AUX based backlight controls */
- if (panel->backlight.edp.intel.sdr_uses_aux)
return;- /* Note we want the actual pwm_level to be 0, regardless of pwm_min */
- panel->backlight.pwm_funcs->disable(conn_state,
intel_panel_sanitize_pwm_level(connector, 0));+}
+static int +intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pipe) +{
- struct drm_device *dev = connector->base.dev;
- struct intel_panel *panel = &connector->panel;
- int ret;
- if (panel->backlight.edp.intel.sdr_uses_aux) {
drm_dbg_kms(dev, "SDR backlight is controlled through DPCD\n");- } else {
drm_dbg_kms(dev, "SDR backlight is controlled through PWM\n");ret = panel->backlight.pwm_funcs->setup(connector, pipe);if (ret < 0) {drm_err(dev, "Failed to setup SDR backlight controls through PWM: %d\n",ret);return ret;}- }
- panel->backlight.max = 512;
- panel->backlight.min = 0;
- panel->backlight.level = intel_dp_aux_hdr_get_backlight(connector);
- panel->backlight.enabled = panel->backlight.level != 0;
- return 0;
+}
/* VESA backlight callbacks */ static void set_vesa_backlight_enable(struct intel_dp *intel_dp, bool enable) { @@ -195,7 +382,7 @@ static bool intel_dp_aux_vesa_set_pwm_freq(struct intel_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_dp *intel_dp = intel_attached_dp(connector);
- const u8 pn = connector->panel.backlight.pwmgen_bit_count;
const u8 pn = connector->panel.backlight.edp.vesa.pwmgen_bit_count; int freq, fxp, f, fxp_actual, fxp_min, fxp_max;
freq = dev_priv->vbt.backlight.pwm_freq_hz;
@@ -236,6 +423,7 @@ intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state, struct drm_i915_private *i915 = dp_to_i915(intel_dp); struct intel_panel *panel = &connector->panel; u8 dpcd_buf, new_dpcd_buf, edp_backlight_mode;
u8 pwmgen_bit_count = panel->backlight.edp.vesa.pwmgen_bit_count;
if (drm_dp_dpcd_readb(&intel_dp->aux, DP_EDP_BACKLIGHT_MODE_SET_REGISTER, &dpcd_buf) != 1) {
@@ -256,7 +444,7 @@ intel_dp_aux_vesa_enable_backlight(const struct intel_crtc_state *crtc_state,
if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_EDP_PWMGEN_BIT_COUNT,
panel->backlight.pwmgen_bit_count) < 0)
pwmgen_bit_count) < 0) drm_dbg_kms(&i915->drm, "Failed to write aux pwmgen bit count\n");@@ -364,7 +552,7 @@ static u32 intel_dp_aux_vesa_calc_max_backlight(struct intel_connector *connecto "Failed to write aux pwmgen bit count\n"); return max_backlight; }
- panel->backlight.pwmgen_bit_count = pn;
panel->backlight.edp.vesa.pwmgen_bit_count = pn;
max_backlight = (1 << pn) - 1;
@@ -405,6 +593,14 @@ intel_dp_aux_supports_vesa_backlight(struct intel_connector *connector) return false; }
+static const struct intel_panel_bl_funcs intel_dp_hdr_bl_funcs = {
- .setup = intel_dp_aux_hdr_setup_backlight,
- .enable = intel_dp_aux_hdr_enable_backlight,
- .disable = intel_dp_aux_hdr_disable_backlight,
- .set = intel_dp_aux_hdr_set_backlight,
- .get = intel_dp_aux_hdr_get_backlight,
+};
static const struct intel_panel_bl_funcs intel_dp_vesa_bl_funcs = { .setup = intel_dp_aux_vesa_setup_backlight, .enable = intel_dp_aux_vesa_enable_backlight, @@ -413,36 +609,34 @@ static const struct intel_panel_bl_funcs intel_dp_vesa_bl_funcs = { .get = intel_dp_aux_vesa_get_backlight, };
-int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector) +int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector) {
- struct intel_panel *panel = &intel_connector->panel;
- struct intel_dp *intel_dp = enc_to_intel_dp(intel_connector->encoder);
- struct drm_device *dev = connector->base.dev;
- struct intel_panel *panel = &connector->panel;
- struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); struct drm_i915_private *i915 = dp_to_i915(intel_dp);
- if (i915->params.enable_dpcd_backlight == 0 ||
!intel_dp_aux_supports_vesa_backlight(intel_connector))
if (i915->params.enable_dpcd_backlight == 0) return -ENODEV;
/*
* There are a lot of machines that don't advertise the backlight* control interface to use properly in their VBIOS, :\
* A lot of eDP panels in the wild will report supporting both the* Intel proprietary backlight control interface, and the VESA* backlight control interface. Many of these panels are liars though,* and will only work with the Intel interface. So, always probe for* that first.
- if (i915->vbt.backlight.type !=
INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE &&
I do wonder about dropping this out. I see it gets added back in the next patch, but this just means it's, I think, necessary to merge the two patches back to back.
Despite the nitpicks, this is the only concern I have, really. I didn't review all the hdr hooks in detail, and if it works, it anyway trumps me checking the code against some crappy powerpoint of a spec I got.
Overall,
Acked-by: Jani Nikula jani.nikula@intel.com
with or without the nitpicks fixed.
i915->params.enable_dpcd_backlight != 1 &&!drm_dp_has_quirk(&intel_dp->desc, intel_dp->edid_quirks,DP_QUIRK_FORCE_DPCD_BACKLIGHT)) {drm_info(&i915->drm,"Panel advertises DPCD backlight support, but ""VBT disagrees. If your backlight controls ""don't work try booting with ""i915.enable_dpcd_backlight=1. If your machine ""needs this, please file a _new_ bug report on ""drm/i915, see " FDO_BUG_URL " for details.\n");return -ENODEV;
- if (intel_dp_aux_supports_hdr_backlight(connector)) {
drm_dbg(dev, "Using Intel proprietary eDP backlight controls\n");
Nitpick, drm_debug_kms, ditto below.
panel->backlight.funcs = &intel_dp_hdr_bl_funcs;return 0;
- panel->backlight.funcs = &intel_dp_vesa_bl_funcs;
- if (intel_dp_aux_supports_vesa_backlight(connector)) {
drm_dbg(dev, "Using VESA eDP backlight controls\n");panel->backlight.funcs = &intel_dp_vesa_bl_funcs;return 0;- }
- return 0;
- return -ENODEV;
} diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c index 41f0d2b2c627..c48aa13b0b90 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.c +++ b/drivers/gpu/drm/i915/display/intel_panel.c @@ -511,7 +511,7 @@ static u32 scale_hw_to_user(struct intel_connector *connector, 0, user_max); }
-static u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 val) +u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 val) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_panel *panel = &connector->panel; @@ -529,7 +529,7 @@ static u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 return val; }
-static void intel_panel_set_pwm_level(const struct drm_connector_state *conn_state, u32 val) +void intel_panel_set_pwm_level(const struct drm_connector_state *conn_state, u32 val) { struct intel_connector *connector = to_intel_connector(conn_state->connector); struct drm_i915_private *i915 = to_i915(connector->base.dev); @@ -539,6 +539,36 @@ static void intel_panel_set_pwm_level(const struct drm_connector_state *conn_sta panel->backlight.pwm_funcs->set(conn_state, val); }
+u32 intel_panel_backlight_level_to_pwm(struct intel_connector *connector, u32 val) +{
- struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
- struct intel_panel *panel = &connector->panel;
- drm_WARN_ON_ONCE(&dev_priv->drm,
panel->backlight.max == 0 || panel->backlight.pwm_max == 0);- val = scale(val, panel->backlight.min, panel->backlight.max,
panel->backlight.pwm_min, panel->backlight.pwm_max);- return intel_panel_sanitize_pwm_level(connector, val);
+}
+u32 intel_panel_backlight_level_from_pwm(struct intel_connector *connector, u32 val) +{
- struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
- struct intel_panel *panel = &connector->panel;
- drm_WARN_ON_ONCE(&dev_priv->drm,
panel->backlight.max == 0 || panel->backlight.pwm_max == 0);- if (dev_priv->params.invert_brightness > 0 ||
(dev_priv->params.invert_brightness == 0 && dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS))val = panel->backlight.pwm_max - (val - panel->backlight.pwm_min);- return scale(val, panel->backlight.pwm_min, panel->backlight.pwm_max,
panel->backlight.min, panel->backlight.max);+}
static u32 lpt_get_backlight(struct intel_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->base.dev); diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h index 5b813fe90557..a548347a975f 100644 --- a/drivers/gpu/drm/i915/display/intel_panel.h +++ b/drivers/gpu/drm/i915/display/intel_panel.h @@ -49,6 +49,10 @@ struct drm_display_mode * intel_panel_edid_fixed_mode(struct intel_connector *connector); struct drm_display_mode * intel_panel_vbt_fixed_mode(struct intel_connector *connector); +void intel_panel_set_pwm_level(const struct drm_connector_state *conn_state, u32 level); +u32 intel_panel_sanitize_pwm_level(struct intel_connector *connector, u32 level); +u32 intel_panel_backlight_level_to_pwm(struct intel_connector *connector, u32 level); +u32 intel_panel_backlight_level_from_pwm(struct intel_connector *connector, u32 val);
#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE) int intel_backlight_device_register(struct intel_connector *connector);
Since we now support controlling panel backlights through DPCD using both the standard VESA interface, and Intel's proprietary HDR backlight interface, we should allow the user to be able to explicitly choose between one or the other in the event that we're wrong about panels reliably reporting support for the Intel HDR interface.
So, this commit adds support for this by introducing two new enable_dpcd_backlight options: 2 which forces i915 to only probe for the VESA interface, and 3 which forces i915 to only probe for the Intel backlight interface (might be useful if we find panels in the wild that report the VESA interface in their VBT, but actually only support the Intel backlight interface).
v3: * Rebase
Signed-off-by: Lyude Paul lyude@redhat.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com --- .../drm/i915/display/intel_dp_aux_backlight.c | 45 +++++++++++++++++-- drivers/gpu/drm/i915/i915_params.c | 2 +- 2 files changed, 43 insertions(+), 4 deletions(-)
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 9a3ff3ffc158..eef14ab6bddc 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -609,15 +609,54 @@ static const struct intel_panel_bl_funcs intel_dp_vesa_bl_funcs = { .get = intel_dp_aux_vesa_get_backlight, };
+enum intel_dp_aux_backlight_modparam { + INTEL_DP_AUX_BACKLIGHT_AUTO = -1, + INTEL_DP_AUX_BACKLIGHT_OFF = 0, + INTEL_DP_AUX_BACKLIGHT_ON = 1, + INTEL_DP_AUX_BACKLIGHT_FORCE_VESA = 2, + INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL = 3, +}; + int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector) { struct drm_device *dev = connector->base.dev; struct intel_panel *panel = &connector->panel; struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); struct drm_i915_private *i915 = dp_to_i915(intel_dp); + bool try_intel_interface = false, try_vesa_interface = false;
- if (i915->params.enable_dpcd_backlight == 0) + /* Check the VBT and user's module parameters to figure out which + * interfaces to probe + */ + switch (i915->params.enable_dpcd_backlight) { + case INTEL_DP_AUX_BACKLIGHT_OFF: return -ENODEV; + case INTEL_DP_AUX_BACKLIGHT_AUTO: + switch (i915->vbt.backlight.type) { + case INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE: + try_vesa_interface = true; + break; + case INTEL_BACKLIGHT_DISPLAY_DDI: + try_intel_interface = true; + try_vesa_interface = true; + break; + default: + return -ENODEV; + } + break; + case INTEL_DP_AUX_BACKLIGHT_ON: + if (i915->vbt.backlight.type != INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE) + try_intel_interface = true; + + try_vesa_interface = true; + break; + case INTEL_DP_AUX_BACKLIGHT_FORCE_VESA: + try_vesa_interface = true; + break; + case INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL: + try_intel_interface = true; + break; + }
/* * A lot of eDP panels in the wild will report supporting both the @@ -626,13 +665,13 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector) * and will only work with the Intel interface. So, always probe for * that first. */ - if (intel_dp_aux_supports_hdr_backlight(connector)) { + if (try_intel_interface && intel_dp_aux_supports_hdr_backlight(connector)) { drm_dbg(dev, "Using Intel proprietary eDP backlight controls\n"); panel->backlight.funcs = &intel_dp_hdr_bl_funcs; return 0; }
- if (intel_dp_aux_supports_vesa_backlight(connector)) { + if (try_vesa_interface && intel_dp_aux_supports_vesa_backlight(connector)) { drm_dbg(dev, "Using VESA eDP backlight controls\n"); panel->backlight.funcs = &intel_dp_vesa_bl_funcs; return 0; diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 7f139ea4a90b..6939634e56ed 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -185,7 +185,7 @@ i915_param_named_unsafe(inject_probe_failure, uint, 0400,
i915_param_named(enable_dpcd_backlight, int, 0400, "Enable support for DPCD backlight control" - "(-1=use per-VBT LFP backlight type setting [default], 0=disabled, 1=enabled)"); + "(-1=use per-VBT LFP backlight type setting [default], 0=disabled, 1=enable, 2=force VESA interface, 3=force Intel interface)");
#if IS_ENABLED(CONFIG_DRM_I915_GVT) i915_param_named(enable_gvt, bool, 0400,
On Fri, 04 Dec 2020, Lyude Paul lyude@redhat.com wrote:
Since we now support controlling panel backlights through DPCD using both the standard VESA interface, and Intel's proprietary HDR backlight interface, we should allow the user to be able to explicitly choose between one or the other in the event that we're wrong about panels reliably reporting support for the Intel HDR interface.
So, this commit adds support for this by introducing two new enable_dpcd_backlight options: 2 which forces i915 to only probe for the VESA interface, and 3 which forces i915 to only probe for the Intel backlight interface (might be useful if we find panels in the wild that report the VESA interface in their VBT, but actually only support the Intel backlight interface).
v3:
- Rebase
Signed-off-by: Lyude Paul lyude@redhat.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com
.../drm/i915/display/intel_dp_aux_backlight.c | 45 +++++++++++++++++-- drivers/gpu/drm/i915/i915_params.c | 2 +- 2 files changed, 43 insertions(+), 4 deletions(-)
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 9a3ff3ffc158..eef14ab6bddc 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -609,15 +609,54 @@ static const struct intel_panel_bl_funcs intel_dp_vesa_bl_funcs = { .get = intel_dp_aux_vesa_get_backlight, };
+enum intel_dp_aux_backlight_modparam {
- INTEL_DP_AUX_BACKLIGHT_AUTO = -1,
- INTEL_DP_AUX_BACKLIGHT_OFF = 0,
- INTEL_DP_AUX_BACKLIGHT_ON = 1,
- INTEL_DP_AUX_BACKLIGHT_FORCE_VESA = 2,
- INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL = 3,
+};
int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector) { struct drm_device *dev = connector->base.dev; struct intel_panel *panel = &connector->panel; struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); struct drm_i915_private *i915 = dp_to_i915(intel_dp);
- bool try_intel_interface = false, try_vesa_interface = false;
- if (i915->params.enable_dpcd_backlight == 0)
- /* Check the VBT and user's module parameters to figure out which
* interfaces to probe*/- switch (i915->params.enable_dpcd_backlight) {
- case INTEL_DP_AUX_BACKLIGHT_OFF: return -ENODEV;
- case INTEL_DP_AUX_BACKLIGHT_AUTO:
switch (i915->vbt.backlight.type) {case INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE:try_vesa_interface = true;break;case INTEL_BACKLIGHT_DISPLAY_DDI:try_intel_interface = true;
I take it this is what the machines report? *rolls eyes*.
try_vesa_interface = true;break;default:return -ENODEV;}break;- case INTEL_DP_AUX_BACKLIGHT_ON:
if (i915->vbt.backlight.type != INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE)try_intel_interface = true;
This could use an explanation - why not try the intel interface in this case?
Anyway, good enough,
Reviewed-by: Jani Nikula jani.nikula@intel.com
try_vesa_interface = true;break;case INTEL_DP_AUX_BACKLIGHT_FORCE_VESA:
try_vesa_interface = true;break;case INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL:
try_intel_interface = true;break;}
/*
- A lot of eDP panels in the wild will report supporting both the
@@ -626,13 +665,13 @@ int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector) * and will only work with the Intel interface. So, always probe for * that first. */
- if (intel_dp_aux_supports_hdr_backlight(connector)) {
- if (try_intel_interface && intel_dp_aux_supports_hdr_backlight(connector)) { drm_dbg(dev, "Using Intel proprietary eDP backlight controls\n"); panel->backlight.funcs = &intel_dp_hdr_bl_funcs; return 0; }
- if (intel_dp_aux_supports_vesa_backlight(connector)) {
- if (try_vesa_interface && intel_dp_aux_supports_vesa_backlight(connector)) { drm_dbg(dev, "Using VESA eDP backlight controls\n"); panel->backlight.funcs = &intel_dp_vesa_bl_funcs; return 0;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 7f139ea4a90b..6939634e56ed 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -185,7 +185,7 @@ i915_param_named_unsafe(inject_probe_failure, uint, 0400,
i915_param_named(enable_dpcd_backlight, int, 0400, "Enable support for DPCD backlight control"
- "(-1=use per-VBT LFP backlight type setting [default], 0=disabled, 1=enabled)");
- "(-1=use per-VBT LFP backlight type setting [default], 0=disabled, 1=enable, 2=force VESA interface, 3=force Intel interface)");
#if IS_ENABLED(CONFIG_DRM_I915_GVT) i915_param_named(enable_gvt, bool, 0400,
This reverts commit 0883ce8146ed6074c76399f4e70dbed788582e12. Originally these quirks were added because of the issues with using the eDP backlight interfaces on certain laptop panels, which made it impossible to properly probe for DPCD backlight support without having a whitelist for panels that we know have working VESA backlight control interfaces over DPCD. As well, it should be noted it was impossible to use the normal sink OUI for recognizing these panels as none of them actually filled out their OUIs, hence needing to resort to checking EDIDs.
At the time we weren't really sure why certain panels had issues with DPCD backlight controls, but we eventually figured out that there was a second interface that these problematic laptop panels actually did work with and advertise properly: Intel's proprietary backlight interface for HDR panels. So far the testing we've done hasn't brought any panels to light that advertise this interface and don't support it properly, which means we finally have a real solution to this problem.
As a result, we now have no need for the force DPCD backlight quirk, and furthermore this also removes the need for any kind of EDID quirk checking in DRM. So, let's just revert it for now since we were the only driver using this.
v3: * Rebase v2: * Fix indenting error picked up by checkpatch in intel_edp_init_connector()
Signed-off-by: Lyude Paul lyude@redhat.com Acked-by: Jani Nikula jani.nikula@intel.com Cc: thaytan@noraisin.net Cc: Vasily Khoruzhick anarsoul@gmail.com --- drivers/gpu/drm/drm_dp_helper.c | 83 +------------------ drivers/gpu/drm/drm_dp_mst_topology.c | 3 +- .../drm/i915/display/intel_display_types.h | 1 - drivers/gpu/drm/i915/display/intel_dp.c | 9 +- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +- drivers/gpu/drm/i915/display/intel_psr.c | 2 +- include/drm/drm_dp_helper.h | 21 +---- 7 files changed, 9 insertions(+), 113 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 5bd0934004e3..62f696fe511e 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -1204,7 +1204,7 @@ bool drm_dp_read_sink_count_cap(struct drm_connector *connector, return connector->connector_type != DRM_MODE_CONNECTOR_eDP && dpcd[DP_DPCD_REV] >= DP_DPCD_REV_11 && dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT && - !drm_dp_has_quirk(desc, 0, DP_DPCD_QUIRK_NO_SINK_COUNT); + !drm_dp_has_quirk(desc, DP_DPCD_QUIRK_NO_SINK_COUNT); } EXPORT_SYMBOL(drm_dp_read_sink_count_cap);
@@ -1925,87 +1925,6 @@ drm_dp_get_quirks(const struct drm_dp_dpcd_ident *ident, bool is_branch) #undef DEVICE_ID_ANY #undef DEVICE_ID
-struct edid_quirk { - u8 mfg_id[2]; - u8 prod_id[2]; - u32 quirks; -}; - -#define MFG(first, second) { (first), (second) } -#define PROD_ID(first, second) { (first), (second) } - -/* - * Some devices have unreliable OUIDs where they don't set the device ID - * correctly, and as a result we need to use the EDID for finding additional - * DP quirks in such cases. - */ -static const struct edid_quirk edid_quirk_list[] = { - /* Optional 4K AMOLED panel in the ThinkPad X1 Extreme 2nd Generation - * only supports DPCD backlight controls - */ - { MFG(0x4c, 0x83), PROD_ID(0x41, 0x41), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, - /* - * Some Dell CML 2020 systems have panels support both AUX and PWM - * backlight control, and some only support AUX backlight control. All - * said panels start up in AUX mode by default, and we don't have any - * support for disabling HDR mode on these panels which would be - * required to switch to PWM backlight control mode (plus, I'm not - * even sure we want PWM backlight controls over DPCD backlight - * controls anyway...). Until we have a better way of detecting these, - * force DPCD backlight mode on all of them. - */ - { MFG(0x06, 0xaf), PROD_ID(0x9b, 0x32), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, - { MFG(0x06, 0xaf), PROD_ID(0xeb, 0x41), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, - { MFG(0x4d, 0x10), PROD_ID(0xc7, 0x14), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, - { MFG(0x4d, 0x10), PROD_ID(0xe6, 0x14), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, - { MFG(0x4c, 0x83), PROD_ID(0x47, 0x41), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, - { MFG(0x09, 0xe5), PROD_ID(0xde, 0x08), BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) }, -}; - -#undef MFG -#undef PROD_ID - -/** - * drm_dp_get_edid_quirks() - Check the EDID of a DP device to find additional - * DP-specific quirks - * @edid: The EDID to check - * - * While OUIDs are meant to be used to recognize a DisplayPort device, a lot - * of manufacturers don't seem to like following standards and neglect to fill - * the dev-ID in, making it impossible to only use OUIDs for determining - * quirks in some cases. This function can be used to check the EDID and look - * up any additional DP quirks. The bits returned by this function correspond - * to the quirk bits in &drm_dp_quirk. - * - * Returns: a bitmask of quirks, if any. The driver can check this using - * drm_dp_has_quirk(). - */ -u32 drm_dp_get_edid_quirks(const struct edid *edid) -{ - const struct edid_quirk *quirk; - u32 quirks = 0; - int i; - - if (!edid) - return 0; - - for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) { - quirk = &edid_quirk_list[i]; - if (memcmp(quirk->mfg_id, edid->mfg_id, - sizeof(edid->mfg_id)) == 0 && - memcmp(quirk->prod_id, edid->prod_code, - sizeof(edid->prod_code)) == 0) - quirks |= quirk->quirks; - } - - DRM_DEBUG_KMS("DP sink: EDID mfg %*phD prod-ID %*phD quirks: 0x%04x\n", - (int)sizeof(edid->mfg_id), edid->mfg_id, - (int)sizeof(edid->prod_code), edid->prod_code, quirks); - - return quirks; -} -EXPORT_SYMBOL(drm_dp_get_edid_quirks); - /** * drm_dp_read_desc - read sink/branch descriptor from DPCD * @aux: DisplayPort AUX channel diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 0401b2f47500..a2e692a0c6c2 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -5824,8 +5824,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port) if (drm_dp_read_desc(port->mgr->aux, &desc, true)) return NULL;
- if (drm_dp_has_quirk(&desc, 0, - DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) && + if (drm_dp_has_quirk(&desc, DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) && port->mgr->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14 && port->parent == port->mgr->mst_primary) { u8 downstreamport; diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 889b6f9c1aa9..fcc2b12564e6 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1381,7 +1381,6 @@ struct intel_dp { int max_link_rate; /* sink or branch descriptor */ struct drm_dp_desc desc; - u32 edid_quirks; struct drm_dp_aux aux; u32 aux_busy_last_status; u8 train_set[4]; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 4cb2bfee9c40..6157eaf0432d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -162,8 +162,7 @@ static void intel_dp_set_sink_rates(struct intel_dp *intel_dp) int i, max_rate; int max_lttpr_rate;
- if (drm_dp_has_quirk(&intel_dp->desc, 0, - DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) { + if (drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) { /* Needed, e.g., for Apple MBP 2017, 15 inch eDP Retina panel */ static const int quirk_rates[] = { 162000, 270000, 324000 };
@@ -2788,8 +2787,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, struct intel_connector *intel_connector = intel_dp->attached_connector; struct intel_digital_connector_state *intel_conn_state = to_intel_digital_connector_state(conn_state); - bool constant_n = drm_dp_has_quirk(&intel_dp->desc, 0, - DP_DPCD_QUIRK_CONSTANT_N); + bool constant_n = drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_CONSTANT_N); int ret = 0, output_bpp;
if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A) @@ -6594,7 +6592,6 @@ intel_dp_set_edid(struct intel_dp *intel_dp) }
drm_dp_cec_set_edid(&intel_dp->aux, edid); - intel_dp->edid_quirks = drm_dp_get_edid_quirks(edid); }
static void @@ -6608,7 +6605,6 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
intel_dp->has_hdmi_sink = false; intel_dp->has_audio = false; - intel_dp->edid_quirks = 0;
intel_dp->dfp.max_bpc = 0; intel_dp->dfp.max_dotclock = 0; @@ -8010,7 +8006,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, if (edid) { if (drm_add_edid_modes(connector, edid)) { drm_connector_update_edid_property(connector, edid); - intel_dp->edid_quirks = drm_dp_get_edid_quirks(edid); } else { kfree(edid); edid = ERR_PTR(-EINVAL); diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 27f04aed8764..3e7bbf8d6620 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -53,8 +53,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder, struct drm_i915_private *i915 = to_i915(connector->base.dev); const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; - bool constant_n = drm_dp_has_quirk(&intel_dp->desc, 0, - DP_DPCD_QUIRK_CONSTANT_N); + bool constant_n = drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_CONSTANT_N); int bpp, slots = -EINVAL;
crtc_state->lane_count = limits->max_lane_count; diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index d9a395c486d3..c5c276a4fb91 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -305,7 +305,7 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) drm_dbg_kms(&dev_priv->drm, "eDP panel supports PSR version %x\n", intel_dp->psr_dpcd[0]);
- if (drm_dp_has_quirk(&intel_dp->desc, 0, DP_DPCD_QUIRK_NO_PSR)) { + if (drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_NO_PSR)) { drm_dbg_kms(&dev_priv->drm, "PSR support not currently available for this panel\n"); return; diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 6b40258927bf..cad6a4b1b4a3 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1837,16 +1837,13 @@ struct drm_dp_desc {
int drm_dp_read_desc(struct drm_dp_aux *aux, struct drm_dp_desc *desc, bool is_branch); -u32 drm_dp_get_edid_quirks(const struct edid *edid);
/** * enum drm_dp_quirk - Display Port sink/branch device specific quirks * * Display Port sink and branch devices in the wild have a variety of bugs, try * to collect them here. The quirks are shared, but it's up to the drivers to - * implement workarounds for them. Note that because some devices have - * unreliable OUIDs, the EDID of sinks should also be checked for quirks using - * drm_dp_get_edid_quirks(). + * implement workarounds for them. */ enum drm_dp_quirk { /** @@ -1878,16 +1875,6 @@ enum drm_dp_quirk { * The DSC caps can be read from the physical aux instead. */ DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD, - /** - * @DP_QUIRK_FORCE_DPCD_BACKLIGHT: - * - * The device is telling the truth when it says that it uses DPCD - * backlight controls, even if the system's firmware disagrees. This - * quirk should be checked against both the ident and panel EDID. - * When present, the driver should honor the DPCD backlight - * capabilities advertised. - */ - DP_QUIRK_FORCE_DPCD_BACKLIGHT, /** * @DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS: * @@ -1900,16 +1887,14 @@ enum drm_dp_quirk { /** * drm_dp_has_quirk() - does the DP device have a specific quirk * @desc: Device descriptor filled by drm_dp_read_desc() - * @edid_quirks: Optional quirk bitmask filled by drm_dp_get_edid_quirks() * @quirk: Quirk to query for * * Return true if DP device identified by @desc has @quirk. */ static inline bool -drm_dp_has_quirk(const struct drm_dp_desc *desc, u32 edid_quirks, - enum drm_dp_quirk quirk) +drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk) { - return (desc->quirks | edid_quirks) & BIT(quirk); + return desc->quirks & BIT(quirk); }
#ifdef CONFIG_DRM_DP_CEC
Hi Lyude,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on next-20201204] [cannot apply to linus/master drm/drm-next v5.10-rc6] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Lyude-Paul/drm-i915-Add-support-for... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: arm-defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/18a741e6e1a9f39966c71ef577ef96600891... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Lyude-Paul/drm-i915-Add-support-for-Intel-s-eDP-backlight-controls/20201205-063902 git checkout 18a741e6e1a9f39966c71ef577ef96600891d7ac # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
drivers/gpu/drm/msm/dp/dp_ctrl.c: In function 'dp_ctrl_use_fixed_nvid':
drivers/gpu/drm/msm/dp/dp_ctrl.c:1421:16: error: implicit declaration of function 'drm_dp_get_edid_quirks'; did you mean 'drm_do_get_edid'? [-Werror=implicit-function-declaration]
1421 | edid_quirks = drm_dp_get_edid_quirks(ctrl->panel->edid); | ^~~~~~~~~~~~~~~~~~~~~~ | drm_do_get_edid
drivers/gpu/drm/msm/dp/dp_ctrl.c:1427:11: error: too many arguments to function 'drm_dp_has_quirk'
1427 | return (drm_dp_has_quirk(&ctrl->panel->desc, edid_quirks, | ^~~~~~~~~~~~~~~~ In file included from drivers/gpu/drm/msm/dp/dp_ctrl.c:14: include/drm/drm_dp_helper.h:1895:1: note: declared here 1895 | drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk) | ^~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors
vim +1421 drivers/gpu/drm/msm/dp/dp_ctrl.c
c943b4948b5848 Chandan Uddaraju 2020-08-27 1415 c943b4948b5848 Chandan Uddaraju 2020-08-27 1416 static bool dp_ctrl_use_fixed_nvid(struct dp_ctrl_private *ctrl) c943b4948b5848 Chandan Uddaraju 2020-08-27 1417 { c943b4948b5848 Chandan Uddaraju 2020-08-27 1418 u8 *dpcd = ctrl->panel->dpcd; c943b4948b5848 Chandan Uddaraju 2020-08-27 1419 u32 edid_quirks = 0; c943b4948b5848 Chandan Uddaraju 2020-08-27 1420 c943b4948b5848 Chandan Uddaraju 2020-08-27 @1421 edid_quirks = drm_dp_get_edid_quirks(ctrl->panel->edid); c943b4948b5848 Chandan Uddaraju 2020-08-27 1422 /* c943b4948b5848 Chandan Uddaraju 2020-08-27 1423 * For better interop experience, used a fixed NVID=0x8000 c943b4948b5848 Chandan Uddaraju 2020-08-27 1424 * whenever connected to a VGA dongle downstream. c943b4948b5848 Chandan Uddaraju 2020-08-27 1425 */ c943b4948b5848 Chandan Uddaraju 2020-08-27 1426 if (drm_dp_is_branch(dpcd)) c943b4948b5848 Chandan Uddaraju 2020-08-27 @1427 return (drm_dp_has_quirk(&ctrl->panel->desc, edid_quirks, c943b4948b5848 Chandan Uddaraju 2020-08-27 1428 DP_DPCD_QUIRK_CONSTANT_N)); c943b4948b5848 Chandan Uddaraju 2020-08-27 1429 c943b4948b5848 Chandan Uddaraju 2020-08-27 1430 return false; c943b4948b5848 Chandan Uddaraju 2020-08-27 1431 } c943b4948b5848 Chandan Uddaraju 2020-08-27 1432
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Poke, can we please get some reviews on this? It's been over a week.
On Fri, 2020-12-04 at 17:35 -0500, Lyude Paul wrote:
A while ago we ran into issues while trying to enable the eDP backlight control interface as defined by VESA, in order to make the DPCD backlight controls on newer laptop panels work. The issue ended up being much more complicated however, as we also apparently needed to add support for an Intel-specific DPCD backlight control interface as the VESA interface is broken on many laptop panels. For lack of a better name, we just call this the Intel HDR backlight interface.
While this only adds support for the SDR backlight mode (I think), this will fix a lot of user's laptop panels that we weren't able to properly automatically detect DPCD backlight controls on previously.
Series-wide changes in v3:
- Pass down brightness values to enable/disable backlight callbacks in a
separate patch
- Rebase
Lyude Paul (9): drm/i915/dp: Program source OUI on eDP panels drm/i915: Rename pwm_* backlight callbacks to ext_pwm_* drm/i915: Pass down brightness values to enable/disable backlight callbacks drm/i915: Keep track of pwm-related backlight hooks separately drm/i915/dp: Rename eDP VESA backlight interface functions drm/i915/dp: Add register definitions for Intel HDR backlight interface drm/i915/dp: Enable Intel's HDR backlight interface (only SDR for now) drm/i915/dp: Allow forcing specific interfaces through enable_dpcd_backlight drm/dp: Revert "drm/dp: Introduce EDID-based quirks"
drivers/gpu/drm/drm_dp_helper.c | 83 +--- drivers/gpu/drm/drm_dp_mst_topology.c | 3 +- .../drm/i915/display/intel_display_types.h | 18 +- drivers/gpu/drm/i915/display/intel_dp.c | 42 +- .../drm/i915/display/intel_dp_aux_backlight.c | 394 +++++++++++++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +- .../i915/display/intel_dsi_dcs_backlight.c | 7 +- drivers/gpu/drm/i915/display/intel_panel.c | 435 ++++++++++-------- drivers/gpu/drm/i915/display/intel_panel.h | 4 + drivers/gpu/drm/i915/display/intel_psr.c | 2 +- drivers/gpu/drm/i915/i915_params.c | 2 +- include/drm/drm_dp_helper.h | 21 +- 12 files changed, 655 insertions(+), 359 deletions(-)
On Fri, 04 Dec 2020, Lyude Paul lyude@redhat.com wrote:
A while ago we ran into issues while trying to enable the eDP backlight control interface as defined by VESA, in order to make the DPCD backlight controls on newer laptop panels work. The issue ended up being much more complicated however, as we also apparently needed to add support for an Intel-specific DPCD backlight control interface as the VESA interface is broken on many laptop panels. For lack of a better name, we just call this the Intel HDR backlight interface.
While this only adds support for the SDR backlight mode (I think), this will fix a lot of user's laptop panels that we weren't able to properly automatically detect DPCD backlight controls on previously.
Series-wide changes in v3:
- Pass down brightness values to enable/disable backlight callbacks in a separate patch
- Rebase
Lyude Paul (9): drm/i915/dp: Program source OUI on eDP panels drm/i915: Rename pwm_* backlight callbacks to ext_pwm_* drm/i915: Pass down brightness values to enable/disable backlight callbacks drm/i915/dp: Rename eDP VESA backlight interface functions drm/i915/dp: Add register definitions for Intel HDR backlight interface
Pushed the above patches to din to move things forward, thanks for the patches.
Still looking at the below.
BR, Jani.
drm/i915: Keep track of pwm-related backlight hooks separately drm/i915/dp: Enable Intel's HDR backlight interface (only SDR for now) drm/i915/dp: Allow forcing specific interfaces through enable_dpcd_backlight drm/dp: Revert "drm/dp: Introduce EDID-based quirks"
drivers/gpu/drm/drm_dp_helper.c | 83 +--- drivers/gpu/drm/drm_dp_mst_topology.c | 3 +- .../drm/i915/display/intel_display_types.h | 18 +- drivers/gpu/drm/i915/display/intel_dp.c | 42 +- .../drm/i915/display/intel_dp_aux_backlight.c | 394 +++++++++++++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +- .../i915/display/intel_dsi_dcs_backlight.c | 7 +- drivers/gpu/drm/i915/display/intel_panel.c | 435 ++++++++++-------- drivers/gpu/drm/i915/display/intel_panel.h | 4 + drivers/gpu/drm/i915/display/intel_psr.c | 2 +- drivers/gpu/drm/i915/i915_params.c | 2 +- include/drm/drm_dp_helper.h | 21 +- 12 files changed, 655 insertions(+), 359 deletions(-)
dri-devel@lists.freedesktop.org
-
Jani Nikula -
kernel test robot -
Lyude Paul