Messages in this thread | | | Date | Tue, 9 Nov 2021 17:00:11 +0530 | Subject | Re: [PATCH v6 1/5] x86/amd_nb: Add support for northbridges on Aldebaran | From | "Chatradhi, Naveen Krishna" <> |
| |
Hi Boris,
On 11/9/2021 12:33 AM, Borislav Petkov wrote: > [CAUTION: External Email] > > On Mon, Nov 08, 2021 at 10:23:49PM +0530, Chatradhi, Naveen Krishna wrote: >> Northbridges on CPUs and GPUs can be described using the elements in the >> above structure. > If you're going to describe *northbridges*, then your struct cannot be called > system_topology... > >> I thought organizing the data in a structure would simplify the >> initialization of cpus and gpus. > Ehh, did you even read my mail where I tried to explain that sprinkling > > if (gpu) > this > else > that > > all over amd_cache_northbridges() is not proper design? > > ;-\ > >> I will modify the patch to enumerate gpu northbridge info only if there are >> >> gpu nodes with pci_device to access the node_map registers. > Why would you do that? What's the advantage? > > How about you answer my questions first so that we agree on the design > first before you go and do things?
I was trying to handle both cpu and cpu northbridge enumeration in the amd_cache_northbridges() itself by reusing the existing structures and APIs.
Should have seen this through more clearly. As, this is working well for the following reasons.
a. Allocating the amd_northbridges.nb after identifying both the cpu and gpu misc devices, would extend node_to_amd_nb(node) for both cpu and gpu nodes.
It is used extensively in this module. However, the roots_per_misc value is different in case of cpus and gpus and that needed to be handled seperately.
b. amd_nb_num(void) is used by other modules in the kernel, returning the total count of CPU and GPU northbridges would break the existing code.
I understood your point now.
When we create separate functions for caching cpu and gpu devices, is it okay to create "struct amd_gpu_nb_info" with the following fields
a. gpu_num; b. struct amd_northbridge *gpu_nb; c. gpu_node_start_id;
While, amd_nb_num(), continues to return number of cpu NBs Add new API amd_gpu_nb_num(), return number of gpu NBs
and modify the node_to_amd_nb(node) to extend the same behavior for gpu devices also
Regards,
naveenk
> > Hmm. > > -- > Regards/Gruss, > Boris. > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.kernel.org%2Ftglx%2Fnotes-about-netiquette&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7Cad9aea0ddff0446d80b108d9a2ea867d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637719950521959255%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=PthEEyzphEyN3O1FrUcvKyMF%2FEb282qifUHPR6psFhg%3D&reserved=0
| |