2023-04-20 01:17:23

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 0/2] DPU1 GC1.8 wiring-up

Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
dspp sub-block in addition to PCCv4. The other block differ a bit
more, but none of them are supported upstream.

This series adds configures the GCv1.8 on all the relevant SoCs.

Signed-off-by: Konrad Dybcio <[email protected]>
---
Konrad Dybcio (2):
drm/msm/dpu1: Rename sm8150_dspp_blk to sdm845_dspp_blk
drm/msm/dpu1: Enable GCv1.8 on many SoCs

drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 16 ++++++++--------
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 16 ++++++++--------
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 16 ++++++++--------
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++--------
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 16 ++++++++--------
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 16 ++++++++--------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 +++-
9 files changed, 55 insertions(+), 53 deletions(-)
---
base-commit: 3cdbc01c40e34c57697f8934f2727a88551696be
change-id: 20230420-topic-dpu_gc-6901f75768db

Best regards,
--
Konrad Dybcio <[email protected]>


2023-04-20 01:18:42

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 2/2] drm/msm/dpu1: Enable GCv1.8 on many SoCs

There's a plethora of S(D)M-era SoCs that have a GC v1.8 but never
declared, let alone enabled it. Do so!

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 ++
9 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
index c555d43ef0e0..a49e4d265b73 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
@@ -117,13 +117,13 @@ static const struct dpu_lm_cfg sm8150_lm[] = {
};

static const struct dpu_dspp_cfg sm8150_dspp[] = {
- DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
};

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
index c8a174352ede..80252a96c2fd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
@@ -118,13 +118,13 @@ static const struct dpu_lm_cfg sm8250_lm[] = {
};

static const struct dpu_dspp_cfg sm8250_dspp[] = {
- DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
};

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h
index 00f82b2c18ff..ea89ba1ab0fd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h
@@ -55,7 +55,7 @@ static const struct dpu_lm_cfg sm6115_lm[] = {
};

static const struct dpu_dspp_cfg sm6115_dspp[] = {
- DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
};

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
index 5f103140abc7..739c1a4f6618 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
@@ -52,7 +52,7 @@ static const struct dpu_lm_cfg qcm2290_lm[] = {
};

static const struct dpu_dspp_cfg qcm2290_dspp[] = {
- DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
};

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
index 257e898fea18..f90eb457ff3d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
@@ -116,13 +116,13 @@ static const struct dpu_lm_cfg sm8350_lm[] = {
};

static const struct dpu_dspp_cfg sm8350_dspp[] = {
- DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
};

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
index e4d4e47418fe..27f14a4fa882 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
@@ -110,13 +110,13 @@ static const struct dpu_lm_cfg sc8280xp_lm[] = {
};

static const struct dpu_dspp_cfg sc8280xp_dspp[] = {
- DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
};

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
index 88ad81e03622..0f97b4cc9e25 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
@@ -117,13 +117,13 @@ static const struct dpu_lm_cfg sm8450_lm[] = {
};

static const struct dpu_dspp_cfg sm8450_dspp[] = {
- DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
};
/* FIXME: interrupts */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
index ecc034f76441..2fdf0382cf1d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
@@ -122,13 +122,13 @@ static const struct dpu_lm_cfg sm8550_lm[] = {
};

static const struct dpu_dspp_cfg sm8550_dspp[] = {
- DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
- DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
+ DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_MSM8998_MASK,
&sdm845_dspp_sblk),
};
static const struct dpu_pingpong_cfg sm8550_pp[] = {
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 69af786b66a0..c4d3aaebb06f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -461,6 +461,8 @@ static const struct dpu_dspp_sub_blks sc7180_dspp_sblk = {
static const struct dpu_dspp_sub_blks sdm845_dspp_sblk = {
.pcc = {.id = DPU_DSPP_PCC, .base = 0x1700,
.len = 0x90, .version = 0x40000},
+ .gc = { .id = DPU_DSPP_GC, .base = 0x17c0,
+ .len = 0x90, .version = 0x10008},
};

#define DSPP_BLK(_name, _id, _base, _mask, _sblk) \

--
2.40.0

2023-04-20 01:23:20

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 1/2] drm/msm/dpu1: Rename sm8150_dspp_blk to sdm845_dspp_blk

SDM845 was the first SoC to include both PCC v4 and GC v1.8.
We don't currently support any other blocks but the common config
for these two can be reused for a large amount of SoCs.

Rename it to indicate the origin of that combo.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 8 ++++----
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
9 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
index 282d410269ff..c555d43ef0e0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
@@ -118,13 +118,13 @@ static const struct dpu_lm_cfg sm8150_lm[] = {

static const struct dpu_dspp_cfg sm8150_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
};

static const struct dpu_pingpong_cfg sm8150_pp[] = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
index 2c40229ea515..c8a174352ede 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
@@ -119,13 +119,13 @@ static const struct dpu_lm_cfg sm8250_lm[] = {

static const struct dpu_dspp_cfg sm8250_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
};

static const struct dpu_pingpong_cfg sm8250_pp[] = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h
index 6f04d8f85c92..00f82b2c18ff 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h
@@ -56,7 +56,7 @@ static const struct dpu_lm_cfg sm6115_lm[] = {

static const struct dpu_dspp_cfg sm6115_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
};

static const struct dpu_pingpong_cfg sm6115_pp[] = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
index 303492d62a5c..5f103140abc7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
@@ -53,7 +53,7 @@ static const struct dpu_lm_cfg qcm2290_lm[] = {

static const struct dpu_dspp_cfg qcm2290_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
};

static const struct dpu_pingpong_cfg qcm2290_pp[] = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
index ca107ca8de46..257e898fea18 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
@@ -117,13 +117,13 @@ static const struct dpu_lm_cfg sm8350_lm[] = {

static const struct dpu_dspp_cfg sm8350_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
};

static const struct dpu_pingpong_cfg sm8350_pp[] = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
index 9aab110b8c44..e4d4e47418fe 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
@@ -111,13 +111,13 @@ static const struct dpu_lm_cfg sc8280xp_lm[] = {

static const struct dpu_dspp_cfg sc8280xp_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
};

static const struct dpu_pingpong_cfg sc8280xp_pp[] = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
index 02a259b6b426..88ad81e03622 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
@@ -118,13 +118,13 @@ static const struct dpu_lm_cfg sm8450_lm[] = {

static const struct dpu_dspp_cfg sm8450_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
};
/* FIXME: interrupts */
static const struct dpu_pingpong_cfg sm8450_pp[] = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
index 9e403034093f..ecc034f76441 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
@@ -123,13 +123,13 @@ static const struct dpu_lm_cfg sm8550_lm[] = {

static const struct dpu_dspp_cfg sm8550_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_1", DSPP_1, 0x56000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_2", DSPP_2, 0x58000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
DSPP_BLK("dspp_3", DSPP_3, 0x5a000, DSPP_SC7180_MASK,
- &sm8150_dspp_sblk),
+ &sdm845_dspp_sblk),
};
static const struct dpu_pingpong_cfg sm8550_pp[] = {
PP_BLK_DIPHER("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, sc7280_pp_sblk,
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 03f162af1a50..69af786b66a0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -458,7 +458,7 @@ static const struct dpu_dspp_sub_blks sc7180_dspp_sblk = {
.len = 0x90, .version = 0x10000},
};

-static const struct dpu_dspp_sub_blks sm8150_dspp_sblk = {
+static const struct dpu_dspp_sub_blks sdm845_dspp_sblk = {
.pcc = {.id = DPU_DSPP_PCC, .base = 0x1700,
.len = 0x90, .version = 0x40000},
};

--
2.40.0

2023-04-20 01:28:31

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

On 20/04/2023 04:14, Konrad Dybcio wrote:
> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
> dspp sub-block in addition to PCCv4. The other block differ a bit
> more, but none of them are supported upstream.
>
> This series adds configures the GCv1.8 on all the relevant SoCs.

Does this mean that we will see gamma_lut support soon?

>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> Konrad Dybcio (2):
> drm/msm/dpu1: Rename sm8150_dspp_blk to sdm845_dspp_blk
> drm/msm/dpu1: Enable GCv1.8 on many SoCs
>
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 16 ++++++++--------
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 16 ++++++++--------
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 4 ++--
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 4 ++--
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 16 ++++++++--------
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++--------
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 16 ++++++++--------
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 16 ++++++++--------
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 +++-
> 9 files changed, 55 insertions(+), 53 deletions(-)
> ---
> base-commit: 3cdbc01c40e34c57697f8934f2727a88551696be
> change-id: 20230420-topic-dpu_gc-6901f75768db
>
> Best regards,

--
With best wishes
Dmitry

2023-04-20 01:29:02

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up



On 20.04.2023 03:25, Dmitry Baryshkov wrote:
> On 20/04/2023 04:14, Konrad Dybcio wrote:
>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>> dspp sub-block in addition to PCCv4. The other block differ a bit
>> more, but none of them are supported upstream.
>>
>> This series adds configures the GCv1.8 on all the relevant SoCs.
>
> Does this mean that we will see gamma_lut support soon?
No promises, my plate is not even full, it's beyond overflowing! :P

Konrad
>
>>
>> Signed-off-by: Konrad Dybcio <[email protected]>
>> ---
>> Konrad Dybcio (2):
>>        drm/msm/dpu1: Rename sm8150_dspp_blk to sdm845_dspp_blk
>>        drm/msm/dpu1: Enable GCv1.8 on many SoCs
>>
>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h   | 16 ++++++++--------
>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h   | 16 ++++++++--------
>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h   |  4 ++--
>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h  |  4 ++--
>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h   | 16 ++++++++--------
>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++--------
>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h   | 16 ++++++++--------
>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h   | 16 ++++++++--------
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c           |  4 +++-
>>   9 files changed, 55 insertions(+), 53 deletions(-)
>> ---
>> base-commit: 3cdbc01c40e34c57697f8934f2727a88551696be
>> change-id: 20230420-topic-dpu_gc-6901f75768db
>>
>> Best regards,
>

2023-04-20 01:30:13

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up



On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>
>
> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>> more, but none of them are supported upstream.
>>>
>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>
>> Does this mean that we will see gamma_lut support soon?
> No promises, my plate is not even full, it's beyond overflowing! :P
>
> Konrad

So I think I wrote about this before during the catalog rework/fixes
that the gc registers are not written to / programmed.

If thats not done, is there any benefit to this series?

>>
>>>
>>> Signed-off-by: Konrad Dybcio <[email protected]>
>>> ---
>>> Konrad Dybcio (2):
>>>        drm/msm/dpu1: Rename sm8150_dspp_blk to sdm845_dspp_blk
>>>        drm/msm/dpu1: Enable GCv1.8 on many SoCs
>>>
>>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h   | 16 ++++++++--------
>>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h   | 16 ++++++++--------
>>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h   |  4 ++--
>>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h  |  4 ++--
>>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h   | 16 ++++++++--------
>>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++--------
>>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h   | 16 ++++++++--------
>>>   drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h   | 16 ++++++++--------
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c           |  4 +++-
>>>   9 files changed, 55 insertions(+), 53 deletions(-)
>>> ---
>>> base-commit: 3cdbc01c40e34c57697f8934f2727a88551696be
>>> change-id: 20230420-topic-dpu_gc-6901f75768db
>>>
>>> Best regards,
>>

2023-04-20 01:40:52

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up



On 20.04.2023 03:28, Abhinav Kumar wrote:
>
>
> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>
>>
>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>> more, but none of them are supported upstream.
>>>>
>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>
>>> Does this mean that we will see gamma_lut support soon?
>> No promises, my plate is not even full, it's beyond overflowing! :P
>>
>> Konrad
>
> So I think I wrote about this before during the catalog rework/fixes that the gc registers are not written to / programmed.
>
> If thats not done, is there any benefit to this series?
Completeness and preparation for the code itself, if nothing else?

Konrad
>
>>>
>>>>
>>>> Signed-off-by: Konrad Dybcio <[email protected]>
>>>> ---
>>>> Konrad Dybcio (2):
>>>>         drm/msm/dpu1: Rename sm8150_dspp_blk to sdm845_dspp_blk
>>>>         drm/msm/dpu1: Enable GCv1.8 on many SoCs
>>>>
>>>>    drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h   | 16 ++++++++--------
>>>>    drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h   | 16 ++++++++--------
>>>>    drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h   |  4 ++--
>>>>    drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h  |  4 ++--
>>>>    drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h   | 16 ++++++++--------
>>>>    drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++--------
>>>>    drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h   | 16 ++++++++--------
>>>>    drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h   | 16 ++++++++--------
>>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c           |  4 +++-
>>>>    9 files changed, 55 insertions(+), 53 deletions(-)
>>>> ---
>>>> base-commit: 3cdbc01c40e34c57697f8934f2727a88551696be
>>>> change-id: 20230420-topic-dpu_gc-6901f75768db
>>>>
>>>> Best regards,
>>>

2023-04-20 11:52:53

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 1/2] drm/msm/dpu1: Rename sm8150_dspp_blk to sdm845_dspp_blk

On 20/04/2023 04:14, Konrad Dybcio wrote:
> SDM845 was the first SoC to include both PCC v4 and GC v1.8.
> We don't currently support any other blocks but the common config
> for these two can be reused for a large amount of SoCs.
>
> Rename it to indicate the origin of that combo.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
> 9 files changed, 27 insertions(+), 27 deletions(-)

For the patch itself:

Reviewed-by: Dmitry Baryshkov <[email protected]>

--
With best wishes
Dmitry

2023-04-20 11:53:25

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 1/2] drm/msm/dpu1: Rename sm8150_dspp_blk to sdm845_dspp_blk

On 20/04/2023 04:14, Konrad Dybcio wrote:
> SDM845 was the first SoC to include both PCC v4 and GC v1.8.
> We don't currently support any other blocks but the common config
> for these two can be reused for a large amount of SoCs.
>
> Rename it to indicate the origin of that combo.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 8 ++++----
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 2 +-
> 9 files changed, 27 insertions(+), 27 deletions(-)

Please follow this by enabling DSPP on sdm845. Otherwise it is strange
to have sdm845_dspp_sblk, but not to have it enabled on sdm845 itself.
If necessary, I can test it on RB3.

--
With best wishes
Dmitry

2023-04-20 18:10:47

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

On 20/04/2023 04:36, Konrad Dybcio wrote:
>
>
> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>
>>
>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>
>>>
>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>>> more, but none of them are supported upstream.
>>>>>
>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>
>>>> Does this mean that we will see gamma_lut support soon?
>>> No promises, my plate is not even full, it's beyond overflowing! :P
>>>
>>> Konrad
>>
>> So I think I wrote about this before during the catalog rework/fixes that the gc registers are not written to / programmed.
>>
>> If thats not done, is there any benefit to this series?
> Completeness and preparation for the code itself, if nothing else?

The usual problem is that if something is not put to use, it quickly
rots or becomes misused for newer platforms. We have seen this with the
some of DPU features.

In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
have three options:
- drop the unused GC from msm8998_sblk.
- keep things as is, single unused GC entry
- fill all the sblk with the correct information in hope that it stays
correct

Each of these options has its own drawbacks. I have slight bias towards
the last option, to have the information in place (as long as it is
accurate).

--
With best wishes
Dmitry

2023-04-20 19:49:57

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up



On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>
>>
>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>
>>>
>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>>>> more, but none of them are supported upstream.
>>>>>>
>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>>
>>>>> Does this mean that we will see gamma_lut support soon?
>>>> No promises, my plate is not even full, it's beyond overflowing! :P
>>>>
>>>> Konrad
>>>
>>> So I think I wrote about this before during the catalog rework/fixes
>>> that the gc registers are not written to / programmed.
>>>
>>> If thats not done, is there any benefit to this series?
>> Completeness and preparation for the code itself, if nothing else?
>
> The usual problem is that if something is not put to use, it quickly
> rots or becomes misused for newer platforms. We have seen this with the
> some of DPU features.
>
> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
> have three options:
> - drop the unused GC from msm8998_sblk.
> - keep things as is, single unused GC entry
> - fill all the sblk with the correct information in hope that it stays
> correct
>
> Each of these options has its own drawbacks. I have slight bias towards
> the last option, to have the information in place (as long as it is
> accurate).
>

My vote is for (1) . Today, GC is unused and from the discussion here,
there is no concrete plan to add it. If we keep extending an unused
bitmask for all the chipsets including the ones which will get added in
the future in the hope that someday the feature comes, it doesnt sound
like a good idea.

I would rather do (1), if someone has time. OR lets stay at (2) till
someone does (1).

When someone implements GC, we can re-use this patch and that time keep
konrad's author rights or co-developed by.


2023-04-20 19:54:37

by Marijn Suijten

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

On 2023-04-20 21:01:04, Dmitry Baryshkov wrote:
> On 20/04/2023 04:36, Konrad Dybcio wrote:
> >
> >
> > On 20.04.2023 03:28, Abhinav Kumar wrote:
> >>
> >>
> >> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
> >>>
> >>>
> >>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
> >>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
> >>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
> >>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
> >>>>> more, but none of them are supported upstream.
> >>>>>
> >>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
> >>>>
> >>>> Does this mean that we will see gamma_lut support soon?
> >>> No promises, my plate is not even full, it's beyond overflowing! :P
> >>>
> >>> Konrad
> >>
> >> So I think I wrote about this before during the catalog rework/fixes that the gc registers are not written to / programmed.
> >>
> >> If thats not done, is there any benefit to this series?
> > Completeness and preparation for the code itself, if nothing else?
>
> The usual problem is that if something is not put to use, it quickly
> rots or becomes misused for newer platforms. We have seen this with the
> some of DPU features.
>
> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
> have three options:
> - drop the unused GC from msm8998_sblk.
> - keep things as is, single unused GC entry
> - fill all the sblk with the correct information in hope that it stays
> correct
>
> Each of these options has its own drawbacks. I have slight bias towards
> the last option, to have the information in place (as long as it is
> accurate).

Normally I'm all for rigorously and completely defining the hardware,
porting the entire downstream DT in one go while looking at it anyway.
(And it leaves less room for error when looking at DT properties while
having no clue where they should end up in the catalog, or why they
wouldn't be there)

In this case though, as you say, it's unused so there's no way to test
and validate anything, especially future changes we **might** make to
the looks and layout of the catalog.

What's worse, this series shows zero efforts towards at the very least
explaining that GC is the Gamma Correction block, what the benefits are
in defining/having it, and that it is currently not used by the DSPP
driver block at all. That's my major reason for NAK'ing this.

- Marijn

2023-04-20 19:55:28

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

On 20/04/2023 22:47, Abhinav Kumar wrote:
>
>
> On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
>> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>>
>>>
>>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>>
>>>>
>>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>>
>>>>>
>>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>>>>> more, but none of them are supported upstream.
>>>>>>>
>>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>>>
>>>>>> Does this mean that we will see gamma_lut support soon?
>>>>> No promises, my plate is not even full, it's beyond overflowing! :P
>>>>>
>>>>> Konrad
>>>>
>>>> So I think I wrote about this before during the catalog rework/fixes
>>>> that the gc registers are not written to / programmed.
>>>>
>>>> If thats not done, is there any benefit to this series?
>>> Completeness and preparation for the code itself, if nothing else?
>>
>> The usual problem is that if something is not put to use, it quickly
>> rots or becomes misused for newer platforms. We have seen this with
>> the some of DPU features.
>>
>> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
>> have three options:
>> - drop the unused GC from msm8998_sblk.
>> - keep things as is, single unused GC entry
>> - fill all the sblk with the correct information in hope that it stays
>> correct
>>
>> Each of these options has its own drawbacks. I have slight bias
>> towards the last option, to have the information in place (as long as
>> it is accurate).
>>
>
> My vote is for (1) . Today, GC is unused and from the discussion here,
> there is no concrete plan to add it. If we keep extending an unused
> bitmask for all the chipsets including the ones which will get added in
> the future in the hope that someday the feature comes, it doesnt sound
> like a good idea.
>
> I would rather do (1), if someone has time.

Agree, this was the second item on my preference list. Could you please
send this oneliner?

> OR lets stay at (2) till
> someone does (1).
>
> When someone implements GC, we can re-use this patch and that time keep
> konrad's author rights or co-developed by.
>
>

--
With best wishes
Dmitry

2023-04-20 19:56:42

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up



On 4/20/2023 12:51 PM, Dmitry Baryshkov wrote:
> On 20/04/2023 22:47, Abhinav Kumar wrote:
>>
>>
>> On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
>>> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>>>
>>>>>
>>>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>>>
>>>>>>
>>>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>>>>>> more, but none of them are supported upstream.
>>>>>>>>
>>>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>>>>
>>>>>>> Does this mean that we will see gamma_lut support soon?
>>>>>> No promises, my plate is not even full, it's beyond overflowing! :P
>>>>>>
>>>>>> Konrad
>>>>>
>>>>> So I think I wrote about this before during the catalog
>>>>> rework/fixes that the gc registers are not written to / programmed.
>>>>>
>>>>> If thats not done, is there any benefit to this series?
>>>> Completeness and preparation for the code itself, if nothing else?
>>>
>>> The usual problem is that if something is not put to use, it quickly
>>> rots or becomes misused for newer platforms. We have seen this with
>>> the some of DPU features.
>>>
>>> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
>>> have three options:
>>> - drop the unused GC from msm8998_sblk.
>>> - keep things as is, single unused GC entry
>>> - fill all the sblk with the correct information in hope that it
>>> stays correct
>>>
>>> Each of these options has its own drawbacks. I have slight bias
>>> towards the last option, to have the information in place (as long as
>>> it is accurate).
>>>
>>
>> My vote is for (1) . Today, GC is unused and from the discussion here,
>> there is no concrete plan to add it. If we keep extending an unused
>> bitmask for all the chipsets including the ones which will get added
>> in the future in the hope that someday the feature comes, it doesnt
>> sound like a good idea.
>>
>> I would rather do (1), if someone has time.
>
> Agree, this was the second item on my preference list. Could you please
> send this oneliner?
>

Sure, i will send this by tomorrow, but its not a oneliner. Need to get
rid of below too:

470 struct dpu_dspp_sub_blks {
471 struct dpu_pp_blk gc;

>> OR lets stay at (2) till someone does (1).
>>
>> When someone implements GC, we can re-use this patch and that time
>> keep konrad's author rights or co-developed by.
>>
>>
>

2023-04-20 19:57:06

by Marijn Suijten

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

On 2023-04-20 22:51:22, Dmitry Baryshkov wrote:
> On 20/04/2023 22:47, Abhinav Kumar wrote:
> >
> >
> > On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
> >> On 20/04/2023 04:36, Konrad Dybcio wrote:
> >>>
> >>>
> >>> On 20.04.2023 03:28, Abhinav Kumar wrote:
> >>>>
> >>>>
> >>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
> >>>>>
> >>>>>
> >>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
> >>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
> >>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
> >>>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
> >>>>>>> more, but none of them are supported upstream.
> >>>>>>>
> >>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
> >>>>>>
> >>>>>> Does this mean that we will see gamma_lut support soon?
> >>>>> No promises, my plate is not even full, it's beyond overflowing! :P
> >>>>>
> >>>>> Konrad
> >>>>
> >>>> So I think I wrote about this before during the catalog rework/fixes
> >>>> that the gc registers are not written to / programmed.
> >>>>
> >>>> If thats not done, is there any benefit to this series?
> >>> Completeness and preparation for the code itself, if nothing else?
> >>
> >> The usual problem is that if something is not put to use, it quickly
> >> rots or becomes misused for newer platforms. We have seen this with
> >> the some of DPU features.
> >>
> >> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
> >> have three options:
> >> - drop the unused GC from msm8998_sblk.
> >> - keep things as is, single unused GC entry
> >> - fill all the sblk with the correct information in hope that it stays
> >> correct
> >>
> >> Each of these options has its own drawbacks. I have slight bias
> >> towards the last option, to have the information in place (as long as
> >> it is accurate).
> >>
> >
> > My vote is for (1) . Today, GC is unused and from the discussion here,
> > there is no concrete plan to add it. If we keep extending an unused
> > bitmask for all the chipsets including the ones which will get added in
> > the future in the hope that someday the feature comes, it doesnt sound
> > like a good idea.
> >
> > I would rather do (1), if someone has time.
>
> Agree, this was the second item on my preference list. Could you please
> send this oneliner?

Nit (to make sure we're on the same thought here): I think it's a
3-liner: remove it from DSPP_MSM8998_MASK as well as msm8998_dspp_sblk.

> > OR lets stay at (2) till
> > someone does (1).

I'm personally okay leaving it in place too, with an eye on implementing
this, IGC, and other blocks at some point if there's a use for it via
standard DRM properties.

> > When someone implements GC, we can re-use this patch and that time keep
> > konrad's author rights or co-developed by.

Good to at least know all these SoCs have the same offset and revision.

- Marijn

2023-04-20 20:17:18

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

On 20/04/2023 22:53, Abhinav Kumar wrote:
>
>
> On 4/20/2023 12:51 PM, Dmitry Baryshkov wrote:
>> On 20/04/2023 22:47, Abhinav Kumar wrote:
>>>
>>>
>>> On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
>>>> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>>>>
>>>>>
>>>>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>>>>
>>>>>>
>>>>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>>>>>>> more, but none of them are supported upstream.
>>>>>>>>>
>>>>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>>>>>
>>>>>>>> Does this mean that we will see gamma_lut support soon?
>>>>>>> No promises, my plate is not even full, it's beyond overflowing! :P
>>>>>>>
>>>>>>> Konrad
>>>>>>
>>>>>> So I think I wrote about this before during the catalog
>>>>>> rework/fixes that the gc registers are not written to / programmed.
>>>>>>
>>>>>> If thats not done, is there any benefit to this series?
>>>>> Completeness and preparation for the code itself, if nothing else?
>>>>
>>>> The usual problem is that if something is not put to use, it quickly
>>>> rots or becomes misused for newer platforms. We have seen this with
>>>> the some of DPU features.
>>>>
>>>> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used)
>>>> we have three options:
>>>> - drop the unused GC from msm8998_sblk.
>>>> - keep things as is, single unused GC entry
>>>> - fill all the sblk with the correct information in hope that it
>>>> stays correct
>>>>
>>>> Each of these options has its own drawbacks. I have slight bias
>>>> towards the last option, to have the information in place (as long
>>>> as it is accurate).
>>>>
>>>
>>> My vote is for (1) . Today, GC is unused and from the discussion
>>> here, there is no concrete plan to add it. If we keep extending an
>>> unused bitmask for all the chipsets including the ones which will get
>>> added in the future in the hope that someday the feature comes, it
>>> doesnt sound like a good idea.
>>>
>>> I would rather do (1), if someone has time.
>>
>> Agree, this was the second item on my preference list. Could you
>> please send this oneliner?
>>
>
> Sure, i will send this by tomorrow, but its not a oneliner. Need to get
> rid of below too:
>
> 470 struct dpu_dspp_sub_blks {
> 471     struct dpu_pp_blk gc;

Agree.

>
>>> OR lets stay at (2) till someone does (1).
>>>
>>> When someone implements GC, we can re-use this patch and that time
>>> keep konrad's author rights or co-developed by.
>>>
>>>
>>

--
With best wishes
Dmitry

2023-04-20 22:57:00

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

On 20/04/2023 22:56, Marijn Suijten wrote:
> On 2023-04-20 22:51:22, Dmitry Baryshkov wrote:
>> On 20/04/2023 22:47, Abhinav Kumar wrote:
>>>
>>>
>>> On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
>>>> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>>>>
>>>>>
>>>>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>>>>
>>>>>>
>>>>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>>>>>>> more, but none of them are supported upstream.
>>>>>>>>>
>>>>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>>>>>
>>>>>>>> Does this mean that we will see gamma_lut support soon?
>>>>>>> No promises, my plate is not even full, it's beyond overflowing! :P
>>>>>>>
>>>>>>> Konrad
>>>>>>
>>>>>> So I think I wrote about this before during the catalog rework/fixes
>>>>>> that the gc registers are not written to / programmed.
>>>>>>
>>>>>> If thats not done, is there any benefit to this series?
>>>>> Completeness and preparation for the code itself, if nothing else?
>>>>
>>>> The usual problem is that if something is not put to use, it quickly
>>>> rots or becomes misused for newer platforms. We have seen this with
>>>> the some of DPU features.
>>>>
>>>> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
>>>> have three options:
>>>> - drop the unused GC from msm8998_sblk.
>>>> - keep things as is, single unused GC entry
>>>> - fill all the sblk with the correct information in hope that it stays
>>>> correct
>>>>
>>>> Each of these options has its own drawbacks. I have slight bias
>>>> towards the last option, to have the information in place (as long as
>>>> it is accurate).
>>>>
>>>
>>> My vote is for (1) . Today, GC is unused and from the discussion here,
>>> there is no concrete plan to add it. If we keep extending an unused
>>> bitmask for all the chipsets including the ones which will get added in
>>> the future in the hope that someday the feature comes, it doesnt sound
>>> like a good idea.
>>>
>>> I would rather do (1), if someone has time.
>>
>> Agree, this was the second item on my preference list. Could you please
>> send this oneliner?
>
> Nit (to make sure we're on the same thought here): I think it's a
> 3-liner: remove it from DSPP_MSM8998_MASK as well as msm8998_dspp_sblk.
>
>>> OR lets stay at (2) till
>>> someone does (1).
>
> I'm personally okay leaving it in place too, with an eye on implementing
> this, IGC, and other blocks at some point if there's a use for it via
> standard DRM properties.

I took a quick glance. I think it is possible, but not straightforward.
But I must admit here, I don't have a full picture regarding different
color encodings, ranges and the rest of gamma/degamma API and usage.

>
>>> When someone implements GC, we can re-use this patch and that time keep
>>> konrad's author rights or co-developed by.
>
> Good to at least know all these SoCs have the same offset and revision.
>
> - Marijn

--
With best wishes
Dmitry

2023-04-21 22:47:54

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

On 22/04/2023 01:34, Abhinav Kumar wrote:
>
>
> On 4/20/2023 3:52 PM, Dmitry Baryshkov wrote:
>> On 20/04/2023 22:56, Marijn Suijten wrote:
>>> On 2023-04-20 22:51:22, Dmitry Baryshkov wrote:
>>>> On 20/04/2023 22:47, Abhinav Kumar wrote:
>>>>>
>>>>>
>>>>> On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
>>>>>> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>>>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>>>>>>>> dspp sub-block in addition to PCCv4. The other block differ a
>>>>>>>>>>> bit
>>>>>>>>>>> more, but none of them are supported upstream.
>>>>>>>>>>>
>>>>>>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>>>>>>>
>>>>>>>>>> Does this mean that we will see gamma_lut support soon?
>>>>>>>>> No promises, my plate is not even full, it's beyond
>>>>>>>>> overflowing! :P
>>>>>>>>>
>>>>>>>>> Konrad
>>>>>>>>
>>>>>>>> So I think I wrote about this before during the catalog
>>>>>>>> rework/fixes
>>>>>>>> that the gc registers are not written to / programmed.
>>>>>>>>
>>>>>>>> If thats not done, is there any benefit to this series?
>>>>>>> Completeness and preparation for the code itself, if nothing else?
>>>>>>
>>>>>> The usual problem is that if something is not put to use, it quickly
>>>>>> rots or becomes misused for newer platforms. We have seen this with
>>>>>> the some of DPU features.
>>>>>>
>>>>>> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
>>>>>> have three options:
>>>>>> - drop the unused GC from msm8998_sblk.
>>>>>> - keep things as is, single unused GC entry
>>>>>> - fill all the sblk with the correct information in hope that it
>>>>>> stays
>>>>>> correct
>>>>>>
>>>>>> Each of these options has its own drawbacks. I have slight bias
>>>>>> towards the last option, to have the information in place (as long as
>>>>>> it is accurate).
>>>>>>
>>>>>
>>>>> My vote is for (1) . Today, GC is unused and from the discussion here,
>>>>> there is no concrete plan to add it. If we keep extending an unused
>>>>> bitmask for all the chipsets including the ones which will get
>>>>> added in
>>>>> the future in the hope that someday the feature comes, it doesnt sound
>>>>> like a good idea.
>>>>>
>>>>> I would rather do (1), if someone has time.
>>>>
>>>> Agree, this was the second item on my preference list. Could you please
>>>> send this oneliner?
>>>
>>> Nit (to make sure we're on the same thought here): I think it's a
>>> 3-liner: remove it from DSPP_MSM8998_MASK as well as msm8998_dspp_sblk.
>>>
>>>>> OR lets stay at (2) till
>>>>> someone does (1).
>>>
>>> I'm personally okay leaving it in place too, with an eye on implementing
>>> this, IGC, and other blocks at some point if there's a use for it via
>>> standard DRM properties.
>>
>> I took a quick glance. I think it is possible, but not
>> straightforward. But I must admit here, I don't have a full picture
>> regarding different color encodings, ranges and the rest of
>> gamma/degamma API and usage.
>>
>
> I think its easier to remove this now and then add it when we add the
> support. As discussed, will post this shortly.
>
> Otherwise, whenever any new chipset gets added, we will run into the
> same question of whether to add GC or not.

Yes, I absolutely agree here.

>
>>>
>>>>> When someone implements GC, we can re-use this patch and that time
>>>>> keep
>>>>> konrad's author rights or co-developed by.
>>>
>>> Good to at least know all these SoCs have the same offset and revision.
>>>
>>> - Marijn
>>

--
With best wishes
Dmitry

2023-04-21 23:05:46

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up



On 4/20/2023 3:52 PM, Dmitry Baryshkov wrote:
> On 20/04/2023 22:56, Marijn Suijten wrote:
>> On 2023-04-20 22:51:22, Dmitry Baryshkov wrote:
>>> On 20/04/2023 22:47, Abhinav Kumar wrote:
>>>>
>>>>
>>>> On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
>>>>> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>>>>>
>>>>>>
>>>>>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>>>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>>>>>>>> more, but none of them are supported upstream.
>>>>>>>>>>
>>>>>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>>>>>>
>>>>>>>>> Does this mean that we will see gamma_lut support soon?
>>>>>>>> No promises, my plate is not even full, it's beyond overflowing! :P
>>>>>>>>
>>>>>>>> Konrad
>>>>>>>
>>>>>>> So I think I wrote about this before during the catalog rework/fixes
>>>>>>> that the gc registers are not written to / programmed.
>>>>>>>
>>>>>>> If thats not done, is there any benefit to this series?
>>>>>> Completeness and preparation for the code itself, if nothing else?
>>>>>
>>>>> The usual problem is that if something is not put to use, it quickly
>>>>> rots or becomes misused for newer platforms. We have seen this with
>>>>> the some of DPU features.
>>>>>
>>>>> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
>>>>> have three options:
>>>>> - drop the unused GC from msm8998_sblk.
>>>>> - keep things as is, single unused GC entry
>>>>> - fill all the sblk with the correct information in hope that it stays
>>>>> correct
>>>>>
>>>>> Each of these options has its own drawbacks. I have slight bias
>>>>> towards the last option, to have the information in place (as long as
>>>>> it is accurate).
>>>>>
>>>>
>>>> My vote is for (1) . Today, GC is unused and from the discussion here,
>>>> there is no concrete plan to add it. If we keep extending an unused
>>>> bitmask for all the chipsets including the ones which will get added in
>>>> the future in the hope that someday the feature comes, it doesnt sound
>>>> like a good idea.
>>>>
>>>> I would rather do (1), if someone has time.
>>>
>>> Agree, this was the second item on my preference list. Could you please
>>> send this oneliner?
>>
>> Nit (to make sure we're on the same thought here): I think it's a
>> 3-liner: remove it from DSPP_MSM8998_MASK as well as msm8998_dspp_sblk.
>>
>>>> OR lets stay at (2) till
>>>> someone does (1).
>>
>> I'm personally okay leaving it in place too, with an eye on implementing
>> this, IGC, and other blocks at some point if there's a use for it via
>> standard DRM properties.
>
> I took a quick glance. I think it is possible, but not straightforward.
> But I must admit here, I don't have a full picture regarding different
> color encodings, ranges and the rest of gamma/degamma API and usage.
>

I think its easier to remove this now and then add it when we add the
support. As discussed, will post this shortly.

Otherwise, whenever any new chipset gets added, we will run into the
same question of whether to add GC or not.

>>
>>>> When someone implements GC, we can re-use this patch and that time keep
>>>> konrad's author rights or co-developed by.
>>
>> Good to at least know all these SoCs have the same offset and revision.
>>
>> - Marijn
>

2023-04-22 12:15:27

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up



On 22.04.2023 00:35, Dmitry Baryshkov wrote:
> On 22/04/2023 01:34, Abhinav Kumar wrote:
>>
>>
>> On 4/20/2023 3:52 PM, Dmitry Baryshkov wrote:
>>> On 20/04/2023 22:56, Marijn Suijten wrote:
>>>> On 2023-04-20 22:51:22, Dmitry Baryshkov wrote:
>>>>> On 20/04/2023 22:47, Abhinav Kumar wrote:
>>>>>>
>>>>>>
>>>>>> On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
>>>>>>> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>>>>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>>>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>>>>>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>>>>>>>>>> more, but none of them are supported upstream.
>>>>>>>>>>>>
>>>>>>>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>>>>>>>>
>>>>>>>>>>> Does this mean that we will see gamma_lut support soon?
>>>>>>>>>> No promises, my plate is not even full, it's beyond overflowing! :P
>>>>>>>>>>
>>>>>>>>>> Konrad
>>>>>>>>>
>>>>>>>>> So I think I wrote about this before during the catalog rework/fixes
>>>>>>>>> that the gc registers are not written to / programmed.
>>>>>>>>>
>>>>>>>>> If thats not done, is there any benefit to this series?
>>>>>>>> Completeness and preparation for the code itself, if nothing else?
>>>>>>>
>>>>>>> The usual problem is that if something is not put to use, it quickly
>>>>>>> rots or becomes misused for newer platforms. We have seen this with
>>>>>>> the some of DPU features.
>>>>>>>
>>>>>>> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
>>>>>>> have three options:
>>>>>>> - drop the unused GC from msm8998_sblk.
>>>>>>> - keep things as is, single unused GC entry
>>>>>>> - fill all the sblk with the correct information in hope that it stays
>>>>>>> correct
>>>>>>>
>>>>>>> Each of these options has its own drawbacks. I have slight bias
>>>>>>> towards the last option, to have the information in place (as long as
>>>>>>> it is accurate).
>>>>>>>
>>>>>>
>>>>>> My vote is for (1) . Today, GC is unused and from the discussion here,
>>>>>> there is no concrete plan to add it. If we keep extending an unused
>>>>>> bitmask for all the chipsets including the ones which will get added in
>>>>>> the future in the hope that someday the feature comes, it doesnt sound
>>>>>> like a good idea.
>>>>>>
>>>>>> I would rather do (1), if someone has time.
>>>>>
>>>>> Agree, this was the second item on my preference list. Could you please
>>>>> send this oneliner?
>>>>
>>>> Nit (to make sure we're on the same thought here): I think it's a
>>>> 3-liner: remove it from DSPP_MSM8998_MASK as well as msm8998_dspp_sblk.
>>>>
>>>>>> OR lets stay at (2) till
>>>>>> someone does (1).
>>>>
>>>> I'm personally okay leaving it in place too, with an eye on implementing
>>>> this, IGC, and other blocks at some point if there's a use for it via
>>>> standard DRM properties.
>>>
>>> I took a quick glance. I think it is possible, but not straightforward. But I must admit here, I don't have a full picture regarding different color encodings, ranges and the rest of gamma/degamma API and usage.
>>>
>>
>> I think its easier to remove this now and then add it when we add the support. As discussed, will post this shortly.
>>
>> Otherwise, whenever any new chipset gets added, we will run into the same question of whether to add GC or not.
>
> Yes, I absolutely agree here.
Sorry for the useless patches, though I guess they were a good
discussion starter..

Konrad
>
>>
>>>>
>>>>>> When someone implements GC, we can re-use this patch and that time keep
>>>>>> konrad's author rights or co-developed by.
>>>>
>>>> Good to at least know all these SoCs have the same offset and revision.
>>>>
>>>> - Marijn
>>>
>

2023-04-22 15:02:23

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up

On 22/04/2023 15:08, Konrad Dybcio wrote:
>
>
> On 22.04.2023 00:35, Dmitry Baryshkov wrote:
>> On 22/04/2023 01:34, Abhinav Kumar wrote:
>>>
>>>
>>> On 4/20/2023 3:52 PM, Dmitry Baryshkov wrote:
>>>> On 20/04/2023 22:56, Marijn Suijten wrote:
>>>>> On 2023-04-20 22:51:22, Dmitry Baryshkov wrote:
>>>>>> On 20/04/2023 22:47, Abhinav Kumar wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
>>>>>>>> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>>>>>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>>>>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
>>>>>>>>>>>>> dspp sub-block in addition to PCCv4. The other block differ a bit
>>>>>>>>>>>>> more, but none of them are supported upstream.
>>>>>>>>>>>>>
>>>>>>>>>>>>> This series adds configures the GCv1.8 on all the relevant SoCs.
>>>>>>>>>>>>
>>>>>>>>>>>> Does this mean that we will see gamma_lut support soon?
>>>>>>>>>>> No promises, my plate is not even full, it's beyond overflowing! :P
>>>>>>>>>>>
>>>>>>>>>>> Konrad
>>>>>>>>>>
>>>>>>>>>> So I think I wrote about this before during the catalog rework/fixes
>>>>>>>>>> that the gc registers are not written to / programmed.
>>>>>>>>>>
>>>>>>>>>> If thats not done, is there any benefit to this series?
>>>>>>>>> Completeness and preparation for the code itself, if nothing else?
>>>>>>>>
>>>>>>>> The usual problem is that if something is not put to use, it quickly
>>>>>>>> rots or becomes misused for newer platforms. We have seen this with
>>>>>>>> the some of DPU features.
>>>>>>>>
>>>>>>>> In case of GC (and the freshly defined DPU_DSPP_IGC, but not used) we
>>>>>>>> have three options:
>>>>>>>> - drop the unused GC from msm8998_sblk.
>>>>>>>> - keep things as is, single unused GC entry
>>>>>>>> - fill all the sblk with the correct information in hope that it stays
>>>>>>>> correct
>>>>>>>>
>>>>>>>> Each of these options has its own drawbacks. I have slight bias
>>>>>>>> towards the last option, to have the information in place (as long as
>>>>>>>> it is accurate).
>>>>>>>>
>>>>>>>
>>>>>>> My vote is for (1) . Today, GC is unused and from the discussion here,
>>>>>>> there is no concrete plan to add it. If we keep extending an unused
>>>>>>> bitmask for all the chipsets including the ones which will get added in
>>>>>>> the future in the hope that someday the feature comes, it doesnt sound
>>>>>>> like a good idea.
>>>>>>>
>>>>>>> I would rather do (1), if someone has time.
>>>>>>
>>>>>> Agree, this was the second item on my preference list. Could you please
>>>>>> send this oneliner?
>>>>>
>>>>> Nit (to make sure we're on the same thought here): I think it's a
>>>>> 3-liner: remove it from DSPP_MSM8998_MASK as well as msm8998_dspp_sblk.
>>>>>
>>>>>>> OR lets stay at (2) till
>>>>>>> someone does (1).
>>>>>
>>>>> I'm personally okay leaving it in place too, with an eye on implementing
>>>>> this, IGC, and other blocks at some point if there's a use for it via
>>>>> standard DRM properties.
>>>>
>>>> I took a quick glance. I think it is possible, but not straightforward. But I must admit here, I don't have a full picture regarding different color encodings, ranges and the rest of gamma/degamma API and usage.
>>>>
>>>
>>> I think its easier to remove this now and then add it when we add the support. As discussed, will post this shortly.
>>>
>>> Otherwise, whenever any new chipset gets added, we will run into the same question of whether to add GC or not.
>>
>> Yes, I absolutely agree here.
> Sorry for the useless patches, though I guess they were a good
> discussion starter..

If they started the discussion, they were not useless.

>
> Konrad
>>
>>>
>>>>>
>>>>>>> When someone implements GC, we can re-use this patch and that time keep
>>>>>>> konrad's author rights or co-developed by.
>>>>>
>>>>> Good to at least know all these SoCs have the same offset and revision.
>>>>>
>>>>> - Marijn
>>>>
>>

--
With best wishes
Dmitry

2023-04-22 19:50:11

by Abhinav Kumar

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up



On 4/22/2023 7:11 AM, Dmitry Baryshkov wrote:
> On 22/04/2023 15:08, Konrad Dybcio wrote:
>>
>>
>> On 22.04.2023 00:35, Dmitry Baryshkov wrote:
>>> On 22/04/2023 01:34, Abhinav Kumar wrote:
>>>>
>>>>
>>>> On 4/20/2023 3:52 PM, Dmitry Baryshkov wrote:
>>>>> On 20/04/2023 22:56, Marijn Suijten wrote:
>>>>>> On 2023-04-20 22:51:22, Dmitry Baryshkov wrote:
>>>>>>> On 20/04/2023 22:47, Abhinav Kumar wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4/20/2023 11:01 AM, Dmitry Baryshkov wrote:
>>>>>>>>> On 20/04/2023 04:36, Konrad Dybcio wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 20.04.2023 03:28, Abhinav Kumar wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 4/19/2023 6:26 PM, Konrad Dybcio wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 20.04.2023 03:25, Dmitry Baryshkov wrote:
>>>>>>>>>>>>> On 20/04/2023 04:14, Konrad Dybcio wrote:
>>>>>>>>>>>>>> Almost all SoCs from SDM845 to SM8550 inclusive feature a
>>>>>>>>>>>>>> GC1.8
>>>>>>>>>>>>>> dspp sub-block in addition to PCCv4. The other block
>>>>>>>>>>>>>> differ a bit
>>>>>>>>>>>>>> more, but none of them are supported upstream.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This series adds configures the GCv1.8 on all the relevant
>>>>>>>>>>>>>> SoCs.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Does this mean that we will see gamma_lut support soon?
>>>>>>>>>>>> No promises, my plate is not even full, it's beyond
>>>>>>>>>>>> overflowing! :P
>>>>>>>>>>>>
>>>>>>>>>>>> Konrad
>>>>>>>>>>>
>>>>>>>>>>> So I think I wrote about this before during the catalog
>>>>>>>>>>> rework/fixes
>>>>>>>>>>> that the gc registers are not written to / programmed.
>>>>>>>>>>>
>>>>>>>>>>> If thats not done, is there any benefit to this series?
>>>>>>>>>> Completeness and preparation for the code itself, if nothing
>>>>>>>>>> else?
>>>>>>>>>
>>>>>>>>> The usual problem is that if something is not put to use, it
>>>>>>>>> quickly
>>>>>>>>> rots or becomes misused for newer platforms. We have seen this
>>>>>>>>> with
>>>>>>>>> the some of DPU features.
>>>>>>>>>
>>>>>>>>> In case of GC (and the freshly defined DPU_DSPP_IGC, but not
>>>>>>>>> used) we
>>>>>>>>> have three options:
>>>>>>>>> - drop the unused GC from msm8998_sblk.
>>>>>>>>> - keep things as is, single unused GC entry
>>>>>>>>> - fill all the sblk with the correct information in hope that
>>>>>>>>> it stays
>>>>>>>>> correct
>>>>>>>>>
>>>>>>>>> Each of these options has its own drawbacks. I have slight bias
>>>>>>>>> towards the last option, to have the information in place (as
>>>>>>>>> long as
>>>>>>>>> it is accurate).
>>>>>>>>>
>>>>>>>>
>>>>>>>> My vote is for (1) . Today, GC is unused and from the discussion
>>>>>>>> here,
>>>>>>>> there is no concrete plan to add it. If we keep extending an unused
>>>>>>>> bitmask for all the chipsets including the ones which will get
>>>>>>>> added in
>>>>>>>> the future in the hope that someday the feature comes, it doesnt
>>>>>>>> sound
>>>>>>>> like a good idea.
>>>>>>>>
>>>>>>>> I would rather do (1), if someone has time.
>>>>>>>
>>>>>>> Agree, this was the second item on my preference list. Could you
>>>>>>> please
>>>>>>> send this oneliner?
>>>>>>
>>>>>> Nit (to make sure we're on the same thought here): I think it's a
>>>>>> 3-liner: remove it from DSPP_MSM8998_MASK as well as
>>>>>> msm8998_dspp_sblk.
>>>>>>
>>>>>>>> OR lets stay at (2) till
>>>>>>>> someone does (1).
>>>>>>
>>>>>> I'm personally okay leaving it in place too, with an eye on
>>>>>> implementing
>>>>>> this, IGC, and other blocks at some point if there's a use for it via
>>>>>> standard DRM properties.
>>>>>
>>>>> I took a quick glance. I think it is possible, but not
>>>>> straightforward. But I must admit here, I don't have a full picture
>>>>> regarding different color encodings, ranges and the rest of
>>>>> gamma/degamma API and usage.
>>>>>
>>>>
>>>> I think its easier to remove this now and then add it when we add
>>>> the support. As discussed, will post this shortly.
>>>>
>>>> Otherwise, whenever any new chipset gets added, we will run into the
>>>> same question of whether to add GC or not.
>>>
>>> Yes, I absolutely agree here.
>> Sorry for the useless patches, though I guess they were a good
>> discussion starter..
>
> If they started the discussion, they were not useless.
>

I second that, they were not useless at all. In fact, like I mentioned
earlier, once GC support is added, we can re-use these catalog changes.

So, this is all good work.

>>
>> Konrad
>>>
>>>>
>>>>>>
>>>>>>>> When someone implements GC, we can re-use this patch and that
>>>>>>>> time keep
>>>>>>>> konrad's author rights or co-developed by.
>>>>>>
>>>>>> Good to at least know all these SoCs have the same offset and
>>>>>> revision.
>>>>>>
>>>>>> - Marijn
>>>>>
>>>
>

2023-07-11 14:39:10

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] DPU1 GC1.8 wiring-up


On Thu, 20 Apr 2023 03:14:53 +0200, Konrad Dybcio wrote:
> Almost all SoCs from SDM845 to SM8550 inclusive feature a GC1.8
> dspp sub-block in addition to PCCv4. The other block differ a bit
> more, but none of them are supported upstream.
>
> This series adds configures the GCv1.8 on all the relevant SoCs.
>
>
> [...]

Applied, thanks!

[1/2] drm/msm/dpu1: Rename sm8150_dspp_blk to sdm845_dspp_blk
https://gitlab.freedesktop.org/lumag/msm/-/commit/9891b3df2b43

Best regards,
--
Dmitry Baryshkov <[email protected]>