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

Commit Message

Marek Vasut April 3, 2023, 10:12 p.m. UTC

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.

Signed-off-by: Marek Vasut <[e-mail]>
---
Cc: Andrzej Hajda <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
Cc: David Airlie <[e-mail]>
Cc: Jagan Teki <[e-mail]>
Cc: Jernej Skrabec <[e-mail]>
Cc: Jonas Karlman <[e-mail]>
Cc: Laurent Pinchart <[e-mail]>
Cc: Michael Walle <[e-mail]>
Cc: Neil Armstrong <[e-mail]>
Cc: Robert Foss <[e-mail]>
Cc: [e-mail]
---
 drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Robert Foss April 5, 2023, 11:20 a.m. UTC | #1

On Tue, Apr 4, 2023 at 12:13 AM Marek Vasut <[e-mail]> 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.
>
> Signed-off-by: Marek Vasut <[e-mail]>
> ---
> Cc: Andrzej Hajda <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Jagan Teki <[e-mail]>
> Cc: Jernej Skrabec <[e-mail]>
> Cc: Jonas Karlman <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Michael Walle <[e-mail]>
> Cc: Neil Armstrong <[e-mail]>
> Cc: Robert Foss <[e-mail]>
> Cc: [e-mail]
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index a25d21a7d5c19..151efe92711c4 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -774,7 +774,9 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611,
>         dsi->lanes = 4;
>         dsi->format = MIPI_DSI_FMT_RGB888;
>         dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> -                         MIPI_DSI_MODE_VIDEO_HSE;
> +                         MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_VIDEO_NO_HSA |
> +                         MIPI_DSI_MODE_VIDEO_NO_HFP | MIPI_DSI_MODE_VIDEO_NO_HBP |
> +                         MIPI_DSI_MODE_NO_EOT_PACKET;
>
>         ret = devm_mipi_dsi_attach(dev, dsi);
>         if (ret < 0) {
> --
> 2.39.2
>

Reviewed-by: Robert Foss <[e-mail]>

Snoozing this patch for a week before applying.

Amit Pundir July 5, 2023, 4:45 a.m. UTC | #2

Hi Marek,

On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.

Regards,
Amit Pundir

>
> Signed-off-by: Marek Vasut <[e-mail]>
> ---
> Cc: Andrzej Hajda <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Jagan Teki <[e-mail]>
> Cc: Jernej Skrabec <[e-mail]>
> Cc: Jonas Karlman <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Michael Walle <[e-mail]>
> Cc: Neil Armstrong <[e-mail]>
> Cc: Robert Foss <[e-mail]>
> Cc: [e-mail]
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index a25d21a7d5c19..151efe92711c4 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -774,7 +774,9 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611,
>         dsi->lanes = 4;
>         dsi->format = MIPI_DSI_FMT_RGB888;
>         dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> -                         MIPI_DSI_MODE_VIDEO_HSE;
> +                         MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_VIDEO_NO_HSA |
> +                         MIPI_DSI_MODE_VIDEO_NO_HFP | MIPI_DSI_MODE_VIDEO_NO_HBP |
> +                         MIPI_DSI_MODE_NO_EOT_PACKET;
>
>         ret = devm_mipi_dsi_attach(dev, dsi);
>         if (ret < 0) {
> --
> 2.39.2
>

Jagan Teki July 5, 2023, 5:30 a.m. UTC | #3

Hi Amit,

On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir <[e-mail]> wrote:
>
> Hi Marek,
>
> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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?

Thanks,
Jagan.

Dmitry Baryshkov July 5, 2023, 5:39 a.m. UTC | #4

[Adding freedreno@ to cc list]

On Wed, 5 Jul 2023 at 08:31, Jagan Teki <[e-mail]> wrote:
>
> Hi Amit,
>
> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir <[e-mail]> wrote:
> >
> > Hi Marek,
> >
> > On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.

Jagan Teki July 5, 2023, 5:46 a.m. UTC | #5

On Wed, Jul 5, 2023 at 11:09 AM Dmitry Baryshkov
<[e-mail]> wrote:
>
> [Adding freedreno@ to cc list]
>
> On Wed, 5 Jul 2023 at 08:31, Jagan Teki <[e-mail]> wrote:
> >
> > Hi Amit,
> >
> > On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir <[e-mail]> wrote:
> > >
> > > Hi Marek,
> > >
> > > On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.

True, But I'm not quite sure, most of these mode_flags were handled
more on the host. Maybe Marek can comment on this.

Thanks,
Jagan.

Marek Vasut July 5, 2023, 7:31 a.m. UTC | #6

On 7/5/23 07:46, Jagan Teki wrote:
> On Wed, Jul 5, 2023 at 11:09 AM Dmitry Baryshkov
> <[e-mail]> wrote:
>>
>> [Adding freedreno@ to cc list]
>>
>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki <[e-mail]> wrote:
>>>
>>> Hi Amit,
>>>
>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir <[e-mail]> wrote:
>>>>
>>>> Hi Marek,
>>>>
>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.
> 
> True, But I'm not quite sure, most of these mode_flags were handled
> more on the host. Maybe Marek can comment on this.

So, we have the same flags, but two different controllers produce 
different DSI streams ? Do we have any way to analyze the stream 
produced by each controller, to figure out which one is wrong and which 
one is right ?

Amit Pundir July 6, 2023, 9:20 a.m. UTC | #7

On Wed, 5 Jul 2023 at 11:09, Dmitry Baryshkov
<[e-mail]> wrote:
>
> [Adding freedreno@ to cc list]
>
> On Wed, 5 Jul 2023 at 08:31, Jagan Teki <[e-mail]> wrote:
> >
> > Hi Amit,
> >
> > On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir <[e-mail]> wrote:
> > >
> > > Hi Marek,
> > >
> > > On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.

Regards,
Amit Pundir
[1] https://people.linaro.org/~amit.pundir/db845c-userdebug/v6.5-broken-display/PXL_20230704_150156326.jpg

>
> --
> With best wishes
> Dmitry

Neil Armstrong July 7, 2023, 7:18 a.m. UTC | #8

Hi,

On 06/07/2023 11:20, Amit Pundir wrote:
> On Wed, 5 Jul 2023 at 11:09, Dmitry Baryshkov
> <[e-mail]> wrote:
>>
>> [Adding freedreno@ to cc list]
>>
>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki <[e-mail]> wrote:
>>>
>>> Hi Amit,
>>>
>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir <[e-mail]> wrote:
>>>>
>>>> Hi Marek,
>>>>
>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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 ?

Neil

> 
> Regards,
> Amit Pundir
> [1] https://people.linaro.org/~amit.pundir/db845c-userdebug/v6.5-broken-display/PXL_20230704_150156326.jpg
> 
>>
>> --
>> With best wishes
>> Dmitry

Neil Armstrong July 7, 2023, 8:47 a.m. UTC | #9

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
>> <[e-mail]> wrote:
>>>
>>> [Adding freedreno@ to cc list]
>>>
>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki <[e-mail]> wrote:
>>>>
>>>> Hi Amit,
>>>>
>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir <[e-mail]> wrote:
>>>>>
>>>>> Hi Marek,
>>>>>
>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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 ?

@Jagan, Andrezej So you have any documentation on what DSIM_xxx_DISABLE_MODE does ?

@Dmitry, so you have access to the lt9611 datasheet to know what's needed here ?

Neil

> 
> Neil
> 
>>
>> Regards,
>> Amit Pundir
>> [1] https://people.linaro.org/~amit.pundir/db845c-userdebug/v6.5-broken-display/PXL_20230704_150156326.jpg
>>
>>>
>>> -- 
>>> With best wishes
>>> Dmitry
>

Thorsten Leemhuis July 8, 2023, 2:08 p.m. UTC | #10

[TLDR: I'm adding this report to the list of tracked Linux kernel
regressions; the text you find below is based on a few templates
paragraphs you might have encountered already in similar form.
See link in footer if these mails annoy you.]

On 05.07.23 06:45, Amit Pundir wrote:
> 
> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced 8ddce13ae69
#regzbot title drm/bridge: lt9611: Dragonboard 845c (SDM845) devboard
broken when running AOSP
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

Marek Vasut July 8, 2023, 3:40 p.m. UTC | #11

On 7/7/23 10:47, 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
>>> <[e-mail]> wrote:
>>>>
>>>> [Adding freedreno@ to cc list]
>>>>
>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki <[e-mail]> 
>>>> wrote:
>>>>>
>>>>> Hi Amit,
>>>>>
>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>> Hi Marek,
>>>>>>
>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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 don't have the LT9611 datasheet, no.

The MX8M DSI (samsung-dsim) skips the HSA/HFP/HBP completely (see 
i.MX8MP RM 13.6.2.7.2 RGB Interface , there is infographics on the 
following pages).

> 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 ?
> 
> @Jagan, Andrezej So you have any documentation on what 
> DSIM_xxx_DISABLE_MODE does ?

See above, i.MX8M M/N/P uses the samsung-dsim block .

> @Dmitry, so you have access to the lt9611 datasheet to know what's 
> needed here ?

[...]

Dmitry Baryshkov July 8, 2023, 3:53 p.m. UTC | #12

On 08/07/2023 18:40, Marek Vasut wrote:
> On 7/7/23 10:47, 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
>>>> <[e-mail]> wrote:
>>>>>
>>>>> [Adding freedreno@ to cc list]
>>>>>
>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki 
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>> Hi Amit,
>>>>>>
>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>>> <[e-mail]> wrote:
>>>>>>>
>>>>>>> Hi Marek,
>>>>>>>
>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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 don't have the LT9611 datasheet, no.

I also don't have an lt9611 datasheet, unfortunately. I was using the 
existing third-party drivers as a source.

> 
> The MX8M DSI (samsung-dsim) skips the HSA/HFP/HBP completely (see 
> i.MX8MP RM 13.6.2.7.2 RGB Interface , there is infographics on the 
> following pages).

Do you know, why did you have to disable HPB/HSA/HFP on your platform? 
What was the result otherwise?

> 
>> 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 ?
>>
>> @Jagan, Andrezej So you have any documentation on what 
>> DSIM_xxx_DISABLE_MODE does ?
> 
> See above, i.MX8M M/N/P uses the samsung-dsim block .
> 
>> @Dmitry, so you have access to the lt9611 datasheet to know what's 
>> needed here ?
> 
> [...]

Marek Vasut July 8, 2023, 7:39 p.m. UTC | #13

On 7/8/23 17:53, Dmitry Baryshkov wrote:
> On 08/07/2023 18:40, Marek Vasut wrote:
>> On 7/7/23 10:47, 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
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>> [Adding freedreno@ to cc list]
>>>>>>
>>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki 
>>>>>> <[e-mail]> wrote:
>>>>>>>
>>>>>>> Hi Amit,
>>>>>>>
>>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>>>> <[e-mail]> wrote:
>>>>>>>>
>>>>>>>> Hi Marek,
>>>>>>>>
>>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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 don't have the LT9611 datasheet, no.
> 
> I also don't have an lt9611 datasheet, unfortunately. I was using the 
> existing third-party drivers as a source.
> 
>>
>> The MX8M DSI (samsung-dsim) skips the HSA/HFP/HBP completely (see 
>> i.MX8MP RM 13.6.2.7.2 RGB Interface , there is infographics on the 
>> following pages).
> 
> Do you know, why did you have to disable HPB/HSA/HFP on your platform? 
> What was the result otherwise?

No image on the HDMI monitor at all. This flags change has fixed the 
problem for multiple other bridges too btw, not just the LT9611, but 
also TI SN65DSI83 and ICN6211. The flags were likely not set in all 
those bridge drivers because no DSI host implemented them so far.

Dmitry Baryshkov July 8, 2023, 7:40 p.m. UTC | #14

On Sat, 8 Jul 2023 at 22:39, Marek Vasut <[e-mail]> wrote:
>
> On 7/8/23 17:53, Dmitry Baryshkov wrote:
> > On 08/07/2023 18:40, Marek Vasut wrote:
> >> On 7/7/23 10:47, 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
> >>>>> <[e-mail]> wrote:
> >>>>>>
> >>>>>> [Adding freedreno@ to cc list]
> >>>>>>
> >>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki
> >>>>>> <[e-mail]> wrote:
> >>>>>>>
> >>>>>>> Hi Amit,
> >>>>>>>
> >>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir
> >>>>>>> <[e-mail]> wrote:
> >>>>>>>>
> >>>>>>>> Hi Marek,
> >>>>>>>>
> >>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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 don't have the LT9611 datasheet, no.
> >
> > I also don't have an lt9611 datasheet, unfortunately. I was using the
> > existing third-party drivers as a source.
> >
> >>
> >> The MX8M DSI (samsung-dsim) skips the HSA/HFP/HBP completely (see
> >> i.MX8MP RM 13.6.2.7.2 RGB Interface , there is infographics on the
> >> following pages).
> >
> > Do you know, why did you have to disable HPB/HSA/HFP on your platform?
> > What was the result otherwise?
>
> No image on the HDMI monitor at all. This flags change has fixed the
> problem for multiple other bridges too btw, not just the LT9611, but
> also TI SN65DSI83 and ICN6211. The flags were likely not set in all
> those bridge drivers because no DSI host implemented them so far.

MSM DSI host have had those implemented for ages, but we never needed
them AFAIR.

Marek Vasut July 8, 2023, 7:47 p.m. UTC | #15

On 7/8/23 21:40, Dmitry Baryshkov wrote:
> On Sat, 8 Jul 2023 at 22:39, Marek Vasut <[e-mail]> wrote:
>>
>> On 7/8/23 17:53, Dmitry Baryshkov wrote:
>>> On 08/07/2023 18:40, Marek Vasut wrote:
>>>> On 7/7/23 10:47, 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
>>>>>>> <[e-mail]> wrote:
>>>>>>>>
>>>>>>>> [Adding freedreno@ to cc list]
>>>>>>>>
>>>>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki
>>>>>>>> <[e-mail]> wrote:
>>>>>>>>>
>>>>>>>>> Hi Amit,
>>>>>>>>>
>>>>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir
>>>>>>>>> <[e-mail]> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Marek,
>>>>>>>>>>
>>>>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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 don't have the LT9611 datasheet, no.
>>>
>>> I also don't have an lt9611 datasheet, unfortunately. I was using the
>>> existing third-party drivers as a source.
>>>
>>>>
>>>> The MX8M DSI (samsung-dsim) skips the HSA/HFP/HBP completely (see
>>>> i.MX8MP RM 13.6.2.7.2 RGB Interface , there is infographics on the
>>>> following pages).
>>>
>>> Do you know, why did you have to disable HPB/HSA/HFP on your platform?
>>> What was the result otherwise?
>>
>> No image on the HDMI monitor at all. This flags change has fixed the
>> problem for multiple other bridges too btw, not just the LT9611, but
>> also TI SN65DSI83 and ICN6211. The flags were likely not set in all
>> those bridge drivers because no DSI host implemented them so far.
> 
> MSM DSI host have had those implemented for ages, but we never needed
> them AFAIR.

The MSM one is exception in more ways than one it seems (in a positive way).

Abhinav Kumar July 9, 2023, 1:03 a.m. UTC | #16

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
>>> <[e-mail]> wrote:
>>>>
>>>> [Adding freedreno@ to cc list]
>>>>
>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki <[e-mail]> 
>>>> wrote:
>>>>>
>>>>> Hi Amit,
>>>>>
>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>> Hi Marek,
>>>>>>
>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.

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.

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.


> @Jagan, Andrezej So you have any documentation on what 
> DSIM_xxx_DISABLE_MODE does ?
> 
> @Dmitry, so you have access to the lt9611 datasheet to know what's 
> needed here ?
> 
> Neil
> 
>>
>> Neil
>>
>>>
>>> Regards,
>>> Amit Pundir
>>> [1] 
>>> https://people.linaro.org/~amit.pundir/db845c-userdebug/v6.5-broken-display/PXL_20230704_150156326.jpg
>>>
>>>>
>>>> -- 
>>>> With best wishes
>>>> Dmitry
>>
>

Marek Vasut July 12, 2023, 5:41 p.m. UTC | #17

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
>>>> <[e-mail]> wrote:
>>>>>
>>>>> [Adding freedreno@ to cc list]
>>>>>
>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki 
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>> Hi Amit,
>>>>>>
>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>>> <[e-mail]> wrote:
>>>>>>>
>>>>>>> Hi Marek,
>>>>>>>
>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.

[...]

Abhinav Kumar July 13, 2023, 6:09 p.m. UTC | #18

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
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>> [Adding freedreno@ to cc list]
>>>>>>
>>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki 
>>>>>> <[e-mail]> wrote:
>>>>>>>
>>>>>>> Hi Amit,
>>>>>>>
>>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>>>> <[e-mail]> wrote:
>>>>>>>>
>>>>>>>> Hi Marek,
>>>>>>>>
>>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.


>> 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?

Standard HDMI monitors follow VESA or CEA which usually have pretty low 
hblank so its not surprising that the LP11 option does not work as the 
blanking time is not sufficient for this LP<->HS transition.

>> 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.
> 
> [...]

Marek Vasut July 13, 2023, 6:28 p.m. UTC | #19

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
>>>>>> <[e-mail]> wrote:
>>>>>>>
>>>>>>> [Adding freedreno@ to cc list]
>>>>>>>
>>>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki 
>>>>>>> <[e-mail]> wrote:
>>>>>>>>
>>>>>>>> Hi Amit,
>>>>>>>>
>>>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>>>>> <[e-mail]> wrote:
>>>>>>>>>
>>>>>>>>> Hi Marek,
>>>>>>>>>
>>>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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".

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

>>> 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 .

[...]

Abhinav Kumar July 13, 2023, 6:34 p.m. UTC | #20

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
>>>>>>> <[e-mail]> wrote:
>>>>>>>>
>>>>>>>> [Adding freedreno@ to cc list]
>>>>>>>>
>>>>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki 
>>>>>>>> <[e-mail]> wrote:
>>>>>>>>>
>>>>>>>>> Hi Amit,
>>>>>>>>>
>>>>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir 
>>>>>>>>> <[e-mail]> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Marek,
>>>>>>>>>>
>>>>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.

> [...]

Jagan Teki July 13, 2023, 6:37 p.m. UTC | #21

On Thu, Jul 13, 2023 at 11:39 PM Abhinav Kumar
<[e-mail]> 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
> >>>>> <[e-mail]> wrote:
> >>>>>>
> >>>>>> [Adding freedreno@ to cc list]
> >>>>>>
> >>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki
> >>>>>> <[e-mail]> wrote:
> >>>>>>>
> >>>>>>> Hi Amit,
> >>>>>>>
> >>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir
> >>>>>>> <[e-mail]> wrote:
> >>>>>>>>
> >>>>>>>> Hi Marek,
> >>>>>>>>
> >>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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.

As many of them know all these flags are generic across controllers
I'm trying to add these flag notations from DSIM controller and how
they handle the driver.

HBP/HFP/HSA mode bits in i.MX8M Mini/Nano/Plus Processor Reference
Manuals specify a naming conversion as 'disable mode bit' due to its
bit definition,  0 = Enable and 1 = Disable.

Example HBP (HbpDisableMode)- Specifies HBP disable mode. If this bit
set, DSI master ignores HBP area in Video mode.
 0 = Enables
 1 = Disables

 if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP)
           reg |= DSIM_HFP_DISABLE_MODE;
 if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP)
           reg |= DSIM_HBP_DISABLE_MODE;
 if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA)
            reg |= DSIM_HSA_DISABLE_MODE;

I'm hoping this will give some information.

Thanks,
Jagan.

Amit Pundir July 14, 2023, 6:11 a.m. UTC | #22

On Thu, 13 Jul 2023 at 23:58, Marek Vasut <[e-mail]> 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
> >>>>>> <[e-mail]> wrote:
> >>>>>>>
> >>>>>>> [Adding freedreno@ to cc list]
> >>>>>>>
> >>>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki
> >>>>>>> <[e-mail]> wrote:
> >>>>>>>>
> >>>>>>>> Hi Amit,
> >>>>>>>>
> >>>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir
> >>>>>>>> <[e-mail]> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi Marek,
> >>>>>>>>>
> >>>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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".
>
> Where do you see that quote above in the DSI spec (which chapter and
> which version do you read) ?
>
> >>> 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 .

I see this breakage on portable HDMI monitors Viewsonic TD1655 [1] and
Cocopar Z173FH7F [2], both running at 1920x1080 resolution.

Regards,
Amit Pundir
[1] https://www.amazon.in/ViewSonic-TD1655-Portable-Touchscreen-Frameless/dp/B08778F756
[2] https://www.amazon.com/Portable-Monitor-FreeSync-Speaker-Nintendo/dp/B07ZLXCVPN

>
> [...]

Thorsten Leemhuis July 26, 2023, 10:09 a.m. UTC | #23

Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.

What's the status wrt to this regression (caused by 8ddce13ae69 from
Marek)? It looks like things are stalled and the regression still is
unresolved, but I ask because I might be missing something.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

On 14.07.23 08:11, Amit Pundir wrote:
> On Thu, 13 Jul 2023 at 23:58, Marek Vasut <[e-mail]> 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
>>>>>>>> <[e-mail]> wrote:
>>>>>>>>>
>>>>>>>>> [Adding freedreno@ to cc list]
>>>>>>>>>
>>>>>>>>> On Wed, 5 Jul 2023 at 08:31, Jagan Teki
>>>>>>>>> <[e-mail]> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Amit,
>>>>>>>>>>
>>>>>>>>>> On Wed, Jul 5, 2023 at 10:15 AM Amit Pundir
>>>>>>>>>> <[e-mail]> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Marek,
>>>>>>>>>>>
>>>>>>>>>>> On Wed, 5 Jul 2023 at 01:48, Marek Vasut <[e-mail]> 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".
>>
>> Where do you see that quote above in the DSI spec (which chapter and
>> which version do you read) ?
>>
>>>>> 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 .
> 
> I see this breakage on portable HDMI monitors Viewsonic TD1655 [1] and
> Cocopar Z173FH7F [2], both running at 1920x1080 resolution.
> 
> Regards,
> Amit Pundir
> [1] https://www.amazon.in/ViewSonic-TD1655-Portable-Touchscreen-Frameless/dp/B08778F756
> [2] https://www.amazon.com/Portable-Monitor-FreeSync-Speaker-Nintendo/dp/B07ZLXCVPN
> 
>>
>> [...]
> 
>

