Re: [PATCH] drm/amdgpu/si: fix ASIC tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux