Re: [PATCH] drm/i915: Wait old forcewake ack to clear on vlv

["S, Deepak" <deepak.s@intel.com>]

On 11/5/2014 3:43 PM, Mika Kuoppala wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
>
>> On Wed, Nov 05, 2014 at 10:18:46AM +0200, Mika Kuoppala wrote:
>>> Don't rush into getting new fw until the clearing
>>> of old one has been acked.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85684
>>> Tested-by: lu hua <huax.lu@intel.com>
>>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> This is just a revert of
>>   commit 5cb13c07dae73380d8b3ddc792740487b8742938
>>   Author: Deepak S <deepak.s@linux.intel.com>
>>   Date:   Thu Sep 18 18:51:50 2014 +0530
>>
>>      drm/i915/vlv: Remove check for Old Ack during forcewake
>>
>> except you left the comment behind.
> I failed at archeology. There is even nice comment from Deepak on
> top I managed to miss.
>
> But after reading this comment and the workaround description, I
> think that this WaRsDontPollForAckOnClearingFWBits is not valid for
> our use case.
>
> We use only one bit per engine, so our use case is not multithreaded
> in that sense.
>
> -Mika

I agree with you, in kernel we use only one bit for FW, but based on my 
understanding of forcewake implementation back to back forcewake should 
not cause any issue.
Don't you think adding old ack might be hiding some other issue? I think 
we need to root cause further.

As of now we can revert the patch & I will start root cause on my 
machine on why the issue is happening.

>> Would be nice to have some kind of real explanation what is going on
>> here. Deepak, any ideas?
>>
>>> ---
>>>   drivers/gpu/drm/i915/intel_uncore.c | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>>> index 9427641..5259b38 100644
>>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>>> @@ -204,6 +204,10 @@ 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) == 0, FORCEWAKE_ACK_TIMEOUT_MS))
>>> +			DRM_ERROR("Timed out: waiting for old Render ack to clear.\n");
>>> +
>>>   		__raw_i915_write32(dev_priv, FORCEWAKE_VLV,
>>>   				   _MASKED_BIT_ENABLE(FORCEWAKE_KERNEL));
>>>   
>>> @@ -217,6 +221,11 @@ 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) == 0, FORCEWAKE_ACK_TIMEOUT_MS))
>>> +			DRM_ERROR("Timed out: waiting for old media 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älä
>> Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx