On Wed, Feb 12, 2020 at 06:17:34PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Most of the pfit functions are of the form:
>
> func()
> {
> if (pfit_enabled) {
> ...
> }
> }
>
> Flip the pfit_enabled check around to flatten the functions.
>
> And while we're touching all this let's do the usual
> s/pipe_config/crtc_state/ replacement.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 233 +++++++++----------
> 1 file changed, 115 insertions(+), 118 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index becc6322b7dc..796e27c4aece 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6233,42 +6233,42 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
> enum pipe pipe = crtc->pipe;
> const struct intel_crtc_scaler_state *scaler_state =
> &crtc_state->scaler_state;
> + u16 uv_rgb_hphase, uv_rgb_vphase;
> + int pfit_w, pfit_h, hscale, vscale;
> + unsigned long irqflags;
> + int id;
>
> - if (crtc_state->pch_pfit.enabled) {
> - u16 uv_rgb_hphase, uv_rgb_vphase;
> - int pfit_w, pfit_h, hscale, vscale;
> - unsigned long irqflags;
> - int id;
> + if (!crtc_state->pch_pfit.enabled)
> + return;
>
> - if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> - return;
> + if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
> + return;
>
> - pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> - pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
> + pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
> + pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
>
> - hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> - vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
> + hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> + vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
>
> - uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
> - uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
> + uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
> + uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
>
> - id = scaler_state->scaler_id;
> + id = scaler_state->scaler_id;
>
> - spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>
> - intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> - PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> - intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> - PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
> - intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
> - PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> - intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
> - crtc_state->pch_pfit.pos);
> - intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> - crtc_state->pch_pfit.size);
> + intel_de_write_fw(dev_priv, SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> + PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
> + intel_de_write_fw(dev_priv, SKL_PS_VPHASE(pipe, id),
> + PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
> + intel_de_write_fw(dev_priv, SKL_PS_HPHASE(pipe, id),
> + PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> + intel_de_write_fw(dev_priv, SKL_PS_WIN_POS(pipe, id),
> + crtc_state->pch_pfit.pos);
> + intel_de_write_fw(dev_priv, SKL_PS_WIN_SZ(pipe, id),
> + crtc_state->pch_pfit.size);
>
> - spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> - }
> + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> }
>
> static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
> @@ -6277,22 +6277,23 @@ static void ilk_pfit_enable(const struct intel_crtc_state *crtc_state)
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum pipe pipe = crtc->pipe;
>
> - if (crtc_state->pch_pfit.enabled) {
> - /* Force use of hard-coded filter coefficients
> - * as some pre-programmed values are broken,
> - * e.g. x201.
> - */
> - if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
> - intel_de_write(dev_priv, PF_CTL(pipe),
> - PF_ENABLE | PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
> - else
> - intel_de_write(dev_priv, PF_CTL(pipe),
> - PF_ENABLE | PF_FILTER_MED_3x3);
> - intel_de_write(dev_priv, PF_WIN_POS(pipe),
> - crtc_state->pch_pfit.pos);
> - intel_de_write(dev_priv, PF_WIN_SZ(pipe),
> - crtc_state->pch_pfit.size);
Why dont we use the intel_de_write_fw() everywhere?
Manasi
> - }
> + if (!crtc_state->pch_pfit.enabled)
> + return;
> +
> + /* Force use of hard-coded filter coefficients
> + * as some pre-programmed values are broken,
> + * e.g. x201.
> + */
> + if (IS_IVYBRIDGE(dev_priv) || IS_HASWELL(dev_priv))
> + intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
> + PF_FILTER_MED_3x3 | PF_PIPE_SEL_IVB(pipe));
> + else
> + intel_de_write(dev_priv, PF_CTL(pipe), PF_ENABLE |
> + PF_FILTER_MED_3x3);
> + intel_de_write(dev_priv, PF_WIN_POS(pipe),
> + crtc_state->pch_pfit.pos);
> + intel_de_write(dev_priv, PF_WIN_SZ(pipe),
> + crtc_state->pch_pfit.size);
> }
>
> void hsw_enable_ips(const struct intel_crtc_state *crtc_state)
> @@ -7107,11 +7108,12 @@ void ilk_pfit_disable(const struct intel_crtc_state *old_crtc_state)
>
> /* 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 (old_crtc_state->pch_pfit.enabled) {
> - intel_de_write(dev_priv, PF_CTL(pipe), 0);
> - intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
> - intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
> - }
> + if (!old_crtc_state->pch_pfit.enabled)
> + return;
> +
> + intel_de_write(dev_priv, PF_CTL(pipe), 0);
> + intel_de_write(dev_priv, PF_WIN_POS(pipe), 0);
> + intel_de_write(dev_priv, PF_WIN_SZ(pipe), 0);
> }
>
> static void ilk_crtc_disable(struct intel_atomic_state *state,
> @@ -7927,39 +7929,35 @@ static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc)
> (crtc->pipe == PIPE_A || IS_I915G(dev_priv));
> }
>
> -static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config)
> +static u32 ilk_pipe_pixel_rate(const struct intel_crtc_state *crtc_state)
> {
> - u32 pixel_rate;
> -
> - pixel_rate = pipe_config->hw.adjusted_mode.crtc_clock;
> + u32 pixel_rate = crtc_state->hw.adjusted_mode.crtc_clock;
> + u32 pfit_size = crtc_state->pch_pfit.size;
> + u64 pipe_w, pipe_h, pfit_w, pfit_h;
>
> /*
> * We only use IF-ID interlacing. If we ever use
> * PF-ID we'll need to adjust the pixel_rate here.
> */
>
> - if (pipe_config->pch_pfit.enabled) {
> - u64 pipe_w, pipe_h, pfit_w, pfit_h;
> - u32 pfit_size = pipe_config->pch_pfit.size;
> -
> - pipe_w = pipe_config->pipe_src_w;
> - pipe_h = pipe_config->pipe_src_h;
> + if (!crtc_state->pch_pfit.enabled)
> + return pixel_rate;
>
> - pfit_w = (pfit_size >> 16) & 0xFFFF;
> - pfit_h = pfit_size & 0xFFFF;
> - if (pipe_w < pfit_w)
> - pipe_w = pfit_w;
> - if (pipe_h < pfit_h)
> - pipe_h = pfit_h;
> + pipe_w = crtc_state->pipe_src_w;
> + pipe_h = crtc_state->pipe_src_h;
>
> - if (WARN_ON(!pfit_w || !pfit_h))
> - return pixel_rate;
> + pfit_w = (pfit_size >> 16) & 0xFFFF;
> + pfit_h = pfit_size & 0xFFFF;
> + if (pipe_w < pfit_w)
> + pipe_w = pfit_w;
> + if (pipe_h < pfit_h)
> + pipe_h = pfit_h;
>
> - pixel_rate = div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
> - pfit_w * pfit_h);
> - }
> + if (WARN_ON(!pfit_w || !pfit_h))
> + return pixel_rate;
>
> - return pixel_rate;
> + return div_u64(mul_u32_u32(pixel_rate, pipe_w * pipe_h),
> + pfit_w * pfit_h);
> }
>
> static void intel_crtc_compute_pixel_rate(struct intel_crtc_state *crtc_state)
> @@ -9153,9 +9151,9 @@ static bool i9xx_has_pfit(struct drm_i915_private *dev_priv)
> IS_PINEVIEW(dev_priv) || IS_MOBILE(dev_priv);
> }
>
> -static void i9xx_get_pfit_config(struct intel_crtc *crtc,
> - struct intel_crtc_state *pipe_config)
> +static void i9xx_get_pfit_config(struct intel_crtc_state *crtc_state)
> {
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> u32 tmp;
>
> @@ -9175,9 +9173,9 @@ static void i9xx_get_pfit_config(struct intel_crtc *crtc,
> return;
> }
>
> - pipe_config->gmch_pfit.control = tmp;
> - pipe_config->gmch_pfit.pgm_ratios = intel_de_read(dev_priv,
> - PFIT_PGM_RATIOS);
> + crtc_state->gmch_pfit.control = tmp;
> + crtc_state->gmch_pfit.pgm_ratios =
> + intel_de_read(dev_priv, PFIT_PGM_RATIOS);
> }
>
> static void vlv_crtc_clock_get(struct intel_crtc *crtc,
> @@ -9427,7 +9425,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> intel_get_pipe_timings(crtc, pipe_config);
> intel_get_pipe_src_size(crtc, pipe_config);
>
> - i9xx_get_pfit_config(crtc, pipe_config);
> + i9xx_get_pfit_config(pipe_config);
>
> if (INTEL_GEN(dev_priv) >= 4) {
> /* No way to read it out on pipes B and C */
> @@ -10393,37 +10391,37 @@ static void ilk_get_fdi_m_n_config(struct intel_crtc *crtc,
> &pipe_config->fdi_m_n, NULL);
> }
>
> -static void skl_get_pfit_config(struct intel_crtc *crtc,
> - struct intel_crtc_state *pipe_config)
> +static void skl_get_pfit_config(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_scaler_state *scaler_state = &pipe_config->scaler_state;
> - u32 ps_ctrl = 0;
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state;
> int id = -1;
> int i;
>
> /* find scaler attached to this pipe */
> for (i = 0; i < crtc->num_scalers; i++) {
> - ps_ctrl = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
> - if (ps_ctrl & PS_SCALER_EN && !(ps_ctrl & PS_PLANE_SEL_MASK)) {
> - id = i;
> - pipe_config->pch_pfit.enabled = true;
> - pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
> - SKL_PS_WIN_POS(crtc->pipe, i));
> - pipe_config->pch_pfit.size = intel_de_read(dev_priv,
> - SKL_PS_WIN_SZ(crtc->pipe, i));
> - scaler_state->scalers[i].in_use = true;
> - break;
> - }
> + u32 tmp;
> +
> + tmp = intel_de_read(dev_priv, SKL_PS_CTRL(crtc->pipe, i));
> + if ((tmp & (PS_SCALER_EN | PS_PLANE_SEL_MASK)) != PS_SCALER_EN)
> + continue;
> +
> + id = i;
> + crtc_state->pch_pfit.enabled = true;
> + crtc_state->pch_pfit.pos =
> + intel_de_read(dev_priv, SKL_PS_WIN_POS(crtc->pipe, i));
> + crtc_state->pch_pfit.size =
> + intel_de_read(dev_priv, SKL_PS_WIN_SZ(crtc->pipe, i));
> + scaler_state->scalers[i].in_use = true;
> + break;
> }
>
> scaler_state->scaler_id = id;
> - if (id >= 0) {
> + if (id >= 0)
> scaler_state->scaler_users |= (1 << SKL_CRTC_INDEX);
> - } else {
> + else
> scaler_state->scaler_users &= ~(1 << SKL_CRTC_INDEX);
> - }
> }
>
> static void
> @@ -10559,30 +10557,29 @@ skl_get_initial_plane_config(struct intel_crtc *crtc,
> kfree(intel_fb);
> }
>
> -static void ilk_get_pfit_config(struct intel_crtc *crtc,
> - struct intel_crtc_state *pipe_config)
> +static void ilk_get_pfit_config(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->uapi.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> u32 tmp;
>
> tmp = intel_de_read(dev_priv, PF_CTL(crtc->pipe));
> + if ((tmp & PF_ENABLE) == 0)
> + return;
>
> - if (tmp & PF_ENABLE) {
> - pipe_config->pch_pfit.enabled = true;
> - pipe_config->pch_pfit.pos = intel_de_read(dev_priv,
> - PF_WIN_POS(crtc->pipe));
> - pipe_config->pch_pfit.size = intel_de_read(dev_priv,
> - PF_WIN_SZ(crtc->pipe));
> -
> - /* We currently do not free assignements of panel fitters on
> - * ivb/hsw (since we don't use the higher upscaling modes which
> - * differentiates them) so just WARN about this case for now. */
> - if (IS_GEN(dev_priv, 7)) {
> - WARN_ON((tmp & PF_PIPE_SEL_MASK_IVB) !=
> - PF_PIPE_SEL_IVB(crtc->pipe));
> - }
> - }
> + crtc_state->pch_pfit.enabled = true;
> + crtc_state->pch_pfit.pos =
> + intel_de_read(dev_priv, PF_WIN_POS(crtc->pipe));
> + crtc_state->pch_pfit.size =
> + intel_de_read(dev_priv, PF_WIN_SZ(crtc->pipe));
> +
> + /*
> + * We currently do not free assignements of panel fitters on
> + * ivb/hsw (since we don't use the higher upscaling modes which
> + * differentiates them) so just WARN about this case for now.
> + */
> + WARN_ON(IS_GEN(dev_priv, 7) &&
> + (tmp & PF_PIPE_SEL_MASK_IVB) != PF_PIPE_SEL_IVB(crtc->pipe));
> }
>
> static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> @@ -10694,7 +10691,7 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
> intel_get_pipe_timings(crtc, pipe_config);
> intel_get_pipe_src_size(crtc, pipe_config);
>
> - ilk_get_pfit_config(crtc, pipe_config);
> + ilk_get_pfit_config(pipe_config);
>
> ret = true;
>
> @@ -11219,9 +11216,9 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
> power_domain_mask |= BIT_ULL(power_domain);
>
> if (INTEL_GEN(dev_priv) >= 9)
> - skl_get_pfit_config(crtc, pipe_config);
> + skl_get_pfit_config(pipe_config);
> else
> - ilk_get_pfit_config(crtc, pipe_config);
> + ilk_get_pfit_config(pipe_config);
> }
>
> if (hsw_crtc_supports_ips(crtc)) {
> --
> 2.24.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
