* [PATCH v2 0/6] Link off between frames for edp
@ 2024-04-12 15:52 Animesh Manna
2024-04-12 15:52 ` [PATCH v2 1/6] drm/i915/alpm: Move alpm parameters from intel_psr Animesh Manna
` (8 more replies)
0 siblings, 9 replies; 26+ messages in thread
From: Animesh Manna @ 2024-04-12 15:52 UTC (permalink / raw)
To: intel-gfx
Cc: dri-devel, jani.nikula, jouni.hogander, arun.r.murthy,
Animesh Manna
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: These patches are not tested, sending early for review feedback.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
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
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/display/intel_alpm.c | 388 ++++++++++++++++++
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 | 5 +
drivers/gpu/drm/i915/display/intel_psr.c | 299 +-------------
include/drm/display/drm_dp.h | 5 +-
8 files changed, 452 insertions(+), 299 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 1/6] drm/i915/alpm: Move alpm parameters from intel_psr
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
@ 2024-04-12 15:52 ` Animesh Manna
2024-04-12 15:52 ` [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a new file Animesh Manna
` (7 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Animesh Manna @ 2024-04-12 15:52 UTC (permalink / raw)
To: intel-gfx
Cc: dri-devel, jani.nikula, jouni.hogander, arun.r.murthy,
Animesh Manna
ALPM can be enabled for non psr panel and currenly aplm-params are
encapsulated under intel_psr struct, so moving out to intel_dp struct.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
.../drm/i915/display/intel_display_types.h | 21 +++++----
drivers/gpu/drm/i915/display/intel_psr.c | 43 +++++++++----------
2 files changed, 31 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 0f4bd5710796..73197f014510 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1741,17 +1741,6 @@ struct intel_psr {
bool psr2_sel_fetch_cff_enabled;
bool req_psr2_sdp_prior_scanline;
u8 sink_sync_latency;
-
- struct {
- u8 io_wake_lines;
- u8 fast_wake_lines;
-
- /* LNL and beyond */
- u8 check_entry_lines;
- u8 silence_period_sym_clocks;
- u8 lfps_half_cycle_num_of_syms;
- } alpm_parameters;
-
ktime_t last_entry_attempt;
ktime_t last_exit;
bool sink_not_reliable;
@@ -1881,6 +1870,16 @@ struct intel_dp {
unsigned long last_oui_write;
bool colorimetry_support;
+
+ struct {
+ u8 io_wake_lines;
+ u8 fast_wake_lines;
+
+ /* LNL and beyond */
+ u8 check_entry_lines;
+ u8 silence_period_sym_clocks;
+ u8 lfps_half_cycle_num_of_syms;
+ } alpm_parameters;
};
enum lspcon_vendor {
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index f5b33335a9ae..acc45c0f4694 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -814,8 +814,8 @@ static u32 intel_psr2_get_tp_time(struct intel_dp *intel_dp)
static int psr2_block_count_lines(struct intel_dp *intel_dp)
{
- return intel_dp->psr.alpm_parameters.io_wake_lines < 9 &&
- intel_dp->psr.alpm_parameters.fast_wake_lines < 9 ? 8 : 12;
+ return intel_dp->alpm_parameters.io_wake_lines < 9 &&
+ intel_dp->alpm_parameters.fast_wake_lines < 9 ? 8 : 12;
}
static int psr2_block_count(struct intel_dp *intel_dp)
@@ -852,7 +852,6 @@ static void dg2_activate_panel_replay(struct intel_dp *intel_dp)
static void hsw_activate_psr2(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
- struct intel_psr *psr = &intel_dp->psr;
enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
u32 val = EDP_PSR2_ENABLE;
u32 psr_val = 0;
@@ -894,18 +893,19 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
*/
int tmp;
- tmp = map[psr->alpm_parameters.io_wake_lines -
+ tmp = map[intel_dp->alpm_parameters.io_wake_lines -
TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(tmp + TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES);
- tmp = map[psr->alpm_parameters.fast_wake_lines - TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
+ tmp = map[intel_dp->alpm_parameters.fast_wake_lines -
+ TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
val |= TGL_EDP_PSR2_FAST_WAKE(tmp + TGL_EDP_PSR2_FAST_WAKE_MIN_LINES);
} else if (DISPLAY_VER(dev_priv) >= 12) {
- val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(psr->alpm_parameters.io_wake_lines);
- val |= TGL_EDP_PSR2_FAST_WAKE(psr->alpm_parameters.fast_wake_lines);
+ val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp->alpm_parameters.io_wake_lines);
+ val |= TGL_EDP_PSR2_FAST_WAKE(intel_dp->alpm_parameters.fast_wake_lines);
} else if (DISPLAY_VER(dev_priv) >= 9) {
- val |= EDP_PSR2_IO_BUFFER_WAKE(psr->alpm_parameters.io_wake_lines);
- val |= EDP_PSR2_FAST_WAKE(psr->alpm_parameters.fast_wake_lines);
+ val |= EDP_PSR2_IO_BUFFER_WAKE(intel_dp->alpm_parameters.io_wake_lines);
+ val |= EDP_PSR2_FAST_WAKE(intel_dp->alpm_parameters.fast_wake_lines);
}
if (intel_dp->psr.req_psr2_sdp_prior_scanline)
@@ -1289,9 +1289,9 @@ static int _lnl_compute_aux_less_alpm_params(struct intel_dp *intel_dp,
if (i915->display.params.psr_safest_params)
aux_less_wake_lines = ALPM_CTL_AUX_LESS_WAKE_TIME_MASK;
- intel_dp->psr.alpm_parameters.fast_wake_lines = aux_less_wake_lines;
- intel_dp->psr.alpm_parameters.silence_period_sym_clocks = silence_period;
- intel_dp->psr.alpm_parameters.lfps_half_cycle_num_of_syms = lfps_half_cycle;
+ intel_dp->alpm_parameters.fast_wake_lines = aux_less_wake_lines;
+ intel_dp->alpm_parameters.silence_period_sym_clocks = silence_period;
+ intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms = lfps_half_cycle;
return true;
}
@@ -1318,7 +1318,7 @@ static bool _lnl_compute_alpm_params(struct intel_dp *intel_dp,
if (i915->display.params.psr_safest_params)
check_entry_lines = 15;
- intel_dp->psr.alpm_parameters.check_entry_lines = check_entry_lines;
+ intel_dp->alpm_parameters.check_entry_lines = check_entry_lines;
return true;
}
@@ -1386,8 +1386,8 @@ static bool _compute_alpm_params(struct intel_dp *intel_dp,
io_wake_lines = fast_wake_lines = max_wake_lines;
/* According to Bspec lower limit should be set as 7 lines. */
- intel_dp->psr.alpm_parameters.io_wake_lines = max(io_wake_lines, 7);
- intel_dp->psr.alpm_parameters.fast_wake_lines = max(fast_wake_lines, 7);
+ intel_dp->alpm_parameters.io_wake_lines = max(io_wake_lines, 7);
+ intel_dp->alpm_parameters.fast_wake_lines = max(fast_wake_lines, 7);
return true;
}
@@ -1767,7 +1767,6 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
- struct intel_psr *psr = &intel_dp->psr;
u32 alpm_ctl;
if (DISPLAY_VER(dev_priv) < 20 || (!intel_dp->psr.psr2_enabled &&
@@ -1788,22 +1787,22 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp)
PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
PORT_ALPM_CTL_SILENCE_PERIOD(
- psr->alpm_parameters.silence_period_sym_clocks));
+ intel_dp->alpm_parameters.silence_period_sym_clocks));
intel_de_write(dev_priv, PORT_ALPM_LFPS_CTL(cpu_transcoder),
PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT(10) |
PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION(
- psr->alpm_parameters.lfps_half_cycle_num_of_syms) |
+ intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms) |
PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION(
- psr->alpm_parameters.lfps_half_cycle_num_of_syms) |
+ intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms) |
PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION(
- psr->alpm_parameters.lfps_half_cycle_num_of_syms));
+ intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms));
} else {
alpm_ctl = ALPM_CTL_EXTENDED_FAST_WAKE_ENABLE |
- ALPM_CTL_EXTENDED_FAST_WAKE_TIME(psr->alpm_parameters.fast_wake_lines);
+ ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp->alpm_parameters.fast_wake_lines);
}
- alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(psr->alpm_parameters.check_entry_lines);
+ 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);
}
--
2.29.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a new file
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
2024-04-12 15:52 ` [PATCH v2 1/6] drm/i915/alpm: Move alpm parameters from intel_psr Animesh Manna
@ 2024-04-12 15:52 ` Animesh Manna
2024-04-15 11:46 ` Jani Nikula
2024-04-12 15:52 ` [PATCH v2 3/6] drm/display: Add missing aux less alpm wake related bits Animesh Manna
` (6 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: Animesh Manna @ 2024-04-12 15:52 UTC (permalink / raw)
To: intel-gfx
Cc: dri-devel, jani.nikula, jouni.hogander, arun.r.murthy,
Animesh Manna
Move ALPM feature related code as it will be used for
non-psr panel also thorugh LOBF feature.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
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 \
diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
new file mode 100644
index 000000000000..13bac3e8c8fa
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_alpm.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2024, Intel Corporation.
+ */
+
+#include "intel_alpm.h"
+#include "intel_crtc.h"
+#include "intel_de.h"
+#include "intel_display_types.h"
+#include "intel_dp.h"
+#include "intel_dp_aux.h"
+#include "intel_psr_regs.h"
+
+/*
+ * See Bspec: 71632 for the table
+ *
+ * Silence_period = tSilence,Min + ((tSilence,Max - tSilence,Min) / 2)
+ *
+ * Half cycle duration:
+ *
+ * Link rates 1.62 - 4.32 and tLFPS_Cycle = 70 ns
+ * FLOOR( (Link Rate * tLFPS_Cycle) / (2 * 10) )
+ *
+ * Link rates 5.4 - 8.1
+ * PORT_ALPM_LFPS_CTL[ LFPS Cycle Count ] = 10
+ * LFPS Period chosen is the mid-point of the min:max values from the table
+ * FLOOR( LFPS Period in Symbol clocks /
+ * (2 * PORT_ALPM_LFPS_CTL[ LFPS Cycle Count ]) )
+ */
+static bool _lnl_get_silence_period_and_lfps_half_cycle(int link_rate,
+ int *silence_period,
+ int *lfps_half_cycle)
+{
+ switch (link_rate) {
+ case 162000:
+ *silence_period = 20;
+ *lfps_half_cycle = 5;
+ break;
+ case 216000:
+ *silence_period = 27;
+ *lfps_half_cycle = 7;
+ break;
+ case 243000:
+ *silence_period = 31;
+ *lfps_half_cycle = 8;
+ break;
+ case 270000:
+ *silence_period = 34;
+ *lfps_half_cycle = 9;
+ break;
+ case 324000:
+ *silence_period = 41;
+ *lfps_half_cycle = 11;
+ break;
+ case 432000:
+ *silence_period = 56;
+ *lfps_half_cycle = 15;
+ break;
+ case 540000:
+ *silence_period = 69;
+ *lfps_half_cycle = 12;
+ break;
+ case 648000:
+ *silence_period = 84;
+ *lfps_half_cycle = 15;
+ break;
+ case 675000:
+ *silence_period = 87;
+ *lfps_half_cycle = 15;
+ break;
+ case 810000:
+ *silence_period = 104;
+ *lfps_half_cycle = 19;
+ break;
+ default:
+ *silence_period = *lfps_half_cycle = -1;
+ return false;
+ }
+ return true;
+}
+
+/*
+ * AUX-Less Wake Time = CEILING( ((PHY P2 to P0) + tLFPS_Period, Max+
+ * tSilence, Max+ tPHY Establishment + tCDS) / tline)
+ * For the "PHY P2 to P0" latency see the PHY Power Control page
+ * (PHY P2 to P0) : https://gfxspecs.intel.com/Predator/Home/Index/68965
+ * : 12 us
+ * The tLFPS_Period, Max term is 800ns
+ * The tSilence, Max term is 180ns
+ * The tPHY Establishment (a.k.a. t1) term is 50us
+ * The tCDS term is 1 or 2 times t2
+ * t2 = Number ML_PHY_LOCK * tML_PHY_LOCK
+ * Number ML_PHY_LOCK = ( 7 + CEILING( 6.5us / tML_PHY_LOCK ) + 1)
+ * Rounding up the 6.5us padding to the next ML_PHY_LOCK boundary and
+ * adding the "+ 1" term ensures all ML_PHY_LOCK sequences that start
+ * within the CDS period complete within the CDS period regardless of
+ * entry into the period
+ * tML_PHY_LOCK = TPS4 Length * ( 10 / (Link Rate in MHz) )
+ * TPS4 Length = 252 Symbols
+ */
+static int _lnl_compute_aux_less_wake_time(int port_clock)
+{
+ int tphy2_p2_to_p0 = 12 * 1000;
+ int tlfps_period_max = 800;
+ int tsilence_max = 180;
+ int t1 = 50 * 1000;
+ int tps4 = 252;
+ int tml_phy_lock = 1000 * 1000 * tps4 * 10 / port_clock;
+ int num_ml_phy_lock = 7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1;
+ int t2 = num_ml_phy_lock * tml_phy_lock;
+ int tcds = 1 * t2;
+
+ return DIV_ROUND_UP(tphy2_p2_to_p0 + tlfps_period_max + tsilence_max +
+ t1 + tcds, 1000);
+}
+
+static int _lnl_compute_aux_less_alpm_params(struct intel_dp *intel_dp,
+ struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ int aux_less_wake_time, aux_less_wake_lines, silence_period,
+ lfps_half_cycle;
+
+ aux_less_wake_time =
+ _lnl_compute_aux_less_wake_time(crtc_state->port_clock);
+ aux_less_wake_lines = intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode,
+ aux_less_wake_time);
+
+ if (!_lnl_get_silence_period_and_lfps_half_cycle(crtc_state->port_clock,
+ &silence_period,
+ &lfps_half_cycle))
+ return false;
+
+ if (aux_less_wake_lines > ALPM_CTL_AUX_LESS_WAKE_TIME_MASK ||
+ silence_period > PORT_ALPM_CTL_SILENCE_PERIOD_MASK ||
+ lfps_half_cycle > PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK)
+ return false;
+
+ if (i915->display.params.psr_safest_params)
+ aux_less_wake_lines = ALPM_CTL_AUX_LESS_WAKE_TIME_MASK;
+
+ intel_dp->alpm_parameters.fast_wake_lines = aux_less_wake_lines;
+ intel_dp->alpm_parameters.silence_period_sym_clocks = silence_period;
+ intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms = lfps_half_cycle;
+
+ return true;
+}
+
+static bool _lnl_compute_alpm_params(struct intel_dp *intel_dp,
+ struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ int check_entry_lines;
+
+ if (DISPLAY_VER(i915) < 20)
+ return true;
+
+ /* ALPM Entry Check = 2 + CEILING( 5us /tline ) */
+ check_entry_lines = 2 +
+ intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode, 5);
+
+ if (check_entry_lines > 15)
+ return false;
+
+ if (!_lnl_compute_aux_less_alpm_params(intel_dp, crtc_state))
+ return false;
+
+ if (i915->display.params.psr_safest_params)
+ check_entry_lines = 15;
+
+ intel_dp->alpm_parameters.check_entry_lines = check_entry_lines;
+
+ return true;
+}
+
+/*
+ * IO wake time for DISPLAY_VER < 12 is not directly mentioned in Bspec. There
+ * are 50 us io wake time and 32 us fast wake time. Clearly preharge pulses are
+ * not (improperly) included in 32 us fast wake time. 50 us - 32 us = 18 us.
+ */
+static int skl_io_buffer_wake_time(void)
+{
+ return 18;
+}
+
+static int tgl_io_buffer_wake_time(void)
+{
+ return 10;
+}
+
+static int io_buffer_wake_time(const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+ if (DISPLAY_VER(i915) >= 12)
+ return tgl_io_buffer_wake_time();
+ else
+ return skl_io_buffer_wake_time();
+}
+
+bool intel_alpm_compute_params(struct intel_dp *intel_dp,
+ struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ int io_wake_lines, io_wake_time, fast_wake_lines, fast_wake_time;
+ int tfw_exit_latency = 20; /* eDP spec */
+ int phy_wake = 4; /* eDP spec */
+ int preamble = 8; /* eDP spec */
+ int precharge = intel_dp_aux_fw_sync_len() - preamble;
+ u8 max_wake_lines;
+
+ io_wake_time = max(precharge, io_buffer_wake_time(crtc_state)) +
+ preamble + phy_wake + tfw_exit_latency;
+ fast_wake_time = precharge + preamble + phy_wake +
+ tfw_exit_latency;
+
+ if (DISPLAY_VER(i915) >= 12)
+ /* TODO: Check how we can use ALPM_CTL fast wake extended field */
+ max_wake_lines = 12;
+ else
+ max_wake_lines = 8;
+
+ io_wake_lines = intel_usecs_to_scanlines(
+ &crtc_state->hw.adjusted_mode, io_wake_time);
+ fast_wake_lines = intel_usecs_to_scanlines(
+ &crtc_state->hw.adjusted_mode, fast_wake_time);
+
+ if (io_wake_lines > max_wake_lines ||
+ fast_wake_lines > max_wake_lines)
+ return false;
+
+ if (!_lnl_compute_alpm_params(intel_dp, crtc_state))
+ return false;
+
+ if (i915->display.params.psr_safest_params)
+ io_wake_lines = fast_wake_lines = max_wake_lines;
+
+ /* According to Bspec lower limit should be set as 7 lines. */
+ intel_dp->alpm_parameters.io_wake_lines = max(io_wake_lines, 7);
+ intel_dp->alpm_parameters.fast_wake_lines = max(fast_wake_lines, 7);
+
+ return true;
+}
+
+static void lnl_alpm_configure(struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+ enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
+ u32 alpm_ctl;
+
+ if (DISPLAY_VER(dev_priv) < 20 || (!intel_dp->psr.psr2_enabled &&
+ !intel_dp_is_edp(intel_dp)))
+ return;
+
+ /*
+ * Panel Replay on eDP is always using ALPM aux less. I.e. no need to
+ * check panel support at this point.
+ */
+ if (intel_dp->psr.panel_replay_enabled && intel_dp_is_edp(intel_dp)) {
+ alpm_ctl = ALPM_CTL_ALPM_ENABLE |
+ ALPM_CTL_ALPM_AUX_LESS_ENABLE |
+ ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS;
+
+ intel_de_write(dev_priv, PORT_ALPM_CTL(cpu_transcoder),
+ PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE |
+ PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
+ PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
+ PORT_ALPM_CTL_SILENCE_PERIOD(
+ intel_dp->alpm_parameters.silence_period_sym_clocks));
+
+ intel_de_write(dev_priv, PORT_ALPM_LFPS_CTL(cpu_transcoder),
+ PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT(10) |
+ PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION(
+ intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms) |
+ PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION(
+ intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms) |
+ PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION(
+ intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms));
+ } else {
+ alpm_ctl = ALPM_CTL_EXTENDED_FAST_WAKE_ENABLE |
+ ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp->alpm_parameters.fast_wake_lines);
+ }
+
+ 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)
+{
+ lnl_alpm_configure(intel_dp);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h b/drivers/gpu/drm/i915/display/intel_alpm.h
new file mode 100644
index 000000000000..c45d078e5a6b
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_alpm.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2024 Intel Corporation
+ */
+
+#ifndef _INTEL_ALPM_H
+#define _INTEL_ALPM_H
+
+#include <linux/types.h>
+
+struct intel_dp;
+struct intel_crtc_state;
+
+bool intel_alpm_compute_params(struct intel_dp *intel_dp,
+ struct intel_crtc_state *crtc_state);
+void intel_alpm_configure(struct intel_dp *intel_dp);
+
+#endif
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index acc45c0f4694..c4ab289dbc15 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -27,6 +27,7 @@
#include "i915_drv.h"
#include "i915_reg.h"
+#include "intel_alpm.h"
#include "intel_atomic.h"
#include "intel_crtc.h"
#include "intel_ddi.h"
@@ -1161,237 +1162,6 @@ static bool _compute_psr2_sdp_prior_scanline_indication(struct intel_dp *intel_d
return true;
}
-/*
- * See Bspec: 71632 for the table
- *
- * Silence_period = tSilence,Min + ((tSilence,Max - tSilence,Min) / 2)
- *
- * Half cycle duration:
- *
- * Link rates 1.62 - 4.32 and tLFPS_Cycle = 70 ns
- * FLOOR( (Link Rate * tLFPS_Cycle) / (2 * 10) )
- *
- * Link rates 5.4 - 8.1
- * PORT_ALPM_LFPS_CTL[ LFPS Cycle Count ] = 10
- * LFPS Period chosen is the mid-point of the min:max values from the table
- * FLOOR( LFPS Period in Symbol clocks /
- * (2 * PORT_ALPM_LFPS_CTL[ LFPS Cycle Count ]) )
- */
-static bool _lnl_get_silence_period_and_lfps_half_cycle(int link_rate,
- int *silence_period,
- int *lfps_half_cycle)
-{
- switch (link_rate) {
- case 162000:
- *silence_period = 20;
- *lfps_half_cycle = 5;
- break;
- case 216000:
- *silence_period = 27;
- *lfps_half_cycle = 7;
- break;
- case 243000:
- *silence_period = 31;
- *lfps_half_cycle = 8;
- break;
- case 270000:
- *silence_period = 34;
- *lfps_half_cycle = 9;
- break;
- case 324000:
- *silence_period = 41;
- *lfps_half_cycle = 11;
- break;
- case 432000:
- *silence_period = 56;
- *lfps_half_cycle = 15;
- break;
- case 540000:
- *silence_period = 69;
- *lfps_half_cycle = 12;
- break;
- case 648000:
- *silence_period = 84;
- *lfps_half_cycle = 15;
- break;
- case 675000:
- *silence_period = 87;
- *lfps_half_cycle = 15;
- break;
- case 810000:
- *silence_period = 104;
- *lfps_half_cycle = 19;
- break;
- default:
- *silence_period = *lfps_half_cycle = -1;
- return false;
- }
- return true;
-}
-
-/*
- * AUX-Less Wake Time = CEILING( ((PHY P2 to P0) + tLFPS_Period, Max+
- * tSilence, Max+ tPHY Establishment + tCDS) / tline)
- * For the "PHY P2 to P0" latency see the PHY Power Control page
- * (PHY P2 to P0) : https://gfxspecs.intel.com/Predator/Home/Index/68965
- * : 12 us
- * The tLFPS_Period, Max term is 800ns
- * The tSilence, Max term is 180ns
- * The tPHY Establishment (a.k.a. t1) term is 50us
- * The tCDS term is 1 or 2 times t2
- * t2 = Number ML_PHY_LOCK * tML_PHY_LOCK
- * Number ML_PHY_LOCK = ( 7 + CEILING( 6.5us / tML_PHY_LOCK ) + 1)
- * Rounding up the 6.5us padding to the next ML_PHY_LOCK boundary and
- * adding the "+ 1" term ensures all ML_PHY_LOCK sequences that start
- * within the CDS period complete within the CDS period regardless of
- * entry into the period
- * tML_PHY_LOCK = TPS4 Length * ( 10 / (Link Rate in MHz) )
- * TPS4 Length = 252 Symbols
- */
-static int _lnl_compute_aux_less_wake_time(int port_clock)
-{
- int tphy2_p2_to_p0 = 12 * 1000;
- int tlfps_period_max = 800;
- int tsilence_max = 180;
- int t1 = 50 * 1000;
- int tps4 = 252;
- int tml_phy_lock = 1000 * 1000 * tps4 * 10 / port_clock;
- int num_ml_phy_lock = 7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1;
- int t2 = num_ml_phy_lock * tml_phy_lock;
- int tcds = 1 * t2;
-
- return DIV_ROUND_UP(tphy2_p2_to_p0 + tlfps_period_max + tsilence_max +
- t1 + tcds, 1000);
-}
-
-static int _lnl_compute_aux_less_alpm_params(struct intel_dp *intel_dp,
- struct intel_crtc_state *crtc_state)
-{
- struct drm_i915_private *i915 = dp_to_i915(intel_dp);
- int aux_less_wake_time, aux_less_wake_lines, silence_period,
- lfps_half_cycle;
-
- aux_less_wake_time =
- _lnl_compute_aux_less_wake_time(crtc_state->port_clock);
- aux_less_wake_lines = intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode,
- aux_less_wake_time);
-
- if (!_lnl_get_silence_period_and_lfps_half_cycle(crtc_state->port_clock,
- &silence_period,
- &lfps_half_cycle))
- return false;
-
- if (aux_less_wake_lines > ALPM_CTL_AUX_LESS_WAKE_TIME_MASK ||
- silence_period > PORT_ALPM_CTL_SILENCE_PERIOD_MASK ||
- lfps_half_cycle > PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK)
- return false;
-
- if (i915->display.params.psr_safest_params)
- aux_less_wake_lines = ALPM_CTL_AUX_LESS_WAKE_TIME_MASK;
-
- intel_dp->alpm_parameters.fast_wake_lines = aux_less_wake_lines;
- intel_dp->alpm_parameters.silence_period_sym_clocks = silence_period;
- intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms = lfps_half_cycle;
-
- return true;
-}
-
-static bool _lnl_compute_alpm_params(struct intel_dp *intel_dp,
- struct intel_crtc_state *crtc_state)
-{
- struct drm_i915_private *i915 = dp_to_i915(intel_dp);
- int check_entry_lines;
-
- if (DISPLAY_VER(i915) < 20)
- return true;
-
- /* ALPM Entry Check = 2 + CEILING( 5us /tline ) */
- check_entry_lines = 2 +
- intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode, 5);
-
- if (check_entry_lines > 15)
- return false;
-
- if (!_lnl_compute_aux_less_alpm_params(intel_dp, crtc_state))
- return false;
-
- if (i915->display.params.psr_safest_params)
- check_entry_lines = 15;
-
- intel_dp->alpm_parameters.check_entry_lines = check_entry_lines;
-
- return true;
-}
-
-/*
- * IO wake time for DISPLAY_VER < 12 is not directly mentioned in Bspec. There
- * are 50 us io wake time and 32 us fast wake time. Clearly preharge pulses are
- * not (improperly) included in 32 us fast wake time. 50 us - 32 us = 18 us.
- */
-static int skl_io_buffer_wake_time(void)
-{
- return 18;
-}
-
-static int tgl_io_buffer_wake_time(void)
-{
- return 10;
-}
-
-static int io_buffer_wake_time(const struct intel_crtc_state *crtc_state)
-{
- struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
-
- if (DISPLAY_VER(i915) >= 12)
- return tgl_io_buffer_wake_time();
- else
- return skl_io_buffer_wake_time();
-}
-
-static bool _compute_alpm_params(struct intel_dp *intel_dp,
- struct intel_crtc_state *crtc_state)
-{
- struct drm_i915_private *i915 = dp_to_i915(intel_dp);
- int io_wake_lines, io_wake_time, fast_wake_lines, fast_wake_time;
- int tfw_exit_latency = 20; /* eDP spec */
- int phy_wake = 4; /* eDP spec */
- int preamble = 8; /* eDP spec */
- int precharge = intel_dp_aux_fw_sync_len() - preamble;
- u8 max_wake_lines;
-
- io_wake_time = max(precharge, io_buffer_wake_time(crtc_state)) +
- preamble + phy_wake + tfw_exit_latency;
- fast_wake_time = precharge + preamble + phy_wake +
- tfw_exit_latency;
-
- if (DISPLAY_VER(i915) >= 12)
- /* TODO: Check how we can use ALPM_CTL fast wake extended field */
- max_wake_lines = 12;
- else
- max_wake_lines = 8;
-
- io_wake_lines = intel_usecs_to_scanlines(
- &crtc_state->hw.adjusted_mode, io_wake_time);
- fast_wake_lines = intel_usecs_to_scanlines(
- &crtc_state->hw.adjusted_mode, fast_wake_time);
-
- if (io_wake_lines > max_wake_lines ||
- fast_wake_lines > max_wake_lines)
- return false;
-
- if (!_lnl_compute_alpm_params(intel_dp, crtc_state))
- return false;
-
- if (i915->display.params.psr_safest_params)
- io_wake_lines = fast_wake_lines = max_wake_lines;
-
- /* According to Bspec lower limit should be set as 7 lines. */
- intel_dp->alpm_parameters.io_wake_lines = max(io_wake_lines, 7);
- intel_dp->alpm_parameters.fast_wake_lines = max(fast_wake_lines, 7);
-
- return true;
-}
-
static int intel_psr_entry_setup_frames(struct intel_dp *intel_dp,
const struct drm_display_mode *adjusted_mode)
{
@@ -1519,7 +1289,7 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
return false;
}
- if (!_compute_alpm_params(intel_dp, crtc_state)) {
+ if (!intel_alpm_compute_params(intel_dp, crtc_state)) {
drm_dbg_kms(&dev_priv->drm,
"PSR2 not enabled, Unable to use long enough wake times\n");
return false;
@@ -1763,50 +1533,6 @@ static void wm_optimization_wa(struct intel_dp *intel_dp,
wa_16013835468_bit_get(intel_dp), 0);
}
-static void lnl_alpm_configure(struct intel_dp *intel_dp)
-{
- struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
- enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
- u32 alpm_ctl;
-
- if (DISPLAY_VER(dev_priv) < 20 || (!intel_dp->psr.psr2_enabled &&
- !intel_dp_is_edp(intel_dp)))
- return;
-
- /*
- * Panel Replay on eDP is always using ALPM aux less. I.e. no need to
- * check panel support at this point.
- */
- if (intel_dp->psr.panel_replay_enabled && intel_dp_is_edp(intel_dp)) {
- alpm_ctl = ALPM_CTL_ALPM_ENABLE |
- ALPM_CTL_ALPM_AUX_LESS_ENABLE |
- ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS;
-
- intel_de_write(dev_priv, PORT_ALPM_CTL(cpu_transcoder),
- PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE |
- PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
- PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
- PORT_ALPM_CTL_SILENCE_PERIOD(
- intel_dp->alpm_parameters.silence_period_sym_clocks));
-
- intel_de_write(dev_priv, PORT_ALPM_LFPS_CTL(cpu_transcoder),
- PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT(10) |
- PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION(
- intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms) |
- PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION(
- intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms) |
- PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION(
- intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms));
- } else {
- alpm_ctl = ALPM_CTL_EXTENDED_FAST_WAKE_ENABLE |
- ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp->alpm_parameters.fast_wake_lines);
- }
-
- 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);
-}
-
static void intel_psr_enable_source(struct intel_dp *intel_dp,
const struct intel_crtc_state *crtc_state)
{
@@ -1885,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))
- lnl_alpm_configure(intel_dp);
+ intel_alpm_configure(intel_dp);
/*
* Wa_16013835468
--
2.29.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 3/6] drm/display: Add missing aux less alpm wake related bits
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
2024-04-12 15:52 ` [PATCH v2 1/6] drm/i915/alpm: Move alpm parameters from intel_psr Animesh Manna
2024-04-12 15:52 ` [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a new file Animesh Manna
@ 2024-04-12 15:52 ` Animesh Manna
2024-04-12 15:52 ` [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf Animesh Manna
` (5 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Animesh Manna @ 2024-04-12 15:52 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel, jani.nikula, jouni.hogander, arun.r.murthy
From: Jouni Högander <jouni.hogander@intel.com>
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 <jouni.hogander@intel.com>
---
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 0b032faa8cf2..ad0cb0a1de87 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)
@@ -677,7 +679,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
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
` (2 preceding siblings ...)
2024-04-12 15:52 ` [PATCH v2 3/6] drm/display: Add missing aux less alpm wake related bits Animesh Manna
@ 2024-04-12 15:52 ` Animesh Manna
2024-04-15 10:06 ` Hogander, Jouni
2024-04-15 11:47 ` Jani Nikula
2024-04-12 15:52 ` [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL Animesh Manna
` (4 subsequent siblings)
8 siblings, 2 replies; 26+ messages in thread
From: Animesh Manna @ 2024-04-12 15:52 UTC (permalink / raw)
To: intel-gfx
Cc: dri-devel, jani.nikula, jouni.hogander, arun.r.murthy,
Animesh Manna
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 <animesh.manna@intel.com>
---
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;
+
+ 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;
+ }
+}
+
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 @@
struct intel_dp;
struct intel_crtc_state;
+struct drm_connector_state;
+bool intel_dp_get_aux_less_alpm_status(struct intel_dp *intel_dp);
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
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 73197f014510..6116c383b543 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1876,10 +1876,14 @@ struct intel_dp {
u8 fast_wake_lines;
/* LNL and beyond */
+ bool auxless_alpm_supported;
u8 check_entry_lines;
u8 silence_period_sym_clocks;
u8 lfps_half_cycle_num_of_syms;
} alpm_parameters;
+
+ /* LOBF flags*/
+ bool lobf_supported;
};
enum lspcon_vendor {
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 163da48bc406..12ec796568d9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -48,6 +48,7 @@
#include "i915_drv.h"
#include "i915_irq.h"
#include "i915_reg.h"
+#include "intel_alpm.h"
#include "intel_atomic.h"
#include "intel_audio.h"
#include "intel_backlight.h"
@@ -3001,6 +3002,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
intel_vrr_compute_config(pipe_config, conn_state);
intel_dp_compute_as_sdp(intel_dp, pipe_config);
intel_psr_compute_config(intel_dp, pipe_config, conn_state);
+ intel_alpm_compute_lobf_config(intel_dp, pipe_config, conn_state);
intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16);
intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state);
@@ -6616,6 +6618,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
intel_pps_init_late(intel_dp);
+ if (intel_dp_get_aux_less_alpm_status(intel_dp))
+ intel_dp->alpm_parameters.auxless_alpm_supported = true;
+
return true;
out_vdd_off:
--
2.29.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
` (3 preceding siblings ...)
2024-04-12 15:52 ` [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf Animesh Manna
@ 2024-04-12 15:52 ` Animesh Manna
2024-04-15 10:09 ` Hogander, Jouni
2024-04-15 11:48 ` Jani Nikula
2024-04-12 15:52 ` [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF Animesh Manna
` (3 subsequent siblings)
8 siblings, 2 replies; 26+ messages in thread
From: Animesh Manna @ 2024-04-12 15:52 UTC (permalink / raw)
To: intel-gfx
Cc: dri-devel, jani.nikula, jouni.hogander, arun.r.murthy,
Animesh Manna
Set the Link Off Between Frames Enable bit in ALPM_CTL register.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
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;
+ }
+
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 {
--
2.29.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
` (4 preceding siblings ...)
2024-04-12 15:52 ` [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL Animesh Manna
@ 2024-04-12 15:52 ` Animesh Manna
2024-04-15 10:14 ` Hogander, Jouni
2024-04-15 11:53 ` Jani Nikula
2024-04-15 12:58 ` ✗ Fi.CI.CHECKPATCH: warning for Link off between frames for edp (rev2) Patchwork
` (2 subsequent siblings)
8 siblings, 2 replies; 26+ messages in thread
From: Animesh Manna @ 2024-04-12 15:52 UTC (permalink / raw)
To: intel-gfx
Cc: dri-devel, jani.nikula, jouni.hogander, arun.r.murthy,
Animesh Manna
For validation purpose add debugfs for LOBF.
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
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;
+}
+
+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);
+
+ 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 ||
--
2.29.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
2024-04-12 15:52 ` [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf Animesh Manna
@ 2024-04-15 10:06 ` Hogander, Jouni
2024-04-16 8:15 ` Manna, Animesh
2024-04-15 11:47 ` Jani Nikula
1 sibling, 1 reply; 26+ messages in thread
From: Hogander, Jouni @ 2024-04-15 10:06 UTC (permalink / raw)
To: Manna, Animesh, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Murthy, Arun R, Nikula, Jani
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 <animesh.manna@intel.com>
> ---
> 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.
> +
> + 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.
> +}
> +
> 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 @@
>
> struct intel_dp;
> struct intel_crtc_state;
> +struct drm_connector_state;
>
> +bool intel_dp_get_aux_less_alpm_status(struct intel_dp *intel_dp);
> 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
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 73197f014510..6116c383b543 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1876,10 +1876,14 @@ struct intel_dp {
> u8 fast_wake_lines;
>
> /* LNL and beyond */
> + bool auxless_alpm_supported;
> u8 check_entry_lines;
> u8 silence_period_sym_clocks;
> u8 lfps_half_cycle_num_of_syms;
> } alpm_parameters;
> +
> + /* LOBF flags*/
> + bool lobf_supported;
I think having it here and naming like this is misleading. How about
intel_crtc_state->has_lobf?
> };
>
> enum lspcon_vendor {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 163da48bc406..12ec796568d9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -48,6 +48,7 @@
> #include "i915_drv.h"
> #include "i915_irq.h"
> #include "i915_reg.h"
> +#include "intel_alpm.h"
> #include "intel_atomic.h"
> #include "intel_audio.h"
> #include "intel_backlight.h"
> @@ -3001,6 +3002,7 @@ intel_dp_compute_config(struct intel_encoder
> *encoder,
> intel_vrr_compute_config(pipe_config, conn_state);
> intel_dp_compute_as_sdp(intel_dp, pipe_config);
> intel_psr_compute_config(intel_dp, pipe_config, conn_state);
> + intel_alpm_compute_lobf_config(intel_dp, pipe_config,
> conn_state);
> intel_dp_drrs_compute_config(connector, pipe_config,
> link_bpp_x16);
> intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp,
> pipe_config, conn_state);
> @@ -6616,6 +6618,9 @@ static bool intel_edp_init_connector(struct
> intel_dp *intel_dp,
>
> intel_pps_init_late(intel_dp);
>
> + if (intel_dp_get_aux_less_alpm_status(intel_dp))
> + intel_dp->alpm_parameters.auxless_alpm_supported =
> true;
> +
intel_dp->alpm_parameters.auxless_alpm_supported =
intel_dp_get_aux_less_alpm_status(intel_dp);
BR,
Jouni Högander
> return true;
>
> out_vdd_off:
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL
2024-04-12 15:52 ` [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL Animesh Manna
@ 2024-04-15 10:09 ` Hogander, Jouni
2024-04-16 8:20 ` Manna, Animesh
2024-04-15 11:48 ` Jani Nikula
1 sibling, 1 reply; 26+ messages in thread
From: Hogander, Jouni @ 2024-04-15 10:09 UTC (permalink / raw)
To: Manna, Animesh, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Murthy, Arun R, Nikula, Jani
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 <animesh.manna@intel.com>
> ---
> 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.
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 {
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
2024-04-12 15:52 ` [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF Animesh Manna
@ 2024-04-15 10:14 ` Hogander, Jouni
2024-04-16 8:21 ` Manna, Animesh
2024-04-15 11:53 ` Jani Nikula
1 sibling, 1 reply; 26+ messages in thread
From: Hogander, Jouni @ 2024-04-15 10:14 UTC (permalink / raw)
To: Manna, Animesh, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Murthy, Arun R, Nikula, Jani
On Fri, 2024-04-12 at 21:22 +0530, Animesh Manna wrote:
> For validation purpose add debugfs for LOBF.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> 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.
> +}
> +
> +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.
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 ||
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a new file
2024-04-12 15:52 ` [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a new file Animesh Manna
@ 2024-04-15 11:46 ` Jani Nikula
2024-04-16 8:22 ` Manna, Animesh
0 siblings, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2024-04-15 11:46 UTC (permalink / raw)
To: Animesh Manna, intel-gfx
Cc: dri-devel, jouni.hogander, arun.r.murthy, Animesh Manna
On Fri, 12 Apr 2024, Animesh Manna <animesh.manna@intel.com> wrote:
> Move ALPM feature related code as it will be used for
> non-psr panel also thorugh LOBF feature.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> 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.
BR,
Jani.
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
2024-04-12 15:52 ` [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf Animesh Manna
2024-04-15 10:06 ` Hogander, Jouni
@ 2024-04-15 11:47 ` Jani Nikula
2024-04-16 8:24 ` Manna, Animesh
1 sibling, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2024-04-15 11:47 UTC (permalink / raw)
To: Animesh Manna, intel-gfx
Cc: dri-devel, jouni.hogander, arun.r.murthy, Animesh Manna
On Fri, 12 Apr 2024, Animesh Manna <animesh.manna@intel.com> 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?
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL
2024-04-12 15:52 ` [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL Animesh Manna
2024-04-15 10:09 ` Hogander, Jouni
@ 2024-04-15 11:48 ` Jani Nikula
2024-04-16 8:36 ` Manna, Animesh
1 sibling, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2024-04-15 11:48 UTC (permalink / raw)
To: Animesh Manna, intel-gfx
Cc: dri-devel, jouni.hogander, arun.r.murthy, Animesh Manna
On Fri, 12 Apr 2024, Animesh Manna <animesh.manna@intel.com> wrote:
> Set the Link Off Between Frames Enable bit in ALPM_CTL register.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> 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.
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
2024-04-12 15:52 ` [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF Animesh Manna
2024-04-15 10:14 ` Hogander, Jouni
@ 2024-04-15 11:53 ` Jani Nikula
2024-04-16 8:43 ` Manna, Animesh
1 sibling, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2024-04-15 11:53 UTC (permalink / raw)
To: Animesh Manna, intel-gfx
Cc: dri-devel, jouni.hogander, arun.r.murthy, Animesh Manna
On Fri, 12 Apr 2024, Animesh Manna <animesh.manna@intel.com> wrote:
> For validation purpose add debugfs for LOBF.
>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
> 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.
> +
> + 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.
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 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 ||
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for Link off between frames for edp (rev2)
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
` (5 preceding siblings ...)
2024-04-12 15:52 ` [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF Animesh Manna
@ 2024-04-15 12:58 ` Patchwork
2024-04-15 12:58 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-04-15 13:08 ` ✗ Fi.CI.BAT: failure " Patchwork
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2024-04-15 12:58 UTC (permalink / raw)
To: Animesh Manna; +Cc: intel-gfx
== Series Details ==
Series: Link off between frames for edp (rev2)
URL : https://patchwork.freedesktop.org/series/130650/
State : warning
== Summary ==
Error: dim checkpatch failed
3237e68ff67b drm/i915/alpm: Move alpm parameters from intel_psr
aaf1cb340d64 drm/i915/alpm: Move alpm related code to a new file
Traceback (most recent call last):
File "scripts/spdxcheck.py", line 6, in <module>
from ply import lex, yacc
ModuleNotFoundError: No module named 'ply'
Traceback (most recent call last):
File "scripts/spdxcheck.py", line 6, in <module>
from ply import lex, yacc
ModuleNotFoundError: No module named 'ply'
-:24: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#24:
new file mode 100644
-:251: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#251: FILE: drivers/gpu/drm/i915/display/intel_alpm.c:223:
+ io_wake_lines = intel_usecs_to_scanlines(
-:253: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#253: FILE: drivers/gpu/drm/i915/display/intel_alpm.c:225:
+ fast_wake_lines = intel_usecs_to_scanlines(
-:264: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#264: FILE: drivers/gpu/drm/i915/display/intel_alpm.c:236:
+ io_wake_lines = fast_wake_lines = max_wake_lines;
-:296: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#296: FILE: drivers/gpu/drm/i915/display/intel_alpm.c:268:
+ PORT_ALPM_CTL_SILENCE_PERIOD(
-:301: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#301: FILE: drivers/gpu/drm/i915/display/intel_alpm.c:273:
+ PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION(
-:303: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#303: FILE: drivers/gpu/drm/i915/display/intel_alpm.c:275:
+ PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION(
-:305: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#305: FILE: drivers/gpu/drm/i915/display/intel_alpm.c:277:
+ PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION(
-:309: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#309: FILE: drivers/gpu/drm/i915/display/intel_alpm.c:281:
+ ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp->alpm_parameters.fast_wake_lines);
total: 0 errors, 2 warnings, 7 checks, 627 lines checked
86d04a0536b2 drm/display: Add missing aux less alpm wake related bits
afa8b9ada2d6 drm/i915/alpm: Add compute config for lobf
88a647045907 drm/i915/alpm: Enable lobf from source in ALPM_CTL
2130607c083a drm/i915/alpm: Add debugfs for LOBF
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✗ Fi.CI.SPARSE: warning for Link off between frames for edp (rev2)
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
` (6 preceding siblings ...)
2024-04-15 12:58 ` ✗ Fi.CI.CHECKPATCH: warning for Link off between frames for edp (rev2) Patchwork
@ 2024-04-15 12:58 ` Patchwork
2024-04-15 13:08 ` ✗ Fi.CI.BAT: failure " Patchwork
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2024-04-15 12:58 UTC (permalink / raw)
To: Animesh Manna; +Cc: intel-gfx
== Series Details ==
Series: Link off between frames for edp (rev2)
URL : https://patchwork.freedesktop.org/series/130650/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✗ Fi.CI.BAT: failure for Link off between frames for edp (rev2)
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
` (7 preceding siblings ...)
2024-04-15 12:58 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2024-04-15 13:08 ` Patchwork
8 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2024-04-15 13:08 UTC (permalink / raw)
To: Animesh Manna; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 11899 bytes --]
== Series Details ==
Series: Link off between frames for edp (rev2)
URL : https://patchwork.freedesktop.org/series/130650/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_14572 -> Patchwork_130650v2
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_130650v2 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_130650v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/index.html
Participating hosts (40 -> 38)
------------------------------
Additional (4): fi-blb-e6850 bat-kbl-2 fi-cfl-8109u bat-mtlp-6
Missing (6): bat-dg1-7 fi-snb-2520m fi-glk-j4005 fi-kbl-8809g fi-elk-e7500 bat-mtlp-8
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_130650v2:
### IGT changes ###
#### Possible regressions ####
* igt@debugfs_test@read_all_entries:
- fi-ivb-3770: [PASS][1] -> [ABORT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14572/fi-ivb-3770/igt@debugfs_test@read_all_entries.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/fi-ivb-3770/igt@debugfs_test@read_all_entries.html
- fi-ilk-650: [PASS][3] -> [ABORT][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14572/fi-ilk-650/igt@debugfs_test@read_all_entries.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/fi-ilk-650/igt@debugfs_test@read_all_entries.html
- fi-blb-e6850: NOTRUN -> [ABORT][5]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/fi-blb-e6850/igt@debugfs_test@read_all_entries.html
- fi-pnv-d510: [PASS][6] -> [ABORT][7]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14572/fi-pnv-d510/igt@debugfs_test@read_all_entries.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/fi-pnv-d510/igt@debugfs_test@read_all_entries.html
Known issues
------------
Here are the changes found in Patchwork_130650v2 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@debugfs_test@basic-hwmon:
- bat-mtlp-6: NOTRUN -> [SKIP][8] ([i915#9318])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@debugfs_test@basic-hwmon.html
* igt@fbdev@info:
- bat-kbl-2: NOTRUN -> [SKIP][9] ([i915#1849])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-kbl-2/igt@fbdev@info.html
- bat-mtlp-6: NOTRUN -> [SKIP][10] ([i915#1849] / [i915#2582])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@fbdev@info.html
* igt@fbdev@write:
- bat-mtlp-6: NOTRUN -> [SKIP][11] ([i915#2582]) +3 other tests skip
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@fbdev@write.html
* igt@gem_huc_copy@huc-copy:
- fi-cfl-8109u: NOTRUN -> [SKIP][12] ([i915#2190])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/fi-cfl-8109u/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@parallel-random-engines:
- bat-kbl-2: NOTRUN -> [SKIP][13] +39 other tests skip
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-kbl-2/igt@gem_lmem_swapping@parallel-random-engines.html
* igt@gem_lmem_swapping@verify-random:
- bat-mtlp-6: NOTRUN -> [SKIP][14] ([i915#4613]) +3 other tests skip
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@gem_lmem_swapping@verify-random.html
- fi-cfl-8109u: NOTRUN -> [SKIP][15] ([i915#4613]) +3 other tests skip
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/fi-cfl-8109u/igt@gem_lmem_swapping@verify-random.html
* igt@gem_mmap@basic:
- bat-mtlp-6: NOTRUN -> [SKIP][16] ([i915#4083])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@gem_mmap@basic.html
* igt@gem_tiled_blits@basic:
- bat-mtlp-6: NOTRUN -> [SKIP][17] ([i915#4077]) +2 other tests skip
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@gem_tiled_blits@basic.html
* igt@gem_tiled_pread_basic:
- bat-mtlp-6: NOTRUN -> [SKIP][18] ([i915#4079]) +1 other test skip
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@gem_tiled_pread_basic.html
* igt@i915_pm_rps@basic-api:
- bat-mtlp-6: NOTRUN -> [SKIP][19] ([i915#6621])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@i915_pm_rps@basic-api.html
* igt@i915_selftest@live@guc_hang:
- bat-dg2-9: [PASS][20] -> [ABORT][21] ([i915#10366])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14572/bat-dg2-9/igt@i915_selftest@live@guc_hang.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-dg2-9/igt@i915_selftest@live@guc_hang.html
* igt@kms_addfb_basic@addfb25-x-tiled-legacy:
- bat-mtlp-6: NOTRUN -> [SKIP][22] ([i915#4212] / [i915#9792]) +8 other tests skip
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@kms_addfb_basic@addfb25-x-tiled-legacy.html
* igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
- bat-mtlp-6: NOTRUN -> [SKIP][23] ([i915#5190] / [i915#9792])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html
* igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
- bat-mtlp-6: NOTRUN -> [SKIP][24] ([i915#9792]) +17 other tests skip
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
* igt@kms_flip@basic-flip-vs-dpms:
- bat-mtlp-6: NOTRUN -> [SKIP][25] ([i915#3637] / [i915#9792]) +3 other tests skip
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@kms_flip@basic-flip-vs-dpms.html
* igt@kms_force_connector_basic@prune-stale-modes:
- bat-mtlp-6: NOTRUN -> [SKIP][26] ([i915#5274] / [i915#9792])
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@kms_force_connector_basic@prune-stale-modes.html
* igt@kms_frontbuffer_tracking@basic:
- bat-mtlp-6: NOTRUN -> [SKIP][27] ([i915#4342] / [i915#5354] / [i915#9792])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@kms_frontbuffer_tracking@basic.html
* igt@kms_pm_backlight@basic-brightness:
- fi-cfl-8109u: NOTRUN -> [SKIP][28] +11 other tests skip
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/fi-cfl-8109u/igt@kms_pm_backlight@basic-brightness.html
- bat-mtlp-6: NOTRUN -> [SKIP][29] ([i915#5354] / [i915#9792])
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@kms_pm_backlight@basic-brightness.html
* igt@kms_psr@psr-cursor-plane-move:
- bat-mtlp-6: NOTRUN -> [SKIP][30] ([i915#1072] / [i915#9673] / [i915#9732] / [i915#9792]) +3 other tests skip
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@kms_psr@psr-cursor-plane-move.html
* igt@kms_setmode@basic-clone-single-crtc:
- bat-mtlp-6: NOTRUN -> [SKIP][31] ([i915#3555] / [i915#8809] / [i915#9792])
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-fence-flip:
- bat-mtlp-6: NOTRUN -> [SKIP][32] ([i915#3708] / [i915#9792])
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@prime_vgem@basic-fence-flip.html
* igt@prime_vgem@basic-fence-mmap:
- bat-mtlp-6: NOTRUN -> [SKIP][33] ([i915#3708] / [i915#4077]) +1 other test skip
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@prime_vgem@basic-fence-mmap.html
* igt@prime_vgem@basic-read:
- bat-mtlp-6: NOTRUN -> [SKIP][34] ([i915#3708]) +1 other test skip
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@prime_vgem@basic-read.html
* igt@prime_vgem@basic-write:
- bat-mtlp-6: NOTRUN -> [SKIP][35] ([i915#10216] / [i915#3708])
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-mtlp-6/igt@prime_vgem@basic-write.html
#### Possible fixes ####
* igt@gem_lmem_swapping@basic@lmem0:
- bat-dg2-11: [FAIL][36] ([i915#10378]) -> [PASS][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14572/bat-dg2-11/igt@gem_lmem_swapping@basic@lmem0.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/bat-dg2-11/igt@gem_lmem_swapping@basic@lmem0.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#10216]: https://gitlab.freedesktop.org/drm/intel/issues/10216
[i915#10366]: https://gitlab.freedesktop.org/drm/intel/issues/10366
[i915#10378]: https://gitlab.freedesktop.org/drm/intel/issues/10378
[i915#10435]: https://gitlab.freedesktop.org/drm/intel/issues/10435
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4342]: https://gitlab.freedesktop.org/drm/intel/issues/4342
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
[i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
[i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#8809]: https://gitlab.freedesktop.org/drm/intel/issues/8809
[i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318
[i915#9673]: https://gitlab.freedesktop.org/drm/intel/issues/9673
[i915#9732]: https://gitlab.freedesktop.org/drm/intel/issues/9732
[i915#9792]: https://gitlab.freedesktop.org/drm/intel/issues/9792
Build changes
-------------
* Linux: CI_DRM_14572 -> Patchwork_130650v2
CI-20190529: 20190529
CI_DRM_14572: 85eef611f85be84edeabab83debdbb1fabeba066 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7806: 849cd963ce7e8222dcf17cc872d355181fd2c2a2 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_130650v2: 85eef611f85be84edeabab83debdbb1fabeba066 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
389189a76134 drm/i915/alpm: Add debugfs for LOBF
36b4755e89e8 drm/i915/alpm: Enable lobf from source in ALPM_CTL
fdda9f08cbac drm/i915/alpm: Add compute config for lobf
43391fc29c14 drm/display: Add missing aux less alpm wake related bits
19827e928f7a drm/i915/alpm: Move alpm related code to a new file
64f144755fba drm/i915/alpm: Move alpm parameters from intel_psr
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130650v2/index.html
[-- Attachment #2: Type: text/html, Size: 14280 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
2024-04-15 10:06 ` Hogander, Jouni
@ 2024-04-16 8:15 ` Manna, Animesh
2024-04-16 9:06 ` Hogander, Jouni
0 siblings, 1 reply; 26+ messages in thread
From: Manna, Animesh @ 2024-04-16 8:15 UTC (permalink / raw)
To: Hogander, Jouni, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Murthy, Arun R, Nikula, Jani
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Monday, April 15, 2024 3:36 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> Subject: Re: [PATCH 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 <animesh.manna@intel.com>
> > ---
> > 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 @@
> >
> > struct intel_dp;
> > struct intel_crtc_state;
> > +struct drm_connector_state;
> >
> > +bool intel_dp_get_aux_less_alpm_status(struct intel_dp *intel_dp);
> > 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
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 73197f014510..6116c383b543 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1876,10 +1876,14 @@ struct intel_dp {
> > u8 fast_wake_lines;
> >
> > /* LNL and beyond */
> > + bool auxless_alpm_supported;
> > u8 check_entry_lines;
> > u8 silence_period_sym_clocks;
> > u8 lfps_half_cycle_num_of_syms;
> > } alpm_parameters;
> > +
> > + /* LOBF flags*/
> > + bool lobf_supported;
>
> I think having it here and naming like this is misleading. How about
> intel_crtc_state->has_lobf?
How about "bool lobf_entry_criteria" ?
>
> > };
> >
> > enum lspcon_vendor {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 163da48bc406..12ec796568d9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -48,6 +48,7 @@
> > #include "i915_drv.h"
> > #include "i915_irq.h"
> > #include "i915_reg.h"
> > +#include "intel_alpm.h"
> > #include "intel_atomic.h"
> > #include "intel_audio.h"
> > #include "intel_backlight.h"
> > @@ -3001,6 +3002,7 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> > intel_vrr_compute_config(pipe_config, conn_state);
> > intel_dp_compute_as_sdp(intel_dp, pipe_config);
> > intel_psr_compute_config(intel_dp, pipe_config, conn_state);
> > + intel_alpm_compute_lobf_config(intel_dp, pipe_config,
> > conn_state);
> > intel_dp_drrs_compute_config(connector, pipe_config,
> > link_bpp_x16);
> > intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp,
> > pipe_config, conn_state);
> > @@ -6616,6 +6618,9 @@ static bool intel_edp_init_connector(struct
> > intel_dp *intel_dp,
> >
> > intel_pps_init_late(intel_dp);
> >
> > + if (intel_dp_get_aux_less_alpm_status(intel_dp))
> > + intel_dp->alpm_parameters.auxless_alpm_supported =
> > true;
> > +
>
> intel_dp->alpm_parameters.auxless_alpm_supported =
> intel_dp_get_aux_less_alpm_status(intel_dp);
Ok.
Regards,
Animesh
>
> BR,
>
> Jouni Högander
>
> > return true;
> >
> > out_vdd_off:
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL
2024-04-15 10:09 ` Hogander, Jouni
@ 2024-04-16 8:20 ` Manna, Animesh
2024-04-16 9:10 ` Hogander, Jouni
0 siblings, 1 reply; 26+ messages in thread
From: Manna, Animesh @ 2024-04-16 8:20 UTC (permalink / raw)
To: Hogander, Jouni, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Murthy, Arun R, Nikula, Jani
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Monday, April 15, 2024 3:39 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> Subject: Re: [PATCH 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 <animesh.manna@intel.com>
> > ---
> > 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 {
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
2024-04-15 10:14 ` Hogander, Jouni
@ 2024-04-16 8:21 ` Manna, Animesh
0 siblings, 0 replies; 26+ messages in thread
From: Manna, Animesh @ 2024-04-16 8:21 UTC (permalink / raw)
To: Hogander, Jouni, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Murthy, Arun R, Nikula, Jani
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Monday, April 15, 2024 3:44 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> Subject: Re: [PATCH 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 <animesh.manna@intel.com>
> > ---
> > 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 ||
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a new file
2024-04-15 11:46 ` Jani Nikula
@ 2024-04-16 8:22 ` Manna, Animesh
0 siblings, 0 replies; 26+ messages in thread
From: Manna, Animesh @ 2024-04-16 8:22 UTC (permalink / raw)
To: Nikula, Jani, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Hogander, Jouni, Murthy, Arun R
> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Monday, April 15, 2024 5:17 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni
> <jouni.hogander@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>;
> Manna, Animesh <animesh.manna@intel.com>
> Subject: Re: [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a
> new file
>
> On Fri, 12 Apr 2024, Animesh Manna <animesh.manna@intel.com> wrote:
> > Move ALPM feature related code as it will be used for non-psr panel
> > also thorugh LOBF feature.
> >
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
2024-04-15 11:47 ` Jani Nikula
@ 2024-04-16 8:24 ` Manna, Animesh
0 siblings, 0 replies; 26+ messages in thread
From: Manna, Animesh @ 2024-04-16 8:24 UTC (permalink / raw)
To: Nikula, Jani, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Hogander, Jouni, Murthy, Arun R
> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Monday, April 15, 2024 5:18 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni
> <jouni.hogander@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>;
> Manna, Animesh <animesh.manna@intel.com>
> Subject: Re: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
>
> On Fri, 12 Apr 2024, Animesh Manna <animesh.manna@intel.com> 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL
2024-04-15 11:48 ` Jani Nikula
@ 2024-04-16 8:36 ` Manna, Animesh
0 siblings, 0 replies; 26+ messages in thread
From: Manna, Animesh @ 2024-04-16 8:36 UTC (permalink / raw)
To: Nikula, Jani, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Hogander, Jouni, Murthy, Arun R
> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Monday, April 15, 2024 5:19 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni
> <jouni.hogander@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>;
> Manna, Animesh <animesh.manna@intel.com>
> Subject: Re: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in
> ALPM_CTL
>
> On Fri, 12 Apr 2024, Animesh Manna <animesh.manna@intel.com> wrote:
> > Set the Link Off Between Frames Enable bit in ALPM_CTL register.
> >
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
2024-04-15 11:53 ` Jani Nikula
@ 2024-04-16 8:43 ` Manna, Animesh
0 siblings, 0 replies; 26+ messages in thread
From: Manna, Animesh @ 2024-04-16 8:43 UTC (permalink / raw)
To: Nikula, Jani, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Hogander, Jouni, Murthy, Arun R
> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Monday, April 15, 2024 5:23 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni
> <jouni.hogander@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>;
> Manna, Animesh <animesh.manna@intel.com>
> Subject: Re: [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF
>
> On Fri, 12 Apr 2024, Animesh Manna <animesh.manna@intel.com> wrote:
> > For validation purpose add debugfs for LOBF.
> >
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> > 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
> > 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 ||
>
> --
> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf
2024-04-16 8:15 ` Manna, Animesh
@ 2024-04-16 9:06 ` Hogander, Jouni
0 siblings, 0 replies; 26+ messages in thread
From: Hogander, Jouni @ 2024-04-16 9:06 UTC (permalink / raw)
To: Manna, Animesh, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Murthy, Arun R, Nikula, Jani
On Tue, 2024-04-16 at 08:15 +0000, Manna, Animesh wrote:
>
>
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander@intel.com>
> > Sent: Monday, April 15, 2024 3:36 PM
> > To: Manna, Animesh <animesh.manna@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> > Subject: Re: [PATCH 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 <animesh.manna@intel.com>
> > > ---
> > > 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 @@
> > >
> > > struct intel_dp;
> > > struct intel_crtc_state;
> > > +struct drm_connector_state;
> > >
> > > +bool intel_dp_get_aux_less_alpm_status(struct intel_dp
> > > *intel_dp);
> > > 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
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 73197f014510..6116c383b543 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1876,10 +1876,14 @@ struct intel_dp {
> > > u8 fast_wake_lines;
> > >
> > > /* LNL and beyond */
> > > + bool auxless_alpm_supported;
> > > u8 check_entry_lines;
> > > u8 silence_period_sym_clocks;
> > > u8 lfps_half_cycle_num_of_syms;
> > > } alpm_parameters;
> > > +
> > > + /* LOBF flags*/
> > > + bool lobf_supported;
> >
> > I think having it here and naming like this is misleading. How
> > about
> > intel_crtc_state->has_lobf?
>
> How about "bool lobf_entry_criteria" ?
My mind is polluted with PSR code: I would have chosen has_lobf. I'm
not against lobf_entry_criteria.
BR,
Jouni Högander
>
> >
> > > };
> > >
> > > enum lspcon_vendor {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 163da48bc406..12ec796568d9 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -48,6 +48,7 @@
> > > #include "i915_drv.h"
> > > #include "i915_irq.h"
> > > #include "i915_reg.h"
> > > +#include "intel_alpm.h"
> > > #include "intel_atomic.h"
> > > #include "intel_audio.h"
> > > #include "intel_backlight.h"
> > > @@ -3001,6 +3002,7 @@ intel_dp_compute_config(struct
> > > intel_encoder
> > > *encoder,
> > > intel_vrr_compute_config(pipe_config, conn_state);
> > > intel_dp_compute_as_sdp(intel_dp, pipe_config);
> > > intel_psr_compute_config(intel_dp, pipe_config,
> > > conn_state);
> > > + intel_alpm_compute_lobf_config(intel_dp, pipe_config,
> > > conn_state);
> > > intel_dp_drrs_compute_config(connector, pipe_config,
> > > link_bpp_x16);
> > > intel_dp_compute_vsc_sdp(intel_dp, pipe_config,
> > > conn_state);
> > > intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp,
> > > pipe_config, conn_state);
> > > @@ -6616,6 +6618,9 @@ static bool intel_edp_init_connector(struct
> > > intel_dp *intel_dp,
> > >
> > > intel_pps_init_late(intel_dp);
> > >
> > > + if (intel_dp_get_aux_less_alpm_status(intel_dp))
> > > + intel_dp->alpm_parameters.auxless_alpm_supported
> > > =
> > > true;
> > > +
> >
> > intel_dp->alpm_parameters.auxless_alpm_supported =
> > intel_dp_get_aux_less_alpm_status(intel_dp);
>
> Ok.
>
> Regards,
> Animesh
>
> >
> > BR,
> >
> > Jouni Högander
> >
> > > return true;
> > >
> > > out_vdd_off:
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL
2024-04-16 8:20 ` Manna, Animesh
@ 2024-04-16 9:10 ` Hogander, Jouni
0 siblings, 0 replies; 26+ messages in thread
From: Hogander, Jouni @ 2024-04-16 9:10 UTC (permalink / raw)
To: Manna, Animesh, intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, Murthy, Arun R, Nikula, Jani
On Tue, 2024-04-16 at 08:20 +0000, Manna, Animesh wrote:
>
>
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander@intel.com>
> > Sent: Monday, April 15, 2024 3:39 PM
> > To: Manna, Animesh <animesh.manna@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> > Subject: Re: [PATCH 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 <animesh.manna@intel.com>
> > > ---
> > > 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.
I might be missing something. E.g. in case of aux_less_alpm PR
lnl_alpm_configure is called from intel_psr_enable_source. Where it is
called for lobf case?
BR,
Jouni Högander
>
> Regards,
> Animesh
>
> >
> > BR,
> >
> > Jouni Högander
> >
> > > alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(intel_dp-
> > > > alpm_parameters.check_entry_lines);
> > >
> > > intel_de_write(dev_priv, ALPM_CTL(cpu_transcoder),
> > > alpm_ctl);
> > > 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 {
>
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2024-04-16 9:10 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-12 15:52 [PATCH v2 0/6] Link off between frames for edp Animesh Manna
2024-04-12 15:52 ` [PATCH v2 1/6] drm/i915/alpm: Move alpm parameters from intel_psr Animesh Manna
2024-04-12 15:52 ` [PATCH v2 2/6] drm/i915/alpm: Move alpm related code to a new file Animesh Manna
2024-04-15 11:46 ` Jani Nikula
2024-04-16 8:22 ` Manna, Animesh
2024-04-12 15:52 ` [PATCH v2 3/6] drm/display: Add missing aux less alpm wake related bits Animesh Manna
2024-04-12 15:52 ` [PATCH v2 4/6] drm/i915/alpm: Add compute config for lobf Animesh Manna
2024-04-15 10:06 ` Hogander, Jouni
2024-04-16 8:15 ` Manna, Animesh
2024-04-16 9:06 ` Hogander, Jouni
2024-04-15 11:47 ` Jani Nikula
2024-04-16 8:24 ` Manna, Animesh
2024-04-12 15:52 ` [PATCH v2 5/6] drm/i915/alpm: Enable lobf from source in ALPM_CTL Animesh Manna
2024-04-15 10:09 ` Hogander, Jouni
2024-04-16 8:20 ` Manna, Animesh
2024-04-16 9:10 ` Hogander, Jouni
2024-04-15 11:48 ` Jani Nikula
2024-04-16 8:36 ` Manna, Animesh
2024-04-12 15:52 ` [PATCH v2 6/6] drm/i915/alpm: Add debugfs for LOBF Animesh Manna
2024-04-15 10:14 ` Hogander, Jouni
2024-04-16 8:21 ` Manna, Animesh
2024-04-15 11:53 ` Jani Nikula
2024-04-16 8:43 ` Manna, Animesh
2024-04-15 12:58 ` ✗ Fi.CI.CHECKPATCH: warning for Link off between frames for edp (rev2) Patchwork
2024-04-15 12:58 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-04-15 13:08 ` ✗ Fi.CI.BAT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.