On 10/10/2016 09:02, Joonas Lahtinen wrote:
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?
I certainly hope so! Display clock in MHz * cpp * latency which is u16. So 2^31 / 2^16 / cpp=8 leaves 4GHz for the clock before it would overflow.
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.lahti...@linux.intel.com> The whole type landscape in watermark code seems bit sloppy to me.
Yes, but I would also like not to introduce regressions. So I am a bit reluctant to push forward with the second half of this series. :I
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
