[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 at 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 at amd.com>


>   	}
>