From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Wait for vblank after enabling the primary plane on BDW Date: Tue, 12 Aug 2014 16:02:17 +0300 Message-ID: <20140812130217.GA4193@intel.com> References: <1403607568-18529-1-git-send-email-ville.syrjala@linux.intel.com> <87ha32iu6x.fsf@intel.com> <20140812082411.GT4193@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTP id 15F9189CAF for ; Tue, 12 Aug 2014 06:02:25 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140812082411.GT4193@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paulo Zanoni Cc: intel-gfx List-Id: intel-gfx@lists.freedesktop.org On Tue, Aug 12, 2014 at 11:24:11AM +0300, Ville Syrj=E4l=E4 wrote: > On Mon, Aug 11, 2014 at 04:44:23PM -0300, Paulo Zanoni wrote: > > 2014-06-30 7:10 GMT-03:00 Jani Nikula : > > > On Thu, 26 Jun 2014, Rodrigo Vivi wrote: > > >> I'm sure this might affect Wayne, so, cc'ing him here. > > >> > > >> from my point of view this is right so: > > >> Reviewed-by: Rodrigo Vivi > > > > > > Pushed to -fixes, thanks for the patch and review. > > > > > > BR, > > > Jani. > > > > > > > > >> > > >> > > >> On Tue, Jun 24, 2014 at 3:59 AM, wro= te: > > >> > > >>> From: Ville Syrj=E4l=E4 > > >>> > > >>> BDW signals the flip done interrupt immediately after the DSPSURF w= rite > > >>> when the plane is disabled. This is true even if we've already armed > > >>> DSPCNTR to enable the plane at the next vblank. This causes major > > >>> problems for our page flip code which relies on the flip done inter= rupts > > >>> happening at vblank time. > > >>> > > >>> So what happens is that we enable the plane, and immediately allow > > >>> userspace to submit a page flip. If the plane is still in the proce= ss > > >>> of being enabled when the page flip is issued, the flip done gets > > >>> signalled immediately. Our DSPSURFLIVE check catches this to prevent > > >>> premature flip completion, but it also means that we don't get a fl= ip > > >>> done interrupt when the plane actually gets enabled, and so the page > > >>> flip is never completed. > > >>> > > >>> Work around this by re-introducing blocking vblank waits on BDW > > >>> whenever we enable the primary plane. > > >>> > > >>> I removed some of the vblank waits here: > > >>> commit 6304cd91e7f05f8802ea6f91287cac09741d9c46 > > >>> Author: Ville Syrj=E4l=E4 > > >>> Date: Fri Apr 25 13:30:12 2014 +0300 > > >>> > > >>> drm/i915: Drop the excessive vblank waits from modeset codepaths > > >>> > > >>> To avoid these blocking vblank waits we should start using the vbla= nk > > >>> interrupt instead of the flip done interrupt to complete page flips. > > >>> But that's material for another patch. > > >>> > > >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3D79354 > > >>> Tested-by: Guo Jinxian > > >>> Signed-off-by: Ville Syrj=E4l=E4 > > >>> --- > > >>> drivers/gpu/drm/i915/intel_display.c | 9 +++++++++ > > >>> drivers/gpu/drm/i915/intel_sprite.c | 8 ++++++++ > > >>> 2 files changed, 17 insertions(+) > > >>> > > >>> diff --git a/drivers/gpu/drm/i915/intel_display.c > > >>> b/drivers/gpu/drm/i915/intel_display.c > > >>> index 9188fed..f92efc6 100644 > > >>> --- a/drivers/gpu/drm/i915/intel_display.c > > >>> +++ b/drivers/gpu/drm/i915/intel_display.c > > >>> @@ -2087,6 +2087,7 @@ void intel_flush_primary_plane(struct > > >>> drm_i915_private *dev_priv, > > >>> static void intel_enable_primary_hw_plane(struct drm_i915_private > > >>> *dev_priv, > > >>> enum plane plane, enum pi= pe pipe) > > >>> { > > >>> + struct drm_device *dev =3D dev_priv->dev; > > >>> struct intel_crtc *intel_crtc =3D > > >>> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > > >>> int reg; > > >>> @@ -2106,6 +2107,14 @@ static void intel_enable_primary_hw_plane(st= ruct > > >>> drm_i915_private *dev_priv, > > >>> > > >>> I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE); > > >>> intel_flush_primary_plane(dev_priv, plane); > > >>> + > > >>> + /* > > >>> + * BDW signals flip done immediately if the plane > > >>> + * is disabled, even if the plane enable is already > > >>> + * armed to occur at the next vblank :( > > >>> + */ > > >>> + if (IS_BROADWELL(dev)) > > >>> + intel_wait_for_vblank(dev, intel_crtc->pipe); > > = > > This chunk triggers "WARN(ret =3D=3D 0)" from drm_wait_one_vblank when > > using HDMI on BDW. > = > Are we still calling drm_vblank_off() too soon or something? Oh it was enable. That could mean we still haven't yet called drm_vblank_on(). But at least the order in intel_crtc_enable_planes() seems correct, and also intel_primary_plane_setplane() should never try enable the primary plane when the crtc is not active ('visible' should be false in that case). > = > > = > > = > > >>> } > > >>> > > >>> /** > > >>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c > > >>> b/drivers/gpu/drm/i915/intel_sprite.c > > >>> index 1b66ddc..9a17b4e 100644 > > >>> --- a/drivers/gpu/drm/i915/intel_sprite.c > > >>> +++ b/drivers/gpu/drm/i915/intel_sprite.c > > >>> @@ -691,6 +691,14 @@ intel_post_enable_primary(struct drm_crtc *crt= c) > > >>> struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > > >>> > > >>> /* > > >>> + * BDW signals flip done immediately if the plane > > >>> + * is disabled, even if the plane enable is already > > >>> + * armed to occur at the next vblank :( > > >>> + */ > > >>> + if (IS_BROADWELL(dev)) > > >>> + intel_wait_for_vblank(dev, intel_crtc->pipe); > > >>> + > > >>> + /* > > >>> * FIXME IPS should be fine as long as one plane is > > >>> * enabled, but in practice it seems to have problems > > >>> * when going from primary only to sprite only and vice > > >>> -- > > >>> 1.8.5.5 > > >>> > > >>> _______________________________________________ > > >>> Intel-gfx mailing list > > >>> Intel-gfx@lists.freedesktop.org > > >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > >>> > > >> > > >> > > >> > > >> -- > > >> Rodrigo Vivi > > >> Blog: http://blog.vivi.eng.br > > >> _______________________________________________ > > >> Intel-gfx mailing list > > >> Intel-gfx@lists.freedesktop.org > > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > -- > > > Jani Nikula, Intel Open Source Technology Center > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > = > > = > > = > > -- = > > Paulo Zanoni > = > -- = > Ville Syrj=E4l=E4 > Intel OTC -- = Ville Syrj=E4l=E4 Intel OTC