All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/12] drm: Add helper for DP++ adaptors
@ 2016-04-04 12:01 Shashank Sharma
  2016-04-04 12:01 ` [PATCH 02/12] drm/i915: Respect DP++ adaptor TMDS clock limit Shashank Sharma
                   ` (11 more replies)
  0 siblings, 12 replies; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

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

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add a helper which aids in he identification of DP dual mode (aka. DP++)
adaptors. There are several types of adaptors specified:
type 1 DVI, type 1 HDMI, type 2 DVI, type 2 HDMI

Type 1 adaptors have a max TMDS clock limit of 165MHz, type 2 adaptors
may go as high as 300MHz and they provide a register informing the
source device what the actual limit is. Supposedly also type 1 adaptors
may optionally implement this register. This TMDS clock limit is the
main reason why we need to identify these adaptors.

Type 1 adaptors provide access to their internal registers and the sink
DDC bus through I2C. Type 2 adaptors provide this access both via I2C
and I2C-over-AUX. A type 2 source device may choose to implement either
or both of these methods. If a source device implements only the
I2C-over-AUX method, then the driver will obviously need specific
support for such adaptors since the port is driven like an HDMI port,
but DDC communication happes over the AUX channel.

This helper should be enough to identify the adaptor type (some
type 1 DVI adaptors may be a slight exception) and the maximum TMDS
clock limit. Another feature that may be available is control over
the TMDS output buffers on the adaptor, possibly allowing for some
power saving when the TMDS link is down.

Other user controllable features that may be available in the adaptors
are downstream i2c bus speed control when using i2c-over-aux, and
some control over the CEC pin. I chose not to provide any helper
functions for those since I have no use for them in i915 at this time.
The rest of the registers in the adaptor are mostly just information,
eg. IEEE OUI, hardware and firmware revision, etc.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/Makefile                  |   2 +-
 drivers/gpu/drm/drm_dp_dual_mode_helper.c | 331 ++++++++++++++++++++++++++++++
 include/drm/drm_dp_dual_mode_helper.h     |  84 ++++++++
 3 files changed, 416 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_dp_dual_mode_helper.c
 create mode 100644 include/drm/drm_dp_dual_mode_helper.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6eb94fc..22228ef 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -23,7 +23,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
 
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
 		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
-		drm_kms_helper_common.o
+		drm_kms_helper_common.o drm_dp_dual_mode_helper.o
 
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
 drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
new file mode 100644
index 0000000..b72b7bb
--- /dev/null
+++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
@@ -0,0 +1,331 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <drm/drm_dp_dual_mode_helper.h>
+#include <drm/drmP.h>
+
+/**
+ * DOC: DP dual mode (aka. DP++) adaptor helpers
+ *
+ * Helper functions to deal with DP dual mode adaptors.
+ *
+ * Type 1:
+ * Adaptor registers (if any) and the sink DDC bus may be accessed via I2C.
+ *
+ * Type 2:
+ * Adaptor registers and sink DDC bus can be accessed either via I2C or
+ * I2C-over-AUX. Source devices may choose to implement either one or
+ * both of these access methods.
+ */
+
+#define DP_DUAL_MODE_SLAVE_ADDRESS 0x40
+
+/**
+ * drm_dp_dual_mode_read - Read from the DP dual mode adaptor register(s)
+ * adapter: I2C adapter for the DDC bus
+ * offset: register offset
+ * buffer: buffer for return data
+ * size: sizo of the buffer
+ *
+ * Reads @size bytes from the DP dual mode adaptor registers
+ * starting at @offset.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure
+ */
+ssize_t drm_dp_dual_mode_read(struct i2c_adapter *adapter,
+			      u8 offset, void *buffer, size_t size)
+{
+	struct i2c_msg msgs[] = {
+		{
+			.addr = DP_DUAL_MODE_SLAVE_ADDRESS,
+			.flags = 0,
+			.len = 1,
+			.buf = &offset,
+		},
+		{
+			.addr = DP_DUAL_MODE_SLAVE_ADDRESS,
+			.flags = I2C_M_RD,
+			.len = size,
+			.buf = buffer,
+		},
+	};
+	int ret;
+
+	ret = i2c_transfer(adapter, msgs, ARRAY_SIZE(msgs));
+	if (ret < 0)
+		return ret;
+	if (ret != ARRAY_SIZE(msgs))
+		return -EPROTO;
+
+	return 0;
+}
+
+/**
+ * drm_dp_dual_mode_write - Write to the DP dual mode adaptor register(s)
+ * adapter: I2C adapter for the DDC bus
+ * offset: register offset
+ * buffer: buffer for write data
+ * size: sizo of the buffer
+ *
+ * Writes @size bytes to the DP dual mode adaptor registers
+ * starting at @offset.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure
+ */
+ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
+			       u8 offset, const void *buffer, size_t size)
+{
+	struct i2c_msg msg = {
+		.addr = DP_DUAL_MODE_SLAVE_ADDRESS,
+		.flags = 0,
+		.len = 1 + size,
+		.buf = NULL,
+	};
+	void *data;
+	int ret;
+
+	data = kmalloc(msg.len, GFP_TEMPORARY);
+	if (!data)
+		return -ENOMEM;
+
+	msg.buf = data;
+
+	memcpy(data, &offset, 1);
+	memcpy(data + 1, buffer, size);
+
+	ret = i2c_transfer(adapter, &msg, 1);
+
+	kfree(data);
+
+	if (ret < 0)
+		return ret;
+	if (ret != 1)
+		return -EPROTO;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_dual_mode_write);
+
+static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
+{
+	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
+		"DP-HDMI ADAPTOR\x04";
+
+	return memcmp(hdmi_id, dp_dual_mode_hdmi_id,
+		      sizeof(dp_dual_mode_hdmi_id)) == 0;
+}
+
+/**
+ * drm_dp_dual_mode_detect - Identyfy the DP dual mode adaptor
+ * adapter: I2C adapter for the DDC bus
+ *
+ * Attempt to identify the type of the DP dual mode adaptor used.
+ *
+ * Note that when the answer is @DRM_DP_DUAL_MODE_NONE it's not
+ * certain whether we're dealing with a native HDMI port or
+ * a type 1 DVI dual mode adaptor. The driver will have to use
+ * some other hardware/driver specific mechanism to make that
+ * distinction.
+ *
+ * Returns:
+ * The type of the DP dual mode adaptor used
+ */
+enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
+{
+	char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] = {};
+	uint8_t adaptor_id = 0x00;
+	ssize_t ret;
+
+	/*
+	 * Let's see if the adaptor is there the by reading the
+	 * HDMI ID registers.
+	 *
+	 * Note that type 1 DVI adaptors are not required to implemnt
+	 * any registers, and that presents a problem for detection.
+	 * If the i2c transfer is nacked, we may or may not be dealing
+	 * with a type 1 DVI adaptor. Some other mechanism of detecting
+	 * the presence of the adaptor is required. One way would be
+	 * to check the state of the CONFIG1 pin, Another method would
+	 * simply require the driver to know whether the port is a DP++
+	 * port or a native HDMI port. Both of these methods are entirely
+	 * hardware/driver specific so we can't deal with them here.
+	 */
+	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_HDMI_ID,
+				    hdmi_id, sizeof(hdmi_id));
+	if (ret)
+		return DRM_DP_DUAL_MODE_NONE;
+
+	/*
+	 * Sigh. Some (maybe all?) type 1 adaptors are broken and ack
+	 * the offset but ignore it, and instead they just always return
+	 * data from the start of the HDMI ID buffer. So for a broken
+	 * type 1 HDMI adaptor a single byte read will always give us
+	 * 0x44, and for a type 1 DVI adaptor it should give 0x00
+	 * (assuming it implements any registers). Fortunately neither
+	 * of those values will match the type 2 signature of the
+	 * DP_DUAL_MODE_ADAPTOR_ID register so we can proceed with
+	 * the type 2 adaptor detection safely even in the presence
+	 * of broken type 1 adaptors.
+	 */
+	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_ADAPTOR_ID,
+				    &adaptor_id, sizeof(adaptor_id));
+	if (ret || (adaptor_id != (DP_DUAL_MODE_TYPE_TYPE2 |
+				   DP_DUAL_MODE_REV_TYPE2))) {
+		if (is_hdmi_adaptor(hdmi_id))
+			return DRM_DP_DUAL_MODE_TYPE1_HDMI;
+		else
+			return DRM_DP_DUAL_MODE_TYPE1_DVI;
+	} else {
+		if (is_hdmi_adaptor(hdmi_id))
+			return DRM_DP_DUAL_MODE_TYPE2_HDMI;
+		else
+			return DRM_DP_DUAL_MODE_TYPE2_DVI;
+	}
+}
+EXPORT_SYMBOL(drm_dp_dual_mode_detect);
+
+/**
+ * drm_dp_dual_mode_max_tmds_clock - Max TMDS clock for DP dual mode adaptor
+ * adapter: I2C adapter for the DDC bus
+ *
+ * Determine the max TMDS clock the adaptor supports based on the
+ * DP_DUAL_MODE_MAX_TMDS_CLOCK register. The register is mandatory for
+ * type 2 adaptors, optional for type 1 adaptors. Hoever, as some type 1
+ * adaptors are broken (see comments in drm_dp_dual_mode_detect() for the
+ * details) one probably shouldn't use this with type 1 adaptors at all.
+ * Type 1 adaptors should anyway be always limited to 165 MHz.
+ *
+ * Returns:
+ * Maximum supported TMDS clock rate for the DP dual mode adaptor in kHz.
+ */
+int drm_dp_dual_mode_max_tmds_clock(struct i2c_adapter *adapter)
+{
+	uint8_t max_tmds_clock;
+	ssize_t ret;
+
+	/*
+	 * Type 1 adaptors are limited to 165MHz
+	 * Type 2 adaptors can tells us their limit
+	 */
+	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_MAX_TMDS_CLOCK,
+				    &max_tmds_clock, sizeof(max_tmds_clock));
+	if (ret)
+		return 165000;
+
+	return max_tmds_clock * 5000 / 2;
+}
+EXPORT_SYMBOL(drm_dp_dual_mode_max_tmds_clock);
+
+/**
+ * drm_dp_dual_mode_get_tmds_output - Get the state of the TMDS
+ * output buffers in the DP dual mode adaptor
+ * adapter: I2C adapter for the DDC bus
+ * enabled: current state of the TMDS output buffers
+ *
+ * Get the state of the TMDS output buffers in the adaptor.
+ * DP_DUAL_MODE_TMDS_OEN register is mandatory for type 2 adaptors,
+ * optionals for type 1 adaptors. Hoever, as some type 1 adaptors are
+ * broken (see comments in drm_dp_dual_mode_detect() for the details)
+ * one probably shouldn't use this with type 1 adaptors at all.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure
+ */
+int drm_dp_dual_mode_get_tmds_output(struct i2c_adapter *adapter, bool *enabled)
+{
+	uint8_t tmds_oen;
+	ssize_t ret;
+
+	ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_TMDS_OEN,
+				    &tmds_oen, sizeof(tmds_oen));
+	if (ret)
+		return ret;
+
+	*enabled = !(tmds_oen & DP_DUAL_MODE_TMDS_DISABLE);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_dual_mode_get_tmds_output);
+
+/**
+ * drm_dp_dual_mode_set_tmds_output - Enable/disable TMDS
+ * output buffers in the DP dual mode adaptor
+ * adapter: I2C adapter for the DDC bus
+ * enable: enable (as opposed to disable) the TMDS output buffers
+ *
+ * Set the state of the TMDS output buffers in the adaptor.
+ * DP_DUAL_MODE_TMDS_OEN register is mandatory for type 2 adaptors,
+ * optionals for type 1 adaptors. Hoever, as some type 1 adaptors are
+ * broken (see comments in drm_dp_dual_mode_detect() for the details)
+ * one probably shouldn't use this with type 1 adaptors at all.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure
+ */
+int drm_dp_dual_mode_set_tmds_output(struct i2c_adapter *adapter, bool enable)
+{
+	uint8_t tmds_oen = enable ? 0 : DP_DUAL_MODE_TMDS_DISABLE;
+	ssize_t ret;
+
+	ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
+				     &tmds_oen, sizeof(tmds_oen));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_dp_dual_mode_set_tmds_output);
+
+/**
+ * drm_dp_get_dual_mode_type_name - Get the name of the
+ * DP dual mode adaptor type as a string
+ * type: DP dual mode adaptor type
+ *
+ * Returns:
+ * String representation of the DP dual mode adaptor type
+ */
+const char *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type)
+{
+	switch (type) {
+	case DRM_DP_DUAL_MODE_NONE:
+		return "none";
+	case DRM_DP_DUAL_MODE_TYPE1_DVI:
+		return "type 1 DVI";
+	case DRM_DP_DUAL_MODE_TYPE1_HDMI:
+		return "type 1 HDMI";
+	case DRM_DP_DUAL_MODE_TYPE2_DVI:
+		return "type 2 DVI";
+	case DRM_DP_DUAL_MODE_TYPE2_HDMI:
+		return "type 2 HDMI";
+	default:
+		return "unknown";
+	};
+
+}
+EXPORT_SYMBOL(drm_dp_get_dual_mode_type_name);
diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
new file mode 100644
index 0000000..8f8a8dc
--- /dev/null
+++ b/include/drm/drm_dp_dual_mode_helper.h
@@ -0,0 +1,84 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef DRM_DP_DUAL_MODE_HELPER_H
+#define DRM_DP_DUAL_MODE_HELPER_H
+
+#include <linux/types.h>
+
+/*
+ * Optional for type 1 DVI adaptors
+ * Mandatory for type 1 HDMI and type 2 adators
+ */
+#define DP_DUAL_MODE_HDMI_ID 0x00 /* 00-0f */
+#define  DP_DUAL_MODE_HDMI_ID_LEN 16
+/*
+ * Optional for type 1 adaptors
+ * Mandatory for type 2 adators
+ */
+#define DP_DUAL_MODE_ADAPTOR_ID 0x10
+#define  DP_DUAL_MODE_REV_MASK 0x07
+#define  DP_DUAL_MODE_REV_TYPE2 0x00
+#define  DP_DUAL_MODE_TYPE_MASK 0xf0
+#define  DP_DUAL_MODE_TYPE_TYPE2 0xa0
+#define DP_DUAL_MODE_IEEE_OUI 0x11 /* 11-13*/
+#define  DP_DUAL_IEEE_OUI_LEN 3
+#define DP_DUAL_DEVICE_ID 0x14 /* 14-19 */
+#define  DP_DUAL_DEVICE_ID_LEN 6
+#define DP_DUAL_MODE_HARDWARE_REV 0x1a
+#define DP_DUAL_MODE_FIRMWARE_MAJOR_REV 0x1b
+#define DP_DUAL_MODE_FIRMWARE_MINOR_REV 0x1c
+#define DP_DUAL_MODE_MAX_TMDS_CLOCK 0x1d
+#define DP_DUAL_MODE_I2C_SPEED_CAP 0x1e
+#define DP_DUAL_MODE_TMDS_OEN 0x20
+#define  DP_DUAL_MODE_TMDS_DISABLE 0x01
+#define DP_DUAL_MODE_HDMI_PIN_CTRL 0x21
+#define  DP_DUAL_MODE_CEC_ENABLE 0x01
+#define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
+#define DP_DUAL_MODE_LAST_RESERVED 0xff
+
+struct i2c_adapter;
+
+ssize_t drm_dp_dual_mode_read(struct i2c_adapter *adapter,
+			      u8 offset, void *buffer, size_t size);
+ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
+			       u8 offset, const void *buffer, size_t size);
+
+enum drm_dp_dual_mode_type {
+	DRM_DP_DUAL_MODE_NONE,
+	DRM_DP_DUAL_MODE_TYPE1_DVI,
+	DRM_DP_DUAL_MODE_TYPE1_HDMI,
+	DRM_DP_DUAL_MODE_TYPE2_DVI,
+	DRM_DP_DUAL_MODE_TYPE2_HDMI,
+};
+
+enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
+int
+drm_dp_dual_mode_max_tmds_clock(struct i2c_adapter *adapter);
+int
+drm_dp_dual_mode_get_tmds_output(struct i2c_adapter *adapter, bool *enabled);
+int
+drm_dp_dual_mode_set_tmds_output(struct i2c_adapter *adapter, bool enable);
+const char
+*drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
+
+#endif
-- 
1.9.1

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

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

* [PATCH 02/12] drm/i915: Respect DP++ adaptor TMDS clock limit
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-04-04 12:01 ` [PATCH 03/12] drm/i915: Enable/disable TMDS output buffers in DP++ adaptor as needed Shashank Sharma
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

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

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Try to detect the max TMDS clock limit for the DP++ adaptor (if any)
and take it into account when checking the port clock.

Note that as with the sink (HDMI vs. DVI) TMDS clock limit we'll ignore
the adaptor TMDS clock limit in the modeset path, in case users are
already "overclocking" their TMDS links. One subtle change here is that
we'll have to respect the adaptor TMDS clock limit when we decide whether
to do 12bpc or 8bpc, otherwise we might end up picking 12bpc and
accidentally driving the TMDS link out of spec even when the user chose
a mode that fits wihting the limits at 8bpc. This means you can't
"overclock" your DP++ dongle at 12bpc anymore, but you can continue to
do so at 8bpc.

Note that for simplicity we'll use the I2C access method for all dual
mode adaptors including type 2. Otherwise we'd have to start mixing
DP AUX and HDMI together. In the future we may need to do that if we
come across any board designs that don't hook up the DDC pins to the
DP++ connectors. Such boards would obviously only work with type 2
dual mode adaptors, and not type 1.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |  3 ++
 drivers/gpu/drm/i915/intel_hdmi.c | 65 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5136eef..d456bed 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -744,6 +744,9 @@ struct cxsr_latency {
 struct intel_hdmi {
 	i915_reg_t hdmi_reg;
 	int ddc_bus;
+	struct {
+		int max_tmds_clock;
+	} dp_dual_mode;
 	bool limited_color_range;
 	bool color_range_auto;
 	bool has_hdmi_sink;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e2dab48..bdbdb69 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -34,6 +34,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_dp_dual_mode_helper.h>
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
@@ -1165,27 +1166,42 @@ static void pch_post_disable_hdmi(struct intel_encoder *encoder)
 	intel_disable_hdmi(encoder);
 }
 
-static int hdmi_port_clock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit)
+static int intel_hdmi_source_max_tmds_clock(struct drm_i915_private *dev_priv)
 {
-	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
-
-	if ((respect_dvi_limit && !hdmi->has_hdmi_sink) || IS_G4X(dev))
+	if (IS_G4X(dev_priv))
 		return 165000;
-	else if (IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8)
+	else if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
 		return 300000;
 	else
 		return 225000;
 }
 
+static int hdmi_port_clock_limit(struct intel_hdmi *hdmi,
+				 bool respect_downstream_limits)
+{
+	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
+	int max_tmds_clock = intel_hdmi_source_max_tmds_clock(to_i915(dev));
+
+	if (respect_downstream_limits) {
+		if (hdmi->dp_dual_mode.max_tmds_clock)
+			max_tmds_clock = min(max_tmds_clock,
+					     hdmi->dp_dual_mode.max_tmds_clock);
+		if (!hdmi->has_hdmi_sink)
+			max_tmds_clock = min(max_tmds_clock, 165000);
+	}
+
+	return max_tmds_clock;
+}
+
 static enum drm_mode_status
 hdmi_port_clock_valid(struct intel_hdmi *hdmi,
-		      int clock, bool respect_dvi_limit)
+		      int clock, bool respect_downstream_limits)
 {
 	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
 
 	if (clock < 25000)
 		return MODE_CLOCK_LOW;
-	if (clock > hdmi_port_clock_limit(hdmi, respect_dvi_limit))
+	if (clock > hdmi_port_clock_limit(hdmi, respect_downstream_limits))
 		return MODE_CLOCK_HIGH;
 
 	/* BXT DPLL can't generate 223-240 MHz */
@@ -1309,7 +1325,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	 * within limits.
 	 */
 	if (pipe_config->pipe_bpp > 8*3 && pipe_config->has_hdmi_sink &&
-	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc, false) == MODE_OK &&
+	    hdmi_port_clock_valid(intel_hdmi, clock_12bpc, true) == MODE_OK &&
 	    hdmi_12bpc_possible(pipe_config)) {
 		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
 		desired_bpp = 12*3;
@@ -1349,10 +1365,41 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
 	intel_hdmi->has_audio = false;
 	intel_hdmi->rgb_quant_range_selectable = false;
 
+	intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
+
 	kfree(to_intel_connector(connector)->detect_edid);
 	to_intel_connector(connector)->detect_edid = NULL;
 }
 
+static void
+intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector)
+{
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+	struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
+	struct i2c_adapter *adapter =
+		intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus);
+	enum drm_dp_dual_mode_type type = drm_dp_dual_mode_detect(adapter);
+
+	if (type == DRM_DP_DUAL_MODE_NONE)
+		return;
+
+	/*
+	 * Type 1 adaptors can be broken unfortunately, so let's
+	 * not trust anything they say beyond the type identification.
+	 */
+	if (type == DRM_DP_DUAL_MODE_TYPE2_DVI ||
+	    type == DRM_DP_DUAL_MODE_TYPE2_HDMI) {
+		hdmi->dp_dual_mode.max_tmds_clock =
+			drm_dp_dual_mode_max_tmds_clock(adapter);
+	} else {
+		hdmi->dp_dual_mode.max_tmds_clock = 165000;
+	}
+
+	DRM_DEBUG_KMS("Adaptor (%s) detected (max TMDS clock: %d kHz)\n",
+		      drm_dp_get_dual_mode_type_name(type),
+		      hdmi->dp_dual_mode.max_tmds_clock);
+}
+
 static bool
 intel_hdmi_set_edid(struct drm_connector *connector, bool force)
 {
@@ -1368,6 +1415,8 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
 				    intel_gmbus_get_adapter(dev_priv,
 				    intel_hdmi->ddc_bus));
 
+		intel_hdmi_dp_dual_mode_detect(connector);
+
 		intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
 	}
 
-- 
1.9.1

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

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

* [PATCH 03/12] drm/i915: Enable/disable TMDS output buffers in DP++ adaptor as needed
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
  2016-04-04 12:01 ` [PATCH 02/12] drm/i915: Respect DP++ adaptor TMDS clock limit Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-04-04 12:01 ` [PATCH 04/12] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT Shashank Sharma
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

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

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

To save a bit of power, let's try to turn off the TMDS output buffers
in DP++ adaptors when we're not driving the port.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d456bed..ab514bd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -746,6 +746,7 @@ struct intel_hdmi {
 	int ddc_bus;
 	struct {
 		int max_tmds_clock;
+		bool tmds_output_control;
 	} dp_dual_mode;
 	bool limited_color_range;
 	bool color_range_auto;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index bdbdb69..3a93337 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -846,6 +846,13 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder)
 	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
 	u32 hdmi_val;
 
+	if (intel_hdmi->dp_dual_mode.tmds_output_control) {
+		struct i2c_adapter *adapter =
+			intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
+
+		drm_dp_dual_mode_set_tmds_output(adapter, true);
+	}
+
 	hdmi_val = SDVO_ENCODING_HDMI;
 	if (!HAS_PCH_SPLIT(dev) && crtc->config->limited_color_range)
 		hdmi_val |= HDMI_COLOR_RANGE_16_235;
@@ -1141,6 +1148,13 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
 	}
 
 	intel_hdmi->set_infoframes(&encoder->base, false, NULL);
+
+	if (intel_hdmi->dp_dual_mode.tmds_output_control) {
+		struct i2c_adapter *adapter =
+			intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
+
+		drm_dp_dual_mode_set_tmds_output(adapter, false);
+	}
 }
 
 static void g4x_disable_hdmi(struct intel_encoder *encoder)
@@ -1366,6 +1380,7 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
 	intel_hdmi->rgb_quant_range_selectable = false;
 
 	intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
+	intel_hdmi->dp_dual_mode.tmds_output_control = false;
 
 	kfree(to_intel_connector(connector)->detect_edid);
 	to_intel_connector(connector)->detect_edid = NULL;
@@ -1389,15 +1404,25 @@ intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector)
 	 */
 	if (type == DRM_DP_DUAL_MODE_TYPE2_DVI ||
 	    type == DRM_DP_DUAL_MODE_TYPE2_HDMI) {
+		bool tmds_enabled;
+
 		hdmi->dp_dual_mode.max_tmds_clock =
 			drm_dp_dual_mode_max_tmds_clock(adapter);
+		hdmi->dp_dual_mode.tmds_output_control =
+			drm_dp_dual_mode_get_tmds_output(adapter,
+				&tmds_enabled) == 0 &&
+			drm_dp_dual_mode_set_tmds_output(adapter,
+				tmds_enabled) == 0;
 	} else {
 		hdmi->dp_dual_mode.max_tmds_clock = 165000;
+		hdmi->dp_dual_mode.tmds_output_control = false;
 	}
 
 	DRM_DEBUG_KMS("Adaptor (%s) detected (max TMDS clock: %d kHz)\n",
 		      drm_dp_get_dual_mode_type_name(type),
 		      hdmi->dp_dual_mode.max_tmds_clock);
+	DRM_DEBUG_KMS("TMDS OE# control: %s)\n",
+			yesno(hdmi->dp_dual_mode.tmds_output_control));
 }
 
 static bool
-- 
1.9.1

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

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

* [PATCH 04/12] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
  2016-04-04 12:01 ` [PATCH 02/12] drm/i915: Respect DP++ adaptor TMDS clock limit Shashank Sharma
  2016-04-04 12:01 ` [PATCH 03/12] drm/i915: Enable/disable TMDS output buffers in DP++ adaptor as needed Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-05-02 14:33   ` Jani Nikula
  2016-04-04 12:01 ` [PATCH 05/12] drm/i915: Add lspcon data structures Shashank Sharma
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

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

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

DP dual mode type 1 DVI adaptors aren't required to implement any
registers, so it's a bit hard to detect them. The best way would
be to check the state of the CONFIG1 pin, but we have no way to
do that. So as a last resort, check the VBT to see if the HDMI
port is in fact a dual mode capable DP port.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  2 ++
 drivers/gpu/drm/i915/intel_bios.c     | 32 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c       |  5 +++++
 drivers/gpu/drm/i915/intel_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c     | 23 +++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_vbt_defs.h |  3 +++
 6 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f330a53..65bb83f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3373,6 +3373,8 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
 bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
 bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
 bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
+bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
+		enum port port);
 
 /* intel_opregion.c */
 #ifdef CONFIG_ACPI
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 083003b..39c520a 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1550,6 +1550,38 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
 	return false;
 }
 
+bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
+				enum port port)
+{
+	const union child_device_config *p_child;
+	int i;
+	static const short port_mapping[] = {
+		[PORT_B] = DVO_PORT_DPB,
+		[PORT_C] = DVO_PORT_DPC,
+		[PORT_D] = DVO_PORT_DPD,
+		[PORT_E] = DVO_PORT_DPE,
+	};
+
+	if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
+		return false;
+
+	if (!dev_priv->vbt.child_dev_num)
+		return false;
+
+	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+		p_child = &dev_priv->vbt.child_dev[i];
+
+		if (p_child->common.dvo_port == port_mapping[port] &&
+		    (p_child->common.device_type &
+				DEVICE_TYPE_DP_DUAL_MODE_BITS) ==
+			(DEVICE_TYPE_DP_DUAL_MODE &
+				DEVICE_TYPE_DP_DUAL_MODE_BITS))
+			return true;
+	}
+	return false;
+}
+
+
 /**
  * intel_bios_is_dsi_present - is DSI present in VBT
  * @dev_priv:	i915 device instance
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3ff8f1d..ba4da0c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5007,6 +5007,11 @@ bool intel_dp_is_edp(struct drm_device *dev, enum port port)
 	return intel_bios_is_port_edp(dev_priv, port);
 }
 
+bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv, enum port port)
+{
+	return intel_bios_is_dp_dual_mode(dev_priv, port);
+}
+
 void
 intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ab514bd..26ef950 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1276,6 +1276,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
 bool intel_dp_compute_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config);
 bool intel_dp_is_edp(struct drm_device *dev, enum port port);
+bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv, enum port port);
 enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
 				  bool long_hpd);
 void intel_edp_backlight_on(struct intel_dp *intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 3a93337..22b5a7e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1387,14 +1387,33 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
 }
 
 static void
-intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector)
+intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector, bool has_edid)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
+	enum port port = hdmi_to_dig_port(hdmi)->port;
 	struct i2c_adapter *adapter =
 		intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus);
 	enum drm_dp_dual_mode_type type = drm_dp_dual_mode_detect(adapter);
 
+	/*
+	 * Type 1 DVI adaptors are not required to implement any
+	 * registers, so we can't always detect their presence.
+	 * Ideally we should be able to check the state of the
+	 * CONFIG1 pin, but no such luck on our hardware.
+	 *
+	 * The only method left to us is to check the VBT to see
+	 * if the port is a dual mode capable DP port. But let's
+	 * only do that when we sucesfully read the EDID, to avoid
+	 * confusing log messages about DP dual mode adaptors when
+	 * there's nothing connected to the port.
+	 */
+	if (type == DRM_DP_DUAL_MODE_NONE && has_edid &&
+	    intel_dp_is_dual_mode(dev_priv, port)) {
+		DRM_DEBUG_KMS("Assuming DP dual mode adaptor (as per VBT)\n");
+		type = DRM_DP_DUAL_MODE_TYPE1_DVI;
+	}
+
 	if (type == DRM_DP_DUAL_MODE_NONE)
 		return;
 
@@ -1440,7 +1459,7 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
 				    intel_gmbus_get_adapter(dev_priv,
 				    intel_hdmi->ddc_bus));
 
-		intel_hdmi_dp_dual_mode_detect(connector);
+		intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
 
 		intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
 	}
diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
index 749dcea..ebaecdb 100644
--- a/drivers/gpu/drm/i915/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
@@ -734,6 +734,7 @@ struct bdb_psr {
 #define	 DEVICE_TYPE_INT_TV	0x1009
 #define	 DEVICE_TYPE_HDMI	0x60D2
 #define	 DEVICE_TYPE_DP		0x68C6
+#define	 DEVICE_TYPE_DP_DUAL_MODE 0x60D6
 #define	 DEVICE_TYPE_eDP	0x78C6
 
 #define  DEVICE_TYPE_CLASS_EXTENSION	(1 << 15)
@@ -768,6 +769,8 @@ struct bdb_psr {
 	 DEVICE_TYPE_DISPLAYPORT_OUTPUT | \
 	 DEVICE_TYPE_ANALOG_OUTPUT)
 
+#define DEVICE_TYPE_DP_DUAL_MODE_BITS ~DEVICE_TYPE_NOT_HDMI_OUTPUT
+
 /* define the DVO port for HDMI output type */
 #define		DVO_B		1
 #define		DVO_C		2
-- 
1.9.1

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

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

* [PATCH 05/12] drm/i915: Add lspcon data structures
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
                   ` (2 preceding siblings ...)
  2016-04-04 12:01 ` [PATCH 04/12] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-04-04 12:01 ` [PATCH 06/12] drm/i915: Add new lspcon file Shashank Sharma
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

This patch adds lspcon structure in intel_dig_port.
These strucres will be used to check runtime status
of LSPCON device.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 26ef950..6e309ea 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -859,12 +859,28 @@ struct intel_dp {
 	bool compliance_test_active;
 };
 
+/* LSPCON possibe modes of operation */
+enum lspcon_mode {
+	/* Invalid */
+	lspcon_mode_invalid,
+	/* level shifter mode */
+	lspcon_mode_ls,
+	/* protocol converter mode */
+	lspcon_mode_pcon,
+};
+
+struct intel_lspcon {
+	bool active;
+	enum lspcon_mode mode_of_op;
+};
+
 struct intel_digital_port {
 	struct intel_encoder base;
 	enum port port;
 	u32 saved_port_bits;
 	struct intel_dp dp;
 	struct intel_hdmi hdmi;
+	struct intel_lspcon lspcon;
 	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
 	bool release_cl2_override;
 	uint8_t max_lanes;
-- 
1.9.1

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

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

* [PATCH 06/12] drm/i915: Add new lspcon file
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
                   ` (3 preceding siblings ...)
  2016-04-04 12:01 ` [PATCH 05/12] drm/i915: Add lspcon data structures Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-05-02 13:37   ` Ville Syrjälä
  2016-05-02 14:35   ` Jani Nikula
  2016-04-04 12:01 ` [PATCH 07/12] drm/i915: Add and initialize lspcon connector Shashank Sharma
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

This patch adds a new file for lspcon with
some basic stuff like:
- Some read/wrire addresses for lspcon device
- Basic read/write functions, using i2c over aux channel
- Utility functions to get lspcon/encoder/connector

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
---
 drivers/gpu/drm/i915/Makefile       |  3 +-
 drivers/gpu/drm/i915/intel_lspcon.c | 56 +++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/intel_lspcon.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 5558a03..00a531a 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -90,7 +90,8 @@ i915-y += dvo_ch7017.o \
 	  intel_lvds.o \
 	  intel_panel.o \
 	  intel_sdvo.o \
-	  intel_tv.o
+	  intel_tv.o \
+	  intel_lspcon.o
 
 # virtual gpu code
 i915-y += i915_vgpu.o
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
new file mode 100644
index 0000000..5a1993b
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -0,0 +1,56 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Shashank Sharma <shashank.sharma@intel.com>
+ * Akashdeep Sharma <akashdeep.sharma@intel.com>
+ *
+ */
+#include <drm/drm_edid.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_dp_dual_mode_helper.h>
+#include "intel_drv.h"
+
+#define LSPCON_I2C_ADDRESS			0x80
+#define LSPCON_MODE_CHANGE_OFFSET		0x40
+#define LSPCON_MODE_CHECK_OFFSET		0x41
+#define LSPCON_ADAPTER_SIGN_OFFSET		0x00
+#define LSPCON_IDENTIFIER_OFFSET		0x10
+#define LSPCON_IDENTIFIER_LENGTH		0x10
+#define LSPCON_MODE_MASK			0x1
+
+struct intel_digital_port *lspcon_to_dig_port(struct intel_lspcon *lspcon)
+{
+	return container_of(lspcon, struct intel_digital_port, lspcon);
+}
+
+struct intel_hdmi *lspcon_to_hdmi(struct intel_lspcon *lspcon)
+{
+	return &lspcon_to_dig_port(lspcon)->hdmi;
+}
+
+struct intel_lspcon *enc_to_lspcon(struct drm_encoder *encoder)
+{
+	struct intel_digital_port *intel_dig_port =
+		container_of(encoder, struct intel_digital_port, base.base);
+	return &intel_dig_port->lspcon;
+}
-- 
1.9.1

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

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

* [PATCH 07/12] drm/i915: Add and initialize lspcon connector
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
                   ` (4 preceding siblings ...)
  2016-04-04 12:01 ` [PATCH 06/12] drm/i915: Add new lspcon file Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-04-25 21:34   ` Zanoni, Paulo R
  2016-04-04 12:01 ` [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support Shashank Sharma
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

lspcon is a device which acts as DP in some cases
and as HDMI in some. Here is how:
- lspcon needs DP(aux) for all the i2c_ove_aux read/write
  transitions so it needs to have some DP level initializations
- lspcon is detected by userspace/sink as HDMI device, so
  it needs to be detectd as HDMI device.

This patch adds a custom connector for lspcon device, which
can pick and chose what it wants from existing functionality.

This patch also adds functions to initialize dp and hdmi connectors
and structures to the minimum required levels, and then play around.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c     | 31 +++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h    |  4 ++
 drivers/gpu/drm/i915/intel_hdmi.c   | 17 ++++++++
 drivers/gpu/drm/i915/intel_lspcon.c | 79 +++++++++++++++++++++++++++++++++++++
 4 files changed, 131 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ba4da0c..f3df1c6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5900,6 +5900,37 @@ fail:
 	return false;
 }
 
+int intel_dp_init_minimum(struct intel_digital_port *intel_dig_port,
+	struct intel_connector *intel_connector)
+{
+	int ret;
+	enum port port = intel_dig_port->port;
+	struct intel_dp *intel_dp = &intel_dig_port->dp;
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	intel_dig_port->dp.output_reg = DDI_BUF_CTL(port);
+	if (WARN(intel_dig_port->max_lanes < 1,
+		 "Not enough lanes (%d) for DP on port %c\n",
+		 intel_dig_port->max_lanes, port_name(port)))
+		return -EINVAL;
+
+	intel_dp->pps_pipe = INVALID_PIPE;
+	intel_dp->get_aux_clock_divider = skl_get_aux_clock_divider;
+	intel_dp->get_aux_send_ctl = skl_get_aux_send_ctl;
+	intel_dp->prepare_link_retrain = intel_ddi_prepare_link_retrain;
+	intel_dp->DP = I915_READ(intel_dp->output_reg);
+	intel_dp->attached_connector = intel_connector;
+	INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
+			  edp_panel_vdd_work);
+
+	ret = intel_dp_aux_init(intel_dp, intel_connector);
+	if (ret)
+		DRM_ERROR("Aux init for LSPCON failed\n");
+
+	return ret;
+}
+
 void
 intel_dp_init(struct drm_device *dev,
 	      i915_reg_t output_reg, enum port port)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6e309ea..d38db7d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1332,6 +1332,8 @@ void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
 bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
 bool
 intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]);
+int intel_dp_init_minimum(struct intel_digital_port *intel_dig_port,
+		struct intel_connector *intel_connector);
 
 /* intel_dp_mst.c */
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
@@ -1397,6 +1399,8 @@ void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv);
 void intel_hdmi_init(struct drm_device *dev, i915_reg_t hdmi_reg, enum port port);
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			       struct intel_connector *intel_connector);
+int intel_hdmi_init_minimum(struct intel_digital_port *intel_dig_port,
+				struct intel_connector *intel_connector);
 struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
 bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct intel_crtc_state *pipe_config);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 22b5a7e..92f5094 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2128,6 +2128,23 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
+int intel_hdmi_init_minimum(struct intel_digital_port *intel_dig_port,
+			       struct intel_connector *intel_connector)
+{
+	struct intel_hdmi *intel_hdmi = &intel_dig_port->hdmi;
+
+	if (WARN(intel_dig_port->max_lanes < 4,
+		"Not enough lanes (%d) for HDMI on port %c\n",
+		intel_dig_port->max_lanes, port_name(intel_dig_port->port)))
+		return -EINVAL;
+
+	intel_hdmi->write_infoframe = hsw_write_infoframe;
+	intel_hdmi->set_infoframes = hsw_set_infoframes;
+	intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
+	intel_hdmi->attached_connector = intel_connector;
+	return 0;
+}
+
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 			       struct intel_connector *intel_connector)
 {
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 5a1993b..e179758 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -54,3 +54,82 @@ struct intel_lspcon *enc_to_lspcon(struct drm_encoder *encoder)
 		container_of(encoder, struct intel_digital_port, base.base);
 	return &intel_dig_port->lspcon;
 }
+
+int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
+{
+	int ret;
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
+	struct drm_device *dev = intel_encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_connector *intel_connector;
+	struct drm_connector *connector;
+	enum port port = intel_dig_port->port;
+
+	intel_connector = intel_connector_alloc();
+	if (!intel_connector)
+		return -ENOMEM;
+
+	connector = &intel_connector->base;
+	connector->interlace_allowed = true;
+	connector->doublescan_allowed = 0;
+
+	/* init DP */
+	ret = intel_dp_init_minimum(intel_dig_port, intel_connector);
+	if (ret) {
+		DRM_ERROR("DP init for LSPCON failed\n");
+		return ret;
+	}
+
+	/* init HDMI */
+	ret = intel_hdmi_init_minimum(intel_dig_port, intel_connector);
+	if (ret) {
+		DRM_ERROR("HDMI init for LSPCON failed\n");
+		return ret;
+	}
+
+	/* Set up the hotplug pin. */
+	switch (port) {
+	case PORT_A:
+		intel_encoder->hpd_pin = HPD_PORT_A;
+		break;
+	case PORT_B:
+		intel_encoder->hpd_pin = HPD_PORT_B;
+		if (IS_BXT_REVID(dev, 0, BXT_REVID_A1))
+			intel_encoder->hpd_pin = HPD_PORT_A;
+		break;
+	case PORT_C:
+		intel_encoder->hpd_pin = HPD_PORT_C;
+		break;
+	case PORT_D:
+		intel_encoder->hpd_pin = HPD_PORT_D;
+		break;
+	case PORT_E:
+		intel_encoder->hpd_pin = HPD_PORT_E;
+		break;
+	default:
+		DRM_ERROR("Invalid port to configure LSPCON\n");
+		return -EINVAL;
+	}
+
+	/*
+	* On BXT A0/A1, sw needs to activate DDIA HPD logic and
+	* interrupts to check the external panel connection.
+	*/
+	if (IS_BXT_REVID(dev, 0, BXT_REVID_A1) && port == PORT_B)
+		dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
+	else
+		dev_priv->hotplug.irq_port[port] = intel_dig_port;
+
+	/* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written
+	* 0xd.  Failure to do so will result in spurious interrupts being
+	* generated on the port when a cable is not attached.
+	*/
+	if (IS_G4X(dev) && !IS_GM45(dev)) {
+		u32 temp = I915_READ(PEG_BAND_GAP_DATA);
+
+		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
+	}
+
+	DRM_DEBUG_DRIVER("Success: LSPCON connector init\n");
+	return 0;
+}
-- 
1.9.1

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

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

* [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
                   ` (5 preceding siblings ...)
  2016-04-04 12:01 ` [PATCH 07/12] drm/i915: Add and initialize lspcon connector Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-05-02 13:49   ` Ville Syrjälä
  2016-04-04 12:01 ` [PATCH 09/12] drm/i915: Add and register lspcon connector functions Shashank Sharma
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

This patch adds support for LSPCON devices in dp++ adaptor
helper layer. LSPCON is DP++ type-2 adaptor with some customized
functionalities, to provide additional features in Intel Gen9 HW.

LSPCON needs I2C-over-aux support to read/write control and
data registers. This patch adds following:
  - Functions for I2C over aux read/write
  - Function to read EDID using I2c-over-aux
  - Function to identify LSPCON among type-2 DP adaptors

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_dp_dual_mode_helper.c | 100 ++++++++++++++++++++++++++++++
 include/drm/drm_dp_dual_mode_helper.h     |  10 +++
 2 files changed, 110 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
index b72b7bb..ce8e11c 100644
--- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
@@ -132,6 +132,99 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
 }
 EXPORT_SYMBOL(drm_dp_dual_mode_write);
 
+int drm_dp_dual_mode_get_edid(void *data,
+	u8 *buf, unsigned int block, size_t len)
+{
+	struct i2c_adapter *adapter = data;
+	unsigned char start = block * EDID_LENGTH;
+	unsigned char segment = block >> 1;
+	unsigned char xfers = segment ? 3 : 2;
+	int ret, retries = 5;
+
+	do {
+		struct i2c_msg msgs[] = {
+			{
+				.addr   = DP_DUAL_MODE_DDC_SEGMENT_ADDR,
+				.flags  = 0,
+				.len    = 1,
+				.buf    = &segment,
+			}, {
+				.addr   = DDC_ADDR,
+				.flags  = 0,
+				.len    = 1,
+				.buf    = &start,
+			}, {
+				.addr   = DDC_ADDR,
+				.flags  = I2C_M_RD,
+				.len    = len,
+				.buf    = buf,
+			}
+		};
+
+		ret = adapter->algo->master_xfer(adapter, &msgs[3 - xfers],
+						xfers);
+
+		if (ret == -ENXIO) {
+			DRM_ERROR("Non-existent adapter %s\n",
+				adapter->name);
+			break;
+		}
+	} while (ret != xfers && --retries);
+
+	return ret == xfers ? 0 : -1;
+}
+EXPORT_SYMBOL(drm_dp_dual_mode_get_edid);
+
+/*
+* drm_dp_dual_mode_ioa_xfer
+* Few dp->hdmi type 2 adaptors allow i2c_over_aux read/write
+* to the control and status registers. These functions help
+* to read/write from those.
+*/
+static int drm_dp_dual_mode_ioa_xfer(struct i2c_adapter *adapter,
+		u8 *buffer, u8 offset, u8 no_of_bytes, u8 rw_flag)
+{
+	int err = 0;
+
+	struct i2c_msg msgs[] = {
+			{
+				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
+				.flags	= 0,
+				.len	= 1,
+				.buf	= &offset,
+			}, {
+				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
+				.flags	= rw_flag,
+				.len	= no_of_bytes,
+				.buf	= buffer,
+			}
+	};
+
+	/* I2C over AUX here */
+	err = adapter->algo->master_xfer(adapter, msgs, 2);
+	if (err < 0)
+		DRM_ERROR("LSPCON: Failed I2C over Aux read(addr=0x%x)\n",
+				(unsigned int)offset);
+
+	return err;
+}
+
+int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
+		u8 offset, u8 no_of_bytes)
+{
+	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
+		no_of_bytes, I2C_M_RD);
+}
+EXPORT_SYMBOL(drm_dp_dual_mode_ioa_read);
+
+int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
+		u8 offset, u8 no_of_bytes)
+{
+	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
+		no_of_bytes, 0);
+}
+EXPORT_SYMBOL(drm_dp_dual_mode_ioa_write);
+
 static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
 {
 	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
@@ -141,6 +234,11 @@ static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
 		      sizeof(dp_dual_mode_hdmi_id)) == 0;
 }
 
+bool is_lspcon_adaptor(const uint8_t adaptor_id)
+{
+	return adaptor_id == 0xa8;
+}
+
 /**
  * drm_dp_dual_mode_detect - Identyfy the DP dual mode adaptor
  * adapter: I2C adapter for the DDC bus
@@ -197,6 +295,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
 				    &adaptor_id, sizeof(adaptor_id));
 	if (ret || (adaptor_id != (DP_DUAL_MODE_TYPE_TYPE2 |
 				   DP_DUAL_MODE_REV_TYPE2))) {
+		if (is_lspcon_adaptor(adaptor_id))
+			return DRM_DP_DUAL_MODE_TYPE2_LSPCON;
 		if (is_hdmi_adaptor(hdmi_id))
 			return DRM_DP_DUAL_MODE_TYPE1_HDMI;
 		else
diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
index 8f8a8dc..26e3dd8 100644
--- a/include/drm/drm_dp_dual_mode_helper.h
+++ b/include/drm/drm_dp_dual_mode_helper.h
@@ -24,6 +24,7 @@
 #define DRM_DP_DUAL_MODE_HELPER_H
 
 #include <linux/types.h>
+#include <drm/drm_edid.h>
 
 /*
  * Optional for type 1 DVI adaptors
@@ -55,6 +56,7 @@
 #define  DP_DUAL_MODE_CEC_ENABLE 0x01
 #define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
 #define DP_DUAL_MODE_LAST_RESERVED 0xff
+#define DP_DUAL_MODE_DDC_SEGMENT_ADDR 0x30
 
 struct i2c_adapter;
 
@@ -69,6 +71,7 @@ enum drm_dp_dual_mode_type {
 	DRM_DP_DUAL_MODE_TYPE1_HDMI,
 	DRM_DP_DUAL_MODE_TYPE2_DVI,
 	DRM_DP_DUAL_MODE_TYPE2_HDMI,
+	DRM_DP_DUAL_MODE_TYPE2_LSPCON,
 };
 
 enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
@@ -81,4 +84,11 @@ drm_dp_dual_mode_set_tmds_output(struct i2c_adapter *adapter, bool enable);
 const char
 *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
 
+int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
+		u8 offset, u8 no_of_bytes);
+int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
+		u8 offset, u8 no_of_bytes);
+int drm_dp_dual_mode_get_edid(void *data,
+	u8 *buf, unsigned int block, size_t len);
+
 #endif
-- 
1.9.1

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

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

* [PATCH 09/12] drm/i915: Add and register lspcon connector functions
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
                   ` (6 preceding siblings ...)
  2016-04-04 12:01 ` [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-04-04 12:01 ` [PATCH 10/12] drm/i915: Add lspcon core functions Shashank Sharma
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

This patch adds various lspcon connector functions. Some
of the functions are newly written, to meet the specific
needs of lspcon HW, whereas few of them are just an
abstraction layer on existing HDMI connector functions.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h     |  11 +-
 drivers/gpu/drm/i915/intel_hdmi.c    |   8 +-
 drivers/gpu/drm/i915/intel_hotplug.c |   2 +-
 drivers/gpu/drm/i915/intel_lspcon.c  | 195 ++++++++++++++++++++++++++++++++++-
 4 files changed, 207 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d38db7d..a6ec946 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1404,7 +1404,12 @@ int intel_hdmi_init_minimum(struct intel_digital_port *intel_dig_port,
 struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
 bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			       struct intel_crtc_state *pipe_config);
-
+int intel_hdmi_get_modes(struct drm_connector *connector);
+int intel_hdmi_set_property(struct drm_connector *connector,
+		struct drm_property *property, uint64_t val);
+void intel_hdmi_destroy(struct drm_connector *connector);
+void intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi,
+		struct drm_connector *connector);
 
 /* intel_lvds.c */
 void intel_lvds_init(struct drm_device *dev);
@@ -1493,6 +1498,10 @@ bool intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv,
 void intel_display_power_put(struct drm_i915_private *dev_priv,
 			     enum intel_display_power_domain domain);
 
+/* intel_hotplug.c */
+bool intel_hpd_irq_event(struct drm_device *dev,
+		struct drm_connector *connector);
+
 static inline void
 assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 92f5094..3f7919d 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1540,7 +1540,7 @@ intel_hdmi_force(struct drm_connector *connector)
 	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
 }
 
-static int intel_hdmi_get_modes(struct drm_connector *connector)
+int intel_hdmi_get_modes(struct drm_connector *connector)
 {
 	struct edid *edid;
 
@@ -1564,7 +1564,7 @@ intel_hdmi_detect_audio(struct drm_connector *connector)
 	return has_audio;
 }
 
-static int
+int
 intel_hdmi_set_property(struct drm_connector *connector,
 			struct drm_property *property,
 			uint64_t val)
@@ -2089,7 +2089,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
 	}
 }
 
-static void intel_hdmi_destroy(struct drm_connector *connector)
+void intel_hdmi_destroy(struct drm_connector *connector)
 {
 	kfree(to_intel_connector(connector)->detect_edid);
 	drm_connector_cleanup(connector);
@@ -2118,7 +2118,7 @@ static const struct drm_encoder_funcs intel_hdmi_enc_funcs = {
 	.destroy = intel_encoder_destroy,
 };
 
-static void
+void
 intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
 {
 	intel_attach_force_audio_property(connector);
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index bee6730..11a3e02 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -226,7 +226,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
 	intel_runtime_pm_put(dev_priv);
 }
 
-static bool intel_hpd_irq_event(struct drm_device *dev,
+bool intel_hpd_irq_event(struct drm_device *dev,
 				struct drm_connector *connector)
 {
 	enum drm_connector_status old_status;
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index e179758..c3c1cd2 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -55,6 +55,178 @@ struct intel_lspcon *enc_to_lspcon(struct drm_encoder *encoder)
 	return &intel_dig_port->lspcon;
 }
 
+static struct intel_lspcon
+*intel_attached_lspcon(struct drm_connector *connector)
+{
+	return enc_to_lspcon(&intel_attached_encoder(connector)->base);
+}
+
+struct edid *lspcon_get_edid(struct intel_lspcon *lspcon, struct drm_connector
+						*connector)
+{
+	struct edid *edid = NULL;
+	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
+	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
+
+	if (lspcon->mode_of_op != lspcon_mode_ls) {
+		DRM_ERROR("EDID read supported in LS mode only\n");
+		return false;
+	}
+
+	/* LS mode, getting EDID using I2C over Aux */
+	edid = drm_do_get_edid(connector, drm_dp_dual_mode_get_edid,
+			(void *)adapter);
+	return edid;
+}
+
+static void
+lspcon_unset_edid(struct drm_connector *connector)
+{
+	struct intel_lspcon *lspcon = intel_attached_lspcon(connector);
+	struct intel_hdmi *intel_hdmi = lspcon_to_hdmi(lspcon);
+
+	intel_hdmi->has_hdmi_sink = false;
+	intel_hdmi->has_audio = false;
+	intel_hdmi->rgb_quant_range_selectable = false;
+
+	kfree(to_intel_connector(connector)->detect_edid);
+	to_intel_connector(connector)->detect_edid = NULL;
+}
+
+static bool
+lspcon_set_edid(struct drm_connector *connector, bool force)
+{
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+	struct intel_lspcon *lspcon = intel_attached_lspcon(connector);
+	struct intel_hdmi *intel_hdmi = lspcon_to_hdmi(lspcon);
+	struct edid *edid = NULL;
+	bool connected = false;
+
+	if (force) {
+		intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
+		edid = lspcon_get_edid(lspcon, connector);
+		intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
+	}
+
+	to_intel_connector(connector)->detect_edid = edid;
+	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
+		intel_hdmi->rgb_quant_range_selectable =
+			drm_rgb_quant_range_selectable(edid);
+
+		intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
+		if (intel_hdmi->force_audio != HDMI_AUDIO_AUTO)
+			intel_hdmi->has_audio =
+				intel_hdmi->force_audio == HDMI_AUDIO_ON;
+
+		if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
+			intel_hdmi->has_hdmi_sink =
+				drm_detect_hdmi_monitor(edid);
+
+		connected = true;
+	}
+	return connected;
+}
+
+static enum drm_connector_status
+lspcon_detect(struct drm_connector *connector, bool force)
+{
+	enum drm_connector_status status;
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+	struct intel_lspcon *lspcon = intel_attached_lspcon(connector);
+	struct intel_hdmi *intel_hdmi = lspcon_to_hdmi(lspcon);
+
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
+		      connector->base.id, connector->name);
+	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
+
+	lspcon_unset_edid(connector);
+	if (lspcon_set_edid(connector, true)) {
+		DRM_DEBUG_DRIVER("HDMI connected\n");
+		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
+		status = connector_status_connected;
+	} else {
+		DRM_DEBUG_DRIVER("HDMI disconnected\n");
+		status = connector_status_disconnected;
+	}
+	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
+	return status;
+}
+
+static int
+lspcon_set_property(struct drm_connector *connector,
+			struct drm_property *property,
+			uint64_t val)
+{
+	return intel_hdmi_set_property(connector, property, val);
+}
+
+static int
+lspcon_get_modes(struct drm_connector *connector)
+{
+	return intel_hdmi_get_modes(connector);
+}
+
+static void
+lspcon_destroy(struct drm_connector *connector)
+{
+	intel_hdmi_destroy(connector);
+}
+
+static enum drm_mode_status
+lspcon_mode_valid(struct drm_connector *connector,
+		      struct drm_display_mode *mode)
+{
+	int clock = mode->clock;
+	int max_dotclk = 675000; /* 4k@60 */
+	struct drm_device *dev = connector->dev;
+
+	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
+		return MODE_NO_DBLESCAN;
+
+	if ((mode->flags & DRM_MODE_FLAG_3D_MASK) ==
+		DRM_MODE_FLAG_3D_FRAME_PACKING)
+		clock *= 2;
+
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+		clock *= 2;
+
+	if (clock < 25000)
+		return MODE_CLOCK_LOW;
+
+	if (clock > max_dotclk)
+		return MODE_CLOCK_HIGH;
+
+	/* BXT DPLL can't generate 223-240 MHz */
+	if (IS_BROXTON(dev) && clock > 223333 && clock < 240000)
+		return MODE_CLOCK_RANGE;
+
+	/* todo: check for 12bpc here */
+	return MODE_OK;
+}
+
+void lspcon_add_properties(struct intel_digital_port *dig_port,
+		struct drm_connector *connector)
+{
+	intel_hdmi_add_properties(&dig_port->hdmi, connector);
+}
+
+static const struct drm_connector_funcs lspcon_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.detect = lspcon_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.set_property = lspcon_set_property,
+	.atomic_get_property = intel_connector_atomic_get_property,
+	.destroy = lspcon_destroy,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+};
+
+static const struct drm_connector_helper_funcs lspcon_connector_helper_funcs = {
+	.get_modes = lspcon_get_modes,
+	.mode_valid = lspcon_mode_valid,
+	.best_encoder = intel_best_encoder,
+};
+
 int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
 {
 	int ret;
@@ -73,18 +245,30 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
 	connector->interlace_allowed = true;
 	connector->doublescan_allowed = 0;
 
+	/* Load connector */
+	drm_connector_init(dev, connector, &lspcon_connector_funcs,
+			DRM_MODE_CONNECTOR_DisplayPort);
+	drm_connector_helper_add(connector, &lspcon_connector_helper_funcs);
+	intel_connector_attach_encoder(intel_connector, intel_encoder);
+	drm_connector_register(connector);
+
+	/* Add properties and functions */
+	lspcon_add_properties(intel_dig_port, connector);
+	intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
+	i915_debugfs_connector_add(connector);
+
 	/* init DP */
 	ret = intel_dp_init_minimum(intel_dig_port, intel_connector);
 	if (ret) {
 		DRM_ERROR("DP init for LSPCON failed\n");
-		return ret;
+		goto fail;
 	}
 
 	/* init HDMI */
 	ret = intel_hdmi_init_minimum(intel_dig_port, intel_connector);
 	if (ret) {
 		DRM_ERROR("HDMI init for LSPCON failed\n");
-		return ret;
+		goto fail;
 	}
 
 	/* Set up the hotplug pin. */
@@ -108,7 +292,7 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
 		break;
 	default:
 		DRM_ERROR("Invalid port to configure LSPCON\n");
-		return -EINVAL;
+		ret = -EINVAL;
 	}
 
 	/*
@@ -132,4 +316,9 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
 
 	DRM_DEBUG_DRIVER("Success: LSPCON connector init\n");
 	return 0;
+
+fail:
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+	return ret;
 }
-- 
1.9.1

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

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

* [PATCH 10/12] drm/i915: Add lspcon core functions
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
                   ` (7 preceding siblings ...)
  2016-04-04 12:01 ` [PATCH 09/12] drm/i915: Add and register lspcon connector functions Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-05-02 13:51   ` Ville Syrjälä
  2016-04-04 12:01 ` [PATCH 11/12] drm/i915: Add lspcon hpd handler Shashank Sharma
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

This patch adds lspcon's internal functions, which work
on the probe layer, and indicate the working status of
lspcon, which are mostly:

probe: A lspcon device is probed only once, during boot
time, as its always present with the device, next to port.
So the i2c_over_aux channel is alwyas read/writeable if DC is
powered on. If VBT says that this port contains lspcon, we
check and probe the HW to verify and initialize it.

get_mode: This function indicates the current mode of operation
of lspcon (ls or pcon mode)

change_mode: This function can change the lspcon's mode of
operation to desired mode.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_lspcon.c | 145 ++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index c3c1cd2..617fe3f 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -61,6 +61,144 @@ static struct intel_lspcon
 	return enc_to_lspcon(&intel_attached_encoder(connector)->base);
 }
 
+enum lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
+{
+	u8 data;
+	int err = 0;
+	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
+	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
+
+	/* Read Status: i2c over aux */
+	err = drm_dp_dual_mode_ioa_read(adapter, &data,
+		LSPCON_MODE_CHECK_OFFSET, sizeof(data));
+	if (err < 0) {
+		DRM_ERROR("LSPCON read mode ioa (0x80, 0x41) failed\n");
+		return lspcon_mode_invalid;
+	}
+
+	DRM_DEBUG_DRIVER("LSPCON mode (0x80, 0x41) = %x\n", (unsigned int)data);
+	return data & LSPCON_MODE_MASK ? lspcon_mode_pcon : lspcon_mode_ls;
+}
+
+int lspcon_change_mode(struct intel_lspcon *lspcon,
+	enum lspcon_mode mode, bool force)
+{
+	u8 data;
+	int err;
+	int time_out = 200;
+	enum lspcon_mode current_mode;
+	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
+
+	current_mode = lspcon_get_current_mode(lspcon);
+	if (current_mode == lspcon_mode_invalid) {
+		DRM_ERROR("Failed to get current LSPCON mode\n");
+		return -EFAULT;
+	}
+
+	if (current_mode == mode && !force) {
+		DRM_DEBUG_DRIVER("Current mode = desired LSPCON mode\n");
+		return 0;
+	}
+
+	if (mode == lspcon_mode_ls)
+		data = ~LSPCON_MODE_MASK;
+	else
+		data = LSPCON_MODE_MASK;
+
+	/* Change mode */
+	err = drm_dp_dual_mode_ioa_write(&dig_port->dp.aux.ddc, &data,
+			LSPCON_MODE_CHANGE_OFFSET, sizeof(data));
+	if (err < 0) {
+		DRM_ERROR("LSPCON mode change failed\n");
+		return err;
+	}
+
+	/*
+	* Confirm mode change by reading the status bit.
+	* Sometimes, it takes a while to change the mode,
+	* so wait and retry until time out or done.
+	*/
+	while (time_out) {
+		current_mode = lspcon_get_current_mode(lspcon);
+		if (current_mode != mode) {
+			mdelay(10);
+			time_out -= 10;
+		} else {
+			lspcon->mode_of_op = mode;
+			DRM_DEBUG_DRIVER("LSPCON mode changed to %s\n",
+				mode == lspcon_mode_ls ? "LS" : "PCON");
+			return 0;
+		}
+	}
+
+	DRM_ERROR("LSPCON mode change timed out\n");
+	return -EFAULT;
+}
+
+bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
+{
+	enum drm_dp_dual_mode_type adaptor_type;
+	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
+	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
+
+	/* Lets probe the adaptor and check its type */
+	adaptor_type = drm_dp_dual_mode_detect(adapter);
+	if (adaptor_type != DRM_DP_DUAL_MODE_TYPE2_LSPCON) {
+		DRM_DEBUG_DRIVER("No LSPCON detected, found %s\n",
+			drm_dp_get_dual_mode_type_name(adaptor_type));
+		return false;
+	}
+
+	/* Yay ... got a LSPCON device */
+	DRM_DEBUG_DRIVER("LSPCON detected\n");
+	return true;
+}
+
+enum lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
+{
+	enum lspcon_mode current_mode;
+
+	/* Detect a valid lspcon */
+	if (!lspcon_detect_identifier(lspcon)) {
+		DRM_DEBUG_DRIVER("Failed to find LSPCON identifier\n");
+		return false;
+	}
+
+	/* LSPCON's mode of operation */
+	current_mode = lspcon_get_current_mode(lspcon);
+	if (current_mode == lspcon_mode_invalid) {
+		DRM_ERROR("Failed to read LSPCON mode\n");
+		return false;
+	}
+
+	/* All is well */
+	lspcon->mode_of_op = current_mode;
+	lspcon->active = true;
+	return current_mode;
+}
+
+bool lspcon_device_init(struct intel_lspcon *lspcon)
+{
+
+	/* Lets check LSPCON now, probe the HW status */
+	lspcon->active = false;
+	lspcon->mode_of_op = lspcon_mode_invalid;
+	if (!lspcon_probe(lspcon)) {
+		DRM_ERROR("Failed to probe lspcon");
+		return false;
+	}
+
+	/* We wish to keep LSPCON in LS mode */
+	if (lspcon->active && lspcon->mode_of_op != lspcon_mode_ls) {
+		if (lspcon_change_mode(lspcon, lspcon_mode_ls, true) < 0) {
+			DRM_ERROR("LSPCON mode change to LS failed\n");
+			return false;
+		}
+	}
+	DRM_DEBUG_DRIVER("LSPCON init success\n");
+	return true;
+}
+
 struct edid *lspcon_get_edid(struct intel_lspcon *lspcon, struct drm_connector
 						*connector)
 {
@@ -233,6 +371,7 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
 	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_device *dev = intel_encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
 	struct intel_connector *intel_connector;
 	struct drm_connector *connector;
 	enum port port = intel_dig_port->port;
@@ -314,6 +453,12 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
 		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
 	}
 
+	/* Now initialize the LSPCON device */
+	if (!lspcon_device_init(lspcon)) {
+		DRM_ERROR("LSPCON device init failed\n");
+		goto fail;
+	}
+
 	DRM_DEBUG_DRIVER("Success: LSPCON connector init\n");
 	return 0;
 
-- 
1.9.1

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

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

* [PATCH 11/12] drm/i915: Add lspcon hpd handler
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
                   ` (8 preceding siblings ...)
  2016-04-04 12:01 ` [PATCH 10/12] drm/i915: Add lspcon core functions Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-04-04 12:01 ` [PATCH 12/12] DO_NOT_MERGE: drm/i915: Hack to enable lspcon initialization Shashank Sharma
  2016-04-04 13:33 ` ✗ Fi.CI.BAT: failure for series starting with [01/12] drm: Add helper for DP++ adaptors Patchwork
  11 siblings, 0 replies; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

This patch adds a new hpd handler for lspcon.
As lspcon has its own way of reading EDID and detecting
the device, it wont be efficient to use the existing hpd
functions to handle the hot_plug scenarios. This new function
reads the EDID and checks the status of the sink device.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_lspcon.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 617fe3f..20f90e0 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -61,6 +61,38 @@ static struct intel_lspcon
 	return enc_to_lspcon(&intel_attached_encoder(connector)->base);
 }
 
+enum irqreturn
+lspcon_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
+{
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
+	struct intel_connector *intel_connector;
+	bool changed = false;
+
+	mutex_lock(&dev->mode_config.mutex);
+	if (intel_encoder->hot_plug)
+		intel_encoder->hot_plug(intel_encoder);
+
+	for_each_intel_connector(dev, intel_connector) {
+		if (intel_connector->encoder == intel_encoder) {
+			struct drm_connector *connector =
+				&intel_connector->base;
+
+			DRM_DEBUG_DRIVER("Hptplug: Connector %s (pin %i).\n",
+				connector->name, intel_encoder->hpd_pin);
+			if (intel_hpd_irq_event(dev, connector))
+				changed = true;
+		}
+	}
+	mutex_unlock(&dev->mode_config.mutex);
+
+	if (changed) {
+		DRM_DEBUG_DRIVER("Sending event for change\n");
+		drm_kms_helper_hotplug_event(dev);
+	}
+	return IRQ_HANDLED;
+}
+
 enum lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
 {
 	u8 data;
@@ -396,6 +428,9 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
 	intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
 	i915_debugfs_connector_add(connector);
 
+	/* HPD handler */
+	intel_dig_port->hpd_pulse = lspcon_hpd_pulse;
+
 	/* init DP */
 	ret = intel_dp_init_minimum(intel_dig_port, intel_connector);
 	if (ret) {
-- 
1.9.1

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

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

* [PATCH 12/12] DO_NOT_MERGE: drm/i915: Hack to enable lspcon initialization
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
                   ` (9 preceding siblings ...)
  2016-04-04 12:01 ` [PATCH 11/12] drm/i915: Add lspcon hpd handler Shashank Sharma
@ 2016-04-04 12:01 ` Shashank Sharma
  2016-04-06 12:18   ` Jani Nikula
  2016-04-04 13:33 ` ✗ Fi.CI.BAT: failure for series starting with [01/12] drm: Add helper for DP++ adaptors Patchwork
  11 siblings, 1 reply; 34+ messages in thread
From: Shashank Sharma @ 2016-04-04 12:01 UTC (permalink / raw)
  To: intel-gfx, ville.syrjala, daniel.vetter, paulo.r.zanoni,
	sonika.jindal, sivakumar.thulasimani

This patch adds a hack to enable lspcon on GEN9 devices.
This should not be merged, and the hack must be replaced
by proper VBT parsing logic.

Expecting this patch to enable lspcon bits in VBT:
https://lists.freedesktop.org/archives/intel-gfx/2016-March/089541.html

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    | 15 +++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h    |  4 ++++
 drivers/gpu/drm/i915/intel_lspcon.c | 10 ++++++++++
 3 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 91654ff..f6c2869 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2169,6 +2169,21 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 	intel_encoder->cloneable = 0;
 
+
+	/* Check if LSPCON is configured on this port */
+	if (is_lspcon_present_on_port(dev_priv, intel_dig_port->port)) {
+		if (!intel_lspcon_init_connector(intel_dig_port)) {
+			DRM_DEBUG_KMS("LSPCON configured for port %c\n",
+				port_name(intel_dig_port->port));
+			return;
+		} else {
+			DRM_ERROR("Can't set LSPCON(port %c), falling to DP/HDMI\n",
+				port_name(intel_dig_port->port));
+			init_dp = true;
+			init_hdmi = true;
+		}
+	}
+
 	if (init_dp) {
 		if (!intel_ddi_init_dp_connector(intel_dig_port))
 			goto err;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a6ec946..922852c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1345,6 +1345,10 @@ void intel_dsi_init(struct drm_device *dev);
 /* intel_dvo.c */
 void intel_dvo_init(struct drm_device *dev);
 
+/* intel_lspcon.c */
+int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port);
+bool is_lspcon_present_on_port(struct drm_i915_private * dev_priv,
+		enum port port);
 
 /* legacy fbdev emulation in intel_fbdev.c */
 #ifdef CONFIG_DRM_FBDEV_EMULATION
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 20f90e0..96e4c71 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -397,6 +397,16 @@ static const struct drm_connector_helper_funcs lspcon_connector_helper_funcs = {
 	.best_encoder = intel_best_encoder,
 };
 
+bool is_lspcon_present_on_port(struct drm_i915_private *dev_priv, enum port port)
+{
+	/*
+	* TODO: HACK
+	* Replace this with proper VBT child dev config check
+	* logic once that patch is available in tree
+	*/
+	return IS_GEN9(dev_priv->dev) && (port == PORT_B);
+}
+
 int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
 {
 	int ret;
-- 
1.9.1

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

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

* ✗ Fi.CI.BAT: failure for series starting with [01/12] drm: Add helper for DP++ adaptors
  2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
                   ` (10 preceding siblings ...)
  2016-04-04 12:01 ` [PATCH 12/12] DO_NOT_MERGE: drm/i915: Hack to enable lspcon initialization Shashank Sharma
@ 2016-04-04 13:33 ` Patchwork
  11 siblings, 0 replies; 34+ messages in thread
From: Patchwork @ 2016-04-04 13:33 UTC (permalink / raw)
  To: Shashank Sharma; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/12] drm: Add helper for DP++ adaptors
URL   : https://patchwork.freedesktop.org/series/5273/
State : failure

== Summary ==

Series 5273v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/5273/revisions/1/mbox/

Test pm_rpm:
        Subgroup basic-rte:
                pass       -> DMESG-WARN (byt-nuc) UNSTABLE

bdw-nuci7        total:196  pass:184  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:196  pass:175  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:196  pass:159  dwarn:0   dfail:0   fail:0   skip:37 
byt-nuc          total:196  pass:160  dwarn:1   dfail:0   fail:0   skip:35 
hsw-brixbox      total:196  pass:174  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:196  pass:179  dwarn:0   dfail:0   fail:0   skip:17 
ilk-hp8440p      total:196  pass:132  dwarn:0   dfail:0   fail:0   skip:64 
ivb-t430s        total:196  pass:171  dwarn:0   dfail:0   fail:0   skip:25 
snb-x220t        total:164  pass:139  dwarn:0   dfail:0   fail:0   skip:25 
BOOT FAILED for skl-i7k-2
BOOT FAILED for skl-nuci5
BOOT FAILED for snb-dellxps

Results at /archive/results/CI_IGT_test/Patchwork_1788/

3e353ec38c8fe68e9a243a9388389a8815115451 drm-intel-nightly: 2016y-04m-04d-11h-13m-54s UTC integration manifest
42274b6522e88e7efec0eb1266424f75f5342706 DO_NOT_MERGE: drm/i915: Hack to enable lspcon initialization
cb2a8679cb098c4f7006d6af69ef5158adcad4a4 drm/i915: Add lspcon hpd handler
d3e68835b36c7376325fe0479578c4025afbfaf5 drm/i915: Add lspcon core functions
774a8bdf98e49645e026ea1a2a54ae191251157c drm/i915: Add and register lspcon connector functions
903d5afde26b1ccdf32ef274110b1bec97bfe3c2 drm: Add lspcon (custom type2 dp->hdmi) support
427b454fba08ac71964b0272fd7bb07481ff7f40 drm/i915: Add and initialize lspcon connector
0a15f3af6d3075ac6ae2acd06547a799c1969601 drm/i915: Add new lspcon file
dcf5003f1e48e4ff3bee7caa80910c5b64a66cc3 drm/i915: Add lspcon data structures
0e3d1b93f904915352c8e2084db59bb6cd9b9f6f drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT
6eb0111dbcfca1f81bd1b7aa09b5caf6ed6d40c3 drm/i915: Enable/disable TMDS output buffers in DP++ adaptor as needed
de089b451d3807e321dd9b544d0d904074db710e drm/i915: Respect DP++ adaptor TMDS clock limit
1e9985bd5082f57571f3da12e3f06a55886c9522 drm: Add helper for DP++ adaptors

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

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

* Re: [PATCH 12/12] DO_NOT_MERGE: drm/i915: Hack to enable lspcon initialization
  2016-04-04 12:01 ` [PATCH 12/12] DO_NOT_MERGE: drm/i915: Hack to enable lspcon initialization Shashank Sharma
@ 2016-04-06 12:18   ` Jani Nikula
  2016-04-06 13:02     ` Sharma, Shashank
  0 siblings, 1 reply; 34+ messages in thread
From: Jani Nikula @ 2016-04-06 12:18 UTC (permalink / raw)
  To: Shashank Sharma, intel-gfx, ville.syrjala, daniel.vetter,
	paulo.r.zanoni, sonika.jindal, sivakumar.thulasimani

On Mon, 04 Apr 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
> This patch adds a hack to enable lspcon on GEN9 devices.
> This should not be merged, and the hack must be replaced
> by proper VBT parsing logic.
>
> Expecting this patch to enable lspcon bits in VBT:
> https://lists.freedesktop.org/archives/intel-gfx/2016-March/089541.html

FYI, an updated version of that patch has been pushed now.

BR,
Jani.

>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c    | 15 +++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h    |  4 ++++
>  drivers/gpu/drm/i915/intel_lspcon.c | 10 ++++++++++
>  3 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 91654ff..f6c2869 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2169,6 +2169,21 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>  	intel_encoder->cloneable = 0;
>  
> +
> +	/* Check if LSPCON is configured on this port */
> +	if (is_lspcon_present_on_port(dev_priv, intel_dig_port->port)) {
> +		if (!intel_lspcon_init_connector(intel_dig_port)) {
> +			DRM_DEBUG_KMS("LSPCON configured for port %c\n",
> +				port_name(intel_dig_port->port));
> +			return;
> +		} else {
> +			DRM_ERROR("Can't set LSPCON(port %c), falling to DP/HDMI\n",
> +				port_name(intel_dig_port->port));
> +			init_dp = true;
> +			init_hdmi = true;
> +		}
> +	}
> +
>  	if (init_dp) {
>  		if (!intel_ddi_init_dp_connector(intel_dig_port))
>  			goto err;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a6ec946..922852c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1345,6 +1345,10 @@ void intel_dsi_init(struct drm_device *dev);
>  /* intel_dvo.c */
>  void intel_dvo_init(struct drm_device *dev);
>  
> +/* intel_lspcon.c */
> +int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port);
> +bool is_lspcon_present_on_port(struct drm_i915_private * dev_priv,
> +		enum port port);
>  
>  /* legacy fbdev emulation in intel_fbdev.c */
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index 20f90e0..96e4c71 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -397,6 +397,16 @@ static const struct drm_connector_helper_funcs lspcon_connector_helper_funcs = {
>  	.best_encoder = intel_best_encoder,
>  };
>  
> +bool is_lspcon_present_on_port(struct drm_i915_private *dev_priv, enum port port)
> +{
> +	/*
> +	* TODO: HACK
> +	* Replace this with proper VBT child dev config check
> +	* logic once that patch is available in tree
> +	*/
> +	return IS_GEN9(dev_priv->dev) && (port == PORT_B);
> +}
> +
>  int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
>  {
>  	int ret;

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

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

* Re: [PATCH 12/12] DO_NOT_MERGE: drm/i915: Hack to enable lspcon initialization
  2016-04-06 12:18   ` Jani Nikula
@ 2016-04-06 13:02     ` Sharma, Shashank
  0 siblings, 0 replies; 34+ messages in thread
From: Sharma, Shashank @ 2016-04-06 13:02 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx, ville.syrjala, daniel.vetter,
	paulo.r.zanoni, sonika.jindal, sivakumar.thulasimani

Regards
Shashank
>>
>> Expecting this patch to enable lspcon bits in VBT:
>> https://lists.freedesktop.org/archives/intel-gfx/2016-March/089541.html
>
> FYI, an updated version of that patch has been pushed now.
>
> BR,
> Jani.
Thanks Jani, will have a look.
>
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_ddi.c    | 15 +++++++++++++++
>>   drivers/gpu/drm/i915/intel_drv.h    |  4 ++++
>>   drivers/gpu/drm/i915/intel_lspcon.c | 10 ++++++++++
>>   3 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index 91654ff..f6c2869 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -2169,6 +2169,21 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>>   	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>>   	intel_encoder->cloneable = 0;
>>
>> +
>> +	/* Check if LSPCON is configured on this port */
>> +	if (is_lspcon_present_on_port(dev_priv, intel_dig_port->port)) {
>> +		if (!intel_lspcon_init_connector(intel_dig_port)) {
>> +			DRM_DEBUG_KMS("LSPCON configured for port %c\n",
>> +				port_name(intel_dig_port->port));
>> +			return;
>> +		} else {
>> +			DRM_ERROR("Can't set LSPCON(port %c), falling to DP/HDMI\n",
>> +				port_name(intel_dig_port->port));
>> +			init_dp = true;
>> +			init_hdmi = true;
>> +		}
>> +	}
>> +
>>   	if (init_dp) {
>>   		if (!intel_ddi_init_dp_connector(intel_dig_port))
>>   			goto err;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index a6ec946..922852c 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1345,6 +1345,10 @@ void intel_dsi_init(struct drm_device *dev);
>>   /* intel_dvo.c */
>>   void intel_dvo_init(struct drm_device *dev);
>>
>> +/* intel_lspcon.c */
>> +int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port);
>> +bool is_lspcon_present_on_port(struct drm_i915_private * dev_priv,
>> +		enum port port);
>>
>>   /* legacy fbdev emulation in intel_fbdev.c */
>>   #ifdef CONFIG_DRM_FBDEV_EMULATION
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> index 20f90e0..96e4c71 100644
>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -397,6 +397,16 @@ static const struct drm_connector_helper_funcs lspcon_connector_helper_funcs = {
>>   	.best_encoder = intel_best_encoder,
>>   };
>>
>> +bool is_lspcon_present_on_port(struct drm_i915_private *dev_priv, enum port port)
>> +{
>> +	/*
>> +	* TODO: HACK
>> +	* Replace this with proper VBT child dev config check
>> +	* logic once that patch is available in tree
>> +	*/
>> +	return IS_GEN9(dev_priv->dev) && (port == PORT_B);
>> +}
>> +
>>   int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
>>   {
>>   	int ret;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 07/12] drm/i915: Add and initialize lspcon connector
  2016-04-04 12:01 ` [PATCH 07/12] drm/i915: Add and initialize lspcon connector Shashank Sharma
@ 2016-04-25 21:34   ` Zanoni, Paulo R
  0 siblings, 0 replies; 34+ messages in thread
From: Zanoni, Paulo R @ 2016-04-25 21:34 UTC (permalink / raw)
  To: ville.syrjala, Sharma, Shashank, intel-gfx, Jindal, Sonika,
	Vetter, Daniel, Thulasimani, Sivakumar

Em Seg, 2016-04-04 às 17:31 +0530, Shashank Sharma escreveu:
> lspcon is a device which acts as DP in some cases
> and as HDMI in some. Here is how:
> - lspcon needs DP(aux) for all the i2c_ove_aux read/write
>   transitions so it needs to have some DP level initializations
> - lspcon is detected by userspace/sink as HDMI device, so
>   it needs to be detectd as HDMI device.
> 
> This patch adds a custom connector for lspcon device, which
> can pick and chose what it wants from existing functionality.
> 
> This patch also adds functions to initialize dp and hdmi connectors
> and structures to the minimum required levels, and then play around.

Hi

I realized I only said this in private discussions, but never in the
mailing list, so let's bring it to the list so more people can join:

My problem with this approach of adding a new connector is the amount
of duplicated code it brings. Considering our constant never-ending
code churn, added with the current plans to rework major pieces of the
DP code, I would estimate that this lspcon connector code would very
quickly get out-of-sync and, as a consequence, broken, due to the fact
that it's very likely that someone would forget to patch the lspcon
connector during reworks of DP/HDMI connectors. So my current
suggestion would be to just add the necessary LSPCON bits to the HDMI
and DP detection functions instead of adding a whole new connector.
Maybe we could even merge/integrate the DP and HDMI connector functions
since in the end both functions are probing the same port, and if one
connector is found, the other connector is certainly not present.

I can also provide are more technical lower-level review of the code
itself, but I think we should focus the discussion in the high level
design for now.

Thanks,
Paulo

> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c     | 31 +++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h    |  4 ++
>  drivers/gpu/drm/i915/intel_hdmi.c   | 17 ++++++++
>  drivers/gpu/drm/i915/intel_lspcon.c | 79
> +++++++++++++++++++++++++++++++++++++
>  4 files changed, 131 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index ba4da0c..f3df1c6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5900,6 +5900,37 @@ fail:
>  	return false;
>  }
>  
> +int intel_dp_init_minimum(struct intel_digital_port *intel_dig_port,
> +	struct intel_connector *intel_connector)
> +{
> +	int ret;
> +	enum port port = intel_dig_port->port;
> +	struct intel_dp *intel_dp = &intel_dig_port->dp;
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	intel_dig_port->dp.output_reg = DDI_BUF_CTL(port);
> +	if (WARN(intel_dig_port->max_lanes < 1,
> +		 "Not enough lanes (%d) for DP on port %c\n",
> +		 intel_dig_port->max_lanes, port_name(port)))
> +		return -EINVAL;
> +
> +	intel_dp->pps_pipe = INVALID_PIPE;
> +	intel_dp->get_aux_clock_divider = skl_get_aux_clock_divider;
> +	intel_dp->get_aux_send_ctl = skl_get_aux_send_ctl;
> +	intel_dp->prepare_link_retrain =
> intel_ddi_prepare_link_retrain;
> +	intel_dp->DP = I915_READ(intel_dp->output_reg);
> +	intel_dp->attached_connector = intel_connector;
> +	INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
> +			  edp_panel_vdd_work);
> +
> +	ret = intel_dp_aux_init(intel_dp, intel_connector);
> +	if (ret)
> +		DRM_ERROR("Aux init for LSPCON failed\n");
> +
> +	return ret;
> +}
> +
>  void
>  intel_dp_init(struct drm_device *dev,
>  	      i915_reg_t output_reg, enum port port)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 6e309ea..d38db7d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1332,6 +1332,8 @@ void intel_dp_compute_rate(struct intel_dp
> *intel_dp, int port_clock,
>  bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
>  bool
>  intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t
> link_status[DP_LINK_STATUS_SIZE]);
> +int intel_dp_init_minimum(struct intel_digital_port *intel_dig_port,
> +		struct intel_connector *intel_connector);
>  
>  /* intel_dp_mst.c */
>  int intel_dp_mst_encoder_init(struct intel_digital_port
> *intel_dig_port, int conn_id);
> @@ -1397,6 +1399,8 @@ void intel_fbc_cleanup_cfb(struct
> drm_i915_private *dev_priv);
>  void intel_hdmi_init(struct drm_device *dev, i915_reg_t hdmi_reg,
> enum port port);
>  void intel_hdmi_init_connector(struct intel_digital_port
> *intel_dig_port,
>  			       struct intel_connector
> *intel_connector);
> +int intel_hdmi_init_minimum(struct intel_digital_port
> *intel_dig_port,
> +				struct intel_connector
> *intel_connector);
>  struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
>  bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  			       struct intel_crtc_state
> *pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 22b5a7e..92f5094 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2128,6 +2128,23 @@ intel_hdmi_add_properties(struct intel_hdmi
> *intel_hdmi, struct drm_connector *c
>  	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>  
> +int intel_hdmi_init_minimum(struct intel_digital_port
> *intel_dig_port,
> +			       struct intel_connector
> *intel_connector)
> +{
> +	struct intel_hdmi *intel_hdmi = &intel_dig_port->hdmi;
> +
> +	if (WARN(intel_dig_port->max_lanes < 4,
> +		"Not enough lanes (%d) for HDMI on port %c\n",
> +		intel_dig_port->max_lanes, port_name(intel_dig_port-
> >port)))
> +		return -EINVAL;
> +
> +	intel_hdmi->write_infoframe = hsw_write_infoframe;
> +	intel_hdmi->set_infoframes = hsw_set_infoframes;
> +	intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
> +	intel_hdmi->attached_connector = intel_connector;
> +	return 0;
> +}
> +
>  void intel_hdmi_init_connector(struct intel_digital_port
> *intel_dig_port,
>  			       struct intel_connector
> *intel_connector)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
> b/drivers/gpu/drm/i915/intel_lspcon.c
> index 5a1993b..e179758 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -54,3 +54,82 @@ struct intel_lspcon *enc_to_lspcon(struct
> drm_encoder *encoder)
>  		container_of(encoder, struct intel_digital_port,
> base.base);
>  	return &intel_dig_port->lspcon;
>  }
> +
> +int intel_lspcon_init_connector(struct intel_digital_port
> *intel_dig_port)
> +{
> +	int ret;
> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> +	struct drm_device *dev = intel_encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_connector *intel_connector;
> +	struct drm_connector *connector;
> +	enum port port = intel_dig_port->port;
> +
> +	intel_connector = intel_connector_alloc();
> +	if (!intel_connector)
> +		return -ENOMEM;
> +
> +	connector = &intel_connector->base;
> +	connector->interlace_allowed = true;
> +	connector->doublescan_allowed = 0;
> +
> +	/* init DP */
> +	ret = intel_dp_init_minimum(intel_dig_port,
> intel_connector);
> +	if (ret) {
> +		DRM_ERROR("DP init for LSPCON failed\n");
> +		return ret;
> +	}
> +
> +	/* init HDMI */
> +	ret = intel_hdmi_init_minimum(intel_dig_port,
> intel_connector);
> +	if (ret) {
> +		DRM_ERROR("HDMI init for LSPCON failed\n");
> +		return ret;
> +	}
> +
> +	/* Set up the hotplug pin. */
> +	switch (port) {
> +	case PORT_A:
> +		intel_encoder->hpd_pin = HPD_PORT_A;
> +		break;
> +	case PORT_B:
> +		intel_encoder->hpd_pin = HPD_PORT_B;
> +		if (IS_BXT_REVID(dev, 0, BXT_REVID_A1))
> +			intel_encoder->hpd_pin = HPD_PORT_A;
> +		break;
> +	case PORT_C:
> +		intel_encoder->hpd_pin = HPD_PORT_C;
> +		break;
> +	case PORT_D:
> +		intel_encoder->hpd_pin = HPD_PORT_D;
> +		break;
> +	case PORT_E:
> +		intel_encoder->hpd_pin = HPD_PORT_E;
> +		break;
> +	default:
> +		DRM_ERROR("Invalid port to configure LSPCON\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	* On BXT A0/A1, sw needs to activate DDIA HPD logic and
> +	* interrupts to check the external panel connection.
> +	*/
> +	if (IS_BXT_REVID(dev, 0, BXT_REVID_A1) && port == PORT_B)
> +		dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> +	else
> +		dev_priv->hotplug.irq_port[port] = intel_dig_port;
> +
> +	/* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be
> written
> +	* 0xd.  Failure to do so will result in spurious interrupts
> being
> +	* generated on the port when a cable is not attached.
> +	*/
> +	if (IS_G4X(dev) && !IS_GM45(dev)) {
> +		u32 temp = I915_READ(PEG_BAND_GAP_DATA);
> +
> +		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
> +	}
> +
> +	DRM_DEBUG_DRIVER("Success: LSPCON connector init\n");
> +	return 0;
> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 06/12] drm/i915: Add new lspcon file
  2016-04-04 12:01 ` [PATCH 06/12] drm/i915: Add new lspcon file Shashank Sharma
@ 2016-05-02 13:37   ` Ville Syrjälä
  2016-05-03 15:39     ` Sharma, Shashank
  2016-05-02 14:35   ` Jani Nikula
  1 sibling, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-05-02 13:37 UTC (permalink / raw)
  To: Shashank Sharma; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

On Mon, Apr 04, 2016 at 05:31:42PM +0530, Shashank Sharma wrote:
> This patch adds a new file for lspcon with
> some basic stuff like:
> - Some read/wrire addresses for lspcon device
> - Basic read/write functions, using i2c over aux channel
> - Utility functions to get lspcon/encoder/connector
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile       |  3 +-
>  drivers/gpu/drm/i915/intel_lspcon.c | 56 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_lspcon.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 5558a03..00a531a 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -90,7 +90,8 @@ i915-y += dvo_ch7017.o \
>  	  intel_lvds.o \
>  	  intel_panel.o \
>  	  intel_sdvo.o \
> -	  intel_tv.o
> +	  intel_tv.o \
> +	  intel_lspcon.o
>  
>  # virtual gpu code
>  i915-y += i915_vgpu.o
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> new file mode 100644
> index 0000000..5a1993b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -0,0 +1,56 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma <shashank.sharma@intel.com>
> + * Akashdeep Sharma <akashdeep.sharma@intel.com>
> + *
> + */
> +#include <drm/drm_edid.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_dp_dual_mode_helper.h>
> +#include "intel_drv.h"
> +
> +#define LSPCON_I2C_ADDRESS			0x80
> +#define LSPCON_MODE_CHANGE_OFFSET		0x40
> +#define LSPCON_MODE_CHECK_OFFSET		0x41
> +#define LSPCON_ADAPTER_SIGN_OFFSET		0x00
> +#define LSPCON_IDENTIFIER_OFFSET		0x10
> +#define LSPCON_IDENTIFIER_LENGTH		0x10
> +#define LSPCON_MODE_MASK			0x1

Bunch of these seem like duplicates of the dp++ stuff.

> +
> +struct intel_digital_port *lspcon_to_dig_port(struct intel_lspcon *lspcon)
> +{
> +	return container_of(lspcon, struct intel_digital_port, lspcon);
> +}
> +
> +struct intel_hdmi *lspcon_to_hdmi(struct intel_lspcon *lspcon)
> +{
> +	return &lspcon_to_dig_port(lspcon)->hdmi;
> +}
> +
> +struct intel_lspcon *enc_to_lspcon(struct drm_encoder *encoder)
> +{
> +	struct intel_digital_port *intel_dig_port =
> +		container_of(encoder, struct intel_digital_port, base.base);
> +	return &intel_dig_port->lspcon;
> +}
> -- 
> 1.9.1

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

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

* Re: [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support
  2016-04-04 12:01 ` [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support Shashank Sharma
@ 2016-05-02 13:49   ` Ville Syrjälä
  2016-05-03 15:45     ` Sharma, Shashank
  0 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-05-02 13:49 UTC (permalink / raw)
  To: Shashank Sharma; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

On Mon, Apr 04, 2016 at 05:31:44PM +0530, Shashank Sharma wrote:
> This patch adds support for LSPCON devices in dp++ adaptor
> helper layer. LSPCON is DP++ type-2 adaptor with some customized
> functionalities, to provide additional features in Intel Gen9 HW.
> 
> LSPCON needs I2C-over-aux support to read/write control and
> data registers. This patch adds following:
>   - Functions for I2C over aux read/write
>   - Function to read EDID using I2c-over-aux
>   - Function to identify LSPCON among type-2 DP adaptors
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_dual_mode_helper.c | 100 ++++++++++++++++++++++++++++++
>  include/drm/drm_dp_dual_mode_helper.h     |  10 +++
>  2 files changed, 110 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> index b72b7bb..ce8e11c 100644
> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> @@ -132,6 +132,99 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
>  }
>  EXPORT_SYMBOL(drm_dp_dual_mode_write);
>  
> +int drm_dp_dual_mode_get_edid(void *data,
> +	u8 *buf, unsigned int block, size_t len)
> +{
> +	struct i2c_adapter *adapter = data;
> +	unsigned char start = block * EDID_LENGTH;
> +	unsigned char segment = block >> 1;
> +	unsigned char xfers = segment ? 3 : 2;
> +	int ret, retries = 5;
> +
> +	do {
> +		struct i2c_msg msgs[] = {
> +			{
> +				.addr   = DP_DUAL_MODE_DDC_SEGMENT_ADDR,
> +				.flags  = 0,
> +				.len    = 1,
> +				.buf    = &segment,
> +			}, {
> +				.addr   = DDC_ADDR,
> +				.flags  = 0,
> +				.len    = 1,
> +				.buf    = &start,
> +			}, {
> +				.addr   = DDC_ADDR,
> +				.flags  = I2C_M_RD,
> +				.len    = len,
> +				.buf    = buf,
> +			}
> +		};
> +
> +		ret = adapter->algo->master_xfer(adapter, &msgs[3 - xfers],
> +						xfers);
> +
> +		if (ret == -ENXIO) {
> +			DRM_ERROR("Non-existent adapter %s\n",
> +				adapter->name);
> +			break;
> +		}
> +	} while (ret != xfers && --retries);
> +
> +	return ret == xfers ? 0 : -1;
> +}
> +EXPORT_SYMBOL(drm_dp_dual_mode_get_edid);

This sort of stuff shouldn't be needed. All that should be needed is
passing the right i2c adapter to drm_get_edid() and whatnot.

> +
> +/*
> +* drm_dp_dual_mode_ioa_xfer
> +* Few dp->hdmi type 2 adaptors allow i2c_over_aux read/write
> +* to the control and status registers. These functions help
> +* to read/write from those.
> +*/
> +static int drm_dp_dual_mode_ioa_xfer(struct i2c_adapter *adapter,
> +		u8 *buffer, u8 offset, u8 no_of_bytes, u8 rw_flag)
> +{
> +	int err = 0;
> +
> +	struct i2c_msg msgs[] = {
> +			{
> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
> +				.flags	= 0,
> +				.len	= 1,
> +				.buf	= &offset,
> +			}, {
> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
> +				.flags	= rw_flag,
> +				.len	= no_of_bytes,
> +				.buf	= buffer,
> +			}
> +	};
> +
> +	/* I2C over AUX here */
> +	err = adapter->algo->master_xfer(adapter, msgs, 2);
> +	if (err < 0)
> +		DRM_ERROR("LSPCON: Failed I2C over Aux read(addr=0x%x)\n",
> +				(unsigned int)offset);
> +
> +	return err;
> +}
> +
> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
> +		u8 offset, u8 no_of_bytes)
> +{
> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
> +		no_of_bytes, I2C_M_RD);
> +}
> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_read);
> +
> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
> +		u8 offset, u8 no_of_bytes)
> +{
> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
> +		no_of_bytes, 0);
> +}
> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_write);

Aren't these just dupes of the dual mode helper read/write functions?

> +
>  static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
>  {
>  	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
> @@ -141,6 +234,11 @@ static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
>  		      sizeof(dp_dual_mode_hdmi_id)) == 0;
>  }
>  
> +bool is_lspcon_adaptor(const uint8_t adaptor_id)
> +{
> +	return adaptor_id == 0xa8;
> +}
> +
>  /**
>   * drm_dp_dual_mode_detect - Identyfy the DP dual mode adaptor
>   * adapter: I2C adapter for the DDC bus
> @@ -197,6 +295,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
>  				    &adaptor_id, sizeof(adaptor_id));
>  	if (ret || (adaptor_id != (DP_DUAL_MODE_TYPE_TYPE2 |
>  				   DP_DUAL_MODE_REV_TYPE2))) {
> +		if (is_lspcon_adaptor(adaptor_id))
> +			return DRM_DP_DUAL_MODE_TYPE2_LSPCON;

We should reorganize this a bit I think. Probably should end up looking
somehting like:

bool is_type2_adaptor();
bool is_lspcon_adaptor();

if (ret == 0) {
	if (is_lspcon_adaptor())
		return TYPE2_LSPCON;
	else if (is_type2_adaptor() && is_hdmi_adaptor)
		return TYPE2_HDMI;
	else if (is_type2_adaptor())
		return TYPE2_DVI;
}

if (is_hdmi...)
	return TYPE1_HDMI;
else
	return TYPE1_DVI;


>  		if (is_hdmi_adaptor(hdmi_id))
>  			return DRM_DP_DUAL_MODE_TYPE1_HDMI;
>  		else
> diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
> index 8f8a8dc..26e3dd8 100644
> --- a/include/drm/drm_dp_dual_mode_helper.h
> +++ b/include/drm/drm_dp_dual_mode_helper.h
> @@ -24,6 +24,7 @@
>  #define DRM_DP_DUAL_MODE_HELPER_H
>  
>  #include <linux/types.h>
> +#include <drm/drm_edid.h>
>  
>  /*
>   * Optional for type 1 DVI adaptors
> @@ -55,6 +56,7 @@
>  #define  DP_DUAL_MODE_CEC_ENABLE 0x01
>  #define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
>  #define DP_DUAL_MODE_LAST_RESERVED 0xff
> +#define DP_DUAL_MODE_DDC_SEGMENT_ADDR 0x30
>  
>  struct i2c_adapter;
>  
> @@ -69,6 +71,7 @@ enum drm_dp_dual_mode_type {
>  	DRM_DP_DUAL_MODE_TYPE1_HDMI,
>  	DRM_DP_DUAL_MODE_TYPE2_DVI,
>  	DRM_DP_DUAL_MODE_TYPE2_HDMI,
> +	DRM_DP_DUAL_MODE_TYPE2_LSPCON,
>  };
>  
>  enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
> @@ -81,4 +84,11 @@ drm_dp_dual_mode_set_tmds_output(struct i2c_adapter *adapter, bool enable);
>  const char
>  *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
>  
> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
> +		u8 offset, u8 no_of_bytes);
> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
> +		u8 offset, u8 no_of_bytes);
> +int drm_dp_dual_mode_get_edid(void *data,
> +	u8 *buf, unsigned int block, size_t len);
> +
>  #endif
> -- 
> 1.9.1

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

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

* Re: [PATCH 10/12] drm/i915: Add lspcon core functions
  2016-04-04 12:01 ` [PATCH 10/12] drm/i915: Add lspcon core functions Shashank Sharma
@ 2016-05-02 13:51   ` Ville Syrjälä
  2016-05-03 15:48     ` Sharma, Shashank
  0 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-05-02 13:51 UTC (permalink / raw)
  To: Shashank Sharma; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

On Mon, Apr 04, 2016 at 05:31:46PM +0530, Shashank Sharma wrote:
> This patch adds lspcon's internal functions, which work
> on the probe layer, and indicate the working status of
> lspcon, which are mostly:
> 
> probe: A lspcon device is probed only once, during boot
> time, as its always present with the device, next to port.
> So the i2c_over_aux channel is alwyas read/writeable if DC is
> powered on. If VBT says that this port contains lspcon, we
> check and probe the HW to verify and initialize it.
> 
> get_mode: This function indicates the current mode of operation
> of lspcon (ls or pcon mode)
> 
> change_mode: This function can change the lspcon's mode of
> operation to desired mode.
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lspcon.c | 145 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 145 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index c3c1cd2..617fe3f 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -61,6 +61,144 @@ static struct intel_lspcon
>  	return enc_to_lspcon(&intel_attached_encoder(connector)->base);
>  }
>  
> +enum lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
> +{
> +	u8 data;
> +	int err = 0;
> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
> +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
> +
> +	/* Read Status: i2c over aux */
> +	err = drm_dp_dual_mode_ioa_read(adapter, &data,
> +		LSPCON_MODE_CHECK_OFFSET, sizeof(data));
> +	if (err < 0) {
> +		DRM_ERROR("LSPCON read mode ioa (0x80, 0x41) failed\n");
> +		return lspcon_mode_invalid;
> +	}
> +
> +	DRM_DEBUG_DRIVER("LSPCON mode (0x80, 0x41) = %x\n", (unsigned int)data);
> +	return data & LSPCON_MODE_MASK ? lspcon_mode_pcon : lspcon_mode_ls;
> +}
> +
> +int lspcon_change_mode(struct intel_lspcon *lspcon,
> +	enum lspcon_mode mode, bool force)
> +{
> +	u8 data;
> +	int err;
> +	int time_out = 200;
> +	enum lspcon_mode current_mode;
> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
> +
> +	current_mode = lspcon_get_current_mode(lspcon);
> +	if (current_mode == lspcon_mode_invalid) {
> +		DRM_ERROR("Failed to get current LSPCON mode\n");
> +		return -EFAULT;
> +	}
> +
> +	if (current_mode == mode && !force) {
> +		DRM_DEBUG_DRIVER("Current mode = desired LSPCON mode\n");
> +		return 0;
> +	}
> +
> +	if (mode == lspcon_mode_ls)
> +		data = ~LSPCON_MODE_MASK;
> +	else
> +		data = LSPCON_MODE_MASK;
> +
> +	/* Change mode */
> +	err = drm_dp_dual_mode_ioa_write(&dig_port->dp.aux.ddc, &data,
> +			LSPCON_MODE_CHANGE_OFFSET, sizeof(data));
> +	if (err < 0) {
> +		DRM_ERROR("LSPCON mode change failed\n");
> +		return err;
> +	}
> +
> +	/*
> +	* Confirm mode change by reading the status bit.
> +	* Sometimes, it takes a while to change the mode,
> +	* so wait and retry until time out or done.
> +	*/
> +	while (time_out) {
> +		current_mode = lspcon_get_current_mode(lspcon);
> +		if (current_mode != mode) {
> +			mdelay(10);
> +			time_out -= 10;
> +		} else {
> +			lspcon->mode_of_op = mode;
> +			DRM_DEBUG_DRIVER("LSPCON mode changed to %s\n",
> +				mode == lspcon_mode_ls ? "LS" : "PCON");
> +			return 0;
> +		}
> +	}
> +
> +	DRM_ERROR("LSPCON mode change timed out\n");
> +	return -EFAULT;
> +}

I think we probably want to put most of this stuff into the helper. We
already have the LSPCON identification there, so having a few helpers
to set/get the ls vs. pconn mode would seem appropriate.

> +
> +bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
> +{
> +	enum drm_dp_dual_mode_type adaptor_type;
> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
> +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
> +
> +	/* Lets probe the adaptor and check its type */
> +	adaptor_type = drm_dp_dual_mode_detect(adapter);
> +	if (adaptor_type != DRM_DP_DUAL_MODE_TYPE2_LSPCON) {
> +		DRM_DEBUG_DRIVER("No LSPCON detected, found %s\n",
> +			drm_dp_get_dual_mode_type_name(adaptor_type));
> +		return false;
> +	}
> +
> +	/* Yay ... got a LSPCON device */
> +	DRM_DEBUG_DRIVER("LSPCON detected\n");
> +	return true;
> +}
> +
> +enum lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
> +{
> +	enum lspcon_mode current_mode;
> +
> +	/* Detect a valid lspcon */
> +	if (!lspcon_detect_identifier(lspcon)) {
> +		DRM_DEBUG_DRIVER("Failed to find LSPCON identifier\n");
> +		return false;
> +	}
> +
> +	/* LSPCON's mode of operation */
> +	current_mode = lspcon_get_current_mode(lspcon);
> +	if (current_mode == lspcon_mode_invalid) {
> +		DRM_ERROR("Failed to read LSPCON mode\n");
> +		return false;
> +	}
> +
> +	/* All is well */
> +	lspcon->mode_of_op = current_mode;
> +	lspcon->active = true;
> +	return current_mode;
> +}
> +
> +bool lspcon_device_init(struct intel_lspcon *lspcon)
> +{
> +
> +	/* Lets check LSPCON now, probe the HW status */
> +	lspcon->active = false;
> +	lspcon->mode_of_op = lspcon_mode_invalid;
> +	if (!lspcon_probe(lspcon)) {
> +		DRM_ERROR("Failed to probe lspcon");
> +		return false;
> +	}
> +
> +	/* We wish to keep LSPCON in LS mode */
> +	if (lspcon->active && lspcon->mode_of_op != lspcon_mode_ls) {
> +		if (lspcon_change_mode(lspcon, lspcon_mode_ls, true) < 0) {
> +			DRM_ERROR("LSPCON mode change to LS failed\n");
> +			return false;
> +		}
> +	}
> +	DRM_DEBUG_DRIVER("LSPCON init success\n");
> +	return true;
> +}
> +
>  struct edid *lspcon_get_edid(struct intel_lspcon *lspcon, struct drm_connector
>  						*connector)
>  {
> @@ -233,6 +371,7 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
>  	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>  	struct drm_device *dev = intel_encoder->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
>  	struct intel_connector *intel_connector;
>  	struct drm_connector *connector;
>  	enum port port = intel_dig_port->port;
> @@ -314,6 +453,12 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
>  		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
>  	}
>  
> +	/* Now initialize the LSPCON device */
> +	if (!lspcon_device_init(lspcon)) {
> +		DRM_ERROR("LSPCON device init failed\n");
> +		goto fail;
> +	}
> +
>  	DRM_DEBUG_DRIVER("Success: LSPCON connector init\n");
>  	return 0;
>  
> -- 
> 1.9.1

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

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

* Re: [PATCH 04/12] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT
  2016-04-04 12:01 ` [PATCH 04/12] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT Shashank Sharma
@ 2016-05-02 14:33   ` Jani Nikula
  2016-05-02 14:39     ` Ville Syrjälä
  0 siblings, 1 reply; 34+ messages in thread
From: Jani Nikula @ 2016-05-02 14:33 UTC (permalink / raw)
  To: Shashank Sharma, intel-gfx, ville.syrjala, daniel.vetter,
	paulo.r.zanoni, sonika.jindal, sivakumar.thulasimani

On Mon, 04 Apr 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> DP dual mode type 1 DVI adaptors aren't required to implement any
> registers, so it's a bit hard to detect them. The best way would
> be to check the state of the CONFIG1 pin, but we have no way to
> do that. So as a last resort, check the VBT to see if the HDMI
> port is in fact a dual mode capable DP port.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  2 ++
>  drivers/gpu/drm/i915/intel_bios.c     | 32 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c       |  5 +++++
>  drivers/gpu/drm/i915/intel_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c     | 23 +++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_vbt_defs.h |  3 +++
>  6 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f330a53..65bb83f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3373,6 +3373,8 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>  bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>  bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
>  bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
> +bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
> +		enum port port);
>  
>  /* intel_opregion.c */
>  #ifdef CONFIG_ACPI
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 083003b..39c520a 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1550,6 +1550,38 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>  	return false;
>  }
>  
> +bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
> +				enum port port)
> +{
> +	const union child_device_config *p_child;
> +	int i;
> +	static const short port_mapping[] = {
> +		[PORT_B] = DVO_PORT_DPB,
> +		[PORT_C] = DVO_PORT_DPC,
> +		[PORT_D] = DVO_PORT_DPD,
> +		[PORT_E] = DVO_PORT_DPE,
> +	};
> +
> +	if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
> +		return false;
> +
> +	if (!dev_priv->vbt.child_dev_num)
> +		return false;
> +
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +		p_child = &dev_priv->vbt.child_dev[i];
> +
> +		if (p_child->common.dvo_port == port_mapping[port] &&
> +		    (p_child->common.device_type &
> +				DEVICE_TYPE_DP_DUAL_MODE_BITS) ==
> +			(DEVICE_TYPE_DP_DUAL_MODE &
> +				DEVICE_TYPE_DP_DUAL_MODE_BITS))
> +			return true;
> +	}
> +	return false;
> +}
> +
> +
>  /**
>   * intel_bios_is_dsi_present - is DSI present in VBT
>   * @dev_priv:	i915 device instance
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3ff8f1d..ba4da0c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5007,6 +5007,11 @@ bool intel_dp_is_edp(struct drm_device *dev, enum port port)
>  	return intel_bios_is_port_edp(dev_priv, port);
>  }
>  
> +bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv, enum port port)
> +{
> +	return intel_bios_is_dp_dual_mode(dev_priv, port);
> +}

Just use intel_bios_is_dp_dual_mode() where you need it.

BR,
Jani.


> +
>  void
>  intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ab514bd..26ef950 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1276,6 +1276,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
>  bool intel_dp_compute_config(struct intel_encoder *encoder,
>  			     struct intel_crtc_state *pipe_config);
>  bool intel_dp_is_edp(struct drm_device *dev, enum port port);
> +bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv, enum port port);
>  enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
>  				  bool long_hpd);
>  void intel_edp_backlight_on(struct intel_dp *intel_dp);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 3a93337..22b5a7e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1387,14 +1387,33 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>  }
>  
>  static void
> -intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector)
> +intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector, bool has_edid)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  	struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
> +	enum port port = hdmi_to_dig_port(hdmi)->port;
>  	struct i2c_adapter *adapter =
>  		intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus);
>  	enum drm_dp_dual_mode_type type = drm_dp_dual_mode_detect(adapter);
>  
> +	/*
> +	 * Type 1 DVI adaptors are not required to implement any
> +	 * registers, so we can't always detect their presence.
> +	 * Ideally we should be able to check the state of the
> +	 * CONFIG1 pin, but no such luck on our hardware.
> +	 *
> +	 * The only method left to us is to check the VBT to see
> +	 * if the port is a dual mode capable DP port. But let's
> +	 * only do that when we sucesfully read the EDID, to avoid
> +	 * confusing log messages about DP dual mode adaptors when
> +	 * there's nothing connected to the port.
> +	 */
> +	if (type == DRM_DP_DUAL_MODE_NONE && has_edid &&
> +	    intel_dp_is_dual_mode(dev_priv, port)) {
> +		DRM_DEBUG_KMS("Assuming DP dual mode adaptor (as per VBT)\n");
> +		type = DRM_DP_DUAL_MODE_TYPE1_DVI;
> +	}
> +
>  	if (type == DRM_DP_DUAL_MODE_NONE)
>  		return;
>  
> @@ -1440,7 +1459,7 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
>  				    intel_gmbus_get_adapter(dev_priv,
>  				    intel_hdmi->ddc_bus));
>  
> -		intel_hdmi_dp_dual_mode_detect(connector);
> +		intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
>  
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
> index 749dcea..ebaecdb 100644
> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> @@ -734,6 +734,7 @@ struct bdb_psr {
>  #define	 DEVICE_TYPE_INT_TV	0x1009
>  #define	 DEVICE_TYPE_HDMI	0x60D2
>  #define	 DEVICE_TYPE_DP		0x68C6
> +#define	 DEVICE_TYPE_DP_DUAL_MODE 0x60D6
>  #define	 DEVICE_TYPE_eDP	0x78C6
>  
>  #define  DEVICE_TYPE_CLASS_EXTENSION	(1 << 15)
> @@ -768,6 +769,8 @@ struct bdb_psr {
>  	 DEVICE_TYPE_DISPLAYPORT_OUTPUT | \
>  	 DEVICE_TYPE_ANALOG_OUTPUT)
>  
> +#define DEVICE_TYPE_DP_DUAL_MODE_BITS ~DEVICE_TYPE_NOT_HDMI_OUTPUT
> +
>  /* define the DVO port for HDMI output type */
>  #define		DVO_B		1
>  #define		DVO_C		2

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

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

* Re: [PATCH 06/12] drm/i915: Add new lspcon file
  2016-04-04 12:01 ` [PATCH 06/12] drm/i915: Add new lspcon file Shashank Sharma
  2016-05-02 13:37   ` Ville Syrjälä
@ 2016-05-02 14:35   ` Jani Nikula
  2016-05-03 15:53     ` Sharma, Shashank
  1 sibling, 1 reply; 34+ messages in thread
From: Jani Nikula @ 2016-05-02 14:35 UTC (permalink / raw)
  To: Shashank Sharma, intel-gfx, ville.syrjala, daniel.vetter,
	paulo.r.zanoni, sonika.jindal, sivakumar.thulasimani

On Mon, 04 Apr 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
> This patch adds a new file for lspcon with
> some basic stuff like:
> - Some read/wrire addresses for lspcon device
> - Basic read/write functions, using i2c over aux channel
> - Utility functions to get lspcon/encoder/connector
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile       |  3 +-
>  drivers/gpu/drm/i915/intel_lspcon.c | 56 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_lspcon.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 5558a03..00a531a 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -90,7 +90,8 @@ i915-y += dvo_ch7017.o \
>  	  intel_lvds.o \
>  	  intel_panel.o \
>  	  intel_sdvo.o \
> -	  intel_tv.o
> +	  intel_tv.o \
> +	  intel_lspcon.o

At the top of the Makefile:

# Please keep these build lists sorted!


BR,
Jani.



>  
>  # virtual gpu code
>  i915-y += i915_vgpu.o
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> new file mode 100644
> index 0000000..5a1993b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -0,0 +1,56 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Authors:
> + * Shashank Sharma <shashank.sharma@intel.com>
> + * Akashdeep Sharma <akashdeep.sharma@intel.com>
> + *
> + */
> +#include <drm/drm_edid.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_dp_dual_mode_helper.h>
> +#include "intel_drv.h"
> +
> +#define LSPCON_I2C_ADDRESS			0x80
> +#define LSPCON_MODE_CHANGE_OFFSET		0x40
> +#define LSPCON_MODE_CHECK_OFFSET		0x41
> +#define LSPCON_ADAPTER_SIGN_OFFSET		0x00
> +#define LSPCON_IDENTIFIER_OFFSET		0x10
> +#define LSPCON_IDENTIFIER_LENGTH		0x10
> +#define LSPCON_MODE_MASK			0x1
> +
> +struct intel_digital_port *lspcon_to_dig_port(struct intel_lspcon *lspcon)
> +{
> +	return container_of(lspcon, struct intel_digital_port, lspcon);
> +}
> +
> +struct intel_hdmi *lspcon_to_hdmi(struct intel_lspcon *lspcon)
> +{
> +	return &lspcon_to_dig_port(lspcon)->hdmi;
> +}
> +
> +struct intel_lspcon *enc_to_lspcon(struct drm_encoder *encoder)
> +{
> +	struct intel_digital_port *intel_dig_port =
> +		container_of(encoder, struct intel_digital_port, base.base);
> +	return &intel_dig_port->lspcon;
> +}

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

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

* Re: [PATCH 04/12] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT
  2016-05-02 14:33   ` Jani Nikula
@ 2016-05-02 14:39     ` Ville Syrjälä
  2016-05-03 15:52       ` Sharma, Shashank
  0 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-05-02 14:39 UTC (permalink / raw)
  To: Jani Nikula; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

On Mon, May 02, 2016 at 05:33:49PM +0300, Jani Nikula wrote:
> On Mon, 04 Apr 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > DP dual mode type 1 DVI adaptors aren't required to implement any
> > registers, so it's a bit hard to detect them. The best way would
> > be to check the state of the CONFIG1 pin, but we have no way to
> > do that. So as a last resort, check the VBT to see if the HDMI
> > port is in fact a dual mode capable DP port.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h       |  2 ++
> >  drivers/gpu/drm/i915/intel_bios.c     | 32 ++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c       |  5 +++++
> >  drivers/gpu/drm/i915/intel_drv.h      |  1 +
> >  drivers/gpu/drm/i915/intel_hdmi.c     | 23 +++++++++++++++++++++--
> >  drivers/gpu/drm/i915/intel_vbt_defs.h |  3 +++
> >  6 files changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index f330a53..65bb83f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3373,6 +3373,8 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
> >  bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
> >  bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
> >  bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
> > +bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
> > +		enum port port);
> >  
> >  /* intel_opregion.c */
> >  #ifdef CONFIG_ACPI
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index 083003b..39c520a 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -1550,6 +1550,38 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
> >  	return false;
> >  }
> >  
> > +bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
> > +				enum port port)
> > +{
> > +	const union child_device_config *p_child;
> > +	int i;
> > +	static const short port_mapping[] = {
> > +		[PORT_B] = DVO_PORT_DPB,
> > +		[PORT_C] = DVO_PORT_DPC,
> > +		[PORT_D] = DVO_PORT_DPD,
> > +		[PORT_E] = DVO_PORT_DPE,
> > +	};
> > +
> > +	if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
> > +		return false;
> > +
> > +	if (!dev_priv->vbt.child_dev_num)
> > +		return false;
> > +
> > +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> > +		p_child = &dev_priv->vbt.child_dev[i];
> > +
> > +		if (p_child->common.dvo_port == port_mapping[port] &&
> > +		    (p_child->common.device_type &
> > +				DEVICE_TYPE_DP_DUAL_MODE_BITS) ==
> > +			(DEVICE_TYPE_DP_DUAL_MODE &
> > +				DEVICE_TYPE_DP_DUAL_MODE_BITS))
> > +			return true;
> > +	}
> > +	return false;
> > +}
> > +
> > +
> >  /**
> >   * intel_bios_is_dsi_present - is DSI present in VBT
> >   * @dev_priv:	i915 device instance
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 3ff8f1d..ba4da0c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5007,6 +5007,11 @@ bool intel_dp_is_edp(struct drm_device *dev, enum port port)
> >  	return intel_bios_is_port_edp(dev_priv, port);
> >  }
> >  
> > +bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv, enum port port)
> > +{
> > +	return intel_bios_is_dp_dual_mode(dev_priv, port);
> > +}
> 
> Just use intel_bios_is_dp_dual_mode() where you need it.

That wasn't in my original patch, and the commit message lacks any
information that the patch has been modified by someone else than the
original author. Such things always need to be documented properly!

Anyways, I should just repost my patches separately (with reviews
comments addresses where appropriate) so that we can get the basic
dual mode stuff in (to fix the 12bpc regressions).

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

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

* Re: [PATCH 06/12] drm/i915: Add new lspcon file
  2016-05-02 13:37   ` Ville Syrjälä
@ 2016-05-03 15:39     ` Sharma, Shashank
  0 siblings, 0 replies; 34+ messages in thread
From: Sharma, Shashank @ 2016-05-03 15:39 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

Regards
Shashank

On 5/2/2016 7:07 PM, Ville Syrjälä wrote:
> On Mon, Apr 04, 2016 at 05:31:42PM +0530, Shashank Sharma wrote:
>> This patch adds a new file for lspcon with
>> some basic stuff like:
>> - Some read/wrire addresses for lspcon device
>> - Basic read/write functions, using i2c over aux channel
>> - Utility functions to get lspcon/encoder/connector
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/Makefile       |  3 +-
>>   drivers/gpu/drm/i915/intel_lspcon.c | 56 +++++++++++++++++++++++++++++++++++++
>>   2 files changed, 58 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/i915/intel_lspcon.c
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 5558a03..00a531a 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -90,7 +90,8 @@ i915-y += dvo_ch7017.o \
>>   	  intel_lvds.o \
>>   	  intel_panel.o \
>>   	  intel_sdvo.o \
>> -	  intel_tv.o
>> +	  intel_tv.o \
>> +	  intel_lspcon.o
>>
>>   # virtual gpu code
>>   i915-y += i915_vgpu.o
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> new file mode 100644
>> index 0000000..5a1993b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -0,0 +1,56 @@
>> +/*
>> + * Copyright © 2016 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + * Shashank Sharma <shashank.sharma@intel.com>
>> + * Akashdeep Sharma <akashdeep.sharma@intel.com>
>> + *
>> + */
>> +#include <drm/drm_edid.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_dp_dual_mode_helper.h>
>> +#include "intel_drv.h"
>> +
>> +#define LSPCON_I2C_ADDRESS			0x80
>> +#define LSPCON_MODE_CHANGE_OFFSET		0x40
>> +#define LSPCON_MODE_CHECK_OFFSET		0x41
>> +#define LSPCON_ADAPTER_SIGN_OFFSET		0x00
>> +#define LSPCON_IDENTIFIER_OFFSET		0x10
>> +#define LSPCON_IDENTIFIER_LENGTH		0x10
>> +#define LSPCON_MODE_MASK			0x1
>
> Bunch of these seem like duplicates of the dp++ stuff.
Yeah, you are right, 0x80, and 0x10 (identifier offset and length) is 
same for dp-dual mode adapters. But mode_check and sign offsets are 
specific to LSPCON. I can reuse few from DP++
>
>> +
>> +struct intel_digital_port *lspcon_to_dig_port(struct intel_lspcon *lspcon)
>> +{
>> +	return container_of(lspcon, struct intel_digital_port, lspcon);
>> +}
>> +
>> +struct intel_hdmi *lspcon_to_hdmi(struct intel_lspcon *lspcon)
>> +{
>> +	return &lspcon_to_dig_port(lspcon)->hdmi;
>> +}
>> +
>> +struct intel_lspcon *enc_to_lspcon(struct drm_encoder *encoder)
>> +{
>> +	struct intel_digital_port *intel_dig_port =
>> +		container_of(encoder, struct intel_digital_port, base.base);
>> +	return &intel_dig_port->lspcon;
>> +}
>> --
>> 1.9.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support
  2016-05-02 13:49   ` Ville Syrjälä
@ 2016-05-03 15:45     ` Sharma, Shashank
  2016-05-03 15:59       ` Ville Syrjälä
  0 siblings, 1 reply; 34+ messages in thread
From: Sharma, Shashank @ 2016-05-03 15:45 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

Regards
Shashank

On 5/2/2016 7:19 PM, Ville Syrjälä wrote:
> On Mon, Apr 04, 2016 at 05:31:44PM +0530, Shashank Sharma wrote:
>> This patch adds support for LSPCON devices in dp++ adaptor
>> helper layer. LSPCON is DP++ type-2 adaptor with some customized
>> functionalities, to provide additional features in Intel Gen9 HW.
>>
>> LSPCON needs I2C-over-aux support to read/write control and
>> data registers. This patch adds following:
>>    - Functions for I2C over aux read/write
>>    - Function to read EDID using I2c-over-aux
>>    - Function to identify LSPCON among type-2 DP adaptors
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_dp_dual_mode_helper.c | 100 ++++++++++++++++++++++++++++++
>>   include/drm/drm_dp_dual_mode_helper.h     |  10 +++
>>   2 files changed, 110 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> index b72b7bb..ce8e11c 100644
>> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> @@ -132,6 +132,99 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
>>   }
>>   EXPORT_SYMBOL(drm_dp_dual_mode_write);
>>
>> +int drm_dp_dual_mode_get_edid(void *data,
>> +	u8 *buf, unsigned int block, size_t len)
>> +{
>> +	struct i2c_adapter *adapter = data;
>> +	unsigned char start = block * EDID_LENGTH;
>> +	unsigned char segment = block >> 1;
>> +	unsigned char xfers = segment ? 3 : 2;
>> +	int ret, retries = 5;
>> +
>> +	do {
>> +		struct i2c_msg msgs[] = {
>> +			{
>> +				.addr   = DP_DUAL_MODE_DDC_SEGMENT_ADDR,
>> +				.flags  = 0,
>> +				.len    = 1,
>> +				.buf    = &segment,
>> +			}, {
>> +				.addr   = DDC_ADDR,
>> +				.flags  = 0,
>> +				.len    = 1,
>> +				.buf    = &start,
>> +			}, {
>> +				.addr   = DDC_ADDR,
>> +				.flags  = I2C_M_RD,
>> +				.len    = len,
>> +				.buf    = buf,
>> +			}
>> +		};
>> +
>> +		ret = adapter->algo->master_xfer(adapter, &msgs[3 - xfers],
>> +						xfers);
>> +
>> +		if (ret == -ENXIO) {
>> +			DRM_ERROR("Non-existent adapter %s\n",
>> +				adapter->name);
>> +			break;
>> +		}
>> +	} while (ret != xfers && --retries);
>> +
>> +	return ret == xfers ? 0 : -1;
>> +}
>> +EXPORT_SYMBOL(drm_dp_dual_mode_get_edid);
>
> This sort of stuff shouldn't be needed. All that should be needed is
> passing the right i2c adapter to drm_get_edid() and whatnot.
>
Yeah, agree, we can optimize this.
>> +
>> +/*
>> +* drm_dp_dual_mode_ioa_xfer
>> +* Few dp->hdmi type 2 adaptors allow i2c_over_aux read/write
>> +* to the control and status registers. These functions help
>> +* to read/write from those.
>> +*/
>> +static int drm_dp_dual_mode_ioa_xfer(struct i2c_adapter *adapter,
>> +		u8 *buffer, u8 offset, u8 no_of_bytes, u8 rw_flag)
>> +{
>> +	int err = 0;
>> +
>> +	struct i2c_msg msgs[] = {
>> +			{
>> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
>> +				.flags	= 0,
>> +				.len	= 1,
>> +				.buf	= &offset,
>> +			}, {
>> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
>> +				.flags	= rw_flag,
>> +				.len	= no_of_bytes,
>> +				.buf	= buffer,
>> +			}
>> +	};
>> +
>> +	/* I2C over AUX here */
>> +	err = adapter->algo->master_xfer(adapter, msgs, 2);
>> +	if (err < 0)
>> +		DRM_ERROR("LSPCON: Failed I2C over Aux read(addr=0x%x)\n",
>> +				(unsigned int)offset);
>> +
>> +	return err;
>> +}
>> +
>> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
>> +		u8 offset, u8 no_of_bytes)
>> +{
>> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
>> +		no_of_bytes, I2C_M_RD);
>> +}
>> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_read);
>> +
>> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
>> +		u8 offset, u8 no_of_bytes)
>> +{
>> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
>> +		no_of_bytes, 0);
>> +}
>> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_write);
>
> Aren't these just dupes of the dual mode helper read/write functions?
No, I2C over aux read uses adapter->algo->master_xfer function, which is 
different in case of aux channel read/write. dp_dual_mode uses i2c_xfer.
>
>> +
>>   static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
>>   {
>>   	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
>> @@ -141,6 +234,11 @@ static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
>>   		      sizeof(dp_dual_mode_hdmi_id)) == 0;
>>   }
>>
>> +bool is_lspcon_adaptor(const uint8_t adaptor_id)
>> +{
>> +	return adaptor_id == 0xa8;
>> +}
>> +
>>   /**
>>    * drm_dp_dual_mode_detect - Identyfy the DP dual mode adaptor
>>    * adapter: I2C adapter for the DDC bus
>> @@ -197,6 +295,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
>>   				    &adaptor_id, sizeof(adaptor_id));
>>   	if (ret || (adaptor_id != (DP_DUAL_MODE_TYPE_TYPE2 |
>>   				   DP_DUAL_MODE_REV_TYPE2))) {
>> +		if (is_lspcon_adaptor(adaptor_id))
>> +			return DRM_DP_DUAL_MODE_TYPE2_LSPCON;
>
> We should reorganize this a bit I think. Probably should end up looking
> somehting like:
>
> bool is_type2_adaptor();
> bool is_lspcon_adaptor();
>
> if (ret == 0) {
> 	if (is_lspcon_adaptor())
> 		return TYPE2_LSPCON;
> 	else if (is_type2_adaptor() && is_hdmi_adaptor)
> 		return TYPE2_HDMI;
> 	else if (is_type2_adaptor())
> 		return TYPE2_DVI;
> }
>
> if (is_hdmi...)
> 	return TYPE1_HDMI;
> else
> 	return TYPE1_DVI;
>
>
Yep, this is a good suggestion.
>>   		if (is_hdmi_adaptor(hdmi_id))
>>   			return DRM_DP_DUAL_MODE_TYPE1_HDMI;
>>   		else
>> diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
>> index 8f8a8dc..26e3dd8 100644
>> --- a/include/drm/drm_dp_dual_mode_helper.h
>> +++ b/include/drm/drm_dp_dual_mode_helper.h
>> @@ -24,6 +24,7 @@
>>   #define DRM_DP_DUAL_MODE_HELPER_H
>>
>>   #include <linux/types.h>
>> +#include <drm/drm_edid.h>
>>
>>   /*
>>    * Optional for type 1 DVI adaptors
>> @@ -55,6 +56,7 @@
>>   #define  DP_DUAL_MODE_CEC_ENABLE 0x01
>>   #define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
>>   #define DP_DUAL_MODE_LAST_RESERVED 0xff
>> +#define DP_DUAL_MODE_DDC_SEGMENT_ADDR 0x30
>>
>>   struct i2c_adapter;
>>
>> @@ -69,6 +71,7 @@ enum drm_dp_dual_mode_type {
>>   	DRM_DP_DUAL_MODE_TYPE1_HDMI,
>>   	DRM_DP_DUAL_MODE_TYPE2_DVI,
>>   	DRM_DP_DUAL_MODE_TYPE2_HDMI,
>> +	DRM_DP_DUAL_MODE_TYPE2_LSPCON,
>>   };
>>
>>   enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
>> @@ -81,4 +84,11 @@ drm_dp_dual_mode_set_tmds_output(struct i2c_adapter *adapter, bool enable);
>>   const char
>>   *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
>>
>> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
>> +		u8 offset, u8 no_of_bytes);
>> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
>> +		u8 offset, u8 no_of_bytes);
>> +int drm_dp_dual_mode_get_edid(void *data,
>> +	u8 *buf, unsigned int block, size_t len);
>> +
>>   #endif
>> --
>> 1.9.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 10/12] drm/i915: Add lspcon core functions
  2016-05-02 13:51   ` Ville Syrjälä
@ 2016-05-03 15:48     ` Sharma, Shashank
  2016-05-03 16:09       ` Ville Syrjälä
  0 siblings, 1 reply; 34+ messages in thread
From: Sharma, Shashank @ 2016-05-03 15:48 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

Regards
Shashank

On 5/2/2016 7:21 PM, Ville Syrjälä wrote:
> On Mon, Apr 04, 2016 at 05:31:46PM +0530, Shashank Sharma wrote:
>> This patch adds lspcon's internal functions, which work
>> on the probe layer, and indicate the working status of
>> lspcon, which are mostly:
>>
>> probe: A lspcon device is probed only once, during boot
>> time, as its always present with the device, next to port.
>> So the i2c_over_aux channel is alwyas read/writeable if DC is
>> powered on. If VBT says that this port contains lspcon, we
>> check and probe the HW to verify and initialize it.
>>
>> get_mode: This function indicates the current mode of operation
>> of lspcon (ls or pcon mode)
>>
>> change_mode: This function can change the lspcon's mode of
>> operation to desired mode.
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_lspcon.c | 145 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 145 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> index c3c1cd2..617fe3f 100644
>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -61,6 +61,144 @@ static struct intel_lspcon
>>   	return enc_to_lspcon(&intel_attached_encoder(connector)->base);
>>   }
>>
>> +enum lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
>> +{
>> +	u8 data;
>> +	int err = 0;
>> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
>> +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
>> +
>> +	/* Read Status: i2c over aux */
>> +	err = drm_dp_dual_mode_ioa_read(adapter, &data,
>> +		LSPCON_MODE_CHECK_OFFSET, sizeof(data));
>> +	if (err < 0) {
>> +		DRM_ERROR("LSPCON read mode ioa (0x80, 0x41) failed\n");
>> +		return lspcon_mode_invalid;
>> +	}
>> +
>> +	DRM_DEBUG_DRIVER("LSPCON mode (0x80, 0x41) = %x\n", (unsigned int)data);
>> +	return data & LSPCON_MODE_MASK ? lspcon_mode_pcon : lspcon_mode_ls;
>> +}
>> +
>> +int lspcon_change_mode(struct intel_lspcon *lspcon,
>> +	enum lspcon_mode mode, bool force)
>> +{
>> +	u8 data;
>> +	int err;
>> +	int time_out = 200;
>> +	enum lspcon_mode current_mode;
>> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
>> +
>> +	current_mode = lspcon_get_current_mode(lspcon);
>> +	if (current_mode == lspcon_mode_invalid) {
>> +		DRM_ERROR("Failed to get current LSPCON mode\n");
>> +		return -EFAULT;
>> +	}
>> +
>> +	if (current_mode == mode && !force) {
>> +		DRM_DEBUG_DRIVER("Current mode = desired LSPCON mode\n");
>> +		return 0;
>> +	}
>> +
>> +	if (mode == lspcon_mode_ls)
>> +		data = ~LSPCON_MODE_MASK;
>> +	else
>> +		data = LSPCON_MODE_MASK;
>> +
>> +	/* Change mode */
>> +	err = drm_dp_dual_mode_ioa_write(&dig_port->dp.aux.ddc, &data,
>> +			LSPCON_MODE_CHANGE_OFFSET, sizeof(data));
>> +	if (err < 0) {
>> +		DRM_ERROR("LSPCON mode change failed\n");
>> +		return err;
>> +	}
>> +
>> +	/*
>> +	* Confirm mode change by reading the status bit.
>> +	* Sometimes, it takes a while to change the mode,
>> +	* so wait and retry until time out or done.
>> +	*/
>> +	while (time_out) {
>> +		current_mode = lspcon_get_current_mode(lspcon);
>> +		if (current_mode != mode) {
>> +			mdelay(10);
>> +			time_out -= 10;
>> +		} else {
>> +			lspcon->mode_of_op = mode;
>> +			DRM_DEBUG_DRIVER("LSPCON mode changed to %s\n",
>> +				mode == lspcon_mode_ls ? "LS" : "PCON");
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	DRM_ERROR("LSPCON mode change timed out\n");
>> +	return -EFAULT;
>> +}
>
> I think we probably want to put most of this stuff into the helper. We
> already have the LSPCON identification there, so having a few helpers
> to set/get the ls vs. pconn mode would seem appropriate.
>
Actually handling of LS/PCON modes are specific to MCA chips, and some 
of the mode change mechanism and few other control stuff may not be same 
on parade or some other vendor's chip, so I am not sure if we should 
create a helper for something which is specific to this chip. You 
suggest so ?
>> +
>> +bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
>> +{
>> +	enum drm_dp_dual_mode_type adaptor_type;
>> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
>> +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
>> +
>> +	/* Lets probe the adaptor and check its type */
>> +	adaptor_type = drm_dp_dual_mode_detect(adapter);
>> +	if (adaptor_type != DRM_DP_DUAL_MODE_TYPE2_LSPCON) {
>> +		DRM_DEBUG_DRIVER("No LSPCON detected, found %s\n",
>> +			drm_dp_get_dual_mode_type_name(adaptor_type));
>> +		return false;
>> +	}
>> +
>> +	/* Yay ... got a LSPCON device */
>> +	DRM_DEBUG_DRIVER("LSPCON detected\n");
>> +	return true;
>> +}
>> +
>> +enum lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
>> +{
>> +	enum lspcon_mode current_mode;
>> +
>> +	/* Detect a valid lspcon */
>> +	if (!lspcon_detect_identifier(lspcon)) {
>> +		DRM_DEBUG_DRIVER("Failed to find LSPCON identifier\n");
>> +		return false;
>> +	}
>> +
>> +	/* LSPCON's mode of operation */
>> +	current_mode = lspcon_get_current_mode(lspcon);
>> +	if (current_mode == lspcon_mode_invalid) {
>> +		DRM_ERROR("Failed to read LSPCON mode\n");
>> +		return false;
>> +	}
>> +
>> +	/* All is well */
>> +	lspcon->mode_of_op = current_mode;
>> +	lspcon->active = true;
>> +	return current_mode;
>> +}
>> +
>> +bool lspcon_device_init(struct intel_lspcon *lspcon)
>> +{
>> +
>> +	/* Lets check LSPCON now, probe the HW status */
>> +	lspcon->active = false;
>> +	lspcon->mode_of_op = lspcon_mode_invalid;
>> +	if (!lspcon_probe(lspcon)) {
>> +		DRM_ERROR("Failed to probe lspcon");
>> +		return false;
>> +	}
>> +
>> +	/* We wish to keep LSPCON in LS mode */
>> +	if (lspcon->active && lspcon->mode_of_op != lspcon_mode_ls) {
>> +		if (lspcon_change_mode(lspcon, lspcon_mode_ls, true) < 0) {
>> +			DRM_ERROR("LSPCON mode change to LS failed\n");
>> +			return false;
>> +		}
>> +	}
>> +	DRM_DEBUG_DRIVER("LSPCON init success\n");
>> +	return true;
>> +}
>> +
>>   struct edid *lspcon_get_edid(struct intel_lspcon *lspcon, struct drm_connector
>>   						*connector)
>>   {
>> @@ -233,6 +371,7 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
>>   	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>>   	struct drm_device *dev = intel_encoder->base.dev;
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
>>   	struct intel_connector *intel_connector;
>>   	struct drm_connector *connector;
>>   	enum port port = intel_dig_port->port;
>> @@ -314,6 +453,12 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
>>   		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
>>   	}
>>
>> +	/* Now initialize the LSPCON device */
>> +	if (!lspcon_device_init(lspcon)) {
>> +		DRM_ERROR("LSPCON device init failed\n");
>> +		goto fail;
>> +	}
>> +
>>   	DRM_DEBUG_DRIVER("Success: LSPCON connector init\n");
>>   	return 0;
>>
>> --
>> 1.9.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 04/12] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT
  2016-05-02 14:39     ` Ville Syrjälä
@ 2016-05-03 15:52       ` Sharma, Shashank
  0 siblings, 0 replies; 34+ messages in thread
From: Sharma, Shashank @ 2016-05-03 15:52 UTC (permalink / raw)
  To: Ville Syrjälä, Jani Nikula
  Cc: daniel.vetter, intel-gfx, paulo.r.zanoni

Regards
Shashank

On 5/2/2016 8:09 PM, Ville Syrjälä wrote:
> On Mon, May 02, 2016 at 05:33:49PM +0300, Jani Nikula wrote:
>> On Mon, 04 Apr 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> DP dual mode type 1 DVI adaptors aren't required to implement any
>>> registers, so it's a bit hard to detect them. The best way would
>>> be to check the state of the CONFIG1 pin, but we have no way to
>>> do that. So as a last resort, check the VBT to see if the HDMI
>>> port is in fact a dual mode capable DP port.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_drv.h       |  2 ++
>>>   drivers/gpu/drm/i915/intel_bios.c     | 32 ++++++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/i915/intel_dp.c       |  5 +++++
>>>   drivers/gpu/drm/i915/intel_drv.h      |  1 +
>>>   drivers/gpu/drm/i915/intel_hdmi.c     | 23 +++++++++++++++++++++--
>>>   drivers/gpu/drm/i915/intel_vbt_defs.h |  3 +++
>>>   6 files changed, 64 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index f330a53..65bb83f 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -3373,6 +3373,8 @@ bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv);
>>>   bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin);
>>>   bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port);
>>>   bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
>>> +bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
>>> +		enum port port);
>>>
>>>   /* intel_opregion.c */
>>>   #ifdef CONFIG_ACPI
>>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>>> index 083003b..39c520a 100644
>>> --- a/drivers/gpu/drm/i915/intel_bios.c
>>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>>> @@ -1550,6 +1550,38 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port)
>>>   	return false;
>>>   }
>>>
>>> +bool intel_bios_is_dp_dual_mode(struct drm_i915_private *dev_priv,
>>> +				enum port port)
>>> +{
>>> +	const union child_device_config *p_child;
>>> +	int i;
>>> +	static const short port_mapping[] = {
>>> +		[PORT_B] = DVO_PORT_DPB,
>>> +		[PORT_C] = DVO_PORT_DPC,
>>> +		[PORT_D] = DVO_PORT_DPD,
>>> +		[PORT_E] = DVO_PORT_DPE,
>>> +	};
>>> +
>>> +	if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
>>> +		return false;
>>> +
>>> +	if (!dev_priv->vbt.child_dev_num)
>>> +		return false;
>>> +
>>> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>>> +		p_child = &dev_priv->vbt.child_dev[i];
>>> +
>>> +		if (p_child->common.dvo_port == port_mapping[port] &&
>>> +		    (p_child->common.device_type &
>>> +				DEVICE_TYPE_DP_DUAL_MODE_BITS) ==
>>> +			(DEVICE_TYPE_DP_DUAL_MODE &
>>> +				DEVICE_TYPE_DP_DUAL_MODE_BITS))
>>> +			return true;
>>> +	}
>>> +	return false;
>>> +}
>>> +
>>> +
>>>   /**
>>>    * intel_bios_is_dsi_present - is DSI present in VBT
>>>    * @dev_priv:	i915 device instance
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 3ff8f1d..ba4da0c 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -5007,6 +5007,11 @@ bool intel_dp_is_edp(struct drm_device *dev, enum port port)
>>>   	return intel_bios_is_port_edp(dev_priv, port);
>>>   }
>>>
>>> +bool intel_dp_is_dual_mode(struct drm_i915_private *dev_priv, enum port port)
>>> +{
>>> +	return intel_bios_is_dp_dual_mode(dev_priv, port);
>>> +}
>>
>> Just use intel_bios_is_dp_dual_mode() where you need it.
>
> That wasn't in my original patch, and the commit message lacks any
> information that the patch has been modified by someone else than the
> original author. Such things always need to be documented properly!
>
> Anyways, I should just repost my patches separately (with reviews
> comments addresses where appropriate) so that we can get the basic
> dual mode stuff in (to fix the 12bpc regressions).
>
While porting this patch, I had to do some changes, and add wrapper 
functions due to recent changes in intel_bios.c and code movement to 
intel_vbt_def.h etc, but I couldn't remember now what was the exact 
reason. So yeah, having it in the commit message would have been a 
better idea.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 06/12] drm/i915: Add new lspcon file
  2016-05-02 14:35   ` Jani Nikula
@ 2016-05-03 15:53     ` Sharma, Shashank
  0 siblings, 0 replies; 34+ messages in thread
From: Sharma, Shashank @ 2016-05-03 15:53 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx, ville.syrjala, daniel.vetter,
	paulo.r.zanoni, sonika.jindal, sivakumar.thulasimani

Regards
Shashank

On 5/2/2016 8:05 PM, Jani Nikula wrote:
> On Mon, 04 Apr 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
>> This patch adds a new file for lspcon with
>> some basic stuff like:
>> - Some read/wrire addresses for lspcon device
>> - Basic read/write functions, using i2c over aux channel
>> - Utility functions to get lspcon/encoder/connector
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/Makefile       |  3 +-
>>   drivers/gpu/drm/i915/intel_lspcon.c | 56 +++++++++++++++++++++++++++++++++++++
>>   2 files changed, 58 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/i915/intel_lspcon.c
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 5558a03..00a531a 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -90,7 +90,8 @@ i915-y += dvo_ch7017.o \
>>   	  intel_lvds.o \
>>   	  intel_panel.o \
>>   	  intel_sdvo.o \
>> -	  intel_tv.o
>> +	  intel_tv.o \
>> +	  intel_lspcon.o
>
> At the top of the Makefile:
>
> # Please keep these build lists sorted!
>
Ah, dint notice the sorting :), will do that.
>
> BR,
> Jani.
>
>
>
>>
>>   # virtual gpu code
>>   i915-y += i915_vgpu.o
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> new file mode 100644
>> index 0000000..5a1993b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -0,0 +1,56 @@
>> +/*
>> + * Copyright © 2016 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + * Shashank Sharma <shashank.sharma@intel.com>
>> + * Akashdeep Sharma <akashdeep.sharma@intel.com>
>> + *
>> + */
>> +#include <drm/drm_edid.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_dp_dual_mode_helper.h>
>> +#include "intel_drv.h"
>> +
>> +#define LSPCON_I2C_ADDRESS			0x80
>> +#define LSPCON_MODE_CHANGE_OFFSET		0x40
>> +#define LSPCON_MODE_CHECK_OFFSET		0x41
>> +#define LSPCON_ADAPTER_SIGN_OFFSET		0x00
>> +#define LSPCON_IDENTIFIER_OFFSET		0x10
>> +#define LSPCON_IDENTIFIER_LENGTH		0x10
>> +#define LSPCON_MODE_MASK			0x1
>> +
>> +struct intel_digital_port *lspcon_to_dig_port(struct intel_lspcon *lspcon)
>> +{
>> +	return container_of(lspcon, struct intel_digital_port, lspcon);
>> +}
>> +
>> +struct intel_hdmi *lspcon_to_hdmi(struct intel_lspcon *lspcon)
>> +{
>> +	return &lspcon_to_dig_port(lspcon)->hdmi;
>> +}
>> +
>> +struct intel_lspcon *enc_to_lspcon(struct drm_encoder *encoder)
>> +{
>> +	struct intel_digital_port *intel_dig_port =
>> +		container_of(encoder, struct intel_digital_port, base.base);
>> +	return &intel_dig_port->lspcon;
>> +}
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support
  2016-05-03 15:45     ` Sharma, Shashank
@ 2016-05-03 15:59       ` Ville Syrjälä
  2016-05-03 16:09         ` Sharma, Shashank
  0 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-05-03 15:59 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

On Tue, May 03, 2016 at 09:15:48PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 5/2/2016 7:19 PM, Ville Syrjälä wrote:
> > On Mon, Apr 04, 2016 at 05:31:44PM +0530, Shashank Sharma wrote:
> >> This patch adds support for LSPCON devices in dp++ adaptor
> >> helper layer. LSPCON is DP++ type-2 adaptor with some customized
> >> functionalities, to provide additional features in Intel Gen9 HW.
> >>
> >> LSPCON needs I2C-over-aux support to read/write control and
> >> data registers. This patch adds following:
> >>    - Functions for I2C over aux read/write
> >>    - Function to read EDID using I2c-over-aux
> >>    - Function to identify LSPCON among type-2 DP adaptors
> >>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/drm_dp_dual_mode_helper.c | 100 ++++++++++++++++++++++++++++++
> >>   include/drm/drm_dp_dual_mode_helper.h     |  10 +++
> >>   2 files changed, 110 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> >> index b72b7bb..ce8e11c 100644
> >> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> >> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> >> @@ -132,6 +132,99 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
> >>   }
> >>   EXPORT_SYMBOL(drm_dp_dual_mode_write);
> >>
> >> +int drm_dp_dual_mode_get_edid(void *data,
> >> +	u8 *buf, unsigned int block, size_t len)
> >> +{
> >> +	struct i2c_adapter *adapter = data;
> >> +	unsigned char start = block * EDID_LENGTH;
> >> +	unsigned char segment = block >> 1;
> >> +	unsigned char xfers = segment ? 3 : 2;
> >> +	int ret, retries = 5;
> >> +
> >> +	do {
> >> +		struct i2c_msg msgs[] = {
> >> +			{
> >> +				.addr   = DP_DUAL_MODE_DDC_SEGMENT_ADDR,
> >> +				.flags  = 0,
> >> +				.len    = 1,
> >> +				.buf    = &segment,
> >> +			}, {
> >> +				.addr   = DDC_ADDR,
> >> +				.flags  = 0,
> >> +				.len    = 1,
> >> +				.buf    = &start,
> >> +			}, {
> >> +				.addr   = DDC_ADDR,
> >> +				.flags  = I2C_M_RD,
> >> +				.len    = len,
> >> +				.buf    = buf,
> >> +			}
> >> +		};
> >> +
> >> +		ret = adapter->algo->master_xfer(adapter, &msgs[3 - xfers],
> >> +						xfers);
> >> +
> >> +		if (ret == -ENXIO) {
> >> +			DRM_ERROR("Non-existent adapter %s\n",
> >> +				adapter->name);
> >> +			break;
> >> +		}
> >> +	} while (ret != xfers && --retries);
> >> +
> >> +	return ret == xfers ? 0 : -1;
> >> +}
> >> +EXPORT_SYMBOL(drm_dp_dual_mode_get_edid);
> >
> > This sort of stuff shouldn't be needed. All that should be needed is
> > passing the right i2c adapter to drm_get_edid() and whatnot.
> >
> Yeah, agree, we can optimize this.
> >> +
> >> +/*
> >> +* drm_dp_dual_mode_ioa_xfer
> >> +* Few dp->hdmi type 2 adaptors allow i2c_over_aux read/write
> >> +* to the control and status registers. These functions help
> >> +* to read/write from those.
> >> +*/
> >> +static int drm_dp_dual_mode_ioa_xfer(struct i2c_adapter *adapter,
> >> +		u8 *buffer, u8 offset, u8 no_of_bytes, u8 rw_flag)
> >> +{
> >> +	int err = 0;
> >> +
> >> +	struct i2c_msg msgs[] = {
> >> +			{
> >> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
> >> +				.flags	= 0,
> >> +				.len	= 1,
> >> +				.buf	= &offset,
> >> +			}, {
> >> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
> >> +				.flags	= rw_flag,
> >> +				.len	= no_of_bytes,
> >> +				.buf	= buffer,
> >> +			}
> >> +	};
> >> +
> >> +	/* I2C over AUX here */
> >> +	err = adapter->algo->master_xfer(adapter, msgs, 2);
> >> +	if (err < 0)
> >> +		DRM_ERROR("LSPCON: Failed I2C over Aux read(addr=0x%x)\n",
> >> +				(unsigned int)offset);
> >> +
> >> +	return err;
> >> +}
> >> +
> >> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
> >> +		u8 offset, u8 no_of_bytes)
> >> +{
> >> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
> >> +		no_of_bytes, I2C_M_RD);
> >> +}
> >> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_read);
> >> +
> >> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
> >> +		u8 offset, u8 no_of_bytes)
> >> +{
> >> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
> >> +		no_of_bytes, 0);
> >> +}
> >> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_write);
> >
> > Aren't these just dupes of the dual mode helper read/write functions?
> No, I2C over aux read uses adapter->algo->master_xfer function, which is 
> different in case of aux channel read/write. dp_dual_mode uses i2c_xfer.

.master_xfer() is just an internal implementation detail of the i2c
bus/algo driver. You're not meant call it directly. i2c_transfer() is
what you call, and it will end up calling .master_xfer() internally.

> >
> >> +
> >>   static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
> >>   {
> >>   	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
> >> @@ -141,6 +234,11 @@ static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
> >>   		      sizeof(dp_dual_mode_hdmi_id)) == 0;
> >>   }
> >>
> >> +bool is_lspcon_adaptor(const uint8_t adaptor_id)
> >> +{
> >> +	return adaptor_id == 0xa8;
> >> +}
> >> +
> >>   /**
> >>    * drm_dp_dual_mode_detect - Identyfy the DP dual mode adaptor
> >>    * adapter: I2C adapter for the DDC bus
> >> @@ -197,6 +295,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
> >>   				    &adaptor_id, sizeof(adaptor_id));
> >>   	if (ret || (adaptor_id != (DP_DUAL_MODE_TYPE_TYPE2 |
> >>   				   DP_DUAL_MODE_REV_TYPE2))) {
> >> +		if (is_lspcon_adaptor(adaptor_id))
> >> +			return DRM_DP_DUAL_MODE_TYPE2_LSPCON;
> >
> > We should reorganize this a bit I think. Probably should end up looking
> > somehting like:
> >
> > bool is_type2_adaptor();
> > bool is_lspcon_adaptor();
> >
> > if (ret == 0) {
> > 	if (is_lspcon_adaptor())
> > 		return TYPE2_LSPCON;
> > 	else if (is_type2_adaptor() && is_hdmi_adaptor)
> > 		return TYPE2_HDMI;
> > 	else if (is_type2_adaptor())
> > 		return TYPE2_DVI;
> > }
> >
> > if (is_hdmi...)
> > 	return TYPE1_HDMI;
> > else
> > 	return TYPE1_DVI;
> >
> >
> Yep, this is a good suggestion.

I already reogranized the type1/2 detection to match this pattern in my
repost of the dp++ stuff, so should be easy to drop this in:
https://lists.freedesktop.org/archives/dri-devel/2016-May/106535.html

> >>   		if (is_hdmi_adaptor(hdmi_id))
> >>   			return DRM_DP_DUAL_MODE_TYPE1_HDMI;
> >>   		else
> >> diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
> >> index 8f8a8dc..26e3dd8 100644
> >> --- a/include/drm/drm_dp_dual_mode_helper.h
> >> +++ b/include/drm/drm_dp_dual_mode_helper.h
> >> @@ -24,6 +24,7 @@
> >>   #define DRM_DP_DUAL_MODE_HELPER_H
> >>
> >>   #include <linux/types.h>
> >> +#include <drm/drm_edid.h>
> >>
> >>   /*
> >>    * Optional for type 1 DVI adaptors
> >> @@ -55,6 +56,7 @@
> >>   #define  DP_DUAL_MODE_CEC_ENABLE 0x01
> >>   #define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
> >>   #define DP_DUAL_MODE_LAST_RESERVED 0xff
> >> +#define DP_DUAL_MODE_DDC_SEGMENT_ADDR 0x30
> >>
> >>   struct i2c_adapter;
> >>
> >> @@ -69,6 +71,7 @@ enum drm_dp_dual_mode_type {
> >>   	DRM_DP_DUAL_MODE_TYPE1_HDMI,
> >>   	DRM_DP_DUAL_MODE_TYPE2_DVI,
> >>   	DRM_DP_DUAL_MODE_TYPE2_HDMI,
> >> +	DRM_DP_DUAL_MODE_TYPE2_LSPCON,
> >>   };
> >>
> >>   enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
> >> @@ -81,4 +84,11 @@ drm_dp_dual_mode_set_tmds_output(struct i2c_adapter *adapter, bool enable);
> >>   const char
> >>   *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
> >>
> >> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
> >> +		u8 offset, u8 no_of_bytes);
> >> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
> >> +		u8 offset, u8 no_of_bytes);
> >> +int drm_dp_dual_mode_get_edid(void *data,
> >> +	u8 *buf, unsigned int block, size_t len);
> >> +
> >>   #endif
> >> --
> >> 1.9.1
> >

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

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

* Re: [PATCH 10/12] drm/i915: Add lspcon core functions
  2016-05-03 15:48     ` Sharma, Shashank
@ 2016-05-03 16:09       ` Ville Syrjälä
  2016-05-03 16:14         ` Sharma, Shashank
  0 siblings, 1 reply; 34+ messages in thread
From: Ville Syrjälä @ 2016-05-03 16:09 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

On Tue, May 03, 2016 at 09:18:49PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 5/2/2016 7:21 PM, Ville Syrjälä wrote:
> > On Mon, Apr 04, 2016 at 05:31:46PM +0530, Shashank Sharma wrote:
> >> This patch adds lspcon's internal functions, which work
> >> on the probe layer, and indicate the working status of
> >> lspcon, which are mostly:
> >>
> >> probe: A lspcon device is probed only once, during boot
> >> time, as its always present with the device, next to port.
> >> So the i2c_over_aux channel is alwyas read/writeable if DC is
> >> powered on. If VBT says that this port contains lspcon, we
> >> check and probe the HW to verify and initialize it.
> >>
> >> get_mode: This function indicates the current mode of operation
> >> of lspcon (ls or pcon mode)
> >>
> >> change_mode: This function can change the lspcon's mode of
> >> operation to desired mode.
> >>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >> Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_lspcon.c | 145 ++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 145 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> >> index c3c1cd2..617fe3f 100644
> >> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> >> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> >> @@ -61,6 +61,144 @@ static struct intel_lspcon
> >>   	return enc_to_lspcon(&intel_attached_encoder(connector)->base);
> >>   }
> >>
> >> +enum lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
> >> +{
> >> +	u8 data;
> >> +	int err = 0;
> >> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
> >> +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
> >> +
> >> +	/* Read Status: i2c over aux */
> >> +	err = drm_dp_dual_mode_ioa_read(adapter, &data,
> >> +		LSPCON_MODE_CHECK_OFFSET, sizeof(data));
> >> +	if (err < 0) {
> >> +		DRM_ERROR("LSPCON read mode ioa (0x80, 0x41) failed\n");
> >> +		return lspcon_mode_invalid;
> >> +	}
> >> +
> >> +	DRM_DEBUG_DRIVER("LSPCON mode (0x80, 0x41) = %x\n", (unsigned int)data);
> >> +	return data & LSPCON_MODE_MASK ? lspcon_mode_pcon : lspcon_mode_ls;
> >> +}
> >> +
> >> +int lspcon_change_mode(struct intel_lspcon *lspcon,
> >> +	enum lspcon_mode mode, bool force)
> >> +{
> >> +	u8 data;
> >> +	int err;
> >> +	int time_out = 200;
> >> +	enum lspcon_mode current_mode;
> >> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
> >> +
> >> +	current_mode = lspcon_get_current_mode(lspcon);
> >> +	if (current_mode == lspcon_mode_invalid) {
> >> +		DRM_ERROR("Failed to get current LSPCON mode\n");
> >> +		return -EFAULT;
> >> +	}
> >> +
> >> +	if (current_mode == mode && !force) {
> >> +		DRM_DEBUG_DRIVER("Current mode = desired LSPCON mode\n");
> >> +		return 0;
> >> +	}
> >> +
> >> +	if (mode == lspcon_mode_ls)
> >> +		data = ~LSPCON_MODE_MASK;
> >> +	else
> >> +		data = LSPCON_MODE_MASK;
> >> +
> >> +	/* Change mode */
> >> +	err = drm_dp_dual_mode_ioa_write(&dig_port->dp.aux.ddc, &data,
> >> +			LSPCON_MODE_CHANGE_OFFSET, sizeof(data));
> >> +	if (err < 0) {
> >> +		DRM_ERROR("LSPCON mode change failed\n");
> >> +		return err;
> >> +	}
> >> +
> >> +	/*
> >> +	* Confirm mode change by reading the status bit.
> >> +	* Sometimes, it takes a while to change the mode,
> >> +	* so wait and retry until time out or done.
> >> +	*/
> >> +	while (time_out) {
> >> +		current_mode = lspcon_get_current_mode(lspcon);
> >> +		if (current_mode != mode) {
> >> +			mdelay(10);
> >> +			time_out -= 10;
> >> +		} else {
> >> +			lspcon->mode_of_op = mode;
> >> +			DRM_DEBUG_DRIVER("LSPCON mode changed to %s\n",
> >> +				mode == lspcon_mode_ls ? "LS" : "PCON");
> >> +			return 0;
> >> +		}
> >> +	}
> >> +
> >> +	DRM_ERROR("LSPCON mode change timed out\n");
> >> +	return -EFAULT;
> >> +}
> >
> > I think we probably want to put most of this stuff into the helper. We
> > already have the LSPCON identification there, so having a few helpers
> > to set/get the ls vs. pconn mode would seem appropriate.
> >
> Actually handling of LS/PCON modes are specific to MCA chips, and some 
> of the mode change mechanism and few other control stuff may not be same 
> on parade or some other vendor's chip, so I am not sure if we should 
> create a helper for something which is specific to this chip. You 
> suggest so ?

Well, if we already put the detection into the helper we already have
some vendor specific stuff there, no? Or would the other vendor's chips
be identifiable in the same way? 

Anyways, I presume someone else than Intel might want to use these same
chips in their products, so having the support in a central place would
seem like a good idea. If we start to see incompatble LSPCON chips from
multiple vendors we'll need to rethink how we support them all, but
until we know how exactly they differ it's rather impossible to design
the helper to deal with them. So as a first step I'd stuff it all into
the helper.

> >> +
> >> +bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
> >> +{
> >> +	enum drm_dp_dual_mode_type adaptor_type;
> >> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
> >> +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
> >> +
> >> +	/* Lets probe the adaptor and check its type */
> >> +	adaptor_type = drm_dp_dual_mode_detect(adapter);
> >> +	if (adaptor_type != DRM_DP_DUAL_MODE_TYPE2_LSPCON) {
> >> +		DRM_DEBUG_DRIVER("No LSPCON detected, found %s\n",
> >> +			drm_dp_get_dual_mode_type_name(adaptor_type));
> >> +		return false;
> >> +	}
> >> +
> >> +	/* Yay ... got a LSPCON device */
> >> +	DRM_DEBUG_DRIVER("LSPCON detected\n");
> >> +	return true;
> >> +}
> >> +
> >> +enum lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
> >> +{
> >> +	enum lspcon_mode current_mode;
> >> +
> >> +	/* Detect a valid lspcon */
> >> +	if (!lspcon_detect_identifier(lspcon)) {
> >> +		DRM_DEBUG_DRIVER("Failed to find LSPCON identifier\n");
> >> +		return false;
> >> +	}
> >> +
> >> +	/* LSPCON's mode of operation */
> >> +	current_mode = lspcon_get_current_mode(lspcon);
> >> +	if (current_mode == lspcon_mode_invalid) {
> >> +		DRM_ERROR("Failed to read LSPCON mode\n");
> >> +		return false;
> >> +	}
> >> +
> >> +	/* All is well */
> >> +	lspcon->mode_of_op = current_mode;
> >> +	lspcon->active = true;
> >> +	return current_mode;
> >> +}
> >> +
> >> +bool lspcon_device_init(struct intel_lspcon *lspcon)
> >> +{
> >> +
> >> +	/* Lets check LSPCON now, probe the HW status */
> >> +	lspcon->active = false;
> >> +	lspcon->mode_of_op = lspcon_mode_invalid;
> >> +	if (!lspcon_probe(lspcon)) {
> >> +		DRM_ERROR("Failed to probe lspcon");
> >> +		return false;
> >> +	}
> >> +
> >> +	/* We wish to keep LSPCON in LS mode */
> >> +	if (lspcon->active && lspcon->mode_of_op != lspcon_mode_ls) {
> >> +		if (lspcon_change_mode(lspcon, lspcon_mode_ls, true) < 0) {
> >> +			DRM_ERROR("LSPCON mode change to LS failed\n");
> >> +			return false;
> >> +		}
> >> +	}
> >> +	DRM_DEBUG_DRIVER("LSPCON init success\n");
> >> +	return true;
> >> +}
> >> +
> >>   struct edid *lspcon_get_edid(struct intel_lspcon *lspcon, struct drm_connector
> >>   						*connector)
> >>   {
> >> @@ -233,6 +371,7 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
> >>   	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> >>   	struct drm_device *dev = intel_encoder->base.dev;
> >>   	struct drm_i915_private *dev_priv = dev->dev_private;
> >> +	struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
> >>   	struct intel_connector *intel_connector;
> >>   	struct drm_connector *connector;
> >>   	enum port port = intel_dig_port->port;
> >> @@ -314,6 +453,12 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
> >>   		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
> >>   	}
> >>
> >> +	/* Now initialize the LSPCON device */
> >> +	if (!lspcon_device_init(lspcon)) {
> >> +		DRM_ERROR("LSPCON device init failed\n");
> >> +		goto fail;
> >> +	}
> >> +
> >>   	DRM_DEBUG_DRIVER("Success: LSPCON connector init\n");
> >>   	return 0;
> >>
> >> --
> >> 1.9.1
> >

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

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

* Re: [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support
  2016-05-03 15:59       ` Ville Syrjälä
@ 2016-05-03 16:09         ` Sharma, Shashank
  2016-05-03 17:19           ` Ville Syrjälä
  0 siblings, 1 reply; 34+ messages in thread
From: Sharma, Shashank @ 2016-05-03 16:09 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

Regards
Shashank

On 5/3/2016 9:29 PM, Ville Syrjälä wrote:
> On Tue, May 03, 2016 at 09:15:48PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>>
>> On 5/2/2016 7:19 PM, Ville Syrjälä wrote:
>>> On Mon, Apr 04, 2016 at 05:31:44PM +0530, Shashank Sharma wrote:
>>>> This patch adds support for LSPCON devices in dp++ adaptor
>>>> helper layer. LSPCON is DP++ type-2 adaptor with some customized
>>>> functionalities, to provide additional features in Intel Gen9 HW.
>>>>
>>>> LSPCON needs I2C-over-aux support to read/write control and
>>>> data registers. This patch adds following:
>>>>     - Functions for I2C over aux read/write
>>>>     - Function to read EDID using I2c-over-aux
>>>>     - Function to identify LSPCON among type-2 DP adaptors
>>>>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/drm_dp_dual_mode_helper.c | 100 ++++++++++++++++++++++++++++++
>>>>    include/drm/drm_dp_dual_mode_helper.h     |  10 +++
>>>>    2 files changed, 110 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>>>> index b72b7bb..ce8e11c 100644
>>>> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>>>> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>>>> @@ -132,6 +132,99 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
>>>>    }
>>>>    EXPORT_SYMBOL(drm_dp_dual_mode_write);
>>>>
>>>> +int drm_dp_dual_mode_get_edid(void *data,
>>>> +	u8 *buf, unsigned int block, size_t len)
>>>> +{
>>>> +	struct i2c_adapter *adapter = data;
>>>> +	unsigned char start = block * EDID_LENGTH;
>>>> +	unsigned char segment = block >> 1;
>>>> +	unsigned char xfers = segment ? 3 : 2;
>>>> +	int ret, retries = 5;
>>>> +
>>>> +	do {
>>>> +		struct i2c_msg msgs[] = {
>>>> +			{
>>>> +				.addr   = DP_DUAL_MODE_DDC_SEGMENT_ADDR,
>>>> +				.flags  = 0,
>>>> +				.len    = 1,
>>>> +				.buf    = &segment,
>>>> +			}, {
>>>> +				.addr   = DDC_ADDR,
>>>> +				.flags  = 0,
>>>> +				.len    = 1,
>>>> +				.buf    = &start,
>>>> +			}, {
>>>> +				.addr   = DDC_ADDR,
>>>> +				.flags  = I2C_M_RD,
>>>> +				.len    = len,
>>>> +				.buf    = buf,
>>>> +			}
>>>> +		};
>>>> +
>>>> +		ret = adapter->algo->master_xfer(adapter, &msgs[3 - xfers],
>>>> +						xfers);
>>>> +
>>>> +		if (ret == -ENXIO) {
>>>> +			DRM_ERROR("Non-existent adapter %s\n",
>>>> +				adapter->name);
>>>> +			break;
>>>> +		}
>>>> +	} while (ret != xfers && --retries);
>>>> +
>>>> +	return ret == xfers ? 0 : -1;
>>>> +}
>>>> +EXPORT_SYMBOL(drm_dp_dual_mode_get_edid);
>>>
>>> This sort of stuff shouldn't be needed. All that should be needed is
>>> passing the right i2c adapter to drm_get_edid() and whatnot.
>>>
>> Yeah, agree, we can optimize this.
>>>> +
>>>> +/*
>>>> +* drm_dp_dual_mode_ioa_xfer
>>>> +* Few dp->hdmi type 2 adaptors allow i2c_over_aux read/write
>>>> +* to the control and status registers. These functions help
>>>> +* to read/write from those.
>>>> +*/
>>>> +static int drm_dp_dual_mode_ioa_xfer(struct i2c_adapter *adapter,
>>>> +		u8 *buffer, u8 offset, u8 no_of_bytes, u8 rw_flag)
>>>> +{
>>>> +	int err = 0;
>>>> +
>>>> +	struct i2c_msg msgs[] = {
>>>> +			{
>>>> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
>>>> +				.flags	= 0,
>>>> +				.len	= 1,
>>>> +				.buf	= &offset,
>>>> +			}, {
>>>> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
>>>> +				.flags	= rw_flag,
>>>> +				.len	= no_of_bytes,
>>>> +				.buf	= buffer,
>>>> +			}
>>>> +	};
>>>> +
>>>> +	/* I2C over AUX here */
>>>> +	err = adapter->algo->master_xfer(adapter, msgs, 2);
>>>> +	if (err < 0)
>>>> +		DRM_ERROR("LSPCON: Failed I2C over Aux read(addr=0x%x)\n",
>>>> +				(unsigned int)offset);
>>>> +
>>>> +	return err;
>>>> +}
>>>> +
>>>> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
>>>> +		u8 offset, u8 no_of_bytes)
>>>> +{
>>>> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
>>>> +		no_of_bytes, I2C_M_RD);
>>>> +}
>>>> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_read);
>>>> +
>>>> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
>>>> +		u8 offset, u8 no_of_bytes)
>>>> +{
>>>> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
>>>> +		no_of_bytes, 0);
>>>> +}
>>>> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_write);
>>>
>>> Aren't these just dupes of the dual mode helper read/write functions?
>> No, I2C over aux read uses adapter->algo->master_xfer function, which is
>> different in case of aux channel read/write. dp_dual_mode uses i2c_xfer.
>
> .master_xfer() is just an internal implementation detail of the i2c
> bus/algo driver. You're not meant call it directly. i2c_transfer() is
> what you call, and it will end up calling .master_xfer() internally.
>
Please correct me if its not the case, but the master_xfer function 
getting loaded in drm_dp_helper layer is drm_dp_i2c_xfer() which does a 
lot of stuff. You mean to say i2c_xfer() function call will map to 
drm_dp_i2c_xfer ?
>>>
>>>> +
>>>>    static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
>>>>    {
>>>>    	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
>>>> @@ -141,6 +234,11 @@ static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
>>>>    		      sizeof(dp_dual_mode_hdmi_id)) == 0;
>>>>    }
>>>>
>>>> +bool is_lspcon_adaptor(const uint8_t adaptor_id)
>>>> +{
>>>> +	return adaptor_id == 0xa8;
>>>> +}
>>>> +
>>>>    /**
>>>>     * drm_dp_dual_mode_detect - Identyfy the DP dual mode adaptor
>>>>     * adapter: I2C adapter for the DDC bus
>>>> @@ -197,6 +295,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
>>>>    				    &adaptor_id, sizeof(adaptor_id));
>>>>    	if (ret || (adaptor_id != (DP_DUAL_MODE_TYPE_TYPE2 |
>>>>    				   DP_DUAL_MODE_REV_TYPE2))) {
>>>> +		if (is_lspcon_adaptor(adaptor_id))
>>>> +			return DRM_DP_DUAL_MODE_TYPE2_LSPCON;
>>>
>>> We should reorganize this a bit I think. Probably should end up looking
>>> somehting like:
>>>
>>> bool is_type2_adaptor();
>>> bool is_lspcon_adaptor();
>>>
>>> if (ret == 0) {
>>> 	if (is_lspcon_adaptor())
>>> 		return TYPE2_LSPCON;
>>> 	else if (is_type2_adaptor() && is_hdmi_adaptor)
>>> 		return TYPE2_HDMI;
>>> 	else if (is_type2_adaptor())
>>> 		return TYPE2_DVI;
>>> }
>>>
>>> if (is_hdmi...)
>>> 	return TYPE1_HDMI;
>>> else
>>> 	return TYPE1_DVI;
>>>
>>>
>> Yep, this is a good suggestion.
>
> I already reogranized the type1/2 detection to match this pattern in my
> repost of the dp++ stuff, so should be easy to drop this in:
> https://lists.freedesktop.org/archives/dri-devel/2016-May/106535.html
>
Yep, just started looking into these patches, I will reuse this.
>>>>    		if (is_hdmi_adaptor(hdmi_id))
>>>>    			return DRM_DP_DUAL_MODE_TYPE1_HDMI;
>>>>    		else
>>>> diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
>>>> index 8f8a8dc..26e3dd8 100644
>>>> --- a/include/drm/drm_dp_dual_mode_helper.h
>>>> +++ b/include/drm/drm_dp_dual_mode_helper.h
>>>> @@ -24,6 +24,7 @@
>>>>    #define DRM_DP_DUAL_MODE_HELPER_H
>>>>
>>>>    #include <linux/types.h>
>>>> +#include <drm/drm_edid.h>
>>>>
>>>>    /*
>>>>     * Optional for type 1 DVI adaptors
>>>> @@ -55,6 +56,7 @@
>>>>    #define  DP_DUAL_MODE_CEC_ENABLE 0x01
>>>>    #define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
>>>>    #define DP_DUAL_MODE_LAST_RESERVED 0xff
>>>> +#define DP_DUAL_MODE_DDC_SEGMENT_ADDR 0x30
>>>>
>>>>    struct i2c_adapter;
>>>>
>>>> @@ -69,6 +71,7 @@ enum drm_dp_dual_mode_type {
>>>>    	DRM_DP_DUAL_MODE_TYPE1_HDMI,
>>>>    	DRM_DP_DUAL_MODE_TYPE2_DVI,
>>>>    	DRM_DP_DUAL_MODE_TYPE2_HDMI,
>>>> +	DRM_DP_DUAL_MODE_TYPE2_LSPCON,
>>>>    };
>>>>
>>>>    enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
>>>> @@ -81,4 +84,11 @@ drm_dp_dual_mode_set_tmds_output(struct i2c_adapter *adapter, bool enable);
>>>>    const char
>>>>    *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
>>>>
>>>> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
>>>> +		u8 offset, u8 no_of_bytes);
>>>> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
>>>> +		u8 offset, u8 no_of_bytes);
>>>> +int drm_dp_dual_mode_get_edid(void *data,
>>>> +	u8 *buf, unsigned int block, size_t len);
>>>> +
>>>>    #endif
>>>> --
>>>> 1.9.1
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 10/12] drm/i915: Add lspcon core functions
  2016-05-03 16:09       ` Ville Syrjälä
@ 2016-05-03 16:14         ` Sharma, Shashank
  2016-05-04 21:09           ` Zanoni, Paulo R
  0 siblings, 1 reply; 34+ messages in thread
From: Sharma, Shashank @ 2016-05-03 16:14 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter



On 5/3/2016 9:39 PM, Ville Syrjälä wrote:
> On Tue, May 03, 2016 at 09:18:49PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>>
>> On 5/2/2016 7:21 PM, Ville Syrjälä wrote:
>>> On Mon, Apr 04, 2016 at 05:31:46PM +0530, Shashank Sharma wrote:
>>>> This patch adds lspcon's internal functions, which work
>>>> on the probe layer, and indicate the working status of
>>>> lspcon, which are mostly:
>>>>
>>>> probe: A lspcon device is probed only once, during boot
>>>> time, as its always present with the device, next to port.
>>>> So the i2c_over_aux channel is alwyas read/writeable if DC is
>>>> powered on. If VBT says that this port contains lspcon, we
>>>> check and probe the HW to verify and initialize it.
>>>>
>>>> get_mode: This function indicates the current mode of operation
>>>> of lspcon (ls or pcon mode)
>>>>
>>>> change_mode: This function can change the lspcon's mode of
>>>> operation to desired mode.
>>>>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>> Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_lspcon.c | 145 ++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 145 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>>>> index c3c1cd2..617fe3f 100644
>>>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
>>>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>>>> @@ -61,6 +61,144 @@ static struct intel_lspcon
>>>>    	return enc_to_lspcon(&intel_attached_encoder(connector)->base);
>>>>    }
>>>>
>>>> +enum lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
>>>> +{
>>>> +	u8 data;
>>>> +	int err = 0;
>>>> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
>>>> +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
>>>> +
>>>> +	/* Read Status: i2c over aux */
>>>> +	err = drm_dp_dual_mode_ioa_read(adapter, &data,
>>>> +		LSPCON_MODE_CHECK_OFFSET, sizeof(data));
>>>> +	if (err < 0) {
>>>> +		DRM_ERROR("LSPCON read mode ioa (0x80, 0x41) failed\n");
>>>> +		return lspcon_mode_invalid;
>>>> +	}
>>>> +
>>>> +	DRM_DEBUG_DRIVER("LSPCON mode (0x80, 0x41) = %x\n", (unsigned int)data);
>>>> +	return data & LSPCON_MODE_MASK ? lspcon_mode_pcon : lspcon_mode_ls;
>>>> +}
>>>> +
>>>> +int lspcon_change_mode(struct intel_lspcon *lspcon,
>>>> +	enum lspcon_mode mode, bool force)
>>>> +{
>>>> +	u8 data;
>>>> +	int err;
>>>> +	int time_out = 200;
>>>> +	enum lspcon_mode current_mode;
>>>> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
>>>> +
>>>> +	current_mode = lspcon_get_current_mode(lspcon);
>>>> +	if (current_mode == lspcon_mode_invalid) {
>>>> +		DRM_ERROR("Failed to get current LSPCON mode\n");
>>>> +		return -EFAULT;
>>>> +	}
>>>> +
>>>> +	if (current_mode == mode && !force) {
>>>> +		DRM_DEBUG_DRIVER("Current mode = desired LSPCON mode\n");
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	if (mode == lspcon_mode_ls)
>>>> +		data = ~LSPCON_MODE_MASK;
>>>> +	else
>>>> +		data = LSPCON_MODE_MASK;
>>>> +
>>>> +	/* Change mode */
>>>> +	err = drm_dp_dual_mode_ioa_write(&dig_port->dp.aux.ddc, &data,
>>>> +			LSPCON_MODE_CHANGE_OFFSET, sizeof(data));
>>>> +	if (err < 0) {
>>>> +		DRM_ERROR("LSPCON mode change failed\n");
>>>> +		return err;
>>>> +	}
>>>> +
>>>> +	/*
>>>> +	* Confirm mode change by reading the status bit.
>>>> +	* Sometimes, it takes a while to change the mode,
>>>> +	* so wait and retry until time out or done.
>>>> +	*/
>>>> +	while (time_out) {
>>>> +		current_mode = lspcon_get_current_mode(lspcon);
>>>> +		if (current_mode != mode) {
>>>> +			mdelay(10);
>>>> +			time_out -= 10;
>>>> +		} else {
>>>> +			lspcon->mode_of_op = mode;
>>>> +			DRM_DEBUG_DRIVER("LSPCON mode changed to %s\n",
>>>> +				mode == lspcon_mode_ls ? "LS" : "PCON");
>>>> +			return 0;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	DRM_ERROR("LSPCON mode change timed out\n");
>>>> +	return -EFAULT;
>>>> +}
>>>
>>> I think we probably want to put most of this stuff into the helper. We
>>> already have the LSPCON identification there, so having a few helpers
>>> to set/get the ls vs. pconn mode would seem appropriate.
>>>
>> Actually handling of LS/PCON modes are specific to MCA chips, and some
>> of the mode change mechanism and few other control stuff may not be same
>> on parade or some other vendor's chip, so I am not sure if we should
>> create a helper for something which is specific to this chip. You
>> suggest so ?
>
> Well, if we already put the detection into the helper we already have
> some vendor specific stuff there, no? Or would the other vendor's chips
> be identifiable in the same way?
>
> Anyways, I presume someone else than Intel might want to use these same
> chips in their products, so having the support in a central place would
> seem like a good idea. If we start to see incompatble LSPCON chips from
> multiple vendors we'll need to rethink how we support them all, but
> until we know how exactly they differ it's rather impossible to design
> the helper to deal with them. So as a first step I'd stuff it all into
> the helper.
>
Yes, makes more sense this way. I will try to go through parade specs 
once, and see if I can find something which needs immediate discussion 
here. Else, I will move this into the central layer.
>>>> +
>>>> +bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
>>>> +{
>>>> +	enum drm_dp_dual_mode_type adaptor_type;
>>>> +	struct intel_digital_port *dig_port = lspcon_to_dig_port(lspcon);
>>>> +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
>>>> +
>>>> +	/* Lets probe the adaptor and check its type */
>>>> +	adaptor_type = drm_dp_dual_mode_detect(adapter);
>>>> +	if (adaptor_type != DRM_DP_DUAL_MODE_TYPE2_LSPCON) {
>>>> +		DRM_DEBUG_DRIVER("No LSPCON detected, found %s\n",
>>>> +			drm_dp_get_dual_mode_type_name(adaptor_type));
>>>> +		return false;
>>>> +	}
>>>> +
>>>> +	/* Yay ... got a LSPCON device */
>>>> +	DRM_DEBUG_DRIVER("LSPCON detected\n");
>>>> +	return true;
>>>> +}
>>>> +
>>>> +enum lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
>>>> +{
>>>> +	enum lspcon_mode current_mode;
>>>> +
>>>> +	/* Detect a valid lspcon */
>>>> +	if (!lspcon_detect_identifier(lspcon)) {
>>>> +		DRM_DEBUG_DRIVER("Failed to find LSPCON identifier\n");
>>>> +		return false;
>>>> +	}
>>>> +
>>>> +	/* LSPCON's mode of operation */
>>>> +	current_mode = lspcon_get_current_mode(lspcon);
>>>> +	if (current_mode == lspcon_mode_invalid) {
>>>> +		DRM_ERROR("Failed to read LSPCON mode\n");
>>>> +		return false;
>>>> +	}
>>>> +
>>>> +	/* All is well */
>>>> +	lspcon->mode_of_op = current_mode;
>>>> +	lspcon->active = true;
>>>> +	return current_mode;
>>>> +}
>>>> +
>>>> +bool lspcon_device_init(struct intel_lspcon *lspcon)
>>>> +{
>>>> +
>>>> +	/* Lets check LSPCON now, probe the HW status */
>>>> +	lspcon->active = false;
>>>> +	lspcon->mode_of_op = lspcon_mode_invalid;
>>>> +	if (!lspcon_probe(lspcon)) {
>>>> +		DRM_ERROR("Failed to probe lspcon");
>>>> +		return false;
>>>> +	}
>>>> +
>>>> +	/* We wish to keep LSPCON in LS mode */
>>>> +	if (lspcon->active && lspcon->mode_of_op != lspcon_mode_ls) {
>>>> +		if (lspcon_change_mode(lspcon, lspcon_mode_ls, true) < 0) {
>>>> +			DRM_ERROR("LSPCON mode change to LS failed\n");
>>>> +			return false;
>>>> +		}
>>>> +	}
>>>> +	DRM_DEBUG_DRIVER("LSPCON init success\n");
>>>> +	return true;
>>>> +}
>>>> +
>>>>    struct edid *lspcon_get_edid(struct intel_lspcon *lspcon, struct drm_connector
>>>>    						*connector)
>>>>    {
>>>> @@ -233,6 +371,7 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
>>>>    	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>>>>    	struct drm_device *dev = intel_encoder->base.dev;
>>>>    	struct drm_i915_private *dev_priv = dev->dev_private;
>>>> +	struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
>>>>    	struct intel_connector *intel_connector;
>>>>    	struct drm_connector *connector;
>>>>    	enum port port = intel_dig_port->port;
>>>> @@ -314,6 +453,12 @@ int intel_lspcon_init_connector(struct intel_digital_port *intel_dig_port)
>>>>    		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
>>>>    	}
>>>>
>>>> +	/* Now initialize the LSPCON device */
>>>> +	if (!lspcon_device_init(lspcon)) {
>>>> +		DRM_ERROR("LSPCON device init failed\n");
>>>> +		goto fail;
>>>> +	}
>>>> +
>>>>    	DRM_DEBUG_DRIVER("Success: LSPCON connector init\n");
>>>>    	return 0;
>>>>
>>>> --
>>>> 1.9.1
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support
  2016-05-03 16:09         ` Sharma, Shashank
@ 2016-05-03 17:19           ` Ville Syrjälä
  0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2016-05-03 17:19 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: paulo.r.zanoni, intel-gfx, daniel.vetter

On Tue, May 03, 2016 at 09:39:49PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 5/3/2016 9:29 PM, Ville Syrjälä wrote:
> > On Tue, May 03, 2016 at 09:15:48PM +0530, Sharma, Shashank wrote:
> >> Regards
> >> Shashank
> >>
> >> On 5/2/2016 7:19 PM, Ville Syrjälä wrote:
> >>> On Mon, Apr 04, 2016 at 05:31:44PM +0530, Shashank Sharma wrote:
> >>>> This patch adds support for LSPCON devices in dp++ adaptor
> >>>> helper layer. LSPCON is DP++ type-2 adaptor with some customized
> >>>> functionalities, to provide additional features in Intel Gen9 HW.
> >>>>
> >>>> LSPCON needs I2C-over-aux support to read/write control and
> >>>> data registers. This patch adds following:
> >>>>     - Functions for I2C over aux read/write
> >>>>     - Function to read EDID using I2c-over-aux
> >>>>     - Function to identify LSPCON among type-2 DP adaptors
> >>>>
> >>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >>>> ---
> >>>>    drivers/gpu/drm/drm_dp_dual_mode_helper.c | 100 ++++++++++++++++++++++++++++++
> >>>>    include/drm/drm_dp_dual_mode_helper.h     |  10 +++
> >>>>    2 files changed, 110 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> >>>> index b72b7bb..ce8e11c 100644
> >>>> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> >>>> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> >>>> @@ -132,6 +132,99 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
> >>>>    }
> >>>>    EXPORT_SYMBOL(drm_dp_dual_mode_write);
> >>>>
> >>>> +int drm_dp_dual_mode_get_edid(void *data,
> >>>> +	u8 *buf, unsigned int block, size_t len)
> >>>> +{
> >>>> +	struct i2c_adapter *adapter = data;
> >>>> +	unsigned char start = block * EDID_LENGTH;
> >>>> +	unsigned char segment = block >> 1;
> >>>> +	unsigned char xfers = segment ? 3 : 2;
> >>>> +	int ret, retries = 5;
> >>>> +
> >>>> +	do {
> >>>> +		struct i2c_msg msgs[] = {
> >>>> +			{
> >>>> +				.addr   = DP_DUAL_MODE_DDC_SEGMENT_ADDR,
> >>>> +				.flags  = 0,
> >>>> +				.len    = 1,
> >>>> +				.buf    = &segment,
> >>>> +			}, {
> >>>> +				.addr   = DDC_ADDR,
> >>>> +				.flags  = 0,
> >>>> +				.len    = 1,
> >>>> +				.buf    = &start,
> >>>> +			}, {
> >>>> +				.addr   = DDC_ADDR,
> >>>> +				.flags  = I2C_M_RD,
> >>>> +				.len    = len,
> >>>> +				.buf    = buf,
> >>>> +			}
> >>>> +		};
> >>>> +
> >>>> +		ret = adapter->algo->master_xfer(adapter, &msgs[3 - xfers],
> >>>> +						xfers);
> >>>> +
> >>>> +		if (ret == -ENXIO) {
> >>>> +			DRM_ERROR("Non-existent adapter %s\n",
> >>>> +				adapter->name);
> >>>> +			break;
> >>>> +		}
> >>>> +	} while (ret != xfers && --retries);
> >>>> +
> >>>> +	return ret == xfers ? 0 : -1;
> >>>> +}
> >>>> +EXPORT_SYMBOL(drm_dp_dual_mode_get_edid);
> >>>
> >>> This sort of stuff shouldn't be needed. All that should be needed is
> >>> passing the right i2c adapter to drm_get_edid() and whatnot.
> >>>
> >> Yeah, agree, we can optimize this.
> >>>> +
> >>>> +/*
> >>>> +* drm_dp_dual_mode_ioa_xfer
> >>>> +* Few dp->hdmi type 2 adaptors allow i2c_over_aux read/write
> >>>> +* to the control and status registers. These functions help
> >>>> +* to read/write from those.
> >>>> +*/
> >>>> +static int drm_dp_dual_mode_ioa_xfer(struct i2c_adapter *adapter,
> >>>> +		u8 *buffer, u8 offset, u8 no_of_bytes, u8 rw_flag)
> >>>> +{
> >>>> +	int err = 0;
> >>>> +
> >>>> +	struct i2c_msg msgs[] = {
> >>>> +			{
> >>>> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
> >>>> +				.flags	= 0,
> >>>> +				.len	= 1,
> >>>> +				.buf	= &offset,
> >>>> +			}, {
> >>>> +				.addr	= DP_DUAL_MODE_SLAVE_ADDRESS,
> >>>> +				.flags	= rw_flag,
> >>>> +				.len	= no_of_bytes,
> >>>> +				.buf	= buffer,
> >>>> +			}
> >>>> +	};
> >>>> +
> >>>> +	/* I2C over AUX here */
> >>>> +	err = adapter->algo->master_xfer(adapter, msgs, 2);
> >>>> +	if (err < 0)
> >>>> +		DRM_ERROR("LSPCON: Failed I2C over Aux read(addr=0x%x)\n",
> >>>> +				(unsigned int)offset);
> >>>> +
> >>>> +	return err;
> >>>> +}
> >>>> +
> >>>> +int drm_dp_dual_mode_ioa_read(struct i2c_adapter *adapter, u8 *buffer,
> >>>> +		u8 offset, u8 no_of_bytes)
> >>>> +{
> >>>> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
> >>>> +		no_of_bytes, I2C_M_RD);
> >>>> +}
> >>>> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_read);
> >>>> +
> >>>> +int drm_dp_dual_mode_ioa_write(struct i2c_adapter *adapter, u8 *buffer,
> >>>> +		u8 offset, u8 no_of_bytes)
> >>>> +{
> >>>> +	return drm_dp_dual_mode_ioa_xfer(adapter, buffer, offset,
> >>>> +		no_of_bytes, 0);
> >>>> +}
> >>>> +EXPORT_SYMBOL(drm_dp_dual_mode_ioa_write);
> >>>
> >>> Aren't these just dupes of the dual mode helper read/write functions?
> >> No, I2C over aux read uses adapter->algo->master_xfer function, which is
> >> different in case of aux channel read/write. dp_dual_mode uses i2c_xfer.
> >
> > .master_xfer() is just an internal implementation detail of the i2c
> > bus/algo driver. You're not meant call it directly. i2c_transfer() is
> > what you call, and it will end up calling .master_xfer() internally.
> >
> Please correct me if its not the case, but the master_xfer function 
> getting loaded in drm_dp_helper layer is drm_dp_i2c_xfer() which does a 
> lot of stuff. You mean to say i2c_xfer() function call will map to 
> drm_dp_i2c_xfer ?

Yes, assuming you pass in the correct i2c adapter.

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

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

* Re: [PATCH 10/12] drm/i915: Add lspcon core functions
  2016-05-03 16:14         ` Sharma, Shashank
@ 2016-05-04 21:09           ` Zanoni, Paulo R
  2016-05-06 10:16             ` Ville Syrjälä
  0 siblings, 1 reply; 34+ messages in thread
From: Zanoni, Paulo R @ 2016-05-04 21:09 UTC (permalink / raw)
  To: ville.syrjala, Sharma, Shashank; +Cc: Vetter, Daniel, intel-gfx

Em Ter, 2016-05-03 às 21:44 +0530, Sharma, Shashank escreveu:
> 
> On 5/3/2016 9:39 PM, Ville Syrjälä wrote:
> > 
> > On Tue, May 03, 2016 at 09:18:49PM +0530, Sharma, Shashank wrote:
> > > 
> > > Regards
> > > Shashank
> > > 
> > > On 5/2/2016 7:21 PM, Ville Syrjälä wrote:
> > > > 
> > > > On Mon, Apr 04, 2016 at 05:31:46PM +0530, Shashank Sharma
> > > > wrote:
> > > > > 
> > > > > This patch adds lspcon's internal functions, which work
> > > > > on the probe layer, and indicate the working status of
> > > > > lspcon, which are mostly:
> > > > > 
> > > > > probe: A lspcon device is probed only once, during boot
> > > > > time, as its always present with the device, next to port.
> > > > > So the i2c_over_aux channel is alwyas read/writeable if DC is
> > > > > powered on. If VBT says that this port contains lspcon, we
> > > > > check and probe the HW to verify and initialize it.
> > > > > 
> > > > > get_mode: This function indicates the current mode of
> > > > > operation
> > > > > of lspcon (ls or pcon mode)
> > > > > 
> > > > > change_mode: This function can change the lspcon's mode of
> > > > > operation to desired mode.
> > > > > 
> > > > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > > > > Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com>
> > > > > ---
> > > > >    drivers/gpu/drm/i915/intel_lspcon.c | 145
> > > > > ++++++++++++++++++++++++++++++++++++
> > > > >    1 file changed, 145 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > b/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > index c3c1cd2..617fe3f 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > @@ -61,6 +61,144 @@ static struct intel_lspcon
> > > > >    	return
> > > > > enc_to_lspcon(&intel_attached_encoder(connector)->base);
> > > > >    }
> > > > > 
> > > > > +enum lspcon_mode lspcon_get_current_mode(struct intel_lspcon
> > > > > *lspcon)
> > > > > +{
> > > > > +	u8 data;
> > > > > +	int err = 0;
> > > > > +	struct intel_digital_port *dig_port =
> > > > > lspcon_to_dig_port(lspcon);
> > > > > +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
> > > > > +
> > > > > +	/* Read Status: i2c over aux */
> > > > > +	err = drm_dp_dual_mode_ioa_read(adapter, &data,
> > > > > +		LSPCON_MODE_CHECK_OFFSET, sizeof(data));
> > > > > +	if (err < 0) {
> > > > > +		DRM_ERROR("LSPCON read mode ioa (0x80, 0x41)
> > > > > failed\n");
> > > > > +		return lspcon_mode_invalid;
> > > > > +	}
> > > > > +
> > > > > +	DRM_DEBUG_DRIVER("LSPCON mode (0x80, 0x41) = %x\n",
> > > > > (unsigned int)data);
> > > > > +	return data & LSPCON_MODE_MASK ? lspcon_mode_pcon :
> > > > > lspcon_mode_ls;
> > > > > +}
> > > > > +
> > > > > +int lspcon_change_mode(struct intel_lspcon *lspcon,
> > > > > +	enum lspcon_mode mode, bool force)
> > > > > +{
> > > > > +	u8 data;
> > > > > +	int err;
> > > > > +	int time_out = 200;
> > > > > +	enum lspcon_mode current_mode;
> > > > > +	struct intel_digital_port *dig_port =
> > > > > lspcon_to_dig_port(lspcon);
> > > > > +
> > > > > +	current_mode = lspcon_get_current_mode(lspcon);
> > > > > +	if (current_mode == lspcon_mode_invalid) {
> > > > > +		DRM_ERROR("Failed to get current LSPCON
> > > > > mode\n");
> > > > > +		return -EFAULT;
> > > > > +	}
> > > > > +
> > > > > +	if (current_mode == mode && !force) {
> > > > > +		DRM_DEBUG_DRIVER("Current mode = desired
> > > > > LSPCON mode\n");
> > > > > +		return 0;
> > > > > +	}
> > > > > +
> > > > > +	if (mode == lspcon_mode_ls)
> > > > > +		data = ~LSPCON_MODE_MASK;
> > > > > +	else
> > > > > +		data = LSPCON_MODE_MASK;
> > > > > +
> > > > > +	/* Change mode */
> > > > > +	err = drm_dp_dual_mode_ioa_write(&dig_port-
> > > > > >dp.aux.ddc, &data,
> > > > > +			LSPCON_MODE_CHANGE_OFFSET,
> > > > > sizeof(data));
> > > > > +	if (err < 0) {
> > > > > +		DRM_ERROR("LSPCON mode change failed\n");
> > > > > +		return err;
> > > > > +	}
> > > > > +
> > > > > +	/*
> > > > > +	* Confirm mode change by reading the status bit.
> > > > > +	* Sometimes, it takes a while to change the mode,
> > > > > +	* so wait and retry until time out or done.
> > > > > +	*/
> > > > > +	while (time_out) {
> > > > > +		current_mode =
> > > > > lspcon_get_current_mode(lspcon);
> > > > > +		if (current_mode != mode) {
> > > > > +			mdelay(10);
> > > > > +			time_out -= 10;
> > > > > +		} else {
> > > > > +			lspcon->mode_of_op = mode;
> > > > > +			DRM_DEBUG_DRIVER("LSPCON mode
> > > > > changed to %s\n",
> > > > > +				mode == lspcon_mode_ls ?
> > > > > "LS" : "PCON");
> > > > > +			return 0;
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > > +	DRM_ERROR("LSPCON mode change timed out\n");
> > > > > +	return -EFAULT;
> > > > > +}
> > > > I think we probably want to put most of this stuff into the
> > > > helper. We
> > > > already have the LSPCON identification there, so having a few
> > > > helpers
> > > > to set/get the ls vs. pconn mode would seem appropriate.
> > > > 
> > > Actually handling of LS/PCON modes are specific to MCA chips, and
> > > some
> > > of the mode change mechanism and few other control stuff may not
> > > be same
> > > on parade or some other vendor's chip, so I am not sure if we
> > > should
> > > create a helper for something which is specific to this chip. You
> > > suggest so ?
> > Well, if we already put the detection into the helper we already
> > have
> > some vendor specific stuff there, no? Or would the other vendor's
> > chips
> > be identifiable in the same way?
> > 
> > Anyways, I presume someone else than Intel might want to use these
> > same
> > chips in their products, so having the support in a central place
> > would
> > seem like a good idea. If we start to see incompatble LSPCON chips
> > from
> > multiple vendors we'll need to rethink how we support them all, but
> > until we know how exactly they differ it's rather impossible to
> > design
> > the helper to deal with them. So as a first step I'd stuff it all
> > into
> > the helper.
> > 
> Yes, makes more sense this way. I will try to go through parade
> specs 
> once, and see if I can find something which needs immediate
> discussion 
> here. Else, I will move this into the central layer.

The big problem with LSPCON is that it uses bits that are reserved by
the VESA specs, such as bit 3 of I2C address 80h offset 10h. Nothing
prevents VESA from defining an actual meaning to that bit that will be
incompatible with LSPCON.

> > 
> > > 
> > > > 
> > > > > 
> > > > > +
> > > > > +bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
> > > > > +{
> > > > > +	enum drm_dp_dual_mode_type adaptor_type;
> > > > > +	struct intel_digital_port *dig_port =
> > > > > lspcon_to_dig_port(lspcon);
> > > > > +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
> > > > > +
> > > > > +	/* Lets probe the adaptor and check its type */
> > > > > +	adaptor_type = drm_dp_dual_mode_detect(adapter);
> > > > > +	if (adaptor_type != DRM_DP_DUAL_MODE_TYPE2_LSPCON) {
> > > > > +		DRM_DEBUG_DRIVER("No LSPCON detected, found
> > > > > %s\n",
> > > > > +			drm_dp_get_dual_mode_type_name(adapt
> > > > > or_type));
> > > > > +		return false;
> > > > > +	}
> > > > > +
> > > > > +	/* Yay ... got a LSPCON device */
> > > > > +	DRM_DEBUG_DRIVER("LSPCON detected\n");
> > > > > +	return true;
> > > > > +}
> > > > > +
> > > > > +enum lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
> > > > > +{
> > > > > +	enum lspcon_mode current_mode;
> > > > > +
> > > > > +	/* Detect a valid lspcon */
> > > > > +	if (!lspcon_detect_identifier(lspcon)) {
> > > > > +		DRM_DEBUG_DRIVER("Failed to find LSPCON
> > > > > identifier\n");
> > > > > +		return false;
> > > > > +	}
> > > > > +
> > > > > +	/* LSPCON's mode of operation */
> > > > > +	current_mode = lspcon_get_current_mode(lspcon);
> > > > > +	if (current_mode == lspcon_mode_invalid) {
> > > > > +		DRM_ERROR("Failed to read LSPCON mode\n");
> > > > > +		return false;
> > > > > +	}
> > > > > +
> > > > > +	/* All is well */
> > > > > +	lspcon->mode_of_op = current_mode;
> > > > > +	lspcon->active = true;
> > > > > +	return current_mode;
> > > > > +}
> > > > > +
> > > > > +bool lspcon_device_init(struct intel_lspcon *lspcon)
> > > > > +{
> > > > > +
> > > > > +	/* Lets check LSPCON now, probe the HW status */
> > > > > +	lspcon->active = false;
> > > > > +	lspcon->mode_of_op = lspcon_mode_invalid;
> > > > > +	if (!lspcon_probe(lspcon)) {
> > > > > +		DRM_ERROR("Failed to probe lspcon");
> > > > > +		return false;
> > > > > +	}
> > > > > +
> > > > > +	/* We wish to keep LSPCON in LS mode */
> > > > > +	if (lspcon->active && lspcon->mode_of_op !=
> > > > > lspcon_mode_ls) {
> > > > > +		if (lspcon_change_mode(lspcon,
> > > > > lspcon_mode_ls, true) < 0) {
> > > > > +			DRM_ERROR("LSPCON mode change to LS
> > > > > failed\n");
> > > > > +			return false;
> > > > > +		}
> > > > > +	}
> > > > > +	DRM_DEBUG_DRIVER("LSPCON init success\n");
> > > > > +	return true;
> > > > > +}
> > > > > +
> > > > >    struct edid *lspcon_get_edid(struct intel_lspcon *lspcon,
> > > > > struct drm_connector
> > > > >    						*connector
> > > > > )
> > > > >    {
> > > > > @@ -233,6 +371,7 @@ int intel_lspcon_init_connector(struct
> > > > > intel_digital_port *intel_dig_port)
> > > > >    	struct intel_encoder *intel_encoder =
> > > > > &intel_dig_port->base;
> > > > >    	struct drm_device *dev = intel_encoder->base.dev;
> > > > >    	struct drm_i915_private *dev_priv = dev-
> > > > > >dev_private;
> > > > > +	struct intel_lspcon *lspcon = &intel_dig_port-
> > > > > >lspcon;
> > > > >    	struct intel_connector *intel_connector;
> > > > >    	struct drm_connector *connector;
> > > > >    	enum port port = intel_dig_port->port;
> > > > > @@ -314,6 +453,12 @@ int intel_lspcon_init_connector(struct
> > > > > intel_digital_port *intel_dig_port)
> > > > >    		I915_WRITE(PEG_BAND_GAP_DATA, (temp &
> > > > > ~0xf) | 0xd);
> > > > >    	}
> > > > > 
> > > > > +	/* Now initialize the LSPCON device */
> > > > > +	if (!lspcon_device_init(lspcon)) {
> > > > > +		DRM_ERROR("LSPCON device init failed\n");
> > > > > +		goto fail;
> > > > > +	}
> > > > > +
> > > > >    	DRM_DEBUG_DRIVER("Success: LSPCON connector
> > > > > init\n");
> > > > >    	return 0;
> > > > > 
> > > > > --
> > > > > 1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 10/12] drm/i915: Add lspcon core functions
  2016-05-04 21:09           ` Zanoni, Paulo R
@ 2016-05-06 10:16             ` Ville Syrjälä
  0 siblings, 0 replies; 34+ messages in thread
From: Ville Syrjälä @ 2016-05-06 10:16 UTC (permalink / raw)
  To: Zanoni, Paulo R; +Cc: intel-gfx, Vetter, Daniel

On Wed, May 04, 2016 at 09:09:06PM +0000, Zanoni, Paulo R wrote:
> Em Ter, 2016-05-03 às 21:44 +0530, Sharma, Shashank escreveu:
> > 
> > On 5/3/2016 9:39 PM, Ville Syrjälä wrote:
> > > 
> > > On Tue, May 03, 2016 at 09:18:49PM +0530, Sharma, Shashank wrote:
> > > > 
> > > > Regards
> > > > Shashank
> > > > 
> > > > On 5/2/2016 7:21 PM, Ville Syrjälä wrote:
> > > > > 
> > > > > On Mon, Apr 04, 2016 at 05:31:46PM +0530, Shashank Sharma
> > > > > wrote:
> > > > > > 
> > > > > > This patch adds lspcon's internal functions, which work
> > > > > > on the probe layer, and indicate the working status of
> > > > > > lspcon, which are mostly:
> > > > > > 
> > > > > > probe: A lspcon device is probed only once, during boot
> > > > > > time, as its always present with the device, next to port.
> > > > > > So the i2c_over_aux channel is alwyas read/writeable if DC is
> > > > > > powered on. If VBT says that this port contains lspcon, we
> > > > > > check and probe the HW to verify and initialize it.
> > > > > > 
> > > > > > get_mode: This function indicates the current mode of
> > > > > > operation
> > > > > > of lspcon (ls or pcon mode)
> > > > > > 
> > > > > > change_mode: This function can change the lspcon's mode of
> > > > > > operation to desired mode.
> > > > > > 
> > > > > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > > > > > Signed-off-by: Akashdeep Sharma <Akashdeep.sharma@intel.com>
> > > > > > ---
> > > > > >    drivers/gpu/drm/i915/intel_lspcon.c | 145
> > > > > > ++++++++++++++++++++++++++++++++++++
> > > > > >    1 file changed, 145 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > > b/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > > index c3c1cd2..617fe3f 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> > > > > > @@ -61,6 +61,144 @@ static struct intel_lspcon
> > > > > >    	return
> > > > > > enc_to_lspcon(&intel_attached_encoder(connector)->base);
> > > > > >    }
> > > > > > 
> > > > > > +enum lspcon_mode lspcon_get_current_mode(struct intel_lspcon
> > > > > > *lspcon)
> > > > > > +{
> > > > > > +	u8 data;
> > > > > > +	int err = 0;
> > > > > > +	struct intel_digital_port *dig_port =
> > > > > > lspcon_to_dig_port(lspcon);
> > > > > > +	struct i2c_adapter *adapter = &dig_port->dp.aux.ddc;
> > > > > > +
> > > > > > +	/* Read Status: i2c over aux */
> > > > > > +	err = drm_dp_dual_mode_ioa_read(adapter, &data,
> > > > > > +		LSPCON_MODE_CHECK_OFFSET, sizeof(data));
> > > > > > +	if (err < 0) {
> > > > > > +		DRM_ERROR("LSPCON read mode ioa (0x80, 0x41)
> > > > > > failed\n");
> > > > > > +		return lspcon_mode_invalid;
> > > > > > +	}
> > > > > > +
> > > > > > +	DRM_DEBUG_DRIVER("LSPCON mode (0x80, 0x41) = %x\n",
> > > > > > (unsigned int)data);
> > > > > > +	return data & LSPCON_MODE_MASK ? lspcon_mode_pcon :
> > > > > > lspcon_mode_ls;
> > > > > > +}
> > > > > > +
> > > > > > +int lspcon_change_mode(struct intel_lspcon *lspcon,
> > > > > > +	enum lspcon_mode mode, bool force)
> > > > > > +{
> > > > > > +	u8 data;
> > > > > > +	int err;
> > > > > > +	int time_out = 200;
> > > > > > +	enum lspcon_mode current_mode;
> > > > > > +	struct intel_digital_port *dig_port =
> > > > > > lspcon_to_dig_port(lspcon);
> > > > > > +
> > > > > > +	current_mode = lspcon_get_current_mode(lspcon);
> > > > > > +	if (current_mode == lspcon_mode_invalid) {
> > > > > > +		DRM_ERROR("Failed to get current LSPCON
> > > > > > mode\n");
> > > > > > +		return -EFAULT;
> > > > > > +	}
> > > > > > +
> > > > > > +	if (current_mode == mode && !force) {
> > > > > > +		DRM_DEBUG_DRIVER("Current mode = desired
> > > > > > LSPCON mode\n");
> > > > > > +		return 0;
> > > > > > +	}
> > > > > > +
> > > > > > +	if (mode == lspcon_mode_ls)
> > > > > > +		data = ~LSPCON_MODE_MASK;
> > > > > > +	else
> > > > > > +		data = LSPCON_MODE_MASK;
> > > > > > +
> > > > > > +	/* Change mode */
> > > > > > +	err = drm_dp_dual_mode_ioa_write(&dig_port-
> > > > > > >dp.aux.ddc, &data,
> > > > > > +			LSPCON_MODE_CHANGE_OFFSET,
> > > > > > sizeof(data));
> > > > > > +	if (err < 0) {
> > > > > > +		DRM_ERROR("LSPCON mode change failed\n");
> > > > > > +		return err;
> > > > > > +	}
> > > > > > +
> > > > > > +	/*
> > > > > > +	* Confirm mode change by reading the status bit.
> > > > > > +	* Sometimes, it takes a while to change the mode,
> > > > > > +	* so wait and retry until time out or done.
> > > > > > +	*/
> > > > > > +	while (time_out) {
> > > > > > +		current_mode =
> > > > > > lspcon_get_current_mode(lspcon);
> > > > > > +		if (current_mode != mode) {
> > > > > > +			mdelay(10);
> > > > > > +			time_out -= 10;
> > > > > > +		} else {
> > > > > > +			lspcon->mode_of_op = mode;
> > > > > > +			DRM_DEBUG_DRIVER("LSPCON mode
> > > > > > changed to %s\n",
> > > > > > +				mode == lspcon_mode_ls ?
> > > > > > "LS" : "PCON");
> > > > > > +			return 0;
> > > > > > +		}
> > > > > > +	}
> > > > > > +
> > > > > > +	DRM_ERROR("LSPCON mode change timed out\n");
> > > > > > +	return -EFAULT;
> > > > > > +}
> > > > > I think we probably want to put most of this stuff into the
> > > > > helper. We
> > > > > already have the LSPCON identification there, so having a few
> > > > > helpers
> > > > > to set/get the ls vs. pconn mode would seem appropriate.
> > > > > 
> > > > Actually handling of LS/PCON modes are specific to MCA chips, and
> > > > some
> > > > of the mode change mechanism and few other control stuff may not
> > > > be same
> > > > on parade or some other vendor's chip, so I am not sure if we
> > > > should
> > > > create a helper for something which is specific to this chip. You
> > > > suggest so ?
> > > Well, if we already put the detection into the helper we already
> > > have
> > > some vendor specific stuff there, no? Or would the other vendor's
> > > chips
> > > be identifiable in the same way?
> > > 
> > > Anyways, I presume someone else than Intel might want to use these
> > > same
> > > chips in their products, so having the support in a central place
> > > would
> > > seem like a good idea. If we start to see incompatble LSPCON chips
> > > from
> > > multiple vendors we'll need to rethink how we support them all, but
> > > until we know how exactly they differ it's rather impossible to
> > > design
> > > the helper to deal with them. So as a first step I'd stuff it all
> > > into
> > > the helper.
> > > 
> > Yes, makes more sense this way. I will try to go through parade
> > specs 
> > once, and see if I can find something which needs immediate
> > discussion 
> > here. Else, I will move this into the central layer.
> 
> The big problem with LSPCON is that it uses bits that are reserved by
> the VESA specs, such as bit 3 of I2C address 80h offset 10h. Nothing
> prevents VESA from defining an actual meaning to that bit that will be
> incompatible with LSPCON.

Well, when/if that happens we'll have to change the way the detection
works, eg. just leave it up to the driver or something. As always, it's
hard to make predictions, especially about the future.

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

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

end of thread, other threads:[~2016-05-06 10:17 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-04 12:01 [PATCH 01/12] drm: Add helper for DP++ adaptors Shashank Sharma
2016-04-04 12:01 ` [PATCH 02/12] drm/i915: Respect DP++ adaptor TMDS clock limit Shashank Sharma
2016-04-04 12:01 ` [PATCH 03/12] drm/i915: Enable/disable TMDS output buffers in DP++ adaptor as needed Shashank Sharma
2016-04-04 12:01 ` [PATCH 04/12] drm/i915: Determine DP++ type 1 DVI adaptor presence based on VBT Shashank Sharma
2016-05-02 14:33   ` Jani Nikula
2016-05-02 14:39     ` Ville Syrjälä
2016-05-03 15:52       ` Sharma, Shashank
2016-04-04 12:01 ` [PATCH 05/12] drm/i915: Add lspcon data structures Shashank Sharma
2016-04-04 12:01 ` [PATCH 06/12] drm/i915: Add new lspcon file Shashank Sharma
2016-05-02 13:37   ` Ville Syrjälä
2016-05-03 15:39     ` Sharma, Shashank
2016-05-02 14:35   ` Jani Nikula
2016-05-03 15:53     ` Sharma, Shashank
2016-04-04 12:01 ` [PATCH 07/12] drm/i915: Add and initialize lspcon connector Shashank Sharma
2016-04-25 21:34   ` Zanoni, Paulo R
2016-04-04 12:01 ` [PATCH 08/12] drm: Add lspcon (custom type2 dp->hdmi) support Shashank Sharma
2016-05-02 13:49   ` Ville Syrjälä
2016-05-03 15:45     ` Sharma, Shashank
2016-05-03 15:59       ` Ville Syrjälä
2016-05-03 16:09         ` Sharma, Shashank
2016-05-03 17:19           ` Ville Syrjälä
2016-04-04 12:01 ` [PATCH 09/12] drm/i915: Add and register lspcon connector functions Shashank Sharma
2016-04-04 12:01 ` [PATCH 10/12] drm/i915: Add lspcon core functions Shashank Sharma
2016-05-02 13:51   ` Ville Syrjälä
2016-05-03 15:48     ` Sharma, Shashank
2016-05-03 16:09       ` Ville Syrjälä
2016-05-03 16:14         ` Sharma, Shashank
2016-05-04 21:09           ` Zanoni, Paulo R
2016-05-06 10:16             ` Ville Syrjälä
2016-04-04 12:01 ` [PATCH 11/12] drm/i915: Add lspcon hpd handler Shashank Sharma
2016-04-04 12:01 ` [PATCH 12/12] DO_NOT_MERGE: drm/i915: Hack to enable lspcon initialization Shashank Sharma
2016-04-06 12:18   ` Jani Nikula
2016-04-06 13:02     ` Sharma, Shashank
2016-04-04 13:33 ` ✗ Fi.CI.BAT: failure for series starting with [01/12] drm: Add helper for DP++ adaptors Patchwork

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