[PATCH 2/3] drm/amdgpu: Add peer-to-peer support among PCIe connected AMD GPUs

[Errabolu, Ramesh]

[AMD Official Use Only - General]

Just posted updated patch addressing the comment

-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling at amd.com> 
Sent: Monday, June 6, 2022 7:57 PM
To: Errabolu, Ramesh <Ramesh.Errabolu at amd.com>; amd-gfx list <amd-gfx at lists.freedesktop.org>
Subject: Re: [PATCH 2/3] drm/amdgpu: Add peer-to-peer support among PCIe connected AMD GPUs


Am 2022-06-04 um 06:23 schrieb Errabolu, Ramesh:
>> +bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev,
>> +				      struct amdgpu_device *peer_adev) { #ifdef 
>> +CONFIG_HSA_AMD_P2P
>> +	bool p2p_access = false;
>> +	uint64_t address_mask = peer_adev->dev->dma_mask ?
>> +		~*peer_adev->dev->dma_mask : ~((1ULL << 32) - 1);
>> +	resource_size_t aper_limit =
>> +		adev->gmc.aper_base + adev->gmc.aper_size - 1;
>> +	p2p_access = !(pci_p2pdma_distance_many(adev->pdev, 
>> +&peer_adev->dev, 1, true) < 0);
> This would give you a checkpatch warning. Please run checkpatch.
>
> Why can't you initialize p2p_access in the declaration above?
>
> Ramesh: I did run checkpatch script, and it didn't complain about the variable being not initialized. Being a variable of extern class, it is initialized at load time to default value of ZERO. This is then overridden in amdgpu_drv.c to true. So initializing it to true does not accomplish anything.

p2p_access is a local variable. Are you talking about pcie_p2p?

Checkpatch is not a static analyzer, it would not complain about uninitialized variables. It's more of a coding style checker. It usually complains when there is no blank line between variable declarations and the function body. That's why I suggested initializing p2p_access with its final value where it's defined 3 lines above, and removing the extra assignment that violates the coding style.

Ramesh: I was misunderstanding your comment as being for pcie_p2p module parameter

Regards,
   Felix