On Fri, Aug 29, 2014 at 12:12:38PM +0200, David Herrmann wrote:
[...]
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 83a2461..5cfa574 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2761,18 +2761,25 @@ void radeon_atombios_fini(struct radeon_device *rdev);
>  /*
>   * RING helpers.
>   */
> -#if DRM_DEBUG_CODE == 0
> +
> +/**
> + * radeon_ring_write - write a value to the ring
> + *
> + * @ring: radeon_ring structure holding ring information
> + * @v: dword (dw) value to write
> + *
> + * Write a value to the requested ring buffer (all asics).
> + */
>  static inline void radeon_ring_write(struct radeon_ring *ring, uint32_t v)
>  {
> +	if (ring->count_dw <= 0)
> +		DRM_ERROR("radeon: writing more dwords to the ring than expected!\n");
> +
>  	ring->ring[ring->wptr++] = v;
>  	ring->wptr &= ring->ptr_mask;
>  	ring->count_dw--;
>  	ring->ring_free_dw--;
>  }
> -#else
> -/* With debugging this is just too big to inline */
> -void radeon_ring_write(struct radeon_ring *ring, uint32_t v);
> -#endif
>  
>  /*
>   * ASICs macro.
> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
> index d656079..6f2a9bd 100644
> --- a/drivers/gpu/drm/radeon/radeon_ring.c
> +++ b/drivers/gpu/drm/radeon/radeon_ring.c
> @@ -45,27 +45,6 @@
>  static int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *ring);
>  
>  /**
> - * radeon_ring_write - write a value to the ring
> - *
> - * @ring: radeon_ring structure holding ring information
> - * @v: dword (dw) value to write
> - *
> - * Write a value to the requested ring buffer (all asics).
> - */
> -void radeon_ring_write(struct radeon_ring *ring, uint32_t v)
> -{
> -#if DRM_DEBUG_CODE
> -	if (ring->count_dw <= 0) {
> -		DRM_ERROR("radeon: writing more dwords to the ring than expected!\n");
> -	}
> -#endif
> -	ring->ring[ring->wptr++] = v;
> -	ring->wptr &= ring->ptr_mask;
> -	ring->count_dw--;
> -	ring->ring_free_dw--;
> -}

The original was clearly never tested with DRM_DEBUG_CODE != 0. I don't
see how it could've built given that the header protects the complete
function whereas the implementation protects only part of the function.

Reviewed-by: Thierry Reding <treding@nvidia.com>