[Intel-gfx] [External] Re: drm/i915/adl_p: Increase CDCLK by 15% if PSR2 is used

[Mark Pearson]

Thanks Stanislav,

On 3/22/22 10:18, Lisovskiy, Stanislav wrote:
> On Tue, Mar 22, 2022 at 09:55:35AM -0400, Mark Pearson wrote:
>> Hi,
>>
>> On 3/21/22 06:49, Stanislav Lisovskiy wrote:
>>> We are currently getting FIFO underruns, in particular
>>> when PSR2 is enabled. There seem to be no existing workaround
>>> or patches, which can fix that issue(were expecting some recent
>>> selective fetch update and DBuf bw/SAGV fixes to help,
>>> which unfortunately didn't).
>>> Current idea is that it looks like for some reason the
>>> DBuf prefill time isn't enough once we exit PSR2, despite its
>>> theoretically correct.
>>> So bump it up a bit by 15%(minimum experimental amount required
>>> to get it working), if PSR2 is enabled.
>>> For PSR1 there is no need in this hack, so we limit it only
>>> to PSR2 and Alderlake.
>>>
>>> v2: - Added comment(Jose Souza)
>>>     - Fixed 15% calculation(Jose Souza)
>>>
>>> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_cdclk.c | 26 ++++++++++++++++++++++
>>>  1 file changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>>> index 8888fda8b701..92d57869983a 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>>> @@ -2325,6 +2325,32 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>>>  					dev_priv->max_cdclk_freq));
>>>  	}
>>>  
>>> +
>>> +	/*
>>> +	 * HACK.  We are getting FIFO underruns, in particular
>>> +	 * when PSR2 is enabled. There seem to be no existing workaround
>>> +	 * or patches as of now.
>>> +	 * Current idea is that it looks like for some reason the
>>> +	 * DBuf prefill time isn't enough once we exit PSR2, despite its
>>> +	 * theoretically correct.
>>> +	 * So bump it up a bit by 15%(minimum experimental amount required
>>> +	 * to get it working), if PSR2 is enabled.
>>> +	 * For PSR1 there is no need in this hack, so we limit it only
>>> +	 * to PSR2 and Alderlake.
>>> +	 */
>>> +	if (IS_ALDERLAKE_P(dev_priv)) {
>>> +		struct intel_encoder *encoder;
>>> +
>>> +		for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
>>> +			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>>> +
>>> +			if (intel_dp->psr.psr2_enabled) {
>>> +				min_cdclk = DIV_ROUND_UP(min_cdclk * 115, 100);
>>> +				break;
>>> +			}
>>> +		}
>>> +	}
>>> +
>>>  	if (min_cdclk > dev_priv->max_cdclk_freq) {
>>>  		drm_dbg_kms(&dev_priv->drm,
>>>  			    "required cdclk (%d kHz) exceeds max (%d kHz)\n",
>>
>> Not sure if this will get thru as I'm not subscribed to this list but I
>> tested the patch below on my X1 Yoga 7 (Intel ADL-P with Step C0
>> silicon) and it didn't fix some screen tearing issues I'm seeing there
>> that are PSR2 related
>>
>> I also tried increasing the timeout to *300 to see if that made any
>> difference and it didn't.
>>
>> Let me know if there's anything else I can try out - I have a couple of
>> ADL-P machines I can test against and both are seeing screen tearing
> 
> Are you getting this screen tearing because of the FIFO underruns?
> Those should be visible in dmesg. This patch can fix only part of underrun
> issues caused by PSR2. 
> If your screen tearing caused by PSR2, but there are no underruns that 
> patch won't help for sure.
> 
Ah - OK, no FIFO underruns that I've noticed in the logs but I was
hoping the two were connected. I'll keep an eye out for those in the
meantime.

I guess I'll just wave a flag and say I'm seeing PSR2 related tearing
issues. If I disable  PSR2 selective fetch it goes away
(i915.enable_psr2_sel_fetch=0) - but as that's a different issue to this
patch thread I don't want to drag the conversation too far sideways.

Thanks!
Mark