* [PATCH 2/2] intel_reg_dumper: Decode FDI_RX_IIR
2012-08-31 13:45 [PATCH 1/2] intel_reg_dumper: Add a single register decode mode Damien Lespiau
@ 2012-08-31 13:45 ` Damien Lespiau
2012-09-02 1:48 ` [PATCH 1/2] intel_reg_dumper: Add a single register decode mode Ben Widawsky
2012-09-03 7:10 ` Jani Nikula
2 siblings, 0 replies; 6+ messages in thread
From: Damien Lespiau @ 2012-08-31 13:45 UTC (permalink / raw)
To: intel-gfx
From: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
tools/intel_reg_dumper.c | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/tools/intel_reg_dumper.c b/tools/intel_reg_dumper.c
index 098affa..d831733 100644
--- a/tools/intel_reg_dumper.c
+++ b/tools/intel_reg_dumper.c
@@ -1310,6 +1310,33 @@ DEBUGSTRING(ironlake_debug_fdi_rx_misc)
snprintf(result, len, "FDI Delay %d", val & ((1 << 13) - 1));
}
+DEBUGSTRING(ironlake_debug_fdi_rx_iir)
+{
+ const char *aligned = val & FDI_RX_INTER_LANE_ALIGN ? "aligned " : "";
+ const char *symbol_locked = val & FDI_RX_SYMBOL_LOCK ?
+ "symbol locked " : "";
+ const char *bit_locked = val & FDI_RX_BIT_LOCK ? "bit locked " : "";
+ const char *tp2_failed = val & FDI_RX_TRAIN_PATTERN_2_FAIL ?
+ "tp2 failed " : "";
+ const char *fs_err = val & FDI_RX_FS_CODE_ERR ? "FS code error " : "";
+ const char *fe_err = val & FDI_RX_FE_CODE_ERR ? "FE code error " : "";
+ const char *ber_high = val & FDI_RX_SYMBOL_ERR_RATE_ABOVE ?
+ "BER high " : "";
+ const char *hdcp_fail = val & FDI_RX_HDCP_LINK_FAIL ?
+ "HDCP link failed " : "";
+ const char *fifo_overflow = val & FDI_RX_PIXEL_FIFO_OVERFLOW ?
+ "FIFO overflow " : "";
+ const char *xclock_overflow = val & FDI_RX_CROSS_CLOCK_OVERFLOW ?
+ "XClock overflow " : "";
+ const char *symbol_overflow = val & FDI_RX_SYMBOL_QUEUE_OVERFLOW ?
+ "Symbol overlfow " : "";
+
+ snprintf(result, len, "%s%s%s%s%s%s%s%s%s%s%s",
+ aligned, symbol_locked, bit_locked, tp2_failed, fs_err, fe_err,
+ ber_high, hdcp_fail, fifo_overflow, xclock_overflow,
+ symbol_overflow);
+}
+
DEBUGSTRING(ironlake_debug_transconf)
{
const char *enable = val & TRANS_ENABLE ? "enable" : "disable";
@@ -1788,9 +1815,9 @@ static struct reg_debug ironlake_debug_regs[] = {
DEFINEREG(FDI_PLL_CTL_1),
DEFINEREG(FDI_PLL_CTL_2),
- DEFINEREG(FDI_RXA_IIR),
+ DEFINEREG2(FDI_RXA_IIR, ironlake_debug_fdi_rx_iir),
DEFINEREG(FDI_RXA_IMR),
- DEFINEREG(FDI_RXB_IIR),
+ DEFINEREG2(FDI_RXB_IIR, ironlake_debug_fdi_rx_iir),
DEFINEREG(FDI_RXB_IMR),
DEFINEREG2(PCH_ADPA, i830_debug_adpa),
--
1.7.11.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] intel_reg_dumper: Add a single register decode mode
2012-08-31 13:45 [PATCH 1/2] intel_reg_dumper: Add a single register decode mode Damien Lespiau
2012-08-31 13:45 ` [PATCH 2/2] intel_reg_dumper: Decode FDI_RX_IIR Damien Lespiau
@ 2012-09-02 1:48 ` Ben Widawsky
2012-09-03 7:06 ` Jani Nikula
2012-09-03 15:35 ` Lespiau, Damien
2012-09-03 7:10 ` Jani Nikula
2 siblings, 2 replies; 6+ messages in thread
From: Ben Widawsky @ 2012-09-02 1:48 UTC (permalink / raw)
To: Damien Lespiau; +Cc: intel-gfx
On 2012-08-31 06:45, Damien Lespiau wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> From time to time, one would like to decode a register value that
> have
> been captured at a certain point in time (and say printed out with a
> printk). intel_reg_dumper has all the knowledge to do that and this
> patch adds a way to ask it to decode a value.
>
> Example usage:
>
> $ ./tools/intel_reg_dumper PCH_PP_CONTROL 0xabcd0002
> PCH_PP_CONTROL: 0xabcd0002 (blacklight disabled, power...
>
> v2: friendlier invocation (Chris Wilson)
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Back to my earlier complaint from the other patch... If the names don't
match the docs then you forced to memorize our historically awful naming
scheme. More useful to me would be to decode a register at an offset for
a given generation.
> ---
> tools/intel_reg_dumper.c | 99
> +++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 86 insertions(+), 13 deletions(-)
>
> diff --git a/tools/intel_reg_dumper.c b/tools/intel_reg_dumper.c
> index b49d967..098affa 100644
> --- a/tools/intel_reg_dumper.c
> +++ b/tools/intel_reg_dumper.c
> @@ -1911,26 +1911,33 @@ static struct reg_debug i945gm_mi_regs[] = {
> DEFINEREG(ECOSKPD),
> };
>
> +static void
> +_intel_dump_reg(struct reg_debug *reg, uint32_t val)
> +{
> + char debug[1024];
> +
> + if (reg->debug_output != NULL) {
> + reg->debug_output(debug, sizeof(debug), reg->reg, val);
> + printf("%30.30s: 0x%08x (%s)\n",
> + reg->name,
> + (unsigned int)val, debug);
> + } else {
> + printf("%30.30s: 0x%08x\n", reg->name,
> + (unsigned int)val);
> + }
> +}
> +
> #define intel_dump_regs(regs) _intel_dump_regs(regs,
> ARRAY_SIZE(regs))
>
> static void
> _intel_dump_regs(struct reg_debug *regs, int count)
> {
> - char debug[1024];
> int i;
>
> for (i = 0; i < count; i++) {
> uint32_t val = INREG(regs[i].reg);
>
> - if (regs[i].debug_output != NULL) {
> - regs[i].debug_output(debug, sizeof(debug), regs[i].reg, val);
> - printf("%30.30s: 0x%08x (%s)\n",
> - regs[i].name,
> - (unsigned int)val, debug);
> - } else {
> - printf("%30.30s: 0x%08x\n", regs[i].name,
> - (unsigned int)val);
> - }
> + _intel_dump_reg(®s[i], val);
> }
> }
>
> @@ -1964,6 +1971,54 @@ static struct reg_debug gen6_rp_debug_regs[] =
> {
> DEFINEREG(GEN6_PMINTRMSK),
> };
>
> +#define DECLARE_REGS(r) { .regs = r, .count = ARRAY_SIZE(r) }
> +static struct {
> + struct reg_debug *regs;
> + int count;
> +} known_registers[] = {
> + DECLARE_REGS(ironlake_debug_regs),
> + DECLARE_REGS(i945gm_mi_regs),
> + DECLARE_REGS(intel_debug_regs),
> + DECLARE_REGS(gen6_rp_debug_regs),
> + DECLARE_REGS(haswell_debug_regs)
> +};
> +#undef DECLARE_REGS
> +
> +static struct reg_debug *
> +find_register_by_name(struct reg_debug *regs, int count,
> + const char *name)
> +{
> + int i;
> +
> + for (i = 0; i < count; i++)
> + if (strcmp(name, regs[i].name) == 0)
> + return ®s[i];
> +
> + return NULL;
> +}
> +
> +static void
> +decode_register(const char *name, uint32_t val)
> +{
> + int i;
> + struct reg_debug *reg = NULL;
> +
> + for (i = 0; i < ARRAY_SIZE(known_registers); i++) {
> + reg = find_register_by_name(known_registers[i].regs,
> + known_registers[i].count,
> + name);
> + if (reg)
> + break;
> + }
> +
> + if (!reg) {
> + fprintf(stderr, "Unknown register: %s\n", name);
> + return;
> + }
> +
> + _intel_dump_reg(reg, val);
> +}
> +
> static void
> intel_dump_other_regs(void)
> {
> @@ -2172,6 +2227,7 @@ intel_dump_other_regs(void)
> static void print_usage(void)
> {
> printf("Usage: intel_reg_dumper [options] [file]\n"
> + " intel_reg_dumper [options] register value\n"
> "Options:\n"
> " -d id when a dump file is used, use 'id' as device id
> (in "
> "hex)\n"
> @@ -2181,8 +2237,9 @@ static void print_usage(void)
> int main(int argc, char** argv)
> {
> struct pci_device *pci_dev;
> - int opt;
> - char *file = NULL;
> + int opt, n_args;
> + char *file = NULL, *reg_name = NULL;
> + uint32_t reg_val;
>
> while ((opt = getopt(argc, argv, "d:h")) != -1) {
> switch (opt) {
> @@ -2197,8 +2254,24 @@ int main(int argc, char** argv)
> return 1;
> }
> }
> - if (optind < argc)
> +
> + n_args = argc - optind;
> + if (n_args == 1) {
> file = argv[optind];
> + } else if (n_args == 2) {
> + reg_name = argv[optind];
> + reg_val = strtoul(argv[optind + 1], NULL, 0);
> + } else if (n_args) {
> + print_usage();
> + return 1;
> + }
> +
> + /* the tool operates in "single" mode, decode a single register
> given
> + * on the command line: intel_reg_dumper PCH_PP_CONTROL 0xabcd0002
> */
> + if (reg_name) {
> + decode_register(reg_name, reg_val);
> + return 0;
> + }
>
> if (file) {
> intel_map_file(file);
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] intel_reg_dumper: Add a single register decode mode
2012-09-02 1:48 ` [PATCH 1/2] intel_reg_dumper: Add a single register decode mode Ben Widawsky
@ 2012-09-03 7:06 ` Jani Nikula
2012-09-03 15:35 ` Lespiau, Damien
1 sibling, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2012-09-03 7:06 UTC (permalink / raw)
To: Ben Widawsky, Damien Lespiau; +Cc: intel-gfx
On Sun, 02 Sep 2012, Ben Widawsky <ben@bwidawsk.net> wrote:
> On 2012-08-31 06:45, Damien Lespiau wrote:
>> From: Damien Lespiau <damien.lespiau@intel.com>
>>
>> From time to time, one would like to decode a register value that
>> have
>> been captured at a certain point in time (and say printed out with a
>> printk). intel_reg_dumper has all the knowledge to do that and this
>> patch adds a way to ask it to decode a value.
>>
>> Example usage:
>>
>> $ ./tools/intel_reg_dumper PCH_PP_CONTROL 0xabcd0002
>> PCH_PP_CONTROL: 0xabcd0002 (blacklight disabled, power...
>>
>> v2: friendlier invocation (Chris Wilson)
>>
>> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
>
> Back to my earlier complaint from the other patch... If the names don't
> match the docs then you forced to memorize our historically awful naming
> scheme. More useful to me would be to decode a register at an offset for
> a given generation.
I think both would be useful. So how about supporting both? Concrete
suggestion:
1) If strtoul eats whole of register name, it's register offset. Print
*all* matches across *all* generations.
2) If it's a register name, print *all* partial matches (from beginning
of string) across *all* generations.
3) Include generation and register offset in the output:
Gen5: PCH_PP_CONTROL (0xc7204): 0xabcd0002 (blacklight disabled, power...
This would help in cross checking historically awful naming against
register offsets, and help in checking for differences between registers
and their values across generations.
These patches could also go in first, and the above added later.
BR,
Jani.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] intel_reg_dumper: Add a single register decode mode
2012-09-02 1:48 ` [PATCH 1/2] intel_reg_dumper: Add a single register decode mode Ben Widawsky
2012-09-03 7:06 ` Jani Nikula
@ 2012-09-03 15:35 ` Lespiau, Damien
1 sibling, 0 replies; 6+ messages in thread
From: Lespiau, Damien @ 2012-09-03 15:35 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Sun, Sep 2, 2012 at 2:48 AM, Ben Widawsky <ben@bwidawsk.net> wrote:
> On 2012-08-31 06:45, Damien Lespiau wrote:
> Back to my earlier complaint from the other patch... If the names don't
> match the docs then you forced to memorize our historically awful naming
> scheme. More useful to me would be to decode a register at an offset for a
> given generation.
Well, renaming the registers sounds a bit tedious and will break the
diffs between two dumps across the change (don't know if it's a big
deal). Hopefully the new little features suggested by Jani make the
single register decode more useful for you.
--
Damien
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] intel_reg_dumper: Add a single register decode mode
2012-08-31 13:45 [PATCH 1/2] intel_reg_dumper: Add a single register decode mode Damien Lespiau
2012-08-31 13:45 ` [PATCH 2/2] intel_reg_dumper: Decode FDI_RX_IIR Damien Lespiau
2012-09-02 1:48 ` [PATCH 1/2] intel_reg_dumper: Add a single register decode mode Ben Widawsky
@ 2012-09-03 7:10 ` Jani Nikula
2 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2012-09-03 7:10 UTC (permalink / raw)
To: Damien Lespiau, intel-gfx
On Fri, 31 Aug 2012, Damien Lespiau <damien.lespiau@gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> From time to time, one would like to decode a register value that have
> been captured at a certain point in time (and say printed out with a
> printk). intel_reg_dumper has all the knowledge to do that and this
> patch adds a way to ask it to decode a value.
>
> Example usage:
>
> $ ./tools/intel_reg_dumper PCH_PP_CONTROL 0xabcd0002
> PCH_PP_CONTROL: 0xabcd0002 (blacklight disabled, power...
>
> v2: friendlier invocation (Chris Wilson)
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
> tools/intel_reg_dumper.c | 99 +++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 86 insertions(+), 13 deletions(-)
>
> diff --git a/tools/intel_reg_dumper.c b/tools/intel_reg_dumper.c
> index b49d967..098affa 100644
> --- a/tools/intel_reg_dumper.c
> +++ b/tools/intel_reg_dumper.c
> @@ -1911,26 +1911,33 @@ static struct reg_debug i945gm_mi_regs[] = {
> DEFINEREG(ECOSKPD),
> };
>
> +static void
> +_intel_dump_reg(struct reg_debug *reg, uint32_t val)
> +{
> + char debug[1024];
> +
> + if (reg->debug_output != NULL) {
> + reg->debug_output(debug, sizeof(debug), reg->reg, val);
> + printf("%30.30s: 0x%08x (%s)\n",
> + reg->name,
> + (unsigned int)val, debug);
Is the cast really required? val is already unsigned. Ditto below in a
few places.
> + } else {
> + printf("%30.30s: 0x%08x\n", reg->name,
> + (unsigned int)val);
> + }
> +}
> +
> #define intel_dump_regs(regs) _intel_dump_regs(regs, ARRAY_SIZE(regs))
>
> static void
> _intel_dump_regs(struct reg_debug *regs, int count)
> {
> - char debug[1024];
> int i;
>
> for (i = 0; i < count; i++) {
> uint32_t val = INREG(regs[i].reg);
>
> - if (regs[i].debug_output != NULL) {
> - regs[i].debug_output(debug, sizeof(debug), regs[i].reg, val);
> - printf("%30.30s: 0x%08x (%s)\n",
> - regs[i].name,
> - (unsigned int)val, debug);
> - } else {
> - printf("%30.30s: 0x%08x\n", regs[i].name,
> - (unsigned int)val);
> - }
> + _intel_dump_reg(®s[i], val);
> }
> }
>
> @@ -1964,6 +1971,54 @@ static struct reg_debug gen6_rp_debug_regs[] = {
> DEFINEREG(GEN6_PMINTRMSK),
> };
>
> +#define DECLARE_REGS(r) { .regs = r, .count = ARRAY_SIZE(r) }
> +static struct {
> + struct reg_debug *regs;
> + int count;
> +} known_registers[] = {
> + DECLARE_REGS(ironlake_debug_regs),
> + DECLARE_REGS(i945gm_mi_regs),
> + DECLARE_REGS(intel_debug_regs),
> + DECLARE_REGS(gen6_rp_debug_regs),
> + DECLARE_REGS(haswell_debug_regs)
> +};
> +#undef DECLARE_REGS
> +
> +static struct reg_debug *
> +find_register_by_name(struct reg_debug *regs, int count,
> + const char *name)
> +{
> + int i;
> +
> + for (i = 0; i < count; i++)
> + if (strcmp(name, regs[i].name) == 0)
> + return ®s[i];
strcasecmp for ease of use?
BR,
Jani.
> +
> + return NULL;
> +}
> +
> +static void
> +decode_register(const char *name, uint32_t val)
> +{
> + int i;
> + struct reg_debug *reg = NULL;
> +
> + for (i = 0; i < ARRAY_SIZE(known_registers); i++) {
> + reg = find_register_by_name(known_registers[i].regs,
> + known_registers[i].count,
> + name);
> + if (reg)
> + break;
> + }
> +
> + if (!reg) {
> + fprintf(stderr, "Unknown register: %s\n", name);
> + return;
> + }
> +
> + _intel_dump_reg(reg, val);
> +}
> +
> static void
> intel_dump_other_regs(void)
> {
> @@ -2172,6 +2227,7 @@ intel_dump_other_regs(void)
> static void print_usage(void)
> {
> printf("Usage: intel_reg_dumper [options] [file]\n"
> + " intel_reg_dumper [options] register value\n"
> "Options:\n"
> " -d id when a dump file is used, use 'id' as device id (in "
> "hex)\n"
> @@ -2181,8 +2237,9 @@ static void print_usage(void)
> int main(int argc, char** argv)
> {
> struct pci_device *pci_dev;
> - int opt;
> - char *file = NULL;
> + int opt, n_args;
> + char *file = NULL, *reg_name = NULL;
> + uint32_t reg_val;
>
> while ((opt = getopt(argc, argv, "d:h")) != -1) {
> switch (opt) {
> @@ -2197,8 +2254,24 @@ int main(int argc, char** argv)
> return 1;
> }
> }
> - if (optind < argc)
> +
> + n_args = argc - optind;
> + if (n_args == 1) {
> file = argv[optind];
> + } else if (n_args == 2) {
> + reg_name = argv[optind];
> + reg_val = strtoul(argv[optind + 1], NULL, 0);
> + } else if (n_args) {
> + print_usage();
> + return 1;
> + }
> +
> + /* the tool operates in "single" mode, decode a single register given
> + * on the command line: intel_reg_dumper PCH_PP_CONTROL 0xabcd0002 */
> + if (reg_name) {
> + decode_register(reg_name, reg_val);
> + return 0;
> + }
>
> if (file) {
> intel_map_file(file);
> --
> 1.7.11.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread