Hello Doug, Ernest,

On 27/05/25 21:14, Doug Anderson wrote:
Hi,

On Mon, May 26, 2025 at 1:41 AM Ernest Van Hoecke
<[email protected]> wrote:

Hi Jayesh,

First of all, thanks for your patch. I applied it to our 6.6-based
downstream kernel supporting a board I have here, and noticed some
strange behaviour with eDP now.

On Thu, May 08, 2025 at 05:24:33PM +0530, Jayesh Choudhary wrote:
+     if (pdata->bridge.type == DRM_MODE_CONNECTOR_eDP)
+             regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
+                                HPD_DISABLE);


On my setup it seems that `pdata->bridge.type` is not yet set here,
because it executes before `ti_sn_bridge_probe`. For the DP use case,
this is not a problem because the type field is 0 (i.e., not
DRM_MODE_CONNECTOR_eDP) in that case. But for eDP, it means that we are
unexpectedly not disabling HDP.

With working HDP, everything is fine in the end for both DP and eDP. But
when the HDP line is not connected, eDP no longer works. So I wonder if
this breaks some functionality for weird eDP panels or board
implementations.

I could certainly be missing something; from my understanding it looks
like without a good HPD signal, the `ti_sn_bridge_probe` and quoted code
are stuck in a loop. `ti_sn65dsi86_enable_comms` runs but does not
disable HDP, after which the probe runs but fails and does not set the
type field, so the next `enable_comms` run fails to disable HDP again,
etc.

This does sound like a real problem.

I'm not sure I'll have the time to analyze it and come up with a
proposal myself right now, but Jayesh: you should make sure you
consider and address this issue before you send your next version.

Thanks!


I see that i2c_driver probe ti_sn65dsi86_probe() is called first
which calls ti_sn65dsi86_resume() and ti_sn65dsi86_enable_comms()
and after that auxiliary_driver probe ti_sn_bridge_probe() is called
where pdata->bridge.type is set.

As per the bindings, I see that we should have "no-hpd" property in
the device description for platforms with bad HPD or disconnected HPD.

Then we can read it in ti_sn65dsi86_probe() before resume call and use
it as a conditional instead.
Since I do not have any "bad HPD signal" board, I would need some
help validating this on such boards from Ernest.

Also, that would mean adding "no-hpd" to all the platforms using
sn65dsi86 for baseline first, even before the driver changes are in.
Because if driver changes go in first, this would enable HPD for all
the platforms. Whereas, the dts changes alone are harmless.
But still I am doubtful about dts changes getting in before driver
changes.
Considering this, please let me know the order of changes and I will
send out the patches accordingly.

Thanks,
Jayesh

Reply via email to