[PATCH] drm/amd/display: Program OTG vtotal min/max selectors unconditionally for DCN1+

[Rodrigo Siqueira Jordao]

Hi Jay,

On 6/1/23 11:09, Aurabindo Pillai wrote:
> Due to FPO, firmware will try to change OTG timings, which would only
> latch if min/max selectors for vtotal are written by the driver.

Could you elaborate a little bit more about this issue? Right now, do we 
have some sort of race between firmware and driver? Is this situation 
causing some problems that we can reproduce? If so, could you also 
describe it?

> Signed-off-by: Aurabindo Pillai <aurabindo.pillai at amd.com>
> ---
>   drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c | 15 +++------------
>   drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c |  6 ++++++
>   2 files changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c
> index e1975991e075..633989fd2514 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c
> @@ -930,19 +930,10 @@ void optc1_set_drr(
>   				OTG_FORCE_LOCK_ON_EVENT, 0,
>   				OTG_SET_V_TOTAL_MIN_MASK_EN, 0,
>   				OTG_SET_V_TOTAL_MIN_MASK, 0);
> -
> -		// Setup manual flow control for EOF via TRIG_A
> -		optc->funcs->setup_manual_trigger(optc);
> -
> -	} else {
> -		REG_UPDATE_4(OTG_V_TOTAL_CONTROL,
> -				OTG_SET_V_TOTAL_MIN_MASK, 0,
> -				OTG_V_TOTAL_MIN_SEL, 0,
> -				OTG_V_TOTAL_MAX_SEL, 0,
> -				OTG_FORCE_LOCK_ON_EVENT, 0);
> -
> -		optc->funcs->set_vtotal_min_max(optc, 0, 0);
>   	}
> +
> +	// Setup manual flow control for EOF via TRIG_A
> +	optc->funcs->setup_manual_trigger(optc);
>   }
>   
>   void optc1_set_vtotal_min_max(struct timing_generator *optc, int vtotal_min, int vtotal_max)
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c
> index e0edc163d767..042ce082965f 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c
> @@ -455,6 +455,12 @@ void optc2_setup_manual_trigger(struct timing_generator *optc)
>   {
>   	struct optc *optc1 = DCN10TG_FROM_TG(optc);
>   
> +	REG_UPDATE_4(OTG_V_TOTAL_CONTROL,

Could you align the below registers right after the open parenthesis?

> +                                OTG_V_TOTAL_MIN_SEL, 1,
> +                                OTG_V_TOTAL_MAX_SEL, 1,
> +                                OTG_FORCE_LOCK_ON_EVENT, 0,

Could you add a comment that describes why we want to set the above values?

> +                                OTG_SET_V_TOTAL_MIN_MASK, (1 << 1)); /* TRIGA */

Why do you use (1 << 1)? Why not set the value directly here? Also, in 
the comment, I guess it should be TRIG_A.

> +
>   	REG_SET_8(OTG_TRIGA_CNTL, 0,
>   			OTG_TRIGA_SOURCE_SELECT, 21,
>   			OTG_TRIGA_SOURCE_PIPE_SELECT, optc->inst,