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@xxxxxxxxxxxxxxx>
The whole type landscape in watermark code seems bit sloppy to me.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
