[Freedreno] [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Marijn Suijten]

On 2023-05-30 23:14:19, Dmitry Baryshkov wrote:
> On Tue, 30 May 2023 at 20:37, Abhinav Kumar <quic_abhinavk at quicinc.com> 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