On Wed, Oct 03, 2018 at 03:37:04PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 91 ++++++++++++++--------------
> 1 file changed, 45 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 264feed76c08..701caab4e382 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -158,9 +158,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
> static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
> static void intel_crtc_init_scalers(struct intel_crtc *crtc,
> struct intel_crtc_state *crtc_state);
> -static void skylake_pfit_enable(struct intel_crtc *crtc);
> -static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
> -static void ironlake_pfit_enable(struct intel_crtc *crtc);
> +static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state);
> +static void ironlake_pfit_disable(const struct intel_crtc_state *old_crtc_state);
> +static void ironlake_pfit_enable(const struct intel_crtc_state *crtc_state);
> static void intel_modeset_setup_hw_state(struct drm_device *dev,
> struct drm_modeset_acquire_ctx *ctx);
> static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc);
> @@ -3919,12 +3919,12 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
> skl_detach_scalers(crtc);
>
> if (new_crtc_state->pch_pfit.enabled)
> - skylake_pfit_enable(crtc);
> + skylake_pfit_enable(new_crtc_state);
> } else if (HAS_PCH_SPLIT(dev_priv)) {
> if (new_crtc_state->pch_pfit.enabled)
> - ironlake_pfit_enable(crtc);
> + ironlake_pfit_enable(new_crtc_state);
> else if (old_crtc_state->pch_pfit.enabled)
> - ironlake_pfit_disable(crtc, true);
> + ironlake_pfit_disable(old_crtc_state);
> }
> }
>
> @@ -5041,19 +5041,19 @@ static void skylake_scaler_disable(struct intel_crtc *crtc)
> skl_detach_scaler(crtc, i);
> }
>
> -static void skylake_pfit_enable(struct intel_crtc *crtc)
> +static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
> {
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - int pipe = crtc->pipe;
> - struct intel_crtc_scaler_state *scaler_state =
> - &crtc->config->scaler_state;
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
> + const struct intel_crtc_scaler_state *scaler_state =
> + &crtc_state->scaler_state;
>
> - if (crtc->config->pch_pfit.enabled) {
> + if (crtc_state->pch_pfit.enabled) {
> u16 uv_rgb_hphase, uv_rgb_vphase;
> int id;
>
> - if (WARN_ON(crtc->config->scaler_state.scaler_id < 0))
> + if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> return;
>
> uv_rgb_hphase = skl_scaler_calc_phase(1, false);
> @@ -5066,18 +5066,18 @@ static void skylake_pfit_enable(struct intel_crtc *crtc)
> PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
> I915_WRITE_FW(SKL_PS_HPHASE(pipe, id),
> PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> - I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc->config->pch_pfit.pos);
> - I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc->config->pch_pfit.size);
> + I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc_state->pch_pfit.pos);
> + I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc_state->pch_pfit.size);
> }
> }
>
> -static void ironlake_pfit_enable(struct intel_crtc *crtc)
> +static void ironlake_pfit_enable(const struct intel_crtc_state *crtc_state)
> {
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> int pipe = crtc->pipe;
>
> - if (crtc->config->pch_pfit.enabled) {
> + if (crtc_state->pch_pfit.enabled) {
> /* Force use of hard-coded filter coefficients
> * as some pre-programmed values are broken,
> * e.g. x201.
> @@ -5087,8 +5087,8 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
> PF_PIPE_SEL_IVB(pipe));
> else
> I915_WRITE(PF_CTL(pipe), PF_ENABLE | PF_FILTER_MED_3x3);
> - I915_WRITE(PF_WIN_POS(pipe), crtc->config->pch_pfit.pos);
> - I915_WRITE(PF_WIN_SZ(pipe), crtc->config->pch_pfit.size);
> + I915_WRITE(PF_WIN_POS(pipe), crtc_state->pch_pfit.pos);
> + I915_WRITE(PF_WIN_SZ(pipe), crtc_state->pch_pfit.size);
> }
> }
>
> @@ -5620,7 +5620,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> assert_fdi_rx_disabled(dev_priv, pipe);
> }
>
> - ironlake_pfit_enable(intel_crtc);
> + ironlake_pfit_enable(pipe_config);
>
> /*
> * On ILK+ LUT must be loaded before the pipe is running but with
> @@ -5752,9 +5752,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
>
> if (INTEL_GEN(dev_priv) >= 9)
> - skylake_pfit_enable(intel_crtc);
> + skylake_pfit_enable(pipe_config);
> else
> - ironlake_pfit_enable(intel_crtc);
> + ironlake_pfit_enable(pipe_config);
>
> /*
> * On ILK+ LUT must be loaded before the pipe is running but with
> @@ -5812,15 +5812,15 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> }
> }
>
> -static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
> +static void ironlake_pfit_disable(const struct intel_crtc_state *old_crtc_state)
> {
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - int pipe = crtc->pipe;
> + struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
>
> /* To avoid upsetting the power well on haswell only disable the pfit if
> * it's in use. The hw state code will make sure we get this right. */
> - if (force || crtc->config->pch_pfit.enabled) {
> + if (old_crtc_state->pch_pfit.enabled) {
I think the removal of 'force' deserves a mention in the commit message
(which was missing entirely I guess?). I do agree that 'force' is not
needed.
So with commit message added
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
I have a feeling many of these lack a commit message. It's probably
going to become repititive considering the topic, but I think we still
want something there to justify the changes (probably could just
copy paste the same thing to each patch).
> I915_WRITE(PF_CTL(pipe), 0);
> I915_WRITE(PF_WIN_POS(pipe), 0);
> I915_WRITE(PF_WIN_SZ(pipe), 0);
> @@ -5851,7 +5851,7 @@ static void ironlake_crtc_disable(struct intel_crtc_state *old_crtc_state,
>
> intel_disable_pipe(old_crtc_state);
>
> - ironlake_pfit_disable(intel_crtc, false);
> + ironlake_pfit_disable(old_crtc_state);
>
> if (intel_crtc->config->has_pch_encoder)
> ironlake_fdi_disable(crtc);
> @@ -5912,7 +5912,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
> if (INTEL_GEN(dev_priv) >= 9)
> skylake_scaler_disable(intel_crtc);
> else
> - ironlake_pfit_disable(intel_crtc, false);
> + ironlake_pfit_disable(old_crtc_state);
>
> intel_encoders_post_disable(crtc, old_crtc_state, old_state);
>
> @@ -5920,13 +5920,12 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
> icl_unmap_plls_to_ports(crtc, old_crtc_state, old_state);
> }
>
> -static void i9xx_pfit_enable(struct intel_crtc *crtc)
> +static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
> {
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_crtc_state *pipe_config = crtc->config;
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>
> - if (!pipe_config->gmch_pfit.control)
> + if (!crtc_state->gmch_pfit.control)
> return;
>
> /*
> @@ -5936,8 +5935,8 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
> WARN_ON(I915_READ(PFIT_CONTROL) & PFIT_ENABLE);
> assert_pipe_disabled(dev_priv, crtc->pipe);
>
> - I915_WRITE(PFIT_PGM_RATIOS, pipe_config->gmch_pfit.pgm_ratios);
> - I915_WRITE(PFIT_CONTROL, pipe_config->gmch_pfit.control);
> + I915_WRITE(PFIT_PGM_RATIOS, crtc_state->gmch_pfit.pgm_ratios);
> + I915_WRITE(PFIT_CONTROL, crtc_state->gmch_pfit.control);
>
> /* Border color in case we don't scale up to the full screen. Black by
> * default, change to something else for debugging. */
> @@ -6093,7 +6092,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>
> intel_encoders_pre_enable(crtc, pipe_config, old_state);
>
> - i9xx_pfit_enable(intel_crtc);
> + i9xx_pfit_enable(pipe_config);
>
> intel_color_load_luts(&pipe_config->base);
>
> @@ -6149,7 +6148,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>
> i9xx_enable_pll(intel_crtc, pipe_config);
>
> - i9xx_pfit_enable(intel_crtc);
> + i9xx_pfit_enable(pipe_config);
>
> intel_color_load_luts(&pipe_config->base);
>
> @@ -6166,12 +6165,12 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
> intel_encoders_enable(crtc, pipe_config, old_state);
> }
>
> -static void i9xx_pfit_disable(struct intel_crtc *crtc)
> +static void i9xx_pfit_disable(const struct intel_crtc_state *old_crtc_state)
> {
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>
> - if (!crtc->config->gmch_pfit.control)
> + if (!old_crtc_state->gmch_pfit.control)
> return;
>
> assert_pipe_disabled(dev_priv, crtc->pipe);
> @@ -6204,7 +6203,7 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
>
> intel_disable_pipe(old_crtc_state);
>
> - i9xx_pfit_disable(intel_crtc);
> + i9xx_pfit_disable(old_crtc_state);
>
> intel_encoders_post_disable(crtc, old_crtc_state, old_state);
>
> --
> 2.19.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
