2021-01-15 02:55:24

by Lyude Paul

[permalink] [raw]
Subject: [PATCH v7 2/5] drm/i915: Keep track of pwm-related backlight hooks separately

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 CPU mode, and switching
to 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 switching from CPU mode to PCH, 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.

v8:
* Go back to getting initial brightness level with
intel_pwm_get_backlight(), the other fix we had was definitely wrong.
v7:
* Use panel->backlight.pwm_funcs->get() to get the backlight level in
intel_pwm_setup_backlight(), lest we upset lockdep
* Rebase
* Rename intel_panel_sanitize_pwm_level() to intel_panel_invert_pwm_level()
v6:
* Make sure to grab connection_mutex before calling
intel_pwm_get_backlight() in intel_pwm_setup_backlight()
v5:
* Fix indenting warnings from checkpatch
v4:
* Fix commit message
* Remove outdated comment in intel_panel.c
* Rename pwm_(min|max) to pwm_level_(min|max)
* Use intel_pwm_get_backlight() in intel_pwm_setup_backlight() instead of
indirection
* Don't move intel_dp_aux_init_bcklight_funcs() call to bottom of
intel_panel_init_backlight_funcs() quite yet
v3:
* Reuse intel_panel_bl_funcs() for pwm_funcs
* Explain why we drop lpt_get_backlight()

Signed-off-by: Lyude Paul <[email protected]>
Reviewed-by: Jani Nikula <[email protected]>
Cc: [email protected]
Cc: Vasily Khoruzhick <[email protected]>

squash! drm/i915: Keep track of pwm-related backlight hooks separately

Signed-off-by: Lyude Paul <[email protected]>
---
.../drm/i915/display/intel_display_types.h | 4 +
drivers/gpu/drm/i915/display/intel_panel.c | 331 ++++++++++--------
2 files changed, 188 insertions(+), 147 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index b1f4ec144207..ee23cc1518df 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_level_min;
+ u32 pwm_level_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 7587aaefc7a0..cc2386ef6219 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_invert_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_level_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_level_max - val + panel->backlight.pwm_level_min;
}

return 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);
+ 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, enum pipe unused)
{
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
@@ -618,12 +627,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_level_max == 0);

if (panel->backlight.combination_mode) {
u8 lbpc;

- lbpc = level * 0xfe / panel->backlight.max + 1;
+ lbpc = level * 0xfe / panel->backlight.pwm_level_max + 1;
level /= lbpc;
pci_write_config_byte(dev_priv->drm.pdev, LBPC, lbpc);
}
@@ -675,9 +684,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);
}

@@ -726,7 +734,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
@@ -754,7 +762,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);
@@ -765,7 +773,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)
@@ -773,7 +781,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);
@@ -786,7 +794,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),
@@ -800,7 +808,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));
@@ -821,7 +829,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));
@@ -900,7 +908,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_level_max << 16;
intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2);

pch_ctl1 = 0;
@@ -917,7 +925,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,
@@ -952,9 +960,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_level_max << 16;
intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2);

pch_ctl1 = 0;
@@ -981,7 +989,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_level_max;
if (panel->backlight.combination_mode)
freq /= 0xff;

@@ -995,7 +1003,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
@@ -1022,7 +1030,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_level_max;
if (panel->backlight.combination_mode)
freq /= 0xff;

@@ -1038,7 +1046,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,
@@ -1057,11 +1065,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_level_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)
@@ -1110,9 +1118,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_level_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)
@@ -1146,9 +1154,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_level_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)
@@ -1168,7 +1176,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);
@@ -1226,10 +1233,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, intel_connector_get_pipe(connector));
- val = intel_panel_compute_brightness(connector, val);
- }

mutex_unlock(&dev_priv->backlight_lock);

@@ -1560,13 +1565,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");
@@ -1585,7 +1590,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_level_max == 0);

/*
* XXX: If the vbt value is 255, it makes min equal to max, which leads
@@ -1602,7 +1607,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_level_max);
}

static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unused)
@@ -1622,29 +1627,28 @@ 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_level_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_level_max)
+ panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);

- if (!panel->backlight.max)
+ if (!panel->backlight.pwm_level_max)
return -ENODEV;

- panel->backlight.min = get_backlight_min_vbt(connector);
+ panel->backlight.pwm_level_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, unused);
- else
- val = lpt_get_backlight(connector, unused);

if (cpu_mode) {
+ val = intel_panel_invert_pwm_level(connector,
+ pch_get_backlight(connector, unused));
+
drm_dbg_kms(&dev_priv->drm,
"CPU backlight register was enabled, switching to PCH override\n");

@@ -1657,7 +1661,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
cpu_ctl2 & ~BLM_PWM_ENABLE);
}

- val = intel_panel_compute_brightness(connector, val);
+ val = intel_panel_invert_pwm_level(connector, val);
panel->backlight.level = clamp(val, panel->backlight.min,
panel->backlight.max);

@@ -1668,29 +1672,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_level_max = pch_ctl2 >> 16;

- if (!panel->backlight.max)
- panel->backlight.max = get_backlight_max_vbt(connector);
+ if (!panel->backlight.pwm_level_max)
+ panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);

- if (!panel->backlight.max)
+ if (!panel->backlight.pwm_level_max)
return -ENODEV;

- panel->backlight.min = get_backlight_min_vbt(connector);
-
- val = pch_get_backlight(connector, unused);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
- panel->backlight.max);
+ panel->backlight.pwm_level_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;
@@ -1710,27 +1709,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_level_max = ctl >> 17;

- if (!panel->backlight.max) {
- panel->backlight.max = get_backlight_max_vbt(connector);
- panel->backlight.max >>= 1;
+ if (!panel->backlight.pwm_level_max) {
+ panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);
+ panel->backlight.pwm_level_max >>= 1;
}

- if (!panel->backlight.max)
+ if (!panel->backlight.pwm_level_max)
return -ENODEV;

if (panel->backlight.combination_mode)
- panel->backlight.max *= 0xff;
+ panel->backlight.pwm_level_max *= 0xff;

- panel->backlight.min = get_backlight_min_vbt(connector);
+ panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);

val = i9xx_get_backlight(connector, unused);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
- panel->backlight.max);
+ val = intel_panel_invert_pwm_level(connector, val);
+ val = clamp(val, panel->backlight.pwm_level_min, panel->backlight.pwm_level_max);

- panel->backlight.enabled = val != 0;
+ panel->backlight.pwm_enabled = val != 0;

return 0;
}
@@ -1739,32 +1737,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_level_max = ctl >> 16;

- if (!panel->backlight.max)
- panel->backlight.max = get_backlight_max_vbt(connector);
+ if (!panel->backlight.pwm_level_max)
+ panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);

- if (!panel->backlight.max)
+ if (!panel->backlight.pwm_level_max)
return -ENODEV;

if (panel->backlight.combination_mode)
- panel->backlight.max *= 0xff;
+ panel->backlight.pwm_level_max *= 0xff;

- panel->backlight.min = get_backlight_min_vbt(connector);
+ panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);

- val = i9xx_get_backlight(connector, unused);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
- panel->backlight.max);
-
- panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE;
+ panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;

return 0;
}
@@ -1773,7 +1766,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;
@@ -1782,22 +1775,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_level_max = ctl >> 16;

- if (!panel->backlight.max)
- panel->backlight.max = get_backlight_max_vbt(connector);
+ if (!panel->backlight.pwm_level_max)
+ panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);

- if (!panel->backlight.max)
+ if (!panel->backlight.pwm_level_max)
return -ENODEV;

- panel->backlight.min = get_backlight_min_vbt(connector);
+ panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);

- val = vlv_get_backlight(connector, pipe);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
- panel->backlight.max);
-
- panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE;
+ panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;

return 0;
}
@@ -1822,24 +1810,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_level_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_level_max)
+ panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);

- if (!panel->backlight.max)
+ if (!panel->backlight.pwm_level_max)
return -ENODEV;

- panel->backlight.min = get_backlight_min_vbt(connector);
+ panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);

- val = bxt_get_backlight(connector, unused);
- 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;
}
@@ -1849,7 +1831,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
@@ -1862,24 +1844,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_level_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_level_max)
+ panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);

- if (!panel->backlight.max)
+ if (!panel->backlight.pwm_level_max)
return -ENODEV;

- panel->backlight.min = get_backlight_min_vbt(connector);
-
- val = bxt_get_backlight(connector, unused);
- val = intel_panel_compute_brightness(connector, val);
- panel->backlight.level = clamp(val, panel->backlight.min,
- panel->backlight.max);
+ panel->backlight.pwm_level_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;
}
@@ -1909,8 +1885,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_level_max = 100; /* 100% */
+ panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);

if (pwm_is_enabled(panel->backlight.pwm)) {
/* PWM is already enabled, use existing settings */
@@ -1918,10 +1894,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_invert_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,
@@ -1937,6 +1911,58 @@ 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_invert_pwm_level(connector, level));
+}
+
+static u32 intel_pwm_get_backlight(struct intel_connector *connector, enum pipe pipe)
+{
+ struct intel_panel *panel = &connector->panel;
+
+ return intel_panel_invert_pwm_level(connector,
+ panel->backlight.pwm_funcs->get(connector, pipe));
+}
+
+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_invert_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_invert_pwm_level(connector, level));
+}
+
+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_level_min;
+ panel->backlight.max = panel->backlight.pwm_level_max;
+ panel->backlight.level = intel_pwm_get_backlight(connector, pipe);
+ 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,
@@ -2010,7 +2036,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,
@@ -2019,7 +2045,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,
@@ -2028,7 +2054,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,
@@ -2037,7 +2063,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,
@@ -2046,7 +2072,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,
@@ -2063,7 +2089,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,
@@ -2072,7 +2098,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,
@@ -2081,7 +2107,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,
@@ -2090,6 +2116,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)
@@ -2107,27 +2141,30 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
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;
}
+
+ /* We're using a standard PWM backlight interface */
+ panel->backlight.funcs = &pwm_bl_funcs;
}

enum drm_connector_status
--
2.29.2


2021-01-15 13:50:45

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH v7 2/5] drm/i915: Keep track of pwm-related backlight hooks separately

On Thu, 14 Jan 2021, Lyude Paul <[email protected]> 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 CPU mode, and switching
> to 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 switching from CPU mode to PCH, 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.
>
> v8:
> * Go back to getting initial brightness level with
> intel_pwm_get_backlight(), the other fix we had was definitely wrong.
> v7:
> * Use panel->backlight.pwm_funcs->get() to get the backlight level in
> intel_pwm_setup_backlight(), lest we upset lockdep
> * Rebase
> * Rename intel_panel_sanitize_pwm_level() to intel_panel_invert_pwm_level()
> v6:
> * Make sure to grab connection_mutex before calling
> intel_pwm_get_backlight() in intel_pwm_setup_backlight()
> v5:
> * Fix indenting warnings from checkpatch
> v4:
> * Fix commit message
> * Remove outdated comment in intel_panel.c
> * Rename pwm_(min|max) to pwm_level_(min|max)
> * Use intel_pwm_get_backlight() in intel_pwm_setup_backlight() instead of
> indirection
> * Don't move intel_dp_aux_init_bcklight_funcs() call to bottom of
> intel_panel_init_backlight_funcs() quite yet
> v3:
> * Reuse intel_panel_bl_funcs() for pwm_funcs
> * Explain why we drop lpt_get_backlight()
>
> Signed-off-by: Lyude Paul <[email protected]>
> Reviewed-by: Jani Nikula <[email protected]>
> Cc: [email protected]
> Cc: Vasily Khoruzhick <[email protected]>
>
> squash! drm/i915: Keep track of pwm-related backlight hooks separately
>
> Signed-off-by: Lyude Paul <[email protected]>
> ---
> .../drm/i915/display/intel_display_types.h | 4 +
> drivers/gpu/drm/i915/display/intel_panel.c | 331 ++++++++++--------
> 2 files changed, 188 insertions(+), 147 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index b1f4ec144207..ee23cc1518df 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_level_min;
> + u32 pwm_level_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 7587aaefc7a0..cc2386ef6219 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_invert_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_level_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_level_max - val + panel->backlight.pwm_level_min;
> }
>
> return 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);
> + 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, enum pipe unused)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> @@ -618,12 +627,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_level_max == 0);
>
> if (panel->backlight.combination_mode) {
> u8 lbpc;
>
> - lbpc = level * 0xfe / panel->backlight.max + 1;
> + lbpc = level * 0xfe / panel->backlight.pwm_level_max + 1;
> level /= lbpc;
> pci_write_config_byte(dev_priv->drm.pdev, LBPC, lbpc);
> }
> @@ -675,9 +684,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);
> }
>
> @@ -726,7 +734,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
> @@ -754,7 +762,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);
> @@ -765,7 +773,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)
> @@ -773,7 +781,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);
> @@ -786,7 +794,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),
> @@ -800,7 +808,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));
> @@ -821,7 +829,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));
> @@ -900,7 +908,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_level_max << 16;
> intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2);
>
> pch_ctl1 = 0;
> @@ -917,7 +925,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,
> @@ -952,9 +960,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_level_max << 16;
> intel_de_write(dev_priv, BLC_PWM_PCH_CTL2, pch_ctl2);
>
> pch_ctl1 = 0;
> @@ -981,7 +989,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_level_max;
> if (panel->backlight.combination_mode)
> freq /= 0xff;
>
> @@ -995,7 +1003,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
> @@ -1022,7 +1030,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_level_max;
> if (panel->backlight.combination_mode)
> freq /= 0xff;
>
> @@ -1038,7 +1046,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,
> @@ -1057,11 +1065,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_level_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)
> @@ -1110,9 +1118,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_level_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)
> @@ -1146,9 +1154,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_level_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)
> @@ -1168,7 +1176,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);
> @@ -1226,10 +1233,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, intel_connector_get_pipe(connector));
> - val = intel_panel_compute_brightness(connector, val);
> - }
>
> mutex_unlock(&dev_priv->backlight_lock);
>
> @@ -1560,13 +1565,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");
> @@ -1585,7 +1590,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_level_max == 0);
>
> /*
> * XXX: If the vbt value is 255, it makes min equal to max, which leads
> @@ -1602,7 +1607,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_level_max);
> }
>
> static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unused)
> @@ -1622,29 +1627,28 @@ 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_level_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_level_max)
> + panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);
>
> - if (!panel->backlight.max)
> + if (!panel->backlight.pwm_level_max)
> return -ENODEV;
>
> - panel->backlight.min = get_backlight_min_vbt(connector);
> + panel->backlight.pwm_level_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, unused);
> - else
> - val = lpt_get_backlight(connector, unused);
>
> if (cpu_mode) {
> + val = intel_panel_invert_pwm_level(connector,
> + pch_get_backlight(connector, unused));
> +

I think the above should be just:

val = pch_get_backlight(connector, unused);

> drm_dbg_kms(&dev_priv->drm,
> "CPU backlight register was enabled, switching to PCH override\n");
>
> @@ -1657,7 +1661,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
> cpu_ctl2 & ~BLM_PWM_ENABLE);
> }
>

And all of the below should be removed:

> - val = intel_panel_compute_brightness(connector, val);
> + val = intel_panel_invert_pwm_level(connector, val);
> panel->backlight.level = clamp(val, panel->backlight.min,
> panel->backlight.max);

With that,

Reviewed-by: Jani Nikula <[email protected]>


>
> @@ -1668,29 +1672,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_level_max = pch_ctl2 >> 16;
>
> - if (!panel->backlight.max)
> - panel->backlight.max = get_backlight_max_vbt(connector);
> + if (!panel->backlight.pwm_level_max)
> + panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);
>
> - if (!panel->backlight.max)
> + if (!panel->backlight.pwm_level_max)
> return -ENODEV;
>
> - panel->backlight.min = get_backlight_min_vbt(connector);
> -
> - val = pch_get_backlight(connector, unused);
> - val = intel_panel_compute_brightness(connector, val);
> - panel->backlight.level = clamp(val, panel->backlight.min,
> - panel->backlight.max);
> + panel->backlight.pwm_level_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;
> @@ -1710,27 +1709,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_level_max = ctl >> 17;
>
> - if (!panel->backlight.max) {
> - panel->backlight.max = get_backlight_max_vbt(connector);
> - panel->backlight.max >>= 1;
> + if (!panel->backlight.pwm_level_max) {
> + panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);
> + panel->backlight.pwm_level_max >>= 1;
> }
>
> - if (!panel->backlight.max)
> + if (!panel->backlight.pwm_level_max)
> return -ENODEV;
>
> if (panel->backlight.combination_mode)
> - panel->backlight.max *= 0xff;
> + panel->backlight.pwm_level_max *= 0xff;
>
> - panel->backlight.min = get_backlight_min_vbt(connector);
> + panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);
>
> val = i9xx_get_backlight(connector, unused);
> - val = intel_panel_compute_brightness(connector, val);
> - panel->backlight.level = clamp(val, panel->backlight.min,
> - panel->backlight.max);
> + val = intel_panel_invert_pwm_level(connector, val);
> + val = clamp(val, panel->backlight.pwm_level_min, panel->backlight.pwm_level_max);
>
> - panel->backlight.enabled = val != 0;
> + panel->backlight.pwm_enabled = val != 0;
>
> return 0;
> }
> @@ -1739,32 +1737,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_level_max = ctl >> 16;
>
> - if (!panel->backlight.max)
> - panel->backlight.max = get_backlight_max_vbt(connector);
> + if (!panel->backlight.pwm_level_max)
> + panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);
>
> - if (!panel->backlight.max)
> + if (!panel->backlight.pwm_level_max)
> return -ENODEV;
>
> if (panel->backlight.combination_mode)
> - panel->backlight.max *= 0xff;
> + panel->backlight.pwm_level_max *= 0xff;
>
> - panel->backlight.min = get_backlight_min_vbt(connector);
> + panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);
>
> - val = i9xx_get_backlight(connector, unused);
> - val = intel_panel_compute_brightness(connector, val);
> - panel->backlight.level = clamp(val, panel->backlight.min,
> - panel->backlight.max);
> -
> - panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE;
> + panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;
>
> return 0;
> }
> @@ -1773,7 +1766,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;
> @@ -1782,22 +1775,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_level_max = ctl >> 16;
>
> - if (!panel->backlight.max)
> - panel->backlight.max = get_backlight_max_vbt(connector);
> + if (!panel->backlight.pwm_level_max)
> + panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);
>
> - if (!panel->backlight.max)
> + if (!panel->backlight.pwm_level_max)
> return -ENODEV;
>
> - panel->backlight.min = get_backlight_min_vbt(connector);
> + panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);
>
> - val = vlv_get_backlight(connector, pipe);
> - val = intel_panel_compute_brightness(connector, val);
> - panel->backlight.level = clamp(val, panel->backlight.min,
> - panel->backlight.max);
> -
> - panel->backlight.enabled = ctl2 & BLM_PWM_ENABLE;
> + panel->backlight.pwm_enabled = ctl2 & BLM_PWM_ENABLE;
>
> return 0;
> }
> @@ -1822,24 +1810,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_level_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_level_max)
> + panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);
>
> - if (!panel->backlight.max)
> + if (!panel->backlight.pwm_level_max)
> return -ENODEV;
>
> - panel->backlight.min = get_backlight_min_vbt(connector);
> + panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);
>
> - val = bxt_get_backlight(connector, unused);
> - 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;
> }
> @@ -1849,7 +1831,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
> @@ -1862,24 +1844,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_level_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_level_max)
> + panel->backlight.pwm_level_max = get_backlight_max_vbt(connector);
>
> - if (!panel->backlight.max)
> + if (!panel->backlight.pwm_level_max)
> return -ENODEV;
>
> - panel->backlight.min = get_backlight_min_vbt(connector);
> -
> - val = bxt_get_backlight(connector, unused);
> - val = intel_panel_compute_brightness(connector, val);
> - panel->backlight.level = clamp(val, panel->backlight.min,
> - panel->backlight.max);
> + panel->backlight.pwm_level_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;
> }
> @@ -1909,8 +1885,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_level_max = 100; /* 100% */
> + panel->backlight.pwm_level_min = get_backlight_min_vbt(connector);
>
> if (pwm_is_enabled(panel->backlight.pwm)) {
> /* PWM is already enabled, use existing settings */
> @@ -1918,10 +1894,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_invert_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,
> @@ -1937,6 +1911,58 @@ 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_invert_pwm_level(connector, level));
> +}
> +
> +static u32 intel_pwm_get_backlight(struct intel_connector *connector, enum pipe pipe)
> +{
> + struct intel_panel *panel = &connector->panel;
> +
> + return intel_panel_invert_pwm_level(connector,
> + panel->backlight.pwm_funcs->get(connector, pipe));
> +}
> +
> +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_invert_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_invert_pwm_level(connector, level));
> +}
> +
> +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_level_min;
> + panel->backlight.max = panel->backlight.pwm_level_max;
> + panel->backlight.level = intel_pwm_get_backlight(connector, pipe);
> + 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,
> @@ -2010,7 +2036,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,
> @@ -2019,7 +2045,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,
> @@ -2028,7 +2054,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,
> @@ -2037,7 +2063,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,
> @@ -2046,7 +2072,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,
> @@ -2063,7 +2089,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,
> @@ -2072,7 +2098,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,
> @@ -2081,7 +2107,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,
> @@ -2090,6 +2116,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)
> @@ -2107,27 +2141,30 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
> 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;
> }
> +
> + /* We're using a standard PWM backlight interface */
> + panel->backlight.funcs = &pwm_bl_funcs;
> }
>
> enum drm_connector_status

--
Jani Nikula, Intel Open Source Graphics Center