Ok. Reviewed-by: Rex Zhu <Rex.Zhu at amd.com> Best Regards Rex -----Original Message----- From: Huang Rui [mailto:ray.huang@xxxxxxx] Sent: Wednesday, June 22, 2016 11:05 AM To: Zhu, Rex Cc: amd-gfx at lists.freedesktop.org; Huang, JinHuiEric; Wang, Qingqing Subject: Re: [PATCH] drm/amdgpu: change pcie_gen_cap magic code to macro On Wed, Jun 22, 2016 at 10:43:48AM +0800, Zhu, Rex wrote: > Hi Ray, > > There are some definitions in amdgpu_device.c. > How about removing them to amd_pci.h and use them in powerplay. > > Best Regards > Rex > > #define AMDGPU_DEFAULT_PCIE_GEN_MASK 0x30007 /* gen: chipset 1/2, > asic 1/2/3 */ #define AMDGPU_DEFAULT_PCIE_MLW_MASK 0x2f0000 /* > 1/2/4/8/16 lanes */ > Maybe move definitions to amd_pcie.h like below should be better to reuse. /* gen: chipset 1/2, asic 1/2/3 */ #define AMDGPU_DEFAULT_PCIE_GEN_MASK (CAIL_PCIE_LINK_SPEED_SUPPORT_GEN1 \ | CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2 \ | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN1 \ | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN2 \ | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN3) /* 1/2/4/8/16 lanes */ #define AMDGPU_DEFAULT_PCIE_MLW_MASK (CAIL_PCIE_LINK_WIDTH_SUPPORT_X1 \ | CAIL_PCIE_LINK_WIDTH_SUPPORT_X2 \ | CAIL_PCIE_LINK_WIDTH_SUPPORT_X4 \ | CAIL_PCIE_LINK_WIDTH_SUPPORT_X8 \ | CAIL_PCIE_LINK_WIDTH_SUPPORT_X16) What do you think of it? Thanks, Rui > -----Original Message----- > From: Huang Rui [mailto:ray.huang at amd.com] > Sent: Wednesday, June 22, 2016 10:35 AM > To: amd-gfx at lists.freedesktop.org > Cc: Huang, Ray; Huang, JinHuiEric; Zhu, Rex; Wang, Qingqing > Subject: [PATCH] drm/amdgpu: change pcie_gen_cap magic code to macro > > This patch changes pcie_gen_cap magic code to macro to make it more readable. > > Signed-off-by: Huang Rui <ray.huang at amd.com> > Cc: Eric Huang <JinHuiEric.Huang at amd.com> > Cc: Rex Zhu <Rex.Zhu at amd.com> > Cc: Ken Wang <Qingqing.Wang at amd.com> > --- > drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 12 ++++++++++-- > drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c | 12 ++++++++++-- > drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 12 ++++++++++-- > 3 files changed, 30 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c > index 27cdcb0..e6ef8e4 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c > @@ -731,7 +731,11 @@ static int fiji_hwmgr_backend_init(struct pp_hwmgr *hwmgr) > sys_info.info_id = CGS_SYSTEM_INFO_PCIE_GEN_INFO; > result = cgs_query_system_info(hwmgr->device, &sys_info); > if (result) > - data->pcie_gen_cap = 0x30007; > + data->pcie_gen_cap = CAIL_PCIE_LINK_SPEED_SUPPORT_GEN1 > + | CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2 > + | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN1 > + | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN2 > + | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN3; > else > data->pcie_gen_cap = (uint32_t)sys_info.value; > if (data->pcie_gen_cap & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) > @@ -740,7 +744,11 @@ static int fiji_hwmgr_backend_init(struct pp_hwmgr *hwmgr) > sys_info.info_id = CGS_SYSTEM_INFO_PCIE_MLW; > result = cgs_query_system_info(hwmgr->device, &sys_info); > if (result) > - data->pcie_lane_cap = 0x2f0000; > + data->pcie_lane_cap = CAIL_PCIE_LINK_WIDTH_SUPPORT_X1 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X2 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X4 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X8 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X16; > else > data->pcie_lane_cap = (uint32_t)sys_info.value; > } else { > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c > index 49c1e88..9b601e5 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c > @@ -3267,7 +3267,11 @@ int polaris10_hwmgr_backend_init(struct pp_hwmgr *hwmgr) > sys_info.info_id = CGS_SYSTEM_INFO_PCIE_GEN_INFO; > result = cgs_query_system_info(hwmgr->device, &sys_info); > if (result) > - data->pcie_gen_cap = 0x30007; > + data->pcie_gen_cap = CAIL_PCIE_LINK_SPEED_SUPPORT_GEN1 > + | CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2 > + | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN1 > + | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN2 > + | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN3; > else > data->pcie_gen_cap = (uint32_t)sys_info.value; > if (data->pcie_gen_cap & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) > @@ -3276,7 +3280,11 @@ int polaris10_hwmgr_backend_init(struct pp_hwmgr *hwmgr) > sys_info.info_id = CGS_SYSTEM_INFO_PCIE_MLW; > result = cgs_query_system_info(hwmgr->device, &sys_info); > if (result) > - data->pcie_lane_cap = 0x2f0000; > + data->pcie_lane_cap = CAIL_PCIE_LINK_WIDTH_SUPPORT_X1 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X2 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X4 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X8 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X16; > else > data->pcie_lane_cap = (uint32_t)sys_info.value; > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > index 33a6fa7..c6f0ea0 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > @@ -4637,7 +4637,11 @@ int tonga_hwmgr_backend_init(struct pp_hwmgr *hwmgr) > sys_info.info_id = CGS_SYSTEM_INFO_PCIE_GEN_INFO; > result = cgs_query_system_info(hwmgr->device, &sys_info); > if (result) > - data->pcie_gen_cap = 0x30007; > + data->pcie_gen_cap = CAIL_PCIE_LINK_SPEED_SUPPORT_GEN1 > + | CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2 > + | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN1 > + | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN2 > + | CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN3; > else > data->pcie_gen_cap = (uint32_t)sys_info.value; > if (data->pcie_gen_cap & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3) > @@ -4646,7 +4650,11 @@ int tonga_hwmgr_backend_init(struct pp_hwmgr *hwmgr) > sys_info.info_id = CGS_SYSTEM_INFO_PCIE_MLW; > result = cgs_query_system_info(hwmgr->device, &sys_info); > if (result) > - data->pcie_lane_cap = 0x2f0000; > + data->pcie_lane_cap = CAIL_PCIE_LINK_WIDTH_SUPPORT_X1 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X2 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X4 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X8 > + | CAIL_PCIE_LINK_WIDTH_SUPPORT_X16; > else > data->pcie_lane_cap = (uint32_t)sys_info.value; > } else { > -- > 2.7.4 >