[igt-dev] [PATCH i-g-t] tests/kms_psr2_su: Print the SU blocks mismatches

[Souza, Jose]

On Wed, 2020-02-26 at 13:14 +0530, Anshuman Gupta wrote:
> On 2020-02-25 at 23:47:33 +0530, Souza, Jose wrote:
> > On Tue, 2020-02-25 at 09:30 +0530, Anshuman Gupta wrote:
> > > On 2020-02-25 at 04:47:47 +0530, Souza, Jose wrote:
> > > > On Mon, 2020-02-24 at 10:48 +0530, Anshuman Gupta wrote:
> > > > > On 2020-01-23 at 11:00:29 -0800, José Roberto de Souza wrote:
> > > > > > This will help us debug failures in this test.
> > > > > > 
> > > > > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > > > > ---
> > > > > >  tests/kms_psr2_su.c | 7 +++++--
> > > > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/tests/kms_psr2_su.c b/tests/kms_psr2_su.c
> > > > > > index f2244b29..9f40c735 100644
> > > > > > --- a/tests/kms_psr2_su.c
> > > > > > +++ b/tests/kms_psr2_su.c
> > > > > > @@ -183,9 +183,13 @@ static bool
> > > > > > update_screen_and_test(data_t
> > > > > > *data)
> > > > > >  		igt_assert_f(data->op, "Operation not
> > > > > > handled\n");
> > > > > >  	}
> > > > > >  
> > > > > > -	if (psr2_wait_su(data->debugfs_fd, &su_blocks))
> > > > > > +	if (psr2_wait_su(data->debugfs_fd, &su_blocks)) {
> > > > > >  		ret = su_blocks == EXPECTED_NUM_SU_BLOCKS;
> > > > > >  
> > > > > > +		if (!ret)
> > > > > > +			igt_debug("Not matching SU blocks read:
> > > > > > %u\n",
> > > > > > su_blocks);
> > > > > AFAIU this will print the SU block for 0th frame, and looks
> > > > > like
> > > > > this test also interested only in 0th frame, i am just
> > > > > curious
> > > > > here
> > > > > to know the 
> > > > > use case of SU block for last 8 frames.
> > > > 
> > > > 0th frame?
> > > I referred PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t" , as
> > > 0th
> > > frame.
> > > Frame:  PSR2 SU blocks:
> > > 0       0
> > > 1       0
> > > ---------
> > > 7       0
> > > What are the use of other 7 frames.
> > 
> > Was used to debug PSR2 during development, we could see how many
> > selective update blocks each of the past 8 had by just reading 3
> > registers. Anyways psr2_wait_su() only cares about the last frame.
> Thanks for explaining this.
> You can use my RB with some description for removal of log.
> Reviewed-by: Anshuman Gupta <anshuman.gupta at intel.com>
> Thanks,
> Anshuman Gupta.

Thanks, just pushed with the description requested

> > > Thanks,
> > > Anshuman Gupta.
> > > > It waits up to 40mili to also handle 30hz panels,
> > > > 1000/30=33mili.
> > > > The SU blocks should be kept set until next flip or frontbuffer
> > > > modification.
> > > > 
> > > > > > +	}
> > > > > > +
> > > > > >  	return ret;
> > > > > >  }
> > > > > >  
> > > > > > @@ -206,7 +210,6 @@ static void run(data_t *data)
> > > > > >  			result = update_screen_and_test(data);
> > > > > >  	}
> > > > > >  
> > > > > > -	igt_debug("Screen changes: %u\n", data-
> > > > > > > screen_changes);
> > > > > IMHO this seems unrealted to this patch considering its
> > > > > commit
> > > > > log,
> > > > > could u please
> > > > > add some description about removal of this log.
> > > > 
> > > > Will add, it is removing this useless debug message I probably
> > > > tought
> > > > that screen_changes would have the information that I'm adding
> > > > in
> > > > the
> > > > block added in the patch.
> > > > 
> > > > > Thanks,
> > > > > Anshuman Gupta.
> > > > > >  	igt_assert_f(result,
> > > > > >  		     "No matching selective update blocks read
> > > > > > from
> > > > > > debugfs\n");
> > > > > >  }
> > > > > > -- 
> > > > > > 2.25.0
> > > > > > 
> > > > > > _______________________________________________
> > > > > > igt-dev mailing list
> > > > > > igt-dev at lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev