All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series
@ 2015-11-11 18:34 ville.syrjala
  2015-11-11 18:34 ` [PATCH 1/7] drm/i915: Replace aux_ch_ctl_reg check with port check ville.syrjala
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: ville.syrjala @ 2015-11-11 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Chris requested that I try to reorder the DP AUX patches in the last
register type safety series [1] to form a better story. Here is the
result. The final code is exactly the same as before (apart from the
kasprintf() changes), so I kept the previous r-b's in place, with
some added version annotations.

[1] http://lists.freedesktop.org/archives/intel-gfx/2015-November/079453.html

Ville Syrjälä (7):
  drm/i915: Replace aux_ch_ctl_reg check with port check
  drm/i915: Replace the aux ddc name switch statement with kasprintf()
  drm/i915: Parametrize AUX registers
  drm/i915: Remove the magic AUX_CTL is at DP + foo tricks
  drm/i915: Store aux data reg offsets in intel_dp->aux_ch_data_reg[]
  drm/i915: Add dev_priv->psr_mmio_base
  drm/i915: Model PSR AUX register selection more like the normal AUX
    code

 drivers/gpu/drm/i915/i915_debugfs.c |   4 +-
 drivers/gpu/drm/i915/i915_drv.h     |   2 +
 drivers/gpu/drm/i915/i915_reg.h     | 115 ++++++++-------
 drivers/gpu/drm/i915/intel_dp.c     | 282 +++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_drv.h    |   1 +
 drivers/gpu/drm/i915/intel_psr.c    |  51 +++++--
 6 files changed, 298 insertions(+), 157 deletions(-)

-- 
2.4.10

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

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

* [PATCH 1/7] drm/i915: Replace aux_ch_ctl_reg check with port check
  2015-11-11 18:34 [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series ville.syrjala
@ 2015-11-11 18:34 ` ville.syrjala
  2015-11-11 18:34 ` [PATCH 2/7] drm/i915: Replace the aux ddc name switch statement with kasprintf() ville.syrjala
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: ville.syrjala @ 2015-11-11 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Instead of checking what aux_ch_ctl_reg is, we can simply check the port
when determining the right timeout value to program.

v2: Reorder patches to reduce churn (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 99b7f1d..891a7f8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -750,7 +750,7 @@ static uint32_t i9xx_get_aux_send_ctl(struct intel_dp *intel_dp,
 	else
 		precharge = 5;
 
-	if (IS_BROADWELL(dev) && intel_dp->aux_ch_ctl_reg == DPA_AUX_CH_CTL)
+	if (IS_BROADWELL(dev) && intel_dig_port->port == PORT_A)
 		timeout = DP_AUX_CH_CTL_TIME_OUT_600us;
 	else
 		timeout = DP_AUX_CH_CTL_TIME_OUT_400us;
-- 
2.4.10

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

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

* [PATCH 2/7] drm/i915: Replace the aux ddc name switch statement with kasprintf()
  2015-11-11 18:34 [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series ville.syrjala
  2015-11-11 18:34 ` [PATCH 1/7] drm/i915: Replace aux_ch_ctl_reg check with port check ville.syrjala
@ 2015-11-11 18:34 ` ville.syrjala
  2015-11-12  9:23   ` Jani Nikula
  2015-11-11 18:34 ` [PATCH 3/7] drm/i915: Parametrize AUX registers ville.syrjala
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: ville.syrjala @ 2015-11-11 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use kasprintf() to generate the "DPDDC-<port>" name for the aux helper.

To deal with errors properly make intel_dp_aux_init() return something,
and adjust the caller to match. It seems we were also missing a
intel_dp_mst_encoder_cleanup() call on edp (non-port A) init failures,
so add that too.

The whole error/cleanup ordering doesn't feel entirely sane to me, but
I'll leave that part alone for now.

v2: Use kasprintf() instead of a table, reorder patches (Chis)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++++++++++----------------
 1 file changed, 46 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 891a7f8..df2a2d2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1009,6 +1009,13 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 }
 
 static void
+intel_dp_aux_fini(struct intel_dp *intel_dp)
+{
+	drm_dp_aux_unregister(&intel_dp->aux);
+	kfree(intel_dp->aux.name);
+}
+
+static int
 intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -1016,7 +1023,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	enum port port = intel_dig_port->port;
 	struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
-	const char *name = NULL;
 	uint32_t porte_aux_ctl_reg = DPA_AUX_CH_CTL;
 	int ret;
 
@@ -1043,23 +1049,18 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	switch (port) {
 	case PORT_A:
 		intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
-		name = "DPDDC-A";
 		break;
 	case PORT_B:
 		intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
-		name = "DPDDC-B";
 		break;
 	case PORT_C:
 		intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
-		name = "DPDDC-C";
 		break;
 	case PORT_D:
 		intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
-		name = "DPDDC-D";
 		break;
 	case PORT_E:
 		intel_dp->aux_ch_ctl_reg = porte_aux_ctl_reg;
-		name = "DPDDC-E";
 		break;
 	default:
 		BUG();
@@ -1077,27 +1078,36 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) && port != PORT_E)
 		intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
 
-	intel_dp->aux.name = name;
+	intel_dp->aux.name = kasprintf(GFP_KERNEL, "DPDDC-%c", port_name(port));
+	if (!intel_dp->aux.name)
+		return -ENOMEM;
+
 	intel_dp->aux.dev = dev->dev;
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
 
-	DRM_DEBUG_KMS("registering %s bus for %s\n", name,
+	DRM_DEBUG_KMS("registering %s bus for %s\n",
+		      intel_dp->aux.name,
 		      connector->base.kdev->kobj.name);
 
 	ret = drm_dp_aux_register(&intel_dp->aux);
 	if (ret < 0) {
 		DRM_ERROR("drm_dp_aux_register() for %s failed (%d)\n",
-			  name, ret);
-		return;
+			  intel_dp->aux.name, ret);
+		kfree(intel_dp->aux.name);
+		return ret;
 	}
 
 	ret = sysfs_create_link(&connector->base.kdev->kobj,
 				&intel_dp->aux.ddc.dev.kobj,
 				intel_dp->aux.ddc.dev.kobj.name);
 	if (ret < 0) {
-		DRM_ERROR("sysfs_create_link() for %s failed (%d)\n", name, ret);
-		drm_dp_aux_unregister(&intel_dp->aux);
+		DRM_ERROR("sysfs_create_link() for %s failed (%d)\n",
+			  intel_dp->aux.name, ret);
+		intel_dp_aux_fini(intel_dp);
+		return ret;
 	}
+
+	return 0;
 }
 
 static void
@@ -4771,7 +4781,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 
-	drm_dp_aux_unregister(&intel_dp->aux);
+	intel_dp_aux_fini(intel_dp);
 	intel_dp_mst_encoder_cleanup(intel_dig_port);
 	if (is_edp(intel_dp)) {
 		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
@@ -5752,7 +5762,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	struct drm_device *dev = intel_encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum port port = intel_dig_port->port;
-	int type;
+	int type, ret;
 
 	intel_dp->pps_pipe = INVALID_PIPE;
 
@@ -5853,7 +5863,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		pps_unlock(intel_dp);
 	}
 
-	intel_dp_aux_init(intel_dp, intel_connector);
+	ret = intel_dp_aux_init(intel_dp, intel_connector);
+	if (ret)
+		goto fail;
 
 	/* init MST on ports that can support it */
 	if (HAS_DP_MST(dev) &&
@@ -5862,20 +5874,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 					  intel_connector->base.base.id);
 
 	if (!intel_edp_init_connector(intel_dp, intel_connector)) {
-		drm_dp_aux_unregister(&intel_dp->aux);
-		if (is_edp(intel_dp)) {
-			cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
-			/*
-			 * vdd might still be enabled do to the delayed vdd off.
-			 * Make sure vdd is actually turned off here.
-			 */
-			pps_lock(intel_dp);
-			edp_panel_vdd_off_sync(intel_dp);
-			pps_unlock(intel_dp);
-		}
-		drm_connector_unregister(connector);
-		drm_connector_cleanup(connector);
-		return false;
+		intel_dp_aux_fini(intel_dp);
+		intel_dp_mst_encoder_cleanup(intel_dig_port);
+		goto fail;
 	}
 
 	intel_dp_add_properties(intel_dp, connector);
@@ -5892,6 +5893,22 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	i915_debugfs_connector_add(connector);
 
 	return true;
+
+fail:
+	if (is_edp(intel_dp)) {
+		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
+		/*
+		 * vdd might still be enabled do to the delayed vdd off.
+		 * Make sure vdd is actually turned off here.
+		 */
+		pps_lock(intel_dp);
+		edp_panel_vdd_off_sync(intel_dp);
+		pps_unlock(intel_dp);
+	}
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+
+	return false;
 }
 
 void
-- 
2.4.10

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

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

* [PATCH 3/7] drm/i915: Parametrize AUX registers
  2015-11-11 18:34 [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series ville.syrjala
  2015-11-11 18:34 ` [PATCH 1/7] drm/i915: Replace aux_ch_ctl_reg check with port check ville.syrjala
  2015-11-11 18:34 ` [PATCH 2/7] drm/i915: Replace the aux ddc name switch statement with kasprintf() ville.syrjala
@ 2015-11-11 18:34 ` ville.syrjala
  2015-11-11 18:34 ` [PATCH 4/7] drm/i915: Remove the magic AUX_CTL is at DP + foo tricks ville.syrjala
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: ville.syrjala @ 2015-11-11 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

v2: Keep some MISSING_CASE() stuff (Jani)
    s/-1/-PIPE_B/ in the register macro
    Fix typo in patch subject
v3: Use PORT_B registers for invalid ports in g4x_aux_ctl_reg() (Jani)
v4: Reorder patches (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com> (v3)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v3)
---
 drivers/gpu/drm/i915/i915_reg.h  | 102 ++++++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_dp.c  |  18 +++----
 drivers/gpu/drm/i915/intel_psr.c |   5 +-
 3 files changed, 62 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8bd2699..3479dfc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3100,11 +3100,7 @@ enum skl_disp_power_wells {
 #define   EDP_PSR_IDLE_FRAME_SHIFT		0
 
 #define EDP_PSR_AUX_CTL(dev)			(EDP_PSR_BASE(dev) + 0x10)
-#define EDP_PSR_AUX_DATA1(dev)			(EDP_PSR_BASE(dev) + 0x14)
-#define EDP_PSR_AUX_DATA2(dev)			(EDP_PSR_BASE(dev) + 0x18)
-#define EDP_PSR_AUX_DATA3(dev)			(EDP_PSR_BASE(dev) + 0x1c)
-#define EDP_PSR_AUX_DATA4(dev)			(EDP_PSR_BASE(dev) + 0x20)
-#define EDP_PSR_AUX_DATA5(dev)			(EDP_PSR_BASE(dev) + 0x24)
+#define EDP_PSR_AUX_DATA(dev, i)		(EDP_PSR_BASE(dev) + 0x14 + (i) * 4) /* 5 registers */
 
 #define EDP_PSR_STATUS_CTL(dev)			(EDP_PSR_BASE(dev) + 0x40)
 #define   EDP_PSR_STATUS_STATE_MASK		(7<<29)
@@ -4232,33 +4228,36 @@ enum skl_disp_power_wells {
  * is 20 bytes in each direction, hence the 5 fixed
  * data registers
  */
-#define DPA_AUX_CH_CTL			0x64010
-#define DPA_AUX_CH_DATA1		0x64014
-#define DPA_AUX_CH_DATA2		0x64018
-#define DPA_AUX_CH_DATA3		0x6401c
-#define DPA_AUX_CH_DATA4		0x64020
-#define DPA_AUX_CH_DATA5		0x64024
-
-#define DPB_AUX_CH_CTL			0x64110
-#define DPB_AUX_CH_DATA1		0x64114
-#define DPB_AUX_CH_DATA2		0x64118
-#define DPB_AUX_CH_DATA3		0x6411c
-#define DPB_AUX_CH_DATA4		0x64120
-#define DPB_AUX_CH_DATA5		0x64124
-
-#define DPC_AUX_CH_CTL			0x64210
-#define DPC_AUX_CH_DATA1		0x64214
-#define DPC_AUX_CH_DATA2		0x64218
-#define DPC_AUX_CH_DATA3		0x6421c
-#define DPC_AUX_CH_DATA4		0x64220
-#define DPC_AUX_CH_DATA5		0x64224
-
-#define DPD_AUX_CH_CTL			0x64310
-#define DPD_AUX_CH_DATA1		0x64314
-#define DPD_AUX_CH_DATA2		0x64318
-#define DPD_AUX_CH_DATA3		0x6431c
-#define DPD_AUX_CH_DATA4		0x64320
-#define DPD_AUX_CH_DATA5		0x64324
+#define _DPA_AUX_CH_CTL			0x64010
+#define _DPA_AUX_CH_DATA1		0x64014
+#define _DPA_AUX_CH_DATA2		0x64018
+#define _DPA_AUX_CH_DATA3		0x6401c
+#define _DPA_AUX_CH_DATA4		0x64020
+#define _DPA_AUX_CH_DATA5		0x64024
+
+#define _DPB_AUX_CH_CTL			0x64110
+#define _DPB_AUX_CH_DATA1		0x64114
+#define _DPB_AUX_CH_DATA2		0x64118
+#define _DPB_AUX_CH_DATA3		0x6411c
+#define _DPB_AUX_CH_DATA4		0x64120
+#define _DPB_AUX_CH_DATA5		0x64124
+
+#define _DPC_AUX_CH_CTL			0x64210
+#define _DPC_AUX_CH_DATA1		0x64214
+#define _DPC_AUX_CH_DATA2		0x64218
+#define _DPC_AUX_CH_DATA3		0x6421c
+#define _DPC_AUX_CH_DATA4		0x64220
+#define _DPC_AUX_CH_DATA5		0x64224
+
+#define _DPD_AUX_CH_CTL			0x64310
+#define _DPD_AUX_CH_DATA1		0x64314
+#define _DPD_AUX_CH_DATA2		0x64318
+#define _DPD_AUX_CH_DATA3		0x6431c
+#define _DPD_AUX_CH_DATA4		0x64320
+#define _DPD_AUX_CH_DATA5		0x64324
+
+#define DP_AUX_CH_CTL(port)	_PORT(port, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
+#define DP_AUX_CH_DATA(port, i)	(_PORT(port, _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
 
 #define   DP_AUX_CH_CTL_SEND_BUSY	    (1 << 31)
 #define   DP_AUX_CH_CTL_DONE		    (1 << 30)
@@ -6609,28 +6608,31 @@ enum skl_disp_power_wells {
 #define BXT_PP_OFF_DELAYS(n)	_PIPE(n, PCH_PP_OFF_DELAYS, _BXT_PP_OFF_DELAYS2)
 
 #define PCH_DP_B		0xe4100
-#define PCH_DPB_AUX_CH_CTL	0xe4110
-#define PCH_DPB_AUX_CH_DATA1	0xe4114
-#define PCH_DPB_AUX_CH_DATA2	0xe4118
-#define PCH_DPB_AUX_CH_DATA3	0xe411c
-#define PCH_DPB_AUX_CH_DATA4	0xe4120
-#define PCH_DPB_AUX_CH_DATA5	0xe4124
+#define _PCH_DPB_AUX_CH_CTL	0xe4110
+#define _PCH_DPB_AUX_CH_DATA1	0xe4114
+#define _PCH_DPB_AUX_CH_DATA2	0xe4118
+#define _PCH_DPB_AUX_CH_DATA3	0xe411c
+#define _PCH_DPB_AUX_CH_DATA4	0xe4120
+#define _PCH_DPB_AUX_CH_DATA5	0xe4124
 
 #define PCH_DP_C		0xe4200
-#define PCH_DPC_AUX_CH_CTL	0xe4210
-#define PCH_DPC_AUX_CH_DATA1	0xe4214
-#define PCH_DPC_AUX_CH_DATA2	0xe4218
-#define PCH_DPC_AUX_CH_DATA3	0xe421c
-#define PCH_DPC_AUX_CH_DATA4	0xe4220
-#define PCH_DPC_AUX_CH_DATA5	0xe4224
+#define _PCH_DPC_AUX_CH_CTL	0xe4210
+#define _PCH_DPC_AUX_CH_DATA1	0xe4214
+#define _PCH_DPC_AUX_CH_DATA2	0xe4218
+#define _PCH_DPC_AUX_CH_DATA3	0xe421c
+#define _PCH_DPC_AUX_CH_DATA4	0xe4220
+#define _PCH_DPC_AUX_CH_DATA5	0xe4224
 
 #define PCH_DP_D		0xe4300
-#define PCH_DPD_AUX_CH_CTL	0xe4310
-#define PCH_DPD_AUX_CH_DATA1	0xe4314
-#define PCH_DPD_AUX_CH_DATA2	0xe4318
-#define PCH_DPD_AUX_CH_DATA3	0xe431c
-#define PCH_DPD_AUX_CH_DATA4	0xe4320
-#define PCH_DPD_AUX_CH_DATA5	0xe4324
+#define _PCH_DPD_AUX_CH_CTL	0xe4310
+#define _PCH_DPD_AUX_CH_DATA1	0xe4314
+#define _PCH_DPD_AUX_CH_DATA2	0xe4318
+#define _PCH_DPD_AUX_CH_DATA3	0xe431c
+#define _PCH_DPD_AUX_CH_DATA4	0xe4320
+#define _PCH_DPD_AUX_CH_DATA5	0xe4324
+
+#define PCH_DP_AUX_CH_CTL(port)		_PORT((port) - PORT_B, _PCH_DPB_AUX_CH_CTL, _PCH_DPC_AUX_CH_CTL)
+#define PCH_DP_AUX_CH_DATA(port, i)	(_PORT((port) - PORT_B, _PCH_DPB_AUX_CH_DATA1, _PCH_DPC_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
 
 /* CPT */
 #define  PORT_TRANS_A_SEL_CPT	0
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index df2a2d2..b07660c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1023,7 +1023,7 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	enum port port = intel_dig_port->port;
 	struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
-	uint32_t porte_aux_ctl_reg = DPA_AUX_CH_CTL;
+	uint32_t porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_A);
 	int ret;
 
 	/* On SKL we don't have Aux for port E so we rely on VBT to set
@@ -1032,32 +1032,28 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) && port == PORT_E) {
 		switch (info->alternate_aux_channel) {
 		case DP_AUX_B:
-			porte_aux_ctl_reg = DPB_AUX_CH_CTL;
+			porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_B);
 			break;
 		case DP_AUX_C:
-			porte_aux_ctl_reg = DPC_AUX_CH_CTL;
+			porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_C);
 			break;
 		case DP_AUX_D:
-			porte_aux_ctl_reg = DPD_AUX_CH_CTL;
+			porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_D);
 			break;
 		case DP_AUX_A:
 		default:
-			porte_aux_ctl_reg = DPA_AUX_CH_CTL;
+			porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_A);
 		}
 	}
 
 	switch (port) {
 	case PORT_A:
-		intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
+		intel_dp->aux_ch_ctl_reg = DP_AUX_CH_CTL(port);
 		break;
 	case PORT_B:
-		intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
-		break;
 	case PORT_C:
-		intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
-		break;
 	case PORT_D:
-		intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
+		intel_dp->aux_ch_ctl_reg = PCH_DP_AUX_CH_CTL(port);
 		break;
 	case PORT_E:
 		intel_dp->aux_ch_ctl_reg = porte_aux_ctl_reg;
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 213581c..ff66718 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -166,6 +166,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 		[3] = 1 - 1,
 		[4] = DP_SET_POWER_D0,
 	};
+	enum port port = dig_port->port;
 	int i;
 
 	BUILD_BUG_ON(sizeof(aux_msg) > 20);
@@ -182,9 +183,9 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 				DP_AUX_FRAME_SYNC_ENABLE);
 
 	aux_data_reg = (INTEL_INFO(dev)->gen >= 9) ?
-				DPA_AUX_CH_DATA1 : EDP_PSR_AUX_DATA1(dev);
+		DP_AUX_CH_DATA(port, 0) : EDP_PSR_AUX_DATA(dev, 0);
 	aux_ctl_reg = (INTEL_INFO(dev)->gen >= 9) ?
-				DPA_AUX_CH_CTL : EDP_PSR_AUX_CTL(dev);
+		DP_AUX_CH_CTL(port) : EDP_PSR_AUX_CTL(dev);
 
 	/* Setup AUX registers */
 	for (i = 0; i < sizeof(aux_msg); i += 4)
-- 
2.4.10

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

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

* [PATCH 4/7] drm/i915: Remove the magic AUX_CTL is at DP + foo tricks
  2015-11-11 18:34 [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series ville.syrjala
                   ` (2 preceding siblings ...)
  2015-11-11 18:34 ` [PATCH 3/7] drm/i915: Parametrize AUX registers ville.syrjala
@ 2015-11-11 18:34 ` ville.syrjala
  2015-11-11 18:34 ` [PATCH 5/7] drm/i915: Store aux data reg offsets in intel_dp->aux_ch_data_reg[] ville.syrjala
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: ville.syrjala @ 2015-11-11 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we determine the location of the AUX registers in a confusing
way. First we assume the PCH registers are used always, but then we
override it for everything but HSW/BDW to use DP+0x10. Very confusing.

Let's just make it straightforward and simply add a few functions to
pick the right AUX_CTL based on the DP port.

To deal with VLV/CHV we'll include the display_mmio_offset into the
AUX register defines.

v2: Reorder patches (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
---
 drivers/gpu/drm/i915/i915_reg.h |  54 ++++++++---------
 drivers/gpu/drm/i915/intel_dp.c | 127 ++++++++++++++++++++++++----------------
 2 files changed, 105 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3479dfc..40a811f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4228,33 +4228,33 @@ enum skl_disp_power_wells {
  * is 20 bytes in each direction, hence the 5 fixed
  * data registers
  */
-#define _DPA_AUX_CH_CTL			0x64010
-#define _DPA_AUX_CH_DATA1		0x64014
-#define _DPA_AUX_CH_DATA2		0x64018
-#define _DPA_AUX_CH_DATA3		0x6401c
-#define _DPA_AUX_CH_DATA4		0x64020
-#define _DPA_AUX_CH_DATA5		0x64024
-
-#define _DPB_AUX_CH_CTL			0x64110
-#define _DPB_AUX_CH_DATA1		0x64114
-#define _DPB_AUX_CH_DATA2		0x64118
-#define _DPB_AUX_CH_DATA3		0x6411c
-#define _DPB_AUX_CH_DATA4		0x64120
-#define _DPB_AUX_CH_DATA5		0x64124
-
-#define _DPC_AUX_CH_CTL			0x64210
-#define _DPC_AUX_CH_DATA1		0x64214
-#define _DPC_AUX_CH_DATA2		0x64218
-#define _DPC_AUX_CH_DATA3		0x6421c
-#define _DPC_AUX_CH_DATA4		0x64220
-#define _DPC_AUX_CH_DATA5		0x64224
-
-#define _DPD_AUX_CH_CTL			0x64310
-#define _DPD_AUX_CH_DATA1		0x64314
-#define _DPD_AUX_CH_DATA2		0x64318
-#define _DPD_AUX_CH_DATA3		0x6431c
-#define _DPD_AUX_CH_DATA4		0x64320
-#define _DPD_AUX_CH_DATA5		0x64324
+#define _DPA_AUX_CH_CTL		(dev_priv->info.display_mmio_offset + 0x64010)
+#define _DPA_AUX_CH_DATA1	(dev_priv->info.display_mmio_offset + 0x64014)
+#define _DPA_AUX_CH_DATA2	(dev_priv->info.display_mmio_offset + 0x64018)
+#define _DPA_AUX_CH_DATA3	(dev_priv->info.display_mmio_offset + 0x6401c)
+#define _DPA_AUX_CH_DATA4	(dev_priv->info.display_mmio_offset + 0x64020)
+#define _DPA_AUX_CH_DATA5	(dev_priv->info.display_mmio_offset + 0x64024)
+
+#define _DPB_AUX_CH_CTL		(dev_priv->info.display_mmio_offset + 0x64110)
+#define _DPB_AUX_CH_DATA1	(dev_priv->info.display_mmio_offset + 0x64114)
+#define _DPB_AUX_CH_DATA2	(dev_priv->info.display_mmio_offset + 0x64118)
+#define _DPB_AUX_CH_DATA3	(dev_priv->info.display_mmio_offset + 0x6411c)
+#define _DPB_AUX_CH_DATA4	(dev_priv->info.display_mmio_offset + 0x64120)
+#define _DPB_AUX_CH_DATA5	(dev_priv->info.display_mmio_offset + 0x64124)
+
+#define _DPC_AUX_CH_CTL		(dev_priv->info.display_mmio_offset + 0x64210)
+#define _DPC_AUX_CH_DATA1	(dev_priv->info.display_mmio_offset + 0x64214)
+#define _DPC_AUX_CH_DATA2	(dev_priv->info.display_mmio_offset + 0x64218)
+#define _DPC_AUX_CH_DATA3	(dev_priv->info.display_mmio_offset + 0x6421c)
+#define _DPC_AUX_CH_DATA4	(dev_priv->info.display_mmio_offset + 0x64220)
+#define _DPC_AUX_CH_DATA5	(dev_priv->info.display_mmio_offset + 0x64224)
+
+#define _DPD_AUX_CH_CTL		(dev_priv->info.display_mmio_offset + 0x64310)
+#define _DPD_AUX_CH_DATA1	(dev_priv->info.display_mmio_offset + 0x64314)
+#define _DPD_AUX_CH_DATA2	(dev_priv->info.display_mmio_offset + 0x64318)
+#define _DPD_AUX_CH_DATA3	(dev_priv->info.display_mmio_offset + 0x6431c)
+#define _DPD_AUX_CH_DATA4	(dev_priv->info.display_mmio_offset + 0x64320)
+#define _DPD_AUX_CH_DATA5	(dev_priv->info.display_mmio_offset + 0x64324)
 
 #define DP_AUX_CH_CTL(port)	_PORT(port, _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
 #define DP_AUX_CH_DATA(port, i)	(_PORT(port, _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b07660c..55550845 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1008,6 +1008,78 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 	return ret;
 }
 
+static uint32_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
+				enum port port)
+{
+	switch (port) {
+	case PORT_B:
+	case PORT_C:
+	case PORT_D:
+		return DP_AUX_CH_CTL(port);
+	default:
+		MISSING_CASE(port);
+		return DP_AUX_CH_CTL(PORT_B);
+	}
+}
+
+static uint32_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
+				enum port port)
+{
+	switch (port) {
+	case PORT_A:
+		return DP_AUX_CH_CTL(port);
+	case PORT_B:
+	case PORT_C:
+	case PORT_D:
+		return PCH_DP_AUX_CH_CTL(port);
+	default:
+		MISSING_CASE(port);
+		return DP_AUX_CH_CTL(PORT_A);
+	}
+}
+
+/*
+ * On SKL we don't have Aux for port E so we rely
+ * on VBT to set a proper alternate aux channel.
+ */
+static enum port skl_porte_aux_port(struct drm_i915_private *dev_priv)
+{
+	const struct ddi_vbt_port_info *info =
+		&dev_priv->vbt.ddi_port_info[PORT_E];
+
+	switch (info->alternate_aux_channel) {
+	case DP_AUX_A:
+		return PORT_A;
+	case DP_AUX_B:
+		return PORT_B;
+	case DP_AUX_C:
+		return PORT_C;
+	case DP_AUX_D:
+		return PORT_D;
+	default:
+		MISSING_CASE(info->alternate_aux_channel);
+		return PORT_A;
+	}
+}
+
+static uint32_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
+				enum port port)
+{
+	if (port == PORT_E)
+		port = skl_porte_aux_port(dev_priv);
+
+	switch (port) {
+	case PORT_A:
+	case PORT_B:
+	case PORT_C:
+	case PORT_D:
+		return DP_AUX_CH_CTL(port);
+	default:
+		MISSING_CASE(port);
+		return DP_AUX_CH_CTL(PORT_A);
+	}
+}
+
 static void
 intel_dp_aux_fini(struct intel_dp *intel_dp)
 {
@@ -1022,57 +1094,14 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	enum port port = intel_dig_port->port;
-	struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
-	uint32_t porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_A);
 	int ret;
 
-	/* On SKL we don't have Aux for port E so we rely on VBT to set
-	 * a proper alternate aux channel.
-	 */
-	if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) && port == PORT_E) {
-		switch (info->alternate_aux_channel) {
-		case DP_AUX_B:
-			porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_B);
-			break;
-		case DP_AUX_C:
-			porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_C);
-			break;
-		case DP_AUX_D:
-			porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_D);
-			break;
-		case DP_AUX_A:
-		default:
-			porte_aux_ctl_reg = DP_AUX_CH_CTL(PORT_A);
-		}
-	}
-
-	switch (port) {
-	case PORT_A:
-		intel_dp->aux_ch_ctl_reg = DP_AUX_CH_CTL(port);
-		break;
-	case PORT_B:
-	case PORT_C:
-	case PORT_D:
-		intel_dp->aux_ch_ctl_reg = PCH_DP_AUX_CH_CTL(port);
-		break;
-	case PORT_E:
-		intel_dp->aux_ch_ctl_reg = porte_aux_ctl_reg;
-		break;
-	default:
-		BUG();
-	}
-
-	/*
-	 * The AUX_CTL register is usually DP_CTL + 0x10.
-	 *
-	 * On Haswell and Broadwell though:
-	 *   - Both port A DDI_BUF_CTL and DDI_AUX_CTL are on the CPU
-	 *   - Port B/C/D AUX channels are on the PCH, DDI_BUF_CTL on the CPU
-	 *
-	 * Skylake moves AUX_CTL back next to DDI_BUF_CTL, on the CPU.
-	 */
-	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) && port != PORT_E)
-		intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
+	if (INTEL_INFO(dev_priv)->gen >= 9)
+		intel_dp->aux_ch_ctl_reg = skl_aux_ctl_reg(dev_priv, port);
+	else if (HAS_PCH_SPLIT(dev_priv))
+		intel_dp->aux_ch_ctl_reg = ilk_aux_ctl_reg(dev_priv, port);
+	else
+		intel_dp->aux_ch_ctl_reg = g4x_aux_ctl_reg(dev_priv, port);
 
 	intel_dp->aux.name = kasprintf(GFP_KERNEL, "DPDDC-%c", port_name(port));
 	if (!intel_dp->aux.name)
-- 
2.4.10

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

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

* [PATCH 5/7] drm/i915: Store aux data reg offsets in intel_dp->aux_ch_data_reg[]
  2015-11-11 18:34 [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series ville.syrjala
                   ` (3 preceding siblings ...)
  2015-11-11 18:34 ` [PATCH 4/7] drm/i915: Remove the magic AUX_CTL is at DP + foo tricks ville.syrjala
@ 2015-11-11 18:34 ` ville.syrjala
  2015-11-11 18:34 ` [PATCH 6/7] drm/i915: Add dev_priv->psr_mmio_base ville.syrjala
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: ville.syrjala @ 2015-11-11 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather than computing on demand, store also the aux data reg
offsets under intel_dp.

v2: Duplicate some code to make things less magic (Jani)
v3: Use PORT_B registers for invalid ports in g4x_aux_data_reg()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c  | 94 +++++++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 85 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 55550845..8071247 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -790,7 +790,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
-	uint32_t ch_data = ch_ctl + 4;
 	uint32_t aux_clock_divider;
 	int i, ret, recv_bytes;
 	uint32_t status;
@@ -856,7 +855,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 		for (try = 0; try < 5; try++) {
 			/* Load the send data into the aux channel data registers */
 			for (i = 0; i < send_bytes; i += 4)
-				I915_WRITE(ch_data + i,
+				I915_WRITE(intel_dp->aux_ch_data_reg[i >> 2],
 					   intel_dp_pack_aux(send + i,
 							     send_bytes - i));
 
@@ -920,7 +919,7 @@ done:
 		recv_bytes = recv_size;
 
 	for (i = 0; i < recv_bytes; i += 4)
-		intel_dp_unpack_aux(I915_READ(ch_data + i),
+		intel_dp_unpack_aux(I915_READ(intel_dp->aux_ch_data_reg[i >> 2]),
 				    recv + i, recv_bytes - i);
 
 	ret = recv_bytes;
@@ -1022,6 +1021,20 @@ static uint32_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
 	}
 }
 
+static uint32_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
+				 enum port port, int index)
+{
+	switch (port) {
+	case PORT_B:
+	case PORT_C:
+	case PORT_D:
+		return DP_AUX_CH_DATA(port, index);
+	default:
+		MISSING_CASE(port);
+		return DP_AUX_CH_DATA(PORT_B, index);
+	}
+}
+
 static uint32_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
 				enum port port)
 {
@@ -1038,6 +1051,22 @@ static uint32_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
 	}
 }
 
+static uint32_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
+				 enum port port, int index)
+{
+	switch (port) {
+	case PORT_A:
+		return DP_AUX_CH_DATA(port, index);
+	case PORT_B:
+	case PORT_C:
+	case PORT_D:
+		return PCH_DP_AUX_CH_DATA(port, index);
+	default:
+		MISSING_CASE(port);
+		return DP_AUX_CH_DATA(PORT_A, index);
+	}
+}
+
 /*
  * On SKL we don't have Aux for port E so we rely
  * on VBT to set a proper alternate aux channel.
@@ -1080,6 +1109,57 @@ static uint32_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
 	}
 }
 
+static uint32_t skl_aux_data_reg(struct drm_i915_private *dev_priv,
+				 enum port port, int index)
+{
+	if (port == PORT_E)
+		port = skl_porte_aux_port(dev_priv);
+
+	switch (port) {
+	case PORT_A:
+	case PORT_B:
+	case PORT_C:
+	case PORT_D:
+		return DP_AUX_CH_DATA(port, index);
+	default:
+		MISSING_CASE(port);
+		return DP_AUX_CH_DATA(PORT_A, index);
+	}
+}
+
+static uint32_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
+				  enum port port)
+{
+	if (INTEL_INFO(dev_priv)->gen >= 9)
+		return skl_aux_ctl_reg(dev_priv, port);
+	else if (HAS_PCH_SPLIT(dev_priv))
+		return ilk_aux_ctl_reg(dev_priv, port);
+	else
+		return g4x_aux_ctl_reg(dev_priv, port);
+}
+
+static uint32_t intel_aux_data_reg(struct drm_i915_private *dev_priv,
+				   enum port port, int index)
+{
+	if (INTEL_INFO(dev_priv)->gen >= 9)
+		return skl_aux_data_reg(dev_priv, port, index);
+	else if (HAS_PCH_SPLIT(dev_priv))
+		return ilk_aux_data_reg(dev_priv, port, index);
+	else
+		return g4x_aux_data_reg(dev_priv, port, index);
+}
+
+static void intel_aux_reg_init(struct intel_dp *intel_dp)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
+	enum port port = dp_to_dig_port(intel_dp)->port;
+	int i;
+
+	intel_dp->aux_ch_ctl_reg = intel_aux_ctl_reg(dev_priv, port);
+	for (i = 0; i < ARRAY_SIZE(intel_dp->aux_ch_data_reg); i++)
+		intel_dp->aux_ch_data_reg[i] = intel_aux_data_reg(dev_priv, port, i);
+}
+
 static void
 intel_dp_aux_fini(struct intel_dp *intel_dp)
 {
@@ -1091,17 +1171,11 @@ static int
 intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	enum port port = intel_dig_port->port;
 	int ret;
 
-	if (INTEL_INFO(dev_priv)->gen >= 9)
-		intel_dp->aux_ch_ctl_reg = skl_aux_ctl_reg(dev_priv, port);
-	else if (HAS_PCH_SPLIT(dev_priv))
-		intel_dp->aux_ch_ctl_reg = ilk_aux_ctl_reg(dev_priv, port);
-	else
-		intel_dp->aux_ch_ctl_reg = g4x_aux_ctl_reg(dev_priv, port);
+	intel_aux_reg_init(intel_dp);
 
 	intel_dp->aux.name = kasprintf(GFP_KERNEL, "DPDDC-%c", port_name(port));
 	if (!intel_dp->aux.name)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e3794d3..6ca5da6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -747,6 +747,7 @@ struct sink_crc {
 struct intel_dp {
 	uint32_t output_reg;
 	uint32_t aux_ch_ctl_reg;
+	uint32_t aux_ch_data_reg[5];
 	uint32_t DP;
 	int link_rate;
 	uint8_t lane_count;
-- 
2.4.10

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

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

* [PATCH 6/7] drm/i915: Add dev_priv->psr_mmio_base
  2015-11-11 18:34 [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series ville.syrjala
                   ` (4 preceding siblings ...)
  2015-11-11 18:34 ` [PATCH 5/7] drm/i915: Store aux data reg offsets in intel_dp->aux_ch_data_reg[] ville.syrjala
@ 2015-11-11 18:34 ` ville.syrjala
  2015-11-11 18:34 ` [PATCH 7/7] drm/i915: Model PSR AUX register selection more like the normal AUX code ville.syrjala
  2015-11-12  9:37 ` [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series Jani Nikula
  7 siblings, 0 replies; 11+ messages in thread
From: ville.syrjala @ 2015-11-11 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Drop the EDP_PSR_BASE() thing, and just stick the PSR register offset
under dev_priv, like we for DSI and GPIO for example.

TODO: could probably move a bunch of this kind of stuff into the device
info instead...

v2: Drop the spurious whitespace change (Jani)

Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h     |  2 ++
 drivers/gpu/drm/i915/i915_reg.h     | 15 ++++++++-------
 drivers/gpu/drm/i915/intel_psr.c    | 27 +++++++++++++++------------
 4 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d6d69f4..f49f1ad 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2560,7 +2560,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 		   yesno(work_busy(&dev_priv->psr.work.work)));
 
 	if (HAS_DDI(dev))
-		enabled = I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
+		enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
 	else {
 		for_each_pipe(dev_priv, pipe) {
 			stat[pipe] = I915_READ(VLV_PSRSTAT(pipe)) &
@@ -2582,7 +2582,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 
 	/* CHV PSR has no kind of performance counter */
 	if (HAS_DDI(dev)) {
-		psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
+		psrperf = I915_READ(EDP_PSR_PERF_CNT) &
 			EDP_PSR_PERF_CNT_MASK;
 
 		seq_printf(m, "Performance_Counter: %u\n", psrperf);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d5cf30b..722f676 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1726,6 +1726,8 @@ struct drm_i915_private {
 	/* MMIO base address for MIPI regs */
 	uint32_t mipi_mmio_base;
 
+	uint32_t psr_mmio_base;
+
 	wait_queue_head_t gmbus_wait_queue;
 
 	struct pci_dev *bridge_dev;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 40a811f..db72f98 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3075,8 +3075,9 @@ enum skl_disp_power_wells {
 #define VLV_PSRSTAT(pipe) _PIPE(pipe, _PSRSTATA, _PSRSTATB)
 
 /* HSW+ eDP PSR registers */
-#define EDP_PSR_BASE(dev)                       (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
-#define EDP_PSR_CTL(dev)			(EDP_PSR_BASE(dev) + 0)
+#define HSW_EDP_PSR_BASE	0x64800
+#define BDW_EDP_PSR_BASE	0x6f800
+#define EDP_PSR_CTL				(dev_priv->psr_mmio_base + 0)
 #define   EDP_PSR_ENABLE			(1<<31)
 #define   BDW_PSR_SINGLE_FRAME			(1<<30)
 #define   EDP_PSR_LINK_STANDBY			(1<<27)
@@ -3099,10 +3100,10 @@ enum skl_disp_power_wells {
 #define   EDP_PSR_TP1_TIME_0us			(3<<4)
 #define   EDP_PSR_IDLE_FRAME_SHIFT		0
 
-#define EDP_PSR_AUX_CTL(dev)			(EDP_PSR_BASE(dev) + 0x10)
-#define EDP_PSR_AUX_DATA(dev, i)		(EDP_PSR_BASE(dev) + 0x14 + (i) * 4) /* 5 registers */
+#define EDP_PSR_AUX_CTL				(dev_priv->psr_mmio_base + 0x10)
+#define EDP_PSR_AUX_DATA(i)			(dev_priv->psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */
 
-#define EDP_PSR_STATUS_CTL(dev)			(EDP_PSR_BASE(dev) + 0x40)
+#define EDP_PSR_STATUS_CTL			(dev_priv->psr_mmio_base + 0x40)
 #define   EDP_PSR_STATUS_STATE_MASK		(7<<29)
 #define   EDP_PSR_STATUS_STATE_IDLE		(0<<29)
 #define   EDP_PSR_STATUS_STATE_SRDONACK		(1<<29)
@@ -3126,10 +3127,10 @@ enum skl_disp_power_wells {
 #define   EDP_PSR_STATUS_SENDING_TP1		(1<<4)
 #define   EDP_PSR_STATUS_IDLE_MASK		0xf
 
-#define EDP_PSR_PERF_CNT(dev)		(EDP_PSR_BASE(dev) + 0x44)
+#define EDP_PSR_PERF_CNT		(dev_priv->psr_mmio_base + 0x44)
 #define   EDP_PSR_PERF_CNT_MASK		0xffffff
 
-#define EDP_PSR_DEBUG_CTL(dev)		(EDP_PSR_BASE(dev) + 0x60)
+#define EDP_PSR_DEBUG_CTL		(dev_priv->psr_mmio_base + 0x60)
 #define   EDP_PSR_DEBUG_MASK_LPSP	(1<<27)
 #define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
 #define   EDP_PSR_DEBUG_MASK_HPD	(1<<25)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index ff66718..3c973c5 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -183,9 +183,9 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 				DP_AUX_FRAME_SYNC_ENABLE);
 
 	aux_data_reg = (INTEL_INFO(dev)->gen >= 9) ?
-		DP_AUX_CH_DATA(port, 0) : EDP_PSR_AUX_DATA(dev, 0);
+		DP_AUX_CH_DATA(port, 0) : EDP_PSR_AUX_DATA(0);
 	aux_ctl_reg = (INTEL_INFO(dev)->gen >= 9) ?
-		DP_AUX_CH_CTL(port) : EDP_PSR_AUX_CTL(dev);
+		DP_AUX_CH_CTL(port) : EDP_PSR_AUX_CTL;
 
 	/* Setup AUX registers */
 	for (i = 0; i < sizeof(aux_msg); i += 4)
@@ -277,7 +277,7 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 		idle_frames += 4;
 	}
 
-	I915_WRITE(EDP_PSR_CTL(dev), val |
+	I915_WRITE(EDP_PSR_CTL, val |
 		   (IS_BROADWELL(dev) ? 0 : link_entry_time) |
 		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
 		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
@@ -341,7 +341,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
+	WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
 	WARN_ON(dev_priv->psr.active);
 	lockdep_assert_held(&dev_priv->psr.lock);
 
@@ -405,7 +405,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		}
 
 		/* Avoid continuous PSR exit by masking memup and hpd */
-		I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
+		I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP |
 			   EDP_PSR_DEBUG_MASK_HPD);
 
 		/* Enable PSR on the panel */
@@ -467,17 +467,17 @@ static void hsw_psr_disable(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (dev_priv->psr.active) {
-		I915_WRITE(EDP_PSR_CTL(dev),
-			   I915_READ(EDP_PSR_CTL(dev)) & ~EDP_PSR_ENABLE);
+		I915_WRITE(EDP_PSR_CTL,
+			   I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
 
 		/* Wait till PSR is idle */
-		if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev)) &
+		if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
 			       EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10))
 			DRM_ERROR("Timed out waiting for PSR Idle State\n");
 
 		dev_priv->psr.active = false;
 	} else {
-		WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE);
+		WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
 	}
 }
 
@@ -524,7 +524,7 @@ static void intel_psr_work(struct work_struct *work)
 	 * and be ready for re-enable.
 	 */
 	if (HAS_DDI(dev_priv->dev)) {
-		if (wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev_priv->dev)) &
+		if (wait_for((I915_READ(EDP_PSR_STATUS_CTL) &
 			      EDP_PSR_STATUS_STATE_MASK) == 0, 50)) {
 			DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
 			return;
@@ -567,11 +567,11 @@ static void intel_psr_exit(struct drm_device *dev)
 		return;
 
 	if (HAS_DDI(dev)) {
-		val = I915_READ(EDP_PSR_CTL(dev));
+		val = I915_READ(EDP_PSR_CTL);
 
 		WARN_ON(!(val & EDP_PSR_ENABLE));
 
-		I915_WRITE(EDP_PSR_CTL(dev), val & ~EDP_PSR_ENABLE);
+		I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
 	} else {
 		val = I915_READ(VLV_PSRCTL(pipe));
 
@@ -752,6 +752,9 @@ void intel_psr_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
+		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
+
 	INIT_DELAYED_WORK(&dev_priv->psr.work, intel_psr_work);
 	mutex_init(&dev_priv->psr.lock);
 }
-- 
2.4.10

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

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

* [PATCH 7/7] drm/i915: Model PSR AUX register selection more like the normal AUX code
  2015-11-11 18:34 [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series ville.syrjala
                   ` (5 preceding siblings ...)
  2015-11-11 18:34 ` [PATCH 6/7] drm/i915: Add dev_priv->psr_mmio_base ville.syrjala
@ 2015-11-11 18:34 ` ville.syrjala
  2015-11-12  9:37 ` [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series Jani Nikula
  7 siblings, 0 replies; 11+ messages in thread
From: ville.syrjala @ 2015-11-11 18:34 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

v2: Split up the ctl vs. data reg handling like in the normal AUX code

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_psr.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 3c973c5..6c32ca3 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -151,13 +151,31 @@ static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
 			   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
 }
 
+static uint32_t psr_aux_ctl_reg(struct drm_i915_private *dev_priv,
+				enum port port)
+{
+	if (INTEL_INFO(dev_priv)->gen >= 9)
+		return DP_AUX_CH_CTL(port);
+	else
+		return EDP_PSR_AUX_CTL;
+}
+
+static uint32_t psr_aux_data_reg(struct drm_i915_private *dev_priv,
+				 enum port port, int index)
+{
+	if (INTEL_INFO(dev_priv)->gen >= 9)
+		return DP_AUX_CH_DATA(port, index);
+	else
+		return EDP_PSR_AUX_DATA(index);
+}
+
 static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t aux_clock_divider;
-	uint32_t aux_data_reg, aux_ctl_reg;
+	uint32_t aux_ctl_reg;
 	int precharge = 0x3;
 	static const uint8_t aux_msg[] = {
 		[0] = DP_AUX_NATIVE_WRITE << 4,
@@ -182,14 +200,11 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 				DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
 				DP_AUX_FRAME_SYNC_ENABLE);
 
-	aux_data_reg = (INTEL_INFO(dev)->gen >= 9) ?
-		DP_AUX_CH_DATA(port, 0) : EDP_PSR_AUX_DATA(0);
-	aux_ctl_reg = (INTEL_INFO(dev)->gen >= 9) ?
-		DP_AUX_CH_CTL(port) : EDP_PSR_AUX_CTL;
+	aux_ctl_reg = psr_aux_ctl_reg(dev_priv, port);
 
 	/* Setup AUX registers */
 	for (i = 0; i < sizeof(aux_msg); i += 4)
-		I915_WRITE(aux_data_reg + i,
+		I915_WRITE(psr_aux_data_reg(dev_priv, port, i >> 2),
 			   intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
 
 	if (INTEL_INFO(dev)->gen >= 9) {
-- 
2.4.10

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

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

* Re: [PATCH 2/7] drm/i915: Replace the aux ddc name switch statement with kasprintf()
  2015-11-11 18:34 ` [PATCH 2/7] drm/i915: Replace the aux ddc name switch statement with kasprintf() ville.syrjala
@ 2015-11-12  9:23   ` Jani Nikula
  0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2015-11-12  9:23 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Wed, 11 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Use kasprintf() to generate the "DPDDC-<port>" name for the aux helper.
>
> To deal with errors properly make intel_dp_aux_init() return something,
> and adjust the caller to match. It seems we were also missing a
> intel_dp_mst_encoder_cleanup() call on edp (non-port A) init failures,
> so add that too.
>
> The whole error/cleanup ordering doesn't feel entirely sane to me, but
> I'll leave that part alone for now.
>
> v2: Use kasprintf() instead of a table, reorder patches (Chis)
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 75 +++++++++++++++++++++++++----------------
>  1 file changed, 46 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 891a7f8..df2a2d2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1009,6 +1009,13 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  }
>  
>  static void
> +intel_dp_aux_fini(struct intel_dp *intel_dp)
> +{
> +	drm_dp_aux_unregister(&intel_dp->aux);
> +	kfree(intel_dp->aux.name);
> +}
> +
> +static int
>  intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
>  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> @@ -1016,7 +1023,6 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	enum port port = intel_dig_port->port;
>  	struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
> -	const char *name = NULL;
>  	uint32_t porte_aux_ctl_reg = DPA_AUX_CH_CTL;
>  	int ret;
>  
> @@ -1043,23 +1049,18 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
>  	switch (port) {
>  	case PORT_A:
>  		intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
> -		name = "DPDDC-A";
>  		break;
>  	case PORT_B:
>  		intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
> -		name = "DPDDC-B";
>  		break;
>  	case PORT_C:
>  		intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
> -		name = "DPDDC-C";
>  		break;
>  	case PORT_D:
>  		intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
> -		name = "DPDDC-D";
>  		break;
>  	case PORT_E:
>  		intel_dp->aux_ch_ctl_reg = porte_aux_ctl_reg;
> -		name = "DPDDC-E";
>  		break;
>  	default:
>  		BUG();
> @@ -1077,27 +1078,36 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
>  	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev) && port != PORT_E)
>  		intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
>  
> -	intel_dp->aux.name = name;
> +	intel_dp->aux.name = kasprintf(GFP_KERNEL, "DPDDC-%c", port_name(port));
> +	if (!intel_dp->aux.name)
> +		return -ENOMEM;
> +
>  	intel_dp->aux.dev = dev->dev;
>  	intel_dp->aux.transfer = intel_dp_aux_transfer;
>  
> -	DRM_DEBUG_KMS("registering %s bus for %s\n", name,
> +	DRM_DEBUG_KMS("registering %s bus for %s\n",
> +		      intel_dp->aux.name,
>  		      connector->base.kdev->kobj.name);
>  
>  	ret = drm_dp_aux_register(&intel_dp->aux);
>  	if (ret < 0) {
>  		DRM_ERROR("drm_dp_aux_register() for %s failed (%d)\n",
> -			  name, ret);
> -		return;
> +			  intel_dp->aux.name, ret);
> +		kfree(intel_dp->aux.name);
> +		return ret;
>  	}
>  
>  	ret = sysfs_create_link(&connector->base.kdev->kobj,
>  				&intel_dp->aux.ddc.dev.kobj,
>  				intel_dp->aux.ddc.dev.kobj.name);
>  	if (ret < 0) {
> -		DRM_ERROR("sysfs_create_link() for %s failed (%d)\n", name, ret);
> -		drm_dp_aux_unregister(&intel_dp->aux);
> +		DRM_ERROR("sysfs_create_link() for %s failed (%d)\n",
> +			  intel_dp->aux.name, ret);
> +		intel_dp_aux_fini(intel_dp);
> +		return ret;
>  	}
> +
> +	return 0;
>  }
>  
>  static void
> @@ -4771,7 +4781,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  
> -	drm_dp_aux_unregister(&intel_dp->aux);
> +	intel_dp_aux_fini(intel_dp);
>  	intel_dp_mst_encoder_cleanup(intel_dig_port);
>  	if (is_edp(intel_dp)) {
>  		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
> @@ -5752,7 +5762,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  	struct drm_device *dev = intel_encoder->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum port port = intel_dig_port->port;
> -	int type;
> +	int type, ret;
>  
>  	intel_dp->pps_pipe = INVALID_PIPE;
>  
> @@ -5853,7 +5863,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  		pps_unlock(intel_dp);
>  	}
>  
> -	intel_dp_aux_init(intel_dp, intel_connector);
> +	ret = intel_dp_aux_init(intel_dp, intel_connector);
> +	if (ret)
> +		goto fail;
>  
>  	/* init MST on ports that can support it */
>  	if (HAS_DP_MST(dev) &&
> @@ -5862,20 +5874,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  					  intel_connector->base.base.id);
>  
>  	if (!intel_edp_init_connector(intel_dp, intel_connector)) {
> -		drm_dp_aux_unregister(&intel_dp->aux);
> -		if (is_edp(intel_dp)) {
> -			cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
> -			/*
> -			 * vdd might still be enabled do to the delayed vdd off.
> -			 * Make sure vdd is actually turned off here.
> -			 */
> -			pps_lock(intel_dp);
> -			edp_panel_vdd_off_sync(intel_dp);
> -			pps_unlock(intel_dp);
> -		}
> -		drm_connector_unregister(connector);
> -		drm_connector_cleanup(connector);
> -		return false;
> +		intel_dp_aux_fini(intel_dp);
> +		intel_dp_mst_encoder_cleanup(intel_dig_port);
> +		goto fail;
>  	}
>  
>  	intel_dp_add_properties(intel_dp, connector);
> @@ -5892,6 +5893,22 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  	i915_debugfs_connector_add(connector);
>  
>  	return true;
> +
> +fail:
> +	if (is_edp(intel_dp)) {
> +		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
> +		/*
> +		 * vdd might still be enabled do to the delayed vdd off.
> +		 * Make sure vdd is actually turned off here.
> +		 */
> +		pps_lock(intel_dp);
> +		edp_panel_vdd_off_sync(intel_dp);
> +		pps_unlock(intel_dp);
> +	}

A follow-up might abstract this sequence as there's three copies now.

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


> +	drm_connector_unregister(connector);
> +	drm_connector_cleanup(connector);
> +
> +	return false;
>  }
>  
>  void

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

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

* Re: [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series
  2015-11-11 18:34 [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series ville.syrjala
                   ` (6 preceding siblings ...)
  2015-11-11 18:34 ` [PATCH 7/7] drm/i915: Model PSR AUX register selection more like the normal AUX code ville.syrjala
@ 2015-11-12  9:37 ` Jani Nikula
  2015-11-16 14:22   ` Ville Syrjälä
  7 siblings, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2015-11-12  9:37 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Wed, 11 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Chris requested that I try to reorder the DP AUX patches in the last
> register type safety series [1] to form a better story. Here is the
> result. The final code is exactly the same as before (apart from the
> kasprintf() changes), so I kept the previous r-b's in place, with
> some added version annotations.

I reviewed the one patch that was missing r-b, and eyeballed the ones
with previous r-b's a bit, and it all looks good to go.

BR,
Jani.


>
> [1] http://lists.freedesktop.org/archives/intel-gfx/2015-November/079453.html
>
> Ville Syrjälä (7):
>   drm/i915: Replace aux_ch_ctl_reg check with port check
>   drm/i915: Replace the aux ddc name switch statement with kasprintf()
>   drm/i915: Parametrize AUX registers
>   drm/i915: Remove the magic AUX_CTL is at DP + foo tricks
>   drm/i915: Store aux data reg offsets in intel_dp->aux_ch_data_reg[]
>   drm/i915: Add dev_priv->psr_mmio_base
>   drm/i915: Model PSR AUX register selection more like the normal AUX
>     code
>
>  drivers/gpu/drm/i915/i915_debugfs.c |   4 +-
>  drivers/gpu/drm/i915/i915_drv.h     |   2 +
>  drivers/gpu/drm/i915/i915_reg.h     | 115 ++++++++-------
>  drivers/gpu/drm/i915/intel_dp.c     | 282 +++++++++++++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_drv.h    |   1 +
>  drivers/gpu/drm/i915/intel_psr.c    |  51 +++++--
>  6 files changed, 298 insertions(+), 157 deletions(-)

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

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

* Re: [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series
  2015-11-12  9:37 ` [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series Jani Nikula
@ 2015-11-16 14:22   ` Ville Syrjälä
  0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2015-11-16 14:22 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Thu, Nov 12, 2015 at 11:37:11AM +0200, Jani Nikula wrote:
> On Wed, 11 Nov 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Chris requested that I try to reorder the DP AUX patches in the last
> > register type safety series [1] to form a better story. Here is the
> > result. The final code is exactly the same as before (apart from the
> > kasprintf() changes), so I kept the previous r-b's in place, with
> > some added version annotations.
> 
> I reviewed the one patch that was missing r-b, and eyeballed the ones
> with previous r-b's a bit, and it all looks good to go.

Thanks for the reviews. Series pushed to dinq.

> 
> BR,
> Jani.
> 
> 
> >
> > [1] http://lists.freedesktop.org/archives/intel-gfx/2015-November/079453.html
> >
> > Ville Syrjälä (7):
> >   drm/i915: Replace aux_ch_ctl_reg check with port check
> >   drm/i915: Replace the aux ddc name switch statement with kasprintf()
> >   drm/i915: Parametrize AUX registers
> >   drm/i915: Remove the magic AUX_CTL is at DP + foo tricks
> >   drm/i915: Store aux data reg offsets in intel_dp->aux_ch_data_reg[]
> >   drm/i915: Add dev_priv->psr_mmio_base
> >   drm/i915: Model PSR AUX register selection more like the normal AUX
> >     code
> >
> >  drivers/gpu/drm/i915/i915_debugfs.c |   4 +-
> >  drivers/gpu/drm/i915/i915_drv.h     |   2 +
> >  drivers/gpu/drm/i915/i915_reg.h     | 115 ++++++++-------
> >  drivers/gpu/drm/i915/intel_dp.c     | 282 +++++++++++++++++++++++++-----------
> >  drivers/gpu/drm/i915/intel_drv.h    |   1 +
> >  drivers/gpu/drm/i915/intel_psr.c    |  51 +++++--
> >  6 files changed, 298 insertions(+), 157 deletions(-)
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

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

end of thread, other threads:[~2015-11-16 14:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-11 18:34 [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series ville.syrjala
2015-11-11 18:34 ` [PATCH 1/7] drm/i915: Replace aux_ch_ctl_reg check with port check ville.syrjala
2015-11-11 18:34 ` [PATCH 2/7] drm/i915: Replace the aux ddc name switch statement with kasprintf() ville.syrjala
2015-11-12  9:23   ` Jani Nikula
2015-11-11 18:34 ` [PATCH 3/7] drm/i915: Parametrize AUX registers ville.syrjala
2015-11-11 18:34 ` [PATCH 4/7] drm/i915: Remove the magic AUX_CTL is at DP + foo tricks ville.syrjala
2015-11-11 18:34 ` [PATCH 5/7] drm/i915: Store aux data reg offsets in intel_dp->aux_ch_data_reg[] ville.syrjala
2015-11-11 18:34 ` [PATCH 6/7] drm/i915: Add dev_priv->psr_mmio_base ville.syrjala
2015-11-11 18:34 ` [PATCH 7/7] drm/i915: Model PSR AUX register selection more like the normal AUX code ville.syrjala
2015-11-12  9:37 ` [PATCH 0/7] drm/i915: Reordered AUX patches from type safety series Jani Nikula
2015-11-16 14:22   ` Ville Syrjälä

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.