drm/i915: switch to drm core dp aux helpers

PATCH In review Jani Nikula 3 participants 20 messages Mar 14, 2014 – Mar 18, 2014

Subsystems (3)

DRM drivers DRM misc Intel i915

lore

Share & tools

lore

Jani Nikula

author

2014-03-14 14:51 UTC #0 lore raw

cover drm/i915: switch to drm core dp aux helpers

Switch over to using Thierry's drm dp aux helpers.

The previous RFC series (from before the helpers were merged) is at [1]. I specifically decided to dodge the sleep state issues pointed out by Ville [2] for now; this series does not make us better or worse in this regard.

Only briefly tested on SNB, but seems to work.

Patch 1 has been posted and reviewed on dri-devel already [3], and is included here for completeness.

(These patches demonstrate that git format-patch needs a "function rewrite" option... the diffs in patches 4 and 6 turned out quite hideous with mixed + and - lines.)

BR,
Jani.

[1] http://mid.gmane.org/[email protected]
[2] http://mid.gmane.org/[email protected]
[3] http://mid.gmane.org/[email protected]

Jani Nikula (6):
drm/dp: let drivers specify the name of the I2C-over-AUX adapter drm/i915/dp: split edp_panel_vdd_on() for reuse drm/i915/dp: move edp vdd enable/disable at a lower level in
i2c-over-aux
drm/i915/dp: use the new drm helpers for dp aux drm/i915/dp: move dp aux ch register init to aux init drm/i915/dp: use the new drm helpers for dp i2c-over-aux

 drivers/gpu/drm/drm_dp_helper.c  |    3 +-
 drivers/gpu/drm/i915/intel_dp.c  |  513 ++++++++++++++------------------------
 drivers/gpu/drm/i915/intel_drv.h |    3 +-
 include/drm/drm_dp_helper.h      |    4 +
 4 files changed, 189 insertions(+), 334 deletions(-)

Signature · 1 line

1.7.9.5

Jani Nikula

author

2014-03-14 14:51 UTC #1 lore raw

PATCH 1/6 drm/dp: let drivers specify the name of the I2C-over-AUX adapter

Let the drivers specify the name of the I2C-over-AUX adapter to maintain backwards compatibility in the sysfs when converting to the new I2C-over-AUX helper infrastructure.

The i915 driver currently uses DPDDC-A to DPDDC-D as names for the DP i2c adapters. These names show up in the i2c sysfs name attribute. We'd like to be able to maintain that when switching over to the new helpers.

Due to i2c device and connector cleanup ordering issues we also recently made the drm device (instead of connector) the parent of the i2c adapters:

commit 80f65de3c9b8101c1613fa82df500ba6a099a11c

Author: Imre Deak <[email protected]>
Date:   Tue Feb 11 17:12:49 2014 +0200

drm/i915: dp: fix order of dp aux i2c device cleanup

With the name picked up from the adapter parent using dev_name(), it would be the same for all i2c adapters with the current I2C-over-AUX helpers.

Signed-off-by Jani Nikula <[email protected]>

Reviewed-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/drm_dp_helper.c |    3 ++-
 include/drm/drm_dp_helper.h     |    4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

drivers/gpu/drm/drm_dp_helper.c +2 -1

index 35251af3b14e..17832d048147 100644
@@ -726,7 +726,8 @@ int drm_dp_aux_register_i2c_bus(struct drm_dp_aux *aux)
 	aux->ddc.dev.parent = aux->dev;
 	aux->ddc.dev.of_node = aux->dev->of_node;
 
-	strncpy(aux->ddc.name, dev_name(aux->dev), sizeof(aux->ddc.name));
+	strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
+		sizeof(aux->ddc.name));
 
 	return i2c_add_adapter(&aux->ddc);
 }

include/drm/drm_dp_helper.h +4 -0

index 42947566e755..b4f58914bf7d 100644
@@ -438,6 +438,9 @@ struct drm_dp_aux_msg {
  * The .dev field should be set to a pointer to the device that implements
  * the AUX channel.
  *
+ * The .name field may be used to specify the name of the I2C adapter. If set to
+ * NULL, dev_name() of .dev will be used.
+ *
  * Drivers provide a hardware-specific implementation of how transactions
  * are executed via the .transfer() function. A pointer to a drm_dp_aux_msg
  * structure describing the transaction is passed into this function. Upon

@@ -455,6 +458,7 @@ struct drm_dp_aux_msg {
  * should call drm_dp_aux_unregister_i2c_bus() to remove the I2C adapter.
  */
 struct drm_dp_aux {
+	const char *name;
 	struct i2c_adapter ddc;
 	struct device *dev;

Signature · 1 line

1.7.9.5

Rodrigo Vivi

Reviewed-by on v1 1/6

2014-03-17 14:09 UTC #2 lore raw

Re: [PATCH 1/6] drm/dp: let drivers specify the name of the I2C-over-AUX adapter

Reviewed-by Rodrigo Vivi <[email protected]>

//Although I know it has been already reviewed ;)

Quoted 72 lines · Jani Nikula

Let the drivers specify the name of the I2C-over-AUX adapter to maintain backwards compatibility in the sysfs when converting to the new I2C-over-AUX helper infrastructure.
The i915 driver currently uses DPDDC-A to DPDDC-D as names for the DP i2c adapters. These names show up in the i2c sysfs name attribute. We'd like to be able to maintain that when switching over to the new helpers.
Due to i2c device and connector cleanup ordering issues we also recently made the drm device (instead of connector) the parent of the i2c adapters:
commit 80f65de3c9b8101c1613fa82df500ba6a099a11c
Author: Imre Deak <[email protected]>
Date:   Tue Feb 11 17:12:49 2014 +0200
drm/i915: dp: fix order of dp aux i2c device cleanup
With the name picked up from the adapter parent using dev_name(), it would be the same for all i2c adapters with the current I2C-over-AUX helpers.
Signed-off-by Jani Nikula <[email protected]>
Reviewed-by Thierry Reding <[email protected]>
---
 drivers/gpu/drm/drm_dp_helper.c |    3 ++-
 include/drm/drm_dp_helper.h     |    4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)
drivers/gpu/drm/drm_dp_helper.c +2 -1
index 35251af3b14e..17832d048147 100644
@@ -726,7 +726,8 @@ int drm_dp_aux_register_i2c_bus(struct drm_dp_aux *aux)
        aux->ddc.dev.parent = aux->dev;
        aux->ddc.dev.of_node = aux->dev->of_node;
 
-       strncpy(aux->ddc.name, dev_name(aux->dev), sizeof(aux->ddc.name));
+       strlcpy(aux->ddc.name, aux->name ? aux->name : dev_name(aux->dev),
+               sizeof(aux->ddc.name));
 
        return i2c_add_adapter(&aux->ddc);
 }
include/drm/drm_dp_helper.h +4 -1
index 42947566e755..b4f58914bf7d 100644
@@ -438,6 +438,9 @@ struct drm_dp_aux_msg {
  * The .dev field should be set to a pointer to the device that implements
  * the AUX channel.
  *
+ * The .name field may be used to specify the name of the I2C adapter. If set to
+ * NULL, dev_name() of .dev will be used.
+ *
  * Drivers provide a hardware-specific implementation of how transactions
  * are executed via the .transfer() function. A pointer to a drm_dp_aux_msg
  * structure describing the transaction is passed into this function. Upon
@@ -455,6 +458,7 @@ struct drm_dp_aux_msg {
  * should call drm_dp_aux_unregister_i2c_bus() to remove the I2C adapter.
  */
 struct drm_dp_aux {
+       const char *name;
        struct i2c_adapter ddc;
        struct device *dev;
 
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

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

Jani Nikula

author

2014-03-14 14:51 UTC #3 lore raw

PATCH 2/6 drm/i915/dp: split edp_panel_vdd_on() for reuse

In reply to Jani Nikula

Introduce _edp_panel_vdd_on() that returns true if the call enabled vdd, and a matching disable is needed. Keep edp_panel_vdd_on() as a helper for when it is expected the vdd is off.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +16 -6

index bb66f9301cd9..94e7577da484 100644
@@ -91,6 +91,7 @@ static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
 }
 
 static void intel_dp_link_down(struct intel_dp *intel_dp);
+static bool _edp_panel_vdd_on(struct intel_dp *intel_dp);
 static void edp_panel_vdd_on(struct intel_dp *intel_dp);
 static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);

@@ -1162,23 +1163,21 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
 	return control;
 }
 
-static void edp_panel_vdd_on(struct intel_dp *intel_dp)
+static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 pp;
 	u32 pp_stat_reg, pp_ctrl_reg;
+	bool need_to_disable = !intel_dp->want_panel_vdd;
 
 	if (!is_edp(intel_dp))
-		return;
-
-	WARN(intel_dp->want_panel_vdd,
-	     "eDP VDD already requested on\n");
+		return false;
 
 	intel_dp->want_panel_vdd = true;
 
 	if (edp_have_panel_vdd(intel_dp))
-		return;
+		return need_to_disable;
 
 	intel_runtime_pm_get(dev_priv);

@@ -1204,6 +1203,17 @@ static void edp_panel_vdd_on(struct intel_dp *intel_dp)
 		DRM_DEBUG_KMS("eDP was not running\n");
 		msleep(intel_dp->panel_power_up_delay);
 	}
+
+	return need_to_disable;
+}
+
+static void edp_panel_vdd_on(struct intel_dp *intel_dp)
+{
+	if (is_edp(intel_dp)) {
+		bool vdd = _edp_panel_vdd_on(intel_dp);
+
+		WARN(!vdd, "eDP VDD already requested on\n");
+	}
 }
 
 static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)

Signature · 1 line

1.7.9.5

Rodrigo Vivi

Reviewed-by on v1 2/6

2014-03-17 14:09 UTC #4 lore raw

Re: [PATCH 2/6] drm/i915/dp: split edp_panel_vdd_on() for reuse

Reviewed-by Rodrigo Vivi <[email protected]>

Quoted 74 lines · Jani Nikula

Introduce _edp_panel_vdd_on() that returns true if the call enabled vdd, and a matching disable is needed. Keep edp_panel_vdd_on() as a helper for when it is expected the vdd is off.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +16 -7

index bb66f9301cd9..94e7577da484 100644
@@ -91,6 +91,7 @@ static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
 }
 
 static void intel_dp_link_down(struct intel_dp *intel_dp);
+static bool _edp_panel_vdd_on(struct intel_dp *intel_dp);
 static void edp_panel_vdd_on(struct intel_dp *intel_dp);
 static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);

@@ -1162,23 +1163,21 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
        return control;
 }
 
-static void edp_panel_vdd_on(struct intel_dp *intel_dp)
+static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
 {
        struct drm_device *dev = intel_dp_to_dev(intel_dp);
        struct drm_i915_private *dev_priv = dev->dev_private;
        u32 pp;
        u32 pp_stat_reg, pp_ctrl_reg;
+       bool need_to_disable = !intel_dp->want_panel_vdd;
 
        if (!is_edp(intel_dp))
-               return;
-
-       WARN(intel_dp->want_panel_vdd,
-            "eDP VDD already requested on\n");
+               return false;
 
        intel_dp->want_panel_vdd = true;
 
        if (edp_have_panel_vdd(intel_dp))
-               return;
+               return need_to_disable;
 
        intel_runtime_pm_get(dev_priv);

@@ -1204,6 +1203,17 @@ static void edp_panel_vdd_on(struct intel_dp *intel_dp)
                DRM_DEBUG_KMS("eDP was not running\n");
                msleep(intel_dp->panel_power_up_delay);
        }
+
+       return need_to_disable;
+}
+
+static void edp_panel_vdd_on(struct intel_dp *intel_dp)
+{
+       if (is_edp(intel_dp)) {
+               bool vdd = _edp_panel_vdd_on(intel_dp);
+
+               WARN(!vdd, "eDP VDD already requested on\n");
+       }
 }
 
 static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
--

1.7.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

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

Rodrigo Vivi

2014-03-17 14:47 UTC #5 lore raw

Re: [PATCH 2/6] drm/i915/dp: split edp_panel_vdd_on() for reuse

forgot to say that this patch needs a rebase on -nightly

Quoted 83 lines · Rodrigo Vivi

Reviewed-by Rodrigo Vivi <[email protected]>

Quoted 70 lines · Jani Nikula

Introduce _edp_panel_vdd_on() that returns true if the call enabled vdd, and a matching disable is needed. Keep edp_panel_vdd_on() as a helper for when it is expected the vdd is off.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +16 -7

index bb66f9301cd9..94e7577da484 100644
@@ -91,6 +91,7 @@ static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
 }
 
 static void intel_dp_link_down(struct intel_dp *intel_dp);
+static bool _edp_panel_vdd_on(struct intel_dp *intel_dp);
 static void edp_panel_vdd_on(struct intel_dp *intel_dp);
 static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);

@@ -1162,23 +1163,21 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
        return control;
 }
 
-static void edp_panel_vdd_on(struct intel_dp *intel_dp)
+static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
 {
        struct drm_device *dev = intel_dp_to_dev(intel_dp);
        struct drm_i915_private *dev_priv = dev->dev_private;
        u32 pp;
        u32 pp_stat_reg, pp_ctrl_reg;
+       bool need_to_disable = !intel_dp->want_panel_vdd;
 
        if (!is_edp(intel_dp))
-               return;
-
-       WARN(intel_dp->want_panel_vdd,
-            "eDP VDD already requested on\n");
+               return false;
 
        intel_dp->want_panel_vdd = true;
 
        if (edp_have_panel_vdd(intel_dp))
-               return;
+               return need_to_disable;
 
        intel_runtime_pm_get(dev_priv);

@@ -1204,6 +1203,17 @@ static void edp_panel_vdd_on(struct intel_dp *intel_dp)
                DRM_DEBUG_KMS("eDP was not running\n");
                msleep(intel_dp->panel_power_up_delay);
        }
+
+       return need_to_disable;
+}
+
+static void edp_panel_vdd_on(struct intel_dp *intel_dp)
+{
+       if (is_edp(intel_dp)) {
+               bool vdd = _edp_panel_vdd_on(intel_dp);
+
+               WARN(!vdd, "eDP VDD already requested on\n");
+       }
 }
 
 static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
--

1.7.9.5

Quoted 4 lines

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Signature · 2 lines

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

Jani Nikula

author

2014-03-14 14:51 UTC #6 lore raw

PATCH 3/6 drm/i915/dp: move edp vdd enable/disable at a lower level in i2c-over-aux

In reply to Jani Nikula

This is prep work for conversion to generic drm i2c-over-aux helpers where we won't have the function to do this at.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +6 -3

index 94e7577da484..22134edc03dd 100644
@@ -461,6 +461,9 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	uint32_t status;
 	int try, clock = 0;
 	bool has_aux_irq = HAS_AUX_IRQ(dev);
+	bool vdd;
+
+	vdd = _edp_panel_vdd_on(intel_dp);
 
 	/* dp aux is extremely sensitive to irq latency, hence request the
 	 * lowest possible wakeup latency and so prevent the cpu from going into

@@ -566,6 +569,9 @@ out:
 	pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
 	intel_aux_display_runtime_put(dev_priv);
 
+	if (vdd)
+		edp_panel_vdd_off(intel_dp, false);
+
 	return ret;
 }

@@ -678,8 +684,6 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
 	int reply_bytes;
 	int ret;
 
-	edp_panel_vdd_on(intel_dp);
-	intel_dp_check_edp(intel_dp);
 	/* Set up the command byte */
 	if (mode & MODE_I2C_READ)
 		msg[0] = DP_AUX_I2C_READ << 4;

@@ -781,7 +785,6 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
 	ret = -EREMOTEIO;
 
 out:
-	edp_panel_vdd_off(intel_dp, false);
 	return ret;
 }

Signature · 1 line

1.7.9.5

Rodrigo Vivi

Reviewed-by on v1 3/6

2014-03-17 14:09 UTC #7 lore raw

Re: [PATCH 3/6] drm/i915/dp: move edp vdd enable/disable at a lower level in i2c-over-aux

Reviewed-by Rodrigo Vivi <[email protected]>

Quoted 56 lines · Jani Nikula

This is prep work for conversion to generic drm i2c-over-aux helpers where we won't have the function to do this at.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +6 -4

index 94e7577da484..22134edc03dd 100644
@@ -461,6 +461,9 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
        uint32_t status;
        int try, clock = 0;
        bool has_aux_irq = HAS_AUX_IRQ(dev);
+       bool vdd;
+
+       vdd = _edp_panel_vdd_on(intel_dp);
 
        /* dp aux is extremely sensitive to irq latency, hence request the
         * lowest possible wakeup latency and so prevent the cpu from going into

@@ -566,6 +569,9 @@ out:
        pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
        intel_aux_display_runtime_put(dev_priv);
 
+       if (vdd)
+               edp_panel_vdd_off(intel_dp, false);
+
        return ret;
 }

@@ -678,8 +684,6 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
        int reply_bytes;
        int ret;
 
-       edp_panel_vdd_on(intel_dp);
-       intel_dp_check_edp(intel_dp);
        /* Set up the command byte */
        if (mode & MODE_I2C_READ)
                msg[0] = DP_AUX_I2C_READ << 4;

@@ -781,7 +785,6 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
        ret = -EREMOTEIO;
 
 out:
-       edp_panel_vdd_off(intel_dp, false);
        return ret;
 }
 
--

1.7.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

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

Jani Nikula

author

2014-03-14 14:51 UTC #8 lore raw

PATCH 4/6 drm/i915/dp: use the new drm helpers for dp aux

In reply to Jani Nikula

Functionality remains largely the same as before.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c  |  257 +++++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 2 files changed, 116 insertions(+), 142 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +115 ~~-142~~

index 22134edc03dd..24cbf4bd36c4 100644
@@ -575,97 +575,77 @@ out:
 	return ret;
 }
 
-/* Write data to the aux channel in native mode */
-static int
-intel_dp_aux_native_write(struct intel_dp *intel_dp,
-			  uint16_t address, uint8_t *send, int send_bytes)
+#define HEADER_SIZE	4
+static ssize_t
+intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
+	struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
+	uint8_t txbuf[20], rxbuf[20];
+	size_t txsize, rxsize;
 	int ret;
-	uint8_t	msg[20];
-	int msg_bytes;
-	uint8_t	ack;
-	int retry;
 
-	if (WARN_ON(send_bytes > 16))
-		return -E2BIG;
+	txbuf[0] = msg->request << 4;
+	txbuf[1] = msg->address >> 8;
+	txbuf[2] = msg->address & 0xff;
+	txbuf[3] = msg->size - 1;
 
-	intel_dp_check_edp(intel_dp);
-	msg[0] = DP_AUX_NATIVE_WRITE << 4;
-	msg[1] = address >> 8;
-	msg[2] = address & 0xff;
-	msg[3] = send_bytes - 1;
-	memcpy(&msg[4], send, send_bytes);
-	msg_bytes = send_bytes + 4;
-	for (retry = 0; retry < 7; retry++) {
-		ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1);
-		if (ret < 0)
-			return ret;
-		ack >>= 4;
-		if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
-			return send_bytes;
-		else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-			usleep_range(400, 500);
-		else
-			return -EIO;
-	}
+	switch (msg->request & ~DP_AUX_I2C_MOT) {
+	case DP_AUX_NATIVE_WRITE:
+	case DP_AUX_I2C_WRITE:
+		txsize = HEADER_SIZE + msg->size;
+		rxsize = 1;
 
-	DRM_ERROR("too many retries, giving up\n");
-	return -EIO;
-}
+		if (WARN_ON(txsize > 20))
+			return -E2BIG;
 
-/* Write a single byte to the aux channel in native mode */
-static int
-intel_dp_aux_native_write_1(struct intel_dp *intel_dp,
-			    uint16_t address, uint8_t byte)
-{
-	return intel_dp_aux_native_write(intel_dp, address, &byte, 1);
-}
+		memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
 
-/* read bytes from a native aux channel */
-static int
-intel_dp_aux_native_read(struct intel_dp *intel_dp,
-			 uint16_t address, uint8_t *recv, int recv_bytes)
-{
-	uint8_t msg[4];
-	int msg_bytes;
-	uint8_t reply[20];
-	int reply_bytes;
-	uint8_t ack;
-	int ret;
-	int retry;
+		ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+		if (ret > 0) {
+			msg->reply = rxbuf[0] >> 4;
 
-	if (WARN_ON(recv_bytes > 19))
-		return -E2BIG;
+			/* Return payload size. */
+			ret = msg->size;
+		}
+		break;
 
-	intel_dp_check_edp(intel_dp);
-	msg[0] = DP_AUX_NATIVE_READ << 4;
-	msg[1] = address >> 8;
-	msg[2] = address & 0xff;
-	msg[3] = recv_bytes - 1;
+	case DP_AUX_NATIVE_READ:
+	case DP_AUX_I2C_READ:
+		txsize = HEADER_SIZE;
+		rxsize = msg->size + 1;
 
-	msg_bytes = 4;
-	reply_bytes = recv_bytes + 1;
+		if (WARN_ON(rxsize > 20))
+			return -E2BIG;
 
-	for (retry = 0; retry < 7; retry++) {
-		ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes,
-				      reply, reply_bytes);
-		if (ret == 0)
-			return -EPROTO;
-		if (ret < 0)
-			return ret;
-		ack = reply[0] >> 4;
-		if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) {
-			memcpy(recv, reply + 1, ret - 1);
-			return ret - 1;
+		ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+		if (ret > 0) {
+			msg->reply = rxbuf[0] >> 4;
+			/*
+			 * Assume happy day, and copy the data. The caller is
+			 * expected to check msg->reply before touching it.
+			 *
+			 * Return payload size.
+			 */
+			ret--;
+			memcpy(msg->buffer, rxbuf + 1, ret);
 		}
-		else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-			usleep_range(400, 500);
-		else
-			return -EIO;
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
 	}
 
-	DRM_ERROR("too many retries, giving up\n");
-	return -EIO;
+	return ret;
+}
+
+static void
+intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+
+	intel_dp->aux.dev = dev->dev;
+	intel_dp->aux.transfer = intel_dp_aux_transfer;
 }
 
 static int

@@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
 		return;
 
 	if (mode != DRM_MODE_DPMS_ON) {
-		ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER,
-						  DP_SET_POWER_D3);
+		ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+					 DP_SET_POWER_D3);
 		if (ret != 1)
 			DRM_DEBUG_DRIVER("failed to write sink power state\n");
 	} else {

@@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
 		 * time to wake up.
 		 */
 		for (i = 0; i < 3; i++) {
-			ret = intel_dp_aux_native_write_1(intel_dp,
-							  DP_SET_POWER,
-							  DP_SET_POWER_D0);
+			ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+						 DP_SET_POWER_D0);
 			if (ret == 1)
 				break;
 			msleep(1);

@@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 
 	/* Enable PSR in sink */
 	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT)
-		intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-					    DP_PSR_ENABLE &
-					    ~DP_PSR_MAIN_LINK_ACTIVE);
+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
 	else
-		intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-					    DP_PSR_ENABLE |
-					    DP_PSR_MAIN_LINK_ACTIVE);
+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
 
 	/* Setup AUX registers */
 	I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND);

@@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder)
 /*
  * Native read with retry for link status and receiver capability reads for
  * cases where the sink may still be asleep.
+ *
+ * Sinks are *supposed* to come up within 1ms from an off state, but we're also
+ * supposed to retry 3 times per the spec.
  */
-static bool
-intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
-			       uint8_t *recv, int recv_bytes)
+static ssize_t
+intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset,
+			void *buffer, size_t size)
 {
-	int ret, i;
+	ssize_t ret;
+	int i;
 
-	/*
-	 * Sinks are *supposed* to come up within 1ms from an off state,
-	 * but we're also supposed to retry 3 times per the spec.
-	 */
 	for (i = 0; i < 3; i++) {
-		ret = intel_dp_aux_native_read(intel_dp, address, recv,
-					       recv_bytes);
-		if (ret == recv_bytes)
-			return true;
+		ret = drm_dp_dpcd_read(aux, offset, buffer, size);
+		if (ret == size)
+			return ret;
 		msleep(1);
 	}
 
-	return false;
+	return ret;
 }
 
 /*

@@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
 static bool
 intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE])
 {
-	return intel_dp_aux_native_read_retry(intel_dp,
-					      DP_LANE0_1_STATUS,
-					      link_status,
-					      DP_LINK_STATUS_SIZE);
+	return intel_dp_dpcd_read_wake(&intel_dp->aux,
+				       DP_LANE0_1_STATUS,
+				       link_status,
+				       DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
 }
 
 /*

@@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 		len = intel_dp->lane_count + 1;
 	}
 
-	ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET,
-					buf, len);
+	ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
+				buf, len);
 
 	return ret == len;
 }

@@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
 	I915_WRITE(intel_dp->output_reg, *DP);
 	POSTING_READ(intel_dp->output_reg);
 
-	ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET,
-					intel_dp->train_set,
-					intel_dp->lane_count);
+	ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
+				intel_dp->train_set, intel_dp->lane_count);
 
 	return ret == intel_dp->lane_count;
 }

@@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 	link_config[1] = intel_dp->lane_count;
 	if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
 		link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
-	intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2);
+	drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
 
 	link_config[0] = 0;
 	link_config[1] = DP_SET_ANSI_8B10B;
-	intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2);
+	drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
 
 	DP |= DP_PORT_EN;

@@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 
 	char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
 
-	if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-					   sizeof(intel_dp->dpcd)) == 0)
+	if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd,
+				    sizeof(intel_dp->dpcd)) < 0)
 		return false; /* aux transfer failed */
 
 	hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),

@@ -2921,9 +2896,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	/* Check if the panel supports PSR */
 	memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
 	if (is_edp(intel_dp)) {
-		intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
-					       intel_dp->psr_dpcd,
-					       sizeof(intel_dp->psr_dpcd));
+		intel_dp_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT,
+					intel_dp->psr_dpcd,
+					sizeof(intel_dp->psr_dpcd));
 		if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
 			dev_priv->psr.sink_support = true;
 			DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");

@@ -2945,9 +2920,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
 		return true; /* no per-port downstream info */
 
-	if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
-					   intel_dp->downstream_ports,
-					   DP_MAX_DOWNSTREAM_PORTS) == 0)
+	if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0,
+				    intel_dp->downstream_ports,
+				    DP_MAX_DOWNSTREAM_PORTS) < 0)
 		return false; /* downstream port status fetch failed */
 
 	return true;

@@ -2963,11 +2938,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
 
 	edp_panel_vdd_on(intel_dp);
 
-	if (intel_dp_aux_native_read_retry(intel_dp, DP_SINK_OUI, buf, 3))
+	if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3)
 		DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
 			      buf[0], buf[1], buf[2]);
 
-	if (intel_dp_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3))
+	if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3)
 		DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
 			      buf[0], buf[1], buf[2]);

@@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
 		to_intel_crtc(intel_dig_port->base.base.crtc);
 	u8 buf[1];
 
-	if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1))
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0);
 		return -EAGAIN;
 
 	if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
 		return -ENOTTY;
 
-	if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK,
-					 DP_TEST_SINK_START))
+	if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
+			       DP_TEST_SINK_START) < 0)
 		return -EAGAIN;
 
 	/* Wait 2 vblanks to be sure we will have the correct CRC value */
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
 
-	if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6))
+	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
 		return -EAGAIN;
 
-	intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0);
+	drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0);
 	return 0;
 }
 
 static bool
 intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 {
-	int ret;
-
-	ret = intel_dp_aux_native_read_retry(intel_dp,
-					     DP_DEVICE_SERVICE_IRQ_VECTOR,
-					     sink_irq_vector, 1);
-	if (!ret)
-		return false;
-
-	return true;
+	return intel_dp_dpcd_read_wake(&intel_dp->aux,
+				       DP_DEVICE_SERVICE_IRQ_VECTOR,
+				       sink_irq_vector, 1) == 1;
 }
 
 static void
 intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
 	/* NAK by default */
-	intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK);
+	drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK);
 }
 
 /*

@@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
 	    intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
 		/* Clear interrupt source */
-		intel_dp_aux_native_write_1(intel_dp,
-					    DP_DEVICE_SERVICE_IRQ_VECTOR,
-					    sink_irq_vector);
+		drm_dp_dpcd_writeb(&intel_dp->aux,
+				   DP_DEVICE_SERVICE_IRQ_VECTOR,
+				   sink_irq_vector);
 
 		if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
 			intel_dp_handle_test_request(intel_dp);

@@ -3097,9 +3066,11 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
 	    intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
 		uint8_t reg;
