Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH v2 0/3] drm/msm/mdss: rework UBWC registers programming
@ 2024-11-23  5:44 Dmitry Baryshkov
  2024-11-23  5:44 ` [PATCH v2 1/3] drm/msm/mdss: define bitfields for the UBWC_STATIC register Dmitry Baryshkov
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Dmitry Baryshkov @ 2024-11-23  5:44 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
	Connor Abbott, David Airlie, Simona Vetter
  Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno

Current way of programming of the UBWC-related registers has been
inherited from vendor's drivers. The ubwc_static was supposed to contain
raw data to be programmed to the hardware, but was later repurposed to
define of the bits. As it can be seen by the commit 3e30296b374a
("drm/msm: fix the highest_bank_bit for sc7180") sometimes this data
gets out of sync.

Rework existing msm_mdss_setup_ubwc_dec_NN() functions to be closer to
the actual hardware bit definitions. Drop the ubwc_static field.

Unfortunately this also introduces several "unknown" bits, for which we
do not document the actual purpose. Hopefully comparing this data with
the more documented Adreno UBWC feature bits will provide information
about the meaning of those bits.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Changes in v2:
- Dropped applied patches
- Added defines for UBWC_AMSBC, UBWC_MIN_ACC_LEN and UBWC_BANK_SPREAD
  and .ubwc_bank_spread flag in struct msm_mdss_data (kudos to Abhinav
  for helping to handle this on Qualcomm side)
- Changed msm_mdss_data to use true/false to set macrotile_mode
- Link to v1: https://lore.kernel.org/r/20240921-msm-mdss-ubwc-v1-0-411dcf309d05@linaro.org

---
Dmitry Baryshkov (3):
      drm/msm/mdss: define bitfields for the UBWC_STATIC register
      drm/msm/mdss: reuse defined bitfields for UBWC 2.0
      drm/msm/mdss: use boolean values for macrotile_mode

 drivers/gpu/drm/msm/msm_mdss.c                 | 71 ++++++++++++++++----------
 drivers/gpu/drm/msm/msm_mdss.h                 |  4 +-
 drivers/gpu/drm/msm/registers/display/mdss.xml | 11 +++-
 3 files changed, 55 insertions(+), 31 deletions(-)
---
base-commit: 86313a9cd152330c634b25d826a281c6a002eb77
change-id: 20240921-msm-mdss-ubwc-105589e05f35

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] drm/msm/mdss: define bitfields for the UBWC_STATIC register
  2024-11-23  5:44 [PATCH v2 0/3] drm/msm/mdss: rework UBWC registers programming Dmitry Baryshkov
@ 2024-11-23  5:44 ` Dmitry Baryshkov
  2024-11-26  2:03   ` Abhinav Kumar
  2024-11-23  5:44 ` [PATCH v2 2/3] drm/msm/mdss: reuse defined bitfields for UBWC 2.0 Dmitry Baryshkov
  2024-11-23  5:44 ` [PATCH v2 3/3] drm/msm/mdss: use boolean values for macrotile_mode Dmitry Baryshkov
  2 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2024-11-23  5:44 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
	Connor Abbott, David Airlie, Simona Vetter
  Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno

Rather than hand-coding UBWC_STATIC value calculation, define
corresponding bitfields and use them to setup the register value.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c                 | 38 +++++++++++++++-----------
 drivers/gpu/drm/msm/msm_mdss.h                 |  3 +-
 drivers/gpu/drm/msm/registers/display/mdss.xml | 11 +++++++-
 3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index b7bd899ead44bf86998e7295bccb31a334fa6811..4b57f39bec4e6232a0f5b4d49f8ae1200e74ac78 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -173,15 +173,17 @@ static void msm_mdss_setup_ubwc_dec_20(struct msm_mdss *msm_mdss)
 static void msm_mdss_setup_ubwc_dec_30(struct msm_mdss *msm_mdss)
 {
 	const struct msm_mdss_data *data = msm_mdss->mdss_data;
-	u32 value = (data->ubwc_swizzle & 0x1) |
-		    (data->highest_bank_bit & 0x3) << 4 |
-		    (data->macrotile_mode & 0x1) << 12;
+	u32 value = MDSS_UBWC_STATIC_UBWC_SWIZZLE(data->ubwc_swizzle & 0x1) |
+		    MDSS_UBWC_STATIC_HIGHEST_BANK_BIT(data->highest_bank_bit);
+
+	if (data->macrotile_mode)
+		value |= MDSS_UBWC_STATIC_MACROTILE_MODE;
 
 	if (data->ubwc_enc_version == UBWC_3_0)
-		value |= BIT(10);
+		value |= MDSS_UBWC_STATIC_UBWC_AMSBC;
 
 	if (data->ubwc_enc_version == UBWC_1_0)
-		value |= BIT(8);
+		value |= MDSS_UBWC_STATIC_UBWC_MIN_ACC_LEN;
 
 	writel_relaxed(value, msm_mdss->mmio + REG_MDSS_UBWC_STATIC);
 }
@@ -189,10 +191,14 @@ static void msm_mdss_setup_ubwc_dec_30(struct msm_mdss *msm_mdss)
 static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss)
 {
 	const struct msm_mdss_data *data = msm_mdss->mdss_data;
-	u32 value = (data->ubwc_swizzle & 0x7) |
-		    (data->ubwc_static & 0x1) << 3 |
-		    (data->highest_bank_bit & 0x7) << 4 |
-		    (data->macrotile_mode & 0x1) << 12;
+	u32 value = MDSS_UBWC_STATIC_UBWC_SWIZZLE(data->ubwc_swizzle) |
+		    MDSS_UBWC_STATIC_HIGHEST_BANK_BIT(data->highest_bank_bit);
+
+	if (data->ubwc_bank_spread)
+		value |= MDSS_UBWC_STATIC_UBWC_BANK_SPREAD;
+
+	if (data->macrotile_mode)
+		value |= MDSS_UBWC_STATIC_MACROTILE_MODE;
 
 	writel_relaxed(value, msm_mdss->mmio + REG_MDSS_UBWC_STATIC);
 
@@ -572,7 +578,7 @@ static const struct msm_mdss_data sa8775p_data = {
 	.ubwc_enc_version = UBWC_4_0,
 	.ubwc_dec_version = UBWC_4_0,
 	.ubwc_swizzle = 4,
-	.ubwc_static = 1,
+	.ubwc_bank_spread = true,
 	.highest_bank_bit = 0,
 	.macrotile_mode = 1,
 	.reg_bus_bw = 74000,
@@ -590,7 +596,7 @@ static const struct msm_mdss_data sc7280_data = {
 	.ubwc_enc_version = UBWC_3_0,
 	.ubwc_dec_version = UBWC_4_0,
 	.ubwc_swizzle = 6,
-	.ubwc_static = 1,
+	.ubwc_bank_spread = true,
 	.highest_bank_bit = 1,
 	.macrotile_mode = 1,
 	.reg_bus_bw = 74000,
@@ -608,7 +614,7 @@ static const struct msm_mdss_data sc8280xp_data = {
 	.ubwc_enc_version = UBWC_4_0,
 	.ubwc_dec_version = UBWC_4_0,
 	.ubwc_swizzle = 6,
-	.ubwc_static = 1,
+	.ubwc_bank_spread = true,
 	.highest_bank_bit = 3,
 	.macrotile_mode = 1,
 	.reg_bus_bw = 76800,
@@ -671,7 +677,7 @@ static const struct msm_mdss_data sm8250_data = {
 	.ubwc_enc_version = UBWC_4_0,
 	.ubwc_dec_version = UBWC_4_0,
 	.ubwc_swizzle = 6,
-	.ubwc_static = 1,
+	.ubwc_bank_spread = true,
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
 	.macrotile_mode = 1,
@@ -682,7 +688,7 @@ static const struct msm_mdss_data sm8350_data = {
 	.ubwc_enc_version = UBWC_4_0,
 	.ubwc_dec_version = UBWC_4_0,
 	.ubwc_swizzle = 6,
-	.ubwc_static = 1,
+	.ubwc_bank_spread = true,
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
 	.macrotile_mode = 1,
@@ -693,7 +699,7 @@ static const struct msm_mdss_data sm8550_data = {
 	.ubwc_enc_version = UBWC_4_0,
 	.ubwc_dec_version = UBWC_4_3,
 	.ubwc_swizzle = 6,
-	.ubwc_static = 1,
+	.ubwc_bank_spread = true,
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
 	.macrotile_mode = 1,
@@ -704,7 +710,7 @@ static const struct msm_mdss_data x1e80100_data = {
 	.ubwc_enc_version = UBWC_4_0,
 	.ubwc_dec_version = UBWC_4_3,
 	.ubwc_swizzle = 6,
-	.ubwc_static = 1,
+	.ubwc_bank_spread = true,
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
 	.macrotile_mode = 1,
diff --git a/drivers/gpu/drm/msm/msm_mdss.h b/drivers/gpu/drm/msm/msm_mdss.h
index 3afef4b1786d28902799333ff66c8b3ad0ab77fa..715e1426093de5a4f3b7d2b66b889573c30b7b5c 100644
--- a/drivers/gpu/drm/msm/msm_mdss.h
+++ b/drivers/gpu/drm/msm/msm_mdss.h
@@ -13,7 +13,8 @@ struct msm_mdss_data {
 	u32 ubwc_swizzle;
 	u32 ubwc_static;
 	u32 highest_bank_bit;
-	u32 macrotile_mode;
+	bool ubwc_bank_spread;
+	bool macrotile_mode;
 	u32 reg_bus_bw;
 };
 
diff --git a/drivers/gpu/drm/msm/registers/display/mdss.xml b/drivers/gpu/drm/msm/registers/display/mdss.xml
index ac85caf1575c7908bcf68f0249da38dccf4f07b6..b6f93984928522a35a782cbad9de006eac225725 100644
--- a/drivers/gpu/drm/msm/registers/display/mdss.xml
+++ b/drivers/gpu/drm/msm/registers/display/mdss.xml
@@ -21,7 +21,16 @@ xsi:schemaLocation="https://gitlab.freedesktop.org/freedreno/ rules-fd.xsd">
 
 	<reg32 offset="0x00058" name="UBWC_DEC_HW_VERSION"/>
 
-	<reg32 offset="0x00144" name="UBWC_STATIC"/>
+	<reg32 offset="0x00144" name="UBWC_STATIC">
+		<bitfield name="UBWC_SWIZZLE" low="0" high="2"/>
+		<bitfield name="UBWC_BANK_SPREAD" pos="3"/>
+		<!-- high=5 for UBWC < 4.0 -->
+		<bitfield name="HIGHEST_BANK_BIT" low="4" high="6"/>
+		<bitfield name="UBWC_MIN_ACC_LEN" pos="8"/>
+		<bitfield name="UBWC_AMSBC" pos="10"/>
+		<bitfield name="MACROTILE_MODE" pos="12"/>
+	</reg32>
+
 	<reg32 offset="0x00150" name="UBWC_CTRL_2"/>
 	<reg32 offset="0x00154" name="UBWC_PREDICTION_MODE"/>
 </domain>

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] drm/msm/mdss: reuse defined bitfields for UBWC 2.0
  2024-11-23  5:44 [PATCH v2 0/3] drm/msm/mdss: rework UBWC registers programming Dmitry Baryshkov
  2024-11-23  5:44 ` [PATCH v2 1/3] drm/msm/mdss: define bitfields for the UBWC_STATIC register Dmitry Baryshkov
@ 2024-11-23  5:44 ` Dmitry Baryshkov
  2024-11-26  2:17   ` Abhinav Kumar
  2024-11-23  5:44 ` [PATCH v2 3/3] drm/msm/mdss: use boolean values for macrotile_mode Dmitry Baryshkov
  2 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2024-11-23  5:44 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
	Connor Abbott, David Airlie, Simona Vetter
  Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno

Follow other msm_mdss_setup_ubwc_dec_nn functions and use individual
bits instead of just specifying the value to be programmed to the
UBWC_STATIC register.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 17 +++++++++++++----
 drivers/gpu/drm/msm/msm_mdss.h |  1 -
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 4b57f39bec4e6232a0f5b4d49f8ae1200e74ac78..2fdad0fa4bc159e9a10755da2c0402fd87734aee 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -166,8 +166,16 @@ static int _msm_mdss_irq_domain_add(struct msm_mdss *msm_mdss)
 static void msm_mdss_setup_ubwc_dec_20(struct msm_mdss *msm_mdss)
 {
 	const struct msm_mdss_data *data = msm_mdss->mdss_data;
+	u32 value = MDSS_UBWC_STATIC_UBWC_SWIZZLE(data->ubwc_swizzle) |
+		    MDSS_UBWC_STATIC_HIGHEST_BANK_BIT(data->highest_bank_bit);
 
-	writel_relaxed(data->ubwc_static, msm_mdss->mmio + REG_MDSS_UBWC_STATIC);
+	if (data->ubwc_bank_spread)
+		value |= MDSS_UBWC_STATIC_UBWC_BANK_SPREAD;
+
+	if (data->ubwc_enc_version == UBWC_1_0)
+		value |= MDSS_UBWC_STATIC_UBWC_MIN_ACC_LEN;
+
+	writel_relaxed(value, msm_mdss->mmio + REG_MDSS_UBWC_STATIC);
 }
 
 static void msm_mdss_setup_ubwc_dec_30(struct msm_mdss *msm_mdss)
@@ -587,7 +595,8 @@ static const struct msm_mdss_data sa8775p_data = {
 static const struct msm_mdss_data sc7180_data = {
 	.ubwc_enc_version = UBWC_2_0,
 	.ubwc_dec_version = UBWC_2_0,
-	.ubwc_static = 0x1e,
+	.ubwc_swizzle = 6,
+	.ubwc_bank_spread = true,
 	.highest_bank_bit = 0x1,
 	.reg_bus_bw = 76800,
 };
@@ -638,7 +647,7 @@ static const struct msm_mdss_data sm6350_data = {
 	.ubwc_enc_version = UBWC_2_0,
 	.ubwc_dec_version = UBWC_2_0,
 	.ubwc_swizzle = 6,
-	.ubwc_static = 0x1e,
+	.ubwc_bank_spread = true,
 	.highest_bank_bit = 1,
 	.reg_bus_bw = 76800,
 };
@@ -661,7 +670,7 @@ static const struct msm_mdss_data sm6115_data = {
 	.ubwc_enc_version = UBWC_1_0,
 	.ubwc_dec_version = UBWC_2_0,
 	.ubwc_swizzle = 7,
-	.ubwc_static = 0x11f,
+	.ubwc_bank_spread = true,
 	.highest_bank_bit = 0x1,
 	.reg_bus_bw = 76800,
 };
diff --git a/drivers/gpu/drm/msm/msm_mdss.h b/drivers/gpu/drm/msm/msm_mdss.h
index 715e1426093de5a4f3b7d2b66b889573c30b7b5c..14dc53704314558841ee1fe08d93309fd2233812 100644
--- a/drivers/gpu/drm/msm/msm_mdss.h
+++ b/drivers/gpu/drm/msm/msm_mdss.h
@@ -11,7 +11,6 @@ struct msm_mdss_data {
 	/* can be read from register 0x58 */
 	u32 ubwc_dec_version;
 	u32 ubwc_swizzle;
-	u32 ubwc_static;
 	u32 highest_bank_bit;
 	bool ubwc_bank_spread;
 	bool macrotile_mode;

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] drm/msm/mdss: use boolean values for macrotile_mode
  2024-11-23  5:44 [PATCH v2 0/3] drm/msm/mdss: rework UBWC registers programming Dmitry Baryshkov
  2024-11-23  5:44 ` [PATCH v2 1/3] drm/msm/mdss: define bitfields for the UBWC_STATIC register Dmitry Baryshkov
  2024-11-23  5:44 ` [PATCH v2 2/3] drm/msm/mdss: reuse defined bitfields for UBWC 2.0 Dmitry Baryshkov
@ 2024-11-23  5:44 ` Dmitry Baryshkov
  2024-11-26  2:19   ` Abhinav Kumar
  2 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2024-11-23  5:44 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
	Connor Abbott, David Airlie, Simona Vetter
  Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno

The macrotile_mode is a flag, not a bit value. Use true/false values to
set it rather than 1/0.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 2fdad0fa4bc159e9a10755da2c0402fd87734aee..2d9db179accb0fd8666fe80371ea44a1fcc15e1f 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -588,7 +588,7 @@ static const struct msm_mdss_data sa8775p_data = {
 	.ubwc_swizzle = 4,
 	.ubwc_bank_spread = true,
 	.highest_bank_bit = 0,
-	.macrotile_mode = 1,
+	.macrotile_mode = true,
 	.reg_bus_bw = 74000,
 };
 
@@ -607,7 +607,7 @@ static const struct msm_mdss_data sc7280_data = {
 	.ubwc_swizzle = 6,
 	.ubwc_bank_spread = true,
 	.highest_bank_bit = 1,
-	.macrotile_mode = 1,
+	.macrotile_mode = true,
 	.reg_bus_bw = 74000,
 };
 
@@ -615,7 +615,7 @@ static const struct msm_mdss_data sc8180x_data = {
 	.ubwc_enc_version = UBWC_3_0,
 	.ubwc_dec_version = UBWC_3_0,
 	.highest_bank_bit = 3,
-	.macrotile_mode = 1,
+	.macrotile_mode = true,
 	.reg_bus_bw = 76800,
 };
 
@@ -625,7 +625,7 @@ static const struct msm_mdss_data sc8280xp_data = {
 	.ubwc_swizzle = 6,
 	.ubwc_bank_spread = true,
 	.highest_bank_bit = 3,
-	.macrotile_mode = 1,
+	.macrotile_mode = true,
 	.reg_bus_bw = 76800,
 };
 
@@ -689,7 +689,7 @@ static const struct msm_mdss_data sm8250_data = {
 	.ubwc_bank_spread = true,
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
-	.macrotile_mode = 1,
+	.macrotile_mode = true,
 	.reg_bus_bw = 76800,
 };
 
@@ -700,7 +700,7 @@ static const struct msm_mdss_data sm8350_data = {
 	.ubwc_bank_spread = true,
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
-	.macrotile_mode = 1,
+	.macrotile_mode = true,
 	.reg_bus_bw = 74000,
 };
 
@@ -711,7 +711,7 @@ static const struct msm_mdss_data sm8550_data = {
 	.ubwc_bank_spread = true,
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
-	.macrotile_mode = 1,
+	.macrotile_mode = true,
 	.reg_bus_bw = 57000,
 };
 
@@ -722,7 +722,7 @@ static const struct msm_mdss_data x1e80100_data = {
 	.ubwc_bank_spread = true,
 	/* TODO: highest_bank_bit = 2 for LP_DDR4 */
 	.highest_bank_bit = 3,
-	.macrotile_mode = 1,
+	.macrotile_mode = true,
 	/* TODO: Add reg_bus_bw with real value */
 };
 

-- 
2.39.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] drm/msm/mdss: define bitfields for the UBWC_STATIC register
  2024-11-23  5:44 ` [PATCH v2 1/3] drm/msm/mdss: define bitfields for the UBWC_STATIC register Dmitry Baryshkov
@ 2024-11-26  2:03   ` Abhinav Kumar
  2024-11-26 12:17     ` Dmitry Baryshkov
  0 siblings, 1 reply; 8+ messages in thread
From: Abhinav Kumar @ 2024-11-26  2:03 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten,
	Connor Abbott, David Airlie, Simona Vetter
  Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno



On 11/22/2024 9:44 PM, Dmitry Baryshkov wrote:
> Rather than hand-coding UBWC_STATIC value calculation, define
> corresponding bitfields and use them to setup the register value.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/msm_mdss.c                 | 38 +++++++++++++++-----------
>   drivers/gpu/drm/msm/msm_mdss.h                 |  3 +-
>   drivers/gpu/drm/msm/registers/display/mdss.xml | 11 +++++++-
>   3 files changed, 34 insertions(+), 18 deletions(-)
> 

<snip>

>   
> diff --git a/drivers/gpu/drm/msm/registers/display/mdss.xml b/drivers/gpu/drm/msm/registers/display/mdss.xml
> index ac85caf1575c7908bcf68f0249da38dccf4f07b6..b6f93984928522a35a782cbad9de006eac225725 100644
> --- a/drivers/gpu/drm/msm/registers/display/mdss.xml
> +++ b/drivers/gpu/drm/msm/registers/display/mdss.xml
> @@ -21,7 +21,16 @@ xsi:schemaLocation="https://gitlab.freedesktop.org/freedreno/ rules-fd.xsd">
>   
>   	<reg32 offset="0x00058" name="UBWC_DEC_HW_VERSION"/>
>   
> -	<reg32 offset="0x00144" name="UBWC_STATIC"/>
> +	<reg32 offset="0x00144" name="UBWC_STATIC">
> +		<bitfield name="UBWC_SWIZZLE" low="0" high="2"/>
> +		<bitfield name="UBWC_BANK_SPREAD" pos="3"/>
> +		<!-- high=5 for UBWC < 4.0 -->
> +		<bitfield name="HIGHEST_BANK_BIT" low="4" high="6"/>
> +		<bitfield name="UBWC_MIN_ACC_LEN" pos="8"/>

MIN_ACC_LEN OR MALSIZE has 2 bits , bits 8 and 9.

But bit 9 is unused today. Hence we were using it as a 1 or 0 today.

Its unused on all the chipsets I checked. Do you want to continue using 
the same way or correct this?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] drm/msm/mdss: reuse defined bitfields for UBWC 2.0
  2024-11-23  5:44 ` [PATCH v2 2/3] drm/msm/mdss: reuse defined bitfields for UBWC 2.0 Dmitry Baryshkov
@ 2024-11-26  2:17   ` Abhinav Kumar
  0 siblings, 0 replies; 8+ messages in thread
From: Abhinav Kumar @ 2024-11-26  2:17 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten,
	Connor Abbott, David Airlie, Simona Vetter
  Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno



On 11/22/2024 9:44 PM, Dmitry Baryshkov wrote:
> Follow other msm_mdss_setup_ubwc_dec_nn functions and use individual
> bits instead of just specifying the value to be programmed to the
> UBWC_STATIC register.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/msm_mdss.c | 17 +++++++++++++----
>   drivers/gpu/drm/msm/msm_mdss.h |  1 -
>   2 files changed, 13 insertions(+), 5 deletions(-)
> 

Same comment as patch 1 but rest LGTM, will ack this one once we decide 
on patch 1.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] drm/msm/mdss: use boolean values for macrotile_mode
  2024-11-23  5:44 ` [PATCH v2 3/3] drm/msm/mdss: use boolean values for macrotile_mode Dmitry Baryshkov
@ 2024-11-26  2:19   ` Abhinav Kumar
  0 siblings, 0 replies; 8+ messages in thread
From: Abhinav Kumar @ 2024-11-26  2:19 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten,
	Connor Abbott, David Airlie, Simona Vetter
  Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno



On 11/22/2024 9:44 PM, Dmitry Baryshkov wrote:
> The macrotile_mode is a flag, not a bit value. Use true/false values to
> set it rather than 1/0.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/msm_mdss.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] drm/msm/mdss: define bitfields for the UBWC_STATIC register
  2024-11-26  2:03   ` Abhinav Kumar
@ 2024-11-26 12:17     ` Dmitry Baryshkov
  0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Baryshkov @ 2024-11-26 12:17 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Rob Clark, Sean Paul, Marijn Suijten, Connor Abbott, David Airlie,
	Simona Vetter, linux-kernel, linux-arm-msm, dri-devel, freedreno

On Mon, Nov 25, 2024 at 06:03:52PM -0800, Abhinav Kumar wrote:
> 
> 
> On 11/22/2024 9:44 PM, Dmitry Baryshkov wrote:
> > Rather than hand-coding UBWC_STATIC value calculation, define
> > corresponding bitfields and use them to setup the register value.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/msm_mdss.c                 | 38 +++++++++++++++-----------
> >   drivers/gpu/drm/msm/msm_mdss.h                 |  3 +-
> >   drivers/gpu/drm/msm/registers/display/mdss.xml | 11 +++++++-
> >   3 files changed, 34 insertions(+), 18 deletions(-)
> > 
> 
> <snip>
> 
> > diff --git a/drivers/gpu/drm/msm/registers/display/mdss.xml b/drivers/gpu/drm/msm/registers/display/mdss.xml
> > index ac85caf1575c7908bcf68f0249da38dccf4f07b6..b6f93984928522a35a782cbad9de006eac225725 100644
> > --- a/drivers/gpu/drm/msm/registers/display/mdss.xml
> > +++ b/drivers/gpu/drm/msm/registers/display/mdss.xml
> > @@ -21,7 +21,16 @@ xsi:schemaLocation="https://gitlab.freedesktop.org/freedreno/ rules-fd.xsd">
> >   	<reg32 offset="0x00058" name="UBWC_DEC_HW_VERSION"/>
> > -	<reg32 offset="0x00144" name="UBWC_STATIC"/>
> > +	<reg32 offset="0x00144" name="UBWC_STATIC">
> > +		<bitfield name="UBWC_SWIZZLE" low="0" high="2"/>
> > +		<bitfield name="UBWC_BANK_SPREAD" pos="3"/>
> > +		<!-- high=5 for UBWC < 4.0 -->
> > +		<bitfield name="HIGHEST_BANK_BIT" low="4" high="6"/>
> > +		<bitfield name="UBWC_MIN_ACC_LEN" pos="8"/>
> 
> MIN_ACC_LEN OR MALSIZE has 2 bits , bits 8 and 9.
> 
> But bit 9 is unused today. Hence we were using it as a 1 or 0 today.
> 
> Its unused on all the chipsets I checked. Do you want to continue using the
> same way or correct this?

Let's correct it. I will send next iteration.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-11-26 12:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-23  5:44 [PATCH v2 0/3] drm/msm/mdss: rework UBWC registers programming Dmitry Baryshkov
2024-11-23  5:44 ` [PATCH v2 1/3] drm/msm/mdss: define bitfields for the UBWC_STATIC register Dmitry Baryshkov
2024-11-26  2:03   ` Abhinav Kumar
2024-11-26 12:17     ` Dmitry Baryshkov
2024-11-23  5:44 ` [PATCH v2 2/3] drm/msm/mdss: reuse defined bitfields for UBWC 2.0 Dmitry Baryshkov
2024-11-26  2:17   ` Abhinav Kumar
2024-11-23  5:44 ` [PATCH v2 3/3] drm/msm/mdss: use boolean values for macrotile_mode Dmitry Baryshkov
2024-11-26  2:19   ` Abhinav Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox