On Wed, Jul 4, 2018 at 9:23 AM, Archit Taneja architt@codeaurora.org wrote:
On Wednesday 04 July 2018 12:28 AM, Rob Clark wrote:
On Tue, Jul 3, 2018 at 12:56 PM, Sean Paul seanpaul@chromium.org 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@gmail.com Signed-off-by: Sean Paul seanpaul@chromium.org
Tested-by: Rob Clark robdclark@gmail.com
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).
jfwiw, fbcon and things using legacy SETCRTC end up triggering a full modeset, which somehow papers over the issue (because we go thru the bridge disable/enable sequence). So now there probably is a double power cycle sequence, although it doesn't seem to be causing any harm (fbcon and x11 still seem happy)
BR, -R
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@codeaurora.org
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 aplug
* event and we're already supposed to be powered, cycle thebridge 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html