* [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2021-12-25 6:31 ` Nikita Yushchenko 0 siblings, 0 replies; 22+ messages in thread From: Nikita Yushchenko @ 2021-12-25 6:31 UTC (permalink / raw) To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Kieran Bingham, Laurent Pinchart, Jacopo Mondi Cc: dri-devel, linux-kernel, Nikita Yushchenko Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() get ignored unless somebody calls drm_bridge_hpd_enable(). When the connector for the bridge is bridge_connector, such a call is done from drm_bridge_connector_enable_hpd(). However drm_bridge_connector_enable_hpd() is never called on init paths, documentation suggests that it is intended for suspend/resume paths. In result, once encoders are switched to bridge_connector, bridge-detected HPD stops working. This patch adds a call to that API on init path. This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD events via interrupts. Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> --- drivers/gpu/drm/drm_bridge_connector.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 791379816837..4f20137ef21d 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, connector_type, ddc); drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); - if (bridge_connector->bridge_hpd) + if (bridge_connector->bridge_hpd) { connector->polled = DRM_CONNECTOR_POLL_HPD; + drm_bridge_connector_enable_hpd(connector); + } else if (bridge_connector->bridge_detect) connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; -- 2.30.2 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2021-12-25 6:31 ` Nikita Yushchenko 0 siblings, 0 replies; 22+ messages in thread From: Nikita Yushchenko @ 2021-12-25 6:31 UTC (permalink / raw) To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Kieran Bingham, Laurent Pinchart, Jacopo Mondi Cc: Nikita Yushchenko, linux-kernel, dri-devel Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() get ignored unless somebody calls drm_bridge_hpd_enable(). When the connector for the bridge is bridge_connector, such a call is done from drm_bridge_connector_enable_hpd(). However drm_bridge_connector_enable_hpd() is never called on init paths, documentation suggests that it is intended for suspend/resume paths. In result, once encoders are switched to bridge_connector, bridge-detected HPD stops working. This patch adds a call to that API on init path. This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD events via interrupts. Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> --- drivers/gpu/drm/drm_bridge_connector.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 791379816837..4f20137ef21d 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, connector_type, ddc); drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); - if (bridge_connector->bridge_hpd) + if (bridge_connector->bridge_hpd) { connector->polled = DRM_CONNECTOR_POLL_HPD; + drm_bridge_connector_enable_hpd(connector); + } else if (bridge_connector->bridge_detect) connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; -- 2.30.2 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2021-12-25 6:31 ` Nikita Yushchenko @ 2021-12-29 23:44 ` Laurent Pinchart -1 siblings, 0 replies; 22+ messages in thread From: Laurent Pinchart @ 2021-12-29 23:44 UTC (permalink / raw) To: Nikita Yushchenko Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Kieran Bingham, Jacopo Mondi, dri-devel, linux-kernel Hi Nikita, Thank you for the patch. On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > connector for the bridge is bridge_connector, such a call is done from > drm_bridge_connector_enable_hpd(). > > However drm_bridge_connector_enable_hpd() is never called on init paths, > documentation suggests that it is intended for suspend/resume paths. Hmmmm... I'm in two minds about this. The problem description is correct, but I wonder if HPD should be enabled unconditionally here, or if this should be left to display drivers to control. drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, other drivers don't. It feels like this should be under control of the display controller driver, but I can't think of a use case for not enabling HPD at init time. Any second opinion from anyone ? > In result, once encoders are switched to bridge_connector, > bridge-detected HPD stops working. > > This patch adds a call to that API on init path. > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD > events via interrupts. > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > --- > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > index 791379816837..4f20137ef21d 100644 > --- a/drivers/gpu/drm/drm_bridge_connector.c > +++ b/drivers/gpu/drm/drm_bridge_connector.c > @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > connector_type, ddc); > drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); > > - if (bridge_connector->bridge_hpd) > + if (bridge_connector->bridge_hpd) { > connector->polled = DRM_CONNECTOR_POLL_HPD; > + drm_bridge_connector_enable_hpd(connector); > + } > else if (bridge_connector->bridge_detect) > connector->polled = DRM_CONNECTOR_POLL_CONNECT > | DRM_CONNECTOR_POLL_DISCONNECT; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2021-12-29 23:44 ` Laurent Pinchart 0 siblings, 0 replies; 22+ messages in thread From: Laurent Pinchart @ 2021-12-29 23:44 UTC (permalink / raw) To: Nikita Yushchenko Cc: Thomas Zimmermann, David Airlie, linux-kernel, Kieran Bingham, Jacopo Mondi, dri-devel Hi Nikita, Thank you for the patch. On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > connector for the bridge is bridge_connector, such a call is done from > drm_bridge_connector_enable_hpd(). > > However drm_bridge_connector_enable_hpd() is never called on init paths, > documentation suggests that it is intended for suspend/resume paths. Hmmmm... I'm in two minds about this. The problem description is correct, but I wonder if HPD should be enabled unconditionally here, or if this should be left to display drivers to control. drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, other drivers don't. It feels like this should be under control of the display controller driver, but I can't think of a use case for not enabling HPD at init time. Any second opinion from anyone ? > In result, once encoders are switched to bridge_connector, > bridge-detected HPD stops working. > > This patch adds a call to that API on init path. > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD > events via interrupts. > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > --- > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > index 791379816837..4f20137ef21d 100644 > --- a/drivers/gpu/drm/drm_bridge_connector.c > +++ b/drivers/gpu/drm/drm_bridge_connector.c > @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > connector_type, ddc); > drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); > > - if (bridge_connector->bridge_hpd) > + if (bridge_connector->bridge_hpd) { > connector->polled = DRM_CONNECTOR_POLL_HPD; > + drm_bridge_connector_enable_hpd(connector); > + } > else if (bridge_connector->bridge_detect) > connector->polled = DRM_CONNECTOR_POLL_CONNECT > | DRM_CONNECTOR_POLL_DISCONNECT; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2021-12-29 23:44 ` Laurent Pinchart @ 2022-01-31 16:38 ` Nikita Yushchenko -1 siblings, 0 replies; 22+ messages in thread From: Nikita Yushchenko @ 2022-01-31 16:38 UTC (permalink / raw) To: Laurent Pinchart Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Kieran Bingham, Jacopo Mondi, dri-devel, linux-kernel >> Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() >> get ignored unless somebody calls drm_bridge_hpd_enable(). When the >> connector for the bridge is bridge_connector, such a call is done from >> drm_bridge_connector_enable_hpd(). >> >> However drm_bridge_connector_enable_hpd() is never called on init paths, >> documentation suggests that it is intended for suspend/resume paths. > > Hmmmm... I'm in two minds about this. The problem description is > correct, but I wonder if HPD should be enabled unconditionally here, or > if this should be left to display drivers to control. > drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > other drivers don't. > > It feels like this should be under control of the display controller > driver, but I can't think of a use case for not enabling HPD at init > time. Any second opinion from anyone ? Hi. Can we somehow move forward here?.. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2022-01-31 16:38 ` Nikita Yushchenko 0 siblings, 0 replies; 22+ messages in thread From: Nikita Yushchenko @ 2022-01-31 16:38 UTC (permalink / raw) To: Laurent Pinchart Cc: Thomas Zimmermann, David Airlie, linux-kernel, Kieran Bingham, Jacopo Mondi, dri-devel >> Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() >> get ignored unless somebody calls drm_bridge_hpd_enable(). When the >> connector for the bridge is bridge_connector, such a call is done from >> drm_bridge_connector_enable_hpd(). >> >> However drm_bridge_connector_enable_hpd() is never called on init paths, >> documentation suggests that it is intended for suspend/resume paths. > > Hmmmm... I'm in two minds about this. The problem description is > correct, but I wonder if HPD should be enabled unconditionally here, or > if this should be left to display drivers to control. > drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > other drivers don't. > > It feels like this should be under control of the display controller > driver, but I can't think of a use case for not enabling HPD at init > time. Any second opinion from anyone ? Hi. Can we somehow move forward here?.. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2021-12-29 23:44 ` Laurent Pinchart @ 2022-02-23 16:17 ` Kieran Bingham -1 siblings, 0 replies; 22+ messages in thread From: Kieran Bingham @ 2022-02-23 16:17 UTC (permalink / raw) To: Laurent Pinchart, Nikita Yushchenko Cc: Thomas Zimmermann, David Airlie, linux-kernel, Jacopo Mondi, dri-devel Hi Laurent, Nikita, Quoting Laurent Pinchart (2021-12-29 23:44:29) > Hi Nikita, > > Thank you for the patch. > > On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > > connector for the bridge is bridge_connector, such a call is done from > > drm_bridge_connector_enable_hpd(). > > > > However drm_bridge_connector_enable_hpd() is never called on init paths, > > documentation suggests that it is intended for suspend/resume paths. > > Hmmmm... I'm in two minds about this. The problem description is > correct, but I wonder if HPD should be enabled unconditionally here, or > if this should be left to display drivers to control. > drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > other drivers don't. > > It feels like this should be under control of the display controller > driver, but I can't think of a use case for not enabling HPD at init > time. Any second opinion from anyone ? This patch solves an issue I have where I have recently enabled HPD on the SN65DSI86, but without this, I do not get calls to my .hpd_enable or .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. So it needs to be enabled somewhere, and this seems reasonable to me? It's not directly related to the display controller - as it's a factor of the bridge? On Falcon-V3U with HPD additions to SN65DSI86: Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > In result, once encoders are switched to bridge_connector, > > bridge-detected HPD stops working. > > > > This patch adds a call to that API on init path. > > > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD > > events via interrupts. > > > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > > --- > > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > > index 791379816837..4f20137ef21d 100644 > > --- a/drivers/gpu/drm/drm_bridge_connector.c > > +++ b/drivers/gpu/drm/drm_bridge_connector.c > > @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > connector_type, ddc); > > drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); > > > > - if (bridge_connector->bridge_hpd) > > + if (bridge_connector->bridge_hpd) { > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > + drm_bridge_connector_enable_hpd(connector); > > + } > > else if (bridge_connector->bridge_detect) > > connector->polled = DRM_CONNECTOR_POLL_CONNECT > > | DRM_CONNECTOR_POLL_DISCONNECT; > > -- > Regards, > > Laurent Pinchart ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2022-02-23 16:17 ` Kieran Bingham 0 siblings, 0 replies; 22+ messages in thread From: Kieran Bingham @ 2022-02-23 16:17 UTC (permalink / raw) To: Laurent Pinchart, Nikita Yushchenko Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Jacopo Mondi, dri-devel, linux-kernel Hi Laurent, Nikita, Quoting Laurent Pinchart (2021-12-29 23:44:29) > Hi Nikita, > > Thank you for the patch. > > On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > > connector for the bridge is bridge_connector, such a call is done from > > drm_bridge_connector_enable_hpd(). > > > > However drm_bridge_connector_enable_hpd() is never called on init paths, > > documentation suggests that it is intended for suspend/resume paths. > > Hmmmm... I'm in two minds about this. The problem description is > correct, but I wonder if HPD should be enabled unconditionally here, or > if this should be left to display drivers to control. > drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > other drivers don't. > > It feels like this should be under control of the display controller > driver, but I can't think of a use case for not enabling HPD at init > time. Any second opinion from anyone ? This patch solves an issue I have where I have recently enabled HPD on the SN65DSI86, but without this, I do not get calls to my .hpd_enable or .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. So it needs to be enabled somewhere, and this seems reasonable to me? It's not directly related to the display controller - as it's a factor of the bridge? On Falcon-V3U with HPD additions to SN65DSI86: Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > In result, once encoders are switched to bridge_connector, > > bridge-detected HPD stops working. > > > > This patch adds a call to that API on init path. > > > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD > > events via interrupts. > > > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > > --- > > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > > index 791379816837..4f20137ef21d 100644 > > --- a/drivers/gpu/drm/drm_bridge_connector.c > > +++ b/drivers/gpu/drm/drm_bridge_connector.c > > @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > connector_type, ddc); > > drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); > > > > - if (bridge_connector->bridge_hpd) > > + if (bridge_connector->bridge_hpd) { > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > + drm_bridge_connector_enable_hpd(connector); > > + } > > else if (bridge_connector->bridge_detect) > > connector->polled = DRM_CONNECTOR_POLL_CONNECT > > | DRM_CONNECTOR_POLL_DISCONNECT; > > -- > Regards, > > Laurent Pinchart ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2022-02-23 16:17 ` Kieran Bingham @ 2022-02-23 16:25 ` Laurent Pinchart -1 siblings, 0 replies; 22+ messages in thread From: Laurent Pinchart @ 2022-02-23 16:25 UTC (permalink / raw) To: Kieran Bingham Cc: Nikita Yushchenko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Jacopo Mondi, dri-devel, linux-kernel Hello, On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: > Quoting Laurent Pinchart (2021-12-29 23:44:29) > > On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > > > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > > > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > > > connector for the bridge is bridge_connector, such a call is done from > > > drm_bridge_connector_enable_hpd(). > > > > > > However drm_bridge_connector_enable_hpd() is never called on init paths, > > > documentation suggests that it is intended for suspend/resume paths. > > > > Hmmmm... I'm in two minds about this. The problem description is > > correct, but I wonder if HPD should be enabled unconditionally here, or > > if this should be left to display drivers to control. > > drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > > other drivers don't. > > > > It feels like this should be under control of the display controller > > driver, but I can't think of a use case for not enabling HPD at init > > time. Any second opinion from anyone ? > > This patch solves an issue I have where I have recently enabled HPD on > the SN65DSI86, but without this, I do not get calls to my .hpd_enable or > .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. > > So it needs to be enabled somewhere, and this seems reasonable to me? > It's not directly related to the display controller - as it's a factor > of the bridge? > > On Falcon-V3U with HPD additions to SN65DSI86: > Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> If you think this is right, then Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > In result, once encoders are switched to bridge_connector, > > > bridge-detected HPD stops working. > > > > > > This patch adds a call to that API on init path. > > > > > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD > > > events via interrupts. > > > > > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") > > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > > > --- > > > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > > > index 791379816837..4f20137ef21d 100644 > > > --- a/drivers/gpu/drm/drm_bridge_connector.c > > > +++ b/drivers/gpu/drm/drm_bridge_connector.c > > > @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > > connector_type, ddc); > > > drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); > > > > > > - if (bridge_connector->bridge_hpd) > > > + if (bridge_connector->bridge_hpd) { > > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > > + drm_bridge_connector_enable_hpd(connector); > > > + } > > > else if (bridge_connector->bridge_detect) > > > connector->polled = DRM_CONNECTOR_POLL_CONNECT > > > | DRM_CONNECTOR_POLL_DISCONNECT; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2022-02-23 16:25 ` Laurent Pinchart 0 siblings, 0 replies; 22+ messages in thread From: Laurent Pinchart @ 2022-02-23 16:25 UTC (permalink / raw) To: Kieran Bingham Cc: Nikita Yushchenko, Thomas Zimmermann, David Airlie, linux-kernel, Jacopo Mondi, dri-devel Hello, On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: > Quoting Laurent Pinchart (2021-12-29 23:44:29) > > On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > > > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > > > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > > > connector for the bridge is bridge_connector, such a call is done from > > > drm_bridge_connector_enable_hpd(). > > > > > > However drm_bridge_connector_enable_hpd() is never called on init paths, > > > documentation suggests that it is intended for suspend/resume paths. > > > > Hmmmm... I'm in two minds about this. The problem description is > > correct, but I wonder if HPD should be enabled unconditionally here, or > > if this should be left to display drivers to control. > > drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > > other drivers don't. > > > > It feels like this should be under control of the display controller > > driver, but I can't think of a use case for not enabling HPD at init > > time. Any second opinion from anyone ? > > This patch solves an issue I have where I have recently enabled HPD on > the SN65DSI86, but without this, I do not get calls to my .hpd_enable or > .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. > > So it needs to be enabled somewhere, and this seems reasonable to me? > It's not directly related to the display controller - as it's a factor > of the bridge? > > On Falcon-V3U with HPD additions to SN65DSI86: > Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> If you think this is right, then Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > In result, once encoders are switched to bridge_connector, > > > bridge-detected HPD stops working. > > > > > > This patch adds a call to that API on init path. > > > > > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD > > > events via interrupts. > > > > > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") > > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > > > --- > > > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > > > index 791379816837..4f20137ef21d 100644 > > > --- a/drivers/gpu/drm/drm_bridge_connector.c > > > +++ b/drivers/gpu/drm/drm_bridge_connector.c > > > @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > > connector_type, ddc); > > > drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); > > > > > > - if (bridge_connector->bridge_hpd) > > > + if (bridge_connector->bridge_hpd) { > > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > > + drm_bridge_connector_enable_hpd(connector); > > > + } > > > else if (bridge_connector->bridge_detect) > > > connector->polled = DRM_CONNECTOR_POLL_CONNECT > > > | DRM_CONNECTOR_POLL_DISCONNECT; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2022-02-23 16:25 ` Laurent Pinchart @ 2022-02-23 17:02 ` Kieran Bingham -1 siblings, 0 replies; 22+ messages in thread From: Kieran Bingham @ 2022-02-23 17:02 UTC (permalink / raw) To: Laurent Pinchart Cc: Nikita Yushchenko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Jacopo Mondi, dri-devel, linux-kernel Quoting Laurent Pinchart (2022-02-23 16:25:28) > Hello, > > On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: > > Quoting Laurent Pinchart (2021-12-29 23:44:29) > > > On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > > > > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > > > > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > > > > connector for the bridge is bridge_connector, such a call is done from > > > > drm_bridge_connector_enable_hpd(). > > > > > > > > However drm_bridge_connector_enable_hpd() is never called on init paths, > > > > documentation suggests that it is intended for suspend/resume paths. > > > > > > Hmmmm... I'm in two minds about this. The problem description is > > > correct, but I wonder if HPD should be enabled unconditionally here, or > > > if this should be left to display drivers to control. > > > drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > > > other drivers don't. > > > > > > It feels like this should be under control of the display controller > > > driver, but I can't think of a use case for not enabling HPD at init > > > time. Any second opinion from anyone ? > > > > This patch solves an issue I have where I have recently enabled HPD on > > the SN65DSI86, but without this, I do not get calls to my .hpd_enable or > > .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. > > > > So it needs to be enabled somewhere, and this seems reasonable to me? > > It's not directly related to the display controller - as it's a factor > > of the bridge? > > > > On Falcon-V3U with HPD additions to SN65DSI86: > > Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > If you think this is right, then > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I do, and at the very least it works for me, and fixes Nikita's issue so to me that's enough for: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > In result, once encoders are switched to bridge_connector, > > > > bridge-detected HPD stops working. > > > > > > > > This patch adds a call to that API on init path. > > > > > > > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD > > > > events via interrupts. > > > > > > > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") > > > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > > > > --- > > > > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > > > > index 791379816837..4f20137ef21d 100644 > > > > --- a/drivers/gpu/drm/drm_bridge_connector.c > > > > +++ b/drivers/gpu/drm/drm_bridge_connector.c > > > > @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > > > connector_type, ddc); > > > > drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); > > > > > > > > - if (bridge_connector->bridge_hpd) > > > > + if (bridge_connector->bridge_hpd) { > > > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > > > + drm_bridge_connector_enable_hpd(connector); > > > > + } > > > > else if (bridge_connector->bridge_detect) > > > > connector->polled = DRM_CONNECTOR_POLL_CONNECT > > > > | DRM_CONNECTOR_POLL_DISCONNECT; > > -- > Regards, > > Laurent Pinchart ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2022-02-23 17:02 ` Kieran Bingham 0 siblings, 0 replies; 22+ messages in thread From: Kieran Bingham @ 2022-02-23 17:02 UTC (permalink / raw) To: Laurent Pinchart Cc: Nikita Yushchenko, Thomas Zimmermann, David Airlie, linux-kernel, Jacopo Mondi, dri-devel Quoting Laurent Pinchart (2022-02-23 16:25:28) > Hello, > > On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: > > Quoting Laurent Pinchart (2021-12-29 23:44:29) > > > On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > > > > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > > > > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > > > > connector for the bridge is bridge_connector, such a call is done from > > > > drm_bridge_connector_enable_hpd(). > > > > > > > > However drm_bridge_connector_enable_hpd() is never called on init paths, > > > > documentation suggests that it is intended for suspend/resume paths. > > > > > > Hmmmm... I'm in two minds about this. The problem description is > > > correct, but I wonder if HPD should be enabled unconditionally here, or > > > if this should be left to display drivers to control. > > > drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > > > other drivers don't. > > > > > > It feels like this should be under control of the display controller > > > driver, but I can't think of a use case for not enabling HPD at init > > > time. Any second opinion from anyone ? > > > > This patch solves an issue I have where I have recently enabled HPD on > > the SN65DSI86, but without this, I do not get calls to my .hpd_enable or > > .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. > > > > So it needs to be enabled somewhere, and this seems reasonable to me? > > It's not directly related to the display controller - as it's a factor > > of the bridge? > > > > On Falcon-V3U with HPD additions to SN65DSI86: > > Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > If you think this is right, then > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I do, and at the very least it works for me, and fixes Nikita's issue so to me that's enough for: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > In result, once encoders are switched to bridge_connector, > > > > bridge-detected HPD stops working. > > > > > > > > This patch adds a call to that API on init path. > > > > > > > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports HPD > > > > events via interrupts. > > > > > > > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() helper") > > > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > > > > --- > > > > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c > > > > index 791379816837..4f20137ef21d 100644 > > > > --- a/drivers/gpu/drm/drm_bridge_connector.c > > > > +++ b/drivers/gpu/drm/drm_bridge_connector.c > > > > @@ -369,8 +369,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, > > > > connector_type, ddc); > > > > drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs); > > > > > > > > - if (bridge_connector->bridge_hpd) > > > > + if (bridge_connector->bridge_hpd) { > > > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > > > + drm_bridge_connector_enable_hpd(connector); > > > > + } > > > > else if (bridge_connector->bridge_detect) > > > > connector->polled = DRM_CONNECTOR_POLL_CONNECT > > > > | DRM_CONNECTOR_POLL_DISCONNECT; > > -- > Regards, > > Laurent Pinchart ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2022-02-23 17:02 ` Kieran Bingham @ 2022-08-31 13:02 ` Tomi Valkeinen -1 siblings, 0 replies; 22+ messages in thread From: Tomi Valkeinen @ 2022-08-31 13:02 UTC (permalink / raw) To: Kieran Bingham, Laurent Pinchart, Nikita Yushchenko Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Jacopo Mondi, dri-devel, linux-kernel Hi, On 23/02/2022 19:02, Kieran Bingham wrote: > Quoting Laurent Pinchart (2022-02-23 16:25:28) >> Hello, >> >> On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: >>> Quoting Laurent Pinchart (2021-12-29 23:44:29) >>>> On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: >>>>> Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() >>>>> get ignored unless somebody calls drm_bridge_hpd_enable(). When the >>>>> connector for the bridge is bridge_connector, such a call is done from >>>>> drm_bridge_connector_enable_hpd(). >>>>> >>>>> However drm_bridge_connector_enable_hpd() is never called on init paths, >>>>> documentation suggests that it is intended for suspend/resume paths. >>>> >>>> Hmmmm... I'm in two minds about this. The problem description is >>>> correct, but I wonder if HPD should be enabled unconditionally here, or >>>> if this should be left to display drivers to control. >>>> drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, >>>> other drivers don't. >>>> >>>> It feels like this should be under control of the display controller >>>> driver, but I can't think of a use case for not enabling HPD at init >>>> time. Any second opinion from anyone ? >>> >>> This patch solves an issue I have where I have recently enabled HPD on >>> the SN65DSI86, but without this, I do not get calls to my .hpd_enable or >>> .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. >>> >>> So it needs to be enabled somewhere, and this seems reasonable to me? >>> It's not directly related to the display controller - as it's a factor >>> of the bridge? >>> >>> On Falcon-V3U with HPD additions to SN65DSI86: >>> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> >> If you think this is right, then >> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I do, and at the very least it works for me, and fixes Nikita's issue so > to me that's enough for: So who disables the HPD now? Is the drm_bridge_connector_enable_hpd & drm_bridge_connector_disable_hpd documentation now wrong as they talk about suspend/resume handlers? Tomi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2022-08-31 13:02 ` Tomi Valkeinen 0 siblings, 0 replies; 22+ messages in thread From: Tomi Valkeinen @ 2022-08-31 13:02 UTC (permalink / raw) To: Kieran Bingham, Laurent Pinchart, Nikita Yushchenko Cc: Thomas Zimmermann, David Airlie, linux-kernel, Jacopo Mondi, dri-devel Hi, On 23/02/2022 19:02, Kieran Bingham wrote: > Quoting Laurent Pinchart (2022-02-23 16:25:28) >> Hello, >> >> On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: >>> Quoting Laurent Pinchart (2021-12-29 23:44:29) >>>> On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: >>>>> Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() >>>>> get ignored unless somebody calls drm_bridge_hpd_enable(). When the >>>>> connector for the bridge is bridge_connector, such a call is done from >>>>> drm_bridge_connector_enable_hpd(). >>>>> >>>>> However drm_bridge_connector_enable_hpd() is never called on init paths, >>>>> documentation suggests that it is intended for suspend/resume paths. >>>> >>>> Hmmmm... I'm in two minds about this. The problem description is >>>> correct, but I wonder if HPD should be enabled unconditionally here, or >>>> if this should be left to display drivers to control. >>>> drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, >>>> other drivers don't. >>>> >>>> It feels like this should be under control of the display controller >>>> driver, but I can't think of a use case for not enabling HPD at init >>>> time. Any second opinion from anyone ? >>> >>> This patch solves an issue I have where I have recently enabled HPD on >>> the SN65DSI86, but without this, I do not get calls to my .hpd_enable or >>> .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. >>> >>> So it needs to be enabled somewhere, and this seems reasonable to me? >>> It's not directly related to the display controller - as it's a factor >>> of the bridge? >>> >>> On Falcon-V3U with HPD additions to SN65DSI86: >>> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> >> If you think this is right, then >> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I do, and at the very least it works for me, and fixes Nikita's issue so > to me that's enough for: So who disables the HPD now? Is the drm_bridge_connector_enable_hpd & drm_bridge_connector_disable_hpd documentation now wrong as they talk about suspend/resume handlers? Tomi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2022-08-31 13:02 ` Tomi Valkeinen @ 2022-09-05 5:26 ` Tomi Valkeinen -1 siblings, 0 replies; 22+ messages in thread From: Tomi Valkeinen @ 2022-09-05 5:26 UTC (permalink / raw) To: Kieran Bingham, Laurent Pinchart, Nikita Yushchenko Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Jacopo Mondi, dri-devel, linux-kernel On 31/08/2022 16:02, Tomi Valkeinen wrote: > Hi, > > On 23/02/2022 19:02, Kieran Bingham wrote: >> Quoting Laurent Pinchart (2022-02-23 16:25:28) >>> Hello, >>> >>> On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: >>>> Quoting Laurent Pinchart (2021-12-29 23:44:29) >>>>> On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: >>>>>> Hotplug events reported by bridge drivers over >>>>>> drm_bridge_hpd_notify() >>>>>> get ignored unless somebody calls drm_bridge_hpd_enable(). When the >>>>>> connector for the bridge is bridge_connector, such a call is done >>>>>> from >>>>>> drm_bridge_connector_enable_hpd(). >>>>>> >>>>>> However drm_bridge_connector_enable_hpd() is never called on init >>>>>> paths, >>>>>> documentation suggests that it is intended for suspend/resume paths. >>>>> >>>>> Hmmmm... I'm in two minds about this. The problem description is >>>>> correct, but I wonder if HPD should be enabled unconditionally >>>>> here, or >>>>> if this should be left to display drivers to control. >>>>> drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, >>>>> other drivers don't. >>>>> >>>>> It feels like this should be under control of the display controller >>>>> driver, but I can't think of a use case for not enabling HPD at init >>>>> time. Any second opinion from anyone ? >>>> >>>> This patch solves an issue I have where I have recently enabled HPD on >>>> the SN65DSI86, but without this, I do not get calls to my >>>> .hpd_enable or >>>> .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. >>>> >>>> So it needs to be enabled somewhere, and this seems reasonable to me? >>>> It's not directly related to the display controller - as it's a factor >>>> of the bridge? >>>> >>>> On Falcon-V3U with HPD additions to SN65DSI86: >>>> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >>> >>> If you think this is right, then >>> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> I do, and at the very least it works for me, and fixes Nikita's issue so >> to me that's enough for: > > So who disables the HPD now? > > Is the drm_bridge_connector_enable_hpd & > drm_bridge_connector_disable_hpd documentation now wrong as they talk > about suspend/resume handlers? To give more context, currently omapdrm enables the HPDs at init time and disables them at remove time. With this patch, the HPDs are enabled twice, leading to a WARN. imx and msm drivers also seem to call drm_bridge_connector_enable_hpd(), so I would guess those also WARN with this patch. Afaics, this patch alone is broken, as it just disregards the drivers that already enable the HPD, and also as it doesn't handle the disabling of the HPD, or give any guidelines on how the drivers should now manage the HPD. My suggestion is to revert this one. Tomi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2022-09-05 5:26 ` Tomi Valkeinen 0 siblings, 0 replies; 22+ messages in thread From: Tomi Valkeinen @ 2022-09-05 5:26 UTC (permalink / raw) To: Kieran Bingham, Laurent Pinchart, Nikita Yushchenko Cc: Thomas Zimmermann, David Airlie, linux-kernel, Jacopo Mondi, dri-devel On 31/08/2022 16:02, Tomi Valkeinen wrote: > Hi, > > On 23/02/2022 19:02, Kieran Bingham wrote: >> Quoting Laurent Pinchart (2022-02-23 16:25:28) >>> Hello, >>> >>> On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: >>>> Quoting Laurent Pinchart (2021-12-29 23:44:29) >>>>> On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: >>>>>> Hotplug events reported by bridge drivers over >>>>>> drm_bridge_hpd_notify() >>>>>> get ignored unless somebody calls drm_bridge_hpd_enable(). When the >>>>>> connector for the bridge is bridge_connector, such a call is done >>>>>> from >>>>>> drm_bridge_connector_enable_hpd(). >>>>>> >>>>>> However drm_bridge_connector_enable_hpd() is never called on init >>>>>> paths, >>>>>> documentation suggests that it is intended for suspend/resume paths. >>>>> >>>>> Hmmmm... I'm in two minds about this. The problem description is >>>>> correct, but I wonder if HPD should be enabled unconditionally >>>>> here, or >>>>> if this should be left to display drivers to control. >>>>> drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, >>>>> other drivers don't. >>>>> >>>>> It feels like this should be under control of the display controller >>>>> driver, but I can't think of a use case for not enabling HPD at init >>>>> time. Any second opinion from anyone ? >>>> >>>> This patch solves an issue I have where I have recently enabled HPD on >>>> the SN65DSI86, but without this, I do not get calls to my >>>> .hpd_enable or >>>> .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. >>>> >>>> So it needs to be enabled somewhere, and this seems reasonable to me? >>>> It's not directly related to the display controller - as it's a factor >>>> of the bridge? >>>> >>>> On Falcon-V3U with HPD additions to SN65DSI86: >>>> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >>> >>> If you think this is right, then >>> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> I do, and at the very least it works for me, and fixes Nikita's issue so >> to me that's enough for: > > So who disables the HPD now? > > Is the drm_bridge_connector_enable_hpd & > drm_bridge_connector_disable_hpd documentation now wrong as they talk > about suspend/resume handlers? To give more context, currently omapdrm enables the HPDs at init time and disables them at remove time. With this patch, the HPDs are enabled twice, leading to a WARN. imx and msm drivers also seem to call drm_bridge_connector_enable_hpd(), so I would guess those also WARN with this patch. Afaics, this patch alone is broken, as it just disregards the drivers that already enable the HPD, and also as it doesn't handle the disabling of the HPD, or give any guidelines on how the drivers should now manage the HPD. My suggestion is to revert this one. Tomi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2022-09-05 5:26 ` Tomi Valkeinen @ 2023-04-11 18:52 ` Yongqin Liu -1 siblings, 0 replies; 22+ messages in thread From: Yongqin Liu @ 2023-04-11 18:52 UTC (permalink / raw) To: Tomi Valkeinen Cc: Kieran Bingham, Laurent Pinchart, Nikita Yushchenko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Jacopo Mondi, dri-devel, linux-kernel HI, All Just an updated here. As the commits listed the the following link merged into the android-mainline kernel: https://lore.kernel.org/linux-arm-kernel/20221102180705.459294-1-dmitry.baryshkov@linaro.org/T/ The problem caused by this commit with x15 build is gone now. Thanks, Yongqin Liu On Mon, 5 Sept 2022 at 13:26, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > On 31/08/2022 16:02, Tomi Valkeinen wrote: > > Hi, > > > > On 23/02/2022 19:02, Kieran Bingham wrote: > >> Quoting Laurent Pinchart (2022-02-23 16:25:28) > >>> Hello, > >>> > >>> On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: > >>>> Quoting Laurent Pinchart (2021-12-29 23:44:29) > >>>>> On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > >>>>>> Hotplug events reported by bridge drivers over > >>>>>> drm_bridge_hpd_notify() > >>>>>> get ignored unless somebody calls drm_bridge_hpd_enable(). When the > >>>>>> connector for the bridge is bridge_connector, such a call is done > >>>>>> from > >>>>>> drm_bridge_connector_enable_hpd(). > >>>>>> > >>>>>> However drm_bridge_connector_enable_hpd() is never called on init > >>>>>> paths, > >>>>>> documentation suggests that it is intended for suspend/resume paths. > >>>>> > >>>>> Hmmmm... I'm in two minds about this. The problem description is > >>>>> correct, but I wonder if HPD should be enabled unconditionally > >>>>> here, or > >>>>> if this should be left to display drivers to control. > >>>>> drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > >>>>> other drivers don't. > >>>>> > >>>>> It feels like this should be under control of the display controller > >>>>> driver, but I can't think of a use case for not enabling HPD at init > >>>>> time. Any second opinion from anyone ? > >>>> > >>>> This patch solves an issue I have where I have recently enabled HPD on > >>>> the SN65DSI86, but without this, I do not get calls to my > >>>> .hpd_enable or > >>>> .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. > >>>> > >>>> So it needs to be enabled somewhere, and this seems reasonable to me? > >>>> It's not directly related to the display controller - as it's a factor > >>>> of the bridge? > >>>> > >>>> On Falcon-V3U with HPD additions to SN65DSI86: > >>>> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > >>> > >>> If you think this is right, then > >>> > >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> > >> I do, and at the very least it works for me, and fixes Nikita's issue so > >> to me that's enough for: > > > > So who disables the HPD now? > > > > Is the drm_bridge_connector_enable_hpd & > > drm_bridge_connector_disable_hpd documentation now wrong as they talk > > about suspend/resume handlers? > > To give more context, currently omapdrm enables the HPDs at init time > and disables them at remove time. With this patch, the HPDs are enabled > twice, leading to a WARN. > > imx and msm drivers also seem to call drm_bridge_connector_enable_hpd(), > so I would guess those also WARN with this patch. > > Afaics, this patch alone is broken, as it just disregards the drivers > that already enable the HPD, and also as it doesn't handle the disabling > of the HPD, or give any guidelines on how the drivers should now manage > the HPD. > > My suggestion is to revert this one. > > Tomi -- Best Regards, Yongqin Liu --------------------------------------------------------------- #mailing list linaro-android@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2023-04-11 18:52 ` Yongqin Liu 0 siblings, 0 replies; 22+ messages in thread From: Yongqin Liu @ 2023-04-11 18:52 UTC (permalink / raw) To: Tomi Valkeinen Cc: Nikita Yushchenko, David Airlie, dri-devel, linux-kernel, Kieran Bingham, Jacopo Mondi, Laurent Pinchart, Thomas Zimmermann HI, All Just an updated here. As the commits listed the the following link merged into the android-mainline kernel: https://lore.kernel.org/linux-arm-kernel/20221102180705.459294-1-dmitry.baryshkov@linaro.org/T/ The problem caused by this commit with x15 build is gone now. Thanks, Yongqin Liu On Mon, 5 Sept 2022 at 13:26, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > On 31/08/2022 16:02, Tomi Valkeinen wrote: > > Hi, > > > > On 23/02/2022 19:02, Kieran Bingham wrote: > >> Quoting Laurent Pinchart (2022-02-23 16:25:28) > >>> Hello, > >>> > >>> On Wed, Feb 23, 2022 at 04:17:22PM +0000, Kieran Bingham wrote: > >>>> Quoting Laurent Pinchart (2021-12-29 23:44:29) > >>>>> On Sat, Dec 25, 2021 at 09:31:51AM +0300, Nikita Yushchenko wrote: > >>>>>> Hotplug events reported by bridge drivers over > >>>>>> drm_bridge_hpd_notify() > >>>>>> get ignored unless somebody calls drm_bridge_hpd_enable(). When the > >>>>>> connector for the bridge is bridge_connector, such a call is done > >>>>>> from > >>>>>> drm_bridge_connector_enable_hpd(). > >>>>>> > >>>>>> However drm_bridge_connector_enable_hpd() is never called on init > >>>>>> paths, > >>>>>> documentation suggests that it is intended for suspend/resume paths. > >>>>> > >>>>> Hmmmm... I'm in two minds about this. The problem description is > >>>>> correct, but I wonder if HPD should be enabled unconditionally > >>>>> here, or > >>>>> if this should be left to display drivers to control. > >>>>> drivers/gpu/drm/imx/dcss/dcss-kms.c enables HPD manually at init time, > >>>>> other drivers don't. > >>>>> > >>>>> It feels like this should be under control of the display controller > >>>>> driver, but I can't think of a use case for not enabling HPD at init > >>>>> time. Any second opinion from anyone ? > >>>> > >>>> This patch solves an issue I have where I have recently enabled HPD on > >>>> the SN65DSI86, but without this, I do not get calls to my > >>>> .hpd_enable or > >>>> .hpd_disable hooks that I have added to the ti_sn_bridge_funcs. > >>>> > >>>> So it needs to be enabled somewhere, and this seems reasonable to me? > >>>> It's not directly related to the display controller - as it's a factor > >>>> of the bridge? > >>>> > >>>> On Falcon-V3U with HPD additions to SN65DSI86: > >>>> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > >>> > >>> If you think this is right, then > >>> > >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> > >> I do, and at the very least it works for me, and fixes Nikita's issue so > >> to me that's enough for: > > > > So who disables the HPD now? > > > > Is the drm_bridge_connector_enable_hpd & > > drm_bridge_connector_disable_hpd documentation now wrong as they talk > > about suspend/resume handlers? > > To give more context, currently omapdrm enables the HPDs at init time > and disables them at remove time. With this patch, the HPDs are enabled > twice, leading to a WARN. > > imx and msm drivers also seem to call drm_bridge_connector_enable_hpd(), > so I would guess those also WARN with this patch. > > Afaics, this patch alone is broken, as it just disregards the drivers > that already enable the HPD, and also as it doesn't handle the disabling > of the HPD, or give any guidelines on how the drivers should now manage > the HPD. > > My suggestion is to revert this one. > > Tomi -- Best Regards, Yongqin Liu --------------------------------------------------------------- #mailing list linaro-android@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2021-12-25 6:31 ` Nikita Yushchenko @ 2022-03-04 19:38 ` Paul Cercueil -1 siblings, 0 replies; 22+ messages in thread From: Paul Cercueil @ 2022-03-04 19:38 UTC (permalink / raw) To: Nikita Yushchenko Cc: Laurent Pinchart, Thomas Zimmermann, David Airlie, linux-kernel, Kieran Bingham, Jacopo Mondi, dri-devel Hi Nikita, Le sam., déc. 25 2021 at 09:31:51 +0300, Nikita Yushchenko <nikita.yoush@cogentembedded.com> a écrit : > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > connector for the bridge is bridge_connector, such a call is done from > drm_bridge_connector_enable_hpd(). > > However drm_bridge_connector_enable_hpd() is never called on init > paths, > documentation suggests that it is intended for suspend/resume paths. > > In result, once encoders are switched to bridge_connector, > bridge-detected HPD stops working. > > This patch adds a call to that API on init path. > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports > HPD > events via interrupts. > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() > helper") > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> Merged to drm-misc-next. Thanks! Cheers, -Paul > --- > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c > b/drivers/gpu/drm/drm_bridge_connector.c > index 791379816837..4f20137ef21d 100644 > --- a/drivers/gpu/drm/drm_bridge_connector.c > +++ b/drivers/gpu/drm/drm_bridge_connector.c > @@ -369,8 +369,10 @@ struct drm_connector > *drm_bridge_connector_init(struct drm_device *drm, > connector_type, ddc); > drm_connector_helper_add(connector, > &drm_bridge_connector_helper_funcs); > > - if (bridge_connector->bridge_hpd) > + if (bridge_connector->bridge_hpd) { > connector->polled = DRM_CONNECTOR_POLL_HPD; > + drm_bridge_connector_enable_hpd(connector); > + } > else if (bridge_connector->bridge_detect) > connector->polled = DRM_CONNECTOR_POLL_CONNECT > | DRM_CONNECTOR_POLL_DISCONNECT; > -- > 2.30.2 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2022-03-04 19:38 ` Paul Cercueil 0 siblings, 0 replies; 22+ messages in thread From: Paul Cercueil @ 2022-03-04 19:38 UTC (permalink / raw) To: Nikita Yushchenko Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Kieran Bingham, Laurent Pinchart, Jacopo Mondi, dri-devel, linux-kernel Hi Nikita, Le sam., déc. 25 2021 at 09:31:51 +0300, Nikita Yushchenko <nikita.yoush@cogentembedded.com> a écrit : > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > connector for the bridge is bridge_connector, such a call is done from > drm_bridge_connector_enable_hpd(). > > However drm_bridge_connector_enable_hpd() is never called on init > paths, > documentation suggests that it is intended for suspend/resume paths. > > In result, once encoders are switched to bridge_connector, > bridge-detected HPD stops working. > > This patch adds a call to that API on init path. > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports > HPD > events via interrupts. > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() > helper") > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> Merged to drm-misc-next. Thanks! Cheers, -Paul > --- > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c > b/drivers/gpu/drm/drm_bridge_connector.c > index 791379816837..4f20137ef21d 100644 > --- a/drivers/gpu/drm/drm_bridge_connector.c > +++ b/drivers/gpu/drm/drm_bridge_connector.c > @@ -369,8 +369,10 @@ struct drm_connector > *drm_bridge_connector_init(struct drm_device *drm, > connector_type, ddc); > drm_connector_helper_add(connector, > &drm_bridge_connector_helper_funcs); > > - if (bridge_connector->bridge_hpd) > + if (bridge_connector->bridge_hpd) { > connector->polled = DRM_CONNECTOR_POLL_HPD; > + drm_bridge_connector_enable_hpd(connector); > + } > else if (bridge_connector->bridge_detect) > connector->polled = DRM_CONNECTOR_POLL_CONNECT > | DRM_CONNECTOR_POLL_DISCONNECT; > -- > 2.30.2 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported 2022-03-04 19:38 ` Paul Cercueil @ 2022-08-12 5:35 ` Yongqin Liu -1 siblings, 0 replies; 22+ messages in thread From: Yongqin Liu @ 2022-08-12 5:35 UTC (permalink / raw) To: Paul Cercueil Cc: Nikita Yushchenko, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Kieran Bingham, Laurent Pinchart, Jacopo Mondi, dri-devel, linux-kernel, Bajjuri, Praneeth, Sumit Semwal Hi, Nikita, All We have one X15 Android build based on the android mainline kernel, and it failed to boot with this change because of one kernel panic. And it would boot to the home screen if this change is reverted. Could you please help have a look and give suggestions on what should be done next to work with this change? The kernel panic is something like the following, for details please check the link here: http://ix.io/47mc [ 38.784301] Internal error: : 1211 [#2] PREEMPT SMP ARM [ 38.784301] Modules linked in: [ 38.788940] Kernel panic - not syncing: Fatal exception in interrupt [ 38.794189] snd_soc_omap_hdmi ti_tpd12s015 pvrsrvkm(O) display_connector omap_wdt omap_aes_driver ti_vpe ti_vpdma ti_csc ti_sc v4l2_mem2mem videobuf2_dma_contig omap_sham rtc_omap snd_soc_tlv320aic3x_i2c snd_soc_tlv320aic3x at24 rtc_palmas omap_des omap_crypto libdes crypto_engine rtc_ds1307 omap_hdq wire phy_gmii_sel ahci_platform libahci_platform libahci libata scsi_mod bsg scsi_common snd_soc_simple_card snd_soc_simple_card_utils [ 38.842315] CPU: 1 PID: 454 Comm: vsync Tainted: G D W O 5.19.0-02213-g3aeacdb764a2 #1 [ 38.851226] Hardware name: Generic DRA74X (Flattened Device Tree) [ 38.857360] PC is at dispc_write_irqenable+0x1c/0x38 [ 38.862335] LR is at omap_irq_enable_vblank+0xbc/0xd8 [ 38.867431] pc : [<c0948e04>] lr : [<c0939e9c>] psr: 60080093 [ 38.873718] sp : ea281d40 ip : 00000000 fp : c3ae3008 [ 38.878967] r10: 00000000 r9 : c3a89cb8 r8 : 60080093 [ 38.884216] r7 : c398f000 r6 : 0812d64c r5 : c398f000 r4 : c398f184 [ 38.890777] r3 : e6137018 r2 : 0812d64c r1 : 0812d64c r0 : c3846000 [ 38.897338] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user [ 38.904571] Control: 30c5387d Table: 84e801c0 DAC: 55555555 [ 38.910339] Register r0 information: slab kmalloc-8k start c3846000 pointer offset 0 size 8192 [ 38.919006] Register r1 information: non-paged memory [ 38.924102] Register r2 information: non-paged memory [ 38.929168] Register r3 information: 0-page vmalloc region starting at 0xe6137000 allocated at __devm_ioremap_resource+0x108/0x1f0 [ 38.940979] Register r4 information: slab kmalloc-512 start c398f000 pointer offset 388 size 512 [ 38.949829] Register r5 information: slab kmalloc-512 start c398f000 pointer offset 0 size 512 [ 38.958496] Register r6 information: non-paged memory [ 38.963562] Register r7 information: slab kmalloc-512 start c398f000 pointer offset 0 size 512 [ 38.972229] Register r8 information: non-paged memory [ 38.977294] Register r9 information: slab kmalloc-1k start c3a89c00 pointer offset 184 size 1024 [ 38.986145] Register r10 information: NULL pointer [ 38.990966] Register r11 information: slab kmalloc-2k start c3ae3000 pointer offset 8 size 2048 [ 38.999725] Register r12 information: NULL pointer [ 39.004547] Process vsync (pid: 454, stack limit = 0xd381b509) [ 39.010406] Stack: (0xea281d40 to 0xea282000) [ 39.014770] 1d40: 00000000 00010000 c398fb0c 00000000 c3a89c00 00000000 c398fa40 c0913a98 [ 39.022979] 1d60: c191b239 c5f7ea80 00000000 00010000 c398fa40 c398fa80 c3a89c00 00000000 [ 39.031219] 1d80: 20080013 c3a89cbc 00000000 c091397c c5f7e800 c39d0280 00000001 c557ff00 [ 39.039428] 1da0: c398fa40 ea281e68 c5f8a300 00000001 ffffffea c3a89c00 c3ae3008 c09150f0 [ 39.047637] 1dc0: 00000000 00000000 00000000 00000000 01140657 ea281e68 ffffffff 00000001 [ 39.055847] 1de0: 00000000 c39d0000 00000000 00000000 693113f4 00000010 fffffff3 c5f8a300 [ 39.064086] 1e00: c3a89c00 c0914f88 ea281e68 00000010 c5f8a300 c08ef2b0 00000000 693113f4 [ 39.072296] 1e20: b44fb158 ea281e68 00000010 c0914f88 ea281e68 c5f21840 c5f8a300 c08ef53c [ 39.080505] 1e40: 0000e200 00000001 c1364999 00000000 00000010 b44fb158 c010643a 0000003a [ 39.088745] 1e60: c10730ac 00000000 00000001 00000001 00000000 00000000 00000000 00000000 [ 39.096954] 1e80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 39.105163] 1ea0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 39.113372] 1ec0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 39.121612] 1ee0: 00000000 00000000 693113f4 c5f21840 c3c00a20 ea281f60 0000000c b44fb158 [ 39.129821] 1f00: c5f21841 00000036 c010643a c048c1c4 ea281f48 c0f24c4c c02fa044 00000000 [ 39.138031] 1f20: 00000000 00000000 693113f4 c5f7e800 60080013 ffffffff ea281f94 c0200b9c [ 39.146240] 1f40: c5f7e800 30c5387d ea281f58 c0f25138 c02001f0 30c5387d 00000000 c0200bb4 [ 39.154449] 1f60: 0000000c c010643a b44fb158 b44fb124 b4647348 b44fb158 0000000c 00000036 [ 39.162689] 1f80: 693113f4 b4647348 b44fb158 0000000c 00000036 c0200298 c5f7e800 00000036 [ 39.170898] 1fa0: 00000000 c0200060 b4647348 b44fb158 0000000c c010643a b44fb158 b44fb124 [ 39.179107] 1fc0: b4647348 b44fb158 0000000c 00000036 b44fb128 c010643a beb2aaa8 00000000 [ 39.187316] 1fe0: 00000078 b44fb110 b5f8301b b5fb7ce4 80080010 0000000c 00000000 00000000 [ 39.195556] dispc_write_irqenable from omap_irq_enable_vblank+0xbc/0xd8 [ 39.202301] omap_irq_enable_vblank from drm_vblank_enable+0x8c/0x184 [ 39.208770] drm_vblank_enable from drm_vblank_get+0x7c/0x10c [ 39.214538] drm_vblank_get from drm_wait_vblank_ioctl+0x168/0x4b8 [ 39.220764] drm_wait_vblank_ioctl from drm_ioctl_kernel+0xe8/0x154 [ 39.227050] drm_ioctl_kernel from drm_ioctl+0x220/0x36c [ 39.232391] drm_ioctl from sys_ioctl+0xbe4/0xc54 [ 39.237121] sys_ioctl from ret_fast_syscall+0x0/0x4c [ 39.242218] Exception stack(0xea281fa8 to 0xea281ff0) [ 39.247283] 1fa0: b4647348 b44fb158 0000000c c010643a b44fb158 b44fb124 [ 39.255493] 1fc0: b4647348 b44fb158 0000000c 00000036 b44fb128 c010643a beb2aaa8 00000000 [ 39.263702] 1fe0: 00000078 b44fb110 b5f8301b b5fb7ce4 [ 39.268798] Code: e5903004 e1c12002 e2833018 e5832000 (e5902004) [ 39.274902] ---[ end trace 0000000000000000 ]--- [ 39.279541] Rebooting in 5 seconds.. Thanks, Yongqin Liu On Sat, 5 Mar 2022 at 04:05, Paul Cercueil <paul@crapouillou.net> wrote: > > Hi Nikita, > > Le sam., déc. 25 2021 at 09:31:51 +0300, Nikita Yushchenko > <nikita.yoush@cogentembedded.com> a écrit : > > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > > connector for the bridge is bridge_connector, such a call is done from > > drm_bridge_connector_enable_hpd(). > > > > However drm_bridge_connector_enable_hpd() is never called on init > > paths, > > documentation suggests that it is intended for suspend/resume paths. > > > > In result, once encoders are switched to bridge_connector, > > bridge-detected HPD stops working. > > > > This patch adds a call to that API on init path. > > > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports > > HPD > > events via interrupts. > > > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() > > helper") > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > > Merged to drm-misc-next. > > Thanks! > > Cheers, > -Paul > > > --- > > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c > > b/drivers/gpu/drm/drm_bridge_connector.c > > index 791379816837..4f20137ef21d 100644 > > --- a/drivers/gpu/drm/drm_bridge_connector.c > > +++ b/drivers/gpu/drm/drm_bridge_connector.c > > @@ -369,8 +369,10 @@ struct drm_connector > > *drm_bridge_connector_init(struct drm_device *drm, > > connector_type, ddc); > > drm_connector_helper_add(connector, > > &drm_bridge_connector_helper_funcs); > > > > - if (bridge_connector->bridge_hpd) > > + if (bridge_connector->bridge_hpd) { > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > + drm_bridge_connector_enable_hpd(connector); > > + } > > else if (bridge_connector->bridge_detect) > > connector->polled = DRM_CONNECTOR_POLL_CONNECT > > | DRM_CONNECTOR_POLL_DISCONNECT; > > -- > > 2.30.2 > > -- Best Regards, Yongqin Liu --------------------------------------------------------------- #mailing list linaro-android@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/bridge_connector: enable HPD by default if supported @ 2022-08-12 5:35 ` Yongqin Liu 0 siblings, 0 replies; 22+ messages in thread From: Yongqin Liu @ 2022-08-12 5:35 UTC (permalink / raw) To: Paul Cercueil Cc: Nikita Yushchenko, Laurent Pinchart, Thomas Zimmermann, David Airlie, linux-kernel, Bajjuri, Praneeth, Kieran Bingham, Jacopo Mondi, dri-devel, Sumit Semwal Hi, Nikita, All We have one X15 Android build based on the android mainline kernel, and it failed to boot with this change because of one kernel panic. And it would boot to the home screen if this change is reverted. Could you please help have a look and give suggestions on what should be done next to work with this change? The kernel panic is something like the following, for details please check the link here: http://ix.io/47mc [ 38.784301] Internal error: : 1211 [#2] PREEMPT SMP ARM [ 38.784301] Modules linked in: [ 38.788940] Kernel panic - not syncing: Fatal exception in interrupt [ 38.794189] snd_soc_omap_hdmi ti_tpd12s015 pvrsrvkm(O) display_connector omap_wdt omap_aes_driver ti_vpe ti_vpdma ti_csc ti_sc v4l2_mem2mem videobuf2_dma_contig omap_sham rtc_omap snd_soc_tlv320aic3x_i2c snd_soc_tlv320aic3x at24 rtc_palmas omap_des omap_crypto libdes crypto_engine rtc_ds1307 omap_hdq wire phy_gmii_sel ahci_platform libahci_platform libahci libata scsi_mod bsg scsi_common snd_soc_simple_card snd_soc_simple_card_utils [ 38.842315] CPU: 1 PID: 454 Comm: vsync Tainted: G D W O 5.19.0-02213-g3aeacdb764a2 #1 [ 38.851226] Hardware name: Generic DRA74X (Flattened Device Tree) [ 38.857360] PC is at dispc_write_irqenable+0x1c/0x38 [ 38.862335] LR is at omap_irq_enable_vblank+0xbc/0xd8 [ 38.867431] pc : [<c0948e04>] lr : [<c0939e9c>] psr: 60080093 [ 38.873718] sp : ea281d40 ip : 00000000 fp : c3ae3008 [ 38.878967] r10: 00000000 r9 : c3a89cb8 r8 : 60080093 [ 38.884216] r7 : c398f000 r6 : 0812d64c r5 : c398f000 r4 : c398f184 [ 38.890777] r3 : e6137018 r2 : 0812d64c r1 : 0812d64c r0 : c3846000 [ 38.897338] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user [ 38.904571] Control: 30c5387d Table: 84e801c0 DAC: 55555555 [ 38.910339] Register r0 information: slab kmalloc-8k start c3846000 pointer offset 0 size 8192 [ 38.919006] Register r1 information: non-paged memory [ 38.924102] Register r2 information: non-paged memory [ 38.929168] Register r3 information: 0-page vmalloc region starting at 0xe6137000 allocated at __devm_ioremap_resource+0x108/0x1f0 [ 38.940979] Register r4 information: slab kmalloc-512 start c398f000 pointer offset 388 size 512 [ 38.949829] Register r5 information: slab kmalloc-512 start c398f000 pointer offset 0 size 512 [ 38.958496] Register r6 information: non-paged memory [ 38.963562] Register r7 information: slab kmalloc-512 start c398f000 pointer offset 0 size 512 [ 38.972229] Register r8 information: non-paged memory [ 38.977294] Register r9 information: slab kmalloc-1k start c3a89c00 pointer offset 184 size 1024 [ 38.986145] Register r10 information: NULL pointer [ 38.990966] Register r11 information: slab kmalloc-2k start c3ae3000 pointer offset 8 size 2048 [ 38.999725] Register r12 information: NULL pointer [ 39.004547] Process vsync (pid: 454, stack limit = 0xd381b509) [ 39.010406] Stack: (0xea281d40 to 0xea282000) [ 39.014770] 1d40: 00000000 00010000 c398fb0c 00000000 c3a89c00 00000000 c398fa40 c0913a98 [ 39.022979] 1d60: c191b239 c5f7ea80 00000000 00010000 c398fa40 c398fa80 c3a89c00 00000000 [ 39.031219] 1d80: 20080013 c3a89cbc 00000000 c091397c c5f7e800 c39d0280 00000001 c557ff00 [ 39.039428] 1da0: c398fa40 ea281e68 c5f8a300 00000001 ffffffea c3a89c00 c3ae3008 c09150f0 [ 39.047637] 1dc0: 00000000 00000000 00000000 00000000 01140657 ea281e68 ffffffff 00000001 [ 39.055847] 1de0: 00000000 c39d0000 00000000 00000000 693113f4 00000010 fffffff3 c5f8a300 [ 39.064086] 1e00: c3a89c00 c0914f88 ea281e68 00000010 c5f8a300 c08ef2b0 00000000 693113f4 [ 39.072296] 1e20: b44fb158 ea281e68 00000010 c0914f88 ea281e68 c5f21840 c5f8a300 c08ef53c [ 39.080505] 1e40: 0000e200 00000001 c1364999 00000000 00000010 b44fb158 c010643a 0000003a [ 39.088745] 1e60: c10730ac 00000000 00000001 00000001 00000000 00000000 00000000 00000000 [ 39.096954] 1e80: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 39.105163] 1ea0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 39.113372] 1ec0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 39.121612] 1ee0: 00000000 00000000 693113f4 c5f21840 c3c00a20 ea281f60 0000000c b44fb158 [ 39.129821] 1f00: c5f21841 00000036 c010643a c048c1c4 ea281f48 c0f24c4c c02fa044 00000000 [ 39.138031] 1f20: 00000000 00000000 693113f4 c5f7e800 60080013 ffffffff ea281f94 c0200b9c [ 39.146240] 1f40: c5f7e800 30c5387d ea281f58 c0f25138 c02001f0 30c5387d 00000000 c0200bb4 [ 39.154449] 1f60: 0000000c c010643a b44fb158 b44fb124 b4647348 b44fb158 0000000c 00000036 [ 39.162689] 1f80: 693113f4 b4647348 b44fb158 0000000c 00000036 c0200298 c5f7e800 00000036 [ 39.170898] 1fa0: 00000000 c0200060 b4647348 b44fb158 0000000c c010643a b44fb158 b44fb124 [ 39.179107] 1fc0: b4647348 b44fb158 0000000c 00000036 b44fb128 c010643a beb2aaa8 00000000 [ 39.187316] 1fe0: 00000078 b44fb110 b5f8301b b5fb7ce4 80080010 0000000c 00000000 00000000 [ 39.195556] dispc_write_irqenable from omap_irq_enable_vblank+0xbc/0xd8 [ 39.202301] omap_irq_enable_vblank from drm_vblank_enable+0x8c/0x184 [ 39.208770] drm_vblank_enable from drm_vblank_get+0x7c/0x10c [ 39.214538] drm_vblank_get from drm_wait_vblank_ioctl+0x168/0x4b8 [ 39.220764] drm_wait_vblank_ioctl from drm_ioctl_kernel+0xe8/0x154 [ 39.227050] drm_ioctl_kernel from drm_ioctl+0x220/0x36c [ 39.232391] drm_ioctl from sys_ioctl+0xbe4/0xc54 [ 39.237121] sys_ioctl from ret_fast_syscall+0x0/0x4c [ 39.242218] Exception stack(0xea281fa8 to 0xea281ff0) [ 39.247283] 1fa0: b4647348 b44fb158 0000000c c010643a b44fb158 b44fb124 [ 39.255493] 1fc0: b4647348 b44fb158 0000000c 00000036 b44fb128 c010643a beb2aaa8 00000000 [ 39.263702] 1fe0: 00000078 b44fb110 b5f8301b b5fb7ce4 [ 39.268798] Code: e5903004 e1c12002 e2833018 e5832000 (e5902004) [ 39.274902] ---[ end trace 0000000000000000 ]--- [ 39.279541] Rebooting in 5 seconds.. Thanks, Yongqin Liu On Sat, 5 Mar 2022 at 04:05, Paul Cercueil <paul@crapouillou.net> wrote: > > Hi Nikita, > > Le sam., déc. 25 2021 at 09:31:51 +0300, Nikita Yushchenko > <nikita.yoush@cogentembedded.com> a écrit : > > Hotplug events reported by bridge drivers over drm_bridge_hpd_notify() > > get ignored unless somebody calls drm_bridge_hpd_enable(). When the > > connector for the bridge is bridge_connector, such a call is done from > > drm_bridge_connector_enable_hpd(). > > > > However drm_bridge_connector_enable_hpd() is never called on init > > paths, > > documentation suggests that it is intended for suspend/resume paths. > > > > In result, once encoders are switched to bridge_connector, > > bridge-detected HPD stops working. > > > > This patch adds a call to that API on init path. > > > > This fixes HDMI HPD with rcar-du + adv7513 case when adv7513 reports > > HPD > > events via interrupts. > > > > Fixes: c24110a8fd09 ("drm: rcar-du: Use drm_bridge_connector_init() > > helper") > > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > > Merged to drm-misc-next. > > Thanks! > > Cheers, > -Paul > > > --- > > drivers/gpu/drm/drm_bridge_connector.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_bridge_connector.c > > b/drivers/gpu/drm/drm_bridge_connector.c > > index 791379816837..4f20137ef21d 100644 > > --- a/drivers/gpu/drm/drm_bridge_connector.c > > +++ b/drivers/gpu/drm/drm_bridge_connector.c > > @@ -369,8 +369,10 @@ struct drm_connector > > *drm_bridge_connector_init(struct drm_device *drm, > > connector_type, ddc); > > drm_connector_helper_add(connector, > > &drm_bridge_connector_helper_funcs); > > > > - if (bridge_connector->bridge_hpd) > > + if (bridge_connector->bridge_hpd) { > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > + drm_bridge_connector_enable_hpd(connector); > > + } > > else if (bridge_connector->bridge_detect) > > connector->polled = DRM_CONNECTOR_POLL_CONNECT > > | DRM_CONNECTOR_POLL_DISCONNECT; > > -- > > 2.30.2 > > -- Best Regards, Yongqin Liu --------------------------------------------------------------- #mailing list linaro-android@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-04-11 18:53 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-25 6:31 [PATCH] drm/bridge_connector: enable HPD by default if supported Nikita Yushchenko 2021-12-25 6:31 ` Nikita Yushchenko 2021-12-29 23:44 ` Laurent Pinchart 2021-12-29 23:44 ` Laurent Pinchart 2022-01-31 16:38 ` Nikita Yushchenko 2022-01-31 16:38 ` Nikita Yushchenko 2022-02-23 16:17 ` Kieran Bingham 2022-02-23 16:17 ` Kieran Bingham 2022-02-23 16:25 ` Laurent Pinchart 2022-02-23 16:25 ` Laurent Pinchart 2022-02-23 17:02 ` Kieran Bingham 2022-02-23 17:02 ` Kieran Bingham 2022-08-31 13:02 ` Tomi Valkeinen 2022-08-31 13:02 ` Tomi Valkeinen 2022-09-05 5:26 ` Tomi Valkeinen 2022-09-05 5:26 ` Tomi Valkeinen 2023-04-11 18:52 ` Yongqin Liu 2023-04-11 18:52 ` Yongqin Liu 2022-03-04 19:38 ` Paul Cercueil 2022-03-04 19:38 ` Paul Cercueil 2022-08-12 5:35 ` Yongqin Liu 2022-08-12 5:35 ` Yongqin Liu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.