| Message ID | 20211207222901.988484-2-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm/dsi: switch msm/dsi to DRM_PANEL_BRIDGE | expand |
On 12/7/2021 2:29 PM, Dmitry Baryshkov wrote: > The DSI subsystem does not fully fall into the pre-enable/enable system > of callbacks, since typically DSI device bridge drivers expect to be > able to communicate with DSI devices at the pre-enable() callback. The > reason is that for some DSI hosts enabling the video stream would > prevent other drivers from sending DSI commands. For example see the > panel-bridge driver, which does drm_panel_prepare() from the > pre_enable() callback (which would be called before our pre_enable() > callback, resulting in panel preparation failures as the link is not yet > ready). > > Therewere several attempts to solve this issue, but currently the best > approach is to power up the DSI link from the mode_set() callback, > allowing next bridge/panel to use DSI transfers in the pre_enable() > time. Follow this approach. > Change looks okay. As per the programming guideline, we should set the VIDEO_MODE_EN register in the DSI controller followed by enabling the timing engine which will still happen even now because we will do it in modeset instead of the pre_enable(). But, this can potentially increase the delay between VIDEO_MODE_EN and TIMING_ENGINE_EN. I dont see anything in the programming guide against this but since this is a change from the original flow, I would like to do one test before acking this. Can you please try adding a huge delay like 200-300ms between VIDEO_MODE_EN and timing engine enable to make sure there are no issues? You can do that here: int msm_dsi_host_enable(struct mipi_dsi_host *host) { struct msm_dsi_host *msm_host = to_msm_dsi_host(host); dsi_op_mode_config(msm_host, !!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO), true); msleep(300); } > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/dsi/dsi_manager.c | 43 +++++++++++++++++++-------- > 1 file changed, 31 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c > index 681ca74fe410..497719efb9e9 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c > @@ -336,13 +336,12 @@ dsi_mgr_connector_best_encoder(struct drm_connector *connector) > return msm_dsi_get_encoder(msm_dsi); > } > > -static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) > +static void dsi_mgr_bridge_power_on(struct drm_bridge *bridge) > { > int id = dsi_mgr_bridge_get_id(bridge); > struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); > struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); > struct mipi_dsi_host *host = msm_dsi->host; > - struct drm_panel *panel = msm_dsi->panel; > struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX]; > bool is_bonded_dsi = IS_BONDED_DSI(); > int ret; > @@ -383,6 +382,34 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) > if (is_bonded_dsi && msm_dsi1) > msm_dsi_host_enable_irq(msm_dsi1->host); > > + return; > + > +host1_on_fail: > + msm_dsi_host_power_off(host); > +host_on_fail: > + dsi_mgr_phy_disable(id); > +phy_en_fail: > + return; > +} > + > +static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) > +{ > + int id = dsi_mgr_bridge_get_id(bridge); > + struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); > + struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); > + struct mipi_dsi_host *host = msm_dsi->host; > + struct drm_panel *panel = msm_dsi->panel; > + bool is_bonded_dsi = IS_BONDED_DSI(); > + int ret; > + > + DBG("id=%d", id); > + if (!msm_dsi_device_connected(msm_dsi)) > + return; > + > + /* Do nothing with the host if it is slave-DSI in case of bonded DSI */ > + if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id)) > + return; > + > /* Always call panel functions once, because even for dual panels, > * there is only one drm_panel instance. > */ > @@ -417,17 +444,7 @@ 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: > - msm_dsi_host_power_off(host); > -host_on_fail: > - dsi_mgr_phy_disable(id); > -phy_en_fail: > return; > } > > @@ -573,6 +590,8 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge, > msm_dsi_host_set_display_mode(host, adjusted_mode); > if (is_bonded_dsi && other_dsi) > msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode); > + > + dsi_mgr_bridge_power_on(bridge); > } > > static const struct drm_connector_funcs dsi_mgr_connector_funcs = {
On Tue, 18 Jan 2022 at 22:29, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 12/7/2021 2:29 PM, Dmitry Baryshkov wrote: > > The DSI subsystem does not fully fall into the pre-enable/enable system > > of callbacks, since typically DSI device bridge drivers expect to be > > able to communicate with DSI devices at the pre-enable() callback. The > > reason is that for some DSI hosts enabling the video stream would > > prevent other drivers from sending DSI commands. For example see the > > panel-bridge driver, which does drm_panel_prepare() from the > > pre_enable() callback (which would be called before our pre_enable() > > callback, resulting in panel preparation failures as the link is not yet > > ready). > > > > Therewere several attempts to solve this issue, but currently the best > > approach is to power up the DSI link from the mode_set() callback, > > allowing next bridge/panel to use DSI transfers in the pre_enable() > > time. Follow this approach. > > > Change looks okay. As per the programming guideline, we should set the > VIDEO_MODE_EN register in the DSI controller followed by enabling the > timing engine which will still happen even now because we will do it in > modeset instead of the pre_enable(). > But, this can potentially increase the delay between VIDEO_MODE_EN > and TIMING_ENGINE_EN. I dont see anything in the programming guide > against this but since this is a change from the original flow, I would > like to do one test before acking this. Can you please try adding a huge > delay like 200-300ms between VIDEO_MODE_EN and timing engine enable to > make sure there are no issues? You can do that here: Fine, I'll do the test as the time permits. > > int msm_dsi_host_enable(struct mipi_dsi_host *host) > { > struct msm_dsi_host *msm_host = to_msm_dsi_host(host); > > dsi_op_mode_config(msm_host, > !!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO), true); > > msleep(300); > } > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/gpu/drm/msm/dsi/dsi_manager.c | 43 +++++++++++++++++++-------- > > 1 file changed, 31 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c > > index 681ca74fe410..497719efb9e9 100644 > > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c > > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c > > @@ -336,13 +336,12 @@ dsi_mgr_connector_best_encoder(struct drm_connector *connector) > > return msm_dsi_get_encoder(msm_dsi); > > } > > > > -static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) > > +static void dsi_mgr_bridge_power_on(struct drm_bridge *bridge) > > { > > int id = dsi_mgr_bridge_get_id(bridge); > > struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); > > struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); > > struct mipi_dsi_host *host = msm_dsi->host; > > - struct drm_panel *panel = msm_dsi->panel; > > struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX]; > > bool is_bonded_dsi = IS_BONDED_DSI(); > > int ret; > > @@ -383,6 +382,34 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) > > if (is_bonded_dsi && msm_dsi1) > > msm_dsi_host_enable_irq(msm_dsi1->host); > > > > + return; > > + > > +host1_on_fail: > > + msm_dsi_host_power_off(host); > > +host_on_fail: > > + dsi_mgr_phy_disable(id); > > +phy_en_fail: > > + return; > > +} > > + > > +static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) > > +{ > > + int id = dsi_mgr_bridge_get_id(bridge); > > + struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); > > + struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); > > + struct mipi_dsi_host *host = msm_dsi->host; > > + struct drm_panel *panel = msm_dsi->panel; > > + bool is_bonded_dsi = IS_BONDED_DSI(); > > + int ret; > > + > > + DBG("id=%d", id); > > + if (!msm_dsi_device_connected(msm_dsi)) > > + return; > > + > > + /* Do nothing with the host if it is slave-DSI in case of bonded DSI */ > > + if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id)) > > + return; > > + > > /* Always call panel functions once, because even for dual panels, > > * there is only one drm_panel instance. > > */ > > @@ -417,17 +444,7 @@ 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: > > - msm_dsi_host_power_off(host); > > -host_on_fail: > > - dsi_mgr_phy_disable(id); > > -phy_en_fail: > > return; > > } > > > > @@ -573,6 +590,8 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge, > > msm_dsi_host_set_display_mode(host, adjusted_mode); > > if (is_bonded_dsi && other_dsi) > > msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode); > > + > > + dsi_mgr_bridge_power_on(bridge); > > } > > > > static const struct drm_connector_funcs dsi_mgr_connector_funcs = {
On 18/01/2022 23:03, Dmitry Baryshkov wrote: > On Tue, 18 Jan 2022 at 22:29, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 12/7/2021 2:29 PM, Dmitry Baryshkov wrote: >>> The DSI subsystem does not fully fall into the pre-enable/enable system >>> of callbacks, since typically DSI device bridge drivers expect to be >>> able to communicate with DSI devices at the pre-enable() callback. The >>> reason is that for some DSI hosts enabling the video stream would >>> prevent other drivers from sending DSI commands. For example see the >>> panel-bridge driver, which does drm_panel_prepare() from the >>> pre_enable() callback (which would be called before our pre_enable() >>> callback, resulting in panel preparation failures as the link is not yet >>> ready). >>> >>> Therewere several attempts to solve this issue, but currently the best >>> approach is to power up the DSI link from the mode_set() callback, >>> allowing next bridge/panel to use DSI transfers in the pre_enable() >>> time. Follow this approach. >>> >> Change looks okay. As per the programming guideline, we should set the >> VIDEO_MODE_EN register in the DSI controller followed by enabling the >> timing engine which will still happen even now because we will do it in >> modeset instead of the pre_enable(). >> But, this can potentially increase the delay between VIDEO_MODE_EN >> and TIMING_ENGINE_EN. I dont see anything in the programming guide >> against this but since this is a change from the original flow, I would >> like to do one test before acking this. Can you please try adding a huge >> delay like 200-300ms between VIDEO_MODE_EN and timing engine enable to >> make sure there are no issues? You can do that here: > > > Fine, I'll do the test as the time permits. I did the tests, the display pipeline works as expected. Let's get this in, it allows using other DSI-controlled bridges. > >> >> int msm_dsi_host_enable(struct mipi_dsi_host *host) >> { >> struct msm_dsi_host *msm_host = to_msm_dsi_host(host); >> >> dsi_op_mode_config(msm_host, >> !!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO), true); >> >> msleep(300); >> } >> >> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> drivers/gpu/drm/msm/dsi/dsi_manager.c | 43 +++++++++++++++++++-------- >>> 1 file changed, 31 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c >>> index 681ca74fe410..497719efb9e9 100644 >>> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c >>> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c >>> @@ -336,13 +336,12 @@ dsi_mgr_connector_best_encoder(struct drm_connector *connector) >>> return msm_dsi_get_encoder(msm_dsi); >>> } >>> >>> -static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) >>> +static void dsi_mgr_bridge_power_on(struct drm_bridge *bridge) >>> { >>> int id = dsi_mgr_bridge_get_id(bridge); >>> struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); >>> struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); >>> struct mipi_dsi_host *host = msm_dsi->host; >>> - struct drm_panel *panel = msm_dsi->panel; >>> struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX]; >>> bool is_bonded_dsi = IS_BONDED_DSI(); >>> int ret; >>> @@ -383,6 +382,34 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) >>> if (is_bonded_dsi && msm_dsi1) >>> msm_dsi_host_enable_irq(msm_dsi1->host); >>> >>> + return; >>> + >>> +host1_on_fail: >>> + msm_dsi_host_power_off(host); >>> +host_on_fail: >>> + dsi_mgr_phy_disable(id); >>> +phy_en_fail: >>> + return; >>> +} >>> + >>> +static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) >>> +{ >>> + int id = dsi_mgr_bridge_get_id(bridge); >>> + struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); >>> + struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); >>> + struct mipi_dsi_host *host = msm_dsi->host; >>> + struct drm_panel *panel = msm_dsi->panel; >>> + bool is_bonded_dsi = IS_BONDED_DSI(); >>> + int ret; >>> + >>> + DBG("id=%d", id); >>> + if (!msm_dsi_device_connected(msm_dsi)) >>> + return; >>> + >>> + /* Do nothing with the host if it is slave-DSI in case of bonded DSI */ >>> + if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id)) >>> + return; >>> + >>> /* Always call panel functions once, because even for dual panels, >>> * there is only one drm_panel instance. >>> */ >>> @@ -417,17 +444,7 @@ 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: >>> - msm_dsi_host_power_off(host); >>> -host_on_fail: >>> - dsi_mgr_phy_disable(id); >>> -phy_en_fail: >>> return; >>> } >>> >>> @@ -573,6 +590,8 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge, >>> msm_dsi_host_set_display_mode(host, adjusted_mode); >>> if (is_bonded_dsi && other_dsi) >>> msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode); >>> + >>> + dsi_mgr_bridge_power_on(bridge); >>> } >>> >>> static const struct drm_connector_funcs dsi_mgr_connector_funcs = { > > >
On 2/11/2022 11:37 AM, Dmitry Baryshkov wrote: > On 18/01/2022 23:03, Dmitry Baryshkov wrote: >> On Tue, 18 Jan 2022 at 22:29, Abhinav Kumar >> <quic_abhinavk@quicinc.com> wrote: >>> >>> >>> >>> On 12/7/2021 2:29 PM, Dmitry Baryshkov wrote: >>>> The DSI subsystem does not fully fall into the pre-enable/enable system >>>> of callbacks, since typically DSI device bridge drivers expect to be >>>> able to communicate with DSI devices at the pre-enable() callback. The >>>> reason is that for some DSI hosts enabling the video stream would >>>> prevent other drivers from sending DSI commands. For example see the >>>> panel-bridge driver, which does drm_panel_prepare() from the >>>> pre_enable() callback (which would be called before our pre_enable() >>>> callback, resulting in panel preparation failures as the link is not >>>> yet >>>> ready). >>>> >>>> Therewere several attempts to solve this issue, but currently the best >>>> approach is to power up the DSI link from the mode_set() callback, >>>> allowing next bridge/panel to use DSI transfers in the pre_enable() >>>> time. Follow this approach. >>>> >>> Change looks okay. As per the programming guideline, we should set the >>> VIDEO_MODE_EN register in the DSI controller followed by enabling the >>> timing engine which will still happen even now because we will do it in >>> modeset instead of the pre_enable(). >>> But, this can potentially increase the delay between VIDEO_MODE_EN >>> and TIMING_ENGINE_EN. I dont see anything in the programming guide >>> against this but since this is a change from the original flow, I would >>> like to do one test before acking this. Can you please try adding a huge >>> delay like 200-300ms between VIDEO_MODE_EN and timing engine enable to >>> make sure there are no issues? You can do that here: >> >> >> Fine, I'll do the test as the time permits. > > I did the tests, the display pipeline works as expected. > > Let's get this in, it allows using other DSI-controlled bridges. Alright, sounds good, Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >> >>> >>> int msm_dsi_host_enable(struct mipi_dsi_host *host) >>> { >>> struct msm_dsi_host *msm_host = to_msm_dsi_host(host); >>> >>> dsi_op_mode_config(msm_host, >>> !!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO), true); >>> >>> msleep(300); >>> } >>> >>> >>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>> --- >>>> drivers/gpu/drm/msm/dsi/dsi_manager.c | 43 >>>> +++++++++++++++++++-------- >>>> 1 file changed, 31 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c >>>> b/drivers/gpu/drm/msm/dsi/dsi_manager.c >>>> index 681ca74fe410..497719efb9e9 100644 >>>> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c >>>> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c >>>> @@ -336,13 +336,12 @@ dsi_mgr_connector_best_encoder(struct >>>> drm_connector *connector) >>>> return msm_dsi_get_encoder(msm_dsi); >>>> } >>>> >>>> -static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) >>>> +static void dsi_mgr_bridge_power_on(struct drm_bridge *bridge) >>>> { >>>> int id = dsi_mgr_bridge_get_id(bridge); >>>> struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); >>>> struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); >>>> struct mipi_dsi_host *host = msm_dsi->host; >>>> - struct drm_panel *panel = msm_dsi->panel; >>>> struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX]; >>>> bool is_bonded_dsi = IS_BONDED_DSI(); >>>> int ret; >>>> @@ -383,6 +382,34 @@ static void dsi_mgr_bridge_pre_enable(struct >>>> drm_bridge *bridge) >>>> if (is_bonded_dsi && msm_dsi1) >>>> msm_dsi_host_enable_irq(msm_dsi1->host); >>>> >>>> + return; >>>> + >>>> +host1_on_fail: >>>> + msm_dsi_host_power_off(host); >>>> +host_on_fail: >>>> + dsi_mgr_phy_disable(id); >>>> +phy_en_fail: >>>> + return; >>>> +} >>>> + >>>> +static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) >>>> +{ >>>> + int id = dsi_mgr_bridge_get_id(bridge); >>>> + struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); >>>> + struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); >>>> + struct mipi_dsi_host *host = msm_dsi->host; >>>> + struct drm_panel *panel = msm_dsi->panel; >>>> + bool is_bonded_dsi = IS_BONDED_DSI(); >>>> + int ret; >>>> + >>>> + DBG("id=%d", id); >>>> + if (!msm_dsi_device_connected(msm_dsi)) >>>> + return; >>>> + >>>> + /* Do nothing with the host if it is slave-DSI in case of >>>> bonded DSI */ >>>> + if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id)) >>>> + return; >>>> + >>>> /* Always call panel functions once, because even for dual >>>> panels, >>>> * there is only one drm_panel instance. >>>> */ >>>> @@ -417,17 +444,7 @@ 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: >>>> - msm_dsi_host_power_off(host); >>>> -host_on_fail: >>>> - dsi_mgr_phy_disable(id); >>>> -phy_en_fail: >>>> return; >>>> } >>>> >>>> @@ -573,6 +590,8 @@ static void dsi_mgr_bridge_mode_set(struct >>>> drm_bridge *bridge, >>>> msm_dsi_host_set_display_mode(host, adjusted_mode); >>>> if (is_bonded_dsi && other_dsi) >>>> msm_dsi_host_set_display_mode(other_dsi->host, >>>> adjusted_mode); >>>> + >>>> + dsi_mgr_bridge_power_on(bridge); >>>> } >>>> >>>> static const struct drm_connector_funcs dsi_mgr_connector_funcs = { >> >> >> > >
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index 681ca74fe410..497719efb9e9 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -336,13 +336,12 @@ dsi_mgr_connector_best_encoder(struct drm_connector *connector) return msm_dsi_get_encoder(msm_dsi); } -static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) +static void dsi_mgr_bridge_power_on(struct drm_bridge *bridge) { int id = dsi_mgr_bridge_get_id(bridge); struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); struct mipi_dsi_host *host = msm_dsi->host; - struct drm_panel *panel = msm_dsi->panel; struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX]; bool is_bonded_dsi = IS_BONDED_DSI(); int ret; @@ -383,6 +382,34 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) if (is_bonded_dsi && msm_dsi1) msm_dsi_host_enable_irq(msm_dsi1->host); + return; + +host1_on_fail: + msm_dsi_host_power_off(host); +host_on_fail: + dsi_mgr_phy_disable(id); +phy_en_fail: + return; +} + +static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) +{ + int id = dsi_mgr_bridge_get_id(bridge); + struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); + struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1); + struct mipi_dsi_host *host = msm_dsi->host; + struct drm_panel *panel = msm_dsi->panel; + bool is_bonded_dsi = IS_BONDED_DSI(); + int ret; + + DBG("id=%d", id); + if (!msm_dsi_device_connected(msm_dsi)) + return; + + /* Do nothing with the host if it is slave-DSI in case of bonded DSI */ + if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id)) + return; + /* Always call panel functions once, because even for dual panels, * there is only one drm_panel instance. */ @@ -417,17 +444,7 @@ 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: - msm_dsi_host_power_off(host); -host_on_fail: - dsi_mgr_phy_disable(id); -phy_en_fail: return; } @@ -573,6 +590,8 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge *bridge, msm_dsi_host_set_display_mode(host, adjusted_mode); if (is_bonded_dsi && other_dsi) msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode); + + dsi_mgr_bridge_power_on(bridge); } static const struct drm_connector_funcs dsi_mgr_connector_funcs = {
The DSI subsystem does not fully fall into the pre-enable/enable system of callbacks, since typically DSI device bridge drivers expect to be able to communicate with DSI devices at the pre-enable() callback. The reason is that for some DSI hosts enabling the video stream would prevent other drivers from sending DSI commands. For example see the panel-bridge driver, which does drm_panel_prepare() from the pre_enable() callback (which would be called before our pre_enable() callback, resulting in panel preparation failures as the link is not yet ready). Therewere several attempts to solve this issue, but currently the best approach is to power up the DSI link from the mode_set() callback, allowing next bridge/panel to use DSI transfers in the pre_enable() time. Follow this approach. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/dsi/dsi_manager.c | 43 +++++++++++++++++++-------- 1 file changed, 31 insertions(+), 12 deletions(-)