[bug report] drm/amdgpu: Add support for CIK parts

[bug report] drm/amdgpu: Add support for CIK parts

[Dan Carpenter]

Hello Alex Deucher,

The patch a2e73f56fa62: "drm/amdgpu: Add support for CIK parts" from
Apr 20, 2015, leads to the following static checker warning:

	drivers/gpu/drm/amd/amdgpu/ci_dpm.c:6293 ci_dpm_sw_init()
	warn: 'adev->pm.dpm.dyn_state.vddc_dependency_on_dispclk.entries' double freed

	drivers/gpu/drm/amd/amdgpu/ci_dpm.c:6293 ci_dpm_sw_init()
	warn: 'adev->pm.dpm.ps' double freed

drivers/gpu/drm/amd/amdgpu/ci_dpm.c
  6274  
  6275          ret = ci_dpm_init_microcode(adev);
  6276          if (ret)
  6277                  return ret;
  6278  
  6279          INIT_WORK(&adev->pm.dpm.thermal.work, amdgpu_dpm_thermal_work_handler);
  6280          mutex_lock(&adev->pm.mutex);
  6281          ret = ci_dpm_init(adev);

ci_dpm_init() has built-in calls to ci_dpm_fini().

  6282          if (ret)
  6283                  goto dpm_failed;
  6284          adev->pm.dpm.current_ps = adev->pm.dpm.requested_ps = adev->pm.dpm.boot_ps;
  6285          if (amdgpu_dpm == 1)
  6286                  amdgpu_pm_print_power_states(adev);
  6287          mutex_unlock(&adev->pm.mutex);
  6288          DRM_INFO("amdgpu: dpm initialized\n");
  6289  
  6290          return 0;
  6291  
  6292  dpm_failed:
  6293          ci_dpm_fini(adev);

Then we call it again here.  This style of error handling is a layering
violation and people think it makes the code so much simpler but it
really doesn't at all...  :/  Just have every function unwind it's own
allocations, doing it that way basically writes a mirroring de-allocate
function for every allocator for no effort.

  6294          mutex_unlock(&adev->pm.mutex);
  6295          DRM_ERROR("amdgpu: dpm initialization failed\n");
  6296          return ret;
  6297  }

regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [bug report] drm/amdgpu: Add support for CIK parts

[Dan Carpenter]

This one still needs to be fixed as well.

regards,
dan carpenter

On Tue, Dec 13, 2016 at 01:34:17PM +0300, Dan Carpenter wrote:
> Hello Alex Deucher,
> 
> The patch a2e73f56fa62: "drm/amdgpu: Add support for CIK parts" from
> Apr 20, 2015, leads to the following static checker warning:
> 
> 	drivers/gpu/drm/amd/amdgpu/ci_dpm.c:6293 ci_dpm_sw_init()
> 	warn: 'adev->pm.dpm.dyn_state.vddc_dependency_on_dispclk.entries' double freed
> 
> 	drivers/gpu/drm/amd/amdgpu/ci_dpm.c:6293 ci_dpm_sw_init()
> 	warn: 'adev->pm.dpm.ps' double freed
> 
> drivers/gpu/drm/amd/amdgpu/ci_dpm.c
>   6274  
>   6275          ret = ci_dpm_init_microcode(adev);
>   6276          if (ret)
>   6277                  return ret;
>   6278  
>   6279          INIT_WORK(&adev->pm.dpm.thermal.work, amdgpu_dpm_thermal_work_handler);
>   6280          mutex_lock(&adev->pm.mutex);
>   6281          ret = ci_dpm_init(adev);
> 
> ci_dpm_init() has built-in calls to ci_dpm_fini().
> 
>   6282          if (ret)
>   6283                  goto dpm_failed;
>   6284          adev->pm.dpm.current_ps = adev->pm.dpm.requested_ps = adev->pm.dpm.boot_ps;
>   6285          if (amdgpu_dpm == 1)
>   6286                  amdgpu_pm_print_power_states(adev);
>   6287          mutex_unlock(&adev->pm.mutex);
>   6288          DRM_INFO("amdgpu: dpm initialized\n");
>   6289  
>   6290          return 0;
>   6291  
>   6292  dpm_failed:
>   6293          ci_dpm_fini(adev);
> 
> Then we call it again here.  This style of error handling is a layering
> violation and people think it makes the code so much simpler but it
> really doesn't at all...  :/  Just have every function unwind it's own
> allocations, doing it that way basically writes a mirroring de-allocate
> function for every allocator for no effort.
> 
>   6294          mutex_unlock(&adev->pm.mutex);
>   6295          DRM_ERROR("amdgpu: dpm initialization failed\n");
>   6296          return ret;
>   6297  }
> 
> regards,
> dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel