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

[Tarun Vyas <tarun.vyas@intel.com>]

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?
> 
> 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.
> > ---
> >  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