Re: [RFC PATCH i-g-t 0/1] April Tools: complete intel reg tool overhaul

Daniel Vetter
2015-04-07T08:35:00Z

On Wed, Apr 01, 2015 at 09:37:22PM +0300, Jani Nikula wrote:
> Hi all -
>
> I think having several overlapping register read/write/dump/etc. tools
> is just silly. I've been working on unsillifying the situation a bit on
> the background, in the idle moments ("while my kernel's compiling").
>
> Here's what I got: a replacement for
> intel_reg_dumper
> quick_dump.py
> intel_reg_read
> intel_reg_write
> intel_iosf_sb_read
> intel_iosf_sb_write
> intel_dpio_read
> intel_dpio_write
> intel_reg_snapshot
> intel_vga_read
> intel_vga_write
>
> I'm calling it "intel_reg". It's a busybox-like tool that understands
> the subcommands
> dump
> read
> write
> decode
> snapshot
>
> The read, write and decode subcommands understand register references in
> the form [(PORTNAME|PORTNUM|MMIO-OFFSET):](REGNAME|REGADDR) letting you
> address all registers on all platforms. For example,
> $ intel_reg read DPIO:TX2_CH0_DW10
> $ intel_reg write 0x70000 0xffff
>
> The register "spec", i.e. the registers known by name and their mapping
> to address, sideband port or mmio offset, is completely separated from
> the decoding. The spec can be provided through quick_dump.py compatible
> files (although it should not be difficult to add other formats). Quick
> addition of new registers to dump is just as easy as for
> quick_dump.py. As fallback, all the registers currently known by
> intel_reg_dumper are built-in.
>
> The decoding is done based on the resolved address, initially using the
> same functionality currently available in intel_reg_dumper.
>
> Now all the decoding and name<->address mapping is available for *all*
> subcommands. All subcommands use the same output format (which,
> admittedly, I didn't polish yet, but that's a few lines of printfs). For
> dumping a bunch of registers with with --count, we also get the names
> for all:
> $ intel_reg read --count 2 BLC_PWM_CPU_CTL2
> 0x00048250 (BLC_PWM_CPU_CTL2): 0x80000000 enable 1, pipe A
> 0x00048254 (BLC_PWM_CPU_CTL): 0x00000206 cycle 518
>
> intel_reg_decode.c is mostly a copy of intel_reg_dumper.c, otherwise
> this is fairly small amount of code for the functionality, compared to
> everything it's aiming to replace.
>
> It's mostly done, probably with plenty of bikeshedding potential, so I
> stopped short from the polishing stage. Let's hear it.
>
> TODO consists of eating a previously done dump and re-decoding the
> values with possibly updated decode code, bash completion for the
> commands and register names, and also replacing intel_audio_dump which
> has become a monster.

Tbh I didn't read through the entire thing, but imo having a unified
interface is very much required and this is awesome work. Imo better to
push this and ignore any requests for color changes. One thing is missing
thought: A bunch of patches on top which replace all the current tools
with bash scripts which calls a suitable subcommand of intel_reg.
Otherwise we'll keep adding divergent changes all over. But with that imo
just go ahead an push it.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation