2023-01-22 12:29:00

by Liu Ying

[permalink] [raw]
Subject: [PATCH v2 0/2] drm/bridge: fsl-ldb: Add i.MX93 LDB support

Hi,

This patch set aims to add i.MX93 LVDS Display Bridge(LDB) support in
the existing i.MX8mp LDB DRM bridge driver. Same to i.MX8mp LDB, i.MX93
LDB is controlled by mediamix blk-ctrl through two registers. i.MX93
LDB supports only one LVDS channel(channel 0) while i.MX8mp supports at
most two.

Patch 1/2 adds device tree binding for i.MX93 LDB in the existing
fsl,ldb.yaml.

Patch 2/2 adds i.MX93 LDB support in the existing i.MX8mp LDB DRM bridge
driver.

v1->v2:
* Drop redundant "device tree binding" from patch 1/2's subject. (Krzysztof)
* Add Krzysztof's A-b tag on patch 1/2.

Liu Ying (2):
dt-bindings: display: bridge: ldb: Add i.MX93 LDB
drm/bridge: fsl-ldb: Add i.MX93 LDB support

.../bindings/display/bridge/fsl,ldb.yaml | 16 +++++-
drivers/gpu/drm/bridge/fsl-ldb.c | 53 +++++++++++++++----
2 files changed, 59 insertions(+), 10 deletions(-)

--
2.37.1


2023-01-22 12:36:50

by Liu Ying

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings: display: bridge: ldb: Add i.MX93 LDB

Same to i.MX8mp LDB, i.MX93 LDB is controlled by mediamix blk-ctrl
through 'ldb' register and 'lvds' register. Also, the 'ldb' clock
is required. i.MX93 LDB supports only one LVDS channel(channel 0,
a.k.a, LVDS Channel-A in the device tree binding documentation), while
i.MX8mp LDB supports at most two. Add i.MX93 LDB device tree binding
in the existing i.MX8mp LDB device tree binding documentation.

Acked-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Liu Ying <[email protected]>
---
v1->v2:
* Drop redundant "device tree binding" from patch subject. (Krzysztof)
* Add Krzysztof's A-b tag.

.../bindings/display/bridge/fsl,ldb.yaml | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
index b19be0804abe..6e0e3ba9b49e 100644
--- a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml
@@ -16,7 +16,9 @@ description: |

properties:
compatible:
- const: fsl,imx8mp-ldb
+ enum:
+ - fsl,imx8mp-ldb
+ - fsl,imx93-ldb

clocks:
maxItems: 1
@@ -57,6 +59,18 @@ required:
- clocks
- ports

+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: fsl,imx93-ldb
+ then:
+ properties:
+ ports:
+ properties:
+ port@2: false
+
additionalProperties: false

examples:
--
2.37.1

2023-01-22 12:17:55

by Liu Ying

[permalink] [raw]
Subject: [PATCH v2 2/2] drm/bridge: fsl-ldb: Add i.MX93 LDB support

Same to i.MX8mp LDB, i.MX93 LDB is controlled by mediamix blk-ctrl
through LDB_CTRL and LVDS_CTRL registers. i.MX93 LDB supports only
one LVDS channel(channel 0) and it's LVDS_CTRL register bit1 is used
as LVDS_EN instead of CH1_EN. Add i.MX93 LDB support in the existing
i.MX8mp LDB bridge driver by adding i.MX93 LDB compatible string and
device data(to reflect different register offsets and LVDS_CTRL register
bit1 definition).

Signed-off-by: Liu Ying <[email protected]>
---
v1->v2:
* No change.

drivers/gpu/drm/bridge/fsl-ldb.c | 53 ++++++++++++++++++++++++++------
1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
index 9bcba8fc57e7..6ad63ac7367c 100644
--- a/drivers/gpu/drm/bridge/fsl-ldb.c
+++ b/drivers/gpu/drm/bridge/fsl-ldb.c
@@ -18,7 +18,6 @@
#include <drm/drm_of.h>
#include <drm/drm_panel.h>

-#define LDB_CTRL 0x5c
#define LDB_CTRL_CH0_ENABLE BIT(0)
#define LDB_CTRL_CH0_DI_SELECT BIT(1)
#define LDB_CTRL_CH1_ENABLE BIT(2)
@@ -35,9 +34,9 @@
#define LDB_CTRL_ASYNC_FIFO_ENABLE BIT(24)
#define LDB_CTRL_ASYNC_FIFO_THRESHOLD_MASK GENMASK(27, 25)

-#define LVDS_CTRL 0x128
#define LVDS_CTRL_CH0_EN BIT(0)
#define LVDS_CTRL_CH1_EN BIT(1)
+#define LVDS_CTRL_LVDS_EN BIT(1)
#define LVDS_CTRL_VBG_EN BIT(2)
#define LVDS_CTRL_HS_EN BIT(3)
#define LVDS_CTRL_PRE_EMPH_EN BIT(4)
@@ -52,6 +51,29 @@
#define LVDS_CTRL_VBG_ADJ(n) (((n) & 0x7) << 17)
#define LVDS_CTRL_VBG_ADJ_MASK GENMASK(19, 17)

+enum fsl_ldb_devtype {
+ IMX8MP_LDB,
+ IMX93_LDB,
+};
+
+struct fsl_ldb_devdata {
+ u32 ldb_ctrl;
+ u32 lvds_ctrl;
+ bool lvds_en_bit;
+};
+
+static const struct fsl_ldb_devdata fsl_ldb_devdata[] = {
+ [IMX8MP_LDB] = {
+ .ldb_ctrl = 0x5c,
+ .lvds_ctrl = 0x128,
+ },
+ [IMX93_LDB] = {
+ .ldb_ctrl = 0x20,
+ .lvds_ctrl = 0x24,
+ .lvds_en_bit = true,
+ },
+};
+
struct fsl_ldb {
struct device *dev;
struct drm_bridge bridge;
@@ -59,6 +81,7 @@ struct fsl_ldb {
struct clk *clk;
struct regmap *regmap;
bool lvds_dual_link;
+ const struct fsl_ldb_devdata *devdata;
};

static inline struct fsl_ldb *to_fsl_ldb(struct drm_bridge *bridge)
@@ -173,12 +196,12 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
reg |= LDB_CTRL_DI1_VSYNC_POLARITY;
}

- regmap_write(fsl_ldb->regmap, LDB_CTRL, reg);
+ regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->ldb_ctrl, reg);

/* Program LVDS_CTRL */
reg = LVDS_CTRL_CC_ADJ(2) | LVDS_CTRL_PRE_EMPH_EN |
LVDS_CTRL_PRE_EMPH_ADJ(3) | LVDS_CTRL_VBG_EN;
- regmap_write(fsl_ldb->regmap, LVDS_CTRL, reg);
+ regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg);

/* Wait for VBG to stabilize. */
usleep_range(15, 20);
@@ -187,7 +210,7 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
if (fsl_ldb->lvds_dual_link)
reg |= LVDS_CTRL_CH1_EN;

- regmap_write(fsl_ldb->regmap, LVDS_CTRL, reg);
+ regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, reg);
}

static void fsl_ldb_atomic_disable(struct drm_bridge *bridge,
@@ -195,9 +218,14 @@ static void fsl_ldb_atomic_disable(struct drm_bridge *bridge,
{
struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge);

- /* Stop both channels. */
- regmap_write(fsl_ldb->regmap, LVDS_CTRL, 0);
- regmap_write(fsl_ldb->regmap, LDB_CTRL, 0);
+ /* Stop channel(s). */
+ if (fsl_ldb->devdata->lvds_en_bit)
+ /* Set LVDS_CTRL_LVDS_EN bit to disable. */
+ regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl,
+ LVDS_CTRL_LVDS_EN);
+ else
+ regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->lvds_ctrl, 0);
+ regmap_write(fsl_ldb->regmap, fsl_ldb->devdata->ldb_ctrl, 0);

clk_disable_unprepare(fsl_ldb->clk);
}
@@ -263,6 +291,10 @@ static int fsl_ldb_probe(struct platform_device *pdev)
if (!fsl_ldb)
return -ENOMEM;

+ fsl_ldb->devdata = of_device_get_match_data(dev);
+ if (!fsl_ldb->devdata)
+ return -EINVAL;
+
fsl_ldb->dev = &pdev->dev;
fsl_ldb->bridge.funcs = &funcs;
fsl_ldb->bridge.of_node = dev->of_node;
@@ -321,7 +353,10 @@ static int fsl_ldb_remove(struct platform_device *pdev)
}

static const struct of_device_id fsl_ldb_match[] = {
- { .compatible = "fsl,imx8mp-ldb", },
+ { .compatible = "fsl,imx8mp-ldb",
+ .data = &fsl_ldb_devdata[IMX8MP_LDB], },
+ { .compatible = "fsl,imx93-ldb",
+ .data = &fsl_ldb_devdata[IMX93_LDB], },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, fsl_ldb_match);
--
2.37.1


2023-01-22 17:14:38

by Marek Vasut

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] drm/bridge: fsl-ldb: Add i.MX93 LDB support

On 1/22/23 13:18, Liu Ying wrote:
> Same to i.MX8mp LDB, i.MX93 LDB is controlled by mediamix blk-ctrl
> through LDB_CTRL and LVDS_CTRL registers. i.MX93 LDB supports only
> one LVDS channel(channel 0) and it's LVDS_CTRL register bit1 is used
> as LVDS_EN instead of CH1_EN. Add i.MX93 LDB support in the existing
> i.MX8mp LDB bridge driver by adding i.MX93 LDB compatible string and
> device data(to reflect different register offsets and LVDS_CTRL register
> bit1 definition).
>
> Signed-off-by: Liu Ying <[email protected]>
> ---
> v1->v2:
> * No change.
>
> drivers/gpu/drm/bridge/fsl-ldb.c | 53 ++++++++++++++++++++++++++------
> 1 file changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> index 9bcba8fc57e7..6ad63ac7367c 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -18,7 +18,6 @@
> #include <drm/drm_of.h>
> #include <drm/drm_panel.h>
>
> -#define LDB_CTRL 0x5c
> #define LDB_CTRL_CH0_ENABLE BIT(0)
> #define LDB_CTRL_CH0_DI_SELECT BIT(1)
> #define LDB_CTRL_CH1_ENABLE BIT(2)
> @@ -35,9 +34,9 @@
> #define LDB_CTRL_ASYNC_FIFO_ENABLE BIT(24)
> #define LDB_CTRL_ASYNC_FIFO_THRESHOLD_MASK GENMASK(27, 25)
>
> -#define LVDS_CTRL 0x128
> #define LVDS_CTRL_CH0_EN BIT(0)
> #define LVDS_CTRL_CH1_EN BIT(1)

It would be good to add a comment here that the bit is poorly named and
that LVDS_CTRL_LVDS_EN=1 means DISABLE, while LVDS_CTRL_LVDS_EN=0 means
ENABLE .

> +#define LVDS_CTRL_LVDS_EN BIT(1)

[...]

With that fixed:

Reviewed-by: Marek Vasut <[email protected]>

Thanks!