-		if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
-						    &reg, 1))
+
+		if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
+					    &reg, 1) < 0)
 			return connector_status_unknown;
+
 		return DP_GET_SINK_COUNT(reg) ? connector_status_connected
 					      : connector_status_disconnected;
 	}

@@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
 	}
 
+	intel_dp_aux_init(intel_dp, intel_connector);
+
 	error = intel_dp_i2c_init(intel_dp, intel_connector, name);
 	WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
 	     error, port_name(port));

drivers/gpu/drm/i915/intel_drv.h +1 -0

index 2546cae0b4f0..561af4b013b1 100644
@@ -491,6 +491,7 @@ struct intel_dp {
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	struct i2c_adapter adapter;
 	struct i2c_algo_dp_aux_data algo;
+	struct drm_dp_aux aux;
 	uint8_t train_set[4];
 	int panel_power_up_delay;
 	int panel_power_down_delay;

Signature · 1 line

1.7.9.5

Rodrigo Vivi

2014-03-17 14:08 UTC #9 lore raw

Re: [PATCH 4/6] drm/i915/dp: use the new drm helpers for dp aux

Quoted 38 lines · Jani Nikula

Functionality remains largely the same as before.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c  |  257 +++++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 2 files changed, 116 insertions(+), 142 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +9 ~~-10~~

index 22134edc03dd..24cbf4bd36c4 100644
@@ -575,97 +575,77 @@ out:
        return ret;
 }
 
-/* Write data to the aux channel in native mode */
-static int
-intel_dp_aux_native_write(struct intel_dp *intel_dp,
-                         uint16_t address, uint8_t *send, int send_bytes)
+#define HEADER_SIZE    4
+static ssize_t
+intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
+       struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
+       uint8_t txbuf[20], rxbuf[20];
+       size_t txsize, rxsize;
        int ret;
-       uint8_t msg[20];
-       int msg_bytes;
-       uint8_t ack;
-       int retry;
 
-       if (WARN_ON(send_bytes > 16))
-               return -E2BIG;
+       txbuf[0] = msg->request << 4;
+       txbuf[1] = msg->address >> 8;
+       txbuf[2] = msg->address & 0xff;

drivers/gpu/drm/i915/intel_dp.c out: · line 599

Quoted 10 lines

drivers/gpu/drm/i915/intel_dp.c +1 -8

+       txbuf[3] = msg->size - 1;
 
-       intel_dp_check_edp(intel_dp);
-       msg[0] = DP_AUX_NATIVE_WRITE << 4;
-       msg[1] = address >> 8;
-       msg[2] = address & 0xff;
-       msg[3] = send_bytes - 1;
-       memcpy(&msg[4], send, send_bytes);
-       msg_bytes = send_bytes + 4;
-       for (retry = 0; retry < 7; retry++) {

we were retrying 7 times always, but now we are now retrying only 3 times or even none.
Couldn't it trigger some new bug or regression?

drivers/gpu/drm/i915/intel_dp.c out: · line 607

Quoted 8 lines

drivers/gpu/drm/i915/intel_dp.c +0 -8

-               ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1);
-               if (ret < 0)
-                       return ret;
-               ack >>= 4;
-               if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
-                       return send_bytes;
-               else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-                       usleep_range(400, 500);

we are also not checking this ack x defer (here nor in native_write) replies anymore.
Don't we need it anymore? why?

Quoted 405 lines

drivers/gpu/drm/i915/intel_dp.c +105 ~~-116~~

-               else
-                       return -EIO;
-       }
+       switch (msg->request & ~DP_AUX_I2C_MOT) {
+       case DP_AUX_NATIVE_WRITE:
+       case DP_AUX_I2C_WRITE:
+               txsize = HEADER_SIZE + msg->size;
+               rxsize = 1;
 
-       DRM_ERROR("too many retries, giving up\n");
-       return -EIO;
-}
+               if (WARN_ON(txsize > 20))
+                       return -E2BIG;
 
-/* Write a single byte to the aux channel in native mode */
-static int
-intel_dp_aux_native_write_1(struct intel_dp *intel_dp,
-                           uint16_t address, uint8_t byte)
-{
-       return intel_dp_aux_native_write(intel_dp, address, &byte, 1);
-}
+               memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
 
-/* read bytes from a native aux channel */
-static int
-intel_dp_aux_native_read(struct intel_dp *intel_dp,
-                        uint16_t address, uint8_t *recv, int recv_bytes)
-{
-       uint8_t msg[4];
-       int msg_bytes;
-       uint8_t reply[20];
-       int reply_bytes;
-       uint8_t ack;
-       int ret;
-       int retry;
+               ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+               if (ret > 0) {
+                       msg->reply = rxbuf[0] >> 4;
 
-       if (WARN_ON(recv_bytes > 19))
-               return -E2BIG;
+                       /* Return payload size. */
+                       ret = msg->size;
+               }
+               break;
 
-       intel_dp_check_edp(intel_dp);
-       msg[0] = DP_AUX_NATIVE_READ << 4;
-       msg[1] = address >> 8;
-       msg[2] = address & 0xff;
-       msg[3] = recv_bytes - 1;
+       case DP_AUX_NATIVE_READ:
+       case DP_AUX_I2C_READ:
+               txsize = HEADER_SIZE;
+               rxsize = msg->size + 1;
 
-       msg_bytes = 4;
-       reply_bytes = recv_bytes + 1;
+               if (WARN_ON(rxsize > 20))
+                       return -E2BIG;
 
-       for (retry = 0; retry < 7; retry++) {
-               ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes,
-                                     reply, reply_bytes);
-               if (ret == 0)
-                       return -EPROTO;
-               if (ret < 0)
-                       return ret;
-               ack = reply[0] >> 4;
-               if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) {
-                       memcpy(recv, reply + 1, ret - 1);
-                       return ret - 1;
+               ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+               if (ret > 0) {
+                       msg->reply = rxbuf[0] >> 4;
+                       /*
+                        * Assume happy day, and copy the data. The caller is
+                        * expected to check msg->reply before touching it.
+                        *
+                        * Return payload size.
+                        */
+                       ret--;
+                       memcpy(msg->buffer, rxbuf + 1, ret);
                }
-               else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-                       usleep_range(400, 500);
-               else
-                       return -EIO;
+               break;
+
+       default:
+               ret = -EINVAL;
+               break;
        }
 
-       DRM_ERROR("too many retries, giving up\n");
-       return -EIO;
+       return ret;
+}
+
+static void
+intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
+{
+       struct drm_device *dev = intel_dp_to_dev(intel_dp);
+
+       intel_dp->aux.dev = dev->dev;
+       intel_dp->aux.transfer = intel_dp_aux_transfer;
 }
 
 static int

@@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
                return;
 
        if (mode != DRM_MODE_DPMS_ON) {
-               ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER,
-                                                 DP_SET_POWER_D3);
+               ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                                        DP_SET_POWER_D3);
                if (ret != 1)
                        DRM_DEBUG_DRIVER("failed to write sink power state\n");
        } else {

@@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
                 * time to wake up.
                 */
                for (i = 0; i < 3; i++) {
-                       ret = intel_dp_aux_native_write_1(intel_dp,
-                                                         DP_SET_POWER,
-                                                         DP_SET_POWER_D0);
+                       ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                                                DP_SET_POWER_D0);
                        if (ret == 1)
                                break;
                        msleep(1);

@@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 
        /* Enable PSR in sink */
        if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT)
-               intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-                                           DP_PSR_ENABLE &
-                                           ~DP_PSR_MAIN_LINK_ACTIVE);
+               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+                                  DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
        else
-               intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-                                           DP_PSR_ENABLE |
-                                           DP_PSR_MAIN_LINK_ACTIVE);
+               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+                                  DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
 
        /* Setup AUX registers */
        I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND);

@@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder)
 /*
  * Native read with retry for link status and receiver capability reads for
  * cases where the sink may still be asleep.
+ *
+ * Sinks are *supposed* to come up within 1ms from an off state, but we're also
+ * supposed to retry 3 times per the spec.
  */
-static bool
-intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
-                              uint8_t *recv, int recv_bytes)
+static ssize_t
+intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset,
+                       void *buffer, size_t size)
 {
-       int ret, i;
+       ssize_t ret;
+       int i;
 
-       /*
-        * Sinks are *supposed* to come up within 1ms from an off state,
-        * but we're also supposed to retry 3 times per the spec.
-        */
        for (i = 0; i < 3; i++) {
-               ret = intel_dp_aux_native_read(intel_dp, address, recv,
-                                              recv_bytes);
-               if (ret == recv_bytes)
-                       return true;
+               ret = drm_dp_dpcd_read(aux, offset, buffer, size);
+               if (ret == size)
+                       return ret;
                msleep(1);
        }
 
-       return false;
+       return ret;
 }
 
 /*

@@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
 static bool
 intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE])
 {
-       return intel_dp_aux_native_read_retry(intel_dp,
-                                             DP_LANE0_1_STATUS,
-                                             link_status,
-                                             DP_LINK_STATUS_SIZE);
+       return intel_dp_dpcd_read_wake(&intel_dp->aux,
+                                      DP_LANE0_1_STATUS,
+                                      link_status,
+                                      DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
 }
 
 /*

@@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
                len = intel_dp->lane_count + 1;
        }
 
-       ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET,
-                                       buf, len);
+       ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
+                               buf, len);
 
        return ret == len;
 }

@@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
        I915_WRITE(intel_dp->output_reg, *DP);
        POSTING_READ(intel_dp->output_reg);
 
-       ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET,
-                                       intel_dp->train_set,
-                                       intel_dp->lane_count);
+       ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
+                               intel_dp->train_set, intel_dp->lane_count);
 
        return ret == intel_dp->lane_count;
 }

@@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
        link_config[1] = intel_dp->lane_count;
        if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
                link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
-       intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2);
+       drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
 
        link_config[0] = 0;
        link_config[1] = DP_SET_ANSI_8B10B;
-       intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2);
+       drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
 
        DP |= DP_PORT_EN;

@@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 
        char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
 
-       if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-                                          sizeof(intel_dp->dpcd)) == 0)
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd,
+                                   sizeof(intel_dp->dpcd)) < 0)
                return false; /* aux transfer failed */
 
        hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),

@@ -2921,9 +2896,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
        /* Check if the panel supports PSR */
        memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
        if (is_edp(intel_dp)) {
-               intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
-                                              intel_dp->psr_dpcd,
-                                              sizeof(intel_dp->psr_dpcd));
+               intel_dp_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT,
+                                       intel_dp->psr_dpcd,
+                                       sizeof(intel_dp->psr_dpcd));
                if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
                        dev_priv->psr.sink_support = true;
                        DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");

@@ -2945,9 +2920,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
                return true; /* no per-port downstream info */
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
-                                          intel_dp->downstream_ports,
-                                          DP_MAX_DOWNSTREAM_PORTS) == 0)
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0,
+                                   intel_dp->downstream_ports,
+                                   DP_MAX_DOWNSTREAM_PORTS) < 0)
                return false; /* downstream port status fetch failed */
 
        return true;

@@ -2963,11 +2938,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
 
        edp_panel_vdd_on(intel_dp);
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_SINK_OUI, buf, 3))
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3)
                DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
                              buf[0], buf[1], buf[2]);
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3))
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3)
                DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
                              buf[0], buf[1], buf[2]);

@@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
                to_intel_crtc(intel_dig_port->base.base.crtc);
        u8 buf[1];
 
-       if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1))
+       if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0);
                return -EAGAIN;
 
        if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
                return -ENOTTY;
 
-       if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK,
-                                        DP_TEST_SINK_START))
+       if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
+                              DP_TEST_SINK_START) < 0)
                return -EAGAIN;
 
        /* Wait 2 vblanks to be sure we will have the correct CRC value */
        intel_wait_for_vblank(dev, intel_crtc->pipe);
        intel_wait_for_vblank(dev, intel_crtc->pipe);
 
-       if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6))
+       if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
                return -EAGAIN;
 
-       intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0);
+       drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0);
        return 0;
 }
 
 static bool
 intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 {
-       int ret;
-
-       ret = intel_dp_aux_native_read_retry(intel_dp,
-                                            DP_DEVICE_SERVICE_IRQ_VECTOR,
-                                            sink_irq_vector, 1);
-       if (!ret)
-               return false;
-
-       return true;
+       return intel_dp_dpcd_read_wake(&intel_dp->aux,
+                                      DP_DEVICE_SERVICE_IRQ_VECTOR,
+                                      sink_irq_vector, 1) == 1;
 }
 
 static void
 intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
        /* NAK by default */
-       intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK);
+       drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK);
 }
 
 /*

@@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
            intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
                /* Clear interrupt source */
-               intel_dp_aux_native_write_1(intel_dp,
-                                           DP_DEVICE_SERVICE_IRQ_VECTOR,
-                                           sink_irq_vector);
+               drm_dp_dpcd_writeb(&intel_dp->aux,
+                                  DP_DEVICE_SERVICE_IRQ_VECTOR,
+                                  sink_irq_vector);
 
                if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
                        intel_dp_handle_test_request(intel_dp);

@@ -3097,9 +3066,11 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
            intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
                uint8_t reg;
-               if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
-                                                   &reg, 1))
+
+               if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
+                                           &reg, 1) < 0)
                        return connector_status_unknown;
+
                return DP_GET_SINK_COUNT(reg) ? connector_status_connected
                                              : connector_status_disconnected;
        }

@@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
                intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
        }
 
+       intel_dp_aux_init(intel_dp, intel_connector);
+
        error = intel_dp_i2c_init(intel_dp, intel_connector, name);
        WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
             error, port_name(port));

drivers/gpu/drm/i915/intel_drv.h +1 -1

index 2546cae0b4f0..561af4b013b1 100644
@@ -491,6 +491,7 @@ struct intel_dp {
        uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
        struct i2c_adapter adapter;
        struct i2c_algo_dp_aux_data algo;
+       struct drm_dp_aux aux;
        uint8_t train_set[4];
        int panel_power_up_delay;
        int panel_power_down_delay;
--

1.7.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

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

Rodrigo Vivi

2014-03-17 14:51 UTC #10 lore raw

Re: [PATCH 4/6] drm/i915/dp: use the new drm helpers for dp aux

Quoted 361 lines · Rodrigo Vivi

Quoted 38 lines · Jani Nikula

Functionality remains largely the same as before.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c  |  257 +++++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 2 files changed, 116 insertions(+), 142 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +9 ~~-10~~

index 22134edc03dd..24cbf4bd36c4 100644
@@ -575,97 +575,77 @@ out:
        return ret;
 }
 
