Hi Ville Syrj?l?, > -----Original Message----- > From: Ville Syrj?l? [mailto:ville.syrjala at linux.intel.com] > Sent: Friday, January 18, 2013 9:14 PM > To: Wang, Xingchao > Cc: intel-gfx at lists.freedesktop.org; daniel.vetter at ffwll.ch; Zanoni, Paulo R > Subject: Re: [PATCH v2] drm/i915: HDMI/DP - ELD info refresh > support for Haswell > > On Fri, Jan 18, 2013 at 12:00:10PM +0000, Wang, Xingchao wrote: > > > > > -----Original Message----- > > > From: Ville Syrj?l? [mailto:ville.syrjala at linux.intel.com] > > > Sent: Friday, January 18, 2013 6:46 PM > > > To: Wang, Xingchao > > > Cc: intel-gfx at lists.freedesktop.org; daniel.vetter at ffwll.ch; Zanoni, > > > Paulo R > > > Subject: Re: [PATCH v2] drm/i915: HDMI/DP - ELD info > > > refresh support for Haswell > > > > > > On Fri, Jan 18, 2013 at 10:51:55AM +0800, Wang Xingchao wrote: > > > > ELD info should be updated dynamically according to hot plug event. > > > > For haswell chip, clear/set the eld valid bit and output enable > > > > bit from callback intel_disable/eanble_ddi(). > > > > > > Hmm. Is it OK to set the ELD valid bit if the ELD hasn't actually been > written? > > > > This triggers an unsolicited event to ALSA driver which continue to read ELD > info. > > I take it we don't want that to happen? > > > > And besides these bits are already set by haswell_write_eld(). > > > > Intel_disable/enable_ddi() was called even after haswell_write_eld(), so we > need set the bits again. > > For the mode set sequence only intel_enable_ddi() is called after > haswell_write_eld(). > > This is how the sequence should end up looking with the current > code: > > intel_set_mode() > -> haswell_crtc_disable() > -> intel_disable_ddi() > -> intel_crtc_mode_set() > -> haswell_crtc_mode_set() > -> intel_ddi_mode_set() > -> intel_write_eld() > -> haswell_write_eld() > -> haswell_crtc_enable() > -> intel_enable_ddi() > > > But for DPMS on->off->on there would be calls to haswell_crtc_disable() and > haswell_crtc_enable() w/o calls to haswell_write_eld(). I suppose this is the > problem you want to fix? Exactly. Intel_disable_ddi() was called after haswell_write_eld() only once during system bootup. I added a flag to monitor ELD valid status. It will be set in haswell_write_eld() and cleared in intel_disable_ddi(). In intel_enable_ddi() , will check this flag and set the ELD valid bit again only if the flag was cleared before. I had tested the new patch and it works well. I will send out the updated patch later. > > So, perhaps there should be a flag somewhere that would be cleared at the > beginning of the mode_set operation, and then intel_write_eld() should set the > flag when the ELD was written succesfully. > intel_enable_ddi() could check the flag and only set the ELD valid bit when the > flag is set. > > Should non-ddi platforms do something similar as well? Would you share more ideas on this? I have not noticed the potential issues here. Thanks --xingchao
