[PATCH v5 1/2] dt-bindings: display: bridge: lvds-codec: Document pixel data sampling edge select
The OnSemi FIN3385 Parallel-to-LVDS encoder has a dedicated input line to select input pixel data sampling edge. Add DT property "pclk-sample", not the same as the one used by display timings but rather the same as used by media, to define the pixel data sampling edge.
Signed-off-by: Marek Vasut marex@denx.de Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Rob Herring robh+dt@kernel.org Cc: Sam Ravnborg sam@ravnborg.org Cc: devicetree@vger.kernel.org To: dri-devel@lists.freedesktop.org --- V4: New patch split from combined V3 V5: Rebase on recent linux-next --- .../bindings/display/bridge/lvds-codec.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml index 1faae3e323a4..708de84ac138 100644 --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml @@ -79,6 +79,14 @@ properties: - port@0 - port@1
+ pclk-sample: + description: + Data sampling on rising or falling edge. + enum: + - 0 # Falling edge + - 1 # Rising edge + default: 0 + powerdown-gpios: description: The GPIO used to control the power down line of this device. @@ -102,6 +110,16 @@ then: properties: data-mapping: false
+if: + not: + properties: + compatible: + contains: + const: lvds-encoder +then: + properties: + pclk-sample: false + required: - compatible - ports
The OnSemi FIN3385 Parallel-to-LVDS encoder has a dedicated input line to select input pixel data sampling edge. Add DT property "pclk-sample", not the same as the one used by display timings but rather the same as used by media, and configure bus flags based on this DT property.
Signed-off-by: Marek Vasut marex@denx.de Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Rob Herring robh+dt@kernel.org Cc: Sam Ravnborg sam@ravnborg.org Cc: devicetree@vger.kernel.org To: dri-devel@lists.freedesktop.org --- V2: - Limit the pixelclk-active to encoders only - Update DT binding document V3: - Determine whether this is encoder from connector, i.e. lvds_codec->connector_type == DRM_MODE_CONNECTOR_LVDS V4: - Switch to pclk-sample. Note that the value of this is inverted, so all the existing users of pixelclk-active using previous previous version of this patch must be reworked V5: Rebase on recent linux-next --- drivers/gpu/drm/bridge/lvds-codec.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/bridge/lvds-codec.c b/drivers/gpu/drm/bridge/lvds-codec.c index f991842a161f..702ea803a743 100644 --- a/drivers/gpu/drm/bridge/lvds-codec.c +++ b/drivers/gpu/drm/bridge/lvds-codec.c @@ -21,6 +21,7 @@ struct lvds_codec { struct device *dev; struct drm_bridge bridge; struct drm_bridge *panel_bridge; + struct drm_bridge_timings timings; struct regulator *vcc; struct gpio_desc *powerdown_gpio; u32 connector_type; @@ -119,6 +120,7 @@ static int lvds_codec_probe(struct platform_device *pdev) struct device_node *bus_node; struct drm_panel *panel; struct lvds_codec *lvds_codec; + u32 val; int ret;
lvds_codec = devm_kzalloc(dev, sizeof(*lvds_codec), GFP_KERNEL); @@ -187,12 +189,25 @@ static int lvds_codec_probe(struct platform_device *pdev) } }
+ /* + * Encoder might sample data on different clock edge than the display, + * for example OnSemi FIN3385 has a dedicated strapping pin to select + * the sampling edge. + */ + if (lvds_codec->connector_type == DRM_MODE_CONNECTOR_LVDS && + !of_property_read_u32(dev->of_node, "pclk-sample", &val)) { + lvds_codec->timings.input_bus_flags = val ? + DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE : + DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE; + } + /* * The panel_bridge bridge is attached to the panel's of_node, * but we need a bridge attached to our of_node for our user * to look up. */ lvds_codec->bridge.of_node = dev->of_node; + lvds_codec->bridge.timings = &lvds_codec->timings; drm_bridge_add(&lvds_codec->bridge);
platform_set_drvdata(pdev, lvds_codec);
Hi Marek,
On Sun, Oct 17, 2021 at 02:12:04AM +0200, Marek Vasut wrote:
Late to the party here - sorry.
I do not understand how this will work. The only field that is set is timings.input_bus_flags but any user will see bridge.timings is set and will think this is all timing info.
Maybe I just misses something obvious?
Sam
On 10/17/21 6:49 PM, Sam Ravnborg wrote:
[...]
Is there anything else in those timings that should be set ? See include/drm/drm_bridge.h around line 640
setup_time_ps/hold_time_ps/dual_link isn't supported by this driver, so it is 0 or false anyway, i.e. no change.
Hi Marek,
On Sun, Oct 17, 2021 at 07:29:51PM +0200, Marek Vasut wrote:
Just me being confused with display_timings. Patch looks good. Reviewed-by: Sam Ravnborg sam@ravnborg.org
Ping me in a few days to apply it if there is no more feedback.
Sam
Hi Marek,
On Mon, Oct 18, 2021 at 09:47:13PM +0200, Marek Vasut wrote:
For this particular chip that's true. I'm still not convinced overall. For some cases it could be a per-port property, and moving it there for lvds-codec too could allow implementing helpers to parse DT properties, without much drawback for this particular use case as far as I can see. It's hard to predict the future with certainty of course, so I won't insist.
Hi Marek,
On Tue, Oct 19, 2021 at 12:18:11AM +0200, Marek Vasut wrote:
I'm thinking about bridges that could have multiple parallel inputs.
DT bindings are not holy beings that live in a mythical heaven way above the mere mortal drivers, they would be useless without implementations. It's not about bending them, which I regularly push against during review, but about structuring them in a way that facilitates implementations when all other things are equal.
As I said, despite wondering whether or not it would be better to move the property to the endpoint (and that was a genuine open question), I won't insist in this case.
On 10/19/21 8:49 AM, Laurent Pinchart wrote:
Hi Marek,
Hi,
Can you draft an example how such a binding would look like within the confines of this lvds-codec.yaml ?
I also have to wonder how such a hypothetical device would work, would it serialize two parallel bussed into single LVDS one ?
Note that the pclk-sample isn't a property of the input, but of the chip, I don't think it is a good idea to say they are equal and conflate them like so.
[...]
On Tue, Oct 19, 2021 at 04:39:05PM +0200, Marek Vasut wrote:
Such a device would require custom bindings I think, as lvds-codec is limited to a single input and a single output. thine,thc63lvd1024.yaml is an example of such a device.
With a chip that has a single input, that's always the case :-)
Anyway, I don't mind a chip-level property for this binding as we're limited to a single port. If other devices need to specify this at the port level, I'm sure we'll be able to cope with the lack of uniformity.
dri-devel@lists.freedesktop.org
-
Laurent Pinchart -
Marek Vasut -
Rob Herring -
Sam Ravnborg