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

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

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)
etc

OR for DSC

========================DSC_xxx ==========================
========================DSC_CTL ========================== (for DSC_xxx)
========================DSC_ENC ===========================(for DSC_xxx)

This is clear enough headers.

> 
>> If we remove this without adding that support first, its a loss of debug
>> functionality.
>>
>> Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
>> different way?
> 
> <snip>