Commit 55e8ff84 authored by Jayesh Choudhary's avatar Jayesh Choudhary Committed by Douglas Anderson
Browse files

drm/bridge: ti-sn65dsi86: Add HPD for DisplayPort connector type 

By default, HPD was disabled on SN65DSI86 bridge. When the driver was
added (commit "a095f15c"), the HPD_DISABLE bit was set in pre-enable
call which was moved to other function calls subsequently.
Later on, commit "c312b0df" added detect utility for DP mode. But with
HPD_DISABLE bit set, all the HPD events are disabled[0] and the debounced
state always return 1 (always connected state).

Set HPD_DISABLE bit conditionally based on display sink's connector type.
Since the HPD_STATE is reflected correctly only after waiting for debounce
time (~100-400ms) and adding this delay in detect() is not feasible
owing to the performace impact (glitches and frame drop), remove runtime
calls in detect() and add hpd_enable()/disable() bridge hooks with runtime
calls, to detect hpd properly without any delay.

[0]: <https://www.ti.com/lit/gpn/SN65DSI86

> (Pg. 32)

Fixes: c312b0df ("drm/bridge: ti-sn65dsi86: Implement bridge connector operations for DP")
Cc: Max Krummenacher <max.krummenacher@toradex.com>
Reviewed-by: default avatarDouglas Anderson <dianders@chromium.org>
Tested-by: default avatarErnest Van Hoecke <ernest.vanhoecke@toradex.com>
Signed-off-by: default avatarJayesh Choudhary <j-choudhary@ti.com>
Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20250624044835.165708-1-j-choudhary@ti.com
parent 10357824

drivers/gpu/drm/bridge/ti-sn65dsi86.c

+60 −9
Original line number Diff line number Diff line
@@ -348,10 +348,16 @@ static void ti_sn65dsi86_enable_comms(struct ti_sn65dsi86 *pdata, 
	 * 200 ms.  We'll assume that the panel driver will have the hardcoded

	 * delay in its prepare and always disable HPD.

	 *

	 * If HPD somehow makes sense on some future panel we'll have to

	 * change this to be conditional on someone specifying that HPD should

	 * be used.

	 * For DisplayPort bridge type, we need HPD. So we use the bridge type

	 * to conditionally disable HPD.

	 * NOTE: The bridge type is set in ti_sn_bridge_probe() but enable_comms()

	 * can be called before. So for DisplayPort, HPD will be enabled once

	 * bridge type is set. We are using bridge type instead of "no-hpd"

	 * property because it is not used properly in devicetree description

	 * and hence is unreliable.

	 */



	if (pdata->bridge.type != DRM_MODE_CONNECTOR_DisplayPort)

		regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,

				   HPD_DISABLE);
@@ -1195,9 +1201,14 @@ static enum drm_connector_status ti_sn_bridge_detect(struct drm_bridge *bridge) 
	struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);

	int val = 0;



	pm_runtime_get_sync(pdata->dev);

	/*

	 * Runtime reference is grabbed in ti_sn_bridge_hpd_enable()

	 * as the chip won't report HPD just after being powered on.

	 * HPD_DEBOUNCED_STATE reflects correct state only after the

	 * debounce time (~100-400 ms).

	 */



	regmap_read(pdata->regmap, SN_HPD_DISABLE_REG, &val);

	pm_runtime_put_autosuspend(pdata->dev);



	return val & HPD_DEBOUNCED_STATE ? connector_status_connected

					 : connector_status_disconnected;
@@ -1220,6 +1231,26 @@ static void ti_sn65dsi86_debugfs_init(struct drm_bridge *bridge, struct dentry * 
	debugfs_create_file("status", 0600, debugfs, pdata, &status_fops);

}



static void ti_sn_bridge_hpd_enable(struct drm_bridge *bridge)

{

	struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);



	/*

	 * Device needs to be powered on before reading the HPD state

	 * for reliable hpd detection in ti_sn_bridge_detect() due to

	 * the high debounce time.

	 */



	pm_runtime_get_sync(pdata->dev);

}



static void ti_sn_bridge_hpd_disable(struct drm_bridge *bridge)

{

	struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);



	pm_runtime_put_autosuspend(pdata->dev);

}



static const struct drm_bridge_funcs ti_sn_bridge_funcs = {

	.attach = ti_sn_bridge_attach,

	.detach = ti_sn_bridge_detach,
@@ -1234,6 +1265,8 @@ static const struct drm_bridge_funcs ti_sn_bridge_funcs = { 
	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,

	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,

	.debugfs_init = ti_sn65dsi86_debugfs_init,

	.hpd_enable = ti_sn_bridge_hpd_enable,

	.hpd_disable = ti_sn_bridge_hpd_disable,

};



static void ti_sn_bridge_parse_lanes(struct ti_sn65dsi86 *pdata,
@@ -1321,8 +1354,26 @@ static int ti_sn_bridge_probe(struct auxiliary_device *adev, 
	pdata->bridge.type = pdata->next_bridge->type == DRM_MODE_CONNECTOR_DisplayPort

			   ? DRM_MODE_CONNECTOR_DisplayPort : DRM_MODE_CONNECTOR_eDP;



	if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort)

		pdata->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT;

	if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort) {

		pdata->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT |

				    DRM_BRIDGE_OP_HPD;

		/*

		 * If comms were already enabled they would have been enabled

		 * with the wrong value of HPD_DISABLE. Update it now. Comms

		 * could be enabled if anyone is holding a pm_runtime reference

		 * (like if a GPIO is in use). Note that in most cases nobody

		 * is doing AUX channel xfers before the bridge is added so

		 * HPD doesn't _really_ matter then. The only exception is in

		 * the eDP case where the panel wants to read the EDID before

		 * the bridge is added. We always consistently have HPD disabled

		 * for eDP.

		 */

		mutex_lock(&pdata->comms_mutex);

		if (pdata->comms_enabled)

			regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG,

					   HPD_DISABLE, 0);

		mutex_unlock(&pdata->comms_mutex);

	};



	drm_bridge_add(&pdata->bridge);