[PATCH 000/100] Add Vega10 Support

Tue Mar 21 12:12:18 UTC 2017

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.

Marek

On Mar 21, 2017 9:46 AM, "Edward O'Callaghan" <funfunctor at folklore1984.net>
wrote:

>
>
> 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 <alexdeucher at gmail.com
> >>> <mailto:alexdeucher at gmail.com>> wrote:
> >>>
> >>>     On Mon, Mar 20, 2017 at 5:36 PM, Jan Ziak <
> 0xe2.0x9a.0x9b at gmail.com
> >>>     <mailto:0xe2.0x9a.0x9b at 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.
>
> 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
> >> amd-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170321/05056190/attachment.html>