Re: [Intel-gfx] [PATCH 09/12] drm/i915: Disable VRR during seamless M/N changes

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Sep 07, 2023 at 11:49:10AM -0700, Manasi Navare wrote:
> Hi Ville,
> 
> Since we are always disabling when update_m_n, that means if in gaming
> mode if VRR enable is requested by userspace, it cannot
> be enabled if update_m_n or dual refresh mode is enabled and say we
> have downclocked from 120Hz - 60Hz?

No, it just means if you have VRR already enabled and want to do
a M/N change VRR gets temporarily disabled and re-enabled during
the commit.

> 
> Doesnt this contradict the purpose of this series to try and do VRR
> update params in fastset because we want VRR range to be
> correctly reflected when in dual refresh mode when we downclock from
> 120-60hz in gaming use case with VRR?
> 
> Am I missing something here?
> 
> Regards
> Manasi
> 
> On Fri, Sep 1, 2023 at 6:05 AM Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make life less confusing by making sure VRR is disabled whenever
> > we do any drastic changes to the display timings, such as seamless
> > M/N changes.
> >
> > Cc: Manasi Navare <navaremanasi@chromium.org>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index c20eaf0e7a91..cbbee303cd00 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -916,13 +916,15 @@ static bool planes_disabling(const struct intel_crtc_state *old_crtc_state,
> >  static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
> >                          const struct intel_crtc_state *new_crtc_state)
> >  {
> > -       return is_enabling(vrr.enable, old_crtc_state, new_crtc_state);
> > +       return is_enabling(vrr.enable, old_crtc_state, new_crtc_state) ||
> > +               (new_crtc_state->vrr.enable && new_crtc_state->update_m_n);
> >  }
> >
> >  static bool vrr_disabling(const struct intel_crtc_state *old_crtc_state,
> >                           const struct intel_crtc_state *new_crtc_state)
> >  {
> > -       return is_disabling(vrr.enable, old_crtc_state, new_crtc_state);
> > +       return is_disabling(vrr.enable, old_crtc_state, new_crtc_state) ||
> > +               (old_crtc_state->vrr.enable && new_crtc_state->update_m_n);
> >  }
> >
> >  #undef is_disabling
> > --
> > 2.41.0
> >

-- 
Ville Syrjälä
Intel