[PATCH v8 01/14] drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags

[Frieder Schrempf]

On 14.11.22 04:16, Marek Vasut wrote:
> On 11/14/22 02:11, Nicolas Boichat wrote:
>> On Sun, Nov 13, 2022 at 8:29 AM Marek Vasut <marex at denx.de> wrote:
>>>
>>> On 11/11/22 13:12, Nicolas Boichat wrote:
>>>
>>> [...]
>>>
>>>>>> BTW, are you sure DSIM_HSE_MODE is correct now?
>>>>>
>>>>> Yes, we have tested in imx8m platforms as well. Sébastien Szymanski
>>>>> initially observed this issue on the imx8m platform.
>>>>
>>>> I'll repeat, are you sure about HSE specifically? You invert the
>>>> polarity for HBP, HFP, and HSA, which makes sense given your patch
>>>> 02/14.
>>>>
>>>> I'm concerned about HSE. Is the bit really a disable bit?
>>>> MIPI_DSI_MODE_VIDEO_HSE is supposed to be an enable flag, so you
>>>> should not do `reg |= DSIM_HSE_DISABLE;`, probably.
>>>
>>> I suspect the HSE bit is a misnomer, but its handling in the driver is
>>> correct.
>>>
>>> i.MX 8M Plus Applications Processor Reference Manual, Rev. 1, 06/2021
>>> Page 5436
>>>
>>> 23 HseDisableMode
>>>
>>> In Vsync pulse and Vporch area, MIPI DSI master transfers only Hsync
>>> start packet to MIPI DSI slave at MIPI DSI spec 1.1r02. This bit
>>> transfers Hsync end packet in Vsync pulse and Vporch area (optional).
>>>
>>> 0 = Disables transfer
>>> 1 = Enables transfer
>>>
>>> In command mode, this bit is ignored.
>>
>> Okay. I'd suggest adding a comment in the code, it'd be so tempting to
>> attempt to "fix" this as the if/or pattern looks different from the
>> others.
>>
>> But it's up to you all.
> 
> I agree. Clearly the discrepancy is confusing and leads to mistakes.

+1 for a comment in the code that explains the misnamed bit.

Otherwise:

Reviewed-By: Frieder Schrempf <frieder.schrempf at kontron.de>