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

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

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 --------------------
> ...
> 

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