[+Eric, see my comment below about iolink atomic flags]
On 2018-09-07 05:37 PM, shaoyunl wrote:
> Change-Id: Ibb6a89ed878fffccb9a8bb4032b07a10ee298a99
> Signed-off-by: shaoyunl
See one comment inline. It's not directly related to this change so this
is Reviewed-by: Felix Kuehling
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 15 +--
> drivers/gpu/drm/amd/amdkfd/kfd_crat.h | 3 ++-
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 1 +
> 3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 130db4d..d4560f1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -353,8 +353,8 @@ static int kfd_parse_subtype_iolink(struct
> crat_subtype_iolink *iolink,
> id_from = iolink->proximity_domain_from;
> id_to = iolink->proximity_domain_to;
>
> - pr_debug("Found IO link entry in CRAT table with id_from=%d\n",
> - id_from);
> + pr_debug("Found IO link entry in CRAT table with id_from=%d, id_to
> %d\n",
> + id_from, id_to);
> list_for_each_entry(dev, device_list, list) {
> if (id_from == dev->proximity_domain) {
> props = kfd_alloc_struct(props);
> @@ -391,12 +391,12 @@ static int kfd_parse_subtype_iolink(struct
> crat_subtype_iolink *iolink,
> /* CPU topology is created before GPUs are detected, so CPU->GPU
>* links are not built at that time. If a PCIe type is discovered, it
>* means a GPU is detected and we are adding GPU->CPU to the topology.
> - * At this time, also add the corresponded CPU->GPU link.
> + * At this time, also add the corresponded CPU->GPU link if GPU
> + * is large bar.
>* For xGMI, we only added the link with one direction in the crat
>* table, add corresponded reversed direction link now.
>*/
> - if (props && (props->iolink_type == CRAT_IOLINK_TYPE_PCIEXPRESS ||
> - props->iolink_type == CRAT_IOLINK_TYPE_XGMI)) {
> + if (props && (iolink->flags & CRAT_IOLINK_FLAGS_BI_DIRECTIONAL)) {
Hmm, looks like we're not populating props->flags at the moment. That's
probably an oversight we should correct. Then we could also move the
atomic flag code from kfd_fill_iolink_non_crat_info into
kfd_fill_gpu_direct_io_link for the GPU->CPU link and this function for
the reverse.
Regards,
Felix
> to_dev = kfd_topology_device_by_proximity_domain(id_to);
> if (!to_dev)
> return -ENODEV;
> @@ -1057,6 +1057,8 @@ static int kfd_fill_gpu_direct_io_link_to_cpu(int
> *avail_size,
> sub_type_hdr->type = CRAT_SUBTYPE_IOLINK_AFFINITY;
> sub_type_hdr->length = sizeof(struct crat_subtype_iolink);
> sub_type_hdr->flags |= CRAT_SUBTYPE_FLAGS_ENABLED;
> + if (kfd_dev_is_large_bar(kdev))
> + sub_type_hdr->flags |= CRAT_IOLINK_FLAGS_BI_DIRECTIONAL;
>
> /* Fill in IOLINK subtype.
>* TODO: Fill-in other fields of iolink subtype
> @@ -1088,7 +1090,8 @@ static int kfd_fill_gpu_xgmi_link_to_gpu(int
> *avail_size,
>
> sub_type_hdr->type = CRAT_SUBTYPE_IOLINK_AFFINITY;
> sub_type_hdr->length = sizeof(struct crat_subtype_iolink);
> - sub_type_hdr->flags |= CRAT_SUBTYPE_FLAGS_ENABLED;
> + sub_type_hdr->flags |= CRAT_SUBTYPE_FLAGS_ENABLED |
> +CRAT_IOLINK_FLAGS_BI_DIRECTIONAL;
>
> sub_type_hdr->io_interface_type = CRAT_IOLINK_TYPE_XGMI;
> sub_type_hdr->proximity_domain_from = proximity_domain_from;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> index 7a93aeb..7c3f192 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> @@ -232,7 +232,8 @@ struct crat_subtype_ccompute {
> #define CRAT_IOLINK_FLAGS_NO_ATOMICS_32_BIT (1 << 2)
> #define CRAT_IOLINK_FLAGS_NO_ATOMICS_64_BIT (1 << 3)
> #define CRAT_IOLINK_FLAGS_NO_PEER_TO_PEER_DMA(1 << 4)
> -#define CRAT_IOLINK_FLAGS_RESERVED_MASK 0xffe0
> +#define CRAT_IOLINK_FLAGS_BI_DIRECTIONAL (1 << 31)
> +#define CRAT_IOLINK_FLAGS_RESERVED_MASK 0x7fe0
>
> /*
> * IO interface types
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 6a5418f..05283c9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -696,6 +696,7 @@ struct amdkfd_ioctl_desc {
> unsigned int cmd_drv;
> const char *name;
> };
> +bool kfd_dev_is_large_bar(struct kfd_dev *dev);
>
> int kfd_process_create_wq(void);
> void kfd_process_destroy_wq(void);
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
