| Message ID | 20190321214431.515-1-radhakrishna.sripada@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/i915/icl: Fix clockgating issue when using scalers | expand |
On Thu, Mar 21, 2019 at 02:44:31PM -0700, Radhakrishna Sripada wrote: > Fixes the clock-gating issue when pipe scaling is enabled. > (Lineage #2006604312) > > V2: Fix typo in headline(Chris) > Handle the non double buffered nature of the register(Ville) > V3: Fix checkpatch warning. BAT failure for V2 on gen3 looks unrelated. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Aditya Swarup <aditya.swarup@intel.com> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 43 +++++++++++++++++----------- > 1 file changed, 27 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7c15b428ff84..cfa19ae12e22 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -469,13 +469,22 @@ static const struct intel_limit intel_limits_bxt = { > static void > skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable) > { > + u32 val = 0; > + > + /* > + * Wa_2006604312:icl > + */ > + if (IS_ICELAKE(dev_priv)) > + val = DPFR_GATING_DIS; > + else > + val = DUPS1_GATING_DIS | DUPS2_GATING_DIS; > + > + /* WA Display #0827: Gen9:all */ You're now conflating two workaround and splitting their implementation between two functions in a confusing way. Better to keep them separate IMO. > if (enable) > - I915_WRITE(CLKGATE_DIS_PSL(pipe), > - DUPS1_GATING_DIS | DUPS2_GATING_DIS); > + I915_WRITE(CLKGATE_DIS_PSL(pipe), val); > else > I915_WRITE(CLKGATE_DIS_PSL(pipe), > - I915_READ(CLKGATE_DIS_PSL(pipe)) & > - ~(DUPS1_GATING_DIS | DUPS2_GATING_DIS)); > + I915_READ(CLKGATE_DIS_PSL(pipe)) & ~val); > } > > static bool > @@ -5481,14 +5490,18 @@ static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s > return !old_crtc_state->ips_enabled; > } > > -static bool needs_nv12_wa(struct drm_i915_private *dev_priv, > - const struct intel_crtc_state *crtc_state) > +static bool skl_needs_clk_wa(struct drm_i915_private *dev_priv, > + const struct intel_crtc_state *crtc_state) > { > - if (!crtc_state->nv12_planes) > - return false; > - > /* WA Display #0827: Gen9:all */ > - if (IS_GEN(dev_priv, 9) && !IS_GEMINILAKE(dev_priv)) > + if (!!crtc_state->nv12_planes && IS_GEN(dev_priv, 9) && > + !IS_GEMINILAKE(dev_priv)) > + return true; > + > + /* > + * Wa_2006604312:icl > + */ > + if (IS_ICELAKE(dev_priv) && crtc_state->pch_pfit.enabled) > return true; > > return false; > @@ -5527,9 +5540,8 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) > intel_post_enable_primary(&crtc->base, pipe_config); > } > > - /* Display WA 827 */ > - if (needs_nv12_wa(dev_priv, old_crtc_state) && > - !needs_nv12_wa(dev_priv, pipe_config)) { > + if (skl_needs_clk_wa(dev_priv, old_crtc_state) && > + !skl_needs_clk_wa(dev_priv, pipe_config)) { > skl_wa_clkgate(dev_priv, crtc->pipe, false); > } > } > @@ -5566,9 +5578,8 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); > } > > - /* Display WA 827 */ > - if (!needs_nv12_wa(dev_priv, old_crtc_state) && > - needs_nv12_wa(dev_priv, pipe_config)) { > + if (!skl_needs_clk_wa(dev_priv, old_crtc_state) && > + skl_needs_clk_wa(dev_priv, pipe_config)) { > skl_wa_clkgate(dev_priv, crtc->pipe, true); > } > > -- > 2.20.0.rc2.7.g965798d1f299
On Fri, 2019-03-22 at 15:14 +0200, Ville Syrjälä wrote: > On Thu, Mar 21, 2019 at 02:44:31PM -0700, Radhakrishna Sripada wrote: > > Fixes the clock-gating issue when pipe scaling is enabled. > > (Lineage #2006604312) > > > > V2: Fix typo in headline(Chris) > > Handle the non double buffered nature of the register(Ville) > > V3: Fix checkpatch warning. BAT failure for V2 on gen3 looks > > unrelated. > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Aditya Swarup <aditya.swarup@intel.com> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com > > > > > --- > > drivers/gpu/drm/i915/intel_display.c | 43 +++++++++++++++++------- > > ---- > > 1 file changed, 27 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 7c15b428ff84..cfa19ae12e22 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -469,13 +469,22 @@ static const struct intel_limit > > intel_limits_bxt = { > > static void > > skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool > > enable) > > { > > + u32 val = 0; > > + > > + /* > > + * Wa_2006604312:icl > > + */ > > + if (IS_ICELAKE(dev_priv)) > > + val = DPFR_GATING_DIS; > > + else > > + val = DUPS1_GATING_DIS | DUPS2_GATING_DIS; > > + > > + /* WA Display #0827: Gen9:all */ > > You're now conflating two workaround and splitting their > implementation > between two functions in a confusing way. Better to keep them > separate > IMO. I encountered a similar question while writing the code. I discussed with Rodrigo about a possible way of implementing a function pointer kind of prototype followed by ICL/SKL wa to differentiate b/ween the two implementations. It was suggested that the approach is overly complicated. Since we do use the same register in both cases and due to the non double buffered nature of the register I have resorted to the stupid/dirty way. Do you suggest otherwise? - Radhakrishna(RK) Sripada > > > if (enable) > > - I915_WRITE(CLKGATE_DIS_PSL(pipe), > > - DUPS1_GATING_DIS | DUPS2_GATING_DIS); > > + I915_WRITE(CLKGATE_DIS_PSL(pipe), val); > > else > > I915_WRITE(CLKGATE_DIS_PSL(pipe), > > - I915_READ(CLKGATE_DIS_PSL(pipe)) & > > - ~(DUPS1_GATING_DIS | DUPS2_GATING_DIS)); > > + I915_READ(CLKGATE_DIS_PSL(pipe)) & ~val); > > } > > > > static bool > > @@ -5481,14 +5490,18 @@ static bool > > hsw_post_update_enable_ips(const struct intel_crtc_state > > *old_crtc_s > > return !old_crtc_state->ips_enabled; > > } > > > > -static bool needs_nv12_wa(struct drm_i915_private *dev_priv, > > - const struct intel_crtc_state *crtc_state) > > +static bool skl_needs_clk_wa(struct drm_i915_private *dev_priv, > > + const struct intel_crtc_state *crtc_state) > > { > > - if (!crtc_state->nv12_planes) > > - return false; > > - > > /* WA Display #0827: Gen9:all */ > > - if (IS_GEN(dev_priv, 9) && !IS_GEMINILAKE(dev_priv)) > > + if (!!crtc_state->nv12_planes && IS_GEN(dev_priv, 9) && > > + !IS_GEMINILAKE(dev_priv)) > > + return true; > > + > > + /* > > + * Wa_2006604312:icl > > + */ > > + if (IS_ICELAKE(dev_priv) && crtc_state->pch_pfit.enabled) > > return true; > > > > return false; > > @@ -5527,9 +5540,8 @@ static void intel_post_plane_update(struct > > intel_crtc_state *old_crtc_state) > > intel_post_enable_primary(&crtc->base, > > pipe_config); > > } > > > > - /* Display WA 827 */ > > - if (needs_nv12_wa(dev_priv, old_crtc_state) && > > - !needs_nv12_wa(dev_priv, pipe_config)) { > > + if (skl_needs_clk_wa(dev_priv, old_crtc_state) && > > + !skl_needs_clk_wa(dev_priv, pipe_config)) { > > skl_wa_clkgate(dev_priv, crtc->pipe, false); > > } > > } > > @@ -5566,9 +5578,8 @@ static void intel_pre_plane_update(struct > > intel_crtc_state *old_crtc_state, > > intel_set_cpu_fifo_underrun_reporting(dev_priv, > > crtc->pipe, false); > > } > > > > - /* Display WA 827 */ > > - if (!needs_nv12_wa(dev_priv, old_crtc_state) && > > - needs_nv12_wa(dev_priv, pipe_config)) { > > + if (!skl_needs_clk_wa(dev_priv, old_crtc_state) && > > + skl_needs_clk_wa(dev_priv, pipe_config)) { > > skl_wa_clkgate(dev_priv, crtc->pipe, true); > > } > > > > -- > > 2.20.0.rc2.7.g965798d1f299 > >
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7c15b428ff84..cfa19ae12e22 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -469,13 +469,22 @@ static const struct intel_limit intel_limits_bxt = { static void skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable) { + u32 val = 0; + + /* + * Wa_2006604312:icl + */ + if (IS_ICELAKE(dev_priv)) + val = DPFR_GATING_DIS; + else + val = DUPS1_GATING_DIS | DUPS2_GATING_DIS; + + /* WA Display #0827: Gen9:all */ if (enable) - I915_WRITE(CLKGATE_DIS_PSL(pipe), - DUPS1_GATING_DIS | DUPS2_GATING_DIS); + I915_WRITE(CLKGATE_DIS_PSL(pipe), val); else I915_WRITE(CLKGATE_DIS_PSL(pipe), - I915_READ(CLKGATE_DIS_PSL(pipe)) & - ~(DUPS1_GATING_DIS | DUPS2_GATING_DIS)); + I915_READ(CLKGATE_DIS_PSL(pipe)) & ~val); } static bool @@ -5481,14 +5490,18 @@ static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s return !old_crtc_state->ips_enabled; } -static bool needs_nv12_wa(struct drm_i915_private *dev_priv, - const struct intel_crtc_state *crtc_state) +static bool skl_needs_clk_wa(struct drm_i915_private *dev_priv, + const struct intel_crtc_state *crtc_state) { - if (!crtc_state->nv12_planes) - return false; - /* WA Display #0827: Gen9:all */ - if (IS_GEN(dev_priv, 9) && !IS_GEMINILAKE(dev_priv)) + if (!!crtc_state->nv12_planes && IS_GEN(dev_priv, 9) && + !IS_GEMINILAKE(dev_priv)) + return true; + + /* + * Wa_2006604312:icl + */ + if (IS_ICELAKE(dev_priv) && crtc_state->pch_pfit.enabled) return true; return false; @@ -5527,9 +5540,8 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) intel_post_enable_primary(&crtc->base, pipe_config); } - /* Display WA 827 */ - if (needs_nv12_wa(dev_priv, old_crtc_state) && - !needs_nv12_wa(dev_priv, pipe_config)) { + if (skl_needs_clk_wa(dev_priv, old_crtc_state) && + !skl_needs_clk_wa(dev_priv, pipe_config)) { skl_wa_clkgate(dev_priv, crtc->pipe, false); } } @@ -5566,9 +5578,8 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); } - /* Display WA 827 */ - if (!needs_nv12_wa(dev_priv, old_crtc_state) && - needs_nv12_wa(dev_priv, pipe_config)) { + if (!skl_needs_clk_wa(dev_priv, old_crtc_state) && + skl_needs_clk_wa(dev_priv, pipe_config)) { skl_wa_clkgate(dev_priv, crtc->pipe, true); }
Fixes the clock-gating issue when pipe scaling is enabled. (Lineage #2006604312) V2: Fix typo in headline(Chris) Handle the non double buffered nature of the register(Ville) V3: Fix checkpatch warning. BAT failure for V2 on gen3 looks unrelated. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Aditya Swarup <aditya.swarup@intel.com> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 43 +++++++++++++++++----------- 1 file changed, 27 insertions(+), 16 deletions(-)