[Intel-gfx] [PATCH 00/11] VRR/Adaptive Sync enabling in i915

[Intel-gfx] [PATCH 00/11] VRR/Adaptive Sync enabling in i915

[Manasi Navare]

This patch series adds support for DP 1.4 feature of
Adaptive Sync also called as Variable Refresh rate
which is used to match the display rate with the render rate
by stretching or shrinking the blanking time of the frame.


Aditya Swarup (1):
  drm/i915/display/dp: Attach and set drm connector VRR property

Manasi Navare (10):
  drm/i915: Add REG_FIELD_PREP to VRR register def
  drm/i915/display/vrr: Create VRR file and add VRR capability check
  drm/i915/display/dp: Add VRR crtc state variables
  drm/i915/display/dp: Compute VRR state in atomic_check
  drm/i915/display/dp: Do not enable PSR if VRR is enabled
  drm/i915/display/vrr: Configure and enable VRR in modeset enable
  drm/i915/display/vrr: Send VRR push to flip the frame
  drm/i915/display/vrr: Disable VRR in modeset disable path
  drm/i915/display/vrr: Set IGNORE_MSA_PAR state in DP Sink
  drm/i915/display: Add HW state readout for VRR

 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/display/intel_ddi.c      |  32 ++++
 drivers/gpu/drm/i915/display/intel_display.c  |  11 +-
 .../drm/i915/display/intel_display_types.h    |   7 +
 drivers/gpu/drm/i915/display/intel_dp.c       |   9 +
 drivers/gpu/drm/i915/display/intel_dp.h       |   1 +
 drivers/gpu/drm/i915/display/intel_psr.c      |   7 +
 drivers/gpu/drm/i915/display/intel_sprite.c   |   5 +
 drivers/gpu/drm/i915/display/intel_vrr.c      | 160 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_vrr.h      |  27 +++
 drivers/gpu/drm/i915/i915_reg.h               |   1 +
 11 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.h

-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 01/11] drm/i915: Add REG_FIELD_PREP to VRR register def

[Manasi Navare]

VRR_CTL register onloy had a GENMASK but no field prep
define for TRANS_VRR_CTL_LINE_COUNT field so add that

Cc: Aditya Swarup <aditya.swarup@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d4952c9875fb..9792c931b4c5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4348,6 +4348,7 @@ enum {
 #define   VRR_CTL_IGN_MAX_SHIFT		REG_BIT(30)
 #define   VRR_CTL_FLIP_LINE_EN		REG_BIT(29)
 #define   VRR_CTL_LINE_COUNT_MASK	REG_GENMASK(10, 3)
+#define   VRR_CTL_LINE_COUNT(x)		REG_FIELD_PREP(VRR_CTL_LINE_COUNT_MASK, (x))
 #define   VRR_CTL_SW_FULLLINE_COUNT	REG_BIT(0)
 
 #define _TRANS_VRR_VMAX_A		0x60424
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 02/11] drm/i915/display/vrr: Create VRR file and add VRR capability check

[Manasi Navare]

We create a new file for all VRR related helpers.
Also add a function to check vrr capability based on
platform support, DPCD bits and EDID monitor range.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/Makefile            |  1 +
 drivers/gpu/drm/i915/display/intel_vrr.c | 28 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_vrr.h | 19 ++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index e5574e506a5c..3beeaf517191 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -249,6 +249,7 @@ i915-y += \
 	display/intel_sdvo.o \
 	display/intel_tv.o \
 	display/intel_vdsc.o \
+	display/intel_vrr.o \
 	display/vlv_dsi.o \
 	display/vlv_dsi_pll.o
 
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
new file mode 100644
index 000000000000..0c8a91fabb64
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2020 Intel Corporation
+ *
+ * Author: Manasi Navare <manasi.d.navare@intel.com>
+ */
+
+#include "i915_drv.h"
+#include "intel_display_types.h"
+#include "intel_vrr.h"
+
+bool intel_is_vrr_capable(struct drm_connector *connector)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
+	const struct drm_display_info *info = &connector->display_info;
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+
+	/*
+	 * DP Sink is capable of Variable refresh video timings if
+	 * Ignore MSA bit is set in DPCD.
+	 * EDID monitor range also should be atleast 10 for reasonable
+	 * Adaptive sync/ VRR end user experience.
+	 */
+	return INTEL_GEN(dev_priv) >= 12 &&
+		drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd) &&
+		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
+}
+
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
new file mode 100644
index 000000000000..755746c7525c
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+*/
+
+#ifndef __INTEL_VRR_H__
+#define __INTEL_VRR_H__
+
+#include <linux/types.h>
+
+struct drm_connector;
+struct drm_i915_private;
+struct intel_crtc_state;
+struct intel_encoder;
+struct intel_dp;
+
+bool intel_is_vrr_capable(struct drm_connector *connector);
+
+#endif /* __INTEL_VRR_H__ */
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 03/11] drm/i915/display/dp: Attach and set drm connector VRR property

[Manasi Navare]

From: Aditya Swarup <aditya.swarup@intel.com>

This function sets the VRR property for connector based
on the platform support, EDID monitor range and DP sink
DPCD capability of outputing video without msa
timing information.

v7:
* Move the helper to separate file (Manasi)
v6:
* Remove unset of prop
v5:
* Fix the vrr prop not being set in kernel (Manasi)
* Unset the prop on connector disconnect (Manasi)
v4:
* Rebase (Mansi)
v3:
* intel_dp_is_vrr_capable can be used for debugfs, make it
non static (Manasi)
v2:
* Just set this in intel_dp_get_modes instead of new hook (Jani)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++++
 drivers/gpu/drm/i915/display/intel_dp.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 818daab252f3..3794b8f35edc 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -62,6 +62,7 @@
 #include "intel_sideband.h"
 #include "intel_tc.h"
 #include "intel_vdsc.h"
+#include "intel_vrr.h"
 
 #define DP_DPRX_ESI_LEN 14
 
@@ -6622,6 +6623,10 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 	edid = intel_connector->detect_edid;
 	if (edid) {
 		int ret = intel_connector_update_modes(connector, edid);
+
+		if (intel_is_vrr_capable(connector))
+			drm_connector_set_vrr_capable_property(connector,
+							       true);
 		if (ret)
 			return ret;
 	}
@@ -7080,6 +7085,9 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
 
 	}
+
+	if (INTEL_GEN(dev_priv) >= 12)
+		drm_connector_attach_vrr_capable_property(connector);
 }
 
 static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 3f862b4fd34f..aaf0a41582d7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -15,6 +15,7 @@ enum pipe;
 enum port;
 struct drm_connector_state;
 struct drm_encoder;
+struct drm_connector;
 struct drm_i915_private;
 struct drm_modeset_acquire_ctx;
 struct drm_dp_vsc_sdp;
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 04/11] drm/i915/display/dp: Add VRR crtc state variables

[Manasi Navare]

Introduce VRR struct in intel_crtc_state and add
VRR crtc state variables Enable, Vtotalmin and Vtotalmax
to be derived from mode timings and VRR crtc property.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index f6f0626649e0..f6f7ec024da7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1092,6 +1092,13 @@ struct intel_crtc_state {
 	struct intel_dsb *dsb;
 
 	u32 psr2_man_track_ctl;
+
+	/* Variable Refresh Rate state */
+	struct {
+		bool enable;
+		u16 vtotalmin;
+		u16 vtotalmax;
+	} vrr;
 };
 
 enum intel_pipe_crc_source {
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 05/11] drm/i915/display/dp: Compute VRR state in atomic_check

[Manasi Navare]

This forces a complete modeset if vrr drm crtc state goes
from enabled to disabled and vice versa.
This patch also computes vrr state variables from the mode timings
and based on the vrr property set by userspace as well as hardware's
vrr capability.

v2:
*Rebase
v3:
* Vmin = max (vtotal, vmin) (Manasi)
v4:
* set crtc_state->vrr.enable = 0 for disable request

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  7 +++-
 drivers/gpu/drm/i915/display/intel_dp.c      |  1 +
 drivers/gpu/drm/i915/display/intel_vrr.c     | 38 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_vrr.h     |  2 ++
 4 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f41b6f8b5618..f70cc3b2a1a4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14213,6 +14213,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 
 	PIPE_CONF_CHECK_I(mst_master_transcoder);
 
+	PIPE_CONF_CHECK_BOOL(vrr.enable);
+	PIPE_CONF_CHECK_I(vrr.vtotalmin);
+	PIPE_CONF_CHECK_I(vrr.vtotalmax);
+
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
 #undef PIPE_CONF_CHECK_BOOL
@@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev,
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (new_crtc_state->inherited != old_crtc_state->inherited)
+		if (new_crtc_state->inherited != old_crtc_state->inherited ||
+		    new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled)
 			new_crtc_state->uapi.mode_changed = true;
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 3794b8f35edc..3185c4ca523d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2752,6 +2752,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	if (!HAS_DDI(dev_priv))
 		intel_dp_set_clock(encoder, pipe_config);
 
+	intel_vrr_compute_config(intel_dp, pipe_config);
 	intel_psr_compute_config(intel_dp, pipe_config);
 	intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp,
 				     constant_n);
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 0c8a91fabb64..56114f535f94 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -26,3 +26,41 @@ bool intel_is_vrr_capable(struct drm_connector *connector)
 		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
 }
 
+void
+intel_vrr_compute_config(struct intel_dp *intel_dp,
+			 struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	struct drm_connector *connector = &intel_connector->base;
+	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+	const struct drm_display_info *info = &connector->display_info;
+
+	if (!intel_is_vrr_capable(connector))
+		return;
+
+	if (!crtc_state->uapi.vrr_enabled) {
+		drm_dbg(&dev_priv->drm,
+			"VRR disable requested by Userspace\n");
+		crtc_state->vrr.enable = false;
+		return;
+	}
+
+	crtc_state->vrr.enable = true;
+	crtc_state->vrr.vtotalmin =
+		max_t(u16, adjusted_mode->crtc_vtotal,
+		      DIV_ROUND_CLOSEST(adjusted_mode->crtc_clock * 1000,
+					adjusted_mode->crtc_htotal *
+					info->monitor_range.max_vfreq));
+	crtc_state->vrr.vtotalmax =
+		max_t(u16, adjusted_mode->crtc_vtotal,
+		      DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
+				   adjusted_mode->crtc_htotal *
+				   info->monitor_range.min_vfreq));
+
+	drm_dbg(&dev_priv->drm,
+		"VRR Config: Enable = %s Vtotal Min = %d Vtotal Max = %d\n",
+		 yesno(crtc_state->vrr.enable), crtc_state->vrr.vtotalmin,
+		 crtc_state->vrr.vtotalmax);
+}
+
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
index 755746c7525c..1e6fe8fe92ec 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -15,5 +15,7 @@ struct intel_encoder;
 struct intel_dp;
 
 bool intel_is_vrr_capable(struct drm_connector *connector);
+void intel_vrr_compute_config(struct intel_dp *intel_dp,
+			      struct intel_crtc_state *crtc_state);
 
 #endif /* __INTEL_VRR_H__ */
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 06/11] drm/i915/display/dp: Do not enable PSR if VRR is enabled

[Manasi Navare]

Even though our HW supports PSR + VRR, the available panels
do not work reliably with PSR and VRR together. So if user
requested VRR and is supported by HW enable that and do not
enable PSR in that case.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index a591a475f148..56d3fbfad719 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -811,6 +811,13 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 		&crtc_state->hw.adjusted_mode;
 	int psr_setup_time;
 
+	/*
+	 * Current PSR panels dont work reliably with VRR enabled
+	 * So if VRR is enabled, do not enable PSR.
+	 */
+	if (crtc_state->vrr.enable)
+		return;
+
 	if (!CAN_PSR(dev_priv))
 		return;
 
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 07/11] drm/i915/display/vrr: Configure and enable VRR in modeset enable

[Manasi Navare]

This patch computes the VRR parameters from VRR crtc states
and configures them in VRR registers during CRTC enable in
the modeset enable sequence.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c |  5 ++++
 drivers/gpu/drm/i915/display/intel_vrr.c | 38 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_vrr.h |  2 ++
 3 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 09811be08cfe..391c51979334 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -49,6 +49,7 @@
 #include "intel_sprite.h"
 #include "intel_tc.h"
 #include "intel_vdsc.h"
+#include "intel_vrr.h"
 
 struct ddi_buf_trans {
 	u32 trans1;	/* balance leg enable, de-emph level */
@@ -4046,6 +4047,10 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
 
 	intel_ddi_enable_transcoder_func(encoder, crtc_state);
 
+	/* Enable VRR if requested through CRTC property */
+	if (crtc_state->vrr.enable)
+		intel_vrr_enable(encoder, crtc_state);
+
 	intel_enable_pipe(crtc_state);
 
 	intel_crtc_vblank_on(crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 56114f535f94..7f1353bac583 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -64,3 +64,41 @@ intel_vrr_compute_config(struct intel_dp *intel_dp,
 		 crtc_state->vrr.vtotalmax);
 }
 
+void intel_vrr_enable(struct intel_encoder *encoder,
+		      const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	enum pipe pipe = crtc->pipe;
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->hw.adjusted_mode;
+	u32 trans_vrr_ctl = 0, trans_vrr_vmax = 0, trans_vrr_vmin = 0, trans_vrr_flipline = 0, trans_push = 0;
+	u16 framestart_to_pipelinefull_linecnt = 0;
+
+	framestart_to_pipelinefull_linecnt =
+		min_t(u16, 255, (crtc_state->vrr.vtotalmin - adjusted_mode->crtc_vdisplay));
+
+	trans_vrr_ctl = VRR_CTL_VRR_ENABLE |  VRR_CTL_IGN_MAX_SHIFT |
+		VRR_CTL_FLIP_LINE_EN | VRR_CTL_LINE_COUNT(framestart_to_pipelinefull_linecnt) |
+		VRR_CTL_SW_FULLLINE_COUNT;
+
+	/* Programming adjustments for 0 based regs */
+	trans_vrr_vmax = crtc_state->vrr.vtotalmax - 1;
+	trans_vrr_vmin = crtc_state->vrr.vtotalmin - 1;
+	trans_vrr_flipline = crtc_state->vrr.vtotalmin - 1;
+
+	trans_push = TRANS_PUSH_EN;
+
+	intel_de_write(dev_priv, TRANS_VRR_VMIN(pipe), trans_vrr_vmin);
+	intel_de_write(dev_priv, TRANS_VRR_VMAX(pipe), trans_vrr_vmax);
+	intel_de_write(dev_priv, TRANS_VRR_CTL(pipe), trans_vrr_ctl);
+	intel_de_write(dev_priv, TRANS_VRR_FLIPLINE(pipe), trans_vrr_flipline);
+	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
+
+	drm_dbg(&dev_priv->drm,	"Enabling VRR on pipe (%c)\n", pipe_name(pipe));
+	drm_dbg(&dev_priv->drm, "VRR Parameters: Vblank - Min = %d, Max = %d Flipline Count = %d, CTL Reg = 0x%08x, TRANS PUSH reg = 0x%08x",
+		crtc_state->vrr.vtotalmin, crtc_state->vrr.vtotalmax,
+		crtc_state->vrr.vtotalmin, trans_vrr_ctl,
+		trans_push);
+}
+
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
index 1e6fe8fe92ec..05d982d6fbae 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -17,5 +17,7 @@ struct intel_dp;
 bool intel_is_vrr_capable(struct drm_connector *connector);
 void intel_vrr_compute_config(struct intel_dp *intel_dp,
 			      struct intel_crtc_state *crtc_state);
+void intel_vrr_enable(struct intel_encoder *encoder,
+		      const struct intel_crtc_state *crtc_state);
 
 #endif /* __INTEL_VRR_H__ */
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 08/11] drm/i915/display/vrr: Send VRR push to flip the frame

[Manasi Navare]

VRR achieves vblank stretching using the HW PUSH functionality.
So once the VRR is enabled during modeset then for each flip
request from userspace, in the atomic tail pipe_update_end()
we need to set the VRR push bit in HW for it to terminate
the vblank at configured flipline or anytime after flipline
or latest at the Vmax.

The HW clears the PUSH bit after the double buffer updates
are completed.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_sprite.c |  5 +++++
 drivers/gpu/drm/i915/display/intel_vrr.c    | 17 +++++++++++++++++
 drivers/gpu/drm/i915/display/intel_vrr.h    |  1 +
 3 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index b6deeb338477..cb10fe462f06 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -49,6 +49,7 @@
 #include "intel_psr.h"
 #include "intel_dsi.h"
 #include "intel_sprite.h"
+#include "intel_vrr.h"
 
 int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
 			     int usecs)
@@ -217,6 +218,10 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 	    intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI))
 		icl_dsi_frame_update(new_crtc_state);
 
+	/* Send VRR Push to terminate Vblank */
+	if (new_crtc_state->vrr.enable)
+		intel_vrr_send_push(new_crtc_state);
+
 	/* We're still in the vblank-evade critical section, this can't race.
 	 * Would be slightly nice to just grab the vblank count and arm the
 	 * event outside of the critical section - the spinlock might spin for a
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 7f1353bac583..ec1ce88e869c 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -102,3 +102,20 @@ void intel_vrr_enable(struct intel_encoder *encoder,
 		trans_push);
 }
 
+void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	u32 trans_push;
+
+	trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe));
+	WARN_ON(!(trans_push & TRANS_PUSH_EN));
+
+	trans_push |= TRANS_PUSH_SEND;
+	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
+
+	drm_dbg(&dev_priv->drm, "Sending VRR Push on Pipe (%c)\n",
+		pipe_name(pipe));
+}
+
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
index 05d982d6fbae..a6b78e1676cb 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -19,5 +19,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp,
 			      struct intel_crtc_state *crtc_state);
 void intel_vrr_enable(struct intel_encoder *encoder,
 		      const struct intel_crtc_state *crtc_state);
+void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
 
 #endif /* __INTEL_VRR_H__ */
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 09/11] drm/i915/display/vrr: Disable VRR in modeset disable path

[Manasi Navare]

This patch disables the VRR enable and VRR PUSH
bits in the HW during commit modeset disable sequence.

Thsi disable will happen when the port is disabled
or when the userspace sets VRR prop to false and
requests to disable VRR.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c |  2 ++
 drivers/gpu/drm/i915/display/intel_vrr.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_vrr.h |  1 +
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 391c51979334..565155af3fb9 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3819,6 +3819,8 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
 
 		intel_disable_pipe(old_crtc_state);
 
+		intel_vrr_disable(old_crtc_state);
+
 		intel_ddi_disable_transcoder_func(old_crtc_state);
 
 		intel_dsc_disable(old_crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index ec1ce88e869c..5075ecb9b5a7 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -119,3 +119,25 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
 		pipe_name(pipe));
 }
 
+void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	u32 trans_vrr_ctl = 0, trans_push = 0;
+
+	if (!old_crtc_state->vrr.enable)
+		return;
+
+	trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(pipe));
+	trans_vrr_ctl &= ~(VRR_CTL_FLIP_LINE_EN | VRR_CTL_VRR_ENABLE);
+	intel_de_write(dev_priv, TRANS_VRR_CTL(pipe), trans_vrr_ctl);
+
+	trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe));
+	trans_push &= ~TRANS_PUSH_EN;
+	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
+
+	drm_dbg(&dev_priv->drm, "Disabling VRR on Pipe (%c)\n",
+		pipe_name(pipe));
+}
+
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
index a6b78e1676cb..8c6fd2d1bee5 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -20,5 +20,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp,
 void intel_vrr_enable(struct intel_encoder *encoder,
 		      const struct intel_crtc_state *crtc_state);
 void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
+void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
 
 #endif /* __INTEL_VRR_H__ */
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 10/11] drm/i915/display/vrr: Set IGNORE_MSA_PAR state in DP Sink

[Manasi Navare]

If VRR is enabled, the sink should ignore MSA parameters
and regenerate incoming video stream without depending
on these parameters. Hence set the MSA_TIMING_PAR_IGNORE_EN
bit if VRR is enabled.
Reset this bit on VRR disable.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 565155af3fb9..195449dfec1e 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3322,6 +3322,22 @@ i915_reg_t dp_tp_status_reg(struct intel_encoder *encoder,
 		return DP_TP_STATUS(encoder->port);
 }
 
+static void intel_dp_sink_set_msa_timing_par_ignore_state(struct intel_dp *intel_dp,
+							  const struct intel_crtc_state *crtc_state,
+							  bool enable)
+{
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+	if (!crtc_state->vrr.enable)
+		return;
+
+	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_DOWNSPREAD_CTRL,
+			       enable ? DP_MSA_TIMING_PAR_IGNORE_EN : 0) <= 0)
+		drm_dbg_kms(&i915->drm,
+			    "Failed to set MSA_TIMING_PAR_IGNORE %s in the sink\n",
+			    enable ? "enable" : "disable");
+}
+
 static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
 					const struct intel_crtc_state *crtc_state)
 {
@@ -3493,6 +3509,12 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	 */
 	intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
 
+	/*
+	 * Sink device should ignore MSA parameters and regenerate
+	 * incoming video stream in case of VRR/Adaptive Sync
+	 */
+	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, crtc_state, true);
+
 	/*
 	 * 7.i Follow DisplayPort specification training sequence (see notes for
 	 *     failure handling)
@@ -4089,6 +4111,9 @@ static void intel_disable_ddi_dp(struct intel_atomic_state *state,
 	/* Disable the decompression in DP Sink */
 	intel_dp_sink_set_decompression_state(intel_dp, old_crtc_state,
 					      false);
+	/* Disable Ignore_MSA bit in DP Sink */
+	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, old_crtc_state,
+						      false);
 }
 
 static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 11/11] drm/i915/display: Add HW state readout for VRR

[Manasi Navare]

This functions gets the VRR config from the VRR registers
to match the crtc state variables for VRR.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  4 ++++
 drivers/gpu/drm/i915/display/intel_vrr.c     | 17 +++++++++++++++++
 drivers/gpu/drm/i915/display/intel_vrr.h     |  2 ++
 3 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f70cc3b2a1a4..30904e466cb5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -56,6 +56,7 @@
 #include "display/intel_sdvo.h"
 #include "display/intel_tv.h"
 #include "display/intel_vdsc.h"
+#include "display/intel_vrr.h"
 
 #include "gt/intel_rps.h"
 
@@ -11387,6 +11388,9 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
 		intel_get_transcoder_timings(crtc, pipe_config);
 	}
 
+	if (INTEL_GEN(dev_priv) >= 12)
+		intel_vrr_get_config(crtc, pipe_config);
+
 	intel_get_pipe_src_size(crtc, pipe_config);
 
 	if (IS_HASWELL(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 5075ecb9b5a7..aede2c86cc71 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -141,3 +141,20 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
 		pipe_name(pipe));
 }
 
+void intel_vrr_get_config(struct intel_crtc *crtc,
+			  struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum pipe pipe = crtc->pipe;
+	u32 trans_vrr_ctl;
+
+	trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(pipe));
+	pipe_config->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
+	if (!pipe_config->vrr.enable)
+		return;
+
+	pipe_config->vrr.vtotalmax = intel_de_read(dev_priv, TRANS_VRR_VMAX(pipe)) + 1;
+	pipe_config->vrr.vtotalmin = intel_de_read(dev_priv, TRANS_VRR_VMIN(pipe)) + 1;
+}
+
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
index 8c6fd2d1bee5..dc20359d0821 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -21,5 +21,7 @@ void intel_vrr_enable(struct intel_encoder *encoder,
 		      const struct intel_crtc_state *crtc_state);
 void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
 void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
+void intel_vrr_get_config(struct intel_crtc *crtc,
+			  struct intel_crtc_state *pipe_config);
 
 #endif /* __INTEL_VRR_H__ */
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BUILD: failure for VRR/Adaptive Sync enabling in i915

[Patchwork]

== Series Details ==

Series: VRR/Adaptive Sync enabling in i915
URL   : https://patchwork.freedesktop.org/series/82966/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  HDRTEST drivers/gpu/drm/i915/display/intel_vrr.h
In file included from <command-line>:
./drivers/gpu/drm/i915/display/intel_vrr.h:24:34: error: ‘struct intel_crtc’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
 void intel_vrr_get_config(struct intel_crtc *crtc,
                                  ^~~~~~~~~~
cc1: all warnings being treated as errors
drivers/gpu/drm/i915/Makefile:305: recipe for target 'drivers/gpu/drm/i915/display/intel_vrr.hdrtest' failed
make[4]: *** [drivers/gpu/drm/i915/display/intel_vrr.hdrtest] Error 1
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1784: recipe for target 'drivers' failed
make: *** [drivers] Error 2


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 11/11] drm/i915/display: Add HW state readout for VRR

[Manasi Navare]

This functions gets the VRR config from the VRR registers
to match the crtc state variables for VRR.

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  4 ++++
 drivers/gpu/drm/i915/display/intel_vrr.c     | 17 +++++++++++++++++
 drivers/gpu/drm/i915/display/intel_vrr.h     |  3 +++
 3 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f70cc3b2a1a4..30904e466cb5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -56,6 +56,7 @@
 #include "display/intel_sdvo.h"
 #include "display/intel_tv.h"
 #include "display/intel_vdsc.h"
+#include "display/intel_vrr.h"
 
 #include "gt/intel_rps.h"
 
@@ -11387,6 +11388,9 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
 		intel_get_transcoder_timings(crtc, pipe_config);
 	}
 
+	if (INTEL_GEN(dev_priv) >= 12)
+		intel_vrr_get_config(crtc, pipe_config);
+
 	intel_get_pipe_src_size(crtc, pipe_config);
 
 	if (IS_HASWELL(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 5075ecb9b5a7..aede2c86cc71 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -141,3 +141,20 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
 		pipe_name(pipe));
 }
 
+void intel_vrr_get_config(struct intel_crtc *crtc,
+			  struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum pipe pipe = crtc->pipe;
+	u32 trans_vrr_ctl;
+
+	trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(pipe));
+	pipe_config->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE;
+	if (!pipe_config->vrr.enable)
+		return;
+
+	pipe_config->vrr.vtotalmax = intel_de_read(dev_priv, TRANS_VRR_VMAX(pipe)) + 1;
+	pipe_config->vrr.vtotalmin = intel_de_read(dev_priv, TRANS_VRR_VMIN(pipe)) + 1;
+}
+
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
index 8c6fd2d1bee5..2c679377c53b 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -13,6 +13,7 @@ struct drm_i915_private;
 struct intel_crtc_state;
 struct intel_encoder;
 struct intel_dp;
+struct intel_crtc;
 
 bool intel_is_vrr_capable(struct drm_connector *connector);
 void intel_vrr_compute_config(struct intel_dp *intel_dp,
@@ -21,5 +22,7 @@ void intel_vrr_enable(struct intel_encoder *encoder,
 		      const struct intel_crtc_state *crtc_state);
 void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
 void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
+void intel_vrr_get_config(struct intel_crtc *crtc,
+			  struct intel_crtc_state *pipe_config);
 
 #endif /* __INTEL_VRR_H__ */
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for VRR/Adaptive Sync enabling in i915 (rev2)

[Patchwork]

== Series Details ==

Series: VRR/Adaptive Sync enabling in i915 (rev2)
URL   : https://patchwork.freedesktop.org/series/82966/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
079903f97657 drm/i915: Add REG_FIELD_PREP to VRR register def
34bcabeb4cdc drm/i915/display/vrr: Create VRR file and add VRR capability check
-:30: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#30: 
new file mode 100644

-:35: WARNING:SPDX_LICENSE_TAG: Improper SPDX comment style for 'drivers/gpu/drm/i915/display/intel_vrr.c', please use '//' instead
#35: FILE: drivers/gpu/drm/i915/display/intel_vrr.c:1:
+/* SPDX-License-Identifier: MIT */

-:35: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#35: FILE: drivers/gpu/drm/i915/display/intel_vrr.c:1:
+/* SPDX-License-Identifier: MIT */

-:72: WARNING:BLOCK_COMMENT_STYLE: Block comments should align the * on each line
#72: FILE: drivers/gpu/drm/i915/display/intel_vrr.h:4:
+ * Copyright © 2019 Intel Corporation
+*/

total: 0 errors, 4 warnings, 0 checks, 54 lines checked
575c776fcd14 drm/i915/display/dp: Attach and set drm connector VRR property
f6539e810029 drm/i915/display/dp: Add VRR crtc state variables
2f683b532029 drm/i915/display/dp: Compute VRR state in atomic_check
4166383fc218 drm/i915/display/dp: Do not enable PSR if VRR is enabled
eb5fbde0f099 drm/i915/display/vrr: Configure and enable VRR in modeset enable
-:56: WARNING:LONG_LINE: line length of 110 exceeds 100 columns
#56: FILE: drivers/gpu/drm/i915/display/intel_vrr.c:75:
+	u32 trans_vrr_ctl = 0, trans_vrr_vmax = 0, trans_vrr_vmin = 0, trans_vrr_flipline = 0, trans_push = 0;

total: 0 errors, 1 warnings, 0 checks, 65 lines checked
e54bfcb81993 drm/i915/display/vrr: Send VRR push to flip the frame
1ad704e2734c drm/i915/display/vrr: Disable VRR in modeset disable path
19de88b31e66 drm/i915/display/vrr: Set IGNORE_MSA_PAR state in DP Sink
fffbfc7cc4a0 drm/i915/display: Add HW state readout for VRR


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for VRR/Adaptive Sync enabling in i915 (rev2)

[Patchwork]

== Series Details ==

Series: VRR/Adaptive Sync enabling in i915 (rev2)
URL   : https://patchwork.freedesktop.org/series/82966/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_reset.c:1312:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gt/selftest_reset.c:100:20:    expected void *in
+drivers/gpu/drm/i915/gt/selftest_reset.c:100:20:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:100:20: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gt/selftest_reset.c:101:46:    expected void const *src
+drivers/gpu/drm/i915/gt/selftest_reset.c:101:46:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:101:46: warning: incorrect type in argument 2 (different address spaces)
+drivers/gpu/drm/i915/gt/selftest_reset.c:136:20:    expected void *in
+drivers/gpu/drm/i915/gt/selftest_reset.c:136:20:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:136:20: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gt/selftest_reset.c:137:46:    expected void const *src
+drivers/gpu/drm/i915/gt/selftest_reset.c:137:46:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:137:46: warning: incorrect type in argument 2 (different address spaces)
+drivers/gpu/drm/i915/gt/selftest_reset.c:98:34:    expected unsigned int [usertype] *s
+drivers/gpu/drm/i915/gt/selftest_reset.c:98:34:    got void [noderef] __iomem *[assigned] s
+drivers/gpu/drm/i915/gt/selftest_reset.c:98:34: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gvt/mmio.c:290:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1440:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1494:15: warning: memset with byte count of 16777216
+./include/linux/seqlock.h:752:24: warning: trying to copy expression type 31
+./include/linux/seqlock.h:778:16: warning: trying to copy expression type 31
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for VRR/Adaptive Sync enabling in i915 (rev2)

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 5290 bytes --]

