| Message ID | 20240916075406.3521433-2-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Vrr refactoring and panel replay workaround | expand |
On Mon, Sep 16, 2024 at 01:24:05PM +0530, Animesh Manna wrote: > As vrr guardband calculation is dependent on modified > vblank start so better to compute late after all > vblank adjustement. > > v1: Initial version. > v2: Split in a separate patch from panel-replay workaround. [Ankit] > v3: Add a function for late vrr related computation. [Ville] > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 2 ++ > drivers/gpu/drm/i915/display/intel_vrr.c | 30 +++++++++++++------- > drivers/gpu/drm/i915/display/intel_vrr.h | 1 + > 3 files changed, 22 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index fdf244a32b24..111e61eceafc 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -4802,6 +4802,8 @@ intel_modeset_pipe_config_late(struct intel_atomic_state *state, > struct drm_connector *connector; > int i; > > + intel_vrr_compute_config_late(crtc_state); > + > for_each_new_connector_in_state(&state->base, connector, > conn_state, i) { > struct intel_encoder *encoder = > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index 9a51f5bac307..2de1c04bf1a5 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -239,18 +239,26 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > (crtc_state->hw.adjusted_mode.crtc_vtotal - > crtc_state->hw.adjusted_mode.vsync_end); > } > +} > > - /* > - * For XE_LPD+, we use guardband and pipeline override > - * is deprecated. > - */ > - if (DISPLAY_VER(display) >= 13) { > - crtc_state->vrr.guardband = > - crtc_state->vrr.vmin + 1 - adjusted_mode->crtc_vblank_start; > - } else { > - crtc_state->vrr.pipeline_full = > - min(255, crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start - > - crtc_state->framestart_delay - 1); > +void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state) > +{ > + struct intel_display *display = to_intel_display(crtc_state); > + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; > + > + if (crtc_state->vrr.enable) { Never use that. VRR state must be computed correctly even when VRR is not actually enabled. That is how we can simply toggle VRR during fastsets. You want to check vrr.flipline just like intel_vrr_set_transcoder_timings() does. We should perhaps add a helper with a decent name for that purpose. intel_vrr_possible() perhaps? Also flip this condition around to eliminate the nasty whole function indentation. > + /* > + * For XE_LPD+, we use guardband and pipeline override > + * is deprecated. > + */ Could drop that redundant comment while at it. > + if (DISPLAY_VER(display) >= 13) { > + crtc_state->vrr.guardband = > + crtc_state->vrr.vmin + 1 - adjusted_mode->crtc_vblank_start; > + } else { > + crtc_state->vrr.pipeline_full = > + min(255, crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start - > + crtc_state->framestart_delay - 1); > + } > } > } > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > index 89937858200d..3127c94e9778 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > @@ -18,6 +18,7 @@ bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh); > void intel_vrr_check_modeset(struct intel_atomic_state *state); > void intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > struct drm_connector_state *conn_state); > +void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state); > void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state); > void intel_vrr_enable(const struct intel_crtc_state *crtc_state); > void intel_vrr_send_push(const struct intel_crtc_state *crtc_state); > -- > 2.29.0
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Friday, September 20, 2024 4:57 PM > To: Manna, Animesh <animesh.manna@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>; > Hogander, Jouni <jouni.hogander@intel.com>; Murthy, Arun R > <arun.r.murthy@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; > Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com> > Subject: Re: [PATCH v11 1/2] drm/i915/vrr: Split vrr-compute-config in two > phases > > On Mon, Sep 16, 2024 at 01:24:05PM +0530, Animesh Manna wrote: > > As vrr guardband calculation is dependent on modified vblank start so > > better to compute late after all vblank adjustement. > > > > v1: Initial version. > > v2: Split in a separate patch from panel-replay workaround. [Ankit] > > v3: Add a function for late vrr related computation. [Ville] > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 2 ++ > > drivers/gpu/drm/i915/display/intel_vrr.c | 30 +++++++++++++------- > > drivers/gpu/drm/i915/display/intel_vrr.h | 1 + > > 3 files changed, 22 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index fdf244a32b24..111e61eceafc 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -4802,6 +4802,8 @@ intel_modeset_pipe_config_late(struct > intel_atomic_state *state, > > struct drm_connector *connector; > > int i; > > > > + intel_vrr_compute_config_late(crtc_state); > > + > > for_each_new_connector_in_state(&state->base, connector, > > conn_state, i) { > > struct intel_encoder *encoder = > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c > > b/drivers/gpu/drm/i915/display/intel_vrr.c > > index 9a51f5bac307..2de1c04bf1a5 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > > @@ -239,18 +239,26 @@ intel_vrr_compute_config(struct intel_crtc_state > *crtc_state, > > (crtc_state->hw.adjusted_mode.crtc_vtotal - > > crtc_state->hw.adjusted_mode.vsync_end); > > } > > +} > > > > - /* > > - * For XE_LPD+, we use guardband and pipeline override > > - * is deprecated. > > - */ > > - if (DISPLAY_VER(display) >= 13) { > > - crtc_state->vrr.guardband = > > - crtc_state->vrr.vmin + 1 - adjusted_mode- > >crtc_vblank_start; > > - } else { > > - crtc_state->vrr.pipeline_full = > > - min(255, crtc_state->vrr.vmin - adjusted_mode- > >crtc_vblank_start - > > - crtc_state->framestart_delay - 1); > > +void intel_vrr_compute_config_late(struct intel_crtc_state > > +*crtc_state) { > > + struct intel_display *display = to_intel_display(crtc_state); > > + struct drm_display_mode *adjusted_mode = > > +&crtc_state->hw.adjusted_mode; > > + > > + if (crtc_state->vrr.enable) { > > Never use that. VRR state must be computed correctly even when VRR is not > actually enabled. That is how we can simply toggle VRR during fastsets. > > You want to check vrr.flipline just like > intel_vrr_set_transcoder_timings() does. We should perhaps add a helper > with a decent name for that purpose. intel_vrr_possible() perhaps? Got it, will take care in next version. > > Also flip this condition around to eliminate the nasty whole function > indentation. Ok. Regards, Animesh > > > + /* > > + * For XE_LPD+, we use guardband and pipeline override > > + * is deprecated. > > + */ > > Could drop that redundant comment while at it. > > > + if (DISPLAY_VER(display) >= 13) { > > + crtc_state->vrr.guardband = > > + crtc_state->vrr.vmin + 1 - adjusted_mode- > >crtc_vblank_start; > > + } else { > > + crtc_state->vrr.pipeline_full = > > + min(255, crtc_state->vrr.vmin - > adjusted_mode->crtc_vblank_start - > > + crtc_state->framestart_delay - 1); > > + } > > } > > } > > > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h > > b/drivers/gpu/drm/i915/display/intel_vrr.h > > index 89937858200d..3127c94e9778 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > > @@ -18,6 +18,7 @@ bool intel_vrr_is_in_range(struct intel_connector > > *connector, int vrefresh); void intel_vrr_check_modeset(struct > > intel_atomic_state *state); void intel_vrr_compute_config(struct > intel_crtc_state *crtc_state, > > struct drm_connector_state *conn_state); > > +void intel_vrr_compute_config_late(struct intel_crtc_state > > +*crtc_state); > > void intel_vrr_set_transcoder_timings(const struct intel_crtc_state > > *crtc_state); void intel_vrr_enable(const struct intel_crtc_state > > *crtc_state); void intel_vrr_send_push(const struct intel_crtc_state > > *crtc_state); > > -- > > 2.29.0 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index fdf244a32b24..111e61eceafc 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -4802,6 +4802,8 @@ intel_modeset_pipe_config_late(struct intel_atomic_state *state, struct drm_connector *connector; int i; + intel_vrr_compute_config_late(crtc_state); + for_each_new_connector_in_state(&state->base, connector, conn_state, i) { struct intel_encoder *encoder = diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index 9a51f5bac307..2de1c04bf1a5 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -239,18 +239,26 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, (crtc_state->hw.adjusted_mode.crtc_vtotal - crtc_state->hw.adjusted_mode.vsync_end); } +} - /* - * For XE_LPD+, we use guardband and pipeline override - * is deprecated. - */ - if (DISPLAY_VER(display) >= 13) { - crtc_state->vrr.guardband = - crtc_state->vrr.vmin + 1 - adjusted_mode->crtc_vblank_start; - } else { - crtc_state->vrr.pipeline_full = - min(255, crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start - - crtc_state->framestart_delay - 1); +void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state) +{ + struct intel_display *display = to_intel_display(crtc_state); + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; + + if (crtc_state->vrr.enable) { + /* + * For XE_LPD+, we use guardband and pipeline override + * is deprecated. + */ + if (DISPLAY_VER(display) >= 13) { + crtc_state->vrr.guardband = + crtc_state->vrr.vmin + 1 - adjusted_mode->crtc_vblank_start; + } else { + crtc_state->vrr.pipeline_full = + min(255, crtc_state->vrr.vmin - adjusted_mode->crtc_vblank_start - + crtc_state->framestart_delay - 1); + } } } diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h index 89937858200d..3127c94e9778 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.h +++ b/drivers/gpu/drm/i915/display/intel_vrr.h @@ -18,6 +18,7 @@ bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh); void intel_vrr_check_modeset(struct intel_atomic_state *state); void intel_vrr_compute_config(struct intel_crtc_state *crtc_state, struct drm_connector_state *conn_state); +void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state); void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state); void intel_vrr_enable(const struct intel_crtc_state *crtc_state); void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
As vrr guardband calculation is dependent on modified vblank start so better to compute late after all vblank adjustement. v1: Initial version. v2: Split in a separate patch from panel-replay workaround. [Ankit] v3: Add a function for late vrr related computation. [Ville] Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 2 ++ drivers/gpu/drm/i915/display/intel_vrr.c | 30 +++++++++++++------- drivers/gpu/drm/i915/display/intel_vrr.h | 1 + 3 files changed, 22 insertions(+), 11 deletions(-)