Am 21.03.2017 um 09:45 schrieb Edward O'Callaghan:
On 03/21/2017 05:36 PM, Christian König wrote:
Am 21.03.2017 um 00:38 schrieb Tom St Denis:
On 03/20/2017 06:34 PM, Jan Ziak wrote:
On Mon, Mar 20, 2017 at 10:41 PM, Alex Deucher <alexdeuc...@gmail.com
<mailto:alexdeuc...@gmail.com>> wrote:
On Mon, Mar 20, 2017 at 5:36 PM, Jan Ziak <0xe2.0x9a.0...@gmail.com
<mailto:0xe2.0x9a.0...@gmail.com>> wrote:
> Hi
>
>
https://cgit.freedesktop.org/~agd5f/linux/plain/drivers/gpu/drm/amd/include/asic_reg/vega10/NBIO/nbio_6_1_sh_mask.h?h=amd-staging-4.9&id=9555ef0ba926df25d9a637d0ea21bc0d231c21d2
<https://cgit.freedesktop.org/~agd5f/linux/plain/drivers/gpu/drm/amd/include/asic_reg/vega10/NBIO/nbio_6_1_sh_mask.h?h=amd-staging-4.9&id=9555ef0ba926df25d9a637d0ea21bc0d231c21d2>
>
> The file nbio_6_1_sh_mask.h is uncompressed. It consists from
133884 lines.
> Only generated C/C++ code will be able to utilize the content
of such a file
> efficiently. All hand-written codes combined will be able to
utilize about
> 1% of the file.
>
> Is there a reason why nbio_6_1_sh_mask.h is huge?
That IP block contains a lot of registers. The idea is to open
source
as much IP as possible to facilitate debugging, new features, etc.
Alex
[This email contains long/wide lines and should be viewed on a
sufficiently wide screen]
For example if I open the file in vim and go to line 66952:
#define
DWC_E12MP_PHY_X4_NS_X4_1_LANE1_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
Then abstracting away some of the digits used in the defined identifier
and using egrep:
$ egrep
"\<DWC_E12MP_PHY_X._NS_X._._LANE._DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_.__DTB_SEL__SHIFT\>"
nbio_6_1_sh_mask.h
#define
DWC_E12MP_PHY_X4_NS_X4_0_LANE0_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_0_LANE1_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_0_LANE2_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_0_LANE3_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_0_LANEX_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_1_LANE0_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_1_LANE1_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_1_LANE2_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_1_LANE3_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_1_LANEX_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_2_LANE0_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_2_LANE1_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_2_LANE2_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_2_LANE3_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_2_LANEX_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_3_LANE0_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_3_LANE1_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_3_LANE2_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_3_LANE3_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
#define
DWC_E12MP_PHY_X4_NS_X4_3_LANEX_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_1__DTB_SEL__SHIFT
0x9
The egrep command produced 20 lines.
Instead of the many #define directives, it is a possibility to define
functions such as:
int
DWC_E12MP_PHY_Xa_NS_Xb_c_LANEd_DIG_RX_VCOCAL_RX_VCO_CAL_CTRL_e__DTB_SEL__SHIFT(int
a, int b, int c, int d, int e) __attribute__((pure));
I suppose the file nbio_6_1_sh_mask.h is the output of a tool (it is a
generated file). It is an option to modify the tool to output C
functions with proper input guards instead of #define directives.
The registers are generated by the HW team and then filtered down to
become what we release publicly. It is machine generated very likely
from the RTL that specifies the hardware itself.
Yes, exactly that. And it was actually quite some work to get to this
point.
Generally speaking if a class of registers share masks/offsets the
lowest (zero'th) is used in programming and offsets are used when
selecting the correct MMIO address to use specific instances.
The problem is that the files we get from the HW team describe the
register block already broken down to the memory mappings. E.g. when an
RTL block is instantiated N times you get N times the same definition.
Having these enumerated though is handy for tools like UMR which would
decode to the correct instance of the register (you could even see
that by watching the logscan via umr). So we make use of them fairly
efficiently. UMR reads the headers to create the arrays of
registers/bitfields which if they were computed at runtime (via helper
functions) would be harder to parse (and could amount to the halting
problem...).
What is in the NBIO header today is what was IP cleared but in theory
it could be filtered down more simply to reduce the unused LOCs in
future kernels.
They don't make for good bedtime reading but imho you'd rather have
more headers not less :-)
Manually merging that back into single definitions is not really an
option, but what we could do is releasing the full blown headers
somewhere and only limit what we push into the linux kernel to actually
used ones.
Hi Christian,
I have to say I prefer your idea here and perhaps they could even become
part of the UMR repo, not sure? May I suggest the reduction to some like
a `kernel-native-dialect` could be automated with a clang plugin. It
seems like heaps of work however maybe not really, clang provides a API
to grammatically do some AST transformers and auto-rewrite large chunks
of code given some rules.
Using clang sounds like overkill to me, but we could write an
awk/sed/perl script to do the job.
Another problem is that we internally doesn't necessary want this during
bringup, so the question is when to apply it.
I also agree with Tom, its certainly nice to have them regardless, just
perhaps the kernel mainline repo is the wrong place for the kitchen sink.
It's not only nice to have, but a must have to publish the headers.
We have put a lot of work into changing the workflow at AMD from
"releasing only what we have to" toward "releasing everything we can".
Loosing that is NOT an option, but I clearly agree that the kernel
mainline repo is not necessary the best place for it.
Using UMR for this is actually a quite nifty idea if you ask me.
Regards,
Christian.
Just my thoughts,
Kind Regards,
Edward.
Christian.
Tom
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
