Skip to contentSkip to conversation

drm/i915/dsi: stop using drm_panel, refactor

PATCH In review Jani Nikula 6 participants 19 messages Mar 6, 2017 – Mar 7, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover drm/i915/dsi: stop using drm_panel, refactor

This is pretty natural continuation to what Hans started. We haven't made use of the drm_panel framework much at all, and there's no need in sight, really. It was good for ensuring a certain amount of separation between the core and the VBT stuff. Let's keep things that way, but without the interface.

BR,
Jani.

Jani Nikula (7):
drm/i915/dsi: remove support for more than one panel driver drm/i915/dsi: call vbt_panel_get_modes directly instead of via
drm_panel
drm/i915/dsi: stop using the drm_panel framework completely drm/i915/dsi: rename intel_dsi_exec_vbt_sequence to
intel_dsi_vbt_exec_sequence
drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable drm/i915/dsi: rename intel_dsi_panel_vbt.c to intel_dsi_vbt.c drm/i915/dsi: arrange intel_dsi.h according to relevant files

 drivers/gpu/drm/i915/Makefile                      |  2 +-
 drivers/gpu/drm/i915/intel_dsi.c                   | 76 ++++++++--------------
 drivers/gpu/drm/i915/intel_dsi.h                   | 14 ++--
 .../{intel_dsi_panel_vbt.c => intel_dsi_vbt.c}     | 46 +++----------
 4 files changed, 43 insertions(+), 95 deletions(-)
 rename drivers/gpu/drm/i915/{intel_dsi_panel_vbt.c => intel_dsi_vbt.c} (95%)
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#1 lore raw

PATCH 1/7 drm/i915/dsi: remove support for more than one panel driver

Fact is, there are no other panel drivers except the VBT based one. Simplify the code and maintenance. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c           | 19 +------------------
 drivers/gpu/drm/i915/intel_dsi.h           |  2 +-
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  2 +-
 3 files changed, 3 insertions(+), 20 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +1 -18
index 323fc097c3ee..316a0d6511fb 100644
@@ -36,16 +36,6 @@
 #include "intel_drv.h"
 #include "intel_dsi.h"
 
-static const struct {
-	u16 panel_id;
-	struct drm_panel * (*init)(struct intel_dsi *intel_dsi, u16 panel_id);
-} intel_dsi_drivers[] = {
-	{
-		.panel_id = MIPI_DSI_GENERIC_PANEL_ID,
-		.init = vbt_panel_init,
-	},
-};
-
 /* return pixels in terms of txbyteclkhs */
 static u16 txbyteclkhs(u16 pixels, int bpp, int lane_count,
 		       u16 burst_mode_ratio)
@@ -1709,7 +1699,6 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	struct drm_connector *connector;
 	struct drm_display_mode *scan, *fixed_mode = NULL;
 	enum port port;
-	unsigned int i;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -1816,13 +1805,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 		intel_dsi->dsi_hosts[port] = host;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(intel_dsi_drivers); i++) {
-		intel_dsi->panel = intel_dsi_drivers[i].init(intel_dsi,
-							     intel_dsi_drivers[i].panel_id);
-		if (intel_dsi->panel)
-			break;
-	}
-
+	intel_dsi->panel = intel_dsi_vbt_init(intel_dsi, MIPI_DSI_GENERIC_PANEL_ID);
 	if (!intel_dsi->panel) {
 		DRM_DEBUG_KMS("no device found\n");
 		goto err;
drivers/gpu/drm/i915/intel_dsi.h +1 -1
index 548649158abd..ac2f9e481f9d 100644
@@ -146,7 +146,7 @@ u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
 void intel_dsi_reset_clocks(struct intel_encoder *encoder,
 			    enum port port);
 
-struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id);
+struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
 enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
 
 #endif /* _INTEL_DSI_H */
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +1 -1
index ab922b6eb36a..67d5092fd830 100644
@@ -518,7 +518,7 @@ static const struct drm_panel_funcs vbt_panel_funcs = {
 	.get_modes = vbt_panel_get_modes,
 };
 
-struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
+struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 {
 	struct drm_device *dev = intel_dsi->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#2 lore raw

PATCH 2/7 drm/i915/dsi: call vbt_panel_get_modes directly instead of via drm_panel

In reply to Jani Nikula

Commit 18a00095a5f3 ("drm/i915/dsi: Make intel_dsi_enable/disable directly exec VBT sequences") started calling the VBT sequence functions directly instead of using the drm_panel hooks. Remove the last drm_panel hook by calling vbt_panel_get_modes() directly. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c           | 2 +-
 drivers/gpu/drm/i915/intel_dsi.h           | 1 +
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 7 +------
 3 files changed, 3 insertions(+), 7 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +1 -1
index 316a0d6511fb..5a38112c8619 100644
@@ -1843,7 +1843,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	drm_panel_attach(intel_dsi->panel, connector);
 
 	mutex_lock(&dev->mode_config.mutex);
-	drm_panel_get_modes(intel_dsi->panel);
+	intel_dsi_vbt_get_modes(intel_dsi->panel);
 	list_for_each_entry(scan, &connector->probed_modes, head) {
 		if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
 			fixed_mode = drm_mode_duplicate(dev, scan);
drivers/gpu/drm/i915/intel_dsi.h +1 -0
index ac2f9e481f9d..bd3ec02daecb 100644
@@ -147,6 +147,7 @@ void intel_dsi_reset_clocks(struct intel_encoder *encoder,
 			    enum port port);
 
 struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
+int intel_dsi_vbt_get_modes(struct drm_panel *panel);
 enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
 
 #endif /* _INTEL_DSI_H */
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +1 -6
index 67d5092fd830..efe4845c56ac 100644
@@ -492,7 +492,7 @@ void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
 	}
 }
 
