[PATCH 0/3] drm/i915: Pass atomic state to backlight.

[PATCH 0/3] drm/i915: Pass atomic state to backlight.

[Maarten Lankhorst]

This fixes the following WARN_ON that may happen when running
kms_cursor_legacy and kms_atomic_transitions on a system with backlight:

[  219.968428] ------------[ cut here ]------------
[  219.968481] WARNING: CPU: 3 PID: 2457 at drivers/gpu/drm/i915/intel_display.c:13881 intel_get_pipe_from_connector+0x62/0x90 [i915]
[  219.968483] WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex))
[  219.968485] Modules linked in: nls_iso8859_1 snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_pcm intel_rapl x86_pkg_temp_thermal coretemp kvm_intel snd_seq_midi snd_seq_midi_event kvm snd_rawmidi irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc snd_seq snd_seq_device serio_raw snd_timer aesni_intel aes_x86_64 crypto_simd glue_helper cryptd lpc_ich snd mei_me shpchp soundcore mei rfkill_gpio mac_hid intel_pmc_ipc parport_pc ppdev lp parport ip_tables x_tables autofs4 hid_generic usbhid igb ahci i915 xhci_pci dca xhci_hcd ptp sdhci_pci sdhci libahci pps_core i2c_hid hid video
[  219.968573] CPU: 3 PID: 2457 Comm: kworker/u8:3 Tainted: G        W       4.10.0-tip-201703010159+ #2
[  219.968575] Hardware name: Intel Corp. Broxton P/NOTEBOOK, BIOS APLKRVPA.X64.0144.B10.1606270006 06/27/2016
[  219.968627] Workqueue: events_unbound intel_atomic_commit_work [i915]
[  219.968629] Call Trace:
[  219.968640]  dump_stack+0x63/0x87
[  219.968646]  __warn+0xd1/0xf0
[  219.968651]  warn_slowpath_fmt+0x4f/0x60
[  219.968657]  ? drm_printk+0x97/0xa0
[  219.968708]  intel_get_pipe_from_connector+0x62/0x90 [i915]
[  219.968756]  intel_panel_enable_backlight+0x19/0xf0 [i915]
[  219.968804]  intel_edp_backlight_on.part.22+0x33/0x40 [i915]
[  219.968852]  intel_edp_backlight_on+0x18/0x20 [i915]
[  219.968900]  intel_enable_ddi+0x94/0xc0 [i915]
[  219.968950]  intel_encoders_enable.isra.93+0x77/0x90 [i915]
[  219.969000]  haswell_crtc_enable+0x310/0x7f0 [i915]
[  219.969051]  intel_update_crtc+0x58/0x100 [i915]
[  219.969101]  skl_update_crtcs+0x218/0x240 [i915]
[  219.969153]  intel_atomic_commit_tail+0x350/0x1000 [i915]
[  219.969159]  ? vtime_account_idle+0xe/0x50
[  219.969164]  ? finish_task_switch+0x107/0x250
[  219.969214]  intel_atomic_commit_work+0x12/0x20 [i915]
[  219.969219]  process_one_work+0x153/0x3f0
[  219.969223]  worker_thread+0x12b/0x4b0
[  219.969227]  kthread+0x101/0x140
[  219.969230]  ? rescuer_thread+0x340/0x340
[  219.969233]  ? kthread_park+0x90/0x90
[  219.969237]  ? do_syscall_64+0x6e/0x180
[  219.969243]  ret_from_fork+0x2c/0x40
[  219.969246] ---[ end trace 0a8fa19387b9ad6d ]---

Maarten Lankhorst (3):
  drm/i915: Pass crtc_state and connector state to backlight
    enable/disable functions
  drm/i915: Pass connector state to intel_panel_set_backlight_acpi
  drm/i915: Pass atomic state to backlight enable/disable/set callbacks.

 drivers/gpu/drm/i915/intel_ddi.c               |   4 +-
 drivers/gpu/drm/i915/intel_dp.c                |  21 +--
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c  |  25 +++-
 drivers/gpu/drm/i915/intel_drv.h               |  19 +--
 drivers/gpu/drm/i915/intel_dsi.c               |   4 +-
 drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c |  22 ++-
 drivers/gpu/drm/i915/intel_lvds.c              |  16 +--
 drivers/gpu/drm/i915/intel_opregion.c          |   2 +-
 drivers/gpu/drm/i915/intel_panel.c             | 177 +++++++++++++------------
 9 files changed, 153 insertions(+), 137 deletions(-)

-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915: Pass crtc_state and connector state to backlight enable/disable functions

[Maarten Lankhorst]

The backlight functions need to determine the pipe and the transcoder the
backlight will be enabled on, so pass crtc_state instead of trying to
dereference the state without holding locks.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100022
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c   |  4 ++--
 drivers/gpu/drm/i915/intel_dp.c    | 21 ++++++++++++---------
 drivers/gpu/drm/i915/intel_drv.h   | 10 ++++++----
 drivers/gpu/drm/i915/intel_dsi.c   |  4 ++--
 drivers/gpu/drm/i915/intel_lvds.c  | 16 +++-------------
 drivers/gpu/drm/i915/intel_panel.c | 12 +++++++-----
 6 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 8bac62805cd1..2d35d97d170e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1845,7 +1845,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
 		if (port == PORT_A && INTEL_GEN(dev_priv) < 9)
 			intel_dp_stop_link_train(intel_dp);
 
-		intel_edp_backlight_on(intel_dp);
+		intel_edp_backlight_on(pipe_config, conn_state);
 		intel_psr_enable(intel_dp);
 		intel_edp_drrs_enable(intel_dp, pipe_config);
 	}
@@ -1875,7 +1875,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
 
 		intel_edp_drrs_disable(intel_dp, old_crtc_state);
 		intel_psr_disable(intel_dp);
-		intel_edp_backlight_off(intel_dp);
+		intel_edp_backlight_off(old_conn_state);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index db5133800d8c..950646fab48c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2311,14 +2311,17 @@ static void _intel_edp_backlight_on(struct intel_dp *intel_dp)
 }
 
 /* Enable backlight PWM and backlight PP control. */
-void intel_edp_backlight_on(struct intel_dp *intel_dp)
+void intel_edp_backlight_on(const struct intel_crtc_state *crtc_state,
+			    const struct drm_connector_state *conn_state)
 {
+	struct intel_dp *intel_dp = enc_to_intel_dp(conn_state->best_encoder);
+
 	if (!is_edp(intel_dp))
 		return;
 
 	DRM_DEBUG_KMS("\n");
 
-	intel_panel_enable_backlight(intel_dp->attached_connector);
+	intel_panel_enable_backlight(crtc_state, conn_state);
 	_intel_edp_backlight_on(intel_dp);
 }
 
@@ -2350,15 +2353,17 @@ static void _intel_edp_backlight_off(struct intel_dp *intel_dp)
 }
 
 /* Disable backlight PP control and backlight PWM. */
-void intel_edp_backlight_off(struct intel_dp *intel_dp)
+void intel_edp_backlight_off(const struct drm_connector_state *old_conn_state)
 {
+	struct intel_dp *intel_dp = enc_to_intel_dp(old_conn_state->best_encoder);
+
 	if (!is_edp(intel_dp))
 		return;
 
 	DRM_DEBUG_KMS("\n");
 
 	_intel_edp_backlight_off(intel_dp);
-	intel_panel_disable_backlight(intel_dp->attached_connector);
+	intel_panel_disable_backlight(old_conn_state);
 }
 
 /*
@@ -2654,7 +2659,7 @@ static void intel_disable_dp(struct intel_encoder *encoder,
 	/* Make sure the panel is off before trying to change the mode. But also
 	 * ensure that we have vdd while we switch off the panel. */
 	intel_edp_panel_vdd_on(intel_dp);
-	intel_edp_backlight_off(intel_dp);
+	intel_edp_backlight_off(old_conn_state);
 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 	intel_edp_panel_off(intel_dp);
 
@@ -2868,10 +2873,8 @@ static void g4x_enable_dp(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config,
 			  struct drm_connector_state *conn_state)
 {
-	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-
 	intel_enable_dp(encoder, pipe_config, conn_state);
-	intel_edp_backlight_on(intel_dp);
+	intel_edp_backlight_on(pipe_config, conn_state);
 }
 
 static void vlv_enable_dp(struct intel_encoder *encoder,
@@ -2880,7 +2883,7 @@ static void vlv_enable_dp(struct intel_encoder *encoder,
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
-	intel_edp_backlight_on(intel_dp);
+	intel_edp_backlight_on(pipe_config, conn_state);
 	intel_psr_enable(intel_dp);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 83dd40905821..605af676a87d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1519,8 +1519,9 @@ bool intel_dp_compute_config(struct intel_encoder *encoder,
 bool intel_dp_is_edp(struct drm_i915_private *dev_priv, enum port port);
 enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
 				  bool long_hpd);
-void intel_edp_backlight_on(struct intel_dp *intel_dp);
-void intel_edp_backlight_off(struct intel_dp *intel_dp);
+void intel_edp_backlight_on(const struct intel_crtc_state *crtc_state,
+			    const struct drm_connector_state *conn_state);
+void intel_edp_backlight_off(const struct drm_connector_state *conn_state);
 void intel_edp_panel_vdd_on(struct intel_dp *intel_dp);
 void intel_edp_panel_on(struct intel_dp *intel_dp);
 void intel_edp_panel_off(struct intel_dp *intel_dp);
@@ -1704,8 +1705,9 @@ void intel_panel_set_backlight_acpi(struct intel_connector *connector,
 				    u32 level, u32 max);
 int intel_panel_setup_backlight(struct drm_connector *connector,
 				enum pipe pipe);
-void intel_panel_enable_backlight(struct intel_connector *connector);
-void intel_panel_disable_backlight(struct intel_connector *connector);
+void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
+				  const struct drm_connector_state *conn_state);
+void intel_panel_disable_backlight(const struct drm_connector_state *old_conn_state);
 void intel_panel_destroy_backlight(struct drm_connector *connector);
 enum drm_connector_status intel_panel_detect(struct drm_i915_private *dev_priv);
 extern struct drm_display_mode *intel_find_panel_downclock(
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 54030b68406a..721f3f3adc1e 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -835,7 +835,7 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 		intel_dsi_port_enable(encoder);
 	}
 
-	intel_panel_enable_backlight(intel_dsi->attached_connector);
+	intel_panel_enable_backlight(pipe_config, conn_state);
 	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 }
 
@@ -866,7 +866,7 @@ static void intel_dsi_disable(struct intel_encoder *encoder,
 	DRM_DEBUG_KMS("\n");
 
 	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
-	intel_panel_disable_backlight(intel_dsi->attached_connector);
+	intel_panel_disable_backlight(old_conn_state);
 
 	/*
 	 * Disable Device ready before the port shutdown in order
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d2c2bca1b327..6fe5d7c3bc23 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -311,8 +311,6 @@ static void intel_enable_lvds(struct intel_encoder *encoder,
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
-	struct intel_connector *intel_connector =
-		&lvds_encoder->attached_connector->base;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
 	I915_WRITE(lvds_encoder->reg, I915_READ(lvds_encoder->reg) | LVDS_PORT_EN);
@@ -322,7 +320,7 @@ static void intel_enable_lvds(struct intel_encoder *encoder,
 	if (intel_wait_for_register(dev_priv, PP_STATUS(0), PP_ON, PP_ON, 1000))
 		DRM_ERROR("timed out waiting for panel to power on\n");
 
-	intel_panel_enable_backlight(intel_connector);
+	intel_panel_enable_backlight(pipe_config, conn_state);
 }
 
 static void intel_disable_lvds(struct intel_encoder *encoder,
@@ -345,11 +343,7 @@ static void gmch_disable_lvds(struct intel_encoder *encoder,
 			      struct drm_connector_state *old_conn_state)
 
 {
-	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
-	struct intel_connector *intel_connector =
-		&lvds_encoder->attached_connector->base;
-
-	intel_panel_disable_backlight(intel_connector);
+	intel_panel_disable_backlight(old_conn_state);
 
 	intel_disable_lvds(encoder, old_crtc_state, old_conn_state);
 }
@@ -358,11 +352,7 @@ static void pch_disable_lvds(struct intel_encoder *encoder,
 			     struct intel_crtc_state *old_crtc_state,
 			     struct drm_connector_state *old_conn_state)
 {
-	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
-	struct intel_connector *intel_connector =
-		&lvds_encoder->attached_connector->base;
-
-	intel_panel_disable_backlight(intel_connector);
+	intel_panel_disable_backlight(old_conn_state);
 }
 
 static void pch_post_disable_lvds(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 4114cb3f14e7..2567533544aa 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -819,8 +819,9 @@ static void pwm_disable_backlight(struct intel_connector *connector)
 	pwm_disable(panel->backlight.pwm);
 }
 
-void intel_panel_disable_backlight(struct intel_connector *connector)
+void intel_panel_disable_backlight(const struct drm_connector_state *old_conn_state)
 {
+	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;
 
@@ -1136,17 +1137,18 @@ static void pwm_enable_backlight(struct intel_connector *connector)
 	intel_panel_actually_set_backlight(connector, panel->backlight.level);
 }
 
-void intel_panel_enable_backlight(struct intel_connector *connector)
+void intel_panel_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
-	enum pipe pipe = intel_get_pipe_from_connector(connector);
+	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
 
 	if (!panel->backlight.present)
 		return;
 
-	if (!WARN_ON_ONCE(pipe == INVALID_PIPE))
-		DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
+	DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
 
 	mutex_lock(&dev_priv->backlight_lock);
 
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Pass connector state to intel_panel_set_backlight_acpi

[Maarten Lankhorst]

Passing the state is also needed to convert the backlight functions
to use the correct state instead of looking it up.

This is done as a separate commit to allow easier bisecting.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100022
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h      | 2 +-
 drivers/gpu/drm/i915/intel_opregion.c | 2 +-
 drivers/gpu/drm/i915/intel_panel.c    | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 605af676a87d..db2042a75d42 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1701,7 +1701,7 @@ void intel_pch_panel_fitting(struct intel_crtc *crtc,
 void intel_gmch_panel_fitting(struct intel_crtc *crtc,
 			      struct intel_crtc_state *pipe_config,
 			      int fitting_mode);
-void intel_panel_set_backlight_acpi(struct intel_connector *connector,
+void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state,
 				    u32 level, u32 max);
 int intel_panel_setup_backlight(struct drm_connector *connector,
 				enum pipe pipe);
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index d44465190dc1..2bd03001cc70 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -461,7 +461,7 @@ static u32 asle_set_backlight(struct drm_i915_private *dev_priv, u32 bclp)
 	DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	for_each_intel_connector_iter(connector, &conn_iter)
-		intel_panel_set_backlight_acpi(connector, bclp, 255);
+		intel_panel_set_backlight_acpi(connector->base.state, bclp, 255);
 	drm_connector_list_iter_end(&conn_iter);
 	asle->cblv = DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID;
 
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 2567533544aa..8cb573166421 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -671,21 +671,21 @@ static void intel_panel_set_backlight(struct intel_connector *connector,
 /* set backlight brightness to level in range [0..max], assuming hw min is
  * respected.
  */
-void intel_panel_set_backlight_acpi(struct intel_connector *connector,
+void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state,
 				    u32 user_level, u32 user_max)
 {
+	struct intel_connector *connector = to_intel_connector(conn_state->connector);
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
-	enum pipe pipe = intel_get_pipe_from_connector(connector);
 	u32 hw_level;
 
 	/*
-	 * INVALID_PIPE may occur during driver init because
+	 * Lack of crtc may occur during driver init because
 	 * connection_mutex isn't held across the entire backlight
 	 * setup + modeset readout, and the BIOS can issue the
 	 * requests at any time.
 	 */
-	if (!panel->backlight.present || pipe == INVALID_PIPE)
+	if (!panel->backlight.present || !conn_state->crtc)
 		return;
 
 	mutex_lock(&dev_priv->backlight_lock);
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Pass atomic state to backlight enable/disable/set callbacks.

[Maarten Lankhorst]

Pass crtc_state to the enable callback, and connector_state to all callbacks.
This will eliminate the need to guess for the correct pipe in these
callbacks.

The crtc state is required for pch_enable_backlight to obtain the correct
cpu_transcoder.

intel_dp_aux_backlight's setup function is called before hw readout, so
crtc_state and connector_state->best_encoder are NULL in the enable()
and set() callbacks.

This fixes the following series of warns from intel_get_pipe_from_connector:
[  219.968428] ------------[ cut here ]------------
[  219.968481] WARNING: CPU: 3 PID: 2457 at
drivers/gpu/drm/i915/intel_display.c:13881
intel_get_pipe_from_connector+0x62/0x90 [i915]
[  219.968483]
WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex))
[  219.968485] Modules linked in: nls_iso8859_1 snd_hda_codec_hdmi
snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec
snd_hda_core snd_hwdep snd_pcm intel_rapl x86_pkg_temp_thermal coretemp
kvm_intel snd_seq_midi snd_seq_midi_event kvm snd_rawmidi irqbypass
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc snd_seq
snd_seq_device serio_raw snd_timer aesni_intel aes_x86_64 crypto_simd
glue_helper cryptd lpc_ich snd mei_me shpchp soundcore mei rfkill_gpio
mac_hid intel_pmc_ipc parport_pc ppdev lp parport ip_tables x_tables
autofs4 hid_generic usbhid igb ahci i915 xhci_pci dca xhci_hcd ptp
sdhci_pci sdhci libahci pps_core i2c_hid hid video
[  219.968573] CPU: 3 PID: 2457 Comm: kworker/u8:3 Tainted: G        W
4.10.0-tip-201703010159+ #2
[  219.968575] Hardware name: Intel Corp. Broxton P/NOTEBOOK, BIOS
APLKRVPA.X64.0144.B10.1606270006 06/27/2016
[  219.968627] Workqueue: events_unbound intel_atomic_commit_work [i915]
[  219.968629] Call Trace:
[  219.968640]  dump_stack+0x63/0x87
[  219.968646]  __warn+0xd1/0xf0
[  219.968651]  warn_slowpath_fmt+0x4f/0x60
[  219.968657]  ? drm_printk+0x97/0xa0
[  219.968708]  intel_get_pipe_from_connector+0x62/0x90 [i915]
[  219.968756]  intel_panel_enable_backlight+0x19/0xf0 [i915]
[  219.968804]  intel_edp_backlight_on.part.22+0x33/0x40 [i915]
[  219.968852]  intel_edp_backlight_on+0x18/0x20 [i915]
[  219.968900]  intel_enable_ddi+0x94/0xc0 [i915]
[  219.968950]  intel_encoders_enable.isra.93+0x77/0x90 [i915]
[  219.969000]  haswell_crtc_enable+0x310/0x7f0 [i915]
[  219.969051]  intel_update_crtc+0x58/0x100 [i915]
[  219.969101]  skl_update_crtcs+0x218/0x240 [i915]
[  219.969153]  intel_atomic_commit_tail+0x350/0x1000 [i915]
[  219.969159]  ? vtime_account_idle+0xe/0x50
[  219.969164]  ? finish_task_switch+0x107/0x250
[  219.969214]  intel_atomic_commit_work+0x12/0x20 [i915]
[  219.969219]  process_one_work+0x153/0x3f0
[  219.969223]  worker_thread+0x12b/0x4b0
[  219.969227]  kthread+0x101/0x140
[  219.969230]  ? rescuer_thread+0x340/0x340
[  219.969233]  ? kthread_park+0x90/0x90
[  219.969237]  ? do_syscall_64+0x6e/0x180
[  219.969243]  ret_from_fork+0x2c/0x40
[  219.969246] ---[ end trace 0a8fa19387b9ad6d ]---

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100022
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp_aux_backlight.c  |  25 ++--
 drivers/gpu/drm/i915/intel_drv.h               |   7 +-
 drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c |  22 ++--
 drivers/gpu/drm/i915/intel_panel.c             | 157 +++++++++++++------------
 4 files changed, 115 insertions(+), 96 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
index a0995c00fc84..6cc62980d0da 100644
--- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
@@ -78,8 +78,13 @@ static uint32_t 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(struct intel_connector *connector, u32 level)
+intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32 level)
 {
+	/*
+	 * conn_state->best_encoder is likely NULL when called from
+	 * intel_dp_aux_setup_backlight()
+	 */
+	struct intel_connector *connector = to_intel_connector(conn_state->connector);
 	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
 	uint8_t vals[2] = { 0x0 };
 
@@ -97,8 +102,14 @@ intel_dp_aux_set_backlight(struct intel_connector *connector, u32 level)
 	}
 }
 
-static void intel_dp_aux_enable_backlight(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)
 {
+	struct intel_connector *connector = to_intel_connector(conn_state->connector);
+	/*
+	 * conn_state->best_encoder (and crtc_state) are NULL when called from
+	 * intel_dp_aux_setup_backlight()
+	 */
 	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
 	uint8_t dpcd_buf = 0;
 	uint8_t edp_backlight_mode = 0;
@@ -131,12 +142,12 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
 	}
 
 	set_aux_backlight_enable(intel_dp, true);
-	intel_dp_aux_set_backlight(connector, connector->panel.backlight.level);
+	intel_dp_aux_set_backlight(conn_state, connector->panel.backlight.level);
 }
 
-static void intel_dp_aux_disable_backlight(struct intel_connector *connector)
+static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old_conn_state)
 {
-	set_aux_backlight_enable(enc_to_intel_dp(&connector->encoder->base), false);
+	set_aux_backlight_enable(enc_to_intel_dp(old_conn_state->best_encoder), false);
 }
 
 static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
@@ -145,7 +156,7 @@ static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
 	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
 	struct intel_panel *panel = &connector->panel;
 
-	intel_dp_aux_enable_backlight(connector);
+	intel_dp_aux_enable_backlight(NULL, connector->base.state);
 
 	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
 		panel->backlight.max = 0xFFFF;
@@ -165,7 +176,7 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
 
-	/* Check the  eDP Display control capabilities registers to determine if
+	/* Check the eDP Display control capabilities registers to determine if
 	 * the panel can support backlight control over the aux channel
 	 */
 	if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index db2042a75d42..18528a477cf6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -288,9 +288,10 @@ struct intel_panel {
 		/* Connector and platform specific backlight functions */
 		int (*setup)(struct intel_connector *connector, enum pipe pipe);
 		uint32_t (*get)(struct intel_connector *connector);
-		void (*set)(struct intel_connector *connector, uint32_t level);
-		void (*disable)(struct intel_connector *connector);
-		void (*enable)(struct intel_connector *connector);
+		void (*set)(const struct drm_connector_state *conn_state, uint32_t level);
+		void (*disable)(const struct drm_connector_state *conn_state);
+		void (*enable)(const struct intel_crtc_state *crtc_state,
+			       const struct drm_connector_state *conn_state);
 		uint32_t (*hz_to_pwm)(struct intel_connector *connector,
 				      uint32_t hz);
 		void (*power)(struct intel_connector *, bool enable);
diff --git a/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c b/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
index ac7c6020c443..6e09ceb71500 100644
--- a/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
+++ b/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
@@ -60,10 +60,9 @@ static u32 dcs_get_backlight(struct intel_connector *connector)
 	return data;
 }
 
-static void dcs_set_backlight(struct intel_connector *connector, u32 level)
+static void dcs_set_backlight(const struct drm_connector_state *conn_state, u32 level)
 {
-	struct intel_encoder *encoder = connector->encoder;
-	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(conn_state->best_encoder);
 	struct mipi_dsi_device *dsi_device;
 	u8 data = level;
 	enum port port;
@@ -76,14 +75,13 @@ static void dcs_set_backlight(struct intel_connector *connector, u32 level)
 	}
 }
 
-static void dcs_disable_backlight(struct intel_connector *connector)
+static void dcs_disable_backlight(const struct drm_connector_state *conn_state)
 {
-	struct intel_encoder *encoder = connector->encoder;
-	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(conn_state->best_encoder);
 	struct mipi_dsi_device *dsi_device;
 	enum port port;
 
-	dcs_set_backlight(connector, 0);
+	dcs_set_backlight(conn_state, 0);
 
 	for_each_dsi_port(port, intel_dsi->dcs_cabc_ports) {
 		u8 cabc = POWER_SAVE_OFF;
@@ -110,11 +108,11 @@ static void dcs_disable_backlight(struct intel_connector *connector)
 	}
 }
 
-static void dcs_enable_backlight(struct intel_connector *connector)
+static void dcs_enable_backlight(const struct intel_crtc_state *crtc_state,
+				 const struct drm_connector_state *conn_state)
 {
-	struct intel_encoder *encoder = connector->encoder;
-	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
-	struct intel_panel *panel = &connector->panel;
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(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 +140,7 @@ static void dcs_enable_backlight(struct intel_connector *connector)
 				   &cabc, sizeof(cabc));
 	}
 
-	dcs_set_backlight(connector, panel->backlight.level);
+	dcs_set_backlight(conn_state, panel->backlight.level);
 }
 
 static int dcs_setup_backlight(struct intel_connector *connector,
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 8cb573166421..96c2cbd81869 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -561,15 +561,18 @@ static u32 intel_panel_get_backlight(struct intel_connector *connector)
 	return val;
 }
 
-static void lpt_set_backlight(struct intel_connector *connector, u32 level)
+static void lpt_set_backlight(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);
+
 	u32 val = I915_READ(BLC_PWM_PCH_CTL2) & ~BACKLIGHT_DUTY_CYCLE_MASK;
 	I915_WRITE(BLC_PWM_PCH_CTL2, val | level);
 }
 
-static void pch_set_backlight(struct intel_connector *connector, u32 level)
+static void pch_set_backlight(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);
 	u32 tmp;
 
@@ -577,8 +580,9 @@ static void pch_set_backlight(struct intel_connector *connector, u32 level)
 	I915_WRITE(BLC_PWM_CPU_CTL, tmp | level);
 }
 
-static void i9xx_set_backlight(struct intel_connector *connector, u32 level)
+static void i9xx_set_backlight(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);
 	struct intel_panel *panel = &connector->panel;
 	u32 tmp, mask;
@@ -604,50 +608,51 @@ static void i9xx_set_backlight(struct intel_connector *connector, u32 level)
 	I915_WRITE(BLC_PWM_CTL, tmp | level);
 }
 
-static void vlv_set_backlight(struct intel_connector *connector, u32 level)
+static void vlv_set_backlight(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);
-	enum pipe pipe = intel_get_pipe_from_connector(connector);
+	enum pipe pipe = to_intel_crtc(conn_state->crtc)->pipe;
 	u32 tmp;
 
-	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
-		return;
-
 	tmp = I915_READ(VLV_BLC_PWM_CTL(pipe)) & ~BACKLIGHT_DUTY_CYCLE_MASK;
 	I915_WRITE(VLV_BLC_PWM_CTL(pipe), tmp | level);
 }
 
-static void bxt_set_backlight(struct intel_connector *connector, u32 level)
+static void bxt_set_backlight(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);
 	struct intel_panel *panel = &connector->panel;
 
 	I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller), level);
 }
 
-static void pwm_set_backlight(struct intel_connector *connector, u32 level)
+static void pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level)
 {
-	struct intel_panel *panel = &connector->panel;
+	struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel;
 	int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100);
 
 	pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS);
 }
 
 static void
-intel_panel_actually_set_backlight(struct intel_connector *connector, u32 level)
+intel_panel_actually_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;
 
 	DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level);
 
 	level = intel_panel_compute_brightness(connector, level);
-	panel->backlight.set(connector, level);
+	panel->backlight.set(conn_state, level);
 }
 
 /* set backlight brightness to level in range [0..max], scaling wrt hw min */
-static void intel_panel_set_backlight(struct intel_connector *connector,
+static void intel_panel_set_backlight(const struct drm_connector_state *conn_state,
 				      u32 user_level, u32 user_max)
 {
+	struct intel_connector *connector = to_intel_connector(conn_state->connector);
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
 	u32 hw_level;
@@ -663,7 +668,7 @@ static void intel_panel_set_backlight(struct intel_connector *connector,
 	panel->backlight.level = hw_level;
 
 	if (panel->backlight.enabled)
-		intel_panel_actually_set_backlight(connector, hw_level);
+		intel_panel_actually_set_backlight(conn_state, hw_level);
 
 	mutex_unlock(&dev_priv->backlight_lock);
 }
@@ -702,17 +707,18 @@ void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state
 					 panel->backlight.device->props.max_brightness);
 
 	if (panel->backlight.enabled)
-		intel_panel_actually_set_backlight(connector, hw_level);
+		intel_panel_actually_set_backlight(conn_state, hw_level);
 
 	mutex_unlock(&dev_priv->backlight_lock);
 }
 
-static void lpt_disable_backlight(struct intel_connector *connector)
+static void lpt_disable_backlight(const struct drm_connector_state *old_conn_state)
 {
+	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(connector, 0);
+	intel_panel_actually_set_backlight(old_conn_state, 0);
 
 	/*
 	 * Although we don't support or enable CPU PWM with LPT/SPT based
@@ -732,12 +738,13 @@ static void lpt_disable_backlight(struct intel_connector *connector)
 	I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
 }
 
-static void pch_disable_backlight(struct intel_connector *connector)
+static void pch_disable_backlight(const struct drm_connector_state *old_conn_state)
 {
+	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(connector, 0);
+	intel_panel_actually_set_backlight(old_conn_state, 0);
 
 	tmp = I915_READ(BLC_PWM_CPU_CTL2);
 	I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
@@ -746,44 +753,43 @@ static void pch_disable_backlight(struct intel_connector *connector)
 	I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
 }
 
-static void i9xx_disable_backlight(struct intel_connector *connector)
+static void i9xx_disable_backlight(const struct drm_connector_state *old_conn_state)
 {
-	intel_panel_actually_set_backlight(connector, 0);
+	intel_panel_actually_set_backlight(old_conn_state, 0);
 }
 
-static void i965_disable_backlight(struct intel_connector *connector)
+static void i965_disable_backlight(const struct drm_connector_state *old_conn_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+	struct drm_i915_private *dev_priv = to_i915(old_conn_state->connector->dev);
 	u32 tmp;
 
-	intel_panel_actually_set_backlight(connector, 0);
+	intel_panel_actually_set_backlight(old_conn_state, 0);
 
 	tmp = I915_READ(BLC_PWM_CTL2);
 	I915_WRITE(BLC_PWM_CTL2, tmp & ~BLM_PWM_ENABLE);
 }
 
-static void vlv_disable_backlight(struct intel_connector *connector)
+static void vlv_disable_backlight(const struct drm_connector_state *old_conn_state)
 {
+	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 = intel_get_pipe_from_connector(connector);
+	enum pipe pipe = to_intel_crtc(old_conn_state->crtc)->pipe;
 	u32 tmp;
 
-	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
-		return;
-
-	intel_panel_actually_set_backlight(connector, 0);
+	intel_panel_actually_set_backlight(old_conn_state, 0);
 
 	tmp = I915_READ(VLV_BLC_PWM_CTL2(pipe));
 	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), tmp & ~BLM_PWM_ENABLE);
 }
 
-static void bxt_disable_backlight(struct intel_connector *connector)
+static void bxt_disable_backlight(const struct drm_connector_state *old_conn_state)
 {
+	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;
 
-	intel_panel_actually_set_backlight(connector, 0);
+	intel_panel_actually_set_backlight(old_conn_state, 0);
 
 	tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
 	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
@@ -796,21 +802,23 @@ static void bxt_disable_backlight(struct intel_connector *connector)
 	}
 }
 
-static void cnp_disable_backlight(struct intel_connector *connector)
+static void cnp_disable_backlight(const struct drm_connector_state *old_conn_state)
 {
+	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(connector, 0);
+	intel_panel_actually_set_backlight(old_conn_state, 0);
 
 	tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
 	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
 		   tmp & ~BXT_BLC_PWM_ENABLE);
 }
 
-static void pwm_disable_backlight(struct intel_connector *connector)
+static void 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;
 
 	/* Disable the backlight */
@@ -844,13 +852,15 @@ 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.disable(connector);
+	panel->backlight.disable(old_conn_state);
 
 	mutex_unlock(&dev_priv->backlight_lock);
 }
 
-static void lpt_enable_backlight(struct intel_connector *connector)
+static void lpt_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
 	u32 pch_ctl1, pch_ctl2, schicken;
@@ -894,22 +904,18 @@ static void lpt_enable_backlight(struct intel_connector *connector)
 	I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
 
 	/* This won't stick until the above enable. */
-	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
 }
 
-static void pch_enable_backlight(struct intel_connector *connector)
+static void pch_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
-	enum pipe pipe = intel_get_pipe_from_connector(connector);
-	enum transcoder cpu_transcoder;
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	u32 cpu_ctl2, pch_ctl1, pch_ctl2;
 
-	if (!WARN_ON_ONCE(pipe == INVALID_PIPE))
-		cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv, pipe);
-	else
-		cpu_transcoder = TRANSCODER_EDP;
-
 	cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
 	if (cpu_ctl2 & BLM_PWM_ENABLE) {
 		DRM_DEBUG_KMS("cpu backlight already enabled\n");
@@ -933,7 +939,7 @@ static void pch_enable_backlight(struct intel_connector *connector)
 	I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE);
 
 	/* This won't stick until the above enable. */
-	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
 
 	pch_ctl2 = panel->backlight.max << 16;
 	I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
@@ -947,8 +953,10 @@ static void pch_enable_backlight(struct intel_connector *connector)
 	I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
 }
 
-static void i9xx_enable_backlight(struct intel_connector *connector)
+static void i9xx_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
 	u32 ctl, freq;
@@ -973,7 +981,7 @@ static void i9xx_enable_backlight(struct intel_connector *connector)
 	POSTING_READ(BLC_PWM_CTL);
 
 	/* XXX: combine this into above write? */
-	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
 
 	/*
 	 * Needed to enable backlight on some 855gm models. BLC_HIST_CTL is
@@ -984,16 +992,15 @@ static void i9xx_enable_backlight(struct intel_connector *connector)
 		I915_WRITE(BLC_HIST_CTL, BLM_HISTOGRAM_ENABLE);
 }
 
-static void i965_enable_backlight(struct intel_connector *connector)
+static void i965_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
-	enum pipe pipe = intel_get_pipe_from_connector(connector);
+	enum pipe pipe = to_intel_crtc(conn_state->crtc)->pipe;
 	u32 ctl, ctl2, freq;
 
-	if (WARN_ON_ONCE(pipe == INVALID_PIPE))
-		pipe = PIPE_A;
-
 	ctl2 = I915_READ(BLC_PWM_CTL2);
 	if (ctl2 & BLM_PWM_ENABLE) {
 		DRM_DEBUG_KMS("backlight already enabled\n");
@@ -1017,19 +1024,18 @@ static void i965_enable_backlight(struct intel_connector *connector)
 	POSTING_READ(BLC_PWM_CTL2);
 	I915_WRITE(BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE);
 
-	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
 }
 
-static void vlv_enable_backlight(struct intel_connector *connector)
+static void vlv_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
-	enum pipe pipe = intel_get_pipe_from_connector(connector);
+	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
 	u32 ctl, ctl2;
 
-	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
-		return;
-
 	ctl2 = I915_READ(VLV_BLC_PWM_CTL2(pipe));
 	if (ctl2 & BLM_PWM_ENABLE) {
 		DRM_DEBUG_KMS("backlight already enabled\n");
@@ -1041,7 +1047,7 @@ static void vlv_enable_backlight(struct intel_connector *connector)
 	I915_WRITE(VLV_BLC_PWM_CTL(pipe), ctl);
 
 	/* XXX: combine this into above write? */
-	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
 
 	ctl2 = 0;
 	if (panel->backlight.active_low_pwm)
@@ -1051,16 +1057,15 @@ static void vlv_enable_backlight(struct intel_connector *connector)
 	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), ctl2 | BLM_PWM_ENABLE);
 }
 
-static void bxt_enable_backlight(struct intel_connector *connector)
+static void bxt_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
-	enum pipe pipe = intel_get_pipe_from_connector(connector);
+	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
 	u32 pwm_ctl, val;
 
-	if (WARN_ON_ONCE(pipe == INVALID_PIPE))
-		pipe = PIPE_A;
-
 	/* Controller 1 uses the utility pin. */
 	if (panel->backlight.controller == 1) {
 		val = I915_READ(UTIL_PIN_CTL);
@@ -1088,7 +1093,7 @@ static void bxt_enable_backlight(struct intel_connector *connector)
 	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
 			panel->backlight.max);
 
-	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
 
 	pwm_ctl = 0;
 	if (panel->backlight.active_low_pwm)
@@ -1100,8 +1105,10 @@ static void bxt_enable_backlight(struct intel_connector *connector)
 			pwm_ctl | BXT_BLC_PWM_ENABLE);
 }
 
-static void cnp_enable_backlight(struct intel_connector *connector)
+static void cnp_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
 	u32 pwm_ctl;
@@ -1117,7 +1124,7 @@ static void cnp_enable_backlight(struct intel_connector *connector)
 	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
 		   panel->backlight.max);
 
-	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
 
 	pwm_ctl = 0;
 	if (panel->backlight.active_low_pwm)
@@ -1129,12 +1136,14 @@ static void cnp_enable_backlight(struct intel_connector *connector)
 		   pwm_ctl | BXT_BLC_PWM_ENABLE);
 }
 
-static void pwm_enable_backlight(struct intel_connector *connector)
+static void 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;
 
 	pwm_enable(panel->backlight.pwm);
-	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
 }
 
 void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
@@ -1163,7 +1172,7 @@ void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
 						 panel->backlight.device->props.max_brightness);
 	}
 
-	panel->backlight.enable(connector);
+	panel->backlight.enable(crtc_state, conn_state);
 	panel->backlight.enabled = true;
 	if (panel->backlight.device)
 		panel->backlight.device->props.power = FB_BLANK_UNBLANK;
@@ -1181,7 +1190,7 @@ static int intel_backlight_device_update_status(struct backlight_device *bd)
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	DRM_DEBUG_KMS("updating intel_backlight, brightness=%d/%d\n",
 		      bd->props.brightness, bd->props.max_brightness);
-	intel_panel_set_backlight(connector, bd->props.brightness,
+	intel_panel_set_backlight(connector->base.state, bd->props.brightness,
 				  bd->props.max_brightness);
 
 	/*
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Pass atomic state to backlight enable/disable/set callbacks.

[Ville Syrjälä]

On Mon, Jun 12, 2017 at 12:21:15PM +0200, Maarten Lankhorst wrote:
> Pass crtc_state to the enable callback, and connector_state to all callbacks.
> This will eliminate the need to guess for the correct pipe in these
> callbacks.
> 
> The crtc state is required for pch_enable_backlight to obtain the correct
> cpu_transcoder.
> 
> intel_dp_aux_backlight's setup function is called before hw readout, so
> crtc_state and connector_state->best_encoder are NULL in the enable()
> and set() callbacks.

This looks like a bug in the code. Massaging the backlight during setup
doesn't seem like the right thing to do. It should read the current
state instead of modifying it. So this is something that the relevant
people should take a look at. Cc:ing some people...

Anyways, the series looks good to me, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> This fixes the following series of warns from intel_get_pipe_from_connector:
> [  219.968428] ------------[ cut here ]------------
> [  219.968481] WARNING: CPU: 3 PID: 2457 at
> drivers/gpu/drm/i915/intel_display.c:13881
> intel_get_pipe_from_connector+0x62/0x90 [i915]
> [  219.968483]
> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex))
> [  219.968485] Modules linked in: nls_iso8859_1 snd_hda_codec_hdmi
> snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec
> snd_hda_core snd_hwdep snd_pcm intel_rapl x86_pkg_temp_thermal coretemp
> kvm_intel snd_seq_midi snd_seq_midi_event kvm snd_rawmidi irqbypass
> crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc snd_seq
> snd_seq_device serio_raw snd_timer aesni_intel aes_x86_64 crypto_simd
> glue_helper cryptd lpc_ich snd mei_me shpchp soundcore mei rfkill_gpio
> mac_hid intel_pmc_ipc parport_pc ppdev lp parport ip_tables x_tables
> autofs4 hid_generic usbhid igb ahci i915 xhci_pci dca xhci_hcd ptp
> sdhci_pci sdhci libahci pps_core i2c_hid hid video
> [  219.968573] CPU: 3 PID: 2457 Comm: kworker/u8:3 Tainted: G        W
> 4.10.0-tip-201703010159+ #2
> [  219.968575] Hardware name: Intel Corp. Broxton P/NOTEBOOK, BIOS
> APLKRVPA.X64.0144.B10.1606270006 06/27/2016
> [  219.968627] Workqueue: events_unbound intel_atomic_commit_work [i915]
> [  219.968629] Call Trace:
> [  219.968640]  dump_stack+0x63/0x87
> [  219.968646]  __warn+0xd1/0xf0
> [  219.968651]  warn_slowpath_fmt+0x4f/0x60
> [  219.968657]  ? drm_printk+0x97/0xa0
> [  219.968708]  intel_get_pipe_from_connector+0x62/0x90 [i915]
> [  219.968756]  intel_panel_enable_backlight+0x19/0xf0 [i915]
> [  219.968804]  intel_edp_backlight_on.part.22+0x33/0x40 [i915]
> [  219.968852]  intel_edp_backlight_on+0x18/0x20 [i915]
> [  219.968900]  intel_enable_ddi+0x94/0xc0 [i915]
> [  219.968950]  intel_encoders_enable.isra.93+0x77/0x90 [i915]
> [  219.969000]  haswell_crtc_enable+0x310/0x7f0 [i915]
> [  219.969051]  intel_update_crtc+0x58/0x100 [i915]
> [  219.969101]  skl_update_crtcs+0x218/0x240 [i915]
> [  219.969153]  intel_atomic_commit_tail+0x350/0x1000 [i915]
> [  219.969159]  ? vtime_account_idle+0xe/0x50
> [  219.969164]  ? finish_task_switch+0x107/0x250
> [  219.969214]  intel_atomic_commit_work+0x12/0x20 [i915]
> [  219.969219]  process_one_work+0x153/0x3f0
> [  219.969223]  worker_thread+0x12b/0x4b0
> [  219.969227]  kthread+0x101/0x140
> [  219.969230]  ? rescuer_thread+0x340/0x340
> [  219.969233]  ? kthread_park+0x90/0x90
> [  219.969237]  ? do_syscall_64+0x6e/0x180
> [  219.969243]  ret_from_fork+0x2c/0x40
> [  219.969246] ---[ end trace 0a8fa19387b9ad6d ]---
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100022
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_aux_backlight.c  |  25 ++--
>  drivers/gpu/drm/i915/intel_drv.h               |   7 +-
>  drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c |  22 ++--
>  drivers/gpu/drm/i915/intel_panel.c             | 157 +++++++++++++------------
>  4 files changed, 115 insertions(+), 96 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> index a0995c00fc84..6cc62980d0da 100644
> --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -78,8 +78,13 @@ static uint32_t 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(struct intel_connector *connector, u32 level)
> +intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32 level)
>  {
> +	/*
> +	 * conn_state->best_encoder is likely NULL when called from
> +	 * intel_dp_aux_setup_backlight()
> +	 */
> +	struct intel_connector *connector = to_intel_connector(conn_state->connector);
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
>  	uint8_t vals[2] = { 0x0 };
>  
> @@ -97,8 +102,14 @@ intel_dp_aux_set_backlight(struct intel_connector *connector, u32 level)
>  	}
>  }
>  
> -static void intel_dp_aux_enable_backlight(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)
>  {
> +	struct intel_connector *connector = to_intel_connector(conn_state->connector);
> +	/*
> +	 * conn_state->best_encoder (and crtc_state) are NULL when called from
> +	 * intel_dp_aux_setup_backlight()
> +	 */
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
>  	uint8_t dpcd_buf = 0;
>  	uint8_t edp_backlight_mode = 0;
> @@ -131,12 +142,12 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
>  	}
>  
>  	set_aux_backlight_enable(intel_dp, true);
> -	intel_dp_aux_set_backlight(connector, connector->panel.backlight.level);
> +	intel_dp_aux_set_backlight(conn_state, connector->panel.backlight.level);
>  }
>  
> -static void intel_dp_aux_disable_backlight(struct intel_connector *connector)
> +static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old_conn_state)
>  {
> -	set_aux_backlight_enable(enc_to_intel_dp(&connector->encoder->base), false);
> +	set_aux_backlight_enable(enc_to_intel_dp(old_conn_state->best_encoder), false);
>  }
>  
>  static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
> @@ -145,7 +156,7 @@ static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
>  	struct intel_panel *panel = &connector->panel;
>  
> -	intel_dp_aux_enable_backlight(connector);
> +	intel_dp_aux_enable_backlight(NULL, connector->base.state);
>  
>  	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
>  		panel->backlight.max = 0xFFFF;
> @@ -165,7 +176,7 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
>  
> -	/* Check the  eDP Display control capabilities registers to determine if
> +	/* Check the eDP Display control capabilities registers to determine if
>  	 * the panel can support backlight control over the aux channel
>  	 */
>  	if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index db2042a75d42..18528a477cf6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -288,9 +288,10 @@ struct intel_panel {
>  		/* Connector and platform specific backlight functions */
>  		int (*setup)(struct intel_connector *connector, enum pipe pipe);
>  		uint32_t (*get)(struct intel_connector *connector);
> -		void (*set)(struct intel_connector *connector, uint32_t level);
> -		void (*disable)(struct intel_connector *connector);
> -		void (*enable)(struct intel_connector *connector);
> +		void (*set)(const struct drm_connector_state *conn_state, uint32_t level);
> +		void (*disable)(const struct drm_connector_state *conn_state);
> +		void (*enable)(const struct intel_crtc_state *crtc_state,
> +			       const struct drm_connector_state *conn_state);
>  		uint32_t (*hz_to_pwm)(struct intel_connector *connector,
>  				      uint32_t hz);
>  		void (*power)(struct intel_connector *, bool enable);
> diff --git a/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c b/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
> index ac7c6020c443..6e09ceb71500 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
> @@ -60,10 +60,9 @@ static u32 dcs_get_backlight(struct intel_connector *connector)
>  	return data;
>  }
>  
> -static void dcs_set_backlight(struct intel_connector *connector, u32 level)
> +static void dcs_set_backlight(const struct drm_connector_state *conn_state, u32 level)
>  {
> -	struct intel_encoder *encoder = connector->encoder;
> -	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(conn_state->best_encoder);
>  	struct mipi_dsi_device *dsi_device;
>  	u8 data = level;
>  	enum port port;
> @@ -76,14 +75,13 @@ static void dcs_set_backlight(struct intel_connector *connector, u32 level)
>  	}
>  }
>  
> -static void dcs_disable_backlight(struct intel_connector *connector)
> +static void dcs_disable_backlight(const struct drm_connector_state *conn_state)
>  {
> -	struct intel_encoder *encoder = connector->encoder;
> -	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(conn_state->best_encoder);
>  	struct mipi_dsi_device *dsi_device;
>  	enum port port;
>  
> -	dcs_set_backlight(connector, 0);
> +	dcs_set_backlight(conn_state, 0);
>  
>  	for_each_dsi_port(port, intel_dsi->dcs_cabc_ports) {
>  		u8 cabc = POWER_SAVE_OFF;
> @@ -110,11 +108,11 @@ static void dcs_disable_backlight(struct intel_connector *connector)
>  	}
>  }
>  
> -static void dcs_enable_backlight(struct intel_connector *connector)
> +static void dcs_enable_backlight(const struct intel_crtc_state *crtc_state,
> +				 const struct drm_connector_state *conn_state)
>  {
> -	struct intel_encoder *encoder = connector->encoder;
> -	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> -	struct intel_panel *panel = &connector->panel;
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(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 +140,7 @@ static void dcs_enable_backlight(struct intel_connector *connector)
>  				   &cabc, sizeof(cabc));
>  	}
>  
> -	dcs_set_backlight(connector, panel->backlight.level);
> +	dcs_set_backlight(conn_state, panel->backlight.level);
>  }
>  
>  static int dcs_setup_backlight(struct intel_connector *connector,
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 8cb573166421..96c2cbd81869 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -561,15 +561,18 @@ static u32 intel_panel_get_backlight(struct intel_connector *connector)
>  	return val;
>  }
>  
> -static void lpt_set_backlight(struct intel_connector *connector, u32 level)
> +static void lpt_set_backlight(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);
> +
>  	u32 val = I915_READ(BLC_PWM_PCH_CTL2) & ~BACKLIGHT_DUTY_CYCLE_MASK;
>  	I915_WRITE(BLC_PWM_PCH_CTL2, val | level);
>  }
>  
> -static void pch_set_backlight(struct intel_connector *connector, u32 level)
> +static void pch_set_backlight(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);
>  	u32 tmp;
>  
> @@ -577,8 +580,9 @@ static void pch_set_backlight(struct intel_connector *connector, u32 level)
>  	I915_WRITE(BLC_PWM_CPU_CTL, tmp | level);
>  }
>  
> -static void i9xx_set_backlight(struct intel_connector *connector, u32 level)
> +static void i9xx_set_backlight(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);
>  	struct intel_panel *panel = &connector->panel;
>  	u32 tmp, mask;
> @@ -604,50 +608,51 @@ static void i9xx_set_backlight(struct intel_connector *connector, u32 level)
>  	I915_WRITE(BLC_PWM_CTL, tmp | level);
>  }
>  
> -static void vlv_set_backlight(struct intel_connector *connector, u32 level)
> +static void vlv_set_backlight(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);
> -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> +	enum pipe pipe = to_intel_crtc(conn_state->crtc)->pipe;
>  	u32 tmp;
>  
> -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> -		return;
> -
>  	tmp = I915_READ(VLV_BLC_PWM_CTL(pipe)) & ~BACKLIGHT_DUTY_CYCLE_MASK;
>  	I915_WRITE(VLV_BLC_PWM_CTL(pipe), tmp | level);
>  }
>  
> -static void bxt_set_backlight(struct intel_connector *connector, u32 level)
> +static void bxt_set_backlight(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);
>  	struct intel_panel *panel = &connector->panel;
>  
>  	I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller), level);
>  }
>  
> -static void pwm_set_backlight(struct intel_connector *connector, u32 level)
> +static void pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level)
>  {
> -	struct intel_panel *panel = &connector->panel;
> +	struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel;
>  	int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100);
>  
>  	pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS);
>  }
>  
>  static void
> -intel_panel_actually_set_backlight(struct intel_connector *connector, u32 level)
> +intel_panel_actually_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;
>  
>  	DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level);
>  
>  	level = intel_panel_compute_brightness(connector, level);
> -	panel->backlight.set(connector, level);
> +	panel->backlight.set(conn_state, level);
>  }
>  
>  /* set backlight brightness to level in range [0..max], scaling wrt hw min */
> -static void intel_panel_set_backlight(struct intel_connector *connector,
> +static void intel_panel_set_backlight(const struct drm_connector_state *conn_state,
>  				      u32 user_level, u32 user_max)
>  {
> +	struct intel_connector *connector = to_intel_connector(conn_state->connector);
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
>  	u32 hw_level;
> @@ -663,7 +668,7 @@ static void intel_panel_set_backlight(struct intel_connector *connector,
>  	panel->backlight.level = hw_level;
>  
>  	if (panel->backlight.enabled)
> -		intel_panel_actually_set_backlight(connector, hw_level);
> +		intel_panel_actually_set_backlight(conn_state, hw_level);
>  
>  	mutex_unlock(&dev_priv->backlight_lock);
>  }
> @@ -702,17 +707,18 @@ void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state
>  					 panel->backlight.device->props.max_brightness);
>  
>  	if (panel->backlight.enabled)
> -		intel_panel_actually_set_backlight(connector, hw_level);
> +		intel_panel_actually_set_backlight(conn_state, hw_level);
>  
>  	mutex_unlock(&dev_priv->backlight_lock);
>  }
>  
> -static void lpt_disable_backlight(struct intel_connector *connector)
> +static void lpt_disable_backlight(const struct drm_connector_state *old_conn_state)
>  {
> +	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(connector, 0);
> +	intel_panel_actually_set_backlight(old_conn_state, 0);
>  
>  	/*
>  	 * Although we don't support or enable CPU PWM with LPT/SPT based
> @@ -732,12 +738,13 @@ static void lpt_disable_backlight(struct intel_connector *connector)
>  	I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
>  }
>  
> -static void pch_disable_backlight(struct intel_connector *connector)
> +static void pch_disable_backlight(const struct drm_connector_state *old_conn_state)
>  {
> +	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(connector, 0);
> +	intel_panel_actually_set_backlight(old_conn_state, 0);
>  
>  	tmp = I915_READ(BLC_PWM_CPU_CTL2);
>  	I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
> @@ -746,44 +753,43 @@ static void pch_disable_backlight(struct intel_connector *connector)
>  	I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
>  }
>  
> -static void i9xx_disable_backlight(struct intel_connector *connector)
> +static void i9xx_disable_backlight(const struct drm_connector_state *old_conn_state)
>  {
> -	intel_panel_actually_set_backlight(connector, 0);
> +	intel_panel_actually_set_backlight(old_conn_state, 0);
>  }
>  
> -static void i965_disable_backlight(struct intel_connector *connector)
> +static void i965_disable_backlight(const struct drm_connector_state *old_conn_state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +	struct drm_i915_private *dev_priv = to_i915(old_conn_state->connector->dev);
>  	u32 tmp;
>  
> -	intel_panel_actually_set_backlight(connector, 0);
> +	intel_panel_actually_set_backlight(old_conn_state, 0);
>  
>  	tmp = I915_READ(BLC_PWM_CTL2);
>  	I915_WRITE(BLC_PWM_CTL2, tmp & ~BLM_PWM_ENABLE);
>  }
>  
> -static void vlv_disable_backlight(struct intel_connector *connector)
> +static void vlv_disable_backlight(const struct drm_connector_state *old_conn_state)
>  {
> +	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 = intel_get_pipe_from_connector(connector);
> +	enum pipe pipe = to_intel_crtc(old_conn_state->crtc)->pipe;
>  	u32 tmp;
>  
> -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> -		return;
> -
> -	intel_panel_actually_set_backlight(connector, 0);
> +	intel_panel_actually_set_backlight(old_conn_state, 0);
>  
>  	tmp = I915_READ(VLV_BLC_PWM_CTL2(pipe));
>  	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), tmp & ~BLM_PWM_ENABLE);
>  }
>  
> -static void bxt_disable_backlight(struct intel_connector *connector)
> +static void bxt_disable_backlight(const struct drm_connector_state *old_conn_state)
>  {
> +	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;
>  
> -	intel_panel_actually_set_backlight(connector, 0);
> +	intel_panel_actually_set_backlight(old_conn_state, 0);
>  
>  	tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>  	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> @@ -796,21 +802,23 @@ static void bxt_disable_backlight(struct intel_connector *connector)
>  	}
>  }
>  
> -static void cnp_disable_backlight(struct intel_connector *connector)
> +static void cnp_disable_backlight(const struct drm_connector_state *old_conn_state)
>  {
> +	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(connector, 0);
> +	intel_panel_actually_set_backlight(old_conn_state, 0);
>  
>  	tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>  	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
>  		   tmp & ~BXT_BLC_PWM_ENABLE);
>  }
>  
> -static void pwm_disable_backlight(struct intel_connector *connector)
> +static void 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;
>  
>  	/* Disable the backlight */
> @@ -844,13 +852,15 @@ 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.disable(connector);
> +	panel->backlight.disable(old_conn_state);
>  
>  	mutex_unlock(&dev_priv->backlight_lock);
>  }
>  
> -static void lpt_enable_backlight(struct intel_connector *connector)
> +static void lpt_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
>  	u32 pch_ctl1, pch_ctl2, schicken;
> @@ -894,22 +904,18 @@ static void lpt_enable_backlight(struct intel_connector *connector)
>  	I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
>  
>  	/* This won't stick until the above enable. */
> -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
>  }
>  
> -static void pch_enable_backlight(struct intel_connector *connector)
> +static void pch_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
> -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> -	enum transcoder cpu_transcoder;
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	u32 cpu_ctl2, pch_ctl1, pch_ctl2;
>  
> -	if (!WARN_ON_ONCE(pipe == INVALID_PIPE))
> -		cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv, pipe);
> -	else
> -		cpu_transcoder = TRANSCODER_EDP;
> -
>  	cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
>  	if (cpu_ctl2 & BLM_PWM_ENABLE) {
>  		DRM_DEBUG_KMS("cpu backlight already enabled\n");
> @@ -933,7 +939,7 @@ static void pch_enable_backlight(struct intel_connector *connector)
>  	I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE);
>  
>  	/* This won't stick until the above enable. */
> -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
>  
>  	pch_ctl2 = panel->backlight.max << 16;
>  	I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
> @@ -947,8 +953,10 @@ static void pch_enable_backlight(struct intel_connector *connector)
>  	I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
>  }
>  
> -static void i9xx_enable_backlight(struct intel_connector *connector)
> +static void i9xx_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
>  	u32 ctl, freq;
> @@ -973,7 +981,7 @@ static void i9xx_enable_backlight(struct intel_connector *connector)
>  	POSTING_READ(BLC_PWM_CTL);
>  
>  	/* XXX: combine this into above write? */
> -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
>  
>  	/*
>  	 * Needed to enable backlight on some 855gm models. BLC_HIST_CTL is
> @@ -984,16 +992,15 @@ static void i9xx_enable_backlight(struct intel_connector *connector)
>  		I915_WRITE(BLC_HIST_CTL, BLM_HISTOGRAM_ENABLE);
>  }
>  
> -static void i965_enable_backlight(struct intel_connector *connector)
> +static void i965_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
> -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> +	enum pipe pipe = to_intel_crtc(conn_state->crtc)->pipe;
>  	u32 ctl, ctl2, freq;
>  
> -	if (WARN_ON_ONCE(pipe == INVALID_PIPE))
> -		pipe = PIPE_A;
> -
>  	ctl2 = I915_READ(BLC_PWM_CTL2);
>  	if (ctl2 & BLM_PWM_ENABLE) {
>  		DRM_DEBUG_KMS("backlight already enabled\n");
> @@ -1017,19 +1024,18 @@ static void i965_enable_backlight(struct intel_connector *connector)
>  	POSTING_READ(BLC_PWM_CTL2);
>  	I915_WRITE(BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE);
>  
> -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
>  }
>  
> -static void vlv_enable_backlight(struct intel_connector *connector)
> +static void vlv_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
> -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> +	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
>  	u32 ctl, ctl2;
>  
> -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> -		return;
> -
>  	ctl2 = I915_READ(VLV_BLC_PWM_CTL2(pipe));
>  	if (ctl2 & BLM_PWM_ENABLE) {
>  		DRM_DEBUG_KMS("backlight already enabled\n");
> @@ -1041,7 +1047,7 @@ static void vlv_enable_backlight(struct intel_connector *connector)
>  	I915_WRITE(VLV_BLC_PWM_CTL(pipe), ctl);
>  
>  	/* XXX: combine this into above write? */
> -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
>  
>  	ctl2 = 0;
>  	if (panel->backlight.active_low_pwm)
> @@ -1051,16 +1057,15 @@ static void vlv_enable_backlight(struct intel_connector *connector)
>  	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), ctl2 | BLM_PWM_ENABLE);
>  }
>  
> -static void bxt_enable_backlight(struct intel_connector *connector)
> +static void bxt_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
> -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> +	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
>  	u32 pwm_ctl, val;
>  
> -	if (WARN_ON_ONCE(pipe == INVALID_PIPE))
> -		pipe = PIPE_A;
> -
>  	/* Controller 1 uses the utility pin. */
>  	if (panel->backlight.controller == 1) {
>  		val = I915_READ(UTIL_PIN_CTL);
> @@ -1088,7 +1093,7 @@ static void bxt_enable_backlight(struct intel_connector *connector)
>  	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
>  			panel->backlight.max);
>  
> -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
>  
>  	pwm_ctl = 0;
>  	if (panel->backlight.active_low_pwm)
> @@ -1100,8 +1105,10 @@ static void bxt_enable_backlight(struct intel_connector *connector)
>  			pwm_ctl | BXT_BLC_PWM_ENABLE);
>  }
>  
> -static void cnp_enable_backlight(struct intel_connector *connector)
> +static void cnp_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
>  	u32 pwm_ctl;
> @@ -1117,7 +1124,7 @@ static void cnp_enable_backlight(struct intel_connector *connector)
>  	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
>  		   panel->backlight.max);
>  
> -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
>  
>  	pwm_ctl = 0;
>  	if (panel->backlight.active_low_pwm)
> @@ -1129,12 +1136,14 @@ static void cnp_enable_backlight(struct intel_connector *connector)
>  		   pwm_ctl | BXT_BLC_PWM_ENABLE);
>  }
>  
> -static void pwm_enable_backlight(struct intel_connector *connector)
> +static void 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;
>  
>  	pwm_enable(panel->backlight.pwm);
> -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
>  }
>  
>  void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
> @@ -1163,7 +1172,7 @@ void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
>  						 panel->backlight.device->props.max_brightness);
>  	}
>  
> -	panel->backlight.enable(connector);
> +	panel->backlight.enable(crtc_state, conn_state);
>  	panel->backlight.enabled = true;
>  	if (panel->backlight.device)
>  		panel->backlight.device->props.power = FB_BLANK_UNBLANK;
> @@ -1181,7 +1190,7 @@ static int intel_backlight_device_update_status(struct backlight_device *bd)
>  	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>  	DRM_DEBUG_KMS("updating intel_backlight, brightness=%d/%d\n",
>  		      bd->props.brightness, bd->props.max_brightness);
> -	intel_panel_set_backlight(connector, bd->props.brightness,
> +	intel_panel_set_backlight(connector->base.state, bd->props.brightness,
>  				  bd->props.max_brightness);
>  
>  	/*
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Pass atomic state to backlight enable/disable/set callbacks.

[Maarten Lankhorst]

Op 12-06-17 om 14:48 schreef Ville Syrjälä:
> On Mon, Jun 12, 2017 at 12:21:15PM +0200, Maarten Lankhorst wrote:
>> Pass crtc_state to the enable callback, and connector_state to all callbacks.
>> This will eliminate the need to guess for the correct pipe in these
>> callbacks.
>>
>> The crtc state is required for pch_enable_backlight to obtain the correct
>> cpu_transcoder.
>>
>> intel_dp_aux_backlight's setup function is called before hw readout, so
>> crtc_state and connector_state->best_encoder are NULL in the enable()
>> and set() callbacks.
> This looks like a bug in the code. Massaging the backlight during setup
> doesn't seem like the right thing to do. It should read the current
> state instead of modifying it. So this is something that the relevant
> people should take a look at. Cc:ing some people...
>
> Anyways, the series looks good to me, so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks, trybot seems to test backlight too (pm_backlight). No errors caught there so pushed. :)

I think you've forgot to attach some some people to cc, I only see the ml.

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Pass atomic state to backlight enable/disable/set callbacks.

[Ville Syrjälä]

On Mon, Jun 12, 2017 at 03:48:04PM +0300, Ville Syrjälä wrote:
> On Mon, Jun 12, 2017 at 12:21:15PM +0200, Maarten Lankhorst wrote:
> > Pass crtc_state to the enable callback, and connector_state to all callbacks.
> > This will eliminate the need to guess for the correct pipe in these
> > callbacks.
> > 
> > The crtc state is required for pch_enable_backlight to obtain the correct
> > cpu_transcoder.
> > 
> > intel_dp_aux_backlight's setup function is called before hw readout, so
> > crtc_state and connector_state->best_encoder are NULL in the enable()
> > and set() callbacks.
> 
> This looks like a bug in the code. Massaging the backlight during setup
> doesn't seem like the right thing to do. It should read the current
> state instead of modifying it. So this is something that the relevant
> people should take a look at. Cc:ing some people...

Now really cc:ing!

> 
> Anyways, the series looks good to me, so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > 
> > This fixes the following series of warns from intel_get_pipe_from_connector:
> > [  219.968428] ------------[ cut here ]------------
> > [  219.968481] WARNING: CPU: 3 PID: 2457 at
> > drivers/gpu/drm/i915/intel_display.c:13881
> > intel_get_pipe_from_connector+0x62/0x90 [i915]
> > [  219.968483]
> > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex))
> > [  219.968485] Modules linked in: nls_iso8859_1 snd_hda_codec_hdmi
> > snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec
> > snd_hda_core snd_hwdep snd_pcm intel_rapl x86_pkg_temp_thermal coretemp
> > kvm_intel snd_seq_midi snd_seq_midi_event kvm snd_rawmidi irqbypass
> > crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc snd_seq
> > snd_seq_device serio_raw snd_timer aesni_intel aes_x86_64 crypto_simd
> > glue_helper cryptd lpc_ich snd mei_me shpchp soundcore mei rfkill_gpio
> > mac_hid intel_pmc_ipc parport_pc ppdev lp parport ip_tables x_tables
> > autofs4 hid_generic usbhid igb ahci i915 xhci_pci dca xhci_hcd ptp
> > sdhci_pci sdhci libahci pps_core i2c_hid hid video
> > [  219.968573] CPU: 3 PID: 2457 Comm: kworker/u8:3 Tainted: G        W
> > 4.10.0-tip-201703010159+ #2
> > [  219.968575] Hardware name: Intel Corp. Broxton P/NOTEBOOK, BIOS
> > APLKRVPA.X64.0144.B10.1606270006 06/27/2016
> > [  219.968627] Workqueue: events_unbound intel_atomic_commit_work [i915]
> > [  219.968629] Call Trace:
> > [  219.968640]  dump_stack+0x63/0x87
> > [  219.968646]  __warn+0xd1/0xf0
> > [  219.968651]  warn_slowpath_fmt+0x4f/0x60
> > [  219.968657]  ? drm_printk+0x97/0xa0
> > [  219.968708]  intel_get_pipe_from_connector+0x62/0x90 [i915]
> > [  219.968756]  intel_panel_enable_backlight+0x19/0xf0 [i915]
> > [  219.968804]  intel_edp_backlight_on.part.22+0x33/0x40 [i915]
> > [  219.968852]  intel_edp_backlight_on+0x18/0x20 [i915]
> > [  219.968900]  intel_enable_ddi+0x94/0xc0 [i915]
> > [  219.968950]  intel_encoders_enable.isra.93+0x77/0x90 [i915]
> > [  219.969000]  haswell_crtc_enable+0x310/0x7f0 [i915]
> > [  219.969051]  intel_update_crtc+0x58/0x100 [i915]
> > [  219.969101]  skl_update_crtcs+0x218/0x240 [i915]
> > [  219.969153]  intel_atomic_commit_tail+0x350/0x1000 [i915]
> > [  219.969159]  ? vtime_account_idle+0xe/0x50
> > [  219.969164]  ? finish_task_switch+0x107/0x250
> > [  219.969214]  intel_atomic_commit_work+0x12/0x20 [i915]
> > [  219.969219]  process_one_work+0x153/0x3f0
> > [  219.969223]  worker_thread+0x12b/0x4b0
> > [  219.969227]  kthread+0x101/0x140
> > [  219.969230]  ? rescuer_thread+0x340/0x340
> > [  219.969233]  ? kthread_park+0x90/0x90
> > [  219.969237]  ? do_syscall_64+0x6e/0x180
> > [  219.969243]  ret_from_fork+0x2c/0x40
> > [  219.969246] ---[ end trace 0a8fa19387b9ad6d ]---
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100022
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp_aux_backlight.c  |  25 ++--
> >  drivers/gpu/drm/i915/intel_drv.h               |   7 +-
> >  drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c |  22 ++--
> >  drivers/gpu/drm/i915/intel_panel.c             | 157 +++++++++++++------------
> >  4 files changed, 115 insertions(+), 96 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > index a0995c00fc84..6cc62980d0da 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> > @@ -78,8 +78,13 @@ static uint32_t 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(struct intel_connector *connector, u32 level)
> > +intel_dp_aux_set_backlight(const struct drm_connector_state *conn_state, u32 level)
> >  {
> > +	/*
> > +	 * conn_state->best_encoder is likely NULL when called from
> > +	 * intel_dp_aux_setup_backlight()
> > +	 */
> > +	struct intel_connector *connector = to_intel_connector(conn_state->connector);
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
> >  	uint8_t vals[2] = { 0x0 };
> >  
> > @@ -97,8 +102,14 @@ intel_dp_aux_set_backlight(struct intel_connector *connector, u32 level)
> >  	}
> >  }
> >  
> > -static void intel_dp_aux_enable_backlight(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)
> >  {
> > +	struct intel_connector *connector = to_intel_connector(conn_state->connector);
> > +	/*
> > +	 * conn_state->best_encoder (and crtc_state) are NULL when called from
> > +	 * intel_dp_aux_setup_backlight()
> > +	 */
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
> >  	uint8_t dpcd_buf = 0;
> >  	uint8_t edp_backlight_mode = 0;
> > @@ -131,12 +142,12 @@ static void intel_dp_aux_enable_backlight(struct intel_connector *connector)
> >  	}
> >  
> >  	set_aux_backlight_enable(intel_dp, true);
> > -	intel_dp_aux_set_backlight(connector, connector->panel.backlight.level);
> > +	intel_dp_aux_set_backlight(conn_state, connector->panel.backlight.level);
> >  }
> >  
> > -static void intel_dp_aux_disable_backlight(struct intel_connector *connector)
> > +static void intel_dp_aux_disable_backlight(const struct drm_connector_state *old_conn_state)
> >  {
> > -	set_aux_backlight_enable(enc_to_intel_dp(&connector->encoder->base), false);
> > +	set_aux_backlight_enable(enc_to_intel_dp(old_conn_state->best_encoder), false);
> >  }
> >  
> >  static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
> > @@ -145,7 +156,7 @@ static int intel_dp_aux_setup_backlight(struct intel_connector *connector,
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
> >  	struct intel_panel *panel = &connector->panel;
> >  
> > -	intel_dp_aux_enable_backlight(connector);
> > +	intel_dp_aux_enable_backlight(NULL, connector->base.state);
> >  
> >  	if (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT)
> >  		panel->backlight.max = 0xFFFF;
> > @@ -165,7 +176,7 @@ intel_dp_aux_display_control_capable(struct intel_connector *connector)
> >  {
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
> >  
> > -	/* Check the  eDP Display control capabilities registers to determine if
> > +	/* Check the eDP Display control capabilities registers to determine if
> >  	 * the panel can support backlight control over the aux channel
> >  	 */
> >  	if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP &&
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index db2042a75d42..18528a477cf6 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -288,9 +288,10 @@ struct intel_panel {
> >  		/* Connector and platform specific backlight functions */
> >  		int (*setup)(struct intel_connector *connector, enum pipe pipe);
> >  		uint32_t (*get)(struct intel_connector *connector);
> > -		void (*set)(struct intel_connector *connector, uint32_t level);
> > -		void (*disable)(struct intel_connector *connector);
> > -		void (*enable)(struct intel_connector *connector);
> > +		void (*set)(const struct drm_connector_state *conn_state, uint32_t level);
> > +		void (*disable)(const struct drm_connector_state *conn_state);
> > +		void (*enable)(const struct intel_crtc_state *crtc_state,
> > +			       const struct drm_connector_state *conn_state);
> >  		uint32_t (*hz_to_pwm)(struct intel_connector *connector,
> >  				      uint32_t hz);
> >  		void (*power)(struct intel_connector *, bool enable);
> > diff --git a/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c b/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
> > index ac7c6020c443..6e09ceb71500 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c
> > @@ -60,10 +60,9 @@ static u32 dcs_get_backlight(struct intel_connector *connector)
> >  	return data;
> >  }
> >  
> > -static void dcs_set_backlight(struct intel_connector *connector, u32 level)
> > +static void dcs_set_backlight(const struct drm_connector_state *conn_state, u32 level)
> >  {
> > -	struct intel_encoder *encoder = connector->encoder;
> > -	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(conn_state->best_encoder);
> >  	struct mipi_dsi_device *dsi_device;
> >  	u8 data = level;
> >  	enum port port;
> > @@ -76,14 +75,13 @@ static void dcs_set_backlight(struct intel_connector *connector, u32 level)
> >  	}
> >  }
> >  
> > -static void dcs_disable_backlight(struct intel_connector *connector)
> > +static void dcs_disable_backlight(const struct drm_connector_state *conn_state)
> >  {
> > -	struct intel_encoder *encoder = connector->encoder;
> > -	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(conn_state->best_encoder);
> >  	struct mipi_dsi_device *dsi_device;
> >  	enum port port;
> >  
> > -	dcs_set_backlight(connector, 0);
> > +	dcs_set_backlight(conn_state, 0);
> >  
> >  	for_each_dsi_port(port, intel_dsi->dcs_cabc_ports) {
> >  		u8 cabc = POWER_SAVE_OFF;
> > @@ -110,11 +108,11 @@ static void dcs_disable_backlight(struct intel_connector *connector)
> >  	}
> >  }
> >  
> > -static void dcs_enable_backlight(struct intel_connector *connector)
> > +static void dcs_enable_backlight(const struct intel_crtc_state *crtc_state,
> > +				 const struct drm_connector_state *conn_state)
> >  {
> > -	struct intel_encoder *encoder = connector->encoder;
> > -	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > -	struct intel_panel *panel = &connector->panel;
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(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 +140,7 @@ static void dcs_enable_backlight(struct intel_connector *connector)
> >  				   &cabc, sizeof(cabc));
> >  	}
> >  
> > -	dcs_set_backlight(connector, panel->backlight.level);
> > +	dcs_set_backlight(conn_state, panel->backlight.level);
> >  }
> >  
> >  static int dcs_setup_backlight(struct intel_connector *connector,
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 8cb573166421..96c2cbd81869 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -561,15 +561,18 @@ static u32 intel_panel_get_backlight(struct intel_connector *connector)
> >  	return val;
> >  }
> >  
> > -static void lpt_set_backlight(struct intel_connector *connector, u32 level)
> > +static void lpt_set_backlight(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);
> > +
> >  	u32 val = I915_READ(BLC_PWM_PCH_CTL2) & ~BACKLIGHT_DUTY_CYCLE_MASK;
> >  	I915_WRITE(BLC_PWM_PCH_CTL2, val | level);
> >  }
> >  
> > -static void pch_set_backlight(struct intel_connector *connector, u32 level)
> > +static void pch_set_backlight(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);
> >  	u32 tmp;
> >  
> > @@ -577,8 +580,9 @@ static void pch_set_backlight(struct intel_connector *connector, u32 level)
> >  	I915_WRITE(BLC_PWM_CPU_CTL, tmp | level);
> >  }
> >  
> > -static void i9xx_set_backlight(struct intel_connector *connector, u32 level)
> > +static void i9xx_set_backlight(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);
> >  	struct intel_panel *panel = &connector->panel;
> >  	u32 tmp, mask;
> > @@ -604,50 +608,51 @@ static void i9xx_set_backlight(struct intel_connector *connector, u32 level)
> >  	I915_WRITE(BLC_PWM_CTL, tmp | level);
> >  }
> >  
> > -static void vlv_set_backlight(struct intel_connector *connector, u32 level)
> > +static void vlv_set_backlight(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);
> > -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> > +	enum pipe pipe = to_intel_crtc(conn_state->crtc)->pipe;
> >  	u32 tmp;
> >  
> > -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> > -		return;
> > -
> >  	tmp = I915_READ(VLV_BLC_PWM_CTL(pipe)) & ~BACKLIGHT_DUTY_CYCLE_MASK;
> >  	I915_WRITE(VLV_BLC_PWM_CTL(pipe), tmp | level);
> >  }
> >  
> > -static void bxt_set_backlight(struct intel_connector *connector, u32 level)
> > +static void bxt_set_backlight(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);
> >  	struct intel_panel *panel = &connector->panel;
> >  
> >  	I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller), level);
> >  }
> >  
> > -static void pwm_set_backlight(struct intel_connector *connector, u32 level)
> > +static void pwm_set_backlight(const struct drm_connector_state *conn_state, u32 level)
> >  {
> > -	struct intel_panel *panel = &connector->panel;
> > +	struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel;
> >  	int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100);
> >  
> >  	pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS);
> >  }
> >  
> >  static void
> > -intel_panel_actually_set_backlight(struct intel_connector *connector, u32 level)
> > +intel_panel_actually_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;
> >  
> >  	DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level);
> >  
> >  	level = intel_panel_compute_brightness(connector, level);
> > -	panel->backlight.set(connector, level);
> > +	panel->backlight.set(conn_state, level);
> >  }
> >  
> >  /* set backlight brightness to level in range [0..max], scaling wrt hw min */
> > -static void intel_panel_set_backlight(struct intel_connector *connector,
> > +static void intel_panel_set_backlight(const struct drm_connector_state *conn_state,
> >  				      u32 user_level, u32 user_max)
> >  {
> > +	struct intel_connector *connector = to_intel_connector(conn_state->connector);
> >  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	struct intel_panel *panel = &connector->panel;
> >  	u32 hw_level;
> > @@ -663,7 +668,7 @@ static void intel_panel_set_backlight(struct intel_connector *connector,
> >  	panel->backlight.level = hw_level;
> >  
> >  	if (panel->backlight.enabled)
> > -		intel_panel_actually_set_backlight(connector, hw_level);
> > +		intel_panel_actually_set_backlight(conn_state, hw_level);
> >  
> >  	mutex_unlock(&dev_priv->backlight_lock);
> >  }
> > @@ -702,17 +707,18 @@ void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state
> >  					 panel->backlight.device->props.max_brightness);
> >  
> >  	if (panel->backlight.enabled)
> > -		intel_panel_actually_set_backlight(connector, hw_level);
> > +		intel_panel_actually_set_backlight(conn_state, hw_level);
> >  
> >  	mutex_unlock(&dev_priv->backlight_lock);
> >  }
> >  
> > -static void lpt_disable_backlight(struct intel_connector *connector)
> > +static void lpt_disable_backlight(const struct drm_connector_state *old_conn_state)
> >  {
> > +	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(connector, 0);
> > +	intel_panel_actually_set_backlight(old_conn_state, 0);
> >  
> >  	/*
> >  	 * Although we don't support or enable CPU PWM with LPT/SPT based
> > @@ -732,12 +738,13 @@ static void lpt_disable_backlight(struct intel_connector *connector)
> >  	I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
> >  }
> >  
> > -static void pch_disable_backlight(struct intel_connector *connector)
> > +static void pch_disable_backlight(const struct drm_connector_state *old_conn_state)
> >  {
> > +	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(connector, 0);
> > +	intel_panel_actually_set_backlight(old_conn_state, 0);
> >  
> >  	tmp = I915_READ(BLC_PWM_CPU_CTL2);
> >  	I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
> > @@ -746,44 +753,43 @@ static void pch_disable_backlight(struct intel_connector *connector)
> >  	I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
> >  }
> >  
> > -static void i9xx_disable_backlight(struct intel_connector *connector)
> > +static void i9xx_disable_backlight(const struct drm_connector_state *old_conn_state)
> >  {
> > -	intel_panel_actually_set_backlight(connector, 0);
> > +	intel_panel_actually_set_backlight(old_conn_state, 0);
> >  }
> >  
> > -static void i965_disable_backlight(struct intel_connector *connector)
> > +static void i965_disable_backlight(const struct drm_connector_state *old_conn_state)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > +	struct drm_i915_private *dev_priv = to_i915(old_conn_state->connector->dev);
> >  	u32 tmp;
> >  
> > -	intel_panel_actually_set_backlight(connector, 0);
> > +	intel_panel_actually_set_backlight(old_conn_state, 0);
> >  
> >  	tmp = I915_READ(BLC_PWM_CTL2);
> >  	I915_WRITE(BLC_PWM_CTL2, tmp & ~BLM_PWM_ENABLE);
> >  }
> >  
> > -static void vlv_disable_backlight(struct intel_connector *connector)
> > +static void vlv_disable_backlight(const struct drm_connector_state *old_conn_state)
> >  {
> > +	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 = intel_get_pipe_from_connector(connector);
> > +	enum pipe pipe = to_intel_crtc(old_conn_state->crtc)->pipe;
> >  	u32 tmp;
> >  
> > -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> > -		return;
> > -
> > -	intel_panel_actually_set_backlight(connector, 0);
> > +	intel_panel_actually_set_backlight(old_conn_state, 0);
> >  
> >  	tmp = I915_READ(VLV_BLC_PWM_CTL2(pipe));
> >  	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), tmp & ~BLM_PWM_ENABLE);
> >  }
> >  
> > -static void bxt_disable_backlight(struct intel_connector *connector)
> > +static void bxt_disable_backlight(const struct drm_connector_state *old_conn_state)
> >  {
> > +	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;
> >  
> > -	intel_panel_actually_set_backlight(connector, 0);
> > +	intel_panel_actually_set_backlight(old_conn_state, 0);
> >  
> >  	tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> >  	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> > @@ -796,21 +802,23 @@ static void bxt_disable_backlight(struct intel_connector *connector)
> >  	}
> >  }
> >  
> > -static void cnp_disable_backlight(struct intel_connector *connector)
> > +static void cnp_disable_backlight(const struct drm_connector_state *old_conn_state)
> >  {
> > +	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(connector, 0);
> > +	intel_panel_actually_set_backlight(old_conn_state, 0);
> >  
> >  	tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> >  	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> >  		   tmp & ~BXT_BLC_PWM_ENABLE);
> >  }
> >  
> > -static void pwm_disable_backlight(struct intel_connector *connector)
> > +static void 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;
> >  
> >  	/* Disable the backlight */
> > @@ -844,13 +852,15 @@ 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.disable(connector);
> > +	panel->backlight.disable(old_conn_state);
> >  
> >  	mutex_unlock(&dev_priv->backlight_lock);
> >  }
> >  
> > -static void lpt_enable_backlight(struct intel_connector *connector)
> > +static void lpt_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	struct intel_panel *panel = &connector->panel;
> >  	u32 pch_ctl1, pch_ctl2, schicken;
> > @@ -894,22 +904,18 @@ static void lpt_enable_backlight(struct intel_connector *connector)
> >  	I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
> >  
> >  	/* This won't stick until the above enable. */
> > -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> > +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
> >  }
> >  
> > -static void pch_enable_backlight(struct intel_connector *connector)
> > +static void pch_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	struct intel_panel *panel = &connector->panel;
> > -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> > -	enum transcoder cpu_transcoder;
> > +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  	u32 cpu_ctl2, pch_ctl1, pch_ctl2;
> >  
> > -	if (!WARN_ON_ONCE(pipe == INVALID_PIPE))
> > -		cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv, pipe);
> > -	else
> > -		cpu_transcoder = TRANSCODER_EDP;
> > -
> >  	cpu_ctl2 = I915_READ(BLC_PWM_CPU_CTL2);
> >  	if (cpu_ctl2 & BLM_PWM_ENABLE) {
> >  		DRM_DEBUG_KMS("cpu backlight already enabled\n");
> > @@ -933,7 +939,7 @@ static void pch_enable_backlight(struct intel_connector *connector)
> >  	I915_WRITE(BLC_PWM_CPU_CTL2, cpu_ctl2 | BLM_PWM_ENABLE);
> >  
> >  	/* This won't stick until the above enable. */
> > -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> > +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
> >  
> >  	pch_ctl2 = panel->backlight.max << 16;
> >  	I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
> > @@ -947,8 +953,10 @@ static void pch_enable_backlight(struct intel_connector *connector)
> >  	I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_PWM_ENABLE);
> >  }
> >  
> > -static void i9xx_enable_backlight(struct intel_connector *connector)
> > +static void i9xx_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	struct intel_panel *panel = &connector->panel;
> >  	u32 ctl, freq;
> > @@ -973,7 +981,7 @@ static void i9xx_enable_backlight(struct intel_connector *connector)
> >  	POSTING_READ(BLC_PWM_CTL);
> >  
> >  	/* XXX: combine this into above write? */
> > -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> > +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
> >  
> >  	/*
> >  	 * Needed to enable backlight on some 855gm models. BLC_HIST_CTL is
> > @@ -984,16 +992,15 @@ static void i9xx_enable_backlight(struct intel_connector *connector)
> >  		I915_WRITE(BLC_HIST_CTL, BLM_HISTOGRAM_ENABLE);
> >  }
> >  
> > -static void i965_enable_backlight(struct intel_connector *connector)
> > +static void i965_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	struct intel_panel *panel = &connector->panel;
> > -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> > +	enum pipe pipe = to_intel_crtc(conn_state->crtc)->pipe;
> >  	u32 ctl, ctl2, freq;
> >  
> > -	if (WARN_ON_ONCE(pipe == INVALID_PIPE))
> > -		pipe = PIPE_A;
> > -
> >  	ctl2 = I915_READ(BLC_PWM_CTL2);
> >  	if (ctl2 & BLM_PWM_ENABLE) {
> >  		DRM_DEBUG_KMS("backlight already enabled\n");
> > @@ -1017,19 +1024,18 @@ static void i965_enable_backlight(struct intel_connector *connector)
> >  	POSTING_READ(BLC_PWM_CTL2);
> >  	I915_WRITE(BLC_PWM_CTL2, ctl2 | BLM_PWM_ENABLE);
> >  
> > -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> > +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
> >  }
> >  
> > -static void vlv_enable_backlight(struct intel_connector *connector)
> > +static void vlv_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	struct intel_panel *panel = &connector->panel;
> > -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> > +	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
> >  	u32 ctl, ctl2;
> >  
> > -	if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> > -		return;
> > -
> >  	ctl2 = I915_READ(VLV_BLC_PWM_CTL2(pipe));
> >  	if (ctl2 & BLM_PWM_ENABLE) {
> >  		DRM_DEBUG_KMS("backlight already enabled\n");
> > @@ -1041,7 +1047,7 @@ static void vlv_enable_backlight(struct intel_connector *connector)
> >  	I915_WRITE(VLV_BLC_PWM_CTL(pipe), ctl);
> >  
> >  	/* XXX: combine this into above write? */
> > -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> > +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
> >  
> >  	ctl2 = 0;
> >  	if (panel->backlight.active_low_pwm)
> > @@ -1051,16 +1057,15 @@ static void vlv_enable_backlight(struct intel_connector *connector)
> >  	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), ctl2 | BLM_PWM_ENABLE);
> >  }
> >  
> > -static void bxt_enable_backlight(struct intel_connector *connector)
> > +static void bxt_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	struct intel_panel *panel = &connector->panel;
> > -	enum pipe pipe = intel_get_pipe_from_connector(connector);
> > +	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
> >  	u32 pwm_ctl, val;
> >  
> > -	if (WARN_ON_ONCE(pipe == INVALID_PIPE))
> > -		pipe = PIPE_A;
> > -
> >  	/* Controller 1 uses the utility pin. */
> >  	if (panel->backlight.controller == 1) {
> >  		val = I915_READ(UTIL_PIN_CTL);
> > @@ -1088,7 +1093,7 @@ static void bxt_enable_backlight(struct intel_connector *connector)
> >  	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
> >  			panel->backlight.max);
> >  
> > -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> > +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
> >  
> >  	pwm_ctl = 0;
> >  	if (panel->backlight.active_low_pwm)
> > @@ -1100,8 +1105,10 @@ static void bxt_enable_backlight(struct intel_connector *connector)
> >  			pwm_ctl | BXT_BLC_PWM_ENABLE);
> >  }
> >  
> > -static void cnp_enable_backlight(struct intel_connector *connector)
> > +static void cnp_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 drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >  	struct intel_panel *panel = &connector->panel;
> >  	u32 pwm_ctl;
> > @@ -1117,7 +1124,7 @@ static void cnp_enable_backlight(struct intel_connector *connector)
> >  	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
> >  		   panel->backlight.max);
> >  
> > -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> > +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
> >  
> >  	pwm_ctl = 0;
> >  	if (panel->backlight.active_low_pwm)
> > @@ -1129,12 +1136,14 @@ static void cnp_enable_backlight(struct intel_connector *connector)
> >  		   pwm_ctl | BXT_BLC_PWM_ENABLE);
> >  }
> >  
> > -static void pwm_enable_backlight(struct intel_connector *connector)
> > +static void 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;
> >  
> >  	pwm_enable(panel->backlight.pwm);
> > -	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> > +	intel_panel_actually_set_backlight(conn_state, panel->backlight.level);
> >  }
> >  
> >  void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
> > @@ -1163,7 +1172,7 @@ void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
> >  						 panel->backlight.device->props.max_brightness);
> >  	}
> >  
> > -	panel->backlight.enable(connector);
> > +	panel->backlight.enable(crtc_state, conn_state);
> >  	panel->backlight.enabled = true;
> >  	if (panel->backlight.device)
> >  		panel->backlight.device->props.power = FB_BLANK_UNBLANK;
> > @@ -1181,7 +1190,7 @@ static int intel_backlight_device_update_status(struct backlight_device *bd)
> >  	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> >  	DRM_DEBUG_KMS("updating intel_backlight, brightness=%d/%d\n",
> >  		      bd->props.brightness, bd->props.max_brightness);
> > -	intel_panel_set_backlight(connector, bd->props.brightness,
> > +	intel_panel_set_backlight(connector->base.state, bd->props.brightness,
> >  				  bd->props.max_brightness);
> >  
> >  	/*
> > -- 
> > 2.11.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Pass atomic state to backlight.

[Patchwork]

== Series Details ==

Series: drm/i915: Pass atomic state to backlight.
URL   : https://patchwork.freedesktop.org/series/25643/
State : success

== Summary ==

Series 25643v1 drm/i915: Pass atomic state to backlight.
https://patchwork.freedesktop.org/api/1.0/series/25643/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_busy:
        Subgroup basic-flip-default-a:
                pass       -> DMESG-WARN (fi-skl-6700hq) fdo#101144 +3
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-skl-6700hq) fdo#101154 +7

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144
fdo#101154 https://bugs.freedesktop.org/show_bug.cgi?id=101154

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:448s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:437s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:588s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:515s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:489s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:480s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:587s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:438s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:413s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:412s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:495s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:470s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:571s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:573s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:466s
fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:413s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:465s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:473s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:437s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:533s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:406s

8e17506ac5b18e097fe7deb1e95e2a4fa6fe90b8 drm-tip: 2017y-06m-12d-08h-21m-35s UTC integration manifest
c375c9e drm/i915: Pass atomic state to backlight enable/disable/set callbacks.
2a0ba53 drm/i915: Pass connector state to intel_panel_set_backlight_acpi
62de910 drm/i915: Pass crtc_state and connector state to backlight enable/disable functions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4934/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx