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

[Abhinav Kumar]

On 7/13/2023 11:28 AM, Marek Vasut wrote:
> On 7/13/23 20:09, Abhinav Kumar wrote:
>>
>>
>> On 7/12/2023 10:41 AM, Marek Vasut wrote:
>>> 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 at linaro.org> wrote:
>>>>>>>>
>>>>>>>> [Adding freedreno@ to cc list]
>>>>>>>>
>>>>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki 
>>>>>>>> <jagan at amarulasolutions.com> wrote:
>>>>>>>>>
>>>>>>>>> Hi Amit,
>>>>>>>>>
>>>>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>>>>>> <amit.pundir at linaro.org> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Marek,
>>>>>>>>>>
>>>>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <marex at 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 ?
>>>
>>
>> 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)
> 
> Esp. the "disable" part. That to me reads as "don't send HBP packet".
> 

Yes the LP11 option doesnt send HBP packet. That comment should have 
said "Dont send HBP packet and use LP11 instead" to match the spec.


Like I said there are two options:

1) Send the blanking (HBP) packet
2) Dont send the packet and transition to LP11

Thats what those flags are controlling and thats what MSM driver does too.

> Where do you see that quote above in the DSI spec (which chapter and 
> which version do you read) ?
> 

I am referring "8.11.2 Non-Burst Mode with Sync Pulses" of MIPI DSI 1.2 
spec ( its slightly old ) but this part doenst change across revisions.

>>>> 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?).
>>>
>>
>> No, there is no need of another new flag. There are only two options 
>> as per the spec.
>>
>> In fact, as per my checking with more folks, requiring LP11 during 
>> those periods is something very rare.
>>
>> Because usually horizontal period is usually a very short period, most 
>> of the time we do not use the LP11 option and send the blanking 
>> packets instead.
>>
>> So its something very unusual for DSIM.
>>
>> That being said, I still think my previous question is important.
>>
>> 1) What is the difference between the resolution you are trying Vs 
>> what Amit is trying?
>>
>> 2) Are you both using just standard HDMI monitors?
> 
> What is a "standard HDMI monitor" ?
> I use DELL U2713HM .
> 

Thats standard enough :)

Please also let us know your resolution (drm_display_mode) to compare 
with what Amit has.

> [...]