[Intel-gfx] [PATCH] drm/i915: panel fitter hw state readout&check support
Daniel Vetter
daniel at ffwll.ch
Fri May 10 14:02:12 CEST 2013
On Fri, May 10, 2013 at 02:37:59PM +0300, Mika Kuoppala wrote:
>
> Hi Daniel,
>
> Daniel Vetter <daniel.vetter at ffwll.ch> writes:
>
> > Pfit state readout is a bit ugly on gen2/3 due to the intermingling
> > with the lvds state, but alas.
> >
> > Also note that since state is always cleared to zero we can
> > unconditonally compare all the state and completely neglect the actual
> > platform we're running on.
> >
> > v2: Properly check for the pfit power domain on haswell.
> >
> > v3: Don't check pgm_ratios on gen4+, they're auto-computed by the hw.
> >
> > v4: Properly clear the lvds border bits, upset the state checker a
> > bit.
> >
> > v5: Unconditionally read out panel dither settings on gen2/3.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 67 ++++++++++++++++++++++++++++++++++--
> > drivers/gpu/drm/i915/intel_lvds.c | 1 +
> > 2 files changed, 66 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index bfbb484..58395e3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4967,6 +4967,36 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> > return ret;
> > }
> >
> > +static void i9xx_get_pfit_config(struct intel_crtc *crtc,
> > + struct intel_crtc_config *pipe_config)
> > +{
> > + struct drm_device *dev = crtc->base.dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + uint32_t tmp;
> > +
> > + tmp = I915_READ(PFIT_CONTROL);
>
> You could do
> const uint32_t ctl = I915_READ(PFIT_CONTROL);
> and...
>
> > + if (INTEL_INFO(dev)->gen < 4) {
> > + if (crtc->pipe != PIPE_B)
> > + return;
> > +
> > + /* gen2/3 store dither state in pfit control, needs to match */
> > + pipe_config->gmch_pfit.control = tmp & PANEL_8TO6_DITHER_ENABLE;
> > + } else {
> > + if ((tmp & PFIT_PIPE_MASK) != (crtc->pipe << PFIT_PIPE_SHIFT))
> > + return;
> > + }
> > +
> > + if (!(tmp & PFIT_ENABLE))
> > + return;
> > +
> > + pipe_config->gmch_pfit.control = I915_READ(PFIT_CONTROL);
>
> and use ctl in here to avoid extra read.
Originally I've had that, but then thought it'd look more symmetric if I
read out all the registers anew once we're sure that we're looking at the
right panel fitter. I can do the cse again if you want.
> > + pipe_config->gmch_pfit.pgm_ratios = I915_READ(PFIT_PGM_RATIOS);
> > + if (INTEL_INFO(dev)->gen < 5)
> > + pipe_config->gmch_pfit.lvds_border_bits =
> > + I915_READ(LVDS) & LVDS_BORDER_ENABLE;
> > +}
> > +
> > static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> > struct intel_crtc_config *pipe_config)
> > {
> > @@ -4980,6 +5010,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> >
> > intel_get_pipe_timings(crtc, pipe_config);
> >
> > + i9xx_get_pfit_config(crtc, pipe_config);
> > +
> > return true;
> > }
> >
> > @@ -5815,6 +5847,21 @@ static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
> > & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
> > }
> >
> > +static void ironlake_get_pfit_config(struct intel_crtc *crtc,
> > + struct intel_crtc_config *pipe_config)
> > +{
> > + struct drm_device *dev = crtc->base.dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + uint32_t tmp;
> > +
> > + tmp = I915_READ(PF_CTL(crtc->pipe));
> > +
> > + if (tmp & PF_ENABLE) {
> > + pipe_config->pch_pfit.pos = I915_READ(PF_WIN_POS(crtc->pipe));
> > + pipe_config->pch_pfit.size = I915_READ(PF_WIN_SZ(crtc->pipe));
> > + }
> > +}
> > +
> > static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> > struct intel_crtc_config *pipe_config)
> > {
> > @@ -5838,6 +5885,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> >
> > intel_get_pipe_timings(crtc, pipe_config);
> >
> > + ironlake_get_pfit_config(crtc, pipe_config);
> > +
> > return true;
> > }
> >
> > @@ -5957,6 +6006,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> > struct drm_device *dev = crtc->base.dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > enum transcoder cpu_transcoder = crtc->config.cpu_transcoder;
> > + enum intel_display_power_domain pfit_domain;
> > uint32_t tmp;
> >
> > if (!intel_display_power_enabled(dev,
> > @@ -5986,6 +6036,10 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >
> > intel_get_pipe_timings(crtc, pipe_config);
> >
> > + pfit_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> > + if (intel_display_power_enabled(dev, pfit_domain))
> > + ironlake_get_pfit_config(crtc, pipe_config);
> > +
> > return true;
> > }
> >
> > @@ -7961,7 +8015,8 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
> > if (mask & (1 <<(intel_crtc)->pipe))
> >
> > static bool
> > -intel_pipe_config_compare(struct intel_crtc_config *current_config,
> > +intel_pipe_config_compare(struct drm_device *dev,
> > + struct intel_crtc_config *current_config,
> > struct intel_crtc_config *pipe_config)
> > {
> > #define PIPE_CONF_CHECK_I(name) \
> > @@ -8010,6 +8065,14 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
> > PIPE_CONF_CHECK_I(requested_mode.hdisplay);
> > PIPE_CONF_CHECK_I(requested_mode.vdisplay);
> >
> > + PIPE_CONF_CHECK_I(gmch_pfit.control);
> > + /* pfit ratios are autocomputed by the hw on gen4+ */
> > + if (INTEL_INFO(dev)->gen < 4)
> > + PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
> > + PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
> > + PIPE_CONF_CHECK_I(pch_pfit.pos);
> > + PIPE_CONF_CHECK_I(pch_pfit.size);
> > +
>
> For ironlake+ we don't get conf checking for the control register part.
> Should we?
ilk pfit control state isn't precomputed (we just use size != 0 as a
signal to set it to a specific value). It's also always the same value, so
I don't think we need to jump through loops here.
That will change of course once we bother to enable the higher upscaling
modes (if we ever do that), since those have global sharing constraints.
But for now I don't see a need.
-Daniel
>
> > #undef PIPE_CONF_CHECK_I
> > #undef PIPE_CONF_CHECK_FLAGS
> >
> > @@ -8121,7 +8184,7 @@ intel_modeset_check_state(struct drm_device *dev)
> > "(expected %i, found %i)\n", crtc->active, active);
> >
> > WARN(active &&
> > - !intel_pipe_config_compare(&crtc->config, &pipe_config),
> > + !intel_pipe_config_compare(dev, &crtc->config, &pipe_config),
> > "pipe state doesn't match!\n");
> > }
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index d256fe4..3294efd 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -116,6 +116,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
> > }
> >
> > /* set the corresponsding LVDS_BORDER bit */
> > + temp &= ~LVDS_BORDER_ENABLE;
> > temp |= intel_crtc->config.gmch_pfit.lvds_border_bits;
> > /* Set the B0-B3 data pairs corresponding to whether we're going to
> > * set the DPLLs for dual-channel mode or not.
> > --
> > 1.7.11.7
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list