[Intel-gfx] [PATCH 75/89] drm/i915/skl: fetch, enable/disable pfit as needed

[Jesse Barnes]

On Tue, 23 Sep 2014 17:50:29 -0300
Paulo Zanoni <przanoni at gmail.com> wrote:

> 2014-09-04 8:27 GMT-03:00 Damien Lespiau <damien.lespiau at intel.com>:
> > From: Jesse Barnes <jbarnes at virtuousgeek.org>
> >
> > This moved around on SKL, so we need to make sure we read/write the
> > correct regs.
> >
> > Signed-off-by: Jesse Barnes <jbarnes at virtuougseek.org>
> > Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      | 13 ++++++++
> >  drivers/gpu/drm/i915/intel_display.c | 61 +++++++++++++++++++++++++++++++++---
> >  2 files changed, 70 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 176e84e..35c0759 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4833,6 +4833,19 @@ enum skl_disp_power_wells {
> >  #define PF_VSCALE(pipe)                _PIPE(pipe, _PFA_VSCALE, _PFB_VSCALE)
> >  #define PF_HSCALE(pipe)                _PIPE(pipe, _PFA_HSCALE, _PFB_HSCALE)
> >
> > +#define _PSA_CTL               0x68180
> > +#define _PSB_CTL               0x68980
> > +#define PS_ENABLE              (1<<31)
> > +#define PS_PIPE_SEL(pipe)      ((pipe)<<27)
> 
> The PS_PIPE_SEL define is wrong, but it is also not used in the code
> you wrote, so just remove this line.

Yeah that sounds fine.

> 
> 
> > +#define _PSA_WIN_SZ            0x68174
> > +#define _PSB_WIN_SZ            0x68974
> > +#define _PSA_WIN_POS           0x68178
> 
> 0x68170
> 
> > +#define _PSB_WIN_POS           0x68978
> 
> 0x68970

Ooh thanks.

> 
> > +
> > +#define PS_CTL(pipe)           _PIPE(pipe, _PSA_CTL, _PSB_CTL)
> > +#define PS_WIN_SZ(pipe)                _PIPE(pipe, _PSA_WIN_SZ, _PSB_WIN_SZ)
> > +#define PS_WIN_POS(pipe)       _PIPE(pipe, _PSA_WIN_POS, _PSB_WIN_POS)
> > +
> >  /* legacy palette */
> >  #define _LGC_PALETTE_A           0x4a000
> >  #define _LGC_PALETTE_B           0x4a800
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 393bd19..9b31342 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3882,6 +3882,19 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
> >         }
> >  }
> >
> > +static void skylake_pfit_enable(struct intel_crtc *crtc)
> > +{
> > +       struct drm_device *dev = crtc->base.dev;
> > +       struct drm_i915_private *dev_priv = dev->dev_private;
> > +       int pipe = crtc->pipe;
> > +
> > +       if (crtc->config.pch_pfit.enabled) {
> > +               I915_WRITE(PS_CTL(pipe), PS_ENABLE);
> > +               I915_WRITE(PS_WIN_POS(pipe), crtc->config.pch_pfit.pos);
> > +               I915_WRITE(PS_WIN_SZ(pipe), crtc->config.pch_pfit.size);
> > +       }
> > +}
> > +
> >  static void ironlake_pfit_enable(struct intel_crtc *crtc)
> >  {
> >         struct drm_device *dev = crtc->base.dev;
> > @@ -4264,7 +4277,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >
> >         intel_ddi_enable_pipe_clock(intel_crtc);
> >
> > -       ironlake_pfit_enable(intel_crtc);
> > +       if (IS_SKYLAKE(dev))
> > +               skylake_pfit_enable(intel_crtc);
> > +       else
> > +               ironlake_pfit_enable(intel_crtc);
> >
> >         /*
> >          * On ILK+ LUT must be loaded before the pipe is running but with
> > @@ -4295,6 +4311,20 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >         intel_crtc_enable_planes(crtc);
> >  }
> >
> > +static void skylake_pfit_disable(struct intel_crtc *crtc)
> > +{
> > +       struct drm_device *dev = crtc->base.dev;
> > +       struct drm_i915_private *dev_priv = dev->dev_private;
> > +       int 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 (crtc->config.pch_pfit.enabled) {
> > +               I915_WRITE(PS_CTL(pipe), 0);
> > +               I915_WRITE(PS_WIN_SZ(pipe), 0);
> 
> Why not also zero _POS just like HSW? Can this affect the HW state
> checker output?
> 
> Everything else looks correct.

The checker will only if the fitter is enabled, but yeah it looks like
it would be safer to zero it.

Damien, did you want to make these changes as part of your re-post or
should I send an updated patch to replace this one?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center