On Wednesday 04 July 2018 12:28 AM, Rob Clark wrote:
On Tue, Jul 3, 2018 at 12:56 PM, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
The bridge loses its hw state when the cable is unplugged. If we detect
this case in the hpd handler, reset its state.
Reported-by: Rob Clark <robdclark@xxxxxxxxx>
Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
Tested-by: Rob Clark <robdclark@xxxxxxxxx>
---
This is the follow-up to "drm/msm: Disable mdp5 crtc when there are no
active planes". I incorrectly assumed the modeset was required from the
msm side, but Rob pointed out that he thought it might be a bridge
issue.
To elaborate, this is an atomic userspace (weston), when it sees the
display disconnected, it removes the planes from the CRTC but does not
disable the CRTC. So drm/msm sets up the LM to scanout a solid color,
and leaves the encoder and dsi cranking along happily. When
replugging the display, the atomic helpers see the CRTC is still
active and (correctly) doesn't do a full modeset. So the bridge is
not re-configured/re-enabled.
The adv7511 connector's detect() op should have re-configured the
registers. I'm assuming it was never called after the cable is
plugged in again in the wetson usecase?
With this patch, in the case of fb emulation/X, I'm wondering if
we will end up trying to re-enable ADV7511 twice. First, in the new code
in adv7511_hpd_work(), and later in adv7511_detect() (i.e, the
connector's detect op).
I'm guessing the 'hpd' variable in the check in adv7511_detect() below
should ideally be false, and avoid us trying to configure the registers
again, but I'm not entirely sure.
if (status == connector_status_connected && hpd && adv7511->powered) {
regcache_mark_dirty(adv7511->regmap);
...
In any case:
Reviewed-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
Arguably this perhaps isn't what weston *wanted* to do, but in the end
the bug is in the bridge.
We could possibly set the connector link_status to BAD as an
alternative. But at least the build of weston I'm using atm doesn't
handle the link_status propery, this approach requires each atomic
userspace to handle this case. Compared to Sean's approach which is
transparent to userspace, which seems attractive.
BR,
-R
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 73021b388e12..dd3ff2f2cdce 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -429,6 +429,18 @@ static void adv7511_hpd_work(struct work_struct *work)
else
status = connector_status_disconnected;
+ /*
+ * The bridge resets its registers on unplug. So when we get a plug
+ * event and we're already supposed to be powered, cycle the bridge to
+ * restore its state.
+ */
+ if (status == connector_status_connected &&
+ adv7511->connector.status == connector_status_disconnected &&
+ adv7511->powered) {
+ regcache_mark_dirty(adv7511->regmap);
+ adv7511_power_on(adv7511);
+ }
+
if (adv7511->connector.status != status) {
adv7511->connector.status = status;
if (status == connector_status_disconnected)
--
Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
