RE: [PATCH v2 06/11] drm/i915/display: Move CMRR crtc_state members under VRR

Sun, 28 Jun 2026 22:24:03 -0700 


> -----Original Message-----
> From: Jani Nikula <[email protected]>
> Sent: 17 June 2026 17:32
> To: Golani, Mitulkumar Ajitkumar <[email protected]>;
> [email protected]
> Cc: [email protected]; Nautiyal, Ankit K
> <[email protected]>; Borah, Chaitanya Kumar
> <[email protected]>
> Subject: Re: [PATCH v2 06/11] drm/i915/display: Move CMRR crtc_state
> members under VRR
> 
> On Tue, 16 Jun 2026, Mitul Golani <[email protected]>
> wrote:
> > Move CMRR crtc state members under VRR infrastructure as it is enabled
> > during fix refresh rate  VRR timing generator is enabled.
> >
> > Signed-off-by: Mitul Golani <[email protected]>
> > ---
> >  .../drm/i915/display/intel_crtc_state_dump.c  |  4 ++--
> > drivers/gpu/drm/i915/display/intel_display.c  |  4 ++--
> >  .../drm/i915/display/intel_display_types.h    |  5 ++++
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  2 +-
> >  drivers/gpu/drm/i915/display/intel_vrr.c      | 24 +++++++++----------
> >  5 files changed, 22 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > index 3a530be64e40..ea337efbe524 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> > @@ -312,9 +312,9 @@ void intel_crtc_state_dump(const struct
> intel_crtc_state *pipe_config,
> >                pipe_config->vrr.dc_balance.max_decrease,
> >                pipe_config->vrr.dc_balance.vblank_target);
> >     drm_printf(&p, "cmrr: %s, video mode: %s, cmrr_m: %llu, cmrr_n:
> %llu\n",
> > -              str_yes_no(pipe_config->cmrr.enable),
> > +              str_yes_no(pipe_config->vrr.cmrr.enable),
> >                str_yes_no(crtc->cmrr.video_mode),
> > -              pipe_config->cmrr.cmrr_m, pipe_config->cmrr.cmrr_n);
> > +              pipe_config->vrr.cmrr.cmrr_m, pipe_config-
> >vrr.cmrr.cmrr_n);
> >
> >     drm_printf(&p, "requested mode: " DRM_MODE_FMT "\n",
> >                DRM_MODE_ARG(&pipe_config->hw.mode));
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index e76aa6c8dab6..e067d484858f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -949,8 +949,8 @@ static bool vrr_params_changed(const struct
> > intel_crtc_state *old_crtc_state,  static bool cmrr_params_changed(const
> struct intel_crtc_state *old_crtc_state,
> >                             const struct intel_crtc_state *new_crtc_state)
> {
> > -   return old_crtc_state->cmrr.cmrr_m != new_crtc_state->cmrr.cmrr_m
> ||
> > -           old_crtc_state->cmrr.cmrr_n != new_crtc_state->cmrr.cmrr_n;
> > +   return old_crtc_state->vrr.cmrr.cmrr_m != new_crtc_state-
> >vrr.cmrr.cmrr_m ||
> > +           old_crtc_state->vrr.cmrr.cmrr_n != new_crtc_state-
> >vrr.cmrr.cmrr_n;
> >  }
> >
> >  static bool intel_crtc_vrr_enabling(struct intel_atomic_state *state,
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 39e11362630c..6096ad02ae45 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1400,6 +1400,11 @@ struct intel_crtc_state {
> >                     u16 max_increase, max_decrease;
> >                     u16 vblank_target;
> >             } dc_balance;
> > +
> > +           struct {
> > +                   bool enable;
> > +                   u64 cmrr_n, cmrr_m;
> > +           } cmrr;
> 
> You're leaving the dupes behind?

Thanks Jani, for Review, I missed. I will update this while floating next 
revision.

Regards,
Mitul

> 
> >     } vrr;
> >
> >     /* Content Match Refresh Rate state */ diff --git
> > a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 3569e61e7fee..a9054b07d9c7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -3230,7 +3230,7 @@ static void intel_dp_compute_as_sdp(struct
> intel_dp *intel_dp,
> >     as_sdp->revision = 0x2;
> >     as_sdp->vtotal = intel_vrr_vmin_vtotal(crtc_state);
> >
> > -   if (crtc_state->cmrr.enable) {
> > +   if (crtc_state->vrr.cmrr.enable) {
> >             as_sdp->mode = DP_AS_SDP_FAVT_TRR_REACHED;
> >             as_sdp->target_rr = drm_mode_vrefresh(adjusted_mode);
> >             as_sdp->target_rr_divider = true;
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> > b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index c979950d32cb..83f25184c66c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -53,7 +53,7 @@ static bool intel_crtc_cmrr_enabling(struct
> intel_atomic_state *state,
> >             return false;
> >
> >     return is_enabling(cmrr.enable, old_crtc_state, new_crtc_state) ||
> > -           (new_crtc_state->cmrr.enable);
> > +           (new_crtc_state->vrr.cmrr.enable);
> >  }
> >
> >  static bool intel_crtc_cmrr_disabling(struct intel_atomic_state
> > *state, @@ -68,7 +68,7 @@ static bool intel_crtc_cmrr_disabling(struct
> intel_atomic_state *state,
> >             return false;
> >
> >     return is_disabling(cmrr.enable, old_crtc_state, new_crtc_state) ||
> > -                   (old_crtc_state->cmrr.enable);
> > +                   (old_crtc_state->vrr.cmrr.enable);
> >  }
> >
> >  bool intel_vrr_is_capable(struct intel_connector *connector) @@
> > -276,11 +276,11 @@ intel_vrr_cmrr_compute_config(struct intel_crtc_state
> *crtc_state)
> >      * is tracked in HW.
> >      */
> >
> > -   crtc_state->cmrr.cmrr_n =
> > +   crtc_state->vrr.cmrr.cmrr_n =
> >             mul_u32_u32(requested_refresh_rate * adjusted_mode-
> >crtc_htotal,
> >                         multiplier_m);
> >     adjusted_pixel_rate = mul_u32_u32(adjusted_mode->crtc_clock,
> 1000) * multiplier_n;
> > -   crtc_state->cmrr.cmrr_m = do_div(adjusted_pixel_rate, crtc_state-
> >cmrr.cmrr_n);
> > +   crtc_state->vrr.cmrr.cmrr_m = do_div(adjusted_pixel_rate,
> > +crtc_state->vrr.cmrr.cmrr_n);
> >
> >     return;
> >  }
> > @@ -875,13 +875,13 @@ intel_vrr_enable_cmrr(const struct
> intel_crtc_state *crtc_state)
> >     enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >
> >     intel_de_write(display, TRANS_CMRR_M_HI(display, cpu_transcoder),
> > -                  upper_32_bits(crtc_state->cmrr.cmrr_m));
> > +                  upper_32_bits(crtc_state->vrr.cmrr.cmrr_m));
> >     intel_de_write(display, TRANS_CMRR_M_LO(display,
> cpu_transcoder),
> > -                  lower_32_bits(crtc_state->cmrr.cmrr_m));
> > +                  lower_32_bits(crtc_state->vrr.cmrr.cmrr_m));
> >     intel_de_write(display, TRANS_CMRR_N_HI(display, cpu_transcoder),
> > -                  upper_32_bits(crtc_state->cmrr.cmrr_n));
> > +                  upper_32_bits(crtc_state->vrr.cmrr.cmrr_n));
> >     intel_de_write(display, TRANS_CMRR_N_LO(display, cpu_transcoder),
> > -                  lower_32_bits(crtc_state->cmrr.cmrr_n));
> > +                  lower_32_bits(crtc_state->vrr.cmrr.cmrr_n));
> >  }
> >
> >  static void
> > @@ -1038,7 +1038,7 @@ void intel_vrr_enable(const struct
> intel_crtc_state *crtc_state)
> >     intel_vrr_enable_dc_balancing(crtc_state);
> >
> >     if (!intel_vrr_always_use_vrr_tg(display))
> > -           intel_vrr_tg_enable(crtc_state, crtc_state->cmrr.enable);
> > +           intel_vrr_tg_enable(crtc_state, crtc_state->vrr.cmrr.enable);
> >  }
> >
> >  void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
> > @@ -1140,10 +1140,10 @@ void intel_vrr_get_config(struct
> intel_crtc_state *crtc_state)
> >     trans_vrr_ctl = intel_de_read(display,
> >                                   TRANS_VRR_CTL(display,
> cpu_transcoder));
> >
> > -   if (crtc_state->cmrr.enable) {
> > -           crtc_state->cmrr.cmrr_n =
> > +   if (crtc_state->vrr.cmrr.enable) {
> > +           crtc_state->vrr.cmrr.cmrr_n =
> >                     intel_de_read64_2x32(display,
> TRANS_CMRR_N_LO(display, cpu_transcoder));
> > -           crtc_state->cmrr.cmrr_m =
> > +           crtc_state->vrr.cmrr.cmrr_m =
> >                     intel_de_read64_2x32(display,
> TRANS_CMRR_M_LO(display, cpu_transcoder));
> >     }
> 
> --
> Jani Nikula, Intel























					Reply via email to