[PATCHv2 21/22] drm/bridge: tc358767: add IRQ and HPD support

[Tomi Valkeinen]

Hi,

On 17/04/2019 10:32, Andrzej Hajda wrote:
> On 15.04.2019 12:59, Tomi Valkeinen wrote:
>> On 15/04/2019 13:42, Andrzej Hajda wrote:
>>
>>> Ok we have here 4 combinations:
>>>
>>> 1. noHPD + eDP.
>>>
>>> 2. noHPD + DP.
>>>
>>> 3. HPD + eDP.
>>>
>>> 4. HPD + DP.
>>>
>>>
>>> Which ones do you want to support, disallow. It is not clear to me.
>> They all should work.
>>
>> If there is HPD, we use it to return connected/disconnected.
> 
> 
> OK, I though that eDP does not use HPD at all.
> 
> 
>> If we don't have HPD:
>> - If there's a panel (i.e. eDP), we presume that it is always there, as
>> by definition it can't be disconnected.
>> - If there's no panel (i.e. DP), we can't know if the cable is connected
>> or not, so we return unknown. I think this could be improved by trying
>> to "ping" the monitor with an AUX transaction, but I didn't look at that.
>>
>>> Moreover what connector_status_unknown should mean here for users?
>> The the driver does not know if there's a monitor or not.
> 
> :)
> 
> More specifically, what user can do with such information.
> 
> OK, he can ensure monitor is connected and then force connected state.

Yes, something like that. I haven't really been testing that kind of
setups, but afaik, that's more about how DRM exposes and expects the
userspace to handle "unknown" connection status than what TC358767 does.

> But shall we expect existence of such configurations, I hoped we could
> eliminate such combination (DP+noHPD) during probe.

Eliminate how? Make HPD required for DP and fail if there's no HPD? I
guess that's an option.

Then again, the solutions the HW people come up with in the embedded
space never ceases to amaze me, so while I don't expect such
configurations, I would not be surprised to see one.

>>>> +	if (tc->hpd_num >= 0 && tc->bridge.dev) {
>>>> +		bool h = val & INT_GPIO_H(tc->hpd_num);
>>>> +		bool lc = val & INT_GPIO_LC(tc->hpd_num);
>>>> +
>>>> +		dev_dbg(tc->dev, "GPIO%d: %s %s\n", tc->hpd_num,
>>>> +			h ? "H" : "", lc ? "LC" : "");
>>>
>>> What does "h" and "lc" mean?
>> These are from the func spec. H means high, LC means low-counter. H is
>> triggered immediately when the HPD line goes high, LC is triggered when
>> the line has been low for the counter-specified time (the counter is
>> configured at probe time).
> 
> 
> It would be good to add this info somewhere, it is hard to guess what lc
> means.

Ok.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki