On 4/25/2023 1:43 PM, Marijn Suijten wrote:
On 2023-04-25 09:47:30, Abhinav Kumar wrote:
On 4/25/2023 9:33 AM, Marijn Suijten wrote:
On 2023-04-25 09:18:58, Abhinav Kumar wrote:
On 4/24/2023 11:54 PM, Marijn Suijten wrote:
On 2023-04-24 16:09:45, Abhinav Kumar wrote:
<snip>
dither block should be present on many other chipsets too but looks like
on sm8550 was enabling it. Not sure how it was validated there. But we
are enabling dither, even other chipsets have this block.
Correct, they all seem to have it starting at sdm845. My patch message
seems to lack the word "exclusively" as the PP on sm8550 appears to
exclusively contain a DITHER subblock (unless other blocks are available
that simply aren't supported within this driver yet) and no other
registers. Hence this aptly named macro exist to emit just the feature
bitflag for that and a .len of zero.
I think after the TE blocks were moved to INTF, dither is the only
sub-block for all Ping-Pongs not just in sm8550.
So you are asking / leaving context to make all >= 5.0.0 pingpong blocks
use this macro with only a single DITHER sblk in PP?
As far as I recall SM8550 is the first SoC to use zero registers in PP,
which is specifically what this macro takes care of too. Then, there
are only a few SoCs downstream still (erroneously?) referencing TE2 as
the only other sub-blk, those SoCs still use sdm845_pp_sblk_te.
So, what I didnt follow is why should sm8450 use PP_BLK_TE Vs sm8550
should use PP_BLK_DIPHER?
Atleast for those two, both should be using PP_BLK_DIPHER.
Thats what I was trying to note here.
This isnt even right as there is no PP_BLK_TE in sm8450.
SM8450 doesn't use PP_BLK_TE (TE2) anymore since the second patch in
this series. If you think it should use the DITHER (not DIPHER!) macro
instead of the regular PP_BLK with a size of 0xd4, we can do that in
another patch as that's not strictly related to this series.
Yes, thanks for pointing the TE2 was removed in the prev patch of this
series for sm8450. I was just focusing too much on this patch.
And Yes, I think we should use the DIPHER ..... oh sorry .... DITHER ;)
Yes, it can go as a different series, like I already wrote many times in
this.
Thanks, that'd be great. I wasn't sure at this point what you wanted to
be changed here, after commenting on a typo fix rather than i.e. patch 2
that deals with the TE2 sub-block of PP :)
The reason I commented on this patch is because all the discussion so
far was surrounding the PP_BLK_DITHER macro which was being touched in
this patch.
So even now, we found out about sm8450 and sm8550 because of the
question that why sm8550 alone should use PP_BLK_DITHER and not sm8450.
This patch led to all the discussion about PP_BLK_DITHER.
Even though it was just a typo fix patch, it uncovered deeper issues in
catalog about why PP_BLK_DITHER wasnt used more often.
But atleast now, someone will remember to do it.
Note that that's the only difference between these macros. The size
becomes 0 but the .features mask is the same (SM8450 uses
PINGPONG_SM8150_MASK).
These patches are anyway already distracting from my series, but were
easier to do in one go as I was touching the PP and INTF catalog blocks
regardless.
While at it, perhaps we should check if the version and offset for the
DITHER block are correct? SM8450 uses SDM845 sblk definitions.
Yes I already checked. the version and offset of dither are same between
sm8450 and sm8550.
Thanks for checking, so then sm8450 is wrong on multiple occasions.
Let's check all other SoCs that use sdm845_pp_sblk whether they should
have used sc7280_pp_sblk instead.
- Marijn