-/* Write data to the aux channel in native mode */
-static int
-intel_dp_aux_native_write(struct intel_dp *intel_dp,
-                         uint16_t address, uint8_t *send, int send_bytes)
+#define HEADER_SIZE    4
+static ssize_t
+intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
+       struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
+       uint8_t txbuf[20], rxbuf[20];
+       size_t txsize, rxsize;
        int ret;
-       uint8_t msg[20];
-       int msg_bytes;
-       uint8_t ack;
-       int retry;
 
-       if (WARN_ON(send_bytes > 16))
-               return -E2BIG;
+       txbuf[0] = msg->request << 4;
+       txbuf[1] = msg->address >> 8;
+       txbuf[2] = msg->address & 0xff;

drivers/gpu/drm/i915/intel_dp.c out: · line 599

Quoted 10 lines

drivers/gpu/drm/i915/intel_dp.c +1 -8

+       txbuf[3] = msg->size - 1;
 
-       intel_dp_check_edp(intel_dp);
-       msg[0] = DP_AUX_NATIVE_WRITE << 4;
-       msg[1] = address >> 8;
-       msg[2] = address & 0xff;
-       msg[3] = send_bytes - 1;
-       memcpy(&msg[4], send, send_bytes);
-       msg_bytes = send_bytes + 4;
-       for (retry = 0; retry < 7; retry++) {

we were retrying 7 times always, but now we are now retrying only 3 times or even none.
Couldn't it trigger some new bug or regression?

drivers/gpu/drm/i915/intel_dp.c out: · line 607

Quoted 8 lines

drivers/gpu/drm/i915/intel_dp.c +0 -8

-               ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1);
-               if (ret < 0)
-                       return ret;
-               ack >>= 4;
-               if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
-                       return send_bytes;
-               else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-                       usleep_range(400, 500);

we are also not checking this ack x defer (here nor in native_write) replies anymore.
Don't we need it anymore? why?

Quoted 294 lines

drivers/gpu/drm/i915/intel_dp.c +87 ~~-96~~

-               else
-                       return -EIO;
-       }
+       switch (msg->request & ~DP_AUX_I2C_MOT) {
+       case DP_AUX_NATIVE_WRITE:
+       case DP_AUX_I2C_WRITE:
+               txsize = HEADER_SIZE + msg->size;
+               rxsize = 1;
 
-       DRM_ERROR("too many retries, giving up\n");
-       return -EIO;
-}
+               if (WARN_ON(txsize > 20))
+                       return -E2BIG;
 
-/* Write a single byte to the aux channel in native mode */
-static int
-intel_dp_aux_native_write_1(struct intel_dp *intel_dp,
-                           uint16_t address, uint8_t byte)
-{
-       return intel_dp_aux_native_write(intel_dp, address, &byte, 1);
-}
+               memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
 
-/* read bytes from a native aux channel */
-static int
-intel_dp_aux_native_read(struct intel_dp *intel_dp,
-                        uint16_t address, uint8_t *recv, int recv_bytes)
-{
-       uint8_t msg[4];
-       int msg_bytes;
-       uint8_t reply[20];
-       int reply_bytes;
-       uint8_t ack;
-       int ret;
-       int retry;
+               ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+               if (ret > 0) {
+                       msg->reply = rxbuf[0] >> 4;
 
-       if (WARN_ON(recv_bytes > 19))
-               return -E2BIG;
+                       /* Return payload size. */
+                       ret = msg->size;
+               }
+               break;
 
-       intel_dp_check_edp(intel_dp);
-       msg[0] = DP_AUX_NATIVE_READ << 4;
-       msg[1] = address >> 8;
-       msg[2] = address & 0xff;
-       msg[3] = recv_bytes - 1;
+       case DP_AUX_NATIVE_READ:
+       case DP_AUX_I2C_READ:
+               txsize = HEADER_SIZE;
+               rxsize = msg->size + 1;
 
-       msg_bytes = 4;
-       reply_bytes = recv_bytes + 1;
+               if (WARN_ON(rxsize > 20))
+                       return -E2BIG;
 
-       for (retry = 0; retry < 7; retry++) {
-               ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes,
-                                     reply, reply_bytes);
-               if (ret == 0)
-                       return -EPROTO;
-               if (ret < 0)
-                       return ret;
-               ack = reply[0] >> 4;
-               if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) {
-                       memcpy(recv, reply + 1, ret - 1);
-                       return ret - 1;
+               ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+               if (ret > 0) {
+                       msg->reply = rxbuf[0] >> 4;
+                       /*
+                        * Assume happy day, and copy the data. The caller is
+                        * expected to check msg->reply before touching it.
+                        *
+                        * Return payload size.
+                        */
+                       ret--;
+                       memcpy(msg->buffer, rxbuf + 1, ret);
                }
-               else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-                       usleep_range(400, 500);
-               else
-                       return -EIO;
+               break;
+
+       default:
+               ret = -EINVAL;
+               break;
        }
 
-       DRM_ERROR("too many retries, giving up\n");
-       return -EIO;
+       return ret;
+}
+
+static void
+intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
+{
+       struct drm_device *dev = intel_dp_to_dev(intel_dp);
+
+       intel_dp->aux.dev = dev->dev;
+       intel_dp->aux.transfer = intel_dp_aux_transfer;
 }
 
 static int

@@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
                return;
 
        if (mode != DRM_MODE_DPMS_ON) {
-               ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER,
-                                                 DP_SET_POWER_D3);
+               ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                                        DP_SET_POWER_D3);
                if (ret != 1)
                        DRM_DEBUG_DRIVER("failed to write sink power state\n");
        } else {

@@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
                 * time to wake up.
                 */
                for (i = 0; i < 3; i++) {
-                       ret = intel_dp_aux_native_write_1(intel_dp,
-                                                         DP_SET_POWER,
-                                                         DP_SET_POWER_D0);
+                       ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                                                DP_SET_POWER_D0);
                        if (ret == 1)
                                break;
                        msleep(1);

@@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 
        /* Enable PSR in sink */
        if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT)
-               intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-                                           DP_PSR_ENABLE &
-                                           ~DP_PSR_MAIN_LINK_ACTIVE);
+               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+                                  DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
        else
-               intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-                                           DP_PSR_ENABLE |
-                                           DP_PSR_MAIN_LINK_ACTIVE);
+               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+                                  DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
 
        /* Setup AUX registers */
        I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND);

@@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder)
 /*
  * Native read with retry for link status and receiver capability reads for
  * cases where the sink may still be asleep.
+ *
+ * Sinks are *supposed* to come up within 1ms from an off state, but we're also
+ * supposed to retry 3 times per the spec.
  */
-static bool
-intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
-                              uint8_t *recv, int recv_bytes)
+static ssize_t
+intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset,
+                       void *buffer, size_t size)
 {
-       int ret, i;
+       ssize_t ret;
+       int i;
 
-       /*
-        * Sinks are *supposed* to come up within 1ms from an off state,
-        * but we're also supposed to retry 3 times per the spec.
-        */
        for (i = 0; i < 3; i++) {
-               ret = intel_dp_aux_native_read(intel_dp, address, recv,
-                                              recv_bytes);
-               if (ret == recv_bytes)
-                       return true;
+               ret = drm_dp_dpcd_read(aux, offset, buffer, size);
+               if (ret == size)
+                       return ret;
                msleep(1);
        }
 
-       return false;
+       return ret;
 }
 
 /*

@@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
 static bool
 intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE])
 {
-       return intel_dp_aux_native_read_retry(intel_dp,
-                                             DP_LANE0_1_STATUS,
-                                             link_status,
-                                             DP_LINK_STATUS_SIZE);
+       return intel_dp_dpcd_read_wake(&intel_dp->aux,
+                                      DP_LANE0_1_STATUS,
+                                      link_status,
+                                      DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
 }
 
 /*

@@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
                len = intel_dp->lane_count + 1;
        }
 
-       ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET,
-                                       buf, len);
+       ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
+                               buf, len);
 
        return ret == len;
 }

@@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
        I915_WRITE(intel_dp->output_reg, *DP);
        POSTING_READ(intel_dp->output_reg);
 
-       ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET,
-                                       intel_dp->train_set,
-                                       intel_dp->lane_count);
+       ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
+                               intel_dp->train_set, intel_dp->lane_count);
 
        return ret == intel_dp->lane_count;
 }

@@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
        link_config[1] = intel_dp->lane_count;
        if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
                link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
-       intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2);
+       drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
 
        link_config[0] = 0;
        link_config[1] = DP_SET_ANSI_8B10B;
-       intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2);
+       drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
 
        DP |= DP_PORT_EN;

@@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 
        char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
 
-       if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-                                          sizeof(intel_dp->dpcd)) == 0)
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd,
+                                   sizeof(intel_dp->dpcd)) < 0)
                return false; /* aux transfer failed */
 
        hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),

@@ -2921,9 +2896,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
        /* Check if the panel supports PSR */
        memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
        if (is_edp(intel_dp)) {
-               intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
-                                              intel_dp->psr_dpcd,
-                                              sizeof(intel_dp->psr_dpcd));
+               intel_dp_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT,
+                                       intel_dp->psr_dpcd,
+                                       sizeof(intel_dp->psr_dpcd));
                if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
                        dev_priv->psr.sink_support = true;
                        DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");

@@ -2945,9 +2920,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
                return true; /* no per-port downstream info */
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
-                                          intel_dp->downstream_ports,
-                                          DP_MAX_DOWNSTREAM_PORTS) == 0)
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0,
+                                   intel_dp->downstream_ports,
+                                   DP_MAX_DOWNSTREAM_PORTS) < 0)
                return false; /* downstream port status fetch failed */
 
        return true;

@@ -2963,11 +2938,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
 
        edp_panel_vdd_on(intel_dp);
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_SINK_OUI, buf, 3))
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3)
                DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
                              buf[0], buf[1], buf[2]);
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3))
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3)
                DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
                              buf[0], buf[1], buf[2]);

Quoted 6 lines

drivers/gpu/drm/i915/intel_dp.c int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) · line 2960

Quoted 6 lines

drivers/gpu/drm/i915/intel_dp.c +1 -1

@@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
                to_intel_crtc(intel_dig_port->base.base.crtc);
        u8 buf[1];
 
-       if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1))
+       if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0);

wrong ";" here..

Quoted 111 lines

Quoted 101 lines

drivers/gpu/drm/i915/intel_dp.c +17 ~~-19~~

                return -EAGAIN;
 
        if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
                return -ENOTTY;
 
-       if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK,
-                                        DP_TEST_SINK_START))
+       if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
+                              DP_TEST_SINK_START) < 0)
                return -EAGAIN;
 
        /* Wait 2 vblanks to be sure we will have the correct CRC value */
        intel_wait_for_vblank(dev, intel_crtc->pipe);
        intel_wait_for_vblank(dev, intel_crtc->pipe);
 
-       if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6))
+       if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
                return -EAGAIN;
 
-       intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0);
+       drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0);
        return 0;
 }
 
 static bool
 intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 {
-       int ret;
-
-       ret = intel_dp_aux_native_read_retry(intel_dp,
-                                            DP_DEVICE_SERVICE_IRQ_VECTOR,
-                                            sink_irq_vector, 1);
-       if (!ret)
-               return false;
-
-       return true;
+       return intel_dp_dpcd_read_wake(&intel_dp->aux,
+                                      DP_DEVICE_SERVICE_IRQ_VECTOR,
+                                      sink_irq_vector, 1) == 1;
 }
 
 static void
 intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
        /* NAK by default */
-       intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK);
+       drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK);
 }
 
 /*

@@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
            intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
                /* Clear interrupt source */
-               intel_dp_aux_native_write_1(intel_dp,
-                                           DP_DEVICE_SERVICE_IRQ_VECTOR,
-                                           sink_irq_vector);
+               drm_dp_dpcd_writeb(&intel_dp->aux,
+                                  DP_DEVICE_SERVICE_IRQ_VECTOR,
+                                  sink_irq_vector);
 
                if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
                        intel_dp_handle_test_request(intel_dp);

@@ -3097,9 +3066,11 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
            intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
                uint8_t reg;
-               if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
-                                                   &reg, 1))
+
+               if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
+                                           &reg, 1) < 0)
                        return connector_status_unknown;
+
                return DP_GET_SINK_COUNT(reg) ? connector_status_connected
                                              : connector_status_disconnected;
        }

@@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
                intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
        }
 
+       intel_dp_aux_init(intel_dp, intel_connector);
+
        error = intel_dp_i2c_init(intel_dp, intel_connector, name);
        WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
             error, port_name(port));

drivers/gpu/drm/i915/intel_drv.h +1 -1

index 2546cae0b4f0..561af4b013b1 100644
@@ -491,6 +491,7 @@ struct intel_dp {
        uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
        struct i2c_adapter adapter;
        struct i2c_algo_dp_aux_data algo;
+       struct drm_dp_aux aux;
        uint8_t train_set[4];
        int panel_power_up_delay;
        int panel_power_down_delay;
--

1.7.9.5

Quoted 4 lines

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Signature · 2 lines

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

Jani Nikula

author

2014-03-17 14:54 UTC #11 lore raw

Re: [PATCH 4/6] drm/i915/dp: use the new drm helpers for dp aux

Quoted 6 lines · Rodrigo Vivi

Quoted 3 lines · Rodrigo Vivi
drivers/gpu/drm/i915/intel_dp.c int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) · line 2960
Quoted 2 lines · Jani Nikula
drivers/gpu/drm/i915/intel_dp.c +1 -1
-       if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1))
+       if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0);
wrong ";" here..

Yikes, good catch. I've been rebasing this way too many times...

Jani.

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Jani Nikula

author

2014-03-18 10:55 UTC #12 lore raw

Re: [PATCH 4/6] drm/i915/dp: use the new drm helpers for dp aux

In reply to Rodrigo Vivi

Quoted 41 lines · Rodrigo Vivi

Quoted 40 lines · Jani Nikula

Functionality remains largely the same as before.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c  |  257 +++++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 2 files changed, 116 insertions(+), 142 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +10 ~~-10~~

index 22134edc03dd..24cbf4bd36c4 100644
@@ -575,97 +575,77 @@ out:
        return ret;
 }
 
-/* Write data to the aux channel in native mode */
-static int
-intel_dp_aux_native_write(struct intel_dp *intel_dp,
-                         uint16_t address, uint8_t *send, int send_bytes)
+#define HEADER_SIZE    4
+static ssize_t
+intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
+       struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
+       uint8_t txbuf[20], rxbuf[20];
+       size_t txsize, rxsize;
        int ret;
-       uint8_t msg[20];
-       int msg_bytes;
-       uint8_t ack;
-       int retry;
 
-       if (WARN_ON(send_bytes > 16))
-               return -E2BIG;
+       txbuf[0] = msg->request << 4;
+       txbuf[1] = msg->address >> 8;
+       txbuf[2] = msg->address & 0xff;
+       txbuf[3] = msg->size - 1;

Quoted 11 lines

drivers/gpu/drm/i915/intel_dp.c out: · line 599
Quoted 8 lines
drivers/gpu/drm/i915/intel_dp.c +0 -8
-       intel_dp_check_edp(intel_dp);
-       msg[0] = DP_AUX_NATIVE_WRITE << 4;
-       msg[1] = address >> 8;
-       msg[2] = address & 0xff;
-       msg[3] = send_bytes - 1;
-       memcpy(&msg[4], send, send_bytes);
-       msg_bytes = send_bytes + 4;
-       for (retry = 0; retry < 7; retry++) {
we were retrying 7 times always, but now we are now retrying only 3 times or even none.

I'm confused, what are you referring to? drm_dp_dpcd_access() in drm_dp_helper.c has the retry loop now.

I think you're mixing this with the intel_dp_dpcd_read_wake() wrapper, but that ends up in drm_dp_dpcd_access() as well.

Quoted 11 lines

Couldn't it trigger some new bug or regression?

drivers/gpu/drm/i915/intel_dp.c out: · line 607

Quoted 8 lines

drivers/gpu/drm/i915/intel_dp.c +0 -8

-               ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1);
-               if (ret < 0)
-                       return ret;
-               ack >>= 4;
-               if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
-                       return send_bytes;
-               else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-                       usleep_range(400, 500);

Quoted 2 lines

we are also not checking this ack x defer (here nor in native_write) replies anymore.

Ditto, we store ack >>= 4 into msg->reply which gets checked in drm_dp_dpcd_access().

BR,
Jani.

Quoted 413 lines

Don't we need it anymore? why?

Quoted 405 lines

drivers/gpu/drm/i915/intel_dp.c +105 ~~-116~~

-               else
-                       return -EIO;
-       }
+       switch (msg->request & ~DP_AUX_I2C_MOT) {
+       case DP_AUX_NATIVE_WRITE:
+       case DP_AUX_I2C_WRITE:
+               txsize = HEADER_SIZE + msg->size;
+               rxsize = 1;
 
-       DRM_ERROR("too many retries, giving up\n");
-       return -EIO;
-}
+               if (WARN_ON(txsize > 20))
+                       return -E2BIG;
 
-/* Write a single byte to the aux channel in native mode */
-static int
-intel_dp_aux_native_write_1(struct intel_dp *intel_dp,
-                           uint16_t address, uint8_t byte)
-{
-       return intel_dp_aux_native_write(intel_dp, address, &byte, 1);
-}
+               memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
 
-/* read bytes from a native aux channel */
-static int
-intel_dp_aux_native_read(struct intel_dp *intel_dp,
-                        uint16_t address, uint8_t *recv, int recv_bytes)
-{
-       uint8_t msg[4];
-       int msg_bytes;
-       uint8_t reply[20];
-       int reply_bytes;
-       uint8_t ack;
-       int ret;
-       int retry;
+               ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+               if (ret > 0) {
+                       msg->reply = rxbuf[0] >> 4;
 
-       if (WARN_ON(recv_bytes > 19))
-               return -E2BIG;
+                       /* Return payload size. */
+                       ret = msg->size;
+               }
+               break;
 
-       intel_dp_check_edp(intel_dp);
-       msg[0] = DP_AUX_NATIVE_READ << 4;
-       msg[1] = address >> 8;
-       msg[2] = address & 0xff;
-       msg[3] = recv_bytes - 1;
+       case DP_AUX_NATIVE_READ:
+       case DP_AUX_I2C_READ:
+               txsize = HEADER_SIZE;
+               rxsize = msg->size + 1;
 
-       msg_bytes = 4;
-       reply_bytes = recv_bytes + 1;
+               if (WARN_ON(rxsize > 20))
+                       return -E2BIG;
 
-       for (retry = 0; retry < 7; retry++) {
-               ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes,
-                                     reply, reply_bytes);
-               if (ret == 0)
-                       return -EPROTO;
-               if (ret < 0)
-                       return ret;
-               ack = reply[0] >> 4;
-               if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) {
-                       memcpy(recv, reply + 1, ret - 1);
-                       return ret - 1;
+               ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+               if (ret > 0) {
+                       msg->reply = rxbuf[0] >> 4;
+                       /*
+                        * Assume happy day, and copy the data. The caller is
+                        * expected to check msg->reply before touching it.
+                        *
+                        * Return payload size.
+                        */
+                       ret--;
+                       memcpy(msg->buffer, rxbuf + 1, ret);
                }
-               else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-                       usleep_range(400, 500);
-               else
-                       return -EIO;
+               break;
+
+       default:
+               ret = -EINVAL;
+               break;
        }
 
-       DRM_ERROR("too many retries, giving up\n");
-       return -EIO;
+       return ret;
+}
+
+static void
+intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
+{
+       struct drm_device *dev = intel_dp_to_dev(intel_dp);
+
+       intel_dp->aux.dev = dev->dev;
+       intel_dp->aux.transfer = intel_dp_aux_transfer;
 }
 
 static int

@@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
                return;
 
        if (mode != DRM_MODE_DPMS_ON) {
-               ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER,
-                                                 DP_SET_POWER_D3);
+               ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                                        DP_SET_POWER_D3);
                if (ret != 1)
                        DRM_DEBUG_DRIVER("failed to write sink power state\n");
        } else {

@@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
                 * time to wake up.
                 */
                for (i = 0; i < 3; i++) {
-                       ret = intel_dp_aux_native_write_1(intel_dp,
-                                                         DP_SET_POWER,
-                                                         DP_SET_POWER_D0);
+                       ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                                                DP_SET_POWER_D0);
                        if (ret == 1)
                                break;
                        msleep(1);

@@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 
        /* Enable PSR in sink */
        if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT)
-               intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-                                           DP_PSR_ENABLE &
-                                           ~DP_PSR_MAIN_LINK_ACTIVE);
+               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+                                  DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
        else
-               intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-                                           DP_PSR_ENABLE |
-                                           DP_PSR_MAIN_LINK_ACTIVE);
+               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+                                  DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
 
        /* Setup AUX registers */
        I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND);

@@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder)
 /*
  * Native read with retry for link status and receiver capability reads for
  * cases where the sink may still be asleep.
+ *
+ * Sinks are *supposed* to come up within 1ms from an off state, but we're also
+ * supposed to retry 3 times per the spec.
  */
-static bool
-intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
-                              uint8_t *recv, int recv_bytes)
+static ssize_t
+intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset,
+                       void *buffer, size_t size)
 {
-       int ret, i;
+       ssize_t ret;
+       int i;
 
-       /*
-        * Sinks are *supposed* to come up within 1ms from an off state,
-        * but we're also supposed to retry 3 times per the spec.
-        */
        for (i = 0; i < 3; i++) {
-               ret = intel_dp_aux_native_read(intel_dp, address, recv,
-                                              recv_bytes);
-               if (ret == recv_bytes)
-                       return true;
+               ret = drm_dp_dpcd_read(aux, offset, buffer, size);
+               if (ret == size)
+                       return ret;
                msleep(1);
        }
 
-       return false;
+       return ret;
 }
 
 /*

@@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
 static bool
 intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE])
 {
-       return intel_dp_aux_native_read_retry(intel_dp,
-                                             DP_LANE0_1_STATUS,
-                                             link_status,
-                                             DP_LINK_STATUS_SIZE);
+       return intel_dp_dpcd_read_wake(&intel_dp->aux,
+                                      DP_LANE0_1_STATUS,
+                                      link_status,
+                                      DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
 }
 
 /*

@@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
                len = intel_dp->lane_count + 1;
        }
 
-       ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET,
-                                       buf, len);
+       ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
+                               buf, len);
 
        return ret == len;
 }

@@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
        I915_WRITE(intel_dp->output_reg, *DP);
        POSTING_READ(intel_dp->output_reg);
 
-       ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET,
-                                       intel_dp->train_set,
-                                       intel_dp->lane_count);
+       ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
+                               intel_dp->train_set, intel_dp->lane_count);
 
        return ret == intel_dp->lane_count;
 }

@@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
        link_config[1] = intel_dp->lane_count;
        if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
                link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
-       intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2);
+       drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
 
        link_config[0] = 0;
        link_config[1] = DP_SET_ANSI_8B10B;
-       intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2);
+       drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
 
        DP |= DP_PORT_EN;

@@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 
        char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
 
-       if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-                                          sizeof(intel_dp->dpcd)) == 0)
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd,
+                                   sizeof(intel_dp->dpcd)) < 0)
                return false; /* aux transfer failed */
 
        hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),

@@ -2921,9 +2896,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
        /* Check if the panel supports PSR */
        memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
        if (is_edp(intel_dp)) {
-               intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
-                                              intel_dp->psr_dpcd,
-                                              sizeof(intel_dp->psr_dpcd));
+               intel_dp_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT,
+                                       intel_dp->psr_dpcd,
+                                       sizeof(intel_dp->psr_dpcd));
                if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
                        dev_priv->psr.sink_support = true;
                        DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");

@@ -2945,9 +2920,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
                return true; /* no per-port downstream info */
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
-                                          intel_dp->downstream_ports,
-                                          DP_MAX_DOWNSTREAM_PORTS) == 0)
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0,
+                                   intel_dp->downstream_ports,
+                                   DP_MAX_DOWNSTREAM_PORTS) < 0)
                return false; /* downstream port status fetch failed */
 
        return true;

@@ -2963,11 +2938,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
 
        edp_panel_vdd_on(intel_dp);
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_SINK_OUI, buf, 3))
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3)
                DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
                              buf[0], buf[1], buf[2]);
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3))
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3)
                DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
                              buf[0], buf[1], buf[2]);

@@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
                to_intel_crtc(intel_dig_port->base.base.crtc);
        u8 buf[1];
 
-       if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1))
+       if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0);
                return -EAGAIN;
 
        if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
                return -ENOTTY;
 
-       if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK,
-                                        DP_TEST_SINK_START))
+       if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
+                              DP_TEST_SINK_START) < 0)
                return -EAGAIN;
 
        /* Wait 2 vblanks to be sure we will have the correct CRC value */
        intel_wait_for_vblank(dev, intel_crtc->pipe);
        intel_wait_for_vblank(dev, intel_crtc->pipe);
 
-       if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6))
+       if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
                return -EAGAIN;
 
-       intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0);
+       drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0);
        return 0;
 }
 
 static bool
 intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 {
-       int ret;
-
-       ret = intel_dp_aux_native_read_retry(intel_dp,
-                                            DP_DEVICE_SERVICE_IRQ_VECTOR,
-                                            sink_irq_vector, 1);
-       if (!ret)
-               return false;
-
-       return true;
+       return intel_dp_dpcd_read_wake(&intel_dp->aux,
+                                      DP_DEVICE_SERVICE_IRQ_VECTOR,
+                                      sink_irq_vector, 1) == 1;
 }
 
 static void
 intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
        /* NAK by default */
-       intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK);
+       drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK);
 }
 
 /*

@@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
            intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
                /* Clear interrupt source */
-               intel_dp_aux_native_write_1(intel_dp,
-                                           DP_DEVICE_SERVICE_IRQ_VECTOR,
-                                           sink_irq_vector);
+               drm_dp_dpcd_writeb(&intel_dp->aux,
+                                  DP_DEVICE_SERVICE_IRQ_VECTOR,
+                                  sink_irq_vector);
 
                if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
                        intel_dp_handle_test_request(intel_dp);

@@ -3097,9 +3066,11 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
            intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
                uint8_t reg;
-               if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
-                                                   &reg, 1))
+
+               if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
+                                           &reg, 1) < 0)
                        return connector_status_unknown;
+
                return DP_GET_SINK_COUNT(reg) ? connector_status_connected
                                              : connector_status_disconnected;
        }

@@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
                intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
        }
 
+       intel_dp_aux_init(intel_dp, intel_connector);
+
        error = intel_dp_i2c_init(intel_dp, intel_connector, name);
        WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
             error, port_name(port));

drivers/gpu/drm/i915/intel_drv.h +1 -1

index 2546cae0b4f0..561af4b013b1 100644
@@ -491,6 +491,7 @@ struct intel_dp {
        uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
        struct i2c_adapter adapter;
        struct i2c_algo_dp_aux_data algo;
+       struct drm_dp_aux aux;
        uint8_t train_set[4];
        int panel_power_up_delay;
        int panel_power_down_delay;
--

1.7.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

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

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Rodrigo Vivi

2014-03-18 14:02 UTC #13 lore raw

Re: [PATCH 4/6] drm/i915/dp: use the new drm helpers for dp aux

Thanks for explanations Jani and Daniel.

Feel free to use Reviewe-by: Rodrigo Vivi <[email protected]>

Quoted 496 lines · Jani Nikula

Quoted 41 lines · Rodrigo Vivi

Quoted 40 lines · Jani Nikula

Functionality remains largely the same as before.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c  |  257 +++++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 2 files changed, 116 insertions(+), 142 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +10 ~~-10~~

index 22134edc03dd..24cbf4bd36c4 100644
@@ -575,97 +575,77 @@ out:
        return ret;
 }
 
-/* Write data to the aux channel in native mode */
-static int
-intel_dp_aux_native_write(struct intel_dp *intel_dp,
-                         uint16_t address, uint8_t *send, int send_bytes)
+#define HEADER_SIZE    4
+static ssize_t
+intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
+       struct intel_dp *intel_dp = container_of(aux, struct intel_dp, aux);
+       uint8_t txbuf[20], rxbuf[20];
+       size_t txsize, rxsize;
        int ret;
-       uint8_t msg[20];
-       int msg_bytes;
-       uint8_t ack;
-       int retry;
 
-       if (WARN_ON(send_bytes > 16))
-               return -E2BIG;
+       txbuf[0] = msg->request << 4;
+       txbuf[1] = msg->address >> 8;
+       txbuf[2] = msg->address & 0xff;
+       txbuf[3] = msg->size - 1;

Quoted 11 lines

drivers/gpu/drm/i915/intel_dp.c out: · line 599
Quoted 8 lines
drivers/gpu/drm/i915/intel_dp.c +0 -8
-       intel_dp_check_edp(intel_dp);
-       msg[0] = DP_AUX_NATIVE_WRITE << 4;
-       msg[1] = address >> 8;
-       msg[2] = address & 0xff;
-       msg[3] = send_bytes - 1;
-       memcpy(&msg[4], send, send_bytes);
-       msg_bytes = send_bytes + 4;
-       for (retry = 0; retry < 7; retry++) {
we were retrying 7 times always, but now we are now retrying only 3 times or even none.

I'm confused, what are you referring to? drm_dp_dpcd_access() in drm_dp_helper.c has the retry loop now.

I think you're mixing this with the intel_dp_dpcd_read_wake() wrapper, but that ends up in drm_dp_dpcd_access() as well.

Quoted 11 lines

Couldn't it trigger some new bug or regression?

drivers/gpu/drm/i915/intel_dp.c out: · line 607

Quoted 8 lines

drivers/gpu/drm/i915/intel_dp.c +0 -8

-               ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1);
-               if (ret < 0)
-                       return ret;
-               ack >>= 4;
-               if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
-                       return send_bytes;
-               else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-                       usleep_range(400, 500);

Quoted 2 lines

we are also not checking this ack x defer (here nor in native_write) replies anymore.

Ditto, we store ack >>= 4 into msg->reply which gets checked in drm_dp_dpcd_access().

BR,
Jani.

Quoted 403 lines

Don't we need it anymore? why?

Quoted 401 lines

drivers/gpu/drm/i915/intel_dp.c +105 ~~-116~~

-               else
-                       return -EIO;
-       }
+       switch (msg->request & ~DP_AUX_I2C_MOT) {
+       case DP_AUX_NATIVE_WRITE:
+       case DP_AUX_I2C_WRITE:
+               txsize = HEADER_SIZE + msg->size;
+               rxsize = 1;
 
-       DRM_ERROR("too many retries, giving up\n");
-       return -EIO;
-}
+               if (WARN_ON(txsize > 20))
+                       return -E2BIG;
 
-/* Write a single byte to the aux channel in native mode */
-static int
-intel_dp_aux_native_write_1(struct intel_dp *intel_dp,
-                           uint16_t address, uint8_t byte)
-{
-       return intel_dp_aux_native_write(intel_dp, address, &byte, 1);
-}
+               memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
 
-/* read bytes from a native aux channel */
-static int
-intel_dp_aux_native_read(struct intel_dp *intel_dp,
-                        uint16_t address, uint8_t *recv, int recv_bytes)
-{
-       uint8_t msg[4];
-       int msg_bytes;
-       uint8_t reply[20];
-       int reply_bytes;
-       uint8_t ack;
-       int ret;
-       int retry;
+               ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+               if (ret > 0) {
+                       msg->reply = rxbuf[0] >> 4;
 
-       if (WARN_ON(recv_bytes > 19))
-               return -E2BIG;
+                       /* Return payload size. */
+                       ret = msg->size;
+               }
+               break;
 
-       intel_dp_check_edp(intel_dp);
-       msg[0] = DP_AUX_NATIVE_READ << 4;
-       msg[1] = address >> 8;
-       msg[2] = address & 0xff;
-       msg[3] = recv_bytes - 1;
+       case DP_AUX_NATIVE_READ:
+       case DP_AUX_I2C_READ:
+               txsize = HEADER_SIZE;
+               rxsize = msg->size + 1;
 
-       msg_bytes = 4;
-       reply_bytes = recv_bytes + 1;
+               if (WARN_ON(rxsize > 20))
+                       return -E2BIG;
 
-       for (retry = 0; retry < 7; retry++) {
-               ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes,
-                                     reply, reply_bytes);
-               if (ret == 0)
-                       return -EPROTO;
-               if (ret < 0)
-                       return ret;
-               ack = reply[0] >> 4;
-               if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) {
-                       memcpy(recv, reply + 1, ret - 1);
-                       return ret - 1;
+               ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize);
+               if (ret > 0) {
+                       msg->reply = rxbuf[0] >> 4;
+                       /*
+                        * Assume happy day, and copy the data. The caller is
+                        * expected to check msg->reply before touching it.
+                        *
+                        * Return payload size.
+                        */
+                       ret--;
+                       memcpy(msg->buffer, rxbuf + 1, ret);
                }
-               else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
-                       usleep_range(400, 500);
-               else
-                       return -EIO;
+               break;
+
+       default:
+               ret = -EINVAL;
+               break;
        }
 
-       DRM_ERROR("too many retries, giving up\n");
-       return -EIO;
+       return ret;
+}
+
+static void
+intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
+{
+       struct drm_device *dev = intel_dp_to_dev(intel_dp);
+
+       intel_dp->aux.dev = dev->dev;
+       intel_dp->aux.transfer = intel_dp_aux_transfer;
 }
 
 static int

@@ -1472,8 +1452,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
                return;
 
        if (mode != DRM_MODE_DPMS_ON) {
-               ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER,
-                                                 DP_SET_POWER_D3);
+               ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                                        DP_SET_POWER_D3);
                if (ret != 1)
                        DRM_DEBUG_DRIVER("failed to write sink power state\n");
        } else {

@@ -1482,9 +1462,8 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
                 * time to wake up.
                 */
                for (i = 0; i < 3; i++) {
-                       ret = intel_dp_aux_native_write_1(intel_dp,
-                                                         DP_SET_POWER,
-                                                         DP_SET_POWER_D0);
+                       ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
+                                                DP_SET_POWER_D0);
                        if (ret == 1)
                                break;
                        msleep(1);

@@ -1708,13 +1687,11 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
 
        /* Enable PSR in sink */
        if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT)
-               intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-                                           DP_PSR_ENABLE &
-                                           ~DP_PSR_MAIN_LINK_ACTIVE);
+               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+                                  DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
        else
-               intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
-                                           DP_PSR_ENABLE |
-                                           DP_PSR_MAIN_LINK_ACTIVE);
+               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+                                  DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
 
        /* Setup AUX registers */
        I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND);

@@ -2026,26 +2003,25 @@ static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder)
 /*
  * Native read with retry for link status and receiver capability reads for
  * cases where the sink may still be asleep.
+ *
+ * Sinks are *supposed* to come up within 1ms from an off state, but we're also
+ * supposed to retry 3 times per the spec.
  */
-static bool
-intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
-                              uint8_t *recv, int recv_bytes)
+static ssize_t
+intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset,
+                       void *buffer, size_t size)
 {
-       int ret, i;
+       ssize_t ret;
+       int i;
 
-       /*
-        * Sinks are *supposed* to come up within 1ms from an off state,
-        * but we're also supposed to retry 3 times per the spec.
-        */
        for (i = 0; i < 3; i++) {
-               ret = intel_dp_aux_native_read(intel_dp, address, recv,
-                                              recv_bytes);
-               if (ret == recv_bytes)
-                       return true;
+               ret = drm_dp_dpcd_read(aux, offset, buffer, size);
+               if (ret == size)
+                       return ret;
                msleep(1);
        }
 
-       return false;
+       return ret;
 }
 
 /*

@@ -2055,10 +2031,10 @@ intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
 static bool
 intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE])
 {
-       return intel_dp_aux_native_read_retry(intel_dp,
-                                             DP_LANE0_1_STATUS,
-                                             link_status,
-                                             DP_LINK_STATUS_SIZE);
+       return intel_dp_dpcd_read_wake(&intel_dp->aux,
+                                      DP_LANE0_1_STATUS,
+                                      link_status,
+                                      DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
 }
 
 /*

@@ -2572,8 +2548,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
                len = intel_dp->lane_count + 1;
        }
 
-       ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_PATTERN_SET,
-                                       buf, len);
+       ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
+                               buf, len);
 
        return ret == len;
 }

@@ -2602,9 +2578,8 @@ intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
        I915_WRITE(intel_dp->output_reg, *DP);
        POSTING_READ(intel_dp->output_reg);
 
-       ret = intel_dp_aux_native_write(intel_dp, DP_TRAINING_LANE0_SET,
-                                       intel_dp->train_set,
-                                       intel_dp->lane_count);
+       ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_LANE0_SET,
+                               intel_dp->train_set, intel_dp->lane_count);
 
        return ret == intel_dp->lane_count;
 }

@@ -2660,11 +2635,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
        link_config[1] = intel_dp->lane_count;
        if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
                link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
-       intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, link_config, 2);
+       drm_dp_dpcd_write(&intel_dp->aux, DP_LINK_BW_SET, link_config, 2);
 
        link_config[0] = 0;
        link_config[1] = DP_SET_ANSI_8B10B;
-       intel_dp_aux_native_write(intel_dp, DP_DOWNSPREAD_CTRL, link_config, 2);
+       drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
 
        DP |= DP_PORT_EN;

@@ -2907,8 +2882,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 
        char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
 
-       if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-                                          sizeof(intel_dp->dpcd)) == 0)
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd,
+                                   sizeof(intel_dp->dpcd)) < 0)
                return false; /* aux transfer failed */
 
        hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),

@@ -2921,9 +2896,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
        /* Check if the panel supports PSR */
        memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd));
        if (is_edp(intel_dp)) {
-               intel_dp_aux_native_read_retry(intel_dp, DP_PSR_SUPPORT,
-                                              intel_dp->psr_dpcd,
-                                              sizeof(intel_dp->psr_dpcd));
+               intel_dp_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT,
+                                       intel_dp->psr_dpcd,
+                                       sizeof(intel_dp->psr_dpcd));
                if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
                        dev_priv->psr.sink_support = true;
                        DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");

@@ -2945,9 +2920,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
                return true; /* no per-port downstream info */
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
-                                          intel_dp->downstream_ports,
-                                          DP_MAX_DOWNSTREAM_PORTS) == 0)
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0,
+                                   intel_dp->downstream_ports,
+                                   DP_MAX_DOWNSTREAM_PORTS) < 0)
                return false; /* downstream port status fetch failed */
 
        return true;

@@ -2963,11 +2938,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
 
        edp_panel_vdd_on(intel_dp);
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_SINK_OUI, buf, 3))
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3)
                DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
                              buf[0], buf[1], buf[2]);
 
-       if (intel_dp_aux_native_read_retry(intel_dp, DP_BRANCH_OUI, buf, 3))
+       if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3)
                DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
                              buf[0], buf[1], buf[2]);

@@ -2982,46 +2957,40 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
                to_intel_crtc(intel_dig_port->base.base.crtc);
        u8 buf[1];
 
-       if (!intel_dp_aux_native_read(intel_dp, DP_TEST_SINK_MISC, buf, 1))
+       if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0);
                return -EAGAIN;
 
        if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
                return -ENOTTY;
 
-       if (!intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK,
-                                        DP_TEST_SINK_START))
+       if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
+                              DP_TEST_SINK_START) < 0)
                return -EAGAIN;
 
        /* Wait 2 vblanks to be sure we will have the correct CRC value */
        intel_wait_for_vblank(dev, intel_crtc->pipe);
        intel_wait_for_vblank(dev, intel_crtc->pipe);
 
-       if (!intel_dp_aux_native_read(intel_dp, DP_TEST_CRC_R_CR, crc, 6))
+       if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
                return -EAGAIN;
 
-       intel_dp_aux_native_write_1(intel_dp, DP_TEST_SINK, 0);
+       drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0);
        return 0;
 }
 
 static bool
 intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector)
 {
-       int ret;
-
-       ret = intel_dp_aux_native_read_retry(intel_dp,
-                                            DP_DEVICE_SERVICE_IRQ_VECTOR,
-                                            sink_irq_vector, 1);
-       if (!ret)
-               return false;
-
-       return true;
+       return intel_dp_dpcd_read_wake(&intel_dp->aux,
+                                      DP_DEVICE_SERVICE_IRQ_VECTOR,
+                                      sink_irq_vector, 1) == 1;
 }
 
 static void
 intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
        /* NAK by default */
-       intel_dp_aux_native_write_1(intel_dp, DP_TEST_RESPONSE, DP_TEST_NAK);
+       drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_RESPONSE, DP_TEST_NAK);
 }
 
 /*

@@ -3060,9 +3029,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
            intel_dp_get_sink_irq(intel_dp, &sink_irq_vector)) {
                /* Clear interrupt source */
-               intel_dp_aux_native_write_1(intel_dp,
-                                           DP_DEVICE_SERVICE_IRQ_VECTOR,
-                                           sink_irq_vector);
+               drm_dp_dpcd_writeb(&intel_dp->aux,
+                                  DP_DEVICE_SERVICE_IRQ_VECTOR,
+                                  sink_irq_vector);
 
                if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
                        intel_dp_handle_test_request(intel_dp);

@@ -3097,9 +3066,11 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
        if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
            intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
                uint8_t reg;
-               if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
-                                                   &reg, 1))
+
+               if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT,
+                                           &reg, 1) < 0)
                        return connector_status_unknown;
+
                return DP_GET_SINK_COUNT(reg) ? connector_status_connected
                                              : connector_status_disconnected;
        }

@@ -3925,6 +3896,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
                intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
        }
 
+       intel_dp_aux_init(intel_dp, intel_connector);
+
        error = intel_dp_i2c_init(intel_dp, intel_connector, name);
        WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
             error, port_name(port));

drivers/gpu/drm/i915/intel_drv.h +1 -1

index 2546cae0b4f0..561af4b013b1 100644
@@ -491,6 +491,7 @@ struct intel_dp {
        uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
        struct i2c_adapter adapter;
        struct i2c_algo_dp_aux_data algo;
+       struct drm_dp_aux aux;
        uint8_t train_set[4];
        int panel_power_up_delay;
        int panel_power_down_delay;
--

1.7.9.5

Quoted 10 lines

Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

--
Jani Nikula, Intel Open Source Technology Center

Signature · 2 lines

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

Daniel Vetter

2014-03-18 14:06 UTC #14 lore raw

Re: [PATCH 4/6] drm/i915/dp: use the new drm helpers for dp aux

Quoted 3 lines · Rodrigo Vivi

Thanks for explanations Jani and Daniel.
Feel free to use Reviewe-by: Rodrigo Vivi <[email protected]>

Thanks to patches & review, all pulled into a topic branch, will probably merge into dinq in a few days.
-Daniel

Signature · 3 lines

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

Jani Nikula

author

2014-03-14 14:51 UTC #15 lore raw

PATCH 5/6 drm/i915/dp: move dp aux ch register init to aux init

In reply to Jani Nikula

Do a slight rearrangement of the switch to prep for follow-up.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c |   42 ++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +22 ~~-20~~

index 24cbf4bd36c4..71a76d00d634 100644
@@ -643,6 +643,28 @@ static void
 intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	enum port port = intel_dig_port->port;
+
+	switch (port) {
+	case PORT_A:
+		intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
+		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;
+		break;
+	default:
+		BUG();
+	}
+
+	if (!HAS_DDI(dev))
+		intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
 
 	intel_dp->aux.dev = dev->dev;
 	intel_dp->aux.transfer = intel_dp_aux_transfer;

@@ -3849,26 +3871,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		intel_connector->get_hw_state = intel_connector_get_hw_state;
 	intel_connector->unregister = intel_dp_connector_unregister;
 
-	intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
-	if (HAS_DDI(dev)) {
-		switch (intel_dig_port->port) {
-		case PORT_A:
-			intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
-			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;
-			break;
-		default:
-			BUG();
-		}
-	}
-
 	/* Set up the DDC bus. */
 	switch (port) {
 	case PORT_A:

Signature · 1 line

1.7.9.5

Rodrigo Vivi

Reviewed-by on v1 5/6

2014-03-17 14:12 UTC #16 lore raw

Re: [PATCH 5/6] drm/i915/dp: move dp aux ch register init to aux init

I would prefer an else instead of unecessary comparisons and assignment that will be override..

But anyway it works, so feel free to use:

Reviewed-by Rodrigo Vivi <[email protected]>

Quoted 74 lines · Jani Nikula

Do a slight rearrangement of the switch to prep for follow-up.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c |   42 ++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +22 ~~-21~~

index 24cbf4bd36c4..71a76d00d634 100644
@@ -643,6 +643,28 @@ static void
 intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 {
        struct drm_device *dev = intel_dp_to_dev(intel_dp);
+       struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+       enum port port = intel_dig_port->port;
+
+       switch (port) {
+       case PORT_A:
+               intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
+               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;
+               break;
+       default:
+               BUG();
+       }
+
+       if (!HAS_DDI(dev))
+               intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
 
        intel_dp->aux.dev = dev->dev;
        intel_dp->aux.transfer = intel_dp_aux_transfer;

@@ -3849,26 +3871,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
                intel_connector->get_hw_state = intel_connector_get_hw_state;
        intel_connector->unregister = intel_dp_connector_unregister;
 
-       intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
-       if (HAS_DDI(dev)) {
-               switch (intel_dig_port->port) {
-               case PORT_A:
-                       intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
-                       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;
-                       break;
-               default:
-                       BUG();
-               }
-       }
-
        /* Set up the DDC bus. */
        switch (port) {
        case PORT_A:
--

1.7.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

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

Jani Nikula

author

2014-03-17 14:49 UTC #17 lore raw

Re: [PATCH 5/6] drm/i915/dp: move dp aux ch register init to aux init

Quoted 2 lines · Rodrigo Vivi

I would prefer an else instead of unecessary comparisons and assignment that will be override..

In general I agree, but see next patch. ;)

BR,
Jani.

Quoted 85 lines

But anyway it works, so feel free to use:

Reviewed-by Rodrigo Vivi <[email protected]>

Quoted 74 lines · Jani Nikula

Do a slight rearrangement of the switch to prep for follow-up.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c |   42 ++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +22 ~~-21~~

index 24cbf4bd36c4..71a76d00d634 100644
@@ -643,6 +643,28 @@ static void
 intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 {
        struct drm_device *dev = intel_dp_to_dev(intel_dp);
+       struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+       enum port port = intel_dig_port->port;
+
+       switch (port) {
+       case PORT_A:
+               intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
+               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;
+               break;
+       default:
+               BUG();
+       }
+
+       if (!HAS_DDI(dev))
+               intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
 
        intel_dp->aux.dev = dev->dev;
        intel_dp->aux.transfer = intel_dp_aux_transfer;

@@ -3849,26 +3871,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
                intel_connector->get_hw_state = intel_connector_get_hw_state;
        intel_connector->unregister = intel_dp_connector_unregister;
 
-       intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
-       if (HAS_DDI(dev)) {
-               switch (intel_dig_port->port) {
-               case PORT_A:
-                       intel_dp->aux_ch_ctl_reg = DPA_AUX_CH_CTL;
-                       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;
-                       break;
-               default:
-                       BUG();
-               }
-       }
-
        /* Set up the DDC bus. */
        switch (port) {
        case PORT_A:
--

1.7.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

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

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Jani Nikula

author

2014-03-14 14:51 UTC #18 lore raw

PATCH 6/6 drm/i915/dp: use the new drm helpers for dp i2c-over-aux

In reply to Jani Nikula

The functionality remains largerly the same. The main difference is that i2c-over-aux defer timeouts are increased to be safe for all use cases instead of depending on DP device type and properties.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c  |  197 ++++++--------------------------------
 drivers/gpu/drm/i915/intel_drv.h |    2 -
 2 files changed, 30 insertions(+), 169 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +30 ~~-167~~

index 71a76d00d634..208fdf075140 100644
@@ -645,19 +645,25 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	enum port port = intel_dig_port->port;
+	const char *name = NULL;
+	int ret;
 
 	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;
 	default:
 		BUG();

@@ -666,128 +672,27 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
 	if (!HAS_DDI(dev))
 		intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
 
+	intel_dp->aux.name = name;
 	intel_dp->aux.dev = dev->dev;
 	intel_dp->aux.transfer = intel_dp_aux_transfer;
-}
 
-static int
-intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
-		    uint8_t write_byte, uint8_t *read_byte)
-{
-	struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
-	struct intel_dp *intel_dp = container_of(adapter,
-						struct intel_dp,
-						adapter);
-	uint16_t address = algo_data->address;
-	uint8_t msg[5];
-	uint8_t reply[2];
-	unsigned retry;
-	int msg_bytes;
-	int reply_bytes;
-	int ret;
-
-	/* Set up the command byte */
-	if (mode & MODE_I2C_READ)
-		msg[0] = DP_AUX_I2C_READ << 4;
-	else
-		msg[0] = DP_AUX_I2C_WRITE << 4;
+	DRM_DEBUG_KMS("registering %s bus for %s\n", name,
+		      connector->base.kdev->kobj.name);
 
-	if (!(mode & MODE_I2C_STOP))
-		msg[0] |= DP_AUX_I2C_MOT << 4;
-
-	msg[1] = address >> 8;
-	msg[2] = address;
-
-	switch (mode) {
-	case MODE_I2C_WRITE:
-		msg[3] = 0;
-		msg[4] = write_byte;
-		msg_bytes = 5;
-		reply_bytes = 1;
-		break;
-	case MODE_I2C_READ:
-		msg[3] = 0;
-		msg_bytes = 4;
-		reply_bytes = 2;
-		break;
-	default:
-		msg_bytes = 3;
-		reply_bytes = 1;
-		break;
+	ret = drm_dp_aux_register_i2c_bus(&intel_dp->aux);
+	if (ret < 0) {
+		DRM_ERROR("drm_dp_aux_register_i2c_bus() for %s failed (%d)\n",
+			  name, ret);
+		return;
 	}
 
-	/*
-	 * DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source device is
-	 * required to retry at least seven times upon receiving AUX_DEFER
-	 * before giving up the AUX transaction.
-	 */
-	for (retry = 0; retry < 7; retry++) {
-		ret = intel_dp_aux_ch(intel_dp,
-				      msg, msg_bytes,
-				      reply, reply_bytes);
-		if (ret < 0) {
-			DRM_DEBUG_KMS("aux_ch failed %d\n", ret);
-			goto out;
-		}
-
-		switch ((reply[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK) {
-		case DP_AUX_NATIVE_REPLY_ACK:
-			/* I2C-over-AUX Reply field is only valid
-			 * when paired with AUX ACK.
-			 */
-			break;
-		case DP_AUX_NATIVE_REPLY_NACK:
-			DRM_DEBUG_KMS("aux_ch native nack\n");
-			ret = -EREMOTEIO;
-			goto out;
-		case DP_AUX_NATIVE_REPLY_DEFER:
-			/*
-			 * For now, just give more slack to branch devices. We
-			 * could check the DPCD for I2C bit rate capabilities,
-			 * and if available, adjust the interval. We could also
-			 * be more careful with DP-to-Legacy adapters where a
-			 * long legacy cable may force very low I2C bit rates.
-			 */
-			if (intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
-			    DP_DWN_STRM_PORT_PRESENT)
-				usleep_range(500, 600);
-			else
-				usleep_range(300, 400);
-			continue;
-		default:
-			DRM_ERROR("aux_ch invalid native reply 0x%02x\n",
-				  reply[0]);
-			ret = -EREMOTEIO;
-			goto out;
-		}
-
-		switch ((reply[0] >> 4) & DP_AUX_I2C_REPLY_MASK) {
-		case DP_AUX_I2C_REPLY_ACK:
-			if (mode == MODE_I2C_READ) {
-				*read_byte = reply[1];
-			}
-			ret = reply_bytes - 1;
-			goto out;
-		case DP_AUX_I2C_REPLY_NACK:
-			DRM_DEBUG_KMS("aux_i2c nack\n");
-			ret = -EREMOTEIO;
-			goto out;
-		case DP_AUX_I2C_REPLY_DEFER:
-			DRM_DEBUG_KMS("aux_i2c defer\n");
-			udelay(100);
-			break;
-		default:
-			DRM_ERROR("aux_i2c invalid reply 0x%02x\n", reply[0]);
-			ret = -EREMOTEIO;
-			goto out;
-		}
+	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_i2c_bus(&intel_dp->aux);
 	}
-
-	DRM_ERROR("too many retries, giving up\n");
-	ret = -EREMOTEIO;
-
-out:
-	return ret;
 }
 
 static void

@@ -796,43 +701,10 @@ intel_dp_connector_unregister(struct intel_connector *intel_connector)
 	struct intel_dp *intel_dp = intel_attached_dp(&intel_connector->base);
 
 	sysfs_remove_link(&intel_connector->base.kdev->kobj,
-			  intel_dp->adapter.dev.kobj.name);
+			  intel_dp->aux.ddc.dev.kobj.name);
 	intel_connector_unregister(intel_connector);
 }
 
-static int
-intel_dp_i2c_init(struct intel_dp *intel_dp,
-		  struct intel_connector *intel_connector, const char *name)
-{
-	int	ret;
-
-	DRM_DEBUG_KMS("i2c_init %s\n", name);
-	intel_dp->algo.running = false;
-	intel_dp->algo.address = 0;
-	intel_dp->algo.aux_ch = intel_dp_i2c_aux_ch;
-
-	memset(&intel_dp->adapter, '\0', sizeof(intel_dp->adapter));
-	intel_dp->adapter.owner = THIS_MODULE;
-	intel_dp->adapter.class = I2C_CLASS_DDC;
-	strncpy(intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) - 1);
-	intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0';
-	intel_dp->adapter.algo_data = &intel_dp->algo;
-	intel_dp->adapter.dev.parent = intel_connector->base.dev->dev;
-
-	ret = i2c_dp_aux_add_bus(&intel_dp->adapter);
-	if (ret < 0)
-		return ret;
-
-	ret = sysfs_create_link(&intel_connector->base.kdev->kobj,
-				&intel_dp->adapter.dev.kobj,
-				intel_dp->adapter.dev.kobj.name);
-
-	if (ret < 0)
-		i2c_del_adapter(&intel_dp->adapter);
-
-	return ret;
-}
-
 static void
 intel_dp_set_clock(struct intel_encoder *encoder,
 		   struct intel_crtc_config *pipe_config, int link_bw)

@@ -3098,7 +2970,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 	}
 
 	/* If no HPD, poke DDC gently */
-	if (drm_probe_ddc(&intel_dp->adapter))
+	if (drm_probe_ddc(&intel_dp->aux.ddc))
 		return connector_status_connected;
 
 	/* Well we tried, say unknown for unreliable port types */

@@ -3266,7 +3138,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	if (intel_dp->force_audio != HDMI_AUDIO_AUTO) {
 		intel_dp->has_audio = (intel_dp->force_audio == HDMI_AUDIO_ON);
 	} else {
-		edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+		edid = intel_dp_get_edid(connector, &intel_dp->aux.ddc);
 		if (edid) {
 			intel_dp->has_audio = drm_detect_monitor_audio(edid);
 			kfree(edid);

@@ -3302,7 +3174,7 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
-	ret = intel_dp_get_edid_modes(connector, &intel_dp->adapter);
+	ret = intel_dp_get_edid_modes(connector, &intel_dp->aux.ddc);
 	intel_display_power_put(dev_priv, power_domain);
 	if (ret)
 		return ret;

@@ -3335,7 +3207,7 @@ intel_dp_detect_audio(struct drm_connector *connector)
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
-	edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+	edid = intel_dp_get_edid(connector, &intel_dp->aux.ddc);
 	if (edid) {
 		has_audio = drm_detect_monitor_audio(edid);
 		kfree(edid);

@@ -3457,7 +3329,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	i2c_del_adapter(&intel_dp->adapter);
+	drm_dp_aux_unregister_i2c_bus(&intel_dp->aux);
 	drm_encoder_cleanup(encoder);
 	if (is_edp(intel_dp)) {
 		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);

@@ -3769,7 +3641,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	/* We now know it's not a ghost, init power sequence regs. */
 	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, power_seq);
 
-	edid = drm_get_edid(connector, &intel_dp->adapter);
+	edid = drm_get_edid(connector, &intel_dp->aux.ddc);
 	if (edid) {
 		if (drm_add_edid_modes(connector, edid)) {
 			drm_mode_connector_update_edid_property(connector,

@@ -3817,8 +3689,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum port port = intel_dig_port->port;
 	struct edp_power_seq power_seq = { 0 };
-	const char *name = NULL;
-	int type, error;
+	int type;
 
 	/* intel_dp vfuncs */
 	if (IS_VALLEYVIEW(dev))

@@ -3871,23 +3742,19 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		intel_connector->get_hw_state = intel_connector_get_hw_state;
 	intel_connector->unregister = intel_dp_connector_unregister;
 
-	/* Set up the DDC bus. */
+	/* Set up the hotplug pin. */
 	switch (port) {
 	case PORT_A:
 		intel_encoder->hpd_pin = HPD_PORT_A;
-		name = "DPDDC-A";
 		break;
 	case PORT_B:
 		intel_encoder->hpd_pin = HPD_PORT_B;
-		name = "DPDDC-B";
 		break;
 	case PORT_C:
 		intel_encoder->hpd_pin = HPD_PORT_C;
-		name = "DPDDC-C";
 		break;
 	case PORT_D:
 		intel_encoder->hpd_pin = HPD_PORT_D;
-		name = "DPDDC-D";
 		break;
 	default:
 		BUG();

@@ -3900,14 +3767,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 
 	intel_dp_aux_init(intel_dp, intel_connector);
 
-	error = intel_dp_i2c_init(intel_dp, intel_connector, name);
-	WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
-	     error, port_name(port));
-
 	intel_dp->psr_setup_done = false;
 
 	if (!intel_edp_init_connector(intel_dp, intel_connector, &power_seq)) {
-		i2c_del_adapter(&intel_dp->adapter);
+		drm_dp_aux_unregister_i2c_bus(&intel_dp->aux);
 		if (is_edp(intel_dp)) {
 			cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
 			mutex_lock(&dev->mode_config.mutex);

drivers/gpu/drm/i915/intel_drv.h +0 -2

index 561af4b013b1..604d57e5e4c8 100644
@@ -489,8 +489,6 @@ struct intel_dp {
 	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;
 	struct drm_dp_aux aux;
 	uint8_t train_set[4];
 	int panel_power_up_delay;

Signature · 1 line

1.7.9.5

Rodrigo Vivi

Reviewed-by on v1 6/6

2014-03-17 14:45 UTC #19 lore raw

Re: [PATCH 6/6] drm/i915/dp: use the new drm helpers for dp i2c-over-aux

Quoted 3 lines · Jani Nikula

The functionality remains largerly the same. The main difference is that i2c-over-aux defer timeouts are increased to be safe for all use cases instead of depending on DP device type and properties.

I was going to ask about different timeouts but you had already answered here ;)

Quoted 335 lines

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c  |  197 ++++++--------------------------------
 drivers/gpu/drm/i915/intel_drv.h |    2 -
 2 files changed, 30 insertions(+), 169 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +30 ~~-167~~

index 71a76d00d634..208fdf075140 100644
@@ -645,19 +645,25 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
        struct drm_device *dev = intel_dp_to_dev(intel_dp);
        struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
        enum port port = intel_dig_port->port;
+       const char *name = NULL;
+       int ret;
 
        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;
        default:
                BUG();

@@ -666,128 +672,27 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct intel_connector *connector)
        if (!HAS_DDI(dev))
                intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
 
+       intel_dp->aux.name = name;
        intel_dp->aux.dev = dev->dev;
        intel_dp->aux.transfer = intel_dp_aux_transfer;
-}
 
-static int
-intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
-                   uint8_t write_byte, uint8_t *read_byte)
-{
-       struct i2c_algo_dp_aux_data *algo_data = adapter->algo_data;
-       struct intel_dp *intel_dp = container_of(adapter,
-                                               struct intel_dp,
-                                               adapter);
-       uint16_t address = algo_data->address;
-       uint8_t msg[5];
-       uint8_t reply[2];
-       unsigned retry;
-       int msg_bytes;
-       int reply_bytes;
-       int ret;
-
-       /* Set up the command byte */
-       if (mode & MODE_I2C_READ)
-               msg[0] = DP_AUX_I2C_READ << 4;
-       else
-               msg[0] = DP_AUX_I2C_WRITE << 4;
+       DRM_DEBUG_KMS("registering %s bus for %s\n", name,
+                     connector->base.kdev->kobj.name);
 
-       if (!(mode & MODE_I2C_STOP))
-               msg[0] |= DP_AUX_I2C_MOT << 4;
-
-       msg[1] = address >> 8;
-       msg[2] = address;
-
-       switch (mode) {
-       case MODE_I2C_WRITE:
-               msg[3] = 0;
-               msg[4] = write_byte;
-               msg_bytes = 5;
-               reply_bytes = 1;
-               break;
-       case MODE_I2C_READ:
-               msg[3] = 0;
-               msg_bytes = 4;
-               reply_bytes = 2;
-               break;
-       default:
-               msg_bytes = 3;
-               reply_bytes = 1;
-               break;
+       ret = drm_dp_aux_register_i2c_bus(&intel_dp->aux);
+       if (ret < 0) {
+               DRM_ERROR("drm_dp_aux_register_i2c_bus() for %s failed (%d)\n",
+                         name, ret);
+               return;
        }
 
-       /*
-        * DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source device is
-        * required to retry at least seven times upon receiving AUX_DEFER
-        * before giving up the AUX transaction.
-        */
-       for (retry = 0; retry < 7; retry++) {
-               ret = intel_dp_aux_ch(intel_dp,
-                                     msg, msg_bytes,
-                                     reply, reply_bytes);
-               if (ret < 0) {
-                       DRM_DEBUG_KMS("aux_ch failed %d\n", ret);
-                       goto out;
-               }
-
-               switch ((reply[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK) {
-               case DP_AUX_NATIVE_REPLY_ACK:
-                       /* I2C-over-AUX Reply field is only valid
-                        * when paired with AUX ACK.
-                        */
-                       break;
-               case DP_AUX_NATIVE_REPLY_NACK:
-                       DRM_DEBUG_KMS("aux_ch native nack\n");
-                       ret = -EREMOTEIO;
-                       goto out;
-               case DP_AUX_NATIVE_REPLY_DEFER:
-                       /*
-                        * For now, just give more slack to branch devices. We
-                        * could check the DPCD for I2C bit rate capabilities,
-                        * and if available, adjust the interval. We could also
-                        * be more careful with DP-to-Legacy adapters where a
-                        * long legacy cable may force very low I2C bit rates.
-                        */
-                       if (intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
-                           DP_DWN_STRM_PORT_PRESENT)
-                               usleep_range(500, 600);
-                       else
-                               usleep_range(300, 400);
-                       continue;
-               default:
-                       DRM_ERROR("aux_ch invalid native reply 0x%02x\n",
-                                 reply[0]);
-                       ret = -EREMOTEIO;
-                       goto out;
-               }
-
-               switch ((reply[0] >> 4) & DP_AUX_I2C_REPLY_MASK) {
-               case DP_AUX_I2C_REPLY_ACK:
-                       if (mode == MODE_I2C_READ) {
-                               *read_byte = reply[1];
-                       }
-                       ret = reply_bytes - 1;
-                       goto out;
-               case DP_AUX_I2C_REPLY_NACK:
-                       DRM_DEBUG_KMS("aux_i2c nack\n");
-                       ret = -EREMOTEIO;
-                       goto out;
-               case DP_AUX_I2C_REPLY_DEFER:
-                       DRM_DEBUG_KMS("aux_i2c defer\n");
-                       udelay(100);
-                       break;
-               default:
-                       DRM_ERROR("aux_i2c invalid reply 0x%02x\n", reply[0]);
-                       ret = -EREMOTEIO;
-                       goto out;
-               }
+       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_i2c_bus(&intel_dp->aux);
        }
-
-       DRM_ERROR("too many retries, giving up\n");
-       ret = -EREMOTEIO;
-
-out:
-       return ret;
 }
 
 static void

@@ -796,43 +701,10 @@ intel_dp_connector_unregister(struct intel_connector *intel_connector)
        struct intel_dp *intel_dp = intel_attached_dp(&intel_connector->base);
 
        sysfs_remove_link(&intel_connector->base.kdev->kobj,
-                         intel_dp->adapter.dev.kobj.name);
+                         intel_dp->aux.ddc.dev.kobj.name);
        intel_connector_unregister(intel_connector);
 }
 
-static int
-intel_dp_i2c_init(struct intel_dp *intel_dp,
-                 struct intel_connector *intel_connector, const char *name)
-{
-       int     ret;
-
-       DRM_DEBUG_KMS("i2c_init %s\n", name);
-       intel_dp->algo.running = false;
-       intel_dp->algo.address = 0;
-       intel_dp->algo.aux_ch = intel_dp_i2c_aux_ch;
-
-       memset(&intel_dp->adapter, '\0', sizeof(intel_dp->adapter));
-       intel_dp->adapter.owner = THIS_MODULE;
-       intel_dp->adapter.class = I2C_CLASS_DDC;
-       strncpy(intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) - 1);
-       intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0';
-       intel_dp->adapter.algo_data = &intel_dp->algo;
-       intel_dp->adapter.dev.parent = intel_connector->base.dev->dev;
-
-       ret = i2c_dp_aux_add_bus(&intel_dp->adapter);
-       if (ret < 0)
-               return ret;
-
-       ret = sysfs_create_link(&intel_connector->base.kdev->kobj,
-                               &intel_dp->adapter.dev.kobj,
-                               intel_dp->adapter.dev.kobj.name);
-
-       if (ret < 0)
-               i2c_del_adapter(&intel_dp->adapter);
-
-       return ret;
-}
-
 static void
 intel_dp_set_clock(struct intel_encoder *encoder,
                   struct intel_crtc_config *pipe_config, int link_bw)

@@ -3098,7 +2970,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
        }
 
        /* If no HPD, poke DDC gently */
-       if (drm_probe_ddc(&intel_dp->adapter))
+       if (drm_probe_ddc(&intel_dp->aux.ddc))
                return connector_status_connected;
 
        /* Well we tried, say unknown for unreliable port types */

@@ -3266,7 +3138,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
        if (intel_dp->force_audio != HDMI_AUDIO_AUTO) {
                intel_dp->has_audio = (intel_dp->force_audio == HDMI_AUDIO_ON);
        } else {
-               edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+               edid = intel_dp_get_edid(connector, &intel_dp->aux.ddc);
                if (edid) {
                        intel_dp->has_audio = drm_detect_monitor_audio(edid);
                        kfree(edid);

@@ -3302,7 +3174,7 @@ static int intel_dp_get_modes(struct drm_connector *connector)
        power_domain = intel_display_port_power_domain(intel_encoder);
        intel_display_power_get(dev_priv, power_domain);
 
-       ret = intel_dp_get_edid_modes(connector, &intel_dp->adapter);
+       ret = intel_dp_get_edid_modes(connector, &intel_dp->aux.ddc);
        intel_display_power_put(dev_priv, power_domain);
        if (ret)
                return ret;

@@ -3335,7 +3207,7 @@ intel_dp_detect_audio(struct drm_connector *connector)
        power_domain = intel_display_port_power_domain(intel_encoder);
        intel_display_power_get(dev_priv, power_domain);
 
-       edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+       edid = intel_dp_get_edid(connector, &intel_dp->aux.ddc);
        if (edid) {
                has_audio = drm_detect_monitor_audio(edid);
                kfree(edid);

@@ -3457,7 +3329,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
        struct intel_dp *intel_dp = &intel_dig_port->dp;
        struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-       i2c_del_adapter(&intel_dp->adapter);
+       drm_dp_aux_unregister_i2c_bus(&intel_dp->aux);
        drm_encoder_cleanup(encoder);
        if (is_edp(intel_dp)) {
                cancel_delayed_work_sync(&intel_dp->panel_vdd_work);

@@ -3769,7 +3641,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
        /* We now know it's not a ghost, init power sequence regs. */
        intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, power_seq);
 
-       edid = drm_get_edid(connector, &intel_dp->adapter);
+       edid = drm_get_edid(connector, &intel_dp->aux.ddc);
        if (edid) {
                if (drm_add_edid_modes(connector, edid)) {
                        drm_mode_connector_update_edid_property(connector,

@@ -3817,8 +3689,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
        struct drm_i915_private *dev_priv = dev->dev_private;
        enum port port = intel_dig_port->port;
        struct edp_power_seq power_seq = { 0 };
-       const char *name = NULL;
-       int type, error;
+       int type;
 
        /* intel_dp vfuncs */
        if (IS_VALLEYVIEW(dev))

@@ -3871,23 +3742,19 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
                intel_connector->get_hw_state = intel_connector_get_hw_state;
        intel_connector->unregister = intel_dp_connector_unregister;
 
-       /* Set up the DDC bus. */
+       /* Set up the hotplug pin. */
        switch (port) {
        case PORT_A:
                intel_encoder->hpd_pin = HPD_PORT_A;
-               name = "DPDDC-A";
                break;
        case PORT_B:
                intel_encoder->hpd_pin = HPD_PORT_B;
-               name = "DPDDC-B";
                break;
        case PORT_C:
                intel_encoder->hpd_pin = HPD_PORT_C;
-               name = "DPDDC-C";
                break;
        case PORT_D:
                intel_encoder->hpd_pin = HPD_PORT_D;
-               name = "DPDDC-D";
                break;
        default:
                BUG();

@@ -3900,14 +3767,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 
        intel_dp_aux_init(intel_dp, intel_connector);
 
-       error = intel_dp_i2c_init(intel_dp, intel_connector, name);
-       WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
-            error, port_name(port));
-
        intel_dp->psr_setup_done = false;
 
        if (!intel_edp_init_connector(intel_dp, intel_connector, &power_seq)) {
-               i2c_del_adapter(&intel_dp->adapter);
+               drm_dp_aux_unregister_i2c_bus(&intel_dp->aux);
                if (is_edp(intel_dp)) {
                        cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
                        mutex_lock(&dev->mode_config.mutex);

drivers/gpu/drm/i915/intel_drv.h +0 -0

index 561af4b013b1..604d57e5e4c8 100644

drivers/gpu/drm/i915/intel_drv.h struct intel_dp { · line 493

Quoted 5 lines

drivers/gpu/drm/i915/intel_drv.h +0 -2

@@ -489,8 +489,6 @@ struct intel_dp {
        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;

is this algo really not needed/used anymore? If this is true patch seems correct so feel free to use:

Reviewed-by Rodrigo Vivi <[email protected]>

Quoted 10 lines

drivers/gpu/drm/i915/intel_drv.h +0 -1
        struct drm_dp_aux aux;
        uint8_t train_set[4];
        int panel_power_up_delay;
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines

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