Skip to content

Commit 55e8ff8

Browse files
jayesh-tidianders
authored andcommitted
drm/bridge: ti-sn65dsi86: Add HPD for DisplayPort connector type
By default, HPD was disabled on SN65DSI86 bridge. When the driver was added (commit "a095f15c00e27"), the HPD_DISABLE bit was set in pre-enable call which was moved to other function calls subsequently. Later on, commit "c312b0df3b13" 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: c312b0d ("drm/bridge: ti-sn65dsi86: Implement bridge connector operations for DP") Cc: Max Krummenacher <[email protected]> Reviewed-by: Douglas Anderson <[email protected]> Tested-by: Ernest Van Hoecke <[email protected]> Signed-off-by: Jayesh Choudhary <[email protected]> Signed-off-by: Douglas Anderson <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 1035782 commit 55e8ff8

File tree

1 file changed

+60
-9
lines changed

1 file changed

+60
-9
lines changed

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

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -348,12 +348,18 @@ static void ti_sn65dsi86_enable_comms(struct ti_sn65dsi86 *pdata,
348348
* 200 ms. We'll assume that the panel driver will have the hardcoded
349349
* delay in its prepare and always disable HPD.
350350
*
351-
* If HPD somehow makes sense on some future panel we'll have to
352-
* change this to be conditional on someone specifying that HPD should
353-
* be used.
351+
* For DisplayPort bridge type, we need HPD. So we use the bridge type
352+
* to conditionally disable HPD.
353+
* NOTE: The bridge type is set in ti_sn_bridge_probe() but enable_comms()
354+
* can be called before. So for DisplayPort, HPD will be enabled once
355+
* bridge type is set. We are using bridge type instead of "no-hpd"
356+
* property because it is not used properly in devicetree description
357+
* and hence is unreliable.
354358
*/
355-
regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
356-
HPD_DISABLE);
359+
360+
if (pdata->bridge.type != DRM_MODE_CONNECTOR_DisplayPort)
361+
regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
362+
HPD_DISABLE);
357363

358364
pdata->comms_enabled = true;
359365

@@ -1195,9 +1201,14 @@ static enum drm_connector_status ti_sn_bridge_detect(struct drm_bridge *bridge)
11951201
struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
11961202
int val = 0;
11971203

1198-
pm_runtime_get_sync(pdata->dev);
1204+
/*
1205+
* Runtime reference is grabbed in ti_sn_bridge_hpd_enable()
1206+
* as the chip won't report HPD just after being powered on.
1207+
* HPD_DEBOUNCED_STATE reflects correct state only after the
1208+
* debounce time (~100-400 ms).
1209+
*/
1210+
11991211
regmap_read(pdata->regmap, SN_HPD_DISABLE_REG, &val);
1200-
pm_runtime_put_autosuspend(pdata->dev);
12011212

12021213
return val & HPD_DEBOUNCED_STATE ? connector_status_connected
12031214
: connector_status_disconnected;
@@ -1220,6 +1231,26 @@ static void ti_sn65dsi86_debugfs_init(struct drm_bridge *bridge, struct dentry *
12201231
debugfs_create_file("status", 0600, debugfs, pdata, &status_fops);
12211232
}
12221233

1234+
static void ti_sn_bridge_hpd_enable(struct drm_bridge *bridge)
1235+
{
1236+
struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
1237+
1238+
/*
1239+
* Device needs to be powered on before reading the HPD state
1240+
* for reliable hpd detection in ti_sn_bridge_detect() due to
1241+
* the high debounce time.
1242+
*/
1243+
1244+
pm_runtime_get_sync(pdata->dev);
1245+
}
1246+
1247+
static void ti_sn_bridge_hpd_disable(struct drm_bridge *bridge)
1248+
{
1249+
struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
1250+
1251+
pm_runtime_put_autosuspend(pdata->dev);
1252+
}
1253+
12231254
static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
12241255
.attach = ti_sn_bridge_attach,
12251256
.detach = ti_sn_bridge_detach,
@@ -1234,6 +1265,8 @@ static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
12341265
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
12351266
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
12361267
.debugfs_init = ti_sn65dsi86_debugfs_init,
1268+
.hpd_enable = ti_sn_bridge_hpd_enable,
1269+
.hpd_disable = ti_sn_bridge_hpd_disable,
12371270
};
12381271

12391272
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,
13211354
pdata->bridge.type = pdata->next_bridge->type == DRM_MODE_CONNECTOR_DisplayPort
13221355
? DRM_MODE_CONNECTOR_DisplayPort : DRM_MODE_CONNECTOR_eDP;
13231356

1324-
if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort)
1325-
pdata->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT;
1357+
if (pdata->bridge.type == DRM_MODE_CONNECTOR_DisplayPort) {
1358+
pdata->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_DETECT |
1359+
DRM_BRIDGE_OP_HPD;
1360+
/*
1361+
* If comms were already enabled they would have been enabled
1362+
* with the wrong value of HPD_DISABLE. Update it now. Comms
1363+
* could be enabled if anyone is holding a pm_runtime reference
1364+
* (like if a GPIO is in use). Note that in most cases nobody
1365+
* is doing AUX channel xfers before the bridge is added so
1366+
* HPD doesn't _really_ matter then. The only exception is in
1367+
* the eDP case where the panel wants to read the EDID before
1368+
* the bridge is added. We always consistently have HPD disabled
1369+
* for eDP.
1370+
*/
1371+
mutex_lock(&pdata->comms_mutex);
1372+
if (pdata->comms_enabled)
1373+
regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG,
1374+
HPD_DISABLE, 0);
1375+
mutex_unlock(&pdata->comms_mutex);
1376+
};
13261377

13271378
drm_bridge_add(&pdata->bridge);
13281379

0 commit comments

Comments
 (0)