Re: [igt-dev] [PATCH i-g-t 1/2] tests/frontbuffer_tracking: Do not assert FBC state after a page flip changing stride

["Souza, Jose" <jose.souza@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2802 bytes --]

On Tue, 2019-04-02 at 11:49 -0700, Dhinakaran Pandiyan wrote:
> On Thu, 2019-03-28 at 15:28 -0700, José Roberto de Souza wrote:
> > When the stridechange subtest was introduced f23ea58f1fbb
> > ("kms_frontbuffer_tracking: expand badstride and stridechange")
> > atomic was not around and change the stride using drmModePageFlip()
> > was not allowed so it was expected that it would return -EINVAL and
> > kernel would keep the old framebuffer with the smaller plane that
> > is
> > know to fit on CFB(if it don't fit the test will skip on the first
> > full-modeset because "not enough stolen memory" is set).
> > 
> > But after the introduction of atomic the subtest was updated by
> > f63e070b469d ("kms_frontbuffer_tracking: Fix tests with the new
> > atomic reality.") to accept a no error return from
> > drmModePageFlip()
> > but the do_assertions() that follows it was not updated.
> > As the subtest function comment states, kernel will do fastsets in
> > this scenario and the allocated CFB could not be enough to keep FBC
> > enabled over the new framebuffer, so here adding the missing
> > DONT_ASSERT_FEATURE_STATUS to ignore the FBC state and just test if
> > CRC match and if kernel do not misbehave.
> > 
> > Other way to solve this issue would be make the kernel do a
> > full-modeset when CFB is not enough for the new plane so FBC is
> > disabled with the CRC freeing the actual CFB and then after enable
> > CRTC again it will try to enable FBC again if it can allocate the
> > required CFB but by the subtest comment this is not intended.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105683
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  tests/kms_frontbuffer_tracking.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index 4d15ce1c..087fc473 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -2950,7 +2950,7 @@ static void stridechange_subtest(const struct
> > test_mode
> > *t)
> >  	 */
> >  	rc = drmModePageFlip(drm.fd, drm.display.pipes[params-
> > >pipe].crtc_id,
> > new_fb->fb_id, 0, NULL);
> >  	igt_assert(rc == -EINVAL || rc == 0);
> > -	do_assertions(0);
> > +	do_assertions(DONT_ASSERT_FEATURE_STATUS);
> The assertion is still valid if the page flip ioctl failed, isn't it?

Huum yeah, I guess non-atomic drivers would also return a error before
changing the hardware state.

So change to:

do_assertions(rc ? 0 : DONT_ASSERT_FEATURE_STATUS);


> 
> -DK
> 
> >  }
> >  
> >  /**

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev