Re: [Intel-gfx] [PATCH 2/6] drm/i915: Always enable the cursor right after the primary plane
On Tue, May 21, 2013 at 05:17:27PM +0200, Egbert Eich wrote: On Wed, May 08, 2013 at 12:50:24PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com 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älä ville.syrj...@linux.intel.com --- 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 *crtc) 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 e...@suse.com Date: Mon Mar 4 09:24:38 2013 -0500 DRM/i915: On G45 enable cursor plane briefly after enabling the display plane. described in https://bugs.freedesktop.org/show_bug.cgi?id=61457 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 e...@suse.com Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915: Always enable the cursor right after the primary plane
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.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com 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älä ville.syrj...@linux.intel.com --- 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 *crtc) 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 e...@suse.com Date: Mon Mar 4 09:24:38 2013 -0500 DRM/i915: On G45 enable cursor plane briefly after enabling the display plane. described in https://bugs.freedesktop.org/show_bug.cgi?id=61457 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 e...@suse.com 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 trick? 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() ... SR off intel_enable_pipe() intel_enable_plane() enable other planes [ maybe intel_wait_for_vblank(), but we seem to have too many of those already... ] SR back on 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älä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915: Always enable the cursor right after the primary plane
On Wed, May 08, 2013 at 12:50:24PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com 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älä ville.syrj...@linux.intel.com --- 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 *crtc) 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 e...@suse.com Date: Mon Mar 4 09:24:38 2013 -0500 DRM/i915: On G45 enable cursor plane briefly after enabling the display plane. described in https://bugs.freedesktop.org/show_bug.cgi?id=61457 I cannot test on the affected hardware as it's not available to me at the moment. If Ville's G4x doesn't show the issue I will test this next time I have access to the HW in question. Therefore: Acked-by: Egbert Eich e...@suse.com Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915: Always enable the cursor right after the primary plane
From: Ville Syrjälä ville.syrj...@linux.intel.com 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älä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 40e014af..e2037d5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3319,6 +3319,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) intel_enable_pipe(dev_priv, pipe, intel_crtc-config.has_pch_encoder); intel_enable_plane(dev_priv, plane, pipe); + intel_crtc_update_cursor(crtc, true); if (intel_crtc-config.has_pch_encoder) ironlake_pch_enable(crtc); @@ -3327,8 +3328,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) intel_update_fbc(dev); mutex_unlock(dev-struct_mutex); - intel_crtc_update_cursor(crtc, true); - for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); @@ -3392,6 +3391,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_enable_pipe(dev_priv, pipe, intel_crtc-config.has_pch_encoder); intel_enable_plane(dev_priv, plane, pipe); + intel_crtc_update_cursor(crtc, true); if (intel_crtc-config.has_pch_encoder) lpt_pch_enable(crtc); @@ -3400,8 +3400,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_update_fbc(dev); mutex_unlock(dev-struct_mutex); - intel_crtc_update_cursor(crtc, true); - for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); @@ -3687,12 +3685,12 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) intel_enable_pipe(dev_priv, pipe, false); intel_enable_plane(dev_priv, plane, pipe); + intel_crtc_update_cursor(crtc, true); intel_update_fbc(dev); /* Give the overlay scaler a chance to enable if it's on this pipe */ intel_crtc_dpms_overlay(intel_crtc, true); - intel_crtc_update_cursor(crtc, true); mutex_unlock(dev_priv-dpio_lock); } @@ -3727,6 +3725,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) 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); @@ -3734,7 +3733,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) /* Give the overlay scaler a chance to enable if it's on this pipe */ intel_crtc_dpms_overlay(intel_crtc, true); - intel_crtc_update_cursor(crtc, true); for_each_encoder_on_crtc(dev, crtc, encoder) encoder-enable(encoder); -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
