Re: [PATCH 1/2] drm/msm/dpu: fix DSC 1.2 block lengths

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23/06/2023 22:37, Abhinav Kumar wrote:


On 6/23/2023 4:37 AM, Dmitry Baryshkov wrote:
On 23/06/2023 09:54, Marijn Suijten wrote:
On 2023-06-22 22:47:04, Abhinav Kumar wrote:
On 6/22/2023 6:37 PM, Dmitry Baryshkov wrote:
All DSC_BLK_1_2 declarations incorrectly pass 0x29c as the block length.
This includes the common block itself, enc subblocks and some empty
space around. Change that to pass 0x4 instead, the length of common
register block itself.

Fixes: 0d1b10c63346 ("drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>

There is no need of a fixes tag for this.

This is not a bug but was intentional.

Till we added sub-block parsing support we had to dump the whole block.

And hence I would suggest this change should be merged after the
sub-block parsing change otherwise we wont have full register dumps for DSC.

This was indeed intentional, we discussed it in [1].

In fact I asked to make it 0xf00 + 0x10 or 0xf80 + 0x10 to also cover
the CTL registers, but that change didn't make it through.  0x29c is an
arbitrary number that I have no clue what it was based on.

This should have been NAKed. or at least TODOed.


Its is not an arbitrary number. Thats an incorrect comment.

Its 4 more than the encoder's last offset which is 0x298 + 0x4 = 0x29c.

There was nothing to NAK or TODO here.

We do not include sub-blocks in the main block area. The SSPP's SRC blocks were cleaned up for this reason. The ENC registers are definitely a sub-block (and are described this way). There should have been a "TODO: reduce block length to 0x4 after adding sub-blocks to dump" comment.



[1]: https://lore.kernel.org/linux-arm-msm/y2whfntyo2rbrg3taazjdw5sijle6k6swzl4uutcxm6tmuayh4@uxdur74uasua/

- Marijn


--
With best wishes
Dmitry




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux