[PATCH 1/2] drm/i915: Free memdup-ed bios data structures on driver_unload

[PATCH 1/2] drm/i915: Free memdup-ed bios data structures on driver_unload

[Hans de Goede]

Add a new intel_bios_cleanup function to free memdup-ed bios data
structures and call it from i915_driver_unload().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/i915_drv.c   |  2 ++
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 11 +++++++++++
 3 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1ec12add34b2..4ecf41724183 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1437,6 +1437,8 @@ void i915_driver_unload(struct drm_device *dev)
 
 	intel_modeset_cleanup(dev);
 
+	intel_bios_cleanup(dev_priv);
+
 	/*
 	 * free the memory space allocated for the child device
 	 * config parsed from VBT
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 454d8f937fae..081190da0818 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3663,6 +3663,7 @@ extern void intel_i2c_reset(struct drm_i915_private *dev_priv);
 
 /* intel_bios.c */
 void intel_bios_init(struct drm_i915_private *dev_priv);
+void intel_bios_cleanup(struct drm_i915_private *dev_priv);
 bool intel_bios_is_valid_vbt(const void *buf, size_t size);
 bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 95f0b310d656..64a0d55df28e 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1588,6 +1588,17 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
 		pci_unmap_rom(pdev, bios);
 }
 
+/**
+ * intel_bios_cleanup - Free any resources allocated by intel_bios_init()
+ * @dev_priv: i915 device instance
+ */
+void intel_bios_cleanup(struct drm_i915_private *dev_priv)
+{
+	kfree(dev_priv->vbt.dsi.data);
+	kfree(dev_priv->vbt.dsi.pps);
+	kfree(dev_priv->vbt.dsi.config);
+}
+
 /**
  * intel_bios_is_tv_present - is integrated TV present in VBT
  * @dev_priv:	i915 device instance
-- 
2.14.3

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

[PATCH 2/2] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3

[Hans de Goede]

So far models of the Dell Venue 8 Pro, with a panel with MIPI panel
index = 3, one of which has been kindly provided to me by Jan Brummer,
where not working with the i915 driver, giving a black screen on the
first modeset.

The problem with at least these Dells is that their VBT defines a MIPI
ASSERT sequence, but not a DEASSERT sequence. Instead they DEASSERT the
reset in their INIT_OTP sequence, but the deassert must be done before
calling intel_dsi_device_ready(), so that is too late.

Simply doing the INIT_OTP sequence earlier is not enough to fix this,
because the INIT_OTP sequence also sends various MIPI packets to the
panel, which can only happen after calling intel_dsi_device_ready().

This commit fixes this by splitting the INIT_OTP sequence into everything
before the first DSI packet and everything else, including the first DSI
packet. The first part (everything before the first DSI packet) is then
used as deassert sequence.

Changed in v2:
-Split the init OTP sequence into a deassert reset and the actual init
 OTP sequence, instead of calling it earlier and then having the first
 mipi_exec_send_packet() call call intel_dsi_device_ready().

Changes in v3:
-Move the whole shebang to intel_bios.c

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880
Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205
Cc: Jan-Michael Brummer <jan.brummer@tabos.org>
Reported-by: Jan-Michael Brummer <jan.brummer@tabos.org>
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_bios.c | 83 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 081190da0818..1f346266956b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1349,6 +1349,7 @@ struct intel_vbt_data {
 		u32 size;
 		u8 *data;
 		const u8 *sequence[MIPI_SEQ_MAX];
+		u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
 	} dsi;
 
 	int crt_ddc_pin;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 64a0d55df28e..cca620f8deb6 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -947,6 +947,86 @@ static int goto_next_sequence_v3(const u8 *data, int index, int total)
 	return 0;
 }
 
+/*
+ * Get len of pre-fixed deassert fragment from a v1 init OTP sequence,
+ * skip all delay + gpio operands and stop at the first DSI packet op.
+ */
+static int get_init_otp_deassert_fragment_len(struct drm_i915_private *dev_priv)
+{
+	const u8 *data = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
+	int index, len;
+
+	if (WARN_ON(!data || dev_priv->vbt.dsi.seq_version != 1))
+		return 0;
+
+	/* index = 1 to skip sequence byte */
+	for (index = 1; data[index] != MIPI_SEQ_ELEM_END; index += len) {
+		switch (data[index]) {
+		case MIPI_SEQ_ELEM_SEND_PKT:
+			return index == 1 ? 0 : index;
+		case MIPI_SEQ_ELEM_DELAY:
+			len = 5; /* 1 byte for operand + uint32 */
+			break;
+		case MIPI_SEQ_ELEM_GPIO:
+			len = 3; /* 1 byte for op, 1 for gpio_nr, 1 for value */
+			break;
+		default:
+			return 0;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Some v1 VBT MIPI sequences do the deassert in the init OTP sequence.
+ * The deassert must be done before calling intel_dsi_device_ready, so for
+ * these devices we split the init OTP sequence into a deassert sequence and
+ * the actual init OTP part.
+ */
+static void fixup_mipi_sequences(struct drm_i915_private *dev_priv)
+{
+	u8 *init_otp;
+	int len;
+
+	/* Limit this to VLV for now. */
+	if (!IS_VALLEYVIEW(dev_priv))
+		return;
+
+	/* Limit this to v1 vid-mode sequences */
+	if (dev_priv->vbt.dsi.config->is_cmd_mode ||
+	    dev_priv->vbt.dsi.seq_version != 1)
+		return;
+
+	/* Only do this if there are otp and assert seqs and no deassert seq */
+	if (!dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] ||
+	    !dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET] ||
+	    dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET])
+		return;
+
+	/* The deassert-sequence ends at the first DSI packet */
+	len = get_init_otp_deassert_fragment_len(dev_priv);
+	if (!len)
+		return;
+
+	DRM_DEBUG_KMS("Using init OTP fragment to deassert reset\n");
+
+	/* Copy the fragment, update seq byte and terminate it */
+	init_otp = (u8 *)dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
+	dev_priv->vbt.dsi.deassert_seq = kmemdup(init_otp, len + 1, GFP_KERNEL);
+	if (!dev_priv->vbt.dsi.deassert_seq)
+		return;
+	dev_priv->vbt.dsi.deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET;
+	dev_priv->vbt.dsi.deassert_seq[len] = MIPI_SEQ_ELEM_END;
+	/* Use the copy for deassert */
+	dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] =
+		dev_priv->vbt.dsi.deassert_seq;
+	/* Replace the last byte of the fragment with init OTP seq byte */
+	init_otp[len - 1] = MIPI_SEQ_INIT_OTP;
+	/* And make MIPI_MIPI_SEQ_INIT_OTP point to it */
+	dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] = init_otp + len - 1;
+}
+
 static void
 parse_mipi_sequence(struct drm_i915_private *dev_priv,
 		    const struct bdb_header *bdb)
@@ -1016,6 +1096,8 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
 	dev_priv->vbt.dsi.size = seq_size;
 	dev_priv->vbt.dsi.seq_version = sequence->version;
 
+	fixup_mipi_sequences(dev_priv);
+
 	DRM_DEBUG_DRIVER("MIPI related VBT parsing complete\n");
 	return;
 
@@ -1594,6 +1676,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
  */
 void intel_bios_cleanup(struct drm_i915_private *dev_priv)
 {
+	kfree(dev_priv->vbt.dsi.deassert_seq);
 	kfree(dev_priv->vbt.dsi.data);
 	kfree(dev_priv->vbt.dsi.pps);
 	kfree(dev_priv->vbt.dsi.config);
-- 
2.14.3

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Free memdup-ed bios data structures on driver_unload

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Free memdup-ed bios data structures on driver_unload
URL   : https://patchwork.freedesktop.org/series/37282/
State : success

== Summary ==

Series 37282v1 series starting with [1/2] drm/i915: Free memdup-ed bios data structures on driver_unload
https://patchwork.freedesktop.org/api/1.0/series/37282/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:421s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:425s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:375s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:487s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:283s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:483s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:489s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:468s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:462s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:281s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:512s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:389s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:398s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:410s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:455s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:415s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:462s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:494s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:504s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:575s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:431s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:525s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:493s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:490s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:416s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:431s
fi-snb-2520m     total:3    pass:2    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:395s
Blacklisted hosts:
fi-cnl-y2        total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:527s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:474s

5f9f3637ad4677971149ec7cde9ed2081a90839b drm-tip: 2018y-01m-29d-13h-02m-54s UTC integration manifest
283ef4a64178 drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3
efb87dbaa7e4 drm/i915: Free memdup-ed bios data structures on driver_unload

== Logs ==

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

✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Free memdup-ed bios data structures on driver_unload

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Free memdup-ed bios data structures on driver_unload
URL   : https://patchwork.freedesktop.org/series/37282/
State : failure

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-rgb565-draw-mmap-cpu:
                pass       -> FAIL       (shard-apl)
        Subgroup fbc-1p-primscrn-shrfb-pgflip-blt:
                pass       -> DMESG-FAIL (shard-apl) fdo#101623 +2
        Subgroup fbc-1p-shrfb-fliptrack:
                fail       -> PASS       (shard-apl) fdo#103167
Test kms_flip:
        Subgroup 2x-flip-vs-expired-vblank-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#102887
        Subgroup 2x-plain-flip-fb-recreate-interruptible:
                pass       -> FAIL       (shard-hsw)
        Subgroup modeset-vs-vblank-race-interruptible:
                fail       -> PASS       (shard-hsw) fdo#103060
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> SKIP       (shard-snb) fdo#103880
Test perf:
        Subgroup oa-exponents:
                fail       -> PASS       (shard-apl) fdo#102254
Test perf_pmu:
        Subgroup frequency:
                fail       -> PASS       (shard-apl) fdo#104829

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103880 https://bugs.freedesktop.org/show_bug.cgi?id=103880
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#104829 https://bugs.freedesktop.org/show_bug.cgi?id=104829

shard-apl        total:2758 pass:1706 dwarn:1   dfail:1   fail:21  skip:1029 time:12189s
shard-hsw        total:2838 pass:1734 dwarn:1   dfail:0   fail:12  skip:1090 time:12060s
shard-snb        total:2838 pass:1329 dwarn:1   dfail:0   fail:10  skip:1498 time:6694s
Blacklisted hosts:
shard-kbl        total:2838 pass:1872 dwarn:5   dfail:0   fail:23  skip:938 time:9628s

== Logs ==

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

Re: [PATCH 1/2] drm/i915: Free memdup-ed bios data structures on driver_unload

[Ville Syrjälä]

On Mon, Jan 29, 2018 at 03:47:34PM +0100, Hans de Goede wrote:
> Add a new intel_bios_cleanup function to free memdup-ed bios data
> structures and call it from i915_driver_unload().
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c   |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_bios.c | 11 +++++++++++
>  3 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ec12add34b2..4ecf41724183 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1437,6 +1437,8 @@ void i915_driver_unload(struct drm_device *dev)
>  
>  	intel_modeset_cleanup(dev);
>  
> +	intel_bios_cleanup(dev_priv);
> +
>  	/*
>  	 * free the memory space allocated for the child device
>  	 * config parsed from VBT

Looks like there's already a bunch of VBT related cleanup just below.
Maybe that should be sucked into the new cleanup function as well?

> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 454d8f937fae..081190da0818 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3663,6 +3663,7 @@ extern void intel_i2c_reset(struct drm_i915_private *dev_priv);
>  
>  /* intel_bios.c */
>  void intel_bios_init(struct drm_i915_private *dev_priv);
> +void intel_bios_cleanup(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>  bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 95f0b310d656..64a0d55df28e 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1588,6 +1588,17 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>  		pci_unmap_rom(pdev, bios);
>  }
>  
> +/**
> + * intel_bios_cleanup - Free any resources allocated by intel_bios_init()
> + * @dev_priv: i915 device instance
> + */
> +void intel_bios_cleanup(struct drm_i915_private *dev_priv)
> +{
> +	kfree(dev_priv->vbt.dsi.data);
> +	kfree(dev_priv->vbt.dsi.pps);
> +	kfree(dev_priv->vbt.dsi.config);
> +}
> +
>  /**
>   * intel_bios_is_tv_present - is integrated TV present in VBT
>   * @dev_priv:	i915 device instance
> -- 
> 2.14.3

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

Re: [PATCH 2/2] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3

[Ville Syrjälä]

On Mon, Jan 29, 2018 at 03:47:35PM +0100, Hans de Goede wrote:
> So far models of the Dell Venue 8 Pro, with a panel with MIPI panel
> index = 3, one of which has been kindly provided to me by Jan Brummer,
> where not working with the i915 driver, giving a black screen on the
> first modeset.
> 
> The problem with at least these Dells is that their VBT defines a MIPI
> ASSERT sequence, but not a DEASSERT sequence. Instead they DEASSERT the
> reset in their INIT_OTP sequence, but the deassert must be done before
> calling intel_dsi_device_ready(), so that is too late.
> 
> Simply doing the INIT_OTP sequence earlier is not enough to fix this,
> because the INIT_OTP sequence also sends various MIPI packets to the
> panel, which can only happen after calling intel_dsi_device_ready().
> 
> This commit fixes this by splitting the INIT_OTP sequence into everything
> before the first DSI packet and everything else, including the first DSI
> packet. The first part (everything before the first DSI packet) is then
> used as deassert sequence.
> 
> Changed in v2:
> -Split the init OTP sequence into a deassert reset and the actual init
>  OTP sequence, instead of calling it earlier and then having the first
>  mipi_exec_send_packet() call call intel_dsi_device_ready().
> 
> Changes in v3:
> -Move the whole shebang to intel_bios.c
> 
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880

Bugzilla:

> Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205

References:

> Cc: Jan-Michael Brummer <jan.brummer@tabos.org>
> Reported-by: Jan-Michael Brummer <jan.brummer@tabos.org>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

This one seems good to me, and Jani hasn't complained about anything so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_bios.c | 83 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 081190da0818..1f346266956b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1349,6 +1349,7 @@ struct intel_vbt_data {
>  		u32 size;
>  		u8 *data;
>  		const u8 *sequence[MIPI_SEQ_MAX];
> +		u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
>  	} dsi;
>  
>  	int crt_ddc_pin;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 64a0d55df28e..cca620f8deb6 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -947,6 +947,86 @@ static int goto_next_sequence_v3(const u8 *data, int index, int total)
>  	return 0;
>  }
>  
> +/*
> + * Get len of pre-fixed deassert fragment from a v1 init OTP sequence,
> + * skip all delay + gpio operands and stop at the first DSI packet op.
> + */
> +static int get_init_otp_deassert_fragment_len(struct drm_i915_private *dev_priv)
> +{
> +	const u8 *data = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
> +	int index, len;
> +
> +	if (WARN_ON(!data || dev_priv->vbt.dsi.seq_version != 1))
> +		return 0;
> +
> +	/* index = 1 to skip sequence byte */
> +	for (index = 1; data[index] != MIPI_SEQ_ELEM_END; index += len) {
> +		switch (data[index]) {
> +		case MIPI_SEQ_ELEM_SEND_PKT:
> +			return index == 1 ? 0 : index;
> +		case MIPI_SEQ_ELEM_DELAY:
> +			len = 5; /* 1 byte for operand + uint32 */
> +			break;
> +		case MIPI_SEQ_ELEM_GPIO:
> +			len = 3; /* 1 byte for op, 1 for gpio_nr, 1 for value */
> +			break;
> +		default:
> +			return 0;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Some v1 VBT MIPI sequences do the deassert in the init OTP sequence.
> + * The deassert must be done before calling intel_dsi_device_ready, so for
> + * these devices we split the init OTP sequence into a deassert sequence and
> + * the actual init OTP part.
> + */
> +static void fixup_mipi_sequences(struct drm_i915_private *dev_priv)
> +{
> +	u8 *init_otp;
> +	int len;
> +
> +	/* Limit this to VLV for now. */
> +	if (!IS_VALLEYVIEW(dev_priv))
> +		return;
> +
> +	/* Limit this to v1 vid-mode sequences */
> +	if (dev_priv->vbt.dsi.config->is_cmd_mode ||
> +	    dev_priv->vbt.dsi.seq_version != 1)
> +		return;
> +
> +	/* Only do this if there are otp and assert seqs and no deassert seq */
> +	if (!dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] ||
> +	    !dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET] ||
> +	    dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET])
> +		return;
> +
> +	/* The deassert-sequence ends at the first DSI packet */
> +	len = get_init_otp_deassert_fragment_len(dev_priv);
> +	if (!len)
> +		return;
> +
> +	DRM_DEBUG_KMS("Using init OTP fragment to deassert reset\n");
> +
> +	/* Copy the fragment, update seq byte and terminate it */
> +	init_otp = (u8 *)dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
> +	dev_priv->vbt.dsi.deassert_seq = kmemdup(init_otp, len + 1, GFP_KERNEL);
> +	if (!dev_priv->vbt.dsi.deassert_seq)
> +		return;
> +	dev_priv->vbt.dsi.deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET;
> +	dev_priv->vbt.dsi.deassert_seq[len] = MIPI_SEQ_ELEM_END;
> +	/* Use the copy for deassert */
> +	dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] =
> +		dev_priv->vbt.dsi.deassert_seq;
> +	/* Replace the last byte of the fragment with init OTP seq byte */
> +	init_otp[len - 1] = MIPI_SEQ_INIT_OTP;
> +	/* And make MIPI_MIPI_SEQ_INIT_OTP point to it */
> +	dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] = init_otp + len - 1;
> +}
> +
>  static void
>  parse_mipi_sequence(struct drm_i915_private *dev_priv,
>  		    const struct bdb_header *bdb)
> @@ -1016,6 +1096,8 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>  	dev_priv->vbt.dsi.size = seq_size;
>  	dev_priv->vbt.dsi.seq_version = sequence->version;
>  
> +	fixup_mipi_sequences(dev_priv);
> +
>  	DRM_DEBUG_DRIVER("MIPI related VBT parsing complete\n");
>  	return;
>  
> @@ -1594,6 +1676,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>   */
>  void intel_bios_cleanup(struct drm_i915_private *dev_priv)
>  {
> +	kfree(dev_priv->vbt.dsi.deassert_seq);
>  	kfree(dev_priv->vbt.dsi.data);
>  	kfree(dev_priv->vbt.dsi.pps);
>  	kfree(dev_priv->vbt.dsi.config);
> -- 
> 2.14.3

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

Re: [PATCH 2/2] drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3

[Jani Nikula]

On Fri, 02 Feb 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Jan 29, 2018 at 03:47:35PM +0100, Hans de Goede wrote:
>> So far models of the Dell Venue 8 Pro, with a panel with MIPI panel
>> index = 3, one of which has been kindly provided to me by Jan Brummer,
>> where not working with the i915 driver, giving a black screen on the
>> first modeset.
>> 
>> The problem with at least these Dells is that their VBT defines a MIPI
>> ASSERT sequence, but not a DEASSERT sequence. Instead they DEASSERT the
>> reset in their INIT_OTP sequence, but the deassert must be done before
>> calling intel_dsi_device_ready(), so that is too late.
>> 
>> Simply doing the INIT_OTP sequence earlier is not enough to fix this,
>> because the INIT_OTP sequence also sends various MIPI packets to the
>> panel, which can only happen after calling intel_dsi_device_ready().
>> 
>> This commit fixes this by splitting the INIT_OTP sequence into everything
>> before the first DSI packet and everything else, including the first DSI
>> packet. The first part (everything before the first DSI packet) is then
>> used as deassert sequence.
>> 
>> Changed in v2:
>> -Split the init OTP sequence into a deassert reset and the actual init
>>  OTP sequence, instead of calling it earlier and then having the first
>>  mipi_exec_send_packet() call call intel_dsi_device_ready().
>> 
>> Changes in v3:
>> -Move the whole shebang to intel_bios.c
>> 
>> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=82880
>
> Bugzilla:
>
>> Related: https://bugs.freedesktop.org/show_bug.cgi?id=101205
>
> References:
>
>> Cc: Jan-Michael Brummer <jan.brummer@tabos.org>
>> Reported-by: Jan-Michael Brummer <jan.brummer@tabos.org>
>> Tested-by: Hans de Goede <hdegoede@redhat.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> This one seems good to me, and Jani hasn't complained about anything so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks for hiding this in one place.

Acked-by: Jani Nikula <jani.nikula@intel.com>


>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>>  drivers/gpu/drm/i915/intel_bios.c | 83 +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 84 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 081190da0818..1f346266956b 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1349,6 +1349,7 @@ struct intel_vbt_data {
>>  		u32 size;
>>  		u8 *data;
>>  		const u8 *sequence[MIPI_SEQ_MAX];
>> +		u8 *deassert_seq; /* Used by fixup_mipi_sequences() */
>>  	} dsi;
>>  
>>  	int crt_ddc_pin;
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 64a0d55df28e..cca620f8deb6 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -947,6 +947,86 @@ static int goto_next_sequence_v3(const u8 *data, int index, int total)
>>  	return 0;
>>  }
>>  
>> +/*
>> + * Get len of pre-fixed deassert fragment from a v1 init OTP sequence,
>> + * skip all delay + gpio operands and stop at the first DSI packet op.
>> + */
>> +static int get_init_otp_deassert_fragment_len(struct drm_i915_private *dev_priv)
>> +{
>> +	const u8 *data = dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
>> +	int index, len;
>> +
>> +	if (WARN_ON(!data || dev_priv->vbt.dsi.seq_version != 1))
>> +		return 0;
>> +
>> +	/* index = 1 to skip sequence byte */
>> +	for (index = 1; data[index] != MIPI_SEQ_ELEM_END; index += len) {
>> +		switch (data[index]) {
>> +		case MIPI_SEQ_ELEM_SEND_PKT:
>> +			return index == 1 ? 0 : index;
>> +		case MIPI_SEQ_ELEM_DELAY:
>> +			len = 5; /* 1 byte for operand + uint32 */
>> +			break;
>> +		case MIPI_SEQ_ELEM_GPIO:
>> +			len = 3; /* 1 byte for op, 1 for gpio_nr, 1 for value */
>> +			break;
>> +		default:
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>> + * Some v1 VBT MIPI sequences do the deassert in the init OTP sequence.
>> + * The deassert must be done before calling intel_dsi_device_ready, so for
>> + * these devices we split the init OTP sequence into a deassert sequence and
>> + * the actual init OTP part.
>> + */
>> +static void fixup_mipi_sequences(struct drm_i915_private *dev_priv)
>> +{
>> +	u8 *init_otp;
>> +	int len;
>> +
>> +	/* Limit this to VLV for now. */
>> +	if (!IS_VALLEYVIEW(dev_priv))
>> +		return;
>> +
>> +	/* Limit this to v1 vid-mode sequences */
>> +	if (dev_priv->vbt.dsi.config->is_cmd_mode ||
>> +	    dev_priv->vbt.dsi.seq_version != 1)
>> +		return;
>> +
>> +	/* Only do this if there are otp and assert seqs and no deassert seq */
>> +	if (!dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] ||
>> +	    !dev_priv->vbt.dsi.sequence[MIPI_SEQ_ASSERT_RESET] ||
>> +	    dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET])
>> +		return;
>> +
>> +	/* The deassert-sequence ends at the first DSI packet */
>> +	len = get_init_otp_deassert_fragment_len(dev_priv);
>> +	if (!len)
>> +		return;
>> +
>> +	DRM_DEBUG_KMS("Using init OTP fragment to deassert reset\n");
>> +
>> +	/* Copy the fragment, update seq byte and terminate it */
>> +	init_otp = (u8 *)dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP];
>> +	dev_priv->vbt.dsi.deassert_seq = kmemdup(init_otp, len + 1, GFP_KERNEL);
>> +	if (!dev_priv->vbt.dsi.deassert_seq)
>> +		return;
>> +	dev_priv->vbt.dsi.deassert_seq[0] = MIPI_SEQ_DEASSERT_RESET;
>> +	dev_priv->vbt.dsi.deassert_seq[len] = MIPI_SEQ_ELEM_END;
>> +	/* Use the copy for deassert */
>> +	dev_priv->vbt.dsi.sequence[MIPI_SEQ_DEASSERT_RESET] =
>> +		dev_priv->vbt.dsi.deassert_seq;
>> +	/* Replace the last byte of the fragment with init OTP seq byte */
>> +	init_otp[len - 1] = MIPI_SEQ_INIT_OTP;
>> +	/* And make MIPI_MIPI_SEQ_INIT_OTP point to it */
>> +	dev_priv->vbt.dsi.sequence[MIPI_SEQ_INIT_OTP] = init_otp + len - 1;
>> +}
>> +
>>  static void
>>  parse_mipi_sequence(struct drm_i915_private *dev_priv,
>>  		    const struct bdb_header *bdb)
>> @@ -1016,6 +1096,8 @@ parse_mipi_sequence(struct drm_i915_private *dev_priv,
>>  	dev_priv->vbt.dsi.size = seq_size;
>>  	dev_priv->vbt.dsi.seq_version = sequence->version;
>>  
>> +	fixup_mipi_sequences(dev_priv);
>> +
>>  	DRM_DEBUG_DRIVER("MIPI related VBT parsing complete\n");
>>  	return;
>>  
>> @@ -1594,6 +1676,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>>   */
>>  void intel_bios_cleanup(struct drm_i915_private *dev_priv)
>>  {
>> +	kfree(dev_priv->vbt.dsi.deassert_seq);
>>  	kfree(dev_priv->vbt.dsi.data);
>>  	kfree(dev_priv->vbt.dsi.pps);
>>  	kfree(dev_priv->vbt.dsi.config);
>> -- 
>> 2.14.3

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

Re: [PATCH 1/2] drm/i915: Free memdup-ed bios data structures on driver_unload

[Hans de Goede]

Hi,

On 02-02-18 17:13, Ville Syrjälä wrote:
> On Mon, Jan 29, 2018 at 03:47:34PM +0100, Hans de Goede wrote:
>> Add a new intel_bios_cleanup function to free memdup-ed bios data
>> structures and call it from i915_driver_unload().
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c   |  2 ++
>>   drivers/gpu/drm/i915/i915_drv.h   |  1 +
>>   drivers/gpu/drm/i915/intel_bios.c | 11 +++++++++++
>>   3 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 1ec12add34b2..4ecf41724183 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1437,6 +1437,8 @@ void i915_driver_unload(struct drm_device *dev)
>>   
>>   	intel_modeset_cleanup(dev);
>>   
>> +	intel_bios_cleanup(dev_priv);
>> +
>>   	/*
>>   	 * free the memory space allocated for the child device
>>   	 * config parsed from VBT
> 
> Looks like there's already a bunch of VBT related cleanup just below.
> Maybe that should be sucked into the new cleanup function as well?

Ah, I somehow missed that, yes that is a good idea. I will send a v3 of
the series with that changed.

Regards,

Hans


> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 454d8f937fae..081190da0818 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3663,6 +3663,7 @@ extern void intel_i2c_reset(struct drm_i915_private *dev_priv);
>>   
>>   /* intel_bios.c */
>>   void intel_bios_init(struct drm_i915_private *dev_priv);
>> +void intel_bios_cleanup(struct drm_i915_private *dev_priv);
>>   bool intel_bios_is_valid_vbt(const void *buf, size_t size);
>>   bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>>   bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 95f0b310d656..64a0d55df28e 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1588,6 +1588,17 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>>   		pci_unmap_rom(pdev, bios);
>>   }
>>   
>> +/**
>> + * intel_bios_cleanup - Free any resources allocated by intel_bios_init()
>> + * @dev_priv: i915 device instance
>> + */
>> +void intel_bios_cleanup(struct drm_i915_private *dev_priv)
>> +{
>> +	kfree(dev_priv->vbt.dsi.data);
>> +	kfree(dev_priv->vbt.dsi.pps);
>> +	kfree(dev_priv->vbt.dsi.config);
>> +}
>> +
>>   /**
>>    * intel_bios_is_tv_present - is integrated TV present in VBT
>>    * @dev_priv:	i915 device instance
>> -- 
>> 2.14.3
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx