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

Mon, 05 May 2025 10:50:51 -0700 



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 <sique...@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























					Reply via email to