[PATCH] [RFC] drm: mxsfb: Implement LCDIF scanout CRC32 support

[Marek Vasut]

On 2/7/22 10:18, Liu Ying wrote:

Hi,

>>> On Sun, 2022-02-06 at 19:56 +0100, Marek Vasut wrote:
>>>> The LCDIF controller as present in i.MX6SX/i.MX8M Mini/Nano has a
>>>> CRC_STAT
>>>> register, which contains CRC32 of the frame as it was clocked out
>>>> of the
>>>> DPI interface of the LCDIF. This is likely meant as a functional
>>>> safety
>>>> register.
>>>>
>>>> Unfortunatelly, there is zero documentation on how the CRC32 is
>>>> calculated,
>>>> there is no documentation of the polynomial, the init value, nor
>>>> on which
>>>> data is the checksum applied.
>>>>
>>>> By applying brute-force on 8 pixel / 2 line frame, which is the
>>>> minimum
>>>> size LCDIF would work with, it turns out the polynomial is
>>>> CRC32_POLY_LE
>>>> 0xedb88320 , init value is 0xffffffff , the input data are
>>>> bitrev32()
>>>> of the entire frame and the resulting CRC has to be also
>>>> bitrev32()ed.
>>>
>>> No idea how the HW calculates the CRC value.
>>> I didn't hear anyone internal tried this feature.
>>
>> It would be nice if the datasheet could be improved.
> 
> Agreed.
> 
>>
>> There are many blank areas which are undocumented, this LCDIF CRC32
>> feature, i.MX8M Mini Arteris NOC at 0x32700000 , the ARM GPV NIC-301
>> at
>> 0x32{0,1,2,3,4,5,6,8}00000 and their master/slave port mapping. The
>> NOC
>> and NICs were documented at least up to i.MX6QP and then that
>> information disappeared from NXP datasheets. I think reconfiguring
>> the
>> NOC/NIC QoS would help mitigate this shift issue described below (*).
> 
> I also think the QoS would help if it is configureable.

It is programmable, it's just the port mapping which is undocumented.

>> Do you know if there is some additional NOC/NIC documentation for
>> i.MX8M
>> Mini available ?
> 
> No.

Can you ask someone internally in NXP maybe ?

>>>> Doing this calculation in software for each frame is unrealistic
>>>> due to
>>>> the CPU demand, implement at least a sysfs attribute which
>>>> permits testing
>>>> the current frame on demand.
>>>
>>> Why not using the existing debugfs CRC support implemented
>>> in drivers/gpu/drm/drm_debugfs_crc.c?
>>
>> I wasn't aware of that, thanks.
> 
> No problem.
> 
>>
>>>> Unfortunatelly, this functionality has another problem. On all of
>>>> those SoCs,
>>>> it is possible to overload interconnect e.g. by concurrent USB
>>>> and uSDHC
>>>> transfers, at which point the LCDIF LFIFO suffers an UNDERFLOW
>>>> condition,
>>>> which results in the image being shifted to the right by exactly
>>>> LFIFO size
>>>> pixels. On i.MX8M Mini, the LFIFO is 76x256 bits = 2432 Byte ~=
>>>> 810 pixel
>>>> at 24bpp. In this case, the LCDIF does not assert UNDERFLOW_IRQ
>>>> bit, the
>>>> frame CRC32 indicated in CRC_STAT register matches the CRC32 of
>>>> the frame
>>>> in DRAM, the RECOVER_ON_UNDERFLOW bit has no effect, so if this
>>>> mode of
>>>> failure occurs, the failure gets undetected and uncorrected.
>>>
>>> Hmmm, interesting, no UNDERFLOW_IRQ bit asserted when LCDIF suffers
>>> an
>>> UNDERFLOW condition?
>>
>> Yes
> 
> Did you ever see UNDERFLOW_IRQ bit asserted in any case?

I didn't see the UNDERFLOW_IRQ bit asserted during my tests, either with 
this IRQ enabled (UNDERFLOW_IRQ_EN=1) or with the IRQ disabled 
(UNDERFLOW_IRQ_EN=0) by reading the CTRL1 register in interrupt handler 
when CUR_FRAME_DONE_IRQ triggered the IRQ handler.

I did see a few auto-recoveries of the panel back into non-shifted 
image, that happened once in some 100-200 tests. Mostly the LCDIF does 
not recover automatically.