All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/7] Add mipi dsi command mode support.
@ 2019-10-14 11:01 Vandita Kulkarni
  2019-10-14 11:01 ` [RFC 1/7] drm/i915/dsi: Define command mode registers Vandita Kulkarni
                   ` (10 more replies)
  0 siblings, 11 replies; 36+ messages in thread
From: Vandita Kulkarni @ 2019-10-14 11:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

This series has mainly patches to configure the dsi in
command mode, TE event handling and initiate a frame
request to the panel. Floating the RFC for review wrt
the above mentioned implementation.
For now we are configuring the MIPI DSI to operate in
TE gate mode and take TE events via GPIO.

There are few places that needs to be fixed to handle,
cases where mpi dsi could be operating in single link
cmd mode on port B.

I have tested dual link with this series on icl-y.
There is one open wrt flipdone which needs to be fixed.
This is WIP.

Madhav Chauhan (1):
  drm/i915/dsi: Helper to find dsi encoder in cmd mode

Vandita Kulkarni (6):
  drm/i915/dsi: Define command mode registers
  drm/i915/dsi: Configure transcoder operation for command mode.
  drm/i915/dsi: Add vblank calculation for command mode
  drm/i915/dsi: Configure TE interrupt for cmd mode
  drm/i915/dsi: Add TE handler for dsi cmd mode.
  drm/i915/dsi: Initiate frame request in cmd mode

 drivers/gpu/drm/i915/display/icl_dsi.c        | 132 +++++++++++++++---
 drivers/gpu/drm/i915/display/intel_display.c  |  16 +++
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/display/intel_dsi.h      |   6 +
 drivers/gpu/drm/i915/i915_irq.c               | 110 ++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h               |  76 ++++++++--
 6 files changed, 315 insertions(+), 28 deletions(-)

-- 
2.21.0.5.gaeb582a

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [RFC 1/7] drm/i915/dsi: Define command mode registers
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
@ 2019-10-14 11:01 ` Vandita Kulkarni
  2019-10-14 16:18   ` Ramalingam C
  2019-10-15  7:07   ` Jani Nikula
  2019-10-14 11:01 ` [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode Vandita Kulkarni
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 36+ messages in thread
From: Vandita Kulkarni @ 2019-10-14 11:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Adding all the register definitions needed
for mipi dsi command mode.

Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 76 +++++++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e24991e54897..73bc85855b79 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4919,14 +4919,20 @@ enum {
 #define   BLM_PCH_POLARITY			(1 << 29)
 #define BLC_PWM_PCH_CTL2	_MMIO(0xc8254)
 
-#define UTIL_PIN_CTL		_MMIO(0x48400)
-#define   UTIL_PIN_ENABLE	(1 << 31)
-
-#define   UTIL_PIN_PIPE(x)     ((x) << 29)
-#define   UTIL_PIN_PIPE_MASK   (3 << 29)
-#define   UTIL_PIN_MODE_PWM    (1 << 24)
-#define   UTIL_PIN_MODE_MASK   (0xf << 24)
-#define   UTIL_PIN_POLARITY    (1 << 22)
+#define UTIL_PIN_CTL			_MMIO(0x48400)
+#define  UTIL_PIN_ENABLE		(1 << 31)
+#define  UTIL_PIN_PIPE_MASK		(3 << 29)
+#define  UTIL_PIN_PIPE(x)		((x) << 29)
+#define  UTIL_PIN_MODE_MASK		(0xf << 24)
+#define  UTIL_PIN_MODE_DATA		(0 << 24)
+#define  UTIL_PIN_MODE_PWM		(1 << 24)
+#define  UTIL_PIN_MODE_VBLANK		(4 << 24)
+#define  UTIL_PIN_MODE_VSYNC		(5 << 24)
+#define  UTIL_PIN_MODE_EYE_LEVEL	(8 << 24)
+#define  UTIL_PIN_OP_DATA		(1 << 23)
+#define  UTIL_PIN_POLARITY		(1 << 22)
+#define  ICL_UTIL_PIN_DIRECTION		(1 << 19)
+#define  ICL_UTIL_PIN_IP_DATA		(1 << 16)
 
 /* BXT backlight register definition. */
 #define _BXT_BLC_PWM_CTL1			0xC8250
@@ -7407,6 +7413,8 @@ enum {
 #define GEN8_DE_PORT_IMR _MMIO(0x44444)
 #define GEN8_DE_PORT_IIR _MMIO(0x44448)
 #define GEN8_DE_PORT_IER _MMIO(0x4444c)
+#define  ICL_DSI_1			(1 << 31)
+#define  ICL_DSI_0			(1 << 30)
 #define  ICL_AUX_CHANNEL_E		(1 << 29)
 #define  CNL_AUX_CHANNEL_F		(1 << 28)
 #define  GEN9_AUX_CHANNEL_D		(1 << 27)
@@ -10659,6 +10667,57 @@ enum skl_power_gate {
 #define  ICL_ESC_CLK_DIV_SHIFT			0
 #define DSI_MAX_ESC_CLK			20000		/* in KHz */
 
+#define _ICL_DSI_CMD_FRMCTL_0		0x6b034
+#define _ICL_DSI_CMD_FRMCTL_1		0x6b834
+#define ICL_DSI_CMD_FRMCTL(port)	_MMIO_PORT(port,	\
+						  _ICL_DSI_CMD_FRMCTL_0,\
+						  _ICL_DSI_CMD_FRMCTL_1)
+#define  ICL_FRAME_UPDATE_REQUEST		(1 << 31)
+#define  ICL_PERIODIC_FRAME_UPDATE_ENABLE	(1 << 29)
+#define  ICL_NULL_PACKET_ENABLE			(1 << 28)
+#define  ICL_FRAME_IN_PROGRESS			(1 << 0)
+
+#define _ICL_DSI_INTR_MASK_REG_0		0x6b070
+#define _ICL_DSI_INTR_MASK_REG_1		0x6b870
+#define ICL_DSI_INTR_MASK_REG(port)	_MMIO_PORT(port,	\
+						  _ICL_DSI_INTR_MASK_REG_0,\
+						  _ICL_DSI_INTR_MASK_REG_1)
+
+#define _ICL_DSI_INTR_IDENT_REG_0		0x6b074
+#define _ICL_DSI_INTR_IDENT_REG_1		0x6b874
+#define ICL_DSI_INTR_IDENT_REG(port)	_MMIO_PORT(port,	\
+						  _ICL_DSI_INTR_IDENT_REG_0,\
+						  _ICL_DSI_INTR_IDENT_REG_1)
+#define  ICL_TE_EVENT				(1 << 31)
+#define  ICL_RX_DATA_OR_BTA_TERMINATED		(1 << 30)
+#define  ICL_TX_DATA				(1 << 29)
+#define  ICL_ULPS_ENTRY_DONE			(1 << 28)
+#define  ICL_NON_TE_TRIGGER_RECEIVED		(1 << 27)
+#define  ICL_HOST_CHKSUM_ERROR			(1 << 26)
+#define  ICL_HOST_MULTI_ECC_ERROR		(1 << 25)
+#define  ICL_HOST_SINGL_ECC_ERROR		(1 << 24)
+#define  ICL_HOST_CONTENTION_DETECTED		(1 << 23)
+#define  ICL_HOST_FALSE_CONTROL_ERROR		(1 << 22)
+#define  ICL_HOST_TIMEOUT_ERROR			(1 << 21)
+#define  ICL_HOST_LOW_POWER_TX_SYNC_ERROR	(1 << 20)
+#define  ICL_HOST_ESCAPE_MODE_ENTRY_ERROR	(1 << 19)
+#define  ICL_FRAME_UPDATE_DONE			(1 << 16)
+#define  ICL_PROTOCOL_VIOLATION_REPORTED	(1 << 15)
+#define  ICL_INVALID_TX_LENGTH			(1 << 13)
+#define  ICL_INVALID_VC				(1 << 12)
+#define  ICL_INVALID_DATA_TYPE			(1 << 11)
+#define  ICL_PERIPHERAL_CHKSUM_ERROR		(1 << 10)
+#define  ICL_PERIPHERAL_MULTI_ECC_ERROR		(1 << 9)
+#define  ICL_PERIPHERAL_SINGLE_ECC_ERROR	(1 << 8)
+#define  ICL_PERIPHERAL_CONTENTION_DETECTED	(1 << 7)
+#define  ICL_PERIPHERAL_FALSE_CTRL_ERROR	(1 << 6)
+#define  ICL_PERIPHERAL_TIMEOUT_ERROR		(1 << 5)
+#define  ICL_PERIPHERAL_LP_TX_SYNC_ERROR	(1 << 4)
+#define  ICL_PERIPHERAL_ESC_MODE_ENTRY_CMD_ERROR	(1 << 3)
+#define  ICL_EOT_SYNC_ERROR			(1 << 2)
+#define  ICL_SOT_SYNC_ERROR			(1 << 1)
+#define  ICL_SOT_ERROR				(1 << 0)
+
 /* Gen4+ Timestamp and Pipe Frame time stamp registers */
 #define GEN4_TIMESTAMP		_MMIO(0x2358)
 #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
@@ -11263,6 +11322,7 @@ enum skl_power_gate {
 #define  CMD_MODE_TE_GATE		(0x1 << 28)
 #define  VIDEO_MODE_SYNC_EVENT		(0x2 << 28)
 #define  VIDEO_MODE_SYNC_PULSE		(0x3 << 28)
+#define  TE_SOURCE_GPIO			(1 << 27)
 #define  LINK_READY			(1 << 20)
 #define  PIX_FMT_MASK			(0x3 << 16)
 #define  PIX_FMT_SHIFT			16
-- 
2.21.0.5.gaeb582a

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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode.
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
  2019-10-14 11:01 ` [RFC 1/7] drm/i915/dsi: Define command mode registers Vandita Kulkarni
@ 2019-10-14 11:01 ` Vandita Kulkarni
  2019-10-15 18:35   ` Jani Nikula
  2019-10-24 11:37     ` [Intel-gfx] " Jani Nikula
  2019-10-14 11:01 ` [RFC 3/7] drm/i915/dsi: Add vblank calculation " Vandita Kulkarni
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 36+ messages in thread
From: Vandita Kulkarni @ 2019-10-14 11:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Configure the transcoder to operate in TE GATE command mode
and  take TE events from GPIO.
Also disable the periodic command mode, that GOP would have
programmed.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/display/icl_dsi.c | 32 ++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 6e398c33a524..8e6c09a1db78 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -704,6 +704,10 @@ gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
 				tmp |= VIDEO_MODE_SYNC_PULSE;
 				break;
 			}
+		} else {
+			tmp &= ~OP_MODE_MASK;
+			tmp |= CMD_MODE_TE_GATE;
+			tmp |= TE_SOURCE_GPIO;
 		}
 
 		I915_WRITE(DSI_TRANS_FUNC_CONF(dsi_trans), tmp);
@@ -953,6 +957,22 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder)
 	}
 }
 
+static void gen11_dsi_config_util_pin(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	u32 tmp;
+
+	/* used only as TE i/p for DSI0 for dual link TE is from slave DSI1 */
+	if (is_vid_mode(intel_dsi) || (intel_dsi->dual_link))
+		return;
+
+	tmp = I915_READ(UTIL_PIN_CTL);
+	tmp |= ICL_UTIL_PIN_DIRECTION;
+	tmp |= UTIL_PIN_ENABLE;
+	I915_WRITE(UTIL_PIN_CTL, tmp);
+}
+
 static void
 gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
 			      const struct intel_crtc_state *pipe_config)
@@ -974,6 +994,9 @@ gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
 	/* setup D-PHY timings */
 	gen11_dsi_setup_dphy_timings(encoder);
 
+	/* Since transcoder is configured to take events from GPIO */
+	gen11_dsi_config_util_pin(encoder);
+
 	/* step 4h: setup DSI protocol timeouts */
 	gen11_dsi_setup_timeouts(encoder);
 
@@ -1104,6 +1127,15 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
 	enum transcoder dsi_trans;
 	u32 tmp;
 
+	/* disable periodic update mode */
+	if (is_cmd_mode(intel_dsi)) {
+		for_each_dsi_port(port, intel_dsi->ports) {
+			tmp = I915_READ(ICL_DSI_CMD_FRMCTL(port));
+			tmp &= ~ICL_PERIODIC_FRAME_UPDATE_ENABLE;
+			I915_WRITE(ICL_DSI_CMD_FRMCTL(port), tmp);
+		}
+	}
+
 	/* put dsi link in ULPS */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		dsi_trans = dsi_port_to_transcoder(port);
-- 
2.21.0.5.gaeb582a

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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [RFC 3/7] drm/i915/dsi: Add vblank calculation for command mode
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
  2019-10-14 11:01 ` [RFC 1/7] drm/i915/dsi: Define command mode registers Vandita Kulkarni
  2019-10-14 11:01 ` [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode Vandita Kulkarni
@ 2019-10-14 11:01 ` Vandita Kulkarni
  2019-10-15 18:45   ` Jani Nikula
  2019-10-14 11:01 ` [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode Vandita Kulkarni
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 36+ messages in thread
From: Vandita Kulkarni @ 2019-10-14 11:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Transcoder timing calculation differ for command mode.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/display/icl_dsi.c | 56 +++++++++++++++++---------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 8e6c09a1db78..5dd9eebab6b1 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -780,6 +780,7 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 	u16 hback_porch;
 	/* vertical timings */
 	u16 vtotal, vactive, vsync_start, vsync_end, vsync_shift;
+	int bpp, line_time_us, byte_clk_period_ns;
 
 	hactive = adjusted_mode->crtc_hdisplay;
 	htotal = adjusted_mode->crtc_htotal;
@@ -841,40 +842,57 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 	}
 
 	/* program TRANS_VTOTAL register */
-	for_each_dsi_port(port, intel_dsi->ports) {
-		dsi_trans = dsi_port_to_transcoder(port);
-		/*
-		 * FIXME: Programing this by assuming progressive mode, since
-		 * non-interlaced info from VBT is not saved inside
-		 * struct drm_display_mode.
-		 * For interlace mode: program required pixel minus 2
-		 */
-		I915_WRITE(VTOTAL(dsi_trans),
-			   (vactive - 1) | ((vtotal - 1) << 16));
+	if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {
+		for_each_dsi_port(port, intel_dsi->ports) {
+			dsi_trans = dsi_port_to_transcoder(port);
+			/*
+			 * FIXME: Programing this by assuming progressive mode,
+			 * since non-interlaced info from VBT is not saved
+			 * inside struct drm_display_mode.
+			 * For interlace mode: program required pixel minus 2
+			 */
+			I915_WRITE(VTOTAL(dsi_trans),
+				   (vactive - 1) | ((vtotal - 1) << 16));
+		}
+	} else {
+		for_each_dsi_port(port, intel_dsi->ports) {
+			bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
+			byte_clk_period_ns = 8 * 1000000 / intel_dsi->pclk;
+			htotal = hactive + 160;
+			line_time_us = (htotal * (bpp / 8) * byte_clk_period_ns) / (1000 * intel_dsi->lane_count);
+			vtotal = vactive + DIV_ROUND_UP(460, line_time_us);
+			I915_WRITE(VTOTAL(dsi_trans),
+				   (vactive - 1) | ((vtotal - 1) << 16));
+		}
 	}
 
+
 	if (vsync_end < vsync_start || vsync_end > vtotal)
 		DRM_ERROR("Invalid vsync_end value\n");
 
 	if (vsync_start < vactive)
 		DRM_ERROR("vsync_start less than vactive\n");
 
-	/* program TRANS_VSYNC register */
-	for_each_dsi_port(port, intel_dsi->ports) {
-		dsi_trans = dsi_port_to_transcoder(port);
-		I915_WRITE(VSYNC(dsi_trans),
-			   (vsync_start - 1) | ((vsync_end - 1) << 16));
+	/* program TRANS_VSYNC register for video mode only */
+	if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {
+		for_each_dsi_port(port, intel_dsi->ports) {
+			dsi_trans = dsi_port_to_transcoder(port);
+			I915_WRITE(VSYNC(dsi_trans),
+				   (vsync_start - 1) | ((vsync_end - 1) << 16));
+		}
 	}
 
 	/*
-	 * FIXME: It has to be programmed only for interlaced
+	 * FIXME: It has to be programmed only for video modes and interlaced
 	 * modes. Put the check condition here once interlaced
 	 * info available as described above.
 	 * program TRANS_VSYNCSHIFT register
 	 */
-	for_each_dsi_port(port, intel_dsi->ports) {
-		dsi_trans = dsi_port_to_transcoder(port);
-		I915_WRITE(VSYNCSHIFT(dsi_trans), vsync_shift);
+	if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {
+		for_each_dsi_port(port, intel_dsi->ports) {
+			dsi_trans = dsi_port_to_transcoder(port);
+			I915_WRITE(VSYNCSHIFT(dsi_trans), vsync_shift);
+		}
 	}
 
 	/* program TRANS_VBLANK register, should be same as vtotal programmed */
-- 
2.21.0.5.gaeb582a

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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
                   ` (2 preceding siblings ...)
  2019-10-14 11:01 ` [RFC 3/7] drm/i915/dsi: Add vblank calculation " Vandita Kulkarni
@ 2019-10-14 11:01 ` Vandita Kulkarni
  2019-10-15 19:20   ` Jani Nikula
  2019-10-14 11:01 ` [RFC 5/7] drm/i915/dsi: Configure TE interrupt for " Vandita Kulkarni
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 36+ messages in thread
From: Vandita Kulkarni @ 2019-10-14 11:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

From: Madhav Chauhan <madhav.chauhan@intel.com>

This patch adds a helper function to find encoder
if DSI is operating in command mode. This function
will be used while enabling/disabling TE interrupts
for DSI.

Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/display/icl_dsi.c   | 17 +++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dsi.h |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 5dd9eebab6b1..877746416e52 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -73,6 +73,23 @@ static enum transcoder dsi_port_to_transcoder(enum port port)
 		return TRANSCODER_DSI_1;
 }
 
+struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct intel_encoder *encoder;
+	struct intel_dsi *intel_dsi;
+
+	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+		if (encoder->type != INTEL_OUTPUT_DSI)
+			continue;
+		intel_dsi = enc_to_intel_dsi(&encoder->base);
+		if (intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE)
+			return encoder;
+	}
+
+	return NULL;
+}
+
 static void wait_for_cmds_dispatched_to_panel(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h b/drivers/gpu/drm/i915/display/intel_dsi.h
index b15be5814599..071dad7ee04a 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.h
+++ b/drivers/gpu/drm/i915/display/intel_dsi.h
@@ -201,6 +201,9 @@ u32 bxt_dsi_get_pclk(struct intel_encoder *encoder,
 		     struct intel_crtc_state *config);
 void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
 
+/* icl_dsi.c */
+struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc);
+
 /* intel_dsi_vbt.c */
 bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
 void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
-- 
2.21.0.5.gaeb582a

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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [RFC 5/7] drm/i915/dsi: Configure TE interrupt for cmd mode
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
                   ` (3 preceding siblings ...)
  2019-10-14 11:01 ` [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode Vandita Kulkarni
@ 2019-10-14 11:01 ` Vandita Kulkarni
  2019-10-16  9:56   ` Ramalingam C
  2019-10-24 11:34     ` [Intel-gfx] " Jani Nikula
  2019-10-14 11:01 ` [RFC 6/7] drm/i915/dsi: Add TE handler for dsi " Vandita Kulkarni
                   ` (5 subsequent siblings)
  10 siblings, 2 replies; 36+ messages in thread
From: Vandita Kulkarni @ 2019-10-14 11:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

We need to configure TE interrupt in two places.
Port interrupt and DSI interrupt mask registers.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 49 ++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 3af7f7914c40..bfb2a63504fb 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -41,6 +41,7 @@
 #include "display/intel_hotplug.h"
 #include "display/intel_lpe_audio.h"
 #include "display/intel_psr.h"
+#include "display/intel_dsi.h"
 
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_irq.h"
@@ -2960,12 +2961,44 @@ int ilk_enable_vblank(struct drm_crtc *crtc)
 	return 0;
 }
 
+static void gen11_dsi_configure_te(struct drm_crtc *crtc, bool enable)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_encoder *encoder = NULL;
+	struct intel_dsi *intel_dsi;
+	enum port port;
+	u32 tmp;
+
+	encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);
+	if (!encoder)
+		return;
+
+	intel_dsi = enc_to_intel_dsi(&encoder->base);
+	/* Assuming single link would always be enabled on PORT_A */
+	port = (intel_dsi->ports & BIT(PORT_B) & BIT(PORT_A)) ? PORT_B : PORT_A;
+	tmp =  I915_READ(ICL_DSI_INTR_MASK_REG(port));
+	if (enable)
+		tmp &= ~ICL_TE_EVENT;
+	else
+		tmp |= ICL_TE_EVENT;
+
+	I915_WRITE(ICL_DSI_INTR_MASK_REG(port), tmp);
+}
+
 int bdw_enable_vblank(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum pipe pipe = to_intel_crtc(crtc)->pipe;
 	unsigned long irqflags;
 
+	if (INTEL_GEN(dev_priv) >= 11 &&
+		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {
+		gen11_dsi_configure_te(crtc, true);
+		return 0;
+	}
+
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 	bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
@@ -3031,9 +3064,16 @@ void ilk_disable_vblank(struct drm_crtc *crtc)
 void bdw_disable_vblank(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
-	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	enum pipe pipe = intel_crtc->pipe;
 	unsigned long irqflags;
 
+	if (INTEL_GEN(dev_priv) >= 11 &&
+		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {
+		gen11_dsi_configure_te(crtc, false);
+		return;
+	}
+
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 	bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
@@ -3726,6 +3766,13 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 		gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
 	}
 
+	if (INTEL_GEN(dev_priv) >= 11) {
+		enum port port;
+
+		if (intel_bios_is_dsi_present(dev_priv, &port))
+			de_port_masked |= ICL_DSI_0 | ICL_DSI_1;
+	}
+
 	for_each_pipe(dev_priv, pipe) {
 		dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
 
-- 
2.21.0.5.gaeb582a

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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [RFC 6/7] drm/i915/dsi: Add TE handler for dsi cmd mode.
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
                   ` (4 preceding siblings ...)
  2019-10-14 11:01 ` [RFC 5/7] drm/i915/dsi: Configure TE interrupt for " Vandita Kulkarni
@ 2019-10-14 11:01 ` Vandita Kulkarni
  2019-10-15  8:28   ` Kulkarni, Vandita
  2019-10-16 10:24   ` Ramalingam C
  2019-10-14 11:01 ` [RFC 7/7] drm/i915/dsi: Initiate frame request in " Vandita Kulkarni
                   ` (4 subsequent siblings)
  10 siblings, 2 replies; 36+ messages in thread
From: Vandita Kulkarni @ 2019-10-14 11:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

In case of dual link, we get the TE on slave.
So clear the TE on slave DSI IIR.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 61 +++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bfb2a63504fb..d12efa72943b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2628,6 +2628,61 @@ gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32 iir)
 		DRM_ERROR("Unexpected DE Misc interrupt\n");
 }
 
+void gen11_dsi_te_interrupt_handler(struct drm_i915_private *dev_priv,
+				    u32 iir_value)
+{
+	enum pipe pipe = INVALID_PIPE;
+	enum port port;
+	enum transcoder dsi_trans;
+	u32 val;
+
+	/*
+	 * Incase of dual link, TE comes from DSI_1
+	 * this is to check if dual link is enabled
+	 */
+	val = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
+	val &= PORT_SYNC_MODE_ENABLE;
+
+	/*
+	 * if dual link is enabled, then read DSI_0
+	 * transcoder registers
+	 */
+	port = ((iir_value & ICL_DSI_1) && val) || (iir_value & ICL_DSI_0) ? PORT_A : PORT_B;
+	dsi_trans = (port == PORT_A) ? TRANSCODER_DSI_0 : TRANSCODER_DSI_1;
+
+	/* Check if DSI configured in command mode */
+	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
+	val = (val & OP_MODE_MASK) >> 28;
+
+	if (val) {
+		DRM_ERROR("DSI trancoder not configured in command mode\n");
+		return;
+	}
+
+	/* Get PIPE for handling VBLANK event */
+	val = I915_READ(TRANS_DDI_FUNC_CTL(dsi_trans));
+	switch (val & TRANS_DDI_EDP_INPUT_MASK) {
+	case TRANS_DDI_EDP_INPUT_A_ON:
+		pipe = PIPE_A;
+		break;
+	case TRANS_DDI_EDP_INPUT_B_ONOFF:
+		pipe = PIPE_B;
+		break;
+	case TRANS_DDI_EDP_INPUT_C_ONOFF:
+		pipe = PIPE_C;
+		break;
+	default:
+		DRM_ERROR("Invalid PIPE\n");
+	}
+
+	/* clear TE in dsi IIR */
+	port = (iir_value & ICL_DSI_1) ? PORT_B : PORT_A;
+	val = I915_READ(ICL_DSI_INTR_IDENT_REG(port));
+	I915_WRITE((ICL_DSI_INTR_IDENT_REG(port)), val);
+
+	drm_handle_vblank(&dev_priv->drm, pipe);
+}
+
 static irqreturn_t
 gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 {
@@ -2692,6 +2747,12 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 				found = true;
 			}
 
+			if ((INTEL_GEN(dev_priv) >= 11) &&
+				(iir & (ICL_DSI_0 | ICL_DSI_1))) {
+				gen11_dsi_te_interrupt_handler(dev_priv, iir);
+				found = true;
+			}
+
 			if (!found)
 				DRM_ERROR("Unexpected DE Port interrupt\n");
 		}
-- 
2.21.0.5.gaeb582a

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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [RFC 7/7] drm/i915/dsi: Initiate frame request in cmd mode
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
                   ` (5 preceding siblings ...)
  2019-10-14 11:01 ` [RFC 6/7] drm/i915/dsi: Add TE handler for dsi " Vandita Kulkarni
@ 2019-10-14 11:01 ` Vandita Kulkarni
  2019-10-16 10:14   ` Ramalingam C
  2019-10-14 16:21 ` ✗ Fi.CI.CHECKPATCH: warning for Add mipi dsi command mode support Patchwork
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 36+ messages in thread
From: Vandita Kulkarni @ 2019-10-14 11:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

In TE Gate mode, on every flip we need to set the
frame update request bit. After this  bit is set
transcoder hardware will automatically send the
frame data to the panel when it receives the TE event.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/display/icl_dsi.c        | 27 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_display.c  | 16 +++++++++++
 .../drm/i915/display/intel_display_types.h    |  3 +++
 drivers/gpu/drm/i915/display/intel_dsi.h      |  3 +++
 4 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index 877746416e52..c72917ddf8e7 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -90,6 +90,33 @@ struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc)
 	return NULL;
 }
 
+void gen11_dsi_check_frame_update_needed(struct intel_crtc *intel_crtc,
+					 struct intel_crtc_state *crtc_state)
+{
+	struct intel_encoder *intel_encoder = NULL;
+
+	intel_encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);
+	if (!intel_encoder)
+		return;
+
+	/* TBD Use bits  to say update on which  dsi port instead of a bool */
+	crtc_state->dsi_frame_update = true;
+}
+
+void gen11_dsi_frame_update(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 tmp;
+
+	/* TBD: add check on port */
+	if (crtc_state->dsi_frame_update) {
+		tmp = I915_READ(ICL_DSI_CMD_FRMCTL(PORT_A));
+		tmp |= ICL_FRAME_UPDATE_REQUEST;
+		I915_WRITE(ICL_DSI_CMD_FRMCTL(PORT_A), tmp);
+	}
+}
+
 static void wait_for_cmds_dispatched_to_panel(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3cf39fc153b3..a902bb2bf075 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -11858,6 +11858,11 @@ static int intel_crtc_atomic_check(struct drm_crtc *_crtc,
 							 crtc_state);
 	}
 
+	if ((INTEL_GEN(dev_priv) >= 11) &&
+		(intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))) {
+		gen11_dsi_check_frame_update_needed(crtc, crtc_state);
+	}
+
 	if (HAS_IPS(dev_priv))
 		crtc_state->ips_enabled = hsw_compute_ips_config(crtc_state);
 
@@ -13618,6 +13623,7 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
 		if (!needs_modeset(new_crtc_state) &&
@@ -14108,6 +14114,16 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 			intel_color_load_luts(new_crtc_state);
 	}
 
+	/*
+	 * Incase of mipi dsi command mode, we need to set frame update
+	 * for every commit
+	 */
+	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+		if (new_crtc_state->base.active &&
+		    !needs_modeset(new_crtc_state) &&
+		    new_crtc_state->dsi_frame_update)
+			gen11_dsi_frame_update(new_crtc_state);
+	}
 	/*
 	 * Now that the vblank has passed, we can go ahead and program the
 	 * optimal watermarks on platforms that need two-step watermark
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 40390d855815..69da4ec45691 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -991,6 +991,9 @@ struct intel_crtc_state {
 
 	/* Forward Error correction State */
 	bool fec_enable;
+
+	/* frame update for dsi command mode */
+	bool dsi_frame_update;
 };
 
 struct intel_crtc {
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h b/drivers/gpu/drm/i915/display/intel_dsi.h
index 071dad7ee04a..d9350b842115 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.h
+++ b/drivers/gpu/drm/i915/display/intel_dsi.h
@@ -203,6 +203,9 @@ void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
 
 /* icl_dsi.c */
 struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc);
+void gen11_dsi_check_frame_update_needed(struct intel_crtc *crtc,
+					 struct intel_crtc_state *crtc_state);
+void gen11_dsi_frame_update(struct intel_crtc_state *crtc_state);
 
 /* intel_dsi_vbt.c */
 bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
-- 
2.21.0.5.gaeb582a

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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [RFC 1/7] drm/i915/dsi: Define command mode registers
  2019-10-14 11:01 ` [RFC 1/7] drm/i915/dsi: Define command mode registers Vandita Kulkarni
@ 2019-10-14 16:18   ` Ramalingam C
  2019-10-15  7:52     ` Kulkarni, Vandita
  2019-10-15  7:07   ` Jani Nikula
  1 sibling, 1 reply; 36+ messages in thread
From: Ramalingam C @ 2019-10-14 16:18 UTC (permalink / raw)
  To: Vandita Kulkarni; +Cc: jani.nikula, intel-gfx

On 2019-10-14 at 16:31:16 +0530, Vandita Kulkarni wrote:
> Adding all the register definitions needed
> for mipi dsi command mode.
> 
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 76 +++++++++++++++++++++++++++++----
>  1 file changed, 68 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e24991e54897..73bc85855b79 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4919,14 +4919,20 @@ enum {
>  #define   BLM_PCH_POLARITY			(1 << 29)
>  #define BLC_PWM_PCH_CTL2	_MMIO(0xc8254)
>  
> -#define UTIL_PIN_CTL		_MMIO(0x48400)
> -#define   UTIL_PIN_ENABLE	(1 << 31)
> -
> -#define   UTIL_PIN_PIPE(x)     ((x) << 29)
> -#define   UTIL_PIN_PIPE_MASK   (3 << 29)
> -#define   UTIL_PIN_MODE_PWM    (1 << 24)
> -#define   UTIL_PIN_MODE_MASK   (0xf << 24)
> -#define   UTIL_PIN_POLARITY    (1 << 22)
> +#define UTIL_PIN_CTL			_MMIO(0x48400)
> +#define  UTIL_PIN_ENABLE		(1 << 31)
> +#define  UTIL_PIN_PIPE_MASK		(3 << 29)
> +#define  UTIL_PIN_PIPE(x)		((x) << 29)
> +#define  UTIL_PIN_MODE_MASK		(0xf << 24)
> +#define  UTIL_PIN_MODE_DATA		(0 << 24)
> +#define  UTIL_PIN_MODE_PWM		(1 << 24)
> +#define  UTIL_PIN_MODE_VBLANK		(4 << 24)
> +#define  UTIL_PIN_MODE_VSYNC		(5 << 24)
> +#define  UTIL_PIN_MODE_EYE_LEVEL	(8 << 24)
> +#define  UTIL_PIN_OP_DATA		(1 << 23)
> +#define  UTIL_PIN_POLARITY		(1 << 22)
> +#define  ICL_UTIL_PIN_DIRECTION		(1 << 19)
> +#define  ICL_UTIL_PIN_IP_DATA		(1 << 16)
Could we drop this ICL prefix to align with existing bit definitions?
>  
>  /* BXT backlight register definition. */
>  #define _BXT_BLC_PWM_CTL1			0xC8250
> @@ -7407,6 +7413,8 @@ enum {
>  #define GEN8_DE_PORT_IMR _MMIO(0x44444)
>  #define GEN8_DE_PORT_IIR _MMIO(0x44448)
>  #define GEN8_DE_PORT_IER _MMIO(0x4444c)
> +#define  ICL_DSI_1			(1 << 31)
> +#define  ICL_DSI_0			(1 << 30)
>  #define  ICL_AUX_CHANNEL_E		(1 << 29)
>  #define  CNL_AUX_CHANNEL_F		(1 << 28)
>  #define  GEN9_AUX_CHANNEL_D		(1 << 27)
> @@ -10659,6 +10667,57 @@ enum skl_power_gate {
>  #define  ICL_ESC_CLK_DIV_SHIFT			0
>  #define DSI_MAX_ESC_CLK			20000		/* in KHz */
>  
> +#define _ICL_DSI_CMD_FRMCTL_0		0x6b034
> +#define _ICL_DSI_CMD_FRMCTL_1		0x6b834
> +#define ICL_DSI_CMD_FRMCTL(port)	_MMIO_PORT(port,	\
> +						  _ICL_DSI_CMD_FRMCTL_0,\
> +						  _ICL_DSI_CMD_FRMCTL_1)
might want to align with first char after (.
> +#define  ICL_FRAME_UPDATE_REQUEST		(1 << 31)
> +#define  ICL_PERIODIC_FRAME_UPDATE_ENABLE	(1 << 29)
> +#define  ICL_NULL_PACKET_ENABLE			(1 << 28)
> +#define  ICL_FRAME_IN_PROGRESS			(1 << 0)
> +
> +#define _ICL_DSI_INTR_MASK_REG_0		0x6b070
> +#define _ICL_DSI_INTR_MASK_REG_1		0x6b870
> +#define ICL_DSI_INTR_MASK_REG(port)	_MMIO_PORT(port,	\
> +						  _ICL_DSI_INTR_MASK_REG_0,\
> +						  _ICL_DSI_INTR_MASK_REG_1)
same here. and all below lines...
> +
> +#define _ICL_DSI_INTR_IDENT_REG_0		0x6b074
> +#define _ICL_DSI_INTR_IDENT_REG_1		0x6b874
> +#define ICL_DSI_INTR_IDENT_REG(port)	_MMIO_PORT(port,	\
> +						  _ICL_DSI_INTR_IDENT_REG_0,\
> +						  _ICL_DSI_INTR_IDENT_REG_1)
> +#define  ICL_TE_EVENT				(1 << 31)
> +#define  ICL_RX_DATA_OR_BTA_TERMINATED		(1 << 30)
> +#define  ICL_TX_DATA				(1 << 29)
> +#define  ICL_ULPS_ENTRY_DONE			(1 << 28)
> +#define  ICL_NON_TE_TRIGGER_RECEIVED		(1 << 27)
> +#define  ICL_HOST_CHKSUM_ERROR			(1 << 26)
> +#define  ICL_HOST_MULTI_ECC_ERROR		(1 << 25)
> +#define  ICL_HOST_SINGL_ECC_ERROR		(1 << 24)
> +#define  ICL_HOST_CONTENTION_DETECTED		(1 << 23)
> +#define  ICL_HOST_FALSE_CONTROL_ERROR		(1 << 22)
> +#define  ICL_HOST_TIMEOUT_ERROR			(1 << 21)
> +#define  ICL_HOST_LOW_POWER_TX_SYNC_ERROR	(1 << 20)
> +#define  ICL_HOST_ESCAPE_MODE_ENTRY_ERROR	(1 << 19)
> +#define  ICL_FRAME_UPDATE_DONE			(1 << 16)
> +#define  ICL_PROTOCOL_VIOLATION_REPORTED	(1 << 15)
> +#define  ICL_INVALID_TX_LENGTH			(1 << 13)
> +#define  ICL_INVALID_VC				(1 << 12)
> +#define  ICL_INVALID_DATA_TYPE			(1 << 11)
> +#define  ICL_PERIPHERAL_CHKSUM_ERROR		(1 << 10)
> +#define  ICL_PERIPHERAL_MULTI_ECC_ERROR		(1 << 9)
> +#define  ICL_PERIPHERAL_SINGLE_ECC_ERROR	(1 << 8)
> +#define  ICL_PERIPHERAL_CONTENTION_DETECTED	(1 << 7)
> +#define  ICL_PERIPHERAL_FALSE_CTRL_ERROR	(1 << 6)
> +#define  ICL_PERIPHERAL_TIMEOUT_ERROR		(1 << 5)
> +#define  ICL_PERIPHERAL_LP_TX_SYNC_ERROR	(1 << 4)
> +#define  ICL_PERIPHERAL_ESC_MODE_ENTRY_CMD_ERROR	(1 << 3)
Too lengthy!? Could we use short form of this macro name?

-Ram
> +#define  ICL_EOT_SYNC_ERROR			(1 << 2)
> +#define  ICL_SOT_SYNC_ERROR			(1 << 1)
> +#define  ICL_SOT_ERROR				(1 << 0)
> +
>  /* Gen4+ Timestamp and Pipe Frame time stamp registers */
>  #define GEN4_TIMESTAMP		_MMIO(0x2358)
>  #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
> @@ -11263,6 +11322,7 @@ enum skl_power_gate {
>  #define  CMD_MODE_TE_GATE		(0x1 << 28)
>  #define  VIDEO_MODE_SYNC_EVENT		(0x2 << 28)
>  #define  VIDEO_MODE_SYNC_PULSE		(0x3 << 28)
> +#define  TE_SOURCE_GPIO			(1 << 27)
>  #define  LINK_READY			(1 << 20)
>  #define  PIX_FMT_MASK			(0x3 << 16)
>  #define  PIX_FMT_SHIFT			16
> -- 
> 2.21.0.5.gaeb582a
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for Add mipi dsi command mode support.
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
                   ` (6 preceding siblings ...)
  2019-10-14 11:01 ` [RFC 7/7] drm/i915/dsi: Initiate frame request in " Vandita Kulkarni
@ 2019-10-14 16:21 ` Patchwork
  2019-10-14 16:24 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2019-10-14 16:21 UTC (permalink / raw)
  To: Vandita Kulkarni; +Cc: intel-gfx

== Series Details ==

Series: Add mipi dsi command mode support.
URL   : https://patchwork.freedesktop.org/series/67974/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
83c4c64b055f drm/i915/dsi: Define command mode registers
900d9ba2700a drm/i915/dsi: Configure transcoder operation for command mode.
-:40: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'intel_dsi->dual_link'
#40: FILE: drivers/gpu/drm/i915/display/icl_dsi.c:967:
+	if (is_vid_mode(intel_dsi) || (intel_dsi->dual_link))

total: 0 errors, 0 warnings, 1 checks, 56 lines checked
c778fa65e35c drm/i915/dsi: Add vblank calculation for command mode
-:53: WARNING:LONG_LINE: line over 100 characters
#53: FILE: drivers/gpu/drm/i915/display/icl_dsi.c:862:
+			line_time_us = (htotal * (bpp / 8) * byte_clk_period_ns) / (1000 * intel_dsi->lane_count);

-:60: CHECK:LINE_SPACING: Please don't use multiple blank lines
#60: FILE: drivers/gpu/drm/i915/display/icl_dsi.c:869:
 
+

total: 0 errors, 1 warnings, 1 checks, 83 lines checked
91f92b97148b drm/i915/dsi: Helper to find dsi encoder in cmd mode
7a4fc89a244f drm/i915/dsi: Configure TE interrupt for cmd mode
-:60: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#60: FILE: drivers/gpu/drm/i915/i915_irq.c:2997:
+	if (INTEL_GEN(dev_priv) >= 11 &&
+		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {

-:78: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#78: FILE: drivers/gpu/drm/i915/i915_irq.c:3072:
+	if (INTEL_GEN(dev_priv) >= 11 &&
+		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {

total: 0 errors, 0 warnings, 2 checks, 81 lines checked
e600d6ff8306 drm/i915/dsi: Add TE handler for dsi cmd mode.
-:82: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#82: FILE: drivers/gpu/drm/i915/i915_irq.c:2751:
+			if ((INTEL_GEN(dev_priv) >= 11) &&
+				(iir & (ICL_DSI_0 | ICL_DSI_1))) {

total: 0 errors, 0 warnings, 1 checks, 73 lines checked
db7de625fd49 drm/i915/dsi: Initiate frame request in cmd mode
-:60: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#60: FILE: drivers/gpu/drm/i915/display/intel_display.c:11862:
+	if ((INTEL_GEN(dev_priv) >= 11) &&
+		(intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))) {

-:71: CHECK:LINE_SPACING: Please don't use multiple blank lines
#71: FILE: drivers/gpu/drm/i915/display/intel_display.c:13626:
 
+

total: 0 errors, 0 warnings, 2 checks, 85 lines checked

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* ✗ Fi.CI.SPARSE: warning for Add mipi dsi command mode support.
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
                   ` (7 preceding siblings ...)
  2019-10-14 16:21 ` ✗ Fi.CI.CHECKPATCH: warning for Add mipi dsi command mode support Patchwork
@ 2019-10-14 16:24 ` Patchwork
  2019-10-14 17:07 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-10-15  0:42 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2019-10-14 16:24 UTC (permalink / raw)
  To: Vandita Kulkarni; +Cc: intel-gfx

== Series Details ==

Series: Add mipi dsi command mode support.
URL   : https://patchwork.freedesktop.org/series/67974/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915/dsi: Define command mode registers
Okay!

Commit: drm/i915/dsi: Configure transcoder operation for command mode.
Okay!

Commit: drm/i915/dsi: Add vblank calculation for command mode
Okay!

Commit: drm/i915/dsi: Helper to find dsi encoder in cmd mode
Okay!

Commit: drm/i915/dsi: Configure TE interrupt for cmd mode
Okay!

Commit: drm/i915/dsi: Add TE handler for dsi cmd mode.
-
+drivers/gpu/drm/i915/i915_irq.c:2631:6: warning: symbol 'gen11_dsi_te_interrupt_handler' was not declared. Should it be static?

Commit: drm/i915/dsi: Initiate frame request in cmd mode
Okay!

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* ✓ Fi.CI.BAT: success for Add mipi dsi command mode support.
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
                   ` (8 preceding siblings ...)
  2019-10-14 16:24 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-10-14 17:07 ` Patchwork
  2019-10-15  0:42 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2019-10-14 17:07 UTC (permalink / raw)
  To: Vandita Kulkarni; +Cc: intel-gfx

== Series Details ==

Series: Add mipi dsi command mode support.
URL   : https://patchwork.freedesktop.org/series/67974/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7086 -> Patchwork_14792
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/index.html

Known issues
------------

  Here are the changes found in Patchwork_14792 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-bxt-dsi:         [PASS][1] -> [INCOMPLETE][2] ([fdo#103927])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html

  * igt@gem_mmap_gtt@basic-write-read-distinct:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/fi-icl-u3/igt@gem_mmap_gtt@basic-write-read-distinct.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/fi-icl-u3/igt@gem_mmap_gtt@basic-write-read-distinct.html

  * igt@i915_selftest@live_hangcheck:
    - fi-hsw-4770r:       [PASS][5] -> [DMESG-FAIL][6] ([fdo#111991])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/fi-hsw-4770r/igt@i915_selftest@live_hangcheck.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][7] -> [FAIL][8] ([fdo#111407])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@gem_flink_basic@flink-lifetime:
    - fi-icl-u3:          [DMESG-WARN][9] ([fdo#107724]) -> [PASS][10] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       [INCOMPLETE][11] ([fdo#107718]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/fi-blb-e6850/igt@i915_module_load@reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/fi-blb-e6850/igt@i915_module_load@reload.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
  [fdo#111736]: https://bugs.freedesktop.org/show_bug.cgi?id=111736
  [fdo#111991]: https://bugs.freedesktop.org/show_bug.cgi?id=111991


Participating hosts (52 -> 46)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7086 -> Patchwork_14792

  CI-20190529: 20190529
  CI_DRM_7086: cb64ca78fe223a03a902aa10ddbfb672ccad1630 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5225: 991ce4eede1c52f76378aebf162a13c20d6f6293 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14792: db7de625fd49b6e33788b8f84c5ae54204e43dcd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

db7de625fd49 drm/i915/dsi: Initiate frame request in cmd mode
e600d6ff8306 drm/i915/dsi: Add TE handler for dsi cmd mode.
7a4fc89a244f drm/i915/dsi: Configure TE interrupt for cmd mode
91f92b97148b drm/i915/dsi: Helper to find dsi encoder in cmd mode
c778fa65e35c drm/i915/dsi: Add vblank calculation for command mode
900d9ba2700a drm/i915/dsi: Configure transcoder operation for command mode.
83c4c64b055f drm/i915/dsi: Define command mode registers

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* ✓ Fi.CI.IGT: success for Add mipi dsi command mode support.
  2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
                   ` (9 preceding siblings ...)
  2019-10-14 17:07 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-10-15  0:42 ` Patchwork
  10 siblings, 0 replies; 36+ messages in thread
From: Patchwork @ 2019-10-15  0:42 UTC (permalink / raw)
  To: Vandita Kulkarni; +Cc: intel-gfx

== Series Details ==

Series: Add mipi dsi command mode support.
URL   : https://patchwork.freedesktop.org/series/67974/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7086_full -> Patchwork_14792_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14792_full:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@i915_pm_dc@dc6-dpms}:
    - {shard-tglb}:       NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-tglb1/igt@i915_pm_dc@dc6-dpms.html

  
Known issues
------------

  Here are the changes found in Patchwork_14792_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vecs0-s3:
    - shard-skl:          [PASS][2] -> [INCOMPLETE][3] ([fdo#104108])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-skl5/igt@gem_ctx_isolation@vecs0-s3.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-skl4/igt@gem_ctx_isolation@vecs0-s3.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][4] -> [SKIP][5] ([fdo#110841])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [PASS][6] -> [SKIP][7] ([fdo#111325]) +4 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb8/igt@gem_exec_async@concurrent-writes-bsd.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb4/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-glk:          [PASS][8] -> [INCOMPLETE][9] ([fdo#103359] / [fdo#108686] / [k.org#198133])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-glk8/igt@gem_tiled_swapping@non-threaded.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-glk9/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_userptr_blits@dmabuf-unsync:
    - shard-snb:          [PASS][10] -> [DMESG-WARN][11] ([fdo#111870])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-snb7/igt@gem_userptr_blits@dmabuf-unsync.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-snb5/igt@gem_userptr_blits@dmabuf-unsync.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
    - shard-hsw:          [PASS][12] -> [DMESG-WARN][13] ([fdo#111870])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-hsw5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-hsw8/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-skl:          [PASS][14] -> [INCOMPLETE][15] ([fdo#104108] / [fdo#107807])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-skl7/igt@i915_pm_rpm@system-suspend-execbuf.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-skl7/igt@i915_pm_rpm@system-suspend-execbuf.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][16] -> [FAIL][17] ([fdo#103167]) +6 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_plane@plane-position-hole-dpms-pipe-c-planes:
    - shard-iclb:         [PASS][18] -> [INCOMPLETE][19] ([fdo#107713])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb2/igt@kms_plane@plane-position-hole-dpms-pipe-c-planes.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb7/igt@kms_plane@plane-position-hole-dpms-pipe-c-planes.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [PASS][20] -> [SKIP][21] ([fdo#109441]) +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb7/igt@kms_psr@psr2_cursor_blt.html

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-apl:          [PASS][22] -> [DMESG-WARN][23] ([fdo#108566]) +3 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-apl1/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-apl5/igt@kms_vblank@pipe-b-ts-continuation-suspend.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [PASS][24] -> [SKIP][25] ([fdo#109276]) +8 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb2/igt@prime_vgem@fence-wait-bsd2.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb8/igt@prime_vgem@fence-wait-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-apl:          [DMESG-WARN][26] ([fdo#108566]) -> [PASS][27] +4 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-apl3/igt@gem_ctx_isolation@bcs0-s3.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-apl1/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_exec_schedule@fifo-bsd:
    - shard-iclb:         [SKIP][28] ([fdo#111325]) -> [PASS][29] +2 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb4/igt@gem_exec_schedule@fifo-bsd.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb5/igt@gem_exec_schedule@fifo-bsd.html

  * igt@gem_exec_schedule@independent-bsd2:
    - shard-iclb:         [SKIP][30] ([fdo#109276]) -> [PASS][31] +7 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb8/igt@gem_exec_schedule@independent-bsd2.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb1/igt@gem_exec_schedule@independent-bsd2.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-snb:          [DMESG-WARN][32] ([fdo#111870]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-snb2/igt@gem_userptr_blits@sync-unmap.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-snb6/igt@gem_userptr_blits@sync-unmap.html
    - shard-hsw:          [DMESG-WARN][34] ([fdo#111870]) -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-hsw7/igt@gem_userptr_blits@sync-unmap.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-hsw6/igt@gem_userptr_blits@sync-unmap.html

  * {igt@i915_pm_dc@dc6-dpms}:
    - shard-iclb:         [FAIL][36] ([fdo#110548]) -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb3/igt@i915_pm_dc@dc6-dpms.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb8/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_suspend@sysfs-reader:
    - shard-skl:          [INCOMPLETE][38] ([fdo#104108]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-skl3/igt@i915_suspend@sysfs-reader.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-skl7/igt@i915_suspend@sysfs-reader.html

  * igt@kms_busy@basic-modeset-c:
    - shard-iclb:         [INCOMPLETE][40] ([fdo#107713]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb7/igt@kms_busy@basic-modeset-c.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb5/igt@kms_busy@basic-modeset-c.html

  * igt@kms_color@pipe-a-ctm-0-25:
    - shard-skl:          [DMESG-WARN][42] ([fdo#106107]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-skl4/igt@kms_color@pipe-a-ctm-0-25.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-skl3/igt@kms_color@pipe-a-ctm-0-25.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-skl:          [INCOMPLETE][44] ([fdo#110741]) -> [PASS][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-skl5/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-skl9/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-xtiled:
    - shard-skl:          [FAIL][46] ([fdo#103184] / [fdo#103232]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-skl4/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-xtiled.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-skl3/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-xtiled.html

  * igt@kms_flip@flip-vs-fences:
    - shard-hsw:          [INCOMPLETE][48] ([fdo#103540]) -> [PASS][49] +1 similar issue
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-hsw4/igt@kms_flip@flip-vs-fences.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-hsw7/igt@kms_flip@flip-vs-fences.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-iclb:         [FAIL][50] ([fdo#103167]) -> [PASS][51] +4 similar issues
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][52] ([fdo#108145]) -> [PASS][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [SKIP][54] ([fdo#109441]) -> [PASS][55] +2 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb1/igt@kms_psr@psr2_primary_mmap_cpu.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html

  
#### Warnings ####

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         [SKIP][56] ([fdo#109441]) -> [DMESG-WARN][57] ([fdo#107724])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7086/shard-iclb1/igt@kms_psr@psr2_suspend.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/shard-iclb2/igt@kms_psr@psr2_suspend.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110548]: https://bugs.freedesktop.org/show_bug.cgi?id=110548
  [fdo#110741]: https://bugs.freedesktop.org/show_bug.cgi?id=110741
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111703]: https://bugs.freedesktop.org/show_bug.cgi?id=111703
  [fdo#111832]: https://bugs.freedesktop.org/show_bug.cgi?id=111832
  [fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7086 -> Patchwork_14792

  CI-20190529: 20190529
  CI_DRM_7086: cb64ca78fe223a03a902aa10ddbfb672ccad1630 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5225: 991ce4eede1c52f76378aebf162a13c20d6f6293 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14792: db7de625fd49b6e33788b8f84c5ae54204e43dcd @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14792/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 1/7] drm/i915/dsi: Define command mode registers
  2019-10-14 11:01 ` [RFC 1/7] drm/i915/dsi: Define command mode registers Vandita Kulkarni
  2019-10-14 16:18   ` Ramalingam C
@ 2019-10-15  7:07   ` Jani Nikula
  2019-10-15  7:50     ` Kulkarni, Vandita
  1 sibling, 1 reply; 36+ messages in thread
From: Jani Nikula @ 2019-10-15  7:07 UTC (permalink / raw)
  To: Vandita Kulkarni, intel-gfx

On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> Adding all the register definitions needed
> for mipi dsi command mode.
>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 76 +++++++++++++++++++++++++++++----
>  1 file changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e24991e54897..73bc85855b79 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4919,14 +4919,20 @@ enum {
>  #define   BLM_PCH_POLARITY			(1 << 29)
>  #define BLC_PWM_PCH_CTL2	_MMIO(0xc8254)
>  
> -#define UTIL_PIN_CTL		_MMIO(0x48400)
> -#define   UTIL_PIN_ENABLE	(1 << 31)
> -
> -#define   UTIL_PIN_PIPE(x)     ((x) << 29)
> -#define   UTIL_PIN_PIPE_MASK   (3 << 29)
> -#define   UTIL_PIN_MODE_PWM    (1 << 24)
> -#define   UTIL_PIN_MODE_MASK   (0xf << 24)
> -#define   UTIL_PIN_POLARITY    (1 << 22)
> +#define UTIL_PIN_CTL			_MMIO(0x48400)
> +#define  UTIL_PIN_ENABLE		(1 << 31)
> +#define  UTIL_PIN_PIPE_MASK		(3 << 29)
> +#define  UTIL_PIN_PIPE(x)		((x) << 29)
> +#define  UTIL_PIN_MODE_MASK		(0xf << 24)
> +#define  UTIL_PIN_MODE_DATA		(0 << 24)
> +#define  UTIL_PIN_MODE_PWM		(1 << 24)
> +#define  UTIL_PIN_MODE_VBLANK		(4 << 24)
> +#define  UTIL_PIN_MODE_VSYNC		(5 << 24)
> +#define  UTIL_PIN_MODE_EYE_LEVEL	(8 << 24)
> +#define  UTIL_PIN_OP_DATA		(1 << 23)
> +#define  UTIL_PIN_POLARITY		(1 << 22)
> +#define  ICL_UTIL_PIN_DIRECTION		(1 << 19)
> +#define  ICL_UTIL_PIN_IP_DATA		(1 << 16)
          ^^

If you change/add everything, please put three spaces there.

Agreed on dropping the ICL prefix here, mainly because the added bits
predate ICL.

On naming, OP and IP aren't from the spec and aren't
self-explanatory. I'd go with the full UTIL_PIN_OUTPUT_DATA and
UTIL_PIN_INPUT_DATA. I think the direction bit should say what the
direction is when it's set, so you don't have to look it up.




>  
>  /* BXT backlight register definition. */
>  #define _BXT_BLC_PWM_CTL1			0xC8250
> @@ -7407,6 +7413,8 @@ enum {
>  #define GEN8_DE_PORT_IMR _MMIO(0x44444)
>  #define GEN8_DE_PORT_IIR _MMIO(0x44448)
>  #define GEN8_DE_PORT_IER _MMIO(0x4444c)
> +#define  ICL_DSI_1			(1 << 31)
> +#define  ICL_DSI_0			(1 << 30)
>  #define  ICL_AUX_CHANNEL_E		(1 << 29)
>  #define  CNL_AUX_CHANNEL_F		(1 << 28)
>  #define  GEN9_AUX_CHANNEL_D		(1 << 27)
> @@ -10659,6 +10667,57 @@ enum skl_power_gate {
>  #define  ICL_ESC_CLK_DIV_SHIFT			0
>  #define DSI_MAX_ESC_CLK			20000		/* in KHz */
>  
> +#define _ICL_DSI_CMD_FRMCTL_0		0x6b034
> +#define _ICL_DSI_CMD_FRMCTL_1		0x6b834
> +#define ICL_DSI_CMD_FRMCTL(port)	_MMIO_PORT(port,	\
> +						  _ICL_DSI_CMD_FRMCTL_0,\
> +						  _ICL_DSI_CMD_FRMCTL_1)
> +#define  ICL_FRAME_UPDATE_REQUEST		(1 << 31)
> +#define  ICL_PERIODIC_FRAME_UPDATE_ENABLE	(1 << 29)
> +#define  ICL_NULL_PACKET_ENABLE			(1 << 28)
> +#define  ICL_FRAME_IN_PROGRESS			(1 << 0)
> +
> +#define _ICL_DSI_INTR_MASK_REG_0		0x6b070
> +#define _ICL_DSI_INTR_MASK_REG_1		0x6b870
> +#define ICL_DSI_INTR_MASK_REG(port)	_MMIO_PORT(port,	\
> +						  _ICL_DSI_INTR_MASK_REG_0,\
> +						  _ICL_DSI_INTR_MASK_REG_1)
> +
> +#define _ICL_DSI_INTR_IDENT_REG_0		0x6b074
> +#define _ICL_DSI_INTR_IDENT_REG_1		0x6b874
> +#define ICL_DSI_INTR_IDENT_REG(port)	_MMIO_PORT(port,	\
> +						  _ICL_DSI_INTR_IDENT_REG_0,\
> +						  _ICL_DSI_INTR_IDENT_REG_1)
> +#define  ICL_TE_EVENT				(1 << 31)
> +#define  ICL_RX_DATA_OR_BTA_TERMINATED		(1 << 30)
> +#define  ICL_TX_DATA				(1 << 29)
> +#define  ICL_ULPS_ENTRY_DONE			(1 << 28)
> +#define  ICL_NON_TE_TRIGGER_RECEIVED		(1 << 27)
> +#define  ICL_HOST_CHKSUM_ERROR			(1 << 26)
> +#define  ICL_HOST_MULTI_ECC_ERROR		(1 << 25)
> +#define  ICL_HOST_SINGL_ECC_ERROR		(1 << 24)
> +#define  ICL_HOST_CONTENTION_DETECTED		(1 << 23)
> +#define  ICL_HOST_FALSE_CONTROL_ERROR		(1 << 22)
> +#define  ICL_HOST_TIMEOUT_ERROR			(1 << 21)
> +#define  ICL_HOST_LOW_POWER_TX_SYNC_ERROR	(1 << 20)
> +#define  ICL_HOST_ESCAPE_MODE_ENTRY_ERROR	(1 << 19)
> +#define  ICL_FRAME_UPDATE_DONE			(1 << 16)
> +#define  ICL_PROTOCOL_VIOLATION_REPORTED	(1 << 15)
> +#define  ICL_INVALID_TX_LENGTH			(1 << 13)
> +#define  ICL_INVALID_VC				(1 << 12)
> +#define  ICL_INVALID_DATA_TYPE			(1 << 11)
> +#define  ICL_PERIPHERAL_CHKSUM_ERROR		(1 << 10)
> +#define  ICL_PERIPHERAL_MULTI_ECC_ERROR		(1 << 9)
> +#define  ICL_PERIPHERAL_SINGLE_ECC_ERROR	(1 << 8)
> +#define  ICL_PERIPHERAL_CONTENTION_DETECTED	(1 << 7)
> +#define  ICL_PERIPHERAL_FALSE_CTRL_ERROR	(1 << 6)
> +#define  ICL_PERIPHERAL_TIMEOUT_ERROR		(1 << 5)
> +#define  ICL_PERIPHERAL_LP_TX_SYNC_ERROR	(1 << 4)
> +#define  ICL_PERIPHERAL_ESC_MODE_ENTRY_CMD_ERROR	(1 << 3)
> +#define  ICL_EOT_SYNC_ERROR			(1 << 2)
> +#define  ICL_SOT_SYNC_ERROR			(1 << 1)
> +#define  ICL_SOT_ERROR				(1 << 0)

All the ICL DSI register defs start with DSI_ which distinguishes them
from VLV DSI which has the MIPI_ prefix. It's not perfect, but I think
clean enough that we don't need the ICL_ prefix here.

BR,
Jani.

> +
>  /* Gen4+ Timestamp and Pipe Frame time stamp registers */
>  #define GEN4_TIMESTAMP		_MMIO(0x2358)
>  #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
> @@ -11263,6 +11322,7 @@ enum skl_power_gate {
>  #define  CMD_MODE_TE_GATE		(0x1 << 28)
>  #define  VIDEO_MODE_SYNC_EVENT		(0x2 << 28)
>  #define  VIDEO_MODE_SYNC_PULSE		(0x3 << 28)
> +#define  TE_SOURCE_GPIO			(1 << 27)
>  #define  LINK_READY			(1 << 20)
>  #define  PIX_FMT_MASK			(0x3 << 16)
>  #define  PIX_FMT_SHIFT			16

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 1/7] drm/i915/dsi: Define command mode registers
  2019-10-15  7:07   ` Jani Nikula
@ 2019-10-15  7:50     ` Kulkarni, Vandita
  0 siblings, 0 replies; 36+ messages in thread
From: Kulkarni, Vandita @ 2019-10-15  7:50 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx



> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Tuesday, October 15, 2019 12:37 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>;
> Kulkarni, Vandita <vandita.kulkarni@intel.com>; Chauhan, Madhav
> <madhav.chauhan@intel.com>
> Subject: Re: [RFC 1/7] drm/i915/dsi: Define command mode registers
> 
> On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> > Adding all the register definitions needed for mipi dsi command mode.
> >
> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 76
> > +++++++++++++++++++++++++++++----
> >  1 file changed, 68 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index e24991e54897..73bc85855b79
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4919,14 +4919,20 @@ enum {
> >  #define   BLM_PCH_POLARITY			(1 << 29)
> >  #define BLC_PWM_PCH_CTL2	_MMIO(0xc8254)
> >
> > -#define UTIL_PIN_CTL		_MMIO(0x48400)
> > -#define   UTIL_PIN_ENABLE	(1 << 31)
> > -
> > -#define   UTIL_PIN_PIPE(x)     ((x) << 29)
> > -#define   UTIL_PIN_PIPE_MASK   (3 << 29)
> > -#define   UTIL_PIN_MODE_PWM    (1 << 24)
> > -#define   UTIL_PIN_MODE_MASK   (0xf << 24)
> > -#define   UTIL_PIN_POLARITY    (1 << 22)
> > +#define UTIL_PIN_CTL			_MMIO(0x48400)
> > +#define  UTIL_PIN_ENABLE		(1 << 31)
> > +#define  UTIL_PIN_PIPE_MASK		(3 << 29)
> > +#define  UTIL_PIN_PIPE(x)		((x) << 29)
> > +#define  UTIL_PIN_MODE_MASK		(0xf << 24)
> > +#define  UTIL_PIN_MODE_DATA		(0 << 24)
> > +#define  UTIL_PIN_MODE_PWM		(1 << 24)
> > +#define  UTIL_PIN_MODE_VBLANK		(4 << 24)
> > +#define  UTIL_PIN_MODE_VSYNC		(5 << 24)
> > +#define  UTIL_PIN_MODE_EYE_LEVEL	(8 << 24)
> > +#define  UTIL_PIN_OP_DATA		(1 << 23)
> > +#define  UTIL_PIN_POLARITY		(1 << 22)
> > +#define  ICL_UTIL_PIN_DIRECTION		(1 << 19)
> > +#define  ICL_UTIL_PIN_IP_DATA		(1 << 16)
>           ^^
> 
> If you change/add everything, please put three spaces there.
> 
Ok.
> Agreed on dropping the ICL prefix here, mainly because the added bits
> predate ICL.
> 
> On naming, OP and IP aren't from the spec and aren't self-explanatory. I'd go
> with the full UTIL_PIN_OUTPUT_DATA and UTIL_PIN_INPUT_DATA. I think the
> direction bit should say what the direction is when it's set, so you don't have
> to look it up.
Ok.
> 
> 
> 
> 
> >
> >  /* BXT backlight register definition. */
> >  #define _BXT_BLC_PWM_CTL1			0xC8250
> > @@ -7407,6 +7413,8 @@ enum {
> >  #define GEN8_DE_PORT_IMR _MMIO(0x44444)  #define
> GEN8_DE_PORT_IIR
> > _MMIO(0x44448)  #define GEN8_DE_PORT_IER _MMIO(0x4444c)
> > +#define  ICL_DSI_1			(1 << 31)
> > +#define  ICL_DSI_0			(1 << 30)
> >  #define  ICL_AUX_CHANNEL_E		(1 << 29)
> >  #define  CNL_AUX_CHANNEL_F		(1 << 28)
> >  #define  GEN9_AUX_CHANNEL_D		(1 << 27)
> > @@ -10659,6 +10667,57 @@ enum skl_power_gate {
> >  #define  ICL_ESC_CLK_DIV_SHIFT			0
> >  #define DSI_MAX_ESC_CLK			20000		/* in KHz */
> >
> > +#define _ICL_DSI_CMD_FRMCTL_0		0x6b034
> > +#define _ICL_DSI_CMD_FRMCTL_1		0x6b834
> > +#define ICL_DSI_CMD_FRMCTL(port)	_MMIO_PORT(port,	\
> > +						  _ICL_DSI_CMD_FRMCTL_0,\
> > +						  _ICL_DSI_CMD_FRMCTL_1)
> > +#define  ICL_FRAME_UPDATE_REQUEST		(1 << 31)
> > +#define  ICL_PERIODIC_FRAME_UPDATE_ENABLE	(1 << 29)
> > +#define  ICL_NULL_PACKET_ENABLE			(1 << 28)
> > +#define  ICL_FRAME_IN_PROGRESS			(1 << 0)
> > +
> > +#define _ICL_DSI_INTR_MASK_REG_0		0x6b070
> > +#define _ICL_DSI_INTR_MASK_REG_1		0x6b870
> > +#define ICL_DSI_INTR_MASK_REG(port)	_MMIO_PORT(port,	\
> > +
> _ICL_DSI_INTR_MASK_REG_0,\
> > +
> _ICL_DSI_INTR_MASK_REG_1)
> > +
> > +#define _ICL_DSI_INTR_IDENT_REG_0		0x6b074
> > +#define _ICL_DSI_INTR_IDENT_REG_1		0x6b874
> > +#define ICL_DSI_INTR_IDENT_REG(port)	_MMIO_PORT(port,	\
> > +
> _ICL_DSI_INTR_IDENT_REG_0,\
> > +
> _ICL_DSI_INTR_IDENT_REG_1)
> > +#define  ICL_TE_EVENT				(1 << 31)
> > +#define  ICL_RX_DATA_OR_BTA_TERMINATED		(1 << 30)
> > +#define  ICL_TX_DATA				(1 << 29)
> > +#define  ICL_ULPS_ENTRY_DONE			(1 << 28)
> > +#define  ICL_NON_TE_TRIGGER_RECEIVED		(1 << 27)
> > +#define  ICL_HOST_CHKSUM_ERROR			(1 << 26)
> > +#define  ICL_HOST_MULTI_ECC_ERROR		(1 << 25)
> > +#define  ICL_HOST_SINGL_ECC_ERROR		(1 << 24)
> > +#define  ICL_HOST_CONTENTION_DETECTED		(1 << 23)
> > +#define  ICL_HOST_FALSE_CONTROL_ERROR		(1 << 22)
> > +#define  ICL_HOST_TIMEOUT_ERROR			(1 << 21)
> > +#define  ICL_HOST_LOW_POWER_TX_SYNC_ERROR	(1 << 20)
> > +#define  ICL_HOST_ESCAPE_MODE_ENTRY_ERROR	(1 << 19)
> > +#define  ICL_FRAME_UPDATE_DONE			(1 << 16)
> > +#define  ICL_PROTOCOL_VIOLATION_REPORTED	(1 << 15)
> > +#define  ICL_INVALID_TX_LENGTH			(1 << 13)
> > +#define  ICL_INVALID_VC				(1 << 12)
> > +#define  ICL_INVALID_DATA_TYPE			(1 << 11)
> > +#define  ICL_PERIPHERAL_CHKSUM_ERROR		(1 << 10)
> > +#define  ICL_PERIPHERAL_MULTI_ECC_ERROR		(1 << 9)
> > +#define  ICL_PERIPHERAL_SINGLE_ECC_ERROR	(1 << 8)
> > +#define  ICL_PERIPHERAL_CONTENTION_DETECTED	(1 << 7)
> > +#define  ICL_PERIPHERAL_FALSE_CTRL_ERROR	(1 << 6)
> > +#define  ICL_PERIPHERAL_TIMEOUT_ERROR		(1 << 5)
> > +#define  ICL_PERIPHERAL_LP_TX_SYNC_ERROR	(1 << 4)
> > +#define  ICL_PERIPHERAL_ESC_MODE_ENTRY_CMD_ERROR	(1 << 3)
> > +#define  ICL_EOT_SYNC_ERROR			(1 << 2)
> > +#define  ICL_SOT_SYNC_ERROR			(1 << 1)
> > +#define  ICL_SOT_ERROR				(1 << 0)
> 
> All the ICL DSI register defs start with DSI_ which distinguishes them from
> VLV DSI which has the MIPI_ prefix. It's not perfect, but I think clean enough
> that we don't need the ICL_ prefix here.
Ok, will remove the ICL prefix and add DSI instead.

Thanks,
Vandita
> 
> BR,
> Jani.
> 
> > +
> >  /* Gen4+ Timestamp and Pipe Frame time stamp registers */
> >  #define GEN4_TIMESTAMP		_MMIO(0x2358)
> >  #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
> > @@ -11263,6 +11322,7 @@ enum skl_power_gate {
> >  #define  CMD_MODE_TE_GATE		(0x1 << 28)
> >  #define  VIDEO_MODE_SYNC_EVENT		(0x2 << 28)
> >  #define  VIDEO_MODE_SYNC_PULSE		(0x3 << 28)
> > +#define  TE_SOURCE_GPIO			(1 << 27)
> >  #define  LINK_READY			(1 << 20)
> >  #define  PIX_FMT_MASK			(0x3 << 16)
> >  #define  PIX_FMT_SHIFT			16
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 1/7] drm/i915/dsi: Define command mode registers
  2019-10-14 16:18   ` Ramalingam C
@ 2019-10-15  7:52     ` Kulkarni, Vandita
  0 siblings, 0 replies; 36+ messages in thread
From: Kulkarni, Vandita @ 2019-10-15  7:52 UTC (permalink / raw)
  To: C, Ramalingam; +Cc: Nikula, Jani, intel-gfx



> -----Original Message-----
> From: C, Ramalingam <ramalingam.c@intel.com>
> Sent: Monday, October 14, 2019 9:49 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>
> Subject: Re: [Intel-gfx] [RFC 1/7] drm/i915/dsi: Define command mode
> registers
> 
> On 2019-10-14 at 16:31:16 +0530, Vandita Kulkarni wrote:
> > Adding all the register definitions needed for mipi dsi command mode.
> >
> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 76
> > +++++++++++++++++++++++++++++----
> >  1 file changed, 68 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index e24991e54897..73bc85855b79
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4919,14 +4919,20 @@ enum {
> >  #define   BLM_PCH_POLARITY			(1 << 29)
> >  #define BLC_PWM_PCH_CTL2	_MMIO(0xc8254)
> >
> > -#define UTIL_PIN_CTL		_MMIO(0x48400)
> > -#define   UTIL_PIN_ENABLE	(1 << 31)
> > -
> > -#define   UTIL_PIN_PIPE(x)     ((x) << 29)
> > -#define   UTIL_PIN_PIPE_MASK   (3 << 29)
> > -#define   UTIL_PIN_MODE_PWM    (1 << 24)
> > -#define   UTIL_PIN_MODE_MASK   (0xf << 24)
> > -#define   UTIL_PIN_POLARITY    (1 << 22)
> > +#define UTIL_PIN_CTL			_MMIO(0x48400)
> > +#define  UTIL_PIN_ENABLE		(1 << 31)
> > +#define  UTIL_PIN_PIPE_MASK		(3 << 29)
> > +#define  UTIL_PIN_PIPE(x)		((x) << 29)
> > +#define  UTIL_PIN_MODE_MASK		(0xf << 24)
> > +#define  UTIL_PIN_MODE_DATA		(0 << 24)
> > +#define  UTIL_PIN_MODE_PWM		(1 << 24)
> > +#define  UTIL_PIN_MODE_VBLANK		(4 << 24)
> > +#define  UTIL_PIN_MODE_VSYNC		(5 << 24)
> > +#define  UTIL_PIN_MODE_EYE_LEVEL	(8 << 24)
> > +#define  UTIL_PIN_OP_DATA		(1 << 23)
> > +#define  UTIL_PIN_POLARITY		(1 << 22)
> > +#define  ICL_UTIL_PIN_DIRECTION		(1 << 19)
> > +#define  ICL_UTIL_PIN_IP_DATA		(1 << 16)
> Could we drop this ICL prefix to align with existing bit definitions?
Sure.
> >
> >  /* BXT backlight register definition. */
> >  #define _BXT_BLC_PWM_CTL1			0xC8250
> > @@ -7407,6 +7413,8 @@ enum {
> >  #define GEN8_DE_PORT_IMR _MMIO(0x44444)  #define
> GEN8_DE_PORT_IIR
> > _MMIO(0x44448)  #define GEN8_DE_PORT_IER _MMIO(0x4444c)
> > +#define  ICL_DSI_1			(1 << 31)
> > +#define  ICL_DSI_0			(1 << 30)
> >  #define  ICL_AUX_CHANNEL_E		(1 << 29)
> >  #define  CNL_AUX_CHANNEL_F		(1 << 28)
> >  #define  GEN9_AUX_CHANNEL_D		(1 << 27)
> > @@ -10659,6 +10667,57 @@ enum skl_power_gate {
> >  #define  ICL_ESC_CLK_DIV_SHIFT			0
> >  #define DSI_MAX_ESC_CLK			20000		/* in KHz */
> >
> > +#define _ICL_DSI_CMD_FRMCTL_0		0x6b034
> > +#define _ICL_DSI_CMD_FRMCTL_1		0x6b834
> > +#define ICL_DSI_CMD_FRMCTL(port)	_MMIO_PORT(port,	\
> > +						  _ICL_DSI_CMD_FRMCTL_0,\
> > +						  _ICL_DSI_CMD_FRMCTL_1)
> might want to align with first char after (.
Ok.
> > +#define  ICL_FRAME_UPDATE_REQUEST		(1 << 31)
> > +#define  ICL_PERIODIC_FRAME_UPDATE_ENABLE	(1 << 29)
> > +#define  ICL_NULL_PACKET_ENABLE			(1 << 28)
> > +#define  ICL_FRAME_IN_PROGRESS			(1 << 0)
> > +
> > +#define _ICL_DSI_INTR_MASK_REG_0		0x6b070
> > +#define _ICL_DSI_INTR_MASK_REG_1		0x6b870
> > +#define ICL_DSI_INTR_MASK_REG(port)	_MMIO_PORT(port,	\
> > +
> _ICL_DSI_INTR_MASK_REG_0,\
> > +
> _ICL_DSI_INTR_MASK_REG_1)
> same here. and all below lines...
> > +
> > +#define _ICL_DSI_INTR_IDENT_REG_0		0x6b074
> > +#define _ICL_DSI_INTR_IDENT_REG_1		0x6b874
> > +#define ICL_DSI_INTR_IDENT_REG(port)	_MMIO_PORT(port,	\
> > +
> _ICL_DSI_INTR_IDENT_REG_0,\
> > +
> _ICL_DSI_INTR_IDENT_REG_1)
> > +#define  ICL_TE_EVENT				(1 << 31)
> > +#define  ICL_RX_DATA_OR_BTA_TERMINATED		(1 << 30)
> > +#define  ICL_TX_DATA				(1 << 29)
> > +#define  ICL_ULPS_ENTRY_DONE			(1 << 28)
> > +#define  ICL_NON_TE_TRIGGER_RECEIVED		(1 << 27)
> > +#define  ICL_HOST_CHKSUM_ERROR			(1 << 26)
> > +#define  ICL_HOST_MULTI_ECC_ERROR		(1 << 25)
> > +#define  ICL_HOST_SINGL_ECC_ERROR		(1 << 24)
> > +#define  ICL_HOST_CONTENTION_DETECTED		(1 << 23)
> > +#define  ICL_HOST_FALSE_CONTROL_ERROR		(1 << 22)
> > +#define  ICL_HOST_TIMEOUT_ERROR			(1 << 21)
> > +#define  ICL_HOST_LOW_POWER_TX_SYNC_ERROR	(1 << 20)
> > +#define  ICL_HOST_ESCAPE_MODE_ENTRY_ERROR	(1 << 19)
> > +#define  ICL_FRAME_UPDATE_DONE			(1 << 16)
> > +#define  ICL_PROTOCOL_VIOLATION_REPORTED	(1 << 15)
> > +#define  ICL_INVALID_TX_LENGTH			(1 << 13)
> > +#define  ICL_INVALID_VC				(1 << 12)
> > +#define  ICL_INVALID_DATA_TYPE			(1 << 11)
> > +#define  ICL_PERIPHERAL_CHKSUM_ERROR		(1 << 10)
> > +#define  ICL_PERIPHERAL_MULTI_ECC_ERROR		(1 << 9)
> > +#define  ICL_PERIPHERAL_SINGLE_ECC_ERROR	(1 << 8)
> > +#define  ICL_PERIPHERAL_CONTENTION_DETECTED	(1 << 7)
> > +#define  ICL_PERIPHERAL_FALSE_CTRL_ERROR	(1 << 6)
> > +#define  ICL_PERIPHERAL_TIMEOUT_ERROR		(1 << 5)
> > +#define  ICL_PERIPHERAL_LP_TX_SYNC_ERROR	(1 << 4)
> > +#define  ICL_PERIPHERAL_ESC_MODE_ENTRY_CMD_ERROR	(1 << 3)
> Too lengthy!? Could we use short form of this macro name?
Ya, will shorten it.

Thanks,
Vandita
> 
> -Ram
> > +#define  ICL_EOT_SYNC_ERROR			(1 << 2)
> > +#define  ICL_SOT_SYNC_ERROR			(1 << 1)
> > +#define  ICL_SOT_ERROR				(1 << 0)
> > +
> >  /* Gen4+ Timestamp and Pipe Frame time stamp registers */
> >  #define GEN4_TIMESTAMP		_MMIO(0x2358)
> >  #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
> > @@ -11263,6 +11322,7 @@ enum skl_power_gate {
> >  #define  CMD_MODE_TE_GATE		(0x1 << 28)
> >  #define  VIDEO_MODE_SYNC_EVENT		(0x2 << 28)
> >  #define  VIDEO_MODE_SYNC_PULSE		(0x3 << 28)
> > +#define  TE_SOURCE_GPIO			(1 << 27)
> >  #define  LINK_READY			(1 << 20)
> >  #define  PIX_FMT_MASK			(0x3 << 16)
> >  #define  PIX_FMT_SHIFT			16
> > --
> > 2.21.0.5.gaeb582a
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 6/7] drm/i915/dsi: Add TE handler for dsi cmd mode.
  2019-10-14 11:01 ` [RFC 6/7] drm/i915/dsi: Add TE handler for dsi " Vandita Kulkarni
@ 2019-10-15  8:28   ` Kulkarni, Vandita
  2019-10-16 10:24   ` Ramalingam C
  1 sibling, 0 replies; 36+ messages in thread
From: Kulkarni, Vandita @ 2019-10-15  8:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Nikula, Jani

Correction in this patch is needed , I will make the below change in the next version.

> -----Original Message-----
> From: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Sent: Monday, October 14, 2019 4:31 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; ville.syrjala@linux.intel.com;
> Shankar, Uma <uma.shankar@intel.com>; Kulkarni, Vandita
> <vandita.kulkarni@intel.com>
> Subject: [RFC 6/7] drm/i915/dsi: Add TE handler for dsi cmd mode.
> 
> In case of dual link, we get the TE on slave.
> So clear the TE on slave DSI IIR.
> 
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 61
> +++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c index bfb2a63504fb..d12efa72943b
> 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2628,6 +2628,61 @@ gen8_de_misc_irq_handler(struct
> drm_i915_private *dev_priv, u32 iir)
>  		DRM_ERROR("Unexpected DE Misc interrupt\n");  }
> 
> +void gen11_dsi_te_interrupt_handler(struct drm_i915_private *dev_priv,
> +				    u32 iir_value)
> +{
> +	enum pipe pipe = INVALID_PIPE;
> +	enum port port;
> +	enum transcoder dsi_trans;
> +	u32 val;
> +
> +	/*
> +	 * Incase of dual link, TE comes from DSI_1
> +	 * this is to check if dual link is enabled
> +	 */
> +	val = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
> +	val &= PORT_SYNC_MODE_ENABLE;
> +
> +	/*
> +	 * if dual link is enabled, then read DSI_0
> +	 * transcoder registers
> +	 */
> +	port = ((iir_value & ICL_DSI_1) && val) || (iir_value & ICL_DSI_0) ?
> PORT_A : PORT_B;
> +	dsi_trans = (port == PORT_A) ? TRANSCODER_DSI_0 :
> TRANSCODER_DSI_1;
> +
> +	/* Check if DSI configured in command mode */
> +	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
> +	val = (val & OP_MODE_MASK) >> 28;
> +
> +	if (val) {
> +		DRM_ERROR("DSI trancoder not configured in command
> mode\n");
> +		return;
> +	}
> +
> +	/* Get PIPE for handling VBLANK event */
> +	val = I915_READ(TRANS_DDI_FUNC_CTL(dsi_trans));
> +	switch (val & TRANS_DDI_EDP_INPUT_MASK) {
> +	case TRANS_DDI_EDP_INPUT_A_ON:
> +		pipe = PIPE_A;
> +		break;
> +	case TRANS_DDI_EDP_INPUT_B_ONOFF:
> +		pipe = PIPE_B;
> +		break;
> +	case TRANS_DDI_EDP_INPUT_C_ONOFF:
> +		pipe = PIPE_C;
> +		break;
> +	default:
> +		DRM_ERROR("Invalid PIPE\n");
> +	}
> +
> +	/* clear TE in dsi IIR */
> +	port = (iir_value & ICL_DSI_1) ? PORT_B : PORT_A;
> +	val = I915_READ(ICL_DSI_INTR_IDENT_REG(port));
> +	I915_WRITE((ICL_DSI_INTR_IDENT_REG(port)), val);
> +
> +	drm_handle_vblank(&dev_priv->drm, pipe); }
> +
>  static irqreturn_t
>  gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)  {
> @@ -2692,6 +2747,12 @@ gen8_de_irq_handler(struct drm_i915_private
> *dev_priv, u32 master_ctl)
>  				found = true;
>  			}
> 
> +			if ((INTEL_GEN(dev_priv) >= 11) &&
> +				(iir & (ICL_DSI_0 | ICL_DSI_1))) {
Instead if ICL_DSI_0 it should be ICL_TE_DSI_0 and instead of ICL_DSI_1 it should be ICL_TE_DSI_1

> +				gen11_dsi_te_interrupt_handler(dev_priv,
> iir);
> +				found = true;
> +			}
> +
>  			if (!found)
>  				DRM_ERROR("Unexpected DE Port
> interrupt\n");
>  		}
> --
> 2.21.0.5.gaeb582a

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode.
  2019-10-14 11:01 ` [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode Vandita Kulkarni
@ 2019-10-15 18:35   ` Jani Nikula
  2019-10-24 11:37     ` [Intel-gfx] " Jani Nikula
  1 sibling, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-15 18:35 UTC (permalink / raw)
  To: Vandita Kulkarni, intel-gfx

On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> Configure the transcoder to operate in TE GATE command mode
> and  take TE events from GPIO.
> Also disable the periodic command mode, that GOP would have
> programmed.
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c | 32 ++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..8e6c09a1db78 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -704,6 +704,10 @@ gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
>  				tmp |= VIDEO_MODE_SYNC_PULSE;
>  				break;
>  			}
> +		} else {
> +			tmp &= ~OP_MODE_MASK;
> +			tmp |= CMD_MODE_TE_GATE;
> +			tmp |= TE_SOURCE_GPIO;
>  		}
>  
>  		I915_WRITE(DSI_TRANS_FUNC_CONF(dsi_trans), tmp);
> @@ -953,6 +957,22 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder)
>  	}
>  }
>  
> +static void gen11_dsi_config_util_pin(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	u32 tmp;
> +
> +	/* used only as TE i/p for DSI0 for dual link TE is from slave DSI1 */
> +	if (is_vid_mode(intel_dsi) || (intel_dsi->dual_link))
> +		return;

Full disclosure: I didn't check the spec on this.

But... where does the TE come for the slave DSI1 then? The comment seems
confusing.

Nitpick, intel_dsi->dual_link does not need parenthesis.

> +
> +	tmp = I915_READ(UTIL_PIN_CTL);
> +	tmp |= ICL_UTIL_PIN_DIRECTION;

If the macro had INPUT in the name, this would be self-explanatory.

> +	tmp |= UTIL_PIN_ENABLE;
> +	I915_WRITE(UTIL_PIN_CTL, tmp);
> +}
> +
>  static void
>  gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
>  			      const struct intel_crtc_state *pipe_config)
> @@ -974,6 +994,9 @@ gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
>  	/* setup D-PHY timings */
>  	gen11_dsi_setup_dphy_timings(encoder);
>  
> +	/* Since transcoder is configured to take events from GPIO */
> +	gen11_dsi_config_util_pin(encoder);
> +
>  	/* step 4h: setup DSI protocol timeouts */
>  	gen11_dsi_setup_timeouts(encoder);
>  
> @@ -1104,6 +1127,15 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
>  	enum transcoder dsi_trans;
>  	u32 tmp;
>  
> +	/* disable periodic update mode */
> +	if (is_cmd_mode(intel_dsi)) {
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			tmp = I915_READ(ICL_DSI_CMD_FRMCTL(port));
> +			tmp &= ~ICL_PERIODIC_FRAME_UPDATE_ENABLE;
> +			I915_WRITE(ICL_DSI_CMD_FRMCTL(port), tmp);
> +		}
> +	}
> +
>  	/* put dsi link in ULPS */
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		dsi_trans = dsi_port_to_transcoder(port);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 3/7] drm/i915/dsi: Add vblank calculation for command mode
  2019-10-14 11:01 ` [RFC 3/7] drm/i915/dsi: Add vblank calculation " Vandita Kulkarni
@ 2019-10-15 18:45   ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-15 18:45 UTC (permalink / raw)
  To: Vandita Kulkarni, intel-gfx

On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> Transcoder timing calculation differ for command mode.
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c | 56 +++++++++++++++++---------
>  1 file changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 8e6c09a1db78..5dd9eebab6b1 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -780,6 +780,7 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
>  	u16 hback_porch;
>  	/* vertical timings */
>  	u16 vtotal, vactive, vsync_start, vsync_end, vsync_shift;
> +	int bpp, line_time_us, byte_clk_period_ns;
>  
>  	hactive = adjusted_mode->crtc_hdisplay;
>  	htotal = adjusted_mode->crtc_htotal;
> @@ -841,40 +842,57 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
>  	}
>  
>  	/* program TRANS_VTOTAL register */
> -	for_each_dsi_port(port, intel_dsi->ports) {
> -		dsi_trans = dsi_port_to_transcoder(port);
> -		/*
> -		 * FIXME: Programing this by assuming progressive mode, since
> -		 * non-interlaced info from VBT is not saved inside
> -		 * struct drm_display_mode.
> -		 * For interlace mode: program required pixel minus 2
> -		 */
> -		I915_WRITE(VTOTAL(dsi_trans),
> -			   (vactive - 1) | ((vtotal - 1) << 16));
> +	if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {

There's is_vid_mode() and is_cmd_mode(). Use them throughout the series
instead of the above.

> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			dsi_trans = dsi_port_to_transcoder(port);
> +			/*
> +			 * FIXME: Programing this by assuming progressive mode,
> +			 * since non-interlaced info from VBT is not saved
> +			 * inside struct drm_display_mode.
> +			 * For interlace mode: program required pixel minus 2
> +			 */
> +			I915_WRITE(VTOTAL(dsi_trans),
> +				   (vactive - 1) | ((vtotal - 1) << 16));
> +		}
> +	} else {
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> +			byte_clk_period_ns = 8 * 1000000 / intel_dsi->pclk;
> +			htotal = hactive + 160;
> +			line_time_us = (htotal * (bpp / 8) * byte_clk_period_ns) / (1000 * intel_dsi->lane_count);
> +			vtotal = vactive + DIV_ROUND_UP(460, line_time_us);

This is a bit hand-wavy, but some of these seem suspicious. Someone(tm)
needs to go through these in detail.

> +			I915_WRITE(VTOTAL(dsi_trans),
> +				   (vactive - 1) | ((vtotal - 1) << 16));
> +		}
>  	}

As the I915_WRITE() is the same for both, albeit with changed values for
cmd mode, I think it would be better to have the mode check within the
for_each_dsi_port() block.

>  
> +
>  	if (vsync_end < vsync_start || vsync_end > vtotal)
>  		DRM_ERROR("Invalid vsync_end value\n");
>  
>  	if (vsync_start < vactive)
>  		DRM_ERROR("vsync_start less than vactive\n");
>  
> -	/* program TRANS_VSYNC register */
> -	for_each_dsi_port(port, intel_dsi->ports) {
> -		dsi_trans = dsi_port_to_transcoder(port);
> -		I915_WRITE(VSYNC(dsi_trans),
> -			   (vsync_start - 1) | ((vsync_end - 1) << 16));
> +	/* program TRANS_VSYNC register for video mode only */
> +	if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			dsi_trans = dsi_port_to_transcoder(port);
> +			I915_WRITE(VSYNC(dsi_trans),
> +				   (vsync_start - 1) | ((vsync_end - 1) << 16));
> +		}
>  	}
>  
>  	/*
> -	 * FIXME: It has to be programmed only for interlaced
> +	 * FIXME: It has to be programmed only for video modes and interlaced
>  	 * modes. Put the check condition here once interlaced
>  	 * info available as described above.
>  	 * program TRANS_VSYNCSHIFT register
>  	 */
> -	for_each_dsi_port(port, intel_dsi->ports) {
> -		dsi_trans = dsi_port_to_transcoder(port);
> -		I915_WRITE(VSYNCSHIFT(dsi_trans), vsync_shift);
> +	if (intel_dsi->operation_mode == INTEL_DSI_VIDEO_MODE) {
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			dsi_trans = dsi_port_to_transcoder(port);
> +			I915_WRITE(VSYNCSHIFT(dsi_trans), vsync_shift);
> +		}
>  	}
>  
>  	/* program TRANS_VBLANK register, should be same as vtotal programmed */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
  2019-10-14 11:01 ` [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode Vandita Kulkarni
@ 2019-10-15 19:20   ` Jani Nikula
  2019-10-16 13:27     ` Kulkarni, Vandita
  0 siblings, 1 reply; 36+ messages in thread
From: Jani Nikula @ 2019-10-15 19:20 UTC (permalink / raw)
  To: Vandita Kulkarni, intel-gfx

On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> From: Madhav Chauhan <madhav.chauhan@intel.com>
>
> This patch adds a helper function to find encoder
> if DSI is operating in command mode. This function
> will be used while enabling/disabling TE interrupts
> for DSI.
>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c   | 17 +++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dsi.h |  3 +++
>  2 files changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 5dd9eebab6b1..877746416e52 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -73,6 +73,23 @@ static enum transcoder dsi_port_to_transcoder(enum port port)
>  		return TRANSCODER_DSI_1;
>  }
>  
> +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct intel_encoder *encoder;
> +	struct intel_dsi *intel_dsi;
> +
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +		if (encoder->type != INTEL_OUTPUT_DSI)
> +			continue;
> +		intel_dsi = enc_to_intel_dsi(&encoder->base);
> +		if (intel_dsi->operation_mode == INTEL_DSI_COMMAND_MODE)
> +			return encoder;
> +	}
> +
> +	return NULL;
> +}

This may be a bit harsh, but everything that feels wrong about the
following patches pretty much boils down to this function. It may get
the job done, and I don't have a better suggestion on how to accomplish
this right now. But it seems like we shouldn't have to do anything like
this, and makes you feel like there's something wrong with the design.

It would be great to be able to handle this using crtc state, but alas
the vblank enable hook only gets passed a struct drm_crtc *. (Patch 7
could easily switch to using crtc state, but need to also solve patch
5.)

I'll get back to you on this later, but in the mean time - Ville, do you
have any ideas?


BR,
Jani.


> +
>  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h b/drivers/gpu/drm/i915/display/intel_dsi.h
> index b15be5814599..071dad7ee04a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
> @@ -201,6 +201,9 @@ u32 bxt_dsi_get_pclk(struct intel_encoder *encoder,
>  		     struct intel_crtc_state *config);
>  void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
>  
> +/* icl_dsi.c */
> +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc);
> +
>  /* intel_dsi_vbt.c */
>  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
>  void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 5/7] drm/i915/dsi: Configure TE interrupt for cmd mode
  2019-10-14 11:01 ` [RFC 5/7] drm/i915/dsi: Configure TE interrupt for " Vandita Kulkarni
@ 2019-10-16  9:56   ` Ramalingam C
  2019-10-24 11:34     ` [Intel-gfx] " Jani Nikula
  1 sibling, 0 replies; 36+ messages in thread
From: Ramalingam C @ 2019-10-16  9:56 UTC (permalink / raw)
  To: Vandita Kulkarni; +Cc: jani.nikula, intel-gfx

On 2019-10-14 at 16:31:20 +0530, Vandita Kulkarni wrote:
> We need to configure TE interrupt in two places.
> Port interrupt and DSI interrupt mask registers.
> 
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 49 ++++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 3af7f7914c40..bfb2a63504fb 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -41,6 +41,7 @@
>  #include "display/intel_hotplug.h"
>  #include "display/intel_lpe_audio.h"
>  #include "display/intel_psr.h"
> +#include "display/intel_dsi.h"
>  
>  #include "gt/intel_gt.h"
>  #include "gt/intel_gt_irq.h"
> @@ -2960,12 +2961,44 @@ int ilk_enable_vblank(struct drm_crtc *crtc)
>  	return 0;
>  }
>  
> +static void gen11_dsi_configure_te(struct drm_crtc *crtc, bool enable)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_encoder *encoder = NULL;
> +	struct intel_dsi *intel_dsi;
> +	enum port port;
> +	u32 tmp;
> +
> +	encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);
> +	if (!encoder)
> +		return;
> +
> +	intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	/* Assuming single link would always be enabled on PORT_A */
> +	port = (intel_dsi->ports & BIT(PORT_B) & BIT(PORT_A)) ? PORT_B : PORT_A;
> +	tmp =  I915_READ(ICL_DSI_INTR_MASK_REG(port));
> +	if (enable)
> +		tmp &= ~ICL_TE_EVENT;
> +	else
> +		tmp |= ICL_TE_EVENT;
> +
> +	I915_WRITE(ICL_DSI_INTR_MASK_REG(port), tmp);
> +}
> +
>  int bdw_enable_vblank(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>  	unsigned long irqflags;
>  
> +	if (INTEL_GEN(dev_priv) >= 11 &&
> +		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {
usually second line of the condition will be aligned to the start of the
first char in first line.
> +		gen11_dsi_configure_te(crtc, true);
> +		return 0;
> +	}
> +
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>  	bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> @@ -3031,9 +3064,16 @@ void ilk_disable_vblank(struct drm_crtc *crtc)
>  void bdw_disable_vblank(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> -	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	enum pipe pipe = intel_crtc->pipe;
>  	unsigned long irqflags;
>  
> +	if (INTEL_GEN(dev_priv) >= 11 &&
> +		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {
same here.

-Ram
> +		gen11_dsi_configure_te(crtc, false);
> +		return;
> +	}
> +
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>  	bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> @@ -3726,6 +3766,13 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  		gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 11) {
> +		enum port port;
> +
> +		if (intel_bios_is_dsi_present(dev_priv, &port))
> +			de_port_masked |= ICL_DSI_0 | ICL_DSI_1;
> +	}
> +
>  	for_each_pipe(dev_priv, pipe) {
>  		dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;
>  
> -- 
> 2.21.0.5.gaeb582a
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 7/7] drm/i915/dsi: Initiate frame request in cmd mode
  2019-10-14 11:01 ` [RFC 7/7] drm/i915/dsi: Initiate frame request in " Vandita Kulkarni
@ 2019-10-16 10:14   ` Ramalingam C
  2019-10-16 12:37     ` Kulkarni, Vandita
  0 siblings, 1 reply; 36+ messages in thread
From: Ramalingam C @ 2019-10-16 10:14 UTC (permalink / raw)
  To: Vandita Kulkarni; +Cc: jani.nikula, intel-gfx

On 2019-10-14 at 16:31:22 +0530, Vandita Kulkarni wrote:
> In TE Gate mode, on every flip we need to set the
> frame update request bit. After this  bit is set
> transcoder hardware will automatically send the
> frame data to the panel when it receives the TE event.
> 
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c        | 27 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_display.c  | 16 +++++++++++
>  .../drm/i915/display/intel_display_types.h    |  3 +++
>  drivers/gpu/drm/i915/display/intel_dsi.h      |  3 +++
>  4 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 877746416e52..c72917ddf8e7 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -90,6 +90,33 @@ struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc)
>  	return NULL;
>  }
>  
> +void gen11_dsi_check_frame_update_needed(struct intel_crtc *intel_crtc,
> +					 struct intel_crtc_state *crtc_state)

> +{
> +	struct intel_encoder *intel_encoder = NULL;
> +
> +	intel_encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);
> +	if (!intel_encoder)
> +		return;
In this func, if you could find a DSI encoder with CMD mode, you set the
flag!?
> +
> +	/* TBD Use bits  to say update on which  dsi port instead of a bool */
> +	crtc_state->dsi_frame_update = true;
> +}
> +
> +void gen11_dsi_frame_update(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	u32 tmp;
> +
> +	/* TBD: add check on port */
> +	if (crtc_state->dsi_frame_update) {
> +		tmp = I915_READ(ICL_DSI_CMD_FRMCTL(PORT_A));
> +		tmp |= ICL_FRAME_UPDATE_REQUEST;
> +		I915_WRITE(ICL_DSI_CMD_FRMCTL(PORT_A), tmp);
> +	}
> +}
> +
>  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 3cf39fc153b3..a902bb2bf075 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11858,6 +11858,11 @@ static int intel_crtc_atomic_check(struct drm_crtc *_crtc,
>  							 crtc_state);
>  	}
>  
> +	if ((INTEL_GEN(dev_priv) >= 11) &&
> +		(intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))) {
Please check this alignment too

-Ram
> +		gen11_dsi_check_frame_update_needed(crtc, crtc_state);
> +	}
> +
>  	if (HAS_IPS(dev_priv))
>  		crtc_state->ips_enabled = hsw_compute_ips_config(crtc_state);
>  
> @@ -13618,6 +13623,7 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
>  		if (!needs_modeset(new_crtc_state) &&
> @@ -14108,6 +14114,16 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>  			intel_color_load_luts(new_crtc_state);
>  	}
>  
> +	/*
> +	 * Incase of mipi dsi command mode, we need to set frame update
> +	 * for every commit
> +	 */
> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (new_crtc_state->base.active &&
> +		    !needs_modeset(new_crtc_state) &&
> +		    new_crtc_state->dsi_frame_update)
> +			gen11_dsi_frame_update(new_crtc_state);
> +	}
>  	/*
>  	 * Now that the vblank has passed, we can go ahead and program the
>  	 * optimal watermarks on platforms that need two-step watermark
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 40390d855815..69da4ec45691 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -991,6 +991,9 @@ struct intel_crtc_state {
>  
>  	/* Forward Error correction State */
>  	bool fec_enable;
> +
> +	/* frame update for dsi command mode */
> +	bool dsi_frame_update;
>  };
>  
>  struct intel_crtc {
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h b/drivers/gpu/drm/i915/display/intel_dsi.h
> index 071dad7ee04a..d9350b842115 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
> @@ -203,6 +203,9 @@ void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
>  
>  /* icl_dsi.c */
>  struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc);
> +void gen11_dsi_check_frame_update_needed(struct intel_crtc *crtc,
> +					 struct intel_crtc_state *crtc_state);
> +void gen11_dsi_frame_update(struct intel_crtc_state *crtc_state);
>  
>  /* intel_dsi_vbt.c */
>  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
> -- 
> 2.21.0.5.gaeb582a
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 6/7] drm/i915/dsi: Add TE handler for dsi cmd mode.
  2019-10-14 11:01 ` [RFC 6/7] drm/i915/dsi: Add TE handler for dsi " Vandita Kulkarni
  2019-10-15  8:28   ` Kulkarni, Vandita
@ 2019-10-16 10:24   ` Ramalingam C
  2019-10-16 12:46     ` Kulkarni, Vandita
  1 sibling, 1 reply; 36+ messages in thread
From: Ramalingam C @ 2019-10-16 10:24 UTC (permalink / raw)
  To: Vandita Kulkarni; +Cc: jani.nikula, intel-gfx

On 2019-10-14 at 16:31:21 +0530, Vandita Kulkarni wrote:
> In case of dual link, we get the TE on slave.
> So clear the TE on slave DSI IIR.
> 
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 61 +++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bfb2a63504fb..d12efa72943b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2628,6 +2628,61 @@ gen8_de_misc_irq_handler(struct drm_i915_private *dev_priv, u32 iir)
>  		DRM_ERROR("Unexpected DE Misc interrupt\n");
>  }
>  
> +void gen11_dsi_te_interrupt_handler(struct drm_i915_private *dev_priv,
> +				    u32 iir_value)
> +{
> +	enum pipe pipe = INVALID_PIPE;
> +	enum port port;
> +	enum transcoder dsi_trans;
> +	u32 val;
usually we order the declarations based on the length.
> +
> +	/*
> +	 * Incase of dual link, TE comes from DSI_1
> +	 * this is to check if dual link is enabled
> +	 */
> +	val = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
> +	val &= PORT_SYNC_MODE_ENABLE;
> +
> +	/*
> +	 * if dual link is enabled, then read DSI_0
> +	 * transcoder registers
> +	 */
> +	port = ((iir_value & ICL_DSI_1) && val) || (iir_value & ICL_DSI_0) ? PORT_A : PORT_B;
wrap it!? beyond 80char?
> +	dsi_trans = (port == PORT_A) ? TRANSCODER_DSI_0 : TRANSCODER_DSI_1;
> +
> +	/* Check if DSI configured in command mode */
> +	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
> +	val = (val & OP_MODE_MASK) >> 28;
Could we use a macro, like GET_OP_MODE()?
> +
> +	if (val) {
> +		DRM_ERROR("DSI trancoder not configured in command mode\n");
> +		return;
> +	}
> +
> +	/* Get PIPE for handling VBLANK event */
> +	val = I915_READ(TRANS_DDI_FUNC_CTL(dsi_trans));
> +	switch (val & TRANS_DDI_EDP_INPUT_MASK) {
> +	case TRANS_DDI_EDP_INPUT_A_ON:
> +		pipe = PIPE_A;
> +		break;
> +	case TRANS_DDI_EDP_INPUT_B_ONOFF:
> +		pipe = PIPE_B;
> +		break;
> +	case TRANS_DDI_EDP_INPUT_C_ONOFF:
> +		pipe = PIPE_C;
> +		break;
> +	default:
> +		DRM_ERROR("Invalid PIPE\n");
> +	}
> +
> +	/* clear TE in dsi IIR */
> +	port = (iir_value & ICL_DSI_1) ? PORT_B : PORT_A;
> +	val = I915_READ(ICL_DSI_INTR_IDENT_REG(port));
> +	I915_WRITE((ICL_DSI_INTR_IDENT_REG(port)), val);
extra () around ICL_DSI_INTR_IDENT_REG(port)
> +
> +	drm_handle_vblank(&dev_priv->drm, pipe);
> +}
> +
>  static irqreturn_t
>  gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  {
> @@ -2692,6 +2747,12 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  				found = true;
>  			}
>  
> +			if ((INTEL_GEN(dev_priv) >= 11) &&
> +				(iir & (ICL_DSI_0 | ICL_DSI_1))) {
alignment.

-Ram
> +				gen11_dsi_te_interrupt_handler(dev_priv, iir);
> +				found = true;
> +			}
> +
>  			if (!found)
>  				DRM_ERROR("Unexpected DE Port interrupt\n");
>  		}
> -- 
> 2.21.0.5.gaeb582a
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 7/7] drm/i915/dsi: Initiate frame request in cmd mode
  2019-10-16 10:14   ` Ramalingam C
@ 2019-10-16 12:37     ` Kulkarni, Vandita
  0 siblings, 0 replies; 36+ messages in thread
From: Kulkarni, Vandita @ 2019-10-16 12:37 UTC (permalink / raw)
  To: C, Ramalingam; +Cc: Nikula, Jani, intel-gfx



> -----Original Message-----
> From: C, Ramalingam <ramalingam.c@intel.com>
> Sent: Wednesday, October 16, 2019 3:45 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>
> Subject: Re: [Intel-gfx] [RFC 7/7] drm/i915/dsi: Initiate frame request in cmd
> mode
> 
> On 2019-10-14 at 16:31:22 +0530, Vandita Kulkarni wrote:
> > In TE Gate mode, on every flip we need to set the frame update request
> > bit. After this  bit is set transcoder hardware will automatically
> > send the frame data to the panel when it receives the TE event.
> >
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/icl_dsi.c        | 27 +++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_display.c  | 16 +++++++++++
> >  .../drm/i915/display/intel_display_types.h    |  3 +++
> >  drivers/gpu/drm/i915/display/intel_dsi.h      |  3 +++
> >  4 files changed, 49 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> > b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 877746416e52..c72917ddf8e7 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -90,6 +90,33 @@ struct intel_encoder
> *gen11_dsi_find_cmd_mode_encoder(struct intel_crtc *crtc)
> >  	return NULL;
> >  }
> >
> > +void gen11_dsi_check_frame_update_needed(struct intel_crtc *intel_crtc,
> > +					 struct intel_crtc_state *crtc_state)
> 
> > +{
> > +	struct intel_encoder *intel_encoder = NULL;
> > +
> > +	intel_encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);
> > +	if (!intel_encoder)
> > +		return;
> In this func, if you could find a DSI encoder with CMD mode, you set the
> flag!?
Yes, the requirement is to set this frame update bit on every flip.
Like Jani has commented on patch 4 where this parsing function is defined, will need to reconsider and replace with a better solution.
Working on it. 

Thanks,
Vandita
> > +
> > +	/* TBD Use bits  to say update on which  dsi port instead of a bool */
> > +	crtc_state->dsi_frame_update = true; }
> > +
> > +void gen11_dsi_frame_update(struct intel_crtc_state *crtc_state) {
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +	u32 tmp;
> > +
> > +	/* TBD: add check on port */
> > +	if (crtc_state->dsi_frame_update) {
> > +		tmp = I915_READ(ICL_DSI_CMD_FRMCTL(PORT_A));
> > +		tmp |= ICL_FRAME_UPDATE_REQUEST;
> > +		I915_WRITE(ICL_DSI_CMD_FRMCTL(PORT_A), tmp);
> > +	}
> > +}
> > +
> >  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder
> > *encoder)  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); diff
> > --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 3cf39fc153b3..a902bb2bf075 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -11858,6 +11858,11 @@ static int intel_crtc_atomic_check(struct
> drm_crtc *_crtc,
> >  							 crtc_state);
> >  	}
> >
> > +	if ((INTEL_GEN(dev_priv) >= 11) &&
> > +		(intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))) {
> Please check this alignment too
> 
> -Ram
> > +		gen11_dsi_check_frame_update_needed(crtc, crtc_state);
> > +	}
> > +
> >  	if (HAS_IPS(dev_priv))
> >  		crtc_state->ips_enabled =
> hsw_compute_ips_config(crtc_state);
> >
> > @@ -13618,6 +13623,7 @@ static int intel_atomic_check(struct
> drm_device *dev,
> >  	if (ret)
> >  		goto fail;
> >
> > +
> >  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> >  					    new_crtc_state, i) {
> >  		if (!needs_modeset(new_crtc_state) && @@ -14108,6
> +14114,16 @@
> > static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> >  			intel_color_load_luts(new_crtc_state);
> >  	}
> >
> > +	/*
> > +	 * Incase of mipi dsi command mode, we need to set frame update
> > +	 * for every commit
> > +	 */
> > +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> > +		if (new_crtc_state->base.active &&
> > +		    !needs_modeset(new_crtc_state) &&
> > +		    new_crtc_state->dsi_frame_update)
> > +			gen11_dsi_frame_update(new_crtc_state);
> > +	}
> >  	/*
> >  	 * Now that the vblank has passed, we can go ahead and program
> the
> >  	 * optimal watermarks on platforms that need two-step watermark
> diff
> > --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 40390d855815..69da4ec45691 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -991,6 +991,9 @@ struct intel_crtc_state {
> >
> >  	/* Forward Error correction State */
> >  	bool fec_enable;
> > +
> > +	/* frame update for dsi command mode */
> > +	bool dsi_frame_update;
> >  };
> >
> >  struct intel_crtc {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dsi.h
> > b/drivers/gpu/drm/i915/display/intel_dsi.h
> > index 071dad7ee04a..d9350b842115 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dsi.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
> > @@ -203,6 +203,9 @@ void bxt_dsi_reset_clocks(struct intel_encoder
> > *encoder, enum port port);
> >
> >  /* icl_dsi.c */
> >  struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
> > intel_crtc *crtc);
> > +void gen11_dsi_check_frame_update_needed(struct intel_crtc *crtc,
> > +					 struct intel_crtc_state *crtc_state);
> void
> > +gen11_dsi_frame_update(struct intel_crtc_state *crtc_state);
> >
> >  /* intel_dsi_vbt.c */
> >  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
> > --
> > 2.21.0.5.gaeb582a
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 6/7] drm/i915/dsi: Add TE handler for dsi cmd mode.
  2019-10-16 10:24   ` Ramalingam C
@ 2019-10-16 12:46     ` Kulkarni, Vandita
  0 siblings, 0 replies; 36+ messages in thread
From: Kulkarni, Vandita @ 2019-10-16 12:46 UTC (permalink / raw)
  To: C, Ramalingam; +Cc: Nikula, Jani, intel-gfx



> -----Original Message-----
> From: C, Ramalingam <ramalingam.c@intel.com>
> Sent: Wednesday, October 16, 2019 3:55 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>
> Subject: Re: [Intel-gfx] [RFC 6/7] drm/i915/dsi: Add TE handler for dsi cmd
> mode.
> 
> On 2019-10-14 at 16:31:21 +0530, Vandita Kulkarni wrote:
> > In case of dual link, we get the TE on slave.
> > So clear the TE on slave DSI IIR.
> >
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 61
> > +++++++++++++++++++++++++++++++++
> >  1 file changed, 61 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c index bfb2a63504fb..d12efa72943b
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2628,6 +2628,61 @@ gen8_de_misc_irq_handler(struct
> drm_i915_private *dev_priv, u32 iir)
> >  		DRM_ERROR("Unexpected DE Misc interrupt\n");  }
> >
> > +void gen11_dsi_te_interrupt_handler(struct drm_i915_private *dev_priv,
> > +				    u32 iir_value)
> > +{
> > +	enum pipe pipe = INVALID_PIPE;
> > +	enum port port;
> > +	enum transcoder dsi_trans;
> > +	u32 val;
> usually we order the declarations based on the length.
Okay.
> > +
> > +	/*
> > +	 * Incase of dual link, TE comes from DSI_1
> > +	 * this is to check if dual link is enabled
> > +	 */
> > +	val = I915_READ(TRANS_DDI_FUNC_CTL2(TRANSCODER_DSI_0));
> > +	val &= PORT_SYNC_MODE_ENABLE;
> > +
> > +	/*
> > +	 * if dual link is enabled, then read DSI_0
> > +	 * transcoder registers
> > +	 */
> > +	port = ((iir_value & ICL_DSI_1) && val) || (iir_value & ICL_DSI_0) ?
> > +PORT_A : PORT_B;
> wrap it!? beyond 80char?
Okay.
> > +	dsi_trans = (port == PORT_A) ? TRANSCODER_DSI_0 :
> TRANSCODER_DSI_1;
> > +
> > +	/* Check if DSI configured in command mode */
> > +	val = I915_READ(DSI_TRANS_FUNC_CONF(dsi_trans));
> > +	val = (val & OP_MODE_MASK) >> 28;
> Could we use a macro, like GET_OP_MODE()?
Ok, will check.
> > +
> > +	if (val) {
> > +		DRM_ERROR("DSI trancoder not configured in command
> mode\n");
> > +		return;
> > +	}
> > +
> > +	/* Get PIPE for handling VBLANK event */
> > +	val = I915_READ(TRANS_DDI_FUNC_CTL(dsi_trans));
> > +	switch (val & TRANS_DDI_EDP_INPUT_MASK) {
> > +	case TRANS_DDI_EDP_INPUT_A_ON:
> > +		pipe = PIPE_A;
> > +		break;
> > +	case TRANS_DDI_EDP_INPUT_B_ONOFF:
> > +		pipe = PIPE_B;
> > +		break;
> > +	case TRANS_DDI_EDP_INPUT_C_ONOFF:
> > +		pipe = PIPE_C;
> > +		break;
> > +	default:
> > +		DRM_ERROR("Invalid PIPE\n");
> > +	}
> > +
> > +	/* clear TE in dsi IIR */
> > +	port = (iir_value & ICL_DSI_1) ? PORT_B : PORT_A;
> > +	val = I915_READ(ICL_DSI_INTR_IDENT_REG(port));
> > +	I915_WRITE((ICL_DSI_INTR_IDENT_REG(port)), val);
> extra () around ICL_DSI_INTR_IDENT_REG(port)
Okay.
> > +
> > +	drm_handle_vblank(&dev_priv->drm, pipe); }
> > +
> >  static irqreturn_t
> >  gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32
> > master_ctl)  { @@ -2692,6 +2747,12 @@ gen8_de_irq_handler(struct
> > drm_i915_private *dev_priv, u32 master_ctl)
> >  				found = true;
> >  			}
> >
> > +			if ((INTEL_GEN(dev_priv) >= 11) &&
> > +				(iir & (ICL_DSI_0 | ICL_DSI_1))) {
> alignment.
Will fix this, also it needs to be fixed with ICL_DSI_0_TE and ICL_DSI_1_TE.

Thanks
Vandita
> 
> -Ram
> > +				gen11_dsi_te_interrupt_handler(dev_priv,
> iir);
> > +				found = true;
> > +			}
> > +
> >  			if (!found)
> >  				DRM_ERROR("Unexpected DE Port
> interrupt\n");
> >  		}
> > --
> > 2.21.0.5.gaeb582a
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
  2019-10-15 19:20   ` Jani Nikula
@ 2019-10-16 13:27     ` Kulkarni, Vandita
  2019-10-24  9:07         ` [Intel-gfx] " Jani Nikula
  0 siblings, 1 reply; 36+ messages in thread
From: Kulkarni, Vandita @ 2019-10-16 13:27 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx



> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Wednesday, October 16, 2019 12:51 AM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>;
> Chauhan, Madhav <madhav.chauhan@intel.com>; Kulkarni, Vandita
> <vandita.kulkarni@intel.com>
> Subject: Re: [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
> 
> On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> > From: Madhav Chauhan <madhav.chauhan@intel.com>
> >
> > This patch adds a helper function to find encoder if DSI is operating
> > in command mode. This function will be used while enabling/disabling
> > TE interrupts for DSI.
> >
> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/icl_dsi.c   | 17 +++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_dsi.h |  3 +++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> > b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 5dd9eebab6b1..877746416e52 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -73,6 +73,23 @@ static enum transcoder
> dsi_port_to_transcoder(enum port port)
> >  		return TRANSCODER_DSI_1;
> >  }
> >
> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
> > +intel_crtc *crtc) {
> > +	struct drm_device *dev = crtc->base.dev;
> > +	struct intel_encoder *encoder;
> > +	struct intel_dsi *intel_dsi;
> > +
> > +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> > +		if (encoder->type != INTEL_OUTPUT_DSI)
> > +			continue;
> > +		intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +		if (intel_dsi->operation_mode ==
> INTEL_DSI_COMMAND_MODE)
> > +			return encoder;
> > +	}
> > +
> > +	return NULL;
> > +}
> 
> This may be a bit harsh, but everything that feels wrong about the following
> patches pretty much boils down to this function. It may get the job done,
> and I don't have a better suggestion on how to accomplish this right now.
> But it seems like we shouldn't have to do anything like this, and makes you
> feel like there's something wrong with the design.
> 
> It would be great to be able to handle this using crtc state, but alas the
> vblank enable hook only gets passed a struct drm_crtc *. (Patch 7 could
> easily switch to using crtc state, but need to also solve patch
> 5.)
.
> 
> I'll get back to you on this later, but in the mean time - Ville, do you have
> any ideas?

Other option of adding in crtc_state, did you mean something like this
https://patchwork.freedesktop.org/patch/336178/ 
@Jani and @Ville  Please let me know your comments.

Thanks,
Vandita
> 
> 
> BR,
> Jani.
> 
> 
> > +
> >  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder
> > *encoder)  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); diff
> > --git a/drivers/gpu/drm/i915/display/intel_dsi.h
> > b/drivers/gpu/drm/i915/display/intel_dsi.h
> > index b15be5814599..071dad7ee04a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dsi.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
> > @@ -201,6 +201,9 @@ u32 bxt_dsi_get_pclk(struct intel_encoder
> *encoder,
> >  		     struct intel_crtc_state *config);  void
> > bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
> >
> > +/* icl_dsi.c */
> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
> > +intel_crtc *crtc);
> > +
> >  /* intel_dsi_vbt.c */
> >  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
> > void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
@ 2019-10-24  9:07         ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-24  9:07 UTC (permalink / raw)
  To: Kulkarni, Vandita, intel-gfx

On Wed, 16 Oct 2019, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Wednesday, October 16, 2019 12:51 AM
>> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>;
>> Chauhan, Madhav <madhav.chauhan@intel.com>; Kulkarni, Vandita
>> <vandita.kulkarni@intel.com>
>> Subject: Re: [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
>> 
>> On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
>> > From: Madhav Chauhan <madhav.chauhan@intel.com>
>> >
>> > This patch adds a helper function to find encoder if DSI is operating
>> > in command mode. This function will be used while enabling/disabling
>> > TE interrupts for DSI.
>> >
>> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/icl_dsi.c   | 17 +++++++++++++++++
>> >  drivers/gpu/drm/i915/display/intel_dsi.h |  3 +++
>> >  2 files changed, 20 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>> > b/drivers/gpu/drm/i915/display/icl_dsi.c
>> > index 5dd9eebab6b1..877746416e52 100644
>> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> > @@ -73,6 +73,23 @@ static enum transcoder
>> dsi_port_to_transcoder(enum port port)
>> >  		return TRANSCODER_DSI_1;
>> >  }
>> >
>> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
>> > +intel_crtc *crtc) {
>> > +	struct drm_device *dev = crtc->base.dev;
>> > +	struct intel_encoder *encoder;
>> > +	struct intel_dsi *intel_dsi;
>> > +
>> > +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>> > +		if (encoder->type != INTEL_OUTPUT_DSI)
>> > +			continue;
>> > +		intel_dsi = enc_to_intel_dsi(&encoder->base);
>> > +		if (intel_dsi->operation_mode ==
>> INTEL_DSI_COMMAND_MODE)
>> > +			return encoder;
>> > +	}
>> > +
>> > +	return NULL;
>> > +}
>> 
>> This may be a bit harsh, but everything that feels wrong about the following
>> patches pretty much boils down to this function. It may get the job done,
>> and I don't have a better suggestion on how to accomplish this right now.
>> But it seems like we shouldn't have to do anything like this, and makes you
>> feel like there's something wrong with the design.
>> 
>> It would be great to be able to handle this using crtc state, but alas the
>> vblank enable hook only gets passed a struct drm_crtc *. (Patch 7 could
>> easily switch to using crtc state, but need to also solve patch
>> 5.)
> .
>> 
>> I'll get back to you on this later, but in the mean time - Ville, do you have
>> any ideas?
>
> Other option of adding in crtc_state, did you mean something like this
> https://patchwork.freedesktop.org/patch/336178/ 
> @Jani and @Ville  Please let me know your comments.

The problem is that we don't have access to the crtc state in patch 5
gen11_dsi_configure_te() because the call path is via drm_crtc_funcs and
bdw_enable_vblank.

BR,
Jani.


>
> Thanks,
> Vandita
>> 
>> 
>> BR,
>> Jani.
>> 
>> 
>> > +
>> >  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder
>> > *encoder)  {
>> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); diff
>> > --git a/drivers/gpu/drm/i915/display/intel_dsi.h
>> > b/drivers/gpu/drm/i915/display/intel_dsi.h
>> > index b15be5814599..071dad7ee04a 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dsi.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
>> > @@ -201,6 +201,9 @@ u32 bxt_dsi_get_pclk(struct intel_encoder
>> *encoder,
>> >  		     struct intel_crtc_state *config);  void
>> > bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
>> >
>> > +/* icl_dsi.c */
>> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
>> > +intel_crtc *crtc);
>> > +
>> >  /* intel_dsi_vbt.c */
>> >  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
>> > void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
>> 
>> --
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
@ 2019-10-24  9:07         ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-24  9:07 UTC (permalink / raw)
  To: Kulkarni, Vandita, intel-gfx

On Wed, 16 Oct 2019, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Wednesday, October 16, 2019 12:51 AM
>> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>;
>> Chauhan, Madhav <madhav.chauhan@intel.com>; Kulkarni, Vandita
>> <vandita.kulkarni@intel.com>
>> Subject: Re: [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
>> 
>> On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
>> > From: Madhav Chauhan <madhav.chauhan@intel.com>
>> >
>> > This patch adds a helper function to find encoder if DSI is operating
>> > in command mode. This function will be used while enabling/disabling
>> > TE interrupts for DSI.
>> >
>> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/icl_dsi.c   | 17 +++++++++++++++++
>> >  drivers/gpu/drm/i915/display/intel_dsi.h |  3 +++
>> >  2 files changed, 20 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>> > b/drivers/gpu/drm/i915/display/icl_dsi.c
>> > index 5dd9eebab6b1..877746416e52 100644
>> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> > @@ -73,6 +73,23 @@ static enum transcoder
>> dsi_port_to_transcoder(enum port port)
>> >  		return TRANSCODER_DSI_1;
>> >  }
>> >
>> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
>> > +intel_crtc *crtc) {
>> > +	struct drm_device *dev = crtc->base.dev;
>> > +	struct intel_encoder *encoder;
>> > +	struct intel_dsi *intel_dsi;
>> > +
>> > +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>> > +		if (encoder->type != INTEL_OUTPUT_DSI)
>> > +			continue;
>> > +		intel_dsi = enc_to_intel_dsi(&encoder->base);
>> > +		if (intel_dsi->operation_mode ==
>> INTEL_DSI_COMMAND_MODE)
>> > +			return encoder;
>> > +	}
>> > +
>> > +	return NULL;
>> > +}
>> 
>> This may be a bit harsh, but everything that feels wrong about the following
>> patches pretty much boils down to this function. It may get the job done,
>> and I don't have a better suggestion on how to accomplish this right now.
>> But it seems like we shouldn't have to do anything like this, and makes you
>> feel like there's something wrong with the design.
>> 
>> It would be great to be able to handle this using crtc state, but alas the
>> vblank enable hook only gets passed a struct drm_crtc *. (Patch 7 could
>> easily switch to using crtc state, but need to also solve patch
>> 5.)
> .
>> 
>> I'll get back to you on this later, but in the mean time - Ville, do you have
>> any ideas?
>
> Other option of adding in crtc_state, did you mean something like this
> https://patchwork.freedesktop.org/patch/336178/ 
> @Jani and @Ville  Please let me know your comments.

The problem is that we don't have access to the crtc state in patch 5
gen11_dsi_configure_te() because the call path is via drm_crtc_funcs and
bdw_enable_vblank.

BR,
Jani.


>
> Thanks,
> Vandita
>> 
>> 
>> BR,
>> Jani.
>> 
>> 
>> > +
>> >  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder
>> > *encoder)  {
>> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); diff
>> > --git a/drivers/gpu/drm/i915/display/intel_dsi.h
>> > b/drivers/gpu/drm/i915/display/intel_dsi.h
>> > index b15be5814599..071dad7ee04a 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dsi.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
>> > @@ -201,6 +201,9 @@ u32 bxt_dsi_get_pclk(struct intel_encoder
>> *encoder,
>> >  		     struct intel_crtc_state *config);  void
>> > bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
>> >
>> > +/* icl_dsi.c */
>> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
>> > +intel_crtc *crtc);
>> > +
>> >  /* intel_dsi_vbt.c */
>> >  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
>> > void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
>> 
>> --
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
@ 2019-10-24  9:11           ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-24  9:11 UTC (permalink / raw)
  To: Kulkarni, Vandita, intel-gfx

On Thu, 24 Oct 2019, Jani Nikula <jani.nikula@intel.com> wrote:
> On Wed, 16 Oct 2019, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote:
>>> -----Original Message-----
>>> From: Nikula, Jani <jani.nikula@intel.com>
>>> Sent: Wednesday, October 16, 2019 12:51 AM
>>> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
>>> gfx@lists.freedesktop.org
>>> Cc: ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>;
>>> Chauhan, Madhav <madhav.chauhan@intel.com>; Kulkarni, Vandita
>>> <vandita.kulkarni@intel.com>
>>> Subject: Re: [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
>>> 
>>> On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
>>> > From: Madhav Chauhan <madhav.chauhan@intel.com>
>>> >
>>> > This patch adds a helper function to find encoder if DSI is operating
>>> > in command mode. This function will be used while enabling/disabling
>>> > TE interrupts for DSI.
>>> >
>>> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>>> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
>>> > ---
>>> >  drivers/gpu/drm/i915/display/icl_dsi.c   | 17 +++++++++++++++++
>>> >  drivers/gpu/drm/i915/display/intel_dsi.h |  3 +++
>>> >  2 files changed, 20 insertions(+)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>>> > b/drivers/gpu/drm/i915/display/icl_dsi.c
>>> > index 5dd9eebab6b1..877746416e52 100644
>>> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>>> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>>> > @@ -73,6 +73,23 @@ static enum transcoder
>>> dsi_port_to_transcoder(enum port port)
>>> >  		return TRANSCODER_DSI_1;
>>> >  }
>>> >
>>> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
>>> > +intel_crtc *crtc) {
>>> > +	struct drm_device *dev = crtc->base.dev;
>>> > +	struct intel_encoder *encoder;
>>> > +	struct intel_dsi *intel_dsi;
>>> > +
>>> > +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>>> > +		if (encoder->type != INTEL_OUTPUT_DSI)
>>> > +			continue;
>>> > +		intel_dsi = enc_to_intel_dsi(&encoder->base);
>>> > +		if (intel_dsi->operation_mode ==
>>> INTEL_DSI_COMMAND_MODE)
>>> > +			return encoder;
>>> > +	}
>>> > +
>>> > +	return NULL;
>>> > +}
>>> 
>>> This may be a bit harsh, but everything that feels wrong about the following
>>> patches pretty much boils down to this function. It may get the job done,
>>> and I don't have a better suggestion on how to accomplish this right now.
>>> But it seems like we shouldn't have to do anything like this, and makes you
>>> feel like there's something wrong with the design.
>>> 
>>> It would be great to be able to handle this using crtc state, but alas the
>>> vblank enable hook only gets passed a struct drm_crtc *. (Patch 7 could
>>> easily switch to using crtc state, but need to also solve patch
>>> 5.)
>> .
>>> 
>>> I'll get back to you on this later, but in the mean time - Ville, do you have
>>> any ideas?
>>
>> Other option of adding in crtc_state, did you mean something like this
>> https://patchwork.freedesktop.org/patch/336178/ 
>> @Jani and @Ville  Please let me know your comments.
>
> The problem is that we don't have access to the crtc state in patch 5
> gen11_dsi_configure_te() because the call path is via drm_crtc_funcs and
> bdw_enable_vblank.

PS. I think moving some of this stuff to the crtc state is probably a
good thing no matter what. It's just that we *also* need some solution
to the problem in patch 5.


>
> BR,
> Jani.
>
>
>>
>> Thanks,
>> Vandita
>>> 
>>> 
>>> BR,
>>> Jani.
>>> 
>>> 
>>> > +
>>> >  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder
>>> > *encoder)  {
>>> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); diff
>>> > --git a/drivers/gpu/drm/i915/display/intel_dsi.h
>>> > b/drivers/gpu/drm/i915/display/intel_dsi.h
>>> > index b15be5814599..071dad7ee04a 100644
>>> > --- a/drivers/gpu/drm/i915/display/intel_dsi.h
>>> > +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
>>> > @@ -201,6 +201,9 @@ u32 bxt_dsi_get_pclk(struct intel_encoder
>>> *encoder,
>>> >  		     struct intel_crtc_state *config);  void
>>> > bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
>>> >
>>> > +/* icl_dsi.c */
>>> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
>>> > +intel_crtc *crtc);
>>> > +
>>> >  /* intel_dsi_vbt.c */
>>> >  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
>>> > void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
>>> 
>>> --
>>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
@ 2019-10-24  9:11           ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-24  9:11 UTC (permalink / raw)
  To: Kulkarni, Vandita, intel-gfx

On Thu, 24 Oct 2019, Jani Nikula <jani.nikula@intel.com> wrote:
> On Wed, 16 Oct 2019, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote:
>>> -----Original Message-----
>>> From: Nikula, Jani <jani.nikula@intel.com>
>>> Sent: Wednesday, October 16, 2019 12:51 AM
>>> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
>>> gfx@lists.freedesktop.org
>>> Cc: ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>;
>>> Chauhan, Madhav <madhav.chauhan@intel.com>; Kulkarni, Vandita
>>> <vandita.kulkarni@intel.com>
>>> Subject: Re: [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode
>>> 
>>> On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
>>> > From: Madhav Chauhan <madhav.chauhan@intel.com>
>>> >
>>> > This patch adds a helper function to find encoder if DSI is operating
>>> > in command mode. This function will be used while enabling/disabling
>>> > TE interrupts for DSI.
>>> >
>>> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>>> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
>>> > ---
>>> >  drivers/gpu/drm/i915/display/icl_dsi.c   | 17 +++++++++++++++++
>>> >  drivers/gpu/drm/i915/display/intel_dsi.h |  3 +++
>>> >  2 files changed, 20 insertions(+)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
>>> > b/drivers/gpu/drm/i915/display/icl_dsi.c
>>> > index 5dd9eebab6b1..877746416e52 100644
>>> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>>> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>>> > @@ -73,6 +73,23 @@ static enum transcoder
>>> dsi_port_to_transcoder(enum port port)
>>> >  		return TRANSCODER_DSI_1;
>>> >  }
>>> >
>>> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
>>> > +intel_crtc *crtc) {
>>> > +	struct drm_device *dev = crtc->base.dev;
>>> > +	struct intel_encoder *encoder;
>>> > +	struct intel_dsi *intel_dsi;
>>> > +
>>> > +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>>> > +		if (encoder->type != INTEL_OUTPUT_DSI)
>>> > +			continue;
>>> > +		intel_dsi = enc_to_intel_dsi(&encoder->base);
>>> > +		if (intel_dsi->operation_mode ==
>>> INTEL_DSI_COMMAND_MODE)
>>> > +			return encoder;
>>> > +	}
>>> > +
>>> > +	return NULL;
>>> > +}
>>> 
>>> This may be a bit harsh, but everything that feels wrong about the following
>>> patches pretty much boils down to this function. It may get the job done,
>>> and I don't have a better suggestion on how to accomplish this right now.
>>> But it seems like we shouldn't have to do anything like this, and makes you
>>> feel like there's something wrong with the design.
>>> 
>>> It would be great to be able to handle this using crtc state, but alas the
>>> vblank enable hook only gets passed a struct drm_crtc *. (Patch 7 could
>>> easily switch to using crtc state, but need to also solve patch
>>> 5.)
>> .
>>> 
>>> I'll get back to you on this later, but in the mean time - Ville, do you have
>>> any ideas?
>>
>> Other option of adding in crtc_state, did you mean something like this
>> https://patchwork.freedesktop.org/patch/336178/ 
>> @Jani and @Ville  Please let me know your comments.
>
> The problem is that we don't have access to the crtc state in patch 5
> gen11_dsi_configure_te() because the call path is via drm_crtc_funcs and
> bdw_enable_vblank.

PS. I think moving some of this stuff to the crtc state is probably a
good thing no matter what. It's just that we *also* need some solution
to the problem in patch 5.


>
> BR,
> Jani.
>
>
>>
>> Thanks,
>> Vandita
>>> 
>>> 
>>> BR,
>>> Jani.
>>> 
>>> 
>>> > +
>>> >  static void wait_for_cmds_dispatched_to_panel(struct intel_encoder
>>> > *encoder)  {
>>> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); diff
>>> > --git a/drivers/gpu/drm/i915/display/intel_dsi.h
>>> > b/drivers/gpu/drm/i915/display/intel_dsi.h
>>> > index b15be5814599..071dad7ee04a 100644
>>> > --- a/drivers/gpu/drm/i915/display/intel_dsi.h
>>> > +++ b/drivers/gpu/drm/i915/display/intel_dsi.h
>>> > @@ -201,6 +201,9 @@ u32 bxt_dsi_get_pclk(struct intel_encoder
>>> *encoder,
>>> >  		     struct intel_crtc_state *config);  void
>>> > bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port port);
>>> >
>>> > +/* icl_dsi.c */
>>> > +struct intel_encoder *gen11_dsi_find_cmd_mode_encoder(struct
>>> > +intel_crtc *crtc);
>>> > +
>>> >  /* intel_dsi_vbt.c */
>>> >  bool intel_dsi_vbt_init(struct intel_dsi *intel_dsi, u16 panel_id);
>>> > void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
>>> 
>>> --
>>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 5/7] drm/i915/dsi: Configure TE interrupt for cmd mode
@ 2019-10-24 11:34     ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-24 11:34 UTC (permalink / raw)
  To: Vandita Kulkarni, intel-gfx

On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> We need to configure TE interrupt in two places.
> Port interrupt and DSI interrupt mask registers.
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 49 ++++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 3af7f7914c40..bfb2a63504fb 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -41,6 +41,7 @@
>  #include "display/intel_hotplug.h"
>  #include "display/intel_lpe_audio.h"
>  #include "display/intel_psr.h"
> +#include "display/intel_dsi.h"
>  
>  #include "gt/intel_gt.h"
>  #include "gt/intel_gt_irq.h"
> @@ -2960,12 +2961,44 @@ int ilk_enable_vblank(struct drm_crtc *crtc)
>  	return 0;
>  }
>  
> +static void gen11_dsi_configure_te(struct drm_crtc *crtc, bool enable)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_encoder *encoder = NULL;
> +	struct intel_dsi *intel_dsi;
> +	enum port port;
> +	u32 tmp;
> +
> +	encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);

Ville's idea for checking for command mode on crtc is to use hwmode
private_flags. See e.g. __intel_get_crtc_scanline. It's not pretty
either though.

> +	if (!encoder)
> +		return;
> +
> +	intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	/* Assuming single link would always be enabled on PORT_A */

I don't think that's an assumption you can make.

> +	port = (intel_dsi->ports & BIT(PORT_B) & BIT(PORT_A)) ? PORT_B : PORT_A;

BIT(PORT_B) & BIT(PORT_A) is always 0.

> +	tmp =  I915_READ(ICL_DSI_INTR_MASK_REG(port));
> +	if (enable)
> +		tmp &= ~ICL_TE_EVENT;
> +	else
> +		tmp |= ICL_TE_EVENT;
> +
> +	I915_WRITE(ICL_DSI_INTR_MASK_REG(port), tmp);
> +}
> +
>  int bdw_enable_vblank(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>  	unsigned long irqflags;
>  
> +	if (INTEL_GEN(dev_priv) >= 11 &&
> +		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {
> +		gen11_dsi_configure_te(crtc, true);
> +		return 0;
> +	}
> +
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>  	bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> @@ -3031,9 +3064,16 @@ void ilk_disable_vblank(struct drm_crtc *crtc)
>  void bdw_disable_vblank(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> -	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	enum pipe pipe = intel_crtc->pipe;
>  	unsigned long irqflags;
>  
> +	if (INTEL_GEN(dev_priv) >= 11 &&
> +		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {
> +		gen11_dsi_configure_te(crtc, false);
> +		return;
> +	}
> +
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>  	bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> @@ -3726,6 +3766,13 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  		gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 11) {
> +		enum port port;
> +
> +		if (intel_bios_is_dsi_present(dev_priv, &port))
> +			de_port_masked |= ICL_DSI_0 | ICL_DSI_1;
> +	}
> +
>  	for_each_pipe(dev_priv, pipe) {
>  		dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [RFC 5/7] drm/i915/dsi: Configure TE interrupt for cmd mode
@ 2019-10-24 11:34     ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-24 11:34 UTC (permalink / raw)
  To: Vandita Kulkarni, intel-gfx

On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> We need to configure TE interrupt in two places.
> Port interrupt and DSI interrupt mask registers.
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 49 ++++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 3af7f7914c40..bfb2a63504fb 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -41,6 +41,7 @@
>  #include "display/intel_hotplug.h"
>  #include "display/intel_lpe_audio.h"
>  #include "display/intel_psr.h"
> +#include "display/intel_dsi.h"
>  
>  #include "gt/intel_gt.h"
>  #include "gt/intel_gt_irq.h"
> @@ -2960,12 +2961,44 @@ int ilk_enable_vblank(struct drm_crtc *crtc)
>  	return 0;
>  }
>  
> +static void gen11_dsi_configure_te(struct drm_crtc *crtc, bool enable)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_encoder *encoder = NULL;
> +	struct intel_dsi *intel_dsi;
> +	enum port port;
> +	u32 tmp;
> +
> +	encoder = gen11_dsi_find_cmd_mode_encoder(intel_crtc);

Ville's idea for checking for command mode on crtc is to use hwmode
private_flags. See e.g. __intel_get_crtc_scanline. It's not pretty
either though.

> +	if (!encoder)
> +		return;
> +
> +	intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	/* Assuming single link would always be enabled on PORT_A */

I don't think that's an assumption you can make.

> +	port = (intel_dsi->ports & BIT(PORT_B) & BIT(PORT_A)) ? PORT_B : PORT_A;

BIT(PORT_B) & BIT(PORT_A) is always 0.

> +	tmp =  I915_READ(ICL_DSI_INTR_MASK_REG(port));
> +	if (enable)
> +		tmp &= ~ICL_TE_EVENT;
> +	else
> +		tmp |= ICL_TE_EVENT;
> +
> +	I915_WRITE(ICL_DSI_INTR_MASK_REG(port), tmp);
> +}
> +
>  int bdw_enable_vblank(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>  	unsigned long irqflags;
>  
> +	if (INTEL_GEN(dev_priv) >= 11 &&
> +		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {
> +		gen11_dsi_configure_te(crtc, true);
> +		return 0;
> +	}
> +
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>  	bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> @@ -3031,9 +3064,16 @@ void ilk_disable_vblank(struct drm_crtc *crtc)
>  void bdw_disable_vblank(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> -	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	enum pipe pipe = intel_crtc->pipe;
>  	unsigned long irqflags;
>  
> +	if (INTEL_GEN(dev_priv) >= 11 &&
> +		(intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))) {
> +		gen11_dsi_configure_te(crtc, false);
> +		return;
> +	}
> +
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>  	bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> @@ -3726,6 +3766,13 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  		gen3_assert_iir_is_zero(uncore, EDP_PSR_IIR);
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 11) {
> +		enum port port;
> +
> +		if (intel_bios_is_dsi_present(dev_priv, &port))
> +			de_port_masked |= ICL_DSI_0 | ICL_DSI_1;
> +	}
> +
>  	for_each_pipe(dev_priv, pipe) {
>  		dev_priv->de_irq_mask[pipe] = ~de_pipe_masked;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode.
@ 2019-10-24 11:37     ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-24 11:37 UTC (permalink / raw)
  To: Vandita Kulkarni, intel-gfx

On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> Configure the transcoder to operate in TE GATE command mode
> and  take TE events from GPIO.
> Also disable the periodic command mode, that GOP would have
> programmed.

Discussing this with Ville, it just might be a good idea to enable
command mode *with* the periodic update first. It dodges a bunch of
issues wrt vblanks and scanlines, yet moves us forward with command
mode. So it might be a viable intermediate step.

BR,
Jani.

>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c | 32 ++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..8e6c09a1db78 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -704,6 +704,10 @@ gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
>  				tmp |= VIDEO_MODE_SYNC_PULSE;
>  				break;
>  			}
> +		} else {
> +			tmp &= ~OP_MODE_MASK;
> +			tmp |= CMD_MODE_TE_GATE;
> +			tmp |= TE_SOURCE_GPIO;
>  		}
>  
>  		I915_WRITE(DSI_TRANS_FUNC_CONF(dsi_trans), tmp);
> @@ -953,6 +957,22 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder)
>  	}
>  }
>  
> +static void gen11_dsi_config_util_pin(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	u32 tmp;
> +
> +	/* used only as TE i/p for DSI0 for dual link TE is from slave DSI1 */
> +	if (is_vid_mode(intel_dsi) || (intel_dsi->dual_link))
> +		return;
> +
> +	tmp = I915_READ(UTIL_PIN_CTL);
> +	tmp |= ICL_UTIL_PIN_DIRECTION;
> +	tmp |= UTIL_PIN_ENABLE;
> +	I915_WRITE(UTIL_PIN_CTL, tmp);
> +}
> +
>  static void
>  gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
>  			      const struct intel_crtc_state *pipe_config)
> @@ -974,6 +994,9 @@ gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
>  	/* setup D-PHY timings */
>  	gen11_dsi_setup_dphy_timings(encoder);
>  
> +	/* Since transcoder is configured to take events from GPIO */
> +	gen11_dsi_config_util_pin(encoder);
> +
>  	/* step 4h: setup DSI protocol timeouts */
>  	gen11_dsi_setup_timeouts(encoder);
>  
> @@ -1104,6 +1127,15 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
>  	enum transcoder dsi_trans;
>  	u32 tmp;
>  
> +	/* disable periodic update mode */
> +	if (is_cmd_mode(intel_dsi)) {
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			tmp = I915_READ(ICL_DSI_CMD_FRMCTL(port));
> +			tmp &= ~ICL_PERIODIC_FRAME_UPDATE_ENABLE;
> +			I915_WRITE(ICL_DSI_CMD_FRMCTL(port), tmp);
> +		}
> +	}
> +
>  	/* put dsi link in ULPS */
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		dsi_trans = dsi_port_to_transcoder(port);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode.
@ 2019-10-24 11:37     ` Jani Nikula
  0 siblings, 0 replies; 36+ messages in thread
From: Jani Nikula @ 2019-10-24 11:37 UTC (permalink / raw)
  To: Vandita Kulkarni, intel-gfx

On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> Configure the transcoder to operate in TE GATE command mode
> and  take TE events from GPIO.
> Also disable the periodic command mode, that GOP would have
> programmed.

Discussing this with Ville, it just might be a good idea to enable
command mode *with* the periodic update first. It dodges a bunch of
issues wrt vblanks and scanlines, yet moves us forward with command
mode. So it might be a viable intermediate step.

BR,
Jani.

>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c | 32 ++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..8e6c09a1db78 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -704,6 +704,10 @@ gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
>  				tmp |= VIDEO_MODE_SYNC_PULSE;
>  				break;
>  			}
> +		} else {
> +			tmp &= ~OP_MODE_MASK;
> +			tmp |= CMD_MODE_TE_GATE;
> +			tmp |= TE_SOURCE_GPIO;
>  		}
>  
>  		I915_WRITE(DSI_TRANS_FUNC_CONF(dsi_trans), tmp);
> @@ -953,6 +957,22 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder)
>  	}
>  }
>  
> +static void gen11_dsi_config_util_pin(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	u32 tmp;
> +
> +	/* used only as TE i/p for DSI0 for dual link TE is from slave DSI1 */
> +	if (is_vid_mode(intel_dsi) || (intel_dsi->dual_link))
> +		return;
> +
> +	tmp = I915_READ(UTIL_PIN_CTL);
> +	tmp |= ICL_UTIL_PIN_DIRECTION;
> +	tmp |= UTIL_PIN_ENABLE;
> +	I915_WRITE(UTIL_PIN_CTL, tmp);
> +}
> +
>  static void
>  gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
>  			      const struct intel_crtc_state *pipe_config)
> @@ -974,6 +994,9 @@ gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
>  	/* setup D-PHY timings */
>  	gen11_dsi_setup_dphy_timings(encoder);
>  
> +	/* Since transcoder is configured to take events from GPIO */
> +	gen11_dsi_config_util_pin(encoder);
> +
>  	/* step 4h: setup DSI protocol timeouts */
>  	gen11_dsi_setup_timeouts(encoder);
>  
> @@ -1104,6 +1127,15 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
>  	enum transcoder dsi_trans;
>  	u32 tmp;
>  
> +	/* disable periodic update mode */
> +	if (is_cmd_mode(intel_dsi)) {
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			tmp = I915_READ(ICL_DSI_CMD_FRMCTL(port));
> +			tmp &= ~ICL_PERIODIC_FRAME_UPDATE_ENABLE;
> +			I915_WRITE(ICL_DSI_CMD_FRMCTL(port), tmp);
> +		}
> +	}
> +
>  	/* put dsi link in ULPS */
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		dsi_trans = dsi_port_to_transcoder(port);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode.
@ 2019-10-24 11:52       ` Kulkarni, Vandita
  0 siblings, 0 replies; 36+ messages in thread
From: Kulkarni, Vandita @ 2019-10-24 11:52 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx

> -----Original Message-----
> From: Jani Nikula <jani.nikula@intel.com>
> Sent: Thursday, October 24, 2019 5:08 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>;
> Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Subject: Re: [RFC 2/7] drm/i915/dsi: Configure transcoder operation for
> command mode.
> 
> On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> > Configure the transcoder to operate in TE GATE command mode and  take
> > TE events from GPIO.
> > Also disable the periodic command mode, that GOP would have
> > programmed.
> 
> Discussing this with Ville, it just might be a good idea to enable command
> mode *with* the periodic update first. It dodges a bunch of issues wrt
> vblanks and scanlines, yet moves us forward with command mode. So it
> might be a viable intermediate step.
That would mean enable vblank and not get TE. I will check with this.

Thanks,
Vandita
> 
> BR,
> Jani.
> 
> >
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/icl_dsi.c | 32
> > ++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> > b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 6e398c33a524..8e6c09a1db78 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -704,6 +704,10 @@ gen11_dsi_configure_transcoder(struct
> intel_encoder *encoder,
> >  				tmp |= VIDEO_MODE_SYNC_PULSE;
> >  				break;
> >  			}
> > +		} else {
> > +			tmp &= ~OP_MODE_MASK;
> > +			tmp |= CMD_MODE_TE_GATE;
> > +			tmp |= TE_SOURCE_GPIO;
> >  		}
> >
> >  		I915_WRITE(DSI_TRANS_FUNC_CONF(dsi_trans), tmp); @@ -
> 953,6 +957,22
> > @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder)
> >  	}
> >  }
> >
> > +static void gen11_dsi_config_util_pin(struct intel_encoder *encoder)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	u32 tmp;
> > +
> > +	/* used only as TE i/p for DSI0 for dual link TE is from slave DSI1 */
> > +	if (is_vid_mode(intel_dsi) || (intel_dsi->dual_link))
> > +		return;
> > +
> > +	tmp = I915_READ(UTIL_PIN_CTL);
> > +	tmp |= ICL_UTIL_PIN_DIRECTION;
> > +	tmp |= UTIL_PIN_ENABLE;
> > +	I915_WRITE(UTIL_PIN_CTL, tmp);
> > +}
> > +
> >  static void
> >  gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
> >  			      const struct intel_crtc_state *pipe_config) @@ -
> 974,6 +994,9
> > @@ gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
> >  	/* setup D-PHY timings */
> >  	gen11_dsi_setup_dphy_timings(encoder);
> >
> > +	/* Since transcoder is configured to take events from GPIO */
> > +	gen11_dsi_config_util_pin(encoder);
> > +
> >  	/* step 4h: setup DSI protocol timeouts */
> >  	gen11_dsi_setup_timeouts(encoder);
> >
> > @@ -1104,6 +1127,15 @@ static void
> gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
> >  	enum transcoder dsi_trans;
> >  	u32 tmp;
> >
> > +	/* disable periodic update mode */
> > +	if (is_cmd_mode(intel_dsi)) {
> > +		for_each_dsi_port(port, intel_dsi->ports) {
> > +			tmp = I915_READ(ICL_DSI_CMD_FRMCTL(port));
> > +			tmp &= ~ICL_PERIODIC_FRAME_UPDATE_ENABLE;
> > +			I915_WRITE(ICL_DSI_CMD_FRMCTL(port), tmp);
> > +		}
> > +	}
> > +
> >  	/* put dsi link in ULPS */
> >  	for_each_dsi_port(port, intel_dsi->ports) {
> >  		dsi_trans = dsi_port_to_transcoder(port);
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Intel-gfx] [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode.
@ 2019-10-24 11:52       ` Kulkarni, Vandita
  0 siblings, 0 replies; 36+ messages in thread
From: Kulkarni, Vandita @ 2019-10-24 11:52 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx

> -----Original Message-----
> From: Jani Nikula <jani.nikula@intel.com>
> Sent: Thursday, October 24, 2019 5:08 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>;
> Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Subject: Re: [RFC 2/7] drm/i915/dsi: Configure transcoder operation for
> command mode.
> 
> On Mon, 14 Oct 2019, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> > Configure the transcoder to operate in TE GATE command mode and  take
> > TE events from GPIO.
> > Also disable the periodic command mode, that GOP would have
> > programmed.
> 
> Discussing this with Ville, it just might be a good idea to enable command
> mode *with* the periodic update first. It dodges a bunch of issues wrt
> vblanks and scanlines, yet moves us forward with command mode. So it
> might be a viable intermediate step.
That would mean enable vblank and not get TE. I will check with this.

Thanks,
Vandita
> 
> BR,
> Jani.
> 
> >
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/icl_dsi.c | 32
> > ++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> > b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 6e398c33a524..8e6c09a1db78 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -704,6 +704,10 @@ gen11_dsi_configure_transcoder(struct
> intel_encoder *encoder,
> >  				tmp |= VIDEO_MODE_SYNC_PULSE;
> >  				break;
> >  			}
> > +		} else {
> > +			tmp &= ~OP_MODE_MASK;
> > +			tmp |= CMD_MODE_TE_GATE;
> > +			tmp |= TE_SOURCE_GPIO;
> >  		}
> >
> >  		I915_WRITE(DSI_TRANS_FUNC_CONF(dsi_trans), tmp); @@ -
> 953,6 +957,22
> > @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder)
> >  	}
> >  }
> >
> > +static void gen11_dsi_config_util_pin(struct intel_encoder *encoder)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	u32 tmp;
> > +
> > +	/* used only as TE i/p for DSI0 for dual link TE is from slave DSI1 */
> > +	if (is_vid_mode(intel_dsi) || (intel_dsi->dual_link))
> > +		return;
> > +
> > +	tmp = I915_READ(UTIL_PIN_CTL);
> > +	tmp |= ICL_UTIL_PIN_DIRECTION;
> > +	tmp |= UTIL_PIN_ENABLE;
> > +	I915_WRITE(UTIL_PIN_CTL, tmp);
> > +}
> > +
> >  static void
> >  gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
> >  			      const struct intel_crtc_state *pipe_config) @@ -
> 974,6 +994,9
> > @@ gen11_dsi_enable_port_and_phy(struct intel_encoder *encoder,
> >  	/* setup D-PHY timings */
> >  	gen11_dsi_setup_dphy_timings(encoder);
> >
> > +	/* Since transcoder is configured to take events from GPIO */
> > +	gen11_dsi_config_util_pin(encoder);
> > +
> >  	/* step 4h: setup DSI protocol timeouts */
> >  	gen11_dsi_setup_timeouts(encoder);
> >
> > @@ -1104,6 +1127,15 @@ static void
> gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
> >  	enum transcoder dsi_trans;
> >  	u32 tmp;
> >
> > +	/* disable periodic update mode */
> > +	if (is_cmd_mode(intel_dsi)) {
> > +		for_each_dsi_port(port, intel_dsi->ports) {
> > +			tmp = I915_READ(ICL_DSI_CMD_FRMCTL(port));
> > +			tmp &= ~ICL_PERIODIC_FRAME_UPDATE_ENABLE;
> > +			I915_WRITE(ICL_DSI_CMD_FRMCTL(port), tmp);
> > +		}
> > +	}
> > +
> >  	/* put dsi link in ULPS */
> >  	for_each_dsi_port(port, intel_dsi->ports) {
> >  		dsi_trans = dsi_port_to_transcoder(port);
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2019-10-24 11:52 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 11:01 [RFC 0/7] Add mipi dsi command mode support Vandita Kulkarni
2019-10-14 11:01 ` [RFC 1/7] drm/i915/dsi: Define command mode registers Vandita Kulkarni
2019-10-14 16:18   ` Ramalingam C
2019-10-15  7:52     ` Kulkarni, Vandita
2019-10-15  7:07   ` Jani Nikula
2019-10-15  7:50     ` Kulkarni, Vandita
2019-10-14 11:01 ` [RFC 2/7] drm/i915/dsi: Configure transcoder operation for command mode Vandita Kulkarni
2019-10-15 18:35   ` Jani Nikula
2019-10-24 11:37   ` Jani Nikula
2019-10-24 11:37     ` [Intel-gfx] " Jani Nikula
2019-10-24 11:52     ` Kulkarni, Vandita
2019-10-24 11:52       ` [Intel-gfx] " Kulkarni, Vandita
2019-10-14 11:01 ` [RFC 3/7] drm/i915/dsi: Add vblank calculation " Vandita Kulkarni
2019-10-15 18:45   ` Jani Nikula
2019-10-14 11:01 ` [RFC 4/7] drm/i915/dsi: Helper to find dsi encoder in cmd mode Vandita Kulkarni
2019-10-15 19:20   ` Jani Nikula
2019-10-16 13:27     ` Kulkarni, Vandita
2019-10-24  9:07       ` Jani Nikula
2019-10-24  9:07         ` [Intel-gfx] " Jani Nikula
2019-10-24  9:11         ` Jani Nikula
2019-10-24  9:11           ` [Intel-gfx] " Jani Nikula
2019-10-14 11:01 ` [RFC 5/7] drm/i915/dsi: Configure TE interrupt for " Vandita Kulkarni
2019-10-16  9:56   ` Ramalingam C
2019-10-24 11:34   ` Jani Nikula
2019-10-24 11:34     ` [Intel-gfx] " Jani Nikula
2019-10-14 11:01 ` [RFC 6/7] drm/i915/dsi: Add TE handler for dsi " Vandita Kulkarni
2019-10-15  8:28   ` Kulkarni, Vandita
2019-10-16 10:24   ` Ramalingam C
2019-10-16 12:46     ` Kulkarni, Vandita
2019-10-14 11:01 ` [RFC 7/7] drm/i915/dsi: Initiate frame request in " Vandita Kulkarni
2019-10-16 10:14   ` Ramalingam C
2019-10-16 12:37     ` Kulkarni, Vandita
2019-10-14 16:21 ` ✗ Fi.CI.CHECKPATCH: warning for Add mipi dsi command mode support Patchwork
2019-10-14 16:24 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-10-14 17:07 ` ✓ Fi.CI.BAT: success " Patchwork
2019-10-15  0:42 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.