[PATCH] drm/amdkfd: Set iolink non-coherent in topology

[PATCH] drm/amdkfd: Set iolink non-coherent in topology

[Eric Huang]

Fix non-coherent bit of iolink properties flag
which always is 0.

Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 6075cbfde59f..8ef3add769d7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1336,6 +1336,28 @@ static void kfd_set_iolink_no_atomics(struct kfd_topology_device *dev,
 	}
 }
 
+static void kfd_set_iolink_non_coherent(struct kfd_topology_device *to_dev,
+		struct kfd_iolink_properties *from_link,
+		struct kfd_iolink_properties *to_link)
+{
+	/* CPU -> GPU with PCIe */
+	if (!to_dev->gpu &&
+			to_link->iolink_type == CRAT_IOLINK_TYPE_PCIEXPRESS)
+		to_link->flags |= CRAT_IOLINK_FLAGS_NON_COHERENT;
+
+	if (to_dev->gpu) {
+		/* GPU <-> GPU with PCIe and
+		 * Vega20 with XGMI
+		 */
+		if (to_link->iolink_type == CRAT_IOLINK_TYPE_PCIEXPRESS ||
+				(to_link->iolink_type == CRAT_IOLINK_TYPE_XGMI &&
+				to_dev->gpu->device_info->asic_family == CHIP_VEGA20)) {
+			from_link->flags |= CRAT_IOLINK_FLAGS_NON_COHERENT;
+			to_link->flags |= CRAT_IOLINK_FLAGS_NON_COHERENT;
+		}
+	}
+}
+
 static void kfd_fill_iolink_non_crat_info(struct kfd_topology_device *dev)
 {
 	struct kfd_iolink_properties *link, *inbound_link;
@@ -1361,6 +1383,7 @@ static void kfd_fill_iolink_non_crat_info(struct kfd_topology_device *dev)
 
 			inbound_link->flags = CRAT_IOLINK_FLAGS_ENABLED;
 			kfd_set_iolink_no_atomics(peer_dev, dev, inbound_link);
+			kfd_set_iolink_non_coherent(peer_dev, link, inbound_link);
 		}
 	}
 
-- 
2.25.1

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

Re: [PATCH] drm/amdkfd: Set iolink non-coherent in topology

[Felix Kuehling]

On 2021-06-16 2:12 p.m., Eric Huang wrote:
> Fix non-coherent bit of iolink properties flag
> which always is 0.
>
> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index 6075cbfde59f..8ef3add769d7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -1336,6 +1336,28 @@ static void kfd_set_iolink_no_atomics(struct kfd_topology_device *dev,
>   	}
>   }
>   
> +static void kfd_set_iolink_non_coherent(struct kfd_topology_device *to_dev,
> +		struct kfd_iolink_properties *from_link,
> +		struct kfd_iolink_properties *to_link)

The parameter names here are really misleading. "from" and "to" are 
relative to a device that's not even in the parameter list of this 
function. from_link is the link to to_dev. Please rename these from_link 
-> outbound_link, to_link -> inbound_link.


> +{
> +	/* CPU -> GPU with PCIe */
> +	if (!to_dev->gpu &&
> +			to_link->iolink_type == CRAT_IOLINK_TYPE_PCIEXPRESS)

The indentation is a bit off here. For readability I prefer indenting 
this to line up with the open ( in the line above (1 TAB + 4 spaces in 
this case).


> +		to_link->flags |= CRAT_IOLINK_FLAGS_NON_COHERENT;
> +
> +	if (to_dev->gpu) {
> +		/* GPU <-> GPU with PCIe and
> +		 * Vega20 with XGMI
> +		 */
> +		if (to_link->iolink_type == CRAT_IOLINK_TYPE_PCIEXPRESS ||
> +				(to_link->iolink_type == CRAT_IOLINK_TYPE_XGMI &&
> +				to_dev->gpu->device_info->asic_family == CHIP_VEGA20)) {

Same comment as above. Also indent the last row one more space to line 
up with the second open (.


> +			from_link->flags |= CRAT_IOLINK_FLAGS_NON_COHERENT;
> +			to_link->flags |= CRAT_IOLINK_FLAGS_NON_COHERENT;
> +		}
> +	}
> +}
> +
>   static void kfd_fill_iolink_non_crat_info(struct kfd_topology_device *dev)
>   {
>   	struct kfd_iolink_properties *link, *inbound_link;
> @@ -1361,6 +1383,7 @@ static void kfd_fill_iolink_non_crat_info(struct kfd_topology_device *dev)
>   
>   			inbound_link->flags = CRAT_IOLINK_FLAGS_ENABLED;
>   			kfd_set_iolink_no_atomics(peer_dev, dev, inbound_link);
> +			kfd_set_iolink_non_coherent(peer_dev, link, inbound_link);
>   		}
The DKMS branch has a second loop here for PCIe P2P links. Please 
prepare a patch for the DKMS branch as well so we don't forget to handle 
this.

With the above nit-picks fixed, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


>   	}
>   
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx