[Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect and get_modes

[Sharma, Shashank]

Hi Daniel / Quanxian / All, 

I have one question about the 'force' flag given to connector's detect() functions.
To design new EDID caching solution, I was trying to re-use this flag. As you all know, detect() gets called from few places in DRM and I915 layer, with this flag status:

drm_sysfs.c : status_show (sysfs status check inquire from USP)						force = 1
drm_crtc_helper.c: drm_helper_probe_single_connector_modes (part of get_connector IOCTL) 		force = 1

drm_crtc_helper.c: output_poll_execute(scheduled from poll work)						force = 0
drm_crtc_helper.c: drm_helper_hpd_irq_event (resume hotplug)						force = 0
i915_irq.c: i915_hotplug_work_function (bottom half of hotplug IRQ)						force = 0

What I am seeing is, the places where it's really required to probe the device, like in IRQ handlers or while resuming the device,  the force flag is 0, 
whereas whenever there is a userspace interaction or query for status, the flag is 1.  Please correct me if my understating is not proper, 
but I feel this should be the opposite way. 

Please let me know your opinion about this.

Regards
Shashank  
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of Sharma, Shashank
Sent: Wednesday, April 09, 2014 12:20 PM
To: Wang, Quanxian; Daniel Vetter
Cc: intel-gfx at lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect and get_modes

Hello Quanxian Wang

This patch is available and working on all MCG tree's (Main, R42B and R44B) We were trying to opensource this patch, but due to the dependency on live_status reg, we had to change the design. 

I was working on that, but couldn't finish the activity yet, Thanks for reminding me I will update soon. :)

Regards
Shashank   
-----Original Message-----
From: Wang, Quanxian
Sent: Wednesday, April 09, 2014 11:49 AM
To: Sharma, Shashank; Daniel Vetter
Cc: intel-gfx at lists.freedesktop.org
Subject: RE: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect and get_modes

Hi, Sharma, Shashank

Is there any following patches to make it happen?

Thanks

Regards

Quanxian Wang

>-----Original Message-----
>From: intel-gfx-bounces at lists.freedesktop.org
>[mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of Sharma, 
>Shashank
>Sent: Tuesday, January 14, 2014 1:20 AM
>To: Daniel Vetter
>Cc: intel-gfx at lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect 
>and get_modes
>
>Thanks again for this explanation Daniel.
>We will work on your suggestions and come up with a new patch.
>
>Regards
>Shashank  / Ramalingam
>-----Original Message-----
>From: daniel.vetter at ffwll.ch [mailto:daniel.vetter at ffwll.ch] On Behalf 
>Of Daniel Vetter
>Sent: Monday, January 13, 2014 6:57 PM
>To: Sharma, Shashank
>Cc: C, Ramalingam; intel-gfx at lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect 
>and get_modes
>
>On Mon, Jan 13, 2014 at 10:39 AM, Sharma, Shashank 
><shashank.sharma at intel.com> wrote:
>> Thanks a lot for your time, for reviewing the changes, and giving us 
>> some
>pointers.
>> Both me and Ramalingam are designing this together, and we discussed 
>> about
>these changes and your suggestions.
>> There are few things we would like to discuss about. Please correct 
>> us if some of
>our understanding is not proper.
>
>First something I've forgotten in the original mail: Overall your 
>patches look really nice and the commit messages and cover letter have been excellent.
>Unfortunately you've run into one of the nastier cases of "reality just 
>wont agree with the spec" :(
>
>> Those two patches provide two solution.
>> 1. Support for soft HPD, and slow removal of HDMI (when the DDC 
>> channel can
>still get the EDID).
>> 2. Try to reduce the EDID reads over DDC channel for get connector 
>> and fill mode
>calls, by caching EDID, and using it until next HPD comes.
>>
>> Patch 2: Reduce the EDID read over DDC channel We are caching the 
>> EDID at every HPD up, on HDMI detect calls, and we are freeing it on 
>> subsequent
>HDMI disconnect calls.
>>
>> The design philosophy here is, to maintain a state machine of HDMI 
>> connector
>status, and differentiate between IOCTL detect calls and HPD detect calls.
>> If there is a detect() or get_modes() call due to any of the IOCTL, 
>> which makes
>sure that input variable force=1, we just use the cached EDID, to serve this calls.
>> But if the detect call is coming from HPD work function, due to a HPD 
>> plug-out,
>we remove/invalidate the old cached EDID, and cache the new EDID, on 
>subsequent HDMI plug-in.
>> From here, the same state machine follows.
>>
>> Can you please let us know, why do you think that we should 
>> invalidate the
>cached EDID after 1-2 seconds ?
>
>Because there are machines out there where hpd never happens. So if you 
>keep onto the cached value forever userspace will never notice a change 
>in output configuration. Of course hotplug handling won't work, but at 
>least users can still manually probe outputs. By unconditionally using 
>the cached edid from ioctls you break this use case. Yes, such machines 
>are broken, but we need to keep them working anyway.
>
>Also in my experience all machines are affected, we have examples 
>covering gm45, ilk, snb & ivb. We haven't seen a case for hsw/byt yet 
>since we don't rely on the hpd bits any more (and so won't see bug reports any more).
>
>Generally if you use the hpd stuff your code must be designed under the 
>assumption that hpd is completely unreliably. We've seen anything from 
>random noise, flat-out not-working at all, stuck bits and unstable hpd 
>values that occasionally flip-flop. So you can't rely on it at all.
>
>> Note: In this same patch, there is additional optimization, which you 
>> pointed out,
>where we check if the connector->status is same as live status.
>> This can be removed independently, as you suggested.
>
>Hm, where have I pointed this out? Some other mail on internal discussions?
>
>> About patch 1:
>> We have done some local experiments and we came to know that for VLV 
>> and
>HSW boards, we can rely on the live status, if we give it some time to 
>settle (~300ms).
>> Probably, we need to modify this patch, as you suggested, until it 
>> becomes
>handy to be used reliably. We are on it, and will send another patch soon.
>>
>> But if somehow we are able to get some consistent results from live 
>> status, do
>you think it would be worth accepting this change, so that it can 
>handle soft HPDs and automation testing.
>> Because I believe we will face this problem whenever we are trying to 
>> test
>something from automation, where the physical device is not removed, 
>and DDC channel is up always.
>
>It's very well possible that all the platforms you have, but experience 
>says that some OEM will horrible screw this up. At least they've 
>consistently botched this in the past on occasional machines.
>
>Now the ghost hdmi detection on slow removal is obviously not great, 
>but we can't use the hpd bits to fix this. One approach would be.
>1. Upon hpd interrupt do an immediate probe of the connector. This way 
>we'll have good userspace experience if the unplug happens quickly and the hw works.
>2. Re-probe with a 1s delay to catch slow-uplugs. The current output 
>probing helpers are clever enough already that if a state-change 
>happens to be detected a uevent will be generate, irrespective of the 
>source of the detect call (i.e. hpd, kernel poll or ioctl/sysfs).
>
>Note that we already track the hpd interrupts on a per-source basis, so 
>doing the re-poll shouldn't be costly. Maybe do the re-poll as part of 
>the EDID invalidation to avoid stalling userspace.
>
>But you can't rely upon the hpd pins unfortunately :(
>
>This way we should be able to implement the 2 features you want, even 
>on unreliable hw.
>
>Cheers, Daniel
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>+41 (0) 79 365 57 48 - http://blog.ffwll.ch
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx