Re: [PATCH 1/2] drm/amdgpu: return the PCIe gen and lanes from the INFO

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 1/4/2023 4:11 AM, Marek Olšák wrote:
> I see. Well, those sysfs files are not usable, and I don't think it 
> would be important even if they were usable, but for completeness:
> 
> The ioctl returns:
>      pcie_gen = 1
>      pcie_num_lanes = 16
> 
> Theoretical bandwidth from those values: 4.0 GB/s
> My DMA test shows this write bandwidth: 3.5 GB/s
> It matches the expectation.
> 
> Let's see the devices (there is only 1 GPU Navi21 in the system):
> $ lspci |egrep '(PCI|VGA).*Navi'
> 0a:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI] Navi 10 XL 
> Upstream Port of PCI Express Switch (rev c3)
> 0b:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI] Navi 10 XL 
> Downstream Port of PCI Express Switch
> 0c:00.0 VGA compatible controller: Advanced Micro Devices, Inc. 
> [AMD/ATI] Navi 21 [Radeon RX 6800/6800 XT / 6900 XT] (rev c3)
> 
> Let's read sysfs:
> 
> $ cat /sys/bus/pci/devices/0000:0a:00.0/current_link_width
> 16
> $ cat /sys/bus/pci/devices/0000:0b:00.0/current_link_width
> 16
> $ cat /sys/bus/pci/devices/0000:0c:00.0/current_link_width
> 16
> $ cat /sys/bus/pci/devices/0000:0a:00.0/current_link_speed
> 2.5 GT/s PCIe
> $ cat /sys/bus/pci/devices/0000:0b:00.0/current_link_speed
> 16.0 GT/s PCIe
> $ cat /sys/bus/pci/devices/0000:0c:00.0/current_link_speed
> 16.0 GT/s PCIe
> 
> Problem 1: None of the speed numbers match 4 GB/s.

US bridge = 2.5GT/s means operating at PCIe Gen 1 speed. Total 
theoretical bandwidth is then derived based on encoding and total number 
of lanes.

> Problem 2: Userspace doesn't know the bus index of the bridges, and it's 
> not clear which bridge should be used.

In general, modern ones have this arch= US->DS->EP. US is the one 
connected to physical link.

> Problem 3: The PCIe gen number is missing.

Current link speed is based on whether it's Gen1/2/3/4/5.

BTW, your patch makes use of capabilities flags which gives the maximum 
supported speed/width by the device. It may not necessarily reflect the 
current speed/width negotiated. I guess in NV, this info is already 
obtained from PMFW and made available through metrics table.

Thanks,
Lijo

> 
> That's all irrelevant because all information should be queryable via 
> the INFO ioctl. It doesn't matter what sysfs contains because UMDs 
> shouldn't have to open and parse extra files just to read a couple of 
> integers.
> 
> Marek
> 
> 
> On Tue, Jan 3, 2023 at 3:31 AM Christian König 
> <ckoenig.leichtzumerken@gmail.com 
> <mailto:ckoenig.leichtzumerken@gmail.com>> wrote:
> 
>     Sure they can, those files are accessible to everyone.
> 
>     The massive advantage is that this is standard for all PCIe devices,
>     so it should work vendor independent.
> 
>     Christian.
> 
>     Am 02.01.23 um 18:55 schrieb Marek Olšák:
>>     Userspace drivers can't access sysfs.
>>
>>     Marek
>>
>>     On Mon, Jan 2, 2023, 10:54 Christian König
>>     <ckoenig.leichtzumerken@gmail.com
>>     <mailto:ckoenig.leichtzumerken@gmail.com>> wrote:
>>
>>         That stuff is already available as current_link_speed and
>>         current_link_width in sysfs.
>>
>>         I'm a bit reluctant duplicating this information in the IOCTL
>>         interface.
>>
>>         Christian.
>>
>>         Am 30.12.22 um 23:07 schrieb Marek Olšák:
>>>         For computing PCIe bandwidth in userspace and troubleshooting
>>>         PCIe
>>>         bandwidth issues.
>>>
>>>         For example, my Navi21 has been limited to PCIe gen 1 and this is
>>>         the first time I noticed it after 2 years.
>>>
>>>         Note that this intentionally fills a hole and padding
>>>         in drm_amdgpu_info_device.
>>>
>>>         Signed-off-by: Marek Olšák <marek.olsak@amd.com
>>>         <mailto:marek.olsak@amd.com>>
>>>
>>>         The patch is attached.
>>>
>>>         Marek
>>>
>>
>