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

[Marek Vasut]

On 11/1/22 15:04, Marco Felsch wrote:
> Hi Marek, Liu,

Hi,

[...]

>>>> Also IMHO the threshold should be taken wisely to not enter panic
>>>> mode
>>>> to early to not block others from the bus e.g. the GPU.
>>>
>>> As far as I understand the PANIC0_THRES, both thresholds are really
>>> watermarks in the FIFO, 0=EMPTY, 1/3=LOW, 2/3=HIGH, 3/3=FULL. Under
>>> normal conditions, the FIFO is filled above 1/3. When the FIFO fill
>>> drops below LOW=1/3, the "PANIC" signal is asserted so the FIFO can
>>> be
>>> refilled faster. When the FIFO fill raises above HIGH=2/3, the
>>> "PANIC"
>>> signal is deasserted so the FIFO refills at normal rate again.
> 
> This matches exactly my picture of the hardware.
> 
>>> It seems to me the LOW=1/3 and HIGH=2/3 thresholds are the kind of
>>> good
>>> balance. The LOW=2/3 and HIGH=FULL=3/3 seems like it would keep the
>>> "PANIC" signal asserted much longer, which could indeed block others
>>> from the bus.
> 
> 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. 
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 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.

btw. can you resend that [PATCH] drm: lcdif: change burst size to 256B 
with Fixes tag , so it can trickle into stable releases ?