All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] Enable eDP PSR functionality at HSW - v2
@ 2013-01-30 18:24 Rodrigo Vivi
  2013-01-30 18:24 ` [PATCH 1/9] drm/i915: Organize VBT stuff inside drm_i915_private Rodrigo Vivi
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 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.

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 (3):
  drm/i915: Organize VBT stuff inside drm_i915_private
  drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
  drm/i915: Hook PSR functionality

Shobhit Kumar (6):
  drm/i915: Added SDP and VSC structures for handling PSR for eDP
  drm/i915: Read the EDP DPCD and PSR Capability
  drm/i915: Setup EDP PSR AUX Registers
  drm/i915: VBT Parsing for the PSR Feature Block for HSW
  drm/i915: Enable/Disable PSR on HSW
  drm/i915: Added debugfs support for PSR Status

 drivers/gpu/drm/i915/i915_debugfs.c  |  34 +++++
 drivers/gpu/drm/i915/i915_dma.c      |   8 +-
 drivers/gpu/drm/i915/i915_drv.h      |  63 +++++---
 drivers/gpu/drm/i915/i915_reg.h      |  53 ++++++-
 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     |   8 +-
 drivers/gpu/drm/i915/intel_display.c |  16 +-
 drivers/gpu/drm/i915/intel_dp.c      | 285 +++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h     |   5 +
 drivers/gpu/drm/i915/intel_hdmi.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 +-
 include/drm/drm_dp_helper.h          |  29 ++++
 16 files changed, 551 insertions(+), 146 deletions(-)

-- 
1.7.11.7

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

* [PATCH 1/9] drm/i915: Organize VBT stuff inside drm_i915_private
  2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
@ 2013-01-30 18:24 ` Rodrigo Vivi
  2013-01-31  9:08   ` Jani Nikula
  2013-01-30 18:24 ` [PATCH 2/9] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe Rodrigo Vivi
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 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 an special structure for vbt stuff.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.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 3f70178..e43711f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1714,10 +1714,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 3189034..e22f6bd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -665,6 +665,36 @@ struct intel_l3_parity {
 	struct work_struct error_work;
 };
 
+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;
@@ -745,6 +775,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;
@@ -753,32 +784,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 */
@@ -928,8 +936,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 71a5eba..3f30095 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -433,7 +433,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) {
@@ -639,7 +639,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 b35902e..9e4db94 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3984,7 +3984,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);
 }
 
@@ -4055,7 +4055,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);
@@ -4156,7 +4156,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)) {
@@ -4768,7 +4768,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;
@@ -5049,8 +5049,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;
@@ -5399,7 +5399,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)
@@ -5512,7 +5512,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 1492706..cbd3236 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2609,11 +2609,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)
@@ -2677,7 +2677,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. */
@@ -2886,8 +2886,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 8c61876..d3be319 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -137,7 +137,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;
@@ -454,7 +454,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 ||
@@ -920,11 +920,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
@@ -1012,7 +1012,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;
 }
@@ -1069,7 +1069,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.7.11.7

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

* [PATCH 2/9] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
  2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
  2013-01-30 18:24 ` [PATCH 1/9] drm/i915: Organize VBT stuff inside drm_i915_private Rodrigo Vivi
@ 2013-01-30 18:24 ` Rodrigo Vivi
  2013-01-31  8:32   ` [Intel-gfx] " Jani Nikula
  2013-01-31 20:18   ` [Intel-gfx] " Paulo Zanoni
  2013-01-30 18:24 ` [PATCH 3/9] drm/i915: Added SDP and VSC structures for handling PSR for eDP Rodrigo Vivi
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 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. To avoid future mistakes transcoders must be used here instead of pipes even though it is working right now by coincidence.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2521617..7bb3134 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3742,14 +3742,14 @@
 #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 _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 d53b731..ab95e05 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -120,13 +120,13 @@ 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 +293,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);
 
@@ -566,7 +566,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.7.11.7

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

* [PATCH 3/9] drm/i915: Added SDP and VSC structures for handling PSR for eDP
  2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
  2013-01-30 18:24 ` [PATCH 1/9] drm/i915: Organize VBT stuff inside drm_i915_private Rodrigo Vivi
  2013-01-30 18:24 ` [PATCH 2/9] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe Rodrigo Vivi
@ 2013-01-30 18:24 ` Rodrigo Vivi
  2013-01-31  9:07   ` Jani Nikula
  2013-01-30 18:24 ` [PATCH 4/9] drm/i915: Read the EDP DPCD and PSR Capability Rodrigo Vivi
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shobhit Kumar, 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 | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index e8e1417..e9b7c4b 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -343,12 +343,41 @@ 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 : 5;   	    /* Bits 0:4 */
+	u8 rsvd1    : 3;   	    /* Bits 5:7 */
+	u8 valid_payload_bytes : 5; /* Bits 0:4 */
+	u8 rsvd2	       : 3; /* Bits 5:7 */
+} __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 unused;
+	u8 psr_state  : 1; 	/* Bit 0 */
+	u8 update_rfb : 1;	/* Bit 1 */
+	u8 valid_crc  : 1;	/* Bit 2 */
+	u8 reserved1  : 5;	/* Bits 3:7 */
+	u8 crc_r_lower;
+	u8 crc_r_higher;
+	u8 crc_g_lower;
+	u8 crc_g_higher;
+	u8 crc_b_lower;
+	u8 crc_b_higher;
+	u8 reserved2[24];
+} __attribute__((packed));
+
 static inline int
 drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
 {
-- 
1.7.11.7

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

* [PATCH 4/9] drm/i915: Read the EDP DPCD and PSR Capability
  2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2013-01-30 18:24 ` [PATCH 3/9] drm/i915: Added SDP and VSC structures for handling PSR for eDP Rodrigo Vivi
@ 2013-01-30 18:24 ` Rodrigo Vivi
  2013-01-31  9:12   ` [Intel-gfx] " Jani Nikula
  2013-01-30 18:24 ` [PATCH 5/9] drm/i915: Setup EDP PSR AUX Registers Rodrigo Vivi
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: 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>
---
 drivers/gpu/drm/i915/intel_dp.c  | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cbd3236..aeb0ef1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1435,6 +1435,11 @@ 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] & 0x1));
+}
+
 static void intel_enable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -2111,6 +2116,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, EDP_PSR_RECEIVER_CAP_SIZE);
+	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 aeff0d1..d4b3bac 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -371,6 +371,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.7.11.7

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

* [PATCH 5/9] drm/i915: Setup EDP PSR AUX Registers
  2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
                   ` (3 preceding siblings ...)
  2013-01-30 18:24 ` [PATCH 4/9] drm/i915: Read the EDP DPCD and PSR Capability Rodrigo Vivi
@ 2013-01-30 18:24 ` Rodrigo Vivi
  2013-01-31 10:17   ` Jani Nikula
  2013-01-31 21:35   ` Paulo Zanoni
  2013-01-30 18:24 ` [PATCH 6/9] drm/i915: VBT Parsing for the PSR Feature Block for HSW Rodrigo Vivi
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 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: Created aux_clock_divider function to avoid duplicated code.
    Unfortunatelly dp_aux_ch and psr_aux_ch aren't so similar to reuse full code.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 13 ++++++
 drivers/gpu/drm/i915/intel_dp.c  | 89 +++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 3 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7bb3134..10732dc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1563,6 +1563,19 @@
 #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
 #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
 
+#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_MASK		(7<<29)
+#define EDP_PSR_PERF_CNT		0x64844
+#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 aeb0ef1..fe83e05 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -364,6 +364,34 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	return status;
 }
 
+static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* The clock divider is based off the hrawclk,
+	 * and would like to run at 2MHz. So, take the
+	 * hrawclk value and divide by 2 and use that
+	 *
+	 * Note that PCH attached eDP panels should use a 125MHz input
+	 * clock divider.
+	 */
+	if (is_cpu_edp(intel_dp)) {
+		if (HAS_DDI(dev))
+			return intel_ddi_get_cdclk_freq(dev_priv) >> 1;
+		else if (IS_VALLEYVIEW(dev))
+			return 100;
+		else if (IS_GEN6(dev) || IS_GEN7(dev))
+			return 200; /* SNB & IVB eDP input clock at 400Mhz */
+		else
+			return 225; /* eDP input clock at 450Mhz */
+	} else if (HAS_PCH_SPLIT(dev))
+		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
+	else
+		return intel_hrawclk(dev) / 2;
+}
+
 static int
 intel_dp_aux_ch(struct intel_dp *intel_dp,
 		uint8_t *send, int send_bytes,
@@ -411,26 +439,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	}
 
 	intel_dp_check_edp(intel_dp);
-	/* The clock divider is based off the hrawclk,
-	 * and would like to run at 2MHz. So, take the
-	 * hrawclk value and divide by 2 and use that
-	 *
-	 * Note that PCH attached eDP panels should use a 125MHz input
-	 * clock divider.
-	 */
-	if (is_cpu_edp(intel_dp)) {
-		if (HAS_DDI(dev))
-			aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1;
-		else if (IS_VALLEYVIEW(dev))
-			aux_clock_divider = 100;
-		else if (IS_GEN6(dev) || IS_GEN7(dev))
-			aux_clock_divider = 200; /* SNB & IVB eDP input clock at 400Mhz */
-		else
-			aux_clock_divider = 225; /* eDP input clock at 450Mhz */
-	} else if (HAS_PCH_SPLIT(dev))
-		aux_clock_divider = DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
-	else
-		aux_clock_divider = intel_hrawclk(dev) / 2;
+	aux_clock_divider = get_aux_clock_divider(intel_dp);
 
 	if (IS_GEN6(dev))
 		precharge = 3;
@@ -1440,6 +1449,46 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
 	return (is_edp(intel_dp) && (intel_dp->psr_dpcd[0] & 0x1));
 }
 
+static void intel_edp_psr_setup(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 aux_clock_divider;
+	int precharge = 0x3;
+	int msg_size = 5;	/* Header(4) + Message(1) */
+
+	/* No need to setup if already done as these setting are persistent
+	 * until power states are entered */
+	if (intel_dp->psr_setup)
+		return;
+
+	/* 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);
+
+	aux_clock_divider = get_aux_clock_divider(intel_dp);
+
+	I915_WRITE(EDP_PSR_AUX_CTL,
+		   DP_AUX_CH_CTL_TIME_OUT_400us |
+		   (msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
+		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
+		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
+
+	/* Setup the debug register */
+	I915_WRITE(EDP_PSR_DEBUG_CTL, I915_READ(EDP_PSR_DEBUG_CTL) |
+			EDP_PSR_DEBUG_MASK_MEMUP |
+			EDP_PSR_DEBUG_MASK_HPD);
+
+	/* This flag can be made to 0 from pm code so as to reinitialize the
+	 * AUX register in case of power states, returning from which will not
+	 * maintain the AUX register settings
+	 */
+	intel_dp->psr_setup = 1;
+}
+
 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 d4b3bac..3fa2dd0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -383,6 +383,7 @@ struct intel_dp {
 	int backlight_on_delay;
 	int backlight_off_delay;
 	struct delayed_work panel_vdd_work;
+	uint8_t psr_setup;
 	bool want_panel_vdd;
 	struct intel_connector *attached_connector;
 };
-- 
1.7.11.7

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

* [PATCH 6/9] drm/i915: VBT Parsing for the PSR Feature Block for HSW
  2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
                   ` (4 preceding siblings ...)
  2013-01-30 18:24 ` [PATCH 5/9] drm/i915: Setup EDP PSR AUX Registers Rodrigo Vivi
@ 2013-01-30 18:24 ` Rodrigo Vivi
  2013-01-30 18:24 ` [PATCH 7/9] drm/i915: Enable/Disable PSR on HSW Rodrigo Vivi
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 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 e22f6bd..b5f1842 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -689,6 +689,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.7.11.7

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

* [PATCH 7/9] drm/i915: Enable/Disable PSR on HSW
  2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
                   ` (5 preceding siblings ...)
  2013-01-30 18:24 ` [PATCH 6/9] drm/i915: VBT Parsing for the PSR Feature Block for HSW Rodrigo Vivi
@ 2013-01-30 18:24 ` Rodrigo Vivi
  2013-01-31 10:40   ` Jani Nikula
  2013-01-31 22:01   ` Paulo Zanoni
  2013-01-30 18:24 ` [PATCH 8/9] drm/i915: Added debugfs support for PSR Status Rodrigo Vivi
  2013-01-30 18:24 ` [PATCH 9/9] drm/i915: Hook PSR functionality Rodrigo Vivi
  8 siblings, 2 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shobhit Kumar, dri-devel

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

Added eDP PSR enable functionality. 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. PSR works only in LPSP mode, so put the PIPE_DDI in DDIA
always on

This patch is heavily based on initial PSR code by Sateesh Kavuri but is
quite different in implementation. Makes use of VBT parsed data and also
the code has been cleaned up.

Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>

v2: fix getting base.crtc from intel_dp and fix DDI_EDP_INPUT_A_ON entry
v3: Add eDP PSR registers here only when they are really used and use
    HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder)

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  24 ++++++
 drivers/gpu/drm/i915/intel_ddi.c |   6 +-
 drivers/gpu/drm/i915/intel_dp.c  | 172 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |   3 +
 4 files changed, 204 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 10732dc..6f87d5b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1563,6 +1563,30 @@
 #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
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 2e904a5..feb7a4a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -985,9 +985,13 @@ void intel_ddi_enable_pipe_func(struct drm_crtc *crtc)
 		temp |= TRANS_DDI_PHSYNC;
 
 	if (cpu_transcoder == TRANSCODER_EDP) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 		switch (pipe) {
 		case PIPE_A:
-			temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
+			if (intel_dp->psr_dpcd[0] & 0x1)
+				temp |= TRANS_DDI_EDP_INPUT_A_ON;
+			else
+				temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
 			break;
 		case PIPE_B:
 			temp |= TRANS_DDI_EDP_INPUT_B_ONOFF;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fe83e05..d136890 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);
@@ -1489,6 +1496,171 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
 	intel_dp->psr_setup = 1;
 }
 
+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) & (1<<31)) ? true : false;
+}
+
+
+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;
+
+	/* Use VBT values which are parsed in
+	 * dev_priv->idle_frames,
+	 * but the BIOS initializes this to zero today
+	 * so hardcode
+	 */
+	uint32_t idle_frames = 6;
+
+	if (intel_dp->psr_dpcd[1] & 0x1) {
+		/* No link training on PSR Exit required */
+		val |= EDP_PSR_TP2_TP3_TIME_0us;
+		val |= EDP_PSR_TP1_TIME_0us;
+		val |= EDP_PSR_SKIP_AUX_EXIT;
+	} else {
+		/* 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;
+		};
+	}
+
+	/* Disable main link. Anyway in HSW steppings today
+	 * link standby does not work
+	 *
+	 * Later used VBT info (already parsed and available)
+	 * while supporting standby we need to program
+	 * val |= EDP_PSR_MIN_LINK_ENTRY_TIME_X_LINES based on VBT
+	 */
+	val = (val & ~EDP_PSR_LINK_STANDBY) |
+		(max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT) |
+		(idle_frames << EDP_PSR_IDLE_FRAME_SHIFT) |
+		EDP_PSR_ENABLE;
+
+	I915_WRITE(EDP_PSR_CTL, val);
+}
+
+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 intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
+	struct edp_vsc_psr psr_vsc;
+	uint32_t reg = HSW_TVIDEO_DIP_AVI_DATA(intel_crtc->cpu_transcoder);
+	uint32_t *vsc_data = (uint32_t *) &psr_vsc;
+	int i = 0, vsc_len = sizeof(struct edp_vsc_psr);
+
+	if (!is_edp_psr(intel_dp))
+		return;
+
+	/* setup AUX registers in case returned from pm states */
+	intel_edp_psr_setup(intel_dp);
+
+	/* Check if PSR is already enabled */
+	if (!intel_edp_is_psr_enabled(intel_dp)) {
+		/* 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;
+
+		/* As per eDP spec, wait for vblank to send SDP VSC packet */
+		intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+		/* Load the VSC DIP packet */
+		for(i = 0; i < vsc_len; i += 4)
+			I915_WRITE((reg + i), vsc_data[i]);
+
+#if 0
+		/* TBD:
+		 * We might not have to do explicitely as hardware will take care of this */
+		/* Enable the DIP register */
+		val = I915_READ(VIDEO_DIP_CTL_EDP);
+		I915_WRITE(VIDEO_DIP_CTL_EDP, val | VIDEOP_DIP_VSC);
+#endif
+		/* 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 &
+						~DP_PSR_MAIN_LINK_ACTIVE);
+
+		/* Enable PSR on the host */
+		intel_edp_psr_enable_src(intel_dp);
+	}
+}
+
+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 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_MASK) == 0, 2000, 10))
+		DRM_ERROR("Timed out waiting for PSR Idle State\n");
+
+	intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+#if 0
+	/* TBD:
+	 * Following is not yet confirmed from H/W team.
+	 * As per last discussion we do not need to disable
+	 * VSC DIP explicitely. Just maintaining the code in
+	 * case we have to do this later at some point
+	 */
+
+	/* Disable VSC DIP */
+	val = I915_READ(VIDEO_DIP_CTL_EDP);
+	I915_WRITE(VIDEO_DIP_CTL_EDP, val & ~VIDEOP_DIP_VSC);
+#endif
+}
+
 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 3fa2dd0..2f48e5c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -692,4 +692,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.7.11.7

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

* [PATCH 8/9] drm/i915: Added debugfs support for PSR Status
  2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
                   ` (6 preceding siblings ...)
  2013-01-30 18:24 ` [PATCH 7/9] drm/i915: Enable/Disable PSR on HSW Rodrigo Vivi
@ 2013-01-30 18:24 ` Rodrigo Vivi
  2013-01-31 10:48   ` Jani Nikula
  2013-01-30 18:24 ` [PATCH 9/9] drm/i915: Hook PSR functionality Rodrigo Vivi
  8 siblings, 1 reply; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Shobhit Kumar, dri-devel

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

Added support for PSR entry counter and performance counters

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

v2: Add psr enabled yes/no info
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 90a6fc5..11d2896 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1659,6 +1659,39 @@ 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 = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int count;
+	u32 psrctl, psrstat, psrperf;
+
+	/* Bit 31 gives the PSR enabled */
+	psrctl = I915_READ(EDP_PSR_CTL);
+	seq_printf(m, "PSR Enabled: %s\n",
+		   yesno(psrctl & EDP_PSR_ENABLE));
+
+	/* Bits 19:16 gives the PSR entry count */
+	psrstat = I915_READ(EDP_PSR_STATUS_CTL);
+	count = ((psrstat >> 16) & 0xf);
+
+	/* Format the PSR Entry Count only for now.
+	 * TBD: Other status information
+	 */
+	seq_printf(m, "EDP_PSR_ENTRY_COUNT: %u\n", count);
+
+	/* Current PSR state */
+	count = ((psrstat >> 29) & 0x7);
+	seq_printf(m, "EDP_PSR_CURRENT_STATE: 0x%x\n", count);
+
+	/* Perfromance counter bit 23:0 */
+	psrperf = (I915_READ(EDP_PSR_PERF_CNT)) & 0xffffff;
+	seq_printf(m, "EDP_PSR_PERF_COUNTER: %u\n", psrperf);
+
+	return 0;
+}
+
 static ssize_t
 i915_wedged_read(struct file *filp,
 		 char __user *ubuf,
@@ -2228,6 +2261,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)
 
-- 
1.7.11.7

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

* [PATCH 9/9] drm/i915: Hook PSR functionality
  2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
                   ` (7 preceding siblings ...)
  2013-01-30 18:24 ` [PATCH 8/9] drm/i915: Added debugfs support for PSR Status Rodrigo Vivi
@ 2013-01-30 18:24 ` Rodrigo Vivi
  2013-01-31 10:50   ` [Intel-gfx] " Jani Nikula
  8 siblings, 1 reply; 22+ messages in thread
From: Rodrigo Vivi @ 2013-01-30 18:24 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 feb7a4a..1dba035d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1306,6 +1306,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);
 	}
 }
 
@@ -1318,6 +1319,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);
 	}
 }
 
-- 
1.7.11.7

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

* Re: [Intel-gfx] [PATCH 2/9] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
  2013-01-30 18:24 ` [PATCH 2/9] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe Rodrigo Vivi
@ 2013-01-31  8:32   ` Jani Nikula
  2013-01-31 20:26     ` Paulo Zanoni
  2013-01-31 20:18   ` [Intel-gfx] " Paulo Zanoni
  1 sibling, 1 reply; 22+ messages in thread
From: Jani Nikula @ 2013-01-31  8:32 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> While old platforms had 3 transcoders and 3 pipes (1:1), HSW has 4 transcoders and 3 pipes. To avoid future mistakes transcoders must be used here instead of pipes even though it is working right now by coincidence.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h   | 16 ++++++++--------
>  drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++------
>  2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2521617..7bb3134 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3742,14 +3742,14 @@
>  #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)

Okay, I'm confused. The patch does make sense, but bspec doesn't... the
spec says the above HSW_TVIDEO_DIP_* registers (in the 0x6xxxx range)
are per pipe, and the TVIDEO_DIP_* registers (in the 0xexxxx range) are
per transcoder. Care to enlighten me here...?

Also, outside of this patch, have we reviewed if there's always a
register for TRANSCODER_EDP where _TRANSCODER() macro is used?

BR,
Jani.


>  
>  #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 d53b731..ab95e05 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -120,13 +120,13 @@ 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 +293,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);
>  
> @@ -566,7 +566,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.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/9] drm/i915: Added SDP and VSC structures for handling PSR for eDP
  2013-01-30 18:24 ` [PATCH 3/9] drm/i915: Added SDP and VSC structures for handling PSR for eDP Rodrigo Vivi
@ 2013-01-31  9:07   ` Jani Nikula
  0 siblings, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2013-01-31  9:07 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> 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>
> ---
>  include/drm/drm_dp_helper.h | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index e8e1417..e9b7c4b 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -343,12 +343,41 @@ 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 */

Section 3.5.

> +struct edp_sdp_header {
> +	u8 id;
> +	u8 type;
> +	u8 revision : 5;   	    /* Bits 0:4 */
> +	u8 rsvd1    : 3;   	    /* Bits 5:7 */
> +	u8 valid_payload_bytes : 5; /* Bits 0:4 */
> +	u8 rsvd2	       : 3; /* Bits 5:7 */
> +} __attribute__((packed));
> +
> +/* SDP VSC header as per eDP 1.3 spec, section 3.6 */

Section 3.5.

I really wouldn't mind SDP and VSC being spelled out for the casual
reader without eDP spec at hand.

> +struct edp_vsc_psr {
> +	struct edp_sdp_header sdp_header;
> +	u8 unused;
> +	u8 psr_state  : 1; 	/* Bit 0 */
> +	u8 update_rfb : 1;	/* Bit 1 */
> +	u8 valid_crc  : 1;	/* Bit 2 */
> +	u8 reserved1  : 5;	/* Bits 3:7 */

I don't think the bitfields here are portable for representing the
data. A bit of googling suggests GCC lays out the bits as you document
above on little-endian machines, but the other way around on big-endian
machines. That would be fine within i915 which assumes little-endian for
obvious reasons, but not in common drm code.

I think you'll just have to use u8 and #defines for the bits and masks.


BR,
Jani.

> +	u8 crc_r_lower;
> +	u8 crc_r_higher;
> +	u8 crc_g_lower;
> +	u8 crc_g_higher;
> +	u8 crc_b_lower;
> +	u8 crc_b_higher;
> +	u8 reserved2[24];
> +} __attribute__((packed));
> +
>  static inline int
>  drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
>  {
> -- 
> 1.7.11.7
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/9] drm/i915: Organize VBT stuff inside drm_i915_private
  2013-01-30 18:24 ` [PATCH 1/9] drm/i915: Organize VBT stuff inside drm_i915_private Rodrigo Vivi
@ 2013-01-31  9:08   ` Jani Nikula
  0 siblings, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2013-01-31  9:08 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> 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 an special structure for vbt stuff.
>

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

> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.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 3f70178..e43711f 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1714,10 +1714,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 3189034..e22f6bd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -665,6 +665,36 @@ struct intel_l3_parity {
>  	struct work_struct error_work;
>  };
>  
> +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;
> @@ -745,6 +775,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;
> @@ -753,32 +784,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 */
> @@ -928,8 +936,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 71a5eba..3f30095 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -433,7 +433,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) {
> @@ -639,7 +639,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 b35902e..9e4db94 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3984,7 +3984,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);
>  }
>  
> @@ -4055,7 +4055,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);
> @@ -4156,7 +4156,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)) {
> @@ -4768,7 +4768,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;
> @@ -5049,8 +5049,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;
> @@ -5399,7 +5399,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)
> @@ -5512,7 +5512,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 1492706..cbd3236 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2609,11 +2609,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)
> @@ -2677,7 +2677,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. */
> @@ -2886,8 +2886,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 8c61876..d3be319 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -137,7 +137,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;
> @@ -454,7 +454,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 ||
> @@ -920,11 +920,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
> @@ -1012,7 +1012,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;
>  }
> @@ -1069,7 +1069,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.7.11.7
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH 4/9] drm/i915: Read the EDP DPCD and PSR Capability
  2013-01-30 18:24 ` [PATCH 4/9] drm/i915: Read the EDP DPCD and PSR Capability Rodrigo Vivi
@ 2013-01-31  9:12   ` Jani Nikula
  0 siblings, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2013-01-31  9:12 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> 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.
>

A couple of nitpicks below, but either way,

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


> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index cbd3236..aeb0ef1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1435,6 +1435,11 @@ 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] & 0x1));

s/0x1/DP_PSR_IS_SUPPORTED/

> +}
> +
>  static void intel_enable_dp(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> @@ -2111,6 +2116,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, EDP_PSR_RECEIVER_CAP_SIZE);

s/EDP_PSR_RECEIVER_CAP_SIZE/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 aeff0d1..d4b3bac 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -371,6 +371,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.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 5/9] drm/i915: Setup EDP PSR AUX Registers
  2013-01-30 18:24 ` [PATCH 5/9] drm/i915: Setup EDP PSR AUX Registers Rodrigo Vivi
@ 2013-01-31 10:17   ` Jani Nikula
  2013-01-31 21:35   ` Paulo Zanoni
  1 sibling, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2013-01-31 10:17 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: Shobhit Kumar, dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> v2: Created aux_clock_divider function to avoid duplicated code.

Bikeshed, that could be a prep patch, but I won't insist on it.

Other comments below.

>     Unfortunatelly dp_aux_ch and psr_aux_ch aren't so similar to reuse full code.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 13 ++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 89 +++++++++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  3 files changed, 83 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7bb3134..10732dc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1563,6 +1563,19 @@
>  #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
>  #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
>  
> +#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_MASK		(7<<29)
> +#define EDP_PSR_PERF_CNT		0x64844
> +#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 aeb0ef1..fe83e05 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -364,6 +364,34 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>  	return status;
>  }
>  
> +static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/* The clock divider is based off the hrawclk,
> +	 * and would like to run at 2MHz. So, take the
> +	 * hrawclk value and divide by 2 and use that
> +	 *
> +	 * Note that PCH attached eDP panels should use a 125MHz input
> +	 * clock divider.
> +	 */
> +	if (is_cpu_edp(intel_dp)) {
> +		if (HAS_DDI(dev))
> +			return intel_ddi_get_cdclk_freq(dev_priv) >> 1;
> +		else if (IS_VALLEYVIEW(dev))
> +			return 100;
> +		else if (IS_GEN6(dev) || IS_GEN7(dev))
> +			return 200; /* SNB & IVB eDP input clock at 400Mhz */
> +		else
> +			return 225; /* eDP input clock at 450Mhz */
> +	} else if (HAS_PCH_SPLIT(dev))
> +		return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> +	else
> +		return intel_hrawclk(dev) / 2;
> +}
> +
>  static int
>  intel_dp_aux_ch(struct intel_dp *intel_dp,
>  		uint8_t *send, int send_bytes,
> @@ -411,26 +439,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  	}
>  
>  	intel_dp_check_edp(intel_dp);
> -	/* The clock divider is based off the hrawclk,
> -	 * and would like to run at 2MHz. So, take the
> -	 * hrawclk value and divide by 2 and use that
> -	 *
> -	 * Note that PCH attached eDP panels should use a 125MHz input
> -	 * clock divider.
> -	 */
> -	if (is_cpu_edp(intel_dp)) {
> -		if (HAS_DDI(dev))
> -			aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1;
> -		else if (IS_VALLEYVIEW(dev))
> -			aux_clock_divider = 100;
> -		else if (IS_GEN6(dev) || IS_GEN7(dev))
> -			aux_clock_divider = 200; /* SNB & IVB eDP input clock at 400Mhz */
> -		else
> -			aux_clock_divider = 225; /* eDP input clock at 450Mhz */
> -	} else if (HAS_PCH_SPLIT(dev))
> -		aux_clock_divider = DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> -	else
> -		aux_clock_divider = intel_hrawclk(dev) / 2;
> +	aux_clock_divider = get_aux_clock_divider(intel_dp);
>  
>  	if (IS_GEN6(dev))
>  		precharge = 3;
> @@ -1440,6 +1449,46 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
>  	return (is_edp(intel_dp) && (intel_dp->psr_dpcd[0] & 0x1));
>  }
>  
> +static void intel_edp_psr_setup(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 aux_clock_divider;
> +	int precharge = 0x3;
> +	int msg_size = 5;	/* Header(4) + Message(1) */
> +
> +	/* No need to setup if already done as these setting are persistent
> +	 * until power states are entered */

I find "no need to setup" a not good enough reason to not do it every
time. However, the specs say you're not supposed to fiddle with the aux
regs while PSR is enabled, which *is* a good reason. So maybe update the
comment, so we'll know what we're doing if we decide to change this in
the future?

> +	if (intel_dp->psr_setup)
> +		return;
> +
> +	/* 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 really really don't like just slamming magic 32-bit values into
registers. Copy-pasting from intel_dp_aux_native_write() and
intel_dp_aux_ch(), you could do something like this here to clarify a
bit:

	uint16_t address = DP_SET_POWER;
	uint8_t msg[] = {
		AUX_NATIVE_WRITE << 4,
		address >> 8,
		address & 0xff,
		0, /* len - 1 */
		DP_SET_POWER_D0,
	};

	for (i = 0; i < sizeof(msg); i += 4)
		I915_WRITE(EDP_PSR_AUX_DATA1 + i,
			   pack_aux(msg + i, sizeof(msg) - i));

The above could be cleaned up too, maybe there's common bits to be found
with the regular aux write stuff etc. But IMHO with that it's already
much easier to figure out, without looking at the specs, what's going
on, and you can (almost?) ditch the comments too.

BR,
Jani.


> +
> +	aux_clock_divider = get_aux_clock_divider(intel_dp);
> +
> +	I915_WRITE(EDP_PSR_AUX_CTL,
> +		   DP_AUX_CH_CTL_TIME_OUT_400us |
> +		   (msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> +		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> +		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
> +
> +	/* Setup the debug register */
> +	I915_WRITE(EDP_PSR_DEBUG_CTL, I915_READ(EDP_PSR_DEBUG_CTL) |
> +			EDP_PSR_DEBUG_MASK_MEMUP |
> +			EDP_PSR_DEBUG_MASK_HPD);
> +
> +	/* This flag can be made to 0 from pm code so as to reinitialize the
> +	 * AUX register in case of power states, returning from which will not
> +	 * maintain the AUX register settings
> +	 */
> +	intel_dp->psr_setup = 1;
> +}
> +
>  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 d4b3bac..3fa2dd0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -383,6 +383,7 @@ struct intel_dp {
>  	int backlight_on_delay;
>  	int backlight_off_delay;
>  	struct delayed_work panel_vdd_work;
> +	uint8_t psr_setup;
>  	bool want_panel_vdd;
>  	struct intel_connector *attached_connector;
>  };
> -- 
> 1.7.11.7
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 7/9] drm/i915: Enable/Disable PSR on HSW
  2013-01-30 18:24 ` [PATCH 7/9] drm/i915: Enable/Disable PSR on HSW Rodrigo Vivi
@ 2013-01-31 10:40   ` Jani Nikula
  2013-01-31 22:01   ` Paulo Zanoni
  1 sibling, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2013-01-31 10:40 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: Shobhit Kumar, dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Added eDP PSR enable functionality. 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. PSR works only in LPSP mode, so put the PIPE_DDI in DDIA
> always on
>
> This patch is heavily based on initial PSR code by Sateesh Kavuri but is
> quite different in implementation. Makes use of VBT parsed data and also
> the code has been cleaned up.
>
> Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> v2: fix getting base.crtc from intel_dp and fix DDI_EDP_INPUT_A_ON entry
> v3: Add eDP PSR registers here only when they are really used and use
>     HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder)

Some nitpicks and bikeshedding below, but regardless,

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

>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  24 ++++++
>  drivers/gpu/drm/i915/intel_ddi.c |   6 +-
>  drivers/gpu/drm/i915/intel_dp.c  | 172 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |   3 +
>  4 files changed, 204 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 10732dc..6f87d5b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1563,6 +1563,30 @@
>  #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
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2e904a5..feb7a4a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -985,9 +985,13 @@ void intel_ddi_enable_pipe_func(struct drm_crtc *crtc)
>  		temp |= TRANS_DDI_PHSYNC;
>  
>  	if (cpu_transcoder == TRANSCODER_EDP) {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  		switch (pipe) {
>  		case PIPE_A:
> -			temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
> +			if (intel_dp->psr_dpcd[0] & 0x1)

s/0x1/DP_PSR_IS_SUPPORTED/

Ditto elsewhere below.

> +				temp |= TRANS_DDI_EDP_INPUT_A_ON;
> +			else
> +				temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
>  			break;
>  		case PIPE_B:
>  			temp |= TRANS_DDI_EDP_INPUT_B_ONOFF;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fe83e05..d136890 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);
> @@ -1489,6 +1496,171 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
>  	intel_dp->psr_setup = 1;
>  }
>  
> +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) & (1<<31)) ? true : false;

	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;
> +
> +	/* Use VBT values which are parsed in
> +	 * dev_priv->idle_frames,
> +	 * but the BIOS initializes this to zero today
> +	 * so hardcode
> +	 */
> +	uint32_t idle_frames = 6;
> +
> +	if (intel_dp->psr_dpcd[1] & 0x1) {
> +		/* No link training on PSR Exit required */
> +		val |= EDP_PSR_TP2_TP3_TIME_0us;
> +		val |= EDP_PSR_TP1_TIME_0us;
> +		val |= EDP_PSR_SKIP_AUX_EXIT;
> +	} else {
> +		/* 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;
> +		};
> +	}
> +
> +	/* Disable main link. Anyway in HSW steppings today
> +	 * link standby does not work
> +	 *
> +	 * Later used VBT info (already parsed and available)
> +	 * while supporting standby we need to program
> +	 * val |= EDP_PSR_MIN_LINK_ENTRY_TIME_X_LINES based on VBT
> +	 */
> +	val = (val & ~EDP_PSR_LINK_STANDBY) |
> +		(max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT) |
> +		(idle_frames << EDP_PSR_IDLE_FRAME_SHIFT) |
> +		EDP_PSR_ENABLE;
> +
> +	I915_WRITE(EDP_PSR_CTL, val);
> +}
> +
> +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 intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
> +	struct edp_vsc_psr psr_vsc;
> +	uint32_t reg = HSW_TVIDEO_DIP_AVI_DATA(intel_crtc->cpu_transcoder);
> +	uint32_t *vsc_data = (uint32_t *) &psr_vsc;
> +	int i = 0, vsc_len = sizeof(struct edp_vsc_psr);
> +
> +	if (!is_edp_psr(intel_dp))
> +		return;
> +
> +	/* setup AUX registers in case returned from pm states */
> +	intel_edp_psr_setup(intel_dp);
> +
> +	/* Check if PSR is already enabled */
> +	if (!intel_edp_is_psr_enabled(intel_dp)) {

I'd rather make this return early if PSR is enabled, and reduce indent
on the lines below.

> +		/* 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;
> +
> +		/* As per eDP spec, wait for vblank to send SDP VSC packet */
> +		intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> +		/* Load the VSC DIP packet */
> +		for(i = 0; i < vsc_len; i += 4)

Just use sizeof here too instead of a temp variable for vsc_len.

> +			I915_WRITE((reg + i), vsc_data[i]);

Unnecessary braces around reg + i.

I guess we can be sure sizeof(struct edp_vsc_psr) % 4 == 0 always.

> +
> +#if 0
> +		/* TBD:
> +		 * We might not have to do explicitely as hardware will take care of this */
> +		/* Enable the DIP register */
> +		val = I915_READ(VIDEO_DIP_CTL_EDP);
> +		I915_WRITE(VIDEO_DIP_CTL_EDP, val | VIDEOP_DIP_VSC);
> +#endif
> +		/* 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 &
> +						~DP_PSR_MAIN_LINK_ACTIVE);
> +
> +		/* Enable PSR on the host */
> +		intel_edp_psr_enable_src(intel_dp);
> +	}
> +}
> +
> +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 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));

Unnecessary braces.

> +
> +	/* Wait till PSR is idle */
> +	if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) & EDP_PSR_STATUS_MASK) == 0, 2000, 10))
> +		DRM_ERROR("Timed out waiting for PSR Idle State\n");
> +
> +	intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> +#if 0
> +	/* TBD:
> +	 * Following is not yet confirmed from H/W team.
> +	 * As per last discussion we do not need to disable
> +	 * VSC DIP explicitely. Just maintaining the code in
> +	 * case we have to do this later at some point
> +	 */
> +
> +	/* Disable VSC DIP */
> +	val = I915_READ(VIDEO_DIP_CTL_EDP);
> +	I915_WRITE(VIDEO_DIP_CTL_EDP, val & ~VIDEOP_DIP_VSC);
> +#endif
> +}
> +
>  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 3fa2dd0..2f48e5c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -692,4 +692,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.7.11.7
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 8/9] drm/i915: Added debugfs support for PSR Status
  2013-01-30 18:24 ` [PATCH 8/9] drm/i915: Added debugfs support for PSR Status Rodrigo Vivi
@ 2013-01-31 10:48   ` Jani Nikula
  0 siblings, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2013-01-31 10:48 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: Shobhit Kumar, dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Added support for PSR entry counter and performance counters
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> v2: Add psr enabled yes/no info

A bunch of nitpicks...

BR,
Jani.


> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 90a6fc5..11d2896 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1659,6 +1659,39 @@ 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 = (struct drm_info_node *) m->private;

Unnecessary cast.

> +	struct drm_device *dev = node->minor->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int count;
> +	u32 psrctl, psrstat, psrperf;
> +
> +	/* Bit 31 gives the PSR enabled */
> +	psrctl = I915_READ(EDP_PSR_CTL);
> +	seq_printf(m, "PSR Enabled: %s\n",
> +		   yesno(psrctl & EDP_PSR_ENABLE));
> +
> +	/* Bits 19:16 gives the PSR entry count */
> +	psrstat = I915_READ(EDP_PSR_STATUS_CTL);
> +	count = ((psrstat >> 16) & 0xf);

Could #define the shift and mask for all of these, and ditch the "bits
n:m" comments.

Unnecessary braces around the whole thing.

> +
> +	/* Format the PSR Entry Count only for now.
> +	 * TBD: Other status information
> +	 */
> +	seq_printf(m, "EDP_PSR_ENTRY_COUNT: %u\n", count);

count should be unsigned for %u.

> +
> +	/* Current PSR state */
> +	count = ((psrstat >> 29) & 0x7);

Unnecessary braces.

> +	seq_printf(m, "EDP_PSR_CURRENT_STATE: 0x%x\n", count);
> +
> +	/* Perfromance counter bit 23:0 */

Spelling.

> +	psrperf = (I915_READ(EDP_PSR_PERF_CNT)) & 0xffffff;

Unnecessary braces.

> +	seq_printf(m, "EDP_PSR_PERF_COUNTER: %u\n", psrperf);
> +
> +	return 0;
> +}
> +
>  static ssize_t
>  i915_wedged_read(struct file *filp,
>  		 char __user *ubuf,
> @@ -2228,6 +2261,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)
>  
> -- 
> 1.7.11.7
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [Intel-gfx] [PATCH 9/9] drm/i915: Hook PSR functionality
  2013-01-30 18:24 ` [PATCH 9/9] drm/i915: Hook PSR functionality Rodrigo Vivi
@ 2013-01-31 10:50   ` Jani Nikula
  0 siblings, 0 replies; 22+ messages in thread
From: Jani Nikula @ 2013-01-31 10:50 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: dri-devel

On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> PSR must be enabled after transcoder and port are running.
> And it is only available for HSW.
>
> v2: move enable/disable to intel_ddi

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

>
> 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 feb7a4a..1dba035d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1306,6 +1306,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);
>  	}
>  }
>  
> @@ -1318,6 +1319,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);
>  	}
>  }
>  
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 2/9] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
  2013-01-30 18:24 ` [PATCH 2/9] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe Rodrigo Vivi
  2013-01-31  8:32   ` [Intel-gfx] " Jani Nikula
@ 2013-01-31 20:18   ` Paulo Zanoni
  1 sibling, 0 replies; 22+ messages in thread
From: Paulo Zanoni @ 2013-01-31 20:18 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dri-devel

Hi

2013/1/30 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> While old platforms had 3 transcoders and 3 pipes (1:1), HSW has 4 transcoders and 3 pipes. To avoid future mistakes transcoders must be used here instead of pipes even though it is working right now by coincidence.

Not by coincidence: these regs are currently only used by HDMI code
and HDMI code never uses TRANSCODER_EDP, so using "pipe" instead of
cpu_transcoder" is correct, since on HDMI pipe is always the same
thing as cpu_transcoder. Now that we're going to start to use these
registers for DP your patch is needed.

My only complaint about this patch (besides the sentence above) would
be due to using more than 80 columns, both on the commit message and
coding :)

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

>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h   | 16 ++++++++--------
>  drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++------
>  2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2521617..7bb3134 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3742,14 +3742,14 @@
>  #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 _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 d53b731..ab95e05 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -120,13 +120,13 @@ 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 +293,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);
>
> @@ -566,7 +566,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.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

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

* Re: [PATCH 2/9] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe
  2013-01-31  8:32   ` [Intel-gfx] " Jani Nikula
@ 2013-01-31 20:26     ` Paulo Zanoni
  0 siblings, 0 replies; 22+ messages in thread
From: Paulo Zanoni @ 2013-01-31 20:26 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, dri-devel

Hi

2013/1/31 Jani Nikula <jani.nikula@linux.intel.com>:
> On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
>> While old platforms had 3 transcoders and 3 pipes (1:1), HSW has 4 transcoders and 3 pipes. To avoid future mistakes transcoders must be used here instead of pipes even though it is working right now by coincidence.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h   | 16 ++++++++--------
>>  drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++------
>>  2 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 2521617..7bb3134 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3742,14 +3742,14 @@
>>  #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)
>
> Okay, I'm confused. The patch does make sense, but bspec doesn't... the
> spec says the above HSW_TVIDEO_DIP_* registers (in the 0x6xxxx range)
> are per pipe, and the TVIDEO_DIP_* registers (in the 0xexxxx range) are
> per transcoder. Care to enlighten me here...?

Yes. TVIDEO_DIP_* registers don't exist on LPT (which is used by
Haswell), they exist on IBX/CPT/PPT. On these older platforms, pipe
and cpu_transcoder are the same thing, so the value is always the
same. Only after Haswell the old "cpu pipe" was split into "cpu pipe"
and "cpu transcoder". These DIP registers migrated from the PCH to the
CPU on Haswell.

>
> Also, outside of this patch, have we reviewed if there's always a
> register for TRANSCODER_EDP where _TRANSCODER() macro is used?

On Haswell it should exist unless it's a register that is never used
on DP code, so its TRANSCODER_EDP version will never be used.

>
> BR,
> Jani.
>
>
>>
>>  #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 d53b731..ab95e05 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -120,13 +120,13 @@ 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 +293,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);
>>
>> @@ -566,7 +566,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.7.11.7
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Paulo Zanoni

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

* Re: [PATCH 5/9] drm/i915: Setup EDP PSR AUX Registers
  2013-01-30 18:24 ` [PATCH 5/9] drm/i915: Setup EDP PSR AUX Registers Rodrigo Vivi
  2013-01-31 10:17   ` Jani Nikula
@ 2013-01-31 21:35   ` Paulo Zanoni
  1 sibling, 0 replies; 22+ messages in thread
From: Paulo Zanoni @ 2013-01-31 21:35 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Shobhit Kumar, intel-gfx, dri-devel

Hi

In addition to Jani's comments:

2013/1/30 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> v2: Created aux_clock_divider function to avoid duplicated code.
>     Unfortunatelly dp_aux_ch and psr_aux_ch aren't so similar to reuse full code.
>

DDI A AUX channel transactions must not be sent while SRD is enabled.
SRD must be completely disabled before DDI A AUX channel transaction
can be sent. So I think we should put a big WARN_ON inside
intel_dp_aux_ch checking for this restriction. This WARN could maybe
go in a separate patch.

SRD AUX channel transactions must not be sent while DDI A AUX is being
used. This patch may also include a big WARN on the newly-added
function.

More below:

> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 13 ++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 89 +++++++++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  3 files changed, 83 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7bb3134..10732dc 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1563,6 +1563,19 @@
>  #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
>  #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
>
> +#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_MASK          (7<<29)
> +#define EDP_PSR_PERF_CNT               0x64844
> +#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 aeb0ef1..fe83e05 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -364,6 +364,34 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>         return status;
>  }
>
> +static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
> +{
> +       struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +       struct drm_device *dev = intel_dig_port->base.base.dev;
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +       /* The clock divider is based off the hrawclk,
> +        * and would like to run at 2MHz. So, take the
> +        * hrawclk value and divide by 2 and use that
> +        *
> +        * Note that PCH attached eDP panels should use a 125MHz input
> +        * clock divider.
> +        */
> +       if (is_cpu_edp(intel_dp)) {
> +               if (HAS_DDI(dev))
> +                       return intel_ddi_get_cdclk_freq(dev_priv) >> 1;
> +               else if (IS_VALLEYVIEW(dev))
> +                       return 100;
> +               else if (IS_GEN6(dev) || IS_GEN7(dev))
> +                       return 200; /* SNB & IVB eDP input clock at 400Mhz */
> +               else
> +                       return 225; /* eDP input clock at 450Mhz */
> +       } else if (HAS_PCH_SPLIT(dev))
> +               return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> +       else
> +               return intel_hrawclk(dev) / 2;
> +}
> +
>  static int
>  intel_dp_aux_ch(struct intel_dp *intel_dp,
>                 uint8_t *send, int send_bytes,
> @@ -411,26 +439,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>         }
>
>         intel_dp_check_edp(intel_dp);
> -       /* The clock divider is based off the hrawclk,
> -        * and would like to run at 2MHz. So, take the
> -        * hrawclk value and divide by 2 and use that
> -        *
> -        * Note that PCH attached eDP panels should use a 125MHz input
> -        * clock divider.
> -        */
> -       if (is_cpu_edp(intel_dp)) {
> -               if (HAS_DDI(dev))
> -                       aux_clock_divider = intel_ddi_get_cdclk_freq(dev_priv) >> 1;
> -               else if (IS_VALLEYVIEW(dev))
> -                       aux_clock_divider = 100;
> -               else if (IS_GEN6(dev) || IS_GEN7(dev))
> -                       aux_clock_divider = 200; /* SNB & IVB eDP input clock at 400Mhz */
> -               else
> -                       aux_clock_divider = 225; /* eDP input clock at 450Mhz */
> -       } else if (HAS_PCH_SPLIT(dev))
> -               aux_clock_divider = DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
> -       else
> -               aux_clock_divider = intel_hrawclk(dev) / 2;
> +       aux_clock_divider = get_aux_clock_divider(intel_dp);
>
>         if (IS_GEN6(dev))
>                 precharge = 3;
> @@ -1440,6 +1449,46 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
>         return (is_edp(intel_dp) && (intel_dp->psr_dpcd[0] & 0x1));
>  }
>
> +static void intel_edp_psr_setup(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 aux_clock_divider;
> +       int precharge = 0x3;
> +       int msg_size = 5;       /* Header(4) + Message(1) */
> +
> +       /* No need to setup if already done as these setting are persistent
> +        * until power states are entered */
> +       if (intel_dp->psr_setup)
> +               return;
> +
> +       /* 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);
> +
> +       aux_clock_divider = get_aux_clock_divider(intel_dp);
> +
> +       I915_WRITE(EDP_PSR_AUX_CTL,
> +                  DP_AUX_CH_CTL_TIME_OUT_400us |
> +                  (msg_size << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> +                  (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
> +                  (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));

On DP_AUX_CH the message "goes" after we write 1 to bit 31. And then
we wait for the "done" bit. How does it work here?

Also, on DP_AUX_CH we try at least 5 times. Shouldn't we do this here too?

> +
> +       /* Setup the debug register */
> +       I915_WRITE(EDP_PSR_DEBUG_CTL, I915_READ(EDP_PSR_DEBUG_CTL) |
> +                       EDP_PSR_DEBUG_MASK_MEMUP |
> +                       EDP_PSR_DEBUG_MASK_HPD);

Why?

> +
> +       /* This flag can be made to 0 from pm code so as to reinitialize the
> +        * AUX register in case of power states, returning from which will not
> +        * maintain the AUX register settings
> +        */
> +       intel_dp->psr_setup = 1;
> +}
> +
>  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 d4b3bac..3fa2dd0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -383,6 +383,7 @@ struct intel_dp {
>         int backlight_on_delay;
>         int backlight_off_delay;
>         struct delayed_work panel_vdd_work;
> +       uint8_t psr_setup;

bool psr_setup_done ?

>         bool want_panel_vdd;
>         struct intel_connector *attached_connector;
>  };
> --
> 1.7.11.7
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Paulo Zanoni

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

* Re: [PATCH 7/9] drm/i915: Enable/Disable PSR on HSW
  2013-01-30 18:24 ` [PATCH 7/9] drm/i915: Enable/Disable PSR on HSW Rodrigo Vivi
  2013-01-31 10:40   ` Jani Nikula
@ 2013-01-31 22:01   ` Paulo Zanoni
  1 sibling, 0 replies; 22+ messages in thread
From: Paulo Zanoni @ 2013-01-31 22:01 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Shobhit Kumar, intel-gfx, dri-devel

Hi

2013/1/30 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
>
> Added eDP PSR enable functionality. 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. PSR works only in LPSP mode, so put the PIPE_DDI in DDIA
> always on
>
> This patch is heavily based on initial PSR code by Sateesh Kavuri but is
> quite different in implementation. Makes use of VBT parsed data and also
> the code has been cleaned up.
>
> Credits-by: Sateesh Kavuri <sateesh.kavuri@intel.com>
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
>
> v2: fix getting base.crtc from intel_dp and fix DDI_EDP_INPUT_A_ON entry
> v3: Add eDP PSR registers here only when they are really used and use
>     HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder)
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  24 ++++++
>  drivers/gpu/drm/i915/intel_ddi.c |   6 +-
>  drivers/gpu/drm/i915/intel_dp.c  | 172 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |   3 +
>  4 files changed, 204 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 10732dc..6f87d5b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1563,6 +1563,30 @@
>  #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
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2e904a5..feb7a4a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -985,9 +985,13 @@ void intel_ddi_enable_pipe_func(struct drm_crtc *crtc)
>                 temp |= TRANS_DDI_PHSYNC;
>
>         if (cpu_transcoder == TRANSCODER_EDP) {
> +               struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>                 switch (pipe) {
>                 case PIPE_A:
> -                       temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
> +                       if (intel_dp->psr_dpcd[0] & 0x1)

if (is_edp_psr(intel_dp))

> +                               temp |= TRANS_DDI_EDP_INPUT_A_ON;
> +                       else
> +                               temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
>                         break;
>                 case PIPE_B:
>                         temp |= TRANS_DDI_EDP_INPUT_B_ONOFF;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fe83e05..d136890 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);
> @@ -1489,6 +1496,171 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
>         intel_dp->psr_setup = 1;
>  }
>
> +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) & (1<<31)) ? true : false;
> +}
> +
> +
> +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;
> +
> +       /* Use VBT values which are parsed in
> +        * dev_priv->idle_frames,
> +        * but the BIOS initializes this to zero today
> +        * so hardcode
> +        */
> +       uint32_t idle_frames = 6;

How old is that comment? Where did we get 6 from?

> +
> +       if (intel_dp->psr_dpcd[1] & 0x1) {

if (is_edp_psr(intel_dp))

> +               /* No link training on PSR Exit required */
> +               val |= EDP_PSR_TP2_TP3_TIME_0us;
> +               val |= EDP_PSR_TP1_TIME_0us;
> +               val |= EDP_PSR_SKIP_AUX_EXIT;
> +       } else {
> +               /* 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;
> +               };
> +       }
> +
> +       /* Disable main link. Anyway in HSW steppings today
> +        * link standby does not work

Is this comment still true?

> +        *
> +        * Later used VBT info (already parsed and available)
> +        * while supporting standby we need to program
> +        * val |= EDP_PSR_MIN_LINK_ENTRY_TIME_X_LINES based on VBT
> +        */
> +       val = (val & ~EDP_PSR_LINK_STANDBY) |
> +               (max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT) |
> +               (idle_frames << EDP_PSR_IDLE_FRAME_SHIFT) |
> +               EDP_PSR_ENABLE;
> +
> +       I915_WRITE(EDP_PSR_CTL, val);
> +}
> +
> +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 intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
> +       struct edp_vsc_psr psr_vsc;
> +       uint32_t reg = HSW_TVIDEO_DIP_AVI_DATA(intel_crtc->cpu_transcoder);

Shouldn't it be HSW_TVIDEO_DIP_VSC_DATA? Shouldn't we get a bunch of
"unclaimed register"s when writing to
HSW_TVIDEO_DIP_AVI_DATA(TRANSCODER_EDP)? Please check whether your
series introduces any new "unclaimed register" messages.

> +       uint32_t *vsc_data = (uint32_t *) &psr_vsc;
> +       int i = 0, vsc_len = sizeof(struct edp_vsc_psr);
> +
> +       if (!is_edp_psr(intel_dp))
> +               return;
> +
> +       /* setup AUX registers in case returned from pm states */
> +       intel_edp_psr_setup(intel_dp);
> +
> +       /* Check if PSR is already enabled */
> +       if (!intel_edp_is_psr_enabled(intel_dp)) {
> +               /* 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;
> +
> +               /* As per eDP spec, wait for vblank to send SDP VSC packet */
> +               intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
> +               /* Load the VSC DIP packet */
> +               for(i = 0; i < vsc_len; i += 4)
> +                       I915_WRITE((reg + i), vsc_data[i]);

How about reusing hsw_write_infoframe from intel_hdmi.c somehow? It
implements all the "restrictions" we need to respect.

> +
> +#if 0
> +               /* TBD:
> +                * We might not have to do explicitely as hardware will take care of this */
> +               /* Enable the DIP register */
> +               val = I915_READ(VIDEO_DIP_CTL_EDP);
> +               I915_WRITE(VIDEO_DIP_CTL_EDP, val | VIDEOP_DIP_VSC);
> +#endif

Hardware will take care of this? You mean the BIOS? Please try to
configure your BIOS to not configure the panel (make it boot on an
external monitor, not on the panel) and then try to boot your machine,
check if PSR still works after our driver loads. Also, please try to
reuse hsw_set_infoframes instead of the code above, if applicable.
Also, I'm not sure we want to add "#if 0"ed code.


> +               /* 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 &
> +                                               ~DP_PSR_MAIN_LINK_ACTIVE);
> +
> +               /* Enable PSR on the host */
> +               intel_edp_psr_enable_src(intel_dp);
> +       }
> +}
> +
> +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 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_MASK) == 0, 2000, 10))
> +               DRM_ERROR("Timed out waiting for PSR Idle State\n");
> +
> +       intel_wait_for_vblank(dev, intel_crtc->pipe);

Where is this vblank wait documented?

> +
> +#if 0
> +       /* TBD:
> +        * Following is not yet confirmed from H/W team.
> +        * As per last discussion we do not need to disable
> +        * VSC DIP explicitely. Just maintaining the code in
> +        * case we have to do this later at some point
> +        */
> +
> +       /* Disable VSC DIP */
> +       val = I915_READ(VIDEO_DIP_CTL_EDP);
> +       I915_WRITE(VIDEO_DIP_CTL_EDP, val & ~VIDEOP_DIP_VSC);
> +#endif

I'd vote to disable and also try to reuse the functions inside
intel_hdmi.c for this.

> +}
> +
>  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 3fa2dd0..2f48e5c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -692,4 +692,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.7.11.7
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Paulo Zanoni

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

end of thread, other threads:[~2013-01-31 22:01 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-30 18:24 [PATCH 0/9] Enable eDP PSR functionality at HSW - v2 Rodrigo Vivi
2013-01-30 18:24 ` [PATCH 1/9] drm/i915: Organize VBT stuff inside drm_i915_private Rodrigo Vivi
2013-01-31  9:08   ` Jani Nikula
2013-01-30 18:24 ` [PATCH 2/9] drm/i915: Use cpu_transcoder for HSW_TVIDEO_DIP_* instead of pipe Rodrigo Vivi
2013-01-31  8:32   ` [Intel-gfx] " Jani Nikula
2013-01-31 20:26     ` Paulo Zanoni
2013-01-31 20:18   ` [Intel-gfx] " Paulo Zanoni
2013-01-30 18:24 ` [PATCH 3/9] drm/i915: Added SDP and VSC structures for handling PSR for eDP Rodrigo Vivi
2013-01-31  9:07   ` Jani Nikula
2013-01-30 18:24 ` [PATCH 4/9] drm/i915: Read the EDP DPCD and PSR Capability Rodrigo Vivi
2013-01-31  9:12   ` [Intel-gfx] " Jani Nikula
2013-01-30 18:24 ` [PATCH 5/9] drm/i915: Setup EDP PSR AUX Registers Rodrigo Vivi
2013-01-31 10:17   ` Jani Nikula
2013-01-31 21:35   ` Paulo Zanoni
2013-01-30 18:24 ` [PATCH 6/9] drm/i915: VBT Parsing for the PSR Feature Block for HSW Rodrigo Vivi
2013-01-30 18:24 ` [PATCH 7/9] drm/i915: Enable/Disable PSR on HSW Rodrigo Vivi
2013-01-31 10:40   ` Jani Nikula
2013-01-31 22:01   ` Paulo Zanoni
2013-01-30 18:24 ` [PATCH 8/9] drm/i915: Added debugfs support for PSR Status Rodrigo Vivi
2013-01-31 10:48   ` Jani Nikula
2013-01-30 18:24 ` [PATCH 9/9] drm/i915: Hook PSR functionality Rodrigo Vivi
2013-01-31 10:50   ` [Intel-gfx] " Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.