| Message ID | 20250124150020.2271747-12-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Use VRR timing generator for fixed refresh rate modes | expand |
On Fri, Jan 24, 2025 at 08:29:56PM +0530, Ankit Nautiyal wrote: > CMRR has a separate logic for computing vrr timings and so it > overwrites the timings prepared for vrr. > > Avoid prepare vrr timings for cmrr. This will help to separate the > helpers for timings for vrr, cmrr and the forthcoming > fixed_rr. > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_vrr.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index 7e69e30b2076..90fd6fe58fce 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -257,8 +257,9 @@ void intel_vrr_compute_cmrr_timings(struct intel_crtc_state *crtc_state) > } > > static > -void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state) > +void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state, int vmin, int vmax) > { > + intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax); > crtc_state->vrr.enable = true; > crtc_state->mode_flags |= I915_MODE_FLAG_VRR; > } > @@ -328,12 +329,12 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > if (vmin >= vmax) > return; > > - intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax); > - > if (crtc_state->uapi.vrr_enabled) > - intel_vrr_compute_vrr_timings(crtc_state); > + intel_vrr_compute_vrr_timings(crtc_state, vmin, vmax); > else if (is_cmrr_frac_required(crtc_state) && is_edp) > intel_vrr_compute_cmrr_timings(crtc_state); > + else > + intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax); I don't understand why the caller is calculating the vrr vmin/vmax and then passing them in to everyone. Looks to me like each of those guys should just calculate the vmin/vmax on their own. The crtc_state->vrr.flipline = crtc_state->vrr.vmin; crtc_state->vrr.vmin -= intel_vrr_min_flipline_offset(display); part could stay in the caller since it's always needed regardless of what kind of timings we use. > > if (HAS_AS_SDP(display)) { > crtc_state->vrr.vsync_start = > -- > 2.45.2
On 1/25/2025 2:39 AM, Ville Syrjälä wrote: > On Fri, Jan 24, 2025 at 08:29:56PM +0530, Ankit Nautiyal wrote: >> CMRR has a separate logic for computing vrr timings and so it >> overwrites the timings prepared for vrr. >> >> Avoid prepare vrr timings for cmrr. This will help to separate the >> helpers for timings for vrr, cmrr and the forthcoming >> fixed_rr. >> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_vrr.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c >> index 7e69e30b2076..90fd6fe58fce 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vrr.c >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c >> @@ -257,8 +257,9 @@ void intel_vrr_compute_cmrr_timings(struct intel_crtc_state *crtc_state) >> } >> >> static >> -void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state) >> +void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state, int vmin, int vmax) >> { >> + intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax); >> crtc_state->vrr.enable = true; >> crtc_state->mode_flags |= I915_MODE_FLAG_VRR; >> } >> @@ -328,12 +329,12 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, >> if (vmin >= vmax) >> return; >> >> - intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax); >> - >> if (crtc_state->uapi.vrr_enabled) >> - intel_vrr_compute_vrr_timings(crtc_state); >> + intel_vrr_compute_vrr_timings(crtc_state, vmin, vmax); >> else if (is_cmrr_frac_required(crtc_state) && is_edp) >> intel_vrr_compute_cmrr_timings(crtc_state); >> + else >> + intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax); > I don't understand why the caller is calculating the vrr vmin/vmax > and then passing them in to everyone. Looks to me like each of those > guys should just calculate the vmin/vmax on their own. We are computing vmin and vmax early to avoid computing variable timings for non vrr panels, based on the condition vmin < vmax. But I get your point. Will simplify this as mentioned below. Thanks & Regards, Ankit > > The > crtc_state->vrr.flipline = crtc_state->vrr.vmin; > crtc_state->vrr.vmin -= intel_vrr_min_flipline_offset(display); > part could stay in the caller since it's always needed regardless > of what kind of timings we use. > > > > >> >> if (HAS_AS_SDP(display)) { >> crtc_state->vrr.vsync_start = >> -- >> 2.45.2
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index 7e69e30b2076..90fd6fe58fce 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -257,8 +257,9 @@ void intel_vrr_compute_cmrr_timings(struct intel_crtc_state *crtc_state) } static -void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state) +void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state, int vmin, int vmax) { + intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax); crtc_state->vrr.enable = true; crtc_state->mode_flags |= I915_MODE_FLAG_VRR; } @@ -328,12 +329,12 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, if (vmin >= vmax) return; - intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax); - if (crtc_state->uapi.vrr_enabled) - intel_vrr_compute_vrr_timings(crtc_state); + intel_vrr_compute_vrr_timings(crtc_state, vmin, vmax); else if (is_cmrr_frac_required(crtc_state) && is_edp) intel_vrr_compute_cmrr_timings(crtc_state); + else + intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax); if (HAS_AS_SDP(display)) { crtc_state->vrr.vsync_start =
CMRR has a separate logic for computing vrr timings and so it overwrites the timings prepared for vrr. Avoid prepare vrr timings for cmrr. This will help to separate the helpers for timings for vrr, cmrr and the forthcoming fixed_rr. Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_vrr.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)