[Freedreno] [PATCH v3 3/7] drm/msm/dp: use drm_bridge_hpd_notify() to report HPD status changes

[Kuogee Hsieh]

On 9/19/2023 2:45 AM, Dmitry Baryshkov wrote:
> On Mon, 18 Sept 2023 at 23:16, Kuogee Hsieh <quic_khsieh at 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 at 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 at quicinc.com>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov at 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.

DP-with-external-bridges?
Are you mean hpd through gpio case which only have hpd_notify without both hpd_enable and pd_disable at dp_bridge_ops?
it will use both EV_HPD_PLUG_INT and EV_HPD_UNPLUG_INT events for plug_in and unplug respectively.
Both will end up use dp_display_send_hpd_notification().
Therefore it will works too.


>>
>>>
>>>> @@ -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
>>>>
>
>