diff mbox series

[1/9] npu2: Split out common helper functions into separate file

Message ID 76d50e65cdb3fd44f43260eb805b3a99943798aa.1513579137.git-series.andrew.donnellan@au1.ibm.com
State Changes Requested
Headers show
Series Initial OpenCAPI 3.0 Support for P9 | expand

Commit Message

Andrew Donnellan Dec. 18, 2017, 7:07 a.m. UTC 
Split out common helper functions for NPU register access into a separate
file, as these will be used extensively by both NVLink and OpenCAPI code.

Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 hw/Makefile.inc     |  2 +-
 hw/npu2-common.c    | 98 ++++++++++++++++++++++++++++++++++++++++++++++-
 hw/npu2.c           | 92 +-------------------------------------------
 include/npu2-regs.h |  5 ++-
 include/npu2.h      |  2 +-
 5 files changed, 106 insertions(+), 93 deletions(-)
 create mode 100644 hw/npu2-common.c

Comments

Alistair Popple Dec. 19, 2017, 12:37 a.m. UTC | #1 
> +bool is_p9dd1(void)

Does OpenCAPI still need this? If not we should just drop it and DD1 support
along with it as it's very unlikely anything still works there.

- Alistair

> +{
> +	struct proc_chip *chip = next_chip(NULL);
> +
> +	return chip &&
> +	       (chip->type == PROC_CHIP_P9_NIMBUS ||
> +		chip->type == PROC_CHIP_P9_CUMULUS) &&
> +	       (chip->ec_level & 0xf0) == 0x10;
> +}
> +
> +/*
> + * We use the indirect method because it uses the same addresses as
> + * the MMIO offsets (NPU RING)
> + */
> +static void npu2_scom_set_addr(uint64_t gcid, uint64_t scom_base,
> +			       uint64_t addr, uint64_t size)
> +{
> +	uint64_t isa = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_ADDR :
> +				    NPU2_MISC_SCOM_IND_SCOM_ADDR;
> +
> +	addr = SETFIELD(NPU2_MISC_DA_ADDR, 0ull, addr);
> +	addr = SETFIELD(NPU2_MISC_DA_LEN, addr, size);
> +	xscom_write(gcid, scom_base + isa, addr);
> +}
> +
> +void npu2_scom_write(uint64_t gcid, uint64_t scom_base,
> +		     uint64_t reg, uint64_t size,
> +		     uint64_t val)
> +{
> +	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
> +				    NPU2_MISC_SCOM_IND_SCOM_DATA;
> +
> +	npu2_scom_set_addr(gcid, scom_base, reg, size);
> +	xscom_write(gcid, scom_base + isd, val);
> +}
> +
> +uint64_t npu2_scom_read(uint64_t gcid, uint64_t scom_base,
> +			uint64_t reg, uint64_t size)
> +{
> +	uint64_t val;
> +	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
> +				    NPU2_MISC_SCOM_IND_SCOM_DATA;
> +
> +	npu2_scom_set_addr(gcid, scom_base, reg, size);
> +	xscom_read(gcid, scom_base + isd, &val);
> +
> +	return val;
> +}
> +
> +void npu2_write_4b(struct npu2 *p, uint64_t reg, uint32_t val)
> +{
> +	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
> +			(uint64_t)val << 32);
> +}
> +
> +uint32_t npu2_read_4b(struct npu2 *p, uint64_t reg)
> +{
> +	return npu2_scom_read(p->chip_id, p->xscom_base, reg,
> +			      NPU2_MISC_DA_LEN_4B) >> 32;
> +}
> +
> +void npu2_write(struct npu2 *p, uint64_t reg, uint64_t val)
> +{
> +	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, val);
> +}
> +
> +uint64_t npu2_read(struct npu2 *p, uint64_t reg)
> +{
> +	return npu2_scom_read(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B);
> +}
> +
> +void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask)
> +{
> +	uint64_t new_val;
> +
> +	new_val = npu2_read(p, reg);
> +	new_val &= ~mask;
> +	new_val |= val & mask;
> +	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, new_val);
> +}
> +
> +void npu2_write_mask_4b(struct npu2 *p, uint64_t reg, uint32_t val, uint32_t mask)
> +{
> +	uint32_t new_val;
> +
> +	new_val = npu2_read_4b(p, reg);
> +	new_val &= ~mask;
> +	new_val |= val & mask;
> +	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
> +			(uint64_t)new_val << 32);
> +}
> diff --git a/hw/npu2.c b/hw/npu2.c
> index 7ffb094..e66ceb9 100644
> --- a/hw/npu2.c
> +++ b/hw/npu2.c
> @@ -64,98 +64,6 @@
>   * configure one particular BAR.
>   */
>  
> -static bool is_p9dd1(void)
> -{
> -	struct proc_chip *chip = next_chip(NULL);
> -
> -	return chip &&
> -	       (chip->type == PROC_CHIP_P9_NIMBUS ||
> -		chip->type == PROC_CHIP_P9_CUMULUS) &&
> -	       (chip->ec_level & 0xf0) == 0x10;
> -}
> -
> -/*
> - * We use the indirect method because it uses the same addresses as
> - * the MMIO offsets (NPU RING)
> - */
> -static void npu2_scom_set_addr(uint64_t gcid, uint64_t scom_base,
> -			       uint64_t addr, uint64_t size)
> -{
> -	uint64_t isa = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_ADDR :
> -				    NPU2_MISC_SCOM_IND_SCOM_ADDR;
> -
> -	addr = SETFIELD(NPU2_MISC_DA_ADDR, 0ull, addr);
> -	addr = SETFIELD(NPU2_MISC_DA_LEN, addr, size);
> -	xscom_write(gcid, scom_base + isa, addr);
> -}
> -
> -static void npu2_scom_write(uint64_t gcid, uint64_t scom_base,
> -			    uint64_t reg, uint64_t size,
> -			    uint64_t val)
> -{
> -	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
> -				    NPU2_MISC_SCOM_IND_SCOM_DATA;
> -
> -	npu2_scom_set_addr(gcid, scom_base, reg, size);
> -	xscom_write(gcid, scom_base + isd, val);
> -}
> -
> -static uint64_t npu2_scom_read(uint64_t gcid, uint64_t scom_base,
> -			       uint64_t reg, uint64_t size)
> -{
> -	uint64_t val;
> -	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
> -				    NPU2_MISC_SCOM_IND_SCOM_DATA;
> -
> -	npu2_scom_set_addr(gcid, scom_base, reg, size);
> -	xscom_read(gcid, scom_base + isd, &val);
> -
> -	return val;
> -}
> -
> -void npu2_write_4b(struct npu2 *p, uint64_t reg, uint32_t val)
> -{
> -	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
> -			(uint64_t)val << 32);
> -}
> -
> -uint32_t npu2_read_4b(struct npu2 *p, uint64_t reg)
> -{
> -	return npu2_scom_read(p->chip_id, p->xscom_base, reg,
> -			      NPU2_MISC_DA_LEN_4B) >> 32;
> -}
> -
> -void npu2_write(struct npu2 *p, uint64_t reg, uint64_t val)
> -{
> -	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, val);
> -}
> -
> -uint64_t npu2_read(struct npu2 *p, uint64_t reg)
> -{
> -	return npu2_scom_read(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B);
> -}
> -
> -void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask)
> -{
> -	uint64_t new_val;
> -
> -	new_val = npu2_read(p, reg);
> -	new_val &= ~mask;
> -	new_val |= val & mask;
> -	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, new_val);
> -}
> -
> -void npu2_write_mask_4b(struct npu2 *p, uint64_t reg, uint32_t val, uint32_t mask)
> -{
> -	uint32_t new_val;
> -
> -	new_val = npu2_read_4b(p, reg);
> -	new_val &= ~mask;
> -	new_val |= val & mask;
> -	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
> -			(uint64_t)new_val << 32);
> -}
> -
>  /* Set a specific flag in the vendor config space */
>  void npu2_set_link_flag(struct npu2_dev *ndev, uint8_t flag)
>  {
> diff --git a/include/npu2-regs.h b/include/npu2-regs.h
> index fdaad19..cf74431 100644
> --- a/include/npu2-regs.h
> +++ b/include/npu2-regs.h
> @@ -23,6 +23,11 @@ void npu2_write4(struct npu2 *p, uint64_t reg, uint64_t val);
>  uint64_t npu2_read(struct npu2 *p, uint64_t reg);
>  void npu2_write(struct npu2 *p, uint64_t reg, uint64_t val);
>  void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask);
> +uint64_t npu2_scom_read(uint64_t gcid, uint64_t scom_base,
> +			uint64_t reg, uint64_t size);
> +void npu2_scom_write(uint64_t gcid, uint64_t scom_base,
> +		     uint64_t reg, uint64_t size,
> +		     uint64_t val);
>  
>  /* These aren't really NPU specific registers but we initialise them in NPU
>   * code */
> diff --git a/include/npu2.h b/include/npu2.h
> index dae152a..d0c9ac5 100644
> --- a/include/npu2.h
> +++ b/include/npu2.h
> @@ -163,4 +163,6 @@ void npu2_dev_procedure_reset(struct npu2_dev *dev);
>  void npu2_set_link_flag(struct npu2_dev *ndev, uint8_t flag);
>  void npu2_clear_link_flag(struct npu2_dev *ndev, uint8_t flag);
>  extern int nv_zcal_nominal;
> +bool is_p9dd1(void);
> +
>  #endif /* __NPU2_H */
>
Andrew Donnellan Dec. 19, 2017, 6:17 a.m. UTC | #2 
On 19/12/17 11:37, Alistair Popple wrote:
>> +bool is_p9dd1(void)
> 
> Does OpenCAPI still need this? If not we should just drop it and DD1 support
> along with it as it's very unlikely anything still works there.

OpenCAPI has no DD1 support whatsoever. If you want to drop DD1 support 
for NVLink then I have no objections!


Andrew

> 
> - Alistair
> 
>> +{
>> +	struct proc_chip *chip = next_chip(NULL);
>> +
>> +	return chip &&
>> +	       (chip->type == PROC_CHIP_P9_NIMBUS ||
>> +		chip->type == PROC_CHIP_P9_CUMULUS) &&
>> +	       (chip->ec_level & 0xf0) == 0x10;
>> +}
>> +
>> +/*
>> + * We use the indirect method because it uses the same addresses as
>> + * the MMIO offsets (NPU RING)
>> + */
>> +static void npu2_scom_set_addr(uint64_t gcid, uint64_t scom_base,
>> +			       uint64_t addr, uint64_t size)
>> +{
>> +	uint64_t isa = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_ADDR :
>> +				    NPU2_MISC_SCOM_IND_SCOM_ADDR;
>> +
>> +	addr = SETFIELD(NPU2_MISC_DA_ADDR, 0ull, addr);
>> +	addr = SETFIELD(NPU2_MISC_DA_LEN, addr, size);
>> +	xscom_write(gcid, scom_base + isa, addr);
>> +}
>> +
>> +void npu2_scom_write(uint64_t gcid, uint64_t scom_base,
>> +		     uint64_t reg, uint64_t size,
>> +		     uint64_t val)
>> +{
>> +	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
>> +				    NPU2_MISC_SCOM_IND_SCOM_DATA;
>> +
>> +	npu2_scom_set_addr(gcid, scom_base, reg, size);
>> +	xscom_write(gcid, scom_base + isd, val);
>> +}
>> +
>> +uint64_t npu2_scom_read(uint64_t gcid, uint64_t scom_base,
>> +			uint64_t reg, uint64_t size)
>> +{
>> +	uint64_t val;
>> +	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
>> +				    NPU2_MISC_SCOM_IND_SCOM_DATA;
>> +
>> +	npu2_scom_set_addr(gcid, scom_base, reg, size);
>> +	xscom_read(gcid, scom_base + isd, &val);
>> +
>> +	return val;
>> +}
>> +
>> +void npu2_write_4b(struct npu2 *p, uint64_t reg, uint32_t val)
>> +{
>> +	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
>> +			(uint64_t)val << 32);
>> +}
>> +
>> +uint32_t npu2_read_4b(struct npu2 *p, uint64_t reg)
>> +{
>> +	return npu2_scom_read(p->chip_id, p->xscom_base, reg,
>> +			      NPU2_MISC_DA_LEN_4B) >> 32;
>> +}
>> +
>> +void npu2_write(struct npu2 *p, uint64_t reg, uint64_t val)
>> +{
>> +	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, val);
>> +}
>> +
>> +uint64_t npu2_read(struct npu2 *p, uint64_t reg)
>> +{
>> +	return npu2_scom_read(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B);
>> +}
>> +
>> +void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask)
>> +{
>> +	uint64_t new_val;
>> +
>> +	new_val = npu2_read(p, reg);
>> +	new_val &= ~mask;
>> +	new_val |= val & mask;
>> +	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, new_val);
>> +}
>> +
>> +void npu2_write_mask_4b(struct npu2 *p, uint64_t reg, uint32_t val, uint32_t mask)
>> +{
>> +	uint32_t new_val;
>> +
>> +	new_val = npu2_read_4b(p, reg);
>> +	new_val &= ~mask;
>> +	new_val |= val & mask;
>> +	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
>> +			(uint64_t)new_val << 32);
>> +}
>> diff --git a/hw/npu2.c b/hw/npu2.c
>> index 7ffb094..e66ceb9 100644
>> --- a/hw/npu2.c
>> +++ b/hw/npu2.c
>> @@ -64,98 +64,6 @@
>>    * configure one particular BAR.
>>    */
>>   
>> -static bool is_p9dd1(void)
>> -{
>> -	struct proc_chip *chip = next_chip(NULL);
>> -
>> -	return chip &&
>> -	       (chip->type == PROC_CHIP_P9_NIMBUS ||
>> -		chip->type == PROC_CHIP_P9_CUMULUS) &&
>> -	       (chip->ec_level & 0xf0) == 0x10;
>> -}
>> -
>> -/*
>> - * We use the indirect method because it uses the same addresses as
>> - * the MMIO offsets (NPU RING)
>> - */
>> -static void npu2_scom_set_addr(uint64_t gcid, uint64_t scom_base,
>> -			       uint64_t addr, uint64_t size)
>> -{
>> -	uint64_t isa = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_ADDR :
>> -				    NPU2_MISC_SCOM_IND_SCOM_ADDR;
>> -
>> -	addr = SETFIELD(NPU2_MISC_DA_ADDR, 0ull, addr);
>> -	addr = SETFIELD(NPU2_MISC_DA_LEN, addr, size);
>> -	xscom_write(gcid, scom_base + isa, addr);
>> -}
>> -
>> -static void npu2_scom_write(uint64_t gcid, uint64_t scom_base,
>> -			    uint64_t reg, uint64_t size,
>> -			    uint64_t val)
>> -{
>> -	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
>> -				    NPU2_MISC_SCOM_IND_SCOM_DATA;
>> -
>> -	npu2_scom_set_addr(gcid, scom_base, reg, size);
>> -	xscom_write(gcid, scom_base + isd, val);
>> -}
>> -
>> -static uint64_t npu2_scom_read(uint64_t gcid, uint64_t scom_base,
>> -			       uint64_t reg, uint64_t size)
>> -{
>> -	uint64_t val;
>> -	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
>> -				    NPU2_MISC_SCOM_IND_SCOM_DATA;
>> -
>> -	npu2_scom_set_addr(gcid, scom_base, reg, size);
>> -	xscom_read(gcid, scom_base + isd, &val);
>> -
>> -	return val;
>> -}
>> -
>> -void npu2_write_4b(struct npu2 *p, uint64_t reg, uint32_t val)
>> -{
>> -	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
>> -			(uint64_t)val << 32);
>> -}
>> -
>> -uint32_t npu2_read_4b(struct npu2 *p, uint64_t reg)
>> -{
>> -	return npu2_scom_read(p->chip_id, p->xscom_base, reg,
>> -			      NPU2_MISC_DA_LEN_4B) >> 32;
>> -}
>> -
>> -void npu2_write(struct npu2 *p, uint64_t reg, uint64_t val)
>> -{
>> -	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, val);
>> -}
>> -
>> -uint64_t npu2_read(struct npu2 *p, uint64_t reg)
>> -{
>> -	return npu2_scom_read(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B);
>> -}
>> -
>> -void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask)
>> -{
>> -	uint64_t new_val;
>> -
>> -	new_val = npu2_read(p, reg);
>> -	new_val &= ~mask;
>> -	new_val |= val & mask;
>> -	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, new_val);
>> -}
>> -
>> -void npu2_write_mask_4b(struct npu2 *p, uint64_t reg, uint32_t val, uint32_t mask)
>> -{
>> -	uint32_t new_val;
>> -
>> -	new_val = npu2_read_4b(p, reg);
>> -	new_val &= ~mask;
>> -	new_val |= val & mask;
>> -	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
>> -			(uint64_t)new_val << 32);
>> -}
>> -
>>   /* Set a specific flag in the vendor config space */
>>   void npu2_set_link_flag(struct npu2_dev *ndev, uint8_t flag)
>>   {
>> diff --git a/include/npu2-regs.h b/include/npu2-regs.h
>> index fdaad19..cf74431 100644
>> --- a/include/npu2-regs.h
>> +++ b/include/npu2-regs.h
>> @@ -23,6 +23,11 @@ void npu2_write4(struct npu2 *p, uint64_t reg, uint64_t val);
>>   uint64_t npu2_read(struct npu2 *p, uint64_t reg);
>>   void npu2_write(struct npu2 *p, uint64_t reg, uint64_t val);
>>   void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask);
>> +uint64_t npu2_scom_read(uint64_t gcid, uint64_t scom_base,
>> +			uint64_t reg, uint64_t size);
>> +void npu2_scom_write(uint64_t gcid, uint64_t scom_base,
>> +		     uint64_t reg, uint64_t size,
>> +		     uint64_t val);
>>   
>>   /* These aren't really NPU specific registers but we initialise them in NPU
>>    * code */
>> diff --git a/include/npu2.h b/include/npu2.h
>> index dae152a..d0c9ac5 100644
>> --- a/include/npu2.h
>> +++ b/include/npu2.h
>> @@ -163,4 +163,6 @@ void npu2_dev_procedure_reset(struct npu2_dev *dev);
>>   void npu2_set_link_flag(struct npu2_dev *ndev, uint8_t flag);
>>   void npu2_clear_link_flag(struct npu2_dev *ndev, uint8_t flag);
>>   extern int nv_zcal_nominal;
>> +bool is_p9dd1(void);
>> +
>>   #endif /* __NPU2_H */
>>
> 
>
Stewart Smith Dec. 22, 2017, 4:22 a.m. UTC | #3 
Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:
> On 19/12/17 11:37, Alistair Popple wrote:
>>> +bool is_p9dd1(void)
>> 
>> Does OpenCAPI still need this? If not we should just drop it and DD1 support
>> along with it as it's very unlikely anything still works there.
>
> OpenCAPI has no DD1 support whatsoever. If you want to drop DD1 support 
> for NVLink then I have no objections!

I have no objection to this.

Anyone currently attempting NVLink on DD1 gets all the checkstops they deserve.
Alistair Popple Dec. 22, 2017, 4:28 a.m. UTC | #4 
On Friday, 22 December 2017 3:22:32 PM AEDT Stewart Smith wrote:
> Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:
> > On 19/12/17 11:37, Alistair Popple wrote:
> >>> +bool is_p9dd1(void)
> >> 
> >> Does OpenCAPI still need this? If not we should just drop it and DD1 support
> >> along with it as it's very unlikely anything still works there.
> >
> > OpenCAPI has no DD1 support whatsoever. If you want to drop DD1 support 
> > for NVLink then I have no objections!
> 
> I have no objection to this.
> 
> Anyone currently attempting NVLink on DD1 gets all the checkstops they deserve.

Yep. I'm going to pull it out. Although it may just lead to xstops on boot for
DD1 ... is that something we care about or should we still detect DD1 and
disable NVLink (which would prevent any checkstops)?

- Alistair

>
>
Stewart Smith Jan. 8, 2018, 2:39 a.m. UTC | #5 
Alistair Popple <alistair@popple.id.au> writes:
> On Friday, 22 December 2017 3:22:32 PM AEDT Stewart Smith wrote:
>> Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:
>> > On 19/12/17 11:37, Alistair Popple wrote:
>> >>> +bool is_p9dd1(void)
>> >> 
>> >> Does OpenCAPI still need this? If not we should just drop it and DD1 support
>> >> along with it as it's very unlikely anything still works there.
>> >
>> > OpenCAPI has no DD1 support whatsoever. If you want to drop DD1 support 
>> > for NVLink then I have no objections!
>> 
>> I have no objection to this.
>> 
>> Anyone currently attempting NVLink on DD1 gets all the checkstops they deserve.
>
> Yep. I'm going to pull it out. Although it may just lead to xstops on boot for
> DD1 ... is that something we care about or should we still detect DD1 and
> disable NVLink (which would prevent any checkstops)?

Detect and disable sounds good. just skip all nvlink init on
dd1... although perhaps we'd have to rip some stuff out of the device
tree?
(or not add it)
Alistair Popple Jan. 8, 2018, 5:18 a.m. UTC | #6 
On Monday, 8 January 2018 1:39:59 PM AEDT Stewart Smith wrote:
> Alistair Popple <alistair@popple.id.au> writes:
> > On Friday, 22 December 2017 3:22:32 PM AEDT Stewart Smith wrote:
> >> Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:
> >> > On 19/12/17 11:37, Alistair Popple wrote:
> >> >>> +bool is_p9dd1(void)
> >> >> 
> >> >> Does OpenCAPI still need this? If not we should just drop it and DD1 support
> >> >> along with it as it's very unlikely anything still works there.
> >> >
> >> > OpenCAPI has no DD1 support whatsoever. If you want to drop DD1 support 
> >> > for NVLink then I have no objections!
> >> 
> >> I have no objection to this.
> >> 
> >> Anyone currently attempting NVLink on DD1 gets all the checkstops they deserve.
> >
> > Yep. I'm going to pull it out. Although it may just lead to xstops on boot for
> > DD1 ... is that something we care about or should we still detect DD1 and
> > disable NVLink (which would prevent any checkstops)?
> 
> Detect and disable sounds good. just skip all nvlink init on
> dd1... although perhaps we'd have to rip some stuff out of the device
> tree?
> (or not add it)

Not adding the NPU2 to the device-tree would be sufficient. Removing
old_witherspoon_probe() would achieve this as I doubt there is any Hostboot that
supports DD1 and passes valid HDAT information for NPU device-node creation.

- Alistair

>
>
Frederic Barrat Jan. 10, 2018, 5:15 p.m. UTC | #7 
Le 18/12/2017 à 08:07, Andrew Donnellan a écrit :
> diff --git a/hw/npu2-common.c b/hw/npu2-common.c
> new file mode 100644
> index 0000000..cf616f3
> --- /dev/null
> +++ b/hw/npu2-common.c
> @@ -0,0 +1,98 @@
> +#include <skiboot.h>
> +#include <xscom.h>
> +#include <pci.h>
> +#include <npu2.h>
> +#include <npu2-regs.h>
> +#include <bitutils.h>

I was tempted to make a comment that it's the only file in that 
directory without a copyright banner statement. But I won't, as you then 
could fire back by asking details about what/why/how and I don't want to 
know. So I'm not making that comment. Really.

   Fred
Andrew Donnellan Jan. 11, 2018, 3:01 a.m. UTC | #8 
On 11/01/18 04:15, Frederic Barrat wrote:
> 
> 
> Le 18/12/2017 à 08:07, Andrew Donnellan a écrit :
>> diff --git a/hw/npu2-common.c b/hw/npu2-common.c
>> new file mode 100644
>> index 0000000..cf616f3
>> --- /dev/null
>> +++ b/hw/npu2-common.c
>> @@ -0,0 +1,98 @@
>> +#include <skiboot.h>
>> +#include <xscom.h>
>> +#include <pci.h>
>> +#include <npu2.h>
>> +#include <npu2-regs.h>
>> +#include <bitutils.h>
> 
> I was tempted to make a comment that it's the only file in that 
> directory without a copyright banner statement. But I won't, as you then 
> could fire back by asking details about what/why/how and I don't want to 
> know. So I'm not making that comment. Really.

Added. :D
diff mbox series

Patch

diff --git a/hw/Makefile.inc b/hw/Makefile.inc
index 04cacd1..cf8649d 100644
--- a/hw/Makefile.inc
+++ b/hw/Makefile.inc
@@ -7,7 +7,7 @@  HW_OBJS += p7ioc.o p7ioc-inits.o p7ioc-phb.o
 HW_OBJS += phb3.o sfc-ctrl.o fake-rtc.o bt.o p8-i2c.o prd.o
 HW_OBJS += dts.o lpc-rtc.o npu.o npu-hw-procedures.o xive.o phb4.o
 HW_OBJS += fake-nvram.o lpc-mbox.o npu2.o npu2-hw-procedures.o
-HW_OBJS += phys-map.o sbe-p9.o capp.o occ-sensor.o vas.o
+HW_OBJS += npu2-common.o phys-map.o sbe-p9.o capp.o occ-sensor.o vas.o
 HW=hw/built-in.o
 
 # FIXME hack this for now
diff --git a/hw/npu2-common.c b/hw/npu2-common.c
new file mode 100644
index 0000000..cf616f3
--- /dev/null
+++ b/hw/npu2-common.c
@@ -0,0 +1,98 @@ 
+#include <skiboot.h>
+#include <xscom.h>
+#include <pci.h>
+#include <npu2.h>
+#include <npu2-regs.h>
+#include <bitutils.h>
+
+bool is_p9dd1(void)
+{
+	struct proc_chip *chip = next_chip(NULL);
+
+	return chip &&
+	       (chip->type == PROC_CHIP_P9_NIMBUS ||
+		chip->type == PROC_CHIP_P9_CUMULUS) &&
+	       (chip->ec_level & 0xf0) == 0x10;
+}
+
+/*
+ * We use the indirect method because it uses the same addresses as
+ * the MMIO offsets (NPU RING)
+ */
+static void npu2_scom_set_addr(uint64_t gcid, uint64_t scom_base,
+			       uint64_t addr, uint64_t size)
+{
+	uint64_t isa = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_ADDR :
+				    NPU2_MISC_SCOM_IND_SCOM_ADDR;
+
+	addr = SETFIELD(NPU2_MISC_DA_ADDR, 0ull, addr);
+	addr = SETFIELD(NPU2_MISC_DA_LEN, addr, size);
+	xscom_write(gcid, scom_base + isa, addr);
+}
+
+void npu2_scom_write(uint64_t gcid, uint64_t scom_base,
+		     uint64_t reg, uint64_t size,
+		     uint64_t val)
+{
+	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
+				    NPU2_MISC_SCOM_IND_SCOM_DATA;
+
+	npu2_scom_set_addr(gcid, scom_base, reg, size);
+	xscom_write(gcid, scom_base + isd, val);
+}
+
+uint64_t npu2_scom_read(uint64_t gcid, uint64_t scom_base,
+			uint64_t reg, uint64_t size)
+{
+	uint64_t val;
+	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
+				    NPU2_MISC_SCOM_IND_SCOM_DATA;
+
+	npu2_scom_set_addr(gcid, scom_base, reg, size);
+	xscom_read(gcid, scom_base + isd, &val);
+
+	return val;
+}
+
+void npu2_write_4b(struct npu2 *p, uint64_t reg, uint32_t val)
+{
+	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
+			(uint64_t)val << 32);
+}
+
+uint32_t npu2_read_4b(struct npu2 *p, uint64_t reg)
+{
+	return npu2_scom_read(p->chip_id, p->xscom_base, reg,
+			      NPU2_MISC_DA_LEN_4B) >> 32;
+}
+
+void npu2_write(struct npu2 *p, uint64_t reg, uint64_t val)
+{
+	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, val);
+}
+
+uint64_t npu2_read(struct npu2 *p, uint64_t reg)
+{
+	return npu2_scom_read(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B);
+}
+
+void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask)
+{
+	uint64_t new_val;
+
+	new_val = npu2_read(p, reg);
+	new_val &= ~mask;
+	new_val |= val & mask;
+	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, new_val);
+}
+
+void npu2_write_mask_4b(struct npu2 *p, uint64_t reg, uint32_t val, uint32_t mask)
+{
+	uint32_t new_val;
+
+	new_val = npu2_read_4b(p, reg);
+	new_val &= ~mask;
+	new_val |= val & mask;
+	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
+			(uint64_t)new_val << 32);
+}
diff --git a/hw/npu2.c b/hw/npu2.c
index 7ffb094..e66ceb9 100644
--- a/hw/npu2.c
+++ b/hw/npu2.c
@@ -64,98 +64,6 @@ 
  * configure one particular BAR.
  */
 
-static bool is_p9dd1(void)
-{
-	struct proc_chip *chip = next_chip(NULL);
-
-	return chip &&
-	       (chip->type == PROC_CHIP_P9_NIMBUS ||
-		chip->type == PROC_CHIP_P9_CUMULUS) &&
-	       (chip->ec_level & 0xf0) == 0x10;
-}
-
-/*
- * We use the indirect method because it uses the same addresses as
- * the MMIO offsets (NPU RING)
- */
-static void npu2_scom_set_addr(uint64_t gcid, uint64_t scom_base,
-			       uint64_t addr, uint64_t size)
-{
-	uint64_t isa = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_ADDR :
-				    NPU2_MISC_SCOM_IND_SCOM_ADDR;
-
-	addr = SETFIELD(NPU2_MISC_DA_ADDR, 0ull, addr);
-	addr = SETFIELD(NPU2_MISC_DA_LEN, addr, size);
-	xscom_write(gcid, scom_base + isa, addr);
-}
-
-static void npu2_scom_write(uint64_t gcid, uint64_t scom_base,
-			    uint64_t reg, uint64_t size,
-			    uint64_t val)
-{
-	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
-				    NPU2_MISC_SCOM_IND_SCOM_DATA;
-
-	npu2_scom_set_addr(gcid, scom_base, reg, size);
-	xscom_write(gcid, scom_base + isd, val);
-}
-
-static uint64_t npu2_scom_read(uint64_t gcid, uint64_t scom_base,
-			       uint64_t reg, uint64_t size)
-{
-	uint64_t val;
-	uint64_t isd = is_p9dd1() ? NPU2_DD1_MISC_SCOM_IND_SCOM_DATA :
-				    NPU2_MISC_SCOM_IND_SCOM_DATA;
-
-	npu2_scom_set_addr(gcid, scom_base, reg, size);
-	xscom_read(gcid, scom_base + isd, &val);
-
-	return val;
-}
-
-void npu2_write_4b(struct npu2 *p, uint64_t reg, uint32_t val)
-{
-	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
-			(uint64_t)val << 32);
-}
-
-uint32_t npu2_read_4b(struct npu2 *p, uint64_t reg)
-{
-	return npu2_scom_read(p->chip_id, p->xscom_base, reg,
-			      NPU2_MISC_DA_LEN_4B) >> 32;
-}
-
-void npu2_write(struct npu2 *p, uint64_t reg, uint64_t val)
-{
-	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, val);
-}
-
-uint64_t npu2_read(struct npu2 *p, uint64_t reg)
-{
-	return npu2_scom_read(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B);
-}
-
-void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask)
-{
-	uint64_t new_val;
-
-	new_val = npu2_read(p, reg);
-	new_val &= ~mask;
-	new_val |= val & mask;
-	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_8B, new_val);
-}
-
-void npu2_write_mask_4b(struct npu2 *p, uint64_t reg, uint32_t val, uint32_t mask)
-{
-	uint32_t new_val;
-
-	new_val = npu2_read_4b(p, reg);
-	new_val &= ~mask;
-	new_val |= val & mask;
-	npu2_scom_write(p->chip_id, p->xscom_base, reg, NPU2_MISC_DA_LEN_4B,
-			(uint64_t)new_val << 32);
-}
-
 /* Set a specific flag in the vendor config space */
 void npu2_set_link_flag(struct npu2_dev *ndev, uint8_t flag)
 {
diff --git a/include/npu2-regs.h b/include/npu2-regs.h
index fdaad19..cf74431 100644
--- a/include/npu2-regs.h
+++ b/include/npu2-regs.h
@@ -23,6 +23,11 @@  void npu2_write4(struct npu2 *p, uint64_t reg, uint64_t val);
 uint64_t npu2_read(struct npu2 *p, uint64_t reg);
 void npu2_write(struct npu2 *p, uint64_t reg, uint64_t val);
 void npu2_write_mask(struct npu2 *p, uint64_t reg, uint64_t val, uint64_t mask);
+uint64_t npu2_scom_read(uint64_t gcid, uint64_t scom_base,
+			uint64_t reg, uint64_t size);
+void npu2_scom_write(uint64_t gcid, uint64_t scom_base,
+		     uint64_t reg, uint64_t size,
+		     uint64_t val);
 
 /* These aren't really NPU specific registers but we initialise them in NPU
  * code */
diff --git a/include/npu2.h b/include/npu2.h
index dae152a..d0c9ac5 100644
--- a/include/npu2.h
+++ b/include/npu2.h
@@ -163,4 +163,6 @@  void npu2_dev_procedure_reset(struct npu2_dev *dev);
 void npu2_set_link_flag(struct npu2_dev *ndev, uint8_t flag);
 void npu2_clear_link_flag(struct npu2_dev *ndev, uint8_t flag);
 extern int nv_zcal_nominal;
+bool is_p9dd1(void);
+
 #endif /* __NPU2_H */