On 4/28/21 11:24 AM, Neil Armstrong wrote: [...]
+static int sn65dsi83_probe(struct i2c_client *client,
const struct i2c_device_id *id)+{
- struct device *dev = &client->dev;
- enum sn65dsi83_model model;
- struct sn65dsi83 *ctx;
- int ret;
- ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
- if (!ctx)
return -ENOMEM;- ctx->dev = dev;
- if (dev->of_node)
model = (enum sn65dsi83_model)of_device_get_match_data(dev);- else
model = id->driver_data;- /* Default to dual-link LVDS on all but DSI83. */
- if (model != MODEL_SN65DSI83)
ctx->lvds_dual_link = true;What if I use the DSI84 with a single link LVDS? I can't see any way to configure that right now.
I assume the simplest way would be to use the "ti,sn65dsi83" compatible string in your dts, since the way you wired it is 'compatible' with sn65dsi83, right?
No this isn't the right way to to, if sn65dsi84 is supported and the bindings only support single lvds link, the driver must only support single link on sn65dsi84, or add the dual link lvds in the bindings only for sn65dsi84.
The driver has a comment about what is supported and tested, as Frieder already pointed out:
Currently supported: - SN65DSI83 = 1x Single-link DSI ~ 1x Single-link LVDS - Supported - Single-link LVDS mode tested - SN65DSI84 = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS - Supported - Dual-link LVDS mode tested - 2x Single-link LVDS mode unsupported (should be easy to add by someone who has the HW) - SN65DSI85 = 2x Single-link or 1x Dual-link DSI ~ 2x Single-link or 1x Dual-link LVDS - Unsupported (should be easy to add by someone who has the HW)
So, DSI83 is always single-link DSI, single-link LVDS. DSI84 is always single-link DSI, and currently always dual-link LVDS.
The DSI83 can do one thing on the LVDS end: - 1x single link lVDS
The DSI84 can do two things on the LVDS end: - 1x single link lVDS - 1x dual link LVDS
There is also some sort of mention in the DSI84 datasheet about 2x single link LVDS, but I suspect that might be copied from DSI85 datasheet instead, which would make sense. The other option is that it behaves as a mirror (i.e. same pixels are scanned out of LVDS channel A and B). Either option can be added by either adding a DT property which would enable the mirror mode, or new port linking the LVDS endpoint to the same panel twice, and/or two new ports for DSI85, so there is no problem to extend the bindings without breaking them. So for now I would ignore this mode.
So ultimately, what we have to sort out is the 1x single / 1x dual link LVDS mode setting on DSI84. Frieder already pointed out how the driver can be tweaked to support the single-link mode on DSI84, so now we need to tie it into DT bindings.
Currently, neither the LVDS panels in upstream in panel-simple nor lvds.yaml provide any indication that the panel is single-link or dual-link. Those dual-link LVDS panels seem to always set 2x pixel clock and let the bridge somehow sort it out.
Maybe that isn't always the best approach, maybe we should add a new DRM_BUS_FLAG for those panels and handle the flag in the bridge driver ? Such a new flag could be added over time to panels where applicable, so old setups won't be broken by that either, they will just ignore the new flag and work as before.
I just saw the note in the header of the driver that says that single link mode is unsupported for the DSI84.
I have hardware with a single link display and if I set ctx->lvds_dual_link = false it works just fine.
How is this supposed to be selected? Does it need an extra devicetree property? And would you mind adding single-link support in the next version or do you prefer adding it in a follow-up patch?
If this has to be supported I think the proper way would be to support two output ports in the dts (e.g. lvds0_out, lvds1_out), in the same way as supported by the 'advantech,idk-2121wr' panel.
Yes, this is why I asked to have the dual-link lvds in the bindings.
Maybe it shouldn't really be in the bindings, see above.