Re: [PATCH v2 1/2] drm: bridge: samsung-dsim: Initialize bridge on attach

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/16/24 9:50 PM, Michael Walle wrote:
Thank you for testing and keeping up with this. I will wait for more
feedback if there is any (Frieder? Lucas? Michael?). If there are no
objections, then I can merge it in a week or two ?

I'll try to use your approach on the tc358775. Hopefully, I'll find
some time this week.

So ... I wonder ... shall I apply these patches or not ?

As mentioned on IRC, I tried it to port it for the mediatek DSI
host, but I gave up and got doubts that this is the way to go. I
think this is too invasive (in a sense that it changes behavior)

I would argue it makes the behavior well defined. If that breaks some
drivers that depended on the undefined behavior before, those should be
fixed too.

Then this behavior should be documented (and accepted) in the
corresponding section:
https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation

I think so too.

This will help DSI host driver developers and we can point all the
host DSI driver maintainers to that document along with the proper
implementation :)

and not that easy to implement on other drivers.

How so ? At least the DSIM and STM32 DW DSI host can switch lanes to
LP11 state. Is the mediatek host not capable of that ?

The controller is certainly capable to do that. But the changes
seems pretty invasive to me and I fear some fallout. Although it's
basically just one line for the DSIM, you seem to be moving the init
of the DSIM to an earlier point(?). I'm no expert with all the DRM
stuff, so I might be wrong here.

I am moving some of the initialization to an earlier point, but only enough of it to configure the lanes to LP11 state before the next bridge(s) start to (pre)enable themselves. And the DSIM controller is RPM suspended again after the lanes are in LP11.

Given that this requirement is far more common across DSI bridges,
I'd favor a more general solution which isn't a workaround.

I think we only had a look at the TI DSI83 / ICN6211 / Toshiba TC358767
bridges, but we did not look at many panels, did we ? Do panels require
lanes in non-LP11 state on start up ?

I'm not talking about panels, just bridges. It's not just one bridge
with a weird behavior but most bridges.

What do you refer to by "weird behavior" ? It seems the DSI bridges we looked at all required data lanes in LP11 state on start up one way or the other, didn't they ?

Was there any progress on the generic LP11 solution, I think you did
mention something was in progress ? How would that even look like ?

I had a new callback implemented:
https://lore.kernel.org/r/20240506-tc358775-fix-powerup-v1-1-545dcf00b8dd@xxxxxxxxxx/

Not sure if that's any better though.

Wouldn't that callback be called by various controllers at various stages of initialization , without consistency on when that callback will be called ? That would be my concern.

At least here, the expectation is that the controller would put data lanes into LP11 before the next bridge can even pre_enable itself , which is not perfect though, because if a bridge needs DSI clock to probe() itself and then data lanes in LP11 to probe itself, that is a really bad situation (and the TC358767 is capable of being used that way, although this is currently not supported by the kernel driver and there is no real interest in supporting it).



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux