Op 11-05-17 om 10:36 schreef Mahesh Kumar:
> 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@xxxxxxxxx>
>>> ---
>>> 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;
Oops, please also only return -EINVAL, definitely not ERANGE.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
