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.lahtinen@xxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