Neil Armstrong Aug. 2, 2023, 8:39 a.m. UTC | #24

Hi Marek,

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 ?

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.

Neil

> 
> Esp. the "disable" part. That to me reads as "don't send HBP packet".
> 
> Where do you see that quote above in the DSI spec (which chapter and which version do you read) ?
> 
>>>> 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 .
> 
> [...]

Marek Vasut Aug. 2, 2023, 12:25 p.m. UTC | #25

On 8/2/23 10:39, [e-mail] 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.

Neil Armstrong Aug. 2, 2023, 1:08 p.m. UTC | #26

Hi Marek,

On 02/08/2023 14:25, Marek Vasut wrote:
> On 8/2/23 10:39, [e-mail] 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 ?

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

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

Neil

Marek Vasut Aug. 2, 2023, 5:25 p.m. UTC | #27

On 8/2/23 15:08, [e-mail] wrote:
> Hi Marek,
> 
> On 02/08/2023 14:25, Marek Vasut wrote:
>> On 8/2/23 10:39, [e-mail] 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.

Abhinav Kumar Aug. 2, 2023, 5:49 p.m. UTC | #28

Hi Marek

On 8/2/2023 10:25 AM, Marek Vasut wrote:
> On 8/2/23 15:08, [e-mail] wrote:
>> Hi Marek,
>>
>> On 02/08/2023 14:25, Marek Vasut wrote:
>>> On 8/2/23 10:39, [e-mail] 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 ?
> 

Like I have written above, the DSI spec gives two options which we can 
do in the HBP/HSA/HFP periods:

1) Transition to LP11 which means blanking packets will not be sent
2) Send blanking packets during those periods

That flag controls exactly that and thats what MSM does.

There is no third option in the spec to not do either.

Now, are you saying that those flags are providing some other third 
option which is not even there in the DSI spec?


>> 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.

Marek Vasut Aug. 2, 2023, 6:46 p.m. UTC | #29

On 8/2/23 19:49, Abhinav Kumar wrote:
> Hi Marek
> 
> On 8/2/2023 10:25 AM, Marek Vasut wrote:
>> On 8/2/23 15:08, [e-mail] wrote:
>>> Hi Marek,
>>>
>>> On 02/08/2023 14:25, Marek Vasut wrote:
>>>> On 8/2/23 10:39, [e-mail] 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 ?
>>
> 
> Like I have written above, the DSI spec gives two options which we can 
> do in the HBP/HSA/HFP periods:
> 
> 1) Transition to LP11 which means blanking packets will not be sent
> 2) Send blanking packets during those periods
> 
> That flag controls exactly that and thats what MSM does.
> 
> There is no third option in the spec to not do either.
> 
> Now, are you saying that those flags are providing some other third 
> option which is not even there in the DSI spec?

Can you please have a look at the i.MX8M reference manual and read 
exactly what that controller does , then compare it to the QCOM 
controller behavior ?

Patch

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
index a25d21a7d5c19..151efe92711c4 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -774,7 +774,9 @@  static struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611,
 	dsi->lanes = 4;
 	dsi->format = MIPI_DSI_FMT_RGB888;
 	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
-			  MIPI_DSI_MODE_VIDEO_HSE;
+			  MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_VIDEO_NO_HSA |
+			  MIPI_DSI_MODE_VIDEO_NO_HFP | MIPI_DSI_MODE_VIDEO_NO_HBP |
+			  MIPI_DSI_MODE_NO_EOT_PACKET;
 
 	ret = devm_mipi_dsi_attach(dev, dsi);
 	if (ret < 0) {