Re: [RFC PATCH 5/7] drm/msm/dpu: Document and enable TEAR interrupts on DSI interfaces

[Abhinav Kumar]

On 4/17/2023 12:41 PM, Marijn Suijten wrote:
> On 2023-02-14 09:54:57, Abhinav Kumar wrote:
> [..]
> > > > > > Just wondering, how were the lengths calculated for the INTF blocks?
> > > > > > The values in general seem a little off to me.
> > >
> > > These (for MSM8998) have been taken from downstream specifically; my
> > > series starts using INTF_STATUS at 0x26C which conveniently is the
> > > register right after 0x268, matching the fact that INTF TE and these
> > > registers weren't supported/available yet on MSM8998.
> > >
> > > > > > For example, I'm looking downstream and it seems to me that the length
> > > > > > for the INTF_0 on MSM8998 should be 0x280. Similarly for SC7280, I'm
> > > > > > seeing that length for INTF + tearcheck should be 0x2c4.
> > >
> > > There are many different downstream sources and tags with seemingly
> > > conflicting/confusing information.  For v2 [2] I've picked the highest
> > > register used by the driver which is INTF_TEAR_AUTOREFRESH_CONFIG at
> > > 0x2B4 (but there might always be more registers that don't need to be
> > > poked at by the driver, but contain magic debug information and the
> > > like... those would be useful to capture in the dump going forward).
> > >
> > > [2]: https://github.com/SoMainline/linux/commit/2bbc609dd28aa0bd0a2dede20163e521912d0072
> >
> > Not entirely correct.TEAR_AUTOREFRESH_STATUS is at 0x2c0 for sm8350 and
> > sm8450 as well so 0x2b4 is a bit short. DPU code uses autorefresh status
> > today.Esp after your changes it will use the autorefresh status from
> > intf te which is at offset 0x2c0
>
> Revisiting this, I don't see current DPU code nor downstream 5.4 / 5.10
> SDE techpack on some of my checkouts use this register, only
> INTF_TEAR_AUTOREFRESH_CONFIG at 0x2b4..0x2b7.  Am I missing something
> critical here?

Wow, I lost context since its been months since your last response.

But, I refreshed some of it. You are right, we use the status bits 
present in the INTF_TEAR_AUTOREFRESH_CONFIG and INTF_TEAR_ 
AUTOREFRESH_STATUS is unused.

I got confused between the status bit present in the two registers as 
both relate to autorefresh.

But, the offset of of INTF_TEAR_ AUTOREFRESH_STATUS is still at 0x2c0 as 
i wrote before so 0x2c4 is the accurate length of this block.

And yes, all the blk lengths should be accurate now in the hw catalog 
after the rework and reviews of that rework.

> > > > > We have discussed INTF lengths in [1]. The current understanding of the
> > > > > block lengths can be found at [2]. Please comment there if any of the
> > > > > fixed lengths sounds incorrect to you.
> > > > >
> > > > > [1] https://patchwork.freedesktop.org/patch/522187/
> > > > > [2] https://patchwork.freedesktop.org/patch/522227/
> > > > >
> > > > > [skipped the rest]
> > > >
> > > > Please correct my understanding here, it was agreed to fix intf blocks
> > > > to 0x2c4 here https://patchwork.freedesktop.org/patch/522227/ but I dont
> > > > see this was merged?
> > > >
> > > > It was agreed to first land INTF_TE and then add the higher addresses
> > >
> > > Seems like it, at least if I interpret [3] correctly.  My series adds a
> > > new define that will hardcode _len to 0x2B8 for now, and Dmitry/Konrad
> > > can later extend it to whatever is stated by the correct downstream
> > > source.
> >
> > Like mentioned above it should be 0x2c0 for intf block.
> >
> > If you face any conflicting information in downstream code, you can
> > always check with me on IRC.
>
> Ack, it looks like others landed these changes for me now via the
> catalog rework, so I have just rebased and kept the lengths in.
>
> - Marijn