Re: [bug report] drm/amdgpu/gfx6: clean up cu configuration

[Flora Cui <flora.cui-5C7GfCeVMHo@public.gmane.org>]

Hi Dan Carpenter,

Thank you for the info. This commit is just a clean up to keep align
with gfx7/8.

On Fri, Mar 31, 2017 at 06:13:25PM +0300, Dan Carpenter wrote:
> Hello Flora Cui,
> 
> The patch 375d6f7057a9: "drm/amdgpu/gfx6: clean up cu configuration"
> from Feb 7, 2017, leads to the following static checker warning:
> 
> 	drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c:3737 gfx_v6_0_get_cu_info()
> 	warn: potential off by one cu_info->bitmap[4]
> 
> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>   3715  static void gfx_v6_0_get_cu_info(struct amdgpu_device *adev)
>   3716  {
>   3717          int i, j, k, counter, active_cu_number = 0;
>   3718          u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
>   3719          struct amdgpu_cu_info *cu_info = &adev->gfx.cu_info;
>   3720          unsigned disable_masks[4 * 2];
>   3721  
>   3722          memset(cu_info, 0, sizeof(*cu_info));
>   3723  
>   3724          amdgpu_gfx_parse_disable_cu(disable_masks, 4, 2);
>   3725  
>   3726          mutex_lock(&adev->grbm_idx_mutex);
>   3727          for (i = 0; i < adev->gfx.config.max_shader_engines; i++) {
>   3728                  for (j = 0; j < adev->gfx.config.max_sh_per_se; j++) {
>   3729                          mask = 1;
>   3730                          ao_bitmap = 0;
>   3731                          counter = 0;
>   3732                          gfx_v6_0_select_se_sh(adev, i, j, 0xffffffff);
>   3733                          if (i < 4 && j < 2)
> 
> If i == 4
> 
>   3734                                  gfx_v6_0_set_user_cu_inactive_bitmap(
>   3735                                          adev, disable_masks[i * 2 + j]);
>   3736                          bitmap = gfx_v6_0_get_cu_enabled(adev);
>   3737                          cu_info->bitmap[i][j] = bitmap;
> 
> then we are beyond the end of this array.  Also, why was this patch even
> applied when it has no commit message?  It's totally not clear to me
> what the patch is trying to do or why it exists...
adev->gfx.config.max_shader_engines is set in gfx_v6_0_gpu_init() and
it must be < 4 so we'll never be beyond the end of the array.

Regards,
Flora Cui
>   3738  
>   3739                          for (k = 0; k < 16; k++) {
>   3740                                  if (bitmap & mask) {
>   3741                                          if (counter < 2)
>   3742                                                  ao_bitmap |= mask;
>   3743                                          counter ++;
>   3744                                  }
>   3745                                  mask <<= 1;
>   3746                          }
>   3747                          active_cu_number += counter;
>   3748                          ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
>   3749                  }
>   3750          }
>   3751  
>   3752          gfx_v6_0_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);
>   3753          mutex_unlock(&adev->grbm_idx_mutex);
>   3754  
>   3755          cu_info->number = active_cu_number;
>   3756          cu_info->ao_cu_mask = ao_cu_mask;
>   3757  }
> 
> regards,
> dan carpenter
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx