drm/bridge: cdns-mhdp: Probe and add the bridge even without firmware

Load the firmware asynchronously and probe and add the bridge even we
do not yet have it. This makes it possible to pull up the DRM device
and get the other displays working even if we still have to wait for
display port firmware. The display port is locked in disconnected
state until the firmware is loaded and the DP IP is properly enabled.

The patch adds one lock to protect hw_state and bridge_attached in
the driver data struct from from a race between the DRM operations and
the asynchronous FW loading.

Signed-off-by: Jyri Sarha <jsarha@ti.com>

drivers/gpu/drm/bridge/cdns-mhdp.c

@@ -577,10 +577,147 @@ static u8 eq_training_pattern_supported(struct cdns_mhdp_host host,
 	return fls(host.pattern_supp & sink.pattern_supp);
 }
 
+static int mhdp_fw_activate(const struct firmware *fw,
+			    struct cdns_mhdp_device *mhdp)
+{
+	unsigned int reg;
+	int ret = 0;
+
+	dev_dbg(mhdp->dev, "%s\n", __func__);
+
+	if (!fw || !fw->data) {
+		dev_err(mhdp->dev, "%s: No firmware.\n", __func__);
+		return -EINVAL;
+	}
+
+	spin_lock(&mhdp->start_lock);
+	if (mhdp->hw_state != MHDP_HW_INACTIVE) {
+		spin_unlock(&mhdp->start_lock);
+		if (mhdp->hw_state != MHDP_HW_STOPPED)
+			dev_err(mhdp->dev, "%s: Bad HW state: %d\n",
+				__func__, mhdp->hw_state);
+		return -EBUSY;
+	}
+	mhdp->hw_state = MHDP_HW_LOADING;
+	spin_unlock(&mhdp->start_lock);
+
+	/* Release uCPU reset and stall it. */
+	writel(CDNS_CPU_STALL, mhdp->regs + CDNS_APB_CTRL);
+
+	memcpy_toio(mhdp->regs + CDNS_MHDP_IMEM, fw->data, fw->size);
+
+	/* Leave debug mode, release stall */
+	writel(0, mhdp->regs + CDNS_APB_CTRL);
+
+	/*
+	 * Wait for the KEEP_ALIVE "message" on the first 8 bits.
+	 * Updated each sched "tick" (~2ms)
+	 */
+	ret = readl_poll_timeout(mhdp->regs + CDNS_KEEP_ALIVE, reg,
+				 reg & CDNS_KEEP_ALIVE_MASK, 500,
+				 CDNS_KEEP_ALIVE_TIMEOUT);
+	if (ret) {
+		dev_err(mhdp->dev,
+			"device didn't give any life sign: reg %d\n", reg);
+		goto error;
+	}
+
+	/* Init events to 0 as it's not cleared by FW at boot but on read */
+	readl(mhdp->regs + CDNS_SW_EVENT0);
+	readl(mhdp->regs + CDNS_SW_EVENT1);
+	readl(mhdp->regs + CDNS_SW_EVENT2);
+	readl(mhdp->regs + CDNS_SW_EVENT3);
+
+	/* Activate uCPU */
+	ret = cdns_mhdp_set_firmware_active(mhdp, true);
+	if (ret) {
+		dev_err(mhdp->dev, "%s: Failed to activate FW: %d\n",
+			__func__, ret);
+		goto error;
+	}
+
+	spin_lock(&mhdp->start_lock);
+
+	mhdp->hw_state = MHDP_HW_READY;
+
+	/*
+	 * Here we must keep the lock while enabling the interrupts
+	 * since it would otherwise be possible that interrupt enable
+	 * code is executed after the bridge is detached. The similar
+	 * situation is not possible in attach()/detach() callbacks
+	 * since the hw_state changes from MHDP_HW_READY to
+	 * MHDP_HW_STOPPED happens only due to driver removal when
+	 * bridge should already be detached.
+	 */
+	if (mhdp->bridge_attached) {
+		/* enable interrupts */
+		writel(0, mhdp->regs + CDNS_APB_INT_MASK);
+		writel(0, mhdp->regs + CDNS_MB_INT_MASK);
+	}
+
+	spin_unlock(&mhdp->start_lock);
+
+	dev_dbg(mhdp->dev, "DP FW activated\n");
+
+	return 0;
+error:
+	spin_lock(&mhdp->start_lock);
+	mhdp->hw_state = MHDP_HW_INACTIVE;
+	spin_unlock(&mhdp->start_lock);
+
+	return ret;
+}
+
+static void mhdp_fw_cb(const struct firmware *fw, void *context)
+{
+	struct cdns_mhdp_device *mhdp = context;
+	bool bridge_attached;
+	int ret;
+
+	dev_dbg(mhdp->dev, "firmware callback\n");
+
+	ret = mhdp_fw_activate(fw, mhdp);
+
+	release_firmware(fw);
+
+	if (ret)
+		return;
+
+	/*
+	 *  XXX how to make sure the bridge is still attached when
+	 *      calling drm_kms_helper_hotplug_event() after releasing
+	 *      the lock? We should not hold the spin lock when
+	 *      calling drm_kms_helper_hotplug_event() since it may
+	 *      cause a dead lock. FB-dev console calls detect from the
+	 *      same thread just down the call stack started here.
+	 */
+	spin_lock(&mhdp->start_lock);
+	bridge_attached = mhdp->bridge_attached;
+	spin_unlock(&mhdp->start_lock);
+	if (bridge_attached)
+		drm_kms_helper_hotplug_event(mhdp->bridge.dev);
+}
+
+static int load_firmware(struct cdns_mhdp_device *mhdp)
+{
+	int ret;
+
+	ret = request_firmware_nowait(THIS_MODULE, true, FW_NAME, mhdp->dev,
+				      GFP_KERNEL, mhdp, mhdp_fw_cb);
+	if (ret) {
+		dev_err(mhdp->dev, "failed to load firmware (%s), ret: %d\n",
+			FW_NAME, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static irqreturn_t mhdp_irq_handler(int irq, void *data)
 {
 	struct cdns_mhdp_device *mhdp = (struct cdns_mhdp_device *)data;
 	u32 mbox_stat, apb_stat, sw_ev0, sw_ev1, sw_ev2, sw_ev3;
+	bool bridge_attached;
 
 	apb_stat = readl(mhdp->regs + CDNS_APB_INT_STATUS);
 	mbox_stat = readl(mhdp->regs + CDNS_MB_INT_STATUS);
@@ -591,7 +728,17 @@ static irqreturn_t mhdp_irq_handler(int irq, void *data)
 
 	//dev_dbg(mhdp->dev, "MHDP IRQ apb %x, mbox %x, sw_ev %x/%x/%x/%x\n", apb_stat, mbox_stat, sw_ev0, sw_ev1, sw_ev2, sw_ev3);
 
-	if (sw_ev0 & CDNS_DPTX_HPD)
+	/*
+	 *  Calling drm_kms_helper_hotplug_event() when not attached
+	 *  to drm device causes an oops because the drm_bridge->dev
+	 *  is NULL. See mhdp_fw_cb() comments for details about the
+	 *  problems related drm_kms_helper_hotplug_event() call.
+	 */
+	spin_lock(&mhdp->start_lock);
+	bridge_attached = mhdp->bridge_attached;
+	spin_unlock(&mhdp->start_lock);
+
+	if (bridge_attached && (sw_ev0 & CDNS_DPTX_HPD))
 		drm_kms_helper_hotplug_event(mhdp->bridge.dev);
 
 	return IRQ_HANDLED;
@@ -674,8 +821,20 @@ static int cdns_mhdp_detect(struct drm_connector *conn,
 			    bool force)
 {
 	struct cdns_mhdp_device *mhdp = connector_to_mhdp(conn);
+	bool hw_ready;
 	int ret;
 
+	dev_dbg(mhdp->dev, "%s\n", __func__);
+
+	spin_lock(&mhdp->start_lock);
+
+	hw_ready = mhdp->hw_state == MHDP_HW_READY;
+
+	spin_unlock(&mhdp->start_lock);
+
+	if (!hw_ready || WARN_ON(!mhdp->bridge_attached))
+		return connector_status_disconnected;
+
 	ret = cdns_mhdp_get_hpd_status(mhdp);
 	if (ret > 0) {
 		mhdp->plugged = true;
@@ -707,8 +866,11 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge)
 	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
 	u32 bus_format = MEDIA_BUS_FMT_RGB121212_1X36;
 	struct drm_connector *conn = &mhdp->connector;
+	bool hw_ready;
 	int ret;
 
+	dev_dbg(mhdp->dev, "%s\n", __func__);
+
 	if (&mhdp->bridge != bridge)
 		return -ENODEV;
 
@@ -743,10 +905,20 @@ static int cdns_mhdp_attach(struct drm_bridge *bridge)
 		return ret;
 	}
 
-	/* enable interrupts */
+	spin_lock(&mhdp->start_lock);
+
+	mhdp->bridge_attached = true;
+	hw_ready = mhdp->hw_state == MHDP_HW_READY;
+
+	spin_unlock(&mhdp->start_lock);
+
+	if (hw_ready) {
+		/* enable interrupts */
+		writel(0, mhdp->regs + CDNS_APB_INT_MASK);
+		writel(0, mhdp->regs + CDNS_MB_INT_MASK);
+	}
+
 	//writel(~CDNS_APB_INT_MASK_SW_EVENT_INT, mhdp->regs + CDNS_APB_INT_MASK);
-	writel(0, mhdp->regs + CDNS_APB_INT_MASK);
-	writel(0, mhdp->regs + CDNS_MB_INT_MASK);
 
 	return 0;
 }
@@ -1209,7 +1381,7 @@ static void cdns_mhdp_disable(struct drm_bridge *bridge)
 	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
 	u32 resp;
 
-	dev_dbg(mhdp->dev, "bridge disable\n");
+	dev_dbg(mhdp->dev, "%s\n", __func__);
 
 	cdns_mhdp_reg_read(mhdp, CDNS_DP_FRAMER_GLOBAL_CONFIG, &resp);
 	resp &= ~CDNS_DP_FRAMER_EN;
@@ -1588,6 +1760,14 @@ static void cdns_mhdp_detach(struct drm_bridge *bridge)
 {
 	struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge);
 
+	dev_dbg(mhdp->dev, "%s\n", __func__);
+
+	spin_lock(&mhdp->start_lock);
+
+	mhdp->bridge_attached = false;
+
+	spin_unlock(&mhdp->start_lock);
+
 	writel(~0, mhdp->regs + CDNS_APB_INT_MASK);
 	writel(~0, mhdp->regs + CDNS_MB_INT_MASK);
 }
@@ -1599,33 +1779,12 @@ static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = {
 	.detach = cdns_mhdp_detach,
 };
 
-static int load_firmware(struct cdns_mhdp_device *mhdp, const char *name,
-			 unsigned int addr)
-{
-	const struct firmware *fw;
-	int ret;
-
-	ret = request_firmware(&fw, name, mhdp->dev);
-	if (ret) {
-		dev_err(mhdp->dev, "failed to load firmware (%s), ret: %d\n",
-			name, ret);
-		return ret;
-	}
-
-	memcpy_toio(mhdp->regs + addr, fw->data, fw->size);
-
-	release_firmware(fw);
-
-	return 0;
-}
-
 static int mhdp_probe(struct platform_device *pdev)
 {
 	struct resource *regs;
 	struct cdns_mhdp_device *mhdp;
 	struct clk *clk;
 	int ret;
-	unsigned int reg;
 	unsigned long rate;
 	int irq;
 	u32 lanes_prop;
@@ -1643,6 +1802,7 @@ static int mhdp_probe(struct platform_device *pdev)
 
 	mhdp->clk = clk;
 	mhdp->dev = &pdev->dev;
+	spin_lock_init(&mhdp->start_lock);
 	dev_set_drvdata(&pdev->dev, mhdp);
 
 	drm_dp_aux_init(&mhdp->aux);
@@ -1679,22 +1839,12 @@ static int mhdp_probe(struct platform_device *pdev)
 		goto runtime_put;
 	}
 
-	/* Release uCPU reset and stall it. */
-	writel(CDNS_CPU_STALL, mhdp->regs + CDNS_APB_CTRL);
-
-	ret = load_firmware(mhdp, FW_NAME, CDNS_MHDP_IMEM);
-	if (ret)
-		return ret;
-
 	rate = clk_get_rate(clk);
 	writel(rate % 1000000, mhdp->regs + CDNS_SW_CLK_L);
 	writel(rate / 1000000, mhdp->regs + CDNS_SW_CLK_H);
 
 	dev_dbg(&pdev->dev, "func clk rate %lu Hz\n", rate);
 
-	/* Leave debug mode, release stall */
-	writel(0, mhdp->regs + CDNS_APB_CTRL);
-
 	writel(~0, mhdp->regs + CDNS_MB_INT_MASK);
 	writel(~0, mhdp->regs + CDNS_APB_INT_MASK);
 
@@ -1708,19 +1858,6 @@ static int mhdp_probe(struct platform_device *pdev)
 		goto runtime_put;
 	}
 
-	/*
-	 * Wait for the KEEP_ALIVE "message" on the first 8 bits.
-	 * Updated each sched "tick" (~2ms)
-	 */
-	ret = readl_poll_timeout(mhdp->regs + CDNS_KEEP_ALIVE, reg,
-				 reg & CDNS_KEEP_ALIVE_MASK, 500,
-				 CDNS_KEEP_ALIVE_TIMEOUT);
-	if (ret) {
-		dev_err(&pdev->dev,
-			"device didn't give any life sign: reg %d\n", reg);
-		return -EIO;
-	}
-
 	/* Read source capabilities, based on PHY's device tree properties. */
 	ret = device_property_read_u32(&(mhdp->phy->dev), "num_lanes",
 				       &(lanes_prop));
@@ -1756,19 +1893,6 @@ static int mhdp_probe(struct platform_device *pdev)
 	mhdp->bridge.of_node = pdev->dev.of_node;
 	mhdp->bridge.funcs = &cdns_mhdp_bridge_funcs;
 
-	/* Init events to 0 as it's not cleared by FW at boot but on read */
-	readl(mhdp->regs + CDNS_SW_EVENT0);
-	readl(mhdp->regs + CDNS_SW_EVENT1);
-	readl(mhdp->regs + CDNS_SW_EVENT2);
-	readl(mhdp->regs + CDNS_SW_EVENT3);
-
-	/* Activate uCPU */
-	ret = cdns_mhdp_set_firmware_active(mhdp, true);
-	if (ret) {
-		dev_err(mhdp->dev, "Failed to activate DP\n");
-		return ret;
-	}
-
 	ret = phy_init(mhdp->phy);
 	if (ret) {
 		dev_err(mhdp->dev, "Failed to initialize PHY: %d\n", ret);
@@ -1777,8 +1901,14 @@ static int mhdp_probe(struct platform_device *pdev)
 
 	drm_bridge_add(&mhdp->bridge);
 
+	ret = load_firmware(mhdp);
+	if (ret)
+		goto phy_exit;
+
 	return 0;
 
+phy_exit:
+	phy_exit(mhdp->phy);
 runtime_put:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
@@ -1791,14 +1921,34 @@ MODULE_FIRMWARE(FW_NAME);
 static int mhdp_remove(struct platform_device *pdev)
 {
 	struct cdns_mhdp_device *mhdp = dev_get_drvdata(&pdev->dev);
-	int ret;
+	unsigned int timeout = 10;
+	bool stop_fw = false;
+	int ret = 0;
 
 	drm_bridge_remove(&mhdp->bridge);
 
-	ret = cdns_mhdp_set_firmware_active(mhdp, false);
-	if (ret) {
-		dev_err(mhdp->dev, "Failed to de-activate DP\n");
-		return ret;
+wait_loading:
+	spin_lock(&mhdp->start_lock);
+	if (mhdp->hw_state == MHDP_HW_LOADING && timeout-- > 0) {
+		spin_unlock(&mhdp->start_lock);
+		msleep(100);
+		goto wait_loading;
+	} else if (mhdp->hw_state == MHDP_HW_READY) {
+		stop_fw = true;
+		timeout = 1; /* We were succesful even if counter reached 0 */
+	}
+	mhdp->hw_state = MHDP_HW_STOPPED;
+	spin_unlock(&mhdp->start_lock);
+
+	if (timeout == 0)
+		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
+			__func__);
+
+	if (stop_fw) {
+		ret = cdns_mhdp_set_firmware_active(mhdp, false);
+		if (ret)
+			dev_err(mhdp->dev, "%s: De-activate FW failed: %d\n",
+				__func__, ret);
 	}
 
 	phy_exit(mhdp->phy);
@@ -1810,7 +1960,7 @@ static int mhdp_remove(struct platform_device *pdev)
 
 	/* FIXME: check for missing functions */
 
-	return 0;
+	return ret;
 }
 
 static struct platform_driver mhdp_driver = {

drivers/gpu/drm/bridge/cdns-mhdp.h

@@ -212,6 +212,18 @@ struct cdns_mhdp_display_fmt {
 	u8 y_only : 1;
 };
 
+/*
+ * These enums present MHDP hw initialization state
+ * Legal state transitions are:
+ * MHDP_HW_INACTIVE <-> MHDP_HW_LOADING -> MHDP_HW_READY
+ *        |                                     |
+ *        '----------> MHDP_HW_STOPPED <--------'
+ */
+enum mhdp_hw_state { MHDP_HW_INACTIVE = 0, /* HW not initialized */
+		     MHDP_HW_LOADING,	   /* HW initialization in progress */
+		     MHDP_HW_READY,	   /* HW ready, FW active*/
+		     MHDP_HW_STOPPED };	   /* Driver removal FW to be stopped */
+
 struct cdns_mhdp_device {
 	void __iomem *regs;
 	void __iomem *j721e_regs;
@@ -233,6 +245,18 @@ struct cdns_mhdp_device {
 
 	u8 link_up : 1;
 	u8 plugged : 1;
+
+	/*
+	 * "start_lock" protects the access to bridge_attached and
+	 * hw_state data members that control the delayed firmware
+	 * loading and attaching the bridge. They are accessed from
+	 * both the DRM core and mhdp_fw_cb(). In most cases just
+	 * protecting the data members is enough, but the irq mask
+	 * setting needs to be protected when enabling the FW.
+	 */
+	spinlock_t start_lock;
+	u8 bridge_attached : 1;
+	enum mhdp_hw_state hw_state;
 };
 
 #define connector_to_mhdp(x) container_of(x, struct cdns_mhdp_device, connector)