From mboxrd@z Thu Jan 1 00:00:00 1970 From: Deepak S Subject: Re: [PATCH] drm/i915/vlv: Remove check for Old Ack during forcewake Date: Sat, 20 Sep 2014 08:53:49 +0530 Message-ID: <541CF345.1000008@intel.com> References: <1411046510-13216-1-git-send-email-deepak.s@linux.intel.com> <20140918083925.GP12416@intel.com> <20140918115313.GO31703@phenom.ffwll.local> <20140918130816.GU12416@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id E209F6E443 for ; Thu, 18 Sep 2014 20:28:57 -0700 (PDT) In-Reply-To: <20140918130816.GU12416@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: =?windows-1252?Q?Ville_Syrj=E4l=E4?= , Daniel Vetter Cc: deepak.s@intel.com, daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org, deepak.s@linux.intel.com List-Id: intel-gfx@lists.freedesktop.org On Thursday 18 September 2014 06:38 PM, Ville Syrj=E4l=E4 wrote: > On Thu, Sep 18, 2014 at 01:53:13PM +0200, Daniel Vetter wrote: >> On Thu, Sep 18, 2014 at 11:39:25AM +0300, Ville Syrj=E4l=E4 wrote: >>> On Thu, Sep 18, 2014 at 06:51:50PM +0530, deepak.s@linux.intel.com wrot= e: >>>> From: Deepak S >>>> >>>> Based on the HW team inputs. We can should not wait for the old ack, >>>> Waiting for old ack might fail, when other forcewake came before the >>>> present one is desserted. >>>> >>>> for example, if forcewake bit 0 was set and before it could get cleared >>>> forcewake bit 1 got set, HW eventually clear bit 0, when the bit 1 >>>> is cleared. i.e, bit 1 is still sent then forcewake bit 0 will still be >>>> set. >>>> >>>> Signed-off-by: Deepak S >>>> --- >>>> drivers/gpu/drm/i915/intel_uncore.c | 10 ---------- >>>> 1 file changed, 10 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i91= 5/intel_uncore.c >>>> index 918b761..bcb0806 100644 >>>> --- a/drivers/gpu/drm/i915/intel_uncore.c >>>> +++ b/drivers/gpu/drm/i915/intel_uncore.c >>>> @@ -196,11 +196,6 @@ static void __vlv_force_wake_get(struct drm_i915_= private *dev_priv, >>>> { >>>> /* Check for Render Engine */ >>>> if (FORCEWAKE_RENDER & fw_engine) { >>>> - if (wait_for_atomic((__raw_i915_read32(dev_priv, >>>> - FORCEWAKE_ACK_VLV) & >>>> - FORCEWAKE_KERNEL) =3D=3D 0, >>>> - FORCEWAKE_ACK_TIMEOUT_MS)) >>>> - DRM_ERROR("Timed out: Render forcewake old ack to clear.\n"); >>>> = >>> Can we have a comment here? Something like this perhaps: >>> >>> /* >>> * WaRsDontPollForAckOnClearingFWBits:vlv >>> * Hardware clears ack bits lazily (only when all ack >>> * bits become 0) so don't poll for individiual ack >>> * bits to be clear here. >>> */ >>> >>> Apart from that: >>> Reviewed-by: Ville Syrj=E4l=E4 >> Shouldn't this be Cc: stable@vger.kernel.org? > Perhaps. But on the other hand LRI to display registers is busted on > VLV so the ddx doesn't do scanline waits which means it doesn't > poke at the forcewake bits either. So only the KERNEL bit gets poked > and so it should be quite impossible to encounter this issue with > current userspace. Which probably explains why no one has ever > reported it, or at least I don't remember seeing such a report > myself. > >> -Daniel Your right, with current userspace we should not see the issue. But if user= space starts using other forcewake bits, we might endup hanging the system due to wait for OLD Ack. Advice from HW t= eam was to avoid this check. Thanks Deepak >>>> __raw_i915_write32(dev_priv, FORCEWAKE_VLV, >>>> _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL)); >>>> @@ -214,11 +209,6 @@ static void __vlv_force_wake_get(struct drm_i915_= private *dev_priv, >>>> = >>>> /* Check for Media Engine */ >>>> if (FORCEWAKE_MEDIA & fw_engine) { >>>> - if (wait_for_atomic((__raw_i915_read32(dev_priv, >>>> - FORCEWAKE_ACK_MEDIA_VLV) & >>>> - FORCEWAKE_KERNEL) =3D=3D 0, >>>> - FORCEWAKE_ACK_TIMEOUT_MS)) >>>> - DRM_ERROR("Timed out: Media forcewake old ack to clear.\n"); >>>> = >>>> __raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_VLV, >>>> _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL)); >>>> -- = >>>> 1.9.1 >>>> >>>> _______________________________________________ >>>> Intel-gfx mailing list >>>> Intel-gfx@lists.freedesktop.org >>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >>> -- = >>> Ville Syrj=E4l=E4 >>> Intel OTC >> -- = >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch