[Intel-gfx] [PATCH] drm/i915: Restrict usage of live status check

[Sharma, Shashank]

Thanks Ville,

Regards
Shashank

On 3/10/2016 5:10 PM, Ville Syrjälä wrote:
> On Thu, Mar 10, 2016 at 05:05:47PM +0530, Shashank Sharma wrote:
>> This patch does the following:
>> - Restricts usage of live status check for HDMI detection.
>>    While testing certain (monitor + cable) combinations with
>>    various intel  platforms, it seems that live status register
>>    doesn't work reliably on some older devices. So limit the
>>    live_status check for HDMI detection, only for platforms
>>    from gen7 onwards.
>
> Not good enough. We have regressions reported on IVB at least.
> Also I'm not convinced more recent platforms are any necessarily
> better here.
Thanks for pointing out the miss for IVB. I will add skip for that.
We have tested VLV, CHT SKL and BXT with this code, so we should be good 
with those. Anything particular in your mind ?
>
>> - Cleans up the retry logic.
>>    There is an extra 'if (try)' check, which can be avoided by
>>    changing the logic slightly, keeping the delay as same(80ms)
>
> I wouldn't call a goto loop clean.
>
I know, its flushing my pipeline, and its not the best thing to do. But 
makes code more readable, and removes one (if) condition, which was 
getting executed for every run of the loop. So I guess still some CPU 
cycles saved in the end :)
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_hdmi.c | 23 +++++++++++++++--------
>>   1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index e2dab48..3f6db91 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1397,24 +1397,31 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>>   	enum drm_connector_status status;
>>   	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>>   	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>> -	bool live_status = false;
>> -	unsigned int try;
>> +	bool live_status = true;
>> +	unsigned int try = 9;
>>
>>   	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>>   		      connector->base.id, connector->name);
>>
>>   	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
>>
>> -	for (try = 0; !live_status && try < 9; try++) {
>> -		if (try)
>> -			msleep(10);
>> +	/*
>> +	 * When tested with specific set of monitors + cables,
>> +	 * live status behavior is not very consistent for older
>> +	 * platforms. So add live status check from GEN7
>> +	 * onwards.
>> +	 */
>> +	if (INTEL_INFO(dev_priv->dev)->gen >= 7) {
>> +retry_ls:
>>   		live_status = intel_digital_port_connected(dev_priv,
>>   				hdmi_to_dig_port(intel_hdmi));
>> +		if (!live_status && --try) {
>> +			msleep(10);
>> +			goto retry_ls;
>> +		}
>> +		DRM_DEBUG_KMS("Live status %s!\n", live_status ? "up" : "down");
>>   	}
>>
>> -	if (!live_status)
>> -		DRM_DEBUG_KMS("Live status not up!");
>> -
>>   	intel_hdmi_unset_edid(connector);
>>
>>   	if (intel_hdmi_set_edid(connector, live_status)) {
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>