drm/msm: Initial add eDP support in msm drm driver (V2)

Modified the hard-coded hdmi connector/encoder implementations in msm drm driver to support both edp and hdmi.

drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c +0 -0

drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c +10 -1

@@ -177,7 +180,28 @@ static void mdp5_encoder_mode_set(struct drm_encoder *encoder,
 	/* probably need to get DATA_EN polarity from panel.. */
 
 	dtv_hsync_skew = 0;  /* get this from panel? */
-	format = 0x213f;     /* get this from panel? */
+
+	/* Get color format from panel, default is 8bpc */
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+		if (connector->encoder == encoder) {
+			switch (connector->display_info.bpc) {
+			case 4:
+				format |= 0;
+				break;
+			case 5:
+				format |= 0x15;

drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c +2 -0

+		}
+	}
 
 	hsync_start_x = (mode->htotal - mode->hsync_start);
 	hsync_end_x = mode->htotal - (mode->hsync_start - mode->hdisplay) - 1;

@@ -187,6 +211,16 @@ static void mdp5_encoder_mode_set(struct drm_encoder *encoder,
 	display_v_start = (mode->vtotal - mode->vsync_start) * mode->htotal + dtv_hsync_skew;

drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +0 -0

drivers/gpu/drm/msm/edp/edp.c new file +13 -0

new file mode 100644
index 0000000..32e21e1
@@ -0,0 +1,211 @@
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+

drivers/gpu/drm/msm/edp/edp.c new file +5 -0

+static void edp_destroy(struct platform_device *pdev)
+{
+	struct msm_edp *edp = platform_get_drvdata(pdev);
+
+	if (!edp)

drivers/gpu/drm/msm/edp/edp.c new file +25 -0

+		ret = -ENXIO;
+		goto fail;
+	}
+
+	edp = devm_kzalloc(&pdev->dev, sizeof(*edp), GFP_KERNEL);
+	if (!edp) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+	DBG("edp probed=%p", edp);
+
+	edp->pdev = pdev;
+	platform_set_drvdata(pdev, edp);
+
+	ret = msm_edp_ctrl_init(edp);
+	if (ret)
+		goto fail;
+
+	return edp;
+
+fail:
+	if (edp)
+		edp_destroy(pdev);
+
+	return ERR_PTR(ret);

drivers/gpu/drm/msm/edp/edp.c new file +18 -0

+/* Second part of initialization, the drm/kms level modeset_init */
+int msm_edp_modeset_init(struct msm_edp *edp,
+		struct drm_device *dev, struct drm_encoder *encoder)
+{
+	struct platform_device *pdev = edp->pdev;
+	struct msm_drm_private *priv = dev->dev_private;
+	int ret;
+
+	edp->encoder = encoder;
+	edp->dev = dev;
+
+	edp->bridge = msm_edp_bridge_init(edp);
+	if (IS_ERR(edp->bridge)) {
+		ret = PTR_ERR(edp->bridge);
+		dev_err(dev->dev, "failed to create eDP bridge: %d\n", ret);
+		edp->bridge = NULL;
+		goto fail;
+	}

drivers/gpu/drm/msm/edp/edp.h +0 -0

drivers/gpu/drm/msm/edp/edp_aux.c +0 -0

+#define to_edp_aux(x) container_of(x, struct edp_aux, drm_aux)

drivers/gpu/drm/msm/edp/edp_aux.c +28 -0

+static int edp_msg_fifo_tx(struct edp_aux *aux, struct drm_dp_aux_msg *msg)
+{
+	u32 data[4];
+	u32 reg, len;
+	bool native = msg->request & (DP_AUX_NATIVE_WRITE & DP_AUX_NATIVE_READ);
+	bool read = msg->request & (DP_AUX_I2C_READ & DP_AUX_NATIVE_READ);
+	u8 *msgdata = msg->buffer;
+	int i;
+
+	if (read)
+		len = 4;
+	else
+		len = msg->size + 4;
+
+	/*
+	 * cmd fifo only has depth of 144 bytes
+	 */
+	if (len > AUX_CMD_FIFO_LEN)
+		return -EINVAL;
+
+	/* Pack cmd and write to HW */
+	data[0] = (msg->address >> 16) & 0xf;	/* addr[19:16] */
+	if (read)
+		data[0] |=  BIT(4);		/* R/W */
+
+	data[1] = (msg->address >> 8) & 0xff;	/* addr[15:8] */
+	data[2] = msg->address & 0xff;		/* addr[7:0] */
+	data[3] = (msg->size - 1) & 0xff;	/* len[7:0] */

+/*
+ * This function does the real job to process an aux transaction.

drivers/gpu/drm/msm/edp/edp_bridge.c +0 -0

drivers/gpu/drm/msm/edp/edp_connector.c +0 -0

drivers/gpu/drm/msm/edp/edp_connector.c +5 -0

+static int edp_connector_get_modes(struct drm_connector *connector)
+{
+	struct edp_connector *edp_connector = to_edp_connector(connector);
+	struct msm_edp *edp = edp_connector->edp;
+

drivers/gpu/drm/msm/edp/edp_connector.c +5 -0

+		ret = drm_add_edid_modes(connector, drm_edid);
+	}
+
+	return ret;
+}

drivers/gpu/drm/msm/edp/edp_connector.c +21 -0

+
+/* initialize connector */
+struct drm_connector *msm_edp_connector_init(struct msm_edp *edp)
+{
+	struct drm_connector *connector = NULL;
+	struct edp_connector *edp_connector;
+	int ret;
+
+	edp_connector = kzalloc(sizeof(*edp_connector), GFP_KERNEL);
+	if (!edp_connector) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	edp_connector->edp = edp;
+
+	connector = &edp_connector->base;
+
+	ret = drm_connector_init(edp->dev, connector, &edp_connector_funcs,
+			DRM_MODE_CONNECTOR_eDP);
+	if (ret)

drivers/gpu/drm/msm/edp/edp_ctrl.c +0 -0

+#define RGB_COMPONENTS 3

drivers/gpu/drm/msm/edp/edp_ctrl.c +11 -0

+struct edp_ctrl {
+	struct platform_device *pdev;
+
+	void __iomem *base;
+
+	/* regulators */
+	struct regulator *vdda_vreg;
+	struct regulator *lvl_reg;
+
+	/* clocks */
+	struct clk *aux_clk;

drivers/gpu/drm/msm/edp/edp_ctrl.c +19 -0

+struct edp_pixel_clk_div {
+	u32 rate; /* in kHz */
+	u32 m;
+	u32 n;
+};
+
+#define EDP_PIXEL_CLK_NUM 8
+static const struct edp_pixel_clk_div clk_divs[2][EDP_PIXEL_CLK_NUM] = {
+	{ /* Link clock = 162MHz, source clock = 810MHz */
+		{119000, 31,  211}, /* WSXGA+ 1680x1050@60Hz CVT */
+		{130250, 32,  199}, /* UXGA 1600x1200@60Hz CVT */
+		{148500, 11,  60},  /* FHD 1920x1080@60Hz */
+		{154000, 50,  263}, /* WUXGA 1920x1200@60Hz CVT */
+		{209250, 31,  120}, /* QXGA 2048x1536@60Hz CVT */
+		{268500, 119, 359}, /* WQXGA 2560x1600@60Hz CVT */
+		{138530, 33,  193}, /* AUO B116HAN03.0 Panel */
+		{141400, 48,  275}, /* AUO B133HTN01.2 Panel */
+	},
+	{ /* Link clock = 270MHz, source clock = 675MHz */

drivers/gpu/drm/msm/edp/edp_ctrl.c +5 -0

+static void edp_clk_deinit(struct edp_ctrl *ctrl)
+{
+	struct device *dev = &ctrl->pdev->dev;
+
+	if (ctrl->aux_clk)

drivers/gpu/drm/msm/edp/edp_ctrl.c +33 -0

+static int edp_regulator_init(struct edp_ctrl *ctrl)
+{
+	struct device *dev = &ctrl->pdev->dev;
+	int ret;
+
+	DBG("");
+	ctrl->vdda_vreg = devm_regulator_get(dev, "vdda");
+	if (IS_ERR(ctrl->vdda_vreg)) {
+		pr_err("%s: Could not get vdda reg, ret = %ld\n", __func__,
+				PTR_ERR(ctrl->vdda_vreg));
+		ctrl->vdda_vreg = NULL;
+		ret = -ENODEV;
+		goto f0;
+	}
+
+	ret = regulator_set_voltage(ctrl->vdda_vreg, VDDA_MIN_UV, VDDA_MAX_UV);
+	if (ret) {
+		pr_err("%s:vdda_vreg set_voltage failed, %d\n", __func__, ret);
+		goto f1;
+	}
+
+	ret = regulator_set_optimum_mode(ctrl->vdda_vreg, VDDA_UA_ON_LOAD);
+	if (ret < 0) {
+		pr_err("%s: vdda_vreg set regulator mode failed.\n", __func__);
+		goto f1;
+	}
+
+	ret = regulator_enable(ctrl->vdda_vreg);
+	if (ret) {
+		pr_err("%s: Failed to enable vdda_vreg regulator.\n", __func__);
+		goto f2;
+	}
+

+/* The power source of the level translation chip is different on different
+ * target boards, i.e. a gpio or a regulator.
+ */

drivers/gpu/drm/msm/edp/edp_ctrl.c +6 -0

+static int edp_sink_power_state(struct edp_ctrl *ctrl, u8 state)
+{
+	u8 s = state;
+
+	DBG("%d", s);
+

drivers/gpu/drm/msm/edp/edp_ctrl.c +4 -0

+static void edp_host_train_set(struct edp_ctrl *ctrl, u32 train)
+{
+	int cnt;
+	u32 data;

drivers/gpu/drm/msm/edp/edp_ctrl.c +2 -0

+static void edp_ctrl_on_worker(struct work_struct *work)
+{

+}

drivers/gpu/drm/msm/edp/edp_ctrl.c +3 -0

+
+irqreturn_t msm_edp_ctrl_irq(struct edp_ctrl *ctrl)
+{

drivers/gpu/drm/msm/edp/edp_ctrl.c +8 -0

+		DBG("idle_patterns_sent");
+		complete(&ctrl->idle_comp);
+	}
+
+	msm_edp_aux_irq(ctrl->aux, isr1);
+
+	return IRQ_HANDLED;
+}

drivers/gpu/drm/msm/edp/edp_ctrl.c +10 -0

+	ret = ctrl->edp_connected;
+	mutex_unlock(&ctrl->dev_mutex);
+
+	return ret;
+}
+
+int msm_edp_ctrl_get_edid(struct edp_ctrl *ctrl,
+		struct drm_connector *connector, struct edid **edid)
+{
+	int ret = 0;

drivers/gpu/drm/msm/edp/edp_ctrl.c +5 -0

+
+	if (!ctrl->power_on) {
+		if (!ctrl->cont_splash)
+			edp_ctrl_phy_aux_enable(ctrl, 1);
+		edp_ctrl_irq_enable(ctrl, 1);

drivers/gpu/drm/msm/edp/edp_ctrl.c +5 -0

+{
+	u32 hstart_from_sync, vstart_from_sync;
+	u32 data;
+	int ret = 0;
+

drivers/gpu/drm/msm/edp/edp_phy.c +0 -0

drivers/gpu/drm/msm/edp/edp_phy.c +11 -0

+		pr_err("%s: PHY NOT ready\n", __func__);
+		return false;
+	} else {
+		return true;
+	}
+}
+
+void msm_edp_phy_ctrl(struct edp_phy *phy, int enable)
+{
+	DBG("enable=%d", enable);
+	if (enable) {

+void msm_edp_phy_lane_power_ctrl(struct edp_phy *phy, int up, int max_lane)

On Fri, Dec 05, 2014 at 04:30:00PM -0500, Hai Li wrote:
[...]

drivers/gpu/drm/msm/edp/edp.c line 93

Quoted 9 lines

drivers/gpu/drm/msm/edp/edp.c new file +9 -0

+     if (IS_ERR(edp))
+             return PTR_ERR(edp);
+     priv->edp = edp;
+
+     return 0;
+}
+
+static void edp_unbind(struct device *dev, struct device *master,
+             void *data)

We typically align parameters on subsequent lines with the first parameter on the first line. But perhaps Rob doesn't care so much.

Quoted 22 lines

drivers/gpu/drm/msm/edp/edp.c new file +22 -0

+/* Second part of initialization, the drm/kms level modeset_init */
+int msm_edp_modeset_init(struct msm_edp *edp,
+             struct drm_device *dev, struct drm_encoder *encoder)
+{
+     struct platform_device *pdev = edp->pdev;
+     struct msm_drm_private *priv = dev->dev_private;
+     int ret;
+
+     edp->encoder = encoder;
+     edp->dev = dev;
+
+     edp->bridge = msm_edp_bridge_init(edp);
+     if (IS_ERR(edp->bridge)) {
+             ret = PTR_ERR(edp->bridge);
+             dev_err(dev->dev, "failed to create eDP bridge: %d\n", ret);
+             edp->bridge = NULL;
+             goto fail;
+     }
+
+     edp->connector = msm_edp_connector_init(edp);
+     if (IS_ERR(edp->connector)) {
+             ret = PTR_ERR(edp->connector);

drivers/gpu/drm/msm/edp/edp.c line 176

Quoted 6 lines

drivers/gpu/drm/msm/edp/edp.c new file +6 -0

+             dev_err(dev->dev, "failed to create eDP connector: %d\n", ret);
+             edp->connector = NULL;
+             goto fail;
+     }
+
+     edp->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);

Why not use the more idiomatic platform_get_irq()?

drivers/gpu/drm/msm/edp/edp.c line 179

Quoted 3 lines

drivers/gpu/drm/msm/edp/edp.c new file +3 -0

+     if (edp->irq < 0) {
+             ret = edp->irq;
+             dev_err(dev->dev, "failed to get irq: %d\n", ret);

s/irq/IRQ/

Quoted 1 line

drivers/gpu/drm/msm/edp/edp.h +0 -0

[...]

drivers/gpu/drm/msm/edp/edp.h line 22

Quoted 9 lines

drivers/gpu/drm/msm/edp/edp.h +9 -0

+#ifndef __EDP_CONNECTOR_H__
+#define __EDP_CONNECTOR_H__
+
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of_gpio.h>
+#include <linux/pwm.h>
+#include <linux/i2c.h>

These should be alphabetically sorted.

Quoted 1 line

drivers/gpu/drm/msm/edp/edp_aux.c +0 -0

[...]

Quoted 1 line

+#define to_edp_aux(x) container_of(x, struct edp_aux, drm_aux)

Perhaps this should be a static inline function for better type safety.

Quoted 14 lines

drivers/gpu/drm/msm/edp/edp_aux.c +13 -0

+static int edp_sink_power_state(struct edp_ctrl *ctrl, u8 state)
+{
+     u8 s = state;
+
+     DBG("%d", s);
+
+     if (ctrl->dp_link.revision < 0x11)
+             return 0;
+
+     if (drm_dp_dpcd_write(ctrl->drm_aux, DP_SET_POWER, &s, 1) < 1) {
+             pr_err("%s: Set power state to panel failed\n", __func__);
+             return -ENOLINK;
+     }

drivers/gpu/drm/msm/edp/edp_aux.c

Quoted 3 lines

drivers/gpu/drm/msm/edp/edp_aux.c +3 -0

+
+     return 0;
+}

This is essentially drm_dp_link_power_up()/drm_dp_link_power_down(). Please use common code where available. And if it's not available yet the code is completely generic, please add a core function so that other drivers can reuse it.

Quoted 2 lines

drivers/gpu/drm/msm/edp/edp_aux.c +2 -0

+irqreturn_t msm_edp_ctrl_irq(struct edp_ctrl *ctrl)
+{

[...]

drivers/gpu/drm/msm/edp/edp_aux.c

Quoted 2 lines

drivers/gpu/drm/msm/edp/edp_aux.c +2 -0

+     if (isr1 & EDP_INTERRUPT_REG_1_HPD)
+             DBG("edp_hpd");

Don't you want to handle this?

drivers/gpu/drm/msm/edp/edp_aux.c

Quoted 5 lines

drivers/gpu/drm/msm/edp/edp_aux.c +5 -0

+
+     if (isr2 & EDP_INTERRUPT_REG_2_READY_FOR_VIDEO)
+             DBG("edp_video_ready");
+
+     if (isr2 & EDP_INTERRUPT_REG_2_IDLE_PATTERNs_SENT) {

s/PATTERNs/PATTERNS/? I was going to make that comment to the definition of this define, but I can't seem to find it. I suspect that it comes from one of the generated headers, but I can't seem to find either the generated header nor the XML.

Quoted 9 lines

drivers/gpu/drm/msm/edp/edp_aux.c +8 -0

+             DBG("idle_patterns_sent");
+             complete(&ctrl->idle_comp);
+     }
+
+     msm_edp_aux_irq(ctrl->aux, isr1);
+
+     return IRQ_HANDLED;
+}

[...]

drivers/gpu/drm/msm/edp/edp_aux.c

Quoted 3 lines

drivers/gpu/drm/msm/edp/edp_aux.c +3 -0

+bool msm_edp_ctrl_panel_connected(struct edp_ctrl *ctrl)
+{
+     bool ret;

This is unnecessary, the only place where this is used is to return the value of ctrl->edp_connected. You can use that directly instead.

[...]

Quoted 14 lines

drivers/gpu/drm/msm/edp/edp_aux.c +14 -0

+     ret = ctrl->edp_connected;
+     mutex_unlock(&ctrl->dev_mutex);
+
+     return ret;
+}
+
+int msm_edp_ctrl_get_edid(struct edp_ctrl *ctrl,
+             struct drm_connector *connector, struct edid **edid)
+{
+     int ret = 0;
+
+     mutex_lock(&ctrl->dev_mutex);
+
+     if (ctrl->edid) {

drivers/gpu/drm/msm/edp/edp_aux.c

Quoted 6 lines

drivers/gpu/drm/msm/edp/edp_aux.c +6 -0

+             if (edid) {
+                     DBG("Just return edid buffer");
+                     *edid = ctrl->edid;
+             }
+             goto unlock_ret;
+     }

Is there a way to invalidate an existing EDID?

drivers/gpu/drm/msm/edp/edp.c line 84

Quoted 10 lines

+static int edp_bind(struct device *dev, struct device *master, void *data)

drivers/gpu/drm/msm/edp/edp.c new file +7 -0

+{
+	struct drm_device *drm = dev_get_drvdata(master);
+	struct msm_drm_private *priv = drm->dev_private;
+	struct msm_edp *edp;
+
+	DBG("");
+	edp = edp_init(to_platform_device(dev));

There's a lot of this casting to platform devices and then using pdev->dev to get at the struct device. I don't immediately see a use for the platform device, so why not just stick with struct device * consistently?

Quoted 13 lines

+ * It will call msm_edp_aux_ctrl() function to reset the aux channel,
+ * if the waiting is timeout.
+ * The caller who triggers the transaction should avoid the
+ * msm_edp_aux_ctrl() running concurrently in other threads, i.e.
+ * start transaction only when aux channel is fully enabled.
+ */
+ssize_t edp_aux_transfer(struct drm_dp_aux *drm_aux, struct drm_dp_aux_msg *msg)

drivers/gpu/drm/msm/edp/edp_aux.c new file +4 -0

+{
+	struct edp_aux *aux = to_edp_aux(drm_aux);
+	ssize_t ret;
+	bool native = msg->request & (DP_AUX_NATIVE_WRITE &

DP_AUX_NATIVE_READ);

Review

Quoted 1 line

(quoted diff) +1 -0

+	bool read = msg->request & (DP_AUX_I2C_READ & DP_AUX_NATIVE_READ);

These checks are confusing. It seems like they might actually work because of how these symbols are defined, but I'd expect something like:

	native = msg->request & (DP_AUX_NATIVE_WRITE | DP_AUX_NATIVE_READ);

Or perhaps even clearer:

	switch (msg->request) {
	case DP_AUX_NATIVE_WRITE:
	case DP_AUX_NATIVE_READ:
		native = true;
		break;

	...
	}

From my understanding, the idea to define these symbols is to take BIT(7)

drivers/gpu/drm/msm/edp/edp_aux.c line 156

Quoted 6 lines

drivers/gpu/drm/msm/edp/edp_aux.c new file +6 -0

+	/* Ignore address only message */
+	if ((msg->size == 0) || (msg->buffer == NULL)) {
+		msg->reply = native ?
+			DP_AUX_NATIVE_REPLY_ACK : DP_AUX_I2C_REPLY_ACK;
+		return msg->size;
+	}

How do you support I2C-over-AUX then? How else will the device know which I2C slave to address?

Quoted 6 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +5 -0

+static int cont_splash;	/* 1 to enable continuous splash screen */
+EXPORT_SYMBOL(cont_splash);
+
+module_param(cont_splash, int, 0);
+MODULE_PARM_DESC(cont_splash, "Enable continuous splash screen on

eDP");

Huh? Is this supposed to allow hand-off from firmware to kernel? If so I don't think that's going to work without having proper support for it across the driver. I don't see support for this in the MDP subdriver, so I doubt that it's going to work at all.

Either way, I don't think using a module parameter for this is the right solution.

Quoted 17 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +17 -0

+struct edp_ctrl {
+	struct platform_device *pdev;
+
+	void __iomem *base;
+
+	/* regulators */
+	struct regulator *vdda_vreg;
+	struct regulator *lvl_reg;
+
+	/* clocks */
+	struct clk *aux_clk;
+	struct clk *pixel_clk;
+	struct clk *ahb_clk;
+	struct clk *link_clk;
+	struct clk *mdp_core_clk;
+
+	/* gpios */

drivers/gpu/drm/msm/edp/edp_ctrl.c line 91

Quoted 4 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +4 -0

+	int gpio_panel_en;
+	int gpio_panel_hpd;
+	int gpio_lvl_en;
+	int gpio_bkl_en;

These should really be using the new gpiod_*() API. Also, at least panel_en and bkl_en seem wrongly placed. They should be handled in the panel and backlight drivers, not the eDP driver.

Quoted 19 lines

+static const struct edp_pixel_clk_div clk_divs[2][EDP_PIXEL_CLK_NUM] = {

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +17 -0

+	{ /* Link clock = 162MHz, source clock = 810MHz */
+		{119000, 31,  211}, /* WSXGA+ 1680x1050@60Hz CVT */
+		{130250, 32,  199}, /* UXGA 1600x1200@60Hz CVT */
+		{148500, 11,  60},  /* FHD 1920x1080@60Hz */
+		{154000, 50,  263}, /* WUXGA 1920x1200@60Hz CVT */
+		{209250, 31,  120}, /* QXGA 2048x1536@60Hz CVT */
+		{268500, 119, 359}, /* WQXGA 2560x1600@60Hz CVT */
+		{138530, 33,  193}, /* AUO B116HAN03.0 Panel */
+		{141400, 48,  275}, /* AUO B133HTN01.2 Panel */
+	},
+	{ /* Link clock = 270MHz, source clock = 675MHz */
+		{119000, 52,  295}, /* WSXGA+ 1680x1050@60Hz CVT */
+		{130250, 11,  57},  /* UXGA 1600x1200@60Hz CVT */
+		{148500, 11,  50},  /* FHD 1920x1080@60Hz */
+		{154000, 47,  206}, /* WUXGA 1920x1200@60Hz CVT */
+		{209250, 31,  100}, /* QXGA 2048x1536@60Hz CVT */
+		{268500, 107, 269}, /* WQXGA 2560x1600@60Hz CVT */

drivers/gpu/drm/msm/edp/edp_ctrl.c line 170

Quoted 4 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +4 -0

+		{138530, 63,  307}, /* AUO B116HAN03.0 Panel */
+		{141400, 53,  253}, /* AUO B133HTN01.2 Panel */
+	},
+};

Can't you compute these programmatically? If you rely on this table you'll need to extend it everytime you want to support a new panel or resolution.

Quoted 2 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +2 -0

+static void edp_ctrl_on_worker(struct work_struct *work)
+{

[...]

Quoted 6 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +6 -0

+}
+
+static void edp_ctrl_off_worker(struct work_struct *work)
+{
+	struct edp_ctrl *ctrl = container_of(
+				work, struct edp_ctrl, off_work);

Quoted 1 line

+}

Why are these two functions workers?

Quoted 3 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +3 -0

+
+irqreturn_t msm_edp_ctrl_irq(struct edp_ctrl *ctrl)
+{

[...]

drivers/gpu/drm/msm/edp/edp_ctrl.c line 1449

Quoted 2 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +2 -0

+	if (isr1 & EDP_INTERRUPT_REG_1_HPD)
+		DBG("edp_hpd");

Don't you want to handle this?

Quoted 10 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +10 -0

+
+	if (!ctrl->power_on) {
+		if (!ctrl->cont_splash)
+			edp_ctrl_phy_aux_enable(ctrl, 1);
+		edp_ctrl_irq_enable(ctrl, 1);
+	}
+
+	ret = drm_dp_link_probe(ctrl->drm_aux, &ctrl->dp_link);
+	if (ret) {
+		pr_err("%s: read dpcd cap failed, %d\n", __func__, ret);

drivers/gpu/drm/msm/edp/edp_ctrl.c line 1698

Quoted 5 lines

drivers/gpu/drm/msm/edp/edp_ctrl.c new file +5 -0

+		goto disable_ret;
+	}
+
+	/* Initialize link rate as panel max link rate */
+	ctrl->link_rate = drm_dp_link_rate_to_bw_code(ctrl->dp_link.rate);

There's a lot of code here that should probably be a separate function rather than be called as part of retrieving the EDID.

Quoted 2 lines

drivers/gpu/drm/msm/edp/edp_phy.c +0 -0

[...]

drivers/gpu/drm/msm/edp/edp_phy.c line 32

Quoted 10 lines

drivers/gpu/drm/msm/edp/edp_phy.c +10 -0

+bool msm_edp_phy_ready(struct edp_phy *phy)
+{
+	u32 status;
+	int cnt;
+
+	cnt = 100;
+	while (--cnt) {
+		status = edp_read(phy->base +
+				REG_EDP_PHY_GLB_PHY_STATUS);
+		if (status & 0x01)

Can you add a define for 0x01?

Quoted 17 lines

drivers/gpu/drm/msm/edp/edp_phy.c +17 -0

+		pr_err("%s: PHY NOT ready\n", __func__);
+		return false;
+	} else {
+		return true;
+	}
+}
+
+void msm_edp_phy_ctrl(struct edp_phy *phy, int enable)
+{
+	DBG("enable=%d", enable);
+	if (enable) {
+		/* Reset */
+		edp_write(phy->base + REG_EDP_PHY_CTRL, 0x005); /* bit 0, 2 */
+		wmb();
+		usleep_range(500, 1000);
+		edp_write(phy->base + REG_EDP_PHY_CTRL, 0x000);
+		edp_write(phy->base + REG_EDP_PHY_GLB_PD_CTL, 0x3f);

drivers/gpu/drm/msm/edp/edp_phy.c line 58

Quoted 4 lines

drivers/gpu/drm/msm/edp/edp_phy.c +4 -0

+		edp_write(phy->base + REG_EDP_PHY_GLB_CFG, 0x1);
+	} else {
+		edp_write(phy->base + REG_EDP_PHY_GLB_PD_CTL, 0xc0);
+	}

Please, also add defines for the values here. It's impossible to tell from the code what this does or what might need fixing if there was a bug.