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

[Marek Vasut <marex@denx.de>]

On 11/1/22 17:51, Marco Felsch wrote:
> On 22-11-01, Marek Vasut wrote:
>> 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.
> 
> According our patch it is:
> 
> #define HDMI_TX_CONTROL0       0x200
> #define HDMI_LCDIF_NOC_HURRY_PRIO_MASK GENMASK(14, 12)
> #define HDMI_LCDIF_NOC_HURRY_PRIO(p)   (((p) << 12) & HDMI_LCDIF_NOC_HURRY_PRIO_MASK)
> 
> and we set it within imx8mp_hdmi_blk_ctrl_power_on() by:
> 
> regmap_set_bits(bc->regmap, HDMI_TX_CONTROL0,
>                  HDMI_LCDIF_NOC_HURRY_PRIO(7));
> 
> imx8mp_hdmi_blk_ctrl_power_off():
> 
> regmap_clear_bits(bc->regmap, HDMI_TX_CONTROL0,
>                  HDMI_LCDIF_NOC_HURRY_PRIO(7));

Hmmm, bc->regmap does not exist in either upstream or downstream ATF 
versions per git grep, maybe you can give me the exact physical address 
of the register you would like me to read?