[Intel-gfx] [PATCH 01/11] drm/i915: fix naming of fixed_16_16 wrapper.

[Matt Roper]

On Mon, May 08, 2017 at 05:18:52PM +0530, Mahesh Kumar wrote:
> fixed_16_16_div_round_up(_u64), wrapper for fixed_16_16 division
> operation don't really round_up the result. Wrapper round_up only the
> fraction part of the result to make it 16-bit.
> This patch eliminates round_up keyword from the wrapper.
> 
> Later patch will introduce the new wrapper to do rounding-off the result
> and give unt32_t output to cleanup mix use of fixed_16_16_t & uint32_t
> variables.
> 
> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>

Yeah, the naming here is confusing.  You're not rounding up to the next
integer value (which is typically what "round up" refers to), but you
are rounding the fractional part up rather than taking a floor.  Do we
actually need to DIV_ROUND_UP the fractional part in the implementation
of the function?  If we can just use a standard '/' there, it will more
closely match the new function name since since there won't be any round
happening.

Not that it really matters much either way.  With or without changes,

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


> ---
>  drivers/gpu/drm/i915/i915_drv.h | 6 ++----
>  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b20ed16da0ad..5804734d30a3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -152,8 +152,7 @@ static inline uint_fixed_16_16_t max_fixed_16_16(uint_fixed_16_16_t max1,
>  	return max;
>  }
>  
> -static inline uint_fixed_16_16_t fixed_16_16_div_round_up(uint32_t val,
> -							  uint32_t d)
> +static inline uint_fixed_16_16_t fixed_16_16_div(uint32_t val, uint32_t d)
>  {
>  	uint_fixed_16_16_t fp, res;
>  
> @@ -162,8 +161,7 @@ static inline uint_fixed_16_16_t fixed_16_16_div_round_up(uint32_t val,
>  	return res;
>  }
>  
> -static inline uint_fixed_16_16_t fixed_16_16_div_round_up_u64(uint32_t val,
> -							      uint32_t d)
> +static inline uint_fixed_16_16_t fixed_16_16_div_u64(uint32_t val, uint32_t d)
>  {
>  	uint_fixed_16_16_t res;
>  	uint64_t interm_val;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index cacb65fa2dd5..91f09dfdb618 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3698,7 +3698,7 @@ static uint_fixed_16_16_t skl_wm_method1(uint32_t pixel_rate, uint8_t cpp,
>  		return FP_16_16_MAX;
>  
>  	wm_intermediate_val = latency * pixel_rate * cpp;
> -	ret = fixed_16_16_div_round_up_u64(wm_intermediate_val, 1000 * 512);
> +	ret = fixed_16_16_div_u64(wm_intermediate_val, 1000 * 512);
>  	return ret;
>  }
>  
> @@ -3834,8 +3834,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	if (y_tiled) {
>  		interm_pbpl = DIV_ROUND_UP(plane_bytes_per_line *
>  					   y_min_scanlines, 512);
> -		plane_blocks_per_line =
> -		      fixed_16_16_div_round_up(interm_pbpl, y_min_scanlines);
> +		plane_blocks_per_line = fixed_16_16_div(interm_pbpl,
> +							y_min_scanlines);
>  	} else if (x_tiled) {
>  		interm_pbpl = DIV_ROUND_UP(plane_bytes_per_line, 512);
>  		plane_blocks_per_line = u32_to_fixed_16_16(interm_pbpl);
> -- 
> 2.11.0
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795