== Series Details ==

Series: VRR/Adaptive Sync enabling in i915 (rev2)
URL   : https://patchwork.freedesktop.org/series/82966/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9191 -> Patchwork_18777
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/index.html

Known issues
------------

  Here are the changes found in Patchwork_18777 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-icl-y:           [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/fi-icl-y/igt@core_hotunplug@unbind-rebind.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/fi-icl-y/igt@core_hotunplug@unbind-rebind.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-byt-j1900:       [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@vgem_basic@unload:
    - fi-skl-guc:         [PASS][5] -> [DMESG-WARN][6] ([i915#2203])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/fi-skl-guc/igt@vgem_basic@unload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/fi-skl-guc/igt@vgem_basic@unload.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-byt-j1900:       [DMESG-WARN][7] ([i915#1982]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       [DMESG-WARN][9] ([i915#165]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-kbl-7500u:       [DMESG-WARN][11] ([i915#2203]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - {fi-kbl-7560u}:     [DMESG-WARN][13] ([i915#1982]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/fi-kbl-7560u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/fi-kbl-7560u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
    - fi-icl-u2:          [DMESG-WARN][15] ([i915#1982]) -> [PASS][16] +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#165]: https://gitlab.freedesktop.org/drm/intel/issues/165
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203


Participating hosts (46 -> 39)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_9191 -> Patchwork_18777

  CI-20190529: 20190529
  CI_DRM_9191: 4b693bbb9b41fda404b5cd081bf5cd8dba240468 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5823: 7dd2fe99bd9dde00456cc5abf7e5ef0c8d7d6118 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18777: fffbfc7cc4a0bf38fa0c38d7b565e68095f613bd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

fffbfc7cc4a0 drm/i915/display: Add HW state readout for VRR
19de88b31e66 drm/i915/display/vrr: Set IGNORE_MSA_PAR state in DP Sink
1ad704e2734c drm/i915/display/vrr: Disable VRR in modeset disable path
e54bfcb81993 drm/i915/display/vrr: Send VRR push to flip the frame
eb5fbde0f099 drm/i915/display/vrr: Configure and enable VRR in modeset enable
4166383fc218 drm/i915/display/dp: Do not enable PSR if VRR is enabled
2f683b532029 drm/i915/display/dp: Compute VRR state in atomic_check
f6539e810029 drm/i915/display/dp: Add VRR crtc state variables
575c776fcd14 drm/i915/display/dp: Attach and set drm connector VRR property
34bcabeb4cdc drm/i915/display/vrr: Create VRR file and add VRR capability check
079903f97657 drm/i915: Add REG_FIELD_PREP to VRR register def

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/index.html

[-- Attachment #1.2: Type: text/html, Size: 6497 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.IGT: failure for VRR/Adaptive Sync enabling in i915 (rev2)

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 16427 bytes --]

== Series Details ==

Series: VRR/Adaptive Sync enabling in i915 (rev2)
URL   : https://patchwork.freedesktop.org/series/82966/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9191_full -> Patchwork_18777_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_18777_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_18777_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_18777_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_flush@basic-uc-pro-default:
    - shard-snb:          NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-snb7/igt@gem_exec_flush@basic-uc-pro-default.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@core_hotunplug@hotrebind}:
    - shard-hsw:          NOTRUN -> [WARN][2]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-hsw2/igt@core_hotunplug@hotrebind.html

  
Known issues
------------

  Here are the changes found in Patchwork_18777_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@kms:
    - shard-snb:          [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-snb7/igt@gem_eio@kms.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-snb2/igt@gem_eio@kms.html

  * igt@gem_exec_reloc@basic-many-active@vecs0:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#2389]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-glk4/igt@gem_exec_reloc@basic-many-active@vecs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-glk5/igt@gem_exec_reloc@basic-many-active@vecs0.html

  * igt@gem_exec_whisper@basic-forked-all:
    - shard-glk:          [PASS][7] -> [DMESG-WARN][8] ([i915#118] / [i915#95])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-glk5/igt@gem_exec_whisper@basic-forked-all.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-glk3/igt@gem_exec_whisper@basic-forked-all.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [PASS][9] -> [DMESG-WARN][10] ([i915#1436] / [i915#716])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl2/igt@gen9_exec_parse@allowed-single.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl2/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-hsw:          [PASS][11] -> [WARN][12] ([i915#1519])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-hsw1/igt@i915_pm_rc6_residency@rc6-idle.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-hsw4/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@kms_cursor_crc@pipe-c-cursor-128x42-offscreen:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([i915#54])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl8/igt@kms_cursor_crc@pipe-c-cursor-128x42-offscreen.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl4/igt@kms_cursor_crc@pipe-c-cursor-128x42-offscreen.html

  * igt@kms_cursor_edge_walk@pipe-c-64x64-right-edge:
    - shard-glk:          [PASS][15] -> [DMESG-WARN][16] ([i915#1982])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-glk3/igt@kms_cursor_edge_walk@pipe-c-64x64-right-edge.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-glk9/igt@kms_cursor_edge_walk@pipe-c-64x64-right-edge.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
    - shard-hsw:          [PASS][17] -> [FAIL][18] ([i915#2370])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-hsw4/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-hdmi-a2:
    - shard-glk:          [PASS][19] -> [FAIL][20] ([i915#2122])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-glk9/igt@kms_flip@basic-flip-vs-wf_vblank@a-hdmi-a2.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-glk1/igt@kms_flip@basic-flip-vs-wf_vblank@a-hdmi-a2.html

  * igt@kms_flip@basic-plain-flip@a-edp1:
    - shard-skl:          [PASS][21] -> [DMESG-WARN][22] ([i915#1982]) +13 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl6/igt@kms_flip@basic-plain-flip@a-edp1.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl6/igt@kms_flip@basic-plain-flip@a-edp1.html

  * igt@kms_flip@flip-vs-absolute-wf_vblank@a-dp1:
    - shard-kbl:          [PASS][23] -> [DMESG-WARN][24] ([i915#1982])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-kbl2/igt@kms_flip@flip-vs-absolute-wf_vblank@a-dp1.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-kbl6/igt@kms_flip@flip-vs-absolute-wf_vblank@a-dp1.html

  * igt@kms_flip@flip-vs-expired-vblank@a-edp1:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([i915#79])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl4/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl7/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html

  * igt@kms_frontbuffer_tracking@fbcpsr-modesetfrombusy:
    - shard-tglb:         [PASS][27] -> [DMESG-WARN][28] ([i915#1982]) +2 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-modesetfrombusy.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-modesetfrombusy.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [PASS][29] -> [FAIL][30] ([i915#1188])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl4/igt@kms_hdr@bpc-switch-suspend.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl7/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][31] -> [FAIL][32] ([fdo#108145] / [i915#265])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [PASS][33] -> [SKIP][34] ([fdo#109441]) +2 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-iclb7/igt@kms_psr@psr2_cursor_render.html

  * igt@prime_mmap_kms@buffer-sharing:
    - shard-snb:          [PASS][35] -> [SKIP][36] ([fdo#109271])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-snb4/igt@prime_mmap_kms@buffer-sharing.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-snb4/igt@prime_mmap_kms@buffer-sharing.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@preservation-s3@vcs0:
    - shard-skl:          [INCOMPLETE][37] ([i915#198]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl4/igt@gem_ctx_isolation@preservation-s3@vcs0.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl9/igt@gem_ctx_isolation@preservation-s3@vcs0.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-skl:          [TIMEOUT][39] ([i915#2424]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl4/igt@gem_userptr_blits@sync-unmap-cycles.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl7/igt@gem_userptr_blits@sync-unmap-cycles.html

  * {igt@kms_async_flips@alternate-sync-async-flip}:
    - shard-kbl:          [FAIL][41] ([i915#2521]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-kbl6/igt@kms_async_flips@alternate-sync-async-flip.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-kbl7/igt@kms_async_flips@alternate-sync-async-flip.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-0:
    - shard-glk:          [DMESG-WARN][43] ([i915#1982]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-glk3/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-glk9/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-c-cursor-128x128-offscreen:
    - shard-skl:          [FAIL][45] ([i915#54]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl2/igt@kms_cursor_crc@pipe-c-cursor-128x128-offscreen.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl2/igt@kms_cursor_crc@pipe-c-cursor-128x128-offscreen.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][47] ([i915#180]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-kbl6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][49] ([i915#96]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-hsw2/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_flip@flip-vs-suspend@c-hdmi-a1:
    - shard-hsw:          [INCOMPLETE][51] ([i915#2055]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-hsw1/igt@kms_flip@flip-vs-suspend@c-hdmi-a1.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-hsw2/igt@kms_flip@flip-vs-suspend@c-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt:
    - shard-snb:          [INCOMPLETE][53] ([i915#82]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-snb5/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-snb7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-kbl:          [DMESG-WARN][55] ([i915#1982]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-kbl4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu.html

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-wc:
    - shard-skl:          [DMESG-WARN][57] ([i915#1982]) -> [PASS][58] +4 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl9/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-wc.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl6/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-wc.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [FAIL][59] ([fdo#108145] / [i915#265]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][61] ([fdo#109642] / [fdo#111068]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-iclb1/igt@kms_psr2_su@page_flip.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-iclb2/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_primary_render:
    - shard-iclb:         [SKIP][63] ([fdo#109441]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-iclb3/igt@kms_psr@psr2_primary_render.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-iclb2/igt@kms_psr@psr2_primary_render.html

  
#### Warnings ####

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-iclb:         [FAIL][65] ([i915#1515]) -> [WARN][66] ([i915#1515])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-iclb5/igt@i915_pm_rc6_residency@rc6-idle.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-iclb5/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-skl:          [DMESG-FAIL][67] ([fdo#108145] / [i915#1982]) -> [FAIL][68] ([fdo#108145] / [i915#265])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9191/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1515]: https://gitlab.freedesktop.org/drm/intel/issues/1515
  [i915#1519]: https://gitlab.freedesktop.org/drm/intel/issues/1519
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2055]: https://gitlab.freedesktop.org/drm/intel/issues/2055
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2370]: https://gitlab.freedesktop.org/drm/intel/issues/2370
  [i915#2389]: https://gitlab.freedesktop.org/drm/intel/issues/2389
  [i915#2424]: https://gitlab.freedesktop.org/drm/intel/issues/2424
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [i915#96]: https://gitlab.freedesktop.org/drm/intel/issues/96


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_9191 -> Patchwork_18777

  CI-20190529: 20190529
  CI_DRM_9191: 4b693bbb9b41fda404b5cd081bf5cd8dba240468 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5823: 7dd2fe99bd9dde00456cc5abf7e5ef0c8d7d6118 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18777: fffbfc7cc4a0bf38fa0c38d7b565e68095f613bd @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18777/index.html

[-- Attachment #1.2: Type: text/html, Size: 18988 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/11] drm/i915: Add REG_FIELD_PREP to VRR register def

[Jani Nikula]

On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> VRR_CTL register onloy had a GENMASK but no field prep
> define for TRANS_VRR_CTL_LINE_COUNT field so add that

For the subject, I think mentioning VRR_CTL_LINK_COUNT is more important
than REG_FIELD_PREP.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Cc: Aditya Swarup <aditya.swarup@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d4952c9875fb..9792c931b4c5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4348,6 +4348,7 @@ enum {
>  #define   VRR_CTL_IGN_MAX_SHIFT		REG_BIT(30)
>  #define   VRR_CTL_FLIP_LINE_EN		REG_BIT(29)
>  #define   VRR_CTL_LINE_COUNT_MASK	REG_GENMASK(10, 3)
> +#define   VRR_CTL_LINE_COUNT(x)		REG_FIELD_PREP(VRR_CTL_LINE_COUNT_MASK, (x))
>  #define   VRR_CTL_SW_FULLLINE_COUNT	REG_BIT(0)
>  
>  #define _TRANS_VRR_VMAX_A		0x60424

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/11] drm/i915/display/vrr: Create VRR file and add VRR capability check

[Jani Nikula]

On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> We create a new file for all VRR related helpers.
> Also add a function to check vrr capability based on
> platform support, DPCD bits and EDID monitor range.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile            |  1 +
>  drivers/gpu/drm/i915/display/intel_vrr.c | 28 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_vrr.h | 19 ++++++++++++++++
>  3 files changed, 48 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e5574e506a5c..3beeaf517191 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -249,6 +249,7 @@ i915-y += \
>  	display/intel_sdvo.o \
>  	display/intel_tv.o \
>  	display/intel_vdsc.o \
> +	display/intel_vrr.o \
>  	display/vlv_dsi.o \
>  	display/vlv_dsi_pll.o
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> new file mode 100644
> index 000000000000..0c8a91fabb64
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2020 Intel Corporation
> + *
> + * Author: Manasi Navare <manasi.d.navare@intel.com>

I have increasingly mixed feelings about adding author lines in files in
big collaborative projects. They tend to go out of date fairly quickly,
and will cease to represent the contributions fairly. And git already
gives us this information.

> + */
> +
> +#include "i915_drv.h"
> +#include "intel_display_types.h"
> +#include "intel_vrr.h"
> +
> +bool intel_is_vrr_capable(struct drm_connector *connector)

Please prefix with intel_vrr_ consistently.

> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));

I kind of feel like either the function should a) ensure it's okay to do
intel_attached_dp() and return false if not, or b) just use struct
intel_dp as the parameter.

As it is, passing a non-dp connector to this function will fail either
subtly or spectacularly, but not graciously.

> +	const struct drm_display_info *info = &connector->display_info;
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);

Please use i915 over dev_priv in all new code.

> +
> +	/*
> +	 * DP Sink is capable of Variable refresh video timings if
> +	 * Ignore MSA bit is set in DPCD.
> +	 * EDID monitor range also should be atleast 10 for reasonable
> +	 * Adaptive sync/ VRR end user experience.
> +	 */

Please fix typos etc.

> +	return INTEL_GEN(dev_priv) >= 12 &&
> +		drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd) &&
> +		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
> +}
> +
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> new file mode 100644
> index 000000000000..755746c7525c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2019 Intel Corporation
> +*/
> +
> +#ifndef __INTEL_VRR_H__
> +#define __INTEL_VRR_H__
> +
> +#include <linux/types.h>
> +
> +struct drm_connector;

All of the below declarations are unnecessary at this commit.

BR,
Jani.

> +struct drm_i915_private;
> +struct intel_crtc_state;
> +struct intel_encoder;
> +struct intel_dp;
> +
> +bool intel_is_vrr_capable(struct drm_connector *connector);
> +
> +#endif /* __INTEL_VRR_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/11] drm/i915/display/dp: Attach and set drm connector VRR property

[Jani Nikula]

On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> From: Aditya Swarup <aditya.swarup@intel.com>
>
> This function sets the VRR property for connector based
> on the platform support, EDID monitor range and DP sink
> DPCD capability of outputing video without msa
> timing information.
>
> v7:
> * Move the helper to separate file (Manasi)
> v6:
> * Remove unset of prop
> v5:
> * Fix the vrr prop not being set in kernel (Manasi)
> * Unset the prop on connector disconnect (Manasi)
> v4:
> * Rebase (Mansi)
> v3:
> * intel_dp_is_vrr_capable can be used for debugfs, make it
> non static (Manasi)
> v2:
> * Just set this in intel_dp_get_modes instead of new hook (Jani)
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++++
>  drivers/gpu/drm/i915/display/intel_dp.h | 1 +
>  2 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 818daab252f3..3794b8f35edc 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -62,6 +62,7 @@
>  #include "intel_sideband.h"
>  #include "intel_tc.h"
>  #include "intel_vdsc.h"
> +#include "intel_vrr.h"
>  
>  #define DP_DPRX_ESI_LEN 14
>  
> @@ -6622,6 +6623,10 @@ static int intel_dp_get_modes(struct drm_connector *connector)
>  	edid = intel_connector->detect_edid;
>  	if (edid) {
>  		int ret = intel_connector_update_modes(connector, edid);
> +
> +		if (intel_is_vrr_capable(connector))
> +			drm_connector_set_vrr_capable_property(connector,
> +							       true);
>  		if (ret)
>  			return ret;
>  	}
> @@ -7080,6 +7085,9 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
>  
>  	}
> +
> +	if (INTEL_GEN(dev_priv) >= 12)

I wonder if we should just add a wrapper

#define HAS_VRR(i915) (INTEL_GEN(i915) >= 12)

to be more descriptive. And use it in the previous patch too.

> +		drm_connector_attach_vrr_capable_property(connector);
>  }
>  
>  static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 3f862b4fd34f..aaf0a41582d7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -15,6 +15,7 @@ enum pipe;
>  enum port;
>  struct drm_connector_state;
>  struct drm_encoder;
> +struct drm_connector;

Unrelated change.

>  struct drm_i915_private;
>  struct drm_modeset_acquire_ctx;
>  struct drm_dp_vsc_sdp;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/11] drm/i915/display/dp: Add VRR crtc state variables

[Jani Nikula]

On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> Introduce VRR struct in intel_crtc_state and add
> VRR crtc state variables Enable, Vtotalmin and Vtotalmax
> to be derived from mode timings and VRR crtc property.

I'd squash this to the patch actually using it.

>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index f6f0626649e0..f6f7ec024da7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1092,6 +1092,13 @@ struct intel_crtc_state {
>  	struct intel_dsb *dsb;
>  
>  	u32 psr2_man_track_ctl;
> +
> +	/* Variable Refresh Rate state */
> +	struct {
> +		bool enable;
> +		u16 vtotalmin;
> +		u16 vtotalmax;
> +	} vrr;
>  };
>  
>  enum intel_pipe_crc_source {

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/11] drm/i915/display/dp: Compute VRR state in atomic_check

[Jani Nikula]

On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> This forces a complete modeset if vrr drm crtc state goes
> from enabled to disabled and vice versa.
> This patch also computes vrr state variables from the mode timings
> and based on the vrr property set by userspace as well as hardware's
> vrr capability.
>
> v2:
> *Rebase
> v3:
> * Vmin = max (vtotal, vmin) (Manasi)
> v4:
> * set crtc_state->vrr.enable = 0 for disable request
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  7 +++-
>  drivers/gpu/drm/i915/display/intel_dp.c      |  1 +
>  drivers/gpu/drm/i915/display/intel_vrr.c     | 38 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_vrr.h     |  2 ++
>  4 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f41b6f8b5618..f70cc3b2a1a4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14213,6 +14213,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  
>  	PIPE_CONF_CHECK_I(mst_master_transcoder);
>  
> +	PIPE_CONF_CHECK_BOOL(vrr.enable);
> +	PIPE_CONF_CHECK_I(vrr.vtotalmin);
> +	PIPE_CONF_CHECK_I(vrr.vtotalmax);
> +
>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
>  #undef PIPE_CONF_CHECK_BOOL
> @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev,
>  
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
> -		if (new_crtc_state->inherited != old_crtc_state->inherited)
> +		if (new_crtc_state->inherited != old_crtc_state->inherited ||
> +		    new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled)

Somehow this feels like a really specific check to add considering the
abstraction level of the function in general.

>  			new_crtc_state->uapi.mode_changed = true;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 3794b8f35edc..3185c4ca523d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2752,6 +2752,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	if (!HAS_DDI(dev_priv))
>  		intel_dp_set_clock(encoder, pipe_config);
>  
> +	intel_vrr_compute_config(intel_dp, pipe_config);
>  	intel_psr_compute_config(intel_dp, pipe_config);
>  	intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp,
>  				     constant_n);
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 0c8a91fabb64..56114f535f94 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -26,3 +26,41 @@ bool intel_is_vrr_capable(struct drm_connector *connector)
>  		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
>  }
>  
> +void
> +intel_vrr_compute_config(struct intel_dp *intel_dp,
> +			 struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
> +	struct drm_connector *connector = &intel_connector->base;
> +	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +	const struct drm_display_info *info = &connector->display_info;
> +
> +	if (!intel_is_vrr_capable(connector))
> +		return;
> +
> +	if (!crtc_state->uapi.vrr_enabled) {
> +		drm_dbg(&dev_priv->drm,
> +			"VRR disable requested by Userspace\n");

drm_dbg_kms, though is this useful information? Quite a bit of log spam
I'd think.

> +		crtc_state->vrr.enable = false;
> +		return;
> +	}
> +
> +	crtc_state->vrr.enable = true;
> +	crtc_state->vrr.vtotalmin =
> +		max_t(u16, adjusted_mode->crtc_vtotal,
> +		      DIV_ROUND_CLOSEST(adjusted_mode->crtc_clock * 1000,
> +					adjusted_mode->crtc_htotal *
> +					info->monitor_range.max_vfreq));
> +	crtc_state->vrr.vtotalmax =
> +		max_t(u16, adjusted_mode->crtc_vtotal,
> +		      DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> +				   adjusted_mode->crtc_htotal *
> +				   info->monitor_range.min_vfreq));
> +
> +	drm_dbg(&dev_priv->drm,

drm_dbg_kms

> +		"VRR Config: Enable = %s Vtotal Min = %d Vtotal Max = %d\n",
> +		 yesno(crtc_state->vrr.enable), crtc_state->vrr.vtotalmin,
> +		 crtc_state->vrr.vtotalmax);
> +}
> +
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index 755746c7525c..1e6fe8fe92ec 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -15,5 +15,7 @@ struct intel_encoder;
>  struct intel_dp;
>  
>  bool intel_is_vrr_capable(struct drm_connector *connector);
> +void intel_vrr_compute_config(struct intel_dp *intel_dp,
> +			      struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_VRR_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/11] drm/i915/display/vrr: Configure and enable VRR in modeset enable

[Jani Nikula]

On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> This patch computes the VRR parameters from VRR crtc states
> and configures them in VRR registers during CRTC enable in
> the modeset enable sequence.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c |  5 ++++
>  drivers/gpu/drm/i915/display/intel_vrr.c | 38 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_vrr.h |  2 ++
>  3 files changed, 45 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 09811be08cfe..391c51979334 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -49,6 +49,7 @@
>  #include "intel_sprite.h"
>  #include "intel_tc.h"
>  #include "intel_vdsc.h"
> +#include "intel_vrr.h"
>  
>  struct ddi_buf_trans {
>  	u32 trans1;	/* balance leg enable, de-emph level */
> @@ -4046,6 +4047,10 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>  
>  	intel_ddi_enable_transcoder_func(encoder, crtc_state);
>  
> +	/* Enable VRR if requested through CRTC property */

I don't think the comment is helpful, really.

> +	if (crtc_state->vrr.enable)
> +		intel_vrr_enable(encoder, crtc_state);

In the disable path you check the vrr.enable within the
function. Perhaps we should do the same here, i.e. call vrr enable
unconditionally and have it early return if not requested.

> +
>  	intel_enable_pipe(crtc_state);
>  
>  	intel_crtc_vblank_on(crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 56114f535f94..7f1353bac583 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -64,3 +64,41 @@ intel_vrr_compute_config(struct intel_dp *intel_dp,
>  		 crtc_state->vrr.vtotalmax);
>  }
>  
> +void intel_vrr_enable(struct intel_encoder *encoder,
> +		      const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	enum pipe pipe = crtc->pipe;
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->hw.adjusted_mode;
> +	u32 trans_vrr_ctl = 0, trans_vrr_vmax = 0, trans_vrr_vmin = 0, trans_vrr_flipline = 0, trans_push = 0;
> +	u16 framestart_to_pipelinefull_linecnt = 0;

All the initializations to 0 are unnecessary.

> +
> +	framestart_to_pipelinefull_linecnt =
> +		min_t(u16, 255, (crtc_state->vrr.vtotalmin - adjusted_mode->crtc_vdisplay));
> +
> +	trans_vrr_ctl = VRR_CTL_VRR_ENABLE |  VRR_CTL_IGN_MAX_SHIFT |
> +		VRR_CTL_FLIP_LINE_EN | VRR_CTL_LINE_COUNT(framestart_to_pipelinefull_linecnt) |
> +		VRR_CTL_SW_FULLLINE_COUNT;
> +
> +	/* Programming adjustments for 0 based regs */
> +	trans_vrr_vmax = crtc_state->vrr.vtotalmax - 1;
> +	trans_vrr_vmin = crtc_state->vrr.vtotalmin - 1;
> +	trans_vrr_flipline = crtc_state->vrr.vtotalmin - 1;
> +
> +	trans_push = TRANS_PUSH_EN;

Frankly I'd just throw away the above four temp variables.

> +
> +	intel_de_write(dev_priv, TRANS_VRR_VMIN(pipe), trans_vrr_vmin);
> +	intel_de_write(dev_priv, TRANS_VRR_VMAX(pipe), trans_vrr_vmax);
> +	intel_de_write(dev_priv, TRANS_VRR_CTL(pipe), trans_vrr_ctl);
> +	intel_de_write(dev_priv, TRANS_VRR_FLIPLINE(pipe), trans_vrr_flipline);
> +	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
> +
> +	drm_dbg(&dev_priv->drm,	"Enabling VRR on pipe (%c)\n", pipe_name(pipe));

drm_dbg_kms. "pipe %c" is the convention, not "pipe (%c)".

> +	drm_dbg(&dev_priv->drm, "VRR Parameters: Vblank - Min = %d, Max = %d Flipline Count = %d, CTL Reg = 0x%08x, TRANS PUSH reg = 0x%08x",
> +		crtc_state->vrr.vtotalmin, crtc_state->vrr.vtotalmax,
> +		crtc_state->vrr.vtotalmin, trans_vrr_ctl,
> +		trans_push);
> +}
> +
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index 1e6fe8fe92ec..05d982d6fbae 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -17,5 +17,7 @@ struct intel_dp;
>  bool intel_is_vrr_capable(struct drm_connector *connector);
>  void intel_vrr_compute_config(struct intel_dp *intel_dp,
>  			      struct intel_crtc_state *crtc_state);
> +void intel_vrr_enable(struct intel_encoder *encoder,
> +		      const struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_VRR_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/11] drm/i915/display/vrr: Send VRR push to flip the frame

[Jani Nikula]

On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> VRR achieves vblank stretching using the HW PUSH functionality.
> So once the VRR is enabled during modeset then for each flip
> request from userspace, in the atomic tail pipe_update_end()
> we need to set the VRR push bit in HW for it to terminate
> the vblank at configured flipline or anytime after flipline
> or latest at the Vmax.
>
> The HW clears the PUSH bit after the double buffer updates
> are completed.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_sprite.c |  5 +++++
>  drivers/gpu/drm/i915/display/intel_vrr.c    | 17 +++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_vrr.h    |  1 +
>  3 files changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index b6deeb338477..cb10fe462f06 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -49,6 +49,7 @@
>  #include "intel_psr.h"
>  #include "intel_dsi.h"
>  #include "intel_sprite.h"
> +#include "intel_vrr.h"
>  
>  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>  			     int usecs)
> @@ -217,6 +218,10 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
>  	    intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI))
>  		icl_dsi_frame_update(new_crtc_state);
>  
> +	/* Send VRR Push to terminate Vblank */
> +	if (new_crtc_state->vrr.enable)
> +		intel_vrr_send_push(new_crtc_state);
> +

Maybe move the vrr.enable check to the function?

>  	/* We're still in the vblank-evade critical section, this can't race.
>  	 * Would be slightly nice to just grab the vblank count and arm the
>  	 * event outside of the critical section - the spinlock might spin for a
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 7f1353bac583..ec1ce88e869c 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -102,3 +102,20 @@ void intel_vrr_enable(struct intel_encoder *encoder,
>  		trans_push);
>  }
>  
> +void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	enum pipe pipe = crtc->pipe;
> +	u32 trans_push;
> +
> +	trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe));
> +	WARN_ON(!(trans_push & TRANS_PUSH_EN));

drm_WARN_ON, and perhaps move this below the register rmw. It doesn't
change the flow anyway.

> +
> +	trans_push |= TRANS_PUSH_SEND;
> +	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
> +
> +	drm_dbg(&dev_priv->drm, "Sending VRR Push on Pipe (%c)\n",
> +		pipe_name(pipe));

drm_dbg_kms

> +}
> +
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index 05d982d6fbae..a6b78e1676cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -19,5 +19,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp,
>  			      struct intel_crtc_state *crtc_state);
>  void intel_vrr_enable(struct intel_encoder *encoder,
>  		      const struct intel_crtc_state *crtc_state);
> +void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_VRR_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/11] drm/i915/display/vrr: Disable VRR in modeset disable path

[Jani Nikula]

On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> This patch disables the VRR enable and VRR PUSH
> bits in the HW during commit modeset disable sequence.
>
> Thsi disable will happen when the port is disabled
> or when the userspace sets VRR prop to false and
> requests to disable VRR.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c |  2 ++
>  drivers/gpu/drm/i915/display/intel_vrr.c | 22 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_vrr.h |  1 +
>  3 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 391c51979334..565155af3fb9 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3819,6 +3819,8 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
>  
>  		intel_disable_pipe(old_crtc_state);
>  
> +		intel_vrr_disable(old_crtc_state);
> +
>  		intel_ddi_disable_transcoder_func(old_crtc_state);
>  
>  		intel_dsc_disable(old_crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index ec1ce88e869c..5075ecb9b5a7 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -119,3 +119,25 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
>  		pipe_name(pipe));
>  }
>  
> +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);

Haven't commented on all patches, but please use i915 instead of
dev_priv for new code, throughout.

> +	enum pipe pipe = crtc->pipe;
> +	u32 trans_vrr_ctl = 0, trans_push = 0;

Unnecessary initializations, and in fact unnecessary variables with
intel_de_rmw.

> +
> +	if (!old_crtc_state->vrr.enable)
> +		return;
> +
> +	trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(pipe));
> +	trans_vrr_ctl &= ~(VRR_CTL_FLIP_LINE_EN | VRR_CTL_VRR_ENABLE);
> +	intel_de_write(dev_priv, TRANS_VRR_CTL(pipe), trans_vrr_ctl);
> +
> +	trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe));
> +	trans_push &= ~TRANS_PUSH_EN;
> +	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);

Please use intel_de_rmw for both.

> +
> +	drm_dbg(&dev_priv->drm, "Disabling VRR on Pipe (%c)\n",
> +		pipe_name(pipe));

drm_dbg_kms, "pipe %c" is the convention.

> +}
> +
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index a6b78e1676cb..8c6fd2d1bee5 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -20,5 +20,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp,
>  void intel_vrr_enable(struct intel_encoder *encoder,
>  		      const struct intel_crtc_state *crtc_state);
>  void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
> +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
>  
>  #endif /* __INTEL_VRR_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/11] drm/i915/display/vrr: Create VRR file and add VRR capability check

[Ville Syrjälä]

On Thu, Oct 22, 2020 at 03:27:00PM -0700, Manasi Navare wrote:
> We create a new file for all VRR related helpers.
> Also add a function to check vrr capability based on
> platform support, DPCD bits and EDID monitor range.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile            |  1 +
>  drivers/gpu/drm/i915/display/intel_vrr.c | 28 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_vrr.h | 19 ++++++++++++++++
>  3 files changed, 48 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index e5574e506a5c..3beeaf517191 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -249,6 +249,7 @@ i915-y += \
>  	display/intel_sdvo.o \
>  	display/intel_tv.o \
>  	display/intel_vdsc.o \
> +	display/intel_vrr.o \
>  	display/vlv_dsi.o \
>  	display/vlv_dsi_pll.o
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> new file mode 100644
> index 000000000000..0c8a91fabb64
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2020 Intel Corporation
> + *
> + * Author: Manasi Navare <manasi.d.navare@intel.com>
> + */
> +
> +#include "i915_drv.h"
> +#include "intel_display_types.h"
> +#include "intel_vrr.h"
> +
> +bool intel_is_vrr_capable(struct drm_connector *connector)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
> +	const struct drm_display_info *info = &connector->display_info;
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +
> +	/*
> +	 * DP Sink is capable of Variable refresh video timings if
> +	 * Ignore MSA bit is set in DPCD.
> +	 * EDID monitor range also should be atleast 10 for reasonable
> +	 * Adaptive sync/ VRR end user experience.
> +	 */
> +	return INTEL_GEN(dev_priv) >= 12 &&
> +		drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd) &&
> +		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
> +}

A whole new file for vrr seems overkill. There's probably
not going to be much to put in there.

> +
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> new file mode 100644
> index 000000000000..755746c7525c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2019 Intel Corporation
> +*/
> +
> +#ifndef __INTEL_VRR_H__
> +#define __INTEL_VRR_H__
> +
> +#include <linux/types.h>
> +
> +struct drm_connector;
> +struct drm_i915_private;
> +struct intel_crtc_state;
> +struct intel_encoder;
> +struct intel_dp;
> +
> +bool intel_is_vrr_capable(struct drm_connector *connector);
> +
> +#endif /* __INTEL_VRR_H__ */
> -- 
> 2.19.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/11] drm/i915/display/vrr: Create VRR file and add VRR capability check

[Navare, Manasi]

On Tue, Nov 10, 2020 at 06:06:49PM +0200, Ville Syrjälä wrote:
> On Thu, Oct 22, 2020 at 03:27:00PM -0700, Manasi Navare wrote:
> > We create a new file for all VRR related helpers.
> > Also add a function to check vrr capability based on
> > platform support, DPCD bits and EDID monitor range.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile            |  1 +
> >  drivers/gpu/drm/i915/display/intel_vrr.c | 28 ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_vrr.h | 19 ++++++++++++++++
> >  3 files changed, 48 insertions(+)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.h
> > 
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index e5574e506a5c..3beeaf517191 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -249,6 +249,7 @@ i915-y += \
> >  	display/intel_sdvo.o \
> >  	display/intel_tv.o \
> >  	display/intel_vdsc.o \
> > +	display/intel_vrr.o \
> >  	display/vlv_dsi.o \
> >  	display/vlv_dsi_pll.o
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > new file mode 100644
> > index 000000000000..0c8a91fabb64
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2020 Intel Corporation
> > + *
> > + * Author: Manasi Navare <manasi.d.navare@intel.com>
> > + */
> > +
> > +#include "i915_drv.h"
> > +#include "intel_display_types.h"
> > +#include "intel_vrr.h"
> > +
> > +bool intel_is_vrr_capable(struct drm_connector *connector)
> > +{
> > +	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
> > +	const struct drm_display_info *info = &connector->display_info;
> > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > +
> > +	/*
> > +	 * DP Sink is capable of Variable refresh video timings if
> > +	 * Ignore MSA bit is set in DPCD.
> > +	 * EDID monitor range also should be atleast 10 for reasonable
> > +	 * Adaptive sync/ VRR end user experience.
> > +	 */
> > +	return INTEL_GEN(dev_priv) >= 12 &&
> > +		drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd) &&
> > +		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
> > +}
> 
> A whole new file for vrr seems overkill. There's probably
> not going to be much to put in there.

There is actually quite a bit of functions and could bemore as we expand this functionality.
So thought it would be cleaner to it here. That way it could also be expanded to
be used on HDMI later.

Manasi

> 
> > +
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > new file mode 100644
> > index 000000000000..755746c7525c
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2019 Intel Corporation
> > +*/
> > +
> > +#ifndef __INTEL_VRR_H__
> > +#define __INTEL_VRR_H__
> > +
> > +#include <linux/types.h>
> > +
> > +struct drm_connector;
> > +struct drm_i915_private;
> > +struct intel_crtc_state;
> > +struct intel_encoder;
> > +struct intel_dp;
> > +
> > +bool intel_is_vrr_capable(struct drm_connector *connector);
> > +
> > +#endif /* __INTEL_VRR_H__ */
> > -- 
> > 2.19.1
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/11] drm/i915/display/vrr: Create VRR file and add VRR capability check

[Navare, Manasi]

On Tue, Nov 10, 2020 at 12:39:08PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > We create a new file for all VRR related helpers.
> > Also add a function to check vrr capability based on
> > platform support, DPCD bits and EDID monitor range.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile            |  1 +
> >  drivers/gpu/drm/i915/display/intel_vrr.c | 28 ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_vrr.h | 19 ++++++++++++++++
> >  3 files changed, 48 insertions(+)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.c
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index e5574e506a5c..3beeaf517191 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -249,6 +249,7 @@ i915-y += \
> >  	display/intel_sdvo.o \
> >  	display/intel_tv.o \
> >  	display/intel_vdsc.o \
> > +	display/intel_vrr.o \
> >  	display/vlv_dsi.o \
> >  	display/vlv_dsi_pll.o
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > new file mode 100644
> > index 000000000000..0c8a91fabb64
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2020 Intel Corporation
> > + *
> > + * Author: Manasi Navare <manasi.d.navare@intel.com>
> 
> I have increasingly mixed feelings about adding author lines in files in
> big collaborative projects. They tend to go out of date fairly quickly,
> and will cease to represent the contributions fairly. And git already
> gives us this information.

Thanks Jani, yes will remove the author name then.

> 
> > + */
> > +
> > +#include "i915_drv.h"
> > +#include "intel_display_types.h"
> > +#include "intel_vrr.h"
> > +
> > +bool intel_is_vrr_capable(struct drm_connector *connector)
> 
> Please prefix with intel_vrr_ consistently.

Will do, and change this to intel_vrr_is_capable()

> 
> > +{
> > +	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
> 
> I kind of feel like either the function should a) ensure it's okay to do
> intel_attached_dp() and return false if not, or b) just use struct
> intel_dp as the parameter.
> 
> As it is, passing a non-dp connector to this function will fail either
> subtly or spectacularly, but not graciously.

Yes I agree here. I think passing intel_dp is a good idea as anyway this function
is currently only called from intel_dp specific functions and after brainstorming a bit
on how this will look for the VRR on native HDMI, it will likely require its own helpers.
So infact I was thinking of even renaming this as intel_vrr_is_capable_dp() and send intel_dp to it
and then intel_vrr_is_capable_hdmi() can be added later.
What do you think?

> 
> > +	const struct drm_display_info *info = &connector->display_info;
> > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> 
> Please use i915 over dev_priv in all new code.

Okay

> 
> > +
> > +	/*
> > +	 * DP Sink is capable of Variable refresh video timings if
> > +	 * Ignore MSA bit is set in DPCD.
> > +	 * EDID monitor range also should be atleast 10 for reasonable
> > +	 * Adaptive sync/ VRR end user experience.
> > +	 */
> 
> Please fix typos etc.

Did I miss some typos, I dont see any in the above comment?

> 
> > +	return INTEL_GEN(dev_priv) >= 12 &&
> > +		drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd) &&
> > +		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
> > +}
> > +
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > new file mode 100644
> > index 000000000000..755746c7525c
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2019 Intel Corporation
> > +*/
> > +
> > +#ifndef __INTEL_VRR_H__
> > +#define __INTEL_VRR_H__
> > +
> > +#include <linux/types.h>
> > +
> > +struct drm_connector;
> 
> All of the below declarations are unnecessary at this commit.

Yes will add them in the next commits as I add the next functions, got it.

Regards
Manasi

> 
> BR,
> Jani.
> 
> > +struct drm_i915_private;
> > +struct intel_crtc_state;
> > +struct intel_encoder;
> > +struct intel_dp;
> > +
> > +bool intel_is_vrr_capable(struct drm_connector *connector);
> > +
> > +#endif /* __INTEL_VRR_H__ */
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/11] drm/i915/display/vrr: Disable VRR in modeset disable path

[Navare, Manasi]

On Tue, Nov 10, 2020 at 01:01:09PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > This patch disables the VRR enable and VRR PUSH
> > bits in the HW during commit modeset disable sequence.
> >
> > Thsi disable will happen when the port is disabled
> > or when the userspace sets VRR prop to false and
> > requests to disable VRR.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c |  2 ++
> >  drivers/gpu/drm/i915/display/intel_vrr.c | 22 ++++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_vrr.h |  1 +
> >  3 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 391c51979334..565155af3fb9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3819,6 +3819,8 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
> >  
> >  		intel_disable_pipe(old_crtc_state);
> >  
> > +		intel_vrr_disable(old_crtc_state);
> > +
> >  		intel_ddi_disable_transcoder_func(old_crtc_state);
> >  
> >  		intel_dsc_disable(old_crtc_state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index ec1ce88e869c..5075ecb9b5a7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -119,3 +119,25 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
> >  		pipe_name(pipe));
> >  }
> >  
> > +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
> > +{
> > +	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> 
> Haven't commented on all patches, but please use i915 instead of
> dev_priv for new code, throughout.
> 
> > +	enum pipe pipe = crtc->pipe;
> > +	u32 trans_vrr_ctl = 0, trans_push = 0;
> 
> Unnecessary initializations, and in fact unnecessary variables with
> intel_de_rmw.
>

Okay yes will try using the intel_de_rmw here and use the (VRR_CTL_FLIP_LINE_EN | VRR_CTL_VRR_ENABLE) directly in the clear field
 
> > +
> > +	if (!old_crtc_state->vrr.enable)
> > +		return;
> > +
> > +	trans_vrr_ctl = intel_de_read(dev_priv, TRANS_VRR_CTL(pipe));
> > +	trans_vrr_ctl &= ~(VRR_CTL_FLIP_LINE_EN | VRR_CTL_VRR_ENABLE);
> > +	intel_de_write(dev_priv, TRANS_VRR_CTL(pipe), trans_vrr_ctl);
> > +
> > +	trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe));
> > +	trans_push &= ~TRANS_PUSH_EN;
> > +	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
> 
> Please use intel_de_rmw for both.
> 
> > +
> > +	drm_dbg(&dev_priv->drm, "Disabling VRR on Pipe (%c)\n",
> > +		pipe_name(pipe));
> 
> drm_dbg_kms, "pipe %c" is the convention.

Okay will correct it

Thanks for the above feedback I will fix them in the next rev

Manasi


> 
> > +}
> > +
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > index a6b78e1676cb..8c6fd2d1bee5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > @@ -20,5 +20,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp,
> >  void intel_vrr_enable(struct intel_encoder *encoder,
> >  		      const struct intel_crtc_state *crtc_state);
> >  void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
> > +void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
> >  
> >  #endif /* __INTEL_VRR_H__ */
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/11] drm/i915: Add REG_FIELD_PREP to VRR register def

[Navare, Manasi]

On Tue, Nov 10, 2020 at 12:13:09PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > VRR_CTL register onloy had a GENMASK but no field prep
> > define for TRANS_VRR_CTL_LINE_COUNT field so add that
> 
> For the subject, I think mentioning VRR_CTL_LINK_COUNT is more important
> than REG_FIELD_PREP.

Yes will make that change in the commit message and use your r-b
Thanks for the review

Manasi
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> >
> > Cc: Aditya Swarup <aditya.swarup@intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index d4952c9875fb..9792c931b4c5 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4348,6 +4348,7 @@ enum {
> >  #define   VRR_CTL_IGN_MAX_SHIFT		REG_BIT(30)
> >  #define   VRR_CTL_FLIP_LINE_EN		REG_BIT(29)
> >  #define   VRR_CTL_LINE_COUNT_MASK	REG_GENMASK(10, 3)
> > +#define   VRR_CTL_LINE_COUNT(x)		REG_FIELD_PREP(VRR_CTL_LINE_COUNT_MASK, (x))
> >  #define   VRR_CTL_SW_FULLLINE_COUNT	REG_BIT(0)
> >  
> >  #define _TRANS_VRR_VMAX_A		0x60424
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/11] drm/i915/display/dp: Attach and set drm connector VRR property

[Navare, Manasi]

On Tue, Nov 10, 2020 at 12:41:07PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > From: Aditya Swarup <aditya.swarup@intel.com>
> >
> > This function sets the VRR property for connector based
> > on the platform support, EDID monitor range and DP sink
> > DPCD capability of outputing video without msa
> > timing information.
> >
> > v7:
> > * Move the helper to separate file (Manasi)
> > v6:
> > * Remove unset of prop
> > v5:
> > * Fix the vrr prop not being set in kernel (Manasi)
> > * Unset the prop on connector disconnect (Manasi)
> > v4:
> > * Rebase (Mansi)
> > v3:
> > * intel_dp_is_vrr_capable can be used for debugfs, make it
> > non static (Manasi)
> > v2:
> > * Just set this in intel_dp_get_modes instead of new hook (Jani)
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++++
> >  drivers/gpu/drm/i915/display/intel_dp.h | 1 +
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 818daab252f3..3794b8f35edc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -62,6 +62,7 @@
> >  #include "intel_sideband.h"
> >  #include "intel_tc.h"
> >  #include "intel_vdsc.h"
> > +#include "intel_vrr.h"
> >  
> >  #define DP_DPRX_ESI_LEN 14
> >  
> > @@ -6622,6 +6623,10 @@ static int intel_dp_get_modes(struct drm_connector *connector)
> >  	edid = intel_connector->detect_edid;
> >  	if (edid) {
> >  		int ret = intel_connector_update_modes(connector, edid);
> > +
> > +		if (intel_is_vrr_capable(connector))
> > +			drm_connector_set_vrr_capable_property(connector,
> > +							       true);
> >  		if (ret)
> >  			return ret;
> >  	}
> > @@ -7080,6 +7085,9 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
> >  		connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT;
> >  
> >  	}
> > +
> > +	if (INTEL_GEN(dev_priv) >= 12)
> 
> I wonder if we should just add a wrapper
> 
> #define HAS_VRR(i915) (INTEL_GEN(i915) >= 12)
> 
> to be more descriptive. And use it in the previous patch too.

Yea I like the idea of adding this wrapper in intel_vrr.c and have the platform check in that
and then use that in intel_vrr_is_capable() ?

> 
> > +		drm_connector_attach_vrr_capable_property(connector);
> >  }
> >  
> >  static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > index 3f862b4fd34f..aaf0a41582d7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -15,6 +15,7 @@ enum pipe;
> >  enum port;
> >  struct drm_connector_state;
> >  struct drm_encoder;
> > +struct drm_connector;
> 
> Unrelated change.

Yea I dont rem now why I added that here probably unrelated will remove

Manasi

> 
> >  struct drm_i915_private;
> >  struct drm_modeset_acquire_ctx;
> >  struct drm_dp_vsc_sdp;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/11] drm/i915/display/dp: Add VRR crtc state variables

[Navare, Manasi]

On Tue, Nov 10, 2020 at 12:41:50PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > Introduce VRR struct in intel_crtc_state and add
> > VRR crtc state variables Enable, Vtotalmin and Vtotalmax
> > to be derived from mode timings and VRR crtc property.
> 
> I'd squash this to the patch actually using it.

Okay will squash with Patch 5

Manasi

> 
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display_types.h | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index f6f0626649e0..f6f7ec024da7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1092,6 +1092,13 @@ struct intel_crtc_state {
> >  	struct intel_dsb *dsb;
> >  
> >  	u32 psr2_man_track_ctl;
> > +
> > +	/* Variable Refresh Rate state */
> > +	struct {
> > +		bool enable;
> > +		u16 vtotalmin;
> > +		u16 vtotalmax;
> > +	} vrr;
> >  };
> >  
> >  enum intel_pipe_crc_source {
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/11] drm/i915/display/dp: Compute VRR state in atomic_check

[Navare, Manasi]

On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > This forces a complete modeset if vrr drm crtc state goes
> > from enabled to disabled and vice versa.
> > This patch also computes vrr state variables from the mode timings
> > and based on the vrr property set by userspace as well as hardware's
> > vrr capability.
> >
> > v2:
> > *Rebase
> > v3:
> > * Vmin = max (vtotal, vmin) (Manasi)
> > v4:
> > * set crtc_state->vrr.enable = 0 for disable request
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c |  7 +++-
> >  drivers/gpu/drm/i915/display/intel_dp.c      |  1 +
> >  drivers/gpu/drm/i915/display/intel_vrr.c     | 38 ++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_vrr.h     |  2 ++
> >  4 files changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index f41b6f8b5618..f70cc3b2a1a4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14213,6 +14213,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> >  
> >  	PIPE_CONF_CHECK_I(mst_master_transcoder);
> >  
> > +	PIPE_CONF_CHECK_BOOL(vrr.enable);
> > +	PIPE_CONF_CHECK_I(vrr.vtotalmin);
> > +	PIPE_CONF_CHECK_I(vrr.vtotalmax);
> > +
> >  #undef PIPE_CONF_CHECK_X
> >  #undef PIPE_CONF_CHECK_I
> >  #undef PIPE_CONF_CHECK_BOOL
> > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev,
> >  
> >  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> >  					    new_crtc_state, i) {
> > -		if (new_crtc_state->inherited != old_crtc_state->inherited)
> > +		if (new_crtc_state->inherited != old_crtc_state->inherited ||
> > +		    new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled)
> 
> Somehow this feels like a really specific check to add considering the
> abstraction level of the function in general.

Should I then create a separate function to force a full modeset by setting mode changed 
if vrr_enabled changed?
And call that from intel_atomic_check() ?

> 
> >  			new_crtc_state->uapi.mode_changed = true;
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 3794b8f35edc..3185c4ca523d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -2752,6 +2752,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  	if (!HAS_DDI(dev_priv))
> >  		intel_dp_set_clock(encoder, pipe_config);
> >  
> > +	intel_vrr_compute_config(intel_dp, pipe_config);
> >  	intel_psr_compute_config(intel_dp, pipe_config);
> >  	intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp,
> >  				     constant_n);
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 0c8a91fabb64..56114f535f94 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -26,3 +26,41 @@ bool intel_is_vrr_capable(struct drm_connector *connector)
> >  		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
> >  }
> >  
> > +void
> > +intel_vrr_compute_config(struct intel_dp *intel_dp,
> > +			 struct intel_crtc_state *crtc_state)
> > +{
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +	struct intel_connector *intel_connector = intel_dp->attached_connector;
> > +	struct drm_connector *connector = &intel_connector->base;
> > +	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> > +	const struct drm_display_info *info = &connector->display_info;
> > +
> > +	if (!intel_is_vrr_capable(connector))
> > +		return;
> > +
> > +	if (!crtc_state->uapi.vrr_enabled) {
> > +		drm_dbg(&dev_priv->drm,
> > +			"VRR disable requested by Userspace\n");
> 
> drm_dbg_kms, though is this useful information? Quite a bit of log spam
> I'd think.

Yea this one can probably remove

Manasi


> 
> > +		crtc_state->vrr.enable = false;
> > +		return;
> > +	}
> > +
> > +	crtc_state->vrr.enable = true;
> > +	crtc_state->vrr.vtotalmin =
> > +		max_t(u16, adjusted_mode->crtc_vtotal,
> > +		      DIV_ROUND_CLOSEST(adjusted_mode->crtc_clock * 1000,
> > +					adjusted_mode->crtc_htotal *
> > +					info->monitor_range.max_vfreq));
> > +	crtc_state->vrr.vtotalmax =
> > +		max_t(u16, adjusted_mode->crtc_vtotal,
> > +		      DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> > +				   adjusted_mode->crtc_htotal *
> > +				   info->monitor_range.min_vfreq));
> > +
> > +	drm_dbg(&dev_priv->drm,
> 
> drm_dbg_kms
> 
> > +		"VRR Config: Enable = %s Vtotal Min = %d Vtotal Max = %d\n",
> > +		 yesno(crtc_state->vrr.enable), crtc_state->vrr.vtotalmin,
> > +		 crtc_state->vrr.vtotalmax);
> > +}
> > +
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > index 755746c7525c..1e6fe8fe92ec 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > @@ -15,5 +15,7 @@ struct intel_encoder;
> >  struct intel_dp;
> >  
> >  bool intel_is_vrr_capable(struct drm_connector *connector);
> > +void intel_vrr_compute_config(struct intel_dp *intel_dp,
> > +			      struct intel_crtc_state *crtc_state);
> >  
> >  #endif /* __INTEL_VRR_H__ */
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/11] drm/i915/display/vrr: Configure and enable VRR in modeset enable

[Navare, Manasi]

On Tue, Nov 10, 2020 at 12:56:40PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > This patch computes the VRR parameters from VRR crtc states
> > and configures them in VRR registers during CRTC enable in
> > the modeset enable sequence.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c |  5 ++++
> >  drivers/gpu/drm/i915/display/intel_vrr.c | 38 ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_vrr.h |  2 ++
> >  3 files changed, 45 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 09811be08cfe..391c51979334 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -49,6 +49,7 @@
> >  #include "intel_sprite.h"
> >  #include "intel_tc.h"
> >  #include "intel_vdsc.h"
> > +#include "intel_vrr.h"
> >  
> >  struct ddi_buf_trans {
> >  	u32 trans1;	/* balance leg enable, de-emph level */
> > @@ -4046,6 +4047,10 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
> >  
> >  	intel_ddi_enable_transcoder_func(encoder, crtc_state);
> >  
> > +	/* Enable VRR if requested through CRTC property */
> 
> I don't think the comment is helpful, really.

Yes will remove

> 
> > +	if (crtc_state->vrr.enable)
> > +		intel_vrr_enable(encoder, crtc_state);
> 
> In the disable path you check the vrr.enable within the
> function. Perhaps we should do the same here, i.e. call vrr enable
> unconditionally and have it early return if not requested.

Okay will do

> 
> > +
> >  	intel_enable_pipe(crtc_state);
> >  
> >  	intel_crtc_vblank_on(crtc_state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 56114f535f94..7f1353bac583 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -64,3 +64,41 @@ intel_vrr_compute_config(struct intel_dp *intel_dp,
> >  		 crtc_state->vrr.vtotalmax);
> >  }
> >  
> > +void intel_vrr_enable(struct intel_encoder *encoder,
> > +		      const struct intel_crtc_state *crtc_state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +	enum pipe pipe = crtc->pipe;
> > +	const struct drm_display_mode *adjusted_mode =
> > +		&crtc_state->hw.adjusted_mode;
> > +	u32 trans_vrr_ctl = 0, trans_vrr_vmax = 0, trans_vrr_vmin = 0, trans_vrr_flipline = 0, trans_push = 0;
> > +	u16 framestart_to_pipelinefull_linecnt = 0;
> 
> All the initializations to 0 are unnecessary.

Will remove

> 
> > +
> > +	framestart_to_pipelinefull_linecnt =
> > +		min_t(u16, 255, (crtc_state->vrr.vtotalmin - adjusted_mode->crtc_vdisplay));
> > +
> > +	trans_vrr_ctl = VRR_CTL_VRR_ENABLE |  VRR_CTL_IGN_MAX_SHIFT |
> > +		VRR_CTL_FLIP_LINE_EN | VRR_CTL_LINE_COUNT(framestart_to_pipelinefull_linecnt) |
> > +		VRR_CTL_SW_FULLLINE_COUNT;
> > +
> > +	/* Programming adjustments for 0 based regs */
> > +	trans_vrr_vmax = crtc_state->vrr.vtotalmax - 1;
> > +	trans_vrr_vmin = crtc_state->vrr.vtotalmin - 1;
> > +	trans_vrr_flipline = crtc_state->vrr.vtotalmin - 1;
> > +
> > +	trans_push = TRANS_PUSH_EN;
> 
> Frankly I'd just throw away the above four temp variables.

Okay and use them directly in de_write ?

> 
> > +
> > +	intel_de_write(dev_priv, TRANS_VRR_VMIN(pipe), trans_vrr_vmin);
> > +	intel_de_write(dev_priv, TRANS_VRR_VMAX(pipe), trans_vrr_vmax);
> > +	intel_de_write(dev_priv, TRANS_VRR_CTL(pipe), trans_vrr_ctl);
> > +	intel_de_write(dev_priv, TRANS_VRR_FLIPLINE(pipe), trans_vrr_flipline);
> > +	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
> > +
> > +	drm_dbg(&dev_priv->drm,	"Enabling VRR on pipe (%c)\n", pipe_name(pipe));
> 
> drm_dbg_kms. "pipe %c" is the convention, not "pipe (%c)".

Got it will fix this

Thanks for the inputs

Manasi


> 
> > +	drm_dbg(&dev_priv->drm, "VRR Parameters: Vblank - Min = %d, Max = %d Flipline Count = %d, CTL Reg = 0x%08x, TRANS PUSH reg = 0x%08x",
> > +		crtc_state->vrr.vtotalmin, crtc_state->vrr.vtotalmax,
> > +		crtc_state->vrr.vtotalmin, trans_vrr_ctl,
> > +		trans_push);
> > +}
> > +
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > index 1e6fe8fe92ec..05d982d6fbae 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > @@ -17,5 +17,7 @@ struct intel_dp;
> >  bool intel_is_vrr_capable(struct drm_connector *connector);
> >  void intel_vrr_compute_config(struct intel_dp *intel_dp,
> >  			      struct intel_crtc_state *crtc_state);
> > +void intel_vrr_enable(struct intel_encoder *encoder,
> > +		      const struct intel_crtc_state *crtc_state);
> >  
> >  #endif /* __INTEL_VRR_H__ */
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/11] drm/i915/display/vrr: Send VRR push to flip the frame

[Navare, Manasi]

On Tue, Nov 10, 2020 at 12:59:10PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > VRR achieves vblank stretching using the HW PUSH functionality.
> > So once the VRR is enabled during modeset then for each flip
> > request from userspace, in the atomic tail pipe_update_end()
> > we need to set the VRR push bit in HW for it to terminate
> > the vblank at configured flipline or anytime after flipline
> > or latest at the Vmax.
> >
> > The HW clears the PUSH bit after the double buffer updates
> > are completed.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_sprite.c |  5 +++++
> >  drivers/gpu/drm/i915/display/intel_vrr.c    | 17 +++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_vrr.h    |  1 +
> >  3 files changed, 23 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index b6deeb338477..cb10fe462f06 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -49,6 +49,7 @@
> >  #include "intel_psr.h"
> >  #include "intel_dsi.h"
> >  #include "intel_sprite.h"
> > +#include "intel_vrr.h"
> >  
> >  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
> >  			     int usecs)
> > @@ -217,6 +218,10 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> >  	    intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI))
> >  		icl_dsi_frame_update(new_crtc_state);
> >  
> > +	/* Send VRR Push to terminate Vblank */
> > +	if (new_crtc_state->vrr.enable)
> > +		intel_vrr_send_push(new_crtc_state);
> > +
> 
> Maybe move the vrr.enable check to the function?

Yes makes sense will do

> 
> >  	/* We're still in the vblank-evade critical section, this can't race.
> >  	 * Would be slightly nice to just grab the vblank count and arm the
> >  	 * event outside of the critical section - the spinlock might spin for a
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 7f1353bac583..ec1ce88e869c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -102,3 +102,20 @@ void intel_vrr_enable(struct intel_encoder *encoder,
> >  		trans_push);
> >  }
> >  
> > +void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
> > +{
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +	enum pipe pipe = crtc->pipe;
> > +	u32 trans_push;
> > +
> > +	trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe));
> > +	WARN_ON(!(trans_push & TRANS_PUSH_EN));
> 
> drm_WARN_ON, and perhaps move this below the register rmw. It doesn't
> change the flow anyway.

Yes will do

Manasi


> 
> > +
> > +	trans_push |= TRANS_PUSH_SEND;
> > +	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
> > +
> > +	drm_dbg(&dev_priv->drm, "Sending VRR Push on Pipe (%c)\n",
> > +		pipe_name(pipe));
> 
> drm_dbg_kms
> 
> > +}
> > +
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > index 05d982d6fbae..a6b78e1676cb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > @@ -19,5 +19,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp,
> >  			      struct intel_crtc_state *crtc_state);
> >  void intel_vrr_enable(struct intel_encoder *encoder,
> >  		      const struct intel_crtc_state *crtc_state);
> > +void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
> >  
> >  #endif /* __INTEL_VRR_H__ */
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/11] drm/i915/display/vrr: Set IGNORE_MSA_PAR state in DP Sink

[Navare, Manasi]

@Jani could you review this as well?

Manasi


On Thu, Oct 22, 2020 at 03:27:08PM -0700, Manasi Navare wrote:
> If VRR is enabled, the sink should ignore MSA parameters
> and regenerate incoming video stream without depending
> on these parameters. Hence set the MSA_TIMING_PAR_IGNORE_EN
> bit if VRR is enabled.
> Reset this bit on VRR disable.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 25 ++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 565155af3fb9..195449dfec1e 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3322,6 +3322,22 @@ i915_reg_t dp_tp_status_reg(struct intel_encoder *encoder,
>  		return DP_TP_STATUS(encoder->port);
>  }
>  
> +static void intel_dp_sink_set_msa_timing_par_ignore_state(struct intel_dp *intel_dp,
> +							  const struct intel_crtc_state *crtc_state,
> +							  bool enable)
> +{
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +
> +	if (!crtc_state->vrr.enable)
> +		return;
> +
> +	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_DOWNSPREAD_CTRL,
> +			       enable ? DP_MSA_TIMING_PAR_IGNORE_EN : 0) <= 0)
> +		drm_dbg_kms(&i915->drm,
> +			    "Failed to set MSA_TIMING_PAR_IGNORE %s in the sink\n",
> +			    enable ? "enable" : "disable");
> +}
> +
>  static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
>  					const struct intel_crtc_state *crtc_state)
>  {
> @@ -3493,6 +3509,12 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	 */
>  	intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
>  
> +	/*
> +	 * Sink device should ignore MSA parameters and regenerate
> +	 * incoming video stream in case of VRR/Adaptive Sync
> +	 */
> +	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, crtc_state, true);
> +
>  	/*
>  	 * 7.i Follow DisplayPort specification training sequence (see notes for
>  	 *     failure handling)
> @@ -4089,6 +4111,9 @@ static void intel_disable_ddi_dp(struct intel_atomic_state *state,
>  	/* Disable the decompression in DP Sink */
>  	intel_dp_sink_set_decompression_state(intel_dp, old_crtc_state,
>  					      false);
> +	/* Disable Ignore_MSA bit in DP Sink */
> +	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, old_crtc_state,
> +						      false);
>  }
>  
>  static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
> -- 
> 2.19.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/11] drm/i915/display/dp: Compute VRR state in atomic_check

[Navare, Manasi]

On Tue, Dec 01, 2020 at 02:52:59PM -0800, Navare, Manasi wrote:
> On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote:
> > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > > This forces a complete modeset if vrr drm crtc state goes
> > > from enabled to disabled and vice versa.
> > > This patch also computes vrr state variables from the mode timings
> > > and based on the vrr property set by userspace as well as hardware's
> > > vrr capability.
> > >
> > > v2:
> > > *Rebase
> > > v3:
> > > * Vmin = max (vtotal, vmin) (Manasi)
> > > v4:
> > > * set crtc_state->vrr.enable = 0 for disable request
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c |  7 +++-
> > >  drivers/gpu/drm/i915/display/intel_dp.c      |  1 +
> > >  drivers/gpu/drm/i915/display/intel_vrr.c     | 38 ++++++++++++++++++++
> > >  drivers/gpu/drm/i915/display/intel_vrr.h     |  2 ++
> > >  4 files changed, 47 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index f41b6f8b5618..f70cc3b2a1a4 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -14213,6 +14213,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> > >  
> > >  	PIPE_CONF_CHECK_I(mst_master_transcoder);
> > >  
> > > +	PIPE_CONF_CHECK_BOOL(vrr.enable);
> > > +	PIPE_CONF_CHECK_I(vrr.vtotalmin);
> > > +	PIPE_CONF_CHECK_I(vrr.vtotalmax);
> > > +
> > >  #undef PIPE_CONF_CHECK_X
> > >  #undef PIPE_CONF_CHECK_I
> > >  #undef PIPE_CONF_CHECK_BOOL
> > > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev,
> > >  
> > >  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> > >  					    new_crtc_state, i) {
> > > -		if (new_crtc_state->inherited != old_crtc_state->inherited)
> > > +		if (new_crtc_state->inherited != old_crtc_state->inherited ||
> > > +		    new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled)
> > 
> > Somehow this feels like a really specific check to add considering the
> > abstraction level of the function in general.

Actually while discussing with @Ville on IRC, he had proposed just adding it here
since we already have this loop existing that loops through the old and new crtc states
and we need to set the mode_changed = true right up at the top.
But if you think its more intuitive to create a separate function for this I could do that

Ville, Jani N any thoughts?

Manasi

> 
> Should I then create a separate function to force a full modeset by setting mode changed 
> if vrr_enabled changed?
> And call that from intel_atomic_check() ?
> 
> > 
> > >  			new_crtc_state->uapi.mode_changed = true;
> > >  	}
> > >  
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 3794b8f35edc..3185c4ca523d 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -2752,6 +2752,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > >  	if (!HAS_DDI(dev_priv))
> > >  		intel_dp_set_clock(encoder, pipe_config);
> > >  
> > > +	intel_vrr_compute_config(intel_dp, pipe_config);
> > >  	intel_psr_compute_config(intel_dp, pipe_config);
> > >  	intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp,
> > >  				     constant_n);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > index 0c8a91fabb64..56114f535f94 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > @@ -26,3 +26,41 @@ bool intel_is_vrr_capable(struct drm_connector *connector)
> > >  		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
> > >  }
> > >  
> > > +void
> > > +intel_vrr_compute_config(struct intel_dp *intel_dp,
> > > +			 struct intel_crtc_state *crtc_state)
> > > +{
> > > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > +	struct intel_connector *intel_connector = intel_dp->attached_connector;
> > > +	struct drm_connector *connector = &intel_connector->base;
> > > +	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> > > +	const struct drm_display_info *info = &connector->display_info;
> > > +
> > > +	if (!intel_is_vrr_capable(connector))
> > > +		return;
> > > +
> > > +	if (!crtc_state->uapi.vrr_enabled) {
> > > +		drm_dbg(&dev_priv->drm,
> > > +			"VRR disable requested by Userspace\n");
> > 
> > drm_dbg_kms, though is this useful information? Quite a bit of log spam
> > I'd think.
> 
> Yea this one can probably remove
> 
> Manasi
> 
> 
> > 
> > > +		crtc_state->vrr.enable = false;
> > > +		return;
> > > +	}
> > > +
> > > +	crtc_state->vrr.enable = true;
> > > +	crtc_state->vrr.vtotalmin =
> > > +		max_t(u16, adjusted_mode->crtc_vtotal,
> > > +		      DIV_ROUND_CLOSEST(adjusted_mode->crtc_clock * 1000,
> > > +					adjusted_mode->crtc_htotal *
> > > +					info->monitor_range.max_vfreq));
> > > +	crtc_state->vrr.vtotalmax =
> > > +		max_t(u16, adjusted_mode->crtc_vtotal,
> > > +		      DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> > > +				   adjusted_mode->crtc_htotal *
> > > +				   info->monitor_range.min_vfreq));
> > > +
> > > +	drm_dbg(&dev_priv->drm,
> > 
> > drm_dbg_kms
> > 
> > > +		"VRR Config: Enable = %s Vtotal Min = %d Vtotal Max = %d\n",
> > > +		 yesno(crtc_state->vrr.enable), crtc_state->vrr.vtotalmin,
> > > +		 crtc_state->vrr.vtotalmax);
> > > +}
> > > +
> > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > > index 755746c7525c..1e6fe8fe92ec 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > > @@ -15,5 +15,7 @@ struct intel_encoder;
> > >  struct intel_dp;
> > >  
> > >  bool intel_is_vrr_capable(struct drm_connector *connector);
> > > +void intel_vrr_compute_config(struct intel_dp *intel_dp,
> > > +			      struct intel_crtc_state *crtc_state);
> > >  
> > >  #endif /* __INTEL_VRR_H__ */
> > 
> > -- 
> > Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/11] drm/i915/display/vrr: Create VRR file and add VRR capability check

[Navare, Manasi]

On Tue, Dec 01, 2020 at 02:21:56PM -0800, Navare, Manasi wrote:
> On Tue, Nov 10, 2020 at 12:39:08PM +0200, Jani Nikula wrote:
> > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > > We create a new file for all VRR related helpers.
> > > Also add a function to check vrr capability based on
> > > platform support, DPCD bits and EDID monitor range.
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/Makefile            |  1 +
> > >  drivers/gpu/drm/i915/display/intel_vrr.c | 28 ++++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/display/intel_vrr.h | 19 ++++++++++++++++
> > >  3 files changed, 48 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.c
> > >  create mode 100644 drivers/gpu/drm/i915/display/intel_vrr.h
> > >
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index e5574e506a5c..3beeaf517191 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -249,6 +249,7 @@ i915-y += \
> > >  	display/intel_sdvo.o \
> > >  	display/intel_tv.o \
> > >  	display/intel_vdsc.o \
> > > +	display/intel_vrr.o \
> > >  	display/vlv_dsi.o \
> > >  	display/vlv_dsi_pll.o
> > >  
> > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > new file mode 100644
> > > index 000000000000..0c8a91fabb64
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > @@ -0,0 +1,28 @@
> > > +/* SPDX-License-Identifier: MIT */
> > > +/*
> > > + * Copyright © 2020 Intel Corporation
> > > + *
> > > + * Author: Manasi Navare <manasi.d.navare@intel.com>
> > 
> > I have increasingly mixed feelings about adding author lines in files in
> > big collaborative projects. They tend to go out of date fairly quickly,
> > and will cease to represent the contributions fairly. And git already
> > gives us this information.
> 
> Thanks Jani, yes will remove the author name then.
> 
> > 
> > > + */
> > > +
> > > +#include "i915_drv.h"
> > > +#include "intel_display_types.h"
> > > +#include "intel_vrr.h"
> > > +
> > > +bool intel_is_vrr_capable(struct drm_connector *connector)
> > 
> > Please prefix with intel_vrr_ consistently.
> 
> Will do, and change this to intel_vrr_is_capable()
> 
> > 
> > > +{
> > > +	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
> > 
> > I kind of feel like either the function should a) ensure it's okay to do
> > intel_attached_dp() and return false if not, or b) just use struct
> > intel_dp as the parameter.
> > 
> > As it is, passing a non-dp connector to this function will fail either
> > subtly or spectacularly, but not graciously.

Actually after doing some code rewriting, I think it is best to still pass
the drm connector but I am adding the check now for connector type
Only if its eDP or DP now I get the intel_dp.
Future we can add HDMI there as well.

Manasi

> 
> Yes I agree here. I think passing intel_dp is a good idea as anyway this function
> is currently only called from intel_dp specific functions and after brainstorming a bit
> on how this will look for the VRR on native HDMI, it will likely require its own helpers.
> So infact I was thinking of even renaming this as intel_vrr_is_capable_dp() and send intel_dp to it
> and then intel_vrr_is_capable_hdmi() can be added later.
> What do you think?
> 
> > 
> > > +	const struct drm_display_info *info = &connector->display_info;
> > > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > 
> > Please use i915 over dev_priv in all new code.
> 
> Okay
> 
> > 
> > > +
> > > +	/*
> > > +	 * DP Sink is capable of Variable refresh video timings if
> > > +	 * Ignore MSA bit is set in DPCD.
> > > +	 * EDID monitor range also should be atleast 10 for reasonable
> > > +	 * Adaptive sync/ VRR end user experience.
> > > +	 */
> > 
> > Please fix typos etc.
> 
> Did I miss some typos, I dont see any in the above comment?
> 
> > 
> > > +	return INTEL_GEN(dev_priv) >= 12 &&
> > > +		drm_dp_sink_can_do_video_without_timing_msa(intel_dp->dpcd) &&
> > > +		info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10;
> > > +}
> > > +
> > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > > new file mode 100644
> > > index 000000000000..755746c7525c
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > > @@ -0,0 +1,19 @@
> > > +/* SPDX-License-Identifier: MIT */
> > > +/*
> > > + * Copyright © 2019 Intel Corporation
> > > +*/
> > > +
> > > +#ifndef __INTEL_VRR_H__
> > > +#define __INTEL_VRR_H__
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +struct drm_connector;
> > 
> > All of the below declarations are unnecessary at this commit.
> 
> Yes will add them in the next commits as I add the next functions, got it.
> 
> Regards
> Manasi
> 
> > 
> > BR,
> > Jani.
> > 
> > > +struct drm_i915_private;
> > > +struct intel_crtc_state;
> > > +struct intel_encoder;
> > > +struct intel_dp;
> > > +
> > > +bool intel_is_vrr_capable(struct drm_connector *connector);
> > > +
> > > +#endif /* __INTEL_VRR_H__ */
> > 
> > -- 
> > Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/11] drm/i915/display/vrr: Create VRR file and add VRR capability check

[Jani Nikula]

On Wed, 02 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote:
> On Tue, Dec 01, 2020 at 02:21:56PM -0800, Navare, Manasi wrote:
>> On Tue, Nov 10, 2020 at 12:39:08PM +0200, Jani Nikula wrote:
>> > > +{
>> > > +	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
>> > 
>> > I kind of feel like either the function should a) ensure it's okay to do
>> > intel_attached_dp() and return false if not, or b) just use struct
>> > intel_dp as the parameter.
>> > 
>> > As it is, passing a non-dp connector to this function will fail either
>> > subtly or spectacularly, but not graciously.
>
> Actually after doing some code rewriting, I think it is best to still pass
> the drm connector but I am adding the check now for connector type
> Only if its eDP or DP now I get the intel_dp.
> Future we can add HDMI there as well.

It's fine, as long as we don't cast to intel_dp unless we know it's
intel_dp.

>> > > +
>> > > +	/*
>> > > +	 * DP Sink is capable of Variable refresh video timings if
>> > > +	 * Ignore MSA bit is set in DPCD.
>> > > +	 * EDID monitor range also should be atleast 10 for reasonable
>> > > +	 * Adaptive sync/ VRR end user experience.
>> > > +	 */
>> > 
>> > Please fix typos etc.
>> 
>> Did I miss some typos, I dont see any in the above comment?

Odd capitalization, "atleast", "sync/ VRR", maybe also reflow the
paragraph.

I know it's nitpicking, but other people will read the comment many,
many more times than you write it. ;)


BR,
Jani.



-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/11] drm/i915/display/dp: Attach and set drm connector VRR property

[Jani Nikula]

On Tue, 01 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote:
> On Tue, Nov 10, 2020 at 12:41:07PM +0200, Jani Nikula wrote:
>> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
>> > +
>> > +	if (INTEL_GEN(dev_priv) >= 12)
>> 
>> I wonder if we should just add a wrapper
>> 
>> #define HAS_VRR(i915) (INTEL_GEN(i915) >= 12)
>> 
>> to be more descriptive. And use it in the previous patch too.
>
> Yea I like the idea of adding this wrapper in intel_vrr.c and have the platform check in that
> and then use that in intel_vrr_is_capable() ?

Works for me, but might just throw that in i915_drv.h with all the other
HAS_ helpers also. *shrug*


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/11] drm/i915/display/dp: Compute VRR state in atomic_check

[Jani Nikula]

On Wed, 02 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote:
> On Tue, Dec 01, 2020 at 02:52:59PM -0800, Navare, Manasi wrote:
>> On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote:
>> > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
>> > > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev,
>> > >  
>> > >  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>> > >  					    new_crtc_state, i) {
>> > > -		if (new_crtc_state->inherited != old_crtc_state->inherited)
>> > > +		if (new_crtc_state->inherited != old_crtc_state->inherited ||
>> > > +		    new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled)
>> > 
>> > Somehow this feels like a really specific check to add considering the
>> > abstraction level of the function in general.
>
> Actually while discussing with @Ville on IRC, he had proposed just adding it here
> since we already have this loop existing that loops through the old and new crtc states
> and we need to set the mode_changed = true right up at the top.
> But if you think its more intuitive to create a separate function for this I could do that
>
> Ville, Jani N any thoughts?

It's just a gut feeling. Kind of uneasy feeling that in the future
people look at that, and see you have this check there, and then add
more, and more.

Anyway, whatever Ville says works for me as well. ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 07/11] drm/i915/display/vrr: Configure and enable VRR in modeset enable

[Jani Nikula]

On Tue, 01 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote:
> On Tue, Nov 10, 2020 at 12:56:40PM +0200, Jani Nikula wrote:
>> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
>> > +	/* Programming adjustments for 0 based regs */
>> > +	trans_vrr_vmax = crtc_state->vrr.vtotalmax - 1;
>> > +	trans_vrr_vmin = crtc_state->vrr.vtotalmin - 1;
>> > +	trans_vrr_flipline = crtc_state->vrr.vtotalmin - 1;
>> > +
>> > +	trans_push = TRANS_PUSH_EN;
>> 
>> Frankly I'd just throw away the above four temp variables.
>
> Okay and use them directly in de_write ?

Yes.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/11] drm/i915/display/vrr: Set IGNORE_MSA_PAR state in DP Sink

[Jani Nikula]

On Tue, 01 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote:
> @Jani could you review this as well?

Okay, I'm going to cop out here and say that, while I don't see anything
wrong here, I also didn't go through all the specs and verify this is
the right place to do this stuff. Let's see the updated version first.

BR,
Jani.

>
> Manasi
>
>
> On Thu, Oct 22, 2020 at 03:27:08PM -0700, Manasi Navare wrote:
>> If VRR is enabled, the sink should ignore MSA parameters
>> and regenerate incoming video stream without depending
>> on these parameters. Hence set the MSA_TIMING_PAR_IGNORE_EN
>> bit if VRR is enabled.
>> Reset this bit on VRR disable.
>> 
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_ddi.c | 25 ++++++++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index 565155af3fb9..195449dfec1e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -3322,6 +3322,22 @@ i915_reg_t dp_tp_status_reg(struct intel_encoder *encoder,
>>  		return DP_TP_STATUS(encoder->port);
>>  }
>>  
>> +static void intel_dp_sink_set_msa_timing_par_ignore_state(struct intel_dp *intel_dp,
>> +							  const struct intel_crtc_state *crtc_state,
>> +							  bool enable)
>> +{
>> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> +
>> +	if (!crtc_state->vrr.enable)
>> +		return;
>> +
>> +	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_DOWNSPREAD_CTRL,
>> +			       enable ? DP_MSA_TIMING_PAR_IGNORE_EN : 0) <= 0)
>> +		drm_dbg_kms(&i915->drm,
>> +			    "Failed to set MSA_TIMING_PAR_IGNORE %s in the sink\n",
>> +			    enable ? "enable" : "disable");
>> +}
>> +
>>  static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
>>  					const struct intel_crtc_state *crtc_state)
>>  {
>> @@ -3493,6 +3509,12 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>>  	 */
>>  	intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
>>  
>> +	/*
>> +	 * Sink device should ignore MSA parameters and regenerate
>> +	 * incoming video stream in case of VRR/Adaptive Sync
>> +	 */
>> +	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, crtc_state, true);
>> +
>>  	/*
>>  	 * 7.i Follow DisplayPort specification training sequence (see notes for
>>  	 *     failure handling)
>> @@ -4089,6 +4111,9 @@ static void intel_disable_ddi_dp(struct intel_atomic_state *state,
>>  	/* Disable the decompression in DP Sink */
>>  	intel_dp_sink_set_decompression_state(intel_dp, old_crtc_state,
>>  					      false);
>> +	/* Disable Ignore_MSA bit in DP Sink */
>> +	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, old_crtc_state,
>> +						      false);
>>  }
>>  
>>  static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
>> -- 
>> 2.19.1
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/11] drm/i915/display/vrr: Set IGNORE_MSA_PAR state in DP Sink

[Navare, Manasi]

On Thu, Dec 03, 2020 at 06:49:27PM +0200, Jani Nikula wrote:
> On Tue, 01 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote:
> > @Jani could you review this as well?
> 
> Okay, I'm going to cop out here and say that, while I don't see anything
> wrong here, I also didn't go through all the specs and verify this is
> the right place to do this stuff. Let's see the updated version first.

Actually the spec just says that before the link is up, source needs to set
this bit in DPCD so that the sink is ready to get the VRR timings and it knows
that the MSA timing can be ignored.

But yes will send out the updated rev today addressinga ll other review comments.
Thanks again for all your feedback so far!

Manasi
> 
> BR,
> Jani.
> 
> >
> > Manasi
> >
> >
> > On Thu, Oct 22, 2020 at 03:27:08PM -0700, Manasi Navare wrote:
> >> If VRR is enabled, the sink should ignore MSA parameters
> >> and regenerate incoming video stream without depending
> >> on these parameters. Hence set the MSA_TIMING_PAR_IGNORE_EN
> >> bit if VRR is enabled.
> >> Reset this bit on VRR disable.
> >> 
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> >> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_ddi.c | 25 ++++++++++++++++++++++++
> >>  1 file changed, 25 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> >> index 565155af3fb9..195449dfec1e 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> >> @@ -3322,6 +3322,22 @@ i915_reg_t dp_tp_status_reg(struct intel_encoder *encoder,
> >>  		return DP_TP_STATUS(encoder->port);
> >>  }
> >>  
> >> +static void intel_dp_sink_set_msa_timing_par_ignore_state(struct intel_dp *intel_dp,
> >> +							  const struct intel_crtc_state *crtc_state,
> >> +							  bool enable)
> >> +{
> >> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> >> +
> >> +	if (!crtc_state->vrr.enable)
> >> +		return;
> >> +
> >> +	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_DOWNSPREAD_CTRL,
> >> +			       enable ? DP_MSA_TIMING_PAR_IGNORE_EN : 0) <= 0)
> >> +		drm_dbg_kms(&i915->drm,
> >> +			    "Failed to set MSA_TIMING_PAR_IGNORE %s in the sink\n",
> >> +			    enable ? "enable" : "disable");
> >> +}
> >> +
> >>  static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
> >>  					const struct intel_crtc_state *crtc_state)
> >>  {
> >> @@ -3493,6 +3509,12 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
> >>  	 */
> >>  	intel_dp_sink_set_fec_ready(intel_dp, crtc_state);
> >>  
> >> +	/*
> >> +	 * Sink device should ignore MSA parameters and regenerate
> >> +	 * incoming video stream in case of VRR/Adaptive Sync
> >> +	 */
> >> +	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, crtc_state, true);
> >> +
> >>  	/*
> >>  	 * 7.i Follow DisplayPort specification training sequence (see notes for
> >>  	 *     failure handling)
> >> @@ -4089,6 +4111,9 @@ static void intel_disable_ddi_dp(struct intel_atomic_state *state,
> >>  	/* Disable the decompression in DP Sink */
> >>  	intel_dp_sink_set_decompression_state(intel_dp, old_crtc_state,
> >>  					      false);
> >> +	/* Disable Ignore_MSA bit in DP Sink */
> >> +	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, old_crtc_state,
> >> +						      false);
> >>  }
> >>  
> >>  static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
> >> -- 
> >> 2.19.1
> >> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/11] drm/i915/display/dp: Compute VRR state in atomic_check

[Navare, Manasi]

On Thu, Dec 03, 2020 at 06:39:43PM +0200, Jani Nikula wrote:
> On Wed, 02 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote:
> > On Tue, Dec 01, 2020 at 02:52:59PM -0800, Navare, Manasi wrote:
> >> On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote:
> >> > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> >> > > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev,
> >> > >  
> >> > >  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> >> > >  					    new_crtc_state, i) {
> >> > > -		if (new_crtc_state->inherited != old_crtc_state->inherited)
> >> > > +		if (new_crtc_state->inherited != old_crtc_state->inherited ||
> >> > > +		    new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled)
> >> > 
> >> > Somehow this feels like a really specific check to add considering the
> >> > abstraction level of the function in general.
> >
> > Actually while discussing with @Ville on IRC, he had proposed just adding it here
> > since we already have this loop existing that loops through the old and new crtc states
> > and we need to set the mode_changed = true right up at the top.
> > But if you think its more intuitive to create a separate function for this I could do that
> >
> > Ville, Jani N any thoughts?
> 
> It's just a gut feeling. Kind of uneasy feeling that in the future
> people look at that, and see you have this check there, and then add
> more, and more.
> 
> Anyway, whatever Ville says works for me as well. ;)

Yea I actually also agree reg this so let me just move this to a separate
function where itw ill loop through crtc states and force mode changed
for this condition.

If Ville thinks otherwise we can remove it since I havent got any review comments
from Ville yet.

Manasi

> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/11] drm/i915/display/dp: Attach and set drm connector VRR property

[Navare, Manasi]

On Thu, Dec 03, 2020 at 06:37:46PM +0200, Jani Nikula wrote:
> On Tue, 01 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote:
> > On Tue, Nov 10, 2020 at 12:41:07PM +0200, Jani Nikula wrote:
> >> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> >> > +
> >> > +	if (INTEL_GEN(dev_priv) >= 12)
> >> 
> >> I wonder if we should just add a wrapper
> >> 
> >> #define HAS_VRR(i915) (INTEL_GEN(i915) >= 12)
> >> 
> >> to be more descriptive. And use it in the previous patch too.
> >
> > Yea I like the idea of adding this wrapper in intel_vrr.c and have the platform check in that
> > and then use that in intel_vrr_is_capable() ?
> 
> Works for me, but might just throw that in i915_drv.h with all the other
> HAS_ helpers also. *shrug*

Yup, thats where I added HAS_VRR macro

Manasi

> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/11] drm/i915/display/vrr: Create VRR file and add VRR capability check

[Navare, Manasi]

On Thu, Dec 03, 2020 at 06:35:29PM +0200, Jani Nikula wrote:
> On Wed, 02 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote:
> > On Tue, Dec 01, 2020 at 02:21:56PM -0800, Navare, Manasi wrote:
> >> On Tue, Nov 10, 2020 at 12:39:08PM +0200, Jani Nikula wrote:
> >> > > +{
> >> > > +	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
> >> > 
> >> > I kind of feel like either the function should a) ensure it's okay to do
> >> > intel_attached_dp() and return false if not, or b) just use struct
> >> > intel_dp as the parameter.
> >> > 
> >> > As it is, passing a non-dp connector to this function will fail either
> >> > subtly or spectacularly, but not graciously.
> >
> > Actually after doing some code rewriting, I think it is best to still pass
> > the drm connector but I am adding the check now for connector type
> > Only if its eDP or DP now I get the intel_dp.
> > Future we can add HDMI there as well.
> 
> It's fine, as long as we don't cast to intel_dp unless we know it's
> intel_dp.
> 
> >> > > +
> >> > > +	/*
> >> > > +	 * DP Sink is capable of Variable refresh video timings if
> >> > > +	 * Ignore MSA bit is set in DPCD.
> >> > > +	 * EDID monitor range also should be atleast 10 for reasonable
> >> > > +	 * Adaptive sync/ VRR end user experience.
> >> > > +	 */
> >> > 
> >> > Please fix typos etc.
> >> 
> >> Did I miss some typos, I dont see any in the above comment?
> 
> Odd capitalization, "atleast", "sync/ VRR", maybe also reflow the
> paragraph.
> 
> I know it's nitpicking, but other people will read the comment many,
> many more times than you write it. ;)

Okay yes will remove the odd capitalizations.

Manasi

> 
> 
> BR,
> Jani.
> 
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/11] drm/i915/display/vrr: Send VRR push to flip the frame

[Navare, Manasi]

Actually one of the opens I had here was regarding the min and max calculation
in intel_pipe_update_start aroudn teh vblank evasion code.

Currently we have:
 min = vblank_start - intel_usecs_to_scanlines(adjusted_mode,
                                                      VBLANK_EVASION_TIME_US);
        max = vblank_start - 1;

But now with VRR, the vblank termination will happen at the flipline or at Vmax
So do we stall the updates wrt the vtotalmax ?

Regards
Manasi


On Tue, Dec 01, 2020 at 02:57:29PM -0800, Navare, Manasi wrote:
> On Tue, Nov 10, 2020 at 12:59:10PM +0200, Jani Nikula wrote:
> > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote:
> > > VRR achieves vblank stretching using the HW PUSH functionality.
> > > So once the VRR is enabled during modeset then for each flip
> > > request from userspace, in the atomic tail pipe_update_end()
> > > we need to set the VRR push bit in HW for it to terminate
> > > the vblank at configured flipline or anytime after flipline
> > > or latest at the Vmax.
> > >
> > > The HW clears the PUSH bit after the double buffer updates
> > > are completed.
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_sprite.c |  5 +++++
> > >  drivers/gpu/drm/i915/display/intel_vrr.c    | 17 +++++++++++++++++
> > >  drivers/gpu/drm/i915/display/intel_vrr.h    |  1 +
> > >  3 files changed, 23 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > index b6deeb338477..cb10fe462f06 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > @@ -49,6 +49,7 @@
> > >  #include "intel_psr.h"
> > >  #include "intel_dsi.h"
> > >  #include "intel_sprite.h"
> > > +#include "intel_vrr.h"
> > >  
> > >  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
> > >  			     int usecs)
> > > @@ -217,6 +218,10 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> > >  	    intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI))
> > >  		icl_dsi_frame_update(new_crtc_state);
> > >  
> > > +	/* Send VRR Push to terminate Vblank */
> > > +	if (new_crtc_state->vrr.enable)
> > > +		intel_vrr_send_push(new_crtc_state);
> > > +
> > 
> > Maybe move the vrr.enable check to the function?
> 
> Yes makes sense will do
> 
> > 
> > >  	/* We're still in the vblank-evade critical section, this can't race.
> > >  	 * Would be slightly nice to just grab the vblank count and arm the
> > >  	 * event outside of the critical section - the spinlock might spin for a
> > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > index 7f1353bac583..ec1ce88e869c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > @@ -102,3 +102,20 @@ void intel_vrr_enable(struct intel_encoder *encoder,
> > >  		trans_push);
> > >  }
> > >  
> > > +void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
> > > +{
> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > +	enum pipe pipe = crtc->pipe;
> > > +	u32 trans_push;
> > > +
> > > +	trans_push = intel_de_read(dev_priv, TRANS_PUSH(pipe));
> > > +	WARN_ON(!(trans_push & TRANS_PUSH_EN));
> > 
> > drm_WARN_ON, and perhaps move this below the register rmw. It doesn't
> > change the flow anyway.
> 
> Yes will do
> 
> Manasi
> 
> 
> > 
> > > +
> > > +	trans_push |= TRANS_PUSH_SEND;
> > > +	intel_de_write(dev_priv, TRANS_PUSH(pipe), trans_push);
> > > +
> > > +	drm_dbg(&dev_priv->drm, "Sending VRR Push on Pipe (%c)\n",
> > > +		pipe_name(pipe));
> > 
> > drm_dbg_kms
> > 
> > > +}
> > > +
> > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> > > index 05d982d6fbae..a6b78e1676cb 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > > @@ -19,5 +19,6 @@ void intel_vrr_compute_config(struct intel_dp *intel_dp,
> > >  			      struct intel_crtc_state *crtc_state);
> > >  void intel_vrr_enable(struct intel_encoder *encoder,
> > >  		      const struct intel_crtc_state *crtc_state);
> > > +void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
> > >  
> > >  #endif /* __INTEL_VRR_H__ */
> > 
> > -- 
> > Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx