| Message ID | 20241205-dp_mst-v1-11-f8618d42a99a@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 2024/12/6 17:14, Dmitry Baryshkov wrote: > On Thu, Dec 05, 2024 at 08:31:42PM -0800, Abhinav Kumar wrote: >> Split dp_ctrl_off() into stream and link parts so that for MST >> cases we can control the link and pixel parts separately. > > Please start by describing the problem. Got it. > >> >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >> --- >> drivers/gpu/drm/msm/dp/dp_ctrl.c | 29 +++-------------------------- >> drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 +- >> drivers/gpu/drm/msm/dp/dp_display.c | 4 +++- >> 3 files changed, 7 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c >> index 118f5ed83e464f9f27f813eb39624f9c3189f5ac..485339eb998cc6c8c1e8ab0a88b5c5d6ef300a1f 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c >> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c >> @@ -1739,7 +1739,8 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl >> * running. Add the global reset just before disabling the >> * link clocks and core clocks. >> */ >> - msm_dp_ctrl_off(&ctrl->msm_dp_ctrl); >> + msm_dp_ctrl_stream_clk_off(&ctrl->msm_dp_ctrl); >> + msm_dp_ctrl_off_link(&ctrl->msm_dp_ctrl); > > Huh? What happened with the rest of the msm_dp_ctrl_off() code sequence? > It got dropped, but the commit message tells nothing about it. > The function msm_dp_ctrl_off has been split into two parts, stream_clk_off and off_link, so it got dropped. This part is a bit confusing, will make it clearer. >> >> ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl); >> if (ret) { >> @@ -2042,7 +2043,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel * >> return ret; >> } >> >> -static void msm_dp_ctrl_stream_clk_off(struct msm_dp_ctrl *msm_dp_ctrl) >> +void msm_dp_ctrl_stream_clk_off(struct msm_dp_ctrl *msm_dp_ctrl) >> { >> struct msm_dp_ctrl_private *ctrl; >> >> @@ -2110,30 +2111,6 @@ void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl) >> phy, phy->init_count, phy->power_count); >> } >> >> -void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl) >> -{ >> - struct msm_dp_ctrl_private *ctrl; >> - struct phy *phy; >> - >> - ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl); >> - phy = ctrl->phy; >> - >> - msm_dp_catalog_panel_disable_vsc_sdp(ctrl->catalog); >> - >> - msm_dp_catalog_ctrl_mainlink_ctrl(ctrl->catalog, false); >> - >> - msm_dp_catalog_ctrl_reset(ctrl->catalog); >> - >> - msm_dp_ctrl_stream_clk_off(msm_dp_ctrl); >> - >> - dev_pm_opp_set_rate(ctrl->dev, 0); >> - msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl); >> - >> - phy_power_off(phy); >> - drm_dbg_dp(ctrl->drm_dev, "phy=%p init=%d power_on=%d\n", >> - phy, phy->init_count, phy->power_count); >> -} >> - >> irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl) >> { >> struct msm_dp_ctrl_private *ctrl; >> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h >> index 547155ffa50fbe2f3a1f2c2e1ee17420daf0f3da..887cf5a866f07cb9038887a0634d3e1a0375879c 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h >> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h >> @@ -22,7 +22,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel * >> int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *dp_ctrl, bool force_link_train); >> void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl); >> void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl); >> -void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl); >> +void msm_dp_ctrl_stream_clk_off(struct msm_dp_ctrl *msm_dp_ctrl); >> void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl); >> irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl); >> void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl); >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >> index c059f749c1f204deac9dfb0c56f537f5545d9acb..b0458bbc89e934ca33ed5af3f2a8ebca30b50824 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_display.c >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c >> @@ -911,7 +911,9 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp) >> if (dp->link->sink_count == 0) >> msm_dp_ctrl_psm_config(dp->ctrl); >> >> - msm_dp_ctrl_off(dp->ctrl); >> + msm_dp_ctrl_stream_clk_off(dp->ctrl); >> + >> + msm_dp_ctrl_off_link(dp->ctrl); >> >> /* re-init the PHY so that we can listen to Dongle disconnect */ >> if (dp->link->sink_count == 0) >> >> -- >> 2.34.1 >> >
On Mon, May 26, 2025 at 08:47:22PM +0800, Yongxing Mou wrote: > > > On 2024/12/6 17:14, Dmitry Baryshkov wrote: > > On Thu, Dec 05, 2024 at 08:31:42PM -0800, Abhinav Kumar wrote: > > > Split dp_ctrl_off() into stream and link parts so that for MST > > > cases we can control the link and pixel parts separately. > > > > Please start by describing the problem. > Got it. > > > > > > > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > > > --- > > > drivers/gpu/drm/msm/dp/dp_ctrl.c | 29 +++-------------------------- > > > drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 +- > > > drivers/gpu/drm/msm/dp/dp_display.c | 4 +++- > > > 3 files changed, 7 insertions(+), 28 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c > > > index 118f5ed83e464f9f27f813eb39624f9c3189f5ac..485339eb998cc6c8c1e8ab0a88b5c5d6ef300a1f 100644 > > > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > > > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c > > > @@ -1739,7 +1739,8 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl > > > * running. Add the global reset just before disabling the > > > * link clocks and core clocks. > > > */ > > > - msm_dp_ctrl_off(&ctrl->msm_dp_ctrl); > > > + msm_dp_ctrl_stream_clk_off(&ctrl->msm_dp_ctrl); > > > + msm_dp_ctrl_off_link(&ctrl->msm_dp_ctrl); > > > > Huh? What happened with the rest of the msm_dp_ctrl_off() code sequence? > > It got dropped, but the commit message tells nothing about it. > > > The function msm_dp_ctrl_off has been split into two parts, stream_clk_off > and off_link, so it got dropped. This part is a bit confusing, will make it > clearer. Then msm_dp_ctrl_off_link() should be a part of this patch. > > > ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl); > > > if (ret) { > > > @@ -2042,7 +2043,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel * > > > return ret; > > > } > > > -static void msm_dp_ctrl_stream_clk_off(struct msm_dp_ctrl *msm_dp_ctrl) > > > +void msm_dp_ctrl_stream_clk_off(struct msm_dp_ctrl *msm_dp_ctrl) > > > { > > > struct msm_dp_ctrl_private *ctrl; > > > @@ -2110,30 +2111,6 @@ void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl) > > > phy, phy->init_count, phy->power_count); > > > } > > > -void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl) > > > -{ > > > - struct msm_dp_ctrl_private *ctrl; > > > - struct phy *phy; > > > - > > > - ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl); > > > - phy = ctrl->phy; > > > - > > > - msm_dp_catalog_panel_disable_vsc_sdp(ctrl->catalog); > > > - > > > - msm_dp_catalog_ctrl_mainlink_ctrl(ctrl->catalog, false); > > > - > > > - msm_dp_catalog_ctrl_reset(ctrl->catalog); > > > - > > > - msm_dp_ctrl_stream_clk_off(msm_dp_ctrl); > > > - > > > - dev_pm_opp_set_rate(ctrl->dev, 0); > > > - msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl); > > > - > > > - phy_power_off(phy); > > > - drm_dbg_dp(ctrl->drm_dev, "phy=%p init=%d power_on=%d\n", > > > - phy, phy->init_count, phy->power_count); > > > -} > > > - > > > irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl) > > > { > > > struct msm_dp_ctrl_private *ctrl; > > > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h > > > index 547155ffa50fbe2f3a1f2c2e1ee17420daf0f3da..887cf5a866f07cb9038887a0634d3e1a0375879c 100644 > > > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h > > > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h > > > @@ -22,7 +22,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel * > > > int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *dp_ctrl, bool force_link_train); > > > void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl); > > > void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl); > > > -void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl); > > > +void msm_dp_ctrl_stream_clk_off(struct msm_dp_ctrl *msm_dp_ctrl); > > > void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl); > > > irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl); > > > void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl); > > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > > > index c059f749c1f204deac9dfb0c56f537f5545d9acb..b0458bbc89e934ca33ed5af3f2a8ebca30b50824 100644 > > > --- a/drivers/gpu/drm/msm/dp/dp_display.c > > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > > > @@ -911,7 +911,9 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp) > > > if (dp->link->sink_count == 0) > > > msm_dp_ctrl_psm_config(dp->ctrl); > > > - msm_dp_ctrl_off(dp->ctrl); > > > + msm_dp_ctrl_stream_clk_off(dp->ctrl); > > > + > > > + msm_dp_ctrl_off_link(dp->ctrl); > > > /* re-init the PHY so that we can listen to Dongle disconnect */ > > > if (dp->link->sink_count == 0) > > > > > > -- > > > 2.34.1 > > > > > >
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 118f5ed83e464f9f27f813eb39624f9c3189f5ac..485339eb998cc6c8c1e8ab0a88b5c5d6ef300a1f 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1739,7 +1739,8 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl * running. Add the global reset just before disabling the * link clocks and core clocks. */ - msm_dp_ctrl_off(&ctrl->msm_dp_ctrl); + msm_dp_ctrl_stream_clk_off(&ctrl->msm_dp_ctrl); + msm_dp_ctrl_off_link(&ctrl->msm_dp_ctrl); ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl); if (ret) { @@ -2042,7 +2043,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel * return ret; } -static void msm_dp_ctrl_stream_clk_off(struct msm_dp_ctrl *msm_dp_ctrl) +void msm_dp_ctrl_stream_clk_off(struct msm_dp_ctrl *msm_dp_ctrl) { struct msm_dp_ctrl_private *ctrl; @@ -2110,30 +2111,6 @@ void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl) phy, phy->init_count, phy->power_count); } -void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl) -{ - struct msm_dp_ctrl_private *ctrl; - struct phy *phy; - - ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl); - phy = ctrl->phy; - - msm_dp_catalog_panel_disable_vsc_sdp(ctrl->catalog); - - msm_dp_catalog_ctrl_mainlink_ctrl(ctrl->catalog, false); - - msm_dp_catalog_ctrl_reset(ctrl->catalog); - - msm_dp_ctrl_stream_clk_off(msm_dp_ctrl); - - dev_pm_opp_set_rate(ctrl->dev, 0); - msm_dp_ctrl_link_clk_disable(&ctrl->msm_dp_ctrl); - - phy_power_off(phy); - drm_dbg_dp(ctrl->drm_dev, "phy=%p init=%d power_on=%d\n", - phy, phy->init_count, phy->power_count); -} - irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl) { struct msm_dp_ctrl_private *ctrl; diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h index 547155ffa50fbe2f3a1f2c2e1ee17420daf0f3da..887cf5a866f07cb9038887a0634d3e1a0375879c 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h @@ -22,7 +22,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel * int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *dp_ctrl, bool force_link_train); void msm_dp_ctrl_off_link_stream(struct msm_dp_ctrl *msm_dp_ctrl); void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl); -void msm_dp_ctrl_off(struct msm_dp_ctrl *msm_dp_ctrl); +void msm_dp_ctrl_stream_clk_off(struct msm_dp_ctrl *msm_dp_ctrl); void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl); irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl); void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl); diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index c059f749c1f204deac9dfb0c56f537f5545d9acb..b0458bbc89e934ca33ed5af3f2a8ebca30b50824 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -911,7 +911,9 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp) if (dp->link->sink_count == 0) msm_dp_ctrl_psm_config(dp->ctrl); - msm_dp_ctrl_off(dp->ctrl); + msm_dp_ctrl_stream_clk_off(dp->ctrl); + + msm_dp_ctrl_off_link(dp->ctrl); /* re-init the PHY so that we can listen to Dongle disconnect */ if (dp->link->sink_count == 0)
Split dp_ctrl_off() into stream and link parts so that for MST cases we can control the link and pixel parts separately. Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> --- drivers/gpu/drm/msm/dp/dp_ctrl.c | 29 +++-------------------------- drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 +- drivers/gpu/drm/msm/dp/dp_display.c | 4 +++- 3 files changed, 7 insertions(+), 28 deletions(-)