| Message ID | 20240424183820.3591593-6-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Link off between frames for edp | expand |
On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote: > Set the Link Off Between Frames Enable bit in ALPM_CTL register. > > Note: Lobf need to be enabled adaptive sync fixed refresh mode > where vmin = vmax = flipline, which will arise after cmmr feature > enablement. Will add enabling sequence in a separate patch. Is adaptive sync needed for both Aux Wake and Aux Less Wake? > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_alpm.c | 13 +++++++++---- > drivers/gpu/drm/i915/display/intel_alpm.h | 4 ++-- > drivers/gpu/drm/i915/display/intel_psr.c | 2 +- > 3 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > b/drivers/gpu/drm/i915/display/intel_alpm.c > index 3bb69ed16aab..b08799586b58 100644 > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > @@ -290,10 +290,11 @@ void intel_alpm_compute_lobf_config(struct > intel_dp *intel_dp, > } > } > > -static void lnl_alpm_configure(struct intel_dp *intel_dp) > +static void lnl_alpm_configure(struct intel_dp *intel_dp, > + const struct intel_crtc_state > *crtc_state) > { > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > - enum transcoder cpu_transcoder = intel_dp->psr.transcoder; > + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > u32 alpm_ctl; > > if (DISPLAY_VER(dev_priv) < 20 || (!intel_dp- > >psr.psr2_enabled && > @@ -329,12 +330,16 @@ static void lnl_alpm_configure(struct intel_dp > *intel_dp) > ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp- > >alpm_parameters.fast_wake_lines); > } > > + if (crtc_state->has_lobf) > + alpm_ctl |= ALPM_CTL_LOBF_ENABLE; > + How you are planning to differentiate between AUX Wake and AUX Less Wake for LOBF? BR, Jouni Högander > alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(intel_dp- > >alpm_parameters.check_entry_lines); > > intel_de_write(dev_priv, ALPM_CTL(cpu_transcoder), alpm_ctl); > } > > -void intel_alpm_configure(struct intel_dp *intel_dp) > +void intel_alpm_configure(struct intel_dp *intel_dp, > + const struct intel_crtc_state *crtc_state) > { > - lnl_alpm_configure(intel_dp); > + lnl_alpm_configure(intel_dp, crtc_state); > } > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > b/drivers/gpu/drm/i915/display/intel_alpm.h > index b9602b71d28f..a9ca190da3e4 100644 > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > @@ -18,6 +18,6 @@ bool intel_alpm_compute_params(struct intel_dp > *intel_dp, > void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, > struct intel_crtc_state > *crtc_state, > struct drm_connector_state > *conn_state); > -void intel_alpm_configure(struct intel_dp *intel_dp); > - > +void intel_alpm_configure(struct intel_dp *intel_dp, > + const struct intel_crtc_state *crtc_state); > #endif > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index c4ab289dbc15..4eb45df20ad2 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1611,7 +1611,7 @@ static void intel_psr_enable_source(struct > intel_dp *intel_dp, > IGNORE_PSR2_HW_TRACKING : 0); > > if (intel_dp_is_edp(intel_dp)) > - intel_alpm_configure(intel_dp); > + intel_alpm_configure(intel_dp, crtc_state); > > /* > * Wa_16013835468
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Friday, May 3, 2024 1:18 PM > To: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com> > Subject: Re: [PATCH v3 5/6] drm/i915/alpm: Enable lobf from source in > ALPM_CTL > > On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote: > > Set the Link Off Between Frames Enable bit in ALPM_CTL register. > > > > Note: Lobf need to be enabled adaptive sync fixed refresh mode where > > vmin = vmax = flipline, which will arise after cmmr feature > > enablement. Will add enabling sequence in a separate patch. > > Is adaptive sync needed for both Aux Wake and Aux Less Wake? AFAIK, ALPM (aux-wake/aux-less) do not have any dependency on adaptive sync. But LOBF is dependent on ALPM (aux-wake/aux-less) and adaptive sync fixed refresh mode. > > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_alpm.c | 13 +++++++++---- > > drivers/gpu/drm/i915/display/intel_alpm.h | 4 ++-- > > drivers/gpu/drm/i915/display/intel_psr.c | 2 +- > > 3 files changed, 12 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > > b/drivers/gpu/drm/i915/display/intel_alpm.c > > index 3bb69ed16aab..b08799586b58 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > > @@ -290,10 +290,11 @@ void intel_alpm_compute_lobf_config(struct > > intel_dp *intel_dp, > > } > > } > > > > -static void lnl_alpm_configure(struct intel_dp *intel_dp) > > +static void lnl_alpm_configure(struct intel_dp *intel_dp, > > + const struct intel_crtc_state > > *crtc_state) > > { > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > - enum transcoder cpu_transcoder = intel_dp->psr.transcoder; > > + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > > u32 alpm_ctl; > > > > if (DISPLAY_VER(dev_priv) < 20 || (!intel_dp- > > >psr.psr2_enabled && > > @@ -329,12 +330,16 @@ static void lnl_alpm_configure(struct intel_dp > > *intel_dp) > > ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp- > > >alpm_parameters.fast_wake_lines); > > } > > > > + if (crtc_state->has_lobf) > > + alpm_ctl |= ALPM_CTL_LOBF_ENABLE; > > + > > How you are planning to differentiate between AUX Wake and AUX Less > Wake for LOBF? LOBF can be enabled in both aux-wake or aux-less alpm. So, check for aux-wake or aux-less is not needed. For aux wake ALPM_CTL[ ALPM AUX Less Enable ] = 0 and for aux less ALPM_CTL[ ALPM AUX Less Enable ] = 1. So, I am plaining to add has_lobf check and enable if needed before ALPM_CTL register is getting programmed. Do you see any issue here? Regards, Animesh > > BR, > > Jouni Högander > > > alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(intel_dp- > > >alpm_parameters.check_entry_lines); > > > > intel_de_write(dev_priv, ALPM_CTL(cpu_transcoder), alpm_ctl); > > } > > > > -void intel_alpm_configure(struct intel_dp *intel_dp) > > +void intel_alpm_configure(struct intel_dp *intel_dp, > > + const struct intel_crtc_state *crtc_state) > > { > > - lnl_alpm_configure(intel_dp); > > + lnl_alpm_configure(intel_dp, crtc_state); > > } > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > > b/drivers/gpu/drm/i915/display/intel_alpm.h > > index b9602b71d28f..a9ca190da3e4 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > > @@ -18,6 +18,6 @@ bool intel_alpm_compute_params(struct intel_dp > > *intel_dp, > > void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, > > struct intel_crtc_state > > *crtc_state, > > struct drm_connector_state > > *conn_state); -void intel_alpm_configure(struct intel_dp *intel_dp); > > - > > +void intel_alpm_configure(struct intel_dp *intel_dp, > > + const struct intel_crtc_state *crtc_state); > > #endif > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index c4ab289dbc15..4eb45df20ad2 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -1611,7 +1611,7 @@ static void intel_psr_enable_source(struct > > intel_dp *intel_dp, > > IGNORE_PSR2_HW_TRACKING : 0); > > > > if (intel_dp_is_edp(intel_dp)) > > - intel_alpm_configure(intel_dp); > > + intel_alpm_configure(intel_dp, crtc_state); > > > > /* > > * Wa_16013835468
On Fri, 2024-05-03 at 08:19 +0000, Manna, Animesh wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogander@intel.com> > > Sent: Friday, May 3, 2024 1:18 PM > > To: Manna, Animesh <animesh.manna@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R > > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com> > > Subject: Re: [PATCH v3 5/6] drm/i915/alpm: Enable lobf from source > > in > > ALPM_CTL > > > > On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote: > > > Set the Link Off Between Frames Enable bit in ALPM_CTL register. > > > > > > Note: Lobf need to be enabled adaptive sync fixed refresh mode > > > where > > > vmin = vmax = flipline, which will arise after cmmr feature > > > enablement. Will add enabling sequence in a separate patch. > > > > Is adaptive sync needed for both Aux Wake and Aux Less Wake? > > AFAIK, ALPM (aux-wake/aux-less) do not have any dependency on > adaptive sync. > But LOBF is dependent on ALPM (aux-wake/aux-less) and adaptive sync > fixed refresh mode. > > > > > > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_alpm.c | 13 +++++++++---- > > > drivers/gpu/drm/i915/display/intel_alpm.h | 4 ++-- > > > drivers/gpu/drm/i915/display/intel_psr.c | 2 +- > > > 3 files changed, 12 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > > > b/drivers/gpu/drm/i915/display/intel_alpm.c > > > index 3bb69ed16aab..b08799586b58 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > > > @@ -290,10 +290,11 @@ void intel_alpm_compute_lobf_config(struct > > > intel_dp *intel_dp, > > > } > > > } > > > > > > -static void lnl_alpm_configure(struct intel_dp *intel_dp) > > > +static void lnl_alpm_configure(struct intel_dp *intel_dp, > > > + const struct intel_crtc_state > > > *crtc_state) > > > { > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > - enum transcoder cpu_transcoder = intel_dp- > > > >psr.transcoder; > > > + enum transcoder cpu_transcoder = crtc_state- > > > >cpu_transcoder; > > > u32 alpm_ctl; > > > > > > if (DISPLAY_VER(dev_priv) < 20 || (!intel_dp- > > > > psr.psr2_enabled && > > > @@ -329,12 +330,16 @@ static void lnl_alpm_configure(struct > > > intel_dp > > > *intel_dp) > > > > > > ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp- > > > > alpm_parameters.fast_wake_lines); > > > } > > > > > > + if (crtc_state->has_lobf) > > > + alpm_ctl |= ALPM_CTL_LOBF_ENABLE; > > > + > > > > How you are planning to differentiate between AUX Wake and AUX Less > > Wake for LOBF? > > LOBF can be enabled in both aux-wake or aux-less alpm. So, check for > aux-wake or aux-less is not needed. > For aux wake ALPM_CTL[ ALPM AUX Less Enable ] = 0 > and for aux less ALPM_CTL[ ALPM AUX Less Enable ] = 1. > So, I am plaining to add has_lobf check and enable if needed before > ALPM_CTL register is getting programmed. Do you see any issue here? No, I was just wondering why this is missing from your patch set? BR, Jouni Högander > > Regards, > Animesh > > > > > BR, > > > > Jouni Högander > > > > > alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(intel_dp- > > > > alpm_parameters.check_entry_lines); > > > > > > intel_de_write(dev_priv, ALPM_CTL(cpu_transcoder), > > > alpm_ctl); > > > } > > > > > > -void intel_alpm_configure(struct intel_dp *intel_dp) > > > +void intel_alpm_configure(struct intel_dp *intel_dp, > > > + const struct intel_crtc_state > > > *crtc_state) > > > { > > > - lnl_alpm_configure(intel_dp); > > > + lnl_alpm_configure(intel_dp, crtc_state); > > > } > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > > > b/drivers/gpu/drm/i915/display/intel_alpm.h > > > index b9602b71d28f..a9ca190da3e4 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > > > @@ -18,6 +18,6 @@ bool intel_alpm_compute_params(struct intel_dp > > > *intel_dp, > > > void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, > > > struct intel_crtc_state > > > *crtc_state, > > > struct drm_connector_state > > > *conn_state); -void intel_alpm_configure(struct intel_dp > > > *intel_dp); > > > - > > > +void intel_alpm_configure(struct intel_dp *intel_dp, > > > + const struct intel_crtc_state > > > *crtc_state); > > > #endif > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > index c4ab289dbc15..4eb45df20ad2 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > @@ -1611,7 +1611,7 @@ static void intel_psr_enable_source(struct > > > intel_dp *intel_dp, > > > IGNORE_PSR2_HW_TRACKING : 0); > > > > > > if (intel_dp_is_edp(intel_dp)) > > > - intel_alpm_configure(intel_dp); > > > + intel_alpm_configure(intel_dp, crtc_state); > > > > > > /* > > > * Wa_16013835468 >
diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c index 3bb69ed16aab..b08799586b58 100644 --- a/drivers/gpu/drm/i915/display/intel_alpm.c +++ b/drivers/gpu/drm/i915/display/intel_alpm.c @@ -290,10 +290,11 @@ void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, } } -static void lnl_alpm_configure(struct intel_dp *intel_dp) +static void lnl_alpm_configure(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); - enum transcoder cpu_transcoder = intel_dp->psr.transcoder; + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; u32 alpm_ctl; if (DISPLAY_VER(dev_priv) < 20 || (!intel_dp->psr.psr2_enabled && @@ -329,12 +330,16 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp) ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp->alpm_parameters.fast_wake_lines); } + if (crtc_state->has_lobf) + alpm_ctl |= ALPM_CTL_LOBF_ENABLE; + alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(intel_dp->alpm_parameters.check_entry_lines); intel_de_write(dev_priv, ALPM_CTL(cpu_transcoder), alpm_ctl); } -void intel_alpm_configure(struct intel_dp *intel_dp) +void intel_alpm_configure(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state) { - lnl_alpm_configure(intel_dp); + lnl_alpm_configure(intel_dp, crtc_state); } diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h b/drivers/gpu/drm/i915/display/intel_alpm.h index b9602b71d28f..a9ca190da3e4 100644 --- a/drivers/gpu/drm/i915/display/intel_alpm.h +++ b/drivers/gpu/drm/i915/display/intel_alpm.h @@ -18,6 +18,6 @@ bool intel_alpm_compute_params(struct intel_dp *intel_dp, void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state, struct drm_connector_state *conn_state); -void intel_alpm_configure(struct intel_dp *intel_dp); - +void intel_alpm_configure(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state); #endif diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index c4ab289dbc15..4eb45df20ad2 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1611,7 +1611,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, IGNORE_PSR2_HW_TRACKING : 0); if (intel_dp_is_edp(intel_dp)) - intel_alpm_configure(intel_dp); + intel_alpm_configure(intel_dp, crtc_state); /* * Wa_16013835468
Set the Link Off Between Frames Enable bit in ALPM_CTL register. Note: Lobf need to be enabled adaptive sync fixed refresh mode where vmin = vmax = flipline, which will arise after cmmr feature enablement. Will add enabling sequence in a separate patch. Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_alpm.c | 13 +++++++++---- drivers/gpu/drm/i915/display/intel_alpm.h | 4 ++-- drivers/gpu/drm/i915/display/intel_psr.c | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-)