<div dir="auto">It would be better to keep the headers as-is, because changing them wouldn't fix anybody's problems, but it would take time from us that we don't have.<div dir="auto"> </div><div dir="auto">Marek</div></div><div class="gmail_extra"> <div class="gmail_quote">On Mar 21, 2017 9:46 AM, "Edward O'Callaghan" <<a href="mailto:funfunctor@folklore1984.net">funfunctor@folklore1984.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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 <<a href="mailto:alexdeucher@gmail.com">alexdeucher@gmail.com</a> >>> <mailto:<a href="mailto:alexdeucher@gmail.com">alexdeucher@gmail.com</a>>> wrote: >>> >>> On Mon, Mar 20, 2017 at 5:36 PM, Jan Ziak <<a href="mailto:0xe2.0x9a.0x9b@gmail.com">0xe2.0x9a.0x9b@gmail.com</a> >>> <mailto:<a href="mailto:0xe2.0x9a.0x9b@gmail.com">0xe2.0x9a.0x9b@gmail.com</a>>> wrote: >>> > Hi >>> > >>> > >>> <a href="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" rel="noreferrer" target="_blank">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</a> >>> >>> <<a href="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" rel="noreferrer" target="_blank">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</a>> >>> >>> > >>> > 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. 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. Just my thoughts, Kind Regards, Edward. > > Christian. > > >> >> Tom >> _______________________________________________ >> amd-gfx mailing list >> <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> >> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> > > > _______________________________________________ > amd-gfx mailing list > <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> _______________________________________________ amd-gfx mailing list <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </blockquote></div></div>