[PATCH 2/4] drm/amd/display: Use optc31_disable_crtc for DCN 31 and 401
Aurabindo Pillai
aurabindo.pillai at amd.com
Mon May 5 17:50:38 UTC 2025
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 at 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
More information about the amd-gfx
mailing list