diff mbox series

[3/3] drm/amd/display: Compensate for pre-DCE12 BTR-VRR hw limitations. (v3)

Message ID 20190426214016.24667-4-mario.kleiner.de@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/3] drm/amd/display: Fix and simplify apply_below_the_range() | expand

Commit Message

Mario Kleiner April 26, 2019, 9:40 p.m. UTC
Pre-DCE12 needs special treatment for BTR / low framerate
compensation for more stable behaviour:

According to comments in the code and some testing on DCE-8
and DCE-11, DCE-11 and earlier only apply VTOTAL_MIN/MAX
programming with a lag of one frame, so the special BTR hw
programming for intermediate fixed duration frames must be
done inside the current frame at flip submission in atomic
commit tail, ie. one vblank earlier, and the fixed refresh
intermediate frame mode must be also terminated one vblank
earlier on pre-DCE12 display engines.

To achieve proper termination on < DCE-12 shift the point
when the switch-back from fixed vblank duration to variable
vblank duration happens from the start of VBLANK (vblank irq,
as done on DCE-12+) to back-porch or end of VBLANK (handled
by vupdate irq handler). We must leave the switch-back code
inside VBLANK irq for DCE12+, as before.

Doing this, we get much better behaviour of BTR for up-sweeps,
ie. going from short to long frame durations (~high to low fps)
and for constant framerate flips, as tested on DCE-8 and
DCE-11. Behaviour is still not quite as good as on DCN-1
though.

On down-sweeps, going from long to short frame durations
(low fps to high fps) < DCE-12 is a little bit improved,
although by far not as much as for up-sweeps and constant
fps.

v2: Fix some wrong locking, as pointed out by Nicholas.
v3: Simplify if-condition in vupdate-irq - nit by Nicholas.
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 +++++++++++++++++--
 1 file changed, 44 insertions(+), 4 deletions(-)

Comments

Kazlauskas, Nicholas April 29, 2019, 12:51 p.m. UTC | #1
On 4/26/19 5:40 PM, Mario Kleiner wrote:
> Pre-DCE12 needs special treatment for BTR / low framerate
> compensation for more stable behaviour:
> 
> According to comments in the code and some testing on DCE-8
> and DCE-11, DCE-11 and earlier only apply VTOTAL_MIN/MAX
> programming with a lag of one frame, so the special BTR hw
> programming for intermediate fixed duration frames must be
> done inside the current frame at flip submission in atomic
> commit tail, ie. one vblank earlier, and the fixed refresh
> intermediate frame mode must be also terminated one vblank
> earlier on pre-DCE12 display engines.
> 
> To achieve proper termination on < DCE-12 shift the point
> when the switch-back from fixed vblank duration to variable
> vblank duration happens from the start of VBLANK (vblank irq,
> as done on DCE-12+) to back-porch or end of VBLANK (handled
> by vupdate irq handler). We must leave the switch-back code
> inside VBLANK irq for DCE12+, as before.
> 
> Doing this, we get much better behaviour of BTR for up-sweeps,
> ie. going from short to long frame durations (~high to low fps)
> and for constant framerate flips, as tested on DCE-8 and
> DCE-11. Behaviour is still not quite as good as on DCN-1
> though.
> 
> On down-sweeps, going from long to short frame durations
> (low fps to high fps) < DCE-12 is a little bit improved,
> although by far not as much as for up-sweeps and constant
> fps.
> 
> v2: Fix some wrong locking, as pointed out by Nicholas.
> v3: Simplify if-condition in vupdate-irq - nit by Nicholas.
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

I'd like to push patches 1+3 if you're okay with this.

I can see the value in the 2nd patch from the graphs and test 
application you've posted but it'll likely take some time to do thorough 
review and testing on this.

The question that needs to be examined with the second is what the 
optimal margin is, if any, for typical usecases across common monitor 
ranges. Not all monitors that support BTR have the same range, and many 
have a lower bound of ~48Hz. To me ~53Hz sounds rather early to be 
entering, but I'm not sure how it affects the user experience overall.

Nicholas Kazlauskas

> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 +++++++++++++++++--
>   1 file changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 76b6e621793f..92b3c2cec7dd 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -364,6 +364,7 @@ static void dm_vupdate_high_irq(void *interrupt_params)
>   	struct amdgpu_device *adev = irq_params->adev;
>   	struct amdgpu_crtc *acrtc;
>   	struct dm_crtc_state *acrtc_state;
> +	unsigned long flags;
>   
>   	acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VUPDATE);
>   
> @@ -379,8 +380,25 @@ static void dm_vupdate_high_irq(void *interrupt_params)
>   		 * page-flip completion events that have been queued to us
>   		 * if a pageflip happened inside front-porch.
>   		 */
> -		if (amdgpu_dm_vrr_active(acrtc_state))
> +		if (amdgpu_dm_vrr_active(acrtc_state)) {
>   			drm_crtc_handle_vblank(&acrtc->base);
> +
> +			/* BTR processing for pre-DCE12 ASICs */
> +			if (acrtc_state->stream &&
> +			    adev->family < AMDGPU_FAMILY_AI) {
> +				spin_lock_irqsave(&adev->ddev->event_lock, flags);
> +				mod_freesync_handle_v_update(
> +				    adev->dm.freesync_module,
> +				    acrtc_state->stream,
> +				    &acrtc_state->vrr_params);
> +
> +				dc_stream_adjust_vmin_vmax(
> +				    adev->dm.dc,
> +				    acrtc_state->stream,
> +				    &acrtc_state->vrr_params.adjust);
> +				spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
> +			}
> +		}
>   	}
>   }
>   
> @@ -390,6 +408,7 @@ static void dm_crtc_high_irq(void *interrupt_params)
>   	struct amdgpu_device *adev = irq_params->adev;
>   	struct amdgpu_crtc *acrtc;
>   	struct dm_crtc_state *acrtc_state;
> +	unsigned long flags;
>   
>   	acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VBLANK);
>   
> @@ -412,9 +431,10 @@ static void dm_crtc_high_irq(void *interrupt_params)
>   		 */
>   		amdgpu_dm_crtc_handle_crc_irq(&acrtc->base);
>   
> -		if (acrtc_state->stream &&
> +		if (acrtc_state->stream && adev->family >= AMDGPU_FAMILY_AI &&
>   		    acrtc_state->vrr_params.supported &&
>   		    acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE) {
> +			spin_lock_irqsave(&adev->ddev->event_lock, flags);
>   			mod_freesync_handle_v_update(
>   				adev->dm.freesync_module,
>   				acrtc_state->stream,
> @@ -424,6 +444,7 @@ static void dm_crtc_high_irq(void *interrupt_params)
>   				adev->dm.dc,
>   				acrtc_state->stream,
>   				&acrtc_state->vrr_params.adjust);
> +			spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
>   		}
>   	}
>   }
> @@ -4878,8 +4899,10 @@ static void update_freesync_state_on_stream(
>   	struct dc_plane_state *surface,
>   	u32 flip_timestamp_in_us)
>   {
> -	struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
> +	struct mod_vrr_params vrr_params;
>   	struct dc_info_packet vrr_infopacket = {0};
> +	struct amdgpu_device *adev = dm->adev;
> +	unsigned long flags;
>   
>   	if (!new_stream)
>   		return;
> @@ -4892,6 +4915,9 @@ static void update_freesync_state_on_stream(
>   	if (!new_stream->timing.h_total || !new_stream->timing.v_total)
>   		return;
>   
> +	spin_lock_irqsave(&adev->ddev->event_lock, flags);
> +	vrr_params = new_crtc_state->vrr_params;
> +
>   	if (surface) {
>   		mod_freesync_handle_preflip(
>   			dm->freesync_module,
> @@ -4899,6 +4925,12 @@ static void update_freesync_state_on_stream(
>   			new_stream,
>   			flip_timestamp_in_us,
>   			&vrr_params);
> +
> +		if (adev->family < AMDGPU_FAMILY_AI &&
> +		    amdgpu_dm_vrr_active(new_crtc_state)) {
> +			mod_freesync_handle_v_update(dm->freesync_module,
> +						     new_stream, &vrr_params);
> +		}
>   	}
>   
>   	mod_freesync_build_vrr_infopacket(
> @@ -4930,6 +4962,8 @@ static void update_freesync_state_on_stream(
>   			      new_crtc_state->base.crtc->base.id,
>   			      (int)new_crtc_state->base.vrr_enabled,
>   			      (int)vrr_params.state);
> +
> +	spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
>   }
>   
>   static void pre_update_freesync_state_on_stream(
> @@ -4937,8 +4971,10 @@ static void pre_update_freesync_state_on_stream(
>   	struct dm_crtc_state *new_crtc_state)
>   {
>   	struct dc_stream_state *new_stream = new_crtc_state->stream;
> -	struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
> +	struct mod_vrr_params vrr_params;
>   	struct mod_freesync_config config = new_crtc_state->freesync_config;
> +	struct amdgpu_device *adev = dm->adev;
> +	unsigned long flags;
>   
>   	if (!new_stream)
>   		return;
> @@ -4950,6 +4986,9 @@ static void pre_update_freesync_state_on_stream(
>   	if (!new_stream->timing.h_total || !new_stream->timing.v_total)
>   		return;
>   
> +	spin_lock_irqsave(&adev->ddev->event_lock, flags);
> +	vrr_params = new_crtc_state->vrr_params;
> +
>   	if (new_crtc_state->vrr_supported &&
>   	    config.min_refresh_in_uhz &&
>   	    config.max_refresh_in_uhz) {
> @@ -4970,6 +5009,7 @@ static void pre_update_freesync_state_on_stream(
>   			sizeof(vrr_params.adjust)) != 0);
>   
>   	new_crtc_state->vrr_params = vrr_params;
> +	spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
>   }
>   
>   static void amdgpu_dm_handle_vrr_transition(struct dm_crtc_state *old_state,
>
Mario Kleiner April 29, 2019, 2:29 p.m. UTC | #2
On Mon, Apr 29, 2019 at 2:51 PM Kazlauskas, Nicholas
<Nicholas.Kazlauskas@amd.com> wrote:
>
> On 4/26/19 5:40 PM, Mario Kleiner wrote:
> > Pre-DCE12 needs special treatment for BTR / low framerate
> > compensation for more stable behaviour:
> >
> > According to comments in the code and some testing on DCE-8
> > and DCE-11, DCE-11 and earlier only apply VTOTAL_MIN/MAX
> > programming with a lag of one frame, so the special BTR hw
> > programming for intermediate fixed duration frames must be
> > done inside the current frame at flip submission in atomic
> > commit tail, ie. one vblank earlier, and the fixed refresh
> > intermediate frame mode must be also terminated one vblank
> > earlier on pre-DCE12 display engines.
> >
> > To achieve proper termination on < DCE-12 shift the point
> > when the switch-back from fixed vblank duration to variable
> > vblank duration happens from the start of VBLANK (vblank irq,
> > as done on DCE-12+) to back-porch or end of VBLANK (handled
> > by vupdate irq handler). We must leave the switch-back code
> > inside VBLANK irq for DCE12+, as before.
> >
> > Doing this, we get much better behaviour of BTR for up-sweeps,
> > ie. going from short to long frame durations (~high to low fps)
> > and for constant framerate flips, as tested on DCE-8 and
> > DCE-11. Behaviour is still not quite as good as on DCN-1
> > though.
> >
> > On down-sweeps, going from long to short frame durations
> > (low fps to high fps) < DCE-12 is a little bit improved,
> > although by far not as much as for up-sweeps and constant
> > fps.
> >
> > v2: Fix some wrong locking, as pointed out by Nicholas.
> > v3: Simplify if-condition in vupdate-irq - nit by Nicholas.
> > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>

Thanks.

> I'd like to push patches 1+3 if you're okay with this.
>

Yes please! I'd love to have these in Linux 5.2, so that would be the
first well suitable kernel to target for my specific VRR use cases.

> I can see the value in the 2nd patch from the graphs and test
> application you've posted but it'll likely take some time to do thorough
> review and testing on this.
>

I understand.

> The question that needs to be examined with the second is what the
> optimal margin is, if any, for typical usecases across common monitor
> ranges. Not all monitors that support BTR have the same range, and many
> have a lower bound of ~48Hz. To me ~53Hz sounds rather early to be
> entering, but I'm not sure how it affects the user experience overall.
>

That's indeed a tricky tuning problem and i didn't spend much thought
on that. Just took the BTR_EXIT_MARGIN already defined as 2 msecs,
because it was conveniently there for the exit path and this way it
was nicely symmetric to the BTR exit path. 1 msec or less might just
be as good, or something more fancy and adaptive. But most value comes
from patch 3/3 anyway.

Atm. i don't have a Freesync capable display at all, so i don't know
how well this patch works out wrt. flicker etc, i can only look at my
plots and the output of the kms_vrr igt test. I just hacked the driver
to accept any DP or HDMI display as VRR capable, so i can run the
synthetic timing tests blindly (A G-Sync display mostly goes blank or
tells me a sad "Out of range signal" on its OSD, a HDMI->VGA connected
CRT monitor cycles between "Out of range", black, and some heavily
distorted image). The G-Sync display has a 30 - 144 Hz range and my
faking patch even set a range 25 - 144 Hz for most tests.

I also thought about sending my "fake VRR display" patch after
cleaning it up. I could add some amdgpu module parameter, e.g., bool
amdgpu.fakevrroutput,
that if set to 1 would allow the driver to accept any DP/HDMI display
as VRR capable, parsing the VRR range from EDID if present (that works
for G-Sync displays) or just faking a range like 30 - 144 Hz or
whatever is realistic? A blanked out display is boring to look at, but
at least it allows to run timing tests or the IGT kms_vrr tests
without the need for a dedicated FreeSync display.

From reading on the web I understand that "FreeSync2 HDR" displays are
also HDR capable and go through some testing and certification at AMD
wrt. to minimal flicker? Would you have some recommendation or
pointers to a display i could buy if i needed a large VRR range,
minimal flicker and also good HDR capabilities - ideally with locally
dimmable backlight or such? I will probably have the money to buy one
FreeSync display, but that should also be useable for working on HDR
related stuff.

Thanks,
-mario

> Nicholas Kazlauskas
>
> > ---
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 +++++++++++++++++--
> >   1 file changed, 44 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 76b6e621793f..92b3c2cec7dd 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -364,6 +364,7 @@ static void dm_vupdate_high_irq(void *interrupt_params)
> >       struct amdgpu_device *adev = irq_params->adev;
> >       struct amdgpu_crtc *acrtc;
> >       struct dm_crtc_state *acrtc_state;
> > +     unsigned long flags;
> >
> >       acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VUPDATE);
> >
> > @@ -379,8 +380,25 @@ static void dm_vupdate_high_irq(void *interrupt_params)
> >                * page-flip completion events that have been queued to us
> >                * if a pageflip happened inside front-porch.
> >                */
> > -             if (amdgpu_dm_vrr_active(acrtc_state))
> > +             if (amdgpu_dm_vrr_active(acrtc_state)) {
> >                       drm_crtc_handle_vblank(&acrtc->base);
> > +
> > +                     /* BTR processing for pre-DCE12 ASICs */
> > +                     if (acrtc_state->stream &&
> > +                         adev->family < AMDGPU_FAMILY_AI) {
> > +                             spin_lock_irqsave(&adev->ddev->event_lock, flags);
> > +                             mod_freesync_handle_v_update(
> > +                                 adev->dm.freesync_module,
> > +                                 acrtc_state->stream,
> > +                                 &acrtc_state->vrr_params);
> > +
> > +                             dc_stream_adjust_vmin_vmax(
> > +                                 adev->dm.dc,
> > +                                 acrtc_state->stream,
> > +                                 &acrtc_state->vrr_params.adjust);
> > +                             spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
> > +                     }
> > +             }
> >       }
> >   }
> >
> > @@ -390,6 +408,7 @@ static void dm_crtc_high_irq(void *interrupt_params)
> >       struct amdgpu_device *adev = irq_params->adev;
> >       struct amdgpu_crtc *acrtc;
> >       struct dm_crtc_state *acrtc_state;
> > +     unsigned long flags;
> >
> >       acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VBLANK);
> >
> > @@ -412,9 +431,10 @@ static void dm_crtc_high_irq(void *interrupt_params)
> >                */
> >               amdgpu_dm_crtc_handle_crc_irq(&acrtc->base);
> >
> > -             if (acrtc_state->stream &&
> > +             if (acrtc_state->stream && adev->family >= AMDGPU_FAMILY_AI &&
> >                   acrtc_state->vrr_params.supported &&
> >                   acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE) {
> > +                     spin_lock_irqsave(&adev->ddev->event_lock, flags);
> >                       mod_freesync_handle_v_update(
> >                               adev->dm.freesync_module,
> >                               acrtc_state->stream,
> > @@ -424,6 +444,7 @@ static void dm_crtc_high_irq(void *interrupt_params)
> >                               adev->dm.dc,
> >                               acrtc_state->stream,
> >                               &acrtc_state->vrr_params.adjust);
> > +                     spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
> >               }
> >       }
> >   }
> > @@ -4878,8 +4899,10 @@ static void update_freesync_state_on_stream(
> >       struct dc_plane_state *surface,
> >       u32 flip_timestamp_in_us)
> >   {
> > -     struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
> > +     struct mod_vrr_params vrr_params;
> >       struct dc_info_packet vrr_infopacket = {0};
> > +     struct amdgpu_device *adev = dm->adev;
> > +     unsigned long flags;
> >
> >       if (!new_stream)
> >               return;
> > @@ -4892,6 +4915,9 @@ static void update_freesync_state_on_stream(
> >       if (!new_stream->timing.h_total || !new_stream->timing.v_total)
> >               return;
> >
> > +     spin_lock_irqsave(&adev->ddev->event_lock, flags);
> > +     vrr_params = new_crtc_state->vrr_params;
> > +
> >       if (surface) {
> >               mod_freesync_handle_preflip(
> >                       dm->freesync_module,
> > @@ -4899,6 +4925,12 @@ static void update_freesync_state_on_stream(
> >                       new_stream,
> >                       flip_timestamp_in_us,
> >                       &vrr_params);
> > +
> > +             if (adev->family < AMDGPU_FAMILY_AI &&
> > +                 amdgpu_dm_vrr_active(new_crtc_state)) {
> > +                     mod_freesync_handle_v_update(dm->freesync_module,
> > +                                                  new_stream, &vrr_params);
> > +             }
> >       }
> >
> >       mod_freesync_build_vrr_infopacket(
> > @@ -4930,6 +4962,8 @@ static void update_freesync_state_on_stream(
> >                             new_crtc_state->base.crtc->base.id,
> >                             (int)new_crtc_state->base.vrr_enabled,
> >                             (int)vrr_params.state);
> > +
> > +     spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
> >   }
> >
> >   static void pre_update_freesync_state_on_stream(
> > @@ -4937,8 +4971,10 @@ static void pre_update_freesync_state_on_stream(
> >       struct dm_crtc_state *new_crtc_state)
> >   {
> >       struct dc_stream_state *new_stream = new_crtc_state->stream;
> > -     struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
> > +     struct mod_vrr_params vrr_params;
> >       struct mod_freesync_config config = new_crtc_state->freesync_config;
> > +     struct amdgpu_device *adev = dm->adev;
> > +     unsigned long flags;
> >
> >       if (!new_stream)
> >               return;
> > @@ -4950,6 +4986,9 @@ static void pre_update_freesync_state_on_stream(
> >       if (!new_stream->timing.h_total || !new_stream->timing.v_total)
> >               return;
> >
> > +     spin_lock_irqsave(&adev->ddev->event_lock, flags);
> > +     vrr_params = new_crtc_state->vrr_params;
> > +
> >       if (new_crtc_state->vrr_supported &&
> >           config.min_refresh_in_uhz &&
> >           config.max_refresh_in_uhz) {
> > @@ -4970,6 +5009,7 @@ static void pre_update_freesync_state_on_stream(
> >                       sizeof(vrr_params.adjust)) != 0);
> >
> >       new_crtc_state->vrr_params = vrr_params;
> > +     spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
> >   }
> >
> >   static void amdgpu_dm_handle_vrr_transition(struct dm_crtc_state *old_state,
> >
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 76b6e621793f..92b3c2cec7dd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -364,6 +364,7 @@  static void dm_vupdate_high_irq(void *interrupt_params)
 	struct amdgpu_device *adev = irq_params->adev;
 	struct amdgpu_crtc *acrtc;
 	struct dm_crtc_state *acrtc_state;
+	unsigned long flags;
 
 	acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VUPDATE);
 
@@ -379,8 +380,25 @@  static void dm_vupdate_high_irq(void *interrupt_params)
 		 * page-flip completion events that have been queued to us
 		 * if a pageflip happened inside front-porch.
 		 */
-		if (amdgpu_dm_vrr_active(acrtc_state))
+		if (amdgpu_dm_vrr_active(acrtc_state)) {
 			drm_crtc_handle_vblank(&acrtc->base);
+
+			/* BTR processing for pre-DCE12 ASICs */
+			if (acrtc_state->stream &&
+			    adev->family < AMDGPU_FAMILY_AI) {
+				spin_lock_irqsave(&adev->ddev->event_lock, flags);
+				mod_freesync_handle_v_update(
+				    adev->dm.freesync_module,
+				    acrtc_state->stream,
+				    &acrtc_state->vrr_params);
+
+				dc_stream_adjust_vmin_vmax(
+				    adev->dm.dc,
+				    acrtc_state->stream,
+				    &acrtc_state->vrr_params.adjust);
+				spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
+			}
+		}
 	}
 }
 
@@ -390,6 +408,7 @@  static void dm_crtc_high_irq(void *interrupt_params)
 	struct amdgpu_device *adev = irq_params->adev;
 	struct amdgpu_crtc *acrtc;
 	struct dm_crtc_state *acrtc_state;
+	unsigned long flags;
 
 	acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_VBLANK);
 
@@ -412,9 +431,10 @@  static void dm_crtc_high_irq(void *interrupt_params)
 		 */
 		amdgpu_dm_crtc_handle_crc_irq(&acrtc->base);
 
-		if (acrtc_state->stream &&
+		if (acrtc_state->stream && adev->family >= AMDGPU_FAMILY_AI &&
 		    acrtc_state->vrr_params.supported &&
 		    acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE) {
+			spin_lock_irqsave(&adev->ddev->event_lock, flags);
 			mod_freesync_handle_v_update(
 				adev->dm.freesync_module,
 				acrtc_state->stream,
@@ -424,6 +444,7 @@  static void dm_crtc_high_irq(void *interrupt_params)
 				adev->dm.dc,
 				acrtc_state->stream,
 				&acrtc_state->vrr_params.adjust);
+			spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
 		}
 	}
 }
@@ -4878,8 +4899,10 @@  static void update_freesync_state_on_stream(
 	struct dc_plane_state *surface,
 	u32 flip_timestamp_in_us)
 {
-	struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
+	struct mod_vrr_params vrr_params;
 	struct dc_info_packet vrr_infopacket = {0};
+	struct amdgpu_device *adev = dm->adev;
+	unsigned long flags;
 
 	if (!new_stream)
 		return;
@@ -4892,6 +4915,9 @@  static void update_freesync_state_on_stream(
 	if (!new_stream->timing.h_total || !new_stream->timing.v_total)
 		return;
 
+	spin_lock_irqsave(&adev->ddev->event_lock, flags);
+	vrr_params = new_crtc_state->vrr_params;
+
 	if (surface) {
 		mod_freesync_handle_preflip(
 			dm->freesync_module,
@@ -4899,6 +4925,12 @@  static void update_freesync_state_on_stream(
 			new_stream,
 			flip_timestamp_in_us,
 			&vrr_params);
+
+		if (adev->family < AMDGPU_FAMILY_AI &&
+		    amdgpu_dm_vrr_active(new_crtc_state)) {
+			mod_freesync_handle_v_update(dm->freesync_module,
+						     new_stream, &vrr_params);
+		}
 	}
 
 	mod_freesync_build_vrr_infopacket(
@@ -4930,6 +4962,8 @@  static void update_freesync_state_on_stream(
 			      new_crtc_state->base.crtc->base.id,
 			      (int)new_crtc_state->base.vrr_enabled,
 			      (int)vrr_params.state);
+
+	spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
 }
 
 static void pre_update_freesync_state_on_stream(
@@ -4937,8 +4971,10 @@  static void pre_update_freesync_state_on_stream(
 	struct dm_crtc_state *new_crtc_state)
 {
 	struct dc_stream_state *new_stream = new_crtc_state->stream;
-	struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
+	struct mod_vrr_params vrr_params;
 	struct mod_freesync_config config = new_crtc_state->freesync_config;
+	struct amdgpu_device *adev = dm->adev;
+	unsigned long flags;
 
 	if (!new_stream)
 		return;
@@ -4950,6 +4986,9 @@  static void pre_update_freesync_state_on_stream(
 	if (!new_stream->timing.h_total || !new_stream->timing.v_total)
 		return;
 
+	spin_lock_irqsave(&adev->ddev->event_lock, flags);
+	vrr_params = new_crtc_state->vrr_params;
+
 	if (new_crtc_state->vrr_supported &&
 	    config.min_refresh_in_uhz &&
 	    config.max_refresh_in_uhz) {
@@ -4970,6 +5009,7 @@  static void pre_update_freesync_state_on_stream(
 			sizeof(vrr_params.adjust)) != 0);
 
 	new_crtc_state->vrr_params = vrr_params;
+	spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
 }
 
 static void amdgpu_dm_handle_vrr_transition(struct dm_crtc_state *old_state,