LinuxLists.cc - [PATCH] Revert "drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet"

[permalink] [raw]

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

On Wed, 2 Aug 2023 at 11:52, Neil Armstrong <[email protected]> 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 <[email protected]>

>
> Cc: Marek Vasut <[email protected]>
> Cc: Robert Foss <[email protected]>
> Cc: Jagan Teki <[email protected]>
> Cc: Dmitry Baryshkov <[email protected]>
> Cc: Abhinav Kumar <[email protected]>
> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> Reported-by: Amit Pundir <[email protected]>
> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> 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 <[email protected]>
>

--
With best wishes
Dmitry

2023-08-02 10:27:43

by Amit Pundir

[permalink] [raw]

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

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

Tested-by: Amit Pundir <[email protected]>

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 <[email protected]>
> Cc: Robert Foss <[email protected]>
> Cc: Jagan Teki <[email protected]>
> Cc: Dmitry Baryshkov <[email protected]>
> Cc: Abhinav Kumar <[email protected]>
> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> Reported-by: Amit Pundir <[email protected]>
> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> 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 <[email protected]>
>

2023-08-02 13:13:54

[permalink] [raw]

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

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 <[email protected]>
>> Cc: Robert Foss <[email protected]>
>> Cc: Jagan Teki <[email protected]>
>> Cc: Dmitry Baryshkov <[email protected]>
>> Cc: Abhinav Kumar <[email protected]>
>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>> and EOT packet")
>> Reported-by: Amit Pundir <[email protected]>
>> Link:
>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
>> Signed-off-by: Neil Armstrong <[email protected]>
>
> 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.

2023-08-02 13:35:19

[permalink] [raw]

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

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 <[email protected]>
> Cc: Robert Foss <[email protected]>
> Cc: Jagan Teki <[email protected]>
> Cc: Dmitry Baryshkov <[email protected]>
> Cc: Abhinav Kumar <[email protected]>
> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
> Reported-by: Amit Pundir <[email protected]>
> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
> Signed-off-by: Neil Armstrong <[email protected]>

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.

2023-08-02 13:39:27

[permalink] [raw]

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

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 <[email protected]>
>>> Cc: Robert Foss <[email protected]>
>>> Cc: Jagan Teki <[email protected]>
>>> Cc: Dmitry Baryshkov <[email protected]>
>>> Cc: Abhinav Kumar <[email protected]>
>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
>>> Reported-by: Amit Pundir <[email protected]>
>>> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
>>> Signed-off-by: Neil Armstrong <[email protected]>
>>
>> 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

2023-08-02 14:16:58

[permalink] [raw]

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

2023-08-02 14:17:41

[permalink] [raw]

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

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

--
With best wishes
Dmitry

2023-08-02 15:02:18

[permalink] [raw]

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

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 <[email protected]>
>> Cc: Robert Foss <[email protected]>
>> Cc: Jagan Teki <[email protected]>
>> Cc: Dmitry Baryshkov <[email protected]>
>> Cc: Abhinav Kumar <[email protected]>
>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>> and EOT packet")
>> Reported-by: Amit Pundir <[email protected]>
>> Link:
>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
>> Signed-off-by: Neil Armstrong <[email protected]>
>
> 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).

--
With best wishes
Dmitry

2023-08-02 17:44:17

[permalink] [raw]

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

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

2023-08-02 17:59:43

[permalink] [raw]

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

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 <[email protected]>
>>> Cc: Robert Foss <[email protected]>
>>> Cc: Jagan Teki <[email protected]>
>>> Cc: Dmitry Baryshkov <[email protected]>
>>> Cc: Abhinav Kumar <[email protected]>
>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>>> and EOT packet")
>>> Reported-by: Amit Pundir <[email protected]>
>>> Link:
>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
>>> Signed-off-by: Neil Armstrong <[email protected]>
>>
>> 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.

2023-08-02 18:09:41

[permalink] [raw]

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

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 <[email protected]>
>>>> Cc: Robert Foss <[email protected]>
>>>> Cc: Jagan Teki <[email protected]>
>>>> Cc: Dmitry Baryshkov <[email protected]>
>>>> Cc: Abhinav Kumar <[email protected]>
>>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>>>> and EOT packet")
>>>> Reported-by: Amit Pundir <[email protected]>
>>>> Link:
>>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
>>>> Signed-off-by: Neil Armstrong <[email protected]>
>>>
>>> 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.

2023-08-02 19:03:52

[permalink] [raw]

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

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 <[email protected]>
>>> Cc: Robert Foss <[email protected]>
>>> Cc: Jagan Teki <[email protected]>
>>> Cc: Dmitry Baryshkov <[email protected]>
>>> Cc: Abhinav Kumar <[email protected]>
>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>>> and EOT packet")
>>> Reported-by: Amit Pundir <[email protected]>
>>> Link:
>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
>>> Signed-off-by: Neil Armstrong <[email protected]>
>>
>> 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.

2023-08-02 19:08:06

[permalink] [raw]

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

On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[email protected]> 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 <[email protected]>
> >> Cc: Robert Foss <[email protected]>
> >> Cc: Jagan Teki <[email protected]>
> >> Cc: Dmitry Baryshkov <[email protected]>
> >> Cc: Abhinav Kumar <[email protected]>
> >> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
> >> and EOT packet")
> >> Reported-by: Amit Pundir <[email protected]>
> >> Link:
> >> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
> >> Signed-off-by: Neil Armstrong <[email protected]>
> >> ---
> >> drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
> >> 1 file changed, 1 insertion(+), 3 deletions(-)
> >>
> > Acked-by: Dmitry Baryshkov <[email protected]> #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?

--
With best wishes
Dmitry

2023-08-02 19:40:06

[permalink] [raw]

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

On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[email protected]> 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 <[email protected]>
> >>>> Cc: Robert Foss <[email protected]>
> >>>> Cc: Jagan Teki <[email protected]>
> >>>> Cc: Dmitry Baryshkov <[email protected]>
> >>>> Cc: Abhinav Kumar <[email protected]>
> >>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
> >>>> and EOT packet")
> >>>> Reported-by: Amit Pundir <[email protected]>
> >>>> Link:
> >>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
> >>>> Signed-off-by: Neil Armstrong <[email protected]>
> >>>
> >>> 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.

--
With best wishes
Dmitry

2023-08-02 19:48:17

by Rob Clark

[permalink] [raw]

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

On Wed, Aug 2, 2023 at 11:16 AM Dmitry Baryshkov
<[email protected]> wrote:
>
> On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[email protected]> 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 <[email protected]>
> > >> Cc: Robert Foss <[email protected]>
> > >> Cc: Jagan Teki <[email protected]>
> > >> Cc: Dmitry Baryshkov <[email protected]>
> > >> Cc: Abhinav Kumar <[email protected]>
> > >> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
> > >> and EOT packet")
> > >> Reported-by: Amit Pundir <[email protected]>
> > >> Link:
> > >> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
> > >> Signed-off-by: Neil Armstrong <[email protected]>
> > >> ---
> > >> drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
> > >> 1 file changed, 1 insertion(+), 3 deletions(-)
> > >>
> > > Acked-by: Dmitry Baryshkov <[email protected]> #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

2023-08-02 19:56:37

[permalink] [raw]

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

On 8/2/23 20:16, Dmitry Baryshkov wrote:
> On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[email protected]> 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 <[email protected]>
>>>> Cc: Robert Foss <[email protected]>
>>>> Cc: Jagan Teki <[email protected]>
>>>> Cc: Dmitry Baryshkov <[email protected]>
>>>> Cc: Abhinav Kumar <[email protected]>
>>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>>>> and EOT packet")
>>>> Reported-by: Amit Pundir <[email protected]>
>>>> Link:
>>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
>>>> Signed-off-by: Neil Armstrong <[email protected]>
>>>> ---
>>>> drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>> Acked-by: Dmitry Baryshkov <[email protected]> #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.

2023-08-02 20:32:16

by Laurent Pinchart

[permalink] [raw]

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

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

--
Regards,

Laurent Pinchart

2023-08-02 20:54:03

[permalink] [raw]

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

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

--
With best wishes
Dmitry

2023-08-03 18:43:13

[permalink] [raw]

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

On 8/2/23 20:49, Rob Clark wrote:
> On Wed, Aug 2, 2023 at 11:16 AM Dmitry Baryshkov
> <[email protected]> wrote:
>>
>> On Wed, 2 Aug 2023 at 20:34, Marek Vasut <[email protected]> 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 <[email protected]>
>>>>> Cc: Robert Foss <[email protected]>
>>>>> Cc: Jagan Teki <[email protected]>
>>>>> Cc: Dmitry Baryshkov <[email protected]>
>>>>> Cc: Abhinav Kumar <[email protected]>
>>>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
>>>>> and EOT packet")
>>>>> Reported-by: Amit Pundir <[email protected]>
>>>>> Link:
>>>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
>>>>> Signed-off-by: Neil Armstrong <[email protected]>
>>>>> ---
>>>>> drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
>>>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>>>
>>>> Acked-by: Dmitry Baryshkov <[email protected]> #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.

2023-08-04 08:51:13

[permalink] [raw]

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

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
>> <[email protected]> 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

2023-08-04 09:13:04

[permalink] [raw]

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

On 02/08/2023 11:08, Dmitry Baryshkov wrote:
> On Wed, 2 Aug 2023 at 11:52, Neil Armstrong <[email protected]> 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 <[email protected]>
>
>>
>> Cc: Marek Vasut <[email protected]>
>> Cc: Robert Foss <[email protected]>
>> Cc: Jagan Teki <[email protected]>
>> Cc: Dmitry Baryshkov <[email protected]>
>> Cc: Abhinav Kumar <[email protected]>
>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and EOT packet")
>> Reported-by: Amit Pundir <[email protected]>
>> Link: https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> 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 <[email protected]>
>>
>
>

2023-08-04 09:15:40