diff mbox

hw/npu2.c: Add ibm, nvlink-speed device-tree property

Message ID 1502957567-12390-1-git-send-email-alistair@popple.id.au
State Superseded
Headers show

Commit Message

Alistair Popple Aug. 17, 2017, 8:12 a.m. UTC
NVLink2 links can support multiple different speeds. However the device driver
has no way of determining which speed was programmed so pass it down as a device
tree property.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 doc/device-tree/nvlink.rst | 7 ++++++-
 hw/npu2.c                  | 8 ++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

Comments

Alistair Popple Aug. 18, 2017, 5:14 a.m. UTC | #1
Stewart,

We're going to change the values of the enum here so I'm going to resubmit this
patch. Thanks.

Regards,

Alistair

On Thu, 17 Aug 2017 06:12:47 PM Alistair Popple wrote:
> NVLink2 links can support multiple different speeds. However the device driver
> has no way of determining which speed was programmed so pass it down as a device
> tree property.
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
>  doc/device-tree/nvlink.rst | 7 ++++++-
>  hw/npu2.c                  | 8 ++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/device-tree/nvlink.rst b/doc/device-tree/nvlink.rst
> index 6ce44e9..2876ca8 100644
> --- a/doc/device-tree/nvlink.rst
> +++ b/doc/device-tree/nvlink.rst
> @@ -105,7 +105,12 @@ Emulated PCI device bindings
>                          vendor-id = <0x1014>;
>                          ibm,gpu = <0x100002f7>; /* phandle pointing the associated GPU PCI device node */
>  			memory-region = <0x10000abc>; /* phandle pointing to the GPU memory */
> -			phandle = <0x100002fc>;
> +                        ibm,nvlink-speed = <0x1>;
> +
> +                ; Denotes the speed the link is running at:
> +                ; 0x0 == 20 Gbps, 0x1 == 25.00000 Gbps, 0x2 = 25.78125 Gbps
> +
> +                        phandle = <0x100002fc>;
>                  };
>  
>                  pci@1 {
> diff --git a/hw/npu2.c b/hw/npu2.c
> index 74e3325..cab7347 100644
> --- a/hw/npu2.c
> +++ b/hw/npu2.c
> @@ -683,6 +683,14 @@ static int npu2_dn_fixup(struct phb *phb,
>  	npu2_dn_fixup_gmb(pd->dn, dev);
>  	dt_add_property_cells(pd->dn, "ibm,nvlink", dev->dt_node->phandle);
>  
> +	/* NVLink supports multiple speeds and device drivers need to know what
> +	 * speed has been set by firmware. The speed is actually controlled by
> +	 * Hostboot, so until we get a HDAT entry telling us what speed they
> +	 * programmed we will just hard code it here and hope it matches. If it
> +	 * doesn't it is always possible to manually override it when installing
> +	 * the device driver. */
> +	dt_add_property_cells(pd->dn, "ibm,nvlink-speed", 0x1);
> +
>  	/* NPU devices require a slot location to associate with GPUs */
>  	dev->slot_label = dt_prop_get_def(pd->dn, "ibm,slot-label", NULL);
>  	if (!dev->slot_label) {
>
Stewart Smith Aug. 18, 2017, 5:42 a.m. UTC | #2
Alistair Popple <alistair@popple.id.au> writes:
> Stewart,
>
> We're going to change the values of the enum here so I'm going to resubmit this
> patch. Thanks.

Ok.

What are your thoughts on merging this "now" versus waiting for the
HDAT? Do we need something in the hands of NVIDIA "now" or is the
agreement on what and where fine for now?
Alistair Popple Aug. 18, 2017, 6:32 a.m. UTC | #3
Stewart,

Should be ok to wait for HDAT.

Thanks,

Alistair

On 18 August 2017 15:42:11 GMT+10:00, Stewart Smith <stewart@linux.vnet.ibm.com> wrote:
>Alistair Popple <alistair@popple.id.au> writes:
>> Stewart,
>>
>> We're going to change the values of the enum here so I'm going to
>resubmit this
>> patch. Thanks.
>
>Ok.
>
>What are your thoughts on merging this "now" versus waiting for the
>HDAT? Do we need something in the hands of NVIDIA "now" or is the
>agreement on what and where fine for now?
>
>-- 
>Stewart Smith
>OPAL Architect, IBM.
diff mbox

Patch

diff --git a/doc/device-tree/nvlink.rst b/doc/device-tree/nvlink.rst
index 6ce44e9..2876ca8 100644
--- a/doc/device-tree/nvlink.rst
+++ b/doc/device-tree/nvlink.rst
@@ -105,7 +105,12 @@  Emulated PCI device bindings
                         vendor-id = <0x1014>;
                         ibm,gpu = <0x100002f7>; /* phandle pointing the associated GPU PCI device node */
 			memory-region = <0x10000abc>; /* phandle pointing to the GPU memory */
-			phandle = <0x100002fc>;
+                        ibm,nvlink-speed = <0x1>;
+
+                ; Denotes the speed the link is running at:
+                ; 0x0 == 20 Gbps, 0x1 == 25.00000 Gbps, 0x2 = 25.78125 Gbps
+
+                        phandle = <0x100002fc>;
                 };
 
                 pci@1 {
diff --git a/hw/npu2.c b/hw/npu2.c
index 74e3325..cab7347 100644
--- a/hw/npu2.c
+++ b/hw/npu2.c
@@ -683,6 +683,14 @@  static int npu2_dn_fixup(struct phb *phb,
 	npu2_dn_fixup_gmb(pd->dn, dev);
 	dt_add_property_cells(pd->dn, "ibm,nvlink", dev->dt_node->phandle);
 
+	/* NVLink supports multiple speeds and device drivers need to know what
+	 * speed has been set by firmware. The speed is actually controlled by
+	 * Hostboot, so until we get a HDAT entry telling us what speed they
+	 * programmed we will just hard code it here and hope it matches. If it
+	 * doesn't it is always possible to manually override it when installing
+	 * the device driver. */
+	dt_add_property_cells(pd->dn, "ibm,nvlink-speed", 0x1);
+
 	/* NPU devices require a slot location to associate with GPUs */
 	dev->slot_label = dt_prop_get_def(pd->dn, "ibm,slot-label", NULL);
 	if (!dev->slot_label) {