| Message ID | 20240923074803.10306-1-lists@steffen.cc (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm: xlnx: zynqmp_dpsub: also call drm_helper_hpd_irq_event | expand |
Hi Steffen, Thank you for the patch. On Mon, Sep 23, 2024 at 09:48:03AM +0200, lists@steffen.cc wrote: > From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com> > > With hpd going through the bridge as of commit eb2d64bfcc17 > ("drm: xlnx: zynqmp_dpsub: Report HPD through the bridge") > we don't get hotplug events in userspace on zynqmp hardware. > Also sending hotplug events with drm_helper_hpd_irq_event works. Why does the driver need to call both drm_helper_hpd_irq_event() and drm_bridge_hpd_notify() ? The latter should end up calling drm_kms_helper_connector_hotplug_event(), which is the same function that drm_helper_hpd_irq_event() calls. > Fixes: eb2d64bfcc17 ("drm: xlnx: zynqmp_dpsub: Report HPD through the bridge") > Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com> > --- > drivers/gpu/drm/xlnx/zynqmp_dp.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c > index 1846c4971fd8..cb823540a412 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > @@ -17,6 +17,7 @@ > #include <drm/drm_fourcc.h> > #include <drm/drm_modes.h> > #include <drm/drm_of.h> > +#include <drm/drm_probe_helper.h> > > #include <linux/clk.h> > #include <linux/delay.h> > @@ -1614,6 +1615,9 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work) > hpd_work.work); > enum drm_connector_status status; > > + if (dp->bridge.dev) > + drm_helper_hpd_irq_event(dp->bridge.dev); > + > status = zynqmp_dp_bridge_detect(&dp->bridge); > drm_bridge_hpd_notify(&dp->bridge, status); > }
Hi Laurent, On Tue, 2024-09-24 at 21:43 +0300, Laurent Pinchart wrote: > Hi Steffen, > > Thank you for the patch. > > On Mon, Sep 23, 2024 at 09:48:03AM +0200, lists@steffen.cc wrote: > > From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com> > > > > With hpd going through the bridge as of commit eb2d64bfcc17 > > ("drm: xlnx: zynqmp_dpsub: Report HPD through the bridge") > > we don't get hotplug events in userspace on zynqmp hardware. > > Also sending hotplug events with drm_helper_hpd_irq_event works. > > Why does the driver need to call both drm_helper_hpd_irq_event() and > drm_bridge_hpd_notify() ? The latter should end up calling > drm_kms_helper_connector_hotplug_event(), which is the same function > that drm_helper_hpd_irq_event() calls. I don't know why we need drm_helper_hpd_irq_event. I'll try to trace what happens on hotplug. > > > Fixes: eb2d64bfcc17 ("drm: xlnx: zynqmp_dpsub: Report HPD through > > the bridge") > > Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com> > > --- > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > index 1846c4971fd8..cb823540a412 100644 > > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > @@ -17,6 +17,7 @@ > > #include <drm/drm_fourcc.h> > > #include <drm/drm_modes.h> > > #include <drm/drm_of.h> > > +#include <drm/drm_probe_helper.h> > > > > #include <linux/clk.h> > > #include <linux/delay.h> > > @@ -1614,6 +1615,9 @@ static void zynqmp_dp_hpd_work_func(struct > > work_struct *work) > > hpd_work.work); > > enum drm_connector_status status; > > > > + if (dp->bridge.dev) > > + drm_helper_hpd_irq_event(dp->bridge.dev); > > + > > status = zynqmp_dp_bridge_detect(&dp->bridge); > > drm_bridge_hpd_notify(&dp->bridge, status); > > } >
Hi Steffen, On Wed, Sep 25, 2024 at 09:54:18AM +0200, Steffen Dirkwinkel wrote: > On Tue, 2024-09-24 at 21:43 +0300, Laurent Pinchart wrote: > > On Mon, Sep 23, 2024 at 09:48:03AM +0200, lists@steffen.cc wrote: > > > From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com> > > > > > > With hpd going through the bridge as of commit eb2d64bfcc17 > > > ("drm: xlnx: zynqmp_dpsub: Report HPD through the bridge") > > > we don't get hotplug events in userspace on zynqmp hardware. > > > Also sending hotplug events with drm_helper_hpd_irq_event works. > > > > Why does the driver need to call both drm_helper_hpd_irq_event() and > > drm_bridge_hpd_notify() ? The latter should end up calling > > drm_kms_helper_connector_hotplug_event(), which is the same function > > that drm_helper_hpd_irq_event() calls. > > I don't know why we need drm_helper_hpd_irq_event. > I'll try to trace what happens on hotplug. Thank you. Let's try to find the best solution based on your findings. > > > Fixes: eb2d64bfcc17 ("drm: xlnx: zynqmp_dpsub: Report HPD through > > > the bridge") > > > Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com> > > > --- > > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > index 1846c4971fd8..cb823540a412 100644 > > > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > > > @@ -17,6 +17,7 @@ > > > #include <drm/drm_fourcc.h> > > > #include <drm/drm_modes.h> > > > #include <drm/drm_of.h> > > > +#include <drm/drm_probe_helper.h> > > > > > > #include <linux/clk.h> > > > #include <linux/delay.h> > > > @@ -1614,6 +1615,9 @@ static void zynqmp_dp_hpd_work_func(struct > > > work_struct *work) > > > hpd_work.work); > > > enum drm_connector_status status; > > > > > > + if (dp->bridge.dev) > > > + drm_helper_hpd_irq_event(dp->bridge.dev); > > > + > > > status = zynqmp_dp_bridge_detect(&dp->bridge); > > > drm_bridge_hpd_notify(&dp->bridge, status); > > > }
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c index 1846c4971fd8..cb823540a412 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c @@ -17,6 +17,7 @@ #include <drm/drm_fourcc.h> #include <drm/drm_modes.h> #include <drm/drm_of.h> +#include <drm/drm_probe_helper.h> #include <linux/clk.h> #include <linux/delay.h> @@ -1614,6 +1615,9 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work) hpd_work.work); enum drm_connector_status status; + if (dp->bridge.dev) + drm_helper_hpd_irq_event(dp->bridge.dev); + status = zynqmp_dp_bridge_detect(&dp->bridge); drm_bridge_hpd_notify(&dp->bridge, status); }