On Wed, Aug 28, 2019 at 11:28 AM Jean Delvare <jdelvare@xxxxxxx> wrote: > > Comparing adev->family with CHIP constants is not correct. > adev->family can only be compared with AMDGPU_FAMILY constants and > adev->asic_type is the struct member to compare with CHIP constants. > They are separate identification spaces. > > Signed-off-by: Jean Delvare <jdelvare@xxxxxxx> > Fixes: 62a37553414a ("drm/amdgpu: add si implementation v10") > Cc: Ken Wang <Qingqing.Wang@xxxxxxx> > Cc: Alex Deucher <alexander.deucher@xxxxxxx> > Cc: "Christian König" <christian.koenig@xxxxxxx> > Cc: "David (ChunMing) Zhou" <David1.Zhou@xxxxxxx> > --- > I stumbled upon this while reading the code. The comparisons look > obviously incorrect, but on the other hand it's hard to believe that > the bug has been there for almost 4 years without any user reporting > any actual problem caused by it. So this probably requires a more > in-depth analysis by someone familiar with the code. > > I tested these changes on my Oland card and did not notice any > difference, but I don't know what I should be looking at. It's leftover from when the code was ported to radeon. radeon used family for asic_type and amdgpu has both. It probably got missed when the code was ported. The differences between those chips and others are pretty minor (a couple of new fields and additional instances of some registers), so it may not have had much effect. Anyway, the change is correct. Applied. Thanks! Alex > > drivers/gpu/drm/amd/amdgpu/si.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > --- linux-5.2.orig/drivers/gpu/drm/amd/amdgpu/si.c 2019-07-08 00:41:56.000000000 +0200 > +++ linux-5.2/drivers/gpu/drm/amd/amdgpu/si.c 2019-08-28 10:29:43.544180131 +0200 > @@ -1867,7 +1867,7 @@ static void si_program_aspm(struct amdgp > if (orig != data) > si_pif_phy1_wreg(adev,PB1_PIF_PWRDOWN_1, data); > > - if ((adev->family != CHIP_OLAND) && (adev->family != CHIP_HAINAN)) { > + if ((adev->asic_type != CHIP_OLAND) && (adev->asic_type != CHIP_HAINAN)) { > orig = data = si_pif_phy0_rreg(adev,PB0_PIF_PWRDOWN_0); > data &= ~PLL_RAMP_UP_TIME_0_MASK; > if (orig != data) > @@ -1916,14 +1916,14 @@ static void si_program_aspm(struct amdgp > > orig = data = si_pif_phy0_rreg(adev,PB0_PIF_CNTL); > data &= ~LS2_EXIT_TIME_MASK; > - if ((adev->family == CHIP_OLAND) || (adev->family == CHIP_HAINAN)) > + if ((adev->asic_type == CHIP_OLAND) || (adev->asic_type == CHIP_HAINAN)) > data |= LS2_EXIT_TIME(5); > if (orig != data) > si_pif_phy0_wreg(adev,PB0_PIF_CNTL, data); > > orig = data = si_pif_phy1_rreg(adev,PB1_PIF_CNTL); > data &= ~LS2_EXIT_TIME_MASK; > - if ((adev->family == CHIP_OLAND) || (adev->family == CHIP_HAINAN)) > + if ((adev->asic_type == CHIP_OLAND) || (adev->asic_type == CHIP_HAINAN)) > data |= LS2_EXIT_TIME(5); > if (orig != data) > si_pif_phy1_wreg(adev,PB1_PIF_CNTL, data); > > > -- > Jean Delvare > SUSE L3 Support > _______________________________________________ > amd-gfx mailing list > amd-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx