[igt-dev] [PATCH i-g-t] tools/dpcd_reg: Introduce dump as the default operation
Tarun Vyas
tarun.vyas at intel.com
Thu Oct 18 18:34:39 UTC 2018
On Thu, Oct 18, 2018 at 11:19:38AM -0700, Rodrigo Vivi wrote:
> 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 at 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 at intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > Signed-off-by: Tarun Vyas <tarun.vyas at 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...
>
Oh ok. I'll integrate the last patch in your series to dump all the available devices, as the default behavior.
Thanks,
Tarun
> > > > ---
> > > > 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
> > > >
More information about the igt-dev
mailing list