drivers/gpu/drm/bridge/ite-it6263.c | 88 ++++++++++++++++++++++++----- 1 file changed, 73 insertions(+), 15 deletions(-)
From: Biju Das <biju.das.jz@bp.renesas.com>
On the RZ/G3L SMARC EVK using PSCI, suspend to RAM powers down the ITE
IT6263 chip. The display controller driver's system PM callbacks invoke
drm_mode_config_helper_{suspend,resume}, which in turn call the bridge's
atomic_{disable,enable} callbacks can handle suspend/resume for the
bridge without dedicated PM ops.
Introduce it6263_bridge_init() and it6263_bridge_uninit() helpers to
consolidate power sequencing, hardware reset, I2C address setup, and
LVDS/HDMI configuration. These replace the open-coded init sequence in
probe() and are hooked into atomic_enable/atomic_disable respectively,
guarded by a powered flag to avoid redundant re-initialisation.
Switch from devm_regulator_bulk_get_enable() to devm_regulator_bulk_get()
so that regulators can be explicitly enabled and disabled across power
cycles. Move reset_gpio and regulator state into the it6263 struct so they
are accessible beyond probe time.
Add a remove() callback to cleanly power down the bridge on driver unbind
via it6263_bridge_uninit().
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
* Dropped system PM callbacks instead using bridge's
atomic_{disable,enable} callbacks to handle suspend/resume.
---
drivers/gpu/drm/bridge/ite-it6263.c | 88 ++++++++++++++++++++++++-----
1 file changed, 73 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c
index 4f3ebb7af4d4..1954bb11f7f4 100644
--- a/drivers/gpu/drm/bridge/ite-it6263.c
+++ b/drivers/gpu/drm/bridge/ite-it6263.c
@@ -200,9 +200,13 @@ struct it6263 {
struct regmap *lvds_regmap;
struct drm_bridge bridge;
struct drm_bridge *next_bridge;
+ struct gpio_desc *reset_gpio;
+ struct regulator_bulk_data *supplies;
+ unsigned int num_supplies;
int lvds_data_mapping;
bool lvds_dual_link;
bool lvds_link12_swap;
+ bool powered;
};
static inline struct it6263 *bridge_to_it6263(struct drm_bridge *bridge)
@@ -578,6 +582,41 @@ static int it6263_read_edid(void *data, u8 *buf, unsigned int block, size_t len)
return 0;
}
+static int it6263_bridge_init(struct it6263 *it)
+{
+ int ret;
+
+ ret = regulator_bulk_enable(it->num_supplies, it->supplies);
+ if (ret) {
+ dev_err(it->dev, "failed to enable power supplies\n");
+ return ret;
+ }
+
+ it6263_hw_reset(it->reset_gpio);
+
+ ret = it6263_lvds_set_i2c_addr(it);
+ if (ret) {
+ dev_err(it->dev, "failed to set I2C addr\n");
+ regulator_bulk_disable(it->num_supplies, it->supplies);
+ return ret;
+ }
+
+ it6263_lvds_config(it);
+ it6263_hdmi_config(it);
+
+ it->powered = true;
+
+ return 0;
+}
+
+static int it6263_bridge_uninit(struct it6263 *it)
+{
+ regulator_bulk_disable(it->num_supplies, it->supplies);
+ it->powered = false;
+
+ return 0;
+}
+
static void it6263_bridge_atomic_disable(struct drm_bridge *bridge,
struct drm_atomic_state *state)
{
@@ -587,6 +626,8 @@ static void it6263_bridge_atomic_disable(struct drm_bridge *bridge,
regmap_write(it->hdmi_regmap, HDMI_REG_PKT_GENERAL_CTRL, 0);
regmap_write(it->hdmi_regmap, HDMI_REG_AFE_DRV_CTRL,
AFE_DRV_RST | AFE_DRV_PWD);
+
+ it6263_bridge_uninit(it);
}
static void it6263_bridge_atomic_enable(struct drm_bridge *bridge,
@@ -603,6 +644,9 @@ static void it6263_bridge_atomic_enable(struct drm_bridge *bridge,
bool pclk_high;
int i, ret;
+ if (!it->powered)
+ it6263_bridge_init(it);
+
connector = drm_atomic_get_new_connector_for_encoder(state,
bridge->encoder);
crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
@@ -840,7 +884,6 @@ static const struct drm_bridge_funcs it6263_bridge_funcs = {
static int it6263_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
- struct gpio_desc *reset_gpio;
struct it6263 *it;
int ret;
@@ -858,13 +901,21 @@ static int it6263_probe(struct i2c_client *client)
return dev_err_probe(dev, PTR_ERR(it->hdmi_regmap),
"failed to init I2C regmap for HDMI\n");
- reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(reset_gpio))
- return dev_err_probe(dev, PTR_ERR(reset_gpio),
+ it->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(it->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(it->reset_gpio),
"failed to get reset gpio\n");
- ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(it6263_supplies),
- it6263_supplies);
+ it->num_supplies = ARRAY_SIZE(it6263_supplies);
+ it->supplies = devm_kcalloc(dev, it->num_supplies,
+ sizeof(*it->supplies), GFP_KERNEL);
+ if (!it->supplies)
+ return -ENOMEM;
+
+ for (unsigned int i = 0; i < it->num_supplies; i++)
+ it->supplies[i].supply = it6263_supplies[i];
+
+ ret = devm_regulator_bulk_get(dev, it->num_supplies, it->supplies);
if (ret)
return dev_err_probe(dev, ret, "failed to get power supplies\n");
@@ -872,12 +923,6 @@ static int it6263_probe(struct i2c_client *client)
if (ret)
return ret;
- it6263_hw_reset(reset_gpio);
-
- ret = it6263_lvds_set_i2c_addr(it);
- if (ret)
- return dev_err_probe(dev, ret, "failed to set I2C addr\n");
-
it->lvds_i2c = devm_i2c_new_dummy_device(dev, client->adapter,
LVDS_INPUT_CTRL_I2C_ADDR);
if (IS_ERR(it->lvds_i2c))
@@ -890,8 +935,9 @@ static int it6263_probe(struct i2c_client *client)
return dev_err_probe(dev, PTR_ERR(it->lvds_regmap),
"failed to init I2C regmap for LVDS\n");
- it6263_lvds_config(it);
- it6263_hdmi_config(it);
+ ret = it6263_bridge_init(it);
+ if (ret)
+ return ret;
i2c_set_clientdata(client, it);
@@ -903,7 +949,18 @@ static int it6263_probe(struct i2c_client *client)
it->bridge.vendor = "ITE";
it->bridge.product = "IT6263";
- return devm_drm_bridge_add(dev, &it->bridge);
+ ret = devm_drm_bridge_add(dev, &it->bridge);
+ if (ret)
+ it6263_bridge_uninit(it);
+
+ return ret;
+}
+
+static void it6263_remove(struct i2c_client *i2c)
+{
+ struct it6263 *it = i2c_get_clientdata(i2c);
+
+ it6263_bridge_uninit(it);
}
static const struct of_device_id it6263_of_match[] = {
@@ -920,6 +977,7 @@ MODULE_DEVICE_TABLE(i2c, it6263_i2c_ids);
static struct i2c_driver it6263_driver = {
.probe = it6263_probe,
+ .remove = it6263_remove,
.driver = {
.name = "it6263",
.of_match_table = it6263_of_match,
--
2.43.0Hi Biju,
On Thu, Apr 16, 2026 at 09:29:25AM +0100, Biju wrote:
> [You don't often get email from biju.das.au@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> On the RZ/G3L SMARC EVK using PSCI, suspend to RAM powers down the ITE
> IT6263 chip. The display controller driver's system PM callbacks invoke
> drm_mode_config_helper_{suspend,resume}, which in turn call the bridge's
> atomic_{disable,enable} callbacks can handle suspend/resume for the
> bridge without dedicated PM ops.
>
> Introduce it6263_bridge_init() and it6263_bridge_uninit() helpers to
> consolidate power sequencing, hardware reset, I2C address setup, and
> LVDS/HDMI configuration. These replace the open-coded init sequence in
> probe() and are hooked into atomic_enable/atomic_disable respectively,
> guarded by a powered flag to avoid redundant re-initialisation.
>
> Switch from devm_regulator_bulk_get_enable() to devm_regulator_bulk_get()
> so that regulators can be explicitly enabled and disabled across power
> cycles. Move reset_gpio and regulator state into the it6263 struct so they
> are accessible beyond probe time.
>
> Add a remove() callback to cleanly power down the bridge on driver unbind
> via it6263_bridge_uninit().
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
> * Dropped system PM callbacks instead using bridge's
> atomic_{disable,enable} callbacks to handle suspend/resume.
> ---
> drivers/gpu/drm/bridge/ite-it6263.c | 88 ++++++++++++++++++++++++-----
> 1 file changed, 73 insertions(+), 15 deletions(-)
"suspend/resume" in subject makes people think that this patch probably
adds runtime PM or system PM support. To avoid this, can you change the
subject to something like:
"drm/bridge: ite-it6263: Support power cycle in runtime"
?
>
> diff --git a/drivers/gpu/drm/bridge/ite-it6263.c b/drivers/gpu/drm/bridge/ite-it6263.c
> index 4f3ebb7af4d4..1954bb11f7f4 100644
> --- a/drivers/gpu/drm/bridge/ite-it6263.c
> +++ b/drivers/gpu/drm/bridge/ite-it6263.c
> @@ -200,9 +200,13 @@ struct it6263 {
> struct regmap *lvds_regmap;
> struct drm_bridge bridge;
> struct drm_bridge *next_bridge;
> + struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data *supplies;
I would move it6263_supplies[] on top of struct it6263 definition and use
'struct regulator_bulk_data supplies[ARRAY_SIZE(it6263_supplies)];' here,
so that you may drop devm_kcalloc() for the supplies array in probe.
> + unsigned int num_supplies;
The above new supplies array has a known size, so this can be dropped and
you may get the number of supplies via ARRAY_SIZE(it->supplies).
> int lvds_data_mapping;
> bool lvds_dual_link;
> bool lvds_link12_swap;
> + bool powered;
> };
>
> static inline struct it6263 *bridge_to_it6263(struct drm_bridge *bridge)
> @@ -578,6 +582,41 @@ static int it6263_read_edid(void *data, u8 *buf, unsigned int block, size_t len)
> return 0;
> }
>
> +static int it6263_bridge_init(struct it6263 *it)
> +{
> + int ret;
> +
> + ret = regulator_bulk_enable(it->num_supplies, it->supplies);
> + if (ret) {
> + dev_err(it->dev, "failed to enable power supplies\n");
> + return ret;
> + }
> +
> + it6263_hw_reset(it->reset_gpio);
> +
> + ret = it6263_lvds_set_i2c_addr(it);
> + if (ret) {
> + dev_err(it->dev, "failed to set I2C addr\n");
> + regulator_bulk_disable(it->num_supplies, it->supplies);
I know that you call it6263_bridge_init() in probe, probably because you
want to enable the regulators for hotplug detect after probe(it6263_detect()
reads register HDMI_REG_SYS_STATUS to do the detection). However, an idea[1]
is to wrap the register read operation with regulator_bulk_enable() and
regulator_bulk_disable() in it6263_detect() so that you may drop
it6263_bridge_init() from probe. With that, it6263_bridge_init() is now
only called from atomic_enable, which means that the initialization code
can be open-coded and the initialization is supposed to be successful(due
to the "atomic" nature) hence no need to do the regulator disablement
bailout(error message in dmesg is sufficient).
> + return ret;
> + }
> +
> + it6263_lvds_config(it);
> + it6263_hdmi_config(it);
> +
> + it->powered = true;
If you drop it6263_bridge_init() from probe, I think 'powered' flag can be
dropped too.
> +
> + return 0;
> +}
> +
> +static int it6263_bridge_uninit(struct it6263 *it)
> +{
> + regulator_bulk_disable(it->num_supplies, it->supplies);
> + it->powered = false;
> +
> + return 0;
> +}
> +
> static void it6263_bridge_atomic_disable(struct drm_bridge *bridge,
> struct drm_atomic_state *state)
> {
> @@ -587,6 +626,8 @@ static void it6263_bridge_atomic_disable(struct drm_bridge *bridge,
> regmap_write(it->hdmi_regmap, HDMI_REG_PKT_GENERAL_CTRL, 0);
> regmap_write(it->hdmi_regmap, HDMI_REG_AFE_DRV_CTRL,
> AFE_DRV_RST | AFE_DRV_PWD);
> +
> + it6263_bridge_uninit(it);
Well, this could effectively disable the regulators and hotplug detection
won't work then. So, again, the above idea[1] helps.
> }
>
> static void it6263_bridge_atomic_enable(struct drm_bridge *bridge,
> @@ -603,6 +644,9 @@ static void it6263_bridge_atomic_enable(struct drm_bridge *bridge,
> bool pclk_high;
> int i, ret;
>
> + if (!it->powered)
> + it6263_bridge_init(it);
> +
> connector = drm_atomic_get_new_connector_for_encoder(state,
> bridge->encoder);
> crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> @@ -840,7 +884,6 @@ static const struct drm_bridge_funcs it6263_bridge_funcs = {
> static int it6263_probe(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> - struct gpio_desc *reset_gpio;
> struct it6263 *it;
> int ret;
>
> @@ -858,13 +901,21 @@ static int it6263_probe(struct i2c_client *client)
> return dev_err_probe(dev, PTR_ERR(it->hdmi_regmap),
> "failed to init I2C regmap for HDMI\n");
>
> - reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> - if (IS_ERR(reset_gpio))
> - return dev_err_probe(dev, PTR_ERR(reset_gpio),
> + it->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(it->reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(it->reset_gpio),
> "failed to get reset gpio\n");
>
> - ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(it6263_supplies),
> - it6263_supplies);
> + it->num_supplies = ARRAY_SIZE(it6263_supplies);
> + it->supplies = devm_kcalloc(dev, it->num_supplies,
> + sizeof(*it->supplies), GFP_KERNEL);
> + if (!it->supplies)
> + return -ENOMEM;
> +
> + for (unsigned int i = 0; i < it->num_supplies; i++)
Nit: I would define i together with the other local variables at the beginning
of this function.
> + it->supplies[i].supply = it6263_supplies[i];
> +
> + ret = devm_regulator_bulk_get(dev, it->num_supplies, it->supplies);
> if (ret)
> return dev_err_probe(dev, ret, "failed to get power supplies\n");
>
> @@ -872,12 +923,6 @@ static int it6263_probe(struct i2c_client *client)
> if (ret)
> return ret;
>
> - it6263_hw_reset(reset_gpio);
> -
> - ret = it6263_lvds_set_i2c_addr(it);
> - if (ret)
> - return dev_err_probe(dev, ret, "failed to set I2C addr\n");
> -
> it->lvds_i2c = devm_i2c_new_dummy_device(dev, client->adapter,
> LVDS_INPUT_CTRL_I2C_ADDR);
> if (IS_ERR(it->lvds_i2c))
> @@ -890,8 +935,9 @@ static int it6263_probe(struct i2c_client *client)
> return dev_err_probe(dev, PTR_ERR(it->lvds_regmap),
> "failed to init I2C regmap for LVDS\n");
>
> - it6263_lvds_config(it);
> - it6263_hdmi_config(it);
> + ret = it6263_bridge_init(it);
> + if (ret)
> + return ret;
>
> i2c_set_clientdata(client, it);
>
> @@ -903,7 +949,18 @@ static int it6263_probe(struct i2c_client *client)
> it->bridge.vendor = "ITE";
> it->bridge.product = "IT6263";
>
> - return devm_drm_bridge_add(dev, &it->bridge);
> + ret = devm_drm_bridge_add(dev, &it->bridge);
> + if (ret)
> + it6263_bridge_uninit(it);
> +
> + return ret;
> +}
> +
> +static void it6263_remove(struct i2c_client *i2c)
> +{
> + struct it6263 *it = i2c_get_clientdata(i2c);
> +
> + it6263_bridge_uninit(it);
> }
>
> static const struct of_device_id it6263_of_match[] = {
> @@ -920,6 +977,7 @@ MODULE_DEVICE_TABLE(i2c, it6263_i2c_ids);
>
> static struct i2c_driver it6263_driver = {
> .probe = it6263_probe,
> + .remove = it6263_remove,
> .driver = {
> .name = "it6263",
> .of_match_table = it6263_of_match,
> --
> 2.43.0
>
--
Regards,
Liu Ying
© 2016 - 2026 Red Hat, Inc.