On Thu, Oct 10, 2024 at 08:33:11AM +0300, Jouni Högander wrote:
> Currently vrr code is overwriting possibly set PSR PR Frame Change Enable
> bit in TRANS_PUSH register. Avoid this by using rmw instead of write.

RMWs are not good if we ever want to do this from DSB/etc. We should
know what the state should be and just program that in.

Does the PSR bit here have to match the PSR_CTL enable bit or
could we just always set the bit here based on has_psr whether
PSR is currently active or not?

> 
> Signed-off-by: Jouni Högander <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c 
> b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 9a51f5bac3071..8b4e3f938efea 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -339,8 +339,8 @@ void intel_vrr_enable(const struct intel_crtc_state 
> *crtc_state)
>       if (!crtc_state->vrr.enable)
>               return;
>  
> -     intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> -                    TRANS_PUSH_EN);
> +     intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder), 0,
> +                  TRANS_PUSH_EN);
>  
>       if (HAS_AS_SDP(display))
>               intel_de_write(display,
> @@ -371,7 +371,9 @@ void intel_vrr_disable(const struct intel_crtc_state 
> *old_crtc_state)
>       intel_de_wait_for_clear(display,
>                               TRANS_VRR_STATUS(display, cpu_transcoder),
>                               VRR_STATUS_VRR_EN_LIVE, 1000);
> -     intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
> +
> +     intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder), 
> TRANS_PUSH_EN,
> +                  0);
>  
>       if (HAS_AS_SDP(display))
>               intel_de_write(display,
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

Reply via email to