| Message ID | 20200212161738.28141-5-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: pfit/scaler rework prep stuff | expand |
On Wed, Feb 12, 2020 at 06:17:34PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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@linux.intel.com> > --- > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Apr 01, 2020 at 04:53:23PM -0700, Manasi Navare wrote: > On Wed, Feb 12, 2020 at 06:17:34PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > 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@linux.intel.com> > > --- > > 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? Because no one thought it mattered much. It only really becomes significant (mainly when lockdep is enabled) when a large number of registers are written. > > 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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Apr 02, 2020 at 04:55:06PM +0300, Ville Syrjälä wrote: > On Wed, Apr 01, 2020 at 04:53:23PM -0700, Manasi Navare wrote: > > On Wed, Feb 12, 2020 at 06:17:34PM +0200, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > 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@linux.intel.com> > > > --- > > > 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? > > Because no one thought it mattered much. It only really becomes > significant (mainly when lockdep is enabled) when a large number > of registers are written. Okay everything else flattening things in the functions make sense for readability Reviewed-by: Manasi Navare <manasi.d.navare@intel.com> Manasi > > > > > 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@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
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); - } + 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)) {