Hi,
This patchset adds a few cleanups to the IT66121 HDMI chip driver, and
most importantly adds support for the IT6610 chip.
The driver was tested with both chips, but as I only own a HDMI monitor
without speakers, HDMI audio may not be working on the IT6610.
Cheers,
-Paul
Paul Cercueil (10):
dt-bindings: display: bridge: it66121: Add compatible string for
IT6610
drm: bridge: it66121: Use devm_regulator_bulk_get_enable()
drm: bridge: it66121: Use regmap_noinc_read()
drm: bridge: it66121: Write AVI infoframe with regmap_bulk_write()
drm: bridge: it66121: Fix wait for DDC ready
drm: bridge: it66121: Don't use DDC error IRQs
drm: bridge: it66121: Don't clear DDC FIFO twice
drm: bridge: it66121: Set DDC preamble only once before reading EDID
drm: bridge: it66121: Move VID/PID to new it66121_chip_info structure
drm: bridge: it66121: Add support for the IT6610
.../bindings/display/bridge/ite,it66121.yaml | 4 +-
drivers/gpu/drm/bridge/ite-it66121.c | 315 +++++++++---------
2 files changed, 157 insertions(+), 162 deletions(-)
--
2.35.1
The function it66121_wait_ddc_ready() would previously read the status
register until "true", which means it never actually polled anything and
would just read the register once.
Now, it will properly wait until the DDC hardware is ready or until it
reported an error.
The 'busy' variable was also renamed to 'error' since these bits are set
on error and not when the DDC hardware is busy.
Since the DDC ready function is now working properly, the msleep(20) can
be removed.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 0a4fdfd7af44..bfb9c87a7019 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -440,15 +440,17 @@ static int it66121_configure_afe(struct it66121_ctx *ctx,
static inline int it66121_wait_ddc_ready(struct it66121_ctx *ctx)
{
int ret, val;
- u32 busy = IT66121_DDC_STATUS_NOACK | IT66121_DDC_STATUS_WAIT_BUS |
- IT66121_DDC_STATUS_ARBI_LOSE;
+ u32 error = IT66121_DDC_STATUS_NOACK | IT66121_DDC_STATUS_WAIT_BUS |
+ IT66121_DDC_STATUS_ARBI_LOSE;
+ u32 done = IT66121_DDC_STATUS_TX_DONE;
- ret = regmap_read_poll_timeout(ctx->regmap, IT66121_DDC_STATUS_REG, val, true,
- IT66121_EDID_SLEEP_US, IT66121_EDID_TIMEOUT_US);
+ ret = regmap_read_poll_timeout(ctx->regmap, IT66121_DDC_STATUS_REG, val,
+ val & (error | done), IT66121_EDID_SLEEP_US,
+ IT66121_EDID_TIMEOUT_US);
if (ret)
return ret;
- if (val & busy)
+ if (val & error)
return -EAGAIN;
return 0;
@@ -582,9 +584,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
offset += cnt;
remain -= cnt;
- /* Per programming manual, sleep here before emptying the FIFO */
- msleep(20);
-
ret = it66121_wait_ddc_ready(ctx);
if (ret)
return ret;
--
2.35.1
Add support for the IT6610 HDMI encoder.
The hardware is very similar, and therefore the driver did not require
too many changes. Some bits are only available on the IT66121, and
vice-versa. Also, the IT6610 requires specific polarities on the DE and
pixel lines.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 108 +++++++++++++++++++++------
1 file changed, 86 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 43b027b85b8e..b34860871627 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -68,6 +68,7 @@
#define IT66121_AFE_XP_ENO BIT(4)
#define IT66121_AFE_XP_RESETB BIT(3)
#define IT66121_AFE_XP_PWDI BIT(2)
+#define IT6610_AFE_XP_BYPASS BIT(0)
#define IT66121_AFE_IP_REG 0x64
#define IT66121_AFE_IP_GAINBIT BIT(7)
@@ -284,7 +285,13 @@
#define IT66121_AFE_CLK_HIGH 80000 /* Khz */
+enum chip_id {
+ ID_IT6610,
+ ID_IT66121,
+};
+
struct it66121_chip_info {
+ enum chip_id id;
u16 vid, pid;
};
@@ -391,16 +398,22 @@ static int it66121_configure_afe(struct it66121_ctx *ctx,
ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG,
IT66121_AFE_IP_GAINBIT |
- IT66121_AFE_IP_ER0 |
- IT66121_AFE_IP_EC1,
+ IT66121_AFE_IP_ER0,
IT66121_AFE_IP_GAINBIT);
if (ret)
return ret;
- ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG,
- IT66121_AFE_XP_EC1_LOWCLK, 0x80);
- if (ret)
- return ret;
+ if (ctx->info->id == ID_IT66121) {
+ ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG,
+ IT66121_AFE_IP_EC1, 0);
+ if (ret)
+ return ret;
+
+ ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG,
+ IT66121_AFE_XP_EC1_LOWCLK, 0x80);
+ if (ret)
+ return ret;
+ }
} else {
ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_REG,
IT66121_AFE_XP_GAINBIT |
@@ -411,17 +424,24 @@ static int it66121_configure_afe(struct it66121_ctx *ctx,
ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG,
IT66121_AFE_IP_GAINBIT |
- IT66121_AFE_IP_ER0 |
- IT66121_AFE_IP_EC1, IT66121_AFE_IP_ER0 |
- IT66121_AFE_IP_EC1);
+ IT66121_AFE_IP_ER0,
+ IT66121_AFE_IP_ER0);
if (ret)
return ret;
- ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG,
- IT66121_AFE_XP_EC1_LOWCLK,
- IT66121_AFE_XP_EC1_LOWCLK);
- if (ret)
- return ret;
+ if (ctx->info->id == ID_IT66121) {
+ ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG,
+ IT66121_AFE_IP_EC1,
+ IT66121_AFE_IP_EC1);
+ if (ret)
+ return ret;
+
+ ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG,
+ IT66121_AFE_XP_EC1_LOWCLK,
+ IT66121_AFE_XP_EC1_LOWCLK);
+ if (ret)
+ return ret;
+ }
}
/* Clear reset flags */
@@ -430,6 +450,14 @@ static int it66121_configure_afe(struct it66121_ctx *ctx,
if (ret)
return ret;
+ if (ctx->info->id == ID_IT6610) {
+ ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_REG,
+ IT6610_AFE_XP_BYPASS,
+ IT6610_AFE_XP_BYPASS);
+ if (ret)
+ return ret;
+ }
+
return it66121_fire_afe(ctx);
}
@@ -491,7 +519,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
unsigned int block, size_t len)
{
struct it66121_ctx *ctx = context;
- unsigned int val;
int remain = len;
int offset = 0;
int ret, cnt;
@@ -572,10 +599,12 @@ static int it66121_bridge_attach(struct drm_bridge *bridge,
if (ret)
return ret;
- ret = regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
- IT66121_CLK_BANK_PWROFF_RCLK, 0);
- if (ret)
- return ret;
+ if (ctx->info->id == ID_IT66121) {
+ ret = regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
+ IT66121_CLK_BANK_PWROFF_RCLK, 0);
+ if (ret)
+ return ret;
+ }
ret = regmap_write_bits(ctx->regmap, IT66121_INT_REG,
IT66121_INT_TX_CLK_OFF, 0);
@@ -713,6 +742,24 @@ static void it66121_bridge_disable(struct drm_bridge *bridge,
ctx->connector = NULL;
}
+static int it66121_bridge_check(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
+
+ if (ctx->info->id == ID_IT6610) {
+ /* The IT6610 only supports these settings */
+ bridge_state->input_bus_cfg.flags |= DRM_BUS_FLAG_DE_HIGH |
+ DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
+ bridge_state->input_bus_cfg.flags &=
+ ~DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
+ }
+
+ return 0;
+}
+
static
void it66121_bridge_mode_set(struct drm_bridge *bridge,
const struct drm_display_mode *mode,
@@ -758,9 +805,12 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, IT66121_HDMI_MODE_HDMI))
goto unlock;
- if (regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
- IT66121_CLK_BANK_PWROFF_TXCLK, IT66121_CLK_BANK_PWROFF_TXCLK))
+ if (ctx->info->id == ID_IT66121 &&
+ regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
+ IT66121_CLK_BANK_PWROFF_TXCLK,
+ IT66121_CLK_BANK_PWROFF_TXCLK)) {
goto unlock;
+ }
if (it66121_configure_input(ctx))
goto unlock;
@@ -768,7 +818,11 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
if (it66121_configure_afe(ctx, adjusted_mode))
goto unlock;
- regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG, IT66121_CLK_BANK_PWROFF_TXCLK, 0);
+ if (ctx->info->id == ID_IT66121 &&
+ regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
+ IT66121_CLK_BANK_PWROFF_TXCLK, 0)) {
+ goto unlock;
+ }
unlock:
mutex_unlock(&ctx->lock);
@@ -859,6 +913,7 @@ static const struct drm_bridge_funcs it66121_bridge_funcs = {
.atomic_get_input_bus_fmts = it66121_bridge_atomic_get_input_bus_fmts,
.atomic_enable = it66121_bridge_enable,
.atomic_disable = it66121_bridge_disable,
+ .atomic_check = it66121_bridge_check,
.mode_set = it66121_bridge_mode_set,
.mode_valid = it66121_bridge_mode_valid,
.detect = it66121_bridge_detect,
@@ -1557,17 +1612,26 @@ static void it66121_remove(struct i2c_client *client)
static const struct of_device_id it66121_dt_match[] = {
{ .compatible = "ite,it66121" },
+ { .compatible = "ite,it6610" },
{ }
};
MODULE_DEVICE_TABLE(of, it66121_dt_match);
static const struct it66121_chip_info it66121_chip_info = {
+ .id = ID_IT66121,
.vid = 0x4954,
.pid = 0x0612,
};
+static const struct it66121_chip_info it6610_chip_info = {
+ .id = ID_IT6610,
+ .vid = 0xca00,
+ .pid = 0x0611,
+};
+
static const struct i2c_device_id it66121_id[] = {
{ "it66121", (kernel_ulong_t) &it66121_chip_info },
+ { "it6610", (kernel_ulong_t) &it6610_chip_info },
{ }
};
MODULE_DEVICE_TABLE(i2c, it66121_id);
--
2.35.1
The DDC error IRQs will fire on the IT6610 every time the FIFO is empty,
which is not very helpful. To resolve this, we can simply disable them,
and handle DDC errors in it66121_wait_ddc_ready().
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 49 ++++++----------------------
1 file changed, 10 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index bfb9c87a7019..06fa59ae5ffc 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -515,16 +515,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
offset = (block % 2) * len;
block = block / 2;
- ret = regmap_read(ctx->regmap, IT66121_INT_STATUS1_REG, &val);
- if (ret)
- return ret;
-
- if (val & IT66121_INT_STATUS1_DDC_BUSHANG) {
- ret = it66121_abort_ddc_ops(ctx);
- if (ret)
- return ret;
- }
-
ret = it66121_clear_ddc_fifo(ctx);
if (ret)
return ret;
@@ -545,16 +535,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
if (ret)
return ret;
- ret = regmap_read(ctx->regmap, IT66121_INT_STATUS1_REG, &val);
- if (ret)
- return ret;
-
- if (val & IT66121_INT_STATUS1_DDC_BUSHANG) {
- ret = it66121_abort_ddc_ops(ctx);
- if (ret)
- return ret;
- }
-
ret = it66121_preamble_ddc(ctx);
if (ret)
return ret;
@@ -585,8 +565,10 @@ static int it66121_get_edid_block(void *context, u8 *buf,
remain -= cnt;
ret = it66121_wait_ddc_ready(ctx);
- if (ret)
+ if (ret) {
+ it66121_abort_ddc_ops(ctx);
return ret;
+ }
ret = regmap_noinc_read(ctx->regmap, IT66121_DDC_RD_FIFO_REG,
buf, cnt);
@@ -671,11 +653,7 @@ static int it66121_bridge_attach(struct drm_bridge *bridge,
/* Per programming manual, sleep here for bridge to settle */
msleep(50);
- /* Start interrupts */
- return regmap_write_bits(ctx->regmap, IT66121_INT_MASK1_REG,
- IT66121_INT_MASK1_DDC_NOACK |
- IT66121_INT_MASK1_DDC_FIFOERR |
- IT66121_INT_MASK1_DDC_BUSHANG, 0);
+ return 0;
}
static int it66121_set_mute(struct it66121_ctx *ctx, bool mute)
@@ -926,21 +904,14 @@ static irqreturn_t it66121_irq_threaded_handler(int irq, void *dev_id)
ret = regmap_read(ctx->regmap, IT66121_INT_STATUS1_REG, &val);
if (ret) {
dev_err(dev, "Cannot read STATUS1_REG %d\n", ret);
- } else {
- if (val & IT66121_INT_STATUS1_DDC_FIFOERR)
- it66121_clear_ddc_fifo(ctx);
- if (val & (IT66121_INT_STATUS1_DDC_BUSHANG |
- IT66121_INT_STATUS1_DDC_NOACK))
- it66121_abort_ddc_ops(ctx);
- if (val & IT66121_INT_STATUS1_HPD_STATUS) {
- regmap_write_bits(ctx->regmap, IT66121_INT_CLR1_REG,
- IT66121_INT_CLR1_HPD, IT66121_INT_CLR1_HPD);
+ } else if (val & IT66121_INT_STATUS1_HPD_STATUS) {
+ regmap_write_bits(ctx->regmap, IT66121_INT_CLR1_REG,
+ IT66121_INT_CLR1_HPD, IT66121_INT_CLR1_HPD);
- status = it66121_is_hpd_detect(ctx) ? connector_status_connected
- : connector_status_disconnected;
+ status = it66121_is_hpd_detect(ctx) ? connector_status_connected
+ : connector_status_disconnected;
- event = true;
- }
+ event = true;
}
regmap_write_bits(ctx->regmap, IT66121_SYS_STATUS_REG,
--
2.35.1
Use regmap_noinc_read() instead of reading the data from the DDC FIFO one
byte at a time.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index a698eec8f250..12222840df30 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -589,13 +589,12 @@ static int it66121_get_edid_block(void *context, u8 *buf,
if (ret)
return ret;
- do {
- ret = regmap_read(ctx->regmap, IT66121_DDC_RD_FIFO_REG, &val);
- if (ret)
- return ret;
- *(buf++) = val;
- cnt--;
- } while (cnt > 0);
+ ret = regmap_noinc_read(ctx->regmap, IT66121_DDC_RD_FIFO_REG,
+ buf, cnt);
+ if (ret)
+ return ret;
+
+ buf += cnt;
}
return 0;
--
2.35.1
Since all AVI infoframe registers are contiguous in the address space,
the AVI infoframe can be written in one go with regmap_bulk_write().
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 12222840df30..0a4fdfd7af44 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -773,24 +773,9 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
const struct drm_display_mode *mode,
const struct drm_display_mode *adjusted_mode)
{
- int ret, i;
u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
- const u16 aviinfo_reg[HDMI_AVI_INFOFRAME_SIZE] = {
- IT66121_AVIINFO_DB1_REG,
- IT66121_AVIINFO_DB2_REG,
- IT66121_AVIINFO_DB3_REG,
- IT66121_AVIINFO_DB4_REG,
- IT66121_AVIINFO_DB5_REG,
- IT66121_AVIINFO_DB6_REG,
- IT66121_AVIINFO_DB7_REG,
- IT66121_AVIINFO_DB8_REG,
- IT66121_AVIINFO_DB9_REG,
- IT66121_AVIINFO_DB10_REG,
- IT66121_AVIINFO_DB11_REG,
- IT66121_AVIINFO_DB12_REG,
- IT66121_AVIINFO_DB13_REG
- };
+ int ret;
mutex_lock(&ctx->lock);
@@ -810,10 +795,12 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
}
/* Write new AVI infoframe packet */
- for (i = 0; i < HDMI_AVI_INFOFRAME_SIZE; i++) {
- if (regmap_write(ctx->regmap, aviinfo_reg[i], buf[i + HDMI_INFOFRAME_HEADER_SIZE]))
- goto unlock;
- }
+ ret = regmap_bulk_write(ctx->regmap, IT66121_AVIINFO_DB1_REG,
+ &buf[HDMI_INFOFRAME_HEADER_SIZE],
+ HDMI_AVI_INFOFRAME_SIZE);
+ if (ret)
+ goto unlock;
+
if (regmap_write(ctx->regmap, IT66121_AVIINFO_CSUM_REG, buf[3]))
goto unlock;
--
2.35.1
Add a new ite,it6610 compatible string to the IT66121 binding
documentation, since the two chips are very similar.
Signed-off-by: Paul Cercueil <[email protected]>
---
.../devicetree/bindings/display/bridge/ite,it66121.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
index 1b2185be92cd..72957be0ba3c 100644
--- a/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
@@ -17,7 +17,9 @@ description: |
properties:
compatible:
- const: ite,it66121
+ enum:
+ - ite,it66121
+ - ite,it6610
reg:
maxItems: 1
--
2.35.1
This will make it easier later to introduce support for new chips in
this driver.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 7972003d4776..43b027b85b8e 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -35,10 +35,6 @@
#define IT66121_DEVICE_ID0_REG 0x02
#define IT66121_DEVICE_ID1_REG 0x03
-#define IT66121_VENDOR_ID0 0x54
-#define IT66121_VENDOR_ID1 0x49
-#define IT66121_DEVICE_ID0 0x12
-#define IT66121_DEVICE_ID1 0x06
#define IT66121_REVISION_MASK GENMASK(7, 4)
#define IT66121_DEVICE_ID1_MASK GENMASK(3, 0)
@@ -286,13 +282,12 @@
#define IT66121_AUD_SWL_16BIT 0x2
#define IT66121_AUD_SWL_NOT_INDICATED 0x0
-#define IT66121_VENDOR_ID0 0x54
-#define IT66121_VENDOR_ID1 0x49
-#define IT66121_DEVICE_ID0 0x12
-#define IT66121_DEVICE_ID1 0x06
-#define IT66121_DEVICE_MASK 0x0F
#define IT66121_AFE_CLK_HIGH 80000 /* Khz */
+struct it66121_chip_info {
+ u16 vid, pid;
+};
+
struct it66121_ctx {
struct regmap *regmap;
struct drm_bridge bridge;
@@ -311,6 +306,7 @@ struct it66121_ctx {
u8 swl;
bool auto_cts;
} audio;
+ const struct it66121_chip_info *info;
};
static const struct regmap_range_cfg it66121_regmap_banks[] = {
@@ -1451,6 +1447,7 @@ static const char * const it66121_supplies[] = {
static int it66121_probe(struct i2c_client *client)
{
+ const struct i2c_device_id *id = i2c_client_get_device_id(client);
u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 };
struct device_node *ep;
int ret;
@@ -1472,6 +1469,7 @@ static int it66121_probe(struct i2c_client *client)
ctx->dev = dev;
ctx->client = client;
+ ctx->info = (const struct it66121_chip_info *) id->driver_data;
of_property_read_u32(ep, "bus-width", &ctx->bus_width);
of_node_put(ep);
@@ -1523,8 +1521,8 @@ static int it66121_probe(struct i2c_client *client)
revision_id = FIELD_GET(IT66121_REVISION_MASK, device_ids[1]);
device_ids[1] &= IT66121_DEVICE_ID1_MASK;
- if (vendor_ids[0] != IT66121_VENDOR_ID0 || vendor_ids[1] != IT66121_VENDOR_ID1 ||
- device_ids[0] != IT66121_DEVICE_ID0 || device_ids[1] != IT66121_DEVICE_ID1) {
+ if ((vendor_ids[1] << 8 | vendor_ids[0]) != ctx->info->vid ||
+ (device_ids[1] << 8 | device_ids[0]) != ctx->info->pid) {
return -ENODEV;
}
@@ -1563,8 +1561,13 @@ static const struct of_device_id it66121_dt_match[] = {
};
MODULE_DEVICE_TABLE(of, it66121_dt_match);
+static const struct it66121_chip_info it66121_chip_info = {
+ .vid = 0x4954,
+ .pid = 0x0612,
+};
+
static const struct i2c_device_id it66121_id[] = {
- { "it66121", 0 },
+ { "it66121", (kernel_ulong_t) &it66121_chip_info },
{ }
};
MODULE_DEVICE_TABLE(i2c, it66121_id);
--
2.35.1
The DDC FIFO was cleared before the loop in it66121_get_edid_block(),
and at the beginning of each iteration; which means that it did not have
to be cleared before the loop.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 06fa59ae5ffc..5335d4abd7c5 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -456,18 +456,6 @@ static inline int it66121_wait_ddc_ready(struct it66121_ctx *ctx)
return 0;
}
-static int it66121_clear_ddc_fifo(struct it66121_ctx *ctx)
-{
- int ret;
-
- ret = it66121_preamble_ddc(ctx);
- if (ret)
- return ret;
-
- return regmap_write(ctx->regmap, IT66121_DDC_COMMAND_REG,
- IT66121_DDC_COMMAND_FIFO_CLR);
-}
-
static int it66121_abort_ddc_ops(struct it66121_ctx *ctx)
{
int ret;
@@ -515,10 +503,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
offset = (block % 2) * len;
block = block / 2;
- ret = it66121_clear_ddc_fifo(ctx);
- if (ret)
- return ret;
-
while (remain > 0) {
cnt = (remain > IT66121_EDID_FIFO_SIZE) ?
IT66121_EDID_FIFO_SIZE : remain;
--
2.35.1
Simplify the code of the driver by using
devm_regulator_bulk_get_enable(), which will handle powering up the
regulators, and disabling them on probe error or module removal.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 34 +++++++---------------------
1 file changed, 8 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 7476cfbf9585..a698eec8f250 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -301,7 +301,6 @@ struct it66121_ctx {
struct device *dev;
struct gpio_desc *gpio_reset;
struct i2c_client *client;
- struct regulator_bulk_data supplies[3];
u32 bus_width;
struct mutex lock; /* Protects fields below and device registers */
struct hdmi_avi_infoframe hdmi_avi_infoframe;
@@ -342,16 +341,6 @@ static void it66121_hw_reset(struct it66121_ctx *ctx)
gpiod_set_value(ctx->gpio_reset, 0);
}
-static inline int ite66121_power_on(struct it66121_ctx *ctx)
-{
- return regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
-}
-
-static inline int ite66121_power_off(struct it66121_ctx *ctx)
-{
- return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
-}
-
static inline int it66121_preamble_ddc(struct it66121_ctx *ctx)
{
return regmap_write(ctx->regmap, IT66121_MASTER_SEL_REG, IT66121_MASTER_SEL_HOST);
@@ -1512,6 +1501,10 @@ static int it66121_audio_codec_init(struct it66121_ctx *ctx, struct device *dev)
return PTR_ERR_OR_ZERO(ctx->audio.pdev);
}
+static const char * const it66121_supplies[] = {
+ "vcn33", "vcn18", "vrf12"
+};
+
static int it66121_probe(struct i2c_client *client)
{
u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 };
@@ -1564,26 +1557,18 @@ static int it66121_probe(struct i2c_client *client)
i2c_set_clientdata(client, ctx);
mutex_init(&ctx->lock);
- ctx->supplies[0].supply = "vcn33";
- ctx->supplies[1].supply = "vcn18";
- ctx->supplies[2].supply = "vrf12";
- ret = devm_regulator_bulk_get(ctx->dev, 3, ctx->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(it66121_supplies),
+ it66121_supplies);
if (ret) {
- dev_err(ctx->dev, "regulator_bulk failed\n");
+ dev_err(dev, "Failed to enable power supplies\n");
return ret;
}
- ret = ite66121_power_on(ctx);
- if (ret)
- return ret;
-
it66121_hw_reset(ctx);
ctx->regmap = devm_regmap_init_i2c(client, &it66121_regmap_config);
- if (IS_ERR(ctx->regmap)) {
- ite66121_power_off(ctx);
+ if (IS_ERR(ctx->regmap))
return PTR_ERR(ctx->regmap);
- }
regmap_read(ctx->regmap, IT66121_VENDOR_ID0_REG, &vendor_ids[0]);
regmap_read(ctx->regmap, IT66121_VENDOR_ID1_REG, &vendor_ids[1]);
@@ -1596,7 +1581,6 @@ static int it66121_probe(struct i2c_client *client)
if (vendor_ids[0] != IT66121_VENDOR_ID0 || vendor_ids[1] != IT66121_VENDOR_ID1 ||
device_ids[0] != IT66121_DEVICE_ID0 || device_ids[1] != IT66121_DEVICE_ID1) {
- ite66121_power_off(ctx);
return -ENODEV;
}
@@ -1609,7 +1593,6 @@ static int it66121_probe(struct i2c_client *client)
IRQF_ONESHOT, dev_name(dev), ctx);
if (ret < 0) {
dev_err(dev, "Failed to request irq %d:%d\n", client->irq, ret);
- ite66121_power_off(ctx);
return ret;
}
@@ -1626,7 +1609,6 @@ static void it66121_remove(struct i2c_client *client)
{
struct it66121_ctx *ctx = i2c_get_clientdata(client);
- ite66121_power_off(ctx);
drm_bridge_remove(&ctx->bridge);
mutex_destroy(&ctx->lock);
}
--
2.35.1
The DDC preamble and target address only need to be set once before
reading the EDID, even if multiple segments have to be read.
Signed-off-by: Paul Cercueil <[email protected]>
---
drivers/gpu/drm/bridge/ite-it66121.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 5335d4abd7c5..7972003d4776 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -506,9 +506,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
while (remain > 0) {
cnt = (remain > IT66121_EDID_FIFO_SIZE) ?
IT66121_EDID_FIFO_SIZE : remain;
- ret = it66121_preamble_ddc(ctx);
- if (ret)
- return ret;
ret = regmap_write(ctx->regmap, IT66121_DDC_COMMAND_REG,
IT66121_DDC_COMMAND_FIFO_CLR);
@@ -519,15 +516,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
if (ret)
return ret;
- ret = it66121_preamble_ddc(ctx);
- if (ret)
- return ret;
-
- ret = regmap_write(ctx->regmap, IT66121_DDC_HEADER_REG,
- IT66121_DDC_HEADER_EDID);
- if (ret)
- return ret;
-
ret = regmap_write(ctx->regmap, IT66121_DDC_OFFSET_REG, offset);
if (ret)
return ret;
@@ -842,9 +830,25 @@ static struct edid *it66121_bridge_get_edid(struct drm_bridge *bridge,
{
struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
struct edid *edid;
+ int ret;
mutex_lock(&ctx->lock);
+ ret = it66121_preamble_ddc(ctx);
+ if (ret) {
+ edid = ERR_PTR(ret);
+ goto out_unlock;
+ }
+
+ ret = regmap_write(ctx->regmap, IT66121_DDC_HEADER_REG,
+ IT66121_DDC_HEADER_EDID);
+ if (ret) {
+ edid = ERR_PTR(ret);
+ goto out_unlock;
+ }
+
edid = drm_do_get_edid(connector, it66121_get_edid_block, ctx);
+
+out_unlock:
mutex_unlock(&ctx->lock);
return edid;
--
2.35.1
On Wed, 14 Dec 2022 at 13:58, Paul Cercueil <[email protected]> wrote:
>
> Simplify the code of the driver by using
> devm_regulator_bulk_get_enable(), which will handle powering up the
> regulators, and disabling them on probe error or module removal.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 34 +++++++---------------------
> 1 file changed, 8 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 7476cfbf9585..a698eec8f250 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -301,7 +301,6 @@ struct it66121_ctx {
> struct device *dev;
> struct gpio_desc *gpio_reset;
> struct i2c_client *client;
> - struct regulator_bulk_data supplies[3];
> u32 bus_width;
> struct mutex lock; /* Protects fields below and device registers */
> struct hdmi_avi_infoframe hdmi_avi_infoframe;
> @@ -342,16 +341,6 @@ static void it66121_hw_reset(struct it66121_ctx *ctx)
> gpiod_set_value(ctx->gpio_reset, 0);
> }
>
> -static inline int ite66121_power_on(struct it66121_ctx *ctx)
> -{
> - return regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> -}
> -
> -static inline int ite66121_power_off(struct it66121_ctx *ctx)
> -{
> - return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> -}
> -
> static inline int it66121_preamble_ddc(struct it66121_ctx *ctx)
> {
> return regmap_write(ctx->regmap, IT66121_MASTER_SEL_REG, IT66121_MASTER_SEL_HOST);
> @@ -1512,6 +1501,10 @@ static int it66121_audio_codec_init(struct it66121_ctx *ctx, struct device *dev)
> return PTR_ERR_OR_ZERO(ctx->audio.pdev);
> }
>
> +static const char * const it66121_supplies[] = {
> + "vcn33", "vcn18", "vrf12"
> +};
> +
> static int it66121_probe(struct i2c_client *client)
> {
> u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 };
> @@ -1564,26 +1557,18 @@ static int it66121_probe(struct i2c_client *client)
> i2c_set_clientdata(client, ctx);
> mutex_init(&ctx->lock);
>
> - ctx->supplies[0].supply = "vcn33";
> - ctx->supplies[1].supply = "vcn18";
> - ctx->supplies[2].supply = "vrf12";
> - ret = devm_regulator_bulk_get(ctx->dev, 3, ctx->supplies);
> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(it66121_supplies),
> + it66121_supplies);
> if (ret) {
> - dev_err(ctx->dev, "regulator_bulk failed\n");
> + dev_err(dev, "Failed to enable power supplies\n");
> return ret;
> }
>
> - ret = ite66121_power_on(ctx);
> - if (ret)
> - return ret;
> -
> it66121_hw_reset(ctx);
>
> ctx->regmap = devm_regmap_init_i2c(client, &it66121_regmap_config);
> - if (IS_ERR(ctx->regmap)) {
> - ite66121_power_off(ctx);
> + if (IS_ERR(ctx->regmap))
> return PTR_ERR(ctx->regmap);
> - }
>
> regmap_read(ctx->regmap, IT66121_VENDOR_ID0_REG, &vendor_ids[0]);
> regmap_read(ctx->regmap, IT66121_VENDOR_ID1_REG, &vendor_ids[1]);
> @@ -1596,7 +1581,6 @@ static int it66121_probe(struct i2c_client *client)
>
> if (vendor_ids[0] != IT66121_VENDOR_ID0 || vendor_ids[1] != IT66121_VENDOR_ID1 ||
> device_ids[0] != IT66121_DEVICE_ID0 || device_ids[1] != IT66121_DEVICE_ID1) {
> - ite66121_power_off(ctx);
> return -ENODEV;
> }
>
> @@ -1609,7 +1593,6 @@ static int it66121_probe(struct i2c_client *client)
> IRQF_ONESHOT, dev_name(dev), ctx);
> if (ret < 0) {
> dev_err(dev, "Failed to request irq %d:%d\n", client->irq, ret);
> - ite66121_power_off(ctx);
> return ret;
> }
>
> @@ -1626,7 +1609,6 @@ static void it66121_remove(struct i2c_client *client)
> {
> struct it66121_ctx *ctx = i2c_get_clientdata(client);
>
> - ite66121_power_off(ctx);
> drm_bridge_remove(&ctx->bridge);
> mutex_destroy(&ctx->lock);
> }
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
On Wed, 14 Dec 2022 at 13:58, Paul Cercueil <[email protected]> wrote:
>
> Use regmap_noinc_read() instead of reading the data from the DDC FIFO one
> byte at a time.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index a698eec8f250..12222840df30 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -589,13 +589,12 @@ static int it66121_get_edid_block(void *context, u8 *buf,
> if (ret)
> return ret;
>
> - do {
> - ret = regmap_read(ctx->regmap, IT66121_DDC_RD_FIFO_REG, &val);
> - if (ret)
> - return ret;
> - *(buf++) = val;
> - cnt--;
> - } while (cnt > 0);
> + ret = regmap_noinc_read(ctx->regmap, IT66121_DDC_RD_FIFO_REG,
> + buf, cnt);
> + if (ret)
> + return ret;
> +
> + buf += cnt;
> }
>
> return 0;
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
On Wed, 14 Dec 2022 at 14:01, Paul Cercueil <[email protected]> wrote:
>
> Add support for the IT6610 HDMI encoder.
>
> The hardware is very similar, and therefore the driver did not require
> too many changes. Some bits are only available on the IT66121, and
> vice-versa. Also, the IT6610 requires specific polarities on the DE and
> pixel lines.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 108 +++++++++++++++++++++------
> 1 file changed, 86 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 43b027b85b8e..b34860871627 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -68,6 +68,7 @@
> #define IT66121_AFE_XP_ENO BIT(4)
> #define IT66121_AFE_XP_RESETB BIT(3)
> #define IT66121_AFE_XP_PWDI BIT(2)
> +#define IT6610_AFE_XP_BYPASS BIT(0)
>
> #define IT66121_AFE_IP_REG 0x64
> #define IT66121_AFE_IP_GAINBIT BIT(7)
> @@ -284,7 +285,13 @@
>
> #define IT66121_AFE_CLK_HIGH 80000 /* Khz */
>
> +enum chip_id {
> + ID_IT6610,
> + ID_IT66121,
> +};
> +
> struct it66121_chip_info {
> + enum chip_id id;
> u16 vid, pid;
> };
>
> @@ -391,16 +398,22 @@ static int it66121_configure_afe(struct it66121_ctx *ctx,
>
> ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG,
> IT66121_AFE_IP_GAINBIT |
> - IT66121_AFE_IP_ER0 |
> - IT66121_AFE_IP_EC1,
> + IT66121_AFE_IP_ER0,
> IT66121_AFE_IP_GAINBIT);
> if (ret)
> return ret;
>
> - ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG,
> - IT66121_AFE_XP_EC1_LOWCLK, 0x80);
> - if (ret)
> - return ret;
> + if (ctx->info->id == ID_IT66121) {
> + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG,
> + IT66121_AFE_IP_EC1, 0);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG,
> + IT66121_AFE_XP_EC1_LOWCLK, 0x80);
> + if (ret)
> + return ret;
> + }
> } else {
> ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_REG,
> IT66121_AFE_XP_GAINBIT |
> @@ -411,17 +424,24 @@ static int it66121_configure_afe(struct it66121_ctx *ctx,
>
> ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG,
> IT66121_AFE_IP_GAINBIT |
> - IT66121_AFE_IP_ER0 |
> - IT66121_AFE_IP_EC1, IT66121_AFE_IP_ER0 |
> - IT66121_AFE_IP_EC1);
> + IT66121_AFE_IP_ER0,
> + IT66121_AFE_IP_ER0);
> if (ret)
> return ret;
>
> - ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG,
> - IT66121_AFE_XP_EC1_LOWCLK,
> - IT66121_AFE_XP_EC1_LOWCLK);
> - if (ret)
> - return ret;
> + if (ctx->info->id == ID_IT66121) {
> + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG,
> + IT66121_AFE_IP_EC1,
> + IT66121_AFE_IP_EC1);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG,
> + IT66121_AFE_XP_EC1_LOWCLK,
> + IT66121_AFE_XP_EC1_LOWCLK);
> + if (ret)
> + return ret;
> + }
> }
>
> /* Clear reset flags */
> @@ -430,6 +450,14 @@ static int it66121_configure_afe(struct it66121_ctx *ctx,
> if (ret)
> return ret;
>
> + if (ctx->info->id == ID_IT6610) {
> + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_REG,
> + IT6610_AFE_XP_BYPASS,
> + IT6610_AFE_XP_BYPASS);
> + if (ret)
> + return ret;
> + }
> +
> return it66121_fire_afe(ctx);
> }
>
> @@ -491,7 +519,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
> unsigned int block, size_t len)
> {
> struct it66121_ctx *ctx = context;
> - unsigned int val;
> int remain = len;
> int offset = 0;
> int ret, cnt;
> @@ -572,10 +599,12 @@ static int it66121_bridge_attach(struct drm_bridge *bridge,
> if (ret)
> return ret;
>
> - ret = regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
> - IT66121_CLK_BANK_PWROFF_RCLK, 0);
> - if (ret)
> - return ret;
> + if (ctx->info->id == ID_IT66121) {
> + ret = regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
> + IT66121_CLK_BANK_PWROFF_RCLK, 0);
> + if (ret)
> + return ret;
> + }
>
> ret = regmap_write_bits(ctx->regmap, IT66121_INT_REG,
> IT66121_INT_TX_CLK_OFF, 0);
> @@ -713,6 +742,24 @@ static void it66121_bridge_disable(struct drm_bridge *bridge,
> ctx->connector = NULL;
> }
>
> +static int it66121_bridge_check(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> +{
> + struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
> +
> + if (ctx->info->id == ID_IT6610) {
> + /* The IT6610 only supports these settings */
> + bridge_state->input_bus_cfg.flags |= DRM_BUS_FLAG_DE_HIGH |
> + DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE;
> + bridge_state->input_bus_cfg.flags &=
> + ~DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE;
> + }
> +
> + return 0;
> +}
> +
> static
> void it66121_bridge_mode_set(struct drm_bridge *bridge,
> const struct drm_display_mode *mode,
> @@ -758,9 +805,12 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
> if (regmap_write(ctx->regmap, IT66121_HDMI_MODE_REG, IT66121_HDMI_MODE_HDMI))
> goto unlock;
>
> - if (regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
> - IT66121_CLK_BANK_PWROFF_TXCLK, IT66121_CLK_BANK_PWROFF_TXCLK))
> + if (ctx->info->id == ID_IT66121 &&
> + regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
> + IT66121_CLK_BANK_PWROFF_TXCLK,
> + IT66121_CLK_BANK_PWROFF_TXCLK)) {
> goto unlock;
> + }
>
> if (it66121_configure_input(ctx))
> goto unlock;
> @@ -768,7 +818,11 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
> if (it66121_configure_afe(ctx, adjusted_mode))
> goto unlock;
>
> - regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG, IT66121_CLK_BANK_PWROFF_TXCLK, 0);
> + if (ctx->info->id == ID_IT66121 &&
> + regmap_write_bits(ctx->regmap, IT66121_CLK_BANK_REG,
> + IT66121_CLK_BANK_PWROFF_TXCLK, 0)) {
> + goto unlock;
> + }
>
> unlock:
> mutex_unlock(&ctx->lock);
> @@ -859,6 +913,7 @@ static const struct drm_bridge_funcs it66121_bridge_funcs = {
> .atomic_get_input_bus_fmts = it66121_bridge_atomic_get_input_bus_fmts,
> .atomic_enable = it66121_bridge_enable,
> .atomic_disable = it66121_bridge_disable,
> + .atomic_check = it66121_bridge_check,
> .mode_set = it66121_bridge_mode_set,
> .mode_valid = it66121_bridge_mode_valid,
> .detect = it66121_bridge_detect,
> @@ -1557,17 +1612,26 @@ static void it66121_remove(struct i2c_client *client)
>
> static const struct of_device_id it66121_dt_match[] = {
> { .compatible = "ite,it66121" },
> + { .compatible = "ite,it6610" },
> { }
> };
> MODULE_DEVICE_TABLE(of, it66121_dt_match);
>
> static const struct it66121_chip_info it66121_chip_info = {
> + .id = ID_IT66121,
> .vid = 0x4954,
> .pid = 0x0612,
> };
>
> +static const struct it66121_chip_info it6610_chip_info = {
> + .id = ID_IT6610,
> + .vid = 0xca00,
> + .pid = 0x0611,
> +};
> +
> static const struct i2c_device_id it66121_id[] = {
> { "it66121", (kernel_ulong_t) &it66121_chip_info },
> + { "it6610", (kernel_ulong_t) &it6610_chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, it66121_id);
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
On Wed, 14 Dec 2022 at 13:59, Paul Cercueil <[email protected]> wrote:
>
> The DDC FIFO was cleared before the loop in it66121_get_edid_block(),
> and at the beginning of each iteration; which means that it did not have
> to be cleared before the loop.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 16 ----------------
> 1 file changed, 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 06fa59ae5ffc..5335d4abd7c5 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -456,18 +456,6 @@ static inline int it66121_wait_ddc_ready(struct it66121_ctx *ctx)
> return 0;
> }
>
> -static int it66121_clear_ddc_fifo(struct it66121_ctx *ctx)
> -{
> - int ret;
> -
> - ret = it66121_preamble_ddc(ctx);
> - if (ret)
> - return ret;
> -
> - return regmap_write(ctx->regmap, IT66121_DDC_COMMAND_REG,
> - IT66121_DDC_COMMAND_FIFO_CLR);
> -}
> -
> static int it66121_abort_ddc_ops(struct it66121_ctx *ctx)
> {
> int ret;
> @@ -515,10 +503,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
> offset = (block % 2) * len;
> block = block / 2;
>
> - ret = it66121_clear_ddc_fifo(ctx);
> - if (ret)
> - return ret;
> -
> while (remain > 0) {
> cnt = (remain > IT66121_EDID_FIFO_SIZE) ?
> IT66121_EDID_FIFO_SIZE : remain;
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
On Wed, 14 Dec 2022 at 13:59, Paul Cercueil <[email protected]> wrote:
>
> The DDC preamble and target address only need to be set once before
> reading the EDID, even if multiple segments have to be read.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 5335d4abd7c5..7972003d4776 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -506,9 +506,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
> while (remain > 0) {
> cnt = (remain > IT66121_EDID_FIFO_SIZE) ?
> IT66121_EDID_FIFO_SIZE : remain;
> - ret = it66121_preamble_ddc(ctx);
> - if (ret)
> - return ret;
>
> ret = regmap_write(ctx->regmap, IT66121_DDC_COMMAND_REG,
> IT66121_DDC_COMMAND_FIFO_CLR);
> @@ -519,15 +516,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
> if (ret)
> return ret;
>
> - ret = it66121_preamble_ddc(ctx);
> - if (ret)
> - return ret;
> -
> - ret = regmap_write(ctx->regmap, IT66121_DDC_HEADER_REG,
> - IT66121_DDC_HEADER_EDID);
> - if (ret)
> - return ret;
> -
> ret = regmap_write(ctx->regmap, IT66121_DDC_OFFSET_REG, offset);
> if (ret)
> return ret;
> @@ -842,9 +830,25 @@ static struct edid *it66121_bridge_get_edid(struct drm_bridge *bridge,
> {
> struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
> struct edid *edid;
> + int ret;
>
> mutex_lock(&ctx->lock);
> + ret = it66121_preamble_ddc(ctx);
> + if (ret) {
> + edid = ERR_PTR(ret);
> + goto out_unlock;
> + }
> +
> + ret = regmap_write(ctx->regmap, IT66121_DDC_HEADER_REG,
> + IT66121_DDC_HEADER_EDID);
> + if (ret) {
> + edid = ERR_PTR(ret);
> + goto out_unlock;
> + }
> +
> edid = drm_do_get_edid(connector, it66121_get_edid_block, ctx);
> +
> +out_unlock:
> mutex_unlock(&ctx->lock);
>
> return edid;
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
On Wed, 14 Dec 2022 at 14:01, Paul Cercueil <[email protected]> wrote:
>
> This will make it easier later to introduce support for new chips in
> this driver.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 7972003d4776..43b027b85b8e 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -35,10 +35,6 @@
> #define IT66121_DEVICE_ID0_REG 0x02
> #define IT66121_DEVICE_ID1_REG 0x03
>
> -#define IT66121_VENDOR_ID0 0x54
> -#define IT66121_VENDOR_ID1 0x49
> -#define IT66121_DEVICE_ID0 0x12
> -#define IT66121_DEVICE_ID1 0x06
> #define IT66121_REVISION_MASK GENMASK(7, 4)
> #define IT66121_DEVICE_ID1_MASK GENMASK(3, 0)
>
> @@ -286,13 +282,12 @@
> #define IT66121_AUD_SWL_16BIT 0x2
> #define IT66121_AUD_SWL_NOT_INDICATED 0x0
>
> -#define IT66121_VENDOR_ID0 0x54
> -#define IT66121_VENDOR_ID1 0x49
> -#define IT66121_DEVICE_ID0 0x12
> -#define IT66121_DEVICE_ID1 0x06
> -#define IT66121_DEVICE_MASK 0x0F
> #define IT66121_AFE_CLK_HIGH 80000 /* Khz */
>
> +struct it66121_chip_info {
> + u16 vid, pid;
> +};
> +
> struct it66121_ctx {
> struct regmap *regmap;
> struct drm_bridge bridge;
> @@ -311,6 +306,7 @@ struct it66121_ctx {
> u8 swl;
> bool auto_cts;
> } audio;
> + const struct it66121_chip_info *info;
> };
>
> static const struct regmap_range_cfg it66121_regmap_banks[] = {
> @@ -1451,6 +1447,7 @@ static const char * const it66121_supplies[] = {
>
> static int it66121_probe(struct i2c_client *client)
> {
> + const struct i2c_device_id *id = i2c_client_get_device_id(client);
> u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 };
> struct device_node *ep;
> int ret;
> @@ -1472,6 +1469,7 @@ static int it66121_probe(struct i2c_client *client)
>
> ctx->dev = dev;
> ctx->client = client;
> + ctx->info = (const struct it66121_chip_info *) id->driver_data;
>
> of_property_read_u32(ep, "bus-width", &ctx->bus_width);
> of_node_put(ep);
> @@ -1523,8 +1521,8 @@ static int it66121_probe(struct i2c_client *client)
> revision_id = FIELD_GET(IT66121_REVISION_MASK, device_ids[1]);
> device_ids[1] &= IT66121_DEVICE_ID1_MASK;
>
> - if (vendor_ids[0] != IT66121_VENDOR_ID0 || vendor_ids[1] != IT66121_VENDOR_ID1 ||
> - device_ids[0] != IT66121_DEVICE_ID0 || device_ids[1] != IT66121_DEVICE_ID1) {
> + if ((vendor_ids[1] << 8 | vendor_ids[0]) != ctx->info->vid ||
> + (device_ids[1] << 8 | device_ids[0]) != ctx->info->pid) {
> return -ENODEV;
> }
>
> @@ -1563,8 +1561,13 @@ static const struct of_device_id it66121_dt_match[] = {
> };
> MODULE_DEVICE_TABLE(of, it66121_dt_match);
>
> +static const struct it66121_chip_info it66121_chip_info = {
> + .vid = 0x4954,
> + .pid = 0x0612,
> +};
> +
> static const struct i2c_device_id it66121_id[] = {
> - { "it66121", 0 },
> + { "it66121", (kernel_ulong_t) &it66121_chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, it66121_id);
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
On Wed, 14 Dec 2022 at 13:58, Paul Cercueil <[email protected]> wrote:
>
> Add a new ite,it6610 compatible string to the IT66121 binding
> documentation, since the two chips are very similar.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> .../devicetree/bindings/display/bridge/ite,it66121.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
> index 1b2185be92cd..72957be0ba3c 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
> @@ -17,7 +17,9 @@ description: |
>
> properties:
> compatible:
> - const: ite,it66121
> + enum:
> + - ite,it66121
> + - ite,it6610
>
> reg:
> maxItems: 1
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
On Wed, 14 Dec 2022 at 13:59, Paul Cercueil <[email protected]> wrote:
>
> The DDC error IRQs will fire on the IT6610 every time the FIFO is empty,
> which is not very helpful. To resolve this, we can simply disable them,
> and handle DDC errors in it66121_wait_ddc_ready().
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 49 ++++++----------------------
> 1 file changed, 10 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index bfb9c87a7019..06fa59ae5ffc 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -515,16 +515,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
> offset = (block % 2) * len;
> block = block / 2;
>
> - ret = regmap_read(ctx->regmap, IT66121_INT_STATUS1_REG, &val);
> - if (ret)
> - return ret;
> -
> - if (val & IT66121_INT_STATUS1_DDC_BUSHANG) {
> - ret = it66121_abort_ddc_ops(ctx);
> - if (ret)
> - return ret;
> - }
> -
> ret = it66121_clear_ddc_fifo(ctx);
> if (ret)
> return ret;
> @@ -545,16 +535,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
> if (ret)
> return ret;
>
> - ret = regmap_read(ctx->regmap, IT66121_INT_STATUS1_REG, &val);
> - if (ret)
> - return ret;
> -
> - if (val & IT66121_INT_STATUS1_DDC_BUSHANG) {
> - ret = it66121_abort_ddc_ops(ctx);
> - if (ret)
> - return ret;
> - }
> -
> ret = it66121_preamble_ddc(ctx);
> if (ret)
> return ret;
> @@ -585,8 +565,10 @@ static int it66121_get_edid_block(void *context, u8 *buf,
> remain -= cnt;
>
> ret = it66121_wait_ddc_ready(ctx);
> - if (ret)
> + if (ret) {
> + it66121_abort_ddc_ops(ctx);
> return ret;
> + }
>
> ret = regmap_noinc_read(ctx->regmap, IT66121_DDC_RD_FIFO_REG,
> buf, cnt);
> @@ -671,11 +653,7 @@ static int it66121_bridge_attach(struct drm_bridge *bridge,
> /* Per programming manual, sleep here for bridge to settle */
> msleep(50);
>
> - /* Start interrupts */
> - return regmap_write_bits(ctx->regmap, IT66121_INT_MASK1_REG,
> - IT66121_INT_MASK1_DDC_NOACK |
> - IT66121_INT_MASK1_DDC_FIFOERR |
> - IT66121_INT_MASK1_DDC_BUSHANG, 0);
> + return 0;
> }
>
> static int it66121_set_mute(struct it66121_ctx *ctx, bool mute)
> @@ -926,21 +904,14 @@ static irqreturn_t it66121_irq_threaded_handler(int irq, void *dev_id)
> ret = regmap_read(ctx->regmap, IT66121_INT_STATUS1_REG, &val);
> if (ret) {
> dev_err(dev, "Cannot read STATUS1_REG %d\n", ret);
> - } else {
> - if (val & IT66121_INT_STATUS1_DDC_FIFOERR)
> - it66121_clear_ddc_fifo(ctx);
> - if (val & (IT66121_INT_STATUS1_DDC_BUSHANG |
> - IT66121_INT_STATUS1_DDC_NOACK))
> - it66121_abort_ddc_ops(ctx);
> - if (val & IT66121_INT_STATUS1_HPD_STATUS) {
> - regmap_write_bits(ctx->regmap, IT66121_INT_CLR1_REG,
> - IT66121_INT_CLR1_HPD, IT66121_INT_CLR1_HPD);
> + } else if (val & IT66121_INT_STATUS1_HPD_STATUS) {
> + regmap_write_bits(ctx->regmap, IT66121_INT_CLR1_REG,
> + IT66121_INT_CLR1_HPD, IT66121_INT_CLR1_HPD);
>
> - status = it66121_is_hpd_detect(ctx) ? connector_status_connected
> - : connector_status_disconnected;
> + status = it66121_is_hpd_detect(ctx) ? connector_status_connected
> + : connector_status_disconnected;
>
> - event = true;
> - }
> + event = true;
> }
>
> regmap_write_bits(ctx->regmap, IT66121_SYS_STATUS_REG,
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
On Wed, 14 Dec 2022 at 13:58, Paul Cercueil <[email protected]> wrote:
>
> Since all AVI infoframe registers are contiguous in the address space,
> the AVI infoframe can be written in one go with regmap_bulk_write().
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 27 +++++++--------------------
> 1 file changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 12222840df30..0a4fdfd7af44 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -773,24 +773,9 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
> const struct drm_display_mode *mode,
> const struct drm_display_mode *adjusted_mode)
> {
> - int ret, i;
> u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
> struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge);
> - const u16 aviinfo_reg[HDMI_AVI_INFOFRAME_SIZE] = {
> - IT66121_AVIINFO_DB1_REG,
> - IT66121_AVIINFO_DB2_REG,
> - IT66121_AVIINFO_DB3_REG,
> - IT66121_AVIINFO_DB4_REG,
> - IT66121_AVIINFO_DB5_REG,
> - IT66121_AVIINFO_DB6_REG,
> - IT66121_AVIINFO_DB7_REG,
> - IT66121_AVIINFO_DB8_REG,
> - IT66121_AVIINFO_DB9_REG,
> - IT66121_AVIINFO_DB10_REG,
> - IT66121_AVIINFO_DB11_REG,
> - IT66121_AVIINFO_DB12_REG,
> - IT66121_AVIINFO_DB13_REG
> - };
> + int ret;
>
> mutex_lock(&ctx->lock);
>
> @@ -810,10 +795,12 @@ void it66121_bridge_mode_set(struct drm_bridge *bridge,
> }
>
> /* Write new AVI infoframe packet */
> - for (i = 0; i < HDMI_AVI_INFOFRAME_SIZE; i++) {
> - if (regmap_write(ctx->regmap, aviinfo_reg[i], buf[i + HDMI_INFOFRAME_HEADER_SIZE]))
> - goto unlock;
> - }
> + ret = regmap_bulk_write(ctx->regmap, IT66121_AVIINFO_DB1_REG,
> + &buf[HDMI_INFOFRAME_HEADER_SIZE],
> + HDMI_AVI_INFOFRAME_SIZE);
> + if (ret)
> + goto unlock;
> +
> if (regmap_write(ctx->regmap, IT66121_AVIINFO_CSUM_REG, buf[3]))
> goto unlock;
>
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
On Wed, 14 Dec 2022 at 13:59, Paul Cercueil <[email protected]> wrote:
>
> The function it66121_wait_ddc_ready() would previously read the status
> register until "true", which means it never actually polled anything and
> would just read the register once.
>
> Now, it will properly wait until the DDC hardware is ready or until it
> reported an error.
>
> The 'busy' variable was also renamed to 'error' since these bits are set
> on error and not when the DDC hardware is busy.
>
> Since the DDC ready function is now working properly, the msleep(20) can
> be removed.
>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 0a4fdfd7af44..bfb9c87a7019 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -440,15 +440,17 @@ static int it66121_configure_afe(struct it66121_ctx *ctx,
> static inline int it66121_wait_ddc_ready(struct it66121_ctx *ctx)
> {
> int ret, val;
> - u32 busy = IT66121_DDC_STATUS_NOACK | IT66121_DDC_STATUS_WAIT_BUS |
> - IT66121_DDC_STATUS_ARBI_LOSE;
> + u32 error = IT66121_DDC_STATUS_NOACK | IT66121_DDC_STATUS_WAIT_BUS |
> + IT66121_DDC_STATUS_ARBI_LOSE;
> + u32 done = IT66121_DDC_STATUS_TX_DONE;
>
> - ret = regmap_read_poll_timeout(ctx->regmap, IT66121_DDC_STATUS_REG, val, true,
> - IT66121_EDID_SLEEP_US, IT66121_EDID_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(ctx->regmap, IT66121_DDC_STATUS_REG, val,
> + val & (error | done), IT66121_EDID_SLEEP_US,
> + IT66121_EDID_TIMEOUT_US);
> if (ret)
> return ret;
>
> - if (val & busy)
> + if (val & error)
> return -EAGAIN;
>
> return 0;
> @@ -582,9 +584,6 @@ static int it66121_get_edid_block(void *context, u8 *buf,
> offset += cnt;
> remain -= cnt;
>
> - /* Per programming manual, sleep here before emptying the FIFO */
> - msleep(20);
> -
> ret = it66121_wait_ddc_ready(ctx);
> if (ret)
> return ret;
> --
> 2.35.1
>
Reviewed-by: Robert Foss <[email protected]>
Le jeudi 15 décembre 2022 à 11:55 +0100, Robert Foss a écrit :
> On Wed, 14 Dec 2022 at 13:58, Paul Cercueil <[email protected]>
> wrote:
> >
> > Add a new ite,it6610 compatible string to the IT66121 binding
> > documentation, since the two chips are very similar.
> >
> > Signed-off-by: Paul Cercueil <[email protected]>
> > ---
> > .../devicetree/bindings/display/bridge/ite,it66121.yaml | 4
> > +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
> > b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
> > index 1b2185be92cd..72957be0ba3c 100644
> > ---
> > a/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
> > +++
> > b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml
> > @@ -17,7 +17,9 @@ description: |
> >
> > properties:
> > compatible:
> > - const: ite,it66121
> > + enum:
> > + - ite,it66121
> > + - ite,it6610
> >
> > reg:
> > maxItems: 1
> > --
> > 2.35.1
> >
>
> Reviewed-by: Robert Foss <[email protected]>
Series applied to drm-misc-next.
Thanks for the reviews!
Cheers,
-Paul
On 16/12/2022 11:46, Paul Cercueil wrote:
>>> properties:
>>> compatible:
>>> - const: ite,it66121
>>> + enum:
>>> + - ite,it66121
>>> + - ite,it6610
These should be ordered alphabetically. What's with the tendency of
adding always to the end?
>>>
>>> reg:
>>> maxItems: 1
>>> --
>>> 2.35.1
>>>
>>
>> Reviewed-by: Robert Foss <[email protected]>
>
> Series applied to drm-misc-next.
>
I wished you give DT maintainers a bit more time than two days.
Best regards,
Krzysztof
Hi Krzysztof,
Le vendredi 16 décembre 2022 à 12:21 +0100, Krzysztof Kozlowski a
écrit :
> On 16/12/2022 11:46, Paul Cercueil wrote:
>
> > > > properties:
> > > > compatible:
> > > > - const: ite,it66121
> > > > + enum:
> > > > + - ite,it66121
> > > > + - ite,it6610
>
> These should be ordered alphabetically. What's with the tendency of
> adding always to the end?
I'm too used to the "inverse christmas tree" sort :)
I can send a quickfix patch if you really want alphabetical order.
> > > >
> > > > reg:
> > > > maxItems: 1
> > > > --
> > > > 2.35.1
> > > >
> > >
> > > Reviewed-by: Robert Foss <[email protected]>
> >
> > Series applied to drm-misc-next.
> >
> I wished you give DT maintainers a bit more time than two days.
Noted. Sorry about that.
Cheers,
-Paul
On 16/12/2022 13:21, Paul Cercueil wrote:
> Hi Krzysztof,
>
> Le vendredi 16 décembre 2022 à 12:21 +0100, Krzysztof Kozlowski a
> écrit :
>> On 16/12/2022 11:46, Paul Cercueil wrote:
>>
>>>>> properties:
>>>>> compatible:
>>>>> - const: ite,it66121
>>>>> + enum:
>>>>> + - ite,it66121
>>>>> + - ite,it6610
>>
>> These should be ordered alphabetically. What's with the tendency of
>> adding always to the end?
>
> I'm too used to the "inverse christmas tree" sort :)
Since these are not variables and they will not get shorter, any
christmas tree sorting here is the same conflict-prone as adding to the end.
>
> I can send a quickfix patch if you really want alphabetical order.
No, no need.
Best regards,
Krzysztof
On Fri, Dec 16, 2022 at 01:21:54PM +0100, Paul Cercueil wrote:
> Hi Krzysztof,
>
> Le vendredi 16 d?cembre 2022 ? 12:21 +0100, Krzysztof Kozlowski a
> ?crit?:
> > On 16/12/2022 11:46, Paul Cercueil wrote:
> >
> > > > > ?properties:
> > > > > ?? compatible:
> > > > > -??? const: ite,it66121
> > > > > +??? enum:
> > > > > +????? - ite,it66121
> > > > > +????? - ite,it6610
> >
> > These should be ordered alphabetically. What's with the tendency of
> > adding always to the end?
>
> I'm too used to the "inverse christmas tree" sort :)
Come on, the DT standard is sideways christmas tree. ;)
Rob