| Message ID | 20190926094725.5231-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Remove begin/finish_crtc_commit, v3. | expand |
On Thu, Sep 26, 2019 at 11:47:25AM +0200, Maarten Lankhorst wrote: > This can all be done from the intel_update_crtc function. Split out the > pipe update into a separate function, just like is done for the planes. > Pull in all the changes done during fastset as well. It makes no sense > for it to still exist as a separate function. > > Changes since v1: > - Inline intel_update_pipe_config() > Changes since v2: > - Add comments suggested by matt. > - Reorder commit_pipe_config() to remove all nesting. (Ville, Matt) > - Use intel_set_pipe_src_size((). (Matt) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 206 +++++++++---------- > 1 file changed, 95 insertions(+), 111 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index b77574cda648..239e0ca3e3ec 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -136,8 +136,6 @@ static void vlv_prepare_pll(struct intel_crtc *crtc, > const struct intel_crtc_state *pipe_config); > static void chv_prepare_pll(struct intel_crtc *crtc, > const struct intel_crtc_state *pipe_config); > -static void intel_begin_crtc_commit(struct intel_atomic_state *, struct intel_crtc *); > -static void intel_finish_crtc_commit(struct intel_atomic_state *, struct intel_crtc *); > static void intel_crtc_init_scalers(struct intel_crtc *crtc, > struct intel_crtc_state *crtc_state); > static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state); > @@ -4409,45 +4407,6 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) > I915_WRITE(PIPE_CHICKEN(pipe), tmp); > } > > -static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, > - const struct intel_crtc_state *new_crtc_state) > -{ > - struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc); > - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > - > - /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > - crtc->base.mode = new_crtc_state->uapi.mode; > - > - /* > - * Update pipe size and adjust fitter if needed: the reason for this is > - * that in compute_mode_changes we check the native mode (not the pfit > - * mode) to see if we can flip rather than do a full mode set. In the > - * fastboot case, we'll flip, but if we don't update the pipesrc and > - * pfit state, we'll end up with a big fb scanned out into the wrong > - * sized surface. > - */ > - > - I915_WRITE(PIPESRC(crtc->pipe), > - ((new_crtc_state->pipe_src_w - 1) << 16) | > - (new_crtc_state->pipe_src_h - 1)); > - > - /* on skylake this is done by detaching scalers */ > - if (INTEL_GEN(dev_priv) >= 9) { > - skl_detach_scalers(new_crtc_state); > - > - if (new_crtc_state->pch_pfit.enabled) > - skylake_pfit_enable(new_crtc_state); > - } else if (HAS_PCH_SPLIT(dev_priv)) { > - if (new_crtc_state->pch_pfit.enabled) > - ironlake_pfit_enable(new_crtc_state); > - else if (old_crtc_state->pch_pfit.enabled) > - ironlake_pfit_disable(old_crtc_state); > - } > - > - if (INTEL_GEN(dev_priv) >= 11) > - icl_set_pipe_chicken(crtc); > -} > - > static void intel_fdi_normal_train(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > @@ -13739,13 +13698,91 @@ u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) > return crtc->base.funcs->get_vblank_counter(&crtc->base); > } > > +void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, > + struct intel_crtc_state *crtc_state) > +{ > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + > + if (!IS_GEN(dev_priv, 2)) > + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); > + > + if (crtc_state->has_pch_encoder) { > + enum pipe pch_transcoder = > + intel_crtc_pch_transcoder(crtc); > + > + intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder, true); > + } > +} > + > +static void commit_pipe_config(struct intel_atomic_state *state, > + struct intel_crtc_state *old_crtc_state, > + struct intel_crtc_state *new_crtc_state) > +{ > + struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc); > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > + bool modeset = needs_modeset(new_crtc_state); > + > + if (dev_priv->display.atomic_update_watermarks) > + dev_priv->display.atomic_update_watermarks(state, > + new_crtc_state); Moving this here widens the gap between programming the FIFO split and programming the planes on VLV/CHV. I'd prefer to keep this at the end to keep it as close to the plane programming as possible. > + > + /* > + * During modesets pipe configuration was programmed as the > + * CRTC was enabled. > + */ > + if (modeset) > + return; > + > + if (INTEL_GEN(dev_priv) >= 9) > + skl_detach_scalers(new_crtc_state); Similarly I'd move this downwards past the next two things to get it a bit closer to the pfit/plane programming. > + > + if (new_crtc_state->uapi.color_mgmt_changed || > + new_crtc_state->update_pipe) > + intel_color_commit(new_crtc_state); > + > + if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) > + bdw_set_pipemisc(new_crtc_state); > + > + if (!new_crtc_state->update_pipe) > + return; > + > + /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > + crtc->base.mode = new_crtc_state->uapi.mode; > + > + /* > + * Update pipe size and adjust fitter if needed: the reason for this is > + * that in compute_mode_changes we check the native mode (not the pfit > + * mode) to see if we can flip rather than do a full mode set. In the > + * fastboot case, we'll flip, but if we don't update the pipesrc and > + * pfit state, we'll end up with a big fb scanned out into the wrong > + * sized surface. > + */ > + intel_set_pipe_src_size(new_crtc_state); > + > + if (INTEL_GEN(dev_priv) >= 9) { > + /* > + * enable pipe scaler here if needed, if > + * it's disabled, that's done by > + * skl_detach_scalers() above. > + */ > + skylake_pfit_enable(new_crtc_state); > + } else if (HAS_PCH_SPLIT(dev_priv)) { > + if (!new_crtc_state->pch_pfit.enabled) > + ironlake_pfit_disable(old_crtc_state); > + else > + ironlake_pfit_enable(new_crtc_state); > + } > + > + if (INTEL_GEN(dev_priv) >= 11) > + icl_set_pipe_chicken(crtc); > +} > + > static void intel_update_crtc(struct intel_crtc *crtc, > struct intel_atomic_state *state, > struct intel_crtc_state *old_crtc_state, > struct intel_crtc_state *new_crtc_state) > { > - struct drm_device *dev = state->base.dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > + struct drm_i915_private *dev_priv = to_i915(state->base.dev); > bool modeset = needs_modeset(new_crtc_state); > struct intel_plane_state *new_plane_state = > intel_atomic_get_new_plane_state(state, > @@ -13769,14 +13806,27 @@ static void intel_update_crtc(struct intel_crtc *crtc, > else if (new_plane_state) > intel_fbc_enable(crtc, new_crtc_state, new_plane_state); > > - intel_begin_crtc_commit(state, crtc); > + /* Perform vblank evasion around commit operation */ > + intel_pipe_update_start(new_crtc_state); > + > + commit_pipe_config(state, old_crtc_state, new_crtc_state); > > if (INTEL_GEN(dev_priv) >= 9) > skl_update_planes_on_crtc(state, crtc); > else > i9xx_update_planes_on_crtc(state, crtc); > > - intel_finish_crtc_commit(state, crtc); > + intel_pipe_update_end(new_crtc_state); > + > + /* > + * We usually enable FIFO underrun interrupts as part of the > + * CRTC enable sequence during modesets. But when we inherit a > + * valid pipe configuration from the BIOS we need to take care > + * of enabling them on the CRTC's first fastset. > + */ > + if (new_crtc_state->update_pipe && !modeset && > + old_crtc_state->hw.mode.private_flags & I915_MODE_FLAG_INHERITED) > + intel_crtc_arm_fifo_underrun(crtc, new_crtc_state); > } > > static void intel_old_crtc_state_disables(struct intel_atomic_state *state, > @@ -14573,72 +14623,6 @@ skl_max_scale(const struct intel_crtc_state *crtc_state, > return max_scale; > } > > -static void intel_begin_crtc_commit(struct intel_atomic_state *state, > - struct intel_crtc *crtc) > -{ > - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > - struct intel_crtc_state *old_crtc_state = > - intel_atomic_get_old_crtc_state(state, crtc); > - struct intel_crtc_state *new_crtc_state = > - intel_atomic_get_new_crtc_state(state, crtc); > - bool modeset = needs_modeset(new_crtc_state); > - > - /* Perform vblank evasion around commit operation */ > - intel_pipe_update_start(new_crtc_state); > - > - if (modeset) > - goto out; > - > - if (new_crtc_state->uapi.color_mgmt_changed || > - new_crtc_state->update_pipe) > - intel_color_commit(new_crtc_state); > - > - if (new_crtc_state->update_pipe) > - intel_update_pipe_config(old_crtc_state, new_crtc_state); > - else if (INTEL_GEN(dev_priv) >= 9) > - skl_detach_scalers(new_crtc_state); > - > - if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) > - bdw_set_pipemisc(new_crtc_state); > - > -out: > - if (dev_priv->display.atomic_update_watermarks) > - dev_priv->display.atomic_update_watermarks(state, > - new_crtc_state); > -} > - > -void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, > - struct intel_crtc_state *crtc_state) > -{ > - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > - > - if (!IS_GEN(dev_priv, 2)) > - intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); > - > - if (crtc_state->has_pch_encoder) { > - enum pipe pch_transcoder = > - intel_crtc_pch_transcoder(crtc); > - > - intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder, true); > - } > -} > - > -static void intel_finish_crtc_commit(struct intel_atomic_state *state, > - struct intel_crtc *crtc) > -{ > - struct intel_crtc_state *old_crtc_state = > - intel_atomic_get_old_crtc_state(state, crtc); > - struct intel_crtc_state *new_crtc_state = > - intel_atomic_get_new_crtc_state(state, crtc); > - > - intel_pipe_update_end(new_crtc_state); > - > - if (new_crtc_state->update_pipe && > - !needs_modeset(new_crtc_state) && > - old_crtc_state->hw.mode.private_flags & I915_MODE_FLAG_INHERITED) > - intel_crtc_arm_fifo_underrun(crtc, new_crtc_state); > -} > - > /** > * intel_plane_destroy - destroy a plane > * @plane: plane to destroy > -- > 2.20.1
Op 26-09-2019 om 18:22 schreef Ville Syrjälä: > On Thu, Sep 26, 2019 at 11:47:25AM +0200, Maarten Lankhorst wrote: >> This can all be done from the intel_update_crtc function. Split out the >> pipe update into a separate function, just like is done for the planes. >> Pull in all the changes done during fastset as well. It makes no sense >> for it to still exist as a separate function. >> >> Changes since v1: >> - Inline intel_update_pipe_config() >> Changes since v2: >> - Add comments suggested by matt. >> - Reorder commit_pipe_config() to remove all nesting. (Ville, Matt) >> - Use intel_set_pipe_src_size((). (Matt) >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 206 +++++++++---------- >> 1 file changed, 95 insertions(+), 111 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index b77574cda648..239e0ca3e3ec 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -136,8 +136,6 @@ static void vlv_prepare_pll(struct intel_crtc *crtc, >> const struct intel_crtc_state *pipe_config); >> static void chv_prepare_pll(struct intel_crtc *crtc, >> const struct intel_crtc_state *pipe_config); >> -static void intel_begin_crtc_commit(struct intel_atomic_state *, struct intel_crtc *); >> -static void intel_finish_crtc_commit(struct intel_atomic_state *, struct intel_crtc *); >> static void intel_crtc_init_scalers(struct intel_crtc *crtc, >> struct intel_crtc_state *crtc_state); >> static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state); >> @@ -4409,45 +4407,6 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) >> I915_WRITE(PIPE_CHICKEN(pipe), tmp); >> } >> >> -static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, >> - const struct intel_crtc_state *new_crtc_state) >> -{ >> - struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc); >> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> - >> - /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ >> - crtc->base.mode = new_crtc_state->uapi.mode; >> - >> - /* >> - * Update pipe size and adjust fitter if needed: the reason for this is >> - * that in compute_mode_changes we check the native mode (not the pfit >> - * mode) to see if we can flip rather than do a full mode set. In the >> - * fastboot case, we'll flip, but if we don't update the pipesrc and >> - * pfit state, we'll end up with a big fb scanned out into the wrong >> - * sized surface. >> - */ >> - >> - I915_WRITE(PIPESRC(crtc->pipe), >> - ((new_crtc_state->pipe_src_w - 1) << 16) | >> - (new_crtc_state->pipe_src_h - 1)); >> - >> - /* on skylake this is done by detaching scalers */ >> - if (INTEL_GEN(dev_priv) >= 9) { >> - skl_detach_scalers(new_crtc_state); >> - >> - if (new_crtc_state->pch_pfit.enabled) >> - skylake_pfit_enable(new_crtc_state); >> - } else if (HAS_PCH_SPLIT(dev_priv)) { >> - if (new_crtc_state->pch_pfit.enabled) >> - ironlake_pfit_enable(new_crtc_state); >> - else if (old_crtc_state->pch_pfit.enabled) >> - ironlake_pfit_disable(old_crtc_state); >> - } >> - >> - if (INTEL_GEN(dev_priv) >= 11) >> - icl_set_pipe_chicken(crtc); >> -} >> - >> static void intel_fdi_normal_train(struct intel_crtc *crtc) >> { >> struct drm_device *dev = crtc->base.dev; >> @@ -13739,13 +13698,91 @@ u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) >> return crtc->base.funcs->get_vblank_counter(&crtc->base); >> } >> >> +void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, >> + struct intel_crtc_state *crtc_state) >> +{ >> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> + >> + if (!IS_GEN(dev_priv, 2)) >> + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); >> + >> + if (crtc_state->has_pch_encoder) { >> + enum pipe pch_transcoder = >> + intel_crtc_pch_transcoder(crtc); >> + >> + intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder, true); >> + } >> +} >> + >> +static void commit_pipe_config(struct intel_atomic_state *state, >> + struct intel_crtc_state *old_crtc_state, >> + struct intel_crtc_state *new_crtc_state) >> +{ >> + struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc); >> + struct drm_i915_private *dev_priv = to_i915(state->base.dev); >> + bool modeset = needs_modeset(new_crtc_state); >> + >> + if (dev_priv->display.atomic_update_watermarks) >> + dev_priv->display.atomic_update_watermarks(state, >> + new_crtc_state); > Moving this here widens the gap between programming the FIFO split and > programming the planes on VLV/CHV. I'd prefer to keep this at the end > to keep it as close to the plane programming as possible. It wouldn't, only on fastset and color updates. >> + >> + /* >> + * During modesets pipe configuration was programmed as the >> + * CRTC was enabled. >> + */ >> + if (modeset) >> + return; >> + >> + if (INTEL_GEN(dev_priv) >= 9) >> + skl_detach_scalers(new_crtc_state); > Similarly I'd move this downwards past the next two things to get > it a bit closer to the pfit/plane programming. Sigh! >> + >> + if (new_crtc_state->uapi.color_mgmt_changed || >> + new_crtc_state->update_pipe) >> + intel_color_commit(new_crtc_state); >> + >> + if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) >> + bdw_set_pipemisc(new_crtc_state); >> + >> + if (!new_crtc_state->update_pipe) >> + return; >> + >> + /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ >> + crtc->base.mode = new_crtc_state->uapi.mode; >> + >> + /* >> + * Update pipe size and adjust fitter if needed: the reason for this is >> + * that in compute_mode_changes we check the native mode (not the pfit >> + * mode) to see if we can flip rather than do a full mode set. In the >> + * fastboot case, we'll flip, but if we don't update the pipesrc and >> + * pfit state, we'll end up with a big fb scanned out into the wrong >> + * sized surface. >> + */ >> + intel_set_pipe_src_size(new_crtc_state); >> + >> + if (INTEL_GEN(dev_priv) >= 9) { >> + /* >> + * enable pipe scaler here if needed, if >> + * it's disabled, that's done by >> + * skl_detach_scalers() above. >> + */ >> + skylake_pfit_enable(new_crtc_state); >> + } else if (HAS_PCH_SPLIT(dev_priv)) { >> + if (!new_crtc_state->pch_pfit.enabled) >> + ironlake_pfit_disable(old_crtc_state); >> + else >> + ironlake_pfit_enable(new_crtc_state); >> + } >> + >> + if (INTEL_GEN(dev_priv) >= 11) >> + icl_set_pipe_chicken(crtc); >> +} >> + >> static void intel_update_crtc(struct intel_crtc *crtc, >> struct intel_atomic_state *state, >> struct intel_crtc_state *old_crtc_state, >> struct intel_crtc_state *new_crtc_state) >> { >> - struct drm_device *dev = state->base.dev; >> - struct drm_i915_private *dev_priv = to_i915(dev); >> + struct drm_i915_private *dev_priv = to_i915(state->base.dev); >> bool modeset = needs_modeset(new_crtc_state); >> struct intel_plane_state *new_plane_state = >> intel_atomic_get_new_plane_state(state, >> @@ -13769,14 +13806,27 @@ static void intel_update_crtc(struct intel_crtc *crtc, >> else if (new_plane_state) >> intel_fbc_enable(crtc, new_crtc_state, new_plane_state); >> >> - intel_begin_crtc_commit(state, crtc); >> + /* Perform vblank evasion around commit operation */ >> + intel_pipe_update_start(new_crtc_state); >> + >> + commit_pipe_config(state, old_crtc_state, new_crtc_state); >> >> if (INTEL_GEN(dev_priv) >= 9) >> skl_update_planes_on_crtc(state, crtc); >> else >> i9xx_update_planes_on_crtc(state, crtc); >> >> - intel_finish_crtc_commit(state, crtc); >> + intel_pipe_update_end(new_crtc_state); >> + >> + /* >> + * We usually enable FIFO underrun interrupts as part of the >> + * CRTC enable sequence during modesets. But when we inherit a >> + * valid pipe configuration from the BIOS we need to take care >> + * of enabling them on the CRTC's first fastset. >> + */ >> + if (new_crtc_state->update_pipe && !modeset && >> + old_crtc_state->hw.mode.private_flags & I915_MODE_FLAG_INHERITED) >> + intel_crtc_arm_fifo_underrun(crtc, new_crtc_state); >> } >> >> static void intel_old_crtc_state_disables(struct intel_atomic_state *state, >> @@ -14573,72 +14623,6 @@ skl_max_scale(const struct intel_crtc_state *crtc_state, >> return max_scale; >> } >> >> -static void intel_begin_crtc_commit(struct intel_atomic_state *state, >> - struct intel_crtc *crtc) >> -{ >> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> - struct intel_crtc_state *old_crtc_state = >> - intel_atomic_get_old_crtc_state(state, crtc); >> - struct intel_crtc_state *new_crtc_state = >> - intel_atomic_get_new_crtc_state(state, crtc); >> - bool modeset = needs_modeset(new_crtc_state); >> - >> - /* Perform vblank evasion around commit operation */ >> - intel_pipe_update_start(new_crtc_state); >> - >> - if (modeset) >> - goto out; >> - >> - if (new_crtc_state->uapi.color_mgmt_changed || >> - new_crtc_state->update_pipe) >> - intel_color_commit(new_crtc_state); >> - >> - if (new_crtc_state->update_pipe) >> - intel_update_pipe_config(old_crtc_state, new_crtc_state); >> - else if (INTEL_GEN(dev_priv) >= 9) >> - skl_detach_scalers(new_crtc_state); >> - >> - if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) >> - bdw_set_pipemisc(new_crtc_state); >> - >> -out: >> - if (dev_priv->display.atomic_update_watermarks) >> - dev_priv->display.atomic_update_watermarks(state, >> - new_crtc_state); >> -} >> - >> -void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, >> - struct intel_crtc_state *crtc_state) >> -{ >> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> - >> - if (!IS_GEN(dev_priv, 2)) >> - intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); >> - >> - if (crtc_state->has_pch_encoder) { >> - enum pipe pch_transcoder = >> - intel_crtc_pch_transcoder(crtc); >> - >> - intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder, true); >> - } >> -} >> - >> -static void intel_finish_crtc_commit(struct intel_atomic_state *state, >> - struct intel_crtc *crtc) >> -{ >> - struct intel_crtc_state *old_crtc_state = >> - intel_atomic_get_old_crtc_state(state, crtc); >> - struct intel_crtc_state *new_crtc_state = >> - intel_atomic_get_new_crtc_state(state, crtc); >> - >> - intel_pipe_update_end(new_crtc_state); >> - >> - if (new_crtc_state->update_pipe && >> - !needs_modeset(new_crtc_state) && >> - old_crtc_state->hw.mode.private_flags & I915_MODE_FLAG_INHERITED) >> - intel_crtc_arm_fifo_underrun(crtc, new_crtc_state); >> -} >> - >> /** >> * intel_plane_destroy - destroy a plane >> * @plane: plane to destroy >> -- >> 2.20.1
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b77574cda648..239e0ca3e3ec 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -136,8 +136,6 @@ static void vlv_prepare_pll(struct intel_crtc *crtc, const struct intel_crtc_state *pipe_config); static void chv_prepare_pll(struct intel_crtc *crtc, const struct intel_crtc_state *pipe_config); -static void intel_begin_crtc_commit(struct intel_atomic_state *, struct intel_crtc *); -static void intel_finish_crtc_commit(struct intel_atomic_state *, struct intel_crtc *); static void intel_crtc_init_scalers(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state); static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state); @@ -4409,45 +4407,6 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) I915_WRITE(PIPE_CHICKEN(pipe), tmp); } -static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, - const struct intel_crtc_state *new_crtc_state) -{ - struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc); - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - - /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ - crtc->base.mode = new_crtc_state->uapi.mode; - - /* - * Update pipe size and adjust fitter if needed: the reason for this is - * that in compute_mode_changes we check the native mode (not the pfit - * mode) to see if we can flip rather than do a full mode set. In the - * fastboot case, we'll flip, but if we don't update the pipesrc and - * pfit state, we'll end up with a big fb scanned out into the wrong - * sized surface. - */ - - I915_WRITE(PIPESRC(crtc->pipe), - ((new_crtc_state->pipe_src_w - 1) << 16) | - (new_crtc_state->pipe_src_h - 1)); - - /* on skylake this is done by detaching scalers */ - if (INTEL_GEN(dev_priv) >= 9) { - skl_detach_scalers(new_crtc_state); - - if (new_crtc_state->pch_pfit.enabled) - skylake_pfit_enable(new_crtc_state); - } else if (HAS_PCH_SPLIT(dev_priv)) { - if (new_crtc_state->pch_pfit.enabled) - ironlake_pfit_enable(new_crtc_state); - else if (old_crtc_state->pch_pfit.enabled) - ironlake_pfit_disable(old_crtc_state); - } - - if (INTEL_GEN(dev_priv) >= 11) - icl_set_pipe_chicken(crtc); -} - static void intel_fdi_normal_train(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; @@ -13739,13 +13698,91 @@ u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) return crtc->base.funcs->get_vblank_counter(&crtc->base); } +void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, + struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + + if (!IS_GEN(dev_priv, 2)) + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); + + if (crtc_state->has_pch_encoder) { + enum pipe pch_transcoder = + intel_crtc_pch_transcoder(crtc); + + intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder, true); + } +} + +static void commit_pipe_config(struct intel_atomic_state *state, + struct intel_crtc_state *old_crtc_state, + struct intel_crtc_state *new_crtc_state) +{ + struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc); + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + bool modeset = needs_modeset(new_crtc_state); + + if (dev_priv->display.atomic_update_watermarks) + dev_priv->display.atomic_update_watermarks(state, + new_crtc_state); + + /* + * During modesets pipe configuration was programmed as the + * CRTC was enabled. + */ + if (modeset) + return; + + if (INTEL_GEN(dev_priv) >= 9) + skl_detach_scalers(new_crtc_state); + + if (new_crtc_state->uapi.color_mgmt_changed || + new_crtc_state->update_pipe) + intel_color_commit(new_crtc_state); + + if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) + bdw_set_pipemisc(new_crtc_state); + + if (!new_crtc_state->update_pipe) + return; + + /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ + crtc->base.mode = new_crtc_state->uapi.mode; + + /* + * Update pipe size and adjust fitter if needed: the reason for this is + * that in compute_mode_changes we check the native mode (not the pfit + * mode) to see if we can flip rather than do a full mode set. In the + * fastboot case, we'll flip, but if we don't update the pipesrc and + * pfit state, we'll end up with a big fb scanned out into the wrong + * sized surface. + */ + intel_set_pipe_src_size(new_crtc_state); + + if (INTEL_GEN(dev_priv) >= 9) { + /* + * enable pipe scaler here if needed, if + * it's disabled, that's done by + * skl_detach_scalers() above. + */ + skylake_pfit_enable(new_crtc_state); + } else if (HAS_PCH_SPLIT(dev_priv)) { + if (!new_crtc_state->pch_pfit.enabled) + ironlake_pfit_disable(old_crtc_state); + else + ironlake_pfit_enable(new_crtc_state); + } + + if (INTEL_GEN(dev_priv) >= 11) + icl_set_pipe_chicken(crtc); +} + static void intel_update_crtc(struct intel_crtc *crtc, struct intel_atomic_state *state, struct intel_crtc_state *old_crtc_state, struct intel_crtc_state *new_crtc_state) { - struct drm_device *dev = state->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = to_i915(state->base.dev); bool modeset = needs_modeset(new_crtc_state); struct intel_plane_state *new_plane_state = intel_atomic_get_new_plane_state(state, @@ -13769,14 +13806,27 @@ static void intel_update_crtc(struct intel_crtc *crtc, else if (new_plane_state) intel_fbc_enable(crtc, new_crtc_state, new_plane_state); - intel_begin_crtc_commit(state, crtc); + /* Perform vblank evasion around commit operation */ + intel_pipe_update_start(new_crtc_state); + + commit_pipe_config(state, old_crtc_state, new_crtc_state); if (INTEL_GEN(dev_priv) >= 9) skl_update_planes_on_crtc(state, crtc); else i9xx_update_planes_on_crtc(state, crtc); - intel_finish_crtc_commit(state, crtc); + intel_pipe_update_end(new_crtc_state); + + /* + * We usually enable FIFO underrun interrupts as part of the + * CRTC enable sequence during modesets. But when we inherit a + * valid pipe configuration from the BIOS we need to take care + * of enabling them on the CRTC's first fastset. + */ + if (new_crtc_state->update_pipe && !modeset && + old_crtc_state->hw.mode.private_flags & I915_MODE_FLAG_INHERITED) + intel_crtc_arm_fifo_underrun(crtc, new_crtc_state); } static void intel_old_crtc_state_disables(struct intel_atomic_state *state, @@ -14573,72 +14623,6 @@ skl_max_scale(const struct intel_crtc_state *crtc_state, return max_scale; } -static void intel_begin_crtc_commit(struct intel_atomic_state *state, - struct intel_crtc *crtc) -{ - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - struct intel_crtc_state *old_crtc_state = - intel_atomic_get_old_crtc_state(state, crtc); - struct intel_crtc_state *new_crtc_state = - intel_atomic_get_new_crtc_state(state, crtc); - bool modeset = needs_modeset(new_crtc_state); - - /* Perform vblank evasion around commit operation */ - intel_pipe_update_start(new_crtc_state); - - if (modeset) - goto out; - - if (new_crtc_state->uapi.color_mgmt_changed || - new_crtc_state->update_pipe) - intel_color_commit(new_crtc_state); - - if (new_crtc_state->update_pipe) - intel_update_pipe_config(old_crtc_state, new_crtc_state); - else if (INTEL_GEN(dev_priv) >= 9) - skl_detach_scalers(new_crtc_state); - - if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) - bdw_set_pipemisc(new_crtc_state); - -out: - if (dev_priv->display.atomic_update_watermarks) - dev_priv->display.atomic_update_watermarks(state, - new_crtc_state); -} - -void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, - struct intel_crtc_state *crtc_state) -{ - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - - if (!IS_GEN(dev_priv, 2)) - intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); - - if (crtc_state->has_pch_encoder) { - enum pipe pch_transcoder = - intel_crtc_pch_transcoder(crtc); - - intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder, true); - } -} - -static void intel_finish_crtc_commit(struct intel_atomic_state *state, - struct intel_crtc *crtc) -{ - struct intel_crtc_state *old_crtc_state = - intel_atomic_get_old_crtc_state(state, crtc); - struct intel_crtc_state *new_crtc_state = - intel_atomic_get_new_crtc_state(state, crtc); - - intel_pipe_update_end(new_crtc_state); - - if (new_crtc_state->update_pipe && - !needs_modeset(new_crtc_state) && - old_crtc_state->hw.mode.private_flags & I915_MODE_FLAG_INHERITED) - intel_crtc_arm_fifo_underrun(crtc, new_crtc_state); -} - /** * intel_plane_destroy - destroy a plane * @plane: plane to destroy
This can all be done from the intel_update_crtc function. Split out the pipe update into a separate function, just like is done for the planes. Pull in all the changes done during fastset as well. It makes no sense for it to still exist as a separate function. Changes since v1: - Inline intel_update_pipe_config() Changes since v2: - Add comments suggested by matt. - Reorder commit_pipe_config() to remove all nesting. (Ville, Matt) - Use intel_set_pipe_src_size((). (Matt) Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 206 +++++++++---------- 1 file changed, 95 insertions(+), 111 deletions(-)