On Mon, Feb 07, 2022 at 08:43:28PM -0800, Bjorn Andersson wrote:
The Qualcomm DisplayPort driver contains traces of the necessary plumbing to hook up USB HPD, in the form of the dp_hpd module and the dp_usbpd_cb struct. Use this as basis for implementing the oob_hotplug_event() callback, by amending the dp_hpd module with the missing logic.
Overall the solution is similar to what's done downstream, but upstream all the code to disect the HPD notification lives on the calling side of drm_connector_oob_hotplug_event().
drm_connector_oob_hotplug_event() performs the lookup of the drm_connector based on fwnode, hence the need to assign the fwnode in dp_drm_connector_init().
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org
drivers/gpu/drm/msm/dp/dp_display.c | 8 ++++++++ drivers/gpu/drm/msm/dp/dp_display.h | 2 ++ drivers/gpu/drm/msm/dp/dp_drm.c | 10 ++++++++++ drivers/gpu/drm/msm/dp/dp_hpd.c | 19 +++++++++++++++++++ drivers/gpu/drm/msm/dp/dp_hpd.h | 4 ++++ 5 files changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 7cc4d21f2091..124a2f794382 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -414,6 +414,13 @@ static int dp_display_usbpd_configure_cb(struct device *dev) return dp_display_process_hpd_high(dp); }
+void dp_display_oob_hotplug_event(struct msm_dp *dp_display, bool hpd_state) +{
- struct dp_display_private *dp = container_of(dp_display, struct dp_display_private, dp_display);
- dp->usbpd->oob_event(dp->usbpd, hpd_state);
+}
static int dp_display_usbpd_disconnect_cb(struct device *dev) { struct dp_display_private *dp = dev_get_dp_display_private(dev); @@ -1251,6 +1258,7 @@ static int dp_display_probe(struct platform_device *pdev) dp->pdev = pdev; dp->name = "drm_dp"; dp->dp_display.connector_type = desc->connector_type;
- dp->dp_display.dev = &pdev->dev;
You did not properly reference count this pointer you just saved. What is to keep that pointer from going away without you knowing about it?
And you already have a pointer to pdev, why save another one here?
rc = dp_init_sub_modules(dp); if (rc) { diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h index e3adcd578a90..1f856b3bca79 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.h +++ b/drivers/gpu/drm/msm/dp/dp_display.h @@ -11,6 +11,7 @@ #include "disp/msm_disp_snapshot.h"
struct msm_dp {
- struct device *dev; struct drm_device *drm_dev; struct device *codec_dev;
So you now have pointers to 3 different devices here? What does 'dev' point to that the other ones do not? This needs to be documented really well here.
thanks,
greg k-h