intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Enable eDP PSR functionality at HSW - v3
@ 2013-02-25 22:55 Rodrigo Vivi
  2013-02-25 22:55 ` [PATCH 1/8] drm/i915: Organize VBT stuff inside drm_i915_private Rodrigo Vivi
                   ` (8 more replies)
  0 siblings, 9 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 22:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

PSR is an eDP feature that allows power saving even with static image at eDP screen.

v3: Accepted many suggestions that I received at v2 review, fixing, cleaning and improving the code.

v2: Main differences in this v2:
- Created vbt struct to get i915 dev_priv more organized and to avoid adding more stuff into it.
- migrated hsw macros to use transcoder instead of pipes than I could address eDP
- remove patch that was only adding edp psr registers and added them on demand

v1:
Shobit Kumar has implemented this patch series some time ago, but had no eDP panel with PSR capability to test them.

I could test and verify that this series fully identify PSR capability and enables it at HSW.
I also verified that it saves from 0.5-1W but only when in blank screen. It seems it is not really entering in sleeping mode with static image at eDP screen yet.

Please accept this series as the first part of PSR enabling while we continue working to improve its functionality.

Rodrigo Vivi (5):
  drm/i915: Organize VBT stuff inside drm_i915_private
  drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
  drm/i915: Enable/Disable PSR
  drm/i915: Added debugfs support for PSR Status
  drm/i915: Hook PSR functionality

Shobhit Kumar (3):
  drm/i915: Added SDP and VSC structures for handling PSR for eDP
  drm/i915: Read the EDP DPCD and PSR Capability
  drm/i915: VBT Parsing for the PSR Feature Block for HSW

 drivers/gpu/drm/i915/i915_debugfs.c  |  92 ++++++++++++++++
 drivers/gpu/drm/i915/i915_dma.c      |   8 +-
 drivers/gpu/drm/i915/i915_drv.h      |  63 ++++++-----
 drivers/gpu/drm/i915/i915_reg.h      |  82 ++++++++++++--
 drivers/gpu/drm/i915/intel_bios.c    | 126 +++++++++++++--------
 drivers/gpu/drm/i915/intel_bios.h    |  20 +++-
 drivers/gpu/drm/i915/intel_crt.c     |   4 +-
 drivers/gpu/drm/i915/intel_ddi.c     |   2 +
 drivers/gpu/drm/i915/intel_display.c |  16 +--
 drivers/gpu/drm/i915/intel_dp.c      | 208 ++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   4 +
 drivers/gpu/drm/i915/intel_hdmi.c    |  13 ++-
 drivers/gpu/drm/i915/intel_lvds.c    |  20 ++--
 drivers/gpu/drm/i915/intel_sdvo.c    |   6 +-
 drivers/gpu/drm/i915/intel_tv.c      |   8 +-
 include/drm/drm_dp_helper.h          |  22 ++++
 16 files changed, 569 insertions(+), 125 deletions(-)

-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/8] drm/i915: Organize VBT stuff inside drm_i915_private
  2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
@ 2013-02-25 22:55 ` Rodrigo Vivi
  2013-02-25 22:55 ` [PATCH 2/8] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe Rodrigo Vivi
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 22:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

drm_i915_private is getting bigger and bigger when adding new vbt stuff.
So, the better way of getting drm_i915_private organized is to create
a special structure for vbt stuff.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |  8 +--
 drivers/gpu/drm/i915/i915_drv.h      | 56 +++++++++++----------
 drivers/gpu/drm/i915/intel_bios.c    | 96 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_crt.c     |  4 +-
 drivers/gpu/drm/i915/intel_display.c | 16 +++---
 drivers/gpu/drm/i915/intel_dp.c      | 12 ++---
 drivers/gpu/drm/i915/intel_lvds.c    | 20 ++++----
 drivers/gpu/drm/i915/intel_sdvo.c    |  6 +--
 drivers/gpu/drm/i915/intel_tv.c      |  8 +--
 9 files changed, 116 insertions(+), 110 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4fa6beb..ee09107 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1715,10 +1715,10 @@ int i915_driver_unload(struct drm_device *dev)
 		 * free the memory space allocated for the child device
 		 * config parsed from VBT
 		 */
-		if (dev_priv->child_dev && dev_priv->child_dev_num) {
-			kfree(dev_priv->child_dev);
-			dev_priv->child_dev = NULL;
-			dev_priv->child_dev_num = 0;
+		if (dev_priv->vbt.child_dev && dev_priv->vbt.child_dev_num) {
+			kfree(dev_priv->vbt.child_dev);
+			dev_priv->vbt.child_dev = NULL;
+			dev_priv->vbt.child_dev_num = 0;
 		}
 
 		vga_switcheroo_unregister_client(dev->pdev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e95337c..7adb6d0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -853,6 +853,36 @@ enum modeset_restore {
 	MODESET_SUSPENDED,
 };
 
+struct intel_vbt_data {
+	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
+	struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
+
+	/* Feature bits */
+	unsigned int int_tv_support:1;
+	unsigned int lvds_dither:1;
+	unsigned int lvds_vbt:1;
+	unsigned int int_crt_support:1;
+	unsigned int lvds_use_ssc:1;
+	unsigned int display_clock_mode:1;
+	int lvds_ssc_freq;
+	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+
+	/* eDP */
+	int edp_rate;
+	int edp_lanes;
+	int edp_preemphasis;
+	int edp_vswing;
+	bool edp_initialized;
+	bool edp_support;
+	int edp_bpp;
+	struct edp_power_seq edp_pps;
+
+	int crt_ddc_pin;
+
+	int child_dev_num;
+	struct child_device_config *child_dev;
+};
+
 typedef struct drm_i915_private {
 	struct drm_device *dev;
 	struct kmem_cache *slab;
@@ -923,6 +953,7 @@ typedef struct drm_i915_private {
 	struct intel_fbc_work *fbc_work;
 
 	struct intel_opregion opregion;
+	struct intel_vbt_data vbt;
 
 	/* overlay */
 	struct intel_overlay *overlay;
@@ -931,32 +962,9 @@ typedef struct drm_i915_private {
 	/* LVDS info */
 	int backlight_level;  /* restore backlight to this value */
 	bool backlight_enabled;
-	struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
-	struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
 
-	/* Feature bits from the VBIOS */
-	unsigned int int_tv_support:1;
-	unsigned int lvds_dither:1;
-	unsigned int lvds_vbt:1;
-	unsigned int int_crt_support:1;
-	unsigned int lvds_use_ssc:1;
-	unsigned int display_clock_mode:1;
-	int lvds_ssc_freq;
-	unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
-	struct {
-		int rate;
-		int lanes;
-		int preemphasis;
-		int vswing;
-
-		bool initialized;
-		bool support;
-		int bpp;
-		struct edp_power_seq pps;
-	} edp;
 	bool no_aux_handshake;
 
-	int crt_ddc_pin;
 	struct drm_i915_fence_reg fence_regs[I915_MAX_NUM_FENCES]; /* assume 965 */
 	int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */
 	int num_fence_regs; /* 8 on pre-965, 16 otherwise */
@@ -1002,8 +1010,6 @@ typedef struct drm_i915_private {
 	/* indicates the reduced downclock for LVDS*/
 	int lvds_downclock;
 	u16 orig_clock;
-	int child_dev_num;
-	struct child_device_config *child_dev;
 
 	bool mchbar_need_disable;
 
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 55ffba1..e145151 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -212,7 +212,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 	if (!lvds_options)
 		return;
 
-	dev_priv->lvds_dither = lvds_options->pixel_dither;
+	dev_priv->vbt.lvds_dither = lvds_options->pixel_dither;
 	if (lvds_options->panel_type == 0xff)
 		return;
 
@@ -226,7 +226,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 	if (!lvds_lfp_data_ptrs)
 		return;
 
-	dev_priv->lvds_vbt = 1;
+	dev_priv->vbt.lvds_vbt = 1;
 
 	panel_dvo_timing = get_lvds_dvo_timing(lvds_lfp_data,
 					       lvds_lfp_data_ptrs,
@@ -238,7 +238,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 
 	fill_detail_timing_data(panel_fixed_mode, panel_dvo_timing);
 
-	dev_priv->lfp_lvds_vbt_mode = panel_fixed_mode;
+	dev_priv->vbt.lfp_lvds_vbt_mode = panel_fixed_mode;
 
 	DRM_DEBUG_KMS("Found panel mode in BIOS VBT tables:\n");
 	drm_mode_debug_printmodeline(panel_fixed_mode);
@@ -274,9 +274,9 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
 		/* check the resolution, just to be sure */
 		if (fp_timing->x_res == panel_fixed_mode->hdisplay &&
 		    fp_timing->y_res == panel_fixed_mode->vdisplay) {
-			dev_priv->bios_lvds_val = fp_timing->lvds_reg_val;
+			dev_priv->vbt.bios_lvds_val = fp_timing->lvds_reg_val;
 			DRM_DEBUG_KMS("VBT initial LVDS value %x\n",
-				      dev_priv->bios_lvds_val);
+				      dev_priv->vbt.bios_lvds_val);
 		}
 	}
 }
@@ -316,7 +316,7 @@ parse_sdvo_panel_data(struct drm_i915_private *dev_priv,
 
 	fill_detail_timing_data(panel_fixed_mode, dvo_timing + index);
 
-	dev_priv->sdvo_lvds_vbt_mode = panel_fixed_mode;
+	dev_priv->vbt.sdvo_lvds_vbt_mode = panel_fixed_mode;
 
 	DRM_DEBUG_KMS("Found SDVO panel mode in BIOS VBT tables:\n");
 	drm_mode_debug_printmodeline(panel_fixed_mode);
@@ -345,18 +345,18 @@ parse_general_features(struct drm_i915_private *dev_priv,
 
 	general = find_section(bdb, BDB_GENERAL_FEATURES);
 	if (general) {
-		dev_priv->int_tv_support = general->int_tv_support;
-		dev_priv->int_crt_support = general->int_crt_support;
-		dev_priv->lvds_use_ssc = general->enable_ssc;
-		dev_priv->lvds_ssc_freq =
+		dev_priv->vbt.int_tv_support = general->int_tv_support;
+		dev_priv->vbt.int_crt_support = general->int_crt_support;
+		dev_priv->vbt.lvds_use_ssc = general->enable_ssc;
+		dev_priv->vbt.lvds_ssc_freq =
 			intel_bios_ssc_frequency(dev, general->ssc_freq);
-		dev_priv->display_clock_mode = general->display_clock_mode;
+		dev_priv->vbt.display_clock_mode = general->display_clock_mode;
 		DRM_DEBUG_KMS("BDB_GENERAL_FEATURES int_tv_support %d int_crt_support %d lvds_use_ssc %d lvds_ssc_freq %d display_clock_mode %d\n",
-			      dev_priv->int_tv_support,
-			      dev_priv->int_crt_support,
-			      dev_priv->lvds_use_ssc,
-			      dev_priv->lvds_ssc_freq,
-			      dev_priv->display_clock_mode);
+			      dev_priv->vbt.int_tv_support,
+			      dev_priv->vbt.int_crt_support,
+			      dev_priv->vbt.lvds_use_ssc,
+			      dev_priv->vbt.lvds_ssc_freq,
+			      dev_priv->vbt.display_clock_mode);
 	}
 }
 
@@ -373,7 +373,7 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
 			int bus_pin = general->crt_ddc_gmbus_pin;
 			DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin);
 			if (intel_gmbus_is_port_valid(bus_pin))
-				dev_priv->crt_ddc_pin = bus_pin;
+				dev_priv->vbt.crt_ddc_pin = bus_pin;
 		} else {
 			DRM_DEBUG_KMS("BDB_GD too small (%d). Invalid.\n",
 				      block_size);
@@ -484,7 +484,7 @@ parse_driver_features(struct drm_i915_private *dev_priv,
 
 	if (SUPPORTS_EDP(dev) &&
 	    driver->lvds_config == BDB_DRIVER_FEATURE_EDP)
-		dev_priv->edp.support = 1;
+		dev_priv->vbt.edp_support = 1;
 
 	if (driver->dual_frequency)
 		dev_priv->render_reclock_avail = true;
@@ -499,20 +499,20 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 
 	edp = find_section(bdb, BDB_EDP);
 	if (!edp) {
-		if (SUPPORTS_EDP(dev_priv->dev) && dev_priv->edp.support)
+		if (SUPPORTS_EDP(dev_priv->dev) && dev_priv->vbt.edp_support)
 			DRM_DEBUG_KMS("No eDP BDB found but eDP panel supported.\n");
 		return;
 	}
 
 	switch ((edp->color_depth >> (panel_type * 2)) & 3) {
 	case EDP_18BPP:
-		dev_priv->edp.bpp = 18;
+		dev_priv->vbt.edp_bpp = 18;
 		break;
 	case EDP_24BPP:
-		dev_priv->edp.bpp = 24;
+		dev_priv->vbt.edp_bpp = 24;
 		break;
 	case EDP_30BPP:
-		dev_priv->edp.bpp = 30;
+		dev_priv->vbt.edp_bpp = 30;
 		break;
 	}
 
@@ -520,48 +520,48 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
 	edp_pps = &edp->power_seqs[panel_type];
 	edp_link_params = &edp->link_params[panel_type];
 
-	dev_priv->edp.pps = *edp_pps;
+	dev_priv->vbt.edp_pps = *edp_pps;
 
-	dev_priv->edp.rate = edp_link_params->rate ? DP_LINK_BW_2_7 :
+	dev_priv->vbt.edp_rate = edp_link_params->rate ? DP_LINK_BW_2_7 :
 		DP_LINK_BW_1_62;
 	switch (edp_link_params->lanes) {
 	case 0:
-		dev_priv->edp.lanes = 1;
+		dev_priv->vbt.edp_lanes = 1;
 		break;
 	case 1:
-		dev_priv->edp.lanes = 2;
+		dev_priv->vbt.edp_lanes = 2;
 		break;
 	case 3:
 	default:
-		dev_priv->edp.lanes = 4;
+		dev_priv->vbt.edp_lanes = 4;
 		break;
 	}
 	switch (edp_link_params->preemphasis) {
 	case 0:
-		dev_priv->edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_0;
+		dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_0;
 		break;
 	case 1:
-		dev_priv->edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_3_5;
+		dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_3_5;
 		break;
 	case 2:
-		dev_priv->edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_6;
+		dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_6;
 		break;
 	case 3:
-		dev_priv->edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_9_5;
+		dev_priv->vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_9_5;
 		break;
 	}
 	switch (edp_link_params->vswing) {
 	case 0:
-		dev_priv->edp.vswing = DP_TRAIN_VOLTAGE_SWING_400;
+		dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_400;
 		break;
 	case 1:
-		dev_priv->edp.vswing = DP_TRAIN_VOLTAGE_SWING_600;
+		dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_600;
 		break;
 	case 2:
-		dev_priv->edp.vswing = DP_TRAIN_VOLTAGE_SWING_800;
+		dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_800;
 		break;
 	case 3:
-		dev_priv->edp.vswing = DP_TRAIN_VOLTAGE_SWING_1200;
+		dev_priv->vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_1200;
 		break;
 	}
 }
@@ -609,13 +609,13 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 		DRM_DEBUG_KMS("no child dev is parsed from VBT\n");
 		return;
 	}
-	dev_priv->child_dev = kcalloc(count, sizeof(*p_child), GFP_KERNEL);
-	if (!dev_priv->child_dev) {
+	dev_priv->vbt.child_dev = kcalloc(count, sizeof(*p_child), GFP_KERNEL);
+	if (!dev_priv->vbt.child_dev) {
 		DRM_DEBUG_KMS("No memory space for child device\n");
 		return;
 	}
 
-	dev_priv->child_dev_num = count;
+	dev_priv->vbt.child_dev_num = count;
 	count = 0;
 	for (i = 0; i < child_device_num; i++) {
 		p_child = &(p_defs->devices[i]);
@@ -623,7 +623,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 			/* skip the device block if device type is invalid */
 			continue;
 		}
-		child_dev_ptr = dev_priv->child_dev + count;
+		child_dev_ptr = dev_priv->vbt.child_dev + count;
 		count++;
 		memcpy((void *)child_dev_ptr, (void *)p_child,
 					sizeof(*p_child));
@@ -636,23 +636,23 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
 
-	dev_priv->crt_ddc_pin = GMBUS_PORT_VGADDC;
+	dev_priv->vbt.crt_ddc_pin = GMBUS_PORT_VGADDC;
 
 	/* LFP panel data */
-	dev_priv->lvds_dither = 1;
-	dev_priv->lvds_vbt = 0;
+	dev_priv->vbt.lvds_dither = 1;
+	dev_priv->vbt.lvds_vbt = 0;
 
 	/* SDVO panel data */
-	dev_priv->sdvo_lvds_vbt_mode = NULL;
+	dev_priv->vbt.sdvo_lvds_vbt_mode = NULL;
 
 	/* general features */
-	dev_priv->int_tv_support = 1;
-	dev_priv->int_crt_support = 1;
+	dev_priv->vbt.int_tv_support = 1;
+	dev_priv->vbt.int_crt_support = 1;
 
 	/* Default to using SSC */
-	dev_priv->lvds_use_ssc = 1;
-	dev_priv->lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1);
-	DRM_DEBUG_KMS("Set default to SSC at %dMHz\n", dev_priv->lvds_ssc_freq);
+	dev_priv->vbt.lvds_use_ssc = 1;
+	dev_priv->vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1);
+	DRM_DEBUG_KMS("Set default to SSC at %dMHz\n", dev_priv->vbt.lvds_ssc_freq);
 }
 
 static int __init intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index cfc9687..2a07e34 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -434,7 +434,7 @@ static bool intel_crt_detect_ddc(struct drm_connector *connector)
 
 	BUG_ON(crt->base.type != INTEL_OUTPUT_ANALOG);
 
-	i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin);
+	i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin);
 	edid = intel_crt_get_edid(connector, i2c);
 
 	if (edid) {
@@ -640,7 +640,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
 	int ret;
 	struct i2c_adapter *i2c;
 
-	i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin);
+	i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin);
 	ret = intel_crt_ddc_get_modes(connector, i2c);
 	if (ret || !IS_G4X(dev))
 		return ret;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6eb3882..d7f81ed 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4001,7 +4001,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 {
 	if (i915_panel_use_ssc >= 0)
 		return i915_panel_use_ssc != 0;
-	return dev_priv->lvds_use_ssc
+	return dev_priv->vbt.lvds_use_ssc
 		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }
 
@@ -4072,7 +4072,7 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc,
 
 		if (intel_encoder->type == INTEL_OUTPUT_EDP) {
 			/* Use VBT settings if we have an eDP panel */
-			unsigned int edp_bpc = dev_priv->edp.bpp / 3;
+			unsigned int edp_bpc = dev_priv->vbt.edp_bpp / 3;
 
 			if (edp_bpc && edp_bpc < display_bpc) {
 				DRM_DEBUG_KMS("clamping display bpc (was %d) to eDP (%d)\n", display_bpc, edp_bpc);
@@ -4173,7 +4173,7 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
 		refclk = vlv_get_refclk(crtc);
 	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
 	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
-		refclk = dev_priv->lvds_ssc_freq * 1000;
+		refclk = dev_priv->vbt.lvds_ssc_freq * 1000;
 		DRM_DEBUG_KMS("using SSC reference clock of %d MHz\n",
 			      refclk / 1000);
 	} else if (!IS_GEN2(dev)) {
@@ -4785,7 +4785,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
 	}
 
 	if (HAS_PCH_IBX(dev)) {
-		has_ck505 = dev_priv->display_clock_mode;
+		has_ck505 = dev_priv->vbt.display_clock_mode;
 		can_ssc = has_ck505;
 	} else {
 		has_ck505 = false;
@@ -5070,8 +5070,8 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
 
 	if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
 		DRM_DEBUG_KMS("using SSC reference clock of %d MHz\n",
-			      dev_priv->lvds_ssc_freq);
-		return dev_priv->lvds_ssc_freq * 1000;
+			      dev_priv->vbt.lvds_ssc_freq);
+		return dev_priv->vbt.lvds_ssc_freq * 1000;
 	}
 
 	return 120000;
@@ -5485,7 +5485,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	factor = 21;
 	if (is_lvds) {
 		if ((intel_panel_use_ssc(dev_priv) &&
-		     dev_priv->lvds_ssc_freq == 100) ||
+		     dev_priv->vbt.lvds_ssc_freq == 100) ||
 		    intel_is_dual_link_lvds(dev))
 			factor = 25;
 	} else if (is_sdvo && is_tv)
@@ -5598,7 +5598,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	/* determine panel color depth */
 	dither = intel_choose_pipe_bpp_dither(crtc, fb, &intel_crtc->bpp,
 					      adjusted_mode);
-	if (is_lvds && dev_priv->lvds_dither)
+	if (is_lvds && dev_priv->vbt.lvds_dither)
 		dither = true;
 
 	fp = clock.n << 16 | clock.m1 << 8 | clock.m2;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7b8bfe8..b4494a2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2616,11 +2616,11 @@ bool intel_dpd_is_edp(struct drm_device *dev)
 	struct child_device_config *p_child;
 	int i;
 
-	if (!dev_priv->child_dev_num)
+	if (!dev_priv->vbt.child_dev_num)
 		return false;
 
-	for (i = 0; i < dev_priv->child_dev_num; i++) {
-		p_child = dev_priv->child_dev + i;
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+		p_child = dev_priv->vbt.child_dev + i;
 
 		if (p_child->dvo_port == PORT_IDPD &&
 		    p_child->device_type == DEVICE_TYPE_eDP)
@@ -2685,7 +2685,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 	DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
 		      cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
 
-	vbt = dev_priv->edp.pps;
+	vbt = dev_priv->vbt.edp_pps;
 
 	/* Upper limits from eDP 1.3 spec. Note that we use the clunky units of
 	 * our hw here, which are all in 100usec. */
@@ -2910,8 +2910,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		}
 
 		/* fallback to VBT if available for eDP */
-		if (!fixed_mode && dev_priv->lfp_lvds_vbt_mode) {
-			fixed_mode = drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
+		if (!fixed_mode && dev_priv->vbt.lfp_lvds_vbt_mode) {
+			fixed_mode = drm_mode_duplicate(dev, dev_priv->vbt.lfp_lvds_vbt_mode);
 			if (fixed_mode)
 				fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 		}
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index c7154bf..940bead 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -136,7 +136,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
 	 * special lvds dither control bit on pch-split platforms, dithering is
 	 * only controlled through the PIPECONF reg. */
 	if (INTEL_INFO(dev)->gen == 4) {
-		if (dev_priv->lvds_dither)
+		if (dev_priv->vbt.lvds_dither)
 			temp |= LVDS_ENABLE_DITHER;
 		else
 			temp &= ~LVDS_ENABLE_DITHER;
@@ -455,7 +455,7 @@ out:
 	}
 
 	/* Make sure pre-965 set dither correctly */
-	if (INTEL_INFO(dev)->gen < 4 && dev_priv->lvds_dither)
+	if (INTEL_INFO(dev)->gen < 4 && dev_priv->vbt.lvds_dither)
 		pfit_control |= PANEL_8TO6_DITHER_ENABLE;
 
 	if (pfit_control != lvds_encoder->pfit_control ||
@@ -919,11 +919,11 @@ static bool lvds_is_present_in_vbt(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int i;
 
-	if (!dev_priv->child_dev_num)
+	if (!dev_priv->vbt.child_dev_num)
 		return true;
 
-	for (i = 0; i < dev_priv->child_dev_num; i++) {
-		struct child_device_config *child = dev_priv->child_dev + i;
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+		struct child_device_config *child = dev_priv->vbt.child_dev + i;
 
 		/* If the device type is not LFP, continue.
 		 * We have to check both the new identifiers as well as the
@@ -1011,7 +1011,7 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder)
 	 */
 	val = I915_READ(lvds_encoder->reg);
 	if (!(val & ~(LVDS_PIPE_MASK | LVDS_DETECTED)))
-		val = dev_priv->bios_lvds_val;
+		val = dev_priv->vbt.bios_lvds_val;
 
 	return (val & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP;
 }
@@ -1068,7 +1068,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	if (HAS_PCH_SPLIT(dev)) {
 		if ((I915_READ(PCH_LVDS) & LVDS_DETECTED) == 0)
 			return false;
-		if (dev_priv->edp.support) {
+		if (dev_priv->vbt.edp_support) {
 			DRM_DEBUG_KMS("disable LVDS for eDP support\n");
 			return false;
 		}
@@ -1190,11 +1190,11 @@ bool intel_lvds_init(struct drm_device *dev)
 	}
 
 	/* Failed to get EDID, what about VBT? */
-	if (dev_priv->lfp_lvds_vbt_mode) {
+	if (dev_priv->vbt.lfp_lvds_vbt_mode) {
 		DRM_DEBUG_KMS("using mode from VBT: ");
-		drm_mode_debug_printmodeline(dev_priv->lfp_lvds_vbt_mode);
+		drm_mode_debug_printmodeline(dev_priv->vbt.lfp_lvds_vbt_mode);
 
-		fixed_mode = drm_mode_duplicate(dev, dev_priv->lfp_lvds_vbt_mode);
+		fixed_mode = drm_mode_duplicate(dev, dev_priv->vbt.lfp_lvds_vbt_mode);
 		if (fixed_mode) {
 			fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
 			goto out;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f01063a..4d0be3d 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1487,7 +1487,7 @@ intel_sdvo_get_analog_edid(struct drm_connector *connector)
 
 	return drm_get_edid(connector,
 			    intel_gmbus_get_adapter(dev_priv,
-						    dev_priv->crt_ddc_pin));
+						    dev_priv->vbt.crt_ddc_pin));
 }
 
 static enum drm_connector_status
@@ -1773,9 +1773,9 @@ static void intel_sdvo_get_lvds_modes(struct drm_connector *connector)
 		goto end;
 
 	/* Fetch modes from VBT */
-	if (dev_priv->sdvo_lvds_vbt_mode != NULL) {
+	if (dev_priv->vbt.sdvo_lvds_vbt_mode != NULL) {
 		newmode = drm_mode_duplicate(connector->dev,
-					     dev_priv->sdvo_lvds_vbt_mode);
+					     dev_priv->vbt.sdvo_lvds_vbt_mode);
 		if (newmode != NULL) {
 			/* Guarantee the mode is preferred */
 			newmode->type = (DRM_MODE_TYPE_PREFERRED |
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 984a113..500f053 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1521,12 +1521,12 @@ static int tv_is_present_in_vbt(struct drm_device *dev)
 	struct child_device_config *p_child;
 	int i, ret;
 
-	if (!dev_priv->child_dev_num)
+	if (!dev_priv->vbt.child_dev_num)
 		return 1;
 
 	ret = 0;
-	for (i = 0; i < dev_priv->child_dev_num; i++) {
-		p_child = dev_priv->child_dev + i;
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+		p_child = dev_priv->vbt.child_dev + i;
 		/*
 		 * If the device type is not TV, continue.
 		 */
@@ -1564,7 +1564,7 @@ intel_tv_init(struct drm_device *dev)
 		return;
 	}
 	/* Even if we have an encoder we may not have a connector */
-	if (!dev_priv->int_tv_support)
+	if (!dev_priv->vbt.int_tv_support)
 		return;
 
 	/*
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/8] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
  2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
  2013-02-25 22:55 ` [PATCH 1/8] drm/i915: Organize VBT stuff inside drm_i915_private Rodrigo Vivi
@ 2013-02-25 22:55 ` Rodrigo Vivi
  2013-02-27 19:44   ` [Intel-gfx] " Paulo Zanoni
  2013-02-25 22:55 ` [PATCH 3/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP Rodrigo Vivi
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 22:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

While old platforms had 3 transcoders and 3 pipes (1:1), HSW has
4 transcoders and 3 pipes.
These regs were being used only by HDMI code where pipe is always the same
thing as cpu_transcoder.
This patch allow us to use them for DP, specially for TRANSCODER_EDP.

v2: Adding HSW_TVIDEO_DIP_VSC_DATA to transmit vsc to eDP.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h   | 18 ++++++++++--------
 drivers/gpu/drm/i915/intel_hdmi.c | 13 +++++++------
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 527b664..b715ecd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3754,14 +3754,16 @@
 #define HSW_VIDEO_DIP_VSC_ECC_B		0x61344
 #define HSW_VIDEO_DIP_GCP_B		0x61210
 
-#define HSW_TVIDEO_DIP_CTL(pipe) \
-	 _PIPE(pipe, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
-#define HSW_TVIDEO_DIP_AVI_DATA(pipe) \
-	 _PIPE(pipe, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
-#define HSW_TVIDEO_DIP_SPD_DATA(pipe) \
-	 _PIPE(pipe, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
-#define HSW_TVIDEO_DIP_GCP(pipe) \
-	_PIPE(pipe, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)
+#define HSW_TVIDEO_DIP_CTL(trans) \
+	 _TRANSCODER(trans, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
+#define HSW_TVIDEO_DIP_AVI_DATA(trans) \
+	 _TRANSCODER(trans, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
+#define HSW_TVIDEO_DIP_SPD_DATA(trans) \
+	 _TRANSCODER(trans, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
+#define HSW_TVIDEO_DIP_GCP(trans) \
+	_TRANSCODER(trans, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)
+#define HSW_TVIDEO_DIP_VSC_DATA(trans) \
+	 _TRANSCODER(trans, HSW_VIDEO_DIP_VSC_DATA_A, HSW_VIDEO_DIP_VSC_DATA_B)
 
 #define _TRANS_HTOTAL_B          0xe1000
 #define _TRANS_HBLANK_B          0xe1004
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 5a6138c..26290c1 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -120,13 +120,14 @@ static u32 hsw_infoframe_enable(struct dip_infoframe *frame)
 	}
 }
 
-static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame, enum pipe pipe)
+static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame,
+				  enum transcoder cpu_transcoder)
 {
 	switch (frame->type) {
 	case DIP_TYPE_AVI:
-		return HSW_TVIDEO_DIP_AVI_DATA(pipe);
+		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
 	case DIP_TYPE_SPD:
-		return HSW_TVIDEO_DIP_SPD_DATA(pipe);
+		return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
 	default:
 		DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
 		return 0;
@@ -293,8 +294,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
-	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
-	u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->pipe);
+	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
+	u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->cpu_transcoder);
 	unsigned int i, len = DIP_HEADER_SIZE + frame->len;
 	u32 val = I915_READ(ctl_reg);
 
@@ -568,7 +569,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder,
 	struct drm_i915_private *dev_priv = encoder->dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
-	u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
+	u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
 	u32 val = I915_READ(reg);
 
 	assert_hdmi_port_disabled(intel_hdmi);
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP
  2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
  2013-02-25 22:55 ` [PATCH 1/8] drm/i915: Organize VBT stuff inside drm_i915_private Rodrigo Vivi
  2013-02-25 22:55 ` [PATCH 2/8] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe Rodrigo Vivi
@ 2013-02-25 22:55 ` Rodrigo Vivi
  2013-02-27 21:52   ` [Intel-gfx] " Paulo Zanoni
  2013-03-03 17:28   ` Daniel Vetter
  2013-02-25 22:55 ` [PATCH 4/8] drm/i915: Read the EDP DPCD and PSR Capability Rodrigo Vivi
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 22:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sateesh Kavuri, dri-devel

From: Shobhit Kumar <shobhit.kumar@intel.com>

Signed-off-by: Sateesh Kavuri <sateesh.kavuri@intel.com>

v2: Modified and corrected the structures to be more in line for
kernel coding guidelines and rebased the code on Paulo's DP patchset

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE
v4: moving them to include/drm/drm_dp_helper.h and also already
    icluding EDP_PSR_RECEIVER_CAP_SIZE to add everything needed
    for PSR at once at drm_dp_helper.h

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 include/drm/drm_dp_helper.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index e8e1417..06c5060 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -343,12 +343,34 @@ u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
 					  int lane);
 
 #define DP_RECEIVER_CAP_SIZE	0xf
+#define EDP_PSR_RECEIVER_CAP_SIZE      2
+
 void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
 void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
 
 u8 drm_dp_link_rate_to_bw_code(int link_rate);
 int drm_dp_bw_code_to_link_rate(u8 link_bw);
 
+/* SDP header as per eDP 1.3 spec, section 3.6 */
+struct edp_sdp_header {
+	u8 id;
+	u8 type;
+	u8 revision;
+	u8 valid_payload_bytes;
+} __attribute__((packed));
+
+/* SDP VSC header as per eDP 1.3 spec, section 3.6 */
+struct edp_vsc_psr {
+	struct edp_sdp_header sdp_header;
+	u8 DB1; /* 0 - PSR State; 1 - Update RFB; 2 - CRC Valid*/
+	u8 DB2; /* CRC value bits 7:0 of the R or Cr component */
+	u8 DB3; /* CRC value bits 15:8 of the R or Cr component */
+	u8 DB4; /* CRC value bits 7:0 of the G or Y component */
+	u8 DB5; /* CRC value bits 15:8 of the G or Y component */
+	u8 DB6; /* CRC value bits 7:0 of the B or Cb component */
+	u8 DB7; /* CRC value bits 15:8 of the B or Cb component */
+} __attribute__((packed));
+
 static inline int
 drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
 {
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/8] drm/i915: Read the EDP DPCD and PSR Capability
  2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2013-02-25 22:55 ` [PATCH 3/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP Rodrigo Vivi
@ 2013-02-25 22:55 ` Rodrigo Vivi
  2013-02-26 15:02   ` Ville Syrjälä
  2013-02-25 22:55 ` [PATCH 5/8] drm/i915: VBT Parsing for the PSR Feature Block for HSW Rodrigo Vivi
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 22:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shobhit Kumar, dri-devel

From: Shobhit Kumar <shobhit.kumar@intel.com>

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

v2: reuse of just created is_edp_psr and put it at right place.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b4494a2..5cfa9f4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1437,6 +1437,12 @@ static void intel_post_disable_dp(struct intel_encoder *encoder)
 	}
 }
 
+static bool is_edp_psr(struct intel_dp *intel_dp)
+{
+	return (is_edp(intel_dp) &&
+		(intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED));
+}
+
 static void intel_enable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -2117,6 +2123,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
 		return false; /* DPCD not present */
 
+	/* Check if the panel supports PSR */
+	memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
+	intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
+				       intel_dp->psr_dpcd,
+				       sizeof(intel_dp->psr_dpcd));
+	if (is_edp_psr(intel_dp))
+		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
 	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
 	      DP_DWN_STRM_PORT_PRESENT))
 		return true; /* native DP sink */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 005a91f..865ede6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -375,6 +375,7 @@ struct intel_dp {
 	uint8_t link_bw;
 	uint8_t lane_count;
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
+	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	struct i2c_adapter adapter;
 	struct i2c_algo_dp_aux_data algo;
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 5/8] drm/i915: VBT Parsing for the PSR Feature Block for HSW
  2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
                   ` (3 preceding siblings ...)
  2013-02-25 22:55 ` [PATCH 4/8] drm/i915: Read the EDP DPCD and PSR Capability Rodrigo Vivi
@ 2013-02-25 22:55 ` Rodrigo Vivi
  2013-02-25 22:55 ` [PATCH 6/8] drm/i915: Enable/Disable PSR Rodrigo Vivi
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 22:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shobhit Kumar, dri-devel

From: Shobhit Kumar <shobhit.kumar@intel.com>

Parse and store useful information in i915_dev_private

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

v2: Add to new vbt struct and call them psr_*

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.h   |  7 +++++++
 drivers/gpu/drm/i915/intel_bios.c | 30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_bios.h | 20 +++++++++++++++++++-
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7adb6d0..1b52dca 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -877,6 +877,13 @@ struct intel_vbt_data {
 	int edp_bpp;
 	struct edp_power_seq edp_pps;
 
+	/* eDP PSR*/
+	u8 psr_full_link_state;
+	u8 psr_wait_lines;
+	u8 psr_idle_frames;
+	u16 psr_wakeup_tp1;
+	u16 psr_wakeup_tp2_tp3;
+
 	int crt_ddc_pin;
 
 	int child_dev_num;
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index e145151..2a3fa8b 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -381,6 +381,35 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
 	}
 }
 
+
+static void
+parse_edp_psr(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
+{
+	struct bdb_psr_features *psr;
+	struct bdb_lvds_options *lvds_opts;
+	int index = 0;
+	lvds_opts = find_section(bdb, BDB_LVDS_OPTIONS);
+	if (!lvds_opts) {
+		DRM_DEBUG_KMS("No LVDS Options block found.\n");
+		return;
+	}
+
+	index = lvds_opts->panel_type;
+
+	psr = find_section(bdb, BDB_PSR_FEATURES);
+	if (!psr) {
+		DRM_DEBUG_KMS("No PSR feature block found.\n");
+		return;
+	}
+
+	dev_priv->vbt.psr_full_link_state = psr[index].link_disable;
+	dev_priv->vbt.psr_wait_lines = psr[index].wait_lines;
+	dev_priv->vbt.psr_idle_frames = psr[index].idle_frames;
+	dev_priv->vbt.psr_wakeup_tp1 = psr[index].wakeup_tp1;
+	dev_priv->vbt.psr_wakeup_tp2_tp3 = psr[index].wakeup_tp2;
+}
+
+
 static void
 parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
 			  struct bdb_header *bdb)
@@ -740,6 +769,7 @@ intel_parse_bios(struct drm_device *dev)
 	parse_device_mapping(dev_priv, bdb);
 	parse_driver_features(dev_priv, bdb);
 	parse_edp(dev_priv, bdb);
+	parse_edp_psr(dev_priv, bdb);
 
 	if (bios)
 		pci_unmap_rom(pdev, bios);
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 36e57f9..c1d39de 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -80,7 +80,7 @@ struct vbios_data {
 #define BDB_EXT_MMIO_REGS	  6
 #define BDB_SWF_IO		  7
 #define BDB_SWF_MMIO		  8
-#define BDB_DOT_CLOCK_TABLE	  9
+#define BDB_PSR_FEATURES	  9
 #define BDB_MODE_REMOVAL_TABLE	 10
 #define BDB_CHILD_DEVICE_TABLE	 11
 #define BDB_DRIVER_FEATURES	 12
@@ -263,6 +263,24 @@ struct bdb_lvds_options {
 	u8 rsvd4;
 } __attribute__((packed));
 
+struct bdb_psr_features {
+	/* psr_enable byte */
+	u8 link_disable:1;
+	u8 require_aux:1;
+	u8 rsvd1:6;
+
+	/* panel wait times */
+	u8 idle_frames:4;
+	u8 wait_lines:3;
+	u8 rsvd2:1;
+
+	/* TP1 wakeup time */
+	u16 wakeup_tp1;
+
+	/* TP2 wakeup time */
+	u16 wakeup_tp2;
+} __attribute__((packed));
+
 /* LFP pointer table contains entries to the struct below */
 struct bdb_lvds_lfp_data_ptr {
 	u16 fp_timing_offset; /* offsets are from start of bdb */
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 6/8] drm/i915: Enable/Disable PSR
  2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
                   ` (4 preceding siblings ...)
  2013-02-25 22:55 ` [PATCH 5/8] drm/i915: VBT Parsing for the PSR Feature Block for HSW Rodrigo Vivi
@ 2013-02-25 22:55 ` Rodrigo Vivi
  2013-02-26 12:48   ` Ville Syrjälä
                     ` (3 more replies)
  2013-02-25 22:55 ` [PATCH 7/8] drm/i915: Added debugfs support for PSR Status Rodrigo Vivi
                   ` (2 subsequent siblings)
  8 siblings, 4 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 22:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Adding Enable and Disable PSR functionalities. This includes setting the
PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
enabling PSR in the sink via DPCD register and finally enabling PSR on
the host.

This patch is heavily based on initial PSR code by Sateesh Kavuri and
Kumar Shobhit but in a different implementation.

Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  40 +++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 183 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |   3 +
 3 files changed, 226 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b715ecd..1e31f23 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -844,6 +844,8 @@
 #define   SNB_CPU_FENCE_ENABLE	(1<<29)
 #define DPFC_CPU_FENCE_OFFSET	0x100104
 
+/* Framebuffer compression for Haswell */
+#define HSW_FBC_CONTROL		0x43208
 
 /*
  * GPIO regs
@@ -1580,6 +1582,44 @@
 #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
 #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
 
+/* HSW eDP PSR registers */
+#define EDP_PSR_CTL				0x64800
+#define   EDP_PSR_ENABLE			(1<<31)
+#define   EDP_PSR_LINK_DISABLE			(0<<27)
+#define   EDP_PSR_LINK_STANDBY			(1<<27)
+#define   EDP_PSR_MIN_LINK_ENTRY_TIME_MASK	(3<<25)
+#define   EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES	(0<<25)
+#define   EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES	(1<<25)
+#define   EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES	(2<<25)
+#define   EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES	(3<<25)
+#define   EDP_PSR_MAX_SLEEP_TIME_SHIFT		20
+#define   EDP_PSR_SKIP_AUX_EXIT			(1<<12)
+#define   EDP_PSR_TP1_TP2_SEL			(0<<11)
+#define   EDP_PSR_TP1_TP3_SEL			(1<<11)
+#define   EDP_PSR_TP2_TP3_TIME_500us		(0<<8)
+#define   EDP_PSR_TP2_TP3_TIME_100us		(1<<8)
+#define   EDP_PSR_TP2_TP3_TIME_2500us		(2<<8)
+#define   EDP_PSR_TP2_TP3_TIME_0us		(3<<8)
+#define   EDP_PSR_TP1_TIME_500us		(0<<4)
+#define   EDP_PSR_TP1_TIME_100us		(1<<4)
+#define   EDP_PSR_TP1_TIME_2500us		(2<<4)
+#define   EDP_PSR_TP1_TIME_0us			(3<<4)
+#define   EDP_PSR_IDLE_FRAME_SHIFT		0
+
+#define EDP_PSR_AUX_CTL			0x64810
+#define EDP_PSR_AUX_DATA1		0x64814
+#define EDP_PSR_AUX_DATA2		0x64818
+#define EDP_PSR_AUX_DATA3		0x6481c
+#define EDP_PSR_AUX_DATA4		0x64820
+#define EDP_PSR_AUX_DATA5		0x64824
+
+#define EDP_PSR_STATUS_CTL			0x64840
+#define   EDP_PSR_STATUS_STATE_MASK		(7<<29)
+
+#define EDP_PSR_DEBUG_CTL		0x64860
+#define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
+#define   EDP_PSR_DEBUG_MASK_HPD	(1<<25)
+
 /* VGA port control */
 #define ADPA			0x61100
 #define PCH_ADPA                0xe1100
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5cfa9f4..a420f0d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -83,6 +83,13 @@ static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
 	return intel_dig_port->base.base.dev;
 }
 
+static struct drm_crtc *intel_dp_to_crtc(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+
+	return intel_dig_port->base.base.crtc;
+}
+
 static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
 {
 	return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
@@ -1443,6 +1450,182 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
 		(intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED));
 }
 
+static bool
+intel_edp_is_psr_enabled(struct intel_dp* intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	return I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
+}
+
+static void
+intel_edp_psr_enable_src(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t max_sleep_time = 0x1f;
+	uint32_t val = 0x0;
+
+	if (dev_priv->vbt.psr_idle_frames)
+	    val |= dev_priv->vbt.psr_idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
+	else
+	    val |= 1 << EDP_PSR_IDLE_FRAME_SHIFT;
+
+	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
+		val |= EDP_PSR_LINK_STANDBY;
+		val |= EDP_PSR_TP2_TP3_TIME_0us;
+		val |= EDP_PSR_TP1_TIME_0us;
+		val |= EDP_PSR_SKIP_AUX_EXIT;
+	} else {
+		val |= EDP_PSR_LINK_DISABLE;
+		/* Use these Values from VBT
+		 * Case values are timings for HSW as of now
+		 * in multiple of 100us
+		 */
+		switch(dev_priv->vbt.psr_wakeup_tp1) {
+			case 1:
+				val |= EDP_PSR_TP1_TIME_100us;
+				break;
+			case 5:
+				val |= EDP_PSR_TP1_TIME_500us;
+				break;
+			case 25:
+				val |= EDP_PSR_TP1_TIME_2500us;
+				break;
+			default:
+				val |= EDP_PSR_TP1_TIME_500us;
+				break;
+		};
+		switch(dev_priv->vbt.psr_wakeup_tp2_tp3) {
+			case 1:
+				val |= EDP_PSR_TP2_TP3_TIME_100us;
+				break;
+			case 5:
+				val |= EDP_PSR_TP2_TP3_TIME_500us;
+				break;
+			case 25:
+				val |= EDP_PSR_TP2_TP3_TIME_2500us;
+				break;
+			default:
+				val |= EDP_PSR_TP2_TP3_TIME_500us;
+				break;
+		};
+	}
+
+	I915_WRITE(EDP_PSR_CTL, val |
+		   EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES |
+		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
+		   EDP_PSR_ENABLE);
+}
+
+void intel_edp_write_vsc_psr(struct intel_dp* intel_dp,
+			     struct edp_vsc_psr *vsc_psr)
+{
+	struct drm_device *dev =  intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc =  to_intel_crtc(intel_dp_to_crtc(intel_dp));
+	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
+	u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(intel_crtc->cpu_transcoder);
+	uint32_t *data = (uint32_t *)vsc_psr;
+	unsigned int i;
+	u32 val = I915_READ(ctl_reg);
+
+	if (data_reg == 0)
+		return;
+
+	/* As per eDP spec, wait for vblank to send SDP VSC packet */
+	intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+	mmiowb();
+	for (i = 0; i < sizeof(struct edp_vsc_psr); i += 4) {
+		I915_WRITE(data_reg + i, *data);
+		data++;
+	}
+	/* Write every possible data byte to force correct ECC calculation. */
+	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
+		I915_WRITE(data_reg + i, 0);
+	mmiowb();
+
+	val |= VIDEO_DIP_ENABLE_VSC_HSW;
+	I915_WRITE(ctl_reg, val);
+	POSTING_READ(ctl_reg);
+}
+
+void intel_edp_enable_psr(struct intel_dp* intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct edp_vsc_psr psr_vsc;
+	int msg_size = 5;       /* Header(4) + Message(1) */
+
+	if (!is_edp_psr(intel_dp) || intel_edp_is_psr_enabled(intel_dp))
+		return;
+
+	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
+	memset(&psr_vsc, 0, sizeof(psr_vsc));
+	psr_vsc.sdp_header.id = 0;
+	psr_vsc.sdp_header.type = 0x7;
+	psr_vsc.sdp_header.revision = 0x2;
+	psr_vsc.sdp_header.valid_payload_bytes = 0x8;
+
+	intel_edp_write_vsc_psr(intel_dp, &psr_vsc);
+
+	/* Enable PSR in sink by setting bit 0 in DPCD config reg
+	 * along with the transmitter state during PSR active
+	 * Transmitter state later can be ready from VBT. As of now
+	 * program the full link down
+	 *
+	 */
+	intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
+				    DP_PSR_ENABLE);
+
+	/* Setup AUX registers */
+	/* Write command on DPCD 0x0600 */
+	I915_WRITE(EDP_PSR_AUX_DATA1, 0x80060000);
+
+	/* Set the state to normal operation D0 in DPCD 0x0600 */
+	I915_WRITE(EDP_PSR_AUX_DATA2, 0x01000000);
+
+	I915_WRITE(EDP_PSR_AUX_CTL,
+		   msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT |
+		   I915_READ(DPA_AUX_CH_CTL));
+
+	/* Enable PSR on the host */
+	intel_edp_psr_enable_src(intel_dp);
+
+	/* WA: FBC/PSR Underrun/Corruption Workaround */
+	I915_WRITE(HSW_FBC_CONTROL, ~FBC_CTL_EN);
+
+	/* WA: Continuous PSR exit Workaround */
+	I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP);
+	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
+}
+
+void intel_edp_disable_psr(struct intel_dp* intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
+	uint32_t reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
+	uint32_t val;
+
+	if (!intel_edp_is_psr_enabled(intel_dp))
+		return;
+
+	val = I915_READ(EDP_PSR_CTL);
+	I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
+
+	/* Wait till PSR is idle */
+	if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
+		       EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
+		DRM_ERROR("Timed out waiting for PSR Idle State\n");
+
+	intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+	val = I915_READ(reg);
+	I915_WRITE(reg, val & ~VIDEO_DIP_ENABLE_VSC_HSW);
+}
+
 static void intel_enable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 865ede6..cfda739 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -696,4 +696,7 @@ extern bool
 intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
 extern void intel_ddi_fdi_disable(struct drm_crtc *crtc);
 
+extern void intel_edp_enable_psr(struct intel_dp* intel_dp);
+extern void intel_edp_disable_psr(struct intel_dp* intel_dp);
+
 #endif /* __INTEL_DRV_H__ */
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 7/8] drm/i915: Added debugfs support for PSR Status
  2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
                   ` (5 preceding siblings ...)
  2013-02-25 22:55 ` [PATCH 6/8] drm/i915: Enable/Disable PSR Rodrigo Vivi
@ 2013-02-25 22:55 ` Rodrigo Vivi
  2013-02-28 17:44   ` [Intel-gfx] " Paulo Zanoni
  2013-02-25 22:55 ` [PATCH 8/8] drm/i915: Hook PSR functionality Rodrigo Vivi
  2013-02-28 17:52 ` [Intel-gfx] [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Paulo Zanoni
  8 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 22:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Adding support for PSR Status, PSR entry counter and performance counters.
Heavily based on initial work from Shobhit.

Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 92 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h     | 24 ++++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7c65ab8..01021f6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1680,6 +1680,97 @@ static int i915_dpio_info(struct seq_file *m, void *data)
 	return 0;
 }
 
+
+static int i915_edp_psr_status(struct seq_file *m, void *data)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned int count;
+	u32 psrctl, psrstat, psrperf;
+
+	psrctl = I915_READ(EDP_PSR_CTL);
+	seq_printf(m, "PSR Enabled: %s\n",
+		   yesno(psrctl & EDP_PSR_ENABLE));
+
+	psrstat = I915_READ(EDP_PSR_STATUS_CTL);
+
+	seq_printf(m, "PSR Current State: ");
+	switch (psrstat & EDP_PSR_STATUS_STATE_MASK) {
+	case EDP_PSR_STATUS_STATE_IDLE:
+		seq_printf(m, "Reset state\n");
+		break;
+	case EDP_PSR_STATUS_STATE_SRDONACK:
+		seq_printf(m, "Wait for TG/Stream to send on frame of data after SRD conditions are met\n");
+		break;
+	case EDP_PSR_STATUS_STATE_SRDENT:
+		seq_printf(m, "SRD entry\n");
+		break;
+	case EDP_PSR_STATUS_STATE_BUFOFF:
+		seq_printf(m, "Wait for buffer turn off\n");
+		break;
+	case EDP_PSR_STATUS_STATE_BUFON:
+			seq_printf(m, "Wait for buffer turn on\n");
+		break;
+	case EDP_PSR_STATUS_STATE_AUXACK:
+			seq_printf(m, "Wait for AUX to acknowledge on SRD exit\n");
+		break;
+	case EDP_PSR_STATUS_STATE_SRDOFFACK:
+			seq_printf(m, "Wait for TG/Stream to acknowledge the SRD VDM exit\n");
+		break;
+	default:
+		seq_printf(m, "Unknown\n");
+		break;
+	}
+
+	seq_printf(m, "Link Status: ");
+	switch (psrstat & EDP_PSR_STATUS_LINK_MASK) {
+	case EDP_PSR_STATUS_LINK_FULL_OFF:
+		seq_printf(m, "Link is fully off\n");
+		break;
+	case EDP_PSR_STATUS_LINK_FULL_ON:
+		seq_printf(m, "Link is fully on\n");
+		break;
+	case EDP_PSR_STATUS_LINK_STANDBY:
+		seq_printf(m, "Link is in standby\n");
+		break;
+	default:
+		seq_printf(m, "Unknown\n");
+		break;
+	}
+
+	seq_printf(m, "PSR Entry Count: %u\n",
+		   psrstat >> EDP_PSR_STATUS_COUNT_SHIFT &
+		   EDP_PSR_STATUS_COUNT_MASK);
+
+	seq_printf(m, "Max Sleep Timer Counter: %u\n",
+		   psrstat >> EDP_PSR_STATUS_MAX_SLEEP_TIMER_SHIFT &
+		   EDP_PSR_STATUS_MAX_SLEEP_TIMER_MASK);
+
+	seq_printf(m, "Had AUX error: %s\n",
+		   yesno(psrstat & EDP_PSR_STATUS_AUX_ERROR));
+
+	seq_printf(m, "Sending AUX: %s\n",
+		   yesno(psrstat & EDP_PSR_STATUS_AUX_SENDING));
+
+	seq_printf(m, "Sending Idle: %s\n",
+		   yesno(psrstat & EDP_PSR_STATUS_SENDING_IDLE));
+
+	seq_printf(m, "Sending TP2 TP3: %s\n",
+		   yesno(psrstat & EDP_PSR_STATUS_SENDING_TP2_TP3));
+
+	seq_printf(m, "Sending TP1: %s\n",
+		   yesno(psrstat & EDP_PSR_STATUS_SENDING_TP1));
+
+	seq_printf(m, "Idle Count: %u\n",
+		   psrstat & EDP_PSR_STATUS_IDLE_MASK);
+
+	psrperf = (I915_READ(EDP_PSR_PERF_CNT)) & EDP_PSR_PERF_CNT_MASK;
+	seq_printf(m, "Performance Counter: %u\n", psrperf);
+
+	return 0;
+}
+
 static ssize_t
 i915_wedged_read(struct file *filp,
 		 char __user *ubuf,
@@ -2249,6 +2340,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_swizzle_info", i915_swizzle_info, 0},
 	{"i915_ppgtt_info", i915_ppgtt_info, 0},
 	{"i915_dpio", i915_dpio_info, 0},
+	{"i915_edp_psr_status", i915_edp_psr_status, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1e31f23..1f7e666 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1615,6 +1615,30 @@
 
 #define EDP_PSR_STATUS_CTL			0x64840
 #define   EDP_PSR_STATUS_STATE_MASK		(7<<29)
+#define   EDP_PSR_STATUS_STATE_IDLE		(0<<29)
+#define   EDP_PSR_STATUS_STATE_SRDONACK		(1<<29)
+#define   EDP_PSR_STATUS_STATE_SRDENT		(2<<29)
+#define   EDP_PSR_STATUS_STATE_BUFOFF		(3<<29)
+#define   EDP_PSR_STATUS_STATE_BUFON		(4<<29)
+#define   EDP_PSR_STATUS_STATE_AUXACK		(5<<29)
+#define   EDP_PSR_STATUS_STATE_SRDOFFACK	(6<<29)
+#define   EDP_PSR_STATUS_LINK_MASK		(3<<29)
+#define   EDP_PSR_STATUS_LINK_FULL_OFF		(0<<29)
+#define   EDP_PSR_STATUS_LINK_FULL_ON		(1<<29)
+#define   EDP_PSR_STATUS_LINK_STANDBY		(2<<29)
+#define   EDP_PSR_STATUS_MAX_SLEEP_TIMER_SHIFT 	20
+#define   EDP_PSR_STATUS_MAX_SLEEP_TIMER_MASK 	0x1f
+#define   EDP_PSR_STATUS_COUNT_SHIFT		16
+#define   EDP_PSR_STATUS_COUNT_MASK		0xf
+#define   EDP_PSR_STATUS_AUX_ERROR		(1<<15)
+#define   EDP_PSR_STATUS_AUX_SENDING		(1<<12)
+#define   EDP_PSR_STATUS_SENDING_IDLE		(1<<9)
+#define   EDP_PSR_STATUS_SENDING_TP2_TP3	(1<<8)
+#define   EDP_PSR_STATUS_SENDING_TP1		(1<<4)
+#define   EDP_PSR_STATUS_IDLE_MASK		0xf
+
+#define EDP_PSR_PERF_CNT		0x64844
+#define   EDP_PSR_PERF_CNT_MASK		0xffffff
 
 #define EDP_PSR_DEBUG_CTL		0x64860
 #define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 8/8] drm/i915: Hook PSR functionality
  2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
                   ` (6 preceding siblings ...)
  2013-02-25 22:55 ` [PATCH 7/8] drm/i915: Added debugfs support for PSR Status Rodrigo Vivi
@ 2013-02-25 22:55 ` Rodrigo Vivi
  2013-02-28 17:47   ` [Intel-gfx] " Paulo Zanoni
  2013-02-28 17:52 ` [Intel-gfx] [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Paulo Zanoni
  8 siblings, 1 reply; 26+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 22:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

PSR must be enabled after transcoder and port are running.
And it is only available for HSW.

v2: move enable/disable to intel_ddi

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 816c45c..bbfdcfd 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1321,6 +1321,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
 		ironlake_edp_backlight_on(intel_dp);
+		intel_edp_enable_psr(intel_dp);
 	}
 
 	if (intel_crtc->eld_vld) {
@@ -1345,6 +1346,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
 		ironlake_edp_backlight_off(intel_dp);
+		intel_edp_disable_psr(intel_dp);
 	}
 
 	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 6/8] drm/i915: Enable/Disable PSR
  2013-02-25 22:55 ` [PATCH 6/8] drm/i915: Enable/Disable PSR Rodrigo Vivi
@ 2013-02-26 12:48   ` Ville Syrjälä
  2013-03-07 16:54     ` Jesse Barnes
  2013-02-26 13:27   ` Jani Nikula
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2013-02-26 12:48 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

On Mon, Feb 25, 2013 at 07:55:20PM -0300, Rodrigo Vivi wrote:
> Adding Enable and Disable PSR functionalities. This includes setting the
> PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
> enabling PSR in the sink via DPCD register and finally enabling PSR on
> the host.
> 
> This patch is heavily based on initial PSR code by Sateesh Kavuri and
> Kumar Shobhit but in a different implementation.
> 
> Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  40 +++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 183 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |   3 +
>  3 files changed, 226 insertions(+)
> 
<snip>
> +void intel_edp_write_vsc_psr(struct intel_dp* intel_dp,
> +			     struct edp_vsc_psr *vsc_psr)
> +{
> +	struct drm_device *dev =  intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc =  to_intel_crtc(intel_dp_to_crtc(intel_dp));
> +	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> +	u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(intel_crtc->cpu_transcoder);
> +	uint32_t *data = (uint32_t *)vsc_psr;
> +	unsigned int i;
> +	u32 val = I915_READ(ctl_reg);
> +
> +	if (data_reg == 0)
> +		return;
> +
> +	/* As per eDP spec, wait for vblank to send SDP VSC packet */
> +	intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> +	mmiowb();

I was curious about these mmiowb()s and apparently they were added to
all infoframe writes "just in case". But AFAICS on x86 mmiowb() ends
up as a compiler barrier, so this stuff seems to be a nop.

And if these writes can get reordered somewhere, why not everything
else too? I'm sure we have places where we write a bunch of registers,
and the final write enables something which requires the earlier
writes to have landed beforehand.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 6/8] drm/i915: Enable/Disable PSR
  2013-02-25 22:55 ` [PATCH 6/8] drm/i915: Enable/Disable PSR Rodrigo Vivi
  2013-02-26 12:48   ` Ville Syrjälä
@ 2013-02-26 13:27   ` Jani Nikula
  2013-02-26 15:02   ` Ville Syrjälä
  2013-02-28 17:21   ` Paulo Zanoni
  3 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2013-02-26 13:27 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: dri-devel

On Tue, 26 Feb 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> Adding Enable and Disable PSR functionalities. This includes setting the
> PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
> enabling PSR in the sink via DPCD register and finally enabling PSR on
> the host.
>
> This patch is heavily based on initial PSR code by Sateesh Kavuri and
> Kumar Shobhit but in a different implementation.
>
> Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  40 +++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 183 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |   3 +
>  3 files changed, 226 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b715ecd..1e31f23 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -844,6 +844,8 @@
>  #define   SNB_CPU_FENCE_ENABLE	(1<<29)
>  #define DPFC_CPU_FENCE_OFFSET	0x100104
>  
> +/* Framebuffer compression for Haswell */
> +#define HSW_FBC_CONTROL		0x43208
>  
>  /*
>   * GPIO regs
> @@ -1580,6 +1582,44 @@
>  #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
>  #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
>  
> +/* HSW eDP PSR registers */
> +#define EDP_PSR_CTL				0x64800
> +#define   EDP_PSR_ENABLE			(1<<31)
> +#define   EDP_PSR_LINK_DISABLE			(0<<27)
> +#define   EDP_PSR_LINK_STANDBY			(1<<27)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_MASK	(3<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES	(0<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES	(1<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES	(2<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES	(3<<25)
> +#define   EDP_PSR_MAX_SLEEP_TIME_SHIFT		20
> +#define   EDP_PSR_SKIP_AUX_EXIT			(1<<12)
> +#define   EDP_PSR_TP1_TP2_SEL			(0<<11)
> +#define   EDP_PSR_TP1_TP3_SEL			(1<<11)
> +#define   EDP_PSR_TP2_TP3_TIME_500us		(0<<8)
> +#define   EDP_PSR_TP2_TP3_TIME_100us		(1<<8)
> +#define   EDP_PSR_TP2_TP3_TIME_2500us		(2<<8)
> +#define   EDP_PSR_TP2_TP3_TIME_0us		(3<<8)
> +#define   EDP_PSR_TP1_TIME_500us		(0<<4)
> +#define   EDP_PSR_TP1_TIME_100us		(1<<4)
> +#define   EDP_PSR_TP1_TIME_2500us		(2<<4)
> +#define   EDP_PSR_TP1_TIME_0us			(3<<4)
> +#define   EDP_PSR_IDLE_FRAME_SHIFT		0
> +
> +#define EDP_PSR_AUX_CTL			0x64810
> +#define EDP_PSR_AUX_DATA1		0x64814
> +#define EDP_PSR_AUX_DATA2		0x64818
> +#define EDP_PSR_AUX_DATA3		0x6481c
> +#define EDP_PSR_AUX_DATA4		0x64820
> +#define EDP_PSR_AUX_DATA5		0x64824
> +
> +#define EDP_PSR_STATUS_CTL			0x64840
> +#define   EDP_PSR_STATUS_STATE_MASK		(7<<29)
> +
> +#define EDP_PSR_DEBUG_CTL		0x64860
> +#define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
> +#define   EDP_PSR_DEBUG_MASK_HPD	(1<<25)
> +
>  /* VGA port control */
>  #define ADPA			0x61100
>  #define PCH_ADPA                0xe1100
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5cfa9f4..a420f0d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -83,6 +83,13 @@ static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
>  	return intel_dig_port->base.base.dev;
>  }
>  
> +static struct drm_crtc *intel_dp_to_crtc(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +
> +	return intel_dig_port->base.base.crtc;
> +}
> +
>  static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
>  {
>  	return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
> @@ -1443,6 +1450,182 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
>  		(intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED));
>  }
>  
> +static bool
> +intel_edp_is_psr_enabled(struct intel_dp* intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	return I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
> +}
> +
> +static void
> +intel_edp_psr_enable_src(struct intel_dp *intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t max_sleep_time = 0x1f;
> +	uint32_t val = 0x0;
> +
> +	if (dev_priv->vbt.psr_idle_frames)
> +	    val |= dev_priv->vbt.psr_idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
> +	else
> +	    val |= 1 << EDP_PSR_IDLE_FRAME_SHIFT;
> +
> +	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
> +		val |= EDP_PSR_LINK_STANDBY;
> +		val |= EDP_PSR_TP2_TP3_TIME_0us;
> +		val |= EDP_PSR_TP1_TIME_0us;
> +		val |= EDP_PSR_SKIP_AUX_EXIT;
> +	} else {
> +		val |= EDP_PSR_LINK_DISABLE;
> +		/* Use these Values from VBT
> +		 * Case values are timings for HSW as of now
> +		 * in multiple of 100us
> +		 */
> +		switch(dev_priv->vbt.psr_wakeup_tp1) {
> +			case 1:
> +				val |= EDP_PSR_TP1_TIME_100us;
> +				break;
> +			case 5:
> +				val |= EDP_PSR_TP1_TIME_500us;
> +				break;
> +			case 25:
> +				val |= EDP_PSR_TP1_TIME_2500us;
> +				break;
> +			default:
> +				val |= EDP_PSR_TP1_TIME_500us;
> +				break;
> +		};
> +		switch(dev_priv->vbt.psr_wakeup_tp2_tp3) {
> +			case 1:
> +				val |= EDP_PSR_TP2_TP3_TIME_100us;
> +				break;
> +			case 5:
> +				val |= EDP_PSR_TP2_TP3_TIME_500us;
> +				break;
> +			case 25:
> +				val |= EDP_PSR_TP2_TP3_TIME_2500us;
> +				break;
> +			default:
> +				val |= EDP_PSR_TP2_TP3_TIME_500us;
> +				break;
> +		};
> +	}
> +
> +	I915_WRITE(EDP_PSR_CTL, val |
> +		   EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES |
> +		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> +		   EDP_PSR_ENABLE);
> +}
> +
> +void intel_edp_write_vsc_psr(struct intel_dp* intel_dp,
> +			     struct edp_vsc_psr *vsc_psr)
> +{
> +	struct drm_device *dev =  intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc =  to_intel_crtc(intel_dp_to_crtc(intel_dp));
> +	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> +	u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(intel_crtc->cpu_transcoder);
> +	uint32_t *data = (uint32_t *)vsc_psr;
> +	unsigned int i;
> +	u32 val = I915_READ(ctl_reg);
> +
> +	if (data_reg == 0)
> +		return;
> +
> +	/* As per eDP spec, wait for vblank to send SDP VSC packet */
> +	intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> +	mmiowb();
> +	for (i = 0; i < sizeof(struct edp_vsc_psr); i += 4) {
> +		I915_WRITE(data_reg + i, *data);
> +		data++;
> +	}
> +	/* Write every possible data byte to force correct ECC calculation. */
> +	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
> +		I915_WRITE(data_reg + i, 0);

Bikeshedding... I think this would be cleaner than the two loops:

	for (i = 0; i < VIDEO_DIP_DATA_SIZE; i += 4) {
		if (i < sizeof(struct edp_vsc_psr))
			I915_WRITE(data_reg + i, *data++);
		else
			I915_WRITE(data_reg + i, 0);
	}

BR,
Jani.


> +	mmiowb();
> +
> +	val |= VIDEO_DIP_ENABLE_VSC_HSW;
> +	I915_WRITE(ctl_reg, val);
> +	POSTING_READ(ctl_reg);
> +}
> +
> +void intel_edp_enable_psr(struct intel_dp* intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct edp_vsc_psr psr_vsc;
> +	int msg_size = 5;       /* Header(4) + Message(1) */
> +
> +	if (!is_edp_psr(intel_dp) || intel_edp_is_psr_enabled(intel_dp))
> +		return;
> +
> +	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
> +	memset(&psr_vsc, 0, sizeof(psr_vsc));
> +	psr_vsc.sdp_header.id = 0;
> +	psr_vsc.sdp_header.type = 0x7;
> +	psr_vsc.sdp_header.revision = 0x2;
> +	psr_vsc.sdp_header.valid_payload_bytes = 0x8;
> +
> +	intel_edp_write_vsc_psr(intel_dp, &psr_vsc);
> +
> +	/* Enable PSR in sink by setting bit 0 in DPCD config reg
> +	 * along with the transmitter state during PSR active
> +	 * Transmitter state later can be ready from VBT. As of now
> +	 * program the full link down
> +	 *
> +	 */
> +	intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
> +				    DP_PSR_ENABLE);
> +
> +	/* Setup AUX registers */
> +	/* Write command on DPCD 0x0600 */
> +	I915_WRITE(EDP_PSR_AUX_DATA1, 0x80060000);
> +
> +	/* Set the state to normal operation D0 in DPCD 0x0600 */
> +	I915_WRITE(EDP_PSR_AUX_DATA2, 0x01000000);
> +
> +	I915_WRITE(EDP_PSR_AUX_CTL,
> +		   msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT |
> +		   I915_READ(DPA_AUX_CH_CTL));
> +
> +	/* Enable PSR on the host */
> +	intel_edp_psr_enable_src(intel_dp);
> +
> +	/* WA: FBC/PSR Underrun/Corruption Workaround */
> +	I915_WRITE(HSW_FBC_CONTROL, ~FBC_CTL_EN);
> +
> +	/* WA: Continuous PSR exit Workaround */
> +	I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP);
> +	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
> +}
> +
> +void intel_edp_disable_psr(struct intel_dp* intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
> +	uint32_t reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> +	uint32_t val;
> +
> +	if (!intel_edp_is_psr_enabled(intel_dp))
> +		return;
> +
> +	val = I915_READ(EDP_PSR_CTL);
> +	I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
> +
> +	/* Wait till PSR is idle */
> +	if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
> +		       EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
> +		DRM_ERROR("Timed out waiting for PSR Idle State\n");
> +
> +	intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> +	val = I915_READ(reg);
> +	I915_WRITE(reg, val & ~VIDEO_DIP_ENABLE_VSC_HSW);
> +}
> +
>  static void intel_enable_dp(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 865ede6..cfda739 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -696,4 +696,7 @@ extern bool
>  intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
>  extern void intel_ddi_fdi_disable(struct drm_crtc *crtc);
>  
> +extern void intel_edp_enable_psr(struct intel_dp* intel_dp);
> +extern void intel_edp_disable_psr(struct intel_dp* intel_dp);
> +
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 1.8.1.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 6/8] drm/i915: Enable/Disable PSR
  2013-02-25 22:55 ` [PATCH 6/8] drm/i915: Enable/Disable PSR Rodrigo Vivi
  2013-02-26 12:48   ` Ville Syrjälä
  2013-02-26 13:27   ` Jani Nikula
@ 2013-02-26 15:02   ` Ville Syrjälä
  2013-02-28 17:30     ` [Intel-gfx] " Paulo Zanoni
  2013-02-28 17:21   ` Paulo Zanoni
  3 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2013-02-26 15:02 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

On Mon, Feb 25, 2013 at 07:55:20PM -0300, Rodrigo Vivi wrote:
> Adding Enable and Disable PSR functionalities. This includes setting the
> PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
> enabling PSR in the sink via DPCD register and finally enabling PSR on
> the host.
> 
> This patch is heavily based on initial PSR code by Sateesh Kavuri and
> Kumar Shobhit but in a different implementation.
> 
> Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  40 +++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 183 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |   3 +
>  3 files changed, 226 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b715ecd..1e31f23 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -844,6 +844,8 @@
>  #define   SNB_CPU_FENCE_ENABLE	(1<<29)
>  #define DPFC_CPU_FENCE_OFFSET	0x100104
>  
> +/* Framebuffer compression for Haswell */
> +#define HSW_FBC_CONTROL		0x43208
>  
>  /*
>   * GPIO regs
> @@ -1580,6 +1582,44 @@
>  #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
>  #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
>  
> +/* HSW eDP PSR registers */
> +#define EDP_PSR_CTL				0x64800
> +#define   EDP_PSR_ENABLE			(1<<31)
> +#define   EDP_PSR_LINK_DISABLE			(0<<27)
> +#define   EDP_PSR_LINK_STANDBY			(1<<27)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_MASK	(3<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES	(0<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES	(1<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES	(2<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES	(3<<25)
> +#define   EDP_PSR_MAX_SLEEP_TIME_SHIFT		20
> +#define   EDP_PSR_SKIP_AUX_EXIT			(1<<12)
> +#define   EDP_PSR_TP1_TP2_SEL			(0<<11)
> +#define   EDP_PSR_TP1_TP3_SEL			(1<<11)
> +#define   EDP_PSR_TP2_TP3_TIME_500us		(0<<8)
> +#define   EDP_PSR_TP2_TP3_TIME_100us		(1<<8)
> +#define   EDP_PSR_TP2_TP3_TIME_2500us		(2<<8)
> +#define   EDP_PSR_TP2_TP3_TIME_0us		(3<<8)
> +#define   EDP_PSR_TP1_TIME_500us		(0<<4)
> +#define   EDP_PSR_TP1_TIME_100us		(1<<4)
> +#define   EDP_PSR_TP1_TIME_2500us		(2<<4)
> +#define   EDP_PSR_TP1_TIME_0us			(3<<4)
> +#define   EDP_PSR_IDLE_FRAME_SHIFT		0
> +
> +#define EDP_PSR_AUX_CTL			0x64810
> +#define EDP_PSR_AUX_DATA1		0x64814
> +#define EDP_PSR_AUX_DATA2		0x64818
> +#define EDP_PSR_AUX_DATA3		0x6481c
> +#define EDP_PSR_AUX_DATA4		0x64820
> +#define EDP_PSR_AUX_DATA5		0x64824
> +
> +#define EDP_PSR_STATUS_CTL			0x64840
> +#define   EDP_PSR_STATUS_STATE_MASK		(7<<29)
> +
> +#define EDP_PSR_DEBUG_CTL		0x64860
> +#define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
> +#define   EDP_PSR_DEBUG_MASK_HPD	(1<<25)
> +
>  /* VGA port control */
>  #define ADPA			0x61100
>  #define PCH_ADPA                0xe1100
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5cfa9f4..a420f0d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -83,6 +83,13 @@ static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
>  	return intel_dig_port->base.base.dev;
>  }
>  
> +static struct drm_crtc *intel_dp_to_crtc(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +
> +	return intel_dig_port->base.base.crtc;
> +}
> +
>  static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
>  {
>  	return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
> @@ -1443,6 +1450,182 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
>  		(intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED));
>  }
>  
> +static bool
> +intel_edp_is_psr_enabled(struct intel_dp* intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	return I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
> +}
> +
> +static void
> +intel_edp_psr_enable_src(struct intel_dp *intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t max_sleep_time = 0x1f;
> +	uint32_t val = 0x0;
> +
> +	if (dev_priv->vbt.psr_idle_frames)
> +	    val |= dev_priv->vbt.psr_idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
> +	else
> +	    val |= 1 << EDP_PSR_IDLE_FRAME_SHIFT;
> +
> +	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
> +		val |= EDP_PSR_LINK_STANDBY;

There were some WAs for link standby not working on all revisions.
Do we care?

> +		val |= EDP_PSR_TP2_TP3_TIME_0us;
> +		val |= EDP_PSR_TP1_TIME_0us;
> +		val |= EDP_PSR_SKIP_AUX_EXIT;
> +	} else {
> +		val |= EDP_PSR_LINK_DISABLE;
> +		/* Use these Values from VBT
> +		 * Case values are timings for HSW as of now
> +		 * in multiple of 100us
> +		 */
> +		switch(dev_priv->vbt.psr_wakeup_tp1) {
> +			case 1:
> +				val |= EDP_PSR_TP1_TIME_100us;
> +				break;
> +			case 5:
> +				val |= EDP_PSR_TP1_TIME_500us;
> +				break;
> +			case 25:
> +				val |= EDP_PSR_TP1_TIME_2500us;
> +				break;
> +			default:
> +				val |= EDP_PSR_TP1_TIME_500us;
> +				break;
> +		};
> +		switch(dev_priv->vbt.psr_wakeup_tp2_tp3) {
> +			case 1:
> +				val |= EDP_PSR_TP2_TP3_TIME_100us;
> +				break;
> +			case 5:
> +				val |= EDP_PSR_TP2_TP3_TIME_500us;
> +				break;
> +			case 25:
> +				val |= EDP_PSR_TP2_TP3_TIME_2500us;
> +				break;
> +			default:
> +				val |= EDP_PSR_TP2_TP3_TIME_500us;
> +				break;
> +		};
> +	}
> +
> +	I915_WRITE(EDP_PSR_CTL, val |
> +		   EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES |
> +		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> +		   EDP_PSR_ENABLE);
> +}
> +
> +void intel_edp_write_vsc_psr(struct intel_dp* intel_dp,
> +			     struct edp_vsc_psr *vsc_psr)
> +{
> +	struct drm_device *dev =  intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc =  to_intel_crtc(intel_dp_to_crtc(intel_dp));
> +	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> +	u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(intel_crtc->cpu_transcoder);
> +	uint32_t *data = (uint32_t *)vsc_psr;
> +	unsigned int i;
> +	u32 val = I915_READ(ctl_reg);
> +
> +	if (data_reg == 0)
> +		return;
> +
> +	/* As per eDP spec, wait for vblank to send SDP VSC packet */
> +	intel_wait_for_vblank(dev, intel_crtc->pipe);

Is this really needed? Isn't the SDP stuff sent out during vblank by
definition?

> +
> +	mmiowb();
> +	for (i = 0; i < sizeof(struct edp_vsc_psr); i += 4) {
> +		I915_WRITE(data_reg + i, *data);
> +		data++;
> +	}
> +	/* Write every possible data byte to force correct ECC calculation. */
> +	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
> +		I915_WRITE(data_reg + i, 0);
> +	mmiowb();
> +
> +	val |= VIDEO_DIP_ENABLE_VSC_HSW;
> +	I915_WRITE(ctl_reg, val);
> +	POSTING_READ(ctl_reg);
> +}
> +
> +void intel_edp_enable_psr(struct intel_dp* intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct edp_vsc_psr psr_vsc;
> +	int msg_size = 5;       /* Header(4) + Message(1) */
> +
> +	if (!is_edp_psr(intel_dp) || intel_edp_is_psr_enabled(intel_dp))
> +		return;
> +
> +	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
> +	memset(&psr_vsc, 0, sizeof(psr_vsc));
> +	psr_vsc.sdp_header.id = 0;
> +	psr_vsc.sdp_header.type = 0x7;
> +	psr_vsc.sdp_header.revision = 0x2;
> +	psr_vsc.sdp_header.valid_payload_bytes = 0x8;
> +
> +	intel_edp_write_vsc_psr(intel_dp, &psr_vsc);
> +
> +	/* Enable PSR in sink by setting bit 0 in DPCD config reg
> +	 * along with the transmitter state during PSR active
> +	 * Transmitter state later can be ready from VBT. As of now
> +	 * program the full link down
> +	 *
> +	 */
> +	intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
> +				    DP_PSR_ENABLE);
> +
> +	/* Setup AUX registers */
> +	/* Write command on DPCD 0x0600 */
> +	I915_WRITE(EDP_PSR_AUX_DATA1, 0x80060000);
> +
> +	/* Set the state to normal operation D0 in DPCD 0x0600 */
> +	I915_WRITE(EDP_PSR_AUX_DATA2, 0x01000000);
> +
> +	I915_WRITE(EDP_PSR_AUX_CTL,
> +		   msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT |
> +		   I915_READ(DPA_AUX_CH_CTL));

Would it be possible to make this stuff a bit more like the normal
aux stuff? Maybe even share some code to set up the data? But then
again it's just these few writes here, so I'm not sure it's worth
the effort.

> +
> +	/* Enable PSR on the host */
> +	intel_edp_psr_enable_src(intel_dp);
> +
> +	/* WA: FBC/PSR Underrun/Corruption Workaround */
> +	I915_WRITE(HSW_FBC_CONTROL, ~FBC_CTL_EN);

Are we expecting that BIOS left FBC enabled? Shouldn't we then disable
it during resume?

> +
> +	/* WA: Continuous PSR exit Workaround */
> +	I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP);

Shouldn't this be done before enabling PSR?

> +	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);

I can't find anything about this in bspec or WA db.

> +}
> +
> +void intel_edp_disable_psr(struct intel_dp* intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
> +	uint32_t reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> +	uint32_t val;
> +
> +	if (!intel_edp_is_psr_enabled(intel_dp))
> +		return;
> +
> +	val = I915_READ(EDP_PSR_CTL);
> +	I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
> +
> +	/* Wait till PSR is idle */
> +	if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
> +		       EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
> +		DRM_ERROR("Timed out waiting for PSR Idle State\n");
> +
> +	intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> +	val = I915_READ(reg);
> +	I915_WRITE(reg, val & ~VIDEO_DIP_ENABLE_VSC_HSW);
> +}
> +
>  static void intel_enable_dp(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 865ede6..cfda739 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -696,4 +696,7 @@ extern bool
>  intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
>  extern void intel_ddi_fdi_disable(struct drm_crtc *crtc);
>  
> +extern void intel_edp_enable_psr(struct intel_dp* intel_dp);
> +extern void intel_edp_disable_psr(struct intel_dp* intel_dp);
> +
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 1.8.1.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/8] drm/i915: Read the EDP DPCD and PSR Capability
  2013-02-25 22:55 ` [PATCH 4/8] drm/i915: Read the EDP DPCD and PSR Capability Rodrigo Vivi
@ 2013-02-26 15:02   ` Ville Syrjälä
  0 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjälä @ 2013-02-26 15:02 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

On Mon, Feb 25, 2013 at 07:55:18PM -0300, Rodrigo Vivi wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> v2: reuse of just created is_edp_psr and put it at right place.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 13 +++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b4494a2..5cfa9f4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1437,6 +1437,12 @@ static void intel_post_disable_dp(struct intel_encoder *encoder)
>  	}
>  }
>  
> +static bool is_edp_psr(struct intel_dp *intel_dp)
> +{
> +	return (is_edp(intel_dp) &&
> +		(intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED));

This should probably also check the setup time vs. vblank restriction.

> +}
> +
>  static void intel_enable_dp(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> @@ -2117,6 +2123,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
>  		return false; /* DPCD not present */
>  
> +	/* Check if the panel supports PSR */
> +	memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
> +	intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
> +				       intel_dp->psr_dpcd,
> +				       sizeof(intel_dp->psr_dpcd));
> +	if (is_edp_psr(intel_dp))
> +		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
>  	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>  	      DP_DWN_STRM_PORT_PRESENT))
>  		return true; /* native DP sink */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 005a91f..865ede6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -375,6 +375,7 @@ struct intel_dp {
>  	uint8_t link_bw;
>  	uint8_t lane_count;
>  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> +	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>  	struct i2c_adapter adapter;
>  	struct i2c_algo_dp_aux_data algo;
> -- 
> 1.8.1.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 2/8] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
  2013-02-25 22:55 ` [PATCH 2/8] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe Rodrigo Vivi
@ 2013-02-27 19:44   ` Paulo Zanoni
  2013-03-03 17:26     ` Daniel Vetter
  0 siblings, 1 reply; 26+ messages in thread
From: Paulo Zanoni @ 2013-02-27 19:44 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

Hi

2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> While old platforms had 3 transcoders and 3 pipes (1:1), HSW has
> 4 transcoders and 3 pipes.
> These regs were being used only by HDMI code where pipe is always the same
> thing as cpu_transcoder.
> This patch allow us to use them for DP, specially for TRANSCODER_EDP.
>
> v2: Adding HSW_TVIDEO_DIP_VSC_DATA to transmit vsc to eDP.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h   | 18 ++++++++++--------
>  drivers/gpu/drm/i915/intel_hdmi.c | 13 +++++++------
>  2 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 527b664..b715ecd 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3754,14 +3754,16 @@
>  #define HSW_VIDEO_DIP_VSC_ECC_B                0x61344
>  #define HSW_VIDEO_DIP_GCP_B            0x61210
>
> -#define HSW_TVIDEO_DIP_CTL(pipe) \
> -        _PIPE(pipe, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
> -#define HSW_TVIDEO_DIP_AVI_DATA(pipe) \
> -        _PIPE(pipe, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
> -#define HSW_TVIDEO_DIP_SPD_DATA(pipe) \
> -        _PIPE(pipe, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
> -#define HSW_TVIDEO_DIP_GCP(pipe) \
> -       _PIPE(pipe, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)
> +#define HSW_TVIDEO_DIP_CTL(trans) \
> +        _TRANSCODER(trans, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
> +#define HSW_TVIDEO_DIP_AVI_DATA(trans) \
> +        _TRANSCODER(trans, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
> +#define HSW_TVIDEO_DIP_SPD_DATA(trans) \
> +        _TRANSCODER(trans, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
> +#define HSW_TVIDEO_DIP_GCP(trans) \
> +       _TRANSCODER(trans, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)
> +#define HSW_TVIDEO_DIP_VSC_DATA(trans) \
> +        _TRANSCODER(trans, HSW_VIDEO_DIP_VSC_DATA_A, HSW_VIDEO_DIP_VSC_DATA_B)
>
>  #define _TRANS_HTOTAL_B          0xe1000
>  #define _TRANS_HBLANK_B          0xe1004
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 5a6138c..26290c1 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -120,13 +120,14 @@ static u32 hsw_infoframe_enable(struct dip_infoframe *frame)
>         }
>  }
>
> -static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame, enum pipe pipe)
> +static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame,
> +                                 enum transcoder cpu_transcoder)
>  {
>         switch (frame->type) {
>         case DIP_TYPE_AVI:
> -               return HSW_TVIDEO_DIP_AVI_DATA(pipe);
> +               return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
>         case DIP_TYPE_SPD:
> -               return HSW_TVIDEO_DIP_SPD_DATA(pipe);
> +               return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
>         default:
>                 DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
>                 return 0;
> @@ -293,8 +294,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>         struct drm_device *dev = encoder->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> -       u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
> -       u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->pipe);
> +       u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> +       u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->cpu_transcoder);
>         unsigned int i, len = DIP_HEADER_SIZE + frame->len;
>         u32 val = I915_READ(ctl_reg);
>
> @@ -568,7 +569,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder,
>         struct drm_i915_private *dev_priv = encoder->dev->dev_private;
>         struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>         struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> -       u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
> +       u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
>         u32 val = I915_READ(reg);
>
>         assert_hdmi_port_disabled(intel_hdmi);
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 3/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP
  2013-02-25 22:55 ` [PATCH 3/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP Rodrigo Vivi
@ 2013-02-27 21:52   ` Paulo Zanoni
  2013-03-03 17:28   ` Daniel Vetter
  1 sibling, 0 replies; 26+ messages in thread
From: Paulo Zanoni @ 2013-02-27 21:52 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

Hi

2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Signed-off-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
>
> v2: Modified and corrected the structures to be more in line for
> kernel coding guidelines and rebased the code on Paulo's DP patchset
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE
> v4: moving them to include/drm/drm_dp_helper.h and also already
>     icluding EDP_PSR_RECEIVER_CAP_SIZE to add everything needed
>     for PSR at once at drm_dp_helper.h
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

I'd group all Signed-off-by tags at the bottom :)

> ---
>  include/drm/drm_dp_helper.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index e8e1417..06c5060 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -343,12 +343,34 @@ u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
>                                           int lane);
>
>  #define DP_RECEIVER_CAP_SIZE   0xf
> +#define EDP_PSR_RECEIVER_CAP_SIZE      2

I know I'll sound annoying, but my OCD will kill me if I don't tell
you that the line above uses tab and you use spaces.

> +
>  void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
>  void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
>
>  u8 drm_dp_link_rate_to_bw_code(int link_rate);
>  int drm_dp_bw_code_to_link_rate(u8 link_bw);
>
> +/* SDP header as per eDP 1.3 spec, section 3.6 */
> +struct edp_sdp_header {
> +       u8 id;
> +       u8 type;
> +       u8 revision;
> +       u8 valid_payload_bytes;
> +} __attribute__((packed));

I think Jani's suggestion on the previous review still applies. How
about something like the following?

u8 id;
u8 type;
u8 hb2; /* 7:5 reserved, 4:0 revision number */
u8 hb3; /* 7:5 reserved, 4:0 number of valid data bytes */

And then something like this:
#define EDP_SDP_HEADER_REVISION_MASK 0x1F
#define EDP_SDP_HEADER_VALID_PAYLOAD_BYTES 0x1F

Also, checkpath.pl complains:
WARNING: __packed is preferred over __attribute__((packed))

> +
> +/* SDP VSC header as per eDP 1.3 spec, section 3.6 */
> +struct edp_vsc_psr {
> +       struct edp_sdp_header sdp_header;

Where's DB0? Its description is on DP spec version 1.2, chapter 2.2.5.6

> +       u8 DB1; /* 0 - PSR State; 1 - Update RFB; 2 - CRC Valid*/
> +       u8 DB2; /* CRC value bits 7:0 of the R or Cr component */
> +       u8 DB3; /* CRC value bits 15:8 of the R or Cr component */
> +       u8 DB4; /* CRC value bits 7:0 of the G or Y component */
> +       u8 DB5; /* CRC value bits 15:8 of the G or Y component */
> +       u8 DB6; /* CRC value bits 7:0 of the B or Cb component */
> +       u8 DB7; /* CRC value bits 15:8 of the B or Cb component */
> +} __attribute__((packed));

And here some more defines to access the individual byte fields would
be cool, like:

#define EDP_VSC_PSR_STATE_ACTIVE 1
#define EDP_VSC_PSR_RFB_UPDATE (1 << 1)
etc.

> +
>  static inline int
>  drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  {
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 6/8] drm/i915: Enable/Disable PSR
  2013-02-25 22:55 ` [PATCH 6/8] drm/i915: Enable/Disable PSR Rodrigo Vivi
                     ` (2 preceding siblings ...)
  2013-02-26 15:02   ` Ville Syrjälä
@ 2013-02-28 17:21   ` Paulo Zanoni
  3 siblings, 0 replies; 26+ messages in thread
From: Paulo Zanoni @ 2013-02-28 17:21 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

Hi

So now I finally have the correct spec in hands!

2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> Adding Enable and Disable PSR functionalities. This includes setting the
> PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
> enabling PSR in the sink via DPCD register and finally enabling PSR on
> the host.
>
> This patch is heavily based on initial PSR code by Sateesh Kavuri and
> Kumar Shobhit but in a different implementation.
>
> Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  40 +++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 183 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |   3 +
>  3 files changed, 226 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b715ecd..1e31f23 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -844,6 +844,8 @@
>  #define   SNB_CPU_FENCE_ENABLE (1<<29)
>  #define DPFC_CPU_FENCE_OFFSET  0x100104
>
> +/* Framebuffer compression for Haswell */
> +#define HSW_FBC_CONTROL                0x43208
>
>  /*
>   * GPIO regs
> @@ -1580,6 +1582,44 @@
>  #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
>  #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
>
> +/* HSW eDP PSR registers */
> +#define EDP_PSR_CTL                            0x64800
> +#define   EDP_PSR_ENABLE                       (1<<31)
> +#define   EDP_PSR_LINK_DISABLE                 (0<<27)
> +#define   EDP_PSR_LINK_STANDBY                 (1<<27)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_MASK     (3<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES  (0<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES  (1<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES  (2<<25)
> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES  (3<<25)
> +#define   EDP_PSR_MAX_SLEEP_TIME_SHIFT         20
> +#define   EDP_PSR_SKIP_AUX_EXIT                        (1<<12)
> +#define   EDP_PSR_TP1_TP2_SEL                  (0<<11)
> +#define   EDP_PSR_TP1_TP3_SEL                  (1<<11)
> +#define   EDP_PSR_TP2_TP3_TIME_500us           (0<<8)
> +#define   EDP_PSR_TP2_TP3_TIME_100us           (1<<8)
> +#define   EDP_PSR_TP2_TP3_TIME_2500us          (2<<8)
> +#define   EDP_PSR_TP2_TP3_TIME_0us             (3<<8)
> +#define   EDP_PSR_TP1_TIME_500us               (0<<4)
> +#define   EDP_PSR_TP1_TIME_100us               (1<<4)
> +#define   EDP_PSR_TP1_TIME_2500us              (2<<4)
> +#define   EDP_PSR_TP1_TIME_0us                 (3<<4)
> +#define   EDP_PSR_IDLE_FRAME_SHIFT             0
> +
> +#define EDP_PSR_AUX_CTL                        0x64810
> +#define EDP_PSR_AUX_DATA1              0x64814
> +#define EDP_PSR_AUX_DATA2              0x64818
> +#define EDP_PSR_AUX_DATA3              0x6481c
> +#define EDP_PSR_AUX_DATA4              0x64820
> +#define EDP_PSR_AUX_DATA5              0x64824
> +
> +#define EDP_PSR_STATUS_CTL                     0x64840
> +#define   EDP_PSR_STATUS_STATE_MASK            (7<<29)
> +
> +#define EDP_PSR_DEBUG_CTL              0x64860
> +#define   EDP_PSR_DEBUG_MASK_MEMUP     (1<<26)
> +#define   EDP_PSR_DEBUG_MASK_HPD       (1<<25)
> +
>  /* VGA port control */
>  #define ADPA                   0x61100
>  #define PCH_ADPA                0xe1100
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5cfa9f4..a420f0d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -83,6 +83,13 @@ static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
>         return intel_dig_port->base.base.dev;
>  }
>
> +static struct drm_crtc *intel_dp_to_crtc(struct intel_dp *intel_dp)
> +{
> +       struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +
> +       return intel_dig_port->base.base.crtc;
> +}
> +
>  static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
>  {
>         return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
> @@ -1443,6 +1450,182 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
>                 (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED));
>  }
>
> +static bool
> +intel_edp_is_psr_enabled(struct intel_dp* intel_dp)
> +{
> +       struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       return I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
> +}
> +
> +static void
> +intel_edp_psr_enable_src(struct intel_dp *intel_dp)
> +{
> +       struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       uint32_t max_sleep_time = 0x1f;
> +       uint32_t val = 0x0;
> +
> +       if (dev_priv->vbt.psr_idle_frames)
> +           val |= dev_priv->vbt.psr_idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
> +       else
> +           val |= 1 << EDP_PSR_IDLE_FRAME_SHIFT;
> +
> +       if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
> +               val |= EDP_PSR_LINK_STANDBY;

If we set this bit, shouldn't we also set DPCD 0x170 bit 1 to 1?

> +               val |= EDP_PSR_TP2_TP3_TIME_0us;
> +               val |= EDP_PSR_TP1_TIME_0us;
> +               val |= EDP_PSR_SKIP_AUX_EXIT;
> +       } else {
> +               val |= EDP_PSR_LINK_DISABLE;
> +               /* Use these Values from VBT
> +                * Case values are timings for HSW as of now
> +                * in multiple of 100us
> +                */
> +               switch(dev_priv->vbt.psr_wakeup_tp1) {
> +                       case 1:
> +                               val |= EDP_PSR_TP1_TIME_100us;
> +                               break;
> +                       case 5:
> +                               val |= EDP_PSR_TP1_TIME_500us;
> +                               break;
> +                       case 25:
> +                               val |= EDP_PSR_TP1_TIME_2500us;
> +                               break;
> +                       default:
> +                               val |= EDP_PSR_TP1_TIME_500us;
> +                               break;
> +               };
> +               switch(dev_priv->vbt.psr_wakeup_tp2_tp3) {
> +                       case 1:
> +                               val |= EDP_PSR_TP2_TP3_TIME_100us;
> +                               break;
> +                       case 5:
> +                               val |= EDP_PSR_TP2_TP3_TIME_500us;
> +                               break;
> +                       case 25:
> +                               val |= EDP_PSR_TP2_TP3_TIME_2500us;
> +                               break;
> +                       default:
> +                               val |= EDP_PSR_TP2_TP3_TIME_500us;
> +                               break;
> +               };
> +       }
> +
> +       I915_WRITE(EDP_PSR_CTL, val |
> +                  EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES |
> +                  max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> +                  EDP_PSR_ENABLE);
> +}
> +
> +void intel_edp_write_vsc_psr(struct intel_dp* intel_dp,
> +                            struct edp_vsc_psr *vsc_psr)
> +{
> +       struct drm_device *dev =  intel_dp_to_dev(intel_dp);
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct intel_crtc *intel_crtc =  to_intel_crtc(intel_dp_to_crtc(intel_dp));

I'd reorganize the declarations to avoid calling both intel_dp_to_crtc
and intel_dp_to_dev :)

> +       u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);

Please don't preserve the register contents here. We don't use this
register anywhere else for DP yet.

> +       u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(intel_crtc->cpu_transcoder);
> +       uint32_t *data = (uint32_t *)vsc_psr;
> +       unsigned int i;
> +       u32 val = I915_READ(ctl_reg);
> +
> +       if (data_reg == 0)
> +               return;

I don't think this is ever expected, so you could probably WARN() or
BUG() here. I see this statement comes from hsw_write_infoframe
(intel_hdmi.c), we should probably try to do the WARN/BUG there too.

> +
> +       /* As per eDP spec, wait for vblank to send SDP VSC packet */
> +       intel_wait_for_vblank(dev, intel_crtc->pipe);
> +

Please see BSpec page "Pipe Video Data Island Packet". After waiting
the vsync you need to disable the DIP first, then write the data.

> +       mmiowb();
> +       for (i = 0; i < sizeof(struct edp_vsc_psr); i += 4) {
> +               I915_WRITE(data_reg + i, *data);
> +               data++;
> +       }
> +       /* Write every possible data byte to force correct ECC calculation. */
> +       for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
> +               I915_WRITE(data_reg + i, 0);
> +       mmiowb();
> +
> +       val |= VIDEO_DIP_ENABLE_VSC_HSW;
> +       I915_WRITE(ctl_reg, val);
> +       POSTING_READ(ctl_reg);

According to the PSR enabling guide, "HW also enables VSC DIP when
required", so I guess the 3 lines above are not needed.

> +}
> +
> +void intel_edp_enable_psr(struct intel_dp* intel_dp)
> +{
> +       struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct edp_vsc_psr psr_vsc;
> +       int msg_size = 5;       /* Header(4) + Message(1) */
> +
> +       if (!is_edp_psr(intel_dp) || intel_edp_is_psr_enabled(intel_dp))

Is it possible to have intel_edp_is_psr_enable here? If not, maybe WARN it.

> +               return;
> +
> +       /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
> +       memset(&psr_vsc, 0, sizeof(psr_vsc));
> +       psr_vsc.sdp_header.id = 0;
> +       psr_vsc.sdp_header.type = 0x7;
> +       psr_vsc.sdp_header.revision = 0x2;
> +       psr_vsc.sdp_header.valid_payload_bytes = 0x8;
> +
> +       intel_edp_write_vsc_psr(intel_dp, &psr_vsc);
> +
> +       /* Enable PSR in sink by setting bit 0 in DPCD config reg
> +        * along with the transmitter state during PSR active
> +        * Transmitter state later can be ready from VBT. As of now
> +        * program the full link down

This comment is probably missing a few periods.

> +        *
> +        */
> +       intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
> +                                   DP_PSR_ENABLE);
> +
> +       /* Setup AUX registers */
> +       /* Write command on DPCD 0x0600 */
> +       I915_WRITE(EDP_PSR_AUX_DATA1, 0x80060000);
> +
> +       /* Set the state to normal operation D0 in DPCD 0x0600 */
> +       I915_WRITE(EDP_PSR_AUX_DATA2, 0x01000000);

I see the lines above match our spec, but I still think Jani's comment
from patch "drm/i915: Setup EDP PSR AUX Registers" is applicable here.

> +
> +       I915_WRITE(EDP_PSR_AUX_CTL,
> +                  msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT |
> +                  I915_READ(DPA_AUX_CH_CTL));

I don't think this is safe, especially since we're not erasing the
original msg size bits from DPA_AUX_CH_CTL. I think we should try to
set all the bits of this register without reading DPA_AUX_CH_CTL
(maybe extract some code from intel_dp_aux_ch into new functions and
call from both here and intel_dp_aux_ch).

Also, I still think you should patch intel_dp_aux_ch to put a WARN in
case EDP_PSR_CTL bit 31 is 1.

> +
> +       /* Enable PSR on the host */
> +       intel_edp_psr_enable_src(intel_dp);
> +
> +       /* WA: FBC/PSR Underrun/Corruption Workaround */
> +       I915_WRITE(HSW_FBC_CONTROL, ~FBC_CTL_EN);

This FBC WA won't be needed on real world Hardware.

> +
> +       /* WA: Continuous PSR exit Workaround */
> +       I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP);
> +       I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);

All they ask is to "disabled unused interrupts", not "disable all interrupts".

> +}
> +
> +void intel_edp_disable_psr(struct intel_dp* intel_dp)
> +{
> +       struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
> +       uint32_t reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> +       uint32_t val;
> +
> +       if (!intel_edp_is_psr_enabled(intel_dp))

Is this possible? Should we WARN here?

> +               return;
> +
> +       val = I915_READ(EDP_PSR_CTL);
> +       I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
> +
> +       /* Wait till PSR is idle */
> +       if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
> +                      EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))

Some big sleeps here. Can't we make them faster?

> +               DRM_ERROR("Timed out waiting for PSR Idle State\n");
> +
> +       intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> +       val = I915_READ(reg);
> +       I915_WRITE(reg, val & ~VIDEO_DIP_ENABLE_VSC_HSW);

So, based on this and on the spec, it means the HW will automagically
do the link training for us if it's needed?

> +}
> +
>  static void intel_enable_dp(struct intel_encoder *encoder)
>  {
>         struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 865ede6..cfda739 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -696,4 +696,7 @@ extern bool
>  intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
>  extern void intel_ddi_fdi_disable(struct drm_crtc *crtc);
>
> +extern void intel_edp_enable_psr(struct intel_dp* intel_dp);
> +extern void intel_edp_disable_psr(struct intel_dp* intel_dp);
> +
>  #endif /* __INTEL_DRV_H__ */
> --
> 1.8.1.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 6/8] drm/i915: Enable/Disable PSR
  2013-02-26 15:02   ` Ville Syrjälä
@ 2013-02-28 17:30     ` Paulo Zanoni
  0 siblings, 0 replies; 26+ messages in thread
From: Paulo Zanoni @ 2013-02-28 17:30 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

Hi

2013/2/26 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Mon, Feb 25, 2013 at 07:55:20PM -0300, Rodrigo Vivi wrote:
>> Adding Enable and Disable PSR functionalities. This includes setting the
>> PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
>> enabling PSR in the sink via DPCD register and finally enabling PSR on
>> the host.
>>
>> This patch is heavily based on initial PSR code by Sateesh Kavuri and
>> Kumar Shobhit but in a different implementation.
>>
>> Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
>> Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h  |  40 +++++++++
>>  drivers/gpu/drm/i915/intel_dp.c  | 183 +++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_drv.h |   3 +
>>  3 files changed, 226 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index b715ecd..1e31f23 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -844,6 +844,8 @@
>>  #define   SNB_CPU_FENCE_ENABLE       (1<<29)
>>  #define DPFC_CPU_FENCE_OFFSET        0x100104
>>
>> +/* Framebuffer compression for Haswell */
>> +#define HSW_FBC_CONTROL              0x43208
>>
>>  /*
>>   * GPIO regs
>> @@ -1580,6 +1582,44 @@
>>  #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
>>  #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
>>
>> +/* HSW eDP PSR registers */
>> +#define EDP_PSR_CTL                          0x64800
>> +#define   EDP_PSR_ENABLE                     (1<<31)
>> +#define   EDP_PSR_LINK_DISABLE                       (0<<27)
>> +#define   EDP_PSR_LINK_STANDBY                       (1<<27)
>> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_MASK   (3<<25)
>> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES        (0<<25)
>> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_4_LINES        (1<<25)
>> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_2_LINES        (2<<25)
>> +#define   EDP_PSR_MIN_LINK_ENTRY_TIME_0_LINES        (3<<25)
>> +#define   EDP_PSR_MAX_SLEEP_TIME_SHIFT               20
>> +#define   EDP_PSR_SKIP_AUX_EXIT                      (1<<12)
>> +#define   EDP_PSR_TP1_TP2_SEL                        (0<<11)
>> +#define   EDP_PSR_TP1_TP3_SEL                        (1<<11)
>> +#define   EDP_PSR_TP2_TP3_TIME_500us         (0<<8)
>> +#define   EDP_PSR_TP2_TP3_TIME_100us         (1<<8)
>> +#define   EDP_PSR_TP2_TP3_TIME_2500us                (2<<8)
>> +#define   EDP_PSR_TP2_TP3_TIME_0us           (3<<8)
>> +#define   EDP_PSR_TP1_TIME_500us             (0<<4)
>> +#define   EDP_PSR_TP1_TIME_100us             (1<<4)
>> +#define   EDP_PSR_TP1_TIME_2500us            (2<<4)
>> +#define   EDP_PSR_TP1_TIME_0us                       (3<<4)
>> +#define   EDP_PSR_IDLE_FRAME_SHIFT           0
>> +
>> +#define EDP_PSR_AUX_CTL                      0x64810
>> +#define EDP_PSR_AUX_DATA1            0x64814
>> +#define EDP_PSR_AUX_DATA2            0x64818
>> +#define EDP_PSR_AUX_DATA3            0x6481c
>> +#define EDP_PSR_AUX_DATA4            0x64820
>> +#define EDP_PSR_AUX_DATA5            0x64824
>> +
>> +#define EDP_PSR_STATUS_CTL                   0x64840
>> +#define   EDP_PSR_STATUS_STATE_MASK          (7<<29)
>> +
>> +#define EDP_PSR_DEBUG_CTL            0x64860
>> +#define   EDP_PSR_DEBUG_MASK_MEMUP   (1<<26)
>> +#define   EDP_PSR_DEBUG_MASK_HPD     (1<<25)
>> +
>>  /* VGA port control */
>>  #define ADPA                 0x61100
>>  #define PCH_ADPA                0xe1100
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 5cfa9f4..a420f0d 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -83,6 +83,13 @@ static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
>>       return intel_dig_port->base.base.dev;
>>  }
>>
>> +static struct drm_crtc *intel_dp_to_crtc(struct intel_dp *intel_dp)
>> +{
>> +     struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> +
>> +     return intel_dig_port->base.base.crtc;
>> +}
>> +
>>  static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
>>  {
>>       return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
>> @@ -1443,6 +1450,182 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
>>               (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED));
>>  }
>>
>> +static bool
>> +intel_edp_is_psr_enabled(struct intel_dp* intel_dp)
>> +{
>> +     struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +     return I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
>> +}
>> +
>> +static void
>> +intel_edp_psr_enable_src(struct intel_dp *intel_dp)
>> +{
>> +     struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +     uint32_t max_sleep_time = 0x1f;
>> +     uint32_t val = 0x0;
>> +
>> +     if (dev_priv->vbt.psr_idle_frames)
>> +         val |= dev_priv->vbt.psr_idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>> +     else
>> +         val |= 1 << EDP_PSR_IDLE_FRAME_SHIFT;
>> +
>> +     if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
>> +             val |= EDP_PSR_LINK_STANDBY;
>
> There were some WAs for link standby not working on all revisions.
> Do we care?

I think we shouldn't :)

>
>> +             val |= EDP_PSR_TP2_TP3_TIME_0us;
>> +             val |= EDP_PSR_TP1_TIME_0us;
>> +             val |= EDP_PSR_SKIP_AUX_EXIT;
>> +     } else {
>> +             val |= EDP_PSR_LINK_DISABLE;
>> +             /* Use these Values from VBT
>> +              * Case values are timings for HSW as of now
>> +              * in multiple of 100us
>> +              */
>> +             switch(dev_priv->vbt.psr_wakeup_tp1) {
>> +                     case 1:
>> +                             val |= EDP_PSR_TP1_TIME_100us;
>> +                             break;
>> +                     case 5:
>> +                             val |= EDP_PSR_TP1_TIME_500us;
>> +                             break;
>> +                     case 25:
>> +                             val |= EDP_PSR_TP1_TIME_2500us;
>> +                             break;
>> +                     default:
>> +                             val |= EDP_PSR_TP1_TIME_500us;
>> +                             break;
>> +             };
>> +             switch(dev_priv->vbt.psr_wakeup_tp2_tp3) {
>> +                     case 1:
>> +                             val |= EDP_PSR_TP2_TP3_TIME_100us;
>> +                             break;
>> +                     case 5:
>> +                             val |= EDP_PSR_TP2_TP3_TIME_500us;
>> +                             break;
>> +                     case 25:
>> +                             val |= EDP_PSR_TP2_TP3_TIME_2500us;
>> +                             break;
>> +                     default:
>> +                             val |= EDP_PSR_TP2_TP3_TIME_500us;
>> +                             break;
>> +             };
>> +     }
>> +
>> +     I915_WRITE(EDP_PSR_CTL, val |
>> +                EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES |
>> +                max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
>> +                EDP_PSR_ENABLE);
>> +}
>> +
>> +void intel_edp_write_vsc_psr(struct intel_dp* intel_dp,
>> +                          struct edp_vsc_psr *vsc_psr)
>> +{
>> +     struct drm_device *dev =  intel_dp_to_dev(intel_dp);
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +     struct intel_crtc *intel_crtc =  to_intel_crtc(intel_dp_to_crtc(intel_dp));
>> +     u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
>> +     u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(intel_crtc->cpu_transcoder);
>> +     uint32_t *data = (uint32_t *)vsc_psr;
>> +     unsigned int i;
>> +     u32 val = I915_READ(ctl_reg);
>> +
>> +     if (data_reg == 0)
>> +             return;
>> +
>> +     /* As per eDP spec, wait for vblank to send SDP VSC packet */
>> +     intel_wait_for_vblank(dev, intel_crtc->pipe);
>
> Is this really needed? Isn't the SDP stuff sent out during vblank by
> definition?

BSpec page "Pipe Video Data Island Packet" says we need to wait for 1
VSync to ensure completion of any pending video DIP transmissions. We
don't do this wait on HDMI code because it runs when the pipe is still
off. Here the pipe is already running. On the other hand, I think we
should disable the DIP on the time window there the frame is not being
sent (during the vsync), but our intel_wait_for_vblank doesn't do
exactly this.

>
>> +
>> +     mmiowb();
>> +     for (i = 0; i < sizeof(struct edp_vsc_psr); i += 4) {
>> +             I915_WRITE(data_reg + i, *data);
>> +             data++;
>> +     }
>> +     /* Write every possible data byte to force correct ECC calculation. */
>> +     for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
>> +             I915_WRITE(data_reg + i, 0);
>> +     mmiowb();
>> +
>> +     val |= VIDEO_DIP_ENABLE_VSC_HSW;
>> +     I915_WRITE(ctl_reg, val);
>> +     POSTING_READ(ctl_reg);
>> +}
>> +
>> +void intel_edp_enable_psr(struct intel_dp* intel_dp)
>> +{
>> +     struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +     struct edp_vsc_psr psr_vsc;
>> +     int msg_size = 5;       /* Header(4) + Message(1) */
>> +
>> +     if (!is_edp_psr(intel_dp) || intel_edp_is_psr_enabled(intel_dp))
>> +             return;
>> +
>> +     /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
>> +     memset(&psr_vsc, 0, sizeof(psr_vsc));
>> +     psr_vsc.sdp_header.id = 0;
>> +     psr_vsc.sdp_header.type = 0x7;
>> +     psr_vsc.sdp_header.revision = 0x2;
>> +     psr_vsc.sdp_header.valid_payload_bytes = 0x8;
>> +
>> +     intel_edp_write_vsc_psr(intel_dp, &psr_vsc);
>> +
>> +     /* Enable PSR in sink by setting bit 0 in DPCD config reg
>> +      * along with the transmitter state during PSR active
>> +      * Transmitter state later can be ready from VBT. As of now
>> +      * program the full link down
>> +      *
>> +      */
>> +     intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
>> +                                 DP_PSR_ENABLE);
>> +
>> +     /* Setup AUX registers */
>> +     /* Write command on DPCD 0x0600 */
>> +     I915_WRITE(EDP_PSR_AUX_DATA1, 0x80060000);
>> +
>> +     /* Set the state to normal operation D0 in DPCD 0x0600 */
>> +     I915_WRITE(EDP_PSR_AUX_DATA2, 0x01000000);
>> +
>> +     I915_WRITE(EDP_PSR_AUX_CTL,
>> +                msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT |
>> +                I915_READ(DPA_AUX_CH_CTL));
>
> Would it be possible to make this stuff a bit more like the normal
> aux stuff? Maybe even share some code to set up the data? But then
> again it's just these few writes here, so I'm not sure it's worth
> the effort.
>
>> +
>> +     /* Enable PSR on the host */
>> +     intel_edp_psr_enable_src(intel_dp);
>> +
>> +     /* WA: FBC/PSR Underrun/Corruption Workaround */
>> +     I915_WRITE(HSW_FBC_CONTROL, ~FBC_CTL_EN);
>
> Are we expecting that BIOS left FBC enabled? Shouldn't we then disable
> it during resume?
>
>> +
>> +     /* WA: Continuous PSR exit Workaround */
>> +     I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP);
>
> Shouldn't this be done before enabling PSR?
>
>> +     I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
>
> I can't find anything about this in bspec or WA db.
>
>> +}
>> +
>> +void intel_edp_disable_psr(struct intel_dp* intel_dp)
>> +{
>> +     struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>> +     struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
>> +     uint32_t reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
>> +     uint32_t val;
>> +
>> +     if (!intel_edp_is_psr_enabled(intel_dp))
>> +             return;
>> +
>> +     val = I915_READ(EDP_PSR_CTL);
>> +     I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
>> +
>> +     /* Wait till PSR is idle */
>> +     if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
>> +                    EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
>> +             DRM_ERROR("Timed out waiting for PSR Idle State\n");
>> +
>> +     intel_wait_for_vblank(dev, intel_crtc->pipe);
>> +
>> +     val = I915_READ(reg);
>> +     I915_WRITE(reg, val & ~VIDEO_DIP_ENABLE_VSC_HSW);
>> +}
>> +
>>  static void intel_enable_dp(struct intel_encoder *encoder)
>>  {
>>       struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 865ede6..cfda739 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -696,4 +696,7 @@ extern bool
>>  intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
>>  extern void intel_ddi_fdi_disable(struct drm_crtc *crtc);
>>
>> +extern void intel_edp_enable_psr(struct intel_dp* intel_dp);
>> +extern void intel_edp_disable_psr(struct intel_dp* intel_dp);
>> +
>>  #endif /* __INTEL_DRV_H__ */
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 7/8] drm/i915: Added debugfs support for PSR Status
  2013-02-25 22:55 ` [PATCH 7/8] drm/i915: Added debugfs support for PSR Status Rodrigo Vivi
@ 2013-02-28 17:44   ` Paulo Zanoni
  0 siblings, 0 replies; 26+ messages in thread
From: Paulo Zanoni @ 2013-02-28 17:44 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

Hi

2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> Adding support for PSR Status, PSR entry counter and performance counters.
> Heavily based on initial work from Shobhit.
>
> Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 92 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h     | 24 ++++++++++
>  2 files changed, 116 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7c65ab8..01021f6 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1680,6 +1680,97 @@ static int i915_dpio_info(struct seq_file *m, void *data)
>         return 0;
>  }
>
> +
> +static int i915_edp_psr_status(struct seq_file *m, void *data)
> +{
> +       struct drm_info_node *node = m->private;
> +       struct drm_device *dev = node->minor->dev;
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       unsigned int count;
> +       u32 psrctl, psrstat, psrperf;
> +
> +       psrctl = I915_READ(EDP_PSR_CTL);
> +       seq_printf(m, "PSR Enabled: %s\n",
> +                  yesno(psrctl & EDP_PSR_ENABLE));
> +
> +       psrstat = I915_READ(EDP_PSR_STATUS_CTL);
> +
> +       seq_printf(m, "PSR Current State: ");
> +       switch (psrstat & EDP_PSR_STATUS_STATE_MASK) {
> +       case EDP_PSR_STATUS_STATE_IDLE:
> +               seq_printf(m, "Reset state\n");
> +               break;
> +       case EDP_PSR_STATUS_STATE_SRDONACK:
> +               seq_printf(m, "Wait for TG/Stream to send on frame of data after SRD conditions are met\n");
> +               break;
> +       case EDP_PSR_STATUS_STATE_SRDENT:
> +               seq_printf(m, "SRD entry\n");
> +               break;
> +       case EDP_PSR_STATUS_STATE_BUFOFF:
> +               seq_printf(m, "Wait for buffer turn off\n");
> +               break;
> +       case EDP_PSR_STATUS_STATE_BUFON:
> +                       seq_printf(m, "Wait for buffer turn on\n");
> +               break;
> +       case EDP_PSR_STATUS_STATE_AUXACK:
> +                       seq_printf(m, "Wait for AUX to acknowledge on SRD exit\n");
> +               break;
> +       case EDP_PSR_STATUS_STATE_SRDOFFACK:
> +                       seq_printf(m, "Wait for TG/Stream to acknowledge the SRD VDM exit\n");
> +               break;
> +       default:
> +               seq_printf(m, "Unknown\n");
> +               break;
> +       }
> +
> +       seq_printf(m, "Link Status: ");
> +       switch (psrstat & EDP_PSR_STATUS_LINK_MASK) {
> +       case EDP_PSR_STATUS_LINK_FULL_OFF:
> +               seq_printf(m, "Link is fully off\n");
> +               break;
> +       case EDP_PSR_STATUS_LINK_FULL_ON:
> +               seq_printf(m, "Link is fully on\n");
> +               break;
> +       case EDP_PSR_STATUS_LINK_STANDBY:
> +               seq_printf(m, "Link is in standby\n");
> +               break;
> +       default:
> +               seq_printf(m, "Unknown\n");
> +               break;
> +       }
> +
> +       seq_printf(m, "PSR Entry Count: %u\n",
> +                  psrstat >> EDP_PSR_STATUS_COUNT_SHIFT &
> +                  EDP_PSR_STATUS_COUNT_MASK);
> +
> +       seq_printf(m, "Max Sleep Timer Counter: %u\n",
> +                  psrstat >> EDP_PSR_STATUS_MAX_SLEEP_TIMER_SHIFT &
> +                  EDP_PSR_STATUS_MAX_SLEEP_TIMER_MASK);
> +
> +       seq_printf(m, "Had AUX error: %s\n",
> +                  yesno(psrstat & EDP_PSR_STATUS_AUX_ERROR));
> +
> +       seq_printf(m, "Sending AUX: %s\n",
> +                  yesno(psrstat & EDP_PSR_STATUS_AUX_SENDING));
> +
> +       seq_printf(m, "Sending Idle: %s\n",
> +                  yesno(psrstat & EDP_PSR_STATUS_SENDING_IDLE));
> +
> +       seq_printf(m, "Sending TP2 TP3: %s\n",
> +                  yesno(psrstat & EDP_PSR_STATUS_SENDING_TP2_TP3));
> +
> +       seq_printf(m, "Sending TP1: %s\n",
> +                  yesno(psrstat & EDP_PSR_STATUS_SENDING_TP1));
> +
> +       seq_printf(m, "Idle Count: %u\n",
> +                  psrstat & EDP_PSR_STATUS_IDLE_MASK);
> +
> +       psrperf = (I915_READ(EDP_PSR_PERF_CNT)) & EDP_PSR_PERF_CNT_MASK;
> +       seq_printf(m, "Performance Counter: %u\n", psrperf);
> +
> +       return 0;
> +}
> +
>  static ssize_t
>  i915_wedged_read(struct file *filp,
>                  char __user *ubuf,
> @@ -2249,6 +2340,7 @@ static struct drm_info_list i915_debugfs_list[] = {
>         {"i915_swizzle_info", i915_swizzle_info, 0},
>         {"i915_ppgtt_info", i915_ppgtt_info, 0},
>         {"i915_dpio", i915_dpio_info, 0},
> +       {"i915_edp_psr_status", i915_edp_psr_status, 0},
>  };
>  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1e31f23..1f7e666 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1615,6 +1615,30 @@
>
>  #define EDP_PSR_STATUS_CTL                     0x64840
>  #define   EDP_PSR_STATUS_STATE_MASK            (7<<29)
> +#define   EDP_PSR_STATUS_STATE_IDLE            (0<<29)
> +#define   EDP_PSR_STATUS_STATE_SRDONACK                (1<<29)
> +#define   EDP_PSR_STATUS_STATE_SRDENT          (2<<29)
> +#define   EDP_PSR_STATUS_STATE_BUFOFF          (3<<29)
> +#define   EDP_PSR_STATUS_STATE_BUFON           (4<<29)
> +#define   EDP_PSR_STATUS_STATE_AUXACK          (5<<29)
> +#define   EDP_PSR_STATUS_STATE_SRDOFFACK       (6<<29)
> +#define   EDP_PSR_STATUS_LINK_MASK             (3<<29)
> +#define   EDP_PSR_STATUS_LINK_FULL_OFF         (0<<29)
> +#define   EDP_PSR_STATUS_LINK_FULL_ON          (1<<29)
> +#define   EDP_PSR_STATUS_LINK_STANDBY          (2<<29)

The 4 lines above are wrong. It's (x<<26).

> +#define   EDP_PSR_STATUS_MAX_SLEEP_TIMER_SHIFT         20
> +#define   EDP_PSR_STATUS_MAX_SLEEP_TIMER_MASK  0x1f
> +#define   EDP_PSR_STATUS_COUNT_SHIFT           16
> +#define   EDP_PSR_STATUS_COUNT_MASK            0xf
> +#define   EDP_PSR_STATUS_AUX_ERROR             (1<<15)
> +#define   EDP_PSR_STATUS_AUX_SENDING           (1<<12)
> +#define   EDP_PSR_STATUS_SENDING_IDLE          (1<<9)
> +#define   EDP_PSR_STATUS_SENDING_TP2_TP3       (1<<8)
> +#define   EDP_PSR_STATUS_SENDING_TP1           (1<<4)
> +#define   EDP_PSR_STATUS_IDLE_MASK             0xf
> +
> +#define EDP_PSR_PERF_CNT               0x64844
> +#define   EDP_PSR_PERF_CNT_MASK                0xffffff
>
>  #define EDP_PSR_DEBUG_CTL              0x64860
>  #define   EDP_PSR_DEBUG_MASK_MEMUP     (1<<26)
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 8/8] drm/i915: Hook PSR functionality
  2013-02-25 22:55 ` [PATCH 8/8] drm/i915: Hook PSR functionality Rodrigo Vivi
@ 2013-02-28 17:47   ` Paulo Zanoni
  0 siblings, 0 replies; 26+ messages in thread
From: Paulo Zanoni @ 2013-02-28 17:47 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

Hi

2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> PSR must be enabled after transcoder and port are running.
> And it is only available for HSW.
>
> v2: move enable/disable to intel_ddi
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 816c45c..bbfdcfd 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1321,6 +1321,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
>                 struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>
>                 ironlake_edp_backlight_on(intel_dp);
> +               intel_edp_enable_psr(intel_dp);
>         }
>
>         if (intel_crtc->eld_vld) {
> @@ -1345,6 +1346,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
>                 struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>
>                 ironlake_edp_backlight_off(intel_dp);
> +               intel_edp_disable_psr(intel_dp);

The spec suggests PSR should be disabled even before backlight (it
also suggests audio should be disabled before backlight, and I notice
this seems to be wrong in our code).

>         }
>
>         tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 0/8] Enable eDP PSR functionality at HSW - v3
  2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
                   ` (7 preceding siblings ...)
  2013-02-25 22:55 ` [PATCH 8/8] drm/i915: Hook PSR functionality Rodrigo Vivi
@ 2013-02-28 17:52 ` Paulo Zanoni
  2013-02-28 18:02   ` Ville Syrjälä
  8 siblings, 1 reply; 26+ messages in thread
From: Paulo Zanoni @ 2013-02-28 17:52 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

Hi

2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> PSR is an eDP feature that allows power saving even with static image at eDP screen.
>
> v3: Accepted many suggestions that I received at v2 review, fixing, cleaning and improving the code.
>
> v2: Main differences in this v2:
> - Created vbt struct to get i915 dev_priv more organized and to avoid adding more stuff into it.
> - migrated hsw macros to use transcoder instead of pipes than I could address eDP
> - remove patch that was only adding edp psr registers and added them on demand
>
> v1:
> Shobit Kumar has implemented this patch series some time ago, but had no eDP panel with PSR capability to test them.
>
> I could test and verify that this series fully identify PSR capability and enables it at HSW.
> I also verified that it saves from 0.5-1W but only when in blank screen. It seems it is not really entering in sleeping mode with static image at eDP screen yet.

What do you mean with "blank screen"? It seems we disable PSR before
blanking the screen, so the 0.5-1W saving could be from the backlight.
Did you try masking more bits on the SRD_DEBUG register to see if it
enters PSR more easily? The first test I'd try would be to set 1 to
all those mask regs and see what happens (maybe we'll enter PSR and
never ever leave it again?).

>
> Please accept this series as the first part of PSR enabling while we continue working to improve its functionality.
>
> Rodrigo Vivi (5):
>   drm/i915: Organize VBT stuff inside drm_i915_private
>   drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
>   drm/i915: Enable/Disable PSR
>   drm/i915: Added debugfs support for PSR Status
>   drm/i915: Hook PSR functionality
>
> Shobhit Kumar (3):
>   drm/i915: Added SDP and VSC structures for handling PSR for eDP
>   drm/i915: Read the EDP DPCD and PSR Capability
>   drm/i915: VBT Parsing for the PSR Feature Block for HSW
>
>  drivers/gpu/drm/i915/i915_debugfs.c  |  92 ++++++++++++++++
>  drivers/gpu/drm/i915/i915_dma.c      |   8 +-
>  drivers/gpu/drm/i915/i915_drv.h      |  63 ++++++-----
>  drivers/gpu/drm/i915/i915_reg.h      |  82 ++++++++++++--
>  drivers/gpu/drm/i915/intel_bios.c    | 126 +++++++++++++--------
>  drivers/gpu/drm/i915/intel_bios.h    |  20 +++-
>  drivers/gpu/drm/i915/intel_crt.c     |   4 +-
>  drivers/gpu/drm/i915/intel_ddi.c     |   2 +
>  drivers/gpu/drm/i915/intel_display.c |  16 +--
>  drivers/gpu/drm/i915/intel_dp.c      | 208 ++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |   4 +
>  drivers/gpu/drm/i915/intel_hdmi.c    |  13 ++-
>  drivers/gpu/drm/i915/intel_lvds.c    |  20 ++--
>  drivers/gpu/drm/i915/intel_sdvo.c    |   6 +-
>  drivers/gpu/drm/i915/intel_tv.c      |   8 +-
>  include/drm/drm_dp_helper.h          |  22 ++++
>  16 files changed, 569 insertions(+), 125 deletions(-)
>
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 0/8] Enable eDP PSR functionality at HSW - v3
  2013-02-28 17:52 ` [Intel-gfx] [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Paulo Zanoni
@ 2013-02-28 18:02   ` Ville Syrjälä
  2013-03-04 23:27     ` Daniel Vetter
  0 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2013-02-28 18:02 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, dri-devel

On Thu, Feb 28, 2013 at 02:52:32PM -0300, Paulo Zanoni wrote:
> Hi
> 
> 2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> > PSR is an eDP feature that allows power saving even with static image at eDP screen.
> >
> > v3: Accepted many suggestions that I received at v2 review, fixing, cleaning and improving the code.
> >
> > v2: Main differences in this v2:
> > - Created vbt struct to get i915 dev_priv more organized and to avoid adding more stuff into it.
> > - migrated hsw macros to use transcoder instead of pipes than I could address eDP
> > - remove patch that was only adding edp psr registers and added them on demand
> >
> > v1:
> > Shobit Kumar has implemented this patch series some time ago, but had no eDP panel with PSR capability to test them.
> >
> > I could test and verify that this series fully identify PSR capability and enables it at HSW.
> > I also verified that it saves from 0.5-1W but only when in blank screen. It seems it is not really entering in sleeping mode with static image at eDP screen yet.
> 
> What do you mean with "blank screen"? It seems we disable PSR before
> blanking the screen, so the 0.5-1W saving could be from the backlight.
> Did you try masking more bits on the SRD_DEBUG register to see if it
> enters PSR more easily? The first test I'd try would be to set 1 to
> all those mask regs and see what happens (maybe we'll enter PSR and
> never ever leave it again?).

One thing I'm wondering if we can even enable PSR w/o implementing the
FBC tracking bits. I mean what happens if someone renders to the front
buffer while PSR is active?

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/8] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
  2013-02-27 19:44   ` [Intel-gfx] " Paulo Zanoni
@ 2013-03-03 17:26     ` Daniel Vetter
  0 siblings, 0 replies; 26+ messages in thread
From: Daniel Vetter @ 2013-03-03 17:26 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, dri-devel

On Wed, Feb 27, 2013 at 04:44:59PM -0300, Paulo Zanoni wrote:
> Hi
> 
> 2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> > While old platforms had 3 transcoders and 3 pipes (1:1), HSW has
> > 4 transcoders and 3 pipes.
> > These regs were being used only by HDMI code where pipe is always the same
> > thing as cpu_transcoder.
> > This patch allow us to use them for DP, specially for TRANSCODER_EDP.
> >
> > v2: Adding HSW_TVIDEO_DIP_VSC_DATA to transmit vsc to eDP.
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP
  2013-02-25 22:55 ` [PATCH 3/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP Rodrigo Vivi
  2013-02-27 21:52   ` [Intel-gfx] " Paulo Zanoni
@ 2013-03-03 17:28   ` Daniel Vetter
  1 sibling, 0 replies; 26+ messages in thread
From: Daniel Vetter @ 2013-03-03 17:28 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Sateesh Kavuri, dri-devel

On Mon, Feb 25, 2013 at 07:55:17PM -0300, Rodrigo Vivi wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> Signed-off-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> 
> v2: Modified and corrected the structures to be more in line for
> kernel coding guidelines and rebased the code on Paulo's DP patchset
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE
> v4: moving them to include/drm/drm_dp_helper.h and also already
>     icluding EDP_PSR_RECEIVER_CAP_SIZE to add everything needed
>     for PSR at once at drm_dp_helper.h
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

Bikeshed for your patch headlines: You still have drm/i915 there, despite
that the #defines and structures are now moved to the drm core dp helpers.
Non-Intel people might simply ignore the patches in that case.
-Daniel

> ---
>  include/drm/drm_dp_helper.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index e8e1417..06c5060 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -343,12 +343,34 @@ u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
>  					  int lane);
>  
>  #define DP_RECEIVER_CAP_SIZE	0xf
> +#define EDP_PSR_RECEIVER_CAP_SIZE      2
> +
>  void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
>  void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
>  
>  u8 drm_dp_link_rate_to_bw_code(int link_rate);
>  int drm_dp_bw_code_to_link_rate(u8 link_bw);
>  
> +/* SDP header as per eDP 1.3 spec, section 3.6 */
> +struct edp_sdp_header {
> +	u8 id;
> +	u8 type;
> +	u8 revision;
> +	u8 valid_payload_bytes;
> +} __attribute__((packed));
> +
> +/* SDP VSC header as per eDP 1.3 spec, section 3.6 */
> +struct edp_vsc_psr {
> +	struct edp_sdp_header sdp_header;
> +	u8 DB1; /* 0 - PSR State; 1 - Update RFB; 2 - CRC Valid*/
> +	u8 DB2; /* CRC value bits 7:0 of the R or Cr component */
> +	u8 DB3; /* CRC value bits 15:8 of the R or Cr component */
> +	u8 DB4; /* CRC value bits 7:0 of the G or Y component */
> +	u8 DB5; /* CRC value bits 15:8 of the G or Y component */
> +	u8 DB6; /* CRC value bits 7:0 of the B or Cb component */
> +	u8 DB7; /* CRC value bits 15:8 of the B or Cb component */
> +} __attribute__((packed));
> +
>  static inline int
>  drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  {
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/8] Enable eDP PSR functionality at HSW - v3
  2013-02-28 18:02   ` Ville Syrjälä
@ 2013-03-04 23:27     ` Daniel Vetter
  2013-03-05  0:39       ` [Intel-gfx] " Rodrigo Vivi
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel Vetter @ 2013-03-04 23:27 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

On Thu, Feb 28, 2013 at 08:02:18PM +0200, Ville Syrjälä wrote:
> On Thu, Feb 28, 2013 at 02:52:32PM -0300, Paulo Zanoni wrote:
> > Hi
> > 
> > 2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> > > PSR is an eDP feature that allows power saving even with static image at eDP screen.
> > >
> > > v3: Accepted many suggestions that I received at v2 review, fixing, cleaning and improving the code.
> > >
> > > v2: Main differences in this v2:
> > > - Created vbt struct to get i915 dev_priv more organized and to avoid adding more stuff into it.
> > > - migrated hsw macros to use transcoder instead of pipes than I could address eDP
> > > - remove patch that was only adding edp psr registers and added them on demand
> > >
> > > v1:
> > > Shobit Kumar has implemented this patch series some time ago, but had no eDP panel with PSR capability to test them.
> > >
> > > I could test and verify that this series fully identify PSR capability and enables it at HSW.
> > > I also verified that it saves from 0.5-1W but only when in blank screen. It seems it is not really entering in sleeping mode with static image at eDP screen yet.
> > 
> > What do you mean with "blank screen"? It seems we disable PSR before
> > blanking the screen, so the 0.5-1W saving could be from the backlight.
> > Did you try masking more bits on the SRD_DEBUG register to see if it
> > enters PSR more easily? The first test I'd try would be to set 1 to
> > all those mask regs and see what happens (maybe we'll enter PSR and
> > never ever leave it again?).
> 
> One thing I'm wondering if we can even enable PSR w/o implementing the
> FBC tracking bits. I mean what happens if someone renders to the front
> buffer while PSR is active?

This is actually my main concern with PSR enabling - our current FBC code
is broken, and historically the hw is not one iota better :( Worst case we
need to manually detect frontbuffer rendering and disable PSR ...

Imo this needs to be resolved before we can enable PSR by default
anywhere.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 0/8] Enable eDP PSR functionality at HSW - v3
  2013-03-04 23:27     ` Daniel Vetter
@ 2013-03-05  0:39       ` Rodrigo Vivi
  0 siblings, 0 replies; 26+ messages in thread
From: Rodrigo Vivi @ 2013-03-05  0:39 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel

Yeah, I completely agree with you. This is the reason of that
separated 2 lines patch 8/8.

On Mon, Mar 4, 2013 at 8:27 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Feb 28, 2013 at 08:02:18PM +0200, Ville Syrjälä wrote:
>> On Thu, Feb 28, 2013 at 02:52:32PM -0300, Paulo Zanoni wrote:
>> > Hi
>> >
>> > 2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
>> > > PSR is an eDP feature that allows power saving even with static image at eDP screen.
>> > >
>> > > v3: Accepted many suggestions that I received at v2 review, fixing, cleaning and improving the code.
>> > >
>> > > v2: Main differences in this v2:
>> > > - Created vbt struct to get i915 dev_priv more organized and to avoid adding more stuff into it.
>> > > - migrated hsw macros to use transcoder instead of pipes than I could address eDP
>> > > - remove patch that was only adding edp psr registers and added them on demand
>> > >
>> > > v1:
>> > > Shobit Kumar has implemented this patch series some time ago, but had no eDP panel with PSR capability to test them.
>> > >
>> > > I could test and verify that this series fully identify PSR capability and enables it at HSW.
>> > > I also verified that it saves from 0.5-1W but only when in blank screen. It seems it is not really entering in sleeping mode with static image at eDP screen yet.
>> >
>> > What do you mean with "blank screen"? It seems we disable PSR before
>> > blanking the screen, so the 0.5-1W saving could be from the backlight.
>> > Did you try masking more bits on the SRD_DEBUG register to see if it
>> > enters PSR more easily? The first test I'd try would be to set 1 to
>> > all those mask regs and see what happens (maybe we'll enter PSR and
>> > never ever leave it again?).
>>
>> One thing I'm wondering if we can even enable PSR w/o implementing the
>> FBC tracking bits. I mean what happens if someone renders to the front
>> buffer while PSR is active?
>
> This is actually my main concern with PSR enabling - our current FBC code
> is broken, and historically the hw is not one iota better :( Worst case we
> need to manually detect frontbuffer rendering and disable PSR ...
>
> Imo this needs to be resolved before we can enable PSR by default
> anywhere.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 6/8] drm/i915: Enable/Disable PSR
  2013-02-26 12:48   ` Ville Syrjälä
@ 2013-03-07 16:54     ` Jesse Barnes
  0 siblings, 0 replies; 26+ messages in thread
From: Jesse Barnes @ 2013-03-07 16:54 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

On Tue, 26 Feb 2013 14:48:33 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Mon, Feb 25, 2013 at 07:55:20PM -0300, Rodrigo Vivi wrote:
> > Adding Enable and Disable PSR functionalities. This includes setting the
> > PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
> > enabling PSR in the sink via DPCD register and finally enabling PSR on
> > the host.
> > 
> > This patch is heavily based on initial PSR code by Sateesh Kavuri and
> > Kumar Shobhit but in a different implementation.
> > 
> > Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> > Credits-by: Shobhit Kumar <shobhit.kumar@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |  40 +++++++++
> >  drivers/gpu/drm/i915/intel_dp.c  | 183 +++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h |   3 +
> >  3 files changed, 226 insertions(+)
> > 
> <snip>
> > +void intel_edp_write_vsc_psr(struct intel_dp* intel_dp,
> > +			     struct edp_vsc_psr *vsc_psr)
> > +{
> > +	struct drm_device *dev =  intel_dp_to_dev(intel_dp);
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct intel_crtc *intel_crtc =  to_intel_crtc(intel_dp_to_crtc(intel_dp));
> > +	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> > +	u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(intel_crtc->cpu_transcoder);
> > +	uint32_t *data = (uint32_t *)vsc_psr;
> > +	unsigned int i;
> > +	u32 val = I915_READ(ctl_reg);
> > +
> > +	if (data_reg == 0)
> > +		return;
> > +
> > +	/* As per eDP spec, wait for vblank to send SDP VSC packet */
> > +	intel_wait_for_vblank(dev, intel_crtc->pipe);
> > +
> > +	mmiowb();
> 
> I was curious about these mmiowb()s and apparently they were added to
> all infoframe writes "just in case". But AFAICS on x86 mmiowb() ends
> up as a compiler barrier, so this stuff seems to be a nop.
> 
> And if these writes can get reordered somewhere, why not everything
> else too? I'm sure we have places where we write a bunch of registers,
> and the final write enables something which requires the earlier
> writes to have landed beforehand.

Yeah we shouldn't need mmiowb() anywhere in our driver (until our
on-chip topology gets really complicated anyway).

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2013-03-07 16:53 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-25 22:55 [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Rodrigo Vivi
2013-02-25 22:55 ` [PATCH 1/8] drm/i915: Organize VBT stuff inside drm_i915_private Rodrigo Vivi
2013-02-25 22:55 ` [PATCH 2/8] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe Rodrigo Vivi
2013-02-27 19:44   ` [Intel-gfx] " Paulo Zanoni
2013-03-03 17:26     ` Daniel Vetter
2013-02-25 22:55 ` [PATCH 3/8] drm/i915: Added SDP and VSC structures for handling PSR for eDP Rodrigo Vivi
2013-02-27 21:52   ` [Intel-gfx] " Paulo Zanoni
2013-03-03 17:28   ` Daniel Vetter
2013-02-25 22:55 ` [PATCH 4/8] drm/i915: Read the EDP DPCD and PSR Capability Rodrigo Vivi
2013-02-26 15:02   ` Ville Syrjälä
2013-02-25 22:55 ` [PATCH 5/8] drm/i915: VBT Parsing for the PSR Feature Block for HSW Rodrigo Vivi
2013-02-25 22:55 ` [PATCH 6/8] drm/i915: Enable/Disable PSR Rodrigo Vivi
2013-02-26 12:48   ` Ville Syrjälä
2013-03-07 16:54     ` Jesse Barnes
2013-02-26 13:27   ` Jani Nikula
2013-02-26 15:02   ` Ville Syrjälä
2013-02-28 17:30     ` [Intel-gfx] " Paulo Zanoni
2013-02-28 17:21   ` Paulo Zanoni
2013-02-25 22:55 ` [PATCH 7/8] drm/i915: Added debugfs support for PSR Status Rodrigo Vivi
2013-02-28 17:44   ` [Intel-gfx] " Paulo Zanoni
2013-02-25 22:55 ` [PATCH 8/8] drm/i915: Hook PSR functionality Rodrigo Vivi
2013-02-28 17:47   ` [Intel-gfx] " Paulo Zanoni
2013-02-28 17:52 ` [Intel-gfx] [PATCH 0/8] Enable eDP PSR functionality at HSW - v3 Paulo Zanoni
2013-02-28 18:02   ` Ville Syrjälä
2013-03-04 23:27     ` Daniel Vetter
2013-03-05  0:39       ` [Intel-gfx] " Rodrigo Vivi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).