On 2023-05-30 23:14:19, Dmitry Baryshkov wrote: > On Tue, 30 May 2023 at 20:37, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > > > > > > > On 5/29/2023 2:36 PM, Marijn Suijten wrote: > > > On 2023-05-24 12:18:09, Abhinav Kumar wrote: > > >> > > >> > > >> On 5/24/2023 2:48 AM, Marijn Suijten wrote: > > >>> On 2023-05-23 13:01:13, Abhinav Kumar wrote: > > >>>> > > >>>> > > >>>> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote: > > >>>>> Drop SSPP-specifig debugfs register dumps in favour of using > > >>>>> debugfs/dri/0/kms or devcoredump. > > >>>>> > > >>>> > > >>>> I did see another series which removes src_blk from the catalog (I am > > >>>> yet to review that one) . Lets assume that one is fine and this change > > >>>> will be going on top of that one right? > > >>> > > >>> It replaces src_blk with directly accessing the blk (non-sub-block) > > >>> directly, because they were overlapping anyway. > > >>> > > >>>> The concern I have with this change is that although I do agree that we > > >>>> should be in favor of using debugfs/dri/0/kms ( i have used it a few > > >>>> times and it works pretty well ), devcoredump does not have the support > > >>>> to dump sub-blocks . Something which we should add with priority because > > >>>> even with DSC blocks with the separation of enc/ctl blocks we need that > > >>>> like I wrote in one of the responses. > > >>>> > > >>>> So the "len" of the blocks having sub-blocks will be ignored in favor of > > >>>> the len of the sub-blocks. > > >>> > > >>> The sub-blocks are not always contiguous with their parent block, are > > >>> they? It's probably better to print the sub-blocks separately with > > >> > > >> Yes, not contiguous otherwise we could have just had them in one big range. > > >> > > >>> clear headers anyway rather than dumping the range parent_blk_base to > > >>> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...). > > >>> > > >>> - Marijn > > >> > > >> When I meant sub-block support to devcoredump, this is how I visualize > > >> them to be printed > > >> > > >> =========================SSPP xxx ======================= > > >> =========================SSPP_CSC =======================(for SSPP_xxx) > > >> =========================SSPP_QSEED =====================(for SSPP_xxx) > > > > > > Yeah something along those lines, though I don't really like the `(for > > > SSPP_xxx)` suffix which we should either drop entirely and make the > > > "heading" less of a "heading" > > > > > > > I wrote that "for SSPP_xxx" to explain the idea, ofcourse it wont be > > part of the print itself. > > > > Without that, it matches what you have shared below. > > > > > > > ========================= SSPP xxx ======================= > > > ... > > > ------------------------- SSPP_CSC ----------------------- > > > ... > > > ------------------------- SSPP_QSEED --------------------- > > > ... > > > > > > And/or inline the numbers: > > > > > > ========================= SSPP xxx ======================= > > > ... > > > ----------------------- SSPP_xxx_CSC --------------------- > > > ... > > > ---------------------- SSPP_xxx_QSEED -------------------- > > > ... > > I'd prefer this format. It eases grepping. Cool. And let's also have spaces around the names :) - Marijn > > > > > > > > sure this is also fine with me. > > > > > Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`) > > > that clearly tells the blocks and sub-blocks apart. > > > > > > - Marijn > > > > -- > With best wishes > Dmitry
