On 27/07/2023 23:03, Marijn Suijten wrote:
On 2023-07-27 19:20:58, Dmitry Baryshkov wrote:
The DPU_PINGPONG_TE bit is set for all PINGPONG blocks on DPU < 5.0.
Rather than checking for the flag, check for the presense of the
corresponding interrupt line.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
That's a smart use of the interrupt field. I both like it, and I do
not. While we didn't do any validation for consistency previously, this
means we now have multiple ways of controlling available "features":
- Feature flags on hardware blocks;
- Presence of certain IRQs;
- DPU core revision.
I hesitated here too. For INTF it is clear that there is no other good
way to check for the TE feature. For PP we can just check for the DPU
revision.
Maybe that is more confusing to follow? Regardless of that I'm
convinced that this patch does what it's supposed to and gets rid of
some ambiguity. Maybe a comment above the IF explaining the "PP TE"
feature could alleviate the above concerns thoo. Hence:
Reviewed-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 9298c166b213..912a3bdf8ad4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -296,7 +296,7 @@ struct dpu_hw_pingpong *dpu_hw_pingpong_init(const struct dpu_pingpong_cfg *cfg,
c->idx = cfg->id;
c->caps = cfg;
- if (test_bit(DPU_PINGPONG_TE, &cfg->features)) {
+ if (cfg->intr_rdptr) {
c->ops.enable_tearcheck = dpu_hw_pp_enable_te;
c->ops.disable_tearcheck = dpu_hw_pp_disable_te;
c->ops.connect_external_te = dpu_hw_pp_connect_external_te;
--
2.39.2
--
With best wishes
Dmitry
