From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/6] drm/i915: Always enable the cursor right after the primary plane Date: Thu, 6 Jun 2013 20:34:05 +0300 Message-ID: <20130606173405.GO5004@intel.com> References: <1368006628-24628-1-git-send-email-ville.syrjala@linux.intel.com> <1368006628-24628-3-git-send-email-ville.syrjala@linux.intel.com> <20130521151726.GG17947@debian> <20130606163052.GH17947@debian> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 1F2FFE673E for ; Thu, 6 Jun 2013 10:34:09 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130606163052.GH17947@debian> 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: Egbert Eich Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Jun 06, 2013 at 06:30:52PM +0200, Egbert Eich wrote: > On Tue, May 21, 2013 at 05:17:27PM +0200, Egbert Eich wrote: > > On Wed, May 08, 2013 at 12:50:24PM +0300, ville.syrjala@linux.intel.com= wrote: > > > From: Ville Syrj=E4l=E4 > > > = > > > Follow the same sequence when enabling the cursor plane during > > > modeset. No point in doing this stuff in different order on different > > > generations. > > > = > > > This should also avoid a needless wait for vblank for the g4x cursor > > > workaround when the cursor gets enabled anyway. > > > = > > > Signed-off-by: Ville Syrj=E4l=E4 > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 10 ++++------ > > > 1 file changed, 4 insertions(+), 6 deletions(-) > > > = > > > @@ -3727,6 +3725,7 @@ static void i9xx_crtc_enable(struct drm_crtc *c= rtc) > > > = > > > intel_enable_pipe(dev_priv, pipe, false); > > > intel_enable_plane(dev_priv, plane, pipe); > > > + intel_crtc_update_cursor(crtc, true); > > > if (IS_G4X(dev)) > > > g4x_fixup_plane(dev_priv, pipe); > > > = > > = > > As discussed on IRC this may interfere with = > > = > > commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595 > > Author: Egbert Eich > > Date: Mon Mar 4 09:24:38 2013 -0500 > > = > > DRM/i915: On G45 enable cursor plane briefly after enabling the dis= play plane. > > = > > described in https://bugs.freedesktop.org/show_bug.cgi?id=3D61457 > > = > = > Today I had the chance to test this. First I tried > if I can still reproduce the blank with this patch > added when I disable my voodoo g4x_fixup_plane(): > It turned out it still happens however very rarely > (like 1 out of 20 tries). When I reenabled my voodoo > the issue still occurred. > I had to switch two lines around, ie: > = > intel_enable_plane(dev_priv, plane, pipe); > if (IS_G4X(dev)) > g4x_fixup_plane(dev_priv, pipe); > + intel_crtc_update_cursor(crtc, true); > = > to avoid the blank screen issue - which is it didn't > happen in ~75 tries. > = > With this change: > = > Acked-by: Egbert Eich Hmm. That means the cursor itself isn't perhaps really relevant, and it's maybe more about the self refresh. BTW did you ever try w/ just the self refresh disable but w/o the cursor tr= ick? We seem to enable self refresh already before we enable any planes (in intel_update_watermarks()). So maybe something like this would work: i9xx_crtc_enable() ... intel_update_watermarks() ... intel_enable_pipe() intel_enable_plane() [ maybe intel_wait_for_vblank(), but we seem to have too many of those alr= eady... ] BTW the spec lists an SR workaround, which we don't follow apparently. It's only relevant for switching between 1 and 2 pipes though. But maybe there's a more general issue enabling even 1 pipe w/ SR enabled... I suspect I need to dig more into the g4x watermark/SR stuff at some point, so this is good practice for me :) I just wish I was able to reproduce the bug. Maybe I need to find a ctg instead of the elk I have... -- = Ville Syrj=E4l=E4 Intel OTC