* [PATCH v2 0/4] Enable lspcon support for GEN9 devices
@ 2016-06-21 15:00 Shashank Sharma
2016-06-21 15:00 ` [PATCH v2 1/4] drm: Helper for lspcon in drm_dp_dual_mode Shashank Sharma
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Shashank Sharma @ 2016-06-21 15:00 UTC (permalink / raw)
To: intel-gfx, ville.syrjala; +Cc: paulo.r.zanoni
LSPCON is essencially a dp++->hdmi adaptor with dual mode of operation.
These modes are:
- Level Shifter mode: In LS mode, this device works as a type2 dp->hdmi
passive dongle, which steps up DP++ output to appropriate HDMI 1.4 signal.
This mode doesn't do any conversion at the protocol level.
- Protocol Converter mode: In PCON mode, this device acts as an
active DP++->HDMI 2.0 dongle, which converts the DP++ output to
compatible HDMI 2.0 output. In PCON mode, lspcon can support 4k@60
outputs, using DP HBR2 mode.
Many of Intel GEN9 devices come with in-built lspcon card
in motherboartd down mode. This patch series adds support for
lspcon devices in I915 driver.
While unit-testing this code, I was able to see a 4k@60 modeset with:
- BXT-T board
- Single HDMI 4k@60 display (ACER S)
- Ubuntu 14.04 desktop
V2: Worked on review comments from Ville
- In general, Ville suggested not to use the dual personality of
DDI to drive lspcon, so this patch set drives it just as DP++ display.
There is no separate detection for lspcon (hpd_pulse is good enough), and
its being driven as a DP display with special initialization and EDID
read sequence. To be able to do this, we driving lspcon in PCON mode only,
where it can serve both HDMI1.3/HDMI1.4 sinks as well as 4k@60 capable
HDMI 2.0 sinks. So compared to previous series, there is one patch less,
as we have dropped lspcon detection patch.
Shashank Sharma (4):
drm: Helper for lspcon in drm_dp_dual_mode
drm/i915: Add lspcon support for I915 driver
drm/i915: Parse VBT data for lspcon
drm/i915: Enable lspcon initialization
drivers/gpu/drm/drm_dp_dual_mode_helper.c | 103 +++++++++++++++++++++
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/intel_bios.c | 49 ++++++++++
drivers/gpu/drm/i915/intel_ddi.c | 29 +++++-
drivers/gpu/drm/i915/intel_drv.h | 13 ++-
drivers/gpu/drm/i915/intel_lspcon.c | 145 ++++++++++++++++++++++++++++++
include/drm/drm_dp_dual_mode_helper.h | 25 ++++++
8 files changed, 366 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_lspcon.c
--
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] 17+ messages in thread
* [PATCH v2 1/4] drm: Helper for lspcon in drm_dp_dual_mode
2016-06-21 15:00 [PATCH v2 0/4] Enable lspcon support for GEN9 devices Shashank Sharma
@ 2016-06-21 15:00 ` Shashank Sharma
2016-06-30 22:16 ` Rodrigo Vivi
2016-06-21 15:00 ` [PATCH v2 2/4] drm/i915: Add lspcon support for I915 driver Shashank Sharma
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Shashank Sharma @ 2016-06-21 15:00 UTC (permalink / raw)
To: intel-gfx, ville.syrjala; +Cc: paulo.r.zanoni
This patch adds lspcon support in dp_dual_mode helper.
lspcon is essentially a dp->hdmi dongle with dual personality.
LS mode: It works as a passive dongle, by level shifting DP++
signals to HDMI signals, in LS mode.
PCON mode: It works as a protocol converter active dongle
in pcon mode, by converting DP++ outputs to HDMI 2.0 outputs.
This patch adds support for lspcon detection and mode set
switch operations, as a dp dual mode dongle.
v2: Addressed review comments from Ville
- add adaptor id for lspcon devices (0x08), use it to identify lspcon
- change function names
old: drm_lspcon_get_current_mode/drm_lspcon_change_mode
new: drm_lspcon_get_mode/drm_lspcon_set_mode
- change drm_lspcon_get_mode type to int, to match
drm_dp_dual_mode_get_tmds_output
- change 'err' to 'ret' to match the rest of the functions
- remove pointless typecasting during call to dual_mode_read
- fix the but while setting value of data, while writing lspcon mode
- fix indentation
- change mdelay(10) -> msleep(10)
- return ETIMEDOUT instead of EFAULT, when lspcon mode change times out
- Add an empty line to separate std regs macros and lspcon regs macros
Indent bit definition
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/drm_dp_dual_mode_helper.c | 103 ++++++++++++++++++++++++++++++
include/drm/drm_dp_dual_mode_helper.h | 25 ++++++++
2 files changed, 128 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
index a7b2a75..404e715 100644
--- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
@@ -148,6 +148,14 @@ static bool is_type2_adaptor(uint8_t adaptor_id)
DP_DUAL_MODE_REV_TYPE2);
}
+bool is_lspcon_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN],
+ const uint8_t adaptor_id)
+{
+ return is_hdmi_adaptor(hdmi_id) &&
+ (adaptor_id == (DP_DUAL_MODE_REV_TYPE2 |
+ DP_DUAL_MODE_TYPE_TYPE2 | DP_DUAL_MODE_TYPE_LSPCON));
+}
+
/**
* drm_dp_dual_mode_detect - Identify the DP dual mode adaptor
* @adapter: I2C adapter for the DDC bus
@@ -203,6 +211,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_ADAPTOR_ID,
&adaptor_id, sizeof(adaptor_id));
if (ret == 0) {
+ if (is_lspcon_adaptor(hdmi_id, adaptor_id))
+ return DRM_DP_DUAL_MODE_LSPCON;
if (is_type2_adaptor(adaptor_id)) {
if (is_hdmi_adaptor(hdmi_id))
return DRM_DP_DUAL_MODE_TYPE2_HDMI;
@@ -364,3 +374,96 @@ const char *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type)
}
}
EXPORT_SYMBOL(drm_dp_get_dual_mode_type_name);
+
+/**
+ * drm_lspcon_get_current_mode: Get LSPCON's current mode of operation by
+ * by reading offset (0x80, 0x41)
+ * @i2c_adapter: I2C-over-aux adapter
+ * @current_mode: out vaiable, current lspcon mode of operation
+ *
+ * Returns:
+ * 0 on success, sets the current_mode value to appropriate mode
+ * -error on failure
+ */
+int drm_lspcon_get_mode(struct i2c_adapter *adapter,
+ enum drm_lspcon_mode *current_mode)
+{
+ u8 data;
+ int ret = 0;
+
+ if (!current_mode) {
+ DRM_ERROR("NULL input\n");
+ return -EINVAL;
+ }
+
+ /* Read Status: i2c over aux */
+ ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_LSPCON_CURRENT_MODE,
+ (void *)&data, sizeof(data));
+ if (ret < 0) {
+ DRM_ERROR("LSPCON read(0x80, 0x41) failed\n");
+ return -EFAULT;
+ }
+
+ if (data & DP_DUAL_MODE_LSPCON_MODE_MASK)
+ *current_mode = DRM_LSPCON_MODE_PCON;
+ else
+ *current_mode = DRM_LSPCON_MODE_LS;
+ return 0;
+}
+EXPORT_SYMBOL(drm_lspcon_get_mode);
+
+/**
+ * drm_lspcon_change_mode: Change LSPCON's mode of operation by
+ * by writing offset (0x80, 0x40)
+ * @i2c_adapter: I2C-over-aux adapter
+ * @reqd_mode: required mode of operation
+ *
+ * Returns:
+ * 0 on success, -error on failure/timeout
+ */
+int drm_lspcon_set_mode(struct i2c_adapter *adapter,
+ enum drm_lspcon_mode reqd_mode)
+{
+ u8 data = 0;
+ int ret;
+ int time_out = 200;
+ enum drm_lspcon_mode changed_mode;
+
+ if (reqd_mode == DRM_LSPCON_MODE_PCON)
+ data = DP_DUAL_MODE_LSPCON_MODE_MASK;
+
+ /* Change mode */
+ ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_LSPCON_MODE_CHANGE,
+ &data, sizeof(data));
+ if (ret < 0) {
+ DRM_ERROR("LSPCON mode change failed\n");
+ return ret;
+ }
+
+ /*
+ * 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.
+ */
+ do {
+ ret = drm_lspcon_get_mode(adapter, &changed_mode);
+ if (ret) {
+ DRM_ERROR("cant confirm LSPCON mode change\n");
+ return ret;
+ } else {
+ if (changed_mode != reqd_mode) {
+ msleep(10);
+ time_out -= 10;
+ } else {
+ DRM_DEBUG_KMS("LSPCON mode changed to %s\n",
+ reqd_mode == DRM_LSPCON_MODE_LS ?
+ "LS" : "PCON");
+ return 0;
+ }
+ }
+ } while (time_out);
+
+ DRM_ERROR("LSPCON mode change timed out\n");
+ return -ETIMEDOUT;
+}
+EXPORT_SYMBOL(drm_lspcon_set_mode);
diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
index e8a9dfd..441c6cb 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
@@ -40,6 +41,7 @@
#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_TYPE_LSPCON 0x08
#define DP_DUAL_MODE_IEEE_OUI 0x11 /* 11-13*/
#define DP_DUAL_IEEE_OUI_LEN 3
#define DP_DUAL_DEVICE_ID 0x14 /* 14-19 */
@@ -55,6 +57,10 @@
#define DP_DUAL_MODE_CEC_ENABLE 0x01
#define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
+#define DP_DUAL_MODE_LSPCON_MODE_CHANGE 0x40
+#define DP_DUAL_MODE_LSPCON_CURRENT_MODE 0x41
+#define DP_DUAL_MODE_LSPCON_MODE_MASK 0x1
+
struct i2c_adapter;
ssize_t drm_dp_dual_mode_read(struct i2c_adapter *adapter,
@@ -63,6 +69,19 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
u8 offset, const void *buffer, size_t size);
/**
+* enum drm_lspcon_mode
+* @lspcon_mode_ls: Level shifter mode of LSPCON
+* which drives DP++ to HDMI 1.4 conversion.
+* @lspcon_mode_pcon: Protocol converter mode of LSPCON
+* which drives DP++ to HDMI 2.0 active conversion.
+*/
+enum drm_lspcon_mode {
+ DRM_LSPCON_MODE_INVALID,
+ DRM_LSPCON_MODE_LS,
+ DRM_LSPCON_MODE_PCON,
+};
+
+/**
* enum drm_dp_dual_mode_type - Type of the DP dual mode adaptor
* @DRM_DP_DUAL_MODE_NONE: No DP dual mode adaptor
* @DRM_DP_DUAL_MODE_UNKNOWN: Could be either none or type 1 DVI adaptor
@@ -70,6 +89,7 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
* @DRM_DP_DUAL_MODE_TYPE1_HDMI: Type 1 HDMI adaptor
* @DRM_DP_DUAL_MODE_TYPE2_DVI: Type 2 DVI adaptor
* @DRM_DP_DUAL_MODE_TYPE2_HDMI: Type 2 HDMI adaptor
+ * @DRM_DP_DUAL_MODE_TYPE2_LSPCON: Level shifter /protocol converter
*/
enum drm_dp_dual_mode_type {
DRM_DP_DUAL_MODE_NONE,
@@ -78,6 +98,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_LSPCON,
};
enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
@@ -89,4 +110,8 @@ int drm_dp_dual_mode_set_tmds_output(enum drm_dp_dual_mode_type type,
struct i2c_adapter *adapter, bool enable);
const char *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
+int drm_lspcon_get_mode(struct i2c_adapter *adapter,
+ enum drm_lspcon_mode *current_mode);
+int drm_lspcon_set_mode(struct i2c_adapter *adapter,
+ enum drm_lspcon_mode reqd_mode);
#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] 17+ messages in thread
* [PATCH v2 2/4] drm/i915: Add lspcon support for I915 driver
2016-06-21 15:00 [PATCH v2 0/4] Enable lspcon support for GEN9 devices Shashank Sharma
2016-06-21 15:00 ` [PATCH v2 1/4] drm: Helper for lspcon in drm_dp_dual_mode Shashank Sharma
@ 2016-06-21 15:00 ` Shashank Sharma
2016-06-30 22:30 ` Rodrigo Vivi
2016-06-21 15:00 ` [PATCH v2 3/4] drm/i915: Parse VBT data for lspcon Shashank Sharma
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Shashank Sharma @ 2016-06-21 15:00 UTC (permalink / raw)
To: intel-gfx, ville.syrjala; +Cc: paulo.r.zanoni
This patch adds a new file, to accommodate lspcon support
for I915 driver. These functions probe, detect, initialize
and configure an on-board lspcon device during the driver
init time.
Also, this patch adds a small structure for lspcon device,
which will provide the runtime status of the device.
V2: addressed ville's review comments
- Clean the leftover macros from previous patch set
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/intel_drv.h | 13 +++-
drivers/gpu/drm/i915/intel_lspcon.c | 145 ++++++++++++++++++++++++++++++++++++
3 files changed, 158 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 276abf1..d40ff7d 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -93,6 +93,7 @@ i915-y += dvo_ch7017.o \
intel_dvo.o \
intel_hdmi.o \
intel_i2c.o \
+ intel_lspcon.o \
intel_lvds.o \
intel_panel.o \
intel_sdvo.o \
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7d0e071..4e49c16 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -894,12 +894,19 @@ struct intel_dp {
bool compliance_test_active;
};
+struct intel_lspcon {
+ bool active;
+ enum drm_lspcon_mode mode_of_op;
+ struct drm_dp_aux *aux;
+};
+
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;
@@ -1450,7 +1457,6 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config);
void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
-
/* intel_lvds.c */
void intel_lvds_init(struct drm_device *dev);
bool intel_is_dual_link_lvds(struct drm_device *dev);
@@ -1745,4 +1751,9 @@ int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state);
void intel_color_set_csc(struct drm_crtc_state *crtc_state);
void intel_color_load_luts(struct drm_crtc_state *crtc_state);
+/* intel_lspcon.c */
+bool lspcon_init(struct intel_digital_port *intel_dig_port);
+enum drm_connector_status
+lspcon_ls_mode_detect(struct drm_connector *connector, bool force);
+bool is_lspcon_active(struct intel_digital_port *dig_port);
#endif /* __INTEL_DRV_H__ */
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
new file mode 100644
index 0000000..fdeff71
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -0,0 +1,145 @@
+/*
+ * 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.
+ *
+ *
+ */
+#include <drm/drm_edid.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_dp_dual_mode_helper.h>
+#include "intel_drv.h"
+
+bool is_lspcon_active(struct intel_digital_port *dig_port)
+{
+ return dig_port->lspcon.active;
+}
+
+enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
+{
+ enum drm_lspcon_mode current_mode = DRM_LSPCON_MODE_INVALID;
+ struct i2c_adapter *adapter = &lspcon->aux->ddc;
+
+ if (drm_lspcon_get_mode(adapter, ¤t_mode))
+ DRM_ERROR("Error reading LSPCON mode\n");
+ else
+ DRM_DEBUG_KMS("Current LSPCON mode %s\n",
+ current_mode == DRM_LSPCON_MODE_PCON ? "PCON" : "LS");
+ return current_mode;
+}
+
+int lspcon_change_mode(struct intel_lspcon *lspcon,
+ enum drm_lspcon_mode mode, bool force)
+{
+ int err;
+ enum drm_lspcon_mode current_mode;
+ struct i2c_adapter *adapter = &lspcon->aux->ddc;
+
+ err = drm_lspcon_get_mode(adapter, ¤t_mode);
+ if (err) {
+ DRM_ERROR("Error reading LSPCON mode\n");
+ return err;
+ }
+
+ if (current_mode == mode && !force) {
+ DRM_DEBUG_KMS("Current mode = desired LSPCON mode\n");
+ return 0;
+ }
+
+ err = drm_lspcon_set_mode(adapter, mode);
+ if (err < 0) {
+ DRM_ERROR("LSPCON mode change failed\n");
+ return err;
+ }
+
+ lspcon->mode_of_op = mode;
+ DRM_DEBUG_KMS("LSPCON mode changed done\n");
+ return 0;
+}
+
+bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
+{
+ enum drm_dp_dual_mode_type adaptor_type;
+ struct i2c_adapter *adapter = &lspcon->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_LSPCON) {
+ DRM_DEBUG_KMS("No LSPCON detected, found %s\n",
+ drm_dp_get_dual_mode_type_name(adaptor_type));
+ return false;
+ }
+
+ /* Yay ... got a LSPCON device */
+ DRM_DEBUG_KMS("LSPCON detected\n");
+ return true;
+}
+
+enum drm_lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
+{
+
+ /* Detect a valid lspcon adaptor */
+ if (!lspcon_detect_identifier(lspcon)) {
+ DRM_DEBUG_KMS("No LSPCON identifier found\n");
+ return false;
+ }
+
+ /* Get LSPCON's mode of operation */
+ lspcon->mode_of_op = lspcon_get_current_mode(lspcon);
+ lspcon->active = true;
+ return true;
+}
+
+bool lspcon_init(struct intel_digital_port *intel_dig_port)
+{
+ struct intel_dp *dp = &intel_dig_port->dp;
+ struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+
+ if (!IS_GEN9(dev)) {
+ DRM_ERROR("LSPCON is supported on GEN9 only\n");
+ return false;
+ }
+
+ lspcon->active = false;
+ lspcon->mode_of_op = DRM_LSPCON_MODE_INVALID;
+ lspcon->aux = &dp->aux;
+
+ if (!lspcon_probe(lspcon)) {
+ DRM_ERROR("Failed to probe lspcon\n");
+ return false;
+ }
+
+ /*
+ * In the SW state machine, lets Put LSPCON in PCON mode only.
+ * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
+ * 2.0 sinks.
+ */
+ if (lspcon->active && lspcon->mode_of_op != DRM_LSPCON_MODE_PCON) {
+ if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON,
+ true) < 0) {
+ DRM_ERROR("LSPCON mode change to PCON failed\n");
+ return false;
+ }
+ }
+
+ DRM_DEBUG_KMS("Success: LSPCON init\n");
+ return true;
+}
--
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] 17+ messages in thread
* [PATCH v2 3/4] drm/i915: Parse VBT data for lspcon
2016-06-21 15:00 [PATCH v2 0/4] Enable lspcon support for GEN9 devices Shashank Sharma
2016-06-21 15:00 ` [PATCH v2 1/4] drm: Helper for lspcon in drm_dp_dual_mode Shashank Sharma
2016-06-21 15:00 ` [PATCH v2 2/4] drm/i915: Add lspcon support for I915 driver Shashank Sharma
@ 2016-06-21 15:00 ` Shashank Sharma
2016-06-30 22:48 ` Rodrigo Vivi
2016-06-21 15:00 ` [PATCH v2 4/4] drm/i915: Enable lspcon initialization Shashank Sharma
2016-06-21 15:38 ` ✗ Ro.CI.BAT: failure for Enable lspcon support for GEN9 devices (rev2) Patchwork
4 siblings, 1 reply; 17+ messages in thread
From: Shashank Sharma @ 2016-06-21 15:00 UTC (permalink / raw)
To: intel-gfx, ville.syrjala; +Cc: paulo.r.zanoni
Many GEN9 boards come with on-board lspcon cards.
Fot these boards, VBT configuration should properly point out
if a particular port contains lspcon device, so that driver can
initialize it properly.
This patch adds a utility function, which checks the VBT flag
for lspcon bit, and tells us if a port is configured to have a
lspcon device or not.
V2: Fixed review comments from Ville
- Do not forget PORT_D while checking lspcon for GEN9
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/intel_bios.c | 49 +++++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 25ebe46..3719b81 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3671,6 +3671,9 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv, enum por
bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
enum port port);
+bool intel_bios_is_lspcon_present(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 da5ed4a..2b00c89 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1759,3 +1759,52 @@ intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
return false;
}
+
+/**
+ * intel_bios_is_lspcon_present - if LSPCON is attached on %port
+ * @dev_priv: i915 device instance
+ * @port: port to check
+ *
+ * Return true if LSPCON is present on this port
+ */
+bool
+intel_bios_is_lspcon_present(struct drm_i915_private *dev_priv,
+ enum port port)
+{
+ int i;
+
+ if (WARN_ON_ONCE(!IS_GEN9(dev_priv)))
+ return false;
+
+ for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
+ if (!dev_priv->vbt.child_dev[i].common.lspcon)
+ continue;
+
+ switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
+ case DVO_PORT_DPA:
+ case DVO_PORT_HDMIA:
+ if (port == PORT_A)
+ return true;
+ break;
+ case DVO_PORT_DPB:
+ case DVO_PORT_HDMIB:
+ if (port == PORT_B)
+ return true;
+ break;
+ case DVO_PORT_DPC:
+ case DVO_PORT_HDMIC:
+ if (port == PORT_C)
+ return true;
+ break;
+ case DVO_PORT_DPD:
+ case DVO_PORT_HDMID:
+ if (port == PORT_D)
+ return true;
+ break;
+ default:
+ break;
+ }
+ }
+
+ return false;
+}
--
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] 17+ messages in thread
* [PATCH v2 4/4] drm/i915: Enable lspcon initialization
2016-06-21 15:00 [PATCH v2 0/4] Enable lspcon support for GEN9 devices Shashank Sharma
` (2 preceding siblings ...)
2016-06-21 15:00 ` [PATCH v2 3/4] drm/i915: Parse VBT data for lspcon Shashank Sharma
@ 2016-06-21 15:00 ` Shashank Sharma
2016-06-30 22:53 ` Rodrigo Vivi
2016-06-21 15:38 ` ✗ Ro.CI.BAT: failure for Enable lspcon support for GEN9 devices (rev2) Patchwork
4 siblings, 1 reply; 17+ messages in thread
From: Shashank Sharma @ 2016-06-21 15:00 UTC (permalink / raw)
To: intel-gfx, ville.syrjala; +Cc: paulo.r.zanoni
This patch adds initialization code for lspcon.
What we are doing here is:
- Check if lspcon is configured in VBT for this port
- If lspcon is configured, initialize it and configure it
as DP port.
V2: Addressed Ville's review comments:
- Not adding AVI IF functions for LSPCON display now.
This part will be added once the dig_port level AVI-IF series
gets merged.
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ad3b0ee..208a1ff 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2323,7 +2323,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
struct intel_digital_port *intel_dig_port;
struct intel_encoder *intel_encoder;
struct drm_encoder *encoder;
- bool init_hdmi, init_dp;
+ bool init_hdmi, init_dp, init_lspcon = false;
int max_lanes;
if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) {
@@ -2355,6 +2355,19 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
dev_priv->vbt.ddi_port_info[port].supports_hdmi);
init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
+
+ if (intel_bios_is_lspcon_present(dev_priv, port)) {
+ /*
+ * Lspcon device needs to be driven with DP connector
+ * with special detection sequence. So make sure DP
+ * is initialized before lspcon.
+ */
+ init_dp = true;
+ init_lspcon = true;
+ init_hdmi = false;
+ DRM_DEBUG_KMS("VBT says port %c has lspcon\n", port_name(port));
+ }
+
if (!init_dp && !init_hdmi) {
DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n",
port_name(port));
@@ -2430,6 +2443,20 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
goto err;
}
+ if (init_lspcon) {
+ if (lspcon_init(intel_dig_port))
+ /* TODO: handle hdmi info frame part */
+ DRM_DEBUG_KMS("LSPCON init success on port %c\n",
+ port_name(port));
+ else
+ /*
+ * LSPCON init faied, but DP init was success, so
+ * lets try to drive as DP++ port.
+ */
+ DRM_ERROR("LSPCON init failed on port %c\n",
+ port_name(port));
+ }
+
return;
err:
--
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] 17+ messages in thread
* ✗ Ro.CI.BAT: failure for Enable lspcon support for GEN9 devices (rev2)
2016-06-21 15:00 [PATCH v2 0/4] Enable lspcon support for GEN9 devices Shashank Sharma
` (3 preceding siblings ...)
2016-06-21 15:00 ` [PATCH v2 4/4] drm/i915: Enable lspcon initialization Shashank Sharma
@ 2016-06-21 15:38 ` Patchwork
4 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2016-06-21 15:38 UTC (permalink / raw)
To: Shashank Sharma; +Cc: intel-gfx
== Series Details ==
Series: Enable lspcon support for GEN9 devices (rev2)
URL : https://patchwork.freedesktop.org/series/8024/
State : failure
== Summary ==
Series 8024v2 Enable lspcon support for GEN9 devices
http://patchwork.freedesktop.org/api/1.0/series/8024/revisions/2/mbox
Test drv_module_reload_basic:
pass -> DMESG-WARN (ro-hsw-i7-4770r)
pass -> DMESG-WARN (ro-bdw-i7-5600u)
pass -> DMESG-WARN (ro-bdw-i7-5557U)
pass -> DMESG-WARN (ro-hsw-i3-4010u)
pass -> DMESG-WARN (ro-bdw-i5-5250u)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass -> FAIL (fi-skl-i5-6260u)
Subgroup suspend-read-crc-pipe-a:
dmesg-warn -> SKIP (ro-bdw-i5-5250u)
Subgroup suspend-read-crc-pipe-c:
dmesg-warn -> SKIP (ro-bdw-i5-5250u)
fi-skl-i5-6260u total:225 pass:199 dwarn:0 dfail:0 fail:3 skip:23
fi-skl-i7-6700k total:225 pass:186 dwarn:0 dfail:0 fail:2 skip:37
fi-snb-i7-2600 total:225 pass:172 dwarn:0 dfail:0 fail:2 skip:51
ro-bdw-i5-5250u total:225 pass:196 dwarn:2 dfail:0 fail:0 skip:27
ro-bdw-i7-5557U total:225 pass:197 dwarn:1 dfail:0 fail:0 skip:27
ro-bdw-i7-5600u total:225 pass:184 dwarn:1 dfail:0 fail:0 skip:40
ro-byt-n2820 total:225 pass:173 dwarn:0 dfail:0 fail:3 skip:49
ro-hsw-i3-4010u total:225 pass:189 dwarn:1 dfail:0 fail:0 skip:35
ro-hsw-i7-4770r total:225 pass:189 dwarn:1 dfail:0 fail:0 skip:35
ro-ilk-i7-620lm total:225 pass:150 dwarn:0 dfail:0 fail:1 skip:74
ro-ilk1-i5-650 total:220 pass:150 dwarn:0 dfail:0 fail:1 skip:69
ro-ivb-i7-3770 total:225 pass:181 dwarn:0 dfail:0 fail:0 skip:44
ro-ivb2-i7-3770 total:225 pass:185 dwarn:0 dfail:0 fail:0 skip:40
ro-skl3-i5-6260u total:225 pass:201 dwarn:1 dfail:0 fail:0 skip:23
ro-snb-i7-2620M total:225 pass:174 dwarn:0 dfail:0 fail:1 skip:50
fi-hsw-i7-4770k failed to connect after reboot
fi-kbl-qkkr failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_1258/
12ce3eec drm-intel-nightly: 2016y-06m-21d-14h-43m-47s UTC integration manifest
e600d56 drm/i915: Enable lspcon initialization
cede9a2 drm/i915: Parse VBT data for lspcon
72229b0 drm/i915: Add lspcon support for I915 driver
b9ee05e drm: Helper for lspcon in drm_dp_dual_mode
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] drm: Helper for lspcon in drm_dp_dual_mode
2016-06-21 15:00 ` [PATCH v2 1/4] drm: Helper for lspcon in drm_dp_dual_mode Shashank Sharma
@ 2016-06-30 22:16 ` Rodrigo Vivi
2016-07-01 5:58 ` Sharma, Shashank
0 siblings, 1 reply; 17+ messages in thread
From: Rodrigo Vivi @ 2016-06-30 22:16 UTC (permalink / raw)
To: Shashank Sharma, Pandiyan, Dhinakaran; +Cc: intel-gfx, Paulo Zanoni
On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
<shashank.sharma@intel.com> wrote:
> This patch adds lspcon support in dp_dual_mode helper.
> lspcon is essentially a dp->hdmi dongle with dual personality.
>
> LS mode: It works as a passive dongle, by level shifting DP++
> signals to HDMI signals, in LS mode.
> PCON mode: It works as a protocol converter active dongle
> in pcon mode, by converting DP++ outputs to HDMI 2.0 outputs.
>
> This patch adds support for lspcon detection and mode set
> switch operations, as a dp dual mode dongle.
>
> v2: Addressed review comments from Ville
> - add adaptor id for lspcon devices (0x08), use it to identify lspcon
> - change function names
> old: drm_lspcon_get_current_mode/drm_lspcon_change_mode
> new: drm_lspcon_get_mode/drm_lspcon_set_mode
> - change drm_lspcon_get_mode type to int, to match
> drm_dp_dual_mode_get_tmds_output
> - change 'err' to 'ret' to match the rest of the functions
> - remove pointless typecasting during call to dual_mode_read
> - fix the but while setting value of data, while writing lspcon mode
> - fix indentation
> - change mdelay(10) -> msleep(10)
> - return ETIMEDOUT instead of EFAULT, when lspcon mode change times out
> - Add an empty line to separate std regs macros and lspcon regs macros
> Indent bit definition
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
> drivers/gpu/drm/drm_dp_dual_mode_helper.c | 103 ++++++++++++++++++++++++++++++
> include/drm/drm_dp_dual_mode_helper.h | 25 ++++++++
> 2 files changed, 128 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> index a7b2a75..404e715 100644
> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
> @@ -148,6 +148,14 @@ static bool is_type2_adaptor(uint8_t adaptor_id)
> DP_DUAL_MODE_REV_TYPE2);
> }
>
> +bool is_lspcon_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN],
> + const uint8_t adaptor_id)
> +{
> + return is_hdmi_adaptor(hdmi_id) &&
> + (adaptor_id == (DP_DUAL_MODE_REV_TYPE2 |
DP_DUAL_MODE_REV_TYPE2 = 0 so useless here and confusing.
> + DP_DUAL_MODE_TYPE_TYPE2 | DP_DUAL_MODE_TYPE_LSPCON));
Also this is confusing and took me a while to uderstand that LSPCON is
the type2 with DPCD
so my suggestion is to define DP_DUAL_MODE_TYPE_HAS_DPCD (1<<3) with a
comment this is defined by LSPCON docs since this is not part of VESA
DP Dual Mode that only defines the Type2 = 0xA0.
so you could use
TYPE2 | HAS_DPCD = LSPCON.
> +}
> +
> /**
> * drm_dp_dual_mode_detect - Identify the DP dual mode adaptor
> * @adapter: I2C adapter for the DDC bus
> @@ -203,6 +211,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
> ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_ADAPTOR_ID,
> &adaptor_id, sizeof(adaptor_id));
> if (ret == 0) {
> + if (is_lspcon_adaptor(hdmi_id, adaptor_id))
> + return DRM_DP_DUAL_MODE_LSPCON;
> if (is_type2_adaptor(adaptor_id)) {
> if (is_hdmi_adaptor(hdmi_id))
> return DRM_DP_DUAL_MODE_TYPE2_HDMI;
> @@ -364,3 +374,96 @@ const char *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type)
> }
> }
> EXPORT_SYMBOL(drm_dp_get_dual_mode_type_name);
> +
> +/**
> + * drm_lspcon_get_current_mode: Get LSPCON's current mode of operation by
> + * by reading offset (0x80, 0x41)
> + * @i2c_adapter: I2C-over-aux adapter
> + * @current_mode: out vaiable, current lspcon mode of operation
> + *
> + * Returns:
> + * 0 on success, sets the current_mode value to appropriate mode
> + * -error on failure
> + */
> +int drm_lspcon_get_mode(struct i2c_adapter *adapter,
> + enum drm_lspcon_mode *current_mode)
> +{
> + u8 data;
> + int ret = 0;
> +
> + if (!current_mode) {
> + DRM_ERROR("NULL input\n");
> + return -EINVAL;
> + }
> +
> + /* Read Status: i2c over aux */
> + ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_LSPCON_CURRENT_MODE,
> + (void *)&data, sizeof(data));
> + if (ret < 0) {
> + DRM_ERROR("LSPCON read(0x80, 0x41) failed\n");
> + return -EFAULT;
> + }
> +
> + if (data & DP_DUAL_MODE_LSPCON_MODE_MASK)
> + *current_mode = DRM_LSPCON_MODE_PCON;
> + else
> + *current_mode = DRM_LSPCON_MODE_LS;
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_lspcon_get_mode);
> +
> +/**
> + * drm_lspcon_change_mode: Change LSPCON's mode of operation by
> + * by writing offset (0x80, 0x40)
> + * @i2c_adapter: I2C-over-aux adapter
> + * @reqd_mode: required mode of operation
> + *
> + * Returns:
> + * 0 on success, -error on failure/timeout
> + */
> +int drm_lspcon_set_mode(struct i2c_adapter *adapter,
> + enum drm_lspcon_mode reqd_mode)
> +{
> + u8 data = 0;
> + int ret;
> + int time_out = 200;
> + enum drm_lspcon_mode changed_mode;
> +
> + if (reqd_mode == DRM_LSPCON_MODE_PCON)
> + data = DP_DUAL_MODE_LSPCON_MODE_MASK;
> +
> + /* Change mode */
> + ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_LSPCON_MODE_CHANGE,
> + &data, sizeof(data));
> + if (ret < 0) {
> + DRM_ERROR("LSPCON mode change failed\n");
> + return ret;
> + }
> +
> + /*
> + * 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.
> + */
> + do {
> + ret = drm_lspcon_get_mode(adapter, &changed_mode);
> + if (ret) {
> + DRM_ERROR("cant confirm LSPCON mode change\n");
> + return ret;
> + } else {
> + if (changed_mode != reqd_mode) {
> + msleep(10);
> + time_out -= 10;
> + } else {
> + DRM_DEBUG_KMS("LSPCON mode changed to %s\n",
> + reqd_mode == DRM_LSPCON_MODE_LS ?
> + "LS" : "PCON");
> + return 0;
> + }
> + }
> + } while (time_out);
> +
> + DRM_ERROR("LSPCON mode change timed out\n");
> + return -ETIMEDOUT;
> +}
> +EXPORT_SYMBOL(drm_lspcon_set_mode);
> diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
> index e8a9dfd..441c6cb 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
> @@ -40,6 +41,7 @@
> #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_TYPE_LSPCON 0x08
> #define DP_DUAL_MODE_IEEE_OUI 0x11 /* 11-13*/
> #define DP_DUAL_IEEE_OUI_LEN 3
> #define DP_DUAL_DEVICE_ID 0x14 /* 14-19 */
> @@ -55,6 +57,10 @@
> #define DP_DUAL_MODE_CEC_ENABLE 0x01
> #define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
>
> +#define DP_DUAL_MODE_LSPCON_MODE_CHANGE 0x40
> +#define DP_DUAL_MODE_LSPCON_CURRENT_MODE 0x41
Where did you get this from? I haven't seen any offset 80, adress 41
defined anywhere in any of the docs I have here. What am I missing?
> +#define DP_DUAL_MODE_LSPCON_MODE_MASK 0x1
I don't believe "mask" is a good term here....
probably
DP_DUAL_MODE_LSPCON_MODE_PCON (1<<0)
> +
> struct i2c_adapter;
>
> ssize_t drm_dp_dual_mode_read(struct i2c_adapter *adapter,
> @@ -63,6 +69,19 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
> u8 offset, const void *buffer, size_t size);
>
> /**
> +* enum drm_lspcon_mode
> +* @lspcon_mode_ls: Level shifter mode of LSPCON
> +* which drives DP++ to HDMI 1.4 conversion.
> +* @lspcon_mode_pcon: Protocol converter mode of LSPCON
> +* which drives DP++ to HDMI 2.0 active conversion.
> +*/
> +enum drm_lspcon_mode {
> + DRM_LSPCON_MODE_INVALID,
> + DRM_LSPCON_MODE_LS,
> + DRM_LSPCON_MODE_PCON,
> +};
> +
> +/**
> * enum drm_dp_dual_mode_type - Type of the DP dual mode adaptor
> * @DRM_DP_DUAL_MODE_NONE: No DP dual mode adaptor
> * @DRM_DP_DUAL_MODE_UNKNOWN: Could be either none or type 1 DVI adaptor
> @@ -70,6 +89,7 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
> * @DRM_DP_DUAL_MODE_TYPE1_HDMI: Type 1 HDMI adaptor
> * @DRM_DP_DUAL_MODE_TYPE2_DVI: Type 2 DVI adaptor
> * @DRM_DP_DUAL_MODE_TYPE2_HDMI: Type 2 HDMI adaptor
> + * @DRM_DP_DUAL_MODE_TYPE2_LSPCON: Level shifter /protocol converter
> */
> enum drm_dp_dual_mode_type {
> DRM_DP_DUAL_MODE_NONE,
> @@ -78,6 +98,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_LSPCON,
> };
>
> enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
> @@ -89,4 +110,8 @@ int drm_dp_dual_mode_set_tmds_output(enum drm_dp_dual_mode_type type,
> struct i2c_adapter *adapter, bool enable);
> const char *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
>
> +int drm_lspcon_get_mode(struct i2c_adapter *adapter,
> + enum drm_lspcon_mode *current_mode);
> +int drm_lspcon_set_mode(struct i2c_adapter *adapter,
> + enum drm_lspcon_mode reqd_mode);
> #endif
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/4] drm/i915: Add lspcon support for I915 driver
2016-06-21 15:00 ` [PATCH v2 2/4] drm/i915: Add lspcon support for I915 driver Shashank Sharma
@ 2016-06-30 22:30 ` Rodrigo Vivi
2016-07-01 6:22 ` Sharma, Shashank
0 siblings, 1 reply; 17+ messages in thread
From: Rodrigo Vivi @ 2016-06-30 22:30 UTC (permalink / raw)
To: Shashank Sharma, Pandiyan, Dhinakaran; +Cc: intel-gfx, Paulo Zanoni
On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
<shashank.sharma@intel.com> wrote:
> This patch adds a new file, to accommodate lspcon support
> for I915 driver. These functions probe, detect, initialize
> and configure an on-board lspcon device during the driver
> init time.
>
> Also, this patch adds a small structure for lspcon device,
> which will provide the runtime status of the device.
>
> V2: addressed ville's review comments
> - Clean the leftover macros from previous patch set
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/intel_drv.h | 13 +++-
> drivers/gpu/drm/i915/intel_lspcon.c | 145 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 158 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 276abf1..d40ff7d 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -93,6 +93,7 @@ i915-y += dvo_ch7017.o \
> intel_dvo.o \
> intel_hdmi.o \
> intel_i2c.o \
> + intel_lspcon.o \
> intel_lvds.o \
> intel_panel.o \
> intel_sdvo.o \
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7d0e071..4e49c16 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -894,12 +894,19 @@ struct intel_dp {
> bool compliance_test_active;
> };
>
> +struct intel_lspcon {
> + bool active;
> + enum drm_lspcon_mode mode_of_op;
> + struct drm_dp_aux *aux;
> +};
> +
> 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;
> @@ -1450,7 +1457,6 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config);
> void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>
> -
> /* intel_lvds.c */
> void intel_lvds_init(struct drm_device *dev);
> bool intel_is_dual_link_lvds(struct drm_device *dev);
> @@ -1745,4 +1751,9 @@ int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state);
> void intel_color_set_csc(struct drm_crtc_state *crtc_state);
> void intel_color_load_luts(struct drm_crtc_state *crtc_state);
>
> +/* intel_lspcon.c */
> +bool lspcon_init(struct intel_digital_port *intel_dig_port);
> +enum drm_connector_status
> +lspcon_ls_mode_detect(struct drm_connector *connector, bool force);
> +bool is_lspcon_active(struct intel_digital_port *dig_port);
> #endif /* __INTEL_DRV_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> new file mode 100644
> index 0000000..fdeff71
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -0,0 +1,145 @@
> +/*
> + * 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.
> + *
> + *
> + */
> +#include <drm/drm_edid.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_dp_dual_mode_helper.h>
> +#include "intel_drv.h"
> +
Does this new file worth/deserves/needs a Docbook?
> +bool is_lspcon_active(struct intel_digital_port *dig_port)
> +{
> + return dig_port->lspcon.active;
> +}
> +
> +enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
> +{
> + enum drm_lspcon_mode current_mode = DRM_LSPCON_MODE_INVALID;
> + struct i2c_adapter *adapter = &lspcon->aux->ddc;
> +
> + if (drm_lspcon_get_mode(adapter, ¤t_mode))
> + DRM_ERROR("Error reading LSPCON mode\n");
> + else
> + DRM_DEBUG_KMS("Current LSPCON mode %s\n",
> + current_mode == DRM_LSPCON_MODE_PCON ? "PCON" : "LS");
> + return current_mode;
> +}
> +
> +int lspcon_change_mode(struct intel_lspcon *lspcon,
> + enum drm_lspcon_mode mode, bool force)
> +{
> + int err;
> + enum drm_lspcon_mode current_mode;
> + struct i2c_adapter *adapter = &lspcon->aux->ddc;
> +
> + err = drm_lspcon_get_mode(adapter, ¤t_mode);
> + if (err) {
> + DRM_ERROR("Error reading LSPCON mode\n");
> + return err;
> + }
> +
> + if (current_mode == mode && !force) {
> + DRM_DEBUG_KMS("Current mode = desired LSPCON mode\n");\
debug or error?
maybe print the desired mode here?
in case desired mode is useless to know and if it happens with
frequency I'd believe we could skip the debug message and just move
on...
> + return 0;
> + }
> +
> + err = drm_lspcon_set_mode(adapter, mode);
> + if (err < 0) {
> + DRM_ERROR("LSPCON mode change failed\n");
> + return err;
> + }
> +
> + lspcon->mode_of_op = mode;
> + DRM_DEBUG_KMS("LSPCON mode changed done\n");
> + return 0;
> +}
> +
> +bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
> +{
> + enum drm_dp_dual_mode_type adaptor_type;
> + struct i2c_adapter *adapter = &lspcon->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_LSPCON) {
shouldn't we do TYPE2 | bit3 here?
> + DRM_DEBUG_KMS("No LSPCON detected, found %s\n",
> + drm_dp_get_dual_mode_type_name(adaptor_type));
> + return false;
> + }
> +
> + /* Yay ... got a LSPCON device */
> + DRM_DEBUG_KMS("LSPCON detected\n");
> + return true;
> +}
> +
> +enum drm_lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
> +{
> +
> + /* Detect a valid lspcon adaptor */
> + if (!lspcon_detect_identifier(lspcon)) {
> + DRM_DEBUG_KMS("No LSPCON identifier found\n");
> + return false;
> + }
> +
> + /* Get LSPCON's mode of operation */
> + lspcon->mode_of_op = lspcon_get_current_mode(lspcon);
> + lspcon->active = true;
> + return true;
> +}
> +
> +bool lspcon_init(struct intel_digital_port *intel_dig_port)
> +{
> + struct intel_dp *dp = &intel_dig_port->dp;
> + struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> +
> + if (!IS_GEN9(dev)) {
> + DRM_ERROR("LSPCON is supported on GEN9 only\n");
> + return false;
> + }
> +
> + lspcon->active = false;
> + lspcon->mode_of_op = DRM_LSPCON_MODE_INVALID;
> + lspcon->aux = &dp->aux;
> +
> + if (!lspcon_probe(lspcon)) {
> + DRM_ERROR("Failed to probe lspcon\n");
> + return false;
> + }
> +
> + /*
> + * In the SW state machine, lets Put LSPCON in PCON mode only.
> + * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
> + * 2.0 sinks.
> + */
Reading the specs seems to me that for HDMI 1.4 the LS is the
recommended and for HDMI 2.0 the PCON is required... but I believe we
don't have a way to determine that right?
> + if (lspcon->active && lspcon->mode_of_op != DRM_LSPCON_MODE_PCON) {
> + if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON,
> + true) < 0) {
> + DRM_ERROR("LSPCON mode change to PCON failed\n");
> + return false;
> + }
> + }
> +
> + DRM_DEBUG_KMS("Success: LSPCON init\n");
> + return true;
> +}
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/4] drm/i915: Parse VBT data for lspcon
2016-06-21 15:00 ` [PATCH v2 3/4] drm/i915: Parse VBT data for lspcon Shashank Sharma
@ 2016-06-30 22:48 ` Rodrigo Vivi
2016-07-01 6:24 ` Sharma, Shashank
0 siblings, 1 reply; 17+ messages in thread
From: Rodrigo Vivi @ 2016-06-30 22:48 UTC (permalink / raw)
To: Shashank Sharma; +Cc: intel-gfx, Paulo Zanoni
On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
<shashank.sharma@intel.com> wrote:
> Many GEN9 boards come with on-board lspcon cards.
> Fot these boards, VBT configuration should properly point out
> if a particular port contains lspcon device, so that driver can
> initialize it properly.
>
> This patch adds a utility function, which checks the VBT flag
> for lspcon bit, and tells us if a port is configured to have a
> lspcon device or not.
>
> V2: Fixed review comments from Ville
> - Do not forget PORT_D while checking lspcon for GEN9
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_bios.c | 49 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 25ebe46..3719b81 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3671,6 +3671,9 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv, enum por
> bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
> bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
> enum port port);
> +bool intel_bios_is_lspcon_present(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 da5ed4a..2b00c89 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1759,3 +1759,52 @@ intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
>
> return false;
> }
> +
> +/**
> + * intel_bios_is_lspcon_present - if LSPCON is attached on %port
> + * @dev_priv: i915 device instance
> + * @port: port to check
> + *
> + * Return true if LSPCON is present on this port
> + */
> +bool
> +intel_bios_is_lspcon_present(struct drm_i915_private *dev_priv,
> + enum port port)
> +{
> + int i;
> +
> + if (WARN_ON_ONCE(!IS_GEN9(dev_priv)))
> + return false;
This will cause noise on other platforms.
Also I'd prefer gen < 9 assuming next gens will inherit this support.
Or also create a HAS_LSPCON on the platform definition and there use gen < 9.
> +
> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> + if (!dev_priv->vbt.child_dev[i].common.lspcon)
> + continue;
> +
> + switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
> + case DVO_PORT_DPA:
> + case DVO_PORT_HDMIA:
> + if (port == PORT_A)
> + return true;
> + break;
> + case DVO_PORT_DPB:
> + case DVO_PORT_HDMIB:
> + if (port == PORT_B)
> + return true;
> + break;
> + case DVO_PORT_DPC:
> + case DVO_PORT_HDMIC:
> + if (port == PORT_C)
> + return true;
> + break;
> + case DVO_PORT_DPD:
> + case DVO_PORT_HDMID:
> + if (port == PORT_D)
> + return true;
> + break;
> + default:
> + break;
> + }
> + }
> +
> + return false;
> +}
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Enable lspcon initialization
2016-06-21 15:00 ` [PATCH v2 4/4] drm/i915: Enable lspcon initialization Shashank Sharma
@ 2016-06-30 22:53 ` Rodrigo Vivi
2016-07-01 6:27 ` Sharma, Shashank
0 siblings, 1 reply; 17+ messages in thread
From: Rodrigo Vivi @ 2016-06-30 22:53 UTC (permalink / raw)
To: Shashank Sharma, Pandiyan, Dhinakaran; +Cc: intel-gfx, Paulo Zanoni
On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
<shashank.sharma@intel.com> wrote:
> This patch adds initialization code for lspcon.
> What we are doing here is:
> - Check if lspcon is configured in VBT for this port
> - If lspcon is configured, initialize it and configure it
> as DP port.
>
> V2: Addressed Ville's review comments:
> - Not adding AVI IF functions for LSPCON display now.
> This part will be added once the dig_port level AVI-IF series
> gets merged.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index ad3b0ee..208a1ff 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2323,7 +2323,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> struct intel_digital_port *intel_dig_port;
> struct intel_encoder *intel_encoder;
> struct drm_encoder *encoder;
> - bool init_hdmi, init_dp;
> + bool init_hdmi, init_dp, init_lspcon = false;
> int max_lanes;
>
> if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) {
> @@ -2355,6 +2355,19 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
> dev_priv->vbt.ddi_port_info[port].supports_hdmi);
> init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
> +
> + if (intel_bios_is_lspcon_present(dev_priv, port)) {
> + /*
> + * Lspcon device needs to be driven with DP connector
> + * with special detection sequence. So make sure DP
> + * is initialized before lspcon.
> + */
> + init_dp = true;
> + init_lspcon = true;
> + init_hdmi = false;
> + DRM_DEBUG_KMS("VBT says port %c has lspcon\n", port_name(port));
> + }
> +
> if (!init_dp && !init_hdmi) {
> DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n",
> port_name(port));
> @@ -2430,6 +2443,20 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> goto err;
> }
>
> + if (init_lspcon) {
> + if (lspcon_init(intel_dig_port))
> + /* TODO: handle hdmi info frame part */
Does it work without handling it? What are we missing here?
Is this related to "drm/i915: Allow DP ports to set/readout infoframe
state (WIP)"
shouldn't it be part of a same series?
> + DRM_DEBUG_KMS("LSPCON init success on port %c\n",
> + port_name(port));
> + else
> + /*
> + * LSPCON init faied, but DP init was success, so
> + * lets try to drive as DP++ port.
> + */
> + DRM_ERROR("LSPCON init failed on port %c\n",
> + port_name(port));
> + }
> +
> return;
>
> err:
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] drm: Helper for lspcon in drm_dp_dual_mode
2016-06-30 22:16 ` Rodrigo Vivi
@ 2016-07-01 5:58 ` Sharma, Shashank
2016-07-01 14:11 ` Rodrigo Vivi
0 siblings, 1 reply; 17+ messages in thread
From: Sharma, Shashank @ 2016-07-01 5:58 UTC (permalink / raw)
To: Rodrigo Vivi, Pandiyan, Dhinakaran; +Cc: intel-gfx, Paulo Zanoni
Thanks for the review Rodrigo. My comments inline.
Regards
Shashank
On 7/1/2016 3:46 AM, Rodrigo Vivi wrote:
> On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
> <shashank.sharma@intel.com> wrote:
>> This patch adds lspcon support in dp_dual_mode helper.
>> lspcon is essentially a dp->hdmi dongle with dual personality.
>>
>> LS mode: It works as a passive dongle, by level shifting DP++
>> signals to HDMI signals, in LS mode.
>> PCON mode: It works as a protocol converter active dongle
>> in pcon mode, by converting DP++ outputs to HDMI 2.0 outputs.
>>
>> This patch adds support for lspcon detection and mode set
>> switch operations, as a dp dual mode dongle.
>>
>> v2: Addressed review comments from Ville
>> - add adaptor id for lspcon devices (0x08), use it to identify lspcon
>> - change function names
>> old: drm_lspcon_get_current_mode/drm_lspcon_change_mode
>> new: drm_lspcon_get_mode/drm_lspcon_set_mode
>> - change drm_lspcon_get_mode type to int, to match
>> drm_dp_dual_mode_get_tmds_output
>> - change 'err' to 'ret' to match the rest of the functions
>> - remove pointless typecasting during call to dual_mode_read
>> - fix the but while setting value of data, while writing lspcon mode
>> - fix indentation
>> - change mdelay(10) -> msleep(10)
>> - return ETIMEDOUT instead of EFAULT, when lspcon mode change times out
>> - Add an empty line to separate std regs macros and lspcon regs macros
>> Indent bit definition
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>> drivers/gpu/drm/drm_dp_dual_mode_helper.c | 103 ++++++++++++++++++++++++++++++
>> include/drm/drm_dp_dual_mode_helper.h | 25 ++++++++
>> 2 files changed, 128 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> index a7b2a75..404e715 100644
>> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> @@ -148,6 +148,14 @@ static bool is_type2_adaptor(uint8_t adaptor_id)
>> DP_DUAL_MODE_REV_TYPE2);
>> }
>>
>> +bool is_lspcon_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN],
>> + const uint8_t adaptor_id)
>> +{
>> + return is_hdmi_adaptor(hdmi_id) &&
>> + (adaptor_id == (DP_DUAL_MODE_REV_TYPE2 |
>
> DP_DUAL_MODE_REV_TYPE2 = 0 so useless here and confusing.
This was a review comment from Ville, on the last patch. I think he
suggested for the better readability of the code.
>
>> + DP_DUAL_MODE_TYPE_TYPE2 | DP_DUAL_MODE_TYPE_LSPCON));
>
> Also this is confusing and took me a while to uderstand that LSPCON is
> the type2 with DPCD
I know, due to LSPCON's dual personality, its complicated to understand.
I tried to cover some theory, in the cover letter, can add some here too:
LSPCON is a dp->hdmi adapter which can operate in two modes
Passive dongle mode / LS mode: in this mode, it acts as type2 dp dual
mode adapter (no DPCD readable here)
Active mode / PCON mode: in this mode, it acts as active DP++->HDMI2.0
protocol convertor dongle, and allows DPCD read/write like a DP++ display.
>
> so my suggestion is to define DP_DUAL_MODE_TYPE_HAS_DPCD (1<<3) with a
> comment this is defined by LSPCON docs since this is not part of VESA
> DP Dual Mode that only defines the Type2 = 0xA0.
>
> so you could use
> TYPE2 | HAS_DPCD = LSPCON.
>
Sure, got it. Will change it like this.
>> +}
>> +
>> /**
>> * drm_dp_dual_mode_detect - Identify the DP dual mode adaptor
>> * @adapter: I2C adapter for the DDC bus
>> @@ -203,6 +211,8 @@ enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
>> ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_ADAPTOR_ID,
>> &adaptor_id, sizeof(adaptor_id));
>> if (ret == 0) {
>> + if (is_lspcon_adaptor(hdmi_id, adaptor_id))
>> + return DRM_DP_DUAL_MODE_LSPCON;
>> if (is_type2_adaptor(adaptor_id)) {
>> if (is_hdmi_adaptor(hdmi_id))
>> return DRM_DP_DUAL_MODE_TYPE2_HDMI;
>> @@ -364,3 +374,96 @@ const char *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type)
>> }
>> }
>> EXPORT_SYMBOL(drm_dp_get_dual_mode_type_name);
>> +
>> +/**
>> + * drm_lspcon_get_current_mode: Get LSPCON's current mode of operation by
>> + * by reading offset (0x80, 0x41)
>> + * @i2c_adapter: I2C-over-aux adapter
>> + * @current_mode: out vaiable, current lspcon mode of operation
>> + *
>> + * Returns:
>> + * 0 on success, sets the current_mode value to appropriate mode
>> + * -error on failure
>> + */
>> +int drm_lspcon_get_mode(struct i2c_adapter *adapter,
>> + enum drm_lspcon_mode *current_mode)
>> +{
>> + u8 data;
>> + int ret = 0;
>> +
>> + if (!current_mode) {
>> + DRM_ERROR("NULL input\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* Read Status: i2c over aux */
>> + ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_LSPCON_CURRENT_MODE,
>> + (void *)&data, sizeof(data));
>> + if (ret < 0) {
>> + DRM_ERROR("LSPCON read(0x80, 0x41) failed\n");
>> + return -EFAULT;
>> + }
>
>> +
>> + if (data & DP_DUAL_MODE_LSPCON_MODE_MASK)
>> + *current_mode = DRM_LSPCON_MODE_PCON;
>> + else
>> + *current_mode = DRM_LSPCON_MODE_LS;
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(drm_lspcon_get_mode);
>> +
>> +/**
>> + * drm_lspcon_change_mode: Change LSPCON's mode of operation by
>> + * by writing offset (0x80, 0x40)
>> + * @i2c_adapter: I2C-over-aux adapter
>> + * @reqd_mode: required mode of operation
>> + *
>> + * Returns:
>> + * 0 on success, -error on failure/timeout
>> + */
>> +int drm_lspcon_set_mode(struct i2c_adapter *adapter,
>> + enum drm_lspcon_mode reqd_mode)
>> +{
>> + u8 data = 0;
>> + int ret;
>> + int time_out = 200;
>> + enum drm_lspcon_mode changed_mode;
>> +
>> + if (reqd_mode == DRM_LSPCON_MODE_PCON)
>> + data = DP_DUAL_MODE_LSPCON_MODE_MASK;
>> +
>> + /* Change mode */
>> + ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_LSPCON_MODE_CHANGE,
>> + &data, sizeof(data));
>> + if (ret < 0) {
>> + DRM_ERROR("LSPCON mode change failed\n");
>> + return ret;
>> + }
>> +
>> + /*
>> + * 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.
>> + */
>> + do {
>> + ret = drm_lspcon_get_mode(adapter, &changed_mode);
>> + if (ret) {
>> + DRM_ERROR("cant confirm LSPCON mode change\n");
>> + return ret;
>> + } else {
>> + if (changed_mode != reqd_mode) {
>> + msleep(10);
>> + time_out -= 10;
>> + } else {
>> + DRM_DEBUG_KMS("LSPCON mode changed to %s\n",
>> + reqd_mode == DRM_LSPCON_MODE_LS ?
>> + "LS" : "PCON");
>> + return 0;
>> + }
>> + }
>> + } while (time_out);
>> +
>> + DRM_ERROR("LSPCON mode change timed out\n");
>> + return -ETIMEDOUT;
>> +}
>> +EXPORT_SYMBOL(drm_lspcon_set_mode);
>> diff --git a/include/drm/drm_dp_dual_mode_helper.h b/include/drm/drm_dp_dual_mode_helper.h
>> index e8a9dfd..441c6cb 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
>> @@ -40,6 +41,7 @@
>> #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_TYPE_LSPCON 0x08
>> #define DP_DUAL_MODE_IEEE_OUI 0x11 /* 11-13*/
>> #define DP_DUAL_IEEE_OUI_LEN 3
>> #define DP_DUAL_DEVICE_ID 0x14 /* 14-19 */
>> @@ -55,6 +57,10 @@
>> #define DP_DUAL_MODE_CEC_ENABLE 0x01
>> #define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
>>
>> +#define DP_DUAL_MODE_LSPCON_MODE_CHANGE 0x40
>> +#define DP_DUAL_MODE_LSPCON_CURRENT_MODE 0x41
>
> Where did you get this from? I haven't seen any offset 80, adress 41
> defined anywhere in any of the docs I have here. What am I missing?
>
This offset was added by MCA after Intel requested them to confirm the
LSPCON modeset success. Initially they gave us just 0x40, which is to
request a change in mode from LS->PCON or other way. But we saw many
times this change was not happening, so Intel requested MCA to have
another register offset (0x41) to confirm the current LSPCON mode.
I dont have an updated MCA doc on this, but I can arrange if you insist
on this.
>> +#define DP_DUAL_MODE_LSPCON_MODE_MASK 0x1
>
> I don't believe "mask" is a good term here....
>
> probably
> DP_DUAL_MODE_LSPCON_MODE_PCON (1<<0)
Agree, will change it to something as you suggested
Shashank
>
>> +
>> struct i2c_adapter;
>>
>> ssize_t drm_dp_dual_mode_read(struct i2c_adapter *adapter,
>> @@ -63,6 +69,19 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
>> u8 offset, const void *buffer, size_t size);
>>
>> /**
>> +* enum drm_lspcon_mode
>> +* @lspcon_mode_ls: Level shifter mode of LSPCON
>> +* which drives DP++ to HDMI 1.4 conversion.
>> +* @lspcon_mode_pcon: Protocol converter mode of LSPCON
>> +* which drives DP++ to HDMI 2.0 active conversion.
>> +*/
>> +enum drm_lspcon_mode {
>> + DRM_LSPCON_MODE_INVALID,
>> + DRM_LSPCON_MODE_LS,
>> + DRM_LSPCON_MODE_PCON,
>> +};
>> +
>> +/**
>> * enum drm_dp_dual_mode_type - Type of the DP dual mode adaptor
>> * @DRM_DP_DUAL_MODE_NONE: No DP dual mode adaptor
>> * @DRM_DP_DUAL_MODE_UNKNOWN: Could be either none or type 1 DVI adaptor
>> @@ -70,6 +89,7 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
>> * @DRM_DP_DUAL_MODE_TYPE1_HDMI: Type 1 HDMI adaptor
>> * @DRM_DP_DUAL_MODE_TYPE2_DVI: Type 2 DVI adaptor
>> * @DRM_DP_DUAL_MODE_TYPE2_HDMI: Type 2 HDMI adaptor
>> + * @DRM_DP_DUAL_MODE_TYPE2_LSPCON: Level shifter /protocol converter
>> */
>> enum drm_dp_dual_mode_type {
>> DRM_DP_DUAL_MODE_NONE,
>> @@ -78,6 +98,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_LSPCON,
>> };
>>
>> enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter *adapter);
>> @@ -89,4 +110,8 @@ int drm_dp_dual_mode_set_tmds_output(enum drm_dp_dual_mode_type type,
>> struct i2c_adapter *adapter, bool enable);
>> const char *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type type);
>>
>> +int drm_lspcon_get_mode(struct i2c_adapter *adapter,
>> + enum drm_lspcon_mode *current_mode);
>> +int drm_lspcon_set_mode(struct i2c_adapter *adapter,
>> + enum drm_lspcon_mode reqd_mode);
>> #endif
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/4] drm/i915: Add lspcon support for I915 driver
2016-06-30 22:30 ` Rodrigo Vivi
@ 2016-07-01 6:22 ` Sharma, Shashank
2016-07-02 0:13 ` Rodrigo Vivi
0 siblings, 1 reply; 17+ messages in thread
From: Sharma, Shashank @ 2016-07-01 6:22 UTC (permalink / raw)
To: Rodrigo Vivi, Pandiyan, Dhinakaran; +Cc: intel-gfx, Paulo Zanoni
Regards
Shashank
On 7/1/2016 4:00 AM, Rodrigo Vivi wrote:
> On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
> <shashank.sharma@intel.com> wrote:
>> This patch adds a new file, to accommodate lspcon support
>> for I915 driver. These functions probe, detect, initialize
>> and configure an on-board lspcon device during the driver
>> init time.
>>
>> Also, this patch adds a small structure for lspcon device,
>> which will provide the runtime status of the device.
>>
>> V2: addressed ville's review comments
>> - Clean the leftover macros from previous patch set
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
>> ---
>> drivers/gpu/drm/i915/Makefile | 1 +
>> drivers/gpu/drm/i915/intel_drv.h | 13 +++-
>> drivers/gpu/drm/i915/intel_lspcon.c | 145 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 158 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 276abf1..d40ff7d 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -93,6 +93,7 @@ i915-y += dvo_ch7017.o \
>> intel_dvo.o \
>> intel_hdmi.o \
>> intel_i2c.o \
>> + intel_lspcon.o \
>> intel_lvds.o \
>> intel_panel.o \
>> intel_sdvo.o \
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 7d0e071..4e49c16 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -894,12 +894,19 @@ struct intel_dp {
>> bool compliance_test_active;
>> };
>>
>> +struct intel_lspcon {
>> + bool active;
>> + enum drm_lspcon_mode mode_of_op;
>> + struct drm_dp_aux *aux;
>> +};
>> +
>> 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;
>> @@ -1450,7 +1457,6 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>> struct intel_crtc_state *pipe_config);
>> void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>>
>> -
>> /* intel_lvds.c */
>> void intel_lvds_init(struct drm_device *dev);
>> bool intel_is_dual_link_lvds(struct drm_device *dev);
>> @@ -1745,4 +1751,9 @@ int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state);
>> void intel_color_set_csc(struct drm_crtc_state *crtc_state);
>> void intel_color_load_luts(struct drm_crtc_state *crtc_state);
>>
>> +/* intel_lspcon.c */
>> +bool lspcon_init(struct intel_digital_port *intel_dig_port);
>> +enum drm_connector_status
>> +lspcon_ls_mode_detect(struct drm_connector *connector, bool force);
>> +bool is_lspcon_active(struct intel_digital_port *dig_port);
>> #endif /* __INTEL_DRV_H__ */
>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
>> new file mode 100644
>> index 0000000..fdeff71
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>> @@ -0,0 +1,145 @@
>> +/*
>> + * 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.
>> + *
>> + *
>> + */
>> +#include <drm/drm_edid.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_dp_dual_mode_helper.h>
>> +#include "intel_drv.h"
>> +
>
> Does this new file worth/deserves/needs a Docbook?
Its a wrapper over the drm_dp_dual_mode layer, where most of the job is
being done. But if you suggest, I can make some documentation.
>
>> +bool is_lspcon_active(struct intel_digital_port *dig_port)
>> +{
>> + return dig_port->lspcon.active;
>> +}
>> +
>> +enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
>> +{
>> + enum drm_lspcon_mode current_mode = DRM_LSPCON_MODE_INVALID;
>> + struct i2c_adapter *adapter = &lspcon->aux->ddc;
>> +
>> + if (drm_lspcon_get_mode(adapter, ¤t_mode))
>> + DRM_ERROR("Error reading LSPCON mode\n");
>> + else
>> + DRM_DEBUG_KMS("Current LSPCON mode %s\n",
>> + current_mode == DRM_LSPCON_MODE_PCON ? "PCON" : "LS");
>> + return current_mode;
>> +}
>> +
>> +int lspcon_change_mode(struct intel_lspcon *lspcon,
>> + enum drm_lspcon_mode mode, bool force)
>> +{
>> + int err;
>> + enum drm_lspcon_mode current_mode;
>> + struct i2c_adapter *adapter = &lspcon->aux->ddc;
>> +
>> + err = drm_lspcon_get_mode(adapter, ¤t_mode);
>> + if (err) {
>> + DRM_ERROR("Error reading LSPCON mode\n");
>> + return err;
>> + }
>> +
>> + if (current_mode == mode && !force) {
>> + DRM_DEBUG_KMS("Current mode = desired LSPCON mode\n");\
>
> debug or error?
> maybe print the desired mode here?
>
> in case desired mode is useless to know and if it happens with
> frequency I'd believe we could skip the debug message and just move
> on...
Actually I was leaving scope for a long-term design, where an IOCTL is
being given to the userspace, which can request a LS->PCON or PCON->LS
mode, and in that case, it would be good to print this considering its a
dumb IOCTL, do you think so ?
>
>> + return 0;
>> + }
>> +
>> + err = drm_lspcon_set_mode(adapter, mode);
>> + if (err < 0) {
>> + DRM_ERROR("LSPCON mode change failed\n");
>> + return err;
>> + }
>> +
>> + lspcon->mode_of_op = mode;
>> + DRM_DEBUG_KMS("LSPCON mode changed done\n");
>> + return 0;
>> +}
>> +
>> +bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
>> +{
>> + enum drm_dp_dual_mode_type adaptor_type;
>> + struct i2c_adapter *adapter = &lspcon->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_LSPCON) {
>
> shouldn't we do TYPE2 | bit3 here?
>
I have added LSPCON detection case in drm_dp_dual_mode_detect()
function, so it directly returns DRM_DP_DUAL_MODE_LSPCON when detected :).
>> + DRM_DEBUG_KMS("No LSPCON detected, found %s\n",
>> + drm_dp_get_dual_mode_type_name(adaptor_type));
>> + return false;
>> + }
>> +
>> + /* Yay ... got a LSPCON device */
>> + DRM_DEBUG_KMS("LSPCON detected\n");
>> + return true;
>> +}
>> +
>> +enum drm_lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
>> +{
>> +
>> + /* Detect a valid lspcon adaptor */
>> + if (!lspcon_detect_identifier(lspcon)) {
>> + DRM_DEBUG_KMS("No LSPCON identifier found\n");
>> + return false;
>> + }
>> +
>> + /* Get LSPCON's mode of operation */
>> + lspcon->mode_of_op = lspcon_get_current_mode(lspcon);
>> + lspcon->active = true;
>> + return true;
>> +}
>> +
>> +bool lspcon_init(struct intel_digital_port *intel_dig_port)
>> +{
>> + struct intel_dp *dp = &intel_dig_port->dp;
>> + struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
>> + struct drm_device *dev = intel_dig_port->base.base.dev;
>> +
>> + if (!IS_GEN9(dev)) {
>> + DRM_ERROR("LSPCON is supported on GEN9 only\n");
>> + return false;
>> + }
>> +
>> + lspcon->active = false;
>> + lspcon->mode_of_op = DRM_LSPCON_MODE_INVALID;
>> + lspcon->aux = &dp->aux;
>> +
>> + if (!lspcon_probe(lspcon)) {
>> + DRM_ERROR("Failed to probe lspcon\n");
>> + return false;
>> + }
>> +
>> + /*
>> + * In the SW state machine, lets Put LSPCON in PCON mode only.
>> + * In this way, it will work with both HDMI 1.4 sinks as well as HDMI
>> + * 2.0 sinks.
>> + */
>
> Reading the specs seems to me that for HDMI 1.4 the LS is the
> recommended and for HDMI 2.0 the PCON is required... but I believe we
> don't have a way to determine that right?
>
If you remember the previous design, we were deciding the mode of
operation, on the modeset, depending on the pixel clock (start in LS
mode, if modeset->clock > 297M make it go on PCON, else keep in LS)
But this approach was causing few problems with previous design and code
review suggestion, like:
- in this scenario, we needed to register dual connectors, one HDMI and
one DP, and this was causing on extra detect to swicth between DP->HDMI
Also, ville suggested not to exploit DDI's dual connector personality
anymore.
- second possibility was to create a new connector for LSPCON, and come
up with all the functions for it. But Paulo thought there was too much
code duplication there, which is making code unnecessary complex.
- If monitor is HDCP2.2 capable, you have to always be in PCON mode to
handle HDCP handshaking.
So based on this history, learning from android products, and code
review suggestions, I thought it would be very simple to have it always
running in PCON mode, which is automatically backward compatible to HDMI
1.4 monitors.
Shashank
>> + if (lspcon->active && lspcon->mode_of_op != DRM_LSPCON_MODE_PCON) {
>> + if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON,
>> + true) < 0) {
>> + DRM_ERROR("LSPCON mode change to PCON failed\n");
>> + return false;
>> + }
>> + }
>> +
>> + DRM_DEBUG_KMS("Success: LSPCON init\n");
>> + return true;
>> +}
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/4] drm/i915: Parse VBT data for lspcon
2016-06-30 22:48 ` Rodrigo Vivi
@ 2016-07-01 6:24 ` Sharma, Shashank
0 siblings, 0 replies; 17+ messages in thread
From: Sharma, Shashank @ 2016-07-01 6:24 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
Regards
Shashank
On 7/1/2016 4:18 AM, Rodrigo Vivi wrote:
> On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
> <shashank.sharma@intel.com> wrote:
>> Many GEN9 boards come with on-board lspcon cards.
>> Fot these boards, VBT configuration should properly point out
>> if a particular port contains lspcon device, so that driver can
>> initialize it properly.
>>
>> This patch adds a utility function, which checks the VBT flag
>> for lspcon bit, and tells us if a port is configured to have a
>> lspcon device or not.
>>
>> V2: Fixed review comments from Ville
>> - Do not forget PORT_D while checking lspcon for GEN9
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 3 +++
>> drivers/gpu/drm/i915/intel_bios.c | 49 +++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 52 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 25ebe46..3719b81 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3671,6 +3671,9 @@ bool intel_bios_is_port_dp_dual_mode(struct drm_i915_private *dev_priv, enum por
>> bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port);
>> bool intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
>> enum port port);
>> +bool intel_bios_is_lspcon_present(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 da5ed4a..2b00c89 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -1759,3 +1759,52 @@ intel_bios_is_port_hpd_inverted(struct drm_i915_private *dev_priv,
>>
>> return false;
>> }
>> +
>> +/**
>> + * intel_bios_is_lspcon_present - if LSPCON is attached on %port
>> + * @dev_priv: i915 device instance
>> + * @port: port to check
>> + *
>> + * Return true if LSPCON is present on this port
>> + */
>> +bool
>> +intel_bios_is_lspcon_present(struct drm_i915_private *dev_priv,
>> + enum port port)
>> +{
>> + int i;
>> +
>> + if (WARN_ON_ONCE(!IS_GEN9(dev_priv)))
>> + return false;
>
> This will cause noise on other platforms.
>
> Also I'd prefer gen < 9 assuming next gens will inherit this support.
> Or also create a HAS_LSPCON on the platform definition and there use gen < 9.
>
That's a good suggestion. I assumed that we have native HDMI 2.0
controllers in gen10 onwards, so LSPCON would be required only for GEN9
devices which are not native HDMI 2.0 capable. Do you see anything which
will inherit LSPCON ?
>> +
>> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> + if (!dev_priv->vbt.child_dev[i].common.lspcon)
>> + continue;
>> +
>> + switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
>> + case DVO_PORT_DPA:
>> + case DVO_PORT_HDMIA:
>> + if (port == PORT_A)
>> + return true;
>> + break;
>> + case DVO_PORT_DPB:
>> + case DVO_PORT_HDMIB:
>> + if (port == PORT_B)
>> + return true;
>> + break;
>> + case DVO_PORT_DPC:
>> + case DVO_PORT_HDMIC:
>> + if (port == PORT_C)
>> + return true;
>> + break;
>> + case DVO_PORT_DPD:
>> + case DVO_PORT_HDMID:
>> + if (port == PORT_D)
>> + return true;
>> + break;
>> + default:
>> + break;
>> + }
>> + }
>> +
>> + return false;
>> +}
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Enable lspcon initialization
2016-06-30 22:53 ` Rodrigo Vivi
@ 2016-07-01 6:27 ` Sharma, Shashank
2016-07-02 0:14 ` Rodrigo Vivi
0 siblings, 1 reply; 17+ messages in thread
From: Sharma, Shashank @ 2016-07-01 6:27 UTC (permalink / raw)
To: Rodrigo Vivi, Pandiyan, Dhinakaran; +Cc: intel-gfx, Paulo Zanoni
Regards
Shashank
On 7/1/2016 4:23 AM, Rodrigo Vivi wrote:
> On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
> <shashank.sharma@intel.com> wrote:
>> This patch adds initialization code for lspcon.
>> What we are doing here is:
>> - Check if lspcon is configured in VBT for this port
>> - If lspcon is configured, initialize it and configure it
>> as DP port.
>>
>> V2: Addressed Ville's review comments:
>> - Not adding AVI IF functions for LSPCON display now.
>> This part will be added once the dig_port level AVI-IF series
>> gets merged.
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_ddi.c | 29 ++++++++++++++++++++++++++++-
>> 1 file changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index ad3b0ee..208a1ff 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -2323,7 +2323,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>> struct intel_digital_port *intel_dig_port;
>> struct intel_encoder *intel_encoder;
>> struct drm_encoder *encoder;
>> - bool init_hdmi, init_dp;
>> + bool init_hdmi, init_dp, init_lspcon = false;
>> int max_lanes;
>>
>> if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) {
>> @@ -2355,6 +2355,19 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>> init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
>> dev_priv->vbt.ddi_port_info[port].supports_hdmi);
>> init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
>> +
>> + if (intel_bios_is_lspcon_present(dev_priv, port)) {
>> + /*
>> + * Lspcon device needs to be driven with DP connector
>> + * with special detection sequence. So make sure DP
>> + * is initialized before lspcon.
>> + */
>> + init_dp = true;
>> + init_lspcon = true;
>> + init_hdmi = false;
>> + DRM_DEBUG_KMS("VBT says port %c has lspcon\n", port_name(port));
>> + }
>> +
>> if (!init_dp && !init_hdmi) {
>> DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP compatible, respect it\n",
>> port_name(port));
>> @@ -2430,6 +2443,20 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>> goto err;
>> }
>>
>> + if (init_lspcon) {
>> + if (lspcon_init(intel_dig_port))
>> + /* TODO: handle hdmi info frame part */
>
> Does it work without handling it? What are we missing here?
> Is this related to "drm/i915: Allow DP ports to set/readout infoframe
> state (WIP)"
Yes, Ville suggested not to handle AVI IF in LSPCON specifically, but he
gave this patch set where its being handled for DDI displays.
> shouldn't it be part of a same series?
>
I am not sure, they are two different threads going on, you can suggest
how to go for this.
>
Shashank
>> + DRM_DEBUG_KMS("LSPCON init success on port %c\n",
>> + port_name(port));
>> + else
>> + /*
>> + * LSPCON init faied, but DP init was success, so
>> + * lets try to drive as DP++ port.
>> + */
>> + DRM_ERROR("LSPCON init failed on port %c\n",
>> + port_name(port));
>> + }
>> +
>> return;
>>
>> err:
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] drm: Helper for lspcon in drm_dp_dual_mode
2016-07-01 5:58 ` Sharma, Shashank
@ 2016-07-01 14:11 ` Rodrigo Vivi
0 siblings, 0 replies; 17+ messages in thread
From: Rodrigo Vivi @ 2016-07-01 14:11 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx, Paulo Zanoni
On Thu, Jun 30, 2016 at 10:58 PM, Sharma, Shashank
<shashank.sharma@intel.com> wrote:
> Thanks for the review Rodrigo. My comments inline.
>
> Regards
> Shashank
>
>
> On 7/1/2016 3:46 AM, Rodrigo Vivi wrote:
>>
>> On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
>> <shashank.sharma@intel.com> wrote:
>>>
>>> This patch adds lspcon support in dp_dual_mode helper.
>>> lspcon is essentially a dp->hdmi dongle with dual personality.
>>>
>>> LS mode: It works as a passive dongle, by level shifting DP++
>>> signals to HDMI signals, in LS mode.
>>> PCON mode: It works as a protocol converter active dongle
>>> in pcon mode, by converting DP++ outputs to HDMI 2.0 outputs.
>>>
>>> This patch adds support for lspcon detection and mode set
>>> switch operations, as a dp dual mode dongle.
>>>
>>> v2: Addressed review comments from Ville
>>> - add adaptor id for lspcon devices (0x08), use it to identify lspcon
>>> - change function names
>>> old: drm_lspcon_get_current_mode/drm_lspcon_change_mode
>>> new: drm_lspcon_get_mode/drm_lspcon_set_mode
>>> - change drm_lspcon_get_mode type to int, to match
>>> drm_dp_dual_mode_get_tmds_output
>>> - change 'err' to 'ret' to match the rest of the functions
>>> - remove pointless typecasting during call to dual_mode_read
>>> - fix the but while setting value of data, while writing lspcon mode
>>> - fix indentation
>>> - change mdelay(10) -> msleep(10)
>>> - return ETIMEDOUT instead of EFAULT, when lspcon mode change times out
>>> - Add an empty line to separate std regs macros and lspcon regs macros
>>> Indent bit definition
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> ---
>>> drivers/gpu/drm/drm_dp_dual_mode_helper.c | 103
>>> ++++++++++++++++++++++++++++++
>>> include/drm/drm_dp_dual_mode_helper.h | 25 ++++++++
>>> 2 files changed, 128 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>>> b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>>> index a7b2a75..404e715 100644
>>> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>>> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>>> @@ -148,6 +148,14 @@ static bool is_type2_adaptor(uint8_t adaptor_id)
>>> DP_DUAL_MODE_REV_TYPE2);
>>> }
>>>
>>> +bool is_lspcon_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN],
>>> + const uint8_t adaptor_id)
>>> +{
>>> + return is_hdmi_adaptor(hdmi_id) &&
>>> + (adaptor_id == (DP_DUAL_MODE_REV_TYPE2 |
>>
>>
>> DP_DUAL_MODE_REV_TYPE2 = 0 so useless here and confusing.
>
> This was a review comment from Ville, on the last patch. I think he
> suggested for the better readability of the code.
>>
>>
>>> + DP_DUAL_MODE_TYPE_TYPE2 |
>>> DP_DUAL_MODE_TYPE_LSPCON));
>>
>>
>> Also this is confusing and took me a while to uderstand that LSPCON is
>> the type2 with DPCD
>
> I know, due to LSPCON's dual personality, its complicated to understand. I
> tried to cover some theory, in the cover letter, can add some here too:
> LSPCON is a dp->hdmi adapter which can operate in two modes
> Passive dongle mode / LS mode: in this mode, it acts as type2 dp dual mode
> adapter (no DPCD readable here)
> Active mode / PCON mode: in this mode, it acts as active DP++->HDMI2.0
> protocol convertor dongle, and allows DPCD read/write like a DP++ display.
>>
>>
>> so my suggestion is to define DP_DUAL_MODE_TYPE_HAS_DPCD (1<<3) with a
>> comment this is defined by LSPCON docs since this is not part of VESA
>> DP Dual Mode that only defines the Type2 = 0xA0.
>>
>> so you could use
>> TYPE2 | HAS_DPCD = LSPCON.
>>
> Sure, got it. Will change it like this.
>
>>> +}
>>> +
>>> /**
>>> * drm_dp_dual_mode_detect - Identify the DP dual mode adaptor
>>> * @adapter: I2C adapter for the DDC bus
>>> @@ -203,6 +211,8 @@ enum drm_dp_dual_mode_type
>>> drm_dp_dual_mode_detect(struct i2c_adapter *adapter)
>>> ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_ADAPTOR_ID,
>>> &adaptor_id, sizeof(adaptor_id));
>>> if (ret == 0) {
>>> + if (is_lspcon_adaptor(hdmi_id, adaptor_id))
>>> + return DRM_DP_DUAL_MODE_LSPCON;
>>> if (is_type2_adaptor(adaptor_id)) {
>>> if (is_hdmi_adaptor(hdmi_id))
>>> return DRM_DP_DUAL_MODE_TYPE2_HDMI;
>>> @@ -364,3 +374,96 @@ const char *drm_dp_get_dual_mode_type_name(enum
>>> drm_dp_dual_mode_type type)
>>> }
>>> }
>>> EXPORT_SYMBOL(drm_dp_get_dual_mode_type_name);
>>> +
>>> +/**
>>> + * drm_lspcon_get_current_mode: Get LSPCON's current mode of operation
>>> by
>>> + * by reading offset (0x80, 0x41)
>>> + * @i2c_adapter: I2C-over-aux adapter
>>> + * @current_mode: out vaiable, current lspcon mode of operation
>>> + *
>>> + * Returns:
>>> + * 0 on success, sets the current_mode value to appropriate mode
>>> + * -error on failure
>>> + */
>>> +int drm_lspcon_get_mode(struct i2c_adapter *adapter,
>>> + enum drm_lspcon_mode *current_mode)
>>> +{
>>> + u8 data;
>>> + int ret = 0;
>>> +
>>> + if (!current_mode) {
>>> + DRM_ERROR("NULL input\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /* Read Status: i2c over aux */
>>> + ret = drm_dp_dual_mode_read(adapter,
>>> DP_DUAL_MODE_LSPCON_CURRENT_MODE,
>>> + (void *)&data, sizeof(data));
>>> + if (ret < 0) {
>>> + DRM_ERROR("LSPCON read(0x80, 0x41) failed\n");
>>> + return -EFAULT;
>>> + }
>>
>>
>>> +
>>> + if (data & DP_DUAL_MODE_LSPCON_MODE_MASK)
>>> + *current_mode = DRM_LSPCON_MODE_PCON;
>>> + else
>>> + *current_mode = DRM_LSPCON_MODE_LS;
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL(drm_lspcon_get_mode);
>>> +
>>> +/**
>>> + * drm_lspcon_change_mode: Change LSPCON's mode of operation by
>>> + * by writing offset (0x80, 0x40)
>>> + * @i2c_adapter: I2C-over-aux adapter
>>> + * @reqd_mode: required mode of operation
>>> + *
>>> + * Returns:
>>> + * 0 on success, -error on failure/timeout
>>> + */
>>> +int drm_lspcon_set_mode(struct i2c_adapter *adapter,
>>> + enum drm_lspcon_mode reqd_mode)
>>> +{
>>> + u8 data = 0;
>>> + int ret;
>>> + int time_out = 200;
>>> + enum drm_lspcon_mode changed_mode;
>>> +
>>> + if (reqd_mode == DRM_LSPCON_MODE_PCON)
>>> + data = DP_DUAL_MODE_LSPCON_MODE_MASK;
>>> +
>>> + /* Change mode */
>>> + ret = drm_dp_dual_mode_write(adapter,
>>> DP_DUAL_MODE_LSPCON_MODE_CHANGE,
>>> + &data, sizeof(data));
>>> + if (ret < 0) {
>>> + DRM_ERROR("LSPCON mode change failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + /*
>>> + * 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.
>>> + */
>>> + do {
>>> + ret = drm_lspcon_get_mode(adapter, &changed_mode);
>>> + if (ret) {
>>> + DRM_ERROR("cant confirm LSPCON mode change\n");
>>> + return ret;
>>> + } else {
>>> + if (changed_mode != reqd_mode) {
>>> + msleep(10);
>>> + time_out -= 10;
>>> + } else {
>>> + DRM_DEBUG_KMS("LSPCON mode changed to
>>> %s\n",
>>> + reqd_mode == DRM_LSPCON_MODE_LS ?
>>> + "LS" : "PCON");
>>> + return 0;
>>> + }
>>> + }
>>> + } while (time_out);
>>> +
>>> + DRM_ERROR("LSPCON mode change timed out\n");
>>> + return -ETIMEDOUT;
>>> +}
>>> +EXPORT_SYMBOL(drm_lspcon_set_mode);
>>> diff --git a/include/drm/drm_dp_dual_mode_helper.h
>>> b/include/drm/drm_dp_dual_mode_helper.h
>>> index e8a9dfd..441c6cb 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
>>> @@ -40,6 +41,7 @@
>>> #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_TYPE_LSPCON 0x08
>>> #define DP_DUAL_MODE_IEEE_OUI 0x11 /* 11-13*/
>>> #define DP_DUAL_IEEE_OUI_LEN 3
>>> #define DP_DUAL_DEVICE_ID 0x14 /* 14-19 */
>>> @@ -55,6 +57,10 @@
>>> #define DP_DUAL_MODE_CEC_ENABLE 0x01
>>> #define DP_DUAL_MODE_I2C_SPEED_CTRL 0x22
>>>
>>> +#define DP_DUAL_MODE_LSPCON_MODE_CHANGE 0x40
>>> +#define DP_DUAL_MODE_LSPCON_CURRENT_MODE 0x41
>>
>>
>> Where did you get this from? I haven't seen any offset 80, adress 41
>> defined anywhere in any of the docs I have here. What am I missing?
>>
> This offset was added by MCA after Intel requested them to confirm the
> LSPCON modeset success. Initially they gave us just 0x40, which is to
> request a change in mode from LS->PCON or other way. But we saw many times
> this change was not happening, so Intel requested MCA to have another
> register offset (0x41) to confirm the current LSPCON mode.
> I dont have an updated MCA doc on this, but I can arrange if you insist on
> this.
No, I won't insist. I take your world, but I believe it worth a
comment in the code explaining it in case some future reader find the
old spec we have.
>>>
>>> +#define DP_DUAL_MODE_LSPCON_MODE_MASK 0x1
>>
>>
>> I don't believe "mask" is a good term here....
>>
>> probably
>> DP_DUAL_MODE_LSPCON_MODE_PCON (1<<0)
>
> Agree, will change it to something as you suggested
>
> Shashank
>
>>
>>> +
>>> struct i2c_adapter;
>>>
>>> ssize_t drm_dp_dual_mode_read(struct i2c_adapter *adapter,
>>> @@ -63,6 +69,19 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter
>>> *adapter,
>>> u8 offset, const void *buffer, size_t
>>> size);
>>>
>>> /**
>>> +* enum drm_lspcon_mode
>>> +* @lspcon_mode_ls: Level shifter mode of LSPCON
>>> +* which drives DP++ to HDMI 1.4 conversion.
>>> +* @lspcon_mode_pcon: Protocol converter mode of LSPCON
>>> +* which drives DP++ to HDMI 2.0 active conversion.
>>> +*/
>>> +enum drm_lspcon_mode {
>>> + DRM_LSPCON_MODE_INVALID,
>>> + DRM_LSPCON_MODE_LS,
>>> + DRM_LSPCON_MODE_PCON,
>>> +};
>>> +
>>> +/**
>>> * enum drm_dp_dual_mode_type - Type of the DP dual mode adaptor
>>> * @DRM_DP_DUAL_MODE_NONE: No DP dual mode adaptor
>>> * @DRM_DP_DUAL_MODE_UNKNOWN: Could be either none or type 1 DVI
>>> adaptor
>>> @@ -70,6 +89,7 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter
>>> *adapter,
>>> * @DRM_DP_DUAL_MODE_TYPE1_HDMI: Type 1 HDMI adaptor
>>> * @DRM_DP_DUAL_MODE_TYPE2_DVI: Type 2 DVI adaptor
>>> * @DRM_DP_DUAL_MODE_TYPE2_HDMI: Type 2 HDMI adaptor
>>> + * @DRM_DP_DUAL_MODE_TYPE2_LSPCON: Level shifter /protocol converter
>>> */
>>> enum drm_dp_dual_mode_type {
>>> DRM_DP_DUAL_MODE_NONE,
>>> @@ -78,6 +98,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_LSPCON,
>>> };
>>>
>>> enum drm_dp_dual_mode_type drm_dp_dual_mode_detect(struct i2c_adapter
>>> *adapter);
>>> @@ -89,4 +110,8 @@ int drm_dp_dual_mode_set_tmds_output(enum
>>> drm_dp_dual_mode_type type,
>>> struct i2c_adapter *adapter, bool
>>> enable);
>>> const char *drm_dp_get_dual_mode_type_name(enum drm_dp_dual_mode_type
>>> type);
>>>
>>> +int drm_lspcon_get_mode(struct i2c_adapter *adapter,
>>> + enum drm_lspcon_mode *current_mode);
>>> +int drm_lspcon_set_mode(struct i2c_adapter *adapter,
>>> + enum drm_lspcon_mode reqd_mode);
>>> #endif
>>> --
>>> 1.9.1
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>>
>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/4] drm/i915: Add lspcon support for I915 driver
2016-07-01 6:22 ` Sharma, Shashank
@ 2016-07-02 0:13 ` Rodrigo Vivi
0 siblings, 0 replies; 17+ messages in thread
From: Rodrigo Vivi @ 2016-07-02 0:13 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx, Paulo Zanoni
On Thu, Jun 30, 2016 at 11:22 PM, Sharma, Shashank
<shashank.sharma@intel.com> wrote:
> Regards
> Shashank
>
>
> On 7/1/2016 4:00 AM, Rodrigo Vivi wrote:
>>
>> On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
>> <shashank.sharma@intel.com> wrote:
>>>
>>> This patch adds a new file, to accommodate lspcon support
>>> for I915 driver. These functions probe, detect, initialize
>>> and configure an on-board lspcon device during the driver
>>> init time.
>>>
>>> Also, this patch adds a small structure for lspcon device,
>>> which will provide the runtime status of the device.
>>>
>>> V2: addressed ville's review comments
>>> - Clean the leftover macros from previous patch set
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/Makefile | 1 +
>>> drivers/gpu/drm/i915/intel_drv.h | 13 +++-
>>> drivers/gpu/drm/i915/intel_lspcon.c | 145
>>> ++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 158 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 276abf1..d40ff7d 100644
>>> --- a/drivers/gpu/drm/i915/Makefile
>>> +++ b/drivers/gpu/drm/i915/Makefile
>>> @@ -93,6 +93,7 @@ i915-y += dvo_ch7017.o \
>>> intel_dvo.o \
>>> intel_hdmi.o \
>>> intel_i2c.o \
>>> + intel_lspcon.o \
>>> intel_lvds.o \
>>> intel_panel.o \
>>> intel_sdvo.o \
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>>> b/drivers/gpu/drm/i915/intel_drv.h
>>> index 7d0e071..4e49c16 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -894,12 +894,19 @@ struct intel_dp {
>>> bool compliance_test_active;
>>> };
>>>
>>> +struct intel_lspcon {
>>> + bool active;
>>> + enum drm_lspcon_mode mode_of_op;
>>> + struct drm_dp_aux *aux;
>>> +};
>>> +
>>> 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;
>>> @@ -1450,7 +1457,6 @@ bool intel_hdmi_compute_config(struct intel_encoder
>>> *encoder,
>>> struct intel_crtc_state *pipe_config);
>>> void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool
>>> enable);
>>>
>>> -
>>> /* intel_lvds.c */
>>> void intel_lvds_init(struct drm_device *dev);
>>> bool intel_is_dual_link_lvds(struct drm_device *dev);
>>> @@ -1745,4 +1751,9 @@ int intel_color_check(struct drm_crtc *crtc, struct
>>> drm_crtc_state *state);
>>> void intel_color_set_csc(struct drm_crtc_state *crtc_state);
>>> void intel_color_load_luts(struct drm_crtc_state *crtc_state);
>>>
>>> +/* intel_lspcon.c */
>>> +bool lspcon_init(struct intel_digital_port *intel_dig_port);
>>> +enum drm_connector_status
>>> +lspcon_ls_mode_detect(struct drm_connector *connector, bool force);
>>> +bool is_lspcon_active(struct intel_digital_port *dig_port);
>>> #endif /* __INTEL_DRV_H__ */
>>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
>>> b/drivers/gpu/drm/i915/intel_lspcon.c
>>> new file mode 100644
>>> index 0000000..fdeff71
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
>>> @@ -0,0 +1,145 @@
>>> +/*
>>> + * 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.
>>> + *
>>> + *
>>> + */
>>> +#include <drm/drm_edid.h>
>>> +#include <drm/drm_atomic_helper.h>
>>> +#include <drm/drm_dp_dual_mode_helper.h>
>>> +#include "intel_drv.h"
>>> +
>>
>>
>> Does this new file worth/deserves/needs a Docbook?
>
> Its a wrapper over the drm_dp_dual_mode layer, where most of the job is
> being done. But if you suggest, I can make some documentation.
you are right... there is where the interfaces matter although LSPCON
we just have here for now.
Anyway no strong opinion from my part.
>
>>
>>> +bool is_lspcon_active(struct intel_digital_port *dig_port)
>>> +{
>>> + return dig_port->lspcon.active;
>>> +}
>>> +
>>> +enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon
>>> *lspcon)
>>> +{
>>> + enum drm_lspcon_mode current_mode = DRM_LSPCON_MODE_INVALID;
>>> + struct i2c_adapter *adapter = &lspcon->aux->ddc;
>>> +
>>> + if (drm_lspcon_get_mode(adapter, ¤t_mode))
>>> + DRM_ERROR("Error reading LSPCON mode\n");
>>> + else
>>> + DRM_DEBUG_KMS("Current LSPCON mode %s\n",
>>> + current_mode == DRM_LSPCON_MODE_PCON ? "PCON" :
>>> "LS");
>>> + return current_mode;
>>> +}
>>> +
>>> +int lspcon_change_mode(struct intel_lspcon *lspcon,
>>> + enum drm_lspcon_mode mode, bool force)
>>> +{
>>> + int err;
>>> + enum drm_lspcon_mode current_mode;
>>> + struct i2c_adapter *adapter = &lspcon->aux->ddc;
>>> +
>>> + err = drm_lspcon_get_mode(adapter, ¤t_mode);
>>> + if (err) {
>>> + DRM_ERROR("Error reading LSPCON mode\n");
>>> + return err;
>>> + }
>>> +
>>> + if (current_mode == mode && !force) {
>>> + DRM_DEBUG_KMS("Current mode = desired LSPCON mode\n");\
>>
>>
>> debug or error?
>> maybe print the desired mode here?
>>
>> in case desired mode is useless to know and if it happens with
>> frequency I'd believe we could skip the debug message and just move
>> on...
>
> Actually I was leaving scope for a long-term design, where an IOCTL is being
> given to the userspace, which can request a LS->PCON or PCON->LS
> mode, and in that case, it would be good to print this considering its a
> dumb IOCTL, do you think so ?
oh I see...
well, if setting with ioctl they probably know what they request
right... so ignore me ;)
>>
>>
>>> + return 0;
>>> + }
>>> +
>>> + err = drm_lspcon_set_mode(adapter, mode);
>>> + if (err < 0) {
>>> + DRM_ERROR("LSPCON mode change failed\n");
>>> + return err;
>>> + }
>>> +
>>> + lspcon->mode_of_op = mode;
>>> + DRM_DEBUG_KMS("LSPCON mode changed done\n");
>>> + return 0;
>>> +}
>>> +
>>> +bool lspcon_detect_identifier(struct intel_lspcon *lspcon)
>>> +{
>>> + enum drm_dp_dual_mode_type adaptor_type;
>>> + struct i2c_adapter *adapter = &lspcon->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_LSPCON) {
>>
>>
>> shouldn't we do TYPE2 | bit3 here?
>>
> I have added LSPCON detection case in drm_dp_dual_mode_detect() function, so
> it directly returns DRM_DP_DUAL_MODE_LSPCON when detected :).
Oh that is true.... now I got why you did the mode like this! ;)
>
>>> + DRM_DEBUG_KMS("No LSPCON detected, found %s\n",
>>> + drm_dp_get_dual_mode_type_name(adaptor_type));
>>> + return false;
>>> + }
>>> +
>>> + /* Yay ... got a LSPCON device */
>>> + DRM_DEBUG_KMS("LSPCON detected\n");
>>> + return true;
>>> +}
>>> +
>>> +enum drm_lspcon_mode lspcon_probe(struct intel_lspcon *lspcon)
>>> +{
>>> +
>>> + /* Detect a valid lspcon adaptor */
>>> + if (!lspcon_detect_identifier(lspcon)) {
>>> + DRM_DEBUG_KMS("No LSPCON identifier found\n");
>>> + return false;
>>> + }
>>> +
>>> + /* Get LSPCON's mode of operation */
>>> + lspcon->mode_of_op = lspcon_get_current_mode(lspcon);
>>> + lspcon->active = true;
>>> + return true;
>>> +}
>>> +
>>> +bool lspcon_init(struct intel_digital_port *intel_dig_port)
>>> +{
>>> + struct intel_dp *dp = &intel_dig_port->dp;
>>> + struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
>>> + struct drm_device *dev = intel_dig_port->base.base.dev;
>>> +
>>> + if (!IS_GEN9(dev)) {
>>> + DRM_ERROR("LSPCON is supported on GEN9 only\n");
>>> + return false;
>>> + }
>>> +
>>> + lspcon->active = false;
>>> + lspcon->mode_of_op = DRM_LSPCON_MODE_INVALID;
>>> + lspcon->aux = &dp->aux;
>>> +
>>> + if (!lspcon_probe(lspcon)) {
>>> + DRM_ERROR("Failed to probe lspcon\n");
>>> + return false;
>>> + }
>>> +
>>> + /*
>>> + * In the SW state machine, lets Put LSPCON in PCON mode only.
>>> + * In this way, it will work with both HDMI 1.4 sinks as well as
>>> HDMI
>>> + * 2.0 sinks.
>>> + */
>>
>>
>> Reading the specs seems to me that for HDMI 1.4 the LS is the
>> recommended and for HDMI 2.0 the PCON is required... but I believe we
>> don't have a way to determine that right?
>>
> If you remember the previous design, we were deciding the mode of operation,
> on the modeset, depending on the pixel clock (start in LS mode, if
> modeset->clock > 297M make it go on PCON, else keep in LS)
>
> But this approach was causing few problems with previous design and code
> review suggestion, like:
> - in this scenario, we needed to register dual connectors, one HDMI and one
> DP, and this was causing on extra detect to swicth between DP->HDMI
> Also, ville suggested not to exploit DDI's dual connector personality
> anymore.
> - second possibility was to create a new connector for LSPCON, and come up
> with all the functions for it. But Paulo thought there was too much code
> duplication there, which is making code unnecessary complex.
> - If monitor is HDCP2.2 capable, you have to always be in PCON mode to
> handle HDCP handshaking.
>
> So based on this history, learning from android products, and code review
> suggestions, I thought it would be very simple to have it always running in
> PCON mode, which is automatically backward compatible to HDMI 1.4 monitors.
if it falls back automatically I agree this is the best cleaner approach.
so, with or without the doc feel free to use:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Shashank
>
>>> + if (lspcon->active && lspcon->mode_of_op != DRM_LSPCON_MODE_PCON)
>>> {
>>> + if (lspcon_change_mode(lspcon, DRM_LSPCON_MODE_PCON,
>>> + true) < 0) {
>>> + DRM_ERROR("LSPCON mode change to PCON failed\n");
>>> + return false;
>>> + }
>>> + }
>>> +
>>> + DRM_DEBUG_KMS("Success: LSPCON init\n");
>>> + return true;
>>> +}
>>> --
>>> 1.9.1
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>>
>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/4] drm/i915: Enable lspcon initialization
2016-07-01 6:27 ` Sharma, Shashank
@ 2016-07-02 0:14 ` Rodrigo Vivi
0 siblings, 0 replies; 17+ messages in thread
From: Rodrigo Vivi @ 2016-07-02 0:14 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx, Paulo Zanoni
On Thu, Jun 30, 2016 at 11:27 PM, Sharma, Shashank
<shashank.sharma@intel.com> wrote:
> Regards
> Shashank
>
>
> On 7/1/2016 4:23 AM, Rodrigo Vivi wrote:
>>
>> On Tue, Jun 21, 2016 at 8:00 AM, Shashank Sharma
>> <shashank.sharma@intel.com> wrote:
>>>
>>> This patch adds initialization code for lspcon.
>>> What we are doing here is:
>>> - Check if lspcon is configured in VBT for this port
>>> - If lspcon is configured, initialize it and configure it
>>> as DP port.
>>>
>>> V2: Addressed Ville's review comments:
>>> - Not adding AVI IF functions for LSPCON display now.
>>> This part will be added once the dig_port level AVI-IF series
>>> gets merged.
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_ddi.c | 29 ++++++++++++++++++++++++++++-
>>> 1 file changed, 28 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>>> b/drivers/gpu/drm/i915/intel_ddi.c
>>> index ad3b0ee..208a1ff 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -2323,7 +2323,7 @@ void intel_ddi_init(struct drm_device *dev, enum
>>> port port)
>>> struct intel_digital_port *intel_dig_port;
>>> struct intel_encoder *intel_encoder;
>>> struct drm_encoder *encoder;
>>> - bool init_hdmi, init_dp;
>>> + bool init_hdmi, init_dp, init_lspcon = false;
>>> int max_lanes;
>>>
>>> if (I915_READ(DDI_BUF_CTL(PORT_A)) & DDI_A_4_LANES) {
>>> @@ -2355,6 +2355,19 @@ void intel_ddi_init(struct drm_device *dev, enum
>>> port port)
>>> init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
>>> dev_priv->vbt.ddi_port_info[port].supports_hdmi);
>>> init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
>>> +
>>> + if (intel_bios_is_lspcon_present(dev_priv, port)) {
>>> + /*
>>> + * Lspcon device needs to be driven with DP connector
>>> + * with special detection sequence. So make sure DP
>>> + * is initialized before lspcon.
>>> + */
>>> + init_dp = true;
>>> + init_lspcon = true;
>>> + init_hdmi = false;
>>> + DRM_DEBUG_KMS("VBT says port %c has lspcon\n",
>>> port_name(port));
>>> + }
>>> +
>>> if (!init_dp && !init_hdmi) {
>>> DRM_DEBUG_KMS("VBT says port %c is not DVI/HDMI/DP
>>> compatible, respect it\n",
>>> port_name(port));
>>> @@ -2430,6 +2443,20 @@ void intel_ddi_init(struct drm_device *dev, enum
>>> port port)
>>> goto err;
>>> }
>>>
>>> + if (init_lspcon) {
>>> + if (lspcon_init(intel_dig_port))
>>> + /* TODO: handle hdmi info frame part */
>>
>>
>> Does it work without handling it? What are we missing here?
>> Is this related to "drm/i915: Allow DP ports to set/readout infoframe
>> state (WIP)"
>
> Yes, Ville suggested not to handle AVI IF in LSPCON specifically, but he
> gave this patch set where its being handled for DDI displays.
>>
>> shouldn't it be part of a same series?
>>
> I am not sure, they are two different threads going on, you can suggest how
> to go for this.
depends on the answer of: Does it work without handling it?
>>
>>
> Shashank
>
>
>>> + DRM_DEBUG_KMS("LSPCON init success on port %c\n",
>>> + port_name(port));
>>> + else
>>> + /*
>>> + * LSPCON init faied, but DP init was success, so
>>> + * lets try to drive as DP++ port.
>>> + */
>>> + DRM_ERROR("LSPCON init failed on port %c\n",
>>> + port_name(port));
>>> + }
>>> +
>>> return;
>>>
>>> err:
>>> --
>>> 1.9.1
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>>
>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-07-02 0:14 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-21 15:00 [PATCH v2 0/4] Enable lspcon support for GEN9 devices Shashank Sharma
2016-06-21 15:00 ` [PATCH v2 1/4] drm: Helper for lspcon in drm_dp_dual_mode Shashank Sharma
2016-06-30 22:16 ` Rodrigo Vivi
2016-07-01 5:58 ` Sharma, Shashank
2016-07-01 14:11 ` Rodrigo Vivi
2016-06-21 15:00 ` [PATCH v2 2/4] drm/i915: Add lspcon support for I915 driver Shashank Sharma
2016-06-30 22:30 ` Rodrigo Vivi
2016-07-01 6:22 ` Sharma, Shashank
2016-07-02 0:13 ` Rodrigo Vivi
2016-06-21 15:00 ` [PATCH v2 3/4] drm/i915: Parse VBT data for lspcon Shashank Sharma
2016-06-30 22:48 ` Rodrigo Vivi
2016-07-01 6:24 ` Sharma, Shashank
2016-06-21 15:00 ` [PATCH v2 4/4] drm/i915: Enable lspcon initialization Shashank Sharma
2016-06-30 22:53 ` Rodrigo Vivi
2016-07-01 6:27 ` Sharma, Shashank
2016-07-02 0:14 ` Rodrigo Vivi
2016-06-21 15:38 ` ✗ Ro.CI.BAT: failure for Enable lspcon support for GEN9 devices (rev2) 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.