[PATCH 1/3] drm/dp_mst: Fix the DDC I2C device unregistration of an MST port

[PATCH 1/3] drm/dp_mst: Fix the DDC I2C device unregistration of an MST port

[Imre Deak]

The WARN below triggers during the removal of an MST port. The problem
is that the parent device's (the connector's kdev) sysfs directory is
removed recursively when the connector is unregistered (even though the
I2C device holds a reference on the parent device). To fix this set
first the Peer Device Type to none which will remove the I2C device.

Note that atm, inconsistently, the parent of the I2C device is initially set to
the DRM kdev and after a Connection Status Notification the parent may be reset
to be the connector's kdev. This problem is addressed by the next patch.

[ 4462.989299] ------------[ cut here ]------------
[ 4463.014940] sysfs group 'power' not found for kobject 'i2c-24'
[ 4463.034664] WARNING: CPU: 0 PID: 970 at fs/sysfs/group.c:281 sysfs_remove_group+0x71/0x80
[ 4463.044357] Modules linked in: snd_hda_intel i915 drm_kms_helper(O) drm netconsole snd_hda_codec_hdmi mei_hdcp x86_pkg_temp_thermal coretemp crct10dif_pclmul snd_intel_dspcf
g crc32_pclmul snd_hda_codec snd_hwdep ghash_clmulni_intel snd_hda_core asix usbnet kvm_intel mii i2c_algo_bit snd_pcm syscopyarea sysfillrect e1000e sysimgblt fb_sys_fops prim
e_numbers ptp pps_core i2c_i801 r8169 mei_me realtek mei [last unloaded: drm]
[ 4463.044399] CPU: 0 PID: 970 Comm: kworker/0:2 Tainted: G           O      5.7.0+ #172
[ 4463.044402] Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP
[ 4463.044423] Workqueue: events drm_dp_delayed_destroy_work [drm_kms_helper]
[ 4463.044428] RIP: 0010:sysfs_remove_group+0x71/0x80
[ 4463.044431] Code: 48 89 df 5b 5d 41 5c e9 cd b6 ff ff 48 89 df e8 95 b4 ff ff eb cb 49 8b 14 24 48 8b 75 00 48 c7 c7 20 0f 3f 82 e8 9f c5 d7 ff <0f> 0b 5b 5d 41 5c c3 0f 1f
84 00 00 00 00 00 48 85 f6 74 31 41 54
[ 4463.044433] RSP: 0018:ffffc900018bfbf0 EFLAGS: 00010282
[ 4463.044436] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000001
[ 4463.044439] RDX: 0000000080000001 RSI: ffff88849e828f38 RDI: 00000000ffffffff
[ 4463.052970] [drm:drm_atomic_get_plane_state [drm]] Added [PLANE:100:plane 2B] 00000000c2160caa state to 00000000d172564a
[ 4463.070533] RBP: ffffffff820cea20 R08: ffff88847f4b8958 R09: 0000000000000000
[ 4463.070535] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88848a725018
[ 4463.070537] R13: 0000000000000000 R14: ffffffff827090e0 R15: 0000000000000002
[ 4463.070539] FS:  0000000000000000(0000) GS:ffff88849e800000(0000) knlGS:0000000000000000
[ 4463.070541] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4463.070543] CR2: 00007fdf8a756538 CR3: 0000000489684001 CR4: 0000000000760ef0
[ 4463.070545] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 4463.070547] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 4463.070549] PKRU: 55555554
[ 4463.070551] Call Trace:
[ 4463.070560]  device_del+0x84/0x400
[ 4463.070571]  cdev_device_del+0x10/0x30
[ 4463.070578]  put_i2c_dev+0x69/0x80
[ 4463.070584]  i2cdev_detach_adapter+0x2e/0x60
[ 4463.070591]  notifier_call_chain+0x34/0x90
[ 4463.070599]  blocking_notifier_call_chain+0x3f/0x60
[ 4463.070606]  device_del+0x7c/0x400
[ 4463.087817]  ? lockdep_init_map_waits+0x57/0x210
[ 4463.087825]  device_unregister+0x11/0x60
[ 4463.087829]  i2c_del_adapter+0x249/0x310
[ 4463.087846]  drm_dp_port_set_pdt+0x6b/0x2c0 [drm_kms_helper]
[ 4463.087862]  drm_dp_delayed_destroy_work+0x2af/0x350 [drm_kms_helper]
[ 4463.087876]  process_one_work+0x268/0x600
[ 4463.105438]  ? __schedule+0x30c/0x920
[ 4463.105451]  worker_thread+0x37/0x380
[ 4463.105457]  ? process_one_work+0x600/0x600
[ 4463.105462]  kthread+0x140/0x160
[ 4463.105466]  ? kthread_park+0x80/0x80
[ 4463.105474]  ret_from_fork+0x24/0x50

Cc: <stable@vger.kernel.org>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 2a309fb2c4cc..02c800b8199f 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4669,12 +4669,13 @@ static void drm_dp_tx_work(struct work_struct *work)
 static inline void
 drm_dp_delayed_destroy_port(struct drm_dp_mst_port *port)
 {
+	drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE, port->mcs);
+
 	if (port->connector) {
 		drm_connector_unregister(port->connector);
 		drm_connector_put(port->connector);
 	}
 
-	drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE, port->mcs);
 	drm_dp_mst_put_port_malloc(port);
 }
 
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/dp_mst: Fix the DDC I2C device registration of an MST port

[Imre Deak]

During the initial MST probing an MST port's I2C device will be
registered using the kdev of the DRM device as a parent. Later after MST
Connection Status Notifications this I2C device will be re-registered
with the kdev of the port's connector. This will also move
inconsistently the I2C device's sysfs entry from the DRM device's sysfs
dir to the connector's dir.

Fix the above by keeping the DRM kdev as the parent of the I2C device.

Ideally the connector's kdev would be used as a parent, similarly to
non-MST connectors, however that needs some more refactoring to ensure
the connector's kdev is already available early enough. So keep the
existing (initial) behavior for now.

Cc: <stable@vger.kernel.org>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 28 +++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 02c800b8199f..083255c33ee0 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -88,8 +88,8 @@ static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
 static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
 				 u8 *guid);
 
-static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux);
-static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux);
+static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port);
+static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port);
 static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
 
 #define DBG_PREFIX "[dp_mst]"
@@ -1993,7 +1993,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
 			}
 
 			/* remove i2c over sideband */
-			drm_dp_mst_unregister_i2c_bus(&port->aux);
+			drm_dp_mst_unregister_i2c_bus(port);
 		} else {
 			mutex_lock(&mgr->lock);
 			drm_dp_mst_topology_put_mstb(port->mstb);
@@ -2008,7 +2008,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
 	if (port->pdt != DP_PEER_DEVICE_NONE) {
 		if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
 			/* add i2c over sideband */
-			ret = drm_dp_mst_register_i2c_bus(&port->aux);
+			ret = drm_dp_mst_register_i2c_bus(port);
 		} else {
 			lct = drm_dp_calculate_rad(port, rad);
 			mstb = drm_dp_add_mst_branch_device(lct, rad);
@@ -5375,22 +5375,26 @@ static const struct i2c_algorithm drm_dp_mst_i2c_algo = {
 
 /**
  * drm_dp_mst_register_i2c_bus() - register an I2C adapter for I2C-over-AUX
- * @aux: DisplayPort AUX channel
+ * @port: The port to add the I2C bus on
  *
  * Returns 0 on success or a negative error code on failure.
  */
-static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux)
+static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port)
 {
+	struct drm_dp_aux *aux = &port->aux;
+	struct device *parent_dev = port->mgr->dev->dev;
+
 	aux->ddc.algo = &drm_dp_mst_i2c_algo;
 	aux->ddc.algo_data = aux;
 	aux->ddc.retries = 3;
 
 	aux->ddc.class = I2C_CLASS_DDC;
 	aux->ddc.owner = THIS_MODULE;
-	aux->ddc.dev.parent = aux->dev;
-	aux->ddc.dev.of_node = aux->dev->of_node;
+	/* FIXME: set the kdev of the port's connector as parent */
+	aux->ddc.dev.parent = parent_dev;
+	aux->ddc.dev.of_node = parent_dev->of_node;
 
-	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
+	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(parent_dev),
 		sizeof(aux->ddc.name));
 
 	return i2c_add_adapter(&aux->ddc);
@@ -5398,11 +5402,11 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux)
 
 /**
  * drm_dp_mst_unregister_i2c_bus() - unregister an I2C-over-AUX adapter
- * @aux: DisplayPort AUX channel
+ * @port: The port to remove the I2C bus from
  */
-static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux)
+static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port)
 {
-	i2c_del_adapter(&aux->ddc);
+	i2c_del_adapter(&port->aux.ddc);
 }
 
 /**
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/dp_mst: Fix flushing the delayed port/mstb destroy work

[Imre Deak]

Atm, a pending delayed destroy work during module removal will be
canceled, leaving behind MST ports, mstbs. Fix this by using a dedicated
workqueue which will be drained of requeued items as well when
destroying it.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 17 ++++++++++++++---
 include/drm/drm_dp_mst_helper.h       |  8 ++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 083255c33ee0..075fb5ac9264 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1630,7 +1630,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref *kref)
 	mutex_lock(&mgr->delayed_destroy_lock);
 	list_add(&mstb->destroy_next, &mgr->destroy_branch_device_list);
 	mutex_unlock(&mgr->delayed_destroy_lock);
-	schedule_work(&mgr->delayed_destroy_work);
+	queue_work(mgr->delayed_destroy_wq, &mgr->delayed_destroy_work);
 }
 
 /**
@@ -1747,7 +1747,7 @@ static void drm_dp_destroy_port(struct kref *kref)
 	mutex_lock(&mgr->delayed_destroy_lock);
 	list_add(&port->next, &mgr->destroy_port_list);
 	mutex_unlock(&mgr->delayed_destroy_lock);
-	schedule_work(&mgr->delayed_destroy_work);
+	queue_work(mgr->delayed_destroy_wq, &mgr->delayed_destroy_work);
 }
 
 /**
@@ -5208,6 +5208,15 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
 	INIT_LIST_HEAD(&mgr->destroy_port_list);
 	INIT_LIST_HEAD(&mgr->destroy_branch_device_list);
 	INIT_LIST_HEAD(&mgr->up_req_list);
+
+	/*
+	 * delayed_destroy_work will be queued on a dedicated WQ, so that any
+	 * requeuing will be also flushed when deiniting the topology manager.
+	 */
+	mgr->delayed_destroy_wq = alloc_ordered_workqueue("drm_dp_mst_wq", 0);
+	if (mgr->delayed_destroy_wq == NULL)
+		return -ENOMEM;
+
 	INIT_WORK(&mgr->work, drm_dp_mst_link_probe_work);
 	INIT_WORK(&mgr->tx_work, drm_dp_tx_work);
 	INIT_WORK(&mgr->delayed_destroy_work, drm_dp_delayed_destroy_work);
@@ -5252,7 +5261,9 @@ void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr)
 {
 	drm_dp_mst_topology_mgr_set_mst(mgr, false);
 	flush_work(&mgr->work);
-	cancel_work_sync(&mgr->delayed_destroy_work);
+	/* The following will also drain any requeued work on the WQ. */
+	destroy_workqueue(mgr->delayed_destroy_wq);
+	mgr->delayed_destroy_wq = NULL;
 	mutex_lock(&mgr->payload_lock);
 	kfree(mgr->payloads);
 	mgr->payloads = NULL;
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index b230ff6f7081..8b9eb4db3381 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -681,6 +681,14 @@ struct drm_dp_mst_topology_mgr {
 	 * @destroy_branch_device_list.
 	 */
 	struct mutex delayed_destroy_lock;
+
+	/**
+	 * @delayed_destroy_wq: Workqueue used for delayed_destroy_work items.
+	 * A dedicated WQ makes it possible to drain any requeued work items
+	 * on it.
+	 */
+	struct workqueue_struct *delayed_destroy_wq;
+
 	/**
 	 * @delayed_destroy_work: Work item to destroy MST port and branch
 	 * devices, needed to avoid locking inversion.
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] drm/dp_mst: Fix the DDC I2C device unregistration of an MST port

[Lisovskiy, Stanislav]

On Mon, Jun 08, 2020 at 12:25:20AM +0300, Imre Deak wrote:
> The WARN below triggers during the removal of an MST port. The problem
> is that the parent device's (the connector's kdev) sysfs directory is
> removed recursively when the connector is unregistered (even though the
> I2C device holds a reference on the parent device). To fix this set
> first the Peer Device Type to none which will remove the I2C device.
> 
> Note that atm, inconsistently, the parent of the I2C device is initially set to
> the DRM kdev and after a Connection Status Notification the parent may be reset
> to be the connector's kdev. This problem is addressed by the next patch.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> 
> [ 4462.989299] ------------[ cut here ]------------
> [ 4463.014940] sysfs group 'power' not found for kobject 'i2c-24'
> [ 4463.034664] WARNING: CPU: 0 PID: 970 at fs/sysfs/group.c:281 sysfs_remove_group+0x71/0x80
> [ 4463.044357] Modules linked in: snd_hda_intel i915 drm_kms_helper(O) drm netconsole snd_hda_codec_hdmi mei_hdcp x86_pkg_temp_thermal coretemp crct10dif_pclmul snd_intel_dspcf
> g crc32_pclmul snd_hda_codec snd_hwdep ghash_clmulni_intel snd_hda_core asix usbnet kvm_intel mii i2c_algo_bit snd_pcm syscopyarea sysfillrect e1000e sysimgblt fb_sys_fops prim
> e_numbers ptp pps_core i2c_i801 r8169 mei_me realtek mei [last unloaded: drm]
> [ 4463.044399] CPU: 0 PID: 970 Comm: kworker/0:2 Tainted: G           O      5.7.0+ #172
> [ 4463.044402] Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP
> [ 4463.044423] Workqueue: events drm_dp_delayed_destroy_work [drm_kms_helper]
> [ 4463.044428] RIP: 0010:sysfs_remove_group+0x71/0x80
> [ 4463.044431] Code: 48 89 df 5b 5d 41 5c e9 cd b6 ff ff 48 89 df e8 95 b4 ff ff eb cb 49 8b 14 24 48 8b 75 00 48 c7 c7 20 0f 3f 82 e8 9f c5 d7 ff <0f> 0b 5b 5d 41 5c c3 0f 1f
> 84 00 00 00 00 00 48 85 f6 74 31 41 54
> [ 4463.044433] RSP: 0018:ffffc900018bfbf0 EFLAGS: 00010282
> [ 4463.044436] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000001
> [ 4463.044439] RDX: 0000000080000001 RSI: ffff88849e828f38 RDI: 00000000ffffffff
> [ 4463.052970] [drm:drm_atomic_get_plane_state [drm]] Added [PLANE:100:plane 2B] 00000000c2160caa state to 00000000d172564a
> [ 4463.070533] RBP: ffffffff820cea20 R08: ffff88847f4b8958 R09: 0000000000000000
> [ 4463.070535] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88848a725018
> [ 4463.070537] R13: 0000000000000000 R14: ffffffff827090e0 R15: 0000000000000002
> [ 4463.070539] FS:  0000000000000000(0000) GS:ffff88849e800000(0000) knlGS:0000000000000000
> [ 4463.070541] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 4463.070543] CR2: 00007fdf8a756538 CR3: 0000000489684001 CR4: 0000000000760ef0
> [ 4463.070545] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 4463.070547] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 4463.070549] PKRU: 55555554
> [ 4463.070551] Call Trace:
> [ 4463.070560]  device_del+0x84/0x400
> [ 4463.070571]  cdev_device_del+0x10/0x30
> [ 4463.070578]  put_i2c_dev+0x69/0x80
> [ 4463.070584]  i2cdev_detach_adapter+0x2e/0x60
> [ 4463.070591]  notifier_call_chain+0x34/0x90
> [ 4463.070599]  blocking_notifier_call_chain+0x3f/0x60
> [ 4463.070606]  device_del+0x7c/0x400
> [ 4463.087817]  ? lockdep_init_map_waits+0x57/0x210
> [ 4463.087825]  device_unregister+0x11/0x60
> [ 4463.087829]  i2c_del_adapter+0x249/0x310
> [ 4463.087846]  drm_dp_port_set_pdt+0x6b/0x2c0 [drm_kms_helper]
> [ 4463.087862]  drm_dp_delayed_destroy_work+0x2af/0x350 [drm_kms_helper]
> [ 4463.087876]  process_one_work+0x268/0x600
> [ 4463.105438]  ? __schedule+0x30c/0x920
> [ 4463.105451]  worker_thread+0x37/0x380
> [ 4463.105457]  ? process_one_work+0x600/0x600
> [ 4463.105462]  kthread+0x140/0x160
> [ 4463.105466]  ? kthread_park+0x80/0x80
> [ 4463.105474]  ret_from_fork+0x24/0x50
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 2a309fb2c4cc..02c800b8199f 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4669,12 +4669,13 @@ static void drm_dp_tx_work(struct work_struct *work)
>  static inline void
>  drm_dp_delayed_destroy_port(struct drm_dp_mst_port *port)
>  {
> +	drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE, port->mcs);
> +
>  	if (port->connector) {
>  		drm_connector_unregister(port->connector);
>  		drm_connector_put(port->connector);
>  	}
>  
> -	drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE, port->mcs);
>  	drm_dp_mst_put_port_malloc(port);
>  }
>  
> -- 
> 2.23.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/dp_mst: Fix flushing the delayed port/mstb destroy work

[Lisovskiy, Stanislav]

On Mon, Jun 08, 2020 at 12:25:22AM +0300, Imre Deak wrote:
> Atm, a pending delayed destroy work during module removal will be
> canceled, leaving behind MST ports, mstbs. Fix this by using a dedicated
> workqueue which will be drained of requeued items as well when
> destroying it.
>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 17 ++++++++++++++---
>  include/drm/drm_dp_mst_helper.h       |  8 ++++++++
>  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 083255c33ee0..075fb5ac9264 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1630,7 +1630,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref *kref)
>  	mutex_lock(&mgr->delayed_destroy_lock);
>  	list_add(&mstb->destroy_next, &mgr->destroy_branch_device_list);
>  	mutex_unlock(&mgr->delayed_destroy_lock);
> -	schedule_work(&mgr->delayed_destroy_work);
> +	queue_work(mgr->delayed_destroy_wq, &mgr->delayed_destroy_work);
>  }
>  
>  /**
> @@ -1747,7 +1747,7 @@ static void drm_dp_destroy_port(struct kref *kref)
>  	mutex_lock(&mgr->delayed_destroy_lock);
>  	list_add(&port->next, &mgr->destroy_port_list);
>  	mutex_unlock(&mgr->delayed_destroy_lock);
> -	schedule_work(&mgr->delayed_destroy_work);
> +	queue_work(mgr->delayed_destroy_wq, &mgr->delayed_destroy_work);
>  }
>  
>  /**
> @@ -5208,6 +5208,15 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
>  	INIT_LIST_HEAD(&mgr->destroy_port_list);
>  	INIT_LIST_HEAD(&mgr->destroy_branch_device_list);
>  	INIT_LIST_HEAD(&mgr->up_req_list);
> +
> +	/*
> +	 * delayed_destroy_work will be queued on a dedicated WQ, so that any
> +	 * requeuing will be also flushed when deiniting the topology manager.
> +	 */
> +	mgr->delayed_destroy_wq = alloc_ordered_workqueue("drm_dp_mst_wq", 0);
> +	if (mgr->delayed_destroy_wq == NULL)
> +		return -ENOMEM;
> +
>  	INIT_WORK(&mgr->work, drm_dp_mst_link_probe_work);
>  	INIT_WORK(&mgr->tx_work, drm_dp_tx_work);
>  	INIT_WORK(&mgr->delayed_destroy_work, drm_dp_delayed_destroy_work);
> @@ -5252,7 +5261,9 @@ void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr)
>  {
>  	drm_dp_mst_topology_mgr_set_mst(mgr, false);
>  	flush_work(&mgr->work);
> -	cancel_work_sync(&mgr->delayed_destroy_work);
> +	/* The following will also drain any requeued work on the WQ. */
> +	destroy_workqueue(mgr->delayed_destroy_wq);
> +	mgr->delayed_destroy_wq = NULL;
>  	mutex_lock(&mgr->payload_lock);
>  	kfree(mgr->payloads);
>  	mgr->payloads = NULL;
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index b230ff6f7081..8b9eb4db3381 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -681,6 +681,14 @@ struct drm_dp_mst_topology_mgr {
>  	 * @destroy_branch_device_list.
>  	 */
>  	struct mutex delayed_destroy_lock;
> +
> +	/**
> +	 * @delayed_destroy_wq: Workqueue used for delayed_destroy_work items.
> +	 * A dedicated WQ makes it possible to drain any requeued work items
> +	 * on it.
> +	 */
> +	struct workqueue_struct *delayed_destroy_wq;
> +
>  	/**
>  	 * @delayed_destroy_work: Work item to destroy MST port and branch
>  	 * devices, needed to avoid locking inversion.
> -- 
> 2.23.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/dp_mst: Fix the DDC I2C device registration of an MST port

[Lisovskiy, Stanislav]

On Mon, Jun 08, 2020 at 12:25:21AM +0300, Imre Deak wrote:
> During the initial MST probing an MST port's I2C device will be
> registered using the kdev of the DRM device as a parent. Later after MST
> Connection Status Notifications this I2C device will be re-registered
> with the kdev of the port's connector. This will also move
> inconsistently the I2C device's sysfs entry from the DRM device's sysfs
> dir to the connector's dir.
> 
> Fix the above by keeping the DRM kdev as the parent of the I2C device.
> 
> Ideally the connector's kdev would be used as a parent, similarly to
> non-MST connectors, however that needs some more refactoring to ensure
> the connector's kdev is already available early enough. So keep the
> existing (initial) behavior for now.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 28 +++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 02c800b8199f..083255c33ee0 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -88,8 +88,8 @@ static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
>  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
>  				 u8 *guid);
>  
> -static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux);
> -static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux);
> +static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port);
> +static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port);
>  static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
>  
>  #define DBG_PREFIX "[dp_mst]"
> @@ -1993,7 +1993,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
>  			}
>  
>  			/* remove i2c over sideband */
> -			drm_dp_mst_unregister_i2c_bus(&port->aux);
> +			drm_dp_mst_unregister_i2c_bus(port);
>  		} else {
>  			mutex_lock(&mgr->lock);
>  			drm_dp_mst_topology_put_mstb(port->mstb);
> @@ -2008,7 +2008,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
>  	if (port->pdt != DP_PEER_DEVICE_NONE) {
>  		if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
>  			/* add i2c over sideband */
> -			ret = drm_dp_mst_register_i2c_bus(&port->aux);
> +			ret = drm_dp_mst_register_i2c_bus(port);
>  		} else {
>  			lct = drm_dp_calculate_rad(port, rad);
>  			mstb = drm_dp_add_mst_branch_device(lct, rad);
> @@ -5375,22 +5375,26 @@ static const struct i2c_algorithm drm_dp_mst_i2c_algo = {
>  
>  /**
>   * drm_dp_mst_register_i2c_bus() - register an I2C adapter for I2C-over-AUX
> - * @aux: DisplayPort AUX channel
> + * @port: The port to add the I2C bus on
>   *
>   * Returns 0 on success or a negative error code on failure.
>   */
> -static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux)
> +static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port)
>  {
> +	struct drm_dp_aux *aux = &port->aux;
> +	struct device *parent_dev = port->mgr->dev->dev;
> +

So are we sure that this will always give us thr kdev of the drm device?
I mean could there be more complex hierarchy? Just wondering if there is 
a way to get drm device kdev in a more explicit way.
 
>  	aux->ddc.algo = &drm_dp_mst_i2c_algo;
>  	aux->ddc.algo_data = aux;
>  	aux->ddc.retries = 3;
>  
>  	aux->ddc.class = I2C_CLASS_DDC;
>  	aux->ddc.owner = THIS_MODULE;
> -	aux->ddc.dev.parent = aux->dev;
> -	aux->ddc.dev.of_node = aux->dev->of_node;
> +	/* FIXME: set the kdev of the port's connector as parent */
> +	aux->ddc.dev.parent = parent_dev;
> +	aux->ddc.dev.of_node = parent_dev->of_node;
>  
> -	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
> +	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(parent_dev),
>  		sizeof(aux->ddc.name));
>  
>  	return i2c_add_adapter(&aux->ddc);
> @@ -5398,11 +5402,11 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux)
>  
>  /**
>   * drm_dp_mst_unregister_i2c_bus() - unregister an I2C-over-AUX adapter
> - * @aux: DisplayPort AUX channel
> + * @port: The port to remove the I2C bus from
>   */
> -static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux)
> +static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port)
>  {
> -	i2c_del_adapter(&aux->ddc);
> +	i2c_del_adapter(&port->aux.ddc);
>  }
>  
>  /**
> -- 
> 2.23.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/dp_mst: Fix the DDC I2C device registration of an MST port

[Imre Deak]

On Wed, Jun 10, 2020 at 11:03:04AM +0300, Lisovskiy, Stanislav wrote:
> On Mon, Jun 08, 2020 at 12:25:21AM +0300, Imre Deak wrote:
> > During the initial MST probing an MST port's I2C device will be
> > registered using the kdev of the DRM device as a parent. Later after MST
> > Connection Status Notifications this I2C device will be re-registered
> > with the kdev of the port's connector. This will also move
> > inconsistently the I2C device's sysfs entry from the DRM device's sysfs
> > dir to the connector's dir.
> > 
> > Fix the above by keeping the DRM kdev as the parent of the I2C device.
> > 
> > Ideally the connector's kdev would be used as a parent, similarly to
> > non-MST connectors, however that needs some more refactoring to ensure
> > the connector's kdev is already available early enough. So keep the
> > existing (initial) behavior for now.
> > 
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 28 +++++++++++++++------------
> >  1 file changed, 16 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 02c800b8199f..083255c33ee0 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -88,8 +88,8 @@ static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> >  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> >  				 u8 *guid);
> >  
> > -static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux);
> > -static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux);
> > +static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port);
> > +static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port);
> >  static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
> >  
> >  #define DBG_PREFIX "[dp_mst]"
> > @@ -1993,7 +1993,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
> >  			}
> >  
> >  			/* remove i2c over sideband */
> > -			drm_dp_mst_unregister_i2c_bus(&port->aux);
> > +			drm_dp_mst_unregister_i2c_bus(port);
> >  		} else {
> >  			mutex_lock(&mgr->lock);
> >  			drm_dp_mst_topology_put_mstb(port->mstb);
> > @@ -2008,7 +2008,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
> >  	if (port->pdt != DP_PEER_DEVICE_NONE) {
> >  		if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
> >  			/* add i2c over sideband */
> > -			ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > +			ret = drm_dp_mst_register_i2c_bus(port);
> >  		} else {
> >  			lct = drm_dp_calculate_rad(port, rad);
> >  			mstb = drm_dp_add_mst_branch_device(lct, rad);
> > @@ -5375,22 +5375,26 @@ static const struct i2c_algorithm drm_dp_mst_i2c_algo = {
> >  
> >  /**
> >   * drm_dp_mst_register_i2c_bus() - register an I2C adapter for I2C-over-AUX
> > - * @aux: DisplayPort AUX channel
> > + * @port: The port to add the I2C bus on
> >   *
> >   * Returns 0 on success or a negative error code on failure.
> >   */
> > -static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux)
> > +static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port)
> >  {
> > +	struct drm_dp_aux *aux = &port->aux;
> > +	struct device *parent_dev = port->mgr->dev->dev;
> > +
> 
> So are we sure that this will always give us thr kdev of the drm device?
> I mean could there be more complex hierarchy? Just wondering if there is 
> a way to get drm device kdev in a more explicit way.

There is a single mgr per DRM driver (kdev) and port objects created by
a given DRM driver will stay owned by the same DRM driver. So the
kdev->port association is static.

> >  	aux->ddc.algo = &drm_dp_mst_i2c_algo;
> >  	aux->ddc.algo_data = aux;
> >  	aux->ddc.retries = 3;
> >  
> >  	aux->ddc.class = I2C_CLASS_DDC;
> >  	aux->ddc.owner = THIS_MODULE;
> > -	aux->ddc.dev.parent = aux->dev;
> > -	aux->ddc.dev.of_node = aux->dev->of_node;
> > +	/* FIXME: set the kdev of the port's connector as parent */
> > +	aux->ddc.dev.parent = parent_dev;
> > +	aux->ddc.dev.of_node = parent_dev->of_node;
> >  
> > -	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
> > +	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(parent_dev),
> >  		sizeof(aux->ddc.name));
> >  
> >  	return i2c_add_adapter(&aux->ddc);
> > @@ -5398,11 +5402,11 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux)
> >  
> >  /**
> >   * drm_dp_mst_unregister_i2c_bus() - unregister an I2C-over-AUX adapter
> > - * @aux: DisplayPort AUX channel
> > + * @port: The port to remove the I2C bus from
> >   */
> > -static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux)
> > +static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port)
> >  {
> > -	i2c_del_adapter(&aux->ddc);
> > +	i2c_del_adapter(&port->aux.ddc);
> >  }
> >  
> >  /**
> > -- 
> > 2.23.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/dp_mst: Fix the DDC I2C device registration of an MST port

[Lisovskiy, Stanislav]

On Wed, Jun 10, 2020 at 01:09:36PM +0300, Imre Deak wrote:
> On Wed, Jun 10, 2020 at 11:03:04AM +0300, Lisovskiy, Stanislav wrote:
> > On Mon, Jun 08, 2020 at 12:25:21AM +0300, Imre Deak wrote:
> > > During the initial MST probing an MST port's I2C device will be
> > > registered using the kdev of the DRM device as a parent. Later after MST
> > > Connection Status Notifications this I2C device will be re-registered
> > > with the kdev of the port's connector. This will also move
> > > inconsistently the I2C device's sysfs entry from the DRM device's sysfs
> > > dir to the connector's dir.
> > > 
> > > Fix the above by keeping the DRM kdev as the parent of the I2C device.
> > > 
> > > Ideally the connector's kdev would be used as a parent, similarly to
> > > non-MST connectors, however that needs some more refactoring to ensure
> > > the connector's kdev is already available early enough. So keep the
> > > existing (initial) behavior for now.
> > > 
> > > Cc: <stable@vger.kernel.org>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_mst_topology.c | 28 +++++++++++++++------------
> > >  1 file changed, 16 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 02c800b8199f..083255c33ee0 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -88,8 +88,8 @@ static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> > >  static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> > >  				 u8 *guid);
> > >  
> > > -static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux);
> > > -static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux);
> > > +static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port);
> > > +static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port);
> > >  static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
> > >  
> > >  #define DBG_PREFIX "[dp_mst]"
> > > @@ -1993,7 +1993,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
> > >  			}
> > >  
> > >  			/* remove i2c over sideband */
> > > -			drm_dp_mst_unregister_i2c_bus(&port->aux);
> > > +			drm_dp_mst_unregister_i2c_bus(port);
> > >  		} else {
> > >  			mutex_lock(&mgr->lock);
> > >  			drm_dp_mst_topology_put_mstb(port->mstb);
> > > @@ -2008,7 +2008,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
> > >  	if (port->pdt != DP_PEER_DEVICE_NONE) {
> > >  		if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
> > >  			/* add i2c over sideband */
> > > -			ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > > +			ret = drm_dp_mst_register_i2c_bus(port);
> > >  		} else {
> > >  			lct = drm_dp_calculate_rad(port, rad);
> > >  			mstb = drm_dp_add_mst_branch_device(lct, rad);
> > > @@ -5375,22 +5375,26 @@ static const struct i2c_algorithm drm_dp_mst_i2c_algo = {
> > >  
> > >  /**
> > >   * drm_dp_mst_register_i2c_bus() - register an I2C adapter for I2C-over-AUX
> > > - * @aux: DisplayPort AUX channel
> > > + * @port: The port to add the I2C bus on
> > >   *
> > >   * Returns 0 on success or a negative error code on failure.
> > >   */
> > > -static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux)
> > > +static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port *port)
> > >  {
> > > +	struct drm_dp_aux *aux = &port->aux;
> > > +	struct device *parent_dev = port->mgr->dev->dev;
> > > +
> > 
> > So are we sure that this will always give us thr kdev of the drm device?
> > I mean could there be more complex hierarchy? Just wondering if there is 
> > a way to get drm device kdev in a more explicit way.
> 
> There is a single mgr per DRM driver (kdev) and port objects created by
> a given DRM driver will stay owned by the same DRM driver. So the
> kdev->port association is static.

Ok, thanks for clarification. lgtm then.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> 
> > >  	aux->ddc.algo = &drm_dp_mst_i2c_algo;
> > >  	aux->ddc.algo_data = aux;
> > >  	aux->ddc.retries = 3;
> > >  
> > >  	aux->ddc.class = I2C_CLASS_DDC;
> > >  	aux->ddc.owner = THIS_MODULE;
> > > -	aux->ddc.dev.parent = aux->dev;
> > > -	aux->ddc.dev.of_node = aux->dev->of_node;
> > > +	/* FIXME: set the kdev of the port's connector as parent */
> > > +	aux->ddc.dev.parent = parent_dev;
> > > +	aux->ddc.dev.of_node = parent_dev->of_node;
> > >  
> > > -	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
> > > +	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(parent_dev),
> > >  		sizeof(aux->ddc.name));
> > >  
> > >  	return i2c_add_adapter(&aux->ddc);
> > > @@ -5398,11 +5402,11 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux)
> > >  
> > >  /**
> > >   * drm_dp_mst_unregister_i2c_bus() - unregister an I2C-over-AUX adapter
> > > - * @aux: DisplayPort AUX channel
> > > + * @port: The port to remove the I2C bus from
> > >   */
> > > -static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux)
> > > +static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port)
> > >  {
> > > -	i2c_del_adapter(&aux->ddc);
> > > +	i2c_del_adapter(&port->aux.ddc);
> > >  }
> > >  
> > >  /**
> > > -- 
> > > 2.23.1
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/3] drm/dp_mst: Fix flushing the delayed port/mstb destroy work

[Imre Deak]

Atm, a pending delayed destroy work during module removal will be
canceled, leaving behind MST ports, mstbs. Fix this by using a dedicated
workqueue which will be drained of requeued items as well when
destroying it.

v2:
- Check if wq is NULL before calling destroy_workqueue().

Cc: Lyude Paul <lyude@redhat.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 19 ++++++++++++++++---
 include/drm/drm_dp_mst_helper.h       |  8 ++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index eff8d6ac0273..a5f67b9db7fa 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1604,7 +1604,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref *kref)
 	mutex_lock(&mgr->delayed_destroy_lock);
 	list_add(&mstb->destroy_next, &mgr->destroy_branch_device_list);
 	mutex_unlock(&mgr->delayed_destroy_lock);
-	schedule_work(&mgr->delayed_destroy_work);
+	queue_work(mgr->delayed_destroy_wq, &mgr->delayed_destroy_work);
 }
 
 /**
@@ -1721,7 +1721,7 @@ static void drm_dp_destroy_port(struct kref *kref)
 	mutex_lock(&mgr->delayed_destroy_lock);
 	list_add(&port->next, &mgr->destroy_port_list);
 	mutex_unlock(&mgr->delayed_destroy_lock);
-	schedule_work(&mgr->delayed_destroy_work);
+	queue_work(mgr->delayed_destroy_wq, &mgr->delayed_destroy_work);
 }
 
 /**
@@ -5182,6 +5182,15 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
 	INIT_LIST_HEAD(&mgr->destroy_port_list);
 	INIT_LIST_HEAD(&mgr->destroy_branch_device_list);
 	INIT_LIST_HEAD(&mgr->up_req_list);
+
+	/*
+	 * delayed_destroy_work will be queued on a dedicated WQ, so that any
+	 * requeuing will be also flushed when deiniting the topology manager.
+	 */
+	mgr->delayed_destroy_wq = alloc_ordered_workqueue("drm_dp_mst_wq", 0);
+	if (mgr->delayed_destroy_wq == NULL)
+		return -ENOMEM;
+
 	INIT_WORK(&mgr->work, drm_dp_mst_link_probe_work);
 	INIT_WORK(&mgr->tx_work, drm_dp_tx_work);
 	INIT_WORK(&mgr->delayed_destroy_work, drm_dp_delayed_destroy_work);
@@ -5226,7 +5235,11 @@ void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr)
 {
 	drm_dp_mst_topology_mgr_set_mst(mgr, false);
 	flush_work(&mgr->work);
-	cancel_work_sync(&mgr->delayed_destroy_work);
+	/* The following will also drain any requeued work on the WQ. */
+	if (mgr->delayed_destroy_wq) {
+		destroy_workqueue(mgr->delayed_destroy_wq);
+		mgr->delayed_destroy_wq = NULL;
+	}
 	mutex_lock(&mgr->payload_lock);
 	kfree(mgr->payloads);
 	mgr->payloads = NULL;
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 9e1ffcd7cb68..17b568c6f4f8 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -672,6 +672,14 @@ struct drm_dp_mst_topology_mgr {
 	 * @destroy_branch_device_list.
 	 */
 	struct mutex delayed_destroy_lock;
+
+	/**
+	 * @delayed_destroy_wq: Workqueue used for delayed_destroy_work items.
+	 * A dedicated WQ makes it possible to drain any requeued work items
+	 * on it.
+	 */
+	struct workqueue_struct *delayed_destroy_wq;
+
 	/**
 	 * @delayed_destroy_work: Work item to destroy MST port and branch
 	 * devices, needed to avoid locking inversion.
-- 
2.23.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/3] drm/dp_mst: Fix flushing the delayed port/mstb destroy work

[Lyude Paul]

my crunch time is over so I can review these on time now :)

one small comment below, although it doesn't stop me from giving my R-B here:

Reviewed-by: Lyude Paul <lyude@redhat.com>


On Wed, 2020-06-10 at 16:47 +0300, Imre Deak wrote:
> Atm, a pending delayed destroy work during module removal will be
> canceled, leaving behind MST ports, mstbs. Fix this by using a dedicated
> workqueue which will be drained of requeued items as well when
> destroying it.
> 
> v2:
> - Check if wq is NULL before calling destroy_workqueue().
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 19 ++++++++++++++++---
>  include/drm/drm_dp_mst_helper.h       |  8 ++++++++
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index eff8d6ac0273..a5f67b9db7fa 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1604,7 +1604,7 @@ static void drm_dp_destroy_mst_branch_device(struct kref
> *kref)
>  	mutex_lock(&mgr->delayed_destroy_lock);
>  	list_add(&mstb->destroy_next, &mgr->destroy_branch_device_list);
>  	mutex_unlock(&mgr->delayed_destroy_lock);
> -	schedule_work(&mgr->delayed_destroy_work);
> +	queue_work(mgr->delayed_destroy_wq, &mgr->delayed_destroy_work);
>  }
>  
>  /**
> @@ -1721,7 +1721,7 @@ static void drm_dp_destroy_port(struct kref *kref)
>  	mutex_lock(&mgr->delayed_destroy_lock);
>  	list_add(&port->next, &mgr->destroy_port_list);
>  	mutex_unlock(&mgr->delayed_destroy_lock);
> -	schedule_work(&mgr->delayed_destroy_work);
> +	queue_work(mgr->delayed_destroy_wq, &mgr->delayed_destroy_work);
>  }
>  
>  /**
> @@ -5182,6 +5182,15 @@ int drm_dp_mst_topology_mgr_init(struct
> drm_dp_mst_topology_mgr *mgr,
>  	INIT_LIST_HEAD(&mgr->destroy_port_list);
>  	INIT_LIST_HEAD(&mgr->destroy_branch_device_list);
>  	INIT_LIST_HEAD(&mgr->up_req_list);
> +
> +	/*
> +	 * delayed_destroy_work will be queued on a dedicated WQ, so that any
> +	 * requeuing will be also flushed when deiniting the topology manager.
> +	 */
> +	mgr->delayed_destroy_wq = alloc_ordered_workqueue("drm_dp_mst_wq", 0);
> +	if (mgr->delayed_destroy_wq == NULL)
> +		return -ENOMEM;
> +
>  	INIT_WORK(&mgr->work, drm_dp_mst_link_probe_work);
>  	INIT_WORK(&mgr->tx_work, drm_dp_tx_work);
>  	INIT_WORK(&mgr->delayed_destroy_work, drm_dp_delayed_destroy_work);
> @@ -5226,7 +5235,11 @@ void drm_dp_mst_topology_mgr_destroy(struct
> drm_dp_mst_topology_mgr *mgr)
>  {
>  	drm_dp_mst_topology_mgr_set_mst(mgr, false);
>  	flush_work(&mgr->work);
> -	cancel_work_sync(&mgr->delayed_destroy_work);
> +	/* The following will also drain any requeued work on the WQ. */
> +	if (mgr->delayed_destroy_wq) {
> +		destroy_workqueue(mgr->delayed_destroy_wq);
> +		mgr->delayed_destroy_wq = NULL;
> +	}

We should definitely cleanup the cleanup in this function, I don't mind
submitting some patches to do it today if you poke me on IRC once you've got
this pushed to drm-misc-next
>  	mutex_lock(&mgr->payload_lock);
>  	kfree(mgr->payloads);
>  	mgr->payloads = NULL;
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 9e1ffcd7cb68..17b568c6f4f8 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -672,6 +672,14 @@ struct drm_dp_mst_topology_mgr {
>  	 * @destroy_branch_device_list.
>  	 */
>  	struct mutex delayed_destroy_lock;
> +
> +	/**
> +	 * @delayed_destroy_wq: Workqueue used for delayed_destroy_work items.
> +	 * A dedicated WQ makes it possible to drain any requeued work items
> +	 * on it.
> +	 */
> +	struct workqueue_struct *delayed_destroy_wq;
> +
>  	/**
>  	 * @delayed_destroy_work: Work item to destroy MST port and branch
>  	 * devices, needed to avoid locking inversion.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: ✓ Fi.CI.IGT: success for series starting with [1/3] drm/dp_mst: Fix the DDC I2C device unregistration of an MST port (rev2)

[Imre Deak]

On Thu, Jun 11, 2020 at 08:46:16AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/3] drm/dp_mst: Fix the DDC I2C device unregistration of an MST port (rev2)
> URL   : https://patchwork.freedesktop.org/series/78100/
> State : success

Thanks for the review, the patchset is pushed to drm-misc-next.

> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_8608_full -> Patchwork_17919_full
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.
> 
>   
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_17919_full:
> 
> ### Piglit changes ###
> 
> #### Possible regressions ####
> 
>   * spec@glsl-4.20@execution@conversion@frag-conversion-implicit-mat4-dmat4-zero-sign (NEW):
>     - {pig-icl-1065g7}:   NOTRUN -> [INCOMPLETE][1]
>    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/pig-icl-1065g7/spec@glsl-4.20@execution@conversion@frag-conversion-implicit-mat4-dmat4-zero-sign.html
> 
>   * spec@glsl-4.20@execution@conversion@vert-conversion-implicit-mat3-dmat3-zero-sign (NEW):
>     - {pig-icl-1065g7}:   NOTRUN -> [CRASH][2] +4 similar issues
>    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/pig-icl-1065g7/spec@glsl-4.20@execution@conversion@vert-conversion-implicit-mat3-dmat3-zero-sign.html
> 
>   
> New tests
> ---------
> 
>   New tests have been introduced between CI_DRM_8608_full and Patchwork_17919_full:
> 
> ### New Piglit tests (6) ###
> 
>   * spec@glsl-4.00@execution@built-in-functions@gs-op-div-dmat4x3-dmat4x3:
>     - Statuses : 1 crash(s)
>     - Exec time: [98.33] s
> 
>   * spec@glsl-4.20@execution@conversion@frag-conversion-implicit-mat4-dmat4-zero-sign:
>     - Statuses : 1 incomplete(s)
>     - Exec time: [0.0] s
> 
>   * spec@glsl-4.20@execution@conversion@geom-conversion-implicit-mat3-dmat3-zero-sign:
>     - Statuses : 1 crash(s)
>     - Exec time: [15.54] s
> 
>   * spec@glsl-4.20@execution@conversion@geom-conversion-implicit-mat4-dmat4-zero-sign:
>     - Statuses : 1 crash(s)
>     - Exec time: [57.24] s
> 
>   * spec@glsl-4.20@execution@conversion@vert-conversion-implicit-mat3-dmat3-zero-sign:
>     - Statuses : 1 crash(s)
>     - Exec time: [3.59] s
> 
>   * spec@glsl-4.20@execution@conversion@vert-conversion-implicit-mat3x4-dmat3x4-zero-sign:
>     - Statuses : 1 crash(s)
>     - Exec time: [25.06] s
> 
>   
> 
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_17919_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_exec_schedule@implicit-boths@bcs0:
>     - shard-snb:          [PASS][3] -> [INCOMPLETE][4] ([i915#82])
>    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-snb4/igt@gem_exec_schedule@implicit-boths@bcs0.html
>    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-snb2/igt@gem_exec_schedule@implicit-boths@bcs0.html
> 
>   * igt@gem_exec_suspend@basic-s3:
>     - shard-kbl:          [PASS][5] -> [DMESG-WARN][6] ([i915#93] / [i915#95]) +3 similar issues
>    [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-kbl6/igt@gem_exec_suspend@basic-s3.html
>    [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-kbl2/igt@gem_exec_suspend@basic-s3.html
> 
>   * igt@gem_exec_whisper@basic-forked-all:
>     - shard-glk:          [PASS][7] -> [DMESG-WARN][8] ([i915#118] / [i915#95])
>    [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-glk4/igt@gem_exec_whisper@basic-forked-all.html
>    [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-glk9/igt@gem_exec_whisper@basic-forked-all.html
> 
>   * igt@i915_suspend@sysfs-reader:
>     - shard-skl:          [PASS][9] -> [INCOMPLETE][10] ([i915#69])
>    [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl2/igt@i915_suspend@sysfs-reader.html
>    [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl6/igt@i915_suspend@sysfs-reader.html
> 
>   * igt@kms_big_fb@y-tiled-64bpp-rotate-0:
>     - shard-glk:          [PASS][11] -> [DMESG-FAIL][12] ([i915#118] / [i915#95])
>    [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-glk2/igt@kms_big_fb@y-tiled-64bpp-rotate-0.html
>    [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-glk8/igt@kms_big_fb@y-tiled-64bpp-rotate-0.html
> 
>   * igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen:
>     - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([i915#95]) +16 similar issues
>    [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-apl6/igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen.html
>    [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-apl6/igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen.html
>     - shard-kbl:          [PASS][15] -> [DMESG-FAIL][16] ([i915#54] / [i915#95]) +1 similar issue
>    [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen.html
>    [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen.html
> 
>   * igt@kms_cursor_crc@pipe-a-cursor-suspend:
>     - shard-kbl:          [PASS][17] -> [DMESG-WARN][18] ([i915#180]) +8 similar issues
>    [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
>    [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
> 
>   * igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy:
>     - shard-kbl:          [PASS][19] -> [DMESG-FAIL][20] ([i915#95])
>    [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-kbl6/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy.html
>    [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-kbl3/igt@kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy.html
> 
>   * igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
>     - shard-apl:          [PASS][21] -> [DMESG-WARN][22] ([i915#180]) +2 similar issues
>    [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-apl1/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
>    [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-apl3/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
> 
>   * igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1:
>     - shard-skl:          [PASS][23] -> [FAIL][24] ([i915#1928])
>    [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl5/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html
>    [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl3/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html
> 
>   * igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1:
>     - shard-skl:          [PASS][25] -> [DMESG-WARN][26] ([i915#1982]) +10 similar issues
>    [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl4/igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1.html
>    [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl10/igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1.html
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
>     - shard-tglb:         [PASS][27] -> [DMESG-WARN][28] ([i915#1982]) +1 similar issue
>    [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-tglb6/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc.html
>    [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-tglb3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc.html
> 
>   * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
>     - shard-skl:          [PASS][29] -> [DMESG-FAIL][30] ([fdo#108145] / [i915#1982])
>    [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
>    [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
> 
>   * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
>     - shard-skl:          [PASS][31] -> [FAIL][32] ([fdo#108145] / [i915#265])
>    [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
>    [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl6/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
> 
>   * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
>     - shard-iclb:         [PASS][33] -> [DMESG-WARN][34] ([i915#1982]) +1 similar issue
>    [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-iclb7/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html
>    [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-iclb3/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html
> 
>   * igt@kms_psr@psr2_cursor_render:
>     - shard-iclb:         [PASS][35] -> [SKIP][36] ([fdo#109441]) +1 similar issue
>    [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
>    [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-iclb5/igt@kms_psr@psr2_cursor_render.html
> 
>   * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
>     - shard-hsw:          [PASS][37] -> [INCOMPLETE][38] ([i915#61])
>    [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-hsw6/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
>    [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-hsw2/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
> 
>   * igt@perf@blocking-parameterized:
>     - shard-tglb:         [PASS][39] -> [FAIL][40] ([i915#1542])
>    [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-tglb3/igt@perf@blocking-parameterized.html
>    [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-tglb8/igt@perf@blocking-parameterized.html
> 
>   * igt@sw_sync@sync_multi_consumer:
>     - shard-tglb:         [PASS][41] -> [DMESG-WARN][42] ([i915#402])
>    [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-tglb2/igt@sw_sync@sync_multi_consumer.html
>    [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-tglb1/igt@sw_sync@sync_multi_consumer.html
> 
>   
> #### Possible fixes ####
> 
>   * igt@gem_exec_create@madvise:
>     - shard-glk:          [DMESG-WARN][43] ([i915#118] / [i915#95]) -> [PASS][44]
>    [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-glk1/igt@gem_exec_create@madvise.html
>    [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-glk7/igt@gem_exec_create@madvise.html
> 
>   * igt@gem_exec_reloc@basic-concurrent0:
>     - shard-glk:          [FAIL][45] ([i915#1930]) -> [PASS][46]
>    [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-glk2/igt@gem_exec_reloc@basic-concurrent0.html
>    [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-glk8/igt@gem_exec_reloc@basic-concurrent0.html
> 
>   * igt@gem_mmap_gtt@basic-small-copy-xy:
>     - shard-iclb:         [DMESG-WARN][47] ([i915#1982]) -> [PASS][48]
>    [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-iclb2/igt@gem_mmap_gtt@basic-small-copy-xy.html
>    [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-iclb5/igt@gem_mmap_gtt@basic-small-copy-xy.html
> 
>   * igt@gen9_exec_parse@allowed-single:
>     - shard-skl:          [DMESG-WARN][49] ([i915#1436] / [i915#716]) -> [PASS][50]
>    [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl10/igt@gen9_exec_parse@allowed-single.html
>    [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl2/igt@gen9_exec_parse@allowed-single.html
> 
>   * igt@i915_suspend@fence-restore-untiled:
>     - shard-skl:          [INCOMPLETE][51] ([i915#69]) -> [PASS][52]
>    [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl1/igt@i915_suspend@fence-restore-untiled.html
>    [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl7/igt@i915_suspend@fence-restore-untiled.html
> 
>   * igt@kms_big_fb@yf-tiled-32bpp-rotate-0:
>     - shard-kbl:          [DMESG-WARN][53] ([i915#1982]) -> [PASS][54]
>    [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-kbl6/igt@kms_big_fb@yf-tiled-32bpp-rotate-0.html
>    [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-kbl6/igt@kms_big_fb@yf-tiled-32bpp-rotate-0.html
>     - shard-glk:          [DMESG-WARN][55] ([i915#1982]) -> [PASS][56]
>    [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-glk1/igt@kms_big_fb@yf-tiled-32bpp-rotate-0.html
>    [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-glk7/igt@kms_big_fb@yf-tiled-32bpp-rotate-0.html
> 
>   * igt@kms_cursor_crc@pipe-c-cursor-suspend:
>     - shard-kbl:          [DMESG-WARN][57] ([i915#180]) -> [PASS][58] +3 similar issues
>    [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
>    [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-kbl6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
> 
>   * igt@kms_cursor_edge_walk@pipe-c-64x64-top-edge:
>     - shard-skl:          [DMESG-WARN][59] ([i915#1982]) -> [PASS][60] +3 similar issues
>    [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl1/igt@kms_cursor_edge_walk@pipe-c-64x64-top-edge.html
>    [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl2/igt@kms_cursor_edge_walk@pipe-c-64x64-top-edge.html
> 
>   * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
>     - shard-apl:          [DMESG-WARN][61] ([i915#180]) -> [PASS][62] +3 similar issues
>    [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-apl1/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
>    [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-apl3/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
> 
>   * igt@kms_frontbuffer_tracking@psr-suspend:
>     - shard-skl:          [INCOMPLETE][63] ([i915#123] / [i915#69]) -> [PASS][64]
>    [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl8/igt@kms_frontbuffer_tracking@psr-suspend.html
>    [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl10/igt@kms_frontbuffer_tracking@psr-suspend.html
> 
>   * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
>     - shard-skl:          [FAIL][65] ([fdo#108145] / [i915#265]) -> [PASS][66] +1 similar issue
>    [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl10/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
>    [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl5/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
> 
>   * igt@kms_psr@psr2_cursor_plane_onoff:
>     - shard-iclb:         [SKIP][67] ([fdo#109441]) -> [PASS][68]
>    [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-iclb6/igt@kms_psr@psr2_cursor_plane_onoff.html
>    [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
> 
>   * igt@kms_setmode@basic:
>     - shard-glk:          [FAIL][69] ([i915#31]) -> [PASS][70]
>    [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-glk9/igt@kms_setmode@basic.html
>    [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-glk4/igt@kms_setmode@basic.html
> 
>   * igt@kms_vblank@pipe-c-query-busy-hang:
>     - shard-tglb:         [DMESG-WARN][71] ([i915#402]) -> [PASS][72] +2 similar issues
>    [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-tglb2/igt@kms_vblank@pipe-c-query-busy-hang.html
>    [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-tglb1/igt@kms_vblank@pipe-c-query-busy-hang.html
> 
>   * igt@perf@blocking-parameterized:
>     - shard-iclb:         [FAIL][73] ([i915#1542]) -> [PASS][74]
>    [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-iclb4/igt@perf@blocking-parameterized.html
>    [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-iclb8/igt@perf@blocking-parameterized.html
>     - shard-hsw:          [FAIL][75] ([i915#1542]) -> [PASS][76] +1 similar issue
>    [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-hsw6/igt@perf@blocking-parameterized.html
>    [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-hsw1/igt@perf@blocking-parameterized.html
> 
>   * igt@syncobj_wait@invalid-wait-illegal-handle:
>     - shard-apl:          [DMESG-WARN][77] ([i915#95]) -> [PASS][78] +10 similar issues
>    [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-apl2/igt@syncobj_wait@invalid-wait-illegal-handle.html
>    [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-apl2/igt@syncobj_wait@invalid-wait-illegal-handle.html
> 
>   
> #### Warnings ####
> 
>   * igt@gem_exec_reloc@basic-concurrent16:
>     - shard-snb:          [FAIL][79] ([i915#1930]) -> [TIMEOUT][80] ([i915#1958])
>    [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-snb2/igt@gem_exec_reloc@basic-concurrent16.html
>    [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-snb6/igt@gem_exec_reloc@basic-concurrent16.html
> 
>   * igt@i915_pm_dc@dc6-psr:
>     - shard-tglb:         [FAIL][81] ([i915#454]) -> [SKIP][82] ([i915#468])
>    [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-tglb3/igt@i915_pm_dc@dc6-psr.html
>    [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-tglb2/igt@i915_pm_dc@dc6-psr.html
>     - shard-skl:          [DMESG-FAIL][83] ([i915#1982]) -> [FAIL][84] ([i915#454])
>    [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-skl8/igt@i915_pm_dc@dc6-psr.html
>    [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-skl1/igt@i915_pm_dc@dc6-psr.html
> 
>   * igt@i915_pm_rc6_residency@rc6-idle:
>     - shard-iclb:         [FAIL][85] ([i915#1515]) -> [WARN][86] ([i915#1515])
>    [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-iclb7/igt@i915_pm_rc6_residency@rc6-idle.html
>    [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-iclb8/igt@i915_pm_rc6_residency@rc6-idle.html
> 
>   * igt@kms_big_fb@yf-tiled-addfb:
>     - shard-snb:          [SKIP][87] ([fdo#109271]) -> [TIMEOUT][88] ([i915#1958])
>    [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-snb2/igt@kms_big_fb@yf-tiled-addfb.html
>    [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-snb6/igt@kms_big_fb@yf-tiled-addfb.html
> 
>   * igt@kms_content_protection@atomic:
>     - shard-apl:          [TIMEOUT][89] ([i915#1319] / [i915#1635]) -> [FAIL][90] ([fdo#110321] / [fdo#110336]) +1 similar issue
>    [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-apl7/igt@kms_content_protection@atomic.html
>    [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-apl7/igt@kms_content_protection@atomic.html
> 
>   * igt@kms_content_protection@atomic-dpms:
>     - shard-apl:          [DMESG-FAIL][91] ([fdo#110321]) -> [TIMEOUT][92] ([i915#1319])
>    [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-apl2/igt@kms_content_protection@atomic-dpms.html
>    [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-apl8/igt@kms_content_protection@atomic-dpms.html
>     - shard-kbl:          [TIMEOUT][93] ([i915#1319] / [i915#1958]) -> [TIMEOUT][94] ([i915#1319])
>    [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-kbl4/igt@kms_content_protection@atomic-dpms.html
>    [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-kbl7/igt@kms_content_protection@atomic-dpms.html
> 
>   * igt@kms_draw_crc@fill-fb:
>     - shard-apl:          [DMESG-WARN][95] ([i915#95]) -> [DMESG-FAIL][96] ([i915#95])
>    [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-apl6/igt@kms_draw_crc@fill-fb.html
>    [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-apl2/igt@kms_draw_crc@fill-fb.html
> 
>   * igt@kms_sysfs_edid_timing:
>     - shard-apl:          [FAIL][97] ([IGT#2]) -> [DMESG-FAIL][98] ([IGT#2] / [i915#95])
>    [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8608/shard-apl3/igt@kms_sysfs_edid_timing.html
>    [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17919/shard-apl7/igt@kms_sysfs_edid_timing.html
> 
>   
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   [IGT#2]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/2
>   [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
>   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>   [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
>   [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
>   [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
>   [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
>   [i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
>   [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
>   [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
>   [i915#1515]: https://gitlab.freedesktop.org/drm/intel/issues/1515
>   [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
>   [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
>   [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
>   [i915#1928]: https://gitlab.freedesktop.org/drm/intel/issues/1928
>   [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
>   [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
>   [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
>   [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
>   [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
>   [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
>   [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
>   [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
>   [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
>   [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
>   [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
>   [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
>   [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
>   [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
>   [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
> 
> 
> Participating hosts (11 -> 11)
> ------------------------------
> 
>   No changes in participating hosts
> 
> 
> Build changes
> -------------
> 
>   * Linux: CI_DRM_8608 -> Patchwork_17919
> 
>   CI-20190529: 20190529
>   CI_DRM_8608: e7b23e6cc4cdd7ad191bb039f803a2f13e4a0e40 @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_5700: 88e379cef970db3dab020966d5dd117de7cc03ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_17919: ae88a71f3c396ee528050ee86509f65af8509303 @ 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_17919/index.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel