From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Sharma, Shashank" Subject: Re: [PATCH 0/2] Optimization on intel HDMI detect and get_modes Date: Thu, 10 Apr 2014 08:10:25 +0000 Message-ID: References: <1389595914-23141-1-git-send-email-ramalingam.c@intel.com> <20140113072911.GM4770@phenom.ffwll.local> <50C7F552BA2A744D84D95119645BEABD014B92EC@SHSMSX101.ccr.corp.intel.com> <20140410080855.GN9262@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 3D02D6EC15 for ; Thu, 10 Apr 2014 01:11:01 -0700 (PDT) In-Reply-To: <20140410080855.GN9262@phenom.ffwll.local> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org Thanks for this clarification. Regards Shashank -----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@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@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@lists.freedesktop.org] On > Behalf Of Sharma, Shashank > Sent: Wednesday, April 09, 2014 12:20 PM > To: Wang, Quanxian; Daniel Vetter > Cc: intel-gfx@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@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@lists.freedesktop.org > >[mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Sharma, > >Shashank > >Sent: Tuesday, January 14, 2014 1:20 AM > >To: Daniel Vetter > >Cc: intel-gfx@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@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On > >Behalf Of Daniel Vetter > >Sent: Monday, January 13, 2014 6:57 PM > >To: Sharma, Shashank > >Cc: C, Ramalingam; intel-gfx@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 > > 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@lists.freedesktop.org > >http://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@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