Re: [RFC] drm/i915/bdw: Initialize BDW workarounds in render ring init fn

Siluvery, Arun
2014-07-28T20:41:21Z

On 28/07/2014 20:22, Daniel Vetter wrote:
> On Mon, Jul 28, 2014 at 08:00:39PM +0300, Ville Syrjälä wrote:
>> On Mon, Jul 28, 2014 at 05:31:46PM +0100, arun.siluvery@linux.intel.com wrote:
>>> From: Arun Siluvery <arun.siluvery@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.siluvery@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.
>
> Yeah, debugfs files that just do what intel_reg_read does are just an
> additional maintaince burden. I know that we have a few that dump lots of
> registers, but most of them dump a lot of other information, too.
> -Daniel
>

I've added this mainly for testing workarounds which can be extended
further as we move WAs for other chipsets but I agree it can be done
with intel_reg_read.

regards
Arun