[Skiboot] [PATCH v2 14/15] hw/npu2-common: Allow mixed mode NPU setups

[Alexey Kardashevskiy]

On 11/01/2019 12:09, Andrew Donnellan wrote:
> Now that we have all the support in place for NPUs with both NVLink and
> OpenCAPI devices, get rid of the error that aborts NPU init when a mixed
> setup is detected.
> 
> While we're there, rename setup_devices() to more accurately reflect what
> it does, and move the calls to the NVLink/OpenCAPI setup code out of there
> into the main probe function.

setup_devices() did set up devices, what inaccurate was about that?

This is my problem with this patchset - functional changes are
intermixed with refactoring, it is quite hard to follow what individual
change is what, it does not have to be that complex...


> 
> Signed-off-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
> Reviewed-by: Frederic Barrat <fbarrat at linux.ibm.com>
> ---
>  hw/npu2-common.c   | 28 ++++++----------------------
>  hw/npu2-opencapi.c |  5 -----
>  2 files changed, 6 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/npu2-common.c b/hw/npu2-common.c
> index e323e71cb8d5..9e1b4bcc250e 100644
> --- a/hw/npu2-common.c
> +++ b/hw/npu2-common.c
> @@ -620,26 +620,19 @@ failed:
>  	return NULL;
>  }
>  
> -static void setup_devices(struct npu2 *npu)
> +static void add_link_type_properties(struct npu2 *npu)
>  {
> -	bool nvlink_detected = false, ocapi_detected = false;
>  	struct npu2_dev *dev;
>  
> -	/*
> -	 * TODO: In future, we'll do brick configuration here to support mixed
> -	 * setups.
> -	 */
>  	for (int i = 0; i < npu->total_devices; i++) {
>  		dev = &npu->devices[i];
>  		switch (dev->type) {
>  		case NPU2_DEV_TYPE_NVLINK:
> -			nvlink_detected = true;
>  			dt_add_property_strings(dev->dt_node,
>  						"ibm,npu-link-type",
>  						"nvlink");
>  			break;
>  		case NPU2_DEV_TYPE_OPENCAPI:
> -			ocapi_detected = true;
>  			dt_add_property_strings(dev->dt_node,
>  						"ibm,npu-link-type",
>  						"opencapi");
> @@ -652,19 +645,6 @@ static void setup_devices(struct npu2 *npu)
>  						"unknown");
>  		}
>  	}
> -
> -	if (nvlink_detected && ocapi_detected) {
> -		prlog(PR_ERR, "NPU: NVLink and OpenCAPI devices on same chip not supported, aborting NPU init\n");
> -		return;
> -	}
> -
> -	assign_bars(npu);
> -	setup_irqs(npu);
> -
> -	if (nvlink_detected)
> -		npu2_nvlink_init_npu(npu);
> -	else if (ocapi_detected)
> -		npu2_opencapi_init_npu(npu);
>  }
>  
>  void probe_npu2(void)
> @@ -700,7 +680,11 @@ void probe_npu2(void)
>  		if (!npu)
>  			continue;
>  		platform.npu2_device_detect(npu);
> +		add_link_type_properties(npu);
>  		set_brick_config(npu);
> -		setup_devices(npu);
> +		assign_bars(npu);
> +		setup_irqs(npu);
> +		npu2_nvlink_init_npu(npu);
> +		npu2_opencapi_init_npu(npu);
>  	}
>  }
> diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c
> index 0c7d4f9ff148..7acf3837fd40 100644
> --- a/hw/npu2-opencapi.c
> +++ b/hw/npu2-opencapi.c
> @@ -19,15 +19,10 @@
>   *
>   * This file provides support for OpenCAPI as implemented on POWER9.
>   *
> - * At present, we initialise the NPU separately from the NVLink code in npu2.c.
> - * As such, we don't currently support mixed NVLink and OpenCAPI configurations
> - * on the same NPU for machines such as Witherspoon.
> - *
>   * Procedure references in this file are to the POWER9 OpenCAPI NPU Workbook
>   * (IBM internal document).
>   *
>   * TODO:
> - *   - Support for mixed NVLink and OpenCAPI on the same NPU
>   *   - Support for link ganging (one AFU using multiple links)
>   *   - Link reset and error handling
>   *   - Presence detection
> 

-- 
Alexey