From mboxrd@z Thu Jan 1 00:00:00 1970 From: Damien Lespiau Subject: Re: [PATCH 75/89] drm/i915/skl: fetch, enable/disable pfit as needed Date: Wed, 24 Sep 2014 11:44:53 +0100 Message-ID: <20140924104453.GA31765@strange.icx.intel.com> References: <1409830075-11139-1-git-send-email-damien.lespiau@intel.com> <1409830075-11139-76-git-send-email-damien.lespiau@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 4F4D66E15E for ; Wed, 24 Sep 2014 03:49:42 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paulo Zanoni Cc: Intel Graphics Development , jbarnes@virtuougseek.org List-Id: intel-gfx@lists.freedesktop.org Hi Jesse, Mind looking at those review comments? -- Damien On Tue, Sep 23, 2014 at 05:50:29PM -0300, Paulo Zanoni wrote: > 2014-09-04 8:27 GMT-03:00 Damien Lespiau : > > From: Jesse Barnes > > > > This moved around on SKL, so we need to make sure we read/write the > > correct regs. > > > > Signed-off-by: Jesse Barnes > > Signed-off-by: Damien Lespiau > > --- > > 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. > > > > +#define _PSA_WIN_SZ 0x68174 > > +#define _PSB_WIN_SZ 0x68974 > > +#define _PSA_WIN_POS 0x68178 > > 0x68170 > > > +#define _PSB_WIN_POS 0x68978 > > 0x68970 > > > + > > +#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. > > > + } > > +} > > + > > static void ironlake_pfit_disable(struct intel_crtc *crtc) > > { > > struct drm_device *dev = crtc->base.dev; > > @@ -4399,7 +4429,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) > > > > intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder); > > > > - ironlake_pfit_disable(intel_crtc); > > + if (IS_SKYLAKE(dev)) > > + skylake_pfit_disable(intel_crtc); > > + else > > + ironlake_pfit_disable(intel_crtc); > > > > intel_ddi_disable_pipe_clock(intel_crtc); > > > > @@ -7382,6 +7415,22 @@ static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc, > > &pipe_config->fdi_m_n, NULL); > > } > > > > +static void skylake_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(PS_CTL(crtc->pipe)); > > + > > + if (tmp & PS_ENABLE) { > > + pipe_config->pch_pfit.enabled = true; > > + pipe_config->pch_pfit.pos = I915_READ(PS_WIN_POS(crtc->pipe)); > > + pipe_config->pch_pfit.size = I915_READ(PS_WIN_SZ(crtc->pipe)); > > + } > > +} > > + > > static void ironlake_get_pfit_config(struct intel_crtc *crtc, > > struct intel_crtc_config *pipe_config) > > { > > @@ -7940,8 +7989,12 @@ 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_priv, pfit_domain)) > > - ironlake_get_pfit_config(crtc, pipe_config); > > + if (intel_display_power_enabled(dev_priv, pfit_domain)) { > > + if (IS_SKYLAKE(dev)) > > + skylake_get_pfit_config(crtc, pipe_config); > > + else > > + ironlake_get_pfit_config(crtc, pipe_config); > > + } > > > > if (IS_HASWELL(dev)) > > pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) && > > -- > > 1.8.3.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni