On Mon, Jul 28, 2014 at 05:31:46PM +0100, arun.siluv...@linux.intel.com wrote:
> From: Arun Siluvery <arun.siluv...@linux.intel.com>
> 
> The workarounds at the moment are initialized in init_clock_gating() but
> they are lost during reset; In case of execlists some workarounds modify
> registers that are part of register state context, since these are not
> initialized until init_clock_gating() default context ends up with
> incorrect values as render context is restored and saved before updated
> by workarounds hence move them to render ring init fn. This should be
> ok as these workarounds are not related to display clock gating.
> 
> Signed-off-by: Arun Siluvery <arun.siluv...@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c     | 46 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_pm.c         | 59 ----------------------------
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 68 
> +++++++++++++++++++++++++++++++++
>  3 files changed, 114 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 083683c..cf7da30 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2397,20 +2397,65 @@ static int i915_shared_dplls_info(struct seq_file *m, 
> void *unused)
>               seq_printf(m, " dpll_md: 0x%08x\n", pll->hw_state.dpll_md);
>               seq_printf(m, " fp0:     0x%08x\n", pll->hw_state.fp0);
>               seq_printf(m, " fp1:     0x%08x\n", pll->hw_state.fp1);
>               seq_printf(m, " wrpll:   0x%08x\n", pll->hw_state.wrpll);
>       }
>       drm_modeset_unlock_all(dev);
>  
>       return 0;
>  }
>  
> +static int i915_workaround_info(struct seq_file *m, void *unused)
> +{
> +     struct drm_info_node *node = (struct drm_info_node *) m->private;
> +     struct drm_device *dev = node->minor->dev;
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     int ret;
> +
> +     ret = mutex_lock_interruptible(&dev->struct_mutex);
> +     if (ret)
> +             return ret;
> +
> +     if (IS_BROADWELL(dev)) {
> +             seq_printf(m, "GEN8_ROW_CHICKEN:\t0x%08x\n",
> +                        I915_READ(GEN8_ROW_CHICKEN));
> +             seq_printf(m, "HALF_SLICE_CHICKEN3:\t0x%08x\n",
> +                        I915_READ(HALF_SLICE_CHICKEN3));
> +             seq_printf(m, "GAMTARBMODE:\t0x%08x\n", I915_READ(GAMTARBMODE));
> +             seq_printf(m, "_3D_CHICKEN3:\t0x%08x\n",
> +                        I915_READ(_3D_CHICKEN3));
> +             seq_printf(m, "COMMON_SLICE_CHICKEN2:\t0x%08x\n",
> +                        I915_READ(COMMON_SLICE_CHICKEN2));
> +             seq_printf(m, "GEN7_HALF_SLICE_CHICKEN1:\t0x%08x\n",
> +                        I915_READ(GEN7_HALF_SLICE_CHICKEN1));
> +             seq_printf(m, "GEN7_ROW_CHICKEN2:\t0x%08x\n",
> +                        I915_READ(GEN7_ROW_CHICKEN2));
> +             seq_printf(m, "GAM_ECOCHK:\t0x%08x\n",
> +                        I915_READ(GAM_ECOCHK));
> +             seq_printf(m, "HDC_CHICKEN0:\t0x%08x\n",
> +                        I915_READ(HDC_CHICKEN0));
> +             seq_printf(m, "GEN7_FF_THREAD_MODE:\t0x%08x\n",
> +                        I915_READ(GEN7_FF_THREAD_MODE));
> +             seq_printf(m, "GEN8_UCGCTL6:\t0x%08x\n",
> +                        I915_READ(GEN8_UCGCTL6));
> +             seq_printf(m, "GEN6_RC_SLEEP_PSMI_CONTROL:\t0x%08x\n",
> +                        I915_READ(GEN6_RC_SLEEP_PSMI_CONTROL));
> +             seq_printf(m, "CACHE_MODE_1:\t0x%08x\n",
> +                        I915_READ(CACHE_MODE_1));
> +     } else
> +             DRM_DEBUG_DRIVER("Not available for Gen%d\n",
> +                              INTEL_INFO(dev)->gen);
> +
> +     mutex_unlock(&dev->struct_mutex);
> +     return 0;
> +}
> +

This smells like a separate patch. But I'm not sure we want at all since
intel_reg_read will provide the same information.

>  struct pipe_crc_info {
>       const char *name;
>       struct drm_device *dev;
>       enum pipe pipe;
>  };
>  
>  static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
>  {
>       struct pipe_crc_info *info = inode->i_private;
>       struct drm_i915_private *dev_priv = info->dev->dev_private;
> @@ -3904,20 +3949,21 @@ static const struct drm_info_list i915_debugfs_list[] 
> = {
>       {"i915_ppgtt_info", i915_ppgtt_info, 0},
>       {"i915_llc", i915_llc, 0},
>       {"i915_edp_psr_status", i915_edp_psr_status, 0},
>       {"i915_sink_crc_eDP1", i915_sink_crc, 0},
>       {"i915_energy_uJ", i915_energy_uJ, 0},
>       {"i915_pc8_status", i915_pc8_status, 0},
>       {"i915_power_domain_info", i915_power_domain_info, 0},
>       {"i915_display_info", i915_display_info, 0},
>       {"i915_semaphore_status", i915_semaphore_status, 0},
>       {"i915_shared_dplls_info", i915_shared_dplls_info, 0},
> +     {"i915_workaround_info", i915_workaround_info, 0},
>  };
>  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
>  
>  static const struct i915_debugfs_files {
>       const char *name;
>       const struct file_operations *fops;
>  } i915_debugfs_files[] = {
>       {"i915_wedged", &i915_wedged_fops},
>       {"i915_max_freq", &i915_max_freq_fops},
>       {"i915_min_freq", &i915_min_freq_fops},
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3f88f29..9597f95 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5393,101 +5393,42 @@ static void gen8_init_clock_gating(struct drm_device 
> *dev)
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       enum pipe pipe;
>  
>       I915_WRITE(WM3_LP_ILK, 0);
>       I915_WRITE(WM2_LP_ILK, 0);
>       I915_WRITE(WM1_LP_ILK, 0);
>  
>       /* FIXME(BDW): Check all the w/a, some might only apply to
>        * pre-production hw. */
>  
> -     /* WaDisablePartialInstShootdown:bdw */
> -     I915_WRITE(GEN8_ROW_CHICKEN,
> -                _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE));
> -
> -     /* WaDisableThreadStallDopClockGating:bdw */
> -     /* FIXME: Unclear whether we really need this on production bdw. */
> -     I915_WRITE(GEN8_ROW_CHICKEN,
> -                _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE));
> -
> -     /*
> -      * This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for
> -      * pre-production hardware
> -      */
> -     I915_WRITE(HALF_SLICE_CHICKEN3,
> -                _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS));
> -     I915_WRITE(HALF_SLICE_CHICKEN3,
> -                _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS));
> -     I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE));
> -
> -     I915_WRITE(_3D_CHICKEN3,
> -                
> _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2)));
> -
> -     I915_WRITE(COMMON_SLICE_CHICKEN2,
> -                _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE));
> -
> -     I915_WRITE(GEN7_HALF_SLICE_CHICKEN1,
> -                _MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE));
> -
> -     /* WaDisableDopClockGating:bdw May not be needed for production */
> -     I915_WRITE(GEN7_ROW_CHICKEN2,
> -                _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE));
> -
> -     /* WaSwitchSolVfFArbitrationPriority:bdw */
> -     I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
>  
>       /* WaPsrDPAMaskVBlankInSRD:bdw */
>       I915_WRITE(CHICKEN_PAR1_1,
>                  I915_READ(CHICKEN_PAR1_1) | DPA_MASK_VBLANK_SRD);
>  
>       /* WaPsrDPRSUnmaskVBlankInSRD:bdw */
>       for_each_pipe(pipe) {
>               I915_WRITE(CHICKEN_PIPESL_1(pipe),
>                          I915_READ(CHICKEN_PIPESL_1(pipe)) |
>                          BDW_DPRS_MASK_VBLANK_SRD);
>       }
>  
> -     /* Use Force Non-Coherent whenever executing a 3D context. This is a
> -      * workaround for for a possible hang in the unlikely event a TLB
> -      * invalidation occurs during a PSD flush.
> -      */
> -     I915_WRITE(HDC_CHICKEN0,
> -                I915_READ(HDC_CHICKEN0) |
> -                _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT));
> -
> -     /* WaVSRefCountFullforceMissDisable:bdw */
> -     /* WaDSRefCountFullforceMissDisable:bdw */
> -     I915_WRITE(GEN7_FF_THREAD_MODE,
> -                I915_READ(GEN7_FF_THREAD_MODE) &
> -                ~(GEN8_FF_DS_REF_CNT_FFME | GEN7_FF_VS_REF_CNT_FFME));
> -
>       /*
>        * BSpec recommends 8x4 when MSAA is used,
>        * however in practice 16x4 seems fastest.
>        *
>        * Note that PS/WM thread counts depend on the WIZ hashing
>        * disable bit, which we don't touch here, but it's good
>        * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
>        */
>       I915_WRITE(GEN7_GT_MODE,
>                  GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);

GT_MODE doesn't get clobbered by a reset? I would have expected it to
behave the same way as CACHE_MODE_{0,1).

> -
> -     I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL,
> -                _MASKED_BIT_ENABLE(GEN8_RC_SEMA_IDLE_MSG_DISABLE));
> -
> -     /* WaDisableSDEUnitClockGating:bdw */
> -     I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> -                GEN8_SDEUNIT_CLOCK_GATE_DISABLE);

And I would have expected the UCGCTL registers to not be clobbered. IIRC
they didn't get clobbered on my IVB, but my memory might be wrong here.

> -
> -     /* Wa4x4STCOptimizationDisable:bdw */
> -     I915_WRITE(CACHE_MODE_1,
> -                _MASKED_BIT_ENABLE(GEN8_4x4_STC_OPTIMIZATION_DISABLE));
>  }
>  
>  static void haswell_init_clock_gating(struct drm_device *dev)
>  {
>       struct drm_i915_private *dev_priv = dev->dev_private;
>  
>       ilk_init_lp_watermarks(dev);
>  
>       /* L3 caching of data atomics doesn't work -- disable it. */
>       I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index b3d8f76..6818e34 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -591,20 +591,85 @@ init_pipe_control(struct intel_engine_cs *ring)
>       return 0;
>  
>  err_unpin:
>       i915_gem_object_ggtt_unpin(ring->scratch.obj);
>  err_unref:
>       drm_gem_object_unreference(&ring->scratch.obj->base);
>  err:
>       return ret;
>  }
>  
> +static void bdw_init_workarounds(struct drm_device *dev)
> +{
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +     /* WaDisablePartialInstShootdown:bdw */
> +     I915_WRITE(GEN8_ROW_CHICKEN,
> +                _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE));
> +
> +     /* WaDisableThreadStallDopClockGating:bdw */
> +     /* FIXME: Unclear whether we really need these on production bdw. */
> +     I915_WRITE(GEN8_ROW_CHICKEN,
> +                _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE));
> +
> +     /*
> +      * This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for
> +      * pre-production hardware
> +      */
> +     I915_WRITE(HALF_SLICE_CHICKEN3,
> +                _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS));
> +     I915_WRITE(HALF_SLICE_CHICKEN3,
> +                _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS));
> +     I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE));
> +
> +     I915_WRITE(_3D_CHICKEN3,
> +                
> _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2)));
> +
> +     I915_WRITE(COMMON_SLICE_CHICKEN2,
> +                _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE));
> +
> +     I915_WRITE(GEN7_HALF_SLICE_CHICKEN1,
> +                _MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE));
> +
> +     /* WaDisableDopClockGating:bdw May not be needed for production */
> +     I915_WRITE(GEN7_ROW_CHICKEN2,
> +                _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE));
> +
> +     /* WaSwitchSolVfFArbitrationPriority:bdw */
> +     I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
> +
> +     /* Use Force Non-Coherent whenever executing a 3D context. This is a
> +      * workaround for for a possible hang in the unlikely event a TLB
> +      * invalidation occurs during a PSD flush.
> +      */
> +     I915_WRITE(HDC_CHICKEN0,
> +                I915_READ(HDC_CHICKEN0) |

You might kill this read since it's a masked register. Maybe as a
separate prep-patch.

> +                _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT));
> +
> +     /* WaVSRefCountFullforceMissDisable:bdw */
> +     /* WaDSRefCountFullforceMissDisable:bdw */
> +     I915_WRITE(GEN7_FF_THREAD_MODE,
> +                I915_READ(GEN7_FF_THREAD_MODE) &
> +                ~(GEN8_FF_DS_REF_CNT_FFME | GEN7_FF_VS_REF_CNT_FFME));
> +
> +     /* WaDisableSDEUnitClockGating:bdw */
> +     I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> +                GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
> +
> +     I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL,
> +                _MASKED_BIT_ENABLE(GEN8_RC_SEMA_IDLE_MSG_DISABLE));
> +
> +     /* Wa4x4STCOptimizationDisable:bdw */
> +     I915_WRITE(CACHE_MODE_1,
> +                _MASKED_BIT_ENABLE(GEN8_4x4_STC_OPTIMIZATION_DISABLE));

I think it would be nice to reorder this function a bit to keep similar 
registers
close to each other. The current mismash makes it easy to overlook a
workaround and then we may end up with duplicates (wouldn't be the first
time that happened).

> +}
> +
>  static int init_render_ring(struct intel_engine_cs *ring)
>  {
>       struct drm_device *dev = ring->dev;
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       int ret = init_ring_common(ring);
>       if (ret)
>               return ret;
>  
>       /* WaTimedSingleVertexDispatch:cl,bw,ctg,elk,ilk,snb */
>       if (INTEL_INFO(dev)->gen >= 4 && INTEL_INFO(dev)->gen < 7)
> @@ -646,20 +711,23 @@ static int init_render_ring(struct intel_engine_cs 
> *ring)
>               I915_WRITE(CACHE_MODE_0,
>                          _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
>       }
>  
>       if (INTEL_INFO(dev)->gen >= 6)
>               I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING));
>  
>       if (HAS_L3_DPF(dev))
>               I915_WRITE_IMR(ring, ~GT_PARITY_ERROR(dev));
>  
> +     if (IS_BROADWELL(dev))
> +             bdw_init_workarounds(dev);
> +
>       return ret;
>  }
>  
>  static void render_ring_cleanup(struct intel_engine_cs *ring)
>  {
>       struct drm_device *dev = ring->dev;
>       struct drm_i915_private *dev_priv = dev->dev_private;
>  
>       if (dev_priv->semaphore_obj) {
>               i915_gem_object_ggtt_unpin(dev_priv->semaphore_obj);
> -- 
> 1.9.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to