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

[Andrew Donnellan]

On 22/1/19 12:05 pm, Alexey Kardashevskiy wrote:
> 
> 
> 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...

I'll change the commit message to explain it more clearly

> 
> 
>>
>> 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
>>
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited