| Message ID | 1694813901-26952-4-git-send-email-quic_khsieh@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | incorporate pm runtime framework and eDP clean up | expand |
On Sat, 16 Sept 2023 at 00:38, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote: > > Currently DP driver use drm_helper_hpd_irq_event(), bypassing drm bridge > framework, to report HPD status changes to user space frame work. > Replace it with drm_bridge_hpd_notify() since DP driver is part of drm > bridge. > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Also see the comment below. > --- > drivers/gpu/drm/msm/dp/dp_display.c | 20 ++------------------ > 1 file changed, 2 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index 18d16c7..59f9d85 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -356,26 +356,10 @@ static bool dp_display_is_sink_count_zero(struct dp_display_private *dp) > (dp->link->sink_count == 0); > } > > -static void dp_display_send_hpd_event(struct msm_dp *dp_display) > -{ > - struct dp_display_private *dp; > - struct drm_connector *connector; > - > - dp = container_of(dp_display, struct dp_display_private, dp_display); > - > - connector = dp->dp_display.connector; > - drm_helper_hpd_irq_event(connector->dev); > -} > - > static int dp_display_send_hpd_notification(struct dp_display_private *dp, > bool hpd) > { > - if ((hpd && dp->dp_display.link_ready) || > - (!hpd && !dp->dp_display.link_ready)) { > - drm_dbg_dp(dp->drm_dev, "HPD already %s\n", > - (hpd ? "on" : "off")); > - return 0; > - } > + struct drm_bridge *bridge = dp->dp_display.bridge; > > /* reset video pattern flag on disconnect */ > if (!hpd) Note, this part (resetting the video_test and setting of is_connected) should be moved to the dp_bridge_hpd_notify() too. Please ignore this comment if this is handled later in the series. > @@ -385,7 +369,7 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp, > > drm_dbg_dp(dp->drm_dev, "type=%d hpd=%d\n", > dp->dp_display.connector_type, hpd); > - dp_display_send_hpd_event(&dp->dp_display); > + drm_bridge_hpd_notify(bridge, dp->dp_display.link_ready); > > return 0; > } > -- > 2.7.4 >
On 9/15/2023 5:41 PM, Dmitry Baryshkov wrote: > On Sat, 16 Sept 2023 at 00:38, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote: >> Currently DP driver use drm_helper_hpd_irq_event(), bypassing drm bridge >> framework, to report HPD status changes to user space frame work. >> Replace it with drm_bridge_hpd_notify() since DP driver is part of drm >> bridge. >> >> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Also see the comment below. > >> --- >> drivers/gpu/drm/msm/dp/dp_display.c | 20 ++------------------ >> 1 file changed, 2 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >> index 18d16c7..59f9d85 100644 >> --- a/drivers/gpu/drm/msm/dp/dp_display.c >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c >> @@ -356,26 +356,10 @@ static bool dp_display_is_sink_count_zero(struct dp_display_private *dp) >> (dp->link->sink_count == 0); >> } >> >> -static void dp_display_send_hpd_event(struct msm_dp *dp_display) >> -{ >> - struct dp_display_private *dp; >> - struct drm_connector *connector; >> - >> - dp = container_of(dp_display, struct dp_display_private, dp_display); >> - >> - connector = dp->dp_display.connector; >> - drm_helper_hpd_irq_event(connector->dev); >> -} >> - >> static int dp_display_send_hpd_notification(struct dp_display_private *dp, >> bool hpd) >> { >> - if ((hpd && dp->dp_display.link_ready) || >> - (!hpd && !dp->dp_display.link_ready)) { >> - drm_dbg_dp(dp->drm_dev, "HPD already %s\n", >> - (hpd ? "on" : "off")); >> - return 0; >> - } >> + struct drm_bridge *bridge = dp->dp_display.bridge; >> >> /* reset video pattern flag on disconnect */ >> if (!hpd) > Note, this part (resetting the video_test and setting of is_connected) > should be moved to the dp_bridge_hpd_notify() too. Please ignore this > comment if this is handled later in the series. I think keep them here is better since eDP does not populate hpd_enable, hpd_disable and hpd_notify at edp_bridge_ops at drm_bridge_attach(). Keep them here will work for both eDP and DP. > > >> @@ -385,7 +369,7 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp, >> >> drm_dbg_dp(dp->drm_dev, "type=%d hpd=%d\n", >> dp->dp_display.connector_type, hpd); >> - dp_display_send_hpd_event(&dp->dp_display); >> + drm_bridge_hpd_notify(bridge, dp->dp_display.link_ready); >> >> return 0; >> } >> -- >> 2.7.4 >> >
On Mon, 18 Sept 2023 at 23:16, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote: > > > On 9/15/2023 5:41 PM, Dmitry Baryshkov wrote: > > On Sat, 16 Sept 2023 at 00:38, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote: > >> Currently DP driver use drm_helper_hpd_irq_event(), bypassing drm bridge > >> framework, to report HPD status changes to user space frame work. > >> Replace it with drm_bridge_hpd_notify() since DP driver is part of drm > >> bridge. > >> > >> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > > Also see the comment below. > > > >> --- > >> drivers/gpu/drm/msm/dp/dp_display.c | 20 ++------------------ > >> 1 file changed, 2 insertions(+), 18 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > >> index 18d16c7..59f9d85 100644 > >> --- a/drivers/gpu/drm/msm/dp/dp_display.c > >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c > >> @@ -356,26 +356,10 @@ static bool dp_display_is_sink_count_zero(struct dp_display_private *dp) > >> (dp->link->sink_count == 0); > >> } > >> > >> -static void dp_display_send_hpd_event(struct msm_dp *dp_display) > >> -{ > >> - struct dp_display_private *dp; > >> - struct drm_connector *connector; > >> - > >> - dp = container_of(dp_display, struct dp_display_private, dp_display); > >> - > >> - connector = dp->dp_display.connector; > >> - drm_helper_hpd_irq_event(connector->dev); > >> -} > >> - > >> static int dp_display_send_hpd_notification(struct dp_display_private *dp, > >> bool hpd) > >> { > >> - if ((hpd && dp->dp_display.link_ready) || > >> - (!hpd && !dp->dp_display.link_ready)) { > >> - drm_dbg_dp(dp->drm_dev, "HPD already %s\n", > >> - (hpd ? "on" : "off")); > >> - return 0; > >> - } > >> + struct drm_bridge *bridge = dp->dp_display.bridge; > >> > >> /* reset video pattern flag on disconnect */ > >> if (!hpd) > > Note, this part (resetting the video_test and setting of is_connected) > > should be moved to the dp_bridge_hpd_notify() too. Please ignore this > > comment if this is handled later in the series. > > I think keep them here is better since eDP does not populate hpd_enable, > hpd_disable and hpd_notify at edp_bridge_ops at drm_bridge_attach(). > > Keep them here will work for both eDP and DP. Having them here doesn't work for DP-with-external-bridges, which will not use dp_display_send_hpd_notification. > > > > > > > >> @@ -385,7 +369,7 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp, > >> > >> drm_dbg_dp(dp->drm_dev, "type=%d hpd=%d\n", > >> dp->dp_display.connector_type, hpd); > >> - dp_display_send_hpd_event(&dp->dp_display); > >> + drm_bridge_hpd_notify(bridge, dp->dp_display.link_ready); > >> > >> return 0; > >> } > >> -- > >> 2.7.4 > >> > >
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 18d16c7..59f9d85 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -356,26 +356,10 @@ static bool dp_display_is_sink_count_zero(struct dp_display_private *dp) (dp->link->sink_count == 0); } -static void dp_display_send_hpd_event(struct msm_dp *dp_display) -{ - struct dp_display_private *dp; - struct drm_connector *connector; - - dp = container_of(dp_display, struct dp_display_private, dp_display); - - connector = dp->dp_display.connector; - drm_helper_hpd_irq_event(connector->dev); -} - static int dp_display_send_hpd_notification(struct dp_display_private *dp, bool hpd) { - if ((hpd && dp->dp_display.link_ready) || - (!hpd && !dp->dp_display.link_ready)) { - drm_dbg_dp(dp->drm_dev, "HPD already %s\n", - (hpd ? "on" : "off")); - return 0; - } + struct drm_bridge *bridge = dp->dp_display.bridge; /* reset video pattern flag on disconnect */ if (!hpd) @@ -385,7 +369,7 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp, drm_dbg_dp(dp->drm_dev, "type=%d hpd=%d\n", dp->dp_display.connector_type, hpd); - dp_display_send_hpd_event(&dp->dp_display); + drm_bridge_hpd_notify(bridge, dp->dp_display.link_ready); return 0; }
Currently DP driver use drm_helper_hpd_irq_event(), bypassing drm bridge framework, to report HPD status changes to user space frame work. Replace it with drm_bridge_hpd_notify() since DP driver is part of drm bridge. Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> --- drivers/gpu/drm/msm/dp/dp_display.c | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-)