[RFC v2 2/2] dt-bindings: mipi-dsi: Add dual-channel DSI related info
Heiko Stübner
heiko at sntech.de
Wed Jun 6 10:46:21 UTC 2018
Hi Archit,
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.
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.
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.
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]
So I guess my proposal would be to have one port for inputs
and one port for outputs for dsi peripherals, with possibly
multiple endpoints in each.
Heiko
[0] github seems to have reliability problems, so for reference my
parsing function:
static int dw_mipi_dsi_is_dual(struct dw_mipi_dsi_rockchip *dsi,
struct device_node **master, struct device_node **slave)
{
struct device_node *local_ep, *remote_port, *ep;
struct device_node *ctrls[2] = { NULL, NULL };
int num = 0, ret = 0, idx;
/* get local panel endpoint of the dsi controller */
local_ep = of_graph_get_endpoint_by_regs(dsi->dev->of_node, 1, 0);
if (!local_ep) {
DRM_DEV_ERROR(dsi->dev, "couldn't find local panel endpoint\n");
return -ENXIO;
}
/* get panel port */
remote_port = of_graph_get_remote_port_parent(local_ep);
of_node_put(local_ep);
if (!remote_port) {
DRM_DEV_ERROR(dsi->dev, "couldn't find panel port\n");
return -ENXIO;
}
/* check other endpoints */
for_each_endpoint_of_node(remote_port, ep) {
struct device_node *np = of_graph_get_remote_port_parent(ep);
if (!np)
continue;
idx = of_property_read_bool(np, "clock-master");
/*
* Either master or slave already defined, drop refcnt
* but catch errors only after the full loop.
*/
if (ctrls[idx])
of_node_put(np);
else
ctrls[idx] = np;
num++;
}
of_node_put(remote_port);
if (num > 2) {
DRM_DEV_ERROR(dsi->dev, "too many dsi devices linked\n");
ret = -EINVAL;
goto cleanup;
}
/* nothing to do */
if (num < 1) {
ret = 0;
goto cleanup;
}
if (!ctrls[1]) {
DRM_DEV_ERROR(dsi->dev, "no master defined in dual-dsi\n");
ret = -ENODEV;
goto cleanup;
}
if (!ctrls[0]) {
DRM_DEV_ERROR(dsi->dev, "no slave defined in dual-dsi\n");
ret = -ENODEV;
goto cleanup;
}
*master = ctrls[1];
*slave = ctrls[0];
return 1;
cleanup:
for (idx = 0; idx < 2; idx++)
if (ctrls[idx])
of_node_put(ctrls[idx]);
return ret;
}
> > So right now I'm operating with a devicetree like
> >
> > &mipi_dsi {
> >
> > status = "okay";
> > clock-master;
> >
> > ports {
> >
> > mipi_out: port at 1 {
> >
> > reg = <1>;
> >
> > mipi_out_panel: endpoint {
> >
> > remote-endpoint = <&mipi_in_panel>;
> >
> > };
> >
> > };
> >
> > };
> >
> > mipi_panel: panel at 0 {
> >
> > compatible = "innolux,p097pfg";
> >
> > reg = <0>;
> > backlight = <&backlight>;
> > enable-gpios = <&gpio4 25 GPIO_ACTIVE_HIGH>;
> > pinctrl-names = "default";
> > pinctrl-0 = <&display_rst_l>;
> >
> > port {
> >
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > mipi_in_panel: endpoint at 0 {
> >
> > reg = <0>;
> > remote-endpoint = <&mipi_out_panel>;
> >
> > };
> >
> > mipi1_in_panel: endpoint at 1 {
> >
> > reg = <1>;
> > remote-endpoint = <&mipi1_out_panel>;
> >
> > };
> >
> > };
> >
> > };
> >
> > };
> >
> > &mipi_dsi1 {
> >
> > status = "okay";
> >
> > ports {
> >
> > mipi1_out: port at 1 {
> >
> > reg = <1>;
> >
> > mipi1_out_panel: endpoint {
> >
> > remote-endpoint = <&mipi1_in_panel>;
> >
> > };
> >
> > };
> >
> > };
> >
> > };
> >
> >
> > I guess it is a matter of preference on what reflects the hardware
> > best, so maybe that's Robs call?
> >
> >
> > Heiko
> >
> >>>> ---
> >>>> v2:
> >>>> - Specify that clock-master is a boolean property.
> >>>> - Drop/add unit-address and #*-cells where applicable.
> >>>>
> >>>> .../devicetree/bindings/display/mipi-dsi-bus.txt | 80
> >>>> ++++++++++++++++++++++ 1 file changed, 80 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/display/mipi-dsi-bus.txt
> >>>> b/Documentation/devicetree/bindings/display/mipi-dsi-bus.txt index
> >>>> 94fb72cb916f..7a3abbedb3fa 100644
> >>>> --- a/Documentation/devicetree/bindings/display/mipi-dsi-bus.txt
> >>>> +++ b/Documentation/devicetree/bindings/display/mipi-dsi-bus.txt
> >>>>
> >>>> @@ -29,6 +29,13 @@ Required properties:
> >>>> - #size-cells: Should be 0. There are cases where it makes sense to
> >>>> use
> >>>> a
> >>>>
> >>>> different value here. See below.
> >>>>
> >>>> +Optional properties:
> >>>> +- clock-master: boolean. Should be enabled if the host is being used
> >>>> in
> >>>> + conjunction with another DSI host to drive the same peripheral.
> >>>> Hardware
> >>>> + supporting such a configuration generally requires the data on both
> >>>> the busses + to be driven by the same clock. Only the DSI host
> >>>> instance
> >>>> controlling this + clock should contain this property.
> >>>> +
> >>>>
> >>>> DSI peripheral
> >>>> ==============
> >>>>
> >>>> @@ -62,6 +69,16 @@ primary control bus, but are also connected to a DSI
> >>>> bus (mostly for the data>>
> >>>>
> >>>> path). Connections between such peripherals and a DSI host can be
> >>>> represented using the graph bindings [1], [2].
> >>>>
> >>>> +Peripherals that support dual channel DSI
> >>>> +-----------------------------------------
> >>>> +
> >>>> +Peripherals with higher bandwidth requirements can be connected to 2
> >>>> DSI
> >>>> +busses. Each DSI bus/channel drives some portion of the pixel data
> >>>> (generally +left/right half of each line of the display, or even/odd
> >>>> lines of the display). +The graph bindings should be used to represent
> >>>> the multiple DSI busses that are +connected to this peripheral. Each
> >>>> DSI
> >>>> host's output endpoint can be linked to +an input endpoint of the DSI
> >>>> peripheral.
> >>>> +
> >>>>
> >>>> [1] Documentation/devicetree/bindings/graph.txt
> >>>> [2] Documentation/devicetree/bindings/media/video-interfaces.txt
> >>>>
> >>>> @@ -71,6 +88,8 @@ Examples
> >>>>
> >>>> with different virtual channel configurations.
> >>>>
> >>>> - (4) is an example of a peripheral on a I2C control bus connected
> >>>> with
> >>>> to
> >>>>
> >>>> a DSI host using of-graph bindings.
> >>>>
> >>>> +- (5) is an example of 2 DSI hosts driving a dual-channel DSI
> >>>> peripheral,
> >>>> + which uses I2C as its primary control bus.
> >>>>
> >>>> 1)
> >>>>
> >>>> dsi-host {
> >>>>
> >>>> @@ -153,3 +172,64 @@ Examples
> >>>>
> >>>> };
> >>>>
> >>>> };
> >>>>
> >>>> };
> >>>>
> >>>> +
> >>>> +5)
> >>>> + i2c-host {
> >>>> + dsi-bridge at 35 {
> >>>> + compatible = "...";
> >>>> + reg = <0x35>;
> >>>> +
> >>>> + ports {
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <0>;
> >>>> +
> >>>> + port at 0 {
> >>>> + reg = <0>;
> >>>> + dsi0_in: endpoint {
> >>>> + remote-endpoint = <&dsi0_out>;
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + port at 1 {
> >>>> + reg = <1>;
> >>>> + dsi1_in: endpoint {
> >>>> + remote-endpoint = <&dsi1_out>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + dsi0-host {
> >>>> + ...
> >>>> +
> >>>> + /*
> >>>> + * this DSI instance drives the clock for both the host
> >>>> + * controllers
> >>>> + */
> >>>> + clock-master;
> >>>> +
> >>>> + ports {
> >>>> + ...
> >>>> +
> >>>> + port {
> >>>> + dsi0_out: endpoint {
> >>>> + remote-endpoint = <&dsi0_in>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + dsi1-host {
> >>>> + ...
> >>>> +
> >>>> + ports {
> >>>> + ...
> >>>> +
> >>>> + port {
> >>>> + dsi1_out: endpoint {
> >>>> + remote-endpoint = <&dsi1_in>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
> >>> in
> >>> the body of a message to majordomo at vger.kernel.org
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the dri-devel
mailing list