diff mbox

[2/2] drm/i915: Also call frontbuffer flip when disabling planes.

Message ID 1437781137-1119-2-git-send-email-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi July 24, 2015, 11:38 p.m. UTC
We also need to call the frontbuffer flip to trigger proper
invalidations when disabling planes. Otherwise we will miss
screen updates when disabling sprites or cursor.

It was caught with kms_psr_sink_crc sprite_plane_onoff
and cursor_plane_onoff subtests.

This is probably a regression since I can also get this
with the manual test case, but with so many changes on atomic
modeset I couldn't track exactly when this was introduced.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Shuang He July 26, 2015, 1:52 a.m. UTC | #1
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6860
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -1              296/296              295/296
SNB                                  316/316              316/316
IVB                                  342/342              342/342
BYT                 -1              286/286              285/286
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@kms_flip@flip-vs-rmfb-interruptible      PASS(1)      DMESG_WARN(1)
*BYT  igt@gem_partial_pwrite_pread@reads-display      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
Rodrigo Vivi Aug. 19, 2015, 8:01 p.m. UTC | #2
I had forgotten I had this patch and lost sometime yesterday debugging and
end up on same fix again :/

Daniel, do you need a reviewer on this? could you please take a quickly
look?

thanks in advance,
Rodrigo.

On Fri, Jul 24, 2015 at 4:41 PM Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:

> We also need to call the frontbuffer flip to trigger proper
> invalidations when disabling planes. Otherwise we will miss
> screen updates when disabling sprites or cursor.
>
> It was caught with kms_psr_sink_crc sprite_plane_onoff
> and cursor_plane_onoff subtests.
>
> This is probably a regression since I can also get this
> with the manual test case, but with so many changes on atomic
> modeset I couldn't track exactly when this was introduced.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index af0bcfe..bb124cc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11716,7 +11716,7 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
>                 intel_crtc->atomic.update_wm_pre = true;
>         }
>
> -       if (visible)
> +       if (visible || was_visible)
>                 intel_crtc->atomic.fb_bits |=
>                         to_intel_plane(plane)->frontbuffer_bit;
>
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Paulo Zanoni Aug. 21, 2015, 9:46 p.m. UTC | #3
2015-07-24 20:38 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> We also need to call the frontbuffer flip to trigger proper
> invalidations when disabling planes. Otherwise we will miss
> screen updates when disabling sprites or cursor.
>
> It was caught with kms_psr_sink_crc sprite_plane_onoff
> and cursor_plane_onoff subtests.
>
> This is probably a regression since I can also get this
> with the manual test case, but with so many changes on atomic
> modeset I couldn't track exactly when this was introduced.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Per our conversation on IRC, you need to mention that this patch only
affects the VLV family since on big core the HW tracking helps hide
the problem.

It would also be good to use the Testcase tags :)

But it looks correct. So with the amended message: Reviewed-by: Paulo
Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index af0bcfe..bb124cc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11716,7 +11716,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>                 intel_crtc->atomic.update_wm_pre = true;
>         }
>
> -       if (visible)
> +       if (visible || was_visible)
>                 intel_crtc->atomic.fb_bits |=
>                         to_intel_plane(plane)->frontbuffer_bit;
>
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index af0bcfe..bb124cc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11716,7 +11716,7 @@  int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 		intel_crtc->atomic.update_wm_pre = true;
 	}
 
-	if (visible)
+	if (visible || was_visible)
 		intel_crtc->atomic.fb_bits |=
 			to_intel_plane(plane)->frontbuffer_bit;