On 05/04/2023 03:39, Abhinav Kumar wrote:
On 4/4/2023 5:33 PM, Dmitry Baryshkov wrote:
On 05/04/2023 01:12, Abhinav Kumar wrote:
On 4/4/2023 6:05 AM, Dmitry Baryshkov wrote:
On sm8450 platform the CTL_0 doesn't differ from the rest of CTL
blocks,
so switch it to CTL_SC7280_MASK too.
Some background: original commit 100d7ef6995d ("drm/msm/dpu: add
support
for SM8450") had all (relevant at that time) bit spelled individually.
Then commit 0e91bcbb0016 ("drm/msm/dpu: Add SM8350 to hw catalog"),
despite being a mismerge, correctly changed all other CTL entries to
use
CTL_SC7280_MASK, except CTL_0.
I think having it spelled individually is better. If we start using
one chipset's mask for another, we are again going down the same path
of this becoming one confused file.
So, even though I agree that 0e91bcbb0016 ("drm/msm/dpu: Add SM8350
to hw catalog") corrected the mask to re-use sc7280, with the
individual catalog file, its better to have it separate and spelled
individually.
This change is not heading in the direction of the rest of the series.
I didn't create duplicates of all the defines. This is done well in
the style of patch37. I'm not going to add all per-SoC feature masks.
Yes, I was actually going to comment even on patch 37.
We are again trying to generalize a CTL's caps based on DPU version, the
same mistake which led us down this path.
So today you have CTL_DPU_0_MASK , CTL_DPU_5_MASK , CTL_DPU_7_MASK and
CTL_DPU_9_MASK and this builds on an assumption that you can get 5 by
ORing ACTIVE_CFG with 0.
+#define CTL_DPU_5_MASK (CTL_DPU_0_MASK | \
+ BIT(DPU_CTL_ACTIVE_CFG))
+
This is again moving towards that problematic pattern.
Why dont we stick to CTL features individually spelling it then work
towards generalizing as we discussed.
Because adding a feature would become a nightmare of touching all the
platforms?
We discussed not merging on major+LM. Glad, I agreed there. But I don't
think that we should remove existing defines without good reason. We
know that they work in the majority of cases.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 6840b22a4159..83f8f83e2b29 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -975,7 +975,7 @@ static const struct dpu_ctl_cfg sm8450_ctl[] = {
{
.name = "ctl_0", .id = CTL_0,
.base = 0x15000, .len = 0x204,
- .features = BIT(DPU_CTL_ACTIVE_CFG) |
BIT(DPU_CTL_SPLIT_DISPLAY) | BIT(DPU_CTL_FETCH_ACTIVE),
+ .features = BIT(DPU_CTL_SPLIT_DISPLAY) | CTL_SC7280_MASK,
.intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
},
{
--
With best wishes
Dmitry
