[PATCH RESEND 1/7] drm/i915/dsi: don't debug log "missing" sequences

[PATCH RESEND 1/7] drm/i915/dsi: don't debug log "missing" sequences

[Jani Nikula]

This is not interesting. They are not "missing", they are just not part
of the VBT sequences for the panel.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index f122484bedfc..4d05f0bfaea3 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -385,11 +385,8 @@ static void generic_exec_sequence(struct drm_panel *panel, enum mipi_seq seq_id)
 		return;
 
 	data = dev_priv->vbt.dsi.sequence[seq_id];
-	if (!data) {
-		DRM_DEBUG_KMS("MIPI sequence %d - %s not available\n",
-			      seq_id, sequence_name(seq_id));
+	if (!data)
 		return;
-	}
 
 	WARN_ON(*data != seq_id);
 
-- 
2.1.4

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

[PATCH RESEND 2/7] drm/i915/dsi: add debug logging to element execution

[Jani Nikula]

Just simple breadcrumbs for now. While at it, rename the i2c skip
function.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 4d05f0bfaea3..3e840a526f53 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -126,6 +126,8 @@ static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi,
 	u16 len;
 	enum port port;
 
+	DRM_DEBUG_KMS("\n");
+
 	flags = *data++;
 	type = *data++;
 
@@ -199,6 +201,8 @@ static const u8 *mipi_exec_delay(struct intel_dsi *intel_dsi, const u8 *data)
 {
 	u32 delay = *((const u32 *) data);
 
+	DRM_DEBUG_KMS("\n");
+
 	usleep_range(delay, delay + 10);
 	data += 4;
 
@@ -307,6 +311,8 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
 	u8 gpio_source, gpio_index;
 	bool value;
 
+	DRM_DEBUG_KMS("\n");
+
 	if (dev_priv->vbt.dsi.seq_version >= 3)
 		data++;
 
@@ -331,8 +337,10 @@ static const u8 *mipi_exec_gpio(struct intel_dsi *intel_dsi, const u8 *data)
 	return data;
 }
 
-static const u8 *mipi_exec_i2c_skip(struct intel_dsi *intel_dsi, const u8 *data)
+static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
 {
+	DRM_DEBUG_KMS("Skipping I2C element execution\n");
+
 	return data + *(data + 6) + 7;
 }
 
@@ -342,7 +350,7 @@ static const fn_mipi_elem_exec exec_elem[] = {
 	[MIPI_SEQ_ELEM_SEND_PKT] = mipi_exec_send_packet,
 	[MIPI_SEQ_ELEM_DELAY] = mipi_exec_delay,
 	[MIPI_SEQ_ELEM_GPIO] = mipi_exec_gpio,
-	[MIPI_SEQ_ELEM_I2C] = mipi_exec_i2c_skip,
+	[MIPI_SEQ_ELEM_I2C] = mipi_exec_i2c,
 };
 
 /*
-- 
2.1.4

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

[PATCH RESEND 3/7] drm/i915/dsi: add skip functions for spi and pmic elements

[Jani Nikula]

In sequence block v3 these are gracefully skipped anyway, but add the
functions so we can have some debug breadcrumbs.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 3e840a526f53..7dd850760c4d 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -344,6 +344,20 @@ static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
 	return data + *(data + 6) + 7;
 }
 
+static const u8 *mipi_exec_spi(struct intel_dsi *intel_dsi, const u8 *data)
+{
+	DRM_DEBUG_KMS("Skipping SPI element execution\n");
+
+	return data + *(data + 5) + 6;
+}
+
+static const u8 *mipi_exec_pmic(struct intel_dsi *intel_dsi, const u8 *data)
+{
+	DRM_DEBUG_KMS("Skipping PMIC element execution\n");
+
+	return data + 14;
+}
+
 typedef const u8 * (*fn_mipi_elem_exec)(struct intel_dsi *intel_dsi,
 					const u8 *data);
 static const fn_mipi_elem_exec exec_elem[] = {
@@ -351,6 +365,8 @@ static const fn_mipi_elem_exec exec_elem[] = {
 	[MIPI_SEQ_ELEM_DELAY] = mipi_exec_delay,
 	[MIPI_SEQ_ELEM_GPIO] = mipi_exec_gpio,
 	[MIPI_SEQ_ELEM_I2C] = mipi_exec_i2c,
+	[MIPI_SEQ_ELEM_SPI] = mipi_exec_spi,
+	[MIPI_SEQ_ELEM_PMIC] = mipi_exec_pmic,
 };
 
 /*
-- 
2.1.4

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

[PATCH RESEND 4/7] drm/i915/dsi: run power on/off sequences in panel prepare/unprepare hooks

[Jani Nikula]

Based on the documentation alone, it's anyone's guess when exactly we
should be running these sequences. Add them where it feels logical. The
drm panel hooks don't currently offer us more granularity anyway.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 7dd850760c4d..e0337a82a6b4 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -459,6 +459,7 @@ static void generic_exec_sequence(struct drm_panel *panel, enum mipi_seq seq_id)
 static int vbt_panel_prepare(struct drm_panel *panel)
 {
 	generic_exec_sequence(panel, MIPI_SEQ_ASSERT_RESET);
+	generic_exec_sequence(panel, MIPI_SEQ_POWER_ON);
 	generic_exec_sequence(panel, MIPI_SEQ_INIT_OTP);
 
 	return 0;
@@ -466,6 +467,7 @@ static int vbt_panel_prepare(struct drm_panel *panel)
 
 static int vbt_panel_unprepare(struct drm_panel *panel)
 {
+	generic_exec_sequence(panel, MIPI_SEQ_POWER_OFF);
 	generic_exec_sequence(panel, MIPI_SEQ_DEASSERT_RESET);
 
 	return 0;
-- 
2.1.4

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

[PATCH RESEND 5/7] drm/i915/dsi: run backlight on/off sequences in panel enable/disable hooks

[Jani Nikula]

Based on the documentation alone, it's anyone's guess when exactly we
should be running these sequences. Add them where it feels logical. The
drm panel hooks don't currently offer us more granularity anyway.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index e0337a82a6b4..d078c5765014 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -476,12 +476,14 @@ static int vbt_panel_unprepare(struct drm_panel *panel)
 static int vbt_panel_enable(struct drm_panel *panel)
 {
 	generic_exec_sequence(panel, MIPI_SEQ_DISPLAY_ON);
+	generic_exec_sequence(panel, MIPI_SEQ_BACKLIGHT_ON);
 
 	return 0;
 }
 
 static int vbt_panel_disable(struct drm_panel *panel)
 {
+	generic_exec_sequence(panel, MIPI_SEQ_BACKLIGHT_OFF);
 	generic_exec_sequence(panel, MIPI_SEQ_DISPLAY_OFF);
 
 	return 0;
-- 
2.1.4

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

[PATCH RESEND 6/7] drm/i915/bios: log about presence of DSI sequences we do not run

[Jani Nikula]

Leave behind some debugging clues in case some panels don't work
properly.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index da5ed4a850b9..e2f47ff156a7 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -996,6 +996,10 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
 			goto err;
 		}
 
+		/* Log about presence of sequences we won't run. */
+		if (seq_id == MIPI_SEQ_TEAR_ON || seq_id == MIPI_SEQ_TEAR_OFF)
+			DRM_DEBUG_KMS("Unsupported sequence %u\n", seq_id);
+
 		dev_priv->vbt.dsi.sequence[seq_id] = data + index;
 
 		if (sequence->version >= 3)
-- 
2.1.4

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

[PATCH RESEND 7/7] drm/i915/dsi: double check element parsing against size if present

[Jani Nikula]

Be a little paranoid in case the specs change or something.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index d078c5765014..30fcb02fce9b 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -441,7 +441,15 @@ static void generic_exec_sequence(struct drm_panel *panel, enum mipi_seq seq_id)
 			operation_size = *data++;
 
 		if (mipi_elem_exec) {
+			const u8 *next = data + operation_size;
+
 			data = mipi_elem_exec(intel_dsi, data);
+
+			/* Consistency check if we have size. */
+			if (operation_size && data != next) {
+				DRM_ERROR("Inconsistent operation size\n");
+				return;
+			}
 		} else if (operation_size) {
 			/* We have size, skip. */
 			DRM_DEBUG_KMS("Unsupported MIPI operation byte %u\n",
-- 
2.1.4

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

✓ Ro.CI.BAT: success for series starting with [RESEND,1/7] drm/i915/dsi: don't debug log "missing" sequences

[Patchwork]

== Series Details ==

Series: series starting with [RESEND,1/7] drm/i915/dsi: don't debug log "missing" sequences
URL   : https://patchwork.freedesktop.org/series/8611/
State : success

== Summary ==

Series 8611v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/8611/revisions/1/mbox

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-bdw-i7-5557u  total:213  pass:201  dwarn:0   dfail:0   fail:0   skip:12 
fi-skl-i5-6260u  total:213  pass:202  dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-i7-6700k  total:213  pass:188  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:213  pass:174  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:213  pass:197  dwarn:2   dfail:0   fail:0   skip:14 
ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:213  pass:173  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:213  pass:149  dwarn:0   dfail:0   fail:2   skip:62 
ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28 
ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0   skip:11 
ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38 
fi-hsw-i7-4770k failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1170/

5aacd93 drm-intel-nightly: 2016y-06m-13d-09h-05m-21s UTC integration manifest
862c3d1 drm/i915/dsi: double check element parsing against size if present
be3d233 drm/i915/bios: log about presence of DSI sequences we do not run
a699949 drm/i915/dsi: run backlight on/off sequences in panel enable/disable hooks
b2b9d4e drm/i915/dsi: run power on/off sequences in panel prepare/unprepare hooks
5ad85d5 drm/i915/dsi: add skip functions for spi and pmic elements
afb1e77 drm/i915/dsi: add debug logging to element execution
9d6140b drm/i915/dsi: don't debug log "missing" sequences

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

Re: [PATCH RESEND 3/7] drm/i915/dsi: add skip functions for spi and pmic elements

[Ville Syrjälä]

On Mon, Jun 13, 2016 at 01:22:14PM +0300, Jani Nikula wrote:
> In sequence block v3 these are gracefully skipped anyway, but add the
> functions so we can have some debug breadcrumbs.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 3e840a526f53..7dd850760c4d 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -344,6 +344,20 @@ static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
>  	return data + *(data + 6) + 7;
>  }
>  
> +static const u8 *mipi_exec_spi(struct intel_dsi *intel_dsi, const u8 *data)
> +{
> +	DRM_DEBUG_KMS("Skipping SPI element execution\n");
> +
> +	return data + *(data + 5) + 6;
> +}
> +
> +static const u8 *mipi_exec_pmic(struct intel_dsi *intel_dsi, const u8 *data)
> +{
> +	DRM_DEBUG_KMS("Skipping PMIC element execution\n");
> +
> +	return data + 14;

Should that be 15?

> +}
> +
>  typedef const u8 * (*fn_mipi_elem_exec)(struct intel_dsi *intel_dsi,
>  					const u8 *data);
>  static const fn_mipi_elem_exec exec_elem[] = {
> @@ -351,6 +365,8 @@ static const fn_mipi_elem_exec exec_elem[] = {
>  	[MIPI_SEQ_ELEM_DELAY] = mipi_exec_delay,
>  	[MIPI_SEQ_ELEM_GPIO] = mipi_exec_gpio,
>  	[MIPI_SEQ_ELEM_I2C] = mipi_exec_i2c,
> +	[MIPI_SEQ_ELEM_SPI] = mipi_exec_spi,
> +	[MIPI_SEQ_ELEM_PMIC] = mipi_exec_pmic,
>  };
>  
>  /*
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH RESEND 4/7] drm/i915/dsi: run power on/off sequences in panel prepare/unprepare hooks

[Ville Syrjälä]

On Mon, Jun 13, 2016 at 01:22:15PM +0300, Jani Nikula wrote:
> Based on the documentation alone, it's anyone's guess when exactly we
> should be running these sequences. Add them where it feels logical. The
> drm panel hooks don't currently offer us more granularity anyway.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 7dd850760c4d..e0337a82a6b4 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -459,6 +459,7 @@ static void generic_exec_sequence(struct drm_panel *panel, enum mipi_seq seq_id)
>  static int vbt_panel_prepare(struct drm_panel *panel)
>  {
>  	generic_exec_sequence(panel, MIPI_SEQ_ASSERT_RESET);
> +	generic_exec_sequence(panel, MIPI_SEQ_POWER_ON);
>  	generic_exec_sequence(panel, MIPI_SEQ_INIT_OTP);

Indeed, our spec is useless here :(

What would make sense to me is

assert reset // just in case it wasn't already asserted
power on
deassert reset
init otp

>  
>  	return 0;
> @@ -466,6 +467,7 @@ static int vbt_panel_prepare(struct drm_panel *panel)
>  
>  static int vbt_panel_unprepare(struct drm_panel *panel)
>  {
> +	generic_exec_sequence(panel, MIPI_SEQ_POWER_OFF);
>  	generic_exec_sequence(panel, MIPI_SEQ_DEASSERT_RESET);

And here I'd do 

assert reset
power off

and nothing more.

This would also seem to agree with a few panel specs I have lying
around.

But of course I could be mistaken, eg. if the panel on/off sequeneces
themselves would had some kind of magic reset (de)assert already inside
them. Nothing in the spec suggests this though.

>  
>  	return 0;
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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