Re: [igt-dev] [PATCH i-g-t] tools/dpcd_reg: Introduce dump as the default operation

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Oct 18, 2018 at 01:30:03PM +0000, Tarun Vyas wrote:
> On Tue, Oct 16, 2018 at 05:23:59PM -0700, Rodrigo Vivi wrote:
> > On Tue, Oct 09, 2018 at 11:44:18PM -0700, Tarun Vyas wrote:
> > > From: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > 
> > > For now this only imports the registers that were used on
> > > i915's debugfs dpcd dump. Later this could be extended.
> > > 
> > > With this, we should be able to kill i915_dpcd from the
> > > kernel debugfs and rely solely on dpcd_reg for dpcd dumps.
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Tarun Vyas <tarun.vyas@intel.com>
> > 
> > This looks good to me and I believe I could just
> > go there and merge since we have 2 people working together
> > and agreeing here right?

no one seems to disagree, so pushed! :)

> > 
> > DK, comments?
> > 
> > Tarun, do you have plans to make this dump without --device
> > option to print all of available dpcd like my old series was
> > doing?
> >
> Thanks Rodrigo. DK and I discussed dumping dpcds for all the devices, but we weren't sure if we want to open every device that may or may not be connected and hence timeout on a read. So, by default we dump device 0, even if the user doesnt specifies anything. For other devices they can specify the device ids.

My idea on those first patches was to only iterate on the existing files.
dev files will only exist if connector exist.

And then, just try a dummy byte read on it to see if there is really a connected
DP there. If it replies print, otherwise skip.

And also print the connector name, so it gets easier when using
this info for debugging...

> > > ---
> > >  tools/dpcd_reg.c | 86 +++++++++++++++++++++++++++++++++++++++++++-------------
> > >  1 file changed, 66 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/tools/dpcd_reg.c b/tools/dpcd_reg.c
> > > index d577aa55..2761168d 100644
> > > --- a/tools/dpcd_reg.c
> > > +++ b/tools/dpcd_reg.c
> > > @@ -41,19 +41,50 @@
> > >  
> > >  const char aux_dev[] = "/dev/drm_dp_aux";
> > >  
> > > +struct dpcd_block {
> > > +	/* DPCD dump start address. */
> > > +	uint32_t offset;
> > > +	/* DPCD number of bytes to read. If unset, defaults to 1. */
> > > +	size_t count;
> > > +};
> > > +
> > >  struct dpcd_data {
> > >  	int devid;
> > >  	int file_op;
> > > -	uint32_t offset;
> > > +	struct dpcd_block rw;
> > >  	enum command {
> > > -		INVALID = -1,
> > > -		READ = 2,
> > > +		DUMP,
> > > +		READ,
> > >  		WRITE,
> > >  	} cmd;
> > > -	size_t count;
> > >  	uint8_t val;
> > >  };
> > >  
> > > +static const struct dpcd_block dump_list[] = {
> > > +	/* DP_DPCD_REV */
> > > +	{ .offset = 0, .count = 15 },
> > > +	/* DP_PSR_SUPPORT to DP_PSR_CAPS*/
> > > +	{ .offset =  0x70, .count = 2 },
> > > +	/* DP_DOWNSTREAM_PORT_0 */
> > > +	{ .offset =  0x80, .count = 16 },
> > > +	/* DP_LINK_BW_SET to DP_EDP_CONFIGURATION_SET */
> > > +	{ .offset = 0x100, .count = 11 },
> > > +	/* DP_SINK_COUNT to DP_ADJUST_REQUEST_LANE2_3 */
> > > +	{ .offset = 0x200, .count = 8 },
> > > +	/* DP_SET_POWER */
> > > +	{ .offset = 0x600 },
> > > +	/* DP_EDP_DPCD_REV */
> > > +	{ .offset =  0x700 },
> > > +	/* DP_EDP_GENERAL_CAP_1 to DP_EDP_GENERAL_CAP_3 */
> > > +	{ .offset = 0x701, .count = 4 },
> > > +	/* DP_EDP_DISPLAY_CONTROL_REGISTER to DP_EDP_BACKLIGHT_FREQ_CAP_MAX_LSB */
> > > +	{ .offset = 0x720, .count = 16},
> > > +	/* DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET to DP_EDP_DBC_MAXIMUM_BRIGHTNESS_SET */
> > > +	{ .offset =  0x732, .count = 2 },
> > > +	/* DP_PSR_STATUS to DP_PSR_STATUS */
> > > +	{ .offset = 0x2008, .count = 1 },
> > > +};
> > > +
> > >  static void print_usage(void)
> > >  {
> > >  	printf("Usage: dpcd_reg [OPTION ...] COMMAND\n\n");
> > > @@ -103,7 +134,7 @@ static int parse_opts(struct dpcd_data *dpcd, int argc, char **argv)
> > >  				print_usage();
> > >  				return EXIT_FAILURE;
> > >  			}
> > > -			dpcd->count = temp;
> > > +			dpcd->rw.count = temp;
> > >  			break;
> > >  		case 'd':
> > >  			temp = strtol(optarg, &endptr, 10);
> > > @@ -131,7 +162,7 @@ static int parse_opts(struct dpcd_data *dpcd, int argc, char **argv)
> > >  				print_usage();
> > >  				return ERANGE;
> > >  			}
> > > -			dpcd->offset = temp;
> > > +			dpcd->rw.offset = temp;
> > >  			break;
> > >  		case 'v':
> > >  			vflag = 'v';
> > > @@ -147,16 +178,15 @@ static int parse_opts(struct dpcd_data *dpcd, int argc, char **argv)
> > >  		/* Command parsing */
> > >  		case 1:
> > >  			if (strcmp(optarg, "read") == 0) {
> > > -				temp = READ;
> > > +				dpcd->cmd = READ;
> > >  			} else if (strcmp(optarg, "write") == 0) {
> > > -				temp = WRITE;
> > > +				dpcd->cmd = WRITE;
> > >  				dpcd->file_op = O_WRONLY;
> > > -			} else {
> > > +			} else if (strcmp(optarg, "dump") != 0) {
> > >  				fprintf(stderr, "Unrecognized command\n");
> > >  				print_usage();
> > >  				return EXIT_FAILURE;
> > >  			}
> > > -			dpcd->cmd = temp;
> > >  			break;
> > >  		case ':':
> > >  			fprintf(stderr, "Option -%c requires an argument\n",
> > > @@ -170,7 +200,7 @@ static int parse_opts(struct dpcd_data *dpcd, int argc, char **argv)
> > >  		}
> > >  	}
> > >  
> > > -	if ((dpcd->count + dpcd->offset) > (MAX_DP_OFFSET + 1)) {
> > > +	if ((dpcd->rw.count + dpcd->rw.offset) > (MAX_DP_OFFSET + 1)) {
> > >  		fprintf(stderr, "Out of bounds. Count + Offset <= 0x100000\n");
> > >  		return ERANGE;
> > >  	}
> > > @@ -207,7 +237,7 @@ static int dpcd_read(int fd, uint32_t offset, size_t count)
> > >  		ret = EXIT_FAILURE;
> > >  	}
> > >  
> > > -	printf("0x%02x: ", offset);
> > > +	printf("0x%04x: ", offset);
> > >  	for (i = 0; i < pret; i++)
> > >  		printf(" %02x", *(buf + i));
> > >  	printf("\n");
> > > @@ -233,6 +263,23 @@ static int dpcd_write(int fd, uint32_t offset, uint8_t val)
> > >  	return ret;
> > >  }
> > >  
> > > +static int dpcd_dump(int fd)
> > > +{
> > > +	size_t count;
> > > +	int ret, i;
> > > +
> > > +	for (i = 0; i < sizeof(dump_list) / sizeof(dump_list[0]); i++) {
> > > +		count = dump_list[i].count ? dump_list[i].count: 1;
> > > +		ret = dpcd_read(fd, dump_list[i].offset, count);
> > > +		if (ret != EXIT_SUCCESS) {
> > > +			fprintf(stderr, "Dump failed while reading %04x\n",
> > > +				 dump_list[i].offset);
> > > +			return ret;
> > > +		}
> > > +	}
> > > +	return ret;
> > > +}
> > > +
> > >  int main(int argc, char **argv)
> > >  {
> > >  	char dev_name[20];
> > > @@ -241,9 +288,9 @@ int main(int argc, char **argv)
> > >  	struct dpcd_data dpcd = {
> > >  		.devid = 0,
> > >  		.file_op = O_RDONLY,
> > > -		.offset = 0x0,
> > > -		.cmd = INVALID,
> > > -		.count = 1,
> > > +		.rw.offset = 0x0,
> > > +		.rw.count = 1,
> > > +		.cmd = DUMP,
> > >  	};
> > >  
> > >  	ret = parse_opts(&dpcd, argc, argv);
> > > @@ -262,15 +309,14 @@ int main(int argc, char **argv)
> > >  
> > >  	switch (dpcd.cmd) {
> > >  	case READ:
> > > -		ret = dpcd_read(fd, dpcd.offset, dpcd.count);
> > > +		ret = dpcd_read(fd, dpcd.rw.offset, dpcd.rw.count);
> > >  		break;
> > >  	case WRITE:
> > > -		ret = dpcd_write(fd, dpcd.offset, dpcd.val);
> > > +		ret = dpcd_write(fd, dpcd.rw.offset, dpcd.val);
> > >  		break;
> > > +	case DUMP:
> > >  	default:
> > > -		fprintf(stderr, "Please specify a command: read/write.\n");
> > > -		print_usage();
> > > -		ret = EXIT_FAILURE;
> > > +		ret = dpcd_dump(fd);
> > >  		break;
> > >  	}
> > >  
> > > -- 
> > > 2.14.1
> > > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev