diff mbox

[01/11] drm/i915: fix naming of fixed_16_16 wrapper.

Message ID 20170508114902.18965-2-mahesh1.kumar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar, Mahesh May 8, 2017, 11:48 a.m. UTC
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@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(-)

Comments

Matt Roper May 12, 2017, 12:21 a.m. UTC | #1
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@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@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
>
diff mbox

Patch

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);