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@xxxxxxxxxxxxxx> >>>>>>>>>>> Looks like a great solution for that problem, so >>>>>>>>>>> Reviewed-by: Heiko Stuebner <heiko@xxxxxxxxx> >>>>>>>>>>> >>>>>>>>>>> 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 > > > > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
