Revert "drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet"

Commit Message

Neil Armstrong Aug. 2, 2023, 8:52 a.m. UTC

This reverts commit [1] to fix display regression on the Dragonboard 845c
(SDM845) devboard.

There's a mismatch on the real action of the following flags:
- MIPI_DSI_MODE_VIDEO_NO_HSA
- MIPI_DSI_MODE_VIDEO_NO_HFP
- MIPI_DSI_MODE_VIDEO_NO_HBP
which leads to a non-working display on qcom platforms.

[1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")

Cc: Marek Vasut <[e-mail]>
Cc: Robert Foss <[e-mail]>
Cc: Jagan Teki <[e-mail]>
Cc: Dmitry Baryshkov <[e-mail]>
Cc: Abhinav Kumar <[e-mail]>
Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
Reported-by: Amit Pundir <[e-mail]>
Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
Signed-off-by: Neil Armstrong <[e-mail]>
---
 drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)


---
base-commit: f590814603bf2dd8620584b7d59ae94d7c186c69
change-id: 20230802-revert-do-not-generate-hfp-hbp-hsa-eot-packet-6f042b1ba813

Best regards,

Comments

Dmitry Baryshkov Aug. 2, 2023, 9:08 a.m. UTC | #1

On Wed, 2 Aug 2023 at 11:52, Neil Armstrong <[e-mail]> wrote:
>
> This reverts commit [1] to fix display regression on the Dragonboard 845c
> (SDM845) devboard.
>
> There's a mismatch on the real action of the following flags:
> - MIPI_DSI_MODE_VIDEO_NO_HSA
> - MIPI_DSI_MODE_VIDEO_NO_HFP
> - MIPI_DSI_MODE_VIDEO_NO_HBP
> which leads to a non-working display on qcom platforms.
>
> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")

Nit: I think the preferred form is to write `... reverts commit abcdef
("foo and bar")', but I might be wrong.

Other than that:

Acked-by: Dmitry Baryshkov <[e-mail]>

>
> Cc: Marek Vasut <[e-mail]>
> Cc: Robert Foss <[e-mail]>
> Cc: Jagan Teki <[e-mail]>
> Cc: Dmitry Baryshkov <[e-mail]>
> Cc: Abhinav Kumar <[e-mail]>
> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> Reported-by: Amit Pundir <[e-mail]>
> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
> Signed-off-by: Neil Armstrong <[e-mail]>
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index 5163e5224aad..9663601ce098 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -774,9 +774,7 @@ 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_NO_HSA |
> -                         MIPI_DSI_MODE_VIDEO_NO_HFP | MIPI_DSI_MODE_VIDEO_NO_HBP |
> -                         MIPI_DSI_MODE_NO_EOT_PACKET;
> +                         MIPI_DSI_MODE_VIDEO_HSE;
>
>         ret = devm_mipi_dsi_attach(dev, dsi);
>         if (ret < 0) {
>
> ---
> base-commit: f590814603bf2dd8620584b7d59ae94d7c186c69
> change-id: 20230802-revert-do-not-generate-hfp-hbp-hsa-eot-packet-6f042b1ba813
>
> Best regards,
> --
> Neil Armstrong <[e-mail]>
>

Amit Pundir Aug. 2, 2023, 9:15 a.m. UTC | #2

On Wed, 2 Aug 2023 at 14:22, Neil Armstrong <[e-mail]> wrote:
>
> This reverts commit [1] to fix display regression on the Dragonboard 845c
> (SDM845) devboard.

Tested-by: Amit Pundir <[e-mail]>

Regards,
Amit Pundir

>
> There's a mismatch on the real action of the following flags:
> - MIPI_DSI_MODE_VIDEO_NO_HSA
> - MIPI_DSI_MODE_VIDEO_NO_HFP
> - MIPI_DSI_MODE_VIDEO_NO_HBP
> which leads to a non-working display on qcom platforms.
>
> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
>
> Cc: Marek Vasut <[e-mail]>
> Cc: Robert Foss <[e-mail]>
> Cc: Jagan Teki <[e-mail]>
> Cc: Dmitry Baryshkov <[e-mail]>
> Cc: Abhinav Kumar <[e-mail]>
> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> Reported-by: Amit Pundir <[e-mail]>
> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
> Signed-off-by: Neil Armstrong <[e-mail]>
> ---
>  drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index 5163e5224aad..9663601ce098 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -774,9 +774,7 @@ 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_NO_HSA |
> -                         MIPI_DSI_MODE_VIDEO_NO_HFP | MIPI_DSI_MODE_VIDEO_NO_HBP |
> -                         MIPI_DSI_MODE_NO_EOT_PACKET;
> +                         MIPI_DSI_MODE_VIDEO_HSE;
>
>         ret = devm_mipi_dsi_attach(dev, dsi);
>         if (ret < 0) {
>
> ---
> base-commit: f590814603bf2dd8620584b7d59ae94d7c186c69
> change-id: 20230802-revert-do-not-generate-hfp-hbp-hsa-eot-packet-6f042b1ba813
>
> Best regards,
> --
> Neil Armstrong <[e-mail]>
>

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

On 8/2/23 10:52, Neil Armstrong wrote:
> This reverts commit [1] to fix display regression on the Dragonboard 845c
> (SDM845) devboard.
> 
> There's a mismatch on the real action of the following flags:
> - MIPI_DSI_MODE_VIDEO_NO_HSA
> - MIPI_DSI_MODE_VIDEO_NO_HFP
> - MIPI_DSI_MODE_VIDEO_NO_HBP
> which leads to a non-working display on qcom platforms.
> 
> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> 
> Cc: Marek Vasut <[e-mail]>
> Cc: Robert Foss <[e-mail]>
> Cc: Jagan Teki <[e-mail]>
> Cc: Dmitry Baryshkov <[e-mail]>
> Cc: Abhinav Kumar <[e-mail]>
> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> Reported-by: Amit Pundir <[e-mail]>
> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
> Signed-off-by: Neil Armstrong <[e-mail]>

This breaks LT9611 operation on i.MX8M Mini/Nano/Plus, so, NAK.

I am currently using this LT9611 with Linux 6.1.y in production and this 
is not acceptable. I also believe the correct fix is on the MSM side, 
not on the LT9611 driver side, since MSM incorrectly implements these flags.

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

On 8/2/23 14:07, Marek Vasut wrote:
> On 8/2/23 10:52, Neil Armstrong wrote:
>> This reverts commit [1] to fix display regression on the Dragonboard 845c
>> (SDM845) devboard.
>>
>> There's a mismatch on the real action of the following flags:
>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>> which leads to a non-working display on qcom platforms.
>>
>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and 
>> EOT packet")
>>
>> Cc: Marek Vasut <[e-mail]>
>> Cc: Robert Foss <[e-mail]>
>> Cc: Jagan Teki <[e-mail]>
>> Cc: Dmitry Baryshkov <[e-mail]>
>> Cc: Abhinav Kumar <[e-mail]>
>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>> and EOT packet")
>> Reported-by: Amit Pundir <[e-mail]>
>> Link: 
>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>> Signed-off-by: Neil Armstrong <[e-mail]>
> 
> This breaks LT9611 operation on i.MX8M Mini/Nano/Plus, so, NAK.
> 
> I am currently using this LT9611 with Linux 6.1.y

Correction, 6.1.y only with the DSIM patches backported.

> in production and this 
> is not acceptable. I also believe the correct fix is on the MSM side, 
> not on the LT9611 driver side, since MSM incorrectly implements these 
> flags.

Neil Armstrong Aug. 2, 2023, 12:37 p.m. UTC | #5

On 02/08/2023 14:28, Marek Vasut wrote:
> On 8/2/23 14:07, Marek Vasut wrote:
>> On 8/2/23 10:52, Neil Armstrong wrote:
>>> This reverts commit [1] to fix display regression on the Dragonboard 845c
>>> (SDM845) devboard.
>>>
>>> There's a mismatch on the real action of the following flags:
>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>>> which leads to a non-working display on qcom platforms.
>>>
>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
>>>
>>> Cc: Marek Vasut <[e-mail]>
>>> Cc: Robert Foss <[e-mail]>
>>> Cc: Jagan Teki <[e-mail]>
>>> Cc: Dmitry Baryshkov <[e-mail]>
>>> Cc: Abhinav Kumar <[e-mail]>
>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
>>> Reported-by: Amit Pundir <[e-mail]>
>>> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>>> Signed-off-by: Neil Armstrong <[e-mail]>
>>
>> This breaks LT9611 operation on i.MX8M Mini/Nano/Plus, so, NAK.
>>
>> I am currently using this LT9611 with Linux 6.1.y
> 
> Correction, 6.1.y only with the DSIM patches backported.

Well you'll need to keep [1] backported on your downstream branch,
this revert won't propagate to v6.1 stable anyway.

> 
>> in production and this is not acceptable. I also believe the correct fix is on the MSM side, not on the LT9611 driver side, since MSM incorrectly implements these flags.
> 

Since [1] breaks Qcom boards on v6.5, and [1] was added for v6.5 to make the bridge work
on i.MX8M Mini/Nano/Plus, it's not acceptable either to keep it for the v6.5 release.

Neil

Dmitry Baryshkov Aug. 2, 2023, 12:38 p.m. UTC | #6

On 02/08/2023 15:07, Marek Vasut wrote:
> On 8/2/23 10:52, Neil Armstrong wrote:
>> This reverts commit [1] to fix display regression on the Dragonboard 845c
>> (SDM845) devboard.
>>
>> There's a mismatch on the real action of the following flags:
>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>> which leads to a non-working display on qcom platforms.
>>
>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and 
>> EOT packet")
>>
>> Cc: Marek Vasut <[e-mail]>
>> Cc: Robert Foss <[e-mail]>
>> Cc: Jagan Teki <[e-mail]>
>> Cc: Dmitry Baryshkov <[e-mail]>
>> Cc: Abhinav Kumar <[e-mail]>
>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>> and EOT packet")
>> Reported-by: Amit Pundir <[e-mail]>
>> Link: 
>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>> Signed-off-by: Neil Armstrong <[e-mail]>
> 
> This breaks LT9611 operation on i.MX8M Mini/Nano/Plus, so, NAK.
> 
> I am currently using this LT9611 with Linux 6.1.y in production and this 
> is not acceptable. I also believe the correct fix is on the MSM side, 
> not on the LT9611 driver side, since MSM incorrectly implements these 
> flags.

Up to now we saw no proof that MSM incorrectly implements the flags.

Dmitry Baryshkov Aug. 2, 2023, 1:16 p.m. UTC | #7

On 02/08/2023 15:07, Marek Vasut wrote:
> On 8/2/23 10:52, Neil Armstrong wrote:
>> This reverts commit [1] to fix display regression on the Dragonboard 845c
>> (SDM845) devboard.
>>
>> There's a mismatch on the real action of the following flags:
>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>> which leads to a non-working display on qcom platforms.
>>
>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and 
>> EOT packet")
>>
>> Cc: Marek Vasut <[e-mail]>
>> Cc: Robert Foss <[e-mail]>
>> Cc: Jagan Teki <[e-mail]>
>> Cc: Dmitry Baryshkov <[e-mail]>
>> Cc: Abhinav Kumar <[e-mail]>
>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>> and EOT packet")
>> Reported-by: Amit Pundir <[e-mail]>
>> Link: 
>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>> Signed-off-by: Neil Armstrong <[e-mail]>
> 
> This breaks LT9611 operation on i.MX8M Mini/Nano/Plus, so, NAK.
> 
> I am currently using this LT9611 with Linux 6.1.y in production and this 
> is not acceptable. I also believe the correct fix is on the MSM side, 
> not on the LT9611 driver side, since MSM incorrectly implements these 
> flags.

There is no indication that MSM gets these flags wrong.

Let me quote the DSI 1.3 (I think Abhinav already quoted DSI 1.2).

Chapter 8.11.1 Transmission Packet Sequences:

========
If a peripheral timing specification for HBP or HFP minimum period is 
zero, the corresponding Blanking
Packet may be omitted. If the HBP or HFP maximum period is zero, the 
corresponding blanking packet
shall be omitted.
========

Next, chapter 8.11.2 Non-Burst Mode with Sync Pulses

======
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, by the spec, sending the HSA / HBP / HFP as blanking packets should 
always be accepted (and it is the default mode). Switching to LP-11 
should be permitted if there is a sufficient time to switch to LP-11 and 
back. Not sending the packets is only possible if the peripheral 
(lt9611) says so.

We already know that lt9611 breaks if we try switching to LP-11 during 
this period. We know that the there is a requirement time for the HSA / 
HBP / HFP, because the HDMI monitor needs them. Thus, I can only 
emphasise that the behaviour before the offending patch was correct.

Last, but not least, breaking the in-kernel platform for the out-of-tree 
peripheral doesn't sound correct.

I can only propose the following steps:

1. land the revert to unbreak existing users.

2. Marek to propose and land the DT bindings & driver change that will 
enable the workaround for the particular platform (i.MX8m).

Dmitry Baryshkov Aug. 2, 2023, 1:38 p.m. UTC | #8

On 02/08/2023 11:52, Neil Armstrong wrote:
> This reverts commit [1] to fix display regression on the Dragonboard 845c
> (SDM845) devboard.
> 
> There's a mismatch on the real action of the following flags:
> - MIPI_DSI_MODE_VIDEO_NO_HSA
> - MIPI_DSI_MODE_VIDEO_NO_HFP
> - MIPI_DSI_MODE_VIDEO_NO_HBP
> which leads to a non-working display on qcom platforms.
> 
> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> 
> Cc: Marek Vasut <[e-mail]>
> Cc: Robert Foss <[e-mail]>
> Cc: Jagan Teki <[e-mail]>
> Cc: Dmitry Baryshkov <[e-mail]>
> Cc: Abhinav Kumar <[e-mail]>
> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> Reported-by: Amit Pundir <[e-mail]>
> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
> Signed-off-by: Neil Armstrong <[e-mail]>
> ---
>   drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
Acked-by: Dmitry Baryshkov <[e-mail]> #fix db845c

The boards broken by [1] are used in production by different parties 
since 5.10, breaking them doesn't seem more acceptable than breaking the 
new out-of-tree iMX8m hardware.

Marek Vasut Aug. 2, 2023, 5:26 p.m. UTC | #9

On 8/2/23 14:38, Dmitry Baryshkov wrote:
> On 02/08/2023 15:07, Marek Vasut wrote:
>> On 8/2/23 10:52, Neil Armstrong wrote:
>>> This reverts commit [1] to fix display regression on the Dragonboard 
>>> 845c
>>> (SDM845) devboard.
>>>
>>> There's a mismatch on the real action of the following flags:
>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>>> which leads to a non-working display on qcom platforms.
>>>
>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>>> and EOT packet")
>>>
>>> Cc: Marek Vasut <[e-mail]>
>>> Cc: Robert Foss <[e-mail]>
>>> Cc: Jagan Teki <[e-mail]>
>>> Cc: Dmitry Baryshkov <[e-mail]>
>>> Cc: Abhinav Kumar <[e-mail]>
>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>>> and EOT packet")
>>> Reported-by: Amit Pundir <[e-mail]>
>>> Link: 
>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>>> Signed-off-by: Neil Armstrong <[e-mail]>
>>
>> This breaks LT9611 operation on i.MX8M Mini/Nano/Plus, so, NAK.
>>
>> I am currently using this LT9611 with Linux 6.1.y in production and 
>> this is not acceptable. I also believe the correct fix is on the MSM 
>> side, not on the LT9611 driver side, since MSM incorrectly implements 
>> these flags.
> 
> Up to now we saw no proof that MSM incorrectly implements the flags.

Please read the whole "[PATCH 2/2] drm/bridge: lt9611: Do not generate 
HFP/HBP/HSA and EOT packet" discussion for context.

Marek Vasut Aug. 2, 2023, 5:29 p.m. UTC | #10

On 8/2/23 15:16, Dmitry Baryshkov wrote:
> On 02/08/2023 15:07, Marek Vasut wrote:
>> On 8/2/23 10:52, Neil Armstrong wrote:
>>> This reverts commit [1] to fix display regression on the Dragonboard 
>>> 845c
>>> (SDM845) devboard.
>>>
>>> There's a mismatch on the real action of the following flags:
>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>>> which leads to a non-working display on qcom platforms.
>>>
>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>>> and EOT packet")
>>>
>>> Cc: Marek Vasut <[e-mail]>
>>> Cc: Robert Foss <[e-mail]>
>>> Cc: Jagan Teki <[e-mail]>
>>> Cc: Dmitry Baryshkov <[e-mail]>
>>> Cc: Abhinav Kumar <[e-mail]>
>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>>> and EOT packet")
>>> Reported-by: Amit Pundir <[e-mail]>
>>> Link: 
>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>>> Signed-off-by: Neil Armstrong <[e-mail]>
>>
>> This breaks LT9611 operation on i.MX8M Mini/Nano/Plus, so, NAK.
>>
>> I am currently using this LT9611 with Linux 6.1.y in production and 
>> this is not acceptable. I also believe the correct fix is on the MSM 
>> side, not on the LT9611 driver side, since MSM incorrectly implements 
>> these flags.
> 
> There is no indication that MSM gets these flags wrong.
> 
> Let me quote the DSI 1.3 (I think Abhinav already quoted DSI 1.2).
> 
> Chapter 8.11.1 Transmission Packet Sequences:
> 
> ========
> If a peripheral timing specification for HBP or HFP minimum period is 
> zero, the corresponding Blanking
> Packet may be omitted. If the HBP or HFP maximum period is zero, the 
> corresponding blanking packet
> shall be omitted.
> ========
> 
> Next, chapter 8.11.2 Non-Burst Mode with Sync Pulses
> 
> ======
> 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, by the spec, sending the HSA / HBP / HFP as blanking packets should 
> always be accepted (and it is the default mode). Switching to LP-11 
> should be permitted if there is a sufficient time to switch to LP-11 and 
> back. Not sending the packets is only possible if the peripheral 
> (lt9611) says so.
> 
> We already know that lt9611 breaks if we try switching to LP-11 during 
> this period. We know that the there is a requirement time for the HSA / 
> HBP / HFP, because the HDMI monitor needs them. Thus, I can only 
> emphasise that the behaviour before the offending patch was correct.
> 
> Last, but not least, breaking the in-kernel platform for the out-of-tree 
> peripheral doesn't sound correct.

Except the MX8M support is all in-tree now, so please drop the 
"out-of-tree" argument. That I am using 6.1.y on those platforms in 
production makes no difference.

> I can only propose the following steps:
> 
> 1. land the revert to unbreak existing users.

That's just trading breaking one set of users for breaking another set 
of users.

> 2. Marek to propose and land the DT bindings & driver change that will 
> enable the workaround for the particular platform (i.MX8m).

Since I have no access to the QCOM hardware or datasheet, can you have a 
look at the NXP.com MX8M M/N/P datasheets (those are available) and 
compare their behavior with the QCOM behavior ? I assume you do have the 
QCOM datasheets available.

Marek Vasut Aug. 2, 2023, 5:30 p.m. UTC | #11

On 8/2/23 15:38, Dmitry Baryshkov wrote:
> On 02/08/2023 11:52, Neil Armstrong wrote:
>> This reverts commit [1] to fix display regression on the Dragonboard 845c
>> (SDM845) devboard.
>>
>> There's a mismatch on the real action of the following flags:
>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>> which leads to a non-working display on qcom platforms.
>>
>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and 
>> EOT packet")
>>
>> Cc: Marek Vasut <[e-mail]>
>> Cc: Robert Foss <[e-mail]>
>> Cc: Jagan Teki <[e-mail]>
>> Cc: Dmitry Baryshkov <[e-mail]>
>> Cc: Abhinav Kumar <[e-mail]>
>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>> and EOT packet")
>> Reported-by: Amit Pundir <[e-mail]>
>> Link: 
>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>> Signed-off-by: Neil Armstrong <[e-mail]>
>> ---
>>   drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
> Acked-by: Dmitry Baryshkov <[e-mail]> #fix db845c
> 
> The boards broken by [1] are used in production by different parties 
> since 5.10, breaking them doesn't seem more acceptable than breaking the 
> new out-of-tree iMX8m hardware.

The MX8M is also in-tree, so this does not apply.

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

On 8/2/23 14:37, Neil Armstrong wrote:
> On 02/08/2023 14:28, Marek Vasut wrote:
>> On 8/2/23 14:07, Marek Vasut wrote:
>>> On 8/2/23 10:52, Neil Armstrong wrote:
>>>> This reverts commit [1] to fix display regression on the Dragonboard 
>>>> 845c
>>>> (SDM845) devboard.
>>>>
>>>> There's a mismatch on the real action of the following flags:
>>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>>>> which leads to a non-working display on qcom platforms.
>>>>
>>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>>>> and EOT packet")
>>>>
>>>> Cc: Marek Vasut <[e-mail]>
>>>> Cc: Robert Foss <[e-mail]>
>>>> Cc: Jagan Teki <[e-mail]>
>>>> Cc: Dmitry Baryshkov <[e-mail]>
>>>> Cc: Abhinav Kumar <[e-mail]>
>>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA 
>>>> and EOT packet")
>>>> Reported-by: Amit Pundir <[e-mail]>
>>>> Link: 
>>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>>>> Signed-off-by: Neil Armstrong <[e-mail]>
>>>
>>> This breaks LT9611 operation on i.MX8M Mini/Nano/Plus, so, NAK.
>>>
>>> I am currently using this LT9611 with Linux 6.1.y
>>
>> Correction, 6.1.y only with the DSIM patches backported.
> 
> Well you'll need to keep [1] backported on your downstream branch,
> this revert won't propagate to v6.1 stable anyway.

I will still have a problem with those MX8M boards during my development 
on linux-next, so that isn't helping.

>>> in production and this is not acceptable. I also believe the correct 
>>> fix is on the MSM side, not on the LT9611 driver side, since MSM 
>>> incorrectly implements these flags.
>>
> 
> Since [1] breaks Qcom boards on v6.5, and [1] was added for v6.5 to make 
> the bridge work
> on i.MX8M Mini/Nano/Plus, it's not acceptable either to keep it for the 
> v6.5 release.

So, we need to find a solution which works for both, I proposed one already.

Dmitry Baryshkov Aug. 2, 2023, 6:12 p.m. UTC | #13

On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[e-mail]> wrote:
>
> On 8/2/23 14:37, Neil Armstrong wrote:
> > On 02/08/2023 14:28, Marek Vasut wrote:
> >> On 8/2/23 14:07, Marek Vasut wrote:
> >>> On 8/2/23 10:52, Neil Armstrong wrote:
> >>>> This reverts commit [1] to fix display regression on the Dragonboard
> >>>> 845c
> >>>> (SDM845) devboard.
> >>>>
> >>>> There's a mismatch on the real action of the following flags:
> >>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
> >>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
> >>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
> >>>> which leads to a non-working display on qcom platforms.
> >>>>
> >>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
> >>>> and EOT packet")
> >>>>
> >>>> Cc: Marek Vasut <[e-mail]>
> >>>> Cc: Robert Foss <[e-mail]>
> >>>> Cc: Jagan Teki <[e-mail]>
> >>>> Cc: Dmitry Baryshkov <[e-mail]>
> >>>> Cc: Abhinav Kumar <[e-mail]>
> >>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
> >>>> and EOT packet")
> >>>> Reported-by: Amit Pundir <[e-mail]>
> >>>> Link:
> >>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
> >>>> Signed-off-by: Neil Armstrong <[e-mail]>
> >>>
> >>> This breaks LT9611 operation on i.MX8M Mini/Nano/Plus, so, NAK.
> >>>
> >>> I am currently using this LT9611 with Linux 6.1.y
> >>
> >> Correction, 6.1.y only with the DSIM patches backported.
> >
> > Well you'll need to keep [1] backported on your downstream branch,
> > this revert won't propagate to v6.1 stable anyway.
>
> I will still have a problem with those MX8M boards during my development
> on linux-next, so that isn't helping.
>
> >>> in production and this is not acceptable. I also believe the correct
> >>> fix is on the MSM side, not on the LT9611 driver side, since MSM
> >>> incorrectly implements these flags.
> >>
> >
> > Since [1] breaks Qcom boards on v6.5, and [1] was added for v6.5 to make
> > the bridge work
> > on i.MX8M Mini/Nano/Plus, it's not acceptable either to keep it for the
> > v6.5 release.
>
> So, we need to find a solution which works for both, I proposed one already.

We proposed another one.

Dmitry Baryshkov Aug. 2, 2023, 6:16 p.m. UTC | #14

On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[e-mail]> wrote:
>
> On 8/2/23 15:38, Dmitry Baryshkov wrote:
> > On 02/08/2023 11:52, Neil Armstrong wrote:
> >> This reverts commit [1] to fix display regression on the Dragonboard 845c
> >> (SDM845) devboard.
> >>
> >> There's a mismatch on the real action of the following flags:
> >> - MIPI_DSI_MODE_VIDEO_NO_HSA
> >> - MIPI_DSI_MODE_VIDEO_NO_HFP
> >> - MIPI_DSI_MODE_VIDEO_NO_HBP
> >> which leads to a non-working display on qcom platforms.
> >>
> >> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and
> >> EOT packet")
> >>
> >> Cc: Marek Vasut <[e-mail]>
> >> Cc: Robert Foss <[e-mail]>
> >> Cc: Jagan Teki <[e-mail]>
> >> Cc: Dmitry Baryshkov <[e-mail]>
> >> Cc: Abhinav Kumar <[e-mail]>
> >> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
> >> and EOT packet")
> >> Reported-by: Amit Pundir <[e-mail]>
> >> Link:
> >> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
> >> Signed-off-by: Neil Armstrong <[e-mail]>
> >> ---
> >>   drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
> >>   1 file changed, 1 insertion(+), 3 deletions(-)
> >>
> > Acked-by: Dmitry Baryshkov <[e-mail]> #fix db845c
> >
> > The boards broken by [1] are used in production by different parties
> > since 5.10, breaking them doesn't seem more acceptable than breaking the
> > new out-of-tree iMX8m hardware.
>
> The MX8M is also in-tree, so this does not apply.

v6.5-rc4:

$ git grep lontium,lt9611 | grep -v 9611uxc
Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
    - lontium,lt9611
Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
      compatible = "lontium,lt9611";
arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },

next-20230802:

$ git grep lontium,lt9611 | grep -v 9611uxc
Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
    - lontium,lt9611
Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
      compatible = "lontium,lt9611";
arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },

Your device is not in the tree. Your commit broke existing users.

Can we please end the argument, land the fix (this revert) for 6.5 and
work on the solution for 6.6 or 6.7?

Marek Vasut Aug. 2, 2023, 6:45 p.m. UTC | #15

On 8/2/23 20:16, Dmitry Baryshkov wrote:
> On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[e-mail]> wrote:
>>
>> On 8/2/23 15:38, Dmitry Baryshkov wrote:
>>> On 02/08/2023 11:52, Neil Armstrong wrote:
>>>> This reverts commit [1] to fix display regression on the Dragonboard 845c
>>>> (SDM845) devboard.
>>>>
>>>> There's a mismatch on the real action of the following flags:
>>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>>>> which leads to a non-working display on qcom platforms.
>>>>
>>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and
>>>> EOT packet")
>>>>
>>>> Cc: Marek Vasut <[e-mail]>
>>>> Cc: Robert Foss <[e-mail]>
>>>> Cc: Jagan Teki <[e-mail]>
>>>> Cc: Dmitry Baryshkov <[e-mail]>
>>>> Cc: Abhinav Kumar <[e-mail]>
>>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>>>> and EOT packet")
>>>> Reported-by: Amit Pundir <[e-mail]>
>>>> Link:
>>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>>>> Signed-off-by: Neil Armstrong <[e-mail]>
>>>> ---
>>>>    drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>> Acked-by: Dmitry Baryshkov <[e-mail]> #fix db845c
>>>
>>> The boards broken by [1] are used in production by different parties
>>> since 5.10, breaking them doesn't seem more acceptable than breaking the
>>> new out-of-tree iMX8m hardware.
>>
>> The MX8M is also in-tree, so this does not apply.
> 
> v6.5-rc4:
> 
> $ git grep lontium,lt9611 | grep -v 9611uxc
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>      - lontium,lt9611
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>        compatible = "lontium,lt9611";
> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
> 
> next-20230802:
> 
> $ git grep lontium,lt9611 | grep -v 9611uxc
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>      - lontium,lt9611
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>        compatible = "lontium,lt9611";
> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
> 
> Your device is not in the tree. Your commit broke existing users.

These devices are in tree:
arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc.dts
arch/arm64/boot/dts/freescale/imx8mp-data-modul-edm-sbc.dts

The LT9211 and LT9611 are both expansion modules handled by DTOs and 
bound to the DSIM (which is also in tree).

> Can we please end the argument, land the fix (this revert) for 6.5 and
> work on the solution for 6.6 or 6.7?

I would much prefer a solution which does not break my existing use 
case. It is still not even clear whether the problem really is on MX8M 
side at all, or whether it is QCOM misinterpreting flags. I cannot debug 
the later, since I have no access to that platform, nor its documentation.

Rob Clark Aug. 2, 2023, 6:49 p.m. UTC | #16

On Wed, Aug 2, 2023 at 11:16 AM Dmitry Baryshkov
<[e-mail]> wrote:
>
> On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[e-mail]> wrote:
> >
> > On 8/2/23 15:38, Dmitry Baryshkov wrote:
> > > On 02/08/2023 11:52, Neil Armstrong wrote:
> > >> This reverts commit [1] to fix display regression on the Dragonboard 845c
> > >> (SDM845) devboard.
> > >>
> > >> There's a mismatch on the real action of the following flags:
> > >> - MIPI_DSI_MODE_VIDEO_NO_HSA
> > >> - MIPI_DSI_MODE_VIDEO_NO_HFP
> > >> - MIPI_DSI_MODE_VIDEO_NO_HBP
> > >> which leads to a non-working display on qcom platforms.
> > >>
> > >> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and
> > >> EOT packet")
> > >>
> > >> Cc: Marek Vasut <[e-mail]>
> > >> Cc: Robert Foss <[e-mail]>
> > >> Cc: Jagan Teki <[e-mail]>
> > >> Cc: Dmitry Baryshkov <[e-mail]>
> > >> Cc: Abhinav Kumar <[e-mail]>
> > >> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
> > >> and EOT packet")
> > >> Reported-by: Amit Pundir <[e-mail]>
> > >> Link:
> > >> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
> > >> Signed-off-by: Neil Armstrong <[e-mail]>
> > >> ---
> > >>   drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
> > >>   1 file changed, 1 insertion(+), 3 deletions(-)
> > >>
> > > Acked-by: Dmitry Baryshkov <[e-mail]> #fix db845c
> > >
> > > The boards broken by [1] are used in production by different parties
> > > since 5.10, breaking them doesn't seem more acceptable than breaking the
> > > new out-of-tree iMX8m hardware.
> >
> > The MX8M is also in-tree, so this does not apply.
>
> v6.5-rc4:
>
> $ git grep lontium,lt9611 | grep -v 9611uxc
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>     - lontium,lt9611
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>       compatible = "lontium,lt9611";
> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
>
> next-20230802:
>
> $ git grep lontium,lt9611 | grep -v 9611uxc
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>     - lontium,lt9611
> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>       compatible = "lontium,lt9611";
> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
>
> Your device is not in the tree. Your commit broke existing users.
>
> Can we please end the argument, land the fix (this revert) for 6.5 and
> work on the solution for 6.6 or 6.7?
>

Even if they were in-tree, breaking existing hw means revert and
try-again.  Especially as we get into later -rc's

BR,
-R

> --
> With best wishes
> Dmitry

Dmitry Baryshkov Aug. 2, 2023, 6:49 p.m. UTC | #17

On 02/08/2023 21:45, Marek Vasut wrote:
> On 8/2/23 20:16, Dmitry Baryshkov wrote:
>> On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[e-mail]> wrote:
>>>
>>> On 8/2/23 15:38, Dmitry Baryshkov wrote:
>>>> On 02/08/2023 11:52, Neil Armstrong wrote:
>>>>> This reverts commit [1] to fix display regression on the 
>>>>> Dragonboard 845c
>>>>> (SDM845) devboard.
>>>>>
>>>>> There's a mismatch on the real action of the following flags:
>>>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>>>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>>>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>>>>> which leads to a non-working display on qcom platforms.
>>>>>
>>>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and
>>>>> EOT packet")
>>>>>
>>>>> Cc: Marek Vasut <[e-mail]>
>>>>> Cc: Robert Foss <[e-mail]>
>>>>> Cc: Jagan Teki <[e-mail]>
>>>>> Cc: Dmitry Baryshkov <[e-mail]>
>>>>> Cc: Abhinav Kumar <[e-mail]>
>>>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>>>>> and EOT packet")
>>>>> Reported-by: Amit Pundir <[e-mail]>
>>>>> Link:
>>>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>>>>> Signed-off-by: Neil Armstrong <[e-mail]>
>>>>> ---
>>>>>    drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>>
>>>> Acked-by: Dmitry Baryshkov <[e-mail]> #fix db845c
>>>>
>>>> The boards broken by [1] are used in production by different parties
>>>> since 5.10, breaking them doesn't seem more acceptable than breaking 
>>>> the
>>>> new out-of-tree iMX8m hardware.
>>>
>>> The MX8M is also in-tree, so this does not apply.
>>
>> v6.5-rc4:
>>
>> $ git grep lontium,lt9611 | grep -v 9611uxc
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
>> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>>      - lontium,lt9611
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>>        compatible = "lontium,lt9611";
>> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = 
>> "lontium,lt9611";
>> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
>> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = 
>> "lontium,lt9611" },
>>
>> next-20230802:
>>
>> $ git grep lontium,lt9611 | grep -v 9611uxc
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
>> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>>      - lontium,lt9611
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>>        compatible = "lontium,lt9611";
>> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = 
>> "lontium,lt9611";
>> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
>> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = 
>> "lontium,lt9611" },
>>
>> Your device is not in the tree. Your commit broke existing users.
> 
> These devices are in tree:
> arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc.dts
> arch/arm64/boot/dts/freescale/imx8mp-data-modul-edm-sbc.dts
> 
> The LT9211 and LT9611 are both expansion modules handled by DTOs and 
> bound to the DSIM (which is also in tree).

And they DT for them is not in the tree, that was my point. You have 
broken the existing user for the DTBO that is not present even in 
linux-next.

> 
>> Can we please end the argument, land the fix (this revert) for 6.5 and
>> work on the solution for 6.6 or 6.7?
> 
> I would much prefer a solution which does not break my existing use 
> case. It is still not even clear whether the problem really is on MX8M 
> side at all, or whether it is QCOM misinterpreting flags. I cannot debug 
> the later, since I have no access to that platform, nor its documentation.

You can get the RB1 for $199 and check the DSI behaviour on that 
platform. It has newer bridge, but the DSI controller is (mostly) the same.

Laurent Pinchart Aug. 2, 2023, 7:12 p.m. UTC | #18

On Wed, Aug 02, 2023 at 09:49:42PM +0300, Dmitry Baryshkov wrote:
> On 02/08/2023 21:45, Marek Vasut wrote:
> > On 8/2/23 20:16, Dmitry Baryshkov wrote:
> >> On Wed, 2 Aug 2023 at 20:34, Marek Vasut wrote:
> >>> On 8/2/23 15:38, Dmitry Baryshkov wrote:
> >>>> On 02/08/2023 11:52, Neil Armstrong wrote:
> >>>>> This reverts commit [1] to fix display regression on the 
> >>>>> Dragonboard 845c
> >>>>> (SDM845) devboard.
> >>>>>
> >>>>> There's a mismatch on the real action of the following flags:
> >>>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
> >>>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
> >>>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
> >>>>> which leads to a non-working display on qcom platforms.
> >>>>>
> >>>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and
> >>>>> EOT packet")
> >>>>>
> >>>>> Cc: Marek Vasut <[e-mail]>
> >>>>> Cc: Robert Foss <[e-mail]>
> >>>>> Cc: Jagan Teki <[e-mail]>
> >>>>> Cc: Dmitry Baryshkov <[e-mail]>
> >>>>> Cc: Abhinav Kumar <[e-mail]>
> >>>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
> >>>>> and EOT packet")
> >>>>> Reported-by: Amit Pundir <[e-mail]>
> >>>>> Link:
> >>>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
> >>>>> Signed-off-by: Neil Armstrong <[e-mail]>
> >>>>> ---
> >>>>>    drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
> >>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
> >>>>>
> >>>> Acked-by: Dmitry Baryshkov <[e-mail]> #fix db845c
> >>>>
> >>>> The boards broken by [1] are used in production by different parties
> >>>> since 5.10, breaking them doesn't seem more acceptable than breaking the
> >>>> new out-of-tree iMX8m hardware.
> >>>
> >>> The MX8M is also in-tree, so this does not apply.
> >>
> >> v6.5-rc4:
> >>
> >> $ git grep lontium,lt9611 | grep -v 9611uxc
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
> >> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
> >>      - lontium,lt9611
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
> >>        compatible = "lontium,lt9611";
> >> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
> >> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
> >> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
> >>
> >> next-20230802:
> >>
> >> $ git grep lontium,lt9611 | grep -v 9611uxc
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
> >> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
> >>      - lontium,lt9611
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
> >>        compatible = "lontium,lt9611";
> >> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
> >> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
> >> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
> >>
> >> Your device is not in the tree. Your commit broke existing users.
> > 
> > These devices are in tree:
> > arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc.dts
> > arch/arm64/boot/dts/freescale/imx8mp-data-modul-edm-sbc.dts
> > 
> > The LT9211 and LT9611 are both expansion modules handled by DTOs and 
> > bound to the DSIM (which is also in tree).
> 
> And they DT for them is not in the tree, that was my point. You have 
> broken the existing user for the DTBO that is not present even in 
> linux-next.
> 
> >> Can we please end the argument, land the fix (this revert) for 6.5 and
> >> work on the solution for 6.6 or 6.7?
> > 
> > I would much prefer a solution which does not break my existing use 
> > case. It is still not even clear whether the problem really is on MX8M 
> > side at all, or whether it is QCOM misinterpreting flags. I cannot debug 
> > the later, since I have no access to that platform, nor its documentation.
> 
> You can get the RB1 for $199 and check the DSI behaviour on that 
> platform. It has newer bridge, but the DSI controller is (mostly) the same.

Could everybody please get away from the keyboard for a few hours, take
a deep breath, and resume the discussion in a less aggressive and more
constructive way ?

Without judging the technical merits of the arguments, and which
platform gets it wrong, the commit being reverted landed in v6.5-rc1,
and breaks in-tree users. Reverting and retrying thus seems the usual
practice to me, as we are getting too close to the v6.5 release to
ensure a correct fix can be developed and merged in time. This will not
cause a regression on i.MX8M, as the commit has never appeared in a
release kernel.

This is however an unfortunate event. It is not a nice feeling to work
on enabling features for a platform and see the work being reverted at
the last minute. Neil, Dmitry, could you please help Marek figuring out
a good solution for v6.6 ? I don't think it's reasonable to ask him to
buy an RB1 and investigate the MSM side, when Linaro has access to
hardware and support.

Marek Vasut Aug. 3, 2023, 6:10 p.m. UTC | #19

On 8/2/23 20:49, Rob Clark wrote:
> On Wed, Aug 2, 2023 at 11:16 AM Dmitry Baryshkov
> <[e-mail]> wrote:
>>
>> On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[e-mail]> wrote:
>>>
>>> On 8/2/23 15:38, Dmitry Baryshkov wrote:
>>>> On 02/08/2023 11:52, Neil Armstrong wrote:
>>>>> This reverts commit [1] to fix display regression on the Dragonboard 845c
>>>>> (SDM845) devboard.
>>>>>
>>>>> There's a mismatch on the real action of the following flags:
>>>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>>>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>>>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>>>>> which leads to a non-working display on qcom platforms.
>>>>>
>>>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and
>>>>> EOT packet")
>>>>>
>>>>> Cc: Marek Vasut <[e-mail]>
>>>>> Cc: Robert Foss <[e-mail]>
>>>>> Cc: Jagan Teki <[e-mail]>
>>>>> Cc: Dmitry Baryshkov <[e-mail]>
>>>>> Cc: Abhinav Kumar <[e-mail]>
>>>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>>>>> and EOT packet")
>>>>> Reported-by: Amit Pundir <[e-mail]>
>>>>> Link:
>>>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>>>>> Signed-off-by: Neil Armstrong <[e-mail]>
>>>>> ---
>>>>>    drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>>
>>>> Acked-by: Dmitry Baryshkov <[e-mail]> #fix db845c
>>>>
>>>> The boards broken by [1] are used in production by different parties
>>>> since 5.10, breaking them doesn't seem more acceptable than breaking the
>>>> new out-of-tree iMX8m hardware.
>>>
>>> The MX8M is also in-tree, so this does not apply.
>>
>> v6.5-rc4:
>>
>> $ git grep lontium,lt9611 | grep -v 9611uxc
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
>> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>>      - lontium,lt9611
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>>        compatible = "lontium,lt9611";
>> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
>> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
>> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
>>
>> next-20230802:
>>
>> $ git grep lontium,lt9611 | grep -v 9611uxc
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
>> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>>      - lontium,lt9611
>> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
>>        compatible = "lontium,lt9611";
>> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
>> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
>> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
>>
>> Your device is not in the tree. Your commit broke existing users.
>>
>> Can we please end the argument, land the fix (this revert) for 6.5 and
>> work on the solution for 6.6 or 6.7?
>>
> 
> Even if they were in-tree, breaking existing hw means revert and
> try-again.  Especially as we get into later -rc's

Then just apply the revert, I don't have time to debug this right this 
moment, and it is anyway meaningless until I can look at the bus with 
DSI bus analyzer.

Neil Armstrong Aug. 4, 2023, 8:27 a.m. UTC | #20

On 02/08/2023 11:08, Dmitry Baryshkov wrote:
> On Wed, 2 Aug 2023 at 11:52, Neil Armstrong <[e-mail]> wrote:
>>
>> This reverts commit [1] to fix display regression on the Dragonboard 845c
>> (SDM845) devboard.
>>
>> There's a mismatch on the real action of the following flags:
>> - MIPI_DSI_MODE_VIDEO_NO_HSA
>> - MIPI_DSI_MODE_VIDEO_NO_HFP
>> - MIPI_DSI_MODE_VIDEO_NO_HBP
>> which leads to a non-working display on qcom platforms.
>>
>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> 
> Nit: I think the preferred form is to write `... reverts commit abcdef
> ("foo and bar")', but I might be wrong.

Yep, I'll fix all that while applying.

Thanks,
Neil

> 
> Other than that:
> 
> Acked-by: Dmitry Baryshkov <[e-mail]>
> 
>>
>> Cc: Marek Vasut <[e-mail]>
>> Cc: Robert Foss <[e-mail]>
>> Cc: Jagan Teki <[e-mail]>
>> Cc: Dmitry Baryshkov <[e-mail]>
>> Cc: Abhinav Kumar <[e-mail]>
>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
>> Reported-by: Amit Pundir <[e-mail]>
>> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=[e-mail]/
>> Signed-off-by: Neil Armstrong <[e-mail]>
>> ---
>>   drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
>> index 5163e5224aad..9663601ce098 100644
>> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
>> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
>> @@ -774,9 +774,7 @@ 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_NO_HSA |
>> -                         MIPI_DSI_MODE_VIDEO_NO_HFP | MIPI_DSI_MODE_VIDEO_NO_HBP |
>> -                         MIPI_DSI_MODE_NO_EOT_PACKET;
>> +                         MIPI_DSI_MODE_VIDEO_HSE;
>>
>>          ret = devm_mipi_dsi_attach(dev, dsi);
>>          if (ret < 0) {
>>
>> ---
>> base-commit: f590814603bf2dd8620584b7d59ae94d7c186c69
>> change-id: 20230802-revert-do-not-generate-hfp-hbp-hsa-eot-packet-6f042b1ba813
>>
>> Best regards,
>> --
>> Neil Armstrong <[e-mail]>
>>
> 
>

Neil Armstrong Aug. 4, 2023, 8:30 a.m. UTC | #21

Hi Marek,

On 03/08/2023 20:10, Marek Vasut wrote:
> On 8/2/23 20:49, Rob Clark wrote:
>> On Wed, Aug 2, 2023 at 11:16 AM Dmitry Baryshkov
>> <[e-mail]> wrote:
>>>

<snip>

>>>
>>> Can we please end the argument, land the fix (this revert) for 6.5 and
>>> work on the solution for 6.6 or 6.7?
>>>
>>
>> Even if they were in-tree, breaking existing hw means revert and
>> try-again.  Especially as we get into later -rc's
> 
> Then just apply the revert, I don't have time to debug this right this moment, and it is anyway meaningless until I can look at the bus with DSI bus analyzer.

I'm applying it, then I'll like to find an explanation when you'll be able to run the DSI bus analyzer.

I'll be able to test for regressions on the db845c board if you require, just ping me on irc.

We should clearly define those flags actions and probably add new flags to reflect the way the qcom
dsi controller acts.

Thanks,
Neil

Neil Armstrong Aug. 4, 2023, 8:30 a.m. UTC | #22

Hi,

On Wed, 02 Aug 2023 10:52:22 +0200, Neil Armstrong wrote:
> This reverts commit [1] to fix display regression on the Dragonboard 845c
> (SDM845) devboard.
> 
> There's a mismatch on the real action of the following flags:
> - MIPI_DSI_MODE_VIDEO_NO_HSA
> - MIPI_DSI_MODE_VIDEO_NO_HFP
> - MIPI_DSI_MODE_VIDEO_NO_HBP
> which leads to a non-working display on qcom platforms.
> 
> [...]

Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-fixes)

[1/1] Revert "drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet"
      https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3c6bd1b7e2043fb00ce6b622709d176609431406

Patch

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
index 5163e5224aad..9663601ce098 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -774,9 +774,7 @@  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_NO_HSA |
-			  MIPI_DSI_MODE_VIDEO_NO_HFP | MIPI_DSI_MODE_VIDEO_NO_HBP |
-			  MIPI_DSI_MODE_NO_EOT_PACKET;
+			  MIPI_DSI_MODE_VIDEO_HSE;
 
 	ret = devm_mipi_dsi_attach(dev, dsi);
 	if (ret < 0) {