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

[Mahesh Kumar <mahesh1.kumar@intel.com>]

Hi,

Thanks for review.

On Wednesday 10 May 2017 06:52 PM, Maarten Lankhorst wrote:
> Op 08-05-17 om 13:49 schreef Mahesh Kumar:
>> 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
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@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 85b9e2f521a0..d64367e810f8 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -10992,6 +10992,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_config);
>> +		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 54f3ff840812..8323fc2ec4f2 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1859,6 +1859,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 92600cf42e12..69b1692ffd07 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -3397,6 +3397,93 @@ skl_plane_downscale_amount(const struct intel_crtc_state *cstate,
>>   	return mul_fixed_16_16(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;
>> +
>> +	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_fixed_16_16(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 check looks buggy, are there any tests?
>
> In general we try to do the same for !active as we do with active, so please to remove any !active checks altogether..
Thanks for catching this, yes it's a bug, during my testing this check 
was not there but I added this as an extra precaution & screwed-up :P
>> +	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 (!pstate->visible)
>> +			continue;
> Best use intel_wm_plane_visible here.
will do,

-Mahesh
>> +
>> +		if (WARN_ON(!pstate->fb))
>> +			continue;
>> +
>> +		intel_pstate = to_intel_plane_state(pstate);
>> +		plane_downscale = skl_plane_downscale_amount(cstate,
>> +							     intel_pstate);
>> +		bpp = pstate->fb->format->cpp[0] * 8;
>> +		if (bpp == 64)
>> +			plane_downscale = mul_fixed_16_16(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_fixed_16_16(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 = fixed_16_16_div_round_up_u64(cdclk,
>> +							   pipe_downscale);
>> +
>> +	if (pipe_max_pixel_rate < crtc_clock) {
>> +		DRM_ERROR("Max supported pixel clock with scaling exceeds\n");
>> +		return -ERANGE;
>> +	}
>> +
>> +	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