| Message ID | 20230613-panel-dsi-disable-v1-1-5e454e409fa8@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panel: move some dsi commands from unprepare to disable | expand |
[added Dmitry to Cc, since he suggested doing this in [1]] On Tue, Jun 13, 2023 at 12:36:52AM +0100, Caleb Connolly wrote: > The commit 007ac0262b0d ("drm/msm/dsi: switch to DRM_PANEL_BRIDGE") > breaks panels which send DSI commands in their .unprepare callbacks. > Migrate to using .disable for that for some SDM845 panels. > > Co-developed-by: Joel Selvaraj <joelselvaraj.oss@gmail.com> > Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com> > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > drivers/gpu/drm/panel/panel-ebbg-ft8719.c | 18 +++++++----------- > drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 11 ++++++++++- > drivers/gpu/drm/panel/panel-visionox-rm69299.c | 11 ++++++++++- > 3 files changed, 27 insertions(+), 13 deletions(-) After Dmitry's description in [1] it's still not quite clear to me if the MSM DSI driver behaves wrong here (perhaps just "differently"?) or if most of the panel drivers have this set up wrong. This patch suggests that panel drivers shouldn't send any DSI commands in .unprepare(). If this is really the case I think this change should be applied to all panel drivers, not just the ones that happen to be used with SDM845 devices. There are several others that also send DSI commands in .unprepare(). I'm still quite confused about what exactly is supposed to be in (un)prepare and what in enable/disable. I've seen some related discussion every now and then but it's still quite inconsistent across different panel drivers... Can someone clarify this? Thanks! Stephan [1]: https://lore.kernel.org/linux-arm-msm/CAA8EJpq_9iC1rkiZVom28Kv_B3QLd4pBgFObxBfSpJ+Xh=Mp1g@mail.gmail.com/
On Tue, Jun 13, 2023 at 11:08 PM Stephan Gerhold <stephan@gerhold.net> wrote: > I'm still quite confused about what exactly is supposed to be in > (un)prepare and what in enable/disable. I've seen some related > discussion every now and then but it's still quite inconsistent across > different panel drivers... Can someone clarify this? It is somewhat clarified in commit 45527d435c5e39b6eec4aa0065a562e7cf05d503 that added the callbacks: Author: Ajay Kumar <ajaykumar.rs@samsung.com> Date: Fri Jul 18 02:13:48 2014 +0530 drm/panel: add .prepare() and .unprepare() functions Panels often require an initialization sequence that consists of three steps: a) powering up the panel, b) starting transmission of video data and c) enabling the panel (e.g. turn on backlight). This is usually necessary to avoid visual glitches at the beginning of video data transmission. Similarly, the shutdown sequence is typically done in three steps as well: a) disable the panel (e.g. turn off backlight), b) cease video data transmission and c) power down the panel. Currently drivers can only implement .enable() and .disable() functions, which is not enough to implement the above sequences. This commit adds a second pair of functions, .prepare() and .unprepare() to allow more fine-grained control over when the above steps are performed. Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> [treding: rewrite changelog, add kerneldoc] Signed-off-by: Thierry Reding <treding@nvidia.com> My interpretation is that .enable/.disable is for enabling/disabling backlight and well, make things show up on the display, and that happens quickly. prepare/unprepare is for everything else setting up/tearing down the data transmission pipeline. In the clock subsystem the enable/disable could be called in fastpath and prepare/unprepare only from slowpath so e.g an IRQ handler can gate a simple gated clock. This semantic seems to have nothing to do with the display semantic. :/ Yours, Linus Walleij
On 14/06/2023 22:58, Linus Walleij wrote: > On Tue, Jun 13, 2023 at 11:08 PM Stephan Gerhold <stephan@gerhold.net> wrote: > >> I'm still quite confused about what exactly is supposed to be in >> (un)prepare and what in enable/disable. I've seen some related >> discussion every now and then but it's still quite inconsistent across >> different panel drivers... Can someone clarify this? > > It is somewhat clarified in commit 45527d435c5e39b6eec4aa0065a562e7cf05d503 > that added the callbacks: > > Author: Ajay Kumar <ajaykumar.rs@samsung.com> > Date: Fri Jul 18 02:13:48 2014 +0530 > > drm/panel: add .prepare() and .unprepare() functions > > Panels often require an initialization sequence that consists of three > steps: a) powering up the panel, b) starting transmission of video data > and c) enabling the panel (e.g. turn on backlight). This is usually > necessary to avoid visual glitches at the beginning of video data > transmission. > > Similarly, the shutdown sequence is typically done in three steps as > well: a) disable the panel (e.g. turn off backlight), b) cease video > data transmission and c) power down the panel. > > Currently drivers can only implement .enable() and .disable() functions, > which is not enough to implement the above sequences. This commit adds a > second pair of functions, .prepare() and .unprepare() to allow more > fine-grained control over when the above steps are performed. > > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> > [treding: rewrite changelog, add kerneldoc] > Signed-off-by: Thierry Reding <treding@nvidia.com> > > My interpretation is that .enable/.disable is for enabling/disabling > backlight and well, make things show up on the display, and that > happens quickly. > > prepare/unprepare is for everything else setting up/tearing down > the data transmission pipeline. > > In the clock subsystem the enable/disable could be called in fastpath > and prepare/unprepare only from slowpath so e.g an IRQ handler > can gate a simple gated clock. This semantic seems to have nothing > to do with the display semantic. :/ It had to do, .prepare is called when the DSI link is at LP11 state before it has switched to the VIDEO mode, and .unprepare is the reverse when VIDEO mode has been disabled and before the DSI link is totally disabled. https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L938 then https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L855 but Doug has started changing this starting with MSM DSI driver, leading to current panel drivers not working anymore with the current DSI init mode and requires setting pre_enable_prev_first for only some DSI hosts who switched out of set_mode(). The DSI init model doesn't fit at all with the atomic bridge model and some DSI controllers doesn't support the same features like the allwinner DSI controller not support sending LP commands when in HS video mode for example. Neil > > Yours, > Linus Walleij
On Thu, Jun 15, 2023 at 09:49:27AM +0200, Neil Armstrong wrote: > On 14/06/2023 22:58, Linus Walleij wrote: > > On Tue, Jun 13, 2023 at 11:08 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > > > I'm still quite confused about what exactly is supposed to be in > > > (un)prepare and what in enable/disable. I've seen some related > > > discussion every now and then but it's still quite inconsistent across > > > different panel drivers... Can someone clarify this? > > > > It is somewhat clarified in commit 45527d435c5e39b6eec4aa0065a562e7cf05d503 > > that added the callbacks: > > > > Author: Ajay Kumar <ajaykumar.rs@samsung.com> > > Date: Fri Jul 18 02:13:48 2014 +0530 > > > > drm/panel: add .prepare() and .unprepare() functions > > > > Panels often require an initialization sequence that consists of three > > steps: a) powering up the panel, b) starting transmission of video data > > and c) enabling the panel (e.g. turn on backlight). This is usually > > necessary to avoid visual glitches at the beginning of video data > > transmission. > > > > Similarly, the shutdown sequence is typically done in three steps as > > well: a) disable the panel (e.g. turn off backlight), b) cease video > > data transmission and c) power down the panel. > > > > Currently drivers can only implement .enable() and .disable() functions, > > which is not enough to implement the above sequences. This commit adds a > > second pair of functions, .prepare() and .unprepare() to allow more > > fine-grained control over when the above steps are performed. > > > > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> > > [treding: rewrite changelog, add kerneldoc] > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > > My interpretation is that .enable/.disable is for enabling/disabling > > backlight and well, make things show up on the display, and that > > happens quickly. > > > > prepare/unprepare is for everything else setting up/tearing down > > the data transmission pipeline. > > > > In the clock subsystem the enable/disable could be called in fastpath > > and prepare/unprepare only from slowpath so e.g an IRQ handler > > can gate a simple gated clock. This semantic seems to have nothing > > to do with the display semantic. :/ > > It had to do, .prepare is called when the DSI link is at LP11 state > before it has switched to the VIDEO mode, and .unprepare is the > reverse when VIDEO mode has been disabled and before the DSI link > is totally disabled. > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L938 > > then > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L855 > > but Doug has started changing this starting with MSM DSI driver, leading to > current panel drivers not working anymore with the current DSI init mode > and requires setting pre_enable_prev_first for only some DSI hosts > who switched out of set_mode(). > Hm, do I understand you correctly that setting bridge->pre_enable_prev_first / panel->prepare_prev_first should work as an alternative to $subject patch, at least for the MSM DSI driver? With it DSI commands should be possible to be sent in .unprepare()? Thanks, Stephan
Hi, On Thu, Jun 15, 2023 at 12:49 AM Neil Armstrong <neil.armstrong@linaro.org> wrote: > > On 14/06/2023 22:58, Linus Walleij wrote: > > On Tue, Jun 13, 2023 at 11:08 PM Stephan Gerhold <stephan@gerhold.net> wrote: > > > >> I'm still quite confused about what exactly is supposed to be in > >> (un)prepare and what in enable/disable. I've seen some related > >> discussion every now and then but it's still quite inconsistent across > >> different panel drivers... Can someone clarify this? > > > > It is somewhat clarified in commit 45527d435c5e39b6eec4aa0065a562e7cf05d503 > > that added the callbacks: > > > > Author: Ajay Kumar <ajaykumar.rs@samsung.com> > > Date: Fri Jul 18 02:13:48 2014 +0530 > > > > drm/panel: add .prepare() and .unprepare() functions > > > > Panels often require an initialization sequence that consists of three > > steps: a) powering up the panel, b) starting transmission of video data > > and c) enabling the panel (e.g. turn on backlight). This is usually > > necessary to avoid visual glitches at the beginning of video data > > transmission. > > > > Similarly, the shutdown sequence is typically done in three steps as > > well: a) disable the panel (e.g. turn off backlight), b) cease video > > data transmission and c) power down the panel. > > > > Currently drivers can only implement .enable() and .disable() functions, > > which is not enough to implement the above sequences. This commit adds a > > second pair of functions, .prepare() and .unprepare() to allow more > > fine-grained control over when the above steps are performed. > > > > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> > > [treding: rewrite changelog, add kerneldoc] > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > > My interpretation is that .enable/.disable is for enabling/disabling > > backlight and well, make things show up on the display, and that > > happens quickly. > > > > prepare/unprepare is for everything else setting up/tearing down > > the data transmission pipeline. > > > > In the clock subsystem the enable/disable could be called in fastpath > > and prepare/unprepare only from slowpath so e.g an IRQ handler > > can gate a simple gated clock. This semantic seems to have nothing > > to do with the display semantic. :/ > > It had to do, .prepare is called when the DSI link is at LP11 state > before it has switched to the VIDEO mode, and .unprepare is the > reverse when VIDEO mode has been disabled and before the DSI link > is totally disabled. > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L938 > > then > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L855 > > but Doug has started changing this starting with MSM DSI driver, leading to > current panel drivers not working anymore with the current DSI init mode > and requires setting pre_enable_prev_first for only some DSI hosts > who switched out of set_mode(). > > The DSI init model doesn't fit at all with the atomic bridge model and > some DSI controllers doesn't support the same features like the allwinner > DSI controller not support sending LP commands when in HS video mode > for example. Summary of the history here as I understand it: 1. Before the switch to DRM_PANEL_BRIDGE, things worked OK. 2. After the switch to DRM_PANEL_BRIDGE, things broke for tc358762. That led to commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset time"), which was a little ugly but sorta OK, except ... 3. Moving the DSI host powerup to modeset time broke ps8640. That led to commit ec7981e6c614 ("drm/msm/dsi: don't powerup at modeset time for parade-ps8640"), which was a hack. 4. We fixed tc358762 using the new "pre_enable_prev_first" in commit 55cac10739d5 ("drm/bridge: tc358762: Set pre_enable_prev_first") and thus were able to undo moving DSI host powerup to modeset time and then undo the ps8640 hack. I talk about this a bit more in the message for commit 9e15123eca79 ("drm/msm/dsi: Stop unconditionally powering up DSI hosts at modeset"). If there are other things that need "pre_enable_prev_first" we could do that. If I understand Dave Stevenson [1], though, this doesn't hurt but technically shouldn't be required. He says that "It is documented that the mipi_dsi_host_ops transfer function should be called with the host in any state [1], so the host driver is failing there." Even if it shouldn't be required, though, "pre_enable_prev_first" can still have a benefit as Dave says [2] because it would mean that the DSI controller doesn't have to power itself up and down for each transfer... If I understand, if the MSM DSI driver did what Dave said (proactive turn on if someone sends commands) then we'd actually be OK even with ps8640, since we don't send any commands in the ps8640 pre_enable() function. I guess one other point of reference is commit a3ee9e0b57f8 ("drm/panel: boe-tv101wum-nl6: Ensure DSI writes succeed during disable"). I think Stephen made that change before "pre_enable_prev_first" was widely available... Hopefully I summarized all that correctly. If I messed up, please yell. I guess the tl;dr (summary of my summary) is: a) Moving panels like this to "pre_enable_prev_first" seems like a reasonable idea anyway and (presumably) works around the issue. b) Moving some commands between disable() / post_diable() or pre_enable() / enable() can also make sense and isn't terrible. As people have pointed out, there is some vagueness on exactly what belongs in each. c) Ideally someone could fix the MSM DSI driver to behave as Dave documented. [1] https://lore.kernel.org/r/CAPY8ntA=Dq6GFQ3gEOm9PzPyOa9bHTr8JrpXLibnai7xKqRbpQ@mail.gmail.com [2] https://lore.kernel.org/r/CAPY8ntAUhVB6UtQTeHAcxNW950Ou+NcEoGwk3JnVWLay89_0Nw@mail.gmail.com
Hi all, On 16.06.23 01:36, Doug Anderson wrote: ... > I guess the tl;dr (summary of my summary) is: > > a) Moving panels like this to "pre_enable_prev_first" seems like a > reasonable idea anyway and (presumably) works around the issue. > > b) Moving some commands between disable() / post_diable() or > pre_enable() / enable() can also make sense and isn't terrible. As > people have pointed out, there is some vagueness on exactly what > belongs in each. > > c) Ideally someone could fix the MSM DSI driver to behave as Dave documented. ... Actually I don't want to disturb the discussion here. Still I would like to point out that option a) "pre_enable_prev_first" doesn't seem to work well yet. On my device samsung-serranove with panel samsung-s6e88a0-ams427ap24 (not in mainline, [1]), when applying "pre_enable_prev_first" I notice two issues. 1) According to commig 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter bridge init order") [2] it's supposed to reverse the order in "post_disable" as well. That doesn't work on my device, the "post_disable" order doesn't get changed by "pre_enable_prev_first". 2) When enabling the panel, for each mipi_dsi_dcs_... command there is an error in dmesg "msm_dsi 1a98000.dsi: [drm:dsi_cmds2buf_tx [msm]] *ERROR* wait for video done timed out". The panel works well nonetheless. However, before commit 007ac0262b0d ("drm/msm/dsi: switch to DRM_PANEL_BRIDGE") these errors didn't show up. I tried to get more insight in the order of issue 1). I added additional debug markers in drivers/gpu/drm/drm_bridge.c (original state linked in [3]) and got the following behavior. To me it's rather hard to understand the decision-making, to be honest. Both in "pre_enable" as well as in "post_disable" first the host and then the panel are processed. At "post_disable" it should be the other way around. I'm currently on kernel 6.4-rc4 with cherry-picked commits from linux-next 9e15123eca79 ("drm/msm/dsi: Stop unconditionally powering up DSI hosts at modeset") and d8dd416cb420 ("drm/msm/dsi: More properly handle errors in regards to dsi_mgr_bridge_power_on()") and added "pre_enable_prev_first" to the panel driver. Device is samsung-serranove in X11 environment. At boot: -------- chain_pre_enable: bridge at beginning: 'host' chain_pre_enable: iter in the list loop at the beginning: 'panel' chain_pre_enable: next if iter prev_first: 'panel' chain_pre_enable: limit if iter prev_first: 'host' chain_pre_enable: next in list loop from_reverse: 'panel' chain_pre_enable: next in list loop from_reverse: 'host' chain_pre_enable: break because 'next == bridge' 'host' chain_pre_enable: marker at end of first list loop block chain_pre_enable: iter at end of first list loop block: 'panel' chain_pre_enable: next at end of first list loop block: 'host' chain_pre_enable: limit at end of first list loop block: 'host' chain_pre_enable: next in 2nd list loop block: 'host' chain_pre_enable: next is not iter, call pre_enable within 2nd list loop block: 'host' call_pre_enable: pre_enable bridge 'host' call_pre_enable: inside 'else if', do pre_enable chain_pre_enable: next in 2nd list loop block: 'panel' chain_pre_enable: break because 'next == iter': 'panel' chain_pre_enable: marker at end of 2nd list loop block chain_pre_enable: iter after 2nd list loop block, call pre_enable: 'panel' call_pre_enable: pre_enable bridge 'panel' call_pre_enable: inside 'if' call_pre_enable: passed second 'if', do atomic_pre_enable msm_dsi 1a98000.dsi: [drm:dsi_cmds2buf_tx [msm]] *ERROR* wait for video done timed out msm_dsi 1a98000.dsi: [drm:dsi_cmds2buf_tx [msm]] *ERROR* wait for video done timed out ... chain_pre_enable: if iter is prev_first...: 'panel' chain_pre_enable: ... change iter to 'limit': 'host' chain_pre_enable: iter at the end of function: 'host' chain_pre_enable: bridge at the end of function: 'host' chain_pre_enable: break if iter is bridge at the end of function: 'host' Then the panel get's turned off: -------------------------------- chain_post_disable: bridge at beginning: 'host' chain_post_disable: bridge of the list loop at the beginning: 'host' chain_post_disable: if entry is not last, set 'next' to next entry: 'panel' chain_post_disable: if 'next' is prev_first, set 'limit' to next: 'panel' chain_post_disable: loop through the list of 'next': 'panel' chain_post_disable: if 'next' is prev_first, change 'next' to: 'host' chain_post_disable: ... and 'limit' to the same: 'host' chain_post_disable: new loop through list of 'next' in reverse order: 'host' chain_post_disable: break because 'next == bridge' 'host' chain_post_disable: after !list_is_last block, call post_disable for bridge: 'host' call_post_disable: post_disable bridge: 'host' call_post_disable: inside 'else if', do post_disable chain_post_disable: if limit available, set 'bridge = limit': 'host' chain_post_disable: bridge of the list loop at the beginning: 'panel' chain_post_disable: after !list_is_last block, call post_disable for bridge: 'panel' call_post_disable: post_disable bridge: 'panel' call_post_disable: inside 'if' call_post_disable: passed second 'if', do atomic_post_disable panel-s6e88a0-ams427ap24 1a98000.dsi.0: Failed to set display off: -22 panel-s6e88a0-ams427ap24 1a98000.dsi.0: Failed to un-initialize panel: -22 It's not my idea to go into detailed debugging. I just wanted to say that option a) "pre_enable_prev_first" currently doesn't work well, at least on my device. I would assume it affects other devices too. Also I didn't want to delay Caleb's patch review. However, it might be related if the "pre_enable_prev_first" approach doesn't work on disable. [1] https://github.com/msm8916-mainline/linux/blob/msm8916/6.4-rc4/drivers/gpu/drm/panel/msm8916-generated/panel-samsung-s6e88a0-ams427ap24.c [2] https://github.com/torvalds/linux/commit/4fb912e5e19075874379cfcf074d90bd51ebf8ea [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_bridge.c?h=v6.4-rc4 Kind regards, Jakob
A typo in my previous e-mail: On 18.06.23 15:47, Jakob Hauser wrote: ... > Then the panel get's turned off: > -------------------------------- ... should be "When..." instead of "Then...".
diff --git a/drivers/gpu/drm/panel/panel-ebbg-ft8719.c b/drivers/gpu/drm/panel/panel-ebbg-ft8719.c index e85d63a176d0..b40a27558e7c 100644 --- a/drivers/gpu/drm/panel/panel-ebbg-ft8719.c +++ b/drivers/gpu/drm/panel/panel-ebbg-ft8719.c @@ -87,22 +87,22 @@ static int ebbg_ft8719_on(struct ebbg_ft8719 *ctx) return 0; } -static int ebbg_ft8719_off(struct ebbg_ft8719 *ctx) +static int ebbg_ft8719_disable(struct drm_panel *panel) { - struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; + struct ebbg_ft8719 *ctx = to_ebbg_ft8719(panel); + struct device *dev = &ctx->dsi->dev; int ret; - dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; + ctx->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; - ret = mipi_dsi_dcs_set_display_off(dsi); + ret = mipi_dsi_dcs_set_display_off(ctx->dsi); if (ret < 0) { dev_err(dev, "Failed to set display off: %d\n", ret); return ret; } usleep_range(10000, 11000); - ret = mipi_dsi_dcs_enter_sleep_mode(dsi); + ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi); if (ret < 0) { dev_err(dev, "Failed to enter sleep mode: %d\n", ret); return ret; @@ -137,13 +137,8 @@ static int ebbg_ft8719_prepare(struct drm_panel *panel) static int ebbg_ft8719_unprepare(struct drm_panel *panel) { struct ebbg_ft8719 *ctx = to_ebbg_ft8719(panel); - struct device *dev = &ctx->dsi->dev; int ret; - ret = ebbg_ft8719_off(ctx); - if (ret < 0) - dev_err(dev, "Failed to un-initialize panel: %d\n", ret); - gpiod_set_value_cansleep(ctx->reset_gpio, 1); ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); @@ -188,6 +183,7 @@ static int ebbg_ft8719_get_modes(struct drm_panel *panel, static const struct drm_panel_funcs ebbg_ft8719_panel_funcs = { .prepare = ebbg_ft8719_prepare, + .disable = ebbg_ft8719_disable, .unprepare = ebbg_ft8719_unprepare, .get_modes = ebbg_ft8719_get_modes, }; diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c index 73bcffa1e0c1..2e1a87001479 100644 --- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c +++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c @@ -115,7 +115,7 @@ static int nt36672a_panel_power_off(struct drm_panel *panel) return ret; } -static int nt36672a_panel_unprepare(struct drm_panel *panel) +static int nt36672a_panel_disable(struct drm_panel *panel) { struct nt36672a_panel *pinfo = to_nt36672a_panel(panel); int ret; @@ -144,6 +144,14 @@ static int nt36672a_panel_unprepare(struct drm_panel *panel) /* 0x3C = 60ms delay */ msleep(60); + return ret; +} + +static int nt36672a_panel_unprepare(struct drm_panel *panel) +{ + struct nt36672a_panel *pinfo = to_nt36672a_panel(panel); + int ret; + ret = nt36672a_panel_power_off(panel); if (ret < 0) dev_err(panel->dev, "power_off failed ret = %d\n", ret); @@ -255,6 +263,7 @@ static int nt36672a_panel_get_modes(struct drm_panel *panel, } static const struct drm_panel_funcs panel_funcs = { + .disable = nt36672a_panel_disable, .unprepare = nt36672a_panel_unprepare, .prepare = nt36672a_panel_prepare, .get_modes = nt36672a_panel_get_modes, diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c b/drivers/gpu/drm/panel/panel-visionox-rm69299.c index ec228c269146..9dc17afbe540 100644 --- a/drivers/gpu/drm/panel/panel-visionox-rm69299.c +++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c @@ -59,7 +59,7 @@ static int visionox_rm69299_power_off(struct visionox_rm69299 *ctx) return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); } -static int visionox_rm69299_unprepare(struct drm_panel *panel) +static int visionox_rm69299_disable(struct drm_panel *panel) { struct visionox_rm69299 *ctx = panel_to_ctx(panel); int ret; @@ -78,6 +78,14 @@ static int visionox_rm69299_unprepare(struct drm_panel *panel) dev_err(ctx->panel.dev, "enter_sleep cmd failed ret = %d\n", ret); } + return ret; +} + +static int visionox_rm69299_unprepare(struct drm_panel *panel) +{ + struct visionox_rm69299 *ctx = panel_to_ctx(panel); + int ret; + ret = visionox_rm69299_power_off(ctx); ctx->prepared = false; @@ -184,6 +192,7 @@ static int visionox_rm69299_get_modes(struct drm_panel *panel, } static const struct drm_panel_funcs visionox_rm69299_drm_funcs = { + .disable = visionox_rm69299_disable, .unprepare = visionox_rm69299_unprepare, .prepare = visionox_rm69299_prepare, .get_modes = visionox_rm69299_get_modes,
The commit 007ac0262b0d ("drm/msm/dsi: switch to DRM_PANEL_BRIDGE") breaks panels which send DSI commands in their .unprepare callbacks. Migrate to using .disable for that for some SDM845 panels. Co-developed-by: Joel Selvaraj <joelselvaraj.oss@gmail.com> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> --- base-commit: 858fd168a95c5b9669aac8db6c14a9aeab446375 // Caleb (they/them) --- drivers/gpu/drm/panel/panel-ebbg-ft8719.c | 18 +++++++----------- drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 11 ++++++++++- drivers/gpu/drm/panel/panel-visionox-rm69299.c | 11 ++++++++++- 3 files changed, 27 insertions(+), 13 deletions(-)