On 1/23/2025 9:19 PM, Dmitry Baryshkov wrote:
On Thu, Jan 23, 2025 at 01:41:14PM -0800, Abhinav Kumar wrote:
On 1/23/2025 1:32 PM, Abhinav Kumar wrote:
On 12/13/2024 2:14 PM, Dmitry Baryshkov wrote:
Continue migration to the MDSS-revision based checks and replace
DPU_PINGPONG_DSC feature bit with the core_major_ver < 7 check.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_1_14_msm8937.h | 2 --
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_1_15_msm8917.h | 1 -
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_1_16_msm8953.h | 2 --
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_1_7_msm8996.h | 6 ++----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 10
++--------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 --
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 2 +-
7 files changed, 5 insertions(+), 20 deletions(-)
<snip>
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
index 36c0ec775b92036eaab26e1fa5331579651ac27c..49e03ecee9e8b567a3f809b977deb83731006ac0
100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -319,7 +319,7 @@ struct dpu_hw_pingpong
*dpu_hw_pingpong_init(struct drm_device *dev,
c->ops.disable_autorefresh = dpu_hw_pp_disable_autorefresh;
}
- if (test_bit(DPU_PINGPONG_DSC, &cfg->features)) {
+ if (mdss_rev->core_major_ver < 7) {
c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
So far in this series, we replaced the feature bits with >= checks of
core_revisions. That kind of works as usually feature bits get added
after a specific version.
With this patch and later, whenever we use < checks it gets a bit tricky
as we might also need an upper bound. Feature bits gave individual
control of chipsets but generalizing that all chipsets < 7 have PP DSC
is also not correct. I have to really cross-check but there could be
some old chipsets which do not have DSC at all.
This raises another question as well.
what if some register was introduced >= X version but was then dropped in a
newer chipset.
Is it not difficult for the user to go back to the files of each of the
sub-blocks and alter these checks rather than just fixing up the catalog.
Well, the obvious example we are going to have is the CTL_LAYER_EXT4,
but if I understand correctly the whole block is going to be dropped, so
maybe it's not that relevant.
Another example might be CWB, where we are going to have 5.x-7.x and
8.x+ DPU ranges.
Basically, yes, when adding support for a new platform we have to audit
HW blocks. But this applied even beforehand, where new platforms could
be drooping existing regs (8.x+ dropping a part of the TOP region).
Right, I am wondering what is the trade-off here.
1) Feature bits allow selective control for every chipset. IOW, the
specific version is already checked for. I do admit that I have seen
errors about using the correct feature mask sometimes but even with this
change, evey developer will need to go and check every sub-block file
which they might not even know about.
2) core_revision certainly can generalize but we might still need
absolute versions for some of the bits anyway. So the checks may not
always be >= or < but it could also end up with something like
if (major_rev == xxx && minor_rev == yyy)
ops = ops_a;
else if (major_rev == aaa &&& minor_rev == bbb)
ops = ops_b
So the revision check will most likely end up being more complicated
than simple range checks. With each catalog feature bit atleast we are
guaranteed that its applied only to that revision.
I do see in the cover letter, about incorrect feature bits sometimes
used but I am trying to assess the trade-offs even with this approach.
