On Mon, 16 Apr 2018, Clint Taylor <clinton.a.taylor@xxxxxxxxx> wrote: > On 04/16/2018 08:53 AM, Imre Deak wrote: >> LSPCON adapters in low-power state may ignore the first I2C write during >> TMDS output buffer enabling, resulting in a blank screen even with an >> otherwise enabled pipe. Fix this by reading back and validating the >> written value a few times. >> >> The problem was noticed on GLK machines with an onboard LSPCON adapter >> after entering/exiting DC5 power state. Doing an I2C read of the adapter >> ID as the first transaction - instead of the I2C write to enable the >> TMDS buffers - returns the correct value. Based on this we assume that >> the transaction itself is sent properly, it's only the adapter that is >> not ready for some reason to accept this first write after waking from >> low-power state. In my case the second I2C write attempt always >> succeeded. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105854 >> Cc: Clinton Taylor <clinton.a.taylor@xxxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> >> --- >> drivers/gpu/drm/drm_dp_dual_mode_helper.c | 39 +++++++++++++++++++++++++------ >> 1 file changed, 32 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c >> index 02a50929af67..e7f4fe2848a5 100644 >> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c >> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c >> @@ -350,19 +350,44 @@ int drm_dp_dual_mode_set_tmds_output(enum drm_dp_dual_mode_type type, >> { >> uint8_t tmds_oen = enable ? 0 : DP_DUAL_MODE_TMDS_DISABLE; >> ssize_t ret; >> + int retry; >> >> if (type < DRM_DP_DUAL_MODE_TYPE2_DVI) >> return 0; >> >> - ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN, >> - &tmds_oen, sizeof(tmds_oen)); >> - if (ret) { >> - DRM_DEBUG_KMS("Failed to %s TMDS output buffers\n", >> - enable ? "enable" : "disable"); >> - return ret; >> + /* >> + * LSPCON adapters in low-power state may ignore the first write, so >> + * read back and verify the written value a few times. >> + */ >> + for (retry = 0; retry < 3; retry++) { >> + uint8_t tmp; >> + >> + ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN, >> + &tmds_oen, sizeof(tmds_oen)); >> + if (ret) { >> + DRM_DEBUG_KMS("Failed to %s TMDS output buffers (%d attempts)\n", >> + enable ? "enable" : "disable", >> + retry + 1); >> + return ret; >> + } >> + >> + ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_TMDS_OEN, >> + &tmp, sizeof(tmp)); >> + if (ret) { >> + DRM_DEBUG_KMS("I2C read failed during TMDS output buffer %s (%d attempts)\n", >> + enable ? "enabling" : "disabling", >> + retry + 1); >> + return ret; >> + } >> + >> + if (tmp == tmds_oen) >> + return 0; >> } >> >> - return 0; >> + DRM_DEBUG_KMS("I2C write value mismatch during TMDS output buffer %s\n", >> + enable ? "enabling" : "disabling"); >> + >> + return -EIO; >> } >> EXPORT_SYMBOL(drm_dp_dual_mode_set_tmds_output); >> > > Appears to fix the issue seen on GLK with LSPCON and DMC firmware > loaded. Customer was concerned about the fix being in DRM instead of > i915. However, there are no other SOCs that use this DRM function. > > Reviewed-by: Clint Taylor <Clinton.A.Taylor@xxxxxxxxx> > Tested-by: Clint Taylor <Clinton.A.Taylor@xxxxxxxxx> Pushed to drm-misc-fixes, thanks for the patch. Clint and Ville, many thanks for your testing and review, alas I screwed up and pushed this without the tags added to the commit. I'm sorry. I expect better from myself. :( BR, Jani. > > -Clint > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
