| Message ID | 20170607094905.16014-1-mahesh1.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 07, 2017 at 03:19:05PM +0530, Mahesh Kumar wrote: > In previous GEN default Interlace mode enabled is IF-ID mode, but IF-ID > mode has many limitations in SKL. This mode doesn't support y-tiling, > 90-270 rotation is not supported & YUV-420 planar source pixel formats > are not supported with above mode. I think we'll want something much more simple for stable. And that should probably be just -EINVAL if we try to do any of those forbidden things with an interlaced mode. > > This patch make changes to use PF-ID Interlace mode in SKL+ platforms. > PF-ID mode require one scaler to be attached in pipe-scaling mode. > During WM calculation adjusted pixel rate need to be doubled. > During max_supported_pixel_format calculation vertical downscale is 2.0. > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90238 > --- > drivers/gpu/drm/i915/intel_atomic.c | 13 ++++++++++ > drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++----- > drivers/gpu/drm/i915/intel_drv.h | 10 ++++++++ > drivers/gpu/drm/i915/intel_panel.c | 9 +++++-- > drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++- > 5 files changed, 87 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index d791b3ef89b5..06ed2fc449d7 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -248,6 +248,13 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > return -EINVAL; > } > > + if (num_scalers_need > 1 && > + scaler_state->scaler_users & (1 << SKL_CRTC_INDEX) && > + scaler_state->scaler_users & (1 << SKL_PF_ID_INDEX)) { > + DRM_ERROR("Can't attach scaler to CRTC in Interlace Mode\n"); > + return -EINVAL; > + } I thought the whole point of PF-ID here was that we can then do pipe scaling (and other things). So this check looks wrong to me. And I'm thinking we shouldn't be adding yet another scaler user for this, and instead it should just be part of the normal pfit setup. > + > /* walkthrough scaler_users bits and start assigning scalers */ > for (i = 0; i < sizeof(scaler_state->scaler_users) * 8; i++) { > int *scaler_id; > @@ -264,6 +271,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > > /* panel fitter case: assign as a crtc scaler */ > scaler_id = &scaler_state->scaler_id; > + } else if (i == SKL_PF_ID_INDEX) { > + name = "PF-ID INTERLACE MODE"; > + idx = intel_crtc->base.base.id; > + > + /* PF-ID interlaced mode case: needs a pipe scaler */ > + scaler_id = &scaler_state->scaler_id; > } else { > name = "PLANE"; > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 85ac32549e85..15c79b46d645 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > */ > need_scaling = src_w != dst_w || src_h != dst_h; > > + if ((crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) > + && scaler_user == SKL_PF_ID_INDEX) > + /* PF-ID Interlace mode needs a scaler */ > + need_scaling = true; > + > /* > * if plane is being disabled or scaler is no more required or force detach > * - free scaler binded to this plane/crtc > @@ -4635,16 +4640,24 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > * scaler can be assigned to other user. Actual register > * update to free the scaler is done in plane/panel-fit programming. > * For this purpose crtc/plane_state->scaler_id isn't reset here. > + * Now Interlace mode is a new user of pipe-scaler, so free the scaler-id > + * only if call is for respective user. If no user for scaler free it. > */ > if (force_detach || !need_scaling) { > - if (*scaler_id >= 0) { > + if (*scaler_id >= 0 && > + scaler_state->scaler_users & (1 << scaler_user)) { > scaler_state->scaler_users &= ~(1 << scaler_user); > scaler_state->scalers[*scaler_id].in_use = 0; > > DRM_DEBUG_KMS("scaler_user index %u.%u: " > "Staged freeing scaler id %d scaler_users = 0x%x\n", > - intel_crtc->pipe, scaler_user, *scaler_id, > - scaler_state->scaler_users); > + intel_crtc->pipe, scaler_user, > + *scaler_id, scaler_state->scaler_users); > + > + *scaler_id = -1; > + } > + if (*scaler_id >= 0 && !scaler_state->scaler_users) { > + scaler_state->scalers[*scaler_id].in_use = 0; > *scaler_id = -1; > } > return 0; > @@ -4691,6 +4704,16 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) > adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay); > } > > +static int skl_update_scaler_crtc_pf_id_output(struct intel_crtc_state *state) > +{ > + const struct drm_display_mode *mode = &state->base.adjusted_mode; > + > + return skl_update_scaler(state, !state->base.active, > + SKL_PF_ID_INDEX, &state->scaler_state.scaler_id, > + state->pipe_src_w, state->pipe_src_h, > + mode->crtc_hdisplay, mode->crtc_vdisplay); > +} > + > /** > * skl_update_scaler_plane - Stages update to scaler state for a given plane. > * > @@ -6258,6 +6281,15 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, > } > } > > + if (INTEL_GEN(dev_priv) >= 9) { > + if (skl_update_scaler_crtc_pf_id_output(pipe_config)) { > + DRM_ERROR("Unable to set scaler for PF-ID mode\n"); > + return -EINVAL; > + } > + intel_pch_panel_fitting(crtc, pipe_config, > + DRM_MODE_SCALE_FULLSCREEN); > + } > + > if (adjusted_mode->crtc_clock > clock_limit) { > DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n", > adjusted_mode->crtc_clock, clock_limit, > @@ -8045,9 +8077,12 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc) > if (IS_HASWELL(dev_priv) && intel_crtc->config->dither) > val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP); > > - if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) > - val |= PIPECONF_INTERLACED_ILK; > - else > + if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) { > + if (INTEL_GEN(dev_priv) >= 9) > + val |= PIPECONF_PFIT_PF_INTERLACED_ILK; > + else > + val |= PIPECONF_INTERLACED_ILK; > + } else > val |= PIPECONF_PROGRESSIVE; > > I915_WRITE(PIPECONF(cpu_transcoder), val); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 83dd40905821..d4546f6498b9 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -483,6 +483,7 @@ struct intel_crtc_scaler_state { > * avilability. > */ > #define SKL_CRTC_INDEX 31 > +#define SKL_PF_ID_INDEX 29 > unsigned scaler_users; > > /* scaler used by crtc for panel fitting purpose */ > @@ -1206,6 +1207,15 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi) > return container_of(intel_hdmi, struct intel_digital_port, hdmi); > } > > +static inline bool > +is_skl_interlace_mode(struct drm_i915_private *dev_priv, > + const struct drm_display_mode *mode) > +{ > + if (INTEL_GEN(dev_priv) < 9) > + return false; > + return mode->flags & DRM_MODE_FLAG_INTERLACE; > +} > + > /* intel_fifo_underrun.c */ > bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv, > enum pipe pipe, bool enable); > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 4114cb3f14e7..fae7fe7802f8 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -108,9 +108,14 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc, > const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; > int x = 0, y = 0, width = 0, height = 0; > > - /* Native modes don't need fitting */ > + /* > + * Native modes don't need fitting > + * PF-ID Interlace mode in SKL+ require a pipe scaler > + */ > if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w && > - adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) > + adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h && > + !(is_skl_interlace_mode(to_i915(intel_crtc->base.dev), > + adjusted_mode))) > goto done; > > switch (fitting_mode) { > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index aa9d8cef7ce0..dfb21f00fbed 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3873,15 +3873,18 @@ static uint_fixed_16_16_t > skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) > { > uint_fixed_16_16_t pipe_downscale = u32_to_fixed_16_16(1); > + const struct drm_display_mode *mode; > > if (!crtc_state->base.enable) > return pipe_downscale; > > + mode = &crtc_state->base.adjusted_mode; > if (crtc_state->pch_pfit.enabled) { > uint32_t src_w, src_h, dst_w, dst_h; > uint32_t pfit_size = crtc_state->pch_pfit.size; > uint_fixed_16_16_t fp_w_ratio, fp_h_ratio; > uint_fixed_16_16_t downscale_h, downscale_w; > + uint_fixed_16_16_t min_h_downscale; > > src_w = crtc_state->pipe_src_w; > src_h = crtc_state->pipe_src_h; > @@ -3891,10 +3894,19 @@ skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) > if (!dst_w || !dst_h) > return pipe_downscale; > > + /* > + * Interlace mode require 1 scaler so no other scaler can be > + * attached to pipe. PF-ID mode is equivalent to 2.0 vertical > + * downscale. > + */ > + if (mode->flags & DRM_MODE_FLAG_INTERLACE) > + min_h_downscale = u32_to_fixed_16_16(2); > + else > + min_h_downscale = u32_to_fixed_16_16(1); > 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)); > + downscale_h = max_fixed_16_16(fp_h_ratio, min_h_downscale); > > pipe_downscale = mul_fixed16(downscale_w, downscale_h); > } > @@ -4418,6 +4430,9 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cstate, > * with additional adjustments for plane-specific scaling. > */ > adjusted_pixel_rate = cstate->pixel_rate; > + if (cstate->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) > + adjusted_pixel_rate *= 2; > + > downscale_amount = skl_plane_downscale_amount(cstate, pstate); > > return mul_round_up_u32_fixed16(adjusted_pixel_rate, > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Ville, Thanks for review On Friday 16 June 2017 07:47 PM, Ville Syrjälä wrote: > On Wed, Jun 07, 2017 at 03:19:05PM +0530, Mahesh Kumar wrote: >> In previous GEN default Interlace mode enabled is IF-ID mode, but IF-ID >> mode has many limitations in SKL. This mode doesn't support y-tiling, >> 90-270 rotation is not supported & YUV-420 planar source pixel formats >> are not supported with above mode. > I think we'll want something much more simple for stable. And that > should probably be just -EINVAL if we try to do any of those > forbidden things with an interlaced mode. Agree. > >> This patch make changes to use PF-ID Interlace mode in SKL+ platforms. >> PF-ID mode require one scaler to be attached in pipe-scaling mode. >> During WM calculation adjusted pixel rate need to be doubled. >> During max_supported_pixel_format calculation vertical downscale is 2.0. >> >> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90238 >> --- >> drivers/gpu/drm/i915/intel_atomic.c | 13 ++++++++++ >> drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++----- >> drivers/gpu/drm/i915/intel_drv.h | 10 ++++++++ >> drivers/gpu/drm/i915/intel_panel.c | 9 +++++-- >> drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++- >> 5 files changed, 87 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c >> index d791b3ef89b5..06ed2fc449d7 100644 >> --- a/drivers/gpu/drm/i915/intel_atomic.c >> +++ b/drivers/gpu/drm/i915/intel_atomic.c >> @@ -248,6 +248,13 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, >> return -EINVAL; >> } >> >> + if (num_scalers_need > 1 && >> + scaler_state->scaler_users & (1 << SKL_CRTC_INDEX) && >> + scaler_state->scaler_users & (1 << SKL_PF_ID_INDEX)) { >> + DRM_ERROR("Can't attach scaler to CRTC in Interlace Mode\n"); >> + return -EINVAL; >> + } > I thought the whole point of PF-ID here was that we can then do pipe > scaling (and other things). So this check looks wrong to me. With PF-ID we can have Y-tiled buffers, & 90/270 rotation enabled, It doesn't explicitly talk about scaling. BSpec doesn't limit us from using 2 scalers as a pipe scaler, still I sent a query to HW team regarding same. Current S/W framework doesn't allows us to attach multiple pipe-scalers in a pipe (multiple scaler_id associated with one crtc_state). that's the reason for above check. > > And I'm thinking we shouldn't be adding yet another scaler user for > this, and instead it should just be part of the normal pfit setup. To use existing scaler framework, we need one scaler-user to be associated with it. If we use existing CRTC_INDEX user then there will be many corner cases to handle. To make it simple I added new scaler-user. This code uses pfit setup only to attach/enable Pipe-scaler. Please let me know if I didn't understood your concern correctly. Any other suggestions/queries are welcome. -Mahesh > >> + >> /* walkthrough scaler_users bits and start assigning scalers */ >> for (i = 0; i < sizeof(scaler_state->scaler_users) * 8; i++) { >> int *scaler_id; >> @@ -264,6 +271,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, >> >> /* panel fitter case: assign as a crtc scaler */ >> scaler_id = &scaler_state->scaler_id; >> + } else if (i == SKL_PF_ID_INDEX) { >> + name = "PF-ID INTERLACE MODE"; >> + idx = intel_crtc->base.base.id; >> + >> + /* PF-ID interlaced mode case: needs a pipe scaler */ >> + scaler_id = &scaler_state->scaler_id; >> } else { >> name = "PLANE"; >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 85ac32549e85..15c79b46d645 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, >> */ >> need_scaling = src_w != dst_w || src_h != dst_h; >> >> + if ((crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) >> + && scaler_user == SKL_PF_ID_INDEX) >> + /* PF-ID Interlace mode needs a scaler */ >> + need_scaling = true; >> + >> /* >> * if plane is being disabled or scaler is no more required or force detach >> * - free scaler binded to this plane/crtc >> @@ -4635,16 +4640,24 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, >> * scaler can be assigned to other user. Actual register >> * update to free the scaler is done in plane/panel-fit programming. >> * For this purpose crtc/plane_state->scaler_id isn't reset here. >> + * Now Interlace mode is a new user of pipe-scaler, so free the scaler-id >> + * only if call is for respective user. If no user for scaler free it. >> */ >> if (force_detach || !need_scaling) { >> - if (*scaler_id >= 0) { >> + if (*scaler_id >= 0 && >> + scaler_state->scaler_users & (1 << scaler_user)) { >> scaler_state->scaler_users &= ~(1 << scaler_user); >> scaler_state->scalers[*scaler_id].in_use = 0; >> >> DRM_DEBUG_KMS("scaler_user index %u.%u: " >> "Staged freeing scaler id %d scaler_users = 0x%x\n", >> - intel_crtc->pipe, scaler_user, *scaler_id, >> - scaler_state->scaler_users); >> + intel_crtc->pipe, scaler_user, >> + *scaler_id, scaler_state->scaler_users); >> + >> + *scaler_id = -1; >> + } >> + if (*scaler_id >= 0 && !scaler_state->scaler_users) { >> + scaler_state->scalers[*scaler_id].in_use = 0; >> *scaler_id = -1; >> } >> return 0; >> @@ -4691,6 +4704,16 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) >> adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay); >> } >> >> +static int skl_update_scaler_crtc_pf_id_output(struct intel_crtc_state *state) >> +{ >> + const struct drm_display_mode *mode = &state->base.adjusted_mode; >> + >> + return skl_update_scaler(state, !state->base.active, >> + SKL_PF_ID_INDEX, &state->scaler_state.scaler_id, >> + state->pipe_src_w, state->pipe_src_h, >> + mode->crtc_hdisplay, mode->crtc_vdisplay); >> +} >> + >> /** >> * skl_update_scaler_plane - Stages update to scaler state for a given plane. >> * >> @@ -6258,6 +6281,15 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, >> } >> } >> >> + if (INTEL_GEN(dev_priv) >= 9) { >> + if (skl_update_scaler_crtc_pf_id_output(pipe_config)) { >> + DRM_ERROR("Unable to set scaler for PF-ID mode\n"); >> + return -EINVAL; >> + } >> + intel_pch_panel_fitting(crtc, pipe_config, >> + DRM_MODE_SCALE_FULLSCREEN); >> + } >> + >> if (adjusted_mode->crtc_clock > clock_limit) { >> DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n", >> adjusted_mode->crtc_clock, clock_limit, >> @@ -8045,9 +8077,12 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc) >> if (IS_HASWELL(dev_priv) && intel_crtc->config->dither) >> val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP); >> >> - if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) >> - val |= PIPECONF_INTERLACED_ILK; >> - else >> + if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) { >> + if (INTEL_GEN(dev_priv) >= 9) >> + val |= PIPECONF_PFIT_PF_INTERLACED_ILK; >> + else >> + val |= PIPECONF_INTERLACED_ILK; >> + } else >> val |= PIPECONF_PROGRESSIVE; >> >> I915_WRITE(PIPECONF(cpu_transcoder), val); >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 83dd40905821..d4546f6498b9 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -483,6 +483,7 @@ struct intel_crtc_scaler_state { >> * avilability. >> */ >> #define SKL_CRTC_INDEX 31 >> +#define SKL_PF_ID_INDEX 29 >> unsigned scaler_users; >> >> /* scaler used by crtc for panel fitting purpose */ >> @@ -1206,6 +1207,15 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi) >> return container_of(intel_hdmi, struct intel_digital_port, hdmi); >> } >> >> +static inline bool >> +is_skl_interlace_mode(struct drm_i915_private *dev_priv, >> + const struct drm_display_mode *mode) >> +{ >> + if (INTEL_GEN(dev_priv) < 9) >> + return false; >> + return mode->flags & DRM_MODE_FLAG_INTERLACE; >> +} >> + >> /* intel_fifo_underrun.c */ >> bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv, >> enum pipe pipe, bool enable); >> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c >> index 4114cb3f14e7..fae7fe7802f8 100644 >> --- a/drivers/gpu/drm/i915/intel_panel.c >> +++ b/drivers/gpu/drm/i915/intel_panel.c >> @@ -108,9 +108,14 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc, >> const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; >> int x = 0, y = 0, width = 0, height = 0; >> >> - /* Native modes don't need fitting */ >> + /* >> + * Native modes don't need fitting >> + * PF-ID Interlace mode in SKL+ require a pipe scaler >> + */ >> if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w && >> - adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) >> + adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h && >> + !(is_skl_interlace_mode(to_i915(intel_crtc->base.dev), >> + adjusted_mode))) >> goto done; >> >> switch (fitting_mode) { >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index aa9d8cef7ce0..dfb21f00fbed 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3873,15 +3873,18 @@ static uint_fixed_16_16_t >> skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) >> { >> uint_fixed_16_16_t pipe_downscale = u32_to_fixed_16_16(1); >> + const struct drm_display_mode *mode; >> >> if (!crtc_state->base.enable) >> return pipe_downscale; >> >> + mode = &crtc_state->base.adjusted_mode; >> if (crtc_state->pch_pfit.enabled) { >> uint32_t src_w, src_h, dst_w, dst_h; >> uint32_t pfit_size = crtc_state->pch_pfit.size; >> uint_fixed_16_16_t fp_w_ratio, fp_h_ratio; >> uint_fixed_16_16_t downscale_h, downscale_w; >> + uint_fixed_16_16_t min_h_downscale; >> >> src_w = crtc_state->pipe_src_w; >> src_h = crtc_state->pipe_src_h; >> @@ -3891,10 +3894,19 @@ skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) >> if (!dst_w || !dst_h) >> return pipe_downscale; >> >> + /* >> + * Interlace mode require 1 scaler so no other scaler can be >> + * attached to pipe. PF-ID mode is equivalent to 2.0 vertical >> + * downscale. >> + */ >> + if (mode->flags & DRM_MODE_FLAG_INTERLACE) >> + min_h_downscale = u32_to_fixed_16_16(2); >> + else >> + min_h_downscale = u32_to_fixed_16_16(1); >> 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)); >> + downscale_h = max_fixed_16_16(fp_h_ratio, min_h_downscale); >> >> pipe_downscale = mul_fixed16(downscale_w, downscale_h); >> } >> @@ -4418,6 +4430,9 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cstate, >> * with additional adjustments for plane-specific scaling. >> */ >> adjusted_pixel_rate = cstate->pixel_rate; >> + if (cstate->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) >> + adjusted_pixel_rate *= 2; >> + >> downscale_amount = skl_plane_downscale_amount(cstate, pstate); >> >> return mul_round_up_u32_fixed16(adjusted_pixel_rate, >> -- >> 2.11.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Jun 19, 2017 at 05:50:28PM +0530, Mahesh Kumar wrote: > Hi Ville, > > Thanks for review > > > On Friday 16 June 2017 07:47 PM, Ville Syrjälä wrote: > > On Wed, Jun 07, 2017 at 03:19:05PM +0530, Mahesh Kumar wrote: > >> In previous GEN default Interlace mode enabled is IF-ID mode, but IF-ID > >> mode has many limitations in SKL. This mode doesn't support y-tiling, > >> 90-270 rotation is not supported & YUV-420 planar source pixel formats > >> are not supported with above mode. > > I think we'll want something much more simple for stable. And that > > should probably be just -EINVAL if we try to do any of those > > forbidden things with an interlaced mode. > Agree. > > > >> This patch make changes to use PF-ID Interlace mode in SKL+ platforms. > >> PF-ID mode require one scaler to be attached in pipe-scaling mode. > >> During WM calculation adjusted pixel rate need to be doubled. > >> During max_supported_pixel_format calculation vertical downscale is 2.0. > >> > >> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90238 > >> --- > >> drivers/gpu/drm/i915/intel_atomic.c | 13 ++++++++++ > >> drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++----- > >> drivers/gpu/drm/i915/intel_drv.h | 10 ++++++++ > >> drivers/gpu/drm/i915/intel_panel.c | 9 +++++-- > >> drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++- > >> 5 files changed, 87 insertions(+), 9 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > >> index d791b3ef89b5..06ed2fc449d7 100644 > >> --- a/drivers/gpu/drm/i915/intel_atomic.c > >> +++ b/drivers/gpu/drm/i915/intel_atomic.c > >> @@ -248,6 +248,13 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > >> return -EINVAL; > >> } > >> > >> + if (num_scalers_need > 1 && > >> + scaler_state->scaler_users & (1 << SKL_CRTC_INDEX) && > >> + scaler_state->scaler_users & (1 << SKL_PF_ID_INDEX)) { > >> + DRM_ERROR("Can't attach scaler to CRTC in Interlace Mode\n"); > >> + return -EINVAL; > >> + } > > I thought the whole point of PF-ID here was that we can then do pipe > > scaling (and other things). So this check looks wrong to me. > With PF-ID we can have Y-tiled buffers, & 90/270 rotation enabled, It > doesn't explicitly talk about scaling. PF-ID itself is scaling. Presumably you don't have to have exactly 2:1 scaling factor with PF-ID. > BSpec doesn't limit us from using 2 scalers as a pipe scaler, still I > sent a query to HW team regarding same. We don't want to use multiple scalers. I dubt that would even work. And before SKL the panel fitter was a dedicated piece of hw anyway so there was no way to even try to assign multiple scalers to the pipe. > > Current S/W framework doesn't allows us to attach multiple pipe-scalers > in a pipe (multiple scaler_id associated with one crtc_state). that's > the reason for above check. > > > > And I'm thinking we shouldn't be adding yet another scaler user for > > this, and instead it should just be part of the normal pfit setup. > To use existing scaler framework, we need one scaler-user to be > associated with it. > If we use existing CRTC_INDEX user then there will be many corner cases > to handle. I'm not 100% whether we need any adjustment in the pfit code itself. The spec doesn't seem to tell us that we need to adjust the panel fitter window in any way. That's a little inconsistent with the resulting scaling factor, but I guess the hw will automagically reduce the destination size by two when the pipe is in the PF-ID mode. So I think it should mostly be a simple matter of calling the pfit code to set up the pos/size, and then you should just adjust ilk_pipe_pixel_rate() to double the resulting pixel rate. That said, I'm not really sure we want to flip PF-ID on automagically. It would result in scaling when the user doesn't necessarily want it. So we might want to think about some kind if property to control this stuff. > To make it simple I added new scaler-user. > This code uses pfit setup only to attach/enable Pipe-scaler. > Please let me know if I didn't understood your concern correctly. > Any other suggestions/queries are welcome. > > -Mahesh > > > > >> + > >> /* walkthrough scaler_users bits and start assigning scalers */ > >> for (i = 0; i < sizeof(scaler_state->scaler_users) * 8; i++) { > >> int *scaler_id; > >> @@ -264,6 +271,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > >> > >> /* panel fitter case: assign as a crtc scaler */ > >> scaler_id = &scaler_state->scaler_id; > >> + } else if (i == SKL_PF_ID_INDEX) { > >> + name = "PF-ID INTERLACE MODE"; > >> + idx = intel_crtc->base.base.id; > >> + > >> + /* PF-ID interlaced mode case: needs a pipe scaler */ > >> + scaler_id = &scaler_state->scaler_id; > >> } else { > >> name = "PLANE"; > >> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >> index 85ac32549e85..15c79b46d645 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > >> */ > >> need_scaling = src_w != dst_w || src_h != dst_h; > >> > >> + if ((crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) > >> + && scaler_user == SKL_PF_ID_INDEX) > >> + /* PF-ID Interlace mode needs a scaler */ > >> + need_scaling = true; > >> + > >> /* > >> * if plane is being disabled or scaler is no more required or force detach > >> * - free scaler binded to this plane/crtc > >> @@ -4635,16 +4640,24 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > >> * scaler can be assigned to other user. Actual register > >> * update to free the scaler is done in plane/panel-fit programming. > >> * For this purpose crtc/plane_state->scaler_id isn't reset here. > >> + * Now Interlace mode is a new user of pipe-scaler, so free the scaler-id > >> + * only if call is for respective user. If no user for scaler free it. > >> */ > >> if (force_detach || !need_scaling) { > >> - if (*scaler_id >= 0) { > >> + if (*scaler_id >= 0 && > >> + scaler_state->scaler_users & (1 << scaler_user)) { > >> scaler_state->scaler_users &= ~(1 << scaler_user); > >> scaler_state->scalers[*scaler_id].in_use = 0; > >> > >> DRM_DEBUG_KMS("scaler_user index %u.%u: " > >> "Staged freeing scaler id %d scaler_users = 0x%x\n", > >> - intel_crtc->pipe, scaler_user, *scaler_id, > >> - scaler_state->scaler_users); > >> + intel_crtc->pipe, scaler_user, > >> + *scaler_id, scaler_state->scaler_users); > >> + > >> + *scaler_id = -1; > >> + } > >> + if (*scaler_id >= 0 && !scaler_state->scaler_users) { > >> + scaler_state->scalers[*scaler_id].in_use = 0; > >> *scaler_id = -1; > >> } > >> return 0; > >> @@ -4691,6 +4704,16 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) > >> adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay); > >> } > >> > >> +static int skl_update_scaler_crtc_pf_id_output(struct intel_crtc_state *state) > >> +{ > >> + const struct drm_display_mode *mode = &state->base.adjusted_mode; > >> + > >> + return skl_update_scaler(state, !state->base.active, > >> + SKL_PF_ID_INDEX, &state->scaler_state.scaler_id, > >> + state->pipe_src_w, state->pipe_src_h, > >> + mode->crtc_hdisplay, mode->crtc_vdisplay); > >> +} > >> + > >> /** > >> * skl_update_scaler_plane - Stages update to scaler state for a given plane. > >> * > >> @@ -6258,6 +6281,15 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, > >> } > >> } > >> > >> + if (INTEL_GEN(dev_priv) >= 9) { > >> + if (skl_update_scaler_crtc_pf_id_output(pipe_config)) { > >> + DRM_ERROR("Unable to set scaler for PF-ID mode\n"); > >> + return -EINVAL; > >> + } > >> + intel_pch_panel_fitting(crtc, pipe_config, > >> + DRM_MODE_SCALE_FULLSCREEN); > >> + } > >> + > >> if (adjusted_mode->crtc_clock > clock_limit) { > >> DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n", > >> adjusted_mode->crtc_clock, clock_limit, > >> @@ -8045,9 +8077,12 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc) > >> if (IS_HASWELL(dev_priv) && intel_crtc->config->dither) > >> val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP); > >> > >> - if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) > >> - val |= PIPECONF_INTERLACED_ILK; > >> - else > >> + if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) { > >> + if (INTEL_GEN(dev_priv) >= 9) > >> + val |= PIPECONF_PFIT_PF_INTERLACED_ILK; > >> + else > >> + val |= PIPECONF_INTERLACED_ILK; > >> + } else > >> val |= PIPECONF_PROGRESSIVE; > >> > >> I915_WRITE(PIPECONF(cpu_transcoder), val); > >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > >> index 83dd40905821..d4546f6498b9 100644 > >> --- a/drivers/gpu/drm/i915/intel_drv.h > >> +++ b/drivers/gpu/drm/i915/intel_drv.h > >> @@ -483,6 +483,7 @@ struct intel_crtc_scaler_state { > >> * avilability. > >> */ > >> #define SKL_CRTC_INDEX 31 > >> +#define SKL_PF_ID_INDEX 29 > >> unsigned scaler_users; > >> > >> /* scaler used by crtc for panel fitting purpose */ > >> @@ -1206,6 +1207,15 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi) > >> return container_of(intel_hdmi, struct intel_digital_port, hdmi); > >> } > >> > >> +static inline bool > >> +is_skl_interlace_mode(struct drm_i915_private *dev_priv, > >> + const struct drm_display_mode *mode) > >> +{ > >> + if (INTEL_GEN(dev_priv) < 9) > >> + return false; > >> + return mode->flags & DRM_MODE_FLAG_INTERLACE; > >> +} > >> + > >> /* intel_fifo_underrun.c */ > >> bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv, > >> enum pipe pipe, bool enable); > >> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > >> index 4114cb3f14e7..fae7fe7802f8 100644 > >> --- a/drivers/gpu/drm/i915/intel_panel.c > >> +++ b/drivers/gpu/drm/i915/intel_panel.c > >> @@ -108,9 +108,14 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc, > >> const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; > >> int x = 0, y = 0, width = 0, height = 0; > >> > >> - /* Native modes don't need fitting */ > >> + /* > >> + * Native modes don't need fitting > >> + * PF-ID Interlace mode in SKL+ require a pipe scaler > >> + */ > >> if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w && > >> - adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) > >> + adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h && > >> + !(is_skl_interlace_mode(to_i915(intel_crtc->base.dev), > >> + adjusted_mode))) > >> goto done; > >> > >> switch (fitting_mode) { > >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > >> index aa9d8cef7ce0..dfb21f00fbed 100644 > >> --- a/drivers/gpu/drm/i915/intel_pm.c > >> +++ b/drivers/gpu/drm/i915/intel_pm.c > >> @@ -3873,15 +3873,18 @@ static uint_fixed_16_16_t > >> skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) > >> { > >> uint_fixed_16_16_t pipe_downscale = u32_to_fixed_16_16(1); > >> + const struct drm_display_mode *mode; > >> > >> if (!crtc_state->base.enable) > >> return pipe_downscale; > >> > >> + mode = &crtc_state->base.adjusted_mode; > >> if (crtc_state->pch_pfit.enabled) { > >> uint32_t src_w, src_h, dst_w, dst_h; > >> uint32_t pfit_size = crtc_state->pch_pfit.size; > >> uint_fixed_16_16_t fp_w_ratio, fp_h_ratio; > >> uint_fixed_16_16_t downscale_h, downscale_w; > >> + uint_fixed_16_16_t min_h_downscale; > >> > >> src_w = crtc_state->pipe_src_w; > >> src_h = crtc_state->pipe_src_h; > >> @@ -3891,10 +3894,19 @@ skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) > >> if (!dst_w || !dst_h) > >> return pipe_downscale; > >> > >> + /* > >> + * Interlace mode require 1 scaler so no other scaler can be > >> + * attached to pipe. PF-ID mode is equivalent to 2.0 vertical > >> + * downscale. > >> + */ > >> + if (mode->flags & DRM_MODE_FLAG_INTERLACE) > >> + min_h_downscale = u32_to_fixed_16_16(2); > >> + else > >> + min_h_downscale = u32_to_fixed_16_16(1); > >> 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)); > >> + downscale_h = max_fixed_16_16(fp_h_ratio, min_h_downscale); > >> > >> pipe_downscale = mul_fixed16(downscale_w, downscale_h); > >> } > >> @@ -4418,6 +4430,9 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cstate, > >> * with additional adjustments for plane-specific scaling. > >> */ > >> adjusted_pixel_rate = cstate->pixel_rate; > >> + if (cstate->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) > >> + adjusted_pixel_rate *= 2; > >> + > >> downscale_amount = skl_plane_downscale_amount(cstate, pstate); > >> > >> return mul_round_up_u32_fixed16(adjusted_pixel_rate, > >> -- > >> 2.11.0 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi, On Monday 19 June 2017 06:35 PM, Ville Syrjälä wrote: > On Mon, Jun 19, 2017 at 05:50:28PM +0530, Mahesh Kumar wrote: >> Hi Ville, >> >> Thanks for review >> >> >> On Friday 16 June 2017 07:47 PM, Ville Syrjälä wrote: >>> On Wed, Jun 07, 2017 at 03:19:05PM +0530, Mahesh Kumar wrote: >>>> In previous GEN default Interlace mode enabled is IF-ID mode, but IF-ID >>>> mode has many limitations in SKL. This mode doesn't support y-tiling, >>>> 90-270 rotation is not supported & YUV-420 planar source pixel formats >>>> are not supported with above mode. >>> I think we'll want something much more simple for stable. And that >>> should probably be just -EINVAL if we try to do any of those >>> forbidden things with an interlaced mode. >> Agree. >>>> This patch make changes to use PF-ID Interlace mode in SKL+ platforms. >>>> PF-ID mode require one scaler to be attached in pipe-scaling mode. >>>> During WM calculation adjusted pixel rate need to be doubled. >>>> During max_supported_pixel_format calculation vertical downscale is 2.0. >>>> >>>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90238 >>>> --- >>>> drivers/gpu/drm/i915/intel_atomic.c | 13 ++++++++++ >>>> drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++----- >>>> drivers/gpu/drm/i915/intel_drv.h | 10 ++++++++ >>>> drivers/gpu/drm/i915/intel_panel.c | 9 +++++-- >>>> drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++- >>>> 5 files changed, 87 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c >>>> index d791b3ef89b5..06ed2fc449d7 100644 >>>> --- a/drivers/gpu/drm/i915/intel_atomic.c >>>> +++ b/drivers/gpu/drm/i915/intel_atomic.c >>>> @@ -248,6 +248,13 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, >>>> return -EINVAL; >>>> } >>>> >>>> + if (num_scalers_need > 1 && >>>> + scaler_state->scaler_users & (1 << SKL_CRTC_INDEX) && >>>> + scaler_state->scaler_users & (1 << SKL_PF_ID_INDEX)) { >>>> + DRM_ERROR("Can't attach scaler to CRTC in Interlace Mode\n"); >>>> + return -EINVAL; >>>> + } >>> I thought the whole point of PF-ID here was that we can then do pipe >>> scaling (and other things). So this check looks wrong to me. >> With PF-ID we can have Y-tiled buffers, & 90/270 rotation enabled, It >> doesn't explicitly talk about scaling. > PF-ID itself is scaling. Presumably you don't have to have exactly > 2:1 scaling factor with PF-ID. > >> BSpec doesn't limit us from using 2 scalers as a pipe scaler, still I >> sent a query to HW team regarding same. > We don't want to use multiple scalers. I dubt that would even work. And > before SKL the panel fitter was a dedicated piece of hw anyway so there > was no way to even try to assign multiple scalers to the pipe. got it.. >> Current S/W framework doesn't allows us to attach multiple pipe-scalers >> in a pipe (multiple scaler_id associated with one crtc_state). that's >> the reason for above check. >>> And I'm thinking we shouldn't be adding yet another scaler user for >>> this, and instead it should just be part of the normal pfit setup. >> To use existing scaler framework, we need one scaler-user to be >> associated with it. >> If we use existing CRTC_INDEX user then there will be many corner cases >> to handle. > I'm not 100% whether we need any adjustment in the pfit code itself. > The spec doesn't seem to tell us that we need to adjust the panel > fitter window in any way. That's a little inconsistent with the > resulting scaling factor, but I guess the hw will automagically reduce > the destination size by two when the pipe is in the PF-ID mode. yes, this is the same behavior as per my experiment, Hardware automatically taking care of everything. (it's not reducing programmed destination size by 2). I'm just setting source-size == destination-size & src-pos == dst-pos for scaler & hardware automagically taking care of all the conversions. > > So I think it should mostly be a simple matter of calling the pfit > code to set up the pos/size, and then you should just adjust > ilk_pipe_pixel_rate() to double the resulting pixel rate. Agree, setting this in ilk_pipe_pixel_rate() will be right place, will incorporate this :) > > That said, I'm not really sure we want to flip PF-ID on automagically. > It would result in scaling when the user doesn't necessarily want it. > So we might want to think about some kind if property to control this > stuff. yeah, PF-ID is needed only if user want Y-tiled buffer and/or 90/270 rotation. But If we add a new property to choose mode (PF-PD / PF-ID / IF-ID) who will be the user for this connector property? We don't have any open-source user available to use this property as of now. Please suggest if that is ok to use? -Mahesh >> To make it simple I added new scaler-user. >> This code uses pfit setup only to attach/enable Pipe-scaler. >> Please let me know if I didn't understood your concern correctly. >> Any other suggestions/queries are welcome. >> >> -Mahesh >> >>>> + >>>> /* walkthrough scaler_users bits and start assigning scalers */ >>>> for (i = 0; i < sizeof(scaler_state->scaler_users) * 8; i++) { >>>> int *scaler_id; >>>> @@ -264,6 +271,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, >>>> >>>> /* panel fitter case: assign as a crtc scaler */ >>>> scaler_id = &scaler_state->scaler_id; >>>> + } else if (i == SKL_PF_ID_INDEX) { >>>> + name = "PF-ID INTERLACE MODE"; >>>> + idx = intel_crtc->base.base.id; >>>> + >>>> + /* PF-ID interlaced mode case: needs a pipe scaler */ >>>> + scaler_id = &scaler_state->scaler_id; >>>> } else { >>>> name = "PLANE"; >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >>>> index 85ac32549e85..15c79b46d645 100644 >>>> --- a/drivers/gpu/drm/i915/intel_display.c >>>> +++ b/drivers/gpu/drm/i915/intel_display.c >>>> @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, >>>> */ >>>> need_scaling = src_w != dst_w || src_h != dst_h; >>>> >>>> + if ((crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) >>>> + && scaler_user == SKL_PF_ID_INDEX) >>>> + /* PF-ID Interlace mode needs a scaler */ >>>> + need_scaling = true; >>>> + >>>> /* >>>> * if plane is being disabled or scaler is no more required or force detach >>>> * - free scaler binded to this plane/crtc >>>> @@ -4635,16 +4640,24 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, >>>> * scaler can be assigned to other user. Actual register >>>> * update to free the scaler is done in plane/panel-fit programming. >>>> * For this purpose crtc/plane_state->scaler_id isn't reset here. >>>> + * Now Interlace mode is a new user of pipe-scaler, so free the scaler-id >>>> + * only if call is for respective user. If no user for scaler free it. >>>> */ >>>> if (force_detach || !need_scaling) { >>>> - if (*scaler_id >= 0) { >>>> + if (*scaler_id >= 0 && >>>> + scaler_state->scaler_users & (1 << scaler_user)) { >>>> scaler_state->scaler_users &= ~(1 << scaler_user); >>>> scaler_state->scalers[*scaler_id].in_use = 0; >>>> >>>> DRM_DEBUG_KMS("scaler_user index %u.%u: " >>>> "Staged freeing scaler id %d scaler_users = 0x%x\n", >>>> - intel_crtc->pipe, scaler_user, *scaler_id, >>>> - scaler_state->scaler_users); >>>> + intel_crtc->pipe, scaler_user, >>>> + *scaler_id, scaler_state->scaler_users); >>>> + >>>> + *scaler_id = -1; >>>> + } >>>> + if (*scaler_id >= 0 && !scaler_state->scaler_users) { >>>> + scaler_state->scalers[*scaler_id].in_use = 0; >>>> *scaler_id = -1; >>>> } >>>> return 0; >>>> @@ -4691,6 +4704,16 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) >>>> adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay); >>>> } >>>> >>>> +static int skl_update_scaler_crtc_pf_id_output(struct intel_crtc_state *state) >>>> +{ >>>> + const struct drm_display_mode *mode = &state->base.adjusted_mode; >>>> + >>>> + return skl_update_scaler(state, !state->base.active, >>>> + SKL_PF_ID_INDEX, &state->scaler_state.scaler_id, >>>> + state->pipe_src_w, state->pipe_src_h, >>>> + mode->crtc_hdisplay, mode->crtc_vdisplay); >>>> +} >>>> + >>>> /** >>>> * skl_update_scaler_plane - Stages update to scaler state for a given plane. >>>> * >>>> @@ -6258,6 +6281,15 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, >>>> } >>>> } >>>> >>>> + if (INTEL_GEN(dev_priv) >= 9) { >>>> + if (skl_update_scaler_crtc_pf_id_output(pipe_config)) { >>>> + DRM_ERROR("Unable to set scaler for PF-ID mode\n"); >>>> + return -EINVAL; >>>> + } >>>> + intel_pch_panel_fitting(crtc, pipe_config, >>>> + DRM_MODE_SCALE_FULLSCREEN); >>>> + } >>>> + >>>> if (adjusted_mode->crtc_clock > clock_limit) { >>>> DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n", >>>> adjusted_mode->crtc_clock, clock_limit, >>>> @@ -8045,9 +8077,12 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc) >>>> if (IS_HASWELL(dev_priv) && intel_crtc->config->dither) >>>> val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP); >>>> >>>> - if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) >>>> - val |= PIPECONF_INTERLACED_ILK; >>>> - else >>>> + if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) { >>>> + if (INTEL_GEN(dev_priv) >= 9) >>>> + val |= PIPECONF_PFIT_PF_INTERLACED_ILK; >>>> + else >>>> + val |= PIPECONF_INTERLACED_ILK; >>>> + } else >>>> val |= PIPECONF_PROGRESSIVE; >>>> >>>> I915_WRITE(PIPECONF(cpu_transcoder), val); >>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>>> index 83dd40905821..d4546f6498b9 100644 >>>> --- a/drivers/gpu/drm/i915/intel_drv.h >>>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>>> @@ -483,6 +483,7 @@ struct intel_crtc_scaler_state { >>>> * avilability. >>>> */ >>>> #define SKL_CRTC_INDEX 31 >>>> +#define SKL_PF_ID_INDEX 29 >>>> unsigned scaler_users; >>>> >>>> /* scaler used by crtc for panel fitting purpose */ >>>> @@ -1206,6 +1207,15 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi) >>>> return container_of(intel_hdmi, struct intel_digital_port, hdmi); >>>> } >>>> >>>> +static inline bool >>>> +is_skl_interlace_mode(struct drm_i915_private *dev_priv, >>>> + const struct drm_display_mode *mode) >>>> +{ >>>> + if (INTEL_GEN(dev_priv) < 9) >>>> + return false; >>>> + return mode->flags & DRM_MODE_FLAG_INTERLACE; >>>> +} >>>> + >>>> /* intel_fifo_underrun.c */ >>>> bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv, >>>> enum pipe pipe, bool enable); >>>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c >>>> index 4114cb3f14e7..fae7fe7802f8 100644 >>>> --- a/drivers/gpu/drm/i915/intel_panel.c >>>> +++ b/drivers/gpu/drm/i915/intel_panel.c >>>> @@ -108,9 +108,14 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc, >>>> const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; >>>> int x = 0, y = 0, width = 0, height = 0; >>>> >>>> - /* Native modes don't need fitting */ >>>> + /* >>>> + * Native modes don't need fitting >>>> + * PF-ID Interlace mode in SKL+ require a pipe scaler >>>> + */ >>>> if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w && >>>> - adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) >>>> + adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h && >>>> + !(is_skl_interlace_mode(to_i915(intel_crtc->base.dev), >>>> + adjusted_mode))) >>>> goto done; >>>> >>>> switch (fitting_mode) { >>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >>>> index aa9d8cef7ce0..dfb21f00fbed 100644 >>>> --- a/drivers/gpu/drm/i915/intel_pm.c >>>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>>> @@ -3873,15 +3873,18 @@ static uint_fixed_16_16_t >>>> skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) >>>> { >>>> uint_fixed_16_16_t pipe_downscale = u32_to_fixed_16_16(1); >>>> + const struct drm_display_mode *mode; >>>> >>>> if (!crtc_state->base.enable) >>>> return pipe_downscale; >>>> >>>> + mode = &crtc_state->base.adjusted_mode; >>>> if (crtc_state->pch_pfit.enabled) { >>>> uint32_t src_w, src_h, dst_w, dst_h; >>>> uint32_t pfit_size = crtc_state->pch_pfit.size; >>>> uint_fixed_16_16_t fp_w_ratio, fp_h_ratio; >>>> uint_fixed_16_16_t downscale_h, downscale_w; >>>> + uint_fixed_16_16_t min_h_downscale; >>>> >>>> src_w = crtc_state->pipe_src_w; >>>> src_h = crtc_state->pipe_src_h; >>>> @@ -3891,10 +3894,19 @@ skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) >>>> if (!dst_w || !dst_h) >>>> return pipe_downscale; >>>> >>>> + /* >>>> + * Interlace mode require 1 scaler so no other scaler can be >>>> + * attached to pipe. PF-ID mode is equivalent to 2.0 vertical >>>> + * downscale. >>>> + */ >>>> + if (mode->flags & DRM_MODE_FLAG_INTERLACE) >>>> + min_h_downscale = u32_to_fixed_16_16(2); >>>> + else >>>> + min_h_downscale = u32_to_fixed_16_16(1); >>>> 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)); >>>> + downscale_h = max_fixed_16_16(fp_h_ratio, min_h_downscale); >>>> >>>> pipe_downscale = mul_fixed16(downscale_w, downscale_h); >>>> } >>>> @@ -4418,6 +4430,9 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cstate, >>>> * with additional adjustments for plane-specific scaling. >>>> */ >>>> adjusted_pixel_rate = cstate->pixel_rate; >>>> + if (cstate->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) >>>> + adjusted_pixel_rate *= 2; >>>> + >>>> downscale_amount = skl_plane_downscale_amount(cstate, pstate); >>>> >>>> return mul_round_up_u32_fixed16(adjusted_pixel_rate, >>>> -- >>>> 2.11.0 >>>> >>>> _______________________________________________ >>>> Intel-gfx mailing list >>>> Intel-gfx@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index d791b3ef89b5..06ed2fc449d7 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -248,6 +248,13 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, return -EINVAL; } + if (num_scalers_need > 1 && + scaler_state->scaler_users & (1 << SKL_CRTC_INDEX) && + scaler_state->scaler_users & (1 << SKL_PF_ID_INDEX)) { + DRM_ERROR("Can't attach scaler to CRTC in Interlace Mode\n"); + return -EINVAL; + } + /* walkthrough scaler_users bits and start assigning scalers */ for (i = 0; i < sizeof(scaler_state->scaler_users) * 8; i++) { int *scaler_id; @@ -264,6 +271,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, /* panel fitter case: assign as a crtc scaler */ scaler_id = &scaler_state->scaler_id; + } else if (i == SKL_PF_ID_INDEX) { + name = "PF-ID INTERLACE MODE"; + idx = intel_crtc->base.base.id; + + /* PF-ID interlaced mode case: needs a pipe scaler */ + scaler_id = &scaler_state->scaler_id; } else { name = "PLANE"; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 85ac32549e85..15c79b46d645 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, */ need_scaling = src_w != dst_w || src_h != dst_h; + if ((crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) + && scaler_user == SKL_PF_ID_INDEX) + /* PF-ID Interlace mode needs a scaler */ + need_scaling = true; + /* * if plane is being disabled or scaler is no more required or force detach * - free scaler binded to this plane/crtc @@ -4635,16 +4640,24 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, * scaler can be assigned to other user. Actual register * update to free the scaler is done in plane/panel-fit programming. * For this purpose crtc/plane_state->scaler_id isn't reset here. + * Now Interlace mode is a new user of pipe-scaler, so free the scaler-id + * only if call is for respective user. If no user for scaler free it. */ if (force_detach || !need_scaling) { - if (*scaler_id >= 0) { + if (*scaler_id >= 0 && + scaler_state->scaler_users & (1 << scaler_user)) { scaler_state->scaler_users &= ~(1 << scaler_user); scaler_state->scalers[*scaler_id].in_use = 0; DRM_DEBUG_KMS("scaler_user index %u.%u: " "Staged freeing scaler id %d scaler_users = 0x%x\n", - intel_crtc->pipe, scaler_user, *scaler_id, - scaler_state->scaler_users); + intel_crtc->pipe, scaler_user, + *scaler_id, scaler_state->scaler_users); + + *scaler_id = -1; + } + if (*scaler_id >= 0 && !scaler_state->scaler_users) { + scaler_state->scalers[*scaler_id].in_use = 0; *scaler_id = -1; } return 0; @@ -4691,6 +4704,16 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state) adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay); } +static int skl_update_scaler_crtc_pf_id_output(struct intel_crtc_state *state) +{ + const struct drm_display_mode *mode = &state->base.adjusted_mode; + + return skl_update_scaler(state, !state->base.active, + SKL_PF_ID_INDEX, &state->scaler_state.scaler_id, + state->pipe_src_w, state->pipe_src_h, + mode->crtc_hdisplay, mode->crtc_vdisplay); +} + /** * skl_update_scaler_plane - Stages update to scaler state for a given plane. * @@ -6258,6 +6281,15 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, } } + if (INTEL_GEN(dev_priv) >= 9) { + if (skl_update_scaler_crtc_pf_id_output(pipe_config)) { + DRM_ERROR("Unable to set scaler for PF-ID mode\n"); + return -EINVAL; + } + intel_pch_panel_fitting(crtc, pipe_config, + DRM_MODE_SCALE_FULLSCREEN); + } + if (adjusted_mode->crtc_clock > clock_limit) { DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n", adjusted_mode->crtc_clock, clock_limit, @@ -8045,9 +8077,12 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc) if (IS_HASWELL(dev_priv) && intel_crtc->config->dither) val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP); - if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) - val |= PIPECONF_INTERLACED_ILK; - else + if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) { + if (INTEL_GEN(dev_priv) >= 9) + val |= PIPECONF_PFIT_PF_INTERLACED_ILK; + else + val |= PIPECONF_INTERLACED_ILK; + } else val |= PIPECONF_PROGRESSIVE; I915_WRITE(PIPECONF(cpu_transcoder), val); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 83dd40905821..d4546f6498b9 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -483,6 +483,7 @@ struct intel_crtc_scaler_state { * avilability. */ #define SKL_CRTC_INDEX 31 +#define SKL_PF_ID_INDEX 29 unsigned scaler_users; /* scaler used by crtc for panel fitting purpose */ @@ -1206,6 +1207,15 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi) return container_of(intel_hdmi, struct intel_digital_port, hdmi); } +static inline bool +is_skl_interlace_mode(struct drm_i915_private *dev_priv, + const struct drm_display_mode *mode) +{ + if (INTEL_GEN(dev_priv) < 9) + return false; + return mode->flags & DRM_MODE_FLAG_INTERLACE; +} + /* intel_fifo_underrun.c */ bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv, enum pipe pipe, bool enable); diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 4114cb3f14e7..fae7fe7802f8 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -108,9 +108,14 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc, const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int x = 0, y = 0, width = 0, height = 0; - /* Native modes don't need fitting */ + /* + * Native modes don't need fitting + * PF-ID Interlace mode in SKL+ require a pipe scaler + */ if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w && - adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) + adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h && + !(is_skl_interlace_mode(to_i915(intel_crtc->base.dev), + adjusted_mode))) goto done; switch (fitting_mode) { diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index aa9d8cef7ce0..dfb21f00fbed 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3873,15 +3873,18 @@ static uint_fixed_16_16_t skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) { uint_fixed_16_16_t pipe_downscale = u32_to_fixed_16_16(1); + const struct drm_display_mode *mode; if (!crtc_state->base.enable) return pipe_downscale; + mode = &crtc_state->base.adjusted_mode; if (crtc_state->pch_pfit.enabled) { uint32_t src_w, src_h, dst_w, dst_h; uint32_t pfit_size = crtc_state->pch_pfit.size; uint_fixed_16_16_t fp_w_ratio, fp_h_ratio; uint_fixed_16_16_t downscale_h, downscale_w; + uint_fixed_16_16_t min_h_downscale; src_w = crtc_state->pipe_src_w; src_h = crtc_state->pipe_src_h; @@ -3891,10 +3894,19 @@ skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state) if (!dst_w || !dst_h) return pipe_downscale; + /* + * Interlace mode require 1 scaler so no other scaler can be + * attached to pipe. PF-ID mode is equivalent to 2.0 vertical + * downscale. + */ + if (mode->flags & DRM_MODE_FLAG_INTERLACE) + min_h_downscale = u32_to_fixed_16_16(2); + else + min_h_downscale = u32_to_fixed_16_16(1); 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)); + downscale_h = max_fixed_16_16(fp_h_ratio, min_h_downscale); pipe_downscale = mul_fixed16(downscale_w, downscale_h); } @@ -4418,6 +4430,9 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cstate, * with additional adjustments for plane-specific scaling. */ adjusted_pixel_rate = cstate->pixel_rate; + if (cstate->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) + adjusted_pixel_rate *= 2; + downscale_amount = skl_plane_downscale_amount(cstate, pstate); return mul_round_up_u32_fixed16(adjusted_pixel_rate,
In previous GEN default Interlace mode enabled is IF-ID mode, but IF-ID mode has many limitations in SKL. This mode doesn't support y-tiling, 90-270 rotation is not supported & YUV-420 planar source pixel formats are not supported with above mode. This patch make changes to use PF-ID Interlace mode in SKL+ platforms. PF-ID mode require one scaler to be attached in pipe-scaling mode. During WM calculation adjusted pixel rate need to be doubled. During max_supported_pixel_format calculation vertical downscale is 2.0. Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90238 --- drivers/gpu/drm/i915/intel_atomic.c | 13 ++++++++++ drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++----- drivers/gpu/drm/i915/intel_drv.h | 10 ++++++++ drivers/gpu/drm/i915/intel_panel.c | 9 +++++-- drivers/gpu/drm/i915/intel_pm.c | 17 ++++++++++++- 5 files changed, 87 insertions(+), 9 deletions(-)