[v2,0/2] drm/bridge: tc358767: Fix DRM_BRIDGE_ATTACH_NO_CONNECTOR case

  These two patches are needed to make tc358767 work in the
DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP connector.

I have tested this with TI AM654 EVM with a tc358767 add-on card
connected to a DP monitor.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Changes in v2:
- Update the format negotiation patch as discussed in https://lore.kernel.org/all/7ddf0edb-2925-4b7c-ad07-27c030dd0232@ti.com/
- Link to v1: https://lore.kernel.org/r/20231031-tc358767-v1-0-392081ad9f4b@ideasonboard.com

---
Aradhya Bhatia (1):
      drm/bridge: tc358767: Add format negotiation hooks for DPI/DSI to (e)DP

Tomi Valkeinen (1):
      drm/bridge: tc358767: Fix link properties discovery

 drivers/gpu/drm/bridge/tc358767.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
---
base-commit: 9d7c8c066916f231ca0ed4e4fce6c4b58ca3e451
change-id: 20231031-tc358767-58e3ebdf95f0

Best regards,
  

Hi Tomi,

Am Mittwoch, 8. November 2023, 12:27:21 CET schrieb Tomi Valkeinen:
> These two patches are needed to make tc358767 work in the
> DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP connector.
> 
> I have tested this with TI AM654 EVM with a tc358767 add-on card
> connected to a DP monitor.

Just a question regarding the usage of this DSI-DP bridge.
What is the state of the DSI lanes after the DSI host has been initialized, 
but before calling atomic_pre_enable? AFAIK this bridge requires LP-11 on DSI 
at any time for accessing the AUX channel.

Best regards,
Alexander

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> Changes in v2:
> - Update the format negotiation patch as discussed in
> https://lore.kernel.org/all/7ddf0edb-2925-4b7c-ad07-27c030dd0232@ti.com/ -
> Link to v1:
> https://lore.kernel.org/r/20231031-tc358767-v1-0-392081ad9f4b@ideasonboard.
> com
> 
> ---
> Aradhya Bhatia (1):
>       drm/bridge: tc358767: Add format negotiation hooks for DPI/DSI to
> (e)DP
> 
> Tomi Valkeinen (1):
>       drm/bridge: tc358767: Fix link properties discovery
> 
>  drivers/gpu/drm/bridge/tc358767.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> ---
> base-commit: 9d7c8c066916f231ca0ed4e4fce6c4b58ca3e451
> change-id: 20231031-tc358767-58e3ebdf95f0
> 
> Best regards,
  

On 08/11/2023 14:45, Alexander Stein wrote:
> Hi Tomi,
> 
> Am Mittwoch, 8. November 2023, 12:27:21 CET schrieb Tomi Valkeinen:
>> These two patches are needed to make tc358767 work in the
>> DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP connector.
>>
>> I have tested this with TI AM654 EVM with a tc358767 add-on card
>> connected to a DP monitor.
> 
> Just a question regarding the usage of this DSI-DP bridge.
> What is the state of the DSI lanes after the DSI host has been initialized,
> but before calling atomic_pre_enable? AFAIK this bridge requires LP-11 on DSI
> at any time for accessing the AUX channel.

Good question. I don't know, as we use it in DPI mode (DPI-DP bridge). I 
guess the DSI state is undefined, as it might well be that the DSI host 
driver's (pre-)enable is the place where the DSI is powered up and 
initialized.

So you think in DSI mode this might fail, as AUX (possibly) won't work 
when calling tc_get_edid()? We could add a check there, and skip the 
tc_get_display_props() if in DSI mode. But tc_get_edid() surely won't 
work then.

It would be good if someone with a board where tc358767 is used in DSI 
mode could test this. But, of course, it'll only be testing that 
particular DSI host behavior...

  Tomi

> 
> Best regards,
> Alexander
> 
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> Changes in v2:
>> - Update the format negotiation patch as discussed in
>> https://lore.kernel.org/all/7ddf0edb-2925-4b7c-ad07-27c030dd0232@ti.com/ -
>> Link to v1:
>> https://lore.kernel.org/r/20231031-tc358767-v1-0-392081ad9f4b@ideasonboard.
>> com
>>
>> ---
>> Aradhya Bhatia (1):
>>        drm/bridge: tc358767: Add format negotiation hooks for DPI/DSI to
>> (e)DP
>>
>> Tomi Valkeinen (1):
>>        drm/bridge: tc358767: Fix link properties discovery
>>
>>   drivers/gpu/drm/bridge/tc358767.c | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>> ---
>> base-commit: 9d7c8c066916f231ca0ed4e4fce6c4b58ca3e451
>> change-id: 20231031-tc358767-58e3ebdf95f0
>>
>> Best regards,
> 
>
  

Hi,

On 08/11/2023 14:45, Alexander Stein wrote:
> Hi Tomi,
> 
> Am Mittwoch, 8. November 2023, 12:27:21 CET schrieb Tomi Valkeinen:
>> These two patches are needed to make tc358767 work in the
>> DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP connector.
>>
>> I have tested this with TI AM654 EVM with a tc358767 add-on card
>> connected to a DP monitor.
> 
> Just a question regarding the usage of this DSI-DP bridge.
> What is the state of the DSI lanes after the DSI host has been initialized,
> but before calling atomic_pre_enable? AFAIK this bridge requires LP-11 on DSI
> at any time for accessing the AUX channel.

We haven't received any test reports for the DSI-DP case... I was 
looking at the datasheet, and I wonder, why do you say the bridge 
requires DSI to be up for the AUX transactions?

  Tomi

> Best regards,
> Alexander
> 
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> Changes in v2:
>> - Update the format negotiation patch as discussed in
>> https://lore.kernel.org/all/7ddf0edb-2925-4b7c-ad07-27c030dd0232@ti.com/ -
>> Link to v1:
>> https://lore.kernel.org/r/20231031-tc358767-v1-0-392081ad9f4b@ideasonboard.
>> com
>>
>> ---
>> Aradhya Bhatia (1):
>>        drm/bridge: tc358767: Add format negotiation hooks for DPI/DSI to
>> (e)DP
>>
>> Tomi Valkeinen (1):
>>        drm/bridge: tc358767: Fix link properties discovery
>>
>>   drivers/gpu/drm/bridge/tc358767.c | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>> ---
>> base-commit: 9d7c8c066916f231ca0ed4e4fce6c4b58ca3e451
>> change-id: 20231031-tc358767-58e3ebdf95f0
>>
>> Best regards,
> 
>
  

On 06/12/23 17:41, Tomi Valkeinen wrote:
> Hi,
> 
> On 08/11/2023 14:45, Alexander Stein wrote:
>> Hi Tomi,
>>
>> Am Mittwoch, 8. November 2023, 12:27:21 CET schrieb Tomi Valkeinen:
>>> These two patches are needed to make tc358767 work in the
>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP connector.
>>>
>>> I have tested this with TI AM654 EVM with a tc358767 add-on card
>>> connected to a DP monitor.
>>
>> Just a question regarding the usage of this DSI-DP bridge.
>> What is the state of the DSI lanes after the DSI host has been
>> initialized,
>> but before calling atomic_pre_enable? AFAIK this bridge requires LP-11
>> on DSI
>> at any time for accessing the AUX channel.

+ Marek

Marek, Alexander,

A quick grep tells me that you have added devicetree for tc358767 in DSI
to (e)DP mode on other platforms. Could you please test these patches
and report if you find any issue?

Regards
Aradhya

> 
> We haven't received any test reports for the DSI-DP case... I was
> looking at the datasheet, and I wonder, why do you say the bridge
> requires DSI to be up for the AUX transactions?
> 
>  Tomi
> 
>> Best regards,
>> Alexander
>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> ---
>>> Changes in v2:
>>> - Update the format negotiation patch as discussed in
>>> https://lore.kernel.org/all/7ddf0edb-2925-4b7c-ad07-27c030dd0232@ti.com/ -
>>> Link to v1:
>>> https://lore.kernel.org/r/20231031-tc358767-v1-0-392081ad9f4b@ideasonboard.
>>> com
>>>
>>> ---
>>> Aradhya Bhatia (1):
>>>        drm/bridge: tc358767: Add format negotiation hooks for DPI/DSI to
>>> (e)DP
>>>
>>> Tomi Valkeinen (1):
>>>        drm/bridge: tc358767: Fix link properties discovery
>>>
>>>   drivers/gpu/drm/bridge/tc358767.c | 32
>>> ++++++++++++++++++++++++++++++++
>>>   1 file changed, 32 insertions(+)
>>> ---
>>> base-commit: 9d7c8c066916f231ca0ed4e4fce6c4b58ca3e451
>>> change-id: 20231031-tc358767-58e3ebdf95f0
>>>
>>> Best regards,
>>
>>
>
  

Hi Tomi,

Am Mittwoch, 6. Dezember 2023, 13:11:59 CET schrieb Tomi Valkeinen:
> Hi,
> 
> On 08/11/2023 14:45, Alexander Stein wrote:
> > Hi Tomi,
> > 
> > Am Mittwoch, 8. November 2023, 12:27:21 CET schrieb Tomi Valkeinen:
> >> These two patches are needed to make tc358767 work in the
> >> DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP connector.
> >> 
> >> I have tested this with TI AM654 EVM with a tc358767 add-on card
> >> connected to a DP monitor.
> > 
> > Just a question regarding the usage of this DSI-DP bridge.
> > What is the state of the DSI lanes after the DSI host has been
> > initialized,
> > but before calling atomic_pre_enable? AFAIK this bridge requires LP-11 on
> > DSI at any time for accessing the AUX channel.
> 
> We haven't received any test reports for the DSI-DP case... I was
> looking at the datasheet, and I wonder, why do you say the bridge
> requires DSI to be up for the AUX transactions?

Looking at Figure 4.20 (Power On Sequence) in the datasheet TC9595XBG (Rev 1.1 
2021-06-23) you can see that RESX can be released (only) after DSI lanes went 
to LP-11 state. I got information, down from the support, that LP-11 must be 
up in order to use AUX channel. This also matches our observations, DSI hosts 
often enable LP-11 only in atomic_prepare. That's too late so we used some 
hacks to enable LP-11 right from the beginning just to get access to AUX 
channel.

Best regards
Alexander

> 
>   Tomi
> 
> > Best regards,
> > Alexander
> > 
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> ---
> >> Changes in v2:
> >> - Update the format negotiation patch as discussed in
> >> https://lore.kernel.org/all/7ddf0edb-2925-4b7c-ad07-27c030dd0232@ti.com/
> >> -
> >> Link to v1:
> >> https://lore.kernel.org/r/20231031-tc358767-v1-0-392081ad9f4b@ideasonboar
> >> d.
> >> com
> >> 
> >> ---
> >> 
> >> Aradhya Bhatia (1):
> >>        drm/bridge: tc358767: Add format negotiation hooks for DPI/DSI to
> >> 
> >> (e)DP
> >> 
> >> Tomi Valkeinen (1):
> >>        drm/bridge: tc358767: Fix link properties discovery
> >>   
> >>   drivers/gpu/drm/bridge/tc358767.c | 32 ++++++++++++++++++++++++++++++++
> >>   1 file changed, 32 insertions(+)
> >> 
> >> ---
> >> base-commit: 9d7c8c066916f231ca0ed4e4fce6c4b58ca3e451
> >> change-id: 20231031-tc358767-58e3ebdf95f0
> >> 
> >> Best regards,
  

On 11.12.23 09:07, Aradhya Bhatia wrote:
> 
> 
> On 06/12/23 17:41, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 08/11/2023 14:45, Alexander Stein wrote:
>>> Hi Tomi,
>>>
>>> Am Mittwoch, 8. November 2023, 12:27:21 CET schrieb Tomi Valkeinen:
>>>> These two patches are needed to make tc358767 work in the
>>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP connector.
>>>>
>>>> I have tested this with TI AM654 EVM with a tc358767 add-on card
>>>> connected to a DP monitor.
>>>
>>> Just a question regarding the usage of this DSI-DP bridge.
>>> What is the state of the DSI lanes after the DSI host has been
>>> initialized,
>>> but before calling atomic_pre_enable? AFAIK this bridge requires LP-11
>>> on DSI
>>> at any time for accessing the AUX channel.
> 
> + Marek
> 
> Marek, Alexander,
> 
> A quick grep tells me that you have added devicetree for tc358767 in DSI
> to (e)DP mode on other platforms. Could you please test these patches
> and report if you find any issue?

Is this the last blocker to move forward with these fixes? I'd really
like to see them finally merged.

Thanks,
Jan
  

Hi everyone,

Am Donnerstag, 15. Februar 2024, 09:53:54 CET schrieb Jan Kiszka:
> On 11.12.23 09:07, Aradhya Bhatia wrote:
> > On 06/12/23 17:41, Tomi Valkeinen wrote:
> >> Hi,
> >> 
> >> On 08/11/2023 14:45, Alexander Stein wrote:
> >>> Hi Tomi,
> >>> 
> >>> Am Mittwoch, 8. November 2023, 12:27:21 CET schrieb Tomi Valkeinen:
> >>>> These two patches are needed to make tc358767 work in the
> >>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP
> >>>> connector.
> >>>> 
> >>>> I have tested this with TI AM654 EVM with a tc358767 add-on card
> >>>> connected to a DP monitor.
> >>> 
> >>> Just a question regarding the usage of this DSI-DP bridge.
> >>> What is the state of the DSI lanes after the DSI host has been
> >>> initialized,
> >>> but before calling atomic_pre_enable? AFAIK this bridge requires LP-11
> >>> on DSI
> >>> at any time for accessing the AUX channel.
> > 
> > + Marek
> > 
> > Marek, Alexander,
> > 
> > A quick grep tells me that you have added devicetree for tc358767 in DSI
> > to (e)DP mode on other platforms. Could you please test these patches
> > and report if you find any issue?

Sorry, I can't provide any feedback here. I've yet to setup the DSI-DP 
correctly.

Best regards,
Alexander

> Is this the last blocker to move forward with these fixes? I'd really
> like to see them finally merged.
> 
> Thanks,
> Jan
  

On 15/02/2024 11:03, Alexander Stein wrote:
> Hi everyone,
> 
> Am Donnerstag, 15. Februar 2024, 09:53:54 CET schrieb Jan Kiszka:
>> On 11.12.23 09:07, Aradhya Bhatia wrote:
>>> On 06/12/23 17:41, Tomi Valkeinen wrote:
>>>> Hi,
>>>>
>>>> On 08/11/2023 14:45, Alexander Stein wrote:
>>>>> Hi Tomi,
>>>>>
>>>>> Am Mittwoch, 8. November 2023, 12:27:21 CET schrieb Tomi Valkeinen:
>>>>>> These two patches are needed to make tc358767 work in the
>>>>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP
>>>>>> connector.
>>>>>>
>>>>>> I have tested this with TI AM654 EVM with a tc358767 add-on card
>>>>>> connected to a DP monitor.
>>>>>
>>>>> Just a question regarding the usage of this DSI-DP bridge.
>>>>> What is the state of the DSI lanes after the DSI host has been
>>>>> initialized,
>>>>> but before calling atomic_pre_enable? AFAIK this bridge requires LP-11
>>>>> on DSI
>>>>> at any time for accessing the AUX channel.
>>>
>>> + Marek
>>>
>>> Marek, Alexander,
>>>
>>> A quick grep tells me that you have added devicetree for tc358767 in DSI
>>> to (e)DP mode on other platforms. Could you please test these patches
>>> and report if you find any issue?
> 
> Sorry, I can't provide any feedback here. I've yet to setup the DSI-DP
> correctly.

Ok. Does anyone have a worry that these patches make the situation worse 
for the DSI case than it was before? Afaics, if the DSI lanes are not 
set up early enough by the DSI host, the driver would break with and 
without these patches.

These do fix the driver for DRM_BRIDGE_ATTACH_NO_CONNECTOR and DPI, so 
I'd like to merge these unless these cause a regression with the DSI case.

  Tomi
  

On 2/16/24 10:10, Tomi Valkeinen wrote:
> On 15/02/2024 11:03, Alexander Stein wrote:
>> Hi everyone,
>>
>> Am Donnerstag, 15. Februar 2024, 09:53:54 CET schrieb Jan Kiszka:
>>> On 11.12.23 09:07, Aradhya Bhatia wrote:
>>>> On 06/12/23 17:41, Tomi Valkeinen wrote:
>>>>> Hi,
>>>>>
>>>>> On 08/11/2023 14:45, Alexander Stein wrote:
>>>>>> Hi Tomi,
>>>>>>
>>>>>> Am Mittwoch, 8. November 2023, 12:27:21 CET schrieb Tomi Valkeinen:
>>>>>>> These two patches are needed to make tc358767 work in the
>>>>>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR case, at least when using a DP
>>>>>>> connector.
>>>>>>>
>>>>>>> I have tested this with TI AM654 EVM with a tc358767 add-on card
>>>>>>> connected to a DP monitor.
>>>>>>
>>>>>> Just a question regarding the usage of this DSI-DP bridge.
>>>>>> What is the state of the DSI lanes after the DSI host has been
>>>>>> initialized,
>>>>>> but before calling atomic_pre_enable? AFAIK this bridge requires 
>>>>>> LP-11
>>>>>> on DSI
>>>>>> at any time for accessing the AUX channel.
>>>>
>>>> + Marek
>>>>
>>>> Marek, Alexander,
>>>>
>>>> A quick grep tells me that you have added devicetree for tc358767 in 
>>>> DSI
>>>> to (e)DP mode on other platforms. Could you please test these patches
>>>> and report if you find any issue?
>>
>> Sorry, I can't provide any feedback here. I've yet to setup the DSI-DP
>> correctly.
> 
> Ok. Does anyone have a worry that these patches make the situation worse 
> for the DSI case than it was before? Afaics, if the DSI lanes are not 
> set up early enough by the DSI host, the driver would break with and 
> without these patches.
> 
> These do fix the driver for DRM_BRIDGE_ATTACH_NO_CONNECTOR and DPI, so 
> I'd like to merge these unless these cause a regression with the DSI case.

1/2 looks good to me, go ahead and apply .