| Message ID | 20240905051841.3012729-3-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Vrr refactoring and panel replay workaround | expand |
On Thu, Sep 05, 2024 at 10:48:39AM +0530, Animesh Manna wrote: > As lobf compute config is depends upon delayed vbalnk so moved Actually no. The code is as follows: guardband = adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - context_latency; Add context_latency to both sides: context_latency + guardband = adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay; Substitute into: crtc_state->has_lobf = (context_latency + guardband) > (first_sdp_position + waketime_in_lines); and you have no vblank delay anywhere. Someone should just rewrite that to use that simple formula. Now, that code is using the non-VRR timings for all this computation so technically it might not even be computing the correct thing when using VRR, but I guess that's more of a problem for the "let's use the VRR timing generator fixed refersh stuff" because we really should match the non-VRR timings there... > lobf compute config in encoder late config(). Lobf is only > for edp connector so introduced intel_dp_late_compute_config(). > > v1: Initial version. > v2: Function name modified. [Jani] > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 5 +++++ > drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++++- > drivers/gpu/drm/i915/display/intel_dp.h | 2 ++ > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index 00fbe9f8c03a..0d12c94793bf 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -4367,6 +4367,11 @@ static int intel_ddi_compute_config_late(struct intel_encoder *encoder, > port_sync_transcoders & ~BIT(crtc_state->cpu_transcoder); > } > > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) || > + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) { > + intel_dp_compute_config_late(encoder, crtc_state); > + } > + > return 0; > } > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 86bc6d79279f..bc0042a1bc3a 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -3107,7 +3107,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, > intel_vrr_compute_config(pipe_config, conn_state); > intel_dp_compute_as_sdp(intel_dp, pipe_config); > intel_psr_compute_config(intel_dp, pipe_config, conn_state); > - intel_alpm_lobf_compute_config(intel_dp, pipe_config); > intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16); > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state); > @@ -3116,6 +3115,14 @@ intel_dp_compute_config(struct intel_encoder *encoder, > pipe_config); > } > > +void intel_dp_compute_config_late(struct intel_encoder *encoder, > + struct intel_crtc_state *crtc_state) > +{ > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > + > + intel_alpm_lobf_compute_config(intel_dp, crtc_state); > +} > + > void intel_dp_set_link_params(struct intel_dp *intel_dp, > int link_rate, int lane_count) > { > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h > index 1b9aaddd8c35..411feece9acb 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.h > +++ b/drivers/gpu/drm/i915/display/intel_dp.h > @@ -74,6 +74,8 @@ void intel_dp_encoder_flush_work(struct drm_encoder *encoder); > int intel_dp_compute_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config, > struct drm_connector_state *conn_state); > +void intel_dp_compute_config_late(struct intel_encoder *encoder, > + struct intel_crtc_state *crtc_state); > int intel_dp_dsc_compute_config(struct intel_dp *intel_dp, > struct intel_crtc_state *pipe_config, > struct drm_connector_state *conn_state, > -- > 2.29.0
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Thursday, September 12, 2024 5:40 AM > 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 v10 2/4] drm/i915/lobf: Move Lobf compute config after > vblank adjustement > > On Thu, Sep 05, 2024 at 10:48:39AM +0530, Animesh Manna wrote: > > As lobf compute config is depends upon delayed vbalnk so moved > > Actually no. > > The code is as follows: > guardband = adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - > context_latency; > > Add context_latency to both sides: > context_latency + guardband = adjusted_mode->crtc_vtotal - > adjusted_mode->crtc_vdisplay; > > Substitute into: > crtc_state->has_lobf = (context_latency + guardband) > > (first_sdp_position + waketime_in_lines); > > and you have no vblank delay anywhere. Got your point, maybe will drop this patch as context_latency + guardband will be same even if we change the vblank_start. > > > Someone should just rewrite that to use that simple formula. > > Now, that code is using the non-VRR timings for all this computation so > technically it might not even be computing the correct thing when using VRR, > but I guess that's more of a problem for the "let's use the VRR timing > generator fixed refersh stuff" because we really should match the non-VRR > timings there... Yeah, LOBF is only applicable for VRR timing generator fixed refersh mode, will relook the above calculation once it is enabled. Regards, Animesh > > > lobf compute config in encoder late config(). Lobf is only for edp > > connector so introduced intel_dp_late_compute_config(). > > > > v1: Initial version. > > v2: Function name modified. [Jani] > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 5 +++++ > > drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++++- > > drivers/gpu/drm/i915/display/intel_dp.h | 2 ++ > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > index 00fbe9f8c03a..0d12c94793bf 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -4367,6 +4367,11 @@ static int intel_ddi_compute_config_late(struct > intel_encoder *encoder, > > port_sync_transcoders & ~BIT(crtc_state- > >cpu_transcoder); > > } > > > > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) || > > + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) { > > + intel_dp_compute_config_late(encoder, crtc_state); > > + } > > + > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 86bc6d79279f..bc0042a1bc3a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -3107,7 +3107,6 @@ intel_dp_compute_config(struct intel_encoder > *encoder, > > intel_vrr_compute_config(pipe_config, conn_state); > > intel_dp_compute_as_sdp(intel_dp, pipe_config); > > intel_psr_compute_config(intel_dp, pipe_config, conn_state); > > - intel_alpm_lobf_compute_config(intel_dp, pipe_config); > > intel_dp_drrs_compute_config(connector, pipe_config, > link_bpp_x16); > > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); > > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, > pipe_config, > > conn_state); @@ -3116,6 +3115,14 @@ intel_dp_compute_config(struct > intel_encoder *encoder, > > pipe_config); > > } > > > > +void intel_dp_compute_config_late(struct intel_encoder *encoder, > > + struct intel_crtc_state *crtc_state) { > > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > + > > + intel_alpm_lobf_compute_config(intel_dp, crtc_state); } > > + > > void intel_dp_set_link_params(struct intel_dp *intel_dp, > > int link_rate, int lane_count) { diff --git > > a/drivers/gpu/drm/i915/display/intel_dp.h > > b/drivers/gpu/drm/i915/display/intel_dp.h > > index 1b9aaddd8c35..411feece9acb 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.h > > +++ b/drivers/gpu/drm/i915/display/intel_dp.h > > @@ -74,6 +74,8 @@ void intel_dp_encoder_flush_work(struct > drm_encoder > > *encoder); int intel_dp_compute_config(struct intel_encoder *encoder, > > struct intel_crtc_state *pipe_config, > > struct drm_connector_state *conn_state); > > +void intel_dp_compute_config_late(struct intel_encoder *encoder, > > + struct intel_crtc_state *crtc_state); > > int intel_dp_dsc_compute_config(struct intel_dp *intel_dp, > > struct intel_crtc_state *pipe_config, > > struct drm_connector_state *conn_state, > > -- > > 2.29.0 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 00fbe9f8c03a..0d12c94793bf 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -4367,6 +4367,11 @@ static int intel_ddi_compute_config_late(struct intel_encoder *encoder, port_sync_transcoders & ~BIT(crtc_state->cpu_transcoder); } + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) || + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) { + intel_dp_compute_config_late(encoder, crtc_state); + } + return 0; } diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 86bc6d79279f..bc0042a1bc3a 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3107,7 +3107,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, intel_vrr_compute_config(pipe_config, conn_state); intel_dp_compute_as_sdp(intel_dp, pipe_config); intel_psr_compute_config(intel_dp, pipe_config, conn_state); - intel_alpm_lobf_compute_config(intel_dp, pipe_config); intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16); intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state); intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state); @@ -3116,6 +3115,14 @@ intel_dp_compute_config(struct intel_encoder *encoder, pipe_config); } +void intel_dp_compute_config_late(struct intel_encoder *encoder, + struct intel_crtc_state *crtc_state) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(encoder); + + intel_alpm_lobf_compute_config(intel_dp, crtc_state); +} + void intel_dp_set_link_params(struct intel_dp *intel_dp, int link_rate, int lane_count) { diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h index 1b9aaddd8c35..411feece9acb 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.h +++ b/drivers/gpu/drm/i915/display/intel_dp.h @@ -74,6 +74,8 @@ void intel_dp_encoder_flush_work(struct drm_encoder *encoder); int intel_dp_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state); +void intel_dp_compute_config_late(struct intel_encoder *encoder, + struct intel_crtc_state *crtc_state); int intel_dp_dsc_compute_config(struct intel_dp *intel_dp, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state,
As lobf compute config is depends upon delayed vbalnk so moved lobf compute config in encoder late config(). Lobf is only for edp connector so introduced intel_dp_late_compute_config(). v1: Initial version. v2: Function name modified. [Jani] Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_ddi.c | 5 +++++ drivers/gpu/drm/i915/display/intel_dp.c | 9 ++++++++- drivers/gpu/drm/i915/display/intel_dp.h | 2 ++ 3 files changed, 15 insertions(+), 1 deletion(-)