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

Sharma, Shashank shashank.sharma at intel.com
Thu Apr 10 10:10:25 CEST 2014 

Thanks for this clarification. 

Regards
Shashank 
-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
Sent: Thursday, April 10, 2014 1:39 PM
To: Sharma, Shashank
Cc: Wang, Quanxian; Daniel Vetter; intel-gfx at lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 0/2] Optimization on intel HDMI detect and get_modes

On Thu, Apr 10, 2014 at 06:46:58AM +0000, Sharma, Shashank wrote:
> 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.

Force is a bit misnomer, a better name might be non-invasive. Without force we don't do stuff like load-detect by default since at least when doing this manually some old crt screens switch on. But this is really only relevant for gen2/3 and tv-out, so not of any concern on modern platforms. Essentially force means "userspace asked for this and it's ok if the screen flickers a bit due to that".

Imo you can do the caching and use the cached version irrespective of force.
-Daniel

> 
> 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

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

More information about the Intel-gfx mailing list