> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Sent: Thursday, September 12, 2024 5:40 AM > To: Manna, Animesh <animesh.manna@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Nikula, Jani <jani.nikula@xxxxxxxxx>; > Hogander, Jouni <jouni.hogander@xxxxxxxxx>; Murthy, Arun R > <arun.r.murthy@xxxxxxxxx>; Nautiyal, Ankit K <ankit.k.nautiyal@xxxxxxxxx>; > Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@xxxxxxxxx> > 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@xxxxxxxxx> > > --- > > 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