[PATCH v4 4/8] drm/i915/lobf: Update lobf if any change in dependent parameters

[Manna, Animesh]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula at intel.com>
> Sent: Tuesday, February 25, 2025 1:32 PM
> To: Manna, Animesh <animesh.manna at intel.com>; intel-
> gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org
> Cc: Hogander, Jouni <jouni.hogander at intel.com>; B, Jeevan
> <jeevan.b at intel.com>
> Subject: RE: [PATCH v4 4/8] drm/i915/lobf: Update lobf if any change in
> dependent parameters
> 
> On Tue, 25 Feb 2025, "Manna, Animesh" <animesh.manna at intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani <jani.nikula at intel.com>
> >> Sent: Monday, February 24, 2025 4:18 PM
> >> To: Manna, Animesh <animesh.manna at intel.com>; intel-
> >> gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org
> >> Cc: Hogander, Jouni <jouni.hogander at intel.com>; B, Jeevan
> >> <jeevan.b at intel.com>; Manna, Animesh <animesh.manna at intel.com>
> >> Subject: Re: [PATCH v4 4/8] drm/i915/lobf: Update lobf if any change
> >> in dependent parameters
> >>
> >> On Mon, 24 Feb 2025, Animesh Manna <animesh.manna at intel.com>
> wrote:
> >> > For every commit the dependent condition for LOBF is checked and
> >> > accordingly update has_lobf flag which will be used to update the
> >> > ALPM_CTL register during commit.
> >> >
> >> > Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/display/intel_alpm.c | 16 ++++++++++++++++
> >> > drivers/gpu/drm/i915/display/intel_alpm.h |  1 +
> >> > drivers/gpu/drm/i915/display/intel_ddi.c  |  2 ++
> >> >  3 files changed, 19 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> >> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> >> > index 1438e125cde1..83719ee1721c 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> >> > @@ -312,6 +312,22 @@ void intel_alpm_lobf_compute_config(struct
> >> intel_dp *intel_dp,
> >> >             (first_sdp_position + waketime_in_lines);  }
> >> >
> >> > +void intel_alpm_lobf_update(const struct intel_crtc_state
> >> > +*crtc_state) {
> >> > +   struct intel_display *display = to_intel_display(crtc_state);
> >> > +   enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >> > +   u32 alpm_ctl;
> >> > +
> >> > +   if (DISPLAY_VER(display) < 20)
> >> > +           return;
> >> > +
> >> > +   alpm_ctl = intel_de_read(display, ALPM_CTL(display,
> >> cpu_transcoder));
> >> > +   if (alpm_ctl & ALPM_CTL_LOBF_ENABLE && !crtc_state->has_lobf) {
> >>
> >> You don't need to even read the register if crtc_state->has_lobf is true.
> >
> > Thanks Jani for review.
> > Reading register to get what is there in ALPM_CTL register, like aux-less or
> aux-wake alpm and etc.
> > For LOBF we need to touch a specific bit, and don't want to modify other
> bitfields.
> 
> You only modify the register if crtc_state->has_lobf is false. You don't need
> to read the register if crtc_state->has_lobf is true.

Ok, will take care in next version.

Regards,
Animesh

> 
> 
> BR,
> Jani.
> 
> 
> >
> > Regards,
> > Animesh
> >
> >>
> >> > +           alpm_ctl &= ~ALPM_CTL_LOBF_ENABLE;
> >> > +           intel_de_write(display, ALPM_CTL(display,
> >> > + cpu_transcoder),
> >> alpm_ctl);
> >> > +   }
> >> > +}
> >> > +
> >> >  static void lnl_alpm_configure(struct intel_dp *intel_dp,
> >> >                            const struct intel_crtc_state
> >> > *crtc_state)  { diff --
> >> git
> >> > a/drivers/gpu/drm/i915/display/intel_alpm.h
> >> > b/drivers/gpu/drm/i915/display/intel_alpm.h
> >> > index 91f51fb24f98..c6efd25c2062 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> >> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> >> > @@ -23,6 +23,7 @@ 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_lobf_update(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);
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> >> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> >> > index 26aa32d4d50e..44258ba0d951 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> >> > @@ -37,6 +37,7 @@
> >> >  #include "icl_dsi.h"
> >> >  #include "intel_alpm.h"
> >> >  #include "intel_audio.h"
> >> > +#include "intel_alpm.h"
> >> >  #include "intel_audio_regs.h"
> >> >  #include "intel_backlight.h"
> >> >  #include "intel_combo_phy.h"
> >> > @@ -3621,6 +3622,7 @@ static void intel_ddi_update_pipe_dp(struct
> >> intel_atomic_state *state,
> >> >     intel_ddi_set_dp_msa(crtc_state, conn_state);
> >> >
> >> >     intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
> >> > +   intel_alpm_lobf_update(crtc_state);
> >> >
> >> >     intel_backlight_update(state, encoder, crtc_state, conn_state);
> >> >     drm_connector_update_privacy_screen(conn_state);
> >>
> >> --
> >> Jani Nikula, Intel
> 
> --
> Jani Nikula, Intel