RE: [PATCH v7 0/6] Link off between frames for edp
> -Original Message- > From: Manna, Animesh > Sent: Thursday, May 30, 2024 1:38 AM > To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: Manna, Animesh > Subject: [PATCH v7 0/6] Link off between frames for edp > > Link Off Between Active Frames (LOBF) allows an eDP link to be turned Off > and On durning long VBLANK durations without enabling any of the > PSR/PSR2/PR modes of operation. > > Bspec: 71477 > > Note: Lobf need to be enabled adaptive sync fixed refresh mode where vmin > = vmax = flipline, which will arise after cmmr feature enablement. Currently > existing code refactored and make compute-config() and enabling function > ready. Will add enabling sequence in a separate patch. > > Signed-off-by: Animesh Manna > > Animesh Manna (5): > drm/i915/alpm: Move alpm parameters from intel_psr > drm/i915/alpm: Move alpm related code to a new file > drm/i915/alpm: Add compute config for lobf > drm/i915/alpm: Enable lobf from source in ALPM_CTL > drm/i915/alpm: Add debugfs for LOBF > > Jouni Högander (1): > drm/display: Add missing aux less alpm wake related bits Above changes pushed to din. Thank you Jouni, Maxime, Ankit for review. Regards, Animesh > > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/display/intel_alpm.c | 411 ++ > drivers/gpu/drm/i915/display/intel_alpm.h | 25 ++ > .../drm/i915/display/intel_display_debugfs.c | 2 + > .../drm/i915/display/intel_display_types.h| 26 +- > drivers/gpu/drm/i915/display/intel_dp.c | 4 + > drivers/gpu/drm/i915/display/intel_psr.c | 303 + > drivers/gpu/drm/xe/Makefile | 1 + > include/drm/display/drm_dp.h | 5 +- > 9 files changed, 475 insertions(+), 303 deletions(-) create mode 100644 > drivers/gpu/drm/i915/display/intel_alpm.c > create mode 100644 drivers/gpu/drm/i915/display/intel_alpm.h > > -- > 2.29.0
RE: [PATCH v6 3/6] drm/display: Add missing aux less alpm wake related bits
+ drm-core maintainer Hi, Could you please have a look and ack this patch. Received r-b from Jouni on 0th patch for the whole patch series. Regards, Animesh > -Original Message- > From: Intel-gfx On Behalf Of > Animesh Manna > Sent: Monday, May 27, 2024 1:57 PM > To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: Nikula, Jani ; Hogander, Jouni > ; Murthy, Arun R > Subject: [PATCH v6 3/6] drm/display: Add missing aux less alpm wake related > bits > > From: Jouni Högander > > eDP1.5 adds some more bits into DP_RECEIVER_ALPM_CAP and > DP_RECEIVER_ALPM_CONFIG registers. Add definitions for these. > > Signed-off-by: Jouni Högander > --- > include/drm/display/drm_dp.h | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 906949ca3cee..3317ff88ed59 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -232,6 +232,8 @@ > > #define DP_RECEIVER_ALPM_CAP 0x02e /* eDP 1.4 */ > # define DP_ALPM_CAP (1 << 0) > +# define DP_ALPM_PM_STATE_2A_SUPPORT (1 << 1) /* eDP 1.5 */ > +# define DP_ALPM_AUX_LESS_CAP(1 << 2) /* eDP 1.5 */ > > #define DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP 0x02f /* eDP 1.4 */ > # define DP_AUX_FRAME_SYNC_CAP (1 << 0) > @@ -683,7 +685,8 @@ > > #define DP_RECEIVER_ALPM_CONFIG 0x116 /* eDP 1.4 */ > # define DP_ALPM_ENABLE (1 << 0) > -# define DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE (1 << 1) > +# define DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE (1 << 1) /* eDP 1.5 */ > +# define DP_ALPM_MODE_AUX_LESS (1 << 2) /* eDP 1.5 */ > > #define DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF 0x117 /* eDP 1.4 */ > # define DP_AUX_FRAME_SYNC_ENABLE(1 << 0) > -- > 2.29.0
RE: [PATCH v4 4/6] drm/i915/alpm: Add compute config for lobf
> -Original Message-
> From: Hogander, Jouni
> Sent: Monday, May 13, 2024 1:02 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani
> Subject: Re: [PATCH v4 4/6] drm/i915/alpm: Add compute config for lobf
>
> On Thu, 2024-05-09 at 11:01 +0530, Animesh Manna wrote:
> > Link Off Between Active Frames, is a new feature for eDP that allows
> > the panel to go to lower power state after transmission of data. This
> > is a feature on top of ALPM, AS SDP.
> > Add compute config during atomic-check phase.
> >
> > v1: RFC version.
> > v2: Add separate flag for auxless-alpm. [Jani]
> > v3:
> > - intel_dp->lobf_supported replaced with crtc_state->has_lobf.
> > [Jouni]
> > - Add DISPLAY_VER() check. [Jouni]
> > - Modify function name of get_aux_less_status. [Jani]
> > v4: Add enum alpm_mode to hold the aux-wake/less capability.
> >
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 58
> > +++
> > drivers/gpu/drm/i915/display/intel_alpm.h | 5 ++
> > .../drm/i915/display/intel_display_types.h | 11
> > drivers/gpu/drm/i915/display/intel_dp.c | 4 ++
> > 4 files changed, 78 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index ee6c2a959f09..5979eab1f2e0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -11,6 +11,23 @@
> > #include "intel_dp_aux.h"
> > #include "intel_psr_regs.h"
> >
> > +enum alpm_mode intel_alpm_get_capability(struct intel_dp *intel_dp) {
> > + u8 alpm_caps = 0;
> > +
> > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> > + &alpm_caps) != 1)
> > + return ALPM_INVALID;
> > +
> > + if (alpm_caps & DP_ALPM_CAP)
> > + return ALPM_AUX_WAKE;
> > +
> > + if (alpm_caps & DP_ALPM_AUX_LESS_CAP)
> > + return ALPM_AUX_LESS;
> > +
> > + return ALPM_NOT_SUPPORTED;
> > +}
>
> This will always return ALPM_AUX_WAKE if both are supported. I don't think
> this is what you want?
>
> You could add alpm_dpcd into intel_dp. Then for this purpose add
> aux_wake_supported() and aux_less_wake_supported()?
Ok, will add in next version.
Regards,
Animesh
>
> BR,
>
> Jouni Högander
>
> > +
> > /*
> > * See Bspec: 71632 for the table
> > *
> > @@ -242,6 +259,47 @@ bool intel_alpm_compute_params(struct intel_dp
> > *intel_dp,
> > return true;
> > }
> >
> > +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp,
> > + struct intel_crtc_state
> > *crtc_state,
> > + struct drm_connector_state
> > *conn_state)
> > +{
> > + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > + struct drm_display_mode *adjusted_mode = &crtc_state-
> > >hw.adjusted_mode;
> > + int waketime_in_lines, first_sdp_position;
> > + int context_latency, guardband;
> > +
> > + if (!intel_dp_is_edp(intel_dp))
> > + return;
> > +
> > + if (DISPLAY_VER(i915) < 20)
> > + return;
> > +
> > + if (!intel_dp_as_sdp_supported(intel_dp))
> > + return;
> > +
> > + if (crtc_state->has_psr)
> > + return;
> > +
> > + if (intel_dp->alpm_parameters.mode == ALPM_INVALID ||
> > + intel_dp->alpm_parameters.mode == ALPM_NOT_SUPPORTED)
> > + return;
> > +
> > + if (!intel_alpm_compute_params(intel_dp, crtc_state))
> > + return;
> > +
> > + context_latency = adjusted_mode->crtc_vblank_start -
> > adjusted_mode->crtc_vdisplay;
> > + guardband = adjusted_mode->crtc_vtotal -
> > + adjusted_mode->crtc_vdisplay - context_latency;
> > + first_sdp_position = adjusted_mode->crtc_vtotal -
> > adjusted_mode->crtc_vsync_start;
> > + if (intel_dp->alpm_parameters.mode == ALPM_AUX_LESS)
> > + waketime_in_lines = intel_dp-
> > >alpm_parameters.io_wake_lines;
> > + else
> > + waketime_in_lines = intel_dp-
RE: [PATCH v3 4/6] drm/i915/alpm: Add compute config for lobf
> -Original Message-
> From: Hogander, Jouni
> Sent: Friday, May 3, 2024 12:49 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani
> Subject: Re: [PATCH v3 4/6] drm/i915/alpm: Add compute config for lobf
>
> On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote:
> > Link Off Between Active Frames, is a new feature for eDP
> > that allows the panel to go to lower power state after
> > transmission of data. This is a feature on top of ALPM, AS SDP.
> > Add compute config during atomic-check phase.
> >
> > v1: RFC version.
> > v2: Add separate flag for auxless-alpm. [Jani]
> > v3:
> > - intel_dp->lobf_supported replaced with crtc_state->has_lobf.
> > [Jouni]
> > - Add DISPLAY_VER() check. [Jouni]
> > - Modify function name of get_aux_less_status. [Jani]
> >
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 48
> > +++
> > drivers/gpu/drm/i915/display/intel_alpm.h | 5 ++
> > .../drm/i915/display/intel_display_types.h | 4 ++
> > drivers/gpu/drm/i915/display/intel_dp.c | 4 ++
> > 4 files changed, 61 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index 13bac3e8c8fa..3bb69ed16aab 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -11,6 +11,16 @@
> > #include "intel_dp_aux.h"
> > #include "intel_psr_regs.h"
> >
> > +bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp)
> > +{
> > + u8 alpm_caps = 0;
> > +
> > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> > + &alpm_caps) != 1)
> > + return false;
> > + return alpm_caps & DP_ALPM_AUX_LESS_CAP;
> > +}
> > +
> > /*
> > * See Bspec: 71632 for the table
> > *
> > @@ -242,6 +252,44 @@ bool intel_alpm_compute_params(struct intel_dp
> > *intel_dp,
> > return true;
> > }
> >
> > +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp,
> > + struct intel_crtc_state
> > *crtc_state,
> > + struct drm_connector_state
> > *conn_state)
> > +{
> > + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > + struct drm_display_mode *adjusted_mode = &crtc_state-
> > >hw.adjusted_mode;
> > + int waketime_in_lines, first_sdp_position;
> > + int context_latency, guardband;
> > +
> > + crtc_state->has_lobf = false;
>
> Drop this line. I think crtc_state is reset before doing this
> compute_config
Sure.
>
> > +
> > + if (!intel_dp_is_edp(intel_dp))
> > + return;
> > +
> > + if (DISPLAY_VER(i915) < 20)
> > + return;
> > +
> > + if (!intel_dp_as_sdp_supported(intel_dp))
> > + return;
> > +
> > + if (crtc_state->has_psr)
> > + return;
> > +
> > + if (intel_alpm_compute_params(intel_dp, crtc_state)) {
>
> I think it is easier to read and helps avoiding big if blocks if you:
>
> if (!intel_alpm_compute_params(intel_dp, crtc_state())
> return;
Ok.
>
> This actually brings up another thing: do we want to spread intel_psr.c
> pollution by continue using these boolean return values? I would prefer
> changing intel_alpm_compute_params return value to "normal" int
> approach and return 0 on success. This would mean one more patch
> changing it.
Ok.
>
> > + context_latency = adjusted_mode->crtc_vblank_start -
> > adjusted_mode->crtc_vdisplay;
> > + guardband = adjusted_mode->crtc_vtotal -
> > + adjusted_mode->crtc_vdisplay -
> > context_latency;
> > + first_sdp_position = adjusted_mode->crtc_vtotal -
> > adjusted_mode->crtc_vsync_start;
> > + if (intel_dp->alpm_parameters.auxless_alpm_supported)
> > + waketime_in_lines = intel_dp-
> > >alpm_parameters.io_wake_lines;
> > + else
> > + waketime_in_lines = intel_dp-
> > >alpm_parameters.fast_wake_lines;
> > +
> > + if ((context_latency + guardband) >
> &g
RE: [PATCH v3 6/6] drm/i915/alpm: Add debugfs for LOBF
> -Original Message-
> From: Hogander, Jouni
> Sent: Friday, May 3, 2024 1:02 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani
> Subject: Re: [PATCH v3 6/6] drm/i915/alpm: Add debugfs for LOBF
>
> On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote:
> > For validation purpose add debugfs for LOBF.
> >
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 48
> > +++
> > drivers/gpu/drm/i915/display/intel_alpm.h | 2 +
> > .../drm/i915/display/intel_display_debugfs.c | 2 +
> > 3 files changed, 52 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index b08799586b58..2d3027c2fb0a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -343,3 +343,51 @@ void intel_alpm_configure(struct intel_dp
> > *intel_dp,
> > {
> > lnl_alpm_configure(intel_dp, crtc_state);
> > }
> > +
> > +static int i915_edp_lobf_info_show(struct seq_file *m, void *data)
> > +{
> > + struct intel_connector *connector = m->private;
> > + struct drm_i915_private *dev_priv = to_i915(connector-
> > >base.dev);
> > + struct drm_crtc *crtc;
> > + struct intel_crtc_state *crtc_state;
> > + enum transcoder cpu_transcoder;
> > + bool lobf_enabled;
> > + int ret;
> > +
> > + ret = drm_modeset_lock_single_interruptible(&dev_priv-
> > >drm.mode_config.connection_mutex);
> > + if (ret)
> > + return ret;
> > +
> > + crtc = connector->base.state->crtc;
> > + if (connector->base.status != connector_status_connected ||
> > !crtc) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + crtc_state = to_intel_crtc_state(crtc->state);
> > + seq_printf(m, "LOBF Criteria met: %s\n",
> > str_yes_no(crtc_state->has_lobf));
>
> I'm still not convinced on this. has_lobf ~= ALPM_CTL_LOBF_ENABLE in
> ALPM_CTL. To my opinion it is enough to dump seq_printf(m, "LOBF
> status: %s\n", str_enabled_disabled(lobf_enabled)) below. Maybe
> AUX_WAKE and AUX_LESS_WAKE could be dumped instead?
Can add aux-wake or aux-less info as well.
But as LOBF feature is dependent on adaptive sync fixed refresh rate mode
(which can be dynamic as per user input) and ALPM. I want to expose both the
conditions are satisfying or not along with status.
Regards,
Animesh
>
> BR,
>
> Jouni Högander
>
> > +
> > + cpu_transcoder = crtc_state->cpu_transcoder;
> > + lobf_enabled = intel_de_read(dev_priv,
> > ALPM_CTL(cpu_transcoder)) & ALPM_CTL_LOBF_ENABLE;
> > + seq_printf(m, "LOBF status: %s\n",
> > str_enabled_disabled(lobf_enabled));
> > +
> > +out:
> > + drm_modeset_unlock(&dev_priv-
> > >drm.mode_config.connection_mutex);
> > +
> > + return ret;
> > +}
> > +
> > +DEFINE_SHOW_ATTRIBUTE(i915_edp_lobf_info);
> > +
> > +void intel_alpm_lobf_debugfs_add(struct intel_connector *connector)
> > +{
> > + struct drm_i915_private *i915 = to_i915(connector->base.dev);
> > + struct dentry *root = connector->base.debugfs_entry;
> > +
> > + if (DISPLAY_VER(i915) < 20 ||
> > + connector->base.connector_type !=
> DRM_MODE_CONNECTOR_eDP)
> > + return;
> > +
> > + debugfs_create_file("i915_edp_lobf_info", 0444, root,
> > + connector, &i915_edp_lobf_info_fops);
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h
> > b/drivers/gpu/drm/i915/display/intel_alpm.h
> > index a9ca190da3e4..01fd08eb96f5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> > @@ -11,6 +11,7 @@
> > struct intel_dp;
> > struct intel_crtc_state;
> > struct drm_connector_state;
> > +struct intel_connector;
> >
> > bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp);
> > bool intel_alpm_compute_params(struct intel_dp *intel_dp,
> > @@ -20,4 +21,5 @@ void intel_alpm_compute_lobf_config(struct intel_dp
> > *intel_dp,
> > struct drm_connector_state
> > *conn_state);
>
RE: [PATCH v3 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL
> -Original Message-
> From: Hogander, Jouni
> Sent: Friday, May 3, 2024 1:18 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani
> 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
> > ---
> > 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
RE: [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
> -Original Message-
> From: Nikula, Jani
> Sent: Monday, April 15, 2024 5:23 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni
> ; Murthy, Arun R ;
> Manna, Animesh
> Subject: Re: [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
>
> On Fri, 12 Apr 2024, Animesh Manna wrote:
> > For validation purpose add debugfs for LOBF.
> >
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 47 +++
> > drivers/gpu/drm/i915/display/intel_alpm.h | 2 +
> > .../drm/i915/display/intel_display_debugfs.c | 2 +
> > 3 files changed, 51 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index ae894c85233c..21dfc06952d7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -339,3 +339,50 @@ void intel_alpm_configure(struct intel_dp
> > *intel_dp) {
> > lnl_alpm_configure(intel_dp);
> > }
> > +
> > +static int i915_edp_lobf_support_show(struct seq_file *m, void *data)
> > +{
> > + struct intel_connector *connector = m->private;
> > + struct intel_dp *intel_dp = intel_attached_dp(connector);
> > +
> > + seq_printf(m, "LOBF support: = %s",
> > + str_yes_no(intel_dp->lobf_supported));
>
> If you have individual debugfs files, where the name tells you what it's
> about,
> what's the point in printing "LOBF support" here?
>
> Moreover, please be more careful, this now prints "LOBF support: = yes".
> And you'll want the \n in the end.
Ok.
>
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_SHOW_ATTRIBUTE(i915_edp_lobf_support);
> > +
> > +static int i915_edp_lobf_status_show(struct seq_file *m, void *data)
> > +{
> > + struct intel_connector *connector = m->private;
> > + struct intel_dp *intel_dp = intel_attached_dp(connector);
> > + const char *status;
> > +
> > + if (intel_dp->lobf_enabled)
> > + status = "enabled";
> > + else
> > + status = "disabled";
> > +
> > + seq_printf(m, "LOBF: %s\n", status);
>
> Ditto. But there's also str_enabled_disabled().
>
> I mean you could have a read-only info file which prints all of this info with
> the prefixes. But if it's one attribute per file, why have the extra prints?
> Maybe it should be just alpm info? Idk.
Sure, will go with a single debugfs entry lobf_info. Thanks for the input.
Regards,
Animesh
>
> BR,
> Jani.
>
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_SHOW_ATTRIBUTE(i915_edp_lobf_status);
> > +
> > +void intel_alpm_lobf_debugfs_add(struct intel_connector *connector) {
> > + struct drm_i915_private *i915 = to_i915(connector->base.dev);
> > + struct dentry *root = connector->base.debugfs_entry;
> > +
> > + if (DISPLAY_VER(i915) >= 20 &&
> > + connector->base.connector_type !=
> DRM_MODE_CONNECTOR_eDP)
> > + return;
> > +
> > + debugfs_create_file("i915_edp_lobf_supported", 0444, root,
> > + connector, &i915_edp_lobf_support_fops);
> > +
> > + debugfs_create_file("i915_edp_lobf_status", 0444, root,
> > + connector, &i915_edp_lobf_status_fops); }
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h
> > b/drivers/gpu/drm/i915/display/intel_alpm.h
> > index c341d2c2b7f7..66e81ed8b2fb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> > @@ -11,6 +11,7 @@
> > struct intel_dp;
> > struct intel_crtc_state;
> > struct drm_connector_state;
> > +struct intel_connector;
> >
> > bool intel_dp_get_aux_less_alpm_status(struct intel_dp *intel_dp);
> > bool intel_alpm_compute_params(struct intel_dp *intel_dp, @@ -19,5
> > +20,6 @@ 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_lobf_debugfs_add(struct intel_connector *connector);
> >
> > #endif
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> &g
RE: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL
> -Original Message-
> From: Nikula, Jani
> Sent: Monday, April 15, 2024 5:19 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni
> ; Murthy, Arun R ;
> Manna, Animesh
> Subject: Re: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in
> ALPM_CTL
>
> On Fri, 12 Apr 2024, Animesh Manna wrote:
> > Set the Link Off Between Frames Enable bit in ALPM_CTL register.
> >
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 5 +
> > drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index 699f2f051766..ae894c85233c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -325,6 +325,11 @@ static void lnl_alpm_configure(struct intel_dp
> *intel_dp)
> >ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp-
> >alpm_parameters.fast_wake_lines);
> > }
> >
> > + if (intel_dp->lobf_supported) {
> > + alpm_ctl |= ALPM_CTL_LOBF_ENABLE;
> > + intel_dp->lobf_enabled = true;
>
> Gut feeling says this should not be part of intel_dp but rather crtc state.
Kept with the same place with alpm parameters, will think over again.
Regards,
Animesh
>
> BR,
> Jani.
>
> > + }
> > +
> > 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); diff
> > --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 6116c383b543..f61ba582429b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1884,6 +1884,7 @@ struct intel_dp {
> >
> > /* LOBF flags*/
> > bool lobf_supported;
> > + bool lobf_enabled;
> > };
> >
> > enum lspcon_vendor {
>
> --
> Jani Nikula, Intel
RE: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
> -Original Message- > From: Nikula, Jani > Sent: Monday, April 15, 2024 5:18 PM > To: Manna, Animesh ; intel- > g...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni > ; Murthy, Arun R ; > Manna, Animesh > Subject: Re: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf > > On Fri, 12 Apr 2024, Animesh Manna wrote: > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > > b/drivers/gpu/drm/i915/display/intel_alpm.h > > index c45d078e5a6b..c341d2c2b7f7 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > > @@ -10,9 +10,14 @@ > > > > struct intel_dp; > > struct intel_crtc_state; > > +struct drm_connector_state; > > > > +bool intel_dp_get_aux_less_alpm_status(struct intel_dp *intel_dp); > > The names here are supposed to be intel_alpm_*. Is the function in the > wrong place or is the name wrong? Sure, will change the function name to intel_alpm_get_auxless_status(). Regards, Animesh > > BR, > Jani. > > > bool intel_alpm_compute_params(struct intel_dp *intel_dp, > >struct intel_crtc_state *crtc_state); > > +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); > > > > #endif > > -- > Jani Nikula, Intel
RE: [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a new file
> -Original Message- > From: Nikula, Jani > Sent: Monday, April 15, 2024 5:17 PM > To: Manna, Animesh ; intel- > g...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni > ; Murthy, Arun R ; > Manna, Animesh > Subject: Re: [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a > new file > > On Fri, 12 Apr 2024, Animesh Manna wrote: > > Move ALPM feature related code as it will be used for non-psr panel > > also thorugh LOBF feature. > > > > Signed-off-by: Animesh Manna > > --- > > drivers/gpu/drm/i915/Makefile | 1 + > > drivers/gpu/drm/i915/display/intel_alpm.c | 292 > > ++ drivers/gpu/drm/i915/display/intel_alpm.h | > > 18 ++ drivers/gpu/drm/i915/display/intel_psr.c | 280 > > + > > 4 files changed, 314 insertions(+), 277 deletions(-) create mode > > 100644 drivers/gpu/drm/i915/display/intel_alpm.c > > create mode 100644 drivers/gpu/drm/i915/display/intel_alpm.h > > > > diff --git a/drivers/gpu/drm/i915/Makefile > > b/drivers/gpu/drm/i915/Makefile index af9e871daf1d..c12b7bd98320 > > 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -246,6 +246,7 @@ i915-y += \ > > display/intel_atomic.o \ > > display/intel_atomic_plane.o \ > > display/intel_audio.o \ > > + display/intel_alpm.o \ > > display/intel_bios.o \ > > display/intel_bw.o \ > > display/intel_cdclk.o \ > > That's not sorted. Agree. Will take care in next version. Regards, Animesh > > BR, > Jani. > > > -- > Jani Nikula, Intel
RE: [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
> -Original Message-
> From: Hogander, Jouni
> Sent: Monday, April 15, 2024 3:44 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani
> Subject: Re: [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
>
> On Fri, 2024-04-12 at 21:22 +0530, Animesh Manna wrote:
> > For validation purpose add debugfs for LOBF.
> >
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 47
> > +++
> > drivers/gpu/drm/i915/display/intel_alpm.h | 2 +
> > .../drm/i915/display/intel_display_debugfs.c | 2 +
> > 3 files changed, 51 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index ae894c85233c..21dfc06952d7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -339,3 +339,50 @@ void intel_alpm_configure(struct intel_dp
> > *intel_dp)
> > {
> > lnl_alpm_configure(intel_dp);
> > }
> > +
> > +static int i915_edp_lobf_support_show(struct seq_file *m, void
> > *data)
> > +{
> > + struct intel_connector *connector = m->private;
> > + struct intel_dp *intel_dp = intel_attached_dp(connector);
> > +
> > + seq_printf(m, "LOBF support: = %s",
> > + str_yes_no(intel_dp->lobf_supported));
> > +
> > + return 0;
>
> What this debugfs is telling? Lobf may be supported by platform, but not
> enabled because PSR is enabled. Saying LOBF support = no is misleading.
How about "LOBF entry criteria met = yes/no"?
>
> > +}
> > +
> > +DEFINE_SHOW_ATTRIBUTE(i915_edp_lobf_support);
> > +
> > +static int i915_edp_lobf_status_show(struct seq_file *m, void *data)
> > +{
> > + struct intel_connector *connector = m->private;
> > + struct intel_dp *intel_dp = intel_attached_dp(connector);
> > + const char *status;
> > +
> > + if (intel_dp->lobf_enabled)
>
> I think better option is to read it from the registers.
Sure, will add.
Regards,
Animesh
>
> BR,
>
> Jouni Högander
>
> > + status = "enabled";
> > + else
> > + status = "disabled";
> > +
> > + seq_printf(m, "LOBF: %s\n", status);
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_SHOW_ATTRIBUTE(i915_edp_lobf_status);
> > +
> > +void intel_alpm_lobf_debugfs_add(struct intel_connector *connector) {
> > + struct drm_i915_private *i915 = to_i915(connector->base.dev);
> > + struct dentry *root = connector->base.debugfs_entry;
> > +
> > + if (DISPLAY_VER(i915) >= 20 &&
> > + connector->base.connector_type !=
> DRM_MODE_CONNECTOR_eDP)
> > + return;
> > +
> > + debugfs_create_file("i915_edp_lobf_supported", 0444, root,
> > + connector, &i915_edp_lobf_support_fops);
> > +
> > + debugfs_create_file("i915_edp_lobf_status", 0444, root,
> > + connector, &i915_edp_lobf_status_fops); }
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h
> > b/drivers/gpu/drm/i915/display/intel_alpm.h
> > index c341d2c2b7f7..66e81ed8b2fb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> > @@ -11,6 +11,7 @@
> > struct intel_dp;
> > struct intel_crtc_state;
> > struct drm_connector_state;
> > +struct intel_connector;
> >
> > bool intel_dp_get_aux_less_alpm_status(struct intel_dp *intel_dp);
> > bool intel_alpm_compute_params(struct intel_dp *intel_dp, @@ -19,5
> > +20,6 @@ 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_lobf_debugfs_add(struct intel_connector *connector);
> >
> > #endif
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 0feffe8d4e45..ba1530149836 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -13,6 +13,7 @@
> > #include "i915_debugfs.h"
> > #include "i915_irq.h"
> > #include "i915_reg.h"
> > +#include "intel_alpm.h"
> > #include "intel_crtc.h"
> > #include "intel_de.h"
> > #include "intel_crtc_state_dump.h"
> > @@ -1542,6 +1543,7 @@ void intel_connector_debugfs_add(struct
> > intel_connector *connector)
> > intel_drrs_connector_debugfs_add(connector);
> > intel_pps_connector_debugfs_add(connector);
> > intel_psr_connector_debugfs_add(connector);
> > + intel_alpm_lobf_debugfs_add(connector);
> >
> > if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > connector_type == DRM_MODE_CONNECTOR_HDMIA ||
RE: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL
> -Original Message-
> From: Hogander, Jouni
> Sent: Monday, April 15, 2024 3:39 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani
> Subject: Re: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in
> ALPM_CTL
>
> On Fri, 2024-04-12 at 21:22 +0530, Animesh Manna wrote:
> > Set the Link Off Between Frames Enable bit in ALPM_CTL register.
> >
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 5 +
> > drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index 699f2f051766..ae894c85233c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -325,6 +325,11 @@ static void lnl_alpm_configure(struct intel_dp
> > *intel_dp)
> > ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp-
> > >alpm_parameters.fast_wake_lines);
> > }
> >
> > + if (intel_dp->lobf_supported) {
> > + alpm_ctl |= ALPM_CTL_LOBF_ENABLE;
> > + intel_dp->lobf_enabled = true;
> > + }
> > +
>
> I don't see lnl_alpm_configure being called for lobf case in your patches.
Enabling/Disabling LOBF will be done along with alpm(aux-wake/aux-less)
enablement.
Here lobf_supported flag is the switch to enable LOBF or not.
Please let me know if I am missing anything.
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);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 6116c383b543..f61ba582429b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1884,6 +1884,7 @@ struct intel_dp {
> >
> > /* LOBF flags*/
> > bool lobf_supported;
> > + bool lobf_enabled;
> > };
> >
> > enum lspcon_vendor {
RE: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
> -Original Message-
> From: Hogander, Jouni
> Sent: Monday, April 15, 2024 3:36 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani
> Subject: Re: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
>
> On Fri, 2024-04-12 at 21:22 +0530, Animesh Manna wrote:
> > Link Off Between Active Frames, is a new feature for eDP that allows
> > the panel to go to lower power state after transmission of data. This
> > is a feature on top of ALPM, AS SDP.
> > Add compute config during atomic-check phase.
> >
> > v1: RFC version.
> > v2: Add separate flag for auxless-alpm. [Jani]
> >
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 44
> > +++
> > drivers/gpu/drm/i915/display/intel_alpm.h | 5 +++
> > .../drm/i915/display/intel_display_types.h | 4 ++
> > drivers/gpu/drm/i915/display/intel_dp.c | 5 +++
> > 4 files changed, 58 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index 13bac3e8c8fa..699f2f051766 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -11,6 +11,16 @@
> > #include "intel_dp_aux.h"
> > #include "intel_psr_regs.h"
> >
> > +bool intel_dp_get_aux_less_alpm_status(struct intel_dp *intel_dp) {
> > + u8 alpm_caps = 0;
> > +
> > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> > + &alpm_caps) != 1)
> > + return false;
> > + return alpm_caps & DP_ALPM_AUX_LESS_CAP; }
> > +
> > /*
> > * See Bspec: 71632 for the table
> > *
> > @@ -242,6 +252,40 @@ bool intel_alpm_compute_params(struct intel_dp
> > *intel_dp,
> > return true;
> > }
> >
> > +void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp,
> > + struct intel_crtc_state
> > *crtc_state,
> > + struct drm_connector_state
> > *conn_state)
> > +{
> > + struct drm_display_mode *adjusted_mode = &crtc_state-
> > >hw.adjusted_mode;
> > + int waketime_in_lines, first_sdp_position;
> > + int context_latency, guardband;
> > +
> > + intel_dp->lobf_supported = false;
> > +
> > + if (!intel_dp_is_edp(intel_dp))
> > + return;
> > +
> > + if (!intel_dp_as_sdp_supported(intel_dp))
> > + return;
> > +
> > + if (crtc_state->has_psr2 || crtc_state->has_panel_replay)
> > + return;
>
> LOBF is not supported with PSR1? I think checking crtc_state->has_psr is
> enough. That covers PSR1/2 and Panel Replay.
Ok.
>
> > +
> > + if (intel_alpm_compute_params(intel_dp, crtc_state)) {
> > + context_latency = adjusted_mode->crtc_vblank_start -
> > adjusted_mode->crtc_vdisplay;
> > + guardband = adjusted_mode->crtc_vtotal -
> > + adjusted_mode->crtc_vdisplay -
> > context_latency;
> > + first_sdp_position = adjusted_mode->crtc_vtotal -
> > adjusted_mode->crtc_vsync_start;
> > + if (intel_dp->alpm_parameters.auxless_alpm_supported)
> > + waketime_in_lines = intel_dp-
> > >alpm_parameters.io_wake_lines;
> > + else
> > + waketime_in_lines = intel_dp-
> > >alpm_parameters.fast_wake_lines;
> > +
> > + if ((context_latency + guardband) >
> > (first_sdp_position + waketime_in_lines))
> > + intel_dp->lobf_supported = true;
> > + }
>
> You are not checking display version here. This is supported only on LNL and
> onwards.
Sure will add, thought as-sdp-support will take care, but it has display_ver >=
13.
>
> > +}
> > +
> > static void lnl_alpm_configure(struct intel_dp *intel_dp)
> > {
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); diff
> > --git a/drivers/gpu/drm/i915/display/intel_alpm.h
> > b/drivers/gpu/drm/i915/display/intel_alpm.h
> > index c45d078e5a6b..c341d2c2b7f7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> > @@ -10,9 +10,14 @@
>
RE: [PATCH v9 1/6] drm/panelreplay: dpcd register definition for panelreplay
> -Original Message- > From: Nikula, Jani > Sent: Thursday, November 9, 2023 6:37 PM > To: Manna, Animesh ; intel- > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: Manna, Animesh ; Hogander, Jouni > ; Murthy, Arun R > Subject: Re: [PATCH v9 1/6] drm/panelreplay: dpcd register definition for > panelreplay > > On Wed, 08 Nov 2023, Animesh Manna wrote: > > Add DPCD register definition for discovering, enabling and checking > > status of panel replay of the sink. > > > > Cc: Jouni Högander > > Cc: Arun R Murthy > > Cc: Jani Nikula > > Reviewed-by: Arun R Murthy > > Signed-off-by: Animesh Manna > > You got the ack, please keep track of it. > > https://lore.kernel.org/r/elcebygxs432bcj7oez7ndlfvb3lru7m7yznyqp2ei4ocjk > vxp@23lf2rh45fmt Thanks Jani and everyone who helped in review. Pushed the initial 5 patches of this series. As 6th patch has dependency on igt changes, will push after igt changes get merged. Regards, Animesh > > > --- > > include/drm/display/drm_dp.h | 23 +++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/include/drm/display/drm_dp.h > > b/include/drm/display/drm_dp.h index e69cece404b3..fc42b622ef32 > 100644 > > --- a/include/drm/display/drm_dp.h > > +++ b/include/drm/display/drm_dp.h > > @@ -543,6 +543,10 @@ > > /* DFP Capability Extension */ > > #define DP_DFP_CAPABILITY_EXTENSION_SUPPORT0x0a3 /* 2.0 */ > > > > +#define DP_PANEL_REPLAY_CAP 0x0b0 /* DP 2.0 */ > > +# define DP_PANEL_REPLAY_SUPPORT(1 << 0) > > +# define DP_PANEL_REPLAY_SU_SUPPORT (1 << 1) > > + > > /* Link Configuration */ > > #defineDP_LINK_BW_SET 0x100 > > # define DP_LINK_RATE_TABLE0x00/* eDP 1.4 */ > > @@ -716,6 +720,13 @@ > > #define DP_BRANCH_DEVICE_CTRL 0x1a1 > > # define DP_BRANCH_DEVICE_IRQ_HPD (1 << 0) > > > > +#define PANEL_REPLAY_CONFIG 0x1b0 /* DP 2.0 */ > > +# define DP_PANEL_REPLAY_ENABLE (1 << 0) > > +# define DP_PANEL_REPLAY_UNRECOVERABLE_ERROR_EN (1 << 3) > > +# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN (1 << 4) > > +# define DP_PANEL_REPLAY_ACTIVE_FRAME_CRC_ERROR_EN (1 << 5) > > +# define DP_PANEL_REPLAY_SU_ENABLE (1 << 6) > > + > > #define DP_PAYLOAD_ALLOCATE_SET0x1c0 > > #define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1 #define > > DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2 @@ -1105,6 +1116,18 > @@ > > #define DP_LANE_ALIGN_STATUS_UPDATED_ESI 0x200e /* status same > as 0x204 */ > > #define DP_SINK_STATUS_ESI 0x200f /* status same as > > 0x205 */ > > > > +#define DP_PANEL_REPLAY_ERROR_STATUS 0x2020 /* DP 2.1*/ > > +# define DP_PANEL_REPLAY_LINK_CRC_ERROR(1 << 0) > > +# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR (1 << 1) > > +# define DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR (1 << 2) > > + > > +#define DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS0x2022 /* > DP 2.1 */ > > +# define DP_SINK_DEVICE_PANEL_REPLAY_STATUS_MASK (7 << 0) > > +# define DP_SINK_FRAME_LOCKED_SHIFT3 > > +# define DP_SINK_FRAME_LOCKED_MASK (3 << 3) > > +# define DP_SINK_FRAME_LOCKED_STATUS_VALID_SHIFT 5 > > +# define DP_SINK_FRAME_LOCKED_STATUS_VALID_MASK(1 << 5) > > + > > /* Extended Receiver Capability: See DP_DPCD_REV for definitions */ > > #define DP_DP13_DPCD_REV0x2200 > > -- > Jani Nikula, Intel
RE: [PATCH v8 6/6] drm/i915/panelreplay: Debugfs support for panel replay
> -Original Message-
> From: Hogander, Jouni
> Sent: Monday, November 6, 2023 1:33 PM
> To: dri-devel@lists.freedesktop.org; Manna, Animesh
> ; intel-...@lists.freedesktop.org
> Cc: Murthy, Arun R ; Nikula, Jani
>
> Subject: Re: [PATCH v8 6/6] drm/i915/panelreplay: Debugfs support for
> panel replay
>
> Hello Animesh,
>
> Thank you for the changes. Now the patch is much shorter, cleaner and
> easier to review. See my inline comments below.
>
> On Sat, 2023-11-04 at 02:30 +0530, Animesh Manna wrote:
> > Add debugfs support which will print source and sink status per
> > connector basis. Existing i915_psr_status and i915_psr_sink_status
> > will be used to get the source and sink status of panel replay.
> >
> > v1: Initial version. [rb-ed by Arun]
> > v2: Added check for DP 2.0 and connector type in
> > connector_debugfs_add().
> > v3: Optimization and cosmetic changes. [Jouni]
> >
> > Cc: Jouni Högander
> > Cc: Arun R Murthy
> > Cc: Jani Nikula
> > Reviewed-by: Arun R Murthy
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_psr.c | 93 +-
> > --
> > 1 file changed, 66 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 8ed4684b4528..8b7c03cd4989 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2813,12 +2813,19 @@ static int
> > psr_get_status_and_error_status(struct intel_dp *intel_dp,
> > {
> > struct drm_dp_aux *aux = &intel_dp->aux;
> > int ret;
> > + unsigned int offset;
> >
> > - ret = drm_dp_dpcd_readb(aux, DP_PSR_STATUS, status);
> > + offset = intel_dp->psr.panel_replay_enabled ?
> > + DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS :
> > DP_PSR_STATUS;
> > +
> > + ret = drm_dp_dpcd_readb(aux, offset, status);
> > if (ret != 1)
> > return ret;
> >
> > - ret = drm_dp_dpcd_readb(aux, DP_PSR_ERROR_STATUS,
> > error_status);
> > + offset = intel_dp->psr.panel_replay_enabled ?
> > + DP_PANEL_REPLAY_ERROR_STATUS : DP_PSR_ERROR_STATUS;
> > +
> > + ret = drm_dp_dpcd_readb(aux, offset, error_status);
> > if (ret != 1)
> > return ret;
> >
> > @@ -3039,7 +3046,7 @@ psr_source_status(struct intel_dp *intel_dp,
> > struct seq_file *m)
> > status = live_status[status_val];
> > }
> >
> > - seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> > val);
> > + seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n",
> > status, val);
> > }
> >
> > static int intel_psr_status(struct seq_file *m, struct intel_dp
> > *intel_dp)
> > @@ -3052,18 +3059,23 @@ static int intel_psr_status(struct seq_file
> > *m, struct intel_dp *intel_dp)
> > bool enabled;
> > u32 val;
> >
> > - seq_printf(m, "Sink support: %s", str_yes_no(psr-
> > >sink_support));
> > - if (psr->sink_support)
> > + seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> > + str_yes_no(psr->sink_support),
> > + str_yes_no(psr->sink_panel_replay_support));
> > +
> > + if (psr->sink_support || psr->sink_panel_replay_support)
> > seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
>
> The output will look like this:
>
> Sink support: PSR = yes, Panel Replay = yes, [0x01]
>
> I think more logical would be:
>
> Sink support: PSR = yes [0x01], Panel Replay = yes
>
>
> > seq_puts(m, "\n");
> >
> > - if (!psr->sink_support)
> > + if (!(psr->sink_support || psr->sink_panel_replay_support))
> > return 0;
> >
> > wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
> > mutex_lock(&psr->lock);
> >
> > - if (psr->enabled)
> > + if (psr->panel_replay_enabled)
> > + status = "Panel Replay Enabled";
> > + else if (psr->enabled)
> > status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1
> > enabled";
> > else
> > status = "disabled";
> &g
RE: [PATCH v7 1/6] drm/panelreplay: dpcd register definition for panelreplay
> -Original Message- > From: Nikula, Jani > Sent: Friday, November 3, 2023 2:55 PM > To: Manna, Animesh ; intel- > g...@lists.freedesktop.org; Maxime Ripard ; Thomas > Zimmermann ; Maarten Lankhorst > > Cc: dri-devel@lists.freedesktop.org; Manna, Animesh > ; Hogander, Jouni > ; Murthy, Arun R > Subject: Re: [PATCH v7 1/6] drm/panelreplay: dpcd register definition for > panelreplay > > On Wed, 11 Oct 2023, Animesh Manna wrote: > > Add DPCD register definition for discovering, enabling and checking > > status of panel replay of the sink. > > > > Cc: Jouni Högander > > Cc: Arun R Murthy > > Cc: Jani Nikula > > Reviewed-by: Arun R Murthy > > Signed-off-by: Animesh Manna > > Maarten, Maxime, Thomas - > > Ack for merging this via drm-intel-next? Ping! Regards, Animesh > > Thanks, > Jani. > > > --- > > include/drm/display/drm_dp.h | 23 +++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/include/drm/display/drm_dp.h > > b/include/drm/display/drm_dp.h index e69cece404b3..fc42b622ef32 > 100644 > > --- a/include/drm/display/drm_dp.h > > +++ b/include/drm/display/drm_dp.h > > @@ -543,6 +543,10 @@ > > /* DFP Capability Extension */ > > #define DP_DFP_CAPABILITY_EXTENSION_SUPPORT0x0a3 /* 2.0 */ > > > > +#define DP_PANEL_REPLAY_CAP 0x0b0 /* DP 2.0 */ > > +# define DP_PANEL_REPLAY_SUPPORT(1 << 0) > > +# define DP_PANEL_REPLAY_SU_SUPPORT (1 << 1) > > + > > /* Link Configuration */ > > #defineDP_LINK_BW_SET 0x100 > > # define DP_LINK_RATE_TABLE0x00/* eDP 1.4 */ > > @@ -716,6 +720,13 @@ > > #define DP_BRANCH_DEVICE_CTRL 0x1a1 > > # define DP_BRANCH_DEVICE_IRQ_HPD (1 << 0) > > > > +#define PANEL_REPLAY_CONFIG 0x1b0 /* DP 2.0 */ > > +# define DP_PANEL_REPLAY_ENABLE (1 << 0) > > +# define DP_PANEL_REPLAY_UNRECOVERABLE_ERROR_EN (1 << 3) > > +# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN (1 << 4) > > +# define DP_PANEL_REPLAY_ACTIVE_FRAME_CRC_ERROR_EN (1 << 5) > > +# define DP_PANEL_REPLAY_SU_ENABLE (1 << 6) > > + > > #define DP_PAYLOAD_ALLOCATE_SET0x1c0 > > #define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1 #define > > DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2 @@ -1105,6 +1116,18 > @@ > > #define DP_LANE_ALIGN_STATUS_UPDATED_ESI 0x200e /* status same > as 0x204 */ > > #define DP_SINK_STATUS_ESI 0x200f /* status same as > > 0x205 */ > > > > +#define DP_PANEL_REPLAY_ERROR_STATUS 0x2020 /* DP 2.1*/ > > +# define DP_PANEL_REPLAY_LINK_CRC_ERROR(1 << 0) > > +# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR (1 << 1) > > +# define DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR (1 << 2) > > + > > +#define DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS0x2022 /* > DP 2.1 */ > > +# define DP_SINK_DEVICE_PANEL_REPLAY_STATUS_MASK (7 << 0) > > +# define DP_SINK_FRAME_LOCKED_SHIFT3 > > +# define DP_SINK_FRAME_LOCKED_MASK (3 << 3) > > +# define DP_SINK_FRAME_LOCKED_STATUS_VALID_SHIFT 5 > > +# define DP_SINK_FRAME_LOCKED_STATUS_VALID_MASK(1 << 5) > > + > > /* Extended Receiver Capability: See DP_DPCD_REV for definitions */ > > #define DP_DP13_DPCD_REV0x2200 > > -- > Jani Nikula, Intel
RE: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay
> -Original Message-
> From: Hogander, Jouni
> Sent: Friday, November 3, 2023 12:32 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani
> Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for
> panel replay
>
> On Fri, 2023-11-03 at 06:10 +, Manna, Animesh wrote:
> >
> >
> > > -Original Message-
> > > From: Hogander, Jouni
> > > Sent: Thursday, November 2, 2023 1:08 PM
> > > To: Manna, Animesh ; intel-
> > > g...@lists.freedesktop.org
> > > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> > > ; Nikula, Jani
> > > Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support
> > > for panel replay
> > >
> > > On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> > > > Add debugfs support which will print source and sink status per
> > > > connector basis.
> > >
> > > Sorry for late review. Noticed only by now that you have added this
> > > patch into you set.
> >
> > Added from v5.
> >
> > >
> > > Can you please describe in commit message how you see the output of
> > > debugfs interface will look like after your changes?
> >
> > Sure.
> >
> > >
> > > >
> > > > v1: Initial version. [rb-ed by Arun]
> > > > v2: Added check for DP 2.0 and connector type in
> > > > connector_debugfs_add().
> > > >
> > > > Cc: Jouni Högander
> > > > Cc: Arun R Murthy
> > > > Cc: Jani Nikula
> > > > Signed-off-by: Animesh Manna
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_psr.c | 136
> > > > +
> > > > --
> > > > 1 file changed, 102 insertions(+), 34 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 80de831c2f60..399fc0a8e636 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -2823,6 +2823,25 @@ static int
> > > > psr_get_status_and_error_status(struct intel_dp *intel_dp,
> > > > return 0;
> > > > }
> > > >
> > > > +static int panel_replay_get_status_and_error_status(struct
> > > > intel_dp
> > > > *intel_dp,
> > > > + u8 *status,
> > > > u8
> > > > *error_status)
> > > > +{
> > > > + struct drm_dp_aux *aux = &intel_dp->aux;
> > > > + int ret;
> > > > +
> > > > + ret = drm_dp_dpcd_readb(aux,
> > > > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> > > > + if (ret != 1)
> > > > + return ret;
> > > > +
> > > > + ret = drm_dp_dpcd_readb(aux,
> > > > DP_PANEL_REPLAY_ERROR_STATUS,
> > > > error_status);
> > > > + if (ret != 1)
> > > > + return ret;
> > > > +
> > > > + *status = *status & DP_PSR_SINK_STATE_MASK;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > >
> > > I think you should modify psr_get_status_and_error_status instead
> > > of duplicating most of it.
> >
> > DPCD addresses are different for panel replay, I did not get the need
> > of it.
>
> I would like to see:
>
> unsigned int offset = intel_dp->psr.panel_replay_enabled ?
> DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS : DP_PSR_STATUS;
>
> ret = drm_dp_dpcd_readb(aux, offset, status);
>
> rather than duplicating it.
Added in v8.
>
> >
> > >
> > > > static void psr_alpm_check(struct intel_dp *intel_dp)
> > > > {
> > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > > @@
> > > > -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp,
> > > > struct seq_file *m)
> > > > status = live_status[status_val];
> > > > }
> > > >
> > > > - seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> > > > val);
> > > > + seq_printf(m, "Source PSR/PanelReplay status: %s
> > > >
RE: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay
> -Original Message-
> From: Hogander, Jouni
> Sent: Thursday, November 2, 2023 1:08 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani
> Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for
> panel replay
>
> On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> > Add debugfs support which will print source and sink status per
> > connector basis.
>
> Sorry for late review. Noticed only by now that you have added this patch
> into you set.
Added from v5.
>
> Can you please describe in commit message how you see the output of
> debugfs interface will look like after your changes?
Sure.
>
> >
> > v1: Initial version. [rb-ed by Arun]
> > v2: Added check for DP 2.0 and connector type in
> > connector_debugfs_add().
> >
> > Cc: Jouni Högander
> > Cc: Arun R Murthy
> > Cc: Jani Nikula
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_psr.c | 136 +
> > --
> > 1 file changed, 102 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 80de831c2f60..399fc0a8e636 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2823,6 +2823,25 @@ static int
> > psr_get_status_and_error_status(struct intel_dp *intel_dp,
> > return 0;
> > }
> >
> > +static int panel_replay_get_status_and_error_status(struct intel_dp
> > *intel_dp,
> > + u8 *status, u8
> > *error_status)
> > +{
> > + struct drm_dp_aux *aux = &intel_dp->aux;
> > + int ret;
> > +
> > + ret = drm_dp_dpcd_readb(aux,
> > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> > + if (ret != 1)
> > + return ret;
> > +
> > + ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS,
> > error_status);
> > + if (ret != 1)
> > + return ret;
> > +
> > + *status = *status & DP_PSR_SINK_STATE_MASK;
> > +
> > + return 0;
> > +}
> > +
>
> I think you should modify psr_get_status_and_error_status instead of
> duplicating most of it.
DPCD addresses are different for panel replay, I did not get the need of it.
>
> > static void psr_alpm_check(struct intel_dp *intel_dp)
> > {
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@
> > -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp, struct
> > seq_file *m)
> > status = live_status[status_val];
> > }
> >
> > - seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> > val);
> > + seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n",
> > status, val);
> > }
> >
> > static int intel_psr_status(struct seq_file *m, struct intel_dp
> > *intel_dp)
> > @@ -3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file
> > *m, struct intel_dp *intel_dp)
> > bool enabled;
> > u32 val;
> >
> > - seq_printf(m, "Sink support: %s", str_yes_no(psr-
> > >sink_support));
> > - if (psr->sink_support)
> > + seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> > + str_yes_no(psr->sink_support),
> > + str_yes_no(psr->sink_panel_replay_support));
> > +
> > + if (psr->sink_support || psr->sink_panel_replay_support)
> > seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
> > seq_puts(m, "\n");
> >
> > - if (!psr->sink_support)
> > + if (!(psr->sink_support || psr->sink_panel_replay_support))
> > return 0;
> >
> > wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
> > mutex_lock(&psr->lock);
> >
> > - if (psr->enabled)
> > + if (psr->panel_replay_enabled)
> > + status = "Panel Replay Enabled";
> > + else if (psr->enabled)
> > status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1
> > enabled";
> > else
> > status = "disabled";
> > @@ -3072,14 +3096,17 @@ static int intel_psr_stat
RE: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd initialization for DP
> -Original Message- > From: Murthy, Arun R > Sent: Monday, October 16, 2023 9:56 AM > To: Manna, Animesh ; intel- > g...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni > ; Nikula, Jani > Subject: RE: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd > initialization for DP > > > > -----Original Message- > > From: Manna, Animesh > > Sent: Wednesday, October 11, 2023 4:40 PM > > To: intel-...@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org; Manna, Animesh > > ; Hogander, Jouni > ; > > Murthy, Arun R ; Nikula, Jani > > > > Subject: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd > > initialization for DP > > > > Due to similarity panel replay dpcd initialization got added in psr > > function which is specific for edp panel. This patch enables panel > > replay initialization for dp connector. > > > If panelreplay initialization then why is the function name psr_init_dpcd() ? > Also it its similar to PSR then these dpcd should already be available. Hi Arun, The first call for intel_psr_init_dpcd() get called from intel_edp_init_dpcd() which is not reachable for DP. So, in this patch need to add intel_psr_init_dpcd() for DP(non-edp) in intel_psr_init(). Panel replay initialization added in intel_psr_init() as per previous feedback just to align panel-replay with psr framework. Regards, Animesh > > Thanks and Regards, > Arun R Murthy > > > > Cc: Jouni Högander > > Cc: Arun R Murthy > > Cc: Jani Nikula > > Signed-off-by: Animesh Manna > > --- > > drivers/gpu/drm/i915/display/intel_psr.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index f9837001aa5f..a2e0637c53fb 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -2738,6 +2738,9 @@ void intel_psr_init(struct intel_dp *intel_dp) > > if (!(HAS_PSR(dev_priv) || HAS_DP20(dev_priv))) > > return; > > > > + if (!intel_dp_is_edp(intel_dp)) > > + intel_psr_init_dpcd(intel_dp); > > + > > /* > > * HSW spec explicitly says PSR is tied to port A. > > * BDW+ platforms have a instance of PSR registers per transcoder > > but > > -- > > 2.29.0
RE: [PATCH v6 3/6] drm/i915/panelreplay: Initializaton and compute config for panel replay
> -Original Message-
> From: Murthy, Arun R
> Sent: Wednesday, October 4, 2023 4:13 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Nikula, Jani ;
> Hogander, Jouni
> Subject: RE: [PATCH v6 3/6] drm/i915/panelreplay: Initializaton and compute
> config for panel replay
>
>
>
> > -----Original Message-
> > From: Manna, Animesh
> > Sent: Thursday, September 21, 2023 11:44 AM
> > To: intel-...@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org; Nikula, Jani
> > ; Hogander, Jouni ;
> > Murthy, Arun R ; Manna, Animesh
> >
> > Subject: [PATCH v6 3/6] drm/i915/panelreplay: Initializaton and
> > compute config for panel replay
> >
> > Modify existing PSR implementation to enable panel replay feature of
> > DP 2.0 which is similar to PSR feature of EDP panel. There is
> > different DPCD address to check panel capability compare to PSR and vsc
> sdp header is different.
> >
> > v1: Initial version.
> > v2:
> > - Set source_panel_replay_support flag under HAS_PANEL_REPLAY()
> > condition check. [Jouni]
> > - Code restructured around intel_panel_replay_init and renamed to
> > intel_panel_replay_init_dpcd. [Jouni]
> > - Remove the initial code modification around has_psr2 flag. [Jouni]
> > - Add CAN_PANEL_REPLAY() in intel_encoder_can_psr which is used to
> > enable in intel_psr_post_plane_update. [Jouni]
> > v3:
> > - Initialize both psr and panel-replay. [Jouni]
> > - Initialize both panel replay and psr if detected. [Jouni]
> > - Refactoring psr function by introducing _psr_compute_config().
> > [Jouni]
> > - Add check for !is_edp while deriving source_panel_replay_support.
> > [Jouni]
> > - Enable panel replay dpcd initialization in a separate patch. [Jouni]
> >
> > v4:
> > - HAS_PANEL_REPLAY() check not needed during sink capability check.
> > [Jouni]
> > - Set either panel replay source support or psr. [Jouni]
> >
> > v5:
> > - HAS_PANEL_REPLAY() removed and use HAS_DP20() instead. [Jouni]
> > - Move psr related code to intel_psr.c. [Jani]
> > - Reset sink_panel_replay_support flag during disconnection. [Jani]
> >
> > v6: return statement restored which is removed by misatke. [Jouni]
> >
> > Cc: Jouni Högander
> > Signed-off-by: Animesh Manna
> > ---
> > .../drm/i915/display/intel_display_types.h| 14 +--
> > drivers/gpu/drm/i915/display/intel_dp.c | 50 --
> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +
> > drivers/gpu/drm/i915/display/intel_psr.c | 95 ++-
> > drivers/gpu/drm/i915/display/intel_psr.h | 7 ++
> > 5 files changed, 123 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 2213ad6c00da..3eadd8e12f63 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1203,6 +1203,7 @@ struct intel_crtc_state {
> > bool has_psr2;
> > bool enable_psr2_sel_fetch;
> > bool req_psr2_sdp_prior_scanline;
> > + bool has_panel_replay;
> > bool wm_level_disabled;
> > u32 dc3co_exitline;
> > u16 su_y_granularity;
> > @@ -1695,6 +1696,8 @@ struct intel_psr {
> > bool irq_aux_error;
> > u16 su_w_granularity;
> > u16 su_y_granularity;
> > + bool source_panel_replay_support;
> > + bool sink_panel_replay_support;
> > u32 dc3co_exitline;
> > u32 dc3co_exit_delay;
> > struct delayed_work dc3co_work;
> > @@ -1982,17 +1985,6 @@ dp_to_lspcon(struct intel_dp *intel_dp)
> >
> > #define dp_to_i915(__intel_dp) to_i915(dp_to_dig_port(__intel_dp)-
> > >base.base.dev)
> >
> > -#define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
> > - (intel_dp)->psr.source_support)
> > -
> > -static inline bool intel_encoder_can_psr(struct intel_encoder *encoder) -{
> > - if (!intel_encoder_is_dp(encoder))
> > - return false;
> > -
> > - return CAN_PSR(enc_to_intel_dp(encoder));
> > -}
> > -
> > static inline struct intel_digital_port * hdmi_to_dig_port(struct
> > intel_hdmi
> > *intel_hdmi) { diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index f16d9fa88fe1..1c7d9b14fd1c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>
RE: [PATCH v6 5/6] drm/i915/panelreplay: enable/disable panel replay
Hi,
> -Original Message-
> From: Murthy, Arun R
> Sent: Wednesday, October 4, 2023 4:01 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Nikula, Jani ;
> Hogander, Jouni
> Subject: RE: [PATCH v6 5/6] drm/i915/panelreplay: enable/disable panel
> replay
>
>
>
> > -----Original Message-
> > From: Manna, Animesh
> > Sent: Thursday, September 21, 2023 11:44 AM
> > To: intel-...@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org; Nikula, Jani
> > ; Hogander, Jouni ;
> > Murthy, Arun R ; Manna, Animesh
> >
> > Subject: [PATCH v6 5/6] drm/i915/panelreplay: enable/disable panel
> > replay
> >
> > TRANS_DP2_CTL register is programmed to enable panel replay from
> > source and sink is enabled through panel replay dpcd configuration
> address.
> >
> > Bspec: 1407940617
> >
> > v1: Initial version.
> > v2:
> > - Use pr_* flags instead psr_* flags. [Jouni]
> > - Remove intel_dp_is_edp check as edp1.5 also has panel replay.
> > [Jouni]
> >
> > v3: Cover letter updated and selective fetch condition check is added
> > before updating its bit in PSR2_MAN_TRK_CTL register. [Jouni]
> >
> > v4: Selective fetch related PSR2_MAN_TRK_CTL programmming dropped.
> > [Jouni]
> >
> > v5: Added PSR2_MAN_TRK_CTL programming as needed for Continuous
> Full
> > Frame (CFF) update.
> >
> > Note: Initial plan is to enable panel replay in full-screen live
> > active frame update mode. In a incremental approach panel replay will
> > be enabled in selctive update mode if there is any gap in curent
> implementation.
> >
> > Cc: Jouni Högander
> > Signed-off-by: Animesh Manna
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 7 +-
> > .../drm/i915/display/intel_display_types.h| 1 +
> > drivers/gpu/drm/i915/display/intel_psr.c | 65 ++-
> > 3 files changed, 56 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 4668de45d6fe..203c56c5e208 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -2717,10 +2717,15 @@ static void intel_ddi_pre_enable_dp(struct
> > intel_atomic_state *state,
> > const struct drm_connector_state
> > *conn_state) {
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > + struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >
> > - if (HAS_DP20(dev_priv))
> > + if (HAS_DP20(dev_priv)) {
> > intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
> > crtc_state);
> > + if (crtc_state->has_panel_replay)
> > + drm_dp_dpcd_writeb(&intel_dp->aux,
> > PANEL_REPLAY_CONFIG,
> > + DP_PANEL_REPLAY_ENABLE);
> > + }
> >
> > if (DISPLAY_VER(dev_priv) >= 14)
> > mtl_ddi_pre_enable_dp(state, encoder, crtc_state,
> conn_state); diff
> > --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 3eadd8e12f63..1cf302e9deed 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1698,6 +1698,7 @@ struct intel_psr {
> > u16 su_y_granularity;
> > bool source_panel_replay_support;
> > bool sink_panel_replay_support;
> > + bool panel_replay_enabled;
> > u32 dc3co_exitline;
> > u32 dc3co_exit_delay;
> > struct delayed_work dc3co_work;
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index a59f13c29c3d..67337b4a421b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -607,8 +607,11 @@ static void intel_psr_enable_sink(struct intel_dp
> > *intel_dp)
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > u8 dpcd_val = DP_PSR_ENABLE;
> >
> > - /* Enable ALPM at sink for psr2 */
> > + if (intel_dp->psr.panel_replay_enabled)
> > + return;
> > +
> > if (intel_dp->psr.psr2_enabled) {
> > + /* Enable ALPM at sink for psr2 */
> > drm_dp_dpcd_writeb(&intel_dp->aux,
> > DP_RECEIVE
RE: [PATCH v3 3/5] drm/i915/panelreplay: Initializaton and compute config for panel replay
> -Original Message-
> From: Souza, Jose
> Sent: Tuesday, January 4, 2022 9:25 PM
> To: dri-devel@lists.freedesktop.org; Manna, Animesh
> ; intel-...@lists.freedesktop.org
> Cc: Mun, Gwan-gyeong ; Nikula, Jani
> ; Kahola, Mika ; Navare,
> Manasi D
> Subject: Re: [PATCH v3 3/5] drm/i915/panelreplay: Initializaton and compute
> config for panel replay
>
> On Tue, 2022-01-04 at 21:21 +0530, Manna, Animesh wrote:
> > Hi,
> >
> > > -Original Message-
> > > From: Souza, Jose
> > > Sent: Wednesday, November 24, 2021 1:19 AM
> > > To: dri-devel@lists.freedesktop.org; Manna, Animesh
> > > ; intel-...@lists.freedesktop.org
> > > Cc: Mun, Gwan-gyeong ; Nikula, Jani
> > > ; Kahola, Mika ;
> > > Navare, Manasi D
> > > Subject: Re: [PATCH v3 3/5] drm/i915/panelreplay: Initializaton and
> > > compute config for panel replay
> > >
> > > On Sun, 2021-10-10 at 17:40 +0530, Animesh Manna wrote:
> > > > As panel replay feature similar to PSR feature of EDP panel, so
> > > > currently utilized existing psr framework for panel replay.
> > > >
> > > > v1: RFC version.
> > > > v2: optimized code, pr_enabled and pr_dpcd variable removed.
> > > > [Jose]
> > > > v3:
> > > > - code comments improved. [Jani]
> > > > - dpcd_readb used instead of dpcd_read. [Jani]
> > > > - panel-repaplay init/compute functions moved inside respective
> > > > psr function. [Jani]
> > > >
> > > > Signed-off-by: Animesh Manna
> > > > ---
> > > > .../drm/i915/display/intel_display_types.h| 2 +
> > > > drivers/gpu/drm/i915/display/intel_dp.c | 43 +
> > > > drivers/gpu/drm/i915/display/intel_psr.c | 48 +++
> > > > drivers/gpu/drm/i915/display/intel_psr.h | 3 ++
> > > > 4 files changed, 87 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > index 39e11eaec1a3..48f7d676ed2c 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > > @@ -1070,6 +1070,7 @@ struct intel_crtc_state {
> > > > bool req_psr2_sdp_prior_scanline;
> > > > u32 dc3co_exitline;
> > > > u16 su_y_granularity;
> > > > + bool has_panel_replay;
> > >
> > > We can drop this and reuse current ones ones, see bellow.
> > >
> > > > struct drm_dp_vsc_sdp psr_vsc;
> > > >
> > > > /*
> > > > @@ -1531,6 +1532,7 @@ struct intel_psr {
> > > > bool irq_aux_error;
> > > > u16 su_w_granularity;
> > > > u16 su_y_granularity;
> > > > + bool sink_panel_replay_support;
> > >
> > > move this closer to has_psr and set both when it is panel replay.
> > > otherwise psr functions will not be executed for panel replay, see
> > > CAN_PSR().
> >
> > AFIU Panel replay do not have any dependency with PSR.
> > So that’s why I have created separate function for panel replay which is
> > doing
> similar thing whatever needed for panel replay.
> > For example intel_panel_replay_compute_config() can be independent of
> intel_psr_compute_config().
> > Do you see any dependency with PSR for panel replay?
>
> There is no dependency but panel replay is PSR for DP so we should re-use PSR
> code as much as possible.
>
> eDP + sink_support = PSR
> DP + sink_support = panel replay
>
> So we can reuse has_psr and all other stuff.
Not sure what to reuse from psr other than selective update part which will not
enable on dg2 for panel replay.
Will check further on this.
Regards,
Animesh
>
> >
> > >
> > > > u32 dc3co_exitline;
> > > > u32 dc3co_exit_delay;
> > > > struct delayed_work dc3co_work;
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > index 10fda20a5bd8..f58a7b72be14 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > @@ -1587,12 +1587,22 @@ static void
> > > intel_dp_compute_vsc_colorimetry(const struct intel_
RE: [PATCH v3 3/5] drm/i915/panelreplay: Initializaton and compute config for panel replay
Hi,
> -Original Message-
> From: Souza, Jose
> Sent: Wednesday, November 24, 2021 1:19 AM
> To: dri-devel@lists.freedesktop.org; Manna, Animesh
> ; intel-...@lists.freedesktop.org
> Cc: Mun, Gwan-gyeong ; Nikula, Jani
> ; Kahola, Mika ; Navare,
> Manasi D
> Subject: Re: [PATCH v3 3/5] drm/i915/panelreplay: Initializaton and compute
> config for panel replay
>
> On Sun, 2021-10-10 at 17:40 +0530, Animesh Manna wrote:
> > As panel replay feature similar to PSR feature of EDP panel, so
> > currently utilized existing psr framework for panel replay.
> >
> > v1: RFC version.
> > v2: optimized code, pr_enabled and pr_dpcd variable removed. [Jose]
> > v3:
> > - code comments improved. [Jani]
> > - dpcd_readb used instead of dpcd_read. [Jani]
> > - panel-repaplay init/compute functions moved inside respective psr
> > function. [Jani]
> >
> > Signed-off-by: Animesh Manna
> > ---
> > .../drm/i915/display/intel_display_types.h| 2 +
> > drivers/gpu/drm/i915/display/intel_dp.c | 43 +
> > drivers/gpu/drm/i915/display/intel_psr.c | 48 +++
> > drivers/gpu/drm/i915/display/intel_psr.h | 3 ++
> > 4 files changed, 87 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 39e11eaec1a3..48f7d676ed2c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1070,6 +1070,7 @@ struct intel_crtc_state {
> > bool req_psr2_sdp_prior_scanline;
> > u32 dc3co_exitline;
> > u16 su_y_granularity;
> > + bool has_panel_replay;
>
> We can drop this and reuse current ones ones, see bellow.
>
> > struct drm_dp_vsc_sdp psr_vsc;
> >
> > /*
> > @@ -1531,6 +1532,7 @@ struct intel_psr {
> > bool irq_aux_error;
> > u16 su_w_granularity;
> > u16 su_y_granularity;
> > + bool sink_panel_replay_support;
>
> move this closer to has_psr and set both when it is panel replay.
> otherwise psr functions will not be executed for panel replay, see CAN_PSR().
AFIU Panel replay do not have any dependency with PSR.
So that’s why I have created separate function for panel replay which is doing
similar thing whatever needed for panel replay.
For example intel_panel_replay_compute_config() can be independent of
intel_psr_compute_config().
Do you see any dependency with PSR for panel replay?
>
> > u32 dc3co_exitline;
> > u32 dc3co_exit_delay;
> > struct delayed_work dc3co_work;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 10fda20a5bd8..f58a7b72be14 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1587,12 +1587,22 @@ static void
> intel_dp_compute_vsc_colorimetry(const struct intel_crtc_state *crtc
> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >
> > - /*
> > -* Prepare VSC Header for SU as per DP 1.4 spec, Table 2-118
> > -* VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
> > -* Colorimetry Format indication.
> > -*/
> > - vsc->revision = 0x5;
> > + if (crtc_state->has_panel_replay) {
> > + /*
> > +* Prepare VSC Header for SU as per DP 2.0 spec, Table 2-223
> > +* VSC SDP supporting 3D stereo, Panel Replay, and Pixel
> > +* Encoding/Colorimetry Format indication.
> > +*/
> > + vsc->revision = 0x7;
> > + } else {
> > + /*
> > +* Prepare VSC Header for SU as per DP 1.4 spec, Table 2-118
> > +* VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
> > +* Colorimetry Format indication.
> > +*/
> > + vsc->revision = 0x5;
> > + }
> > +
> > vsc->length = 0x13;
> >
> > /* DP 1.4a spec, Table 2-120 */
> > @@ -1701,6 +1711,21 @@ void intel_dp_compute_psr_vsc_sdp(struct
> intel_dp *intel_dp,
> > vsc->revision = 0x4;
> > vsc->length = 0xe;
> > }
> > + } else if (intel_dp->psr.enabled && !intel_dp_is_edp(intel_dp)) {
> > + if (intel_dp->psr.colorimetry_support &&
> > + intel_dp_needs
RE: [PATCH v3 1/5] drm/i915/panelreplay: dpcd register definition for panelreplay
> -Original Message- > From: Souza, Jose > Sent: Wednesday, November 24, 2021 1:07 AM > To: dri-devel@lists.freedesktop.org; Manna, Animesh > ; intel-...@lists.freedesktop.org > Cc: Mun, Gwan-gyeong ; Nikula, Jani > ; Kahola, Mika ; Navare, > Manasi D > Subject: Re: [PATCH v3 1/5] drm/i915/panelreplay: dpcd register definition for > panelreplay > > On Sun, 2021-10-10 at 17:40 +0530, Animesh Manna wrote: > > DPCD register definition added to check and enable panel replay > > capability of the sink. > > > > Signed-off-by: Animesh Manna > > --- > > include/drm/drm_dp_helper.h | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > > index b52df4db3e8f..8a2b929c3f88 100644 > > --- a/include/drm/drm_dp_helper.h > > +++ b/include/drm/drm_dp_helper.h > > @@ -541,6 +541,9 @@ struct drm_panel; > > /* DFP Capability Extension */ > > #define DP_DFP_CAPABILITY_EXTENSION_SUPPORT0x0a3 /* 2.0 */ > > > > +#define DP_PANEL_REPLAY_CAP 0x0b0 > > +# define PANEL_REPLAY_SUPPORT (1 << 0) > > Missing bit 1, that is very important when panel do not support selective > update > panel replay needs to act like PSR1 when it is sets it needs to act like PSR2. > > > + > > /* Link Configuration */ > > #defineDP_LINK_BW_SET 0x100 > > # define DP_LINK_RATE_TABLE0x00/* eDP 1.4 */ > > @@ -709,6 +712,9 @@ struct drm_panel; > > #define DP_BRANCH_DEVICE_CTRL 0x1a1 > > # define DP_BRANCH_DEVICE_IRQ_HPD (1 << 0) > > > > +#define PANEL_REPLAY_CONFIG 0x1b0 > > +# define PANEL_REPLAY_ENABLE(1 << 0) > > All other bits are also important, for the errors ones we have PSR counter > parts > and your are missing the error status register. Thanks for review. Added the suggested changes in current version. Regards, Animesh > > > + > > #define DP_PAYLOAD_ALLOCATE_SET0x1c0 > > #define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1 #define > > DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2
Re: [PATCH v3 3/9] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation
On 04-01-2020 05:18, Manasi Navare wrote:
On Thu, Jan 02, 2020 at 03:56:09PM +0530, Manna, Animesh wrote:
On 02-01-2020 14:48, Jani Nikula wrote:
On Mon, 30 Dec 2019, Animesh Manna wrote:
vswing/pre-emphasis adjustment calculation is needed in processing
of auto phy compliance request other than link training, so moved
the same function in intel_dp.c.
I guess I'm still asking why you think this is better located in
intel_dp.c than intel_dp_link_training.c, as the function has been moved
once in the other direction already to split out stuff from intel_dp.c
and to make the file smaller. Even the file name suggests it should
really be in intel_dp_link_training.c, right?
Just a thought, can we change the name to "intel_dp_link_config.c" from
"intel_dp_link_training.c" which will provide little wider scope
and all the function playing with link configuration can be under it and also
exposed through header file.
AFAIK, processing phy compliance request always do not need link training. I
understood link training is very specific process consisting of clock recovery
+ channel eq.
So I am afraid of exposing intel_get_adjust_train() from
intel_dp_link_training.c which is not only specific to link-training. Need your
suggestion.
Regards,
Animesh
I agree with Jani here and I think I had even suggested this earlier that
instead of moving this function to intel_dp.c
we should make it non static so it can be used even for PHY compliance but
since this function still deals
with adjusting training patterns IMHO it should still stay in
intel_dp_link_training.c
Manasi
Sure Manasi, I will make intel_get_adjust_train() non-static and keep in
intel_dp_link_training.c.
Now as suggested by Jani before
(https://patchwork.freedesktop.org/patch/345823/?series=71121&rev=1#comment_640087)
other non static functions are not following the rule,
should I change the function name or keep it as it is?
Regards,
Animesh
BR,
Jani.
No functional change.
v1: initial patch.
v2:
- used "intel_dp" prefix in function name. (Jani)
- used array notation instead pointer for link_status. (Ville)
Signed-off-by: Animesh Manna
---
drivers/gpu/drm/i915/display/intel_dp.c | 34 ++
drivers/gpu/drm/i915/display/intel_dp.h | 4 +++
.../drm/i915/display/intel_dp_link_training.c | 36 ++-
3 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 991f343579ef..2a27ee106089 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4110,6 +4110,40 @@ ivb_cpu_edp_signal_levels(u8 train_set)
}
}
+void
+intel_dp_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 link_status[DP_LINK_STATUS_SIZE])
+{
+ u8 v = 0;
+ u8 p = 0;
+ int lane;
+ u8 voltage_max;
+ u8 preemph_max;
+
+ for (lane = 0; lane < intel_dp->lane_count; lane++) {
+ u8 this_v = drm_dp_get_adjust_request_voltage(link_status,
+ lane);
+ u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status,
+ lane);
+
+ if (this_v > v)
+ v = this_v;
+ if (this_p > p)
+ p = this_p;
+ }
+
+ voltage_max = intel_dp_voltage_max(intel_dp);
+ if (v >= voltage_max)
+ v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
+
+ preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
+ if (p >= preemph_max)
+ p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
+
+ for (lane = 0; lane < 4; lane++)
+ intel_dp->train_set[lane] = v | p;
+}
+
void
intel_dp_set_signal_levels(struct intel_dp *intel_dp)
{
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
b/drivers/gpu/drm/i915/display/intel_dp.h
index 3da166054788..83eadc87af26 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -9,6 +9,7 @@
#include
#include
+#include
#include "i915_reg.h"
@@ -91,6 +92,9 @@ void
intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
u8 dp_train_pat);
void
+intel_dp_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 link_status[DP_LINK_STATUS_SIZE]);
+void
intel_dp_set_signal_levels(struct intel_dp *intel_dp);
void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
u8
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 2a1130dd1ad0..e8ff9e279800 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel
Re: [PATCH v3 3/9] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation
On 02-01-2020 14:48, Jani Nikula wrote:
On Mon, 30 Dec 2019, Animesh Manna wrote:
vswing/pre-emphasis adjustment calculation is needed in processing
of auto phy compliance request other than link training, so moved
the same function in intel_dp.c.
I guess I'm still asking why you think this is better located in
intel_dp.c than intel_dp_link_training.c, as the function has been moved
once in the other direction already to split out stuff from intel_dp.c
and to make the file smaller. Even the file name suggests it should
really be in intel_dp_link_training.c, right?
Just a thought, can we change the name to "intel_dp_link_config.c" from
"intel_dp_link_training.c" which will provide little wider scope
and all the function playing with link configuration can be under it and also
exposed through header file.
AFAIK, processing phy compliance request always do not need link training. I
understood link training is very specific process consisting of clock recovery
+ channel eq.
So I am afraid of exposing intel_get_adjust_train() from
intel_dp_link_training.c which is not only specific to link-training. Need your
suggestion.
Regards,
Animesh
BR,
Jani.
No functional change.
v1: initial patch.
v2:
- used "intel_dp" prefix in function name. (Jani)
- used array notation instead pointer for link_status. (Ville)
Signed-off-by: Animesh Manna
---
drivers/gpu/drm/i915/display/intel_dp.c | 34 ++
drivers/gpu/drm/i915/display/intel_dp.h | 4 +++
.../drm/i915/display/intel_dp_link_training.c | 36 ++-
3 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 991f343579ef..2a27ee106089 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4110,6 +4110,40 @@ ivb_cpu_edp_signal_levels(u8 train_set)
}
}
+void
+intel_dp_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 link_status[DP_LINK_STATUS_SIZE])
+{
+ u8 v = 0;
+ u8 p = 0;
+ int lane;
+ u8 voltage_max;
+ u8 preemph_max;
+
+ for (lane = 0; lane < intel_dp->lane_count; lane++) {
+ u8 this_v = drm_dp_get_adjust_request_voltage(link_status,
+ lane);
+ u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status,
+ lane);
+
+ if (this_v > v)
+ v = this_v;
+ if (this_p > p)
+ p = this_p;
+ }
+
+ voltage_max = intel_dp_voltage_max(intel_dp);
+ if (v >= voltage_max)
+ v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
+
+ preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
+ if (p >= preemph_max)
+ p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
+
+ for (lane = 0; lane < 4; lane++)
+ intel_dp->train_set[lane] = v | p;
+}
+
void
intel_dp_set_signal_levels(struct intel_dp *intel_dp)
{
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
b/drivers/gpu/drm/i915/display/intel_dp.h
index 3da166054788..83eadc87af26 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -9,6 +9,7 @@
#include
#include
+#include
#include "i915_reg.h"
@@ -91,6 +92,9 @@ void
intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
u8 dp_train_pat);
void
+intel_dp_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 link_status[DP_LINK_STATUS_SIZE]);
+void
intel_dp_set_signal_levels(struct intel_dp *intel_dp);
void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
u8
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 2a1130dd1ad0..e8ff9e279800 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -34,38 +34,6 @@ intel_dp_dump_link_status(const u8
link_status[DP_LINK_STATUS_SIZE])
link_status[3], link_status[4], link_status[5]);
}
-static void
-intel_get_adjust_train(struct intel_dp *intel_dp,
- const u8 link_status[DP_LINK_STATUS_SIZE])
-{
- u8 v = 0;
- u8 p = 0;
- int lane;
- u8 voltage_max;
- u8 preemph_max;
-
- for (lane = 0; lane < intel_dp->lane_count; lane++) {
- u8 this_v = drm_dp_get_adjust_request_voltage(link_status,
lane);
- u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status,
lane);
-
- if (this_v > v)
- v = this_v;
- if (this_p > p)
- p = this_p;
- }
-
- voltage_max = intel_dp_voltage_m
Re: [PATCH v3 2/9] drm/dp: get/set phy compliance pattern
On 30-12-2019 21:41, Harry Wentland wrote:
On 2019-12-30 11:05 a.m., Manna, Animesh wrote:
On 24-12-2019 01:23, Harry Wentland wrote:
On 2019-12-23 12:03 p.m., Animesh Manna wrote:
During phy compliance auto test mode source need to read
requested test pattern from sink through DPCD. After processing
the request source need to set the pattern. So set/get method
added in drm layer as it is DP protocol.
v2: As per review feedback from Manasi on RFC version,
- added dp revision as function argument in set_phy_pattern api.
- used int for link_rate and u8 for lane_count to align with
existing code.
Signed-off-by: Animesh Manna
---
drivers/gpu/drm/drm_dp_helper.c | 93
+
include/drm/drm_dp_helper.h | 31 +++
2 files changed, 124 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c
b/drivers/gpu/drm/drm_dp_helper.c
index 2c7870aef469..91c80973aa83 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1371,3 +1371,96 @@ int
drm_dp_dsc_sink_supported_input_bpcs(const u8
dsc_dpcd[DP_DSC_RECEIVER_CAP_S
return num_bpc;
}
EXPORT_SYMBOL(drm_dp_dsc_sink_supported_input_bpcs);
+
+/**
+ * drm_dp_get_phy_test_pattern() - get the requested pattern from
the sink.
+ * @aux: DisplayPort AUX channel
+ * @data: DP phy compliance test parameters.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_get_phy_test_pattern(struct drm_dp_aux *aux,
+ struct drm_dp_phy_test_params *data)
+{
+ int err;
+ u8 rate, lanes;
+
+ err = drm_dp_dpcd_readb(aux, DP_TEST_LINK_RATE, &rate);
+ if (err < 0)
+ return err;
+ data->link_rate = drm_dp_bw_code_to_link_rate(rate);
+
+ err = drm_dp_dpcd_readb(aux, DP_TEST_LANE_COUNT, &lanes);
+ if (err < 0)
+ return err;
+ data->num_lanes = lanes & DP_MAX_LANE_COUNT_MASK;
+
+ if (lanes & DP_ENHANCED_FRAME_CAP)
+ data->enahanced_frame_cap = true;
+
+ err = drm_dp_dpcd_readb(aux, DP_PHY_TEST_PATTERN,
&data->phy_pattern);
+ if (err < 0)
+ return err;
+
+ switch (data->phy_pattern) {
+ case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
+ err = drm_dp_dpcd_read(aux, DP_TEST_80BIT_CUSTOM_PATTERN_7_0,
+ &data->custom80, 10);
Using sizeof(data->custom80) might be safer.
+ if (err < 0)
+ return err;
+
+ break;
+ case DP_PHY_TEST_PATTERN_CP2520:
+ err = drm_dp_dpcd_read(aux, DP_TEST_HBR2_SCRAMBLER_RESET,
+ &data->hbr2_reset, 2);
Same here, using sizeof(data->hbr2_reset).
+ if (err < 0)
+ return err;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_get_phy_test_pattern);
+
+/**
+ * drm_dp_set_phy_test_pattern() - set the pattern to the sink.
+ * @aux: DisplayPort AUX channel
+ * @data: DP phy compliance test parameters.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_set_phy_test_pattern(struct drm_dp_aux *aux,
+ struct drm_dp_phy_test_params *data, u8 dp_rev)
+{
+ int err, i;
+ u8 link_config[2];
+ u8 test_pattern;
+
+ link_config[0] = drm_dp_link_rate_to_bw_code(data->link_rate);
+ link_config[1] = data->num_lanes;
+ if (data->enahanced_frame_cap)
+ link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
+ err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, link_config, 2);
+ if (err < 0)
+ return err;
+
+ test_pattern = data->phy_pattern;
+ if (dp_rev < 0x12) {
+ test_pattern = (test_pattern << 2) &
+ DP_LINK_QUAL_PATTERN_11_MASK;
+ err = drm_dp_dpcd_writeb(aux, DP_TRAINING_PATTERN_SET,
+ test_pattern);
+ if (err < 0)
+ return err;
+ } else {
+ for (i = 0; i < data->num_lanes; i++) {
+ err = drm_dp_dpcd_writeb(aux,
+ DP_LINK_QUAL_LANE0_SET + i,
+ test_pattern);
+ if (err < 0)
+ return err;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_set_phy_test_pattern);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index d6e560870fb1..42a364748308 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -700,6 +700,15 @@
# define DP_TEST_COUNT_MASK 0xf
#define DP_PHY_TEST_PATTERN 0x248
+# define DP_PHY_TEST_PATTERN_SEL_MASK 0x7
+# define DP_PHY_TEST_PATTERN_NONE 0x0
+# define DP_PHY_TEST_PATTERN_D10_2 0x1
+# define DP_PHY_TEST_PATTERN_ERROR_COUNT 0x2
+# define DP_PHY_TEST_PATTERN_PRBS7 0x3
+# define DP_PHY_TEST_PATTERN_80BIT_CUSTOM 0x4
+# define DP_PHY_TEST_PATTERN_CP2520 0x5
+
+#define DP_TEST_HBR2_SCRAMBLER_RESET 0x24A
#define DP_TEST_80BIT_CUSTOM_PATTERN_7_0 0x250
#define DP_TEST_80BIT_CUSTOM_PAT
Re: [PATCH v3 2/9] drm/dp: get/set phy compliance pattern
On 24-12-2019 01:23, Harry Wentland wrote:
On 2019-12-23 12:03 p.m., Animesh Manna wrote:
During phy compliance auto test mode source need to read
requested test pattern from sink through DPCD. After processing
the request source need to set the pattern. So set/get method
added in drm layer as it is DP protocol.
v2: As per review feedback from Manasi on RFC version,
- added dp revision as function argument in set_phy_pattern api.
- used int for link_rate and u8 for lane_count to align with existing code.
Signed-off-by: Animesh Manna
---
drivers/gpu/drm/drm_dp_helper.c | 93 +
include/drm/drm_dp_helper.h | 31 +++
2 files changed, 124 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 2c7870aef469..91c80973aa83 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1371,3 +1371,96 @@ int drm_dp_dsc_sink_supported_input_bpcs(const u8
dsc_dpcd[DP_DSC_RECEIVER_CAP_S
return num_bpc;
}
EXPORT_SYMBOL(drm_dp_dsc_sink_supported_input_bpcs);
+
+/**
+ * drm_dp_get_phy_test_pattern() - get the requested pattern from the sink.
+ * @aux: DisplayPort AUX channel
+ * @data: DP phy compliance test parameters.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_get_phy_test_pattern(struct drm_dp_aux *aux,
+ struct drm_dp_phy_test_params *data)
+{
+ int err;
+ u8 rate, lanes;
+
+ err = drm_dp_dpcd_readb(aux, DP_TEST_LINK_RATE, &rate);
+ if (err < 0)
+ return err;
+ data->link_rate = drm_dp_bw_code_to_link_rate(rate);
+
+ err = drm_dp_dpcd_readb(aux, DP_TEST_LANE_COUNT, &lanes);
+ if (err < 0)
+ return err;
+ data->num_lanes = lanes & DP_MAX_LANE_COUNT_MASK;
+
+ if (lanes & DP_ENHANCED_FRAME_CAP)
+ data->enahanced_frame_cap = true;
+
+ err = drm_dp_dpcd_readb(aux, DP_PHY_TEST_PATTERN, &data->phy_pattern);
+ if (err < 0)
+ return err;
+
+ switch (data->phy_pattern) {
+ case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
+ err = drm_dp_dpcd_read(aux, DP_TEST_80BIT_CUSTOM_PATTERN_7_0,
+ &data->custom80, 10);
Using sizeof(data->custom80) might be safer.
+ if (err < 0)
+ return err;
+
+ break;
+ case DP_PHY_TEST_PATTERN_CP2520:
+ err = drm_dp_dpcd_read(aux, DP_TEST_HBR2_SCRAMBLER_RESET,
+ &data->hbr2_reset, 2);
Same here, using sizeof(data->hbr2_reset).
+ if (err < 0)
+ return err;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_get_phy_test_pattern);
+
+/**
+ * drm_dp_set_phy_test_pattern() - set the pattern to the sink.
+ * @aux: DisplayPort AUX channel
+ * @data: DP phy compliance test parameters.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_set_phy_test_pattern(struct drm_dp_aux *aux,
+ struct drm_dp_phy_test_params *data, u8 dp_rev)
+{
+ int err, i;
+ u8 link_config[2];
+ u8 test_pattern;
+
+ link_config[0] = drm_dp_link_rate_to_bw_code(data->link_rate);
+ link_config[1] = data->num_lanes;
+ if (data->enahanced_frame_cap)
+ link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
+ err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, link_config, 2);
+ if (err < 0)
+ return err;
+
+ test_pattern = data->phy_pattern;
+ if (dp_rev < 0x12) {
+ test_pattern = (test_pattern << 2) &
+ DP_LINK_QUAL_PATTERN_11_MASK;
+ err = drm_dp_dpcd_writeb(aux, DP_TRAINING_PATTERN_SET,
+test_pattern);
+ if (err < 0)
+ return err;
+ } else {
+ for (i = 0; i < data->num_lanes; i++) {
+ err = drm_dp_dpcd_writeb(aux,
+DP_LINK_QUAL_LANE0_SET + i,
+test_pattern);
+ if (err < 0)
+ return err;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_set_phy_test_pattern);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index d6e560870fb1..42a364748308 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -700,6 +700,15 @@
# define DP_TEST_COUNT_MASK 0xf
#define DP_PHY_TEST_PATTERN 0x248
+# define DP_PHY_TEST_PATTERN_SEL_MASK 0x7
+# define DP_PHY_TEST_PATTERN_NONE 0x0
+# define DP_PHY_TEST_PATTERN_D10_2 0x1
+# define DP_PHY_TEST_PATTERN_ERROR_COUNT0x2
+# define DP_PHY_TEST_PATTERN_PRBS7 0x3
+# define DP_PHY_TEST_PATTERN_80BIT_CUSTOM 0
Recall: [PATCH v3 3/9] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation
Manna, Animesh would like to recall the message, "[PATCH v3 3/9] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation". ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/9] drm/amd/display: Fix compilation issue.
On 19-12-2019 05:23, Manasi Navare wrote: On Wed, Dec 18, 2019 at 09:43:49PM +0530, Manna, Animesh wrote: On 18-12-2019 21:12, Harry Wentland wrote: On 2019-12-18 10:13 a.m., Animesh Manna wrote: [Why]: Aligh with DP spec wanted to follow same naming convention. [How]: Changed the macro name of the dpcd address used for getting requested test-pattern. Please roll this into your patch that renames the definition. All patches should compile on their own. Thanks Harry for review, wanted to follow similar commit-description format followed in amd-driver compare to i915 and created a separate patch. Maybe is it good idea to change the patch sequence and make it as first patch. Regards, Animesh Like Harry said, all these changes should happen in the same patch that renames the DP_TEST_PHY_PATTERN which is patch 1/9 because like you see the build still fails now since patch 1 doesnt compile. So the idea would be in patch 1 - rename, make changes in AMD and existing place where it gets used Patch 2 - get/set PHY test paarams that use this renamed value Thanks Manasi. Yes, I want to mean the same. Regards, Animesh Manasi Thanks, Harry Cc: Harry Wentland Cc: Alex Deucher Signed-off-by: Animesh Manna --- drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c index 42aa889fd0f5..1a6109be2fce 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c @@ -2491,7 +2491,7 @@ static void dp_test_send_phy_test_pattern(struct dc_link *link) /* get phy test pattern and pattern parameters from DP receiver */ core_link_read_dpcd( link, - DP_TEST_PHY_PATTERN, + DP_PHY_TEST_PATTERN, &dpcd_test_pattern.raw, sizeof(dpcd_test_pattern)); core_link_read_dpcd( ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/9] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation
On 19-12-2019 18:03, Ville Syrjälä wrote:
On Wed, Dec 18, 2019 at 08:43:44PM +0530, Animesh Manna wrote:
vswing/pre-emphasis adjustment calculation is needed in processing
of auto phy compliance request other than link training, so moved
the same function in intel_dp.c.
No functional change.
Signed-off-by: Animesh Manna
---
drivers/gpu/drm/i915/display/intel_dp.c | 32 +++
drivers/gpu/drm/i915/display/intel_dp.h | 3 ++
.../drm/i915/display/intel_dp_link_training.c | 32 ---
3 files changed, 35 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 2f31d226c6eb..ca82835b6dcf 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4110,6 +4110,38 @@ ivb_cpu_edp_signal_levels(u8 train_set)
}
}
+void
+intel_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 *link_status)
I'd prefer to keep the arrayish notation so we have some idea how big
this is supposed to be. I guess that woukld mean including some
drm dp header in intel_dp.h?
Yes, will add.
Regards,
Animesh
+{
+ u8 v = 0;
+ u8 p = 0;
+ int lane;
+ u8 voltage_max;
+ u8 preemph_max;
+
+ for (lane = 0; lane < intel_dp->lane_count; lane++) {
+ u8 this_v = drm_dp_get_adjust_request_voltage(link_status,
lane);
+ u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status,
lane);
+
+ if (this_v > v)
+ v = this_v;
+ if (this_p > p)
+ p = this_p;
+ }
+
+ voltage_max = intel_dp_voltage_max(intel_dp);
+ if (v >= voltage_max)
+ v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
+
+ preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
+ if (p >= preemph_max)
+ p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
+
+ for (lane = 0; lane < 4; lane++)
+ intel_dp->train_set[lane] = v | p;
+}
+
void
intel_dp_set_signal_levels(struct intel_dp *intel_dp)
{
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
b/drivers/gpu/drm/i915/display/intel_dp.h
index 3da166054788..0d0cb692f701 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -91,6 +91,9 @@ void
intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
u8 dp_train_pat);
void
+intel_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 *link_status);
+void
intel_dp_set_signal_levels(struct intel_dp *intel_dp);
void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
u8
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 2a1130dd1ad0..1e38584e7d56 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -34,38 +34,6 @@ intel_dp_dump_link_status(const u8
link_status[DP_LINK_STATUS_SIZE])
link_status[3], link_status[4], link_status[5]);
}
-static void
-intel_get_adjust_train(struct intel_dp *intel_dp,
- const u8 link_status[DP_LINK_STATUS_SIZE])
-{
- u8 v = 0;
- u8 p = 0;
- int lane;
- u8 voltage_max;
- u8 preemph_max;
-
- for (lane = 0; lane < intel_dp->lane_count; lane++) {
- u8 this_v = drm_dp_get_adjust_request_voltage(link_status,
lane);
- u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status,
lane);
-
- if (this_v > v)
- v = this_v;
- if (this_p > p)
- p = this_p;
- }
-
- voltage_max = intel_dp_voltage_max(intel_dp);
- if (v >= voltage_max)
- v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
-
- preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
- if (p >= preemph_max)
- p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
-
- for (lane = 0; lane < 4; lane++)
- intel_dp->train_set[lane] = v | p;
-}
-
static bool
intel_dp_set_link_train(struct intel_dp *intel_dp,
u8 dp_train_pat)
--
2.24.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/9] drm/i915/dp: Move vswing/pre-emphasis adjustment calculation
On 19-12-2019 16:21, Jani Nikula wrote:
On Wed, 18 Dec 2019, Animesh Manna wrote:
vswing/pre-emphasis adjustment calculation is needed in processing
of auto phy compliance request other than link training, so moved
the same function in intel_dp.c.
No functional change.
Signed-off-by: Animesh Manna
---
drivers/gpu/drm/i915/display/intel_dp.c | 32 +++
drivers/gpu/drm/i915/display/intel_dp.h | 3 ++
.../drm/i915/display/intel_dp_link_training.c | 32 ---
3 files changed, 35 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 2f31d226c6eb..ca82835b6dcf 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4110,6 +4110,38 @@ ivb_cpu_edp_signal_levels(u8 train_set)
}
}
+void
+intel_get_adjust_train(struct intel_dp *intel_dp,
Please follow the naming convention of prefixing non-static functions in
foo.c with foo_. I.e. intel_dp_ here.
Sure, will do.
Regards,
Animesh
BR,
Jani.
+ const u8 *link_status)
+{
+ u8 v = 0;
+ u8 p = 0;
+ int lane;
+ u8 voltage_max;
+ u8 preemph_max;
+
+ for (lane = 0; lane < intel_dp->lane_count; lane++) {
+ u8 this_v = drm_dp_get_adjust_request_voltage(link_status,
lane);
+ u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status,
lane);
+
+ if (this_v > v)
+ v = this_v;
+ if (this_p > p)
+ p = this_p;
+ }
+
+ voltage_max = intel_dp_voltage_max(intel_dp);
+ if (v >= voltage_max)
+ v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
+
+ preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
+ if (p >= preemph_max)
+ p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
+
+ for (lane = 0; lane < 4; lane++)
+ intel_dp->train_set[lane] = v | p;
+}
+
void
intel_dp_set_signal_levels(struct intel_dp *intel_dp)
{
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
b/drivers/gpu/drm/i915/display/intel_dp.h
index 3da166054788..0d0cb692f701 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -91,6 +91,9 @@ void
intel_dp_program_link_training_pattern(struct intel_dp *intel_dp,
u8 dp_train_pat);
void
+intel_get_adjust_train(struct intel_dp *intel_dp,
+ const u8 *link_status);
+void
intel_dp_set_signal_levels(struct intel_dp *intel_dp);
void intel_dp_set_idle_link_train(struct intel_dp *intel_dp);
u8
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 2a1130dd1ad0..1e38584e7d56 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -34,38 +34,6 @@ intel_dp_dump_link_status(const u8
link_status[DP_LINK_STATUS_SIZE])
link_status[3], link_status[4], link_status[5]);
}
-static void
-intel_get_adjust_train(struct intel_dp *intel_dp,
- const u8 link_status[DP_LINK_STATUS_SIZE])
-{
- u8 v = 0;
- u8 p = 0;
- int lane;
- u8 voltage_max;
- u8 preemph_max;
-
- for (lane = 0; lane < intel_dp->lane_count; lane++) {
- u8 this_v = drm_dp_get_adjust_request_voltage(link_status,
lane);
- u8 this_p = drm_dp_get_adjust_request_pre_emphasis(link_status,
lane);
-
- if (this_v > v)
- v = this_v;
- if (this_p > p)
- p = this_p;
- }
-
- voltage_max = intel_dp_voltage_max(intel_dp);
- if (v >= voltage_max)
- v = voltage_max | DP_TRAIN_MAX_SWING_REACHED;
-
- preemph_max = intel_dp_pre_emphasis_max(intel_dp, v);
- if (p >= preemph_max)
- p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
-
- for (lane = 0; lane < 4; lane++)
- intel_dp->train_set[lane] = v | p;
-}
-
static bool
intel_dp_set_link_train(struct intel_dp *intel_dp,
u8 dp_train_pat)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/9] drm/amd/display: Fix compilation issue.
On 18-12-2019 21:12, Harry Wentland wrote: On 2019-12-18 10:13 a.m., Animesh Manna wrote: [Why]: Aligh with DP spec wanted to follow same naming convention. [How]: Changed the macro name of the dpcd address used for getting requested test-pattern. Please roll this into your patch that renames the definition. All patches should compile on their own. Thanks Harry for review, wanted to follow similar commit-description format followed in amd-driver compare to i915 and created a separate patch. Maybe is it good idea to change the patch sequence and make it as first patch. Regards, Animesh Thanks, Harry Cc: Harry Wentland Cc: Alex Deucher Signed-off-by: Animesh Manna --- drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c index 42aa889fd0f5..1a6109be2fce 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c @@ -2491,7 +2491,7 @@ static void dp_test_send_phy_test_pattern(struct dc_link *link) /* get phy test pattern and pattern parameters from DP receiver */ core_link_read_dpcd( link, - DP_TEST_PHY_PATTERN, + DP_PHY_TEST_PATTERN, &dpcd_test_pattern.raw, sizeof(dpcd_test_pattern)); core_link_read_dpcd( ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
