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@xxxxxxxxxxxxxxxx>
If you think this is right, then
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
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
