| Message ID | 20211002010830.647416-1-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v3] drm/msm/dsi: do not enable irq handler before powering up the host | expand |
On 2021-10-01 18:08, Dmitry Baryshkov wrote: > The DSI host might be left in some state by the bootloader. If this > state generates an IRQ, it might hang the system by holding the > interrupt line before the driver sets up the DSI host to the known > state. > > Move the request_irq into msm_dsi_host_init and pass IRQF_NO_AUTOEN to > it. Call enable/disable_irq after msm_dsi_host_power_on/_off() > functions, so that we can be sure that the interrupt is delivered when > the host is in the known state. > > It is not possible to defer the interrupt enablement to a later point, > because drm_panel_prepare might need to communicate with the panel over > the DSI link and that requires working interrupt. > > Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support") > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org> > --- > drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ > drivers/gpu/drm/msm/dsi/dsi_host.c | 48 +++++++++++++++++---------- > drivers/gpu/drm/msm/dsi/dsi_manager.c | 16 +++++++++ > 3 files changed, 49 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi.h > b/drivers/gpu/drm/msm/dsi/dsi.h > index b50db91cb8a7..569c8ff062ba 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi.h > +++ b/drivers/gpu/drm/msm/dsi/dsi.h > @@ -107,6 +107,8 @@ void msm_dsi_host_cmd_xfer_commit(struct > mipi_dsi_host *host, > u32 dma_base, u32 len); > int msm_dsi_host_enable(struct mipi_dsi_host *host); > int msm_dsi_host_disable(struct mipi_dsi_host *host); > +void msm_dsi_host_enable_irq(struct mipi_dsi_host *host); > +void msm_dsi_host_disable_irq(struct mipi_dsi_host *host); > int msm_dsi_host_power_on(struct mipi_dsi_host *host, > struct msm_dsi_phy_shared_timings *phy_shared_timings, > bool is_bonded_dsi, struct msm_dsi_phy *phy); > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c > b/drivers/gpu/drm/msm/dsi/dsi_host.c > index e269df285136..ce26eb78cb6c 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -1898,6 +1898,23 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) > return ret; > } > > + msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0); > + if (msm_host->irq < 0) { > + ret = msm_host->irq; > + dev_err(&pdev->dev, "failed to get irq: %d\n", ret); > + return ret; > + } > + > + /* do not autoenable, will be enabled later */ > + ret = devm_request_irq(&pdev->dev, msm_host->irq, dsi_host_irq, > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT | IRQF_NO_AUTOEN, > + "dsi_isr", msm_host); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to request IRQ%u: %d\n", > + msm_host->irq, ret); > + return ret; > + } > + > init_completion(&msm_host->dma_comp); > init_completion(&msm_host->video_comp); > mutex_init(&msm_host->dev_mutex); > @@ -1941,25 +1958,8 @@ int msm_dsi_host_modeset_init(struct > mipi_dsi_host *host, > { > struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd; > - struct platform_device *pdev = msm_host->pdev; > int ret; > > - msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0); > - if (msm_host->irq < 0) { > - ret = msm_host->irq; > - DRM_DEV_ERROR(dev->dev, "failed to get irq: %d\n", ret); > - return ret; > - } > - > - ret = devm_request_irq(&pdev->dev, msm_host->irq, > - dsi_host_irq, IRQF_TRIGGER_HIGH | IRQF_ONESHOT, > - "dsi_isr", msm_host); > - if (ret < 0) { > - DRM_DEV_ERROR(&pdev->dev, "failed to request IRQ%u: %d\n", > - msm_host->irq, ret); > - return ret; > - } > - > msm_host->dev = dev; > ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K); > if (ret) { > @@ -2315,6 +2315,20 @@ void msm_dsi_host_get_phy_clk_req(struct > mipi_dsi_host *host, > clk_req->escclk_rate = msm_host->esc_clk_rate; > } > > +void msm_dsi_host_enable_irq(struct mipi_dsi_host *host) > +{ > + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > + > + enable_irq(msm_host->irq); > +} > + > +void msm_dsi_host_disable_irq(struct mipi_dsi_host *host) > +{ > + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > + > + disable_irq(msm_host->irq); > +} > + > int msm_dsi_host_enable(struct mipi_dsi_host *host) > { > struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c > b/drivers/gpu/drm/msm/dsi/dsi_manager.c > index c41d39f5b7cf..fb4ccffdcfe1 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c > @@ -377,6 +377,14 @@ static void dsi_mgr_bridge_pre_enable(struct > drm_bridge *bridge) > } > } > > + /* > + * Enable before preparing the panel, disable after unpreparing, so > + * that the panel can communicate over the DSI link. > + */ > + msm_dsi_host_enable_irq(host); > + if (is_bonded_dsi && msm_dsi1) > + msm_dsi_host_enable_irq(msm_dsi1->host); > + > /* Always call panel functions once, because even for dual panels, > * there is only one drm_panel instance. > */ > @@ -411,6 +419,10 @@ static void dsi_mgr_bridge_pre_enable(struct > drm_bridge *bridge) > if (panel) > drm_panel_unprepare(panel); > panel_prep_fail: > + msm_dsi_host_disable_irq(host); > + if (is_bonded_dsi && msm_dsi1) > + msm_dsi_host_disable_irq(msm_dsi1->host); > + > if (is_bonded_dsi && msm_dsi1) > msm_dsi_host_power_off(msm_dsi1->host); > host1_on_fail: > @@ -523,6 +535,10 @@ static void dsi_mgr_bridge_post_disable(struct > drm_bridge *bridge) > id, ret); > } > > + msm_dsi_host_disable_irq(host); > + if (is_bonded_dsi && msm_dsi1) > + msm_dsi_host_disable_irq(msm_dsi1->host); > + > /* Save PHY status if it is a clock source */ > msm_dsi_phy_pll_save_state(msm_dsi->phy);
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index b50db91cb8a7..569c8ff062ba 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -107,6 +107,8 @@ void msm_dsi_host_cmd_xfer_commit(struct mipi_dsi_host *host, u32 dma_base, u32 len); int msm_dsi_host_enable(struct mipi_dsi_host *host); int msm_dsi_host_disable(struct mipi_dsi_host *host); +void msm_dsi_host_enable_irq(struct mipi_dsi_host *host); +void msm_dsi_host_disable_irq(struct mipi_dsi_host *host); int msm_dsi_host_power_on(struct mipi_dsi_host *host, struct msm_dsi_phy_shared_timings *phy_shared_timings, bool is_bonded_dsi, struct msm_dsi_phy *phy); diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index e269df285136..ce26eb78cb6c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1898,6 +1898,23 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) return ret; } + msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0); + if (msm_host->irq < 0) { + ret = msm_host->irq; + dev_err(&pdev->dev, "failed to get irq: %d\n", ret); + return ret; + } + + /* do not autoenable, will be enabled later */ + ret = devm_request_irq(&pdev->dev, msm_host->irq, dsi_host_irq, + IRQF_TRIGGER_HIGH | IRQF_ONESHOT | IRQF_NO_AUTOEN, + "dsi_isr", msm_host); + if (ret < 0) { + dev_err(&pdev->dev, "failed to request IRQ%u: %d\n", + msm_host->irq, ret); + return ret; + } + init_completion(&msm_host->dma_comp); init_completion(&msm_host->video_comp); mutex_init(&msm_host->dev_mutex); @@ -1941,25 +1958,8 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host, { struct msm_dsi_host *msm_host = to_msm_dsi_host(host); const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd; - struct platform_device *pdev = msm_host->pdev; int ret; - msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0); - if (msm_host->irq < 0) { - ret = msm_host->irq; - DRM_DEV_ERROR(dev->dev, "failed to get irq: %d\n", ret); - return ret; - } - - ret = devm_request_irq(&pdev->dev, msm_host->irq, - dsi_host_irq, IRQF_TRIGGER_HIGH | IRQF_ONESHOT, - "dsi_isr", msm_host); - if (ret < 0) { - DRM_DEV_ERROR(&pdev->dev, "failed to request IRQ%u: %d\n", - msm_host->irq, ret); - return ret; - } - msm_host->dev = dev; ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K); if (ret) { @@ -2315,6 +2315,20 @@ void msm_dsi_host_get_phy_clk_req(struct mipi_dsi_host *host, clk_req->escclk_rate = msm_host->esc_clk_rate; } +void msm_dsi_host_enable_irq(struct mipi_dsi_host *host) +{ + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); + + enable_irq(msm_host->irq); +} + +void msm_dsi_host_disable_irq(struct mipi_dsi_host *host) +{ + struct msm_dsi_host *msm_host = to_msm_dsi_host(host); + + disable_irq(msm_host->irq); +} + int msm_dsi_host_enable(struct mipi_dsi_host *host) { struct msm_dsi_host *msm_host = to_msm_dsi_host(host); diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index c41d39f5b7cf..fb4ccffdcfe1 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -377,6 +377,14 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) } } + /* + * Enable before preparing the panel, disable after unpreparing, so + * that the panel can communicate over the DSI link. + */ + msm_dsi_host_enable_irq(host); + if (is_bonded_dsi && msm_dsi1) + msm_dsi_host_enable_irq(msm_dsi1->host); + /* Always call panel functions once, because even for dual panels, * there is only one drm_panel instance. */ @@ -411,6 +419,10 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) if (panel) drm_panel_unprepare(panel); panel_prep_fail: + msm_dsi_host_disable_irq(host); + if (is_bonded_dsi && msm_dsi1) + msm_dsi_host_disable_irq(msm_dsi1->host); + if (is_bonded_dsi && msm_dsi1) msm_dsi_host_power_off(msm_dsi1->host); host1_on_fail: @@ -523,6 +535,10 @@ static void dsi_mgr_bridge_post_disable(struct drm_bridge *bridge) id, ret); } + msm_dsi_host_disable_irq(host); + if (is_bonded_dsi && msm_dsi1) + msm_dsi_host_disable_irq(msm_dsi1->host); + /* Save PHY status if it is a clock source */ msm_dsi_phy_pll_save_state(msm_dsi->phy);
The DSI host might be left in some state by the bootloader. If this state generates an IRQ, it might hang the system by holding the interrupt line before the driver sets up the DSI host to the known state. Move the request_irq into msm_dsi_host_init and pass IRQF_NO_AUTOEN to it. Call enable/disable_irq after msm_dsi_host_power_on/_off() functions, so that we can be sure that the interrupt is delivered when the host is in the known state. It is not possible to defer the interrupt enablement to a later point, because drm_panel_prepare might need to communicate with the panel over the DSI link and that requires working interrupt. Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support") Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/dsi/dsi.h | 2 ++ drivers/gpu/drm/msm/dsi/dsi_host.c | 48 +++++++++++++++++---------- drivers/gpu/drm/msm/dsi/dsi_manager.c | 16 +++++++++ 3 files changed, 49 insertions(+), 17 deletions(-)