Re: [PATCH 2/6] drm/i915/hsw: use intel_compute_linetime_wm function for linetime wm

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jun 13, 2017 at 11:34:46AM +0530, Mahesh Kumar wrote:
> linetime wm is time taken to fill a single display line with given clock
> rate, multiplied by 8.
> This patch reuses the common code of hsw_compute_linetime_wm &
> skl_compute_linetime_wm.
> 
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c  | 23 ++++++++++++++---------
>  2 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 83dd40905821..ea2d29e68bfe 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1877,6 +1877,7 @@ bool ilk_disable_lp_wm(struct drm_device *dev);
>  int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6);
>  int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
>  				  struct intel_crtc_state *cstate);
> +uint32_t intel_compute_linetime_wm(const struct intel_crtc_state *cstate);
>  static inline int intel_enable_rc6(void)
>  {
>  	return i915.enable_rc6;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 155f54a1f516..76ffb00c6ce4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2746,8 +2746,7 @@ hsw_compute_linetime_wm(const struct intel_crtc_state *cstate)
>  	/* The WM are computed with base on how long it takes to fill a single
>  	 * row at the given clock rate, multiplied by 8.
>  	 * */
> -	linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
> -				     adjusted_mode->crtc_clock);
> +	linetime = intel_compute_linetime_wm(cstate);

HSW/BDW spec says "The WM_LINETIME register Line Time field (bits 8:0) is
not adjusted for panel fitter down scaling.", so NAK.

>  	ips_linetime = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
>  					 intel_state->cdclk.logical.cdclk);
>  
> @@ -4382,7 +4381,7 @@ static uint_fixed_16_16_t skl_wm_method2(uint32_t pixel_rate,
>  }
>  
>  static uint_fixed_16_16_t
> -intel_get_linetime_us(struct intel_crtc_state *cstate)
> +intel_get_linetime_us(const struct intel_crtc_state *cstate)
>  {
>  	uint32_t pixel_rate;
>  	uint32_t crtc_htotal;
> @@ -4604,20 +4603,26 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
>  	return 0;
>  }
>  
> -static uint32_t
> -skl_compute_linetime_wm(struct intel_crtc_state *cstate)
> +uint32_t intel_compute_linetime_wm(const struct intel_crtc_state *cstate)
>  {
> -	struct drm_atomic_state *state = cstate->base.state;
> -	struct drm_i915_private *dev_priv = to_i915(state->dev);
>  	uint_fixed_16_16_t linetime_us;
> -	uint32_t linetime_wm;
>  
>  	linetime_us = intel_get_linetime_us(cstate);
>  
>  	if (is_fixed16_zero(linetime_us))
>  		return 0;
>  
> -	linetime_wm = fixed16_to_u32_round_up(mul_u32_fixed16(8, linetime_us));
> +	return fixed16_to_u32_round_up(mul_u32_fixed16(8, linetime_us));

Am I the only one that finds this fixed16 stuff totally illegible?
This is also inefficient since it's doing multiple divisions where
one would suffice. I'd prefer people just wrote the code the in
the obvious (and more optimal) way.

> +}
> +
> +static uint32_t
> +skl_compute_linetime_wm(struct intel_crtc_state *cstate)
> +{
> +	struct drm_atomic_state *state = cstate->base.state;
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> +	uint32_t linetime_wm;
> +
> +	linetime_wm = intel_compute_linetime_wm(cstate);
>  
>  	/* Display WA #1135: bxt. */
>  	if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled)
> -- 
> 2.13.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx