[Intel-gfx] [PATCH 8/9] drm/i915: Make intel_calculate_wm return unsigned int

[Joonas Lahtinen]

On pe, 2016-10-07 at 14:34 +0100, Tvrtko Ursulin wrote:
> @@ -595,18 +595,17 @@ static unsigned long intel_calculate_wm(unsigned int clock_in_khz,
>  	 * clocks go from a few thousand to several hundred thousand.
>  	 * latency is usually a few thousand
>  	 */
> -	entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) /
> -		1000;
> +	entries_required = ((clock_in_khz / 1000) * cpp * latency_ns) / 1000;

Is the intermediary result always within int?

>  	entries_required = DIV_ROUND_UP(entries_required, wm->cacheline_size);
>  
> -	DRM_DEBUG_KMS("FIFO entries required for mode: %ld\n", entries_required);
> +	DRM_DEBUG_KMS("FIFO entries required for mode: %d\n", entries_required);
>  
>  	wm_size = fifo_size - (entries_required + wm->guard_size);
>  
> -	DRM_DEBUG_KMS("FIFO watermark level: %ld\n", wm_size);
> +	DRM_DEBUG_KMS("FIFO watermark level: %d\n", wm_size);
>  
>  	/* Don't promote wm_size to unsigned... */
> -	if (wm_size > (long)wm->max_wm)
> +	if (wm_size > (int)wm->max_wm)
>  		wm_size = wm->max_wm;
>  	if (wm_size <= 0)
>  		wm_size = wm->default_wm;

This could be

	if (wm_size <= 0)
		wm_size = wm->default_wm;
	else if (wm_size > U16_MAX || (u16)wm_size > wm->max_wm)
		wm_size = wm->max_wm;

or something?

Other than that, types look better that they used to.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>

The whole type landscape in watermark code seems bit sloppy to me.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation