Re: [RFC][PATCH] drm/bridge: lvds-codec: Add support for pixel data sampling edge select
On 10/19/20 2:54 AM, Laurent Pinchart wrote: > Hi Marek, Hi, > Thank you for the patch. > > On Sat, Oct 03, 2020 at 01:08:23AM +0200, Marek Vasut wrote: >> The OnSemi FIN3385 Parallel-to-LVDS encoder has a dedicated input line to >> select input pixel data sampling edge. Add DT property "pixelclk-active", >> same as the one used by display timings, and configure bus flags based on >> this DT property. > > The feature looks good to me. I however wonder if we shouldn't use the > standard pclk-sample endpoint property (documented in [1]) instead of a > custom properly. Either is fine by me, but I think pixelclk-active, which comes from panel-timings.yaml is closer to the video than multimedia bindings. (the multimedia and video bindings should be aligned, sigh) > The DT bindings for the lvds-codec should be updated accordingly. And > the property should only be taken into account when operating in encoder > mode, as for decoder mode there's no polarity for the sampling of LVDS > signals, as you've explained in a reply to Sam. So, we need to rework the whole compatible data and identify what is encoder/decoder in addition to connector there. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] drm/bridge: lvds-codec: Add support for pixel data sampling edge select
Hi Marek,
Thank you for the patch.
On Sat, Oct 03, 2020 at 01:08:23AM +0200, Marek Vasut wrote:
> The OnSemi FIN3385 Parallel-to-LVDS encoder has a dedicated input line to
> select input pixel data sampling edge. Add DT property "pixelclk-active",
> same as the one used by display timings, and configure bus flags based on
> this DT property.
The feature looks good to me. I however wonder if we shouldn't use the
standard pclk-sample endpoint property (documented in [1]) instead of a
custom properly.
The DT bindings for the lvds-codec should be updated accordingly. And
the property should only be taken into account when operating in encoder
mode, as for decoder mode there's no polarity for the sampling of LVDS
signals, as you've explained in a reply to Sam.
[1] Documentation/devicetree/bindings/media/video-interfaces.txt
> Signed-off-by: Marek Vasut
> Cc: Alexandre Torgue
> Cc: Andrzej Hajda
> Cc: Antonio Borneo
> Cc: Benjamin Gaignard
> Cc: Biju Das
> Cc: Laurent Pinchart
> Cc: Maxime Coquelin
> Cc: Philippe Cornu
> Cc: Sam Ravnborg
> Cc: Vincent Abriou
> Cc: Yannick Fertre
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-st...@st-md-mailman.stormreply.com
> To: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/bridge/lvds-codec.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/lvds-codec.c
> b/drivers/gpu/drm/bridge/lvds-codec.c
> index f52ccffc1bd1..bc941d4fb5b9 100644
> --- a/drivers/gpu/drm/bridge/lvds-codec.c
> +++ b/drivers/gpu/drm/bridge/lvds-codec.c
> @@ -19,6 +19,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;
> @@ -80,6 +81,7 @@ static int lvds_codec_probe(struct platform_device *pdev)
> struct device_node *panel_node;
> struct drm_panel *panel;
> struct lvds_codec *lvds_codec;
> + u32 val;
> int ret;
>
> lvds_codec = devm_kzalloc(dev, sizeof(*lvds_codec), GFP_KERNEL);
> @@ -124,6 +126,12 @@ static int lvds_codec_probe(struct platform_device *pdev)
> if (IS_ERR(lvds_codec->panel_bridge))
> return PTR_ERR(lvds_codec->panel_bridge);
>
> + if (!of_property_read_u32(dev->of_node, "pixelclk-active", )) {
> + lvds_codec->timings.input_bus_flags = val ?
> + DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE :
> + DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE;
> + }
> +
> /*
>* 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
> @@ -131,6 +139,7 @@ static int lvds_codec_probe(struct platform_device *pdev)
>*/
> lvds_codec->bridge.of_node = dev->of_node;
> lvds_codec->bridge.funcs =
> + lvds_codec->bridge.timings = _codec->timings;
> drm_bridge_add(_codec->bridge);
>
> platform_set_drvdata(pdev, lvds_codec);
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] drm/bridge: lvds-codec: Add support for pixel data sampling edge select
On 10/16/20 6:25 PM, Sam Ravnborg wrote: > Hi Marek. Hello Sam, > On Sat, Oct 03, 2020 at 01:08:23AM +0200, Marek Vasut wrote: >> The OnSemi FIN3385 Parallel-to-LVDS encoder has a dedicated input line to >> select input pixel data sampling edge. Add DT property "pixelclk-active", >> same as the one used by display timings, and configure bus flags based on >> this DT property. > > Why is it that this information cannot come from the panel driver? Because the sampling edge of the panel can be different (is there even such an edge for LVDS panel?). What we care about specifying here is the edge on which the FIN3385 samples the data on the Parallel bus, and that is a property of the FIN3385, because that is configured via a dedicated pin on the FIN3385. > The property tell when data are sampled and the FIN3385 Parallel-to-LVDS > is the one that transmit the data - not then one that samples the data. > Correct? Not correct I'm afraid, see above. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] drm/bridge: lvds-codec: Add support for pixel data sampling edge select
Hi Marek.
On Sat, Oct 03, 2020 at 01:08:23AM +0200, Marek Vasut wrote:
> The OnSemi FIN3385 Parallel-to-LVDS encoder has a dedicated input line to
> select input pixel data sampling edge. Add DT property "pixelclk-active",
> same as the one used by display timings, and configure bus flags based on
> this DT property.
Why is it that this information cannot come from the panel driver?
The property tell when data are sampled and the FIN3385 Parallel-to-LVDS
is the one that transmit the data - not then one that samples the data.
Correct?
Sam
>
> Signed-off-by: Marek Vasut
> Cc: Alexandre Torgue
> Cc: Andrzej Hajda
> Cc: Antonio Borneo
> Cc: Benjamin Gaignard
> Cc: Biju Das
> Cc: Laurent Pinchart
> Cc: Maxime Coquelin
> Cc: Philippe Cornu
> Cc: Sam Ravnborg
> Cc: Vincent Abriou
> Cc: Yannick Fertre
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-st...@st-md-mailman.stormreply.com
> To: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/bridge/lvds-codec.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/lvds-codec.c
> b/drivers/gpu/drm/bridge/lvds-codec.c
> index f52ccffc1bd1..bc941d4fb5b9 100644
> --- a/drivers/gpu/drm/bridge/lvds-codec.c
> +++ b/drivers/gpu/drm/bridge/lvds-codec.c
> @@ -19,6 +19,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;
> @@ -80,6 +81,7 @@ static int lvds_codec_probe(struct platform_device *pdev)
> struct device_node *panel_node;
> struct drm_panel *panel;
> struct lvds_codec *lvds_codec;
> + u32 val;
> int ret;
>
> lvds_codec = devm_kzalloc(dev, sizeof(*lvds_codec), GFP_KERNEL);
> @@ -124,6 +126,12 @@ static int lvds_codec_probe(struct platform_device *pdev)
> if (IS_ERR(lvds_codec->panel_bridge))
> return PTR_ERR(lvds_codec->panel_bridge);
>
> + if (!of_property_read_u32(dev->of_node, "pixelclk-active", )) {
> + lvds_codec->timings.input_bus_flags = val ?
> + DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE :
> + DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE;
> + }
> +
> /*
>* 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
> @@ -131,6 +139,7 @@ static int lvds_codec_probe(struct platform_device *pdev)
>*/
> lvds_codec->bridge.of_node = dev->of_node;
> lvds_codec->bridge.funcs =
> + lvds_codec->bridge.timings = _codec->timings;
> drm_bridge_add(_codec->bridge);
>
> platform_set_drvdata(pdev, lvds_codec);
> --
> 2.28.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH] drm/bridge: lvds-codec: Add support for 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 "pixelclk-active",
same as the one used by display timings, and configure bus flags based on
this DT property.
Signed-off-by: Marek Vasut
Cc: Alexandre Torgue
Cc: Andrzej Hajda
Cc: Antonio Borneo
Cc: Benjamin Gaignard
Cc: Biju Das
Cc: Laurent Pinchart
Cc: Maxime Coquelin
Cc: Philippe Cornu
Cc: Sam Ravnborg
Cc: Vincent Abriou
Cc: Yannick Fertre
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-st...@st-md-mailman.stormreply.com
To: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/bridge/lvds-codec.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/bridge/lvds-codec.c
b/drivers/gpu/drm/bridge/lvds-codec.c
index f52ccffc1bd1..bc941d4fb5b9 100644
--- a/drivers/gpu/drm/bridge/lvds-codec.c
+++ b/drivers/gpu/drm/bridge/lvds-codec.c
@@ -19,6 +19,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;
@@ -80,6 +81,7 @@ static int lvds_codec_probe(struct platform_device *pdev)
struct device_node *panel_node;
struct drm_panel *panel;
struct lvds_codec *lvds_codec;
+ u32 val;
int ret;
lvds_codec = devm_kzalloc(dev, sizeof(*lvds_codec), GFP_KERNEL);
@@ -124,6 +126,12 @@ static int lvds_codec_probe(struct platform_device *pdev)
if (IS_ERR(lvds_codec->panel_bridge))
return PTR_ERR(lvds_codec->panel_bridge);
+ if (!of_property_read_u32(dev->of_node, "pixelclk-active", )) {
+ lvds_codec->timings.input_bus_flags = val ?
+ DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE :
+ DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE;
+ }
+
/*
* 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
@@ -131,6 +139,7 @@ static int lvds_codec_probe(struct platform_device *pdev)
*/
lvds_codec->bridge.of_node = dev->of_node;
lvds_codec->bridge.funcs =
+ lvds_codec->bridge.timings = _codec->timings;
drm_bridge_add(_codec->bridge);
platform_set_drvdata(pdev, lvds_codec);
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