-static int vbt_panel_get_modes(struct drm_panel *panel)
+int intel_dsi_vbt_get_modes(struct drm_panel *panel)
 {
 	struct vbt_panel *vbt_panel = to_vbt_panel(panel);
 	struct intel_dsi *intel_dsi = vbt_panel->intel_dsi;
@@ -514,10 +514,6 @@ static int vbt_panel_get_modes(struct drm_panel *panel)
 	return 1;
 }
 
-static const struct drm_panel_funcs vbt_panel_funcs = {
-	.get_modes = vbt_panel_get_modes,
-};
-
 struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 {
 	struct drm_device *dev = intel_dsi->base.base.dev;
@@ -816,7 +812,6 @@ struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 
 	vbt_panel->intel_dsi = intel_dsi;
 	drm_panel_init(&vbt_panel->panel);
-	vbt_panel->panel.funcs = &vbt_panel_funcs;
 	drm_panel_add(&vbt_panel->panel);
 
 	/* a regular driver would get the device in probe */
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#3 lore raw

PATCH 3/7 drm/i915/dsi: stop using the drm_panel framework completely

In reply to Jani Nikula

Now that we've stopped using the drm_panel hooks, there aren't any benefits left with using the drm_panel framework. Remove the rest of the drm_panel use. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c           | 14 ++---------
 drivers/gpu/drm/i915/intel_dsi.h           |  5 ++--
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 39 ++++++------------------------
 3 files changed, 11 insertions(+), 47 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +2 -12
index 5a38112c8619..b9ba2d55e4e1 100644
@@ -28,7 +28,6 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
 #include <drm/i915_drm.h>
-#include <drm/drm_panel.h>
 #include <drm/drm_mipi_dsi.h>
 #include <linux/slab.h>
 #include <linux/gpio/consumer.h>
@@ -1642,12 +1641,6 @@ static void intel_dsi_encoder_destroy(struct drm_encoder *encoder)
 {
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 
-	if (intel_dsi->panel) {
-		drm_panel_detach(intel_dsi->panel);
-		/* XXX: Logically this call belongs in the panel driver. */
-		drm_panel_remove(intel_dsi->panel);
-	}
-
 	/* dispose of the gpios */
 	if (intel_dsi->gpio_panel)
 		gpiod_put(intel_dsi->gpio_panel);
@@ -1805,8 +1798,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 		intel_dsi->dsi_hosts[port] = host;
 	}
 
-	intel_dsi->panel = intel_dsi_vbt_init(intel_dsi, MIPI_DSI_GENERIC_PANEL_ID);
-	if (!intel_dsi->panel) {
+	if (!intel_dsi_vbt_init(intel_dsi, MIPI_DSI_GENERIC_PANEL_ID)) {
 		DRM_DEBUG_KMS("no device found\n");
 		goto err;
 	}
@@ -1840,10 +1832,8 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 
-	drm_panel_attach(intel_dsi->panel, connector);
-
 	mutex_lock(&dev->mode_config.mutex);
-	intel_dsi_vbt_get_modes(intel_dsi->panel);
+	intel_dsi_vbt_get_modes(intel_dsi);
 	list_for_each_entry(scan, &connector->probed_modes, head) {
 		if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
 			fixed_mode = drm_mode_duplicate(dev, scan);
drivers/gpu/drm/i915/intel_dsi.h +2 -3
index bd3ec02daecb..0a3f133e8888 100644
@@ -39,7 +39,6 @@ struct intel_dsi_host;
 struct intel_dsi {
 	struct intel_encoder base;
 
-	struct drm_panel *panel;
 	struct intel_dsi_host *dsi_hosts[I915_MAX_PORTS];
 
 	/* GPIO Desc for CRC based Panel control */
@@ -146,8 +145,8 @@ u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
 void intel_dsi_reset_clocks(struct intel_encoder *encoder,
 			    enum port port);
 
-struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
-int intel_dsi_vbt_get_modes(struct drm_panel *panel);
+bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
+int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi);
 enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
 
 #endif /* _INTEL_DSI_H */
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +7 -32
index efe4845c56ac..af1783f94d23 100644
@@ -28,7 +28,6 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
 #include <drm/i915_drm.h>
-#include <drm/drm_panel.h>
 #include <linux/gpio/consumer.h>
 #include <linux/slab.h>
 #include <video/mipi_display.h>
@@ -38,16 +37,6 @@
 #include "intel_drv.h"
 #include "intel_dsi.h"
 
-struct vbt_panel {
-	struct drm_panel panel;
-	struct intel_dsi *intel_dsi;
-};
-
-static inline struct vbt_panel *to_vbt_panel(struct drm_panel *panel)
-{
-	return container_of(panel, struct vbt_panel, panel);
-}
-
 #define MIPI_TRANSFER_MODE_SHIFT	0
 #define MIPI_VIRTUAL_CHANNEL_SHIFT	1
 #define MIPI_PORT_SHIFT			3
@@ -492,36 +481,31 @@ void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
 	}
 }
 
-int intel_dsi_vbt_get_modes(struct drm_panel *panel)
+int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi)
 {
-	struct vbt_panel *vbt_panel = to_vbt_panel(panel);
-	struct intel_dsi *intel_dsi = vbt_panel->intel_dsi;
+	struct intel_connector *connector = intel_dsi->attached_connector;
 	struct drm_device *dev = intel_dsi->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_display_mode *mode;
 
-	if (!panel->connector)
-		return 0;
-
 	mode = drm_mode_duplicate(dev, dev_priv->vbt.lfp_lvds_vbt_mode);
 	if (!mode)
 		return 0;
 
 	mode->type |= DRM_MODE_TYPE_PREFERRED;
 
-	drm_mode_probed_add(panel->connector, mode);
+	drm_mode_probed_add(&connector->base, mode);
 
 	return 1;
 }
 
-struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
+bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 {
 	struct drm_device *dev = intel_dsi->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
 	struct mipi_pps_data *pps = dev_priv->vbt.dsi.pps;
 	struct drm_display_mode *mode = dev_priv->vbt.lfp_lvds_vbt_mode;
-	struct vbt_panel *vbt_panel;
 	u32 bpp;
 	u32 tlpx_ns, extra_byte_count, bitrate, tlpx_ui;
 	u32 ui_num, ui_den;
@@ -584,7 +568,7 @@ struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 			if (mipi_config->target_burst_mode_freq <
 								computed_ddr) {
 				DRM_ERROR("Burst mode freq is less than computed\n");
-				return NULL;
+				return false;
 			}
 
 			burst_mode_ratio = DIV_ROUND_UP(
@@ -594,7 +578,7 @@ struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 			pclk = DIV_ROUND_UP(pclk * burst_mode_ratio, 100);
 		} else {
 			DRM_ERROR("Burst mode target is not set\n");
-			return NULL;
+			return false;
 		}
 	} else
 		burst_mode_ratio = 100;
@@ -805,19 +789,10 @@ struct drm_panel *intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	intel_dsi->panel_off_delay = pps->panel_off_delay / 10;
 	intel_dsi->panel_pwr_cycle_delay = pps->panel_power_cycle_delay / 10;
 
-	/* This is cheating a bit with the cleanup. */
-	vbt_panel = devm_kzalloc(dev->dev, sizeof(*vbt_panel), GFP_KERNEL);
-	if (!vbt_panel)
-		return NULL;
-
-	vbt_panel->intel_dsi = intel_dsi;
-	drm_panel_init(&vbt_panel->panel);
-	drm_panel_add(&vbt_panel->panel);
-
 	/* a regular driver would get the device in probe */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		mipi_dsi_attach(intel_dsi->dsi_hosts[port]->device);
 	}
 
-	return &vbt_panel->panel;
+	return true;
 }
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#4 lore raw

PATCH 4/7 drm/i915/dsi: rename intel_dsi_exec_vbt_sequence to intel_dsi_vbt_exec_sequence

In reply to Jani Nikula

Use the prefix intel_dsi_vbt for all the DSI VBT functions. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c           | 24 ++++++++++++------------
 drivers/gpu/drm/i915/intel_dsi.h           |  5 ++---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  2 +-
 3 files changed, 15 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +12 -12
index b9ba2d55e4e1..189b91478f8e 100644
@@ -806,38 +806,38 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 	/* Power on, try both CRC pmic gpio and VBT */
 	if (intel_dsi->gpio_panel)
 		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
 	intel_dsi_msleep(intel_dsi, intel_dsi->panel_on_delay);
 
 	/* Deassert reset */
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
 
 	/* Put device in ready state (LP-11) */
 	intel_dsi_device_ready(encoder);
 
 	/* Send initialization commands in LP mode */
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);
 
 	/* Enable port in pre-enable phase itself because as per hw team
 	 * recommendation, port should be enabled befor plane & pipe */
 	if (is_cmd_mode(intel_dsi)) {
 		for_each_dsi_port(port, intel_dsi->ports)
 			I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4);
-		intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_TEAR_ON);
-		intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
+		intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_TEAR_ON);
+		intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
 	} else {
 		msleep(20); /* XXX */
 		for_each_dsi_port(port, intel_dsi->ports)
 			dpi_send_cmd(intel_dsi, TURN_ON, false, port);
 		intel_dsi_msleep(intel_dsi, 100);
 
-		intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
+		intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
 
 		intel_dsi_port_enable(encoder);
 	}
 
 	intel_panel_enable_backlight(intel_dsi->attached_connector);
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 }
 
 static void intel_dsi_enable_nop(struct intel_encoder *encoder,
@@ -863,7 +863,7 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder,
 
 	DRM_DEBUG_KMS("\n");
 
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
 	intel_panel_disable_backlight(intel_dsi->attached_connector);
 
 	/*
@@ -925,8 +925,8 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder,
 	 * some next enable sequence send turn on packet error is observed
 	 */
 	if (is_cmd_mode(intel_dsi))
-		intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_TEAR_OFF);
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_OFF);
+		intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_TEAR_OFF);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_OFF);
 
 	/* Transition to LP-00 */
 	intel_dsi_clear_device_ready(encoder);
@@ -953,11 +953,11 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder,
 	}
 
 	/* Assert reset */
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
 
 	/* Power off, try both CRC pmic gpio and VBT */
 	intel_dsi_msleep(intel_dsi, intel_dsi->panel_off_delay);
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_POWER_OFF);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_POWER_OFF);
 	if (intel_dsi->gpio_panel)
 		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
 
drivers/gpu/drm/i915/intel_dsi.h +2 -3
index 0a3f133e8888..d395f6de041c 100644
@@ -131,9 +131,6 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 
 void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
 
-void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
-				 enum mipi_seq seq_id);
-
 bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
 int intel_compute_dsi_pll(struct intel_encoder *encoder,
 			  struct intel_crtc_state *config);
@@ -147,6 +144,8 @@ void intel_dsi_reset_clocks(struct intel_encoder *encoder,
 
 bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
 int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi);
+void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
+				 enum mipi_seq seq_id);
 enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
 
 #endif /* _INTEL_DSI_H */
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +1 -1
index af1783f94d23..0dce7792643a 100644
@@ -415,7 +415,7 @@ static const char *sequence_name(enum mipi_seq seq_id)
 		return "(unknown)";
 }
 
-void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
+void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
 				 enum mipi_seq seq_id)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#6 lore raw

PATCH 5/7 drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable

In reply to Jani Nikula

The hook names reflect more the phase in the mode set sequence the hooks are called in than what they actually do in terms of the specific encoder. Stick to that scheme, and rename intel_dsi_pre_disable to intel_dsi_disable. Unify the comments around this while at it. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +12 -9
index 189b91478f8e..ebe1f55b20d6 100644
@@ -840,21 +840,24 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 }
 
+/*
+ * For DSI port enable has to be done before pipe and plane enable, so port
+ * enable is done in pre_enable phase unlike other encoders.
+ */
 static void intel_dsi_enable_nop(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config,
 				 struct drm_connector_state *conn_state)
 {
 	DRM_DEBUG_KMS("\n");
-
-	/* for DSI port enable has to be done before pipe
-	 * and plane enable, so port enable is done in
-	 * pre_enable phase itself unlike other encoders
-	 */
 }
 
-static void intel_dsi_pre_disable(struct intel_encoder *encoder,
-				  struct intel_crtc_state *old_crtc_state,
-				  struct drm_connector_state *old_conn_state)
+/*
+ * For DSI port disable has to be done after pipe and plane disable, so port
+ * disable is done in post_disable phase unlike other encoders.
+ */
+static void intel_dsi_disable(struct intel_encoder *encoder,
+			      struct intel_crtc_state *old_crtc_state,
+			      struct drm_connector_state *old_conn_state)
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1730,7 +1733,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	intel_encoder->compute_config = intel_dsi_compute_config;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
 	intel_encoder->enable = intel_dsi_enable_nop;
-	intel_encoder->disable = intel_dsi_pre_disable;
+	intel_encoder->disable = intel_dsi_disable;
 	intel_encoder->post_disable = intel_dsi_post_disable;
 	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
 	intel_encoder->get_config = intel_dsi_get_config;
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#12 lore raw

PATCH 6/7 drm/i915/dsi: rename intel_dsi_panel_vbt.c to intel_dsi_vbt.c

In reply to Jani Nikula

Emphasize that the VBT file is nowadays more about initializing and running stuff based on the VBT contents, not so much about being a "panel driver". No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/Makefile                                   | 2 +-
 drivers/gpu/drm/i915/{intel_dsi_panel_vbt.c => intel_dsi_vbt.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/gpu/drm/i915/{intel_dsi_panel_vbt.c => intel_dsi_vbt.c} (100%)
drivers/gpu/drm/i915/Makefile +1 -1
index b1b580337c7a..2cf04504e494 100644
@@ -105,8 +105,8 @@ i915-y += dvo_ch7017.o \
 	  intel_dp.o \
 	  intel_dsi.o \
 	  intel_dsi_dcs_backlight.o \
-	  intel_dsi_panel_vbt.o \
 	  intel_dsi_pll.o \
+	  intel_dsi_vbt.o \
 	  intel_dvo.o \
 	  intel_hdmi.o \
 	  intel_i2c.o \
drivers/gpu/drm/i915/intel_dsi_vbt.c renamed from drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +0 -0
similarity index 100%
rename from drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
rename to drivers/gpu/drm/i915/intel_dsi_vbt.c
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#13 lore raw

PATCH 7/7 drm/i915/dsi: arrange intel_dsi.h according to relevant files

In reply to Jani Nikula

No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/intel_dsi.h +4 -1
index d395f6de041c..7afeb9580f41 100644
@@ -129,8 +129,11 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 	return container_of(encoder, struct intel_dsi, base.base);
 }
 
+/* intel_dsi.c */
 void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
+enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
 
+/* intel_dsi_pll.c */
 bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
 int intel_compute_dsi_pll(struct intel_encoder *encoder,
 			  struct intel_crtc_state *config);
@@ -142,10 +145,10 @@ u32 intel_dsi_get_pclk(struct intel_encoder *encoder, int pipe_bpp,
 void intel_dsi_reset_clocks(struct intel_encoder *encoder,
 			    enum port port);
 
+/* intel_dsi_vbt.c */
 bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
 int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi);
 void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
 				 enum mipi_seq seq_id);
-enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
 
 #endif /* _INTEL_DSI_H */
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#7 lore raw

Re: [PATCH 5/7] drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable

In reply to Jani Nikula
Quoted 19 lines · Jani Nikula

The hook names reflect more the phase in the mode set sequence the hooks are called in than what they actually do in terms of the specific encoder. Stick to that scheme, and rename intel_dsi_pre_disable to intel_dsi_disable. Unify the comments around this while at it. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +0 -0
index 189b91478f8e..ebe1f55b20d6 100644

drivers/gpu/drm/i915/intel_dsi.c static void intel_dsi_pre_enable(struct intel_encoder *encoder, · line 845

Quoted 6 lines
drivers/gpu/drm/i915/intel_dsi.c +3 -0
@@ -840,21 +840,24 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 }
 
+/*
+ * For DSI port enable has to be done before pipe and plane enable, so port
+ * enable is done in pre_enable phase unlike other encoders.

The "unlike other encoders" part is nonsense.

Quoted 42 lines
drivers/gpu/drm/i915/intel_dsi.c +9 -9
+ */
 static void intel_dsi_enable_nop(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config,
 				 struct drm_connector_state *conn_state)
 {
 	DRM_DEBUG_KMS("\n");
-
-	/* for DSI port enable has to be done before pipe
-	 * and plane enable, so port enable is done in
-	 * pre_enable phase itself unlike other encoders
-	 */
 }
 
-static void intel_dsi_pre_disable(struct intel_encoder *encoder,
-				  struct intel_crtc_state *old_crtc_state,
-				  struct drm_connector_state *old_conn_state)
+/*
+ * For DSI port disable has to be done after pipe and plane disable, so port
+ * disable is done in post_disable phase unlike other encoders.
+ */
+static void intel_dsi_disable(struct intel_encoder *encoder,
+			      struct intel_crtc_state *old_crtc_state,
+			      struct drm_connector_state *old_conn_state)
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1730,7 +1733,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	intel_encoder->compute_config = intel_dsi_compute_config;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
 	intel_encoder->enable = intel_dsi_enable_nop;
-	intel_encoder->disable = intel_dsi_pre_disable;
+	intel_encoder->disable = intel_dsi_disable;
 	intel_encoder->post_disable = intel_dsi_post_disable;
 	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
 	intel_encoder->get_config = intel_dsi_get_config;
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on the v1 series
#14 lore raw
In reply to Jani Nikula

Hi,

Quoted 8 lines · Jani Nikula

This is pretty natural continuation to what Hans started. We haven't made use of the drm_panel framework much at all, and there's no need in sight, really. It was good for ensuring a certain amount of separation between the core and the VBT stuff. Let's keep things that way, but without the interface.

BR,
Jani.

I like and the series looks good to me:

Regards,

Hans

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#8 lore raw

Re: [PATCH 5/7] drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable

In reply to Ville Syrjälä
Quoted 23 lines · Ville Syrjälä
Quoted 22 lines · Jani Nikula

The hook names reflect more the phase in the mode set sequence the hooks are called in than what they actually do in terms of the specific encoder. Stick to that scheme, and rename intel_dsi_pre_disable to intel_dsi_disable. Unify the comments around this while at it. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +0 -0
index 189b91478f8e..ebe1f55b20d6 100644
@@ -840,21 +840,24 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 }
 
Quoted 5 lines

drivers/gpu/drm/i915/intel_dsi.c static void intel_dsi_pre_enable(struct intel_encoder *encoder, · line 845

Quoted 3 lines
drivers/gpu/drm/i915/intel_dsi.c +3 -0
+/*
+ * For DSI port enable has to be done before pipe and plane enable, so port
+ * enable is done in pre_enable phase unlike other encoders.

The "unlike other encoders" part is nonsense.

Copy-paste ftw... I'll drop it.

J.

Quoted 43 lines
Quoted 43 lines
drivers/gpu/drm/i915/intel_dsi.c +9 -9
+ */
 static void intel_dsi_enable_nop(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config,
 				 struct drm_connector_state *conn_state)
 {
 	DRM_DEBUG_KMS("\n");
-
-	/* for DSI port enable has to be done before pipe
-	 * and plane enable, so port enable is done in
-	 * pre_enable phase itself unlike other encoders
-	 */
 }
 
-static void intel_dsi_pre_disable(struct intel_encoder *encoder,
-				  struct intel_crtc_state *old_crtc_state,
-				  struct drm_connector_state *old_conn_state)
+/*
+ * For DSI port disable has to be done after pipe and plane disable, so port
+ * disable is done in post_disable phase unlike other encoders.
+ */
+static void intel_dsi_disable(struct intel_encoder *encoder,
+			      struct intel_crtc_state *old_crtc_state,
+			      struct drm_connector_state *old_conn_state)
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1730,7 +1733,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	intel_encoder->compute_config = intel_dsi_compute_config;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
 	intel_encoder->enable = intel_dsi_enable_nop;
-	intel_encoder->disable = intel_dsi_pre_disable;
+	intel_encoder->disable = intel_dsi_disable;
 	intel_encoder->post_disable = intel_dsi_post_disable;
 	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
 	intel_encoder->get_config = intel_dsi_get_config;
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#15 lore raw

✓ Fi.CI.BAT: success for drm/i915/dsi: stop using drm_panel, refactor

In reply to Jani Nikula
== Series Details ==

== Series Details ==

Series: drm/i915/dsi: stop using drm_panel, refactor
URL : https://patchwork.freedesktop.org/series/20767/
State : success

== Summary ==

Series 20767v1 drm/i915/dsi: stop using drm_panel, refactor
https://patchwork.freedesktop.org/api/1.0/series/20767/revisions/1/mbox/

Test gem_mmap_gtt:

        Subgroup basic-small-bo:
                dmesg-warn -> PASS       (fi-bsw-n3050)
fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 460s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 618s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 533s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 627s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 509s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 504s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 443s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 441s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 444s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 509s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 490s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 475s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 504s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 593s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 503s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 551s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 552s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 418s

e06000745435e65b4c056fe8f5bf149b298a0526 drm-tip: 2017y-03m-06d-14h-39m-38s UTC integration manifest 33528c6 drm/i915/dsi: arrange intel_dsi.h according to relevant files
0812e2f drm/i915/dsi: rename intel_dsi_panel_vbt.c to intel_dsi_vbt.c
4e7dcbd drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable
f74601c drm/i915/dsi: rename intel_dsi_exec_vbt_sequence to intel_dsi_vbt_exec_sequence c88e0f8 drm/i915/dsi: stop using the drm_panel framework completely
4c0b90a drm/i915/dsi: call vbt_panel_get_modes directly instead of via drm_panel 48092217 drm/i915/dsi: remove support for more than one panel driver

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4074/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#5 lore raw

Re: [PATCH 4/7] drm/i915/dsi: rename intel_dsi_exec_vbt_sequence to intel_dsi_vbt_exec_sequence

In reply to Jani Nikula
Quoted 7 lines · Jani Nikula

Use the prefix intel_dsi_vbt for all the DSI VBT functions. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>

Patches 1-4: Reviewed-by: Daniel Vetter <[email protected]>

Quoted 133 lines
---
 drivers/gpu/drm/i915/intel_dsi.c           | 24 ++++++++++++------------
 drivers/gpu/drm/i915/intel_dsi.h           |  5 ++---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |  2 +-
 3 files changed, 15 insertions(+), 16 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +12 -12
index b9ba2d55e4e1..189b91478f8e 100644
@@ -806,38 +806,38 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 	/* Power on, try both CRC pmic gpio and VBT */
 	if (intel_dsi->gpio_panel)
 		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
 	intel_dsi_msleep(intel_dsi, intel_dsi->panel_on_delay);
 
 	/* Deassert reset */
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
 
 	/* Put device in ready state (LP-11) */
 	intel_dsi_device_ready(encoder);
 
 	/* Send initialization commands in LP mode */
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);
 
 	/* Enable port in pre-enable phase itself because as per hw team
 	 * recommendation, port should be enabled befor plane & pipe */
 	if (is_cmd_mode(intel_dsi)) {
 		for_each_dsi_port(port, intel_dsi->ports)
 			I915_WRITE(MIPI_MAX_RETURN_PKT_SIZE(port), 8 * 4);
-		intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_TEAR_ON);
-		intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
+		intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_TEAR_ON);
+		intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
 	} else {
 		msleep(20); /* XXX */
 		for_each_dsi_port(port, intel_dsi->ports)
 			dpi_send_cmd(intel_dsi, TURN_ON, false, port);
 		intel_dsi_msleep(intel_dsi, 100);
 
-		intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
+		intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
 
 		intel_dsi_port_enable(encoder);
 	}
 
 	intel_panel_enable_backlight(intel_dsi->attached_connector);
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 }
 
 static void intel_dsi_enable_nop(struct intel_encoder *encoder,
@@ -863,7 +863,7 @@ static void intel_dsi_pre_disable(struct intel_encoder *encoder,
 
 	DRM_DEBUG_KMS("\n");
 
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
 	intel_panel_disable_backlight(intel_dsi->attached_connector);
 
 	/*
@@ -925,8 +925,8 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder,
 	 * some next enable sequence send turn on packet error is observed
 	 */
 	if (is_cmd_mode(intel_dsi))
-		intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_TEAR_OFF);
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_OFF);
+		intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_TEAR_OFF);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_DISPLAY_OFF);
 
 	/* Transition to LP-00 */
 	intel_dsi_clear_device_ready(encoder);
@@ -953,11 +953,11 @@ static void intel_dsi_post_disable(struct intel_encoder *encoder,
 	}
 
 	/* Assert reset */
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
 
 	/* Power off, try both CRC pmic gpio and VBT */
 	intel_dsi_msleep(intel_dsi, intel_dsi->panel_off_delay);
-	intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_POWER_OFF);
+	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_POWER_OFF);
 	if (intel_dsi->gpio_panel)
 		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
 
drivers/gpu/drm/i915/intel_dsi.h +2 -3
index 0a3f133e8888..d395f6de041c 100644
@@ -131,9 +131,6 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 
 void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
 
-void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
-				 enum mipi_seq seq_id);
-
 bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
 int intel_compute_dsi_pll(struct intel_encoder *encoder,
 			  struct intel_crtc_state *config);
@@ -147,6 +144,8 @@ void intel_dsi_reset_clocks(struct intel_encoder *encoder,
 
 bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
 int intel_dsi_vbt_get_modes(struct intel_dsi *intel_dsi);
+void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
+				 enum mipi_seq seq_id);
 enum mipi_dsi_pixel_format pixel_format_from_register_bits(u32 fmt);
 
 #endif /* _INTEL_DSI_H */
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c +1 -1
index af1783f94d23..0dce7792643a 100644
@@ -415,7 +415,7 @@ static const char *sequence_name(enum mipi_seq seq_id)
 		return "(unknown)";
 }
 
-void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
+void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
 				 enum mipi_seq seq_id)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#9 lore raw

Re: [PATCH 5/7] drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable

In reply to Ville Syrjälä
Quoted 23 lines · Ville Syrjälä
Quoted 22 lines · Jani Nikula

The hook names reflect more the phase in the mode set sequence the hooks are called in than what they actually do in terms of the specific encoder. Stick to that scheme, and rename intel_dsi_pre_disable to intel_dsi_disable. Unify the comments around this while at it. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +0 -0
index 189b91478f8e..ebe1f55b20d6 100644
@@ -840,21 +840,24 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 }
 
Quoted 5 lines

drivers/gpu/drm/i915/intel_dsi.c static void intel_dsi_pre_enable(struct intel_encoder *encoder, · line 845

Quoted 3 lines
drivers/gpu/drm/i915/intel_dsi.c +3 -0
+/*
+ * For DSI port enable has to be done before pipe and plane enable, so port
+ * enable is done in pre_enable phase unlike other encoders.

The "unlike other encoders" part is nonsense.

We might want to go one step further even and make the hook optional and remove these few lines here ... Probably not worth it though. -Daniel

Quoted 51 lines
Quoted 43 lines
drivers/gpu/drm/i915/intel_dsi.c +9 -9
+ */
 static void intel_dsi_enable_nop(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config,
 				 struct drm_connector_state *conn_state)
 {
 	DRM_DEBUG_KMS("\n");
-
-	/* for DSI port enable has to be done before pipe
-	 * and plane enable, so port enable is done in
-	 * pre_enable phase itself unlike other encoders
-	 */
 }
 
-static void intel_dsi_pre_disable(struct intel_encoder *encoder,
-				  struct intel_crtc_state *old_crtc_state,
-				  struct drm_connector_state *old_conn_state)
+/*
+ * For DSI port disable has to be done after pipe and plane disable, so port
+ * disable is done in post_disable phase unlike other encoders.
+ */
+static void intel_dsi_disable(struct intel_encoder *encoder,
+			      struct intel_crtc_state *old_crtc_state,
+			      struct drm_connector_state *old_conn_state)
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1730,7 +1733,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	intel_encoder->compute_config = intel_dsi_compute_config;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
 	intel_encoder->enable = intel_dsi_enable_nop;
-	intel_encoder->disable = intel_dsi_pre_disable;
+	intel_encoder->disable = intel_dsi_disable;
 	intel_encoder->post_disable = intel_dsi_post_disable;
 	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
 	intel_encoder->get_config = intel_dsi_get_config;
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on the v1 series
#16 lore raw
In reply to Jani Nikula
Quoted 8 lines · On Mon, 6 Mar 2017 16:31:23 +0200 Jani Nikula <[email protected]> wrote:

This is pretty natural continuation to what Hans started. We haven't made use of the drm_panel framework much at all, and there's no need in sight, really. It was good for ensuring a certain amount of separation between the core and the VBT stuff. Let's keep things that way, but without the interface.

BR,
Jani.

This all looks good to me also, so with the update to patch 5

Quoted 19 lines

Jani Nikula (7):
drm/i915/dsi: remove support for more than one panel driver drm/i915/dsi: call vbt_panel_get_modes directly instead of via
drm_panel
drm/i915/dsi: stop using the drm_panel framework completely drm/i915/dsi: rename intel_dsi_exec_vbt_sequence to
intel_dsi_vbt_exec_sequence
drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable drm/i915/dsi: rename intel_dsi_panel_vbt.c to intel_dsi_vbt.c drm/i915/dsi: arrange intel_dsi.h according to relevant files

 drivers/gpu/drm/i915/Makefile                      |  2 +-
 drivers/gpu/drm/i915/intel_dsi.c                   | 76 ++++++++--------------
 drivers/gpu/drm/i915/intel_dsi.h                   | 14 ++--
 .../{intel_dsi_panel_vbt.c => intel_dsi_vbt.c}     | 46 +++----------
 4 files changed, 43 insertions(+), 95 deletions(-)
 rename drivers/gpu/drm/i915/{intel_dsi_panel_vbt.c => intel_dsi_vbt.c} (95%)
Signature · 11 lines
--
Bob Paauwe                  
[email protected]
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#10 lore raw

Re: [PATCH 5/7] drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable

In reply to Daniel Vetter
Quoted 24 lines · Daniel Vetter
Quoted 23 lines · Ville Syrjälä
Quoted 22 lines · Jani Nikula

The hook names reflect more the phase in the mode set sequence the hooks are called in than what they actually do in terms of the specific encoder. Stick to that scheme, and rename intel_dsi_pre_disable to intel_dsi_disable. Unify the comments around this while at it. No functional changes.

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +0 -0
index 189b91478f8e..ebe1f55b20d6 100644
@@ -840,21 +840,24 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 }
 
Quoted 8 lines
Quoted 5 lines

drivers/gpu/drm/i915/intel_dsi.c static void intel_dsi_pre_enable(struct intel_encoder *encoder, · line 845

Quoted 3 lines
drivers/gpu/drm/i915/intel_dsi.c +3 -0
+/*
+ * For DSI port enable has to be done before pipe and plane enable, so port
+ * enable is done in pre_enable phase unlike other encoders.

The "unlike other encoders" part is nonsense.

We might want to go one step further even and make the hook optional and remove these few lines here ... Probably not worth it though.

I looked into it before, and decided it's better to keep intel_display.c nice and clean, as most if not all other encoders provide the hook.

BR,
Jani.

Quoted 52 lines

-Daniel

Quoted 51 lines
Quoted 43 lines
drivers/gpu/drm/i915/intel_dsi.c +9 -9
+ */
 static void intel_dsi_enable_nop(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config,
 				 struct drm_connector_state *conn_state)
 {
 	DRM_DEBUG_KMS("\n");
-
-	/* for DSI port enable has to be done before pipe
-	 * and plane enable, so port enable is done in
-	 * pre_enable phase itself unlike other encoders
-	 */
 }
 
-static void intel_dsi_pre_disable(struct intel_encoder *encoder,
-				  struct intel_crtc_state *old_crtc_state,
-				  struct drm_connector_state *old_conn_state)
+/*
+ * For DSI port disable has to be done after pipe and plane disable, so port
+ * disable is done in post_disable phase unlike other encoders.
+ */
+static void intel_dsi_disable(struct intel_encoder *encoder,
+			      struct intel_crtc_state *old_crtc_state,
+			      struct drm_connector_state *old_conn_state)
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1730,7 +1733,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	intel_encoder->compute_config = intel_dsi_compute_config;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
 	intel_encoder->enable = intel_dsi_enable_nop;
-	intel_encoder->disable = intel_dsi_pre_disable;
+	intel_encoder->disable = intel_dsi_disable;
 	intel_encoder->post_disable = intel_dsi_post_disable;
 	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
 	intel_encoder->get_config = intel_dsi_get_config;
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#11 lore raw

PATCH v2 1/1 drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable

In reply to Jani Nikula

The hook names reflect more the phase in the mode set sequence the hooks are called in than what they actually do in terms of the specific encoder. Stick to that scheme, and rename intel_dsi_pre_disable to intel_dsi_disable. Unify the comments around this while at it. No functional changes.

v2: Add more sense in the enable/disable hook comments (Ville)

Cc Madhav Chauhan <[email protected]>
Cc Hans de Goede <[email protected]>
Cc Bob Paauwe <[email protected]>
Cc Ville Syrjälä <[email protected]>
---
 drivers/gpu/drm/i915/intel_dsi.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/gpu/drm/i915/intel_dsi.c +12 -9
index 189b91478f8e..3ffe8b1f1d48 100644
@@ -840,21 +840,24 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 	intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 }
 
+/*
+ * DSI port enable has to be done before pipe and plane enable, so we do it in
+ * the pre_enable hook.
+ */
 static void intel_dsi_enable_nop(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config,
 				 struct drm_connector_state *conn_state)
 {
 	DRM_DEBUG_KMS("\n");
-
-	/* for DSI port enable has to be done before pipe
-	 * and plane enable, so port enable is done in
-	 * pre_enable phase itself unlike other encoders
-	 */
 }
 
-static void intel_dsi_pre_disable(struct intel_encoder *encoder,
-				  struct intel_crtc_state *old_crtc_state,
-				  struct drm_connector_state *old_conn_state)
+/*
+ * DSI port disable has to be done after pipe and plane disable, so we do it in
+ * the post_disable hook.
+ */
+static void intel_dsi_disable(struct intel_encoder *encoder,
+			      struct intel_crtc_state *old_crtc_state,
+			      struct drm_connector_state *old_conn_state)
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1730,7 +1733,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	intel_encoder->compute_config = intel_dsi_compute_config;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
 	intel_encoder->enable = intel_dsi_enable_nop;
-	intel_encoder->disable = intel_dsi_pre_disable;
+	intel_encoder->disable = intel_dsi_disable;
 	intel_encoder->post_disable = intel_dsi_post_disable;
 	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
 	intel_encoder->get_config = intel_dsi_get_config;
Signature · 6 lines
2.1.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#17 lore raw

✓ Fi.CI.BAT: success for drm/i915/dsi: stop using drm_panel, refactor (rev2)

In reply to Jani Nikula
== Series Details ==

== Series Details ==

Series: drm/i915/dsi: stop using drm_panel, refactor (rev2)
URL : https://patchwork.freedesktop.org/series/20767/
State : success

== Summary ==

Series 20767v2 drm/i915/dsi: stop using drm_panel, refactor
https://patchwork.freedesktop.org/api/1.0/series/20767/revisions/2/mbox/

Test gem_exec_flush:

        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 465s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 611s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 534s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 602s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 505s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 502s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 437s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 437s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 434s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 512s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 493s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 475s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 511s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 616s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 500s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 558s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 556s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 429s

bf077370616c6c41acd0ab6d46158569244941a8 drm-tip: 2017y-03m-07d-07h-57m-10s UTC integration manifest fc3e15c drm/i915/dsi: arrange intel_dsi.h according to relevant files
883adbb drm/i915/dsi: rename intel_dsi_panel_vbt.c to intel_dsi_vbt.c
750262f drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable
401cfac drm/i915/dsi: rename intel_dsi_exec_vbt_sequence to intel_dsi_vbt_exec_sequence f65e775 drm/i915/dsi: stop using the drm_panel framework completely
17f938d drm/i915/dsi: call vbt_panel_get_modes directly instead of via drm_panel ebf9a18 drm/i915/dsi: remove support for more than one panel driver

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4080/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#18 lore raw
In reply to Jani Nikula
Quoted 5 lines · Jani Nikula

This is pretty natural continuation to what Hans started. We haven't made use of the drm_panel framework much at all, and there's no need in sight, really. It was good for ensuring a certain amount of separation between the core and the VBT stuff. Let's keep things that way, but without the interface.

Pushed the lot, thanks for the review everyone!

BR,
Jani.

Quoted 21 lines

BR,
Jani.

Jani Nikula (7):
drm/i915/dsi: remove support for more than one panel driver drm/i915/dsi: call vbt_panel_get_modes directly instead of via
drm_panel
drm/i915/dsi: stop using the drm_panel framework completely drm/i915/dsi: rename intel_dsi_exec_vbt_sequence to
intel_dsi_vbt_exec_sequence
drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable drm/i915/dsi: rename intel_dsi_panel_vbt.c to intel_dsi_vbt.c drm/i915/dsi: arrange intel_dsi.h according to relevant files

 drivers/gpu/drm/i915/Makefile                      |  2 +-
 drivers/gpu/drm/i915/intel_dsi.c                   | 76 ++++++++--------------
 drivers/gpu/drm/i915/intel_dsi.h                   | 14 ++--
 .../{intel_dsi_panel_vbt.c => intel_dsi_vbt.c}     | 46 +++----------
 4 files changed, 43 insertions(+), 95 deletions(-)
 rename drivers/gpu/drm/i915/{intel_dsi_panel_vbt.c => intel_dsi_vbt.c} (95%)
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx