From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Siluvery, Arun" Subject: Re: [RFC] drm/i915/bdw: Initialize BDW workarounds in render ring init fn Date: Mon, 28 Jul 2014 21:41:15 +0100 Message-ID: <53D6B56B.4010008@linux.intel.com> References: <1406565106-27522-1-git-send-email-arun.siluvery@linux.intel.com> <1406565106-27522-2-git-send-email-arun.siluvery@linux.intel.com> <20140728170039.GN27580@intel.com> <20140728192212.GJ4747@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 6C0DA6E320 for ; Mon, 28 Jul 2014 13:41:21 -0700 (PDT) In-Reply-To: <20140728192212.GJ4747@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter , =?ISO-8859-1?Q?Ville_Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 28/07/2014 20:22, Daniel Vetter wrote: > On Mon, Jul 28, 2014 at 08:00:39PM +0300, Ville Syrj=E4l=E4 wrote: >> On Mon, Jul 28, 2014 at 05:31:46PM +0100, arun.siluvery@linux.intel.com = wrote: >>> From: Arun Siluvery >>> >>> 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 >>> --- >>> 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_fi= le *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 =3D (struct drm_info_node *) m->private; >>> + struct drm_device *dev =3D node->minor->dev; >>> + struct drm_i915_private *dev_priv =3D dev->dev_private; >>> + int ret; >>> + >>> + ret =3D 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