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

[Marek Vasut <marex@denx.de>]

On 7/9/23 03:03, Abhinav Kumar wrote:
> 
> 
> On 7/7/2023 1:47 AM, Neil Armstrong wrote:
>> On 07/07/2023 09:18, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On 06/07/2023 11:20, Amit Pundir wrote:
>>>> On Wed, 5 Jul 2023 at 11:09, Dmitry Baryshkov
>>>> <dmitry.baryshkov@linaro.org> wrote:
>>>>>
>>>>> [Adding freedreno@ to cc list]
>>>>>
>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki 
>>>>> <jagan@amarulasolutions.com> wrote:
>>>>>>
>>>>>> Hi Amit,
>>>>>>
>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>>> <amit.pundir@linaro.org> wrote:
>>>>>>>
>>>>>>> Hi Marek,
>>>>>>>
>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <marex@denx.de> wrote:
>>>>>>>>
>>>>>>>> Do not generate the HS front and back porch gaps, the HSA gap and
>>>>>>>> EOT packet, as these packets are not required. This makes the 
>>>>>>>> bridge
>>>>>>>> work with Samsung DSIM on i.MX8MM and i.MX8MP.
>>>>>>>
>>>>>>> This patch broke display on Dragonboard 845c (SDM845) devboard 
>>>>>>> running
>>>>>>> AOSP. This is what I see
>>>>>>> https://people.linaro.org/~amit.pundir/db845c-userdebug/v6.5-broken-display/PXL_20230704_150156326.jpg.
>>>>>>> Reverting this patch fixes this regression for me.
>>>>>>
>>>>>> Might be msm dsi host require proper handling on these updated
>>>>>> mode_flags? did they?
>>>>>
>>>>> The msm DSI host supports those flags. Also, I'd like to point out
>>>>> that the patch didn't change the rest of the driver code. So even if
>>>>> drm/msm ignored some of the flags, it should not have caused the
>>>>> issue. Most likely the issue is on the lt9611 side. I's suspect that
>>>>> additional programming is required to make it work with these flags.
>>>>
>>>> I spent some time today on smoke testing these flags (individually and
>>>> in limited combination) on DB845c, to narrow down this breakage to one
>>>> or more flag(s) triggering it. Here are my observations in limited
>>>> testing done so far.
>>>>
>>>> There is no regression with MIPI_DSI_MODE_NO_EOT_PACKET when enabled
>>>> alone and system boots to UI as usual.
>>>>
>>>> MIPI_DSI_MODE_VIDEO_NO_HFP always trigger the broken display as in the
>>>> screenshot[1] shared earlier as well.
>>>>
>>>> Adding either of MIPI_DSI_MODE_VIDEO_NO_HSA and
>>>> MIPI_DSI_MODE_VIDEO_NO_HBP always result in no display, unless paired
>>>> with MIPI_DSI_MODE_VIDEO_NO_HFP and in that case we get the broken
>>>> display as reported.
>>>>
>>>> In short other than MIPI_DSI_MODE_NO_EOT_PACKET flag, all other flags
>>>> added in this commit break the display on DB845c one way or another.
>>>
>>> I think the investigation would be to understand why samsung-dsim 
>>> requires
>>> such flags and/or what are the difference in behavior between MSM DSI 
>>> and samsung DSIM
>>> for those flags ?
>>>
>>> If someone has access to the lt9611 datasheet, so it requires 
>>> HSA/HFP/HBP to be
>>> skipped ? and does MSM DSI and samsung DSIM skip them in the same way ?
>>
>> I think there's a mismatch, where on one side this flags sets the link 
>> in LP-11 while
>> in HSA/HFP/HPB while on the other it completely removes those blanking 
>> packets.
>>
>> The name MIPI_DSI_MODE_VIDEO_NO_HBP suggests removal of HPB, not LP-11 
>> while HPB.
>> the registers used in both controllers are different:
>> - samsung-dsim: DSIM_HBP_DISABLE_MODE
>> - msm dsi: DSI_VID_CFG0_HBP_POWER_STOP
>>
>> The first one suggest removing the packet, while the second one 
>> suggests powering
>> off the line while in the blanking packet period.
>>
>> @Abhinav, can you comment on that ?
>>
> 
> I dont get what it means by completely removes blanking packets in DSIM.

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 ?

> It should be replacing those periods with LP11 too.
> 
> The traffic mode being used on this bridge is 
> MIPI_DSI_MODE_VIDEO_SYNC_PULSE which is "Non-Burst Mode with Sync Pulses".
> 
> As per this traffic mode in the DSI spec,
> 
> "Normally, periods shown as HSA (Horizontal Sync Active), HBP 
> (Horizontal Back Porch) and HFP (Horizontal Front Porch) are filled by 
> Blanking Packets, with lengths (including packet overhead)  calculated 
> to match the period specified by the peripheral’s data sheet. 
> Alternatively, if there is sufficient time to transition from HS to LP 
> mode and back again, a timed interval in LP mode may substitute for a 
> Blanking Packet, thus saving power. During HSA, HBP and HFP periods, the 
> bus should stay in the LP-11 state."
> 
> So we can either send the blanking packets or transition to LP state and 
> those 3 flags are controlling exactly that during those periods for MSM 
> driver.
> 
> If you stop sending the blanking packets, you need to replace that gap 
> with LP.

I don't think that's what MIPI_DSI_MODE_VIDEO_NO_HBP means, the way I 
understand MIPI_DSI_MODE_VIDEO_NO_HBP is that it skips the HBP 
completely. So if you want HBP, then do not set 
MIPI_DSI_MODE_VIDEO_NO_HBP . And if you want LP11 during HBP, that is I 
think up to the controller (or maybe another new flag?).

> One reason I can think of why this could break with MSM is perhaps we do 
> not have sufficient time in those periods for the LP-HS transition like 
> the spec has written.
> 
> 1) What is the resolution which is getting broken on db845c with this? I 
> would like to know the full drm_display_mode for it to see how much time 
> we have in those periods. Is any resolution working or all are broken.
> 
> 2) I also do not completely get the last line of the DSI spec on this 
> traffic mode. Is it suggesting that we *must* use only LP11 for those 
> periods in this traffic mode? I need to check little more on that. 
> Because if thats the case the change is doing just that and we need to 
> investigate the MSM failure little more. If not and its indeed optional 
> to save power like the DSI spec says, then its weird why DSIM should be 
> blank without that too.

[...]