[RFC v2 2/2] dt-bindings: mipi-dsi: Add dual-channel DSI related info

Fri Jun 8 08:47:05 UTC 2018

On 08.06.2018 00:50, Heiko Stuebner wrote:
> Am Donnerstag, 7. Juni 2018, 23:10:20 CEST schrieb Brian Norris:
>> Hi,
>>
>> I only have a little bit to add, but Heiko did solicit my opinion.
> yep ... and I realized that I am/was way to attached to my (working)
> endpoint-based thingy to really appreciate the other arguments ;-)
>
> And your DSI-related writings below, did provide some new thought-
> directions for me, so thanks for that.
>
>> On Thu, Jun 07, 2018 at 03:25:18PM +0200, Heiko Stuebner wrote:
>>> Am Donnerstag, 7. Juni 2018, 12:39:03 CEST schrieb Andrzej Hajda:
>>>> On 07.06.2018 01:08, Heiko Stuebner wrote:
>>>>> Am Mittwoch, 6. Juni 2018, 18:07:46 CEST schrieb Archit Taneja:
>>>>>> On Wednesday 06 June 2018 04:16 PM, Heiko Stübner wrote:
>>>>>>> Am Mittwoch, 6. Juni 2018, 12:21:16 CEST schrieb Archit Taneja:
>>>>>>>> On Wednesday 06 June 2018 02:00 PM, Heiko Stübner wrote:
>>>>>>>>> Am Mittwoch, 6. Juni 2018, 07:59:29 CEST schrieb Archit Taneja:
>>>>>>>>>> On Monday 04 June 2018 05:47 PM, Heiko Stuebner wrote:
>>>>>>>>>>> Am Donnerstag, 18. Januar 2018, 05:53:55 CEST schrieb Archit Taneja:
>>>>>>>>>>>> Add binding info for peripherals that support dual-channel DSI. Add
>>>>>>>>>>>> corresponding optional bindings for DSI host controllers that may
>>>>>>>>>>>> be configured in this mode. Add an example of an I2C controlled
>>>>>>>>>>>> device operating in dual-channel DSI mode.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Archit Taneja <architt at codeaurora.org>
>>>>>>>>>>> Looks like a great solution for that problem, so
>>>>>>>>>>> Reviewed-by: Heiko Stuebner <heiko at sntech.de>
>>>>>>>>>>>
>>>>>>>>>>> As I'm looking into that for my rk3399-scarlet device right now and
>>>>>>>>>>> couldn't find this patchset in the kernel yet, is it planned to
>>>>>>>>>>> merge or refresh these binding changes or were problems encountered.
>>>>>>>>>>>
>>>>>>>>>>> At least an Ack/Review from Rob seems to be missing.
>>>>>>>>>> I forgot about these patches. Rob had reviewed the first one in
>>>>>>>>>> the set the second one still needed an Ack. I'll post a v3
>>>>>>>>>> that adds the Reviewed-bys and fixes a small typo.
>>>>>>>>> very nice ... because it looks like yesterday I managed to make the
>>>>>>>>> Rockchip dsi work in dual mode following this.
>>>>>>>>>
>>>>>>>>> But one question came up, do you really want two input ports on the
>>>>>>>>> panel
>>>>>>>>> side? I.e. hardware-wise, I guess the panel will have one 8-lane or so
>>>>>>>>> input thatonly gets split up on the soc side onto 2 dsi controllers?
>>>>>>>> I think all dual DSI panels actually have 2 DSI controllers/parsers
>>>>>>>> within them, one on each port. The MIPI DSI spec doesn't support 8
>>>>>>>> lanes. Also, the pixels coming out of the host are distributed among
>>>>>>>> the lanes differently than what would have been the case with a
>>>>>>>> 'theoretical' 8 lane receiver.
>>>>>>>>
>>>>>>>> Other than that, some dual DSI panels only accept DSI commands on the
>>>>>>>> 'master' port, where as others expect the same command to be sent
>>>>>>>> across
>>>>>>>> both the ports.
>>>>>>>>
>>>>>>>> Therefore, I think it's better to represent dual DSI panels having 2
>>>>>>>> DSI input ports.
>>>>>>>>
>>>>>>>> Your DT looks good to me.
>>>>>>> Hmm, that doesn't match up then ;-) ... as my dt uses 2 endpoints
>>>>>>> in one port for the dsi-links.
>>>>>> Sorry, I didn't notice you'd created two endpoints within a single port.
>>>>>>
>>>>>> I don't think I'm particular about 2 ports vs 1 port with 2 endpoints.
>>>>>> They both seem okay to me as long as we follow it consistently. I'm
>>>>>> myself not 100% sure of how to figure where one should prefer endpoints
>>>>>> over ports. Maybe someone more familiar with the of graph bindings
>>>>>> could comment here.
>>>> Port should describe physical port, endpoint are used to describe
>>>> multiple links to the same port. If the panel has two physical DSI
>>>> interfaces it should use two ports.
>>> That again leaves a bit of space for interpretation ;-) .
>> Does it really? DSI maxes out at 4 lanes, so more than 4 lanes is
>> clearly not a single standard port. Additionally, the code we're
>> currently using in Chrome OS actually sends the same commands to each
>> DSI separately (I'm not sure if it's actually required by the panels
>> were using), so again, they are to some extent logically independent.
>>
>>> A dual-dsi display is probably pretty useless with only one controller
>>> connected to it, as its 4 lanes cannot satisfy the required 8 lanes
>>> of the panel.
>> Probably, although technically you can usually display on half the
>> screen, if you have only have 1 working DSI.
>>
>>> See [0] for current WIP panel addition.
>>> [Was already on the lists but needs cleanup]
>>>
>>>
>>>
>>> [0] https://github.com/mmind/linux-rockchip/commit/459bc1a1599377c2c5408724e82619a4602a953d#diff-5d6d6ddab4fd282102d23d2c02e835f8R381
>>>
>>>
>>>>>>> The issue I see with using ports and not endpoints for dual-dsi links
>>>>>>> is with distinguishing between input and output ports.
>>>>>>>
>>>>>>> For a panel that's easy, as you every port will be an input port and if
>>>>>>> you have 2, it's supposed dual-dsi. But for example I guess there might
>>>>>>> exist some dual-dsi-to-something bridges, where you would end up
>>>>>>> with say 3 (or even more) ports ... two dual-dsi inputs and 1 or more
>>>>>>> outputs.
>>>>>> Okay, I get your point here. Although, even if the remote device had
>>>>>> exactly 2 ports. Is it safe to assume that port #0 is an input port and
>>>>>> port #1 is an output port? Is that the norm?
>>>>> I don't think that anything like that is specified anywhere, so you cannot
>>>>> assume anything about what a port contains.
>>>> Ports are defined per device-node and port number assignment should be
>>>> described in particular device bindings. So device driver knows well
>>>> which functions should be assigned to which port. But it is private
>>>> matter of the device/device driver. Different device bindings can assign
>>>> different number for input and output ports, but since ports are private
>>>> thing for devices it should not be a problem.
>> Agreed. And if Heiko needs a specific example close to home: all the
>> Rockchip display bindings do this similarly; for example
>> display/rockchip/analogix_dp-rockchip.txt describes exactly which port
>> numbers and enpoints mean what:
>>
>> - ports: there are 2 port nodes with endpoint definitions as defined in
>>   Documentation/devicetree/bindings/media/video-interfaces.txt.
>>     Port 0: contained 2 endpoints, connecting to the output of vop.
>>     Port 1: contained 1 endpoint, connecting to the input of panel.
> Agreed here. My confusion stems from the issue that with the way
> the code is right now, the dsi controller would need to know how
> the ports of the panel are structured, to find its way to the 2nd
> controller.
>
> But then again, maybe the implementation just needs to change ;-) .
>
>
>>>> I do not see why do you have such issue, could you describe it more.
>>>>
>>>>>> I've at least seen one device (toshiba,tc358767 bridge) that can
>>>>>> actually take either DPI as input or DSI based on how you configure it.
>>>>>> There are 2 input ports here, port #0 for DSI and port #1 for DPI. Would
>>>>>> it have made sense here to have a single port and 2 endpoints here too?
>>>>> Nope, I guess having a port for DPI input, one port for DSI input makes
>>>>> quite a lot of sense. And then you can have the input-specifics living in
>>>>> the endpoints like dual links or so.
>>>>>
>>>>>>> While the following argument might not be 100% valid from a dt-purity
>>>>>>> standpoint implementing this might get hairy for _any_ operating system,
>>>>>>> as you will need each panel/bridge to tell what the ports are used for.
>>>>>> Yeah.
>>>> Each panel/bridge should know what his ports are used for, but to who
>>>> and why you want to communicate it?
>>> I'll do that below :-)
>>>
>>>
>>>>>>> I.e. in my endpoint based mapping, right now I have this nice and
>>>>>>> generic
>>>>>>> WIP function to parse the of_graph and get the master+slave nodes:
>>>>>>>
>>>>>>> https://github.com/mmind/linux-rockchip/blob/tmp/rk3399-gru-bob-scarlet/
>>>>>>> drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#L697 [0]
>>>>>> I'd tried out something locally before posting this patch, I don't have
>>>>>> the code for it, but I can describe the steps I took. This code expects
>>>>>> the panel/bridge to have 2 input ports.
>>>>> Which again would be very much panel-specific as you cannot assume
>>>>> to much about the ports.
>>>>>
>>>>>> 1. DSI0 host driver looks up its output port, gets the remote endpoint,
>>>>>> and get this endpoint's parent (using
>>>>>> of_graph_get_remote_port_parent()). Keeps the parent device node in a
>>>>>> temp variable.
>>>>>>
>>>>>> 2. DSI1 host driver does the same thing.
>>>>>>
>>>>>> 3. DSI0 and DSI1 check whether their outputs are connected to the
>>>>>> same device. If so, they're in dual DSI mode. If not, they are
>>>>>> operating independently.
>>>>>>
>>>>>> The positive of this approach is that we don't need to make any
>>>>>> assumptions about the panel/bridge's port numbers, which is great.
>>>>>> The negative is that our DSI controller instances now need to query
>>>>>> each other, which can be messy, but not too hard to implement.
>>>> I think detection of dual dsi and whole configuration thing of dual-dsi
>>>> should be done using in-kernel frameworks (maybe new ones :) ).
>>>> Could you describe necessary configuration steps to be performed, what
>>>> info should be passed from who to who, etc. It would be easier to discuss.
>>> The base setup is the rk3399 soc, which has 2 completely separate
>>> dw-mipi-dsi controllers.
>>>
>>> Gru-Scarlet (a ChromeOS tablet device) uses a high-res dual-DSI display
>>> (2500x1500 pixels or so) which is driven by both controllers together to
>>> achieve the required number of lanes to it.
>>>
>>> So you end up with one (specific) controller being the master and the
>>> other being the slave, deferring to the master for most everything.
>>> As the controllers are somehow build to work together in that case
>>> via special soc-specific settings.
>>>
>>> See [1] for current WIP code, I lifted out of the chromeos kernel and
>>> modified to not have that special rockchip,dual-channel =<&...>; property.
>>> That patch was on the lists already last year I think.
>>>
>>> So each dw-mipi-dsi instance needs to at least know that it is in a
>>> master-slave-relationship and right now also needs access to the other
>>> driver instance. Doing the master->slave access should be fine in that
>>> special case, as the IP is the same type.
>>>
>>> Also the crtc<->dsi interaction is quite a bit of handling-different between
>>> one crtc talking to 2 DSIs or 2 crtc talking to the 2 DSIs separately.
>> That's probably the bigger key: to treat them as completely separate
>> ports means that you get separate CRTCs, IIUC (I'll admit, I'm still a
>> bit rusty on navigating some DRM concepts).
> One thing I realized with your mention of DSI maxing out at 4 lanes is,
> that this makes it easy to detect the existence of a dual-dsi situation ...
> simply when the device reports 8 lanes.
> (via of_find_mipi_dsi_device_by_node presumably)

Hmm, device has two DSI interfaces, on each it has 4 lanes, so it report
4 lanes.
There is different problem with current implementation of panel lookup
code - drm_panel is identified by device_node of physical device, as a
result there can be only one panel per device.
I think proper identification should be by device_node of OF graph port,
or by pair: device_node of physical device and port number (practically
it is the same).
I think fixing it should not be a big deal.

>
> As a dual-dsi situation requires a clock-master property in one,
> both master and slave also would be able to determine their
> master or slave status, thus the global dual-dsi config could be
> done by the master (GRF stuff in the Rockchip case)
>
>
> The only thing that makes my head explode now, is how to
> make the slave actually react to settings sent to the master
> in a sane way.

Wouldn't be enough if the panel passes different bus info on DSI0 and DSI1?

Regards
Andrzej

>
> But that is a drm-specific implementation-detail, I guess ;-) .
> And hopefully someone might have a great idea how this
> could be done better than in my current implementation.
>
>
> Heiko
>
>
>
>
>