[PATCH v4 1/3] drm/i915: Add intel_bios_cleanup() function

[PATCH v4 1/3] drm/i915: Add intel_bios_cleanup() function

[Hans de Goede]

Add an intel_bios_cleanup() function to act as counterpart of
intel_bios_init() and move the cleanup of vbt related resources there,
putting it in the same file as the allocation.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e9f1daf258fe..7f094bbc2a7f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1446,19 +1446,7 @@ void i915_driver_unload(struct drm_device *dev)
 
 	intel_modeset_cleanup(dev);
 
-	/*
-	 * free the memory space allocated for the child device
-	 * config parsed from VBT
-	 */
-	if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {
-		kfree(dev_priv->vbt.child_dev);
-		dev_priv->vbt.child_dev = NULL;
-		dev_priv->vbt.child_dev_num = 0;
-	}
-	kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
-	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
-	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
-	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
+	intel_bios_cleanup(dev_priv);
 
 	vga_switcheroo_unregister_client(pdev);
 	vga_client_register(pdev, NULL, NULL, NULL);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d6b5ac2a563d..1cccea1b87bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3675,6 +3675,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 4e74aa2f16bc..f9550507bb9f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1610,6 +1610,27 @@ 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)
+{
+	/*
+	 * free the memory space allocated for the child device
+	 * config parsed from VBT
+	 */
+	if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {
+		kfree(dev_priv->vbt.child_dev);
+		dev_priv->vbt.child_dev = NULL;
+		dev_priv->vbt.child_dev_num = 0;
+	}
+	kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
+	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
+	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
+	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
+}
+
 /**
  * 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 v4 2/3] drm/i915: Free memdup-ed DSI VBT data structures on driver_unload

[Hans de Goede]

Make intel_bios_cleanup function free the DSI VBT data structures which
are memdup-ed by parse_mipi_config() and parse_mipi_sequence().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index f9550507bb9f..18110bbd9ee2 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1629,6 +1629,12 @@ void intel_bios_cleanup(struct drm_i915_private *dev_priv)
 	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
 	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
 	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
+	kfree(dev_priv->vbt.dsi.data);
+	dev_priv->vbt.dsi.data = NULL;
+	kfree(dev_priv->vbt.dsi.pps);
+	dev_priv->vbt.dsi.pps = NULL;
+	kfree(dev_priv->vbt.dsi.config);
+	dev_priv->vbt.dsi.config = NULL;
 }
 
 /**
-- 
2.14.3

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

[PATCH v4 3/3] 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

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82880
References: 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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.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 | 84 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1cccea1b87bc..b3c4fde600eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1356,6 +1356,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 18110bbd9ee2..797d1aecda03 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;
 
@@ -1635,6 +1717,8 @@ void intel_bios_cleanup(struct drm_i915_private *dev_priv)
 	dev_priv->vbt.dsi.pps = NULL;
 	kfree(dev_priv->vbt.dsi.config);
 	dev_priv->vbt.dsi.config = NULL;
+	kfree(dev_priv->vbt.dsi.deassert_seq);
+	dev_priv->vbt.dsi.deassert_seq = NULL;
 }
 
 /**
-- 
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 [v4,1/3] drm/i915: Add intel_bios_cleanup() function

[Patchwork]

== Series Details ==

Series: series starting with [v4,1/3] drm/i915: Add intel_bios_cleanup() function
URL   : https://patchwork.freedesktop.org/series/37741/
State : success

== Summary ==

Series 37741v1 series starting with [v4,1/3] drm/i915: Add intel_bios_cleanup() function
https://patchwork.freedesktop.org/api/1.0/series/37741/revisions/1/mbox/

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:424s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:371s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:489s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:480s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:474s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:460s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:568s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:584s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:412s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:286s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:517s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:398s
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:413s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:462s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:426s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:458s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:495s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:449s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:500s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:594s
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:526s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:489s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:489s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:419s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:430s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:526s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:408s
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:465s

078873da383505cf8d6940229007115b31f1d5e0 drm-tip: 2018y-02m-06d-11h-21m-36s UTC integration manifest
1f3e7871638c drm/i915: Fix DSI panels with v1 MIPI sequences without a DEASSERT sequence v3
dcf004ec7001 drm/i915: Free memdup-ed DSI VBT data structures on driver_unload
595a5c38746a drm/i915: Add intel_bios_cleanup() function

== Logs ==

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

✗ Fi.CI.IGT: warning for series starting with [v4,1/3] drm/i915: Add intel_bios_cleanup() function

[Patchwork]

== Series Details ==

Series: series starting with [v4,1/3] drm/i915: Add intel_bios_cleanup() function
URL   : https://patchwork.freedesktop.org/series/37741/
State : warning

== Summary ==

Test gem_exec_schedule:
        Subgroup reorder-wide-vebox:
                incomplete -> PASS       (shard-apl) fdo#102848
Test perf:
        Subgroup buffer-fill:
                fail       -> PASS       (shard-apl) fdo#103755
        Subgroup blocking:
                pass       -> FAIL       (shard-hsw) fdo#102252
        Subgroup enable-disable:
                fail       -> PASS       (shard-apl) fdo#103715
        Subgroup oa-exponents:
                pass       -> FAIL       (shard-apl) fdo#102254
Test kms_rmfb:
        Subgroup close-fd:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_cursor_crc:
        Subgroup cursor-128x128-suspend:
                pass       -> INCOMPLETE (shard-hsw) fdo#103540
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test kms_mmap_write_crc:
                skip       -> PASS       (shard-apl)

fdo#102848 https://bugs.freedesktop.org/show_bug.cgi?id=102848
fdo#103755 https://bugs.freedesktop.org/show_bug.cgi?id=103755
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103715 https://bugs.freedesktop.org/show_bug.cgi?id=103715
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-apl        total:3383 pass:1752 dwarn:1   dfail:0   fail:21  skip:1608 time:12174s
shard-hsw        total:3420 pass:1744 dwarn:2   dfail:0   fail:11  skip:1661 time:11329s
shard-snb        total:3442 pass:1351 dwarn:1   dfail:0   fail:10  skip:2080 time:6629s
Blacklisted hosts:
shard-kbl        total:3442 pass:1909 dwarn:1   dfail:0   fail:21  skip:1511 time:9611s

== Logs ==

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

Re: [PATCH v4 1/3] drm/i915: Add intel_bios_cleanup() function

[Hans de Goede]

Hi,

On 06-02-18 15:12, Hans de Goede wrote:
> Add an intel_bios_cleanup() function to act as counterpart of
> intel_bios_init() and move the cleanup of vbt related resources there,
> putting it in the same file as the allocation.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Can I get an Acked-by for this patch and 2/3 please? Then I can
push this series.

Regards,

Hans

> ---
>   drivers/gpu/drm/i915/i915_drv.c   | 14 +-------------
>   drivers/gpu/drm/i915/i915_drv.h   |  1 +
>   drivers/gpu/drm/i915/intel_bios.c | 21 +++++++++++++++++++++
>   3 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e9f1daf258fe..7f094bbc2a7f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1446,19 +1446,7 @@ void i915_driver_unload(struct drm_device *dev)
>   
>   	intel_modeset_cleanup(dev);
>   
> -	/*
> -	 * free the memory space allocated for the child device
> -	 * config parsed from VBT
> -	 */
> -	if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {
> -		kfree(dev_priv->vbt.child_dev);
> -		dev_priv->vbt.child_dev = NULL;
> -		dev_priv->vbt.child_dev_num = 0;
> -	}
> -	kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
> -	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
> -	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
> -	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
> +	intel_bios_cleanup(dev_priv);
>   
>   	vga_switcheroo_unregister_client(pdev);
>   	vga_client_register(pdev, NULL, NULL, NULL);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d6b5ac2a563d..1cccea1b87bc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3675,6 +3675,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 4e74aa2f16bc..f9550507bb9f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1610,6 +1610,27 @@ 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)
> +{
> +	/*
> +	 * free the memory space allocated for the child device
> +	 * config parsed from VBT
> +	 */
> +	if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {
> +		kfree(dev_priv->vbt.child_dev);
> +		dev_priv->vbt.child_dev = NULL;
> +		dev_priv->vbt.child_dev_num = 0;
> +	}
> +	kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
> +	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
> +	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
> +	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
> +}
> +
>   /**
>    * intel_bios_is_tv_present - is integrated TV present in VBT
>    * @dev_priv:	i915 device instance
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v4 1/3] drm/i915: Add intel_bios_cleanup() function

[Ville Syrjälä]

On Tue, Feb 06, 2018 at 03:12:37PM +0100, Hans de Goede wrote:
> Add an intel_bios_cleanup() function to act as counterpart of
> intel_bios_init() and move the cleanup of vbt related resources there,
> putting it in the same file as the allocation.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c   | 14 +-------------
>  drivers/gpu/drm/i915/i915_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_bios.c | 21 +++++++++++++++++++++
>  3 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e9f1daf258fe..7f094bbc2a7f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1446,19 +1446,7 @@ void i915_driver_unload(struct drm_device *dev)
>  
>  	intel_modeset_cleanup(dev);
>  
> -	/*
> -	 * free the memory space allocated for the child device
> -	 * config parsed from VBT
> -	 */
> -	if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {
> -		kfree(dev_priv->vbt.child_dev);
> -		dev_priv->vbt.child_dev = NULL;
> -		dev_priv->vbt.child_dev_num = 0;
> -	}
> -	kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
> -	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
> -	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
> -	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
> +	intel_bios_cleanup(dev_priv);
>  
>  	vga_switcheroo_unregister_client(pdev);
>  	vga_client_register(pdev, NULL, NULL, NULL);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d6b5ac2a563d..1cccea1b87bc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3675,6 +3675,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 4e74aa2f16bc..f9550507bb9f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1610,6 +1610,27 @@ 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)
> +{
> +	/*
> +	 * free the memory space allocated for the child device
> +	 * config parsed from VBT
> +	 */
> +	if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {

The comment and if() seem pointless. IMO just drop them.

With that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		kfree(dev_priv->vbt.child_dev);
> +		dev_priv->vbt.child_dev = NULL;
> +		dev_priv->vbt.child_dev_num = 0;
> +	}
> +	kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
> +	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
> +	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
> +	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
> +}
> +
>  /**
>   * intel_bios_is_tv_present - is integrated TV present in VBT
>   * @dev_priv:	i915 device instance
> -- 
> 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 v4 2/3] drm/i915: Free memdup-ed DSI VBT data structures on driver_unload

[Ville Syrjälä]

On Tue, Feb 06, 2018 at 03:12:38PM +0100, Hans de Goede wrote:
> Make intel_bios_cleanup function free the DSI VBT data structures which
> are memdup-ed by parse_mipi_config() and parse_mipi_sequence().
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_bios.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index f9550507bb9f..18110bbd9ee2 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1629,6 +1629,12 @@ void intel_bios_cleanup(struct drm_i915_private *dev_priv)
>  	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
>  	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
>  	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
> +	kfree(dev_priv->vbt.dsi.data);
> +	dev_priv->vbt.dsi.data = NULL;
> +	kfree(dev_priv->vbt.dsi.pps);
> +	dev_priv->vbt.dsi.pps = NULL;
> +	kfree(dev_priv->vbt.dsi.config);
> +	dev_priv->vbt.dsi.config = NULL;
>  }
>  
>  /**
> -- 
> 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 v4 1/3] drm/i915: Add intel_bios_cleanup() function

[Hans de Goede]

Hi,

On 12-02-18 20:45, Ville Syrjälä wrote:
> On Tue, Feb 06, 2018 at 03:12:37PM +0100, Hans de Goede wrote:
>> Add an intel_bios_cleanup() function to act as counterpart of
>> intel_bios_init() and move the cleanup of vbt related resources there,
>> putting it in the same file as the allocation.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c   | 14 +-------------
>>   drivers/gpu/drm/i915/i915_drv.h   |  1 +
>>   drivers/gpu/drm/i915/intel_bios.c | 21 +++++++++++++++++++++
>>   3 files changed, 23 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index e9f1daf258fe..7f094bbc2a7f 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1446,19 +1446,7 @@ void i915_driver_unload(struct drm_device *dev)
>>   
>>   	intel_modeset_cleanup(dev);
>>   
>> -	/*
>> -	 * free the memory space allocated for the child device
>> -	 * config parsed from VBT
>> -	 */
>> -	if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {
>> -		kfree(dev_priv->vbt.child_dev);
>> -		dev_priv->vbt.child_dev = NULL;
>> -		dev_priv->vbt.child_dev_num = 0;
>> -	}
>> -	kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
>> -	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
>> -	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
>> -	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
>> +	intel_bios_cleanup(dev_priv);
>>   
>>   	vga_switcheroo_unregister_client(pdev);
>>   	vga_client_register(pdev, NULL, NULL, NULL);
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index d6b5ac2a563d..1cccea1b87bc 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3675,6 +3675,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 4e74aa2f16bc..f9550507bb9f 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1610,6 +1610,27 @@ 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)
>> +{
>> +	/*
>> +	 * free the memory space allocated for the child device
>> +	 * config parsed from VBT
>> +	 */
>> +	if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {
> 
> The comment and if() seem pointless. IMO just drop them.

Ok I will send out a new version of the series with a v2 of
this patch with this fixed (for patchwork / CI) and when CI is
happy with this, push the series.

 > With that
 > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks & Regards,

Hans



> 
>> +		kfree(dev_priv->vbt.child_dev);
>> +		dev_priv->vbt.child_dev = NULL;
>> +		dev_priv->vbt.child_dev_num = 0;
>> +	}
>> +	kfree(dev_priv->vbt.sdvo_lvds_vbt_mode);
>> +	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
>> +	kfree(dev_priv->vbt.lfp_lvds_vbt_mode);
>> +	dev_priv->vbt.lfp_lvds_vbt_mode = NULL;
>> +}
>> +
>>   /**
>>    * 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