Messages in this thread | | | Date | Thu, 12 May 2022 23:58:59 +0300 | Subject | Re: [PATCH v3] drm/msm/dsi: only powerup at modeset time if "early_poweron" modparam | From | Dmitry Baryshkov <> |
| |
On 12/05/2022 23:52, Douglas Anderson wrote: > Commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset > time") caused sc7180 Chromebooks that use the parade-ps8640 bridge > chip to fail to turn the display back on after it turns off. > > Unfortunately, it doesn't look easy to fix the parade-ps8640 driver to > handle the new power sequence. The Linux driver has almost nothing in > it and most of the logic for this bridge chip is in black-box firmware > that the bridge chip uses. > > Also unfortunately, reverting the patch will break "tc358762". > > The long term solution here is probably Dave Stevenson's series [1] > that would give more flexibility. However, that is likely not a quick > fix. > > For the short term, let's introduce a module parameter that selects > between the two behaviors. This is a short term hack but at least can > keep both users working. We'll default the value of the module > parameter to the old behavior. Given that the old behavior has existed > for longer it's probably a safer default. > > [1] https://lore.kernel.org/r/cover.1646406653.git.dave.stevenson@raspberrypi.com > > Fixes: 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset time") > Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Two minor issues below.
> --- > > Changes in v3: > - No longer a revert; now a module parameter. > > Changes in v2: > - Remove the mud from my face. > > drivers/gpu/drm/msm/dsi/dsi_manager.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c > index 50b987658b1f..2bf4123ef5df 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c > @@ -34,6 +34,10 @@ static struct msm_dsi_manager msm_dsim_glb; > #define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed) > #define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id) > > +bool early_poweron; > +MODULE_PARM_DESC(early_poweron, "Power DSI controller early"); > +module_param(early_poweron, bool, 0600);
Nit: dsi_early_poweron (to be clear that it related to DSI only).
I thought about suggesting 'dsi_no_early_poweron' instead to catch possible issues with other bridges. But... I think with Dave's series will have to enable bridges one by one, so it doesn't make real sense.
> + > static inline struct msm_dsi *dsi_mgr_get_dsi(int id) > { > return msm_dsim_glb.dsi[id]; > @@ -389,6 +393,9 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) > if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id)) > return; > > + if (!early_poweron) > + dsi_mgr_bridge_power_on(bridge); > + > /* Always call panel functions once, because even for dual panels, > * there is only one drm_panel instance. > */ > @@ -570,7 +577,8 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge, > if (is_bonded_dsi && other_dsi) > msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode); > > - dsi_mgr_bridge_power_on(bridge); > + if (early_poweron) > + dsi_mgr_bridge_power_on(bridge); > } > > static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
-- With best wishes Dmitry
| |