[Intel-gfx] [PATCH] drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state

[Jani Nikula]

On Mon, 16 Apr 2018, Clint Taylor <clinton.a.taylor at intel.com> 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 at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Signed-off-by: Imre Deak <imre.deak at intel.com>
>> ---
>>   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 at intel.com>
> Tested-by: Clint Taylor <Clinton.A.Taylor at intel.com>

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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center