Re: [PATCH i-g-t 7/7] tools/intel_watermark: Try not to dump nonexistent planes on SKL+

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Nov 21, 2017 at 12:50:51AM +0000, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Fri, 2017-09-15 at 20:57 +0300, Ville Syrjala wrote:
> > From: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> This should be yours :)
> 
> > 
> > Having registers for nonexistent planes in the dumpo might end up being
> > rather confusing. Try to only include real planes.
> > 
> 
> 
> Thanks for resubmitting 3/7 and the cleanups.
> 
> One nit, this patch leaves a hard-coded line that looks ugly on SKL.
> "printf("LEVEL   CURSOR   PLANE_1   PLANE_2   PLANE_3   PLANE_4\n");"

The PLANE_4 part? Yeah, I guess we could tweak it a bit. But maybe leave
that for the day when we have anyway support more planes in the tool.
That's assuming the watermark registers aren't going to change
significantly when that happens.

> 
> 
> Btw, I notice a possibly spurious watermark level for plane 3, pipe A on
> my SKL even when the display is off.

We don't actually use plane 3 on SKL. It's aliased with the cursor
and we only ever use it in the cursor mode. And I guess we just
leave the default watermarks in the registers for plane 3.

> 
> LINETIME: 0 (0.000 usec)
> LEVEL   CURSOR   PLANE_1   PLANE_2   PLANE_3   PLANE_4
>     0    0 ( 0)    0 ( 0)    0 ( 0)    7 ( 1)
>     1    0 ( 0)    0 ( 0)    0 ( 0)    7 ( 1)
>     2    0 ( 0)    0 ( 0)    0 ( 0)    7 ( 1)
>     3    0 ( 0)    0 ( 0)    0 ( 0)    7 ( 1)
>     4    0 ( 0)    0 ( 0)    0 ( 0)    7 ( 1)
>     5    0 ( 0)    0 ( 0)    0 ( 0)    7 ( 1)
>     6    0 ( 0)    0 ( 0)    0 ( 0)    7 ( 1)
>     7    0 ( 0)    0 ( 0)    0 ( 0)    7 ( 1)
> TRANS:   0 ( 0)    0 ( 0)    0 ( 0)    7 ( 1)
> 
> 
> The series lgtm, 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> for all
> patches except 3/7, which I've partially authored.

Thanks. Pushed to master.

> 
> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  tools/intel_watermark.c | 40 +++++++++++++++++++++++++++++++++-------
> >  1 file changed, 33 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/intel_watermark.c b/tools/intel_watermark.c
> > index ce920192295b..d81e95771efb 100644
> > --- a/tools/intel_watermark.c
> > +++ b/tools/intel_watermark.c
> > @@ -135,23 +135,42 @@ static int is_hsw_plus(uint32_t d)
> >  	return !(IS_GEN5(d) || IS_GEN6(d) || IS_IVYBRIDGE(d));
> >  }
> >  
> > +static int skl_num_planes(uint32_t d, int pipe)
> > +{
> > +	if (IS_GEN10(d) || IS_GEMINILAKE(d))
> > +		return 5;
> > +	else if (IS_BROXTON(d))
> > +		return pipe == 2 ? 4 : 5;
> > +	else
> > +		return 4;
> > +}
> > +
> > +static int skl_max_planes(uint32_t d)
> > +{
> > +	if (IS_GEN10(d) || IS_GEMINILAKE(d) || IS_BROXTON(d))
> > +		return 5;
> > +	else
> > +		return 4;
> > +}
> >  
> >  static void skl_wm_dump(void)
> >  {
> >  	int pipe, plane, level;
> >  	int num_pipes = 3;
> > -	int num_planes = 5;
> > +	int max_planes = skl_max_planes(devid);
> >  	int num_levels = 8;
> >  	uint32_t base_addr = 0x70000, addr, wm_offset;
> > -	uint32_t wm[num_levels][num_pipes][num_planes];
> > -	uint32_t wm_trans[num_pipes][num_planes];
> > -	uint32_t buf_cfg[num_pipes][num_planes];
> > +	uint32_t wm[num_levels][num_pipes][max_planes];
> > +	uint32_t wm_trans[num_pipes][max_planes];
> > +	uint32_t buf_cfg[num_pipes][max_planes];
> >  	uint32_t wm_linetime[num_pipes];
> >  	char reg_name[20];
> >  
> >  	intel_register_access_init(intel_get_pci_device(), 0, drm_fd);
> >  
> >  	for (pipe = 0; pipe < num_pipes; pipe++) {
> > +		int num_planes = skl_num_planes(devid, pipe);
> > +
> >  		wm_linetime[pipe] = read_reg(0x45270 + pipe * 0x4);
> >  
> >  		for (plane = 0; plane < num_planes; plane++) {
> > @@ -173,9 +192,11 @@ static void skl_wm_dump(void)
> >  	}
> >  	printf("\n\n");
> >  
> > -	for (plane = 0; plane < num_planes; plane++) {
> > +	for (plane = 0; plane < max_planes; plane++) {
> >  		for (level = 0; level < num_levels; level++) {
> >  			for (pipe = 0; pipe < num_pipes; pipe++) {
> > +				if (plane >= skl_num_planes(devid, pipe))
> > +					break;
> >  				if (plane == 0)
> >  					snprintf(reg_name, sizeof(reg_name), "CUR_WM_%c_%1d",
> >  						 pipe_name(pipe), level);
> > @@ -190,8 +211,10 @@ static void skl_wm_dump(void)
> >  		printf("\n");
> >  	}
> >  
> > -	for (plane = 0; plane < num_planes; plane++) {
> > +	for (plane = 0; plane < max_planes; plane++) {
> >  		for (pipe = 0; pipe < num_pipes; pipe++) {
> > +			if (plane >= skl_num_planes(devid, pipe))
> > +				break;
> >  			if (plane == 0)
> >  				snprintf(reg_name, sizeof(reg_name), "CUR_WM_TRANS_%c",
> >  					 pipe_name(pipe));
> > @@ -205,8 +228,10 @@ static void skl_wm_dump(void)
> >  	}
> >  	printf("\n");
> >  
> > -	for (plane = 0; plane < num_planes; plane++) {
> > +	for (plane = 0; plane < max_planes; plane++) {
> >  		for (pipe = 0; pipe < num_pipes; pipe++) {
> > +			if (plane >= skl_num_planes(devid, pipe))
> > +				break;
> >  			if (plane == 0)
> >  				snprintf(reg_name, sizeof(reg_name), "CUR_BUF_CFG_%c",
> >  					 pipe_name(pipe));
> > @@ -224,6 +249,7 @@ static void skl_wm_dump(void)
> >  		uint32_t start, end, size;
> >  		uint32_t lines, blocks, enable;
> >  		uint32_t linetime;
> > +		int num_planes = skl_num_planes(devid, pipe);
> >  
> >  		printf("PIPE_%c\n", pipe_name(pipe));
> >  
> 
> 
> 
> 
> 
> 
> 

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx