Re: [PATCH 2/2] drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet

[Marek Vasut <marex@denx.de>]

On 8/2/23 15:08, neil.armstrong@linaro.org wrote:
> Hi Marek,
> 
> On 02/08/2023 14:25, Marek Vasut wrote:
>> On 8/2/23 10:39, neil.armstrong@linaro.org wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On 13/07/2023 20:28, Marek Vasut wrote:
>>>
>>> <snip>
>>>
>>>>>>
>>>>>> MIPI_DSI_MODE_VIDEO_NO_HFP means the HBP period is just skipped by 
>>>>>> DSIM.
>>>>>>
>>>>>> Maybe there is a need for new set of flags which differentiate 
>>>>>> between HBP skipped (i.e. NO HBP) and HBP LP11 ?
>>>>>>
>>>>>
>>>>> No, the section of the MIPI DSI spec I posted below clearly states 
>>>>> there are two options:
>>>>>
>>>>> 1) send blanking packets during those periods
>>>>> 2) transition to LP11 during those periods
>>>>>
>>>>> There is no 3rd option in the spec of not doing both like what you 
>>>>> are suggesting. So DSIM should also be only transitioning to LP11 
>>>>> during those periods if its not sending the blanking packets with 
>>>>> those flags set.
>>>>>
>>>>> So, there is no need for any new set of flags to differentiate.
>>>>>
>>>>> The flags and their interpretation is correct in MSM driver. I 
>>>>> cannot comment on what exactly DSIM does with those flags.
>>>>
>>>> How do you explain the comment in include/drm/drm_mipi_dsi.h:
>>>>
>>>> 128 /* disable hback-porch area */
>>>> 129 #define MIPI_DSI_MODE_VIDEO_NO_HBP      BIT(6)
>>>
>>> Can you specify how you determined those flags were needed on DSIM ? 
>>> a vendor tree ? a datasheet ?
>>
>> The following upstream commit:
>>
>> 996e1defca344 ("drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags")
>>
>>> In the meantime, we should revert this patch because it regresses 
>>> some Qcom
>>> based platforms until we figure out what's missing to make DSIM based 
>>> boards
>>> happy.
>>>
>>> I'll send a revert change afterwards.
>>
>> That change would break existing use case on i.MX8M then, I disagree 
>> with that revert.
> 
> As I understand the timeline is :
> 
> - 996e1defca344 was merged in v6.2-rc2 and caused regression on NXP 
> platforms
> 
> - 8ddce13ae696 was merged in v6.5-rc1 to fix that but caused regression 
> on QCOM platforms
> 
> Did I miss something ?

That looks about right.

> I don't know how to handle this apart reverting 8ddce13ae696 and trying 
> to find a proper fix that doesn't regress QCOM.

I provided a suggestion above -- I believe QCOM is misinterpreting the 
NO_H* flags and it needs separate flags for its behavior. The NXP 
hardware per MX8M{M,N,P} reference manual (which is available at 
NXP.com) skips the H* areas in the transfer, which matches the flags 
description:

include/drm/drm_mipi_dsi.h-/* disable hback-porch area */
include/drm/drm_mipi_dsi.h:#define MIPI_DSI_MODE_VIDEO_NO_HBP   BIT(6)

If the QCOM hardware does something else, it should introduce its own 
set of flags for that something else and that would be problem solved, 
for both platforms.

I don't have access to the QCOM hardware or datasheet however, is either 
available ?

> So, The main issue is around the real meaning of the 
> IPI_DSI_MODE_VIDEO_NO_* flags,
> Exynos DRM removed the HSA, HBP and HFP packets, Qcom DSI moves the DSI 
> lanes
> state to LP-11 during the period.
> 
> The behavior is significantly different and the naming doesn't suggest any
> correct behavior.
> 
> The only solution is to find out why :
> - On Qcom platforms, having the HSA, HBP and HFP periods is OK, but not 
> on DSIM
> - On DSIM, removing the HSA, HBP and HFP periods is fine
> - What's the exact requirement of the lt9611 bridge concerning those 
> periods

See above.