On 10/4/2023 6:19 PM, Ilpo Järvinen wrote: > On Sat, 30 Sep 2023, Shyam Sundar S K wrote: > >> In order to provide GPU inputs to TA for the Smart PC solution to work, we >> need to have interface between the PMF driver and the AMDGPU driver. >> >> Add the initial code path for get interface from AMDGPU. >> >> Co-developed-by: Mario Limonciello <mario.limonciello@xxxxxxx> >> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx> > >> @@ -355,6 +356,21 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev) >> return amd_pmf_start_policy_engine(dev); >> } >> >> +static int amd_pmf_get_gpu_handle(struct pci_dev *pdev, void *data) >> +{ >> + struct amd_pmf_dev *dev = data; >> + >> + if (pdev->vendor == PCI_VENDOR_ID_ATI && pdev->devfn == 0) { >> + /* get the amdgpu handle from the pci root after walking through the pci bus */ > > I can see from the code that you assign to amdgpu handle so this comment > added no information. > > It doesn't really answer at all why you're doing this second step. Based > on the give parameters to pci_get_device(), it looks as if you're asking > for the same device you already have in pdev to be searched to you. Not sure if I understand you remark completely. amd_pmf_get_gpu_handle() is a callback function for pci_walk_bus (which is done below). What I am trying to do here is to get the PCI handle for the GPU device by walking the PCI bus. I think the 'pdev' here refers to the pci root, using that root we walk the entire tree and only stop walking when we find a handle to GPU device. Do you want me to change the "pdev" parameter to be renamed as "root" ? Am I missing something? Thanks, Shyam > >> + dev->gfx_data.gpu_dev = pci_get_device(pdev->vendor, pdev->device, NULL); >> + if (dev->gfx_data.gpu_dev) { >> + pci_dev_put(pdev); >> + return 1; /* stop walking */ >> + } >> + } >> + return 0; /* continue walking */ >> +} >> + >> static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data) >> { >> return ver->impl_id == TEE_IMPL_ID_AMDTEE; >> @@ -451,6 +467,15 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) >> INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd); >> amd_pmf_set_dram_addr(dev); >> amd_pmf_get_bios_buffer(dev); >> + >> + /* get amdgpu handle */ >> + pci_walk_bus(dev->root->bus, amd_pmf_get_gpu_handle, dev); >> + if (!dev->gfx_data.gpu_dev) >> + dev_err(dev->dev, "GPU handle not found!\n"); >> + >> + if (!amd_pmf_gpu_init(&dev->gfx_data)) >> + dev->gfx_data.gpu_dev_en = true; >> + > >