[PATCH] drm: lcdif: Set and enable FIFO Panic threshold

[Marek Vasut]

On 11/1/22 17:06, Marco Felsch wrote:

Hi,

>>> Also I understood the thresholds in such a way, that the FIFO watermark
>>> must be higher but there is no place left when it is set to 3/3. In such
>>> case this means that the PANIC will never left once it was entered.
>>
>> I think this part is wrong.
>>
>> Consider that the FIFO fill drops below 2/3 so PANIC signal asserts.
> 
> ? I thought the PANIC is triggered if the FIFO drops below the 1/3
> threshold and is active till the 2/3 threshold.

Yes, although I think the ASSERT/DEASSERT are one-way switches.

>> After a bit of time, the FIFO fill reaches full 3/3 (maybe during
>> blanking period, where the data can be read in quickly without being
>> scanned out again), and the PANIC signal de-asserts.
>>
>> So the LCDIF won't be in constant PANIC asserted, but it will be there for
>> quite a bit longer.
>>
>>>>> It also seems to me that tuning these thresholds might be related to
>>>>> some special use-case of the SoC, and it is most likely not just the
>>>>> LCDIF thresholds which have been adjusted in such case, I would
>>>>> expect
>>>>> the NOC and GPV NIC priorities to be adjusted at that point too.
>>>
>>> As far as I understood, the PANIC signal triggers the NOC to use the
>>> PANIC signal priorities instead of the normal ones. I found a patch
>>> laying in our downstream [1] repo which configures the threshold. This
>>> raises the question which PANIC prio do you use? Do you have a patch for
>>> this? If I remember correctly some TF-A's like the NXP downstream one
>>> configure this but the upstream TF-A don't. Which TF-A do you use?
>>
>> Upstream 2.6 or 2.7 , so this tuning does not apply.
> 
> So your panic priority is what?

If you tell me which register (physical address) to read, I will do that 
on this board right now.

>>>>> So unless there are further details for that use-case which justify
>>>>> making this somehow configurable, then maybe we should just stick to
>>>>> 1/3 and 2/3 for now. And once there is a valid use-case which does
>>>>> justify making this configurable, then we can add the DT properties
>>>>> and all.
>>>>>
>>>>> What do you think ?
>>>>
>>>> No strong opinion from me on using LOW=1/3 and HIGH=2/3 thresholds for
>>>> now if they satisfy all current users of the upstream kernel.  Tuning
>>>> them in a certain way will be indeed needed once an usecase comes along
>>>> and still suffers from the FIFO underflows with those thresholds.
>>>
>>> I think that 1/3 and 2/3 should be fine for now too.
>>
>> All right, lemme re-test this, send V2, and then we can go from there.
> 
> Okay :)
> 
>> btw. can you resend that [PATCH] drm: lcdif: change burst size to 256B with
>> Fixes tag , so it can trickle into stable releases ?
> 
> Shure I will resend it with the tag added.

Thanks, I'll pick it via drm-misc then.