> -----Original Message----- > From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf > Of Yu, Xiangliang > Sent: Tuesday, December 20, 2016 12:41 AM > To: Alex Deucher > Cc: dl.SRDC_SW_GPUVirtualization; amd-gfx list > Subject: RE: [PATCH 09/23] drm/amdgpu: enable virtualization feature for > FIJI/TONGA > > > > -----Original Message----- > > From: Alex Deucher [mailto:alexdeucher at gmail.com] > > Sent: Tuesday, December 20, 2016 7:17 AM > > To: Yu, Xiangliang <Xiangliang.Yu at amd.com> > > Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>; > > dl.SRDC_SW_GPUVirtualization > <dl.SRDC_SW_GPUVirtualization at amd.com> > > Subject: Re: [PATCH 09/23] drm/amdgpu: enable virtualization feature for > > FIJI/TONGA > > > > On Sat, Dec 17, 2016 at 11:16 AM, Xiangliang Yu <Xiangliang.Yu at amd.com> > > wrote: > > > According to chip device id to set VF flag, and call virtual interface > > > to setup all realted IP blocks. > > > > > > Signed-off-by: Xiangliang Yu <Xiangliang.Yu at amd.com> > > > --- > > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 ++++- > > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++-- > > > 2 files changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > > index c4075b7..ab8c8bb5 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > > @@ -1285,7 +1285,10 @@ static int amdgpu_early_init(struct > > amdgpu_device *adev) > > > else > > > adev->family = AMDGPU_FAMILY_VI; > > > > > > - r = vi_set_ip_blocks(adev); > > > + if (adev->flags & AMD_IS_VF) > > > + r = amd_xgpu_set_ip_blocks(adev); > > > > As far as I can see there's no need for a special > > amd_xgpu_set_ip_blocks() function. Just handle the VF case directly in > > vi_set_ip_blocks() and avoid all the extra indirection. > > My idea is that all virtualization chip share one common interface as a special > chip family. It will bring some benefits: > 1. Logic code is very clear; > 2. Avoid to scatter virtualization code throughout all amdgpu components; > 3. Easy to support next virtualization chip without change amdgpu code; > I don't mind having a separate IP module for special VF related setup, but I think the differences in the list of IP modules is small enough that it doesn't warrant all of the redirection. Basically just: if (VF) { add_ip_module(A); add_ip_module(X); add_ip_module(C); } else { add_ip_module(A); add_ip_module(B); add_ip_module(C); add_ip_module(D); } That way it's obvious in one place which modules are present in the VF and bare metal cases without having to trace through a bunch of indirection. It also makes it easier to update the lists if we ever rework the ip module interface or add a new IP module or something like that. Alex > > Alex > > > > > + else > > > + r = vi_set_ip_blocks(adev); > > > if (r) > > > return r; > > > break; > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > > index 93c4704..5a18111 100755 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > > > @@ -385,13 +385,13 @@ static const struct pci_device_id pciidlist[] = { > > > {0x1002, 0x6928, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TONGA}, > > > {0x1002, 0x6929, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TONGA}, > > > {0x1002, 0x692B, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TONGA}, > > > - {0x1002, 0x692F, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TONGA}, > > > + {0x1002, 0x692F, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TONGA | > > > + AMD_IS_VF}, > > > {0x1002, 0x6930, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TONGA}, > > > {0x1002, 0x6938, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TONGA}, > > > {0x1002, 0x6939, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_TONGA}, > > > /* fiji */ > > > {0x1002, 0x7300, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_FIJI}, > > > - {0x1002, 0x730F, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_FIJI}, > > > + {0x1002, 0x730F, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_FIJI | > > > + AMD_IS_VF}, > > > /* carrizo */ > > > {0x1002, 0x9870, PCI_ANY_ID, PCI_ANY_ID, 0, 0, > > CHIP_CARRIZO|AMD_IS_APU}, > > > {0x1002, 0x9874, PCI_ANY_ID, PCI_ANY_ID, 0, 0, > > > CHIP_CARRIZO|AMD_IS_APU}, > > > -- > > > 2.7.4 > > > > > > _______________________________________________ > > > amd-gfx mailing list > > > amd-gfx at lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
