[PATCH 0/4] Reuse some functions from the previous DCN in the newest DCNs

[PATCH 0/4] Reuse some functions from the previous DCN in the newest DCNs

[Rodrigo Siqueira]

DCN has multiple callbacks, some of which can be replaced with specific
operations for a particular ASIC. Sometimes, the implementation of the
previous ASIC can be used in a new one; traditionally, the display
approach for those cases has been to use the old function implementation
in the new ASICs if nothing changes. For example, if function Y
implemented in DCN 10 did not change anything in the DCN 30, the DCN 10
implementation is used in the DCN 30 callback. Sometimes, this approach
is not followed, which may lead to code duplication. This series
addresses some of those cases by promoting the reuse of old ASIC
implementation in new ASICs.

Thanks

Rodrigo Siqueira (4):
  drm/amd/display: Use optc31_enable_crtc implementation for new DCNs
  drm/amd/display: Use optc31_disable_crtc for DCN 31 and 401
  drm/amd/display: Uses optc31_disable_crtc for DCN35
  drm/amd/display: Replace dcn35_update_odm with dcn314_update_odm

 .../amd/display/dc/hwss/dcn35/dcn35_hwseq.c   | 52 --------------
 .../amd/display/dc/hwss/dcn35/dcn35_hwseq.h   |  2 -
 .../amd/display/dc/hwss/dcn35/dcn35_init.c    |  2 +-
 .../amd/display/dc/hwss/dcn351/dcn351_init.c  |  3 +-
 .../amd/display/dc/optc/dcn31/dcn31_optc.c    | 15 ++--
 .../amd/display/dc/optc/dcn31/dcn31_optc.h    |  4 ++
 .../amd/display/dc/optc/dcn314/dcn314_optc.c  | 27 +-------
 .../amd/display/dc/optc/dcn32/dcn32_optc.c    | 33 +--------
 .../amd/display/dc/optc/dcn35/dcn35_optc.c    | 62 +----------------
 .../amd/display/dc/optc/dcn401/dcn401_optc.c  | 68 +------------------
 .../amd/display/dc/optc/dcn401/dcn401_optc.h  |  2 -
 11 files changed, 24 insertions(+), 246 deletions(-)

-- 
2.47.2

[PATCH 1/4] drm/amd/display: Use optc31_enable_crtc implementation for new DCNs

[Rodrigo Siqueira]

The *_enable_crtc function from DCN31 to the latest DCNs has the same
implementation. This commit removes all the duplications in favor of
using the optc31_enable_crtc implementation instead of duplicating the
code across different DCNs.

Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
---
 .../amd/display/dc/optc/dcn31/dcn31_optc.c    | 13 +++++--
 .../amd/display/dc/optc/dcn31/dcn31_optc.h    |  2 ++
 .../amd/display/dc/optc/dcn314/dcn314_optc.c  | 27 +--------------
 .../amd/display/dc/optc/dcn32/dcn32_optc.c    | 33 +-----------------
 .../amd/display/dc/optc/dcn35/dcn35_optc.c    | 27 +--------------
 .../amd/display/dc/optc/dcn401/dcn401_optc.c  | 34 +------------------
 .../amd/display/dc/optc/dcn401/dcn401_optc.h  |  1 -
 7 files changed, 16 insertions(+), 121 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
index ef536f37b4ed..13c1f95b5ced 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
@@ -89,10 +89,17 @@ static void optc31_set_odm_combine(struct timing_generator *optc, int *opp_id, i
 	optc1->opp_count = opp_cnt;
 }
 
-/*
- * Enable CRTC - call ASIC Control Object to enable Timing generator.
+/**
+ * optc31_enable_crtc() - Enable CRTC
+ *
+ * @optc: Pointer to the timing generator structure
+ *
+ * This function calls ASIC Control Object to enable Timing generator.
+ *
+ * Return:
+ * Always returns true
  */
-static bool optc31_enable_crtc(struct timing_generator *optc)
+bool optc31_enable_crtc(struct timing_generator *optc)
 {
 	struct optc *optc1 = DCN10TG_FROM_TG(optc);
 
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
index 0f72c274f40b..af67eeaf8505 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
@@ -267,6 +267,8 @@ void dcn31_timing_generator_init(struct optc *optc1);
 
 bool optc31_immediate_disable_crtc(struct timing_generator *optc);
 
+bool optc31_enable_crtc(struct timing_generator *optc);
+
 void optc31_set_drr(struct timing_generator *optc, const struct drr_params *params);
 
 void optc3_init_odm(struct timing_generator *optc);
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn314/dcn314_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn314/dcn314_optc.c
index 0e603bad0d12..7b5478b09a97 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn314/dcn314_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn314/dcn314_optc.c
@@ -102,31 +102,6 @@ static void optc314_set_odm_combine(struct timing_generator *optc, int *opp_id,
 	optc1->opp_count = opp_cnt;
 }
 
-static bool optc314_enable_crtc(struct timing_generator *optc)
-{
-	struct optc *optc1 = DCN10TG_FROM_TG(optc);
-
-	/* opp instance for OTG, 1 to 1 mapping and odm will adjust */
-	REG_UPDATE(OPTC_DATA_SOURCE_SELECT,
-			OPTC_SEG0_SRC_SEL, optc->inst);
-
-	/* VTG enable first is for HW workaround */
-	REG_UPDATE(CONTROL,
-			VTG0_ENABLE, 1);
-
-	REG_SEQ_START();
-
-	/* Enable CRTC */
-	REG_UPDATE_2(OTG_CONTROL,
-			OTG_DISABLE_POINT_CNTL, 2,
-			OTG_MASTER_EN, 1);
-
-	REG_SEQ_SUBMIT();
-	REG_SEQ_WAIT_DONE();
-
-	return true;
-}
-
 /* disable_crtc */
 static bool optc314_disable_crtc(struct timing_generator *optc)
 {
@@ -199,7 +174,7 @@ static struct timing_generator_funcs dcn314_tg_funcs = {
 		.setup_vertical_interrupt1 = optc1_setup_vertical_interrupt1,
 		.setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
 		.program_global_sync = optc1_program_global_sync,
-		.enable_crtc = optc314_enable_crtc,
+		.enable_crtc = optc31_enable_crtc,
 		.disable_crtc = optc314_disable_crtc,
 		.immediate_disable_crtc = optc31_immediate_disable_crtc,
 		.phantom_crtc_post_enable = optc314_phantom_crtc_post_enable,
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.c
index 2cdd19ba634b..5a626b23ea6b 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn32/dcn32_optc.c
@@ -135,37 +135,6 @@ void optc32_set_h_timing_div_manual_mode(struct timing_generator *optc, bool man
 	REG_UPDATE(OTG_H_TIMING_CNTL,
 			OTG_H_TIMING_DIV_MODE_MANUAL, manual_mode ? 1 : 0);
 }
-/**
- * optc32_enable_crtc() - Enable CRTC - call ASIC Control Object to enable Timing generator.
- *
- * @optc: timing_generator instance.
- *
- * Return: If CRTC is enabled, return true.
- */
-static bool optc32_enable_crtc(struct timing_generator *optc)
-{
-	struct optc *optc1 = DCN10TG_FROM_TG(optc);
-
-	/* opp instance for OTG, 1 to 1 mapping and odm will adjust */
-	REG_UPDATE(OPTC_DATA_SOURCE_SELECT,
-			OPTC_SEG0_SRC_SEL, optc->inst);
-
-	/* VTG enable first is for HW workaround */
-	REG_UPDATE(CONTROL,
-			VTG0_ENABLE, 1);
-
-	REG_SEQ_START();
-
-	/* Enable CRTC */
-	REG_UPDATE_2(OTG_CONTROL,
-			OTG_DISABLE_POINT_CNTL, 2,
-			OTG_MASTER_EN, 1);
-
-	REG_SEQ_SUBMIT();
-	REG_SEQ_WAIT_DONE();
-
-	return true;
-}
 
 /* disable_crtc */
 static bool optc32_disable_crtc(struct timing_generator *optc)
@@ -304,7 +273,7 @@ static struct timing_generator_funcs dcn32_tg_funcs = {
 		.setup_vertical_interrupt1 = optc1_setup_vertical_interrupt1,
 		.setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
 		.program_global_sync = optc1_program_global_sync,
-		.enable_crtc = optc32_enable_crtc,
+		.enable_crtc = optc31_enable_crtc,
 		.disable_crtc = optc32_disable_crtc,
 		.phantom_crtc_post_enable = optc32_phantom_crtc_post_enable,
 		.disable_phantom_crtc = optc32_disable_phantom_otg,
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c
index 4cfc6c0fa147..5835aa6c3c18 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c
@@ -109,31 +109,6 @@ static void optc35_set_odm_combine(struct timing_generator *optc, int *opp_id, i
 	optc1->opp_count = opp_cnt;
 }
 
-static bool optc35_enable_crtc(struct timing_generator *optc)
-{
-	struct optc *optc1 = DCN10TG_FROM_TG(optc);
-
-	/* opp instance for OTG, 1 to 1 mapping and odm will adjust */
-	REG_UPDATE(OPTC_DATA_SOURCE_SELECT,
-			OPTC_SEG0_SRC_SEL, optc->inst);
-
-	/* VTG enable first is for HW workaround */
-	REG_UPDATE(CONTROL,
-			VTG0_ENABLE, 1);
-
-	REG_SEQ_START();
-
-	/* Enable CRTC */
-	REG_UPDATE_2(OTG_CONTROL,
-			OTG_DISABLE_POINT_CNTL, 2,
-			OTG_MASTER_EN, 1);
-
-	REG_SEQ_SUBMIT();
-	REG_SEQ_WAIT_DONE();
-
-	return true;
-}
-
 /* disable_crtc */
 static bool optc35_disable_crtc(struct timing_generator *optc)
 {
@@ -435,7 +410,7 @@ static struct timing_generator_funcs dcn35_tg_funcs = {
 		.setup_vertical_interrupt1 = optc1_setup_vertical_interrupt1,
 		.setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
 		.program_global_sync = optc1_program_global_sync,
-		.enable_crtc = optc35_enable_crtc,
+		.enable_crtc = optc31_enable_crtc,
 		.disable_crtc = optc35_disable_crtc,
 		.immediate_disable_crtc = optc31_immediate_disable_crtc,
 		.phantom_crtc_post_enable = optc35_phantom_crtc_post_enable,
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
index 382ac18e7854..6eba48de58ff 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
@@ -169,38 +169,6 @@ void optc401_set_h_timing_div_manual_mode(struct timing_generator *optc, bool ma
 	REG_UPDATE(OTG_H_TIMING_CNTL,
 			OTG_H_TIMING_DIV_MODE_MANUAL, manual_mode ? 1 : 0);
 }
-/**
- * optc401_enable_crtc() - Enable CRTC
- * @optc: Pointer to the timing generator structure
- *
- * This function calls ASIC Control Object to enable Timing generator.
- *
- * Return: Always returns true
- */
-bool optc401_enable_crtc(struct timing_generator *optc)
-{
-	struct optc *optc1 = DCN10TG_FROM_TG(optc);
-
-	/* opp instance for OTG, 1 to 1 mapping and odm will adjust */
-	REG_UPDATE(OPTC_DATA_SOURCE_SELECT,
-			OPTC_SEG0_SRC_SEL, optc->inst);
-
-	/* VTG enable first is for HW workaround */
-	REG_UPDATE(CONTROL,
-			VTG0_ENABLE, 1);
-
-	REG_SEQ_START();
-
-	/* Enable CRTC */
-	REG_UPDATE_2(OTG_CONTROL,
-			OTG_DISABLE_POINT_CNTL, 2,
-			OTG_MASTER_EN, 1);
-
-	REG_SEQ_SUBMIT();
-	REG_SEQ_WAIT_DONE();
-
-	return true;
-}
 
 /* disable_crtc */
 bool optc401_disable_crtc(struct timing_generator *optc)
@@ -466,7 +434,7 @@ static struct timing_generator_funcs dcn401_tg_funcs = {
 		.setup_vertical_interrupt1 = optc1_setup_vertical_interrupt1,
 		.setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
 		.program_global_sync = optc401_program_global_sync,
-		.enable_crtc = optc401_enable_crtc,
+		.enable_crtc = optc31_enable_crtc,
 		.disable_crtc = optc401_disable_crtc,
 		.phantom_crtc_post_enable = optc401_phantom_crtc_post_enable,
 		.disable_phantom_crtc = optc401_disable_phantom_otg,
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
index fa62737b5b1b..8e795e06e615 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
@@ -180,7 +180,6 @@ void optc401_program_global_sync(
 		int vupdate_offset,
 		int vupdate_width,
 		int pstate_keepout);
-bool optc401_enable_crtc(struct timing_generator *optc);
 bool optc401_disable_crtc(struct timing_generator *optc);
 void optc401_phantom_crtc_post_enable(struct timing_generator *optc);
 void optc401_disable_phantom_otg(struct timing_generator *optc);
-- 
2.47.2

[PATCH 2/4] drm/amd/display: Use optc31_disable_crtc for DCN 31 and 401

[Rodrigo Siqueira]

DCN401 and DCN31 share the same implementation for disabling CRTC. This
commit makes DCN401 use the DCN31 implementation and removes the code
duplication in the DCN401.

Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
---
 .../amd/display/dc/optc/dcn31/dcn31_optc.c    |  2 +-
 .../amd/display/dc/optc/dcn31/dcn31_optc.h    |  2 ++
 .../amd/display/dc/optc/dcn401/dcn401_optc.c  | 34 +------------------
 .../amd/display/dc/optc/dcn401/dcn401_optc.h  |  1 -
 4 files changed, 4 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
index 13c1f95b5ced..e6246e5ba86f 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
@@ -125,7 +125,7 @@ bool optc31_enable_crtc(struct timing_generator *optc)
 }
 
 /* disable_crtc - call ASIC Control Object to disable Timing generator. */
-static bool optc31_disable_crtc(struct timing_generator *optc)
+bool optc31_disable_crtc(struct timing_generator *optc)
 {
 	struct optc *optc1 = DCN10TG_FROM_TG(optc);
 
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
index af67eeaf8505..db7e51fc787e 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
@@ -267,6 +267,8 @@ void dcn31_timing_generator_init(struct optc *optc1);
 
 bool optc31_immediate_disable_crtc(struct timing_generator *optc);
 
+bool optc31_disable_crtc(struct timing_generator *optc);
+
 bool optc31_enable_crtc(struct timing_generator *optc);
 
 void optc31_set_drr(struct timing_generator *optc, const struct drr_params *params);
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
index 6eba48de58ff..f472d2efe026 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
@@ -170,38 +170,6 @@ void optc401_set_h_timing_div_manual_mode(struct timing_generator *optc, bool ma
 			OTG_H_TIMING_DIV_MODE_MANUAL, manual_mode ? 1 : 0);
 }
 
-/* disable_crtc */
-bool optc401_disable_crtc(struct timing_generator *optc)
-{
-	struct optc *optc1 = DCN10TG_FROM_TG(optc);
-
-	REG_UPDATE_5(OPTC_DATA_SOURCE_SELECT,
-			OPTC_SEG0_SRC_SEL, 0xf,
-			OPTC_SEG1_SRC_SEL, 0xf,
-			OPTC_SEG2_SRC_SEL, 0xf,
-			OPTC_SEG3_SRC_SEL, 0xf,
-			OPTC_NUM_OF_INPUT_SEGMENT, 0);
-
-	REG_UPDATE(OPTC_MEMORY_CONFIG,
-			OPTC_MEM_SEL, 0);
-
-	/* disable otg request until end of the first line
-	 * in the vertical blank region
-	 */
-	REG_UPDATE(OTG_CONTROL,
-			OTG_MASTER_EN, 0);
-
-	REG_UPDATE(CONTROL,
-			VTG0_ENABLE, 0);
-
-	/* CRTC disabled, so disable  clock. */
-	REG_WAIT(OTG_CLOCK_CONTROL,
-			OTG_BUSY, 0,
-			1, 150000);
-
-	return true;
-}
-
 void optc401_phantom_crtc_post_enable(struct timing_generator *optc)
 {
 	struct optc *optc1 = DCN10TG_FROM_TG(optc);
@@ -435,7 +403,7 @@ static struct timing_generator_funcs dcn401_tg_funcs = {
 		.setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
 		.program_global_sync = optc401_program_global_sync,
 		.enable_crtc = optc31_enable_crtc,
-		.disable_crtc = optc401_disable_crtc,
+		.disable_crtc = optc31_disable_crtc,
 		.phantom_crtc_post_enable = optc401_phantom_crtc_post_enable,
 		.disable_phantom_crtc = optc401_disable_phantom_otg,
 		/* used by enable_timing_synchronization. Not need for FPGA */
diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
index 8e795e06e615..be74fd709d43 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
@@ -180,7 +180,6 @@ void optc401_program_global_sync(
 		int vupdate_offset,
 		int vupdate_width,
 		int pstate_keepout);
-bool optc401_disable_crtc(struct timing_generator *optc);
 void optc401_phantom_crtc_post_enable(struct timing_generator *optc);
 void optc401_disable_phantom_otg(struct timing_generator *optc);
 void optc401_set_odm_bypass(struct timing_generator *optc,
-- 
2.47.2

[PATCH 3/4] drm/amd/display: Uses optc31_disable_crtc for DCN35

[Rodrigo Siqueira]

The optc35_disable_crtc() function is a copy & paste from
optc31_disable_crtc. This commit removes the duplication in favor of
using optc31_disable_crtc.

Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
---
 .../amd/display/dc/optc/dcn35/dcn35_optc.c    | 35 +------------------
 1 file changed, 1 insertion(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c
index 5835aa6c3c18..ec12aead3ced 100644
--- a/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c
+++ b/drivers/gpu/drm/amd/display/dc/optc/dcn35/dcn35_optc.c
@@ -109,39 +109,6 @@ static void optc35_set_odm_combine(struct timing_generator *optc, int *opp_id, i
 	optc1->opp_count = opp_cnt;
 }
 
-/* disable_crtc */
-static bool optc35_disable_crtc(struct timing_generator *optc)
-{
-	struct optc *optc1 = DCN10TG_FROM_TG(optc);
-
-	REG_UPDATE_5(OPTC_DATA_SOURCE_SELECT,
-			OPTC_SEG0_SRC_SEL, 0xf,
-			OPTC_SEG1_SRC_SEL, 0xf,
-			OPTC_SEG2_SRC_SEL, 0xf,
-			OPTC_SEG3_SRC_SEL, 0xf,
-			OPTC_NUM_OF_INPUT_SEGMENT, 0);
-
-	REG_UPDATE(OPTC_MEMORY_CONFIG,
-			OPTC_MEM_SEL, 0);
-
-	/* disable otg request until end of the first line
-	 * in the vertical blank region
-	 */
-	REG_UPDATE(OTG_CONTROL,
-			OTG_MASTER_EN, 0);
-
-	REG_UPDATE(CONTROL,
-			VTG0_ENABLE, 0);
-
-	/* CRTC disabled, so disable  clock. */
-	REG_WAIT(OTG_CLOCK_CONTROL,
-			OTG_BUSY, 0,
-			1, 100000);
-	optc1_clear_optc_underflow(optc);
-
-	return true;
-}
-
 static void optc35_phantom_crtc_post_enable(struct timing_generator *optc)
 {
 	struct optc *optc1 = DCN10TG_FROM_TG(optc);
@@ -411,7 +378,7 @@ static struct timing_generator_funcs dcn35_tg_funcs = {
 		.setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
 		.program_global_sync = optc1_program_global_sync,
 		.enable_crtc = optc31_enable_crtc,
-		.disable_crtc = optc35_disable_crtc,
+		.disable_crtc = optc31_disable_crtc,
 		.immediate_disable_crtc = optc31_immediate_disable_crtc,
 		.phantom_crtc_post_enable = optc35_phantom_crtc_post_enable,
 		/* used by enable_timing_synchronization. Not need for FPGA */
-- 
2.47.2

[PATCH 4/4] drm/amd/display: Replace dcn35_update_odm with dcn314_update_odm

[Rodrigo Siqueira]

dcn35_update_odm has the same implementation as dcn314_update_odm. This
commit removes the duplicate implementation by using dcn314_update_odm
in the DCN35 code.

Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
---
 .../amd/display/dc/hwss/dcn35/dcn35_hwseq.c   | 52 -------------------
 .../amd/display/dc/hwss/dcn35/dcn35_hwseq.h   |  2 -
 .../amd/display/dc/hwss/dcn35/dcn35_init.c    |  2 +-
 .../amd/display/dc/hwss/dcn351/dcn351_init.c  |  3 +-
 4 files changed, 3 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
index 922b8d71cf1a..abcbd1759465 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.c
@@ -420,58 +420,6 @@ static unsigned int get_odm_config(struct pipe_ctx *pipe_ctx, unsigned int *opp_
 	return opp_count;
 }
 
-void dcn35_update_odm(struct dc *dc, struct dc_state *context, struct pipe_ctx *pipe_ctx)
-{
-	struct pipe_ctx *odm_pipe;
-	int opp_cnt = 0;
-	int opp_inst[MAX_PIPES] = {0};
-	int odm_slice_width = resource_get_odm_slice_dst_width(pipe_ctx, false);
-	int last_odm_slice_width = resource_get_odm_slice_dst_width(pipe_ctx, true);
-	struct mpc *mpc = dc->res_pool->mpc;
-	int i;
-
-	opp_cnt = get_odm_config(pipe_ctx, opp_inst);
-
-	if (opp_cnt > 1)
-		pipe_ctx->stream_res.tg->funcs->set_odm_combine(
-				pipe_ctx->stream_res.tg,
-				opp_inst, opp_cnt,
-				odm_slice_width, last_odm_slice_width);
-	else
-		pipe_ctx->stream_res.tg->funcs->set_odm_bypass(
-				pipe_ctx->stream_res.tg, &pipe_ctx->stream->timing);
-
-	if (mpc->funcs->set_out_rate_control) {
-		for (i = 0; i < opp_cnt; ++i) {
-			mpc->funcs->set_out_rate_control(
-					mpc, opp_inst[i],
-					false,
-					0,
-					NULL);
-		}
-	}
-
-	for (odm_pipe = pipe_ctx->next_odm_pipe; odm_pipe; odm_pipe = odm_pipe->next_odm_pipe) {
-		odm_pipe->stream_res.opp->funcs->opp_pipe_clock_control(
-				odm_pipe->stream_res.opp,
-				true);
-	}
-
-	if (pipe_ctx->stream_res.dsc) {
-		struct pipe_ctx *current_pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[pipe_ctx->pipe_idx];
-
-		update_dsc_on_stream(pipe_ctx, pipe_ctx->stream->timing.flags.DSC);
-
-		/* Check if no longer using pipe for ODM, then need to disconnect DSC for that pipe */
-		if (!pipe_ctx->next_odm_pipe && current_pipe_ctx->next_odm_pipe &&
-				current_pipe_ctx->next_odm_pipe->stream_res.dsc) {
-			struct display_stream_compressor *dsc = current_pipe_ctx->next_odm_pipe->stream_res.dsc;
-			/* disconnect DSC block from stream */
-			dsc->funcs->dsc_disconnect(dsc);
-		}
-	}
-}
-
 void dcn35_dpp_root_clock_control(struct dce_hwseq *hws, unsigned int dpp_inst, bool clock_on)
 {
 	if (!hws->ctx->dc->debug.root_clock_optimization.bits.dpp)
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.h b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.h
index 0b1d6f608edd..6ca6924e7379 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.h
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_hwseq.h
@@ -31,8 +31,6 @@
 
 struct dc;
 
-void dcn35_update_odm(struct dc *dc, struct dc_state *context, struct pipe_ctx *pipe_ctx);
-
 void dcn35_dsc_pg_control(struct dce_hwseq *hws, unsigned int dsc_inst, bool power_on);
 
 void dcn35_dpp_root_clock_control(struct dce_hwseq *hws, unsigned int dpp_inst, bool clock_on);
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_init.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_init.c
index a3ccf805bd16..3a14c6374324 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_init.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn35/dcn35_init.c
@@ -155,7 +155,7 @@ static const struct hwseq_private_funcs dcn35_private_funcs = {
 	.dpstream_root_clock_control = dcn35_dpstream_root_clock_control,
 	.physymclk_root_clock_control = dcn35_physymclk_root_clock_control,
 	.program_all_writeback_pipes_in_tree = dcn30_program_all_writeback_pipes_in_tree,
-	.update_odm = dcn35_update_odm,
+	.update_odm = dcn314_update_odm,
 	.set_hdr_multiplier = dcn10_set_hdr_multiplier,
 	.verify_allow_pstate_change_high = dcn10_verify_allow_pstate_change_high,
 	.wait_for_blank_complete = dcn20_wait_for_blank_complete,
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn351/dcn351_init.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn351/dcn351_init.c
index 58f2be2a326b..24bd1bbf4461 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn351/dcn351_init.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn351/dcn351_init.c
@@ -30,6 +30,7 @@
 #include "dcn30/dcn30_hwseq.h"
 #include "dcn301/dcn301_hwseq.h"
 #include "dcn31/dcn31_hwseq.h"
+#include "dcn314/dcn314_hwseq.h"
 #include "dcn32/dcn32_hwseq.h"
 #include "dcn35/dcn35_hwseq.h"
 #include "dcn351/dcn351_hwseq.h"
@@ -150,7 +151,7 @@ static const struct hwseq_private_funcs dcn351_private_funcs = {
 	.dpstream_root_clock_control = dcn35_dpstream_root_clock_control,
 	.physymclk_root_clock_control = dcn35_physymclk_root_clock_control,
 	.program_all_writeback_pipes_in_tree = dcn30_program_all_writeback_pipes_in_tree,
-	.update_odm = dcn35_update_odm,
+	.update_odm = dcn314_update_odm,
 	.set_hdr_multiplier = dcn10_set_hdr_multiplier,
 	.verify_allow_pstate_change_high = dcn10_verify_allow_pstate_change_high,
 	.wait_for_blank_complete = dcn20_wait_for_blank_complete,
-- 
2.47.2

Re: [PATCH 2/4] drm/amd/display: Use optc31_disable_crtc for DCN 31 and 401

[Alex Hung]

On 5/4/25 16:11, Rodrigo Siqueira wrote:
> DCN401 and DCN31 share the same implementation for disabling CRTC. This
> commit makes DCN401 use the DCN31 implementation and removes the code
> duplication in the DCN401.

Hi Rodrigo

optc31_disable_crtc is not the same as optc401_disable_crtc. Please see 
the dfiff below:

< /* disable_crtc - call ASIC Control Object to disable Timing generator. */
< static bool optc31_disable_crtc(struct timing_generator *optc)
---
 > /* disable_crtc */
 > bool optc401_disable_crtc(struct timing_generator *optc)
147,148c232
<                       1, 100000);
<       optc1_clear_optc_underflow(optc);
---
 >                       1, 150000);
152,155c236,237


However, optc31_disable_crtc is the same as optc35_disable_crtc (patch 
3?) and optc32_disable_crtc is the same as optc401_disable_crtc.

> 
> Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
> ---
>   .../amd/display/dc/optc/dcn31/dcn31_optc.c    |  2 +-
>   .../amd/display/dc/optc/dcn31/dcn31_optc.h    |  2 ++
>   .../amd/display/dc/optc/dcn401/dcn401_optc.c  | 34 +------------------
>   .../amd/display/dc/optc/dcn401/dcn401_optc.h  |  1 -
>   4 files changed, 4 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
> index 13c1f95b5ced..e6246e5ba86f 100644
> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
> @@ -125,7 +125,7 @@ bool optc31_enable_crtc(struct timing_generator *optc)
>   }
>   
>   /* disable_crtc - call ASIC Control Object to disable Timing generator. */
> -static bool optc31_disable_crtc(struct timing_generator *optc)
> +bool optc31_disable_crtc(struct timing_generator *optc)
>   {
>   	struct optc *optc1 = DCN10TG_FROM_TG(optc);
>   
> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
> index af67eeaf8505..db7e51fc787e 100644
> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
> @@ -267,6 +267,8 @@ void dcn31_timing_generator_init(struct optc *optc1);
>   
>   bool optc31_immediate_disable_crtc(struct timing_generator *optc);
>   
> +bool optc31_disable_crtc(struct timing_generator *optc);
> +
>   bool optc31_enable_crtc(struct timing_generator *optc);
>   
>   void optc31_set_drr(struct timing_generator *optc, const struct drr_params *params);
> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
> index 6eba48de58ff..f472d2efe026 100644
> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
> @@ -170,38 +170,6 @@ void optc401_set_h_timing_div_manual_mode(struct timing_generator *optc, bool ma
>   			OTG_H_TIMING_DIV_MODE_MANUAL, manual_mode ? 1 : 0);
>   }
>   
> -/* disable_crtc */
> -bool optc401_disable_crtc(struct timing_generator *optc)
> -{
> -	struct optc *optc1 = DCN10TG_FROM_TG(optc);
> -
> -	REG_UPDATE_5(OPTC_DATA_SOURCE_SELECT,
> -			OPTC_SEG0_SRC_SEL, 0xf,
> -			OPTC_SEG1_SRC_SEL, 0xf,
> -			OPTC_SEG2_SRC_SEL, 0xf,
> -			OPTC_SEG3_SRC_SEL, 0xf,
> -			OPTC_NUM_OF_INPUT_SEGMENT, 0);
> -
> -	REG_UPDATE(OPTC_MEMORY_CONFIG,
> -			OPTC_MEM_SEL, 0);
> -
> -	/* disable otg request until end of the first line
> -	 * in the vertical blank region
> -	 */
> -	REG_UPDATE(OTG_CONTROL,
> -			OTG_MASTER_EN, 0);
> -
> -	REG_UPDATE(CONTROL,
> -			VTG0_ENABLE, 0);
> -
> -	/* CRTC disabled, so disable  clock. */
> -	REG_WAIT(OTG_CLOCK_CONTROL,
> -			OTG_BUSY, 0,
> -			1, 150000);
> -
> -	return true;
> -}
> -
>   void optc401_phantom_crtc_post_enable(struct timing_generator *optc)
>   {
>   	struct optc *optc1 = DCN10TG_FROM_TG(optc);
> @@ -435,7 +403,7 @@ static struct timing_generator_funcs dcn401_tg_funcs = {
>   		.setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
>   		.program_global_sync = optc401_program_global_sync,
>   		.enable_crtc = optc31_enable_crtc,
> -		.disable_crtc = optc401_disable_crtc,
> +		.disable_crtc = optc31_disable_crtc,
>   		.phantom_crtc_post_enable = optc401_phantom_crtc_post_enable,
>   		.disable_phantom_crtc = optc401_disable_phantom_otg,
>   		/* used by enable_timing_synchronization. Not need for FPGA */
> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
> index 8e795e06e615..be74fd709d43 100644
> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
> @@ -180,7 +180,6 @@ void optc401_program_global_sync(
>   		int vupdate_offset,
>   		int vupdate_width,
>   		int pstate_keepout);
> -bool optc401_disable_crtc(struct timing_generator *optc);
>   void optc401_phantom_crtc_post_enable(struct timing_generator *optc);
>   void optc401_disable_phantom_otg(struct timing_generator *optc);
>   void optc401_set_odm_bypass(struct timing_generator *optc,

Re: [PATCH 2/4] drm/amd/display: Use optc31_disable_crtc for DCN 31 and 401

[Mario Limonciello]

On 5/5/2025 11:27 AM, Alex Hung wrote:
> On 5/4/25 16:11, Rodrigo Siqueira wrote:
>> DCN401 and DCN31 share the same implementation for disabling CRTC. This
>> commit makes DCN401 use the DCN31 implementation and removes the code
>> duplication in the DCN401.
> 
> Hi Rodrigo
> 
> optc31_disable_crtc is not the same as optc401_disable_crtc. Please see 
> the dfiff below:
> 
> < /* disable_crtc - call ASIC Control Object to disable Timing 
> generator. */
> < static bool optc31_disable_crtc(struct timing_generator *optc)
> ---
>  > /* disable_crtc */
>  > bool optc401_disable_crtc(struct timing_generator *optc)
> 147,148c232
> <                       1, 100000);
> <       optc1_clear_optc_underflow(optc);
> ---
>  >                       1, 150000);
> 152,155c236,237
> 
> 
> However, optc31_disable_crtc is the same as optc35_disable_crtc (patch 
> 3?) and optc32_disable_crtc is the same as optc401_disable_crtc.

Is that last argument a timeout?  How about just extending the timeout 
to be the same for all of them?  That should be relatively harmless, no?

> 
>>
>> Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
>> ---
>>   .../amd/display/dc/optc/dcn31/dcn31_optc.c    |  2 +-
>>   .../amd/display/dc/optc/dcn31/dcn31_optc.h    |  2 ++
>>   .../amd/display/dc/optc/dcn401/dcn401_optc.c  | 34 +------------------
>>   .../amd/display/dc/optc/dcn401/dcn401_optc.h  |  1 -
>>   4 files changed, 4 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c b/ 
>> drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
>> index 13c1f95b5ced..e6246e5ba86f 100644
>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
>> @@ -125,7 +125,7 @@ bool optc31_enable_crtc(struct timing_generator 
>> *optc)
>>   }
>>   /* disable_crtc - call ASIC Control Object to disable Timing 
>> generator. */
>> -static bool optc31_disable_crtc(struct timing_generator *optc)
>> +bool optc31_disable_crtc(struct timing_generator *optc)
>>   {
>>       struct optc *optc1 = DCN10TG_FROM_TG(optc);
>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h b/ 
>> drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
>> index af67eeaf8505..db7e51fc787e 100644
>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
>> @@ -267,6 +267,8 @@ void dcn31_timing_generator_init(struct optc *optc1);
>>   bool optc31_immediate_disable_crtc(struct timing_generator *optc);
>> +bool optc31_disable_crtc(struct timing_generator *optc);
>> +
>>   bool optc31_enable_crtc(struct timing_generator *optc);
>>   void optc31_set_drr(struct timing_generator *optc, const struct 
>> drr_params *params);
>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c 
>> b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
>> index 6eba48de58ff..f472d2efe026 100644
>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
>> @@ -170,38 +170,6 @@ void optc401_set_h_timing_div_manual_mode(struct 
>> timing_generator *optc, bool ma
>>               OTG_H_TIMING_DIV_MODE_MANUAL, manual_mode ? 1 : 0);
>>   }
>> -/* disable_crtc */
>> -bool optc401_disable_crtc(struct timing_generator *optc)
>> -{
>> -    struct optc *optc1 = DCN10TG_FROM_TG(optc);
>> -
>> -    REG_UPDATE_5(OPTC_DATA_SOURCE_SELECT,
>> -            OPTC_SEG0_SRC_SEL, 0xf,
>> -            OPTC_SEG1_SRC_SEL, 0xf,
>> -            OPTC_SEG2_SRC_SEL, 0xf,
>> -            OPTC_SEG3_SRC_SEL, 0xf,
>> -            OPTC_NUM_OF_INPUT_SEGMENT, 0);
>> -
>> -    REG_UPDATE(OPTC_MEMORY_CONFIG,
>> -            OPTC_MEM_SEL, 0);
>> -
>> -    /* disable otg request until end of the first line
>> -     * in the vertical blank region
>> -     */
>> -    REG_UPDATE(OTG_CONTROL,
>> -            OTG_MASTER_EN, 0);
>> -
>> -    REG_UPDATE(CONTROL,
>> -            VTG0_ENABLE, 0);
>> -
>> -    /* CRTC disabled, so disable  clock. */
>> -    REG_WAIT(OTG_CLOCK_CONTROL,
>> -            OTG_BUSY, 0,
>> -            1, 150000);
>> -
>> -    return true;
>> -}
>> -
>>   void optc401_phantom_crtc_post_enable(struct timing_generator *optc)
>>   {
>>       struct optc *optc1 = DCN10TG_FROM_TG(optc);
>> @@ -435,7 +403,7 @@ static struct timing_generator_funcs 
>> dcn401_tg_funcs = {
>>           .setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
>>           .program_global_sync = optc401_program_global_sync,
>>           .enable_crtc = optc31_enable_crtc,
>> -        .disable_crtc = optc401_disable_crtc,
>> +        .disable_crtc = optc31_disable_crtc,
>>           .phantom_crtc_post_enable = optc401_phantom_crtc_post_enable,
>>           .disable_phantom_crtc = optc401_disable_phantom_otg,
>>           /* used by enable_timing_synchronization. Not need for FPGA */
>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h 
>> b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
>> index 8e795e06e615..be74fd709d43 100644
>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
>> @@ -180,7 +180,6 @@ void optc401_program_global_sync(
>>           int vupdate_offset,
>>           int vupdate_width,
>>           int pstate_keepout);
>> -bool optc401_disable_crtc(struct timing_generator *optc);
>>   void optc401_phantom_crtc_post_enable(struct timing_generator *optc);
>>   void optc401_disable_phantom_otg(struct timing_generator *optc);
>>   void optc401_set_odm_bypass(struct timing_generator *optc,
> 

Re: [PATCH 2/4] drm/amd/display: Use optc31_disable_crtc for DCN 31 and 401

[Alex Hung]

On 5/5/25 10:32, Mario Limonciello wrote:
> On 5/5/2025 11:27 AM, Alex Hung wrote:
>> On 5/4/25 16:11, Rodrigo Siqueira wrote:
>>> DCN401 and DCN31 share the same implementation for disabling CRTC. This
>>> commit makes DCN401 use the DCN31 implementation and removes the code
>>> duplication in the DCN401.
>>
>> Hi Rodrigo
>>
>> optc31_disable_crtc is not the same as optc401_disable_crtc. Please 
>> see the dfiff below:
>>
>> < /* disable_crtc - call ASIC Control Object to disable Timing 
>> generator. */
>> < static bool optc31_disable_crtc(struct timing_generator *optc)
>> ---
>>  > /* disable_crtc */
>>  > bool optc401_disable_crtc(struct timing_generator *optc)
>> 147,148c232
>> <                       1, 100000);
>> <       optc1_clear_optc_underflow(optc);
>> ---
>>  >                       1, 150000);
>> 152,155c236,237
>>
>>
>> However, optc31_disable_crtc is the same as optc35_disable_crtc (patch 
>> 3?) and optc32_disable_crtc is the same as optc401_disable_crtc.
> 
> Is that last argument a timeout?  How about just extending the timeout 
> to be the same for all of them?  That should be relatively harmless, no?
> 

Hi Mario,

"fa28030a83a6 drm/amd/display: increase hardware status wait time" 
changed timeout from 100000 to 150000 and 401 is based on 32.

Do you mean we change all of timeout to 150000?

Hi Aurabindo,

do you have any comments?


>>
>>>
>>> Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
>>> ---
>>>   .../amd/display/dc/optc/dcn31/dcn31_optc.c    |  2 +-
>>>   .../amd/display/dc/optc/dcn31/dcn31_optc.h    |  2 ++
>>>   .../amd/display/dc/optc/dcn401/dcn401_optc.c  | 34 +------------------
>>>   .../amd/display/dc/optc/dcn401/dcn401_optc.h  |  1 -
>>>   4 files changed, 4 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c 
>>> b/ drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
>>> index 13c1f95b5ced..e6246e5ba86f 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
>>> @@ -125,7 +125,7 @@ bool optc31_enable_crtc(struct timing_generator 
>>> *optc)
>>>   }
>>>   /* disable_crtc - call ASIC Control Object to disable Timing 
>>> generator. */
>>> -static bool optc31_disable_crtc(struct timing_generator *optc)
>>> +bool optc31_disable_crtc(struct timing_generator *optc)
>>>   {
>>>       struct optc *optc1 = DCN10TG_FROM_TG(optc);
>>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h 
>>> b/ drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
>>> index af67eeaf8505..db7e51fc787e 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
>>> @@ -267,6 +267,8 @@ void dcn31_timing_generator_init(struct optc 
>>> *optc1);
>>>   bool optc31_immediate_disable_crtc(struct timing_generator *optc);
>>> +bool optc31_disable_crtc(struct timing_generator *optc);
>>> +
>>>   bool optc31_enable_crtc(struct timing_generator *optc);
>>>   void optc31_set_drr(struct timing_generator *optc, const struct 
>>> drr_params *params);
>>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c 
>>> b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
>>> index 6eba48de58ff..f472d2efe026 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
>>> @@ -170,38 +170,6 @@ void optc401_set_h_timing_div_manual_mode(struct 
>>> timing_generator *optc, bool ma
>>>               OTG_H_TIMING_DIV_MODE_MANUAL, manual_mode ? 1 : 0);
>>>   }
>>> -/* disable_crtc */
>>> -bool optc401_disable_crtc(struct timing_generator *optc)
>>> -{
>>> -    struct optc *optc1 = DCN10TG_FROM_TG(optc);
>>> -
>>> -    REG_UPDATE_5(OPTC_DATA_SOURCE_SELECT,
>>> -            OPTC_SEG0_SRC_SEL, 0xf,
>>> -            OPTC_SEG1_SRC_SEL, 0xf,
>>> -            OPTC_SEG2_SRC_SEL, 0xf,
>>> -            OPTC_SEG3_SRC_SEL, 0xf,
>>> -            OPTC_NUM_OF_INPUT_SEGMENT, 0);
>>> -
>>> -    REG_UPDATE(OPTC_MEMORY_CONFIG,
>>> -            OPTC_MEM_SEL, 0);
>>> -
>>> -    /* disable otg request until end of the first line
>>> -     * in the vertical blank region
>>> -     */
>>> -    REG_UPDATE(OTG_CONTROL,
>>> -            OTG_MASTER_EN, 0);
>>> -
>>> -    REG_UPDATE(CONTROL,
>>> -            VTG0_ENABLE, 0);
>>> -
>>> -    /* CRTC disabled, so disable  clock. */
>>> -    REG_WAIT(OTG_CLOCK_CONTROL,
>>> -            OTG_BUSY, 0,
>>> -            1, 150000);
>>> -
>>> -    return true;
>>> -}
>>> -
>>>   void optc401_phantom_crtc_post_enable(struct timing_generator *optc)
>>>   {
>>>       struct optc *optc1 = DCN10TG_FROM_TG(optc);
>>> @@ -435,7 +403,7 @@ static struct timing_generator_funcs 
>>> dcn401_tg_funcs = {
>>>           .setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
>>>           .program_global_sync = optc401_program_global_sync,
>>>           .enable_crtc = optc31_enable_crtc,
>>> -        .disable_crtc = optc401_disable_crtc,
>>> +        .disable_crtc = optc31_disable_crtc,
>>>           .phantom_crtc_post_enable = optc401_phantom_crtc_post_enable,
>>>           .disable_phantom_crtc = optc401_disable_phantom_otg,
>>>           /* used by enable_timing_synchronization. Not need for FPGA */
>>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h 
>>> b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
>>> index 8e795e06e615..be74fd709d43 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
>>> @@ -180,7 +180,6 @@ void optc401_program_global_sync(
>>>           int vupdate_offset,
>>>           int vupdate_width,
>>>           int pstate_keepout);
>>> -bool optc401_disable_crtc(struct timing_generator *optc);
>>>   void optc401_phantom_crtc_post_enable(struct timing_generator *optc);
>>>   void optc401_disable_phantom_otg(struct timing_generator *optc);
>>>   void optc401_set_odm_bypass(struct timing_generator *optc,
>>
> 

Re: [PATCH 2/4] drm/amd/display: Use optc31_disable_crtc for DCN 31 and 401

[Aurabindo Pillai]

On 2025-05-05 13:37, Alex Hung wrote:
> 
> 
> On 5/5/25 10:32, Mario Limonciello wrote:
>> On 5/5/2025 11:27 AM, Alex Hung wrote:
>>> On 5/4/25 16:11, Rodrigo Siqueira wrote:
>>>> DCN401 and DCN31 share the same implementation for disabling CRTC. This
>>>> commit makes DCN401 use the DCN31 implementation and removes the code
>>>> duplication in the DCN401.
>>>
>>> Hi Rodrigo
>>>
>>> optc31_disable_crtc is not the same as optc401_disable_crtc. Please 
>>> see the dfiff below:
>>>
>>> < /* disable_crtc - call ASIC Control Object to disable Timing 
>>> generator. */
>>> < static bool optc31_disable_crtc(struct timing_generator *optc)
>>> ---
>>>  > /* disable_crtc */
>>>  > bool optc401_disable_crtc(struct timing_generator *optc)
>>> 147,148c232
>>> <                       1, 100000);
>>> <       optc1_clear_optc_underflow(optc);
>>> ---
>>>  >                       1, 150000);
>>> 152,155c236,237
>>>
>>>
>>> However, optc31_disable_crtc is the same as optc35_disable_crtc 
>>> (patch 3?) and optc32_disable_crtc is the same as optc401_disable_crtc.
>>
>> Is that last argument a timeout?  How about just extending the timeout 
>> to be the same for all of them?  That should be relatively harmless, no?
>>
> 
> Hi Mario,
> 
> "fa28030a83a6 drm/amd/display: increase hardware status wait time" 
> changed timeout from 100000 to 150000 and 401 is based on 32.
> 
> Do you mean we change all of timeout to 150000?
> 
> Hi Aurabindo,
> 
> do you have any comments?

No concerns. Making the timeout value consistent should be fine.

> 
> 
>>>
>>>>
>>>> Signed-off-by: Rodrigo Siqueira <siqueira@igalia.com>
>>>> ---
>>>>   .../amd/display/dc/optc/dcn31/dcn31_optc.c    |  2 +-
>>>>   .../amd/display/dc/optc/dcn31/dcn31_optc.h    |  2 ++
>>>>   .../amd/display/dc/optc/dcn401/dcn401_optc.c  | 34 
>>>> +------------------
>>>>   .../amd/display/dc/optc/dcn401/dcn401_optc.h  |  1 -
>>>>   4 files changed, 4 insertions(+), 35 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c 
>>>> b/ drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
>>>> index 13c1f95b5ced..e6246e5ba86f 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
>>>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.c
>>>> @@ -125,7 +125,7 @@ bool optc31_enable_crtc(struct timing_generator 
>>>> *optc)
>>>>   }
>>>>   /* disable_crtc - call ASIC Control Object to disable Timing 
>>>> generator. */
>>>> -static bool optc31_disable_crtc(struct timing_generator *optc)
>>>> +bool optc31_disable_crtc(struct timing_generator *optc)
>>>>   {
>>>>       struct optc *optc1 = DCN10TG_FROM_TG(optc);
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h 
>>>> b/ drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
>>>> index af67eeaf8505..db7e51fc787e 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
>>>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn31/dcn31_optc.h
>>>> @@ -267,6 +267,8 @@ void dcn31_timing_generator_init(struct optc 
>>>> *optc1);
>>>>   bool optc31_immediate_disable_crtc(struct timing_generator *optc);
>>>> +bool optc31_disable_crtc(struct timing_generator *optc);
>>>> +
>>>>   bool optc31_enable_crtc(struct timing_generator *optc);
>>>>   void optc31_set_drr(struct timing_generator *optc, const struct 
>>>> drr_params *params);
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/ 
>>>> dcn401_optc.c b/drivers/gpu/drm/amd/display/dc/optc/dcn401/ 
>>>> dcn401_optc.c
>>>> index 6eba48de58ff..f472d2efe026 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
>>>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.c
>>>> @@ -170,38 +170,6 @@ void 
>>>> optc401_set_h_timing_div_manual_mode(struct timing_generator *optc, 
>>>> bool ma
>>>>               OTG_H_TIMING_DIV_MODE_MANUAL, manual_mode ? 1 : 0);
>>>>   }
>>>> -/* disable_crtc */
>>>> -bool optc401_disable_crtc(struct timing_generator *optc)
>>>> -{
>>>> -    struct optc *optc1 = DCN10TG_FROM_TG(optc);
>>>> -
>>>> -    REG_UPDATE_5(OPTC_DATA_SOURCE_SELECT,
>>>> -            OPTC_SEG0_SRC_SEL, 0xf,
>>>> -            OPTC_SEG1_SRC_SEL, 0xf,
>>>> -            OPTC_SEG2_SRC_SEL, 0xf,
>>>> -            OPTC_SEG3_SRC_SEL, 0xf,
>>>> -            OPTC_NUM_OF_INPUT_SEGMENT, 0);
>>>> -
>>>> -    REG_UPDATE(OPTC_MEMORY_CONFIG,
>>>> -            OPTC_MEM_SEL, 0);
>>>> -
>>>> -    /* disable otg request until end of the first line
>>>> -     * in the vertical blank region
>>>> -     */
>>>> -    REG_UPDATE(OTG_CONTROL,
>>>> -            OTG_MASTER_EN, 0);
>>>> -
>>>> -    REG_UPDATE(CONTROL,
>>>> -            VTG0_ENABLE, 0);
>>>> -
>>>> -    /* CRTC disabled, so disable  clock. */
>>>> -    REG_WAIT(OTG_CLOCK_CONTROL,
>>>> -            OTG_BUSY, 0,
>>>> -            1, 150000);
>>>> -
>>>> -    return true;
>>>> -}
>>>> -
>>>>   void optc401_phantom_crtc_post_enable(struct timing_generator *optc)
>>>>   {
>>>>       struct optc *optc1 = DCN10TG_FROM_TG(optc);
>>>> @@ -435,7 +403,7 @@ static struct timing_generator_funcs 
>>>> dcn401_tg_funcs = {
>>>>           .setup_vertical_interrupt2 = optc1_setup_vertical_interrupt2,
>>>>           .program_global_sync = optc401_program_global_sync,
>>>>           .enable_crtc = optc31_enable_crtc,
>>>> -        .disable_crtc = optc401_disable_crtc,
>>>> +        .disable_crtc = optc31_disable_crtc,
>>>>           .phantom_crtc_post_enable = optc401_phantom_crtc_post_enable,
>>>>           .disable_phantom_crtc = optc401_disable_phantom_otg,
>>>>           /* used by enable_timing_synchronization. Not need for 
>>>> FPGA */
>>>> diff --git a/drivers/gpu/drm/amd/display/dc/optc/dcn401/ 
>>>> dcn401_optc.h b/drivers/gpu/drm/amd/display/dc/optc/dcn401/ 
>>>> dcn401_optc.h
>>>> index 8e795e06e615..be74fd709d43 100644
>>>> --- a/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
>>>> +++ b/drivers/gpu/drm/amd/display/dc/optc/dcn401/dcn401_optc.h
>>>> @@ -180,7 +180,6 @@ void optc401_program_global_sync(
>>>>           int vupdate_offset,
>>>>           int vupdate_width,
>>>>           int pstate_keepout);
>>>> -bool optc401_disable_crtc(struct timing_generator *optc);
>>>>   void optc401_phantom_crtc_post_enable(struct timing_generator *optc);
>>>>   void optc401_disable_phantom_otg(struct timing_generator *optc);
>>>>   void optc401_set_odm_bypass(struct timing_generator *optc,
>>>
>>
> 

-- 
Thanks & Regards,
Aurabindo Pillai