[PATCH v2 1/2] drm/i915/lobf: Add lobf enablement in post plane update
Hogander, Jouni
jouni.hogander at intel.com
Mon Dec 9 11:59:17 UTC 2024
On Mon, 2024-12-09 at 11:54 +0000, Manna, Animesh wrote:
>
>
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander at intel.com>
> > Sent: Monday, December 9, 2024 5:04 PM
> > To: Manna, Animesh <animesh.manna at intel.com>; intel-
> > gfx at lists.freedesktop.org
> > Cc: Nikula, Jani <jani.nikula at intel.com>; B, Jeevan
> > <jeevan.b at intel.com>
> > Subject: Re: [PATCH v2 1/2] drm/i915/lobf: Add lobf enablement in
> > post
> > plane update
> >
> > On Mon, 2024-12-09 at 11:19 +0000, Manna, Animesh wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Hogander, Jouni <jouni.hogander at intel.com>
> > > > Sent: Monday, December 9, 2024 2:06 PM
> > > > To: Manna, Animesh <animesh.manna at intel.com>; intel-
> > > > gfx at lists.freedesktop.org
> > > > Cc: Nikula, Jani <jani.nikula at intel.com>; B, Jeevan
> > > > <jeevan.b at intel.com>
> > > > Subject: Re: [PATCH v2 1/2] drm/i915/lobf: Add lobf enablement
> > > > in
> > > > post plane update
> > > >
> > > > On Wed, 2024-12-04 at 15:33 +0530, Animesh Manna wrote:
> > > > > Enablement of LOBF is added in post plane update whenever
> > > > > has_lobf
> > > > > flag is set. As LOBF can be enabled in non-psr case as well
> > > > > so
> > > > > adding in post plane update. There is no change of
> > > > > configuring
> > > > > alpm with psr path.
> > > > >
> > > > > v1: Initial version.
> > > > > v2: Use encoder-mask to find the associated encoder from
> > > > > crtc-
> > > > > state.
> > > > > [Jani]
> > > > >
> > > > > Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/display/intel_alpm.c | 25
> > > > > ++++++++++++++++++++
> > > > > drivers/gpu/drm/i915/display/intel_alpm.h | 4 ++++
> > > > > drivers/gpu/drm/i915/display/intel_display.c | 3 +++
> > > > > 3 files changed, 32 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > > > > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > > > > index 55f3ae1e68c9..75063342a100 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > > > > @@ -367,6 +367,31 @@ void intel_alpm_configure(struct
> > > > > intel_dp
> > > > > *intel_dp,
> > > > > lnl_alpm_configure(intel_dp, crtc_state);
> > > > > }
> > > > >
> > > > > +void intel_alpm_post_plane_update(struct intel_atomic_state
> > > > > *state,
> > > > > + struct intel_crtc *crtc) {
> > > > > + struct intel_display *display =
> > > > > to_intel_display(state);
> > > > > + const struct intel_crtc_state *crtc_state =
> > > > > + intel_atomic_get_new_crtc_state(state, crtc);
> > > > > + struct intel_encoder *encoder;
> > > > > +
> > > > > + if (!crtc_state->has_lobf)
> > > > > + return;
> > > > > +
> > > > > + for_each_intel_encoder_mask(display->drm, encoder,
> > > > > + crtc_state-
> > > > > > uapi.encoder_mask) {
> > > > > + struct intel_dp *intel_dp;
> > > > > +
> > > > > + if (!intel_encoder_is_dp(encoder))
> > > > > + continue;
> > > > > +
> > > > > + intel_dp = enc_to_intel_dp(encoder);
> > > > > +
> > > > > + if (intel_dp_is_edp(intel_dp))
> > > > > + intel_alpm_configure(intel_dp,
> > > > > crtc_state);
> > > >
> > > > This is reconfiguring lobf even if it's already configured? Is
> > > > that
> > > > wanted?
> > >
> > > has_psr and has_lobf are mutually exclusive, so alpm_configure()
> > > will
> > > be called once.
> >
> > has_psr is unrelated. Consider e.g. case where xe.enable_psr=0 is
> > in
> > bootcmd. has_psr is always false. ALPM_CTL is written with lobf
> > enabled on
> > every commit if has_lobf is true.
>
> Currently VRR fixed refresh mode check is not added in this patch
> series which can take care of resetting the has_lobf flag as well,
> thanks for catching, will add in next version.
> Hope with the above change the below comments will be addressed.
Please note that it is not about just resetting the flag. Someone has
to reset lobf enable bit in ALPM_CTL as well.
BR,
Jouni Högander
>
> Regards,
> Animesh
>
> >
> > Another thing here is that lobf is never disabled after it gets
> > enabled?
> >
> > Please also review cases where lobf needs to be disabled. E.g.
> > refresh rate
> > change. See bspec 71041.
> >
> > BR,
> >
> > Jouni Högander
> > >
> > > Regards,
> > > Animesh
> > >
> > > >
> > > > BR,
> > > >
> > > > Jouni Högander
> > > > > + }
> > > > > +}
> > > > > +
> > > > > static int i915_edp_lobf_info_show(struct seq_file *m, void
> > > > > *data)
> > > > > {
> > > > > struct intel_connector *connector = m->private; diff
> > > > > --
> > > > > git a/drivers/gpu/drm/i915/display/intel_alpm.h
> > > > > b/drivers/gpu/drm/i915/display/intel_alpm.h
> > > > > index 8c409b10dce6..2f862b0476a8 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> > > > > @@ -12,6 +12,8 @@ struct intel_dp;
> > > > > struct intel_crtc_state;
> > > > > struct drm_connector_state;
> > > > > struct intel_connector;
> > > > > +struct intel_atomic_state;
> > > > > +struct intel_crtc;
> > > > >
> > > > > void intel_alpm_init_dpcd(struct intel_dp *intel_dp);
> > > > > bool intel_alpm_compute_params(struct intel_dp *intel_dp, @@
> > > > > -
> > > > > 21,6
> > > > > +23,8 @@ void intel_alpm_lobf_compute_config(struct intel_dp
> > > > > *intel_dp,
> > > > > struct
> > > > > drm_connector_state
> > > > > *conn_state);
> > > > > void intel_alpm_configure(struct intel_dp *intel_dp,
> > > > > const struct intel_crtc_state
> > > > > *crtc_state);
> > > > > +void intel_alpm_post_plane_update(struct intel_atomic_state
> > > > > *state,
> > > > > + struct intel_crtc *crtc);
> > > > > void intel_alpm_lobf_debugfs_add(struct intel_connector
> > > > > *connector);
> > > > > bool intel_alpm_aux_wake_supported(struct intel_dp
> > > > > *intel_dp);
> > > > > bool intel_alpm_aux_less_wake_supported(struct intel_dp
> > > > > *intel_dp); diff --git
> > > > > a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 4805bf682d43..ffd96b187e5e 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -55,6 +55,7 @@
> > > > > #include "i9xx_plane.h"
> > > > > #include "i9xx_plane_regs.h"
> > > > > #include "i9xx_wm.h"
> > > > > +#include "intel_alpm.h"
> > > > > #include "intel_atomic.h"
> > > > > #include "intel_atomic_plane.h"
> > > > > #include "intel_audio.h"
> > > > > @@ -1190,6 +1191,8 @@ static void
> > > > > intel_post_plane_update(struct
> > > > > intel_atomic_state *state,
> > > > >
> > > > > intel_psr_post_plane_update(state, crtc);
> > > > >
> > > > > + intel_alpm_post_plane_update(state, crtc);
> > > > > +
> > > > > intel_frontbuffer_flip(dev_priv, new_crtc_state-
> > > > > > fb_bits);
> > > > >
> > > > > if (new_crtc_state->update_wm_post && new_crtc_state-
> > > > > > hw.active)
> > >
>
More information about the Intel-gfx
mailing list