Re: [PATCH 2/2] drm/i915/skl+: consider max supported plane pixel rate while scaling

["Lankhorst, Maarten" <maarten.lankhorst@intel.com>]

Mahesh Kumar schreef op do 18-05-2017 om 15:39 [+0530]:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
> 
> A display resolution is only supported if it meets all the
> restrictions
> below for Maximum Pipe Pixel Rate.
> 
> The display resolution must fit within the maximum pixel rate output
> from the pipe. Make sure that the display pipe is able to feed pixels
> at
> a rate required to support the desired resolution.
> For each enabled plane on the pipe {
>     If plane scaling enabled {
> 	Horizontal down scale amount = Maximum[1, plane horizontal size
> /
> 		    scaler horizontal window size]
> 	Vertical down scale amount = Maximum[1, plane vertical size /
> 		    scaler vertical window size]
> 	Plane down scale amount = Horizontal down scale amount *
> 		    Vertical down scale amount
> 	Plane Ratio = 1 / Plane down scale amount
>     }
>     Else {
> 	Plane Ratio = 1
>     }
>     If plane source pixel format is 64 bits per pixel {
> 	Plane Ratio = Plane Ratio * 8/9
>     }
> }
> 
> Pipe Ratio = Minimum Plane Ratio of all enabled planes on the pipe
> 
> If pipe scaling is enabled {
>     Horizontal down scale amount = Maximum[1, pipe horizontal source
> size /
> 		scaler horizontal window size]
>     Vertical down scale amount = Maximum[1, pipe vertical source size
> /
> 		scaler vertical window size]
>     Note: The progressive fetch - interlace display mode is
> equivalent to a
> 		2.0 vertical down scale
>     Pipe down scale amount = Horizontal down scale amount *
> 		Vertical down scale amount
>     Pipe Ratio = Pipe Ratio / Pipe down scale amount
> }
> 
> Pipe maximum pixel rate = CDCLK frequency * Pipe Ratio
> 
> In this patch our calculation is based on pipe downscale amount
> (plane max downscale amount * pipe downscale amount) instead of Pipe
> Ratio. So,
> max supported crtc clock with given scaling = CDCLK / pipe downscale.
> Flip will fail if,
> current crtc clock > max supported crct clock with given scaling.
> 
> Changes since V1:
>  - separate out fixed_16_16 wrapper API definition
> Changes since V2:
>  - Fix buggy crtc !active condition (Maarten)
>  - use intel_wm_plane_visible wrapper as per Maarten's suggestion
> Changes since V3:
>  - Change failure return from ERANGE to EINVAL
> Changes since V4:
>  - Rebase based on previous patch changes
> Changes since V5:
>  - return EINVAL instead of continue (Maarten)
> Changes since V6:
>  - Improve commit message
>  - Address review comment
> 
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  3 ++
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +
>  drivers/gpu/drm/i915/intel_pm.c      | 87
> ++++++++++++++++++++++++++++++++++++
>  3 files changed, 92 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 8217ed0e7132..df0b3e77129b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11177,6 +11177,9 @@ static int intel_crtc_atomic_check(struct
> drm_crtc *crtc,
>  			ret = skl_update_scaler_crtc(pipe_config);
>  
>  		if (!ret)
> +			ret =
> skl_check_pipe_max_pixel_rate(intel_crtc,
> +							    pipe_con
> fig);
> +		if (!ret)
>  			ret = intel_atomic_setup_scalers(dev_priv,
> intel_crtc,
>  							 pipe_config
> );
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index bd500977b3fc..93afac4a83fa 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1885,6 +1885,8 @@ bool skl_ddb_allocation_overlaps(const struct
> skl_ddb_entry **entries,
>  				 int ignore);
>  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);
>  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 fb4cec3fb92c..4edc5f4ee598 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3863,6 +3863,93 @@ skl_plane_downscale_amount(const struct
> intel_crtc_state *cstate,
>  	return mul_fixed16(downscale_w, downscale_h);
>  }
>  
> +static uint_fixed_16_16_t
> +skl_pipe_downscale_amount(const struct intel_crtc_state *config)
> +{
> +	uint_fixed_16_16_t pipe_downscale = u32_to_fixed_16_16(1);
> +
> +	if (!config->base.active)
> +		return pipe_downscale;
Sorry, just one more thing I failed to notice the first time around.

I think this should be config->base.enable, in case of !active the crtc
is configured as if it's on, and it's ok to run scaling checks here.
The existing code expects this as well.

config should probably be renamed to crtc_state, or pipe_config. :)
Either is more consistent with current code.
> +
> +	if (config->pch_pfit.enabled) {
> +		uint32_t src_w, src_h, dst_w, dst_h;
> +		uint32_t pfit_size = config->pch_pfit.size;
> +		uint_fixed_16_16_t fp_w_ratio, fp_h_ratio;
> +		uint_fixed_16_16_t downscale_h, downscale_w;
> +
> +		src_w = config->pipe_src_w;
> +		src_h = config->pipe_src_h;
> +		dst_w = pfit_size >> 16;
> +		dst_h = pfit_size & 0xffff;
> +
> +		if (!dst_w || !dst_h)
> +			return pipe_downscale;
> +
> +		fp_w_ratio = fixed_16_16_div(src_w, dst_w);
> +		fp_h_ratio = fixed_16_16_div(src_h, dst_h);
> +		downscale_w = max_fixed_16_16(fp_w_ratio,
> u32_to_fixed_16_16(1));
> +		downscale_h = max_fixed_16_16(fp_h_ratio,
> u32_to_fixed_16_16(1));
> +
> +		pipe_downscale = mul_fixed16(downscale_w,
> downscale_h);
> +	}
> +
> +	return pipe_downscale;
> +}
> +
> +int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
> +				  struct intel_crtc_state *cstate)
> +{
> +	struct drm_crtc_state *crtc_state = &cstate->base;
> +	struct drm_atomic_state *state = crtc_state->state;
> +	struct drm_plane *plane;
> +	const struct drm_plane_state *pstate;
> +	struct intel_plane_state *intel_pstate;
> +	int crtc_clock, cdclk;
> +	uint32_t pipe_max_pixel_rate;
> +	uint_fixed_16_16_t pipe_downscale;
> +	uint_fixed_16_16_t max_downscale = u32_to_fixed_16_16(1);
> +
> +	if (!cstate->base.active)
> +		return 0;
This one can be changed to enable as well I hope, though the FIXME in
intel_wm_plane_visible applies.. :)

> +	drm_atomic_crtc_state_for_each_plane_state(plane, pstate,
> crtc_state) {
> +		uint_fixed_16_16_t plane_downscale;
> +		uint_fixed_16_16_t fp_9_div_8 = fixed_16_16_div(9,
> 8);
> +		int bpp;
> +
> +		if (!intel_wm_plane_visible(cstate,
> +					    to_intel_plane_state(pst
> ate)))
> +			continue;
> +
> +		if (WARN_ON(!pstate->fb))
> +			return -EINVAL;
> +
> +		intel_pstate = to_intel_plane_state(pstate);
> +		plane_downscale = skl_plane_downscale_amount(cstate,
> +							     intel_p
> state);
> +		bpp = pstate->fb->format->cpp[0] * 8;
> +		if (bpp == 64)
> +			plane_downscale =
> mul_fixed16(plane_downscale,
> +						      fp_9_div_8);
> +
> +		max_downscale = max_fixed_16_16(plane_downscale,
> max_downscale);
> +	}
> +	pipe_downscale = skl_pipe_downscale_amount(cstate);
> +
> +	pipe_downscale = mul_fixed16(pipe_downscale, max_downscale);
> +
> +	crtc_clock = crtc_state->adjusted_mode.crtc_clock;
> +	cdclk = to_intel_atomic_state(state)->cdclk.logical.cdclk;
> +	pipe_max_pixel_rate = div_round_up_u32_fixed16(cdclk,
> pipe_downscale);
> +
> +	if (pipe_max_pixel_rate < crtc_clock) {
> +		DRM_ERROR("Max supported pixel clock with scaling
> exceeded\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static unsigned int
>  skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
>  			     const struct drm_plane_state *pstate,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx