> -----Original Message-----
> From: Nikula, Jani <jani.nikula@xxxxxxxxx>
> Sent: Tuesday, February 25, 2025 1:32 PM
> To: Manna, Animesh <animesh.manna@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx
> Cc: Hogander, Jouni <jouni.hogander@xxxxxxxxx>; B, Jeevan
> <jeevan.b@xxxxxxxxx>
> 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@xxxxxxxxx>
> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani <jani.nikula@xxxxxxxxx>
> >> Sent: Monday, February 24, 2025 4:18 PM
> >> To: Manna, Animesh <animesh.manna@xxxxxxxxx>; intel-
> >> gfx@xxxxxxxxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx
> >> Cc: Hogander, Jouni <jouni.hogander@xxxxxxxxx>; B, Jeevan
> >> <jeevan.b@xxxxxxxxx>; Manna, Animesh <animesh.manna@xxxxxxxxx>
> >> 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@xxxxxxxxx>
> 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@xxxxxxxxx>
> >> > ---
> >> > 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
