From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell Date: Thu, 5 Sep 2013 14:27:29 +0200 Message-ID: <20130905122729.GD27291@phenom.ffwll.local> References: <1377820220-8251-1-git-send-email-mengdong.lin@intel.com> <20130905113320.GY11428@intel.com> <20130905114547.GC27291@phenom.ffwll.local> <20130905122101.GB11428@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f54.google.com (mail-ee0-f54.google.com [74.125.83.54]) by gabe.freedesktop.org (Postfix) with ESMTP id 02989E5C1D for ; Thu, 5 Sep 2013 05:27:16 -0700 (PDT) Received: by mail-ee0-f54.google.com with SMTP id e53so868275eek.13 for ; Thu, 05 Sep 2013 05:27:16 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130905122101.GB11428@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx , Mukesh List-Id: intel-gfx@lists.freedesktop.org On Thu, Sep 05, 2013 at 03:21:01PM +0300, Ville Syrj=E4l=E4 wrote: > On Thu, Sep 05, 2013 at 01:45:47PM +0200, Daniel Vetter wrote: > > On Thu, Sep 05, 2013 at 02:33:20PM +0300, Ville Syrj=E4l=E4 wrote: > > > On Wed, Sep 04, 2013 at 08:50:13PM +0200, Daniel Vetter wrote: > > > > On Fri, Aug 30, 2013 at 1:50 AM, wrote: > > > > > + /* Wait for 2 vertical blanks */ > > > > > + intel_wait_for_vblank(dev, pipe); > > > > > + intel_wait_for_vblank(dev, pipe); > > > > > + > > > > > + /* Disable audio PD. This is optional as per Bspec. */ > > > > > + temp =3D I915_READ(HSW_AUD_PIN_ELD_CP_VLD); > > > > > + temp &=3D ~(AUDIO_OUTPUT_ENABLE_A << (pipe * 4)); > > > > > + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp); > > > > = > > > > If this is optional do we really need the two vblank waits above? > > > > Adding them just for fun when we generally try to rip out as many > > > > vblank waits as possible from the modeset code isn't all that great > > > > ... > > > = > > > One idea I had for these kinds of vblank waits (there also one requir= ed > > > for IPS for instance) is that we might just sample a vblank counter > > > after the first step, then at the latest point we can, we'd wait for = the > > > frame counter to have passed the sampled vaoue + whatever extra is > > > needed. That might allow us to do other stuff in parallel while the > > > required number of vblanks will elapese. > > = > > My solution for this is to have vblank work items that we can use to ch= ain > > off all these things. We also need them for pageflips e.g. when > > re-enabling fbc or similar stuff. The problem is a bit that for switchi= ng > > things off like in a modeset the synchronization can get hairy and will= be > > little-tested. For enabling as long as we share the code with the nucle= ar > > pageflip code we should be fine though ... > > = > > Hence why I think we should try rather hard to avoid these vblank waits= in > > the first place. > = > Yeah, for enabling we definitely want to execute all that stuff > asynchronously. > = > The disable case is more problematic. For atomic pageflip we might get > away with returning -EAGAIN or something, but we have to block in the > full modeset case unless we want to move the entire modeset to a work > queue. So this trick would mostly be relevant for the disable during > modeset case. > = > But of course we could do that stuff via some kind of > schedule_vblank_work(currentl_vbl_count + X) and synchronize_vblank_work() > just before the critical step that needs the vblank work to be done. But > the locking and bugs might be more complicated in this case. Yeah we'll end up doing a maze of what will essentially be async tasklets and callbacks. A declarative way to do that would imo be a requirement, but such fanciness is hard to pull of in C without turning into something really ugly ... -Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch