| Message ID | 20221102180705.459294-6-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge_connector: perform HPD enablement automatically | expand |
On Wed, Nov 02, 2022 at 09:07:03PM +0300, Dmitry Baryshkov wrote: > The functionality of drm_bridge_connector_enable_hpd() and > drm_bridge_connector_disable_hpd() is provided automatically by the > drm_kms_poll helpers. Stop calling these functions manually. I stumbled over this one when investigating a hotplug-related crash in the MSM DRM driver which this series prevents by moving hotplug notification enable to drm_kms_helper_poll_enable(). > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/hdmi/hdmi.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c > index 93fe61b86967..a540c45d4fd3 100644 > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c > @@ -348,8 +348,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, > goto fail; > } > > - drm_bridge_connector_enable_hpd(hdmi->connector); > - > ret = msm_hdmi_hpd_enable(hdmi->bridge); > if (ret < 0) { > DRM_DEV_ERROR(&hdmi->pdev->dev, "failed to enable HPD: %d\n", ret); It looks like you are now enabling hotplug events before the DRM driver is ready to receive them (i.e. msm_hdmi_hpd_enable() is called before drm_bridge_connector_enable_hpd()). Could this not lead to missed events or is the state being setup correctly somewhere else? Shouldn't the call to msm_hdmi_hpd_enable() be moved to when HPD is enabled either way (e.g. by being converted to a hpd_enable callback)? Johan
On Wed, 8 Mar 2023 at 12:20, Johan Hovold <johan@kernel.org> wrote: > > On Wed, Nov 02, 2022 at 09:07:03PM +0300, Dmitry Baryshkov wrote: > > The functionality of drm_bridge_connector_enable_hpd() and > > drm_bridge_connector_disable_hpd() is provided automatically by the > > drm_kms_poll helpers. Stop calling these functions manually. > > I stumbled over this one when investigating a hotplug-related crash in > the MSM DRM driver which this series prevents by moving hotplug > notification enable to drm_kms_helper_poll_enable(). > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/gpu/drm/msm/hdmi/hdmi.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c > > index 93fe61b86967..a540c45d4fd3 100644 > > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c > > @@ -348,8 +348,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, > > goto fail; > > } > > > > - drm_bridge_connector_enable_hpd(hdmi->connector); > > - > > ret = msm_hdmi_hpd_enable(hdmi->bridge); > > if (ret < 0) { > > DRM_DEV_ERROR(&hdmi->pdev->dev, "failed to enable HPD: %d\n", ret); > > It looks like you are now enabling hotplug events before the DRM driver > is ready to receive them (i.e. msm_hdmi_hpd_enable() is called before > drm_bridge_connector_enable_hpd()). > > Could this not lead to missed events or is the state being setup > correctly somewhere else? > > Shouldn't the call to msm_hdmi_hpd_enable() be moved to when HPD is > enabled either way (e.g. by being converted to a hpd_enable callback)? Eventually I'll get to it (hopefully during this weekend). There is one item which needs to be investigated, see [1]. I have to check if this is applicable to earlier generations, which also means resurrecting the msm8974 HDMI PHY patchset posted ages ago. I think the initial status is determined correctly using the .detect(). At least I saw no issues with this patchset. However, thanks for the pointer. [1] https://git.codelinaro.org/linaro/qcomlt/kernel/-/commit/6ae2c308555f470ba63f90b7171519a242f96a67
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 93fe61b86967..a540c45d4fd3 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -348,8 +348,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, goto fail; } - drm_bridge_connector_enable_hpd(hdmi->connector); - ret = msm_hdmi_hpd_enable(hdmi->bridge); if (ret < 0) { DRM_DEV_ERROR(&hdmi->pdev->dev, "failed to enable HPD: %d\n", ret);
The functionality of drm_bridge_connector_enable_hpd() and drm_bridge_connector_disable_hpd() is provided automatically by the drm_kms_poll helpers. Stop calling these functions manually. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/hdmi/hdmi.c | 2 -- 1 file changed, 2 deletions(-)