Re: [PATCH v2] drm/bridge: ti-sn65dsi86: Add HPD for DisplayPort connector type

[Jayesh Choudhary <j-choudhary@ti.com>]

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
> <ernestvanhoecke@gmail.com> 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