Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/frontbuffer_tracking: Assert the status of other features on stridechange subtest

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

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

On Tue, 2019-04-02 at 14:24 -0700, Dhinakaran Pandiyan wrote:
> On Tue, 2019-04-02 at 13:59 -0700, José Roberto de Souza wrote:
> > As explained in c1edee186d18 ("tests/frontbuffer_tracking: Do not
> > assert FBC state after a page flip changing stride") after changing
> > the plane stride there is the possibility that CFB will not be big
> > enough to keep FBC enabled, that is why do_assertions() is called
> > with DONT_ASSERT_FEATURE_STATUS but DONT_ASSERT_FEATURE_STATUS is
> > overkill and will not check the status of the other features like
> > PSR
> > and DRRS when running combined feature tests and possibly hiding
> > bugs.
> > 
> > So lets add a new flag that will only not assert FBC.
> 
> You are adding this flag to be only used in stridechange_subtest().
> But, is
> stride change even relevant for PSR and DRRS? I didn't think so.
> While harmless,
> I don't see any much value in asserting PSR and DRRS stay enabled.

I'm also not aware of any case that would prevent PSR or DRRS to be
enabled but that is why we have the tests, to catch missing details
from spec, regressions and bugs.

Without this change we are not testing the other features states in
this tests:

fbcpsr-stridechange
fbcdrrs-stridechange
fbcpsrdrrs-stridechange

> 
> 
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  tests/kms_frontbuffer_tracking.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index caef6755..ee13b138 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -1509,6 +1509,7 @@ static void do_flush(const struct test_mode
> > *t)
> >  
> >  #define DONT_ASSERT_CRC			(1 << 0)
> >  #define DONT_ASSERT_FEATURE_STATUS	(1 << 1)
> > +#define DONT_ASSERT_FBC_STATUS		(1 << 12)
> >  
> >  #define FBC_ASSERT_FLAGS		(0xF << 2)
> >  #define ASSERT_FBC_ENABLED		(1 << 2)
> > @@ -1539,7 +1540,7 @@ static int adjust_assertion_flags(const
> > struct test_mode
> > *t, int flags)
> >  			flags |= ASSERT_DRRS_HIGH;
> >  	}
> >  
> > -	if ((t->feature & FEATURE_FBC) == 0)
> > +	if ((t->feature & FEATURE_FBC) == 0 || (flags &
> > DONT_ASSERT_FBC_STATUS))
> >  		flags &= ~FBC_ASSERT_FLAGS;
> >  	if ((t->feature & FEATURE_PSR) == 0)
> >  		flags &= ~PSR_ASSERT_FLAGS;
> > @@ -2910,7 +2911,7 @@ static void stridechange_subtest(const struct
> > test_mode
> > *t)
> >  	fill_fb_region(&params->primary, COLOR_PRIM_BG);
> >  
> >  	set_mode_for_params(params);
> > -	do_assertions(DONT_ASSERT_FEATURE_STATUS);
> > +	do_assertions(DONT_ASSERT_FBC_STATUS);
> >  
> >  	/* Go back to the fb that can have FBC. */
> >  	params->primary.fb = old_fb;
> > @@ -2920,7 +2921,7 @@ static void stridechange_subtest(const struct
> > test_mode
> > *t)
> >  	/* This operation is the same as above, but with the planes
> > API. */
> >  	params->primary.fb = new_fb;
> >  	set_prim_plane_for_params(params);
> > -	do_assertions(DONT_ASSERT_FEATURE_STATUS);
> > +	do_assertions(DONT_ASSERT_FBC_STATUS);
> >  
> >  	params->primary.fb = old_fb;
> >  	set_prim_plane_for_params(params);
> > @@ -2932,7 +2933,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(rc ? 0 : DONT_ASSERT_FEATURE_STATUS);
> > +	do_assertions(rc ? 0 : DONT_ASSERT_FBC_STATUS);
> >  }
> >  
> >  /**

[-- 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