| Message ID | 20231023-display-support-v7-2-6703f3e26831@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add display support for the MT8365-EVK board | expand |
On Fri, 2025-01-10 at 14:31 +0100, Alexandre Mergnat wrote: > External email : Please do not click links or open attachments until you have verified the sender or the content. > > > Currently, mtk_dsi_lane_ready (which setup the DSI lane) is triggered > before mtk_dsi_poweron. lanes_ready flag toggle to true during > mtk_dsi_lane_ready function, and the DSI module is set up during > mtk_dsi_poweron. > > Later, during panel driver init, mtk_dsi_lane_ready is triggered but does > nothing because lanes are considered ready. Unfortunately, when the panel > driver try to communicate, the DSI returns a timeout. > > The solution found here is to put lanes_ready flag to false after the DSI > module setup into mtk_dsi_poweron to init the DSI lanes after the power / > setup of the DSI module. I'm not clear about what happen. I think this DSI flow has worked for a long time. So only some panel has problem? And another question. Do you mean mtk_dsi_lane_ready() do some setting to hardware, but lane is not actually ready? Or mtk_dsi_lane_ready() configure the hardware and lane is is actually ready, but something make it not ready again, what's the thing which break lane ready? If this is a bug fix, add Fixes tag. Regards, CK > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > index e61b9bc68e9a..dcf0d93881b5 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -724,6 +724,8 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) > mtk_dsi_config_vdo_timing(dsi); > mtk_dsi_set_interrupt_enable(dsi); > > + dsi->lanes_ready = false; > + > return 0; > err_disable_engine_clk: > clk_disable_unprepare(dsi->engine_clk); > > -- > 2.25.1 >
Hi CK. On 17/02/2025 08:56, CK Hu (胡俊光) wrote: > On Fri, 2025-01-10 at 14:31 +0100, Alexandre Mergnat wrote: >> External email : Please do not click links or open attachments until you have verified the sender or the content. >> >> >> Currently, mtk_dsi_lane_ready (which setup the DSI lane) is triggered >> before mtk_dsi_poweron. lanes_ready flag toggle to true during >> mtk_dsi_lane_ready function, and the DSI module is set up during >> mtk_dsi_poweron. >> >> Later, during panel driver init, mtk_dsi_lane_ready is triggered but does >> nothing because lanes are considered ready. Unfortunately, when the panel >> driver try to communicate, the DSI returns a timeout. >> >> The solution found here is to put lanes_ready flag to false after the DSI >> module setup into mtk_dsi_poweron to init the DSI lanes after the power / >> setup of the DSI module. > > I'm not clear about what happen. > I think this DSI flow has worked for a long time. > So only some panel has problem? I don't know if it's related to a specific panel or not. > > And another question. > Do you mean mtk_dsi_lane_ready() do some setting to hardware, but lane is not actually ready? The workflow should be: ... | dsi->lanes_ready = false | Power-on | setup dsi lanes | dsi->lanes_ready = true (to avoid re-do dsi lanes setup) | ... I observe (print function name called + dsi->lanes_ready value): [ 9.086030] mtk_dsi_probe 0 [ 9.662319] mtk_dsi_host_attach 0 [ 9.662941] mtk_dsi_encoder_init [ 9.984685] mtk_dsi_poweron 0 [ 10.043755] mtk_dsi_host_transfer 0 [ 10.043769] mtk_dsi_lane_ready 0 [ 10.055837] mtk_dsi_host_transfer 1 [ 10.055853] mtk_dsi_lane_ready 1 [ 10.179788] mtk_dsi_host_transfer 1 [ 10.179803] mtk_dsi_lane_ready 1 [ 10.179880] mtk_dsi_host_transfer 1 [ 10.179885] mtk_dsi_lane_ready 1 [ 10.179920] mtk_dsi_host_transfer 1 [ 10.179923] mtk_dsi_lane_ready 1 [ 10.179986] mtk_dsi_host_transfer 1 [ 10.179993] mtk_dsi_lane_ready 1 [ 10.180134] mtk_dsi_host_transfer 1 [ 10.180143] mtk_dsi_lane_ready 1 [ 10.180175] mtk_dsi_host_transfer 1 [ 10.180178] mtk_dsi_lane_ready 1 [ 10.180223] mtk_dsi_host_transfer 1 [ 10.180226] mtk_dsi_lane_ready 1 [ 10.180245] mtk_dsi_host_transfer 1 [ 10.180248] mtk_dsi_lane_ready 1 [ 10.180278] mtk_dsi_host_transfer 1 [ 10.180280] mtk_dsi_lane_ready 1 [ 10.180312] mtk_dsi_host_transfer 1 [ 10.180314] mtk_dsi_lane_ready 1 [ 10.203774] mtk_dsi_bridge_atomic_pre_enable [ 10.203787] mtk_dsi_poweron 1 [ 10.203793] mtk_output_dsi_enable [ 10.203795] mtk_dsi_lane_ready 1 [ 10.471517] mtk_dsi_host_transfer 1 [ 10.486962] mtk_dsi_lane_ready 1 [ 10.487244] mtk_dsi_host_transfer 1 [ 10.503733] mtk_dsi_lane_ready 1 Here the mtk_dsi_lane_ready function: static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) { if (!dsi->lanes_ready) { dsi->lanes_ready = true; mtk_dsi_rxtx_control(dsi); usleep_range(30, 100); mtk_dsi_reset_dphy(dsi); mtk_dsi_clk_ulp_mode_leave(dsi); mtk_dsi_lane0_ulp_mode_leave(dsi); mtk_dsi_clk_hs_mode(dsi, 0); usleep_range(1000, 3000); /* The reaction time after pulling up the mipi signal for dsi_rx */ } } As you can see, something call "mtk_dsi_bridge_atomic_pre_enable" then mtk_dsi_poweron is called a second time. This issue is probably due to the probe order (race condition). After all, IMHO, after a poweron, the registers status should be consider as unknown (or at least HW default value), so, lanes setup has to be done. This solution improve the driver's robustness. > Or mtk_dsi_lane_ready() configure the hardware and lane is is actually ready, > but something make it not ready again, what's the thing which break lane ready? > > If this is a bug fix, add Fixes tag. > > Regards, > CK > >> >> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >> --- >> drivers/gpu/drm/mediatek/mtk_dsi.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c >> index e61b9bc68e9a..dcf0d93881b5 100644 >> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c >> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c >> @@ -724,6 +724,8 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) >> mtk_dsi_config_vdo_timing(dsi); >> mtk_dsi_set_interrupt_enable(dsi); >> >> + dsi->lanes_ready = false; >> + >> return 0; >> err_disable_engine_clk: >> clk_disable_unprepare(dsi->engine_clk); >> >> -- >> 2.25.1 >> > > > ************* MEDIATEK Confidentiality Notice > ******************** > The information contained in this e-mail message (including any > attachments) may be confidential, proprietary, privileged, or otherwise > exempt from disclosure under applicable laws. It is intended to be > conveyed only to the designated recipient(s). Any use, dissemination, > distribution, printing, retaining or copying of this e-mail (including its > attachments) by unintended recipient(s) is strictly prohibited and may > be unlawful. If you are not an intended recipient of this e-mail, or believe > > that you have received this e-mail in error, please notify the sender > immediately (by replying to this e-mail), delete any and all copies of > this e-mail (including any attachments) from your system, and do not > disclose the content of this e-mail to any other person. Thank you! >
Il 17/02/25 16:03, Alexandre Mergnat ha scritto: > Hi CK. > > On 17/02/2025 08:56, CK Hu (胡俊光) wrote: >> On Fri, 2025-01-10 at 14:31 +0100, Alexandre Mergnat wrote: >>> External email : Please do not click links or open attachments until you have >>> verified the sender or the content. >>> >>> >>> Currently, mtk_dsi_lane_ready (which setup the DSI lane) is triggered >>> before mtk_dsi_poweron. lanes_ready flag toggle to true during >>> mtk_dsi_lane_ready function, and the DSI module is set up during >>> mtk_dsi_poweron. >>> >>> Later, during panel driver init, mtk_dsi_lane_ready is triggered but does >>> nothing because lanes are considered ready. Unfortunately, when the panel >>> driver try to communicate, the DSI returns a timeout. >>> >>> The solution found here is to put lanes_ready flag to false after the DSI >>> module setup into mtk_dsi_poweron to init the DSI lanes after the power / >>> setup of the DSI module. >> >> I'm not clear about what happen. >> I think this DSI flow has worked for a long time. >> So only some panel has problem? > > I don't know if it's related to a specific panel or not. > >> >> And another question. >> Do you mean mtk_dsi_lane_ready() do some setting to hardware, but lane is not >> actually ready? > > The workflow should be: > ... | dsi->lanes_ready = false | Power-on | setup dsi lanes | dsi->lanes_ready = > true (to avoid re-do dsi lanes setup) | ... > > I observe (print function name called + dsi->lanes_ready value): Alex, the first poweron is called by mtk_dsi_ddp_start() - and the start callback is internal to the mediatek-drm driver. That callback is called by mtk_crtc during setup and during bridge enable(), and there we go with suboptimal code design backfiring - instead of using what the DRM APIs provide, this driver uses something custom *and* the DRM APIs, giving this issue. Part of what mtk_crtc does is duplicated with what the DRM APIs want to do, so there you go, that's your problem here :-) Should I go on with describing the next step(s), or is that obvious for everyone? :-) Cheers, Angelo > > [ 9.086030] mtk_dsi_probe 0 > [ 9.662319] mtk_dsi_host_attach 0 > [ 9.662941] mtk_dsi_encoder_init > [ 9.984685] mtk_dsi_poweron 0 > [ 10.043755] mtk_dsi_host_transfer 0 > [ 10.043769] mtk_dsi_lane_ready 0 > [ 10.055837] mtk_dsi_host_transfer 1 > [ 10.055853] mtk_dsi_lane_ready 1 > [ 10.179788] mtk_dsi_host_transfer 1 > [ 10.179803] mtk_dsi_lane_ready 1 > [ 10.179880] mtk_dsi_host_transfer 1 > [ 10.179885] mtk_dsi_lane_ready 1 > [ 10.179920] mtk_dsi_host_transfer 1 > [ 10.179923] mtk_dsi_lane_ready 1 > [ 10.179986] mtk_dsi_host_transfer 1 > [ 10.179993] mtk_dsi_lane_ready 1 > [ 10.180134] mtk_dsi_host_transfer 1 > [ 10.180143] mtk_dsi_lane_ready 1 > [ 10.180175] mtk_dsi_host_transfer 1 > [ 10.180178] mtk_dsi_lane_ready 1 > [ 10.180223] mtk_dsi_host_transfer 1 > [ 10.180226] mtk_dsi_lane_ready 1 > [ 10.180245] mtk_dsi_host_transfer 1 > [ 10.180248] mtk_dsi_lane_ready 1 > [ 10.180278] mtk_dsi_host_transfer 1 > [ 10.180280] mtk_dsi_lane_ready 1 > [ 10.180312] mtk_dsi_host_transfer 1 > [ 10.180314] mtk_dsi_lane_ready 1 > [ 10.203774] mtk_dsi_bridge_atomic_pre_enable > [ 10.203787] mtk_dsi_poweron 1 > [ 10.203793] mtk_output_dsi_enable > [ 10.203795] mtk_dsi_lane_ready 1 > [ 10.471517] mtk_dsi_host_transfer 1 > [ 10.486962] mtk_dsi_lane_ready 1 > [ 10.487244] mtk_dsi_host_transfer 1 > [ 10.503733] mtk_dsi_lane_ready 1 > > Here the mtk_dsi_lane_ready function: > > static void mtk_dsi_lane_ready(struct mtk_dsi *dsi) > { > if (!dsi->lanes_ready) { > dsi->lanes_ready = true; > mtk_dsi_rxtx_control(dsi); > usleep_range(30, 100); > mtk_dsi_reset_dphy(dsi); > mtk_dsi_clk_ulp_mode_leave(dsi); > mtk_dsi_lane0_ulp_mode_leave(dsi); > mtk_dsi_clk_hs_mode(dsi, 0); > usleep_range(1000, 3000); > /* The reaction time after pulling up the mipi signal for dsi_rx */ > } > } > > > As you can see, something call "mtk_dsi_bridge_atomic_pre_enable" then > mtk_dsi_poweron is called a second time. This issue is probably due to the probe > order (race condition). > After all, IMHO, after a poweron, the registers status should be consider as > unknown (or at least HW default value), so, lanes setup has to be done. This > solution improve the driver's robustness. > > >> Or mtk_dsi_lane_ready() configure the hardware and lane is is actually ready, >> but something make it not ready again, what's the thing which break lane ready? >> >> If this is a bug fix, add Fixes tag. >> >> Regards, >> CK >> >>> >>> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> >>> --- >>> drivers/gpu/drm/mediatek/mtk_dsi.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/ >>> mtk_dsi.c >>> index e61b9bc68e9a..dcf0d93881b5 100644 >>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c >>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c >>> @@ -724,6 +724,8 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) >>> mtk_dsi_config_vdo_timing(dsi); >>> mtk_dsi_set_interrupt_enable(dsi); >>> >>> + dsi->lanes_ready = false; >>> + >>> return 0; >>> err_disable_engine_clk: >>> clk_disable_unprepare(dsi->engine_clk); >>> >>> -- >>> 2.25.1 >>> >>
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index e61b9bc68e9a..dcf0d93881b5 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -724,6 +724,8 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) mtk_dsi_config_vdo_timing(dsi); mtk_dsi_set_interrupt_enable(dsi); + dsi->lanes_ready = false; + return 0; err_disable_engine_clk: clk_disable_unprepare(dsi->engine_clk);
Currently, mtk_dsi_lane_ready (which setup the DSI lane) is triggered before mtk_dsi_poweron. lanes_ready flag toggle to true during mtk_dsi_lane_ready function, and the DSI module is set up during mtk_dsi_poweron. Later, during panel driver init, mtk_dsi_lane_ready is triggered but does nothing because lanes are considered ready. Unfortunately, when the panel driver try to communicate, the DSI returns a timeout. The solution found here is to put lanes_ready flag to false after the DSI module setup into mtk_dsi_poweron to init the DSI lanes after the power / setup of the DSI module. Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com> --- drivers/gpu/drm/mediatek/mtk_dsi.c | 2 ++ 1 file changed, 2 insertions(+)