| Message ID | 20170508114902.18965-12-mahesh1.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.. > + 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. > + > + 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,
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, >
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@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; Oops, please also only return -EINVAL, definitely not ERANGE. ~Maarten
On Thursday 11 May 2017 03:18 PM, Maarten Lankhorst wrote: > 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@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; > Oops, please also only return -EINVAL, definitely not ERANGE. Requested scaling here is out of supported range that's why used -ERANGE, should I still change it to -EINVAL? -Mahesh > > ~Maarten
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; + + 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; + + 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,
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(+)