To use MHL we currently need the MHL chip to be permanently on, which
consumes unnecessary power. Let's use extcon attached to MUIC to enable
the MHL chip only if it detects an MHL cable.
Signed-off-by: Henrik Grimler <henrik@grimler.se>
---
drivers/gpu/drm/bridge/sii9234.c | 89 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 86 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
index 0e0bb1bf71fdcef788715cfd6fa158a6992def33..4d84ba01ea76816bebdbc29d48a041c9c6cd508e 100644
--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -19,6 +19,7 @@
#include <linux/delay.h>
#include <linux/err.h>
+#include <linux/extcon.h>
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
@@ -26,6 +27,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/of_graph.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
@@ -170,8 +172,12 @@ struct sii9234 {
struct drm_bridge bridge;
struct device *dev;
struct gpio_desc *gpio_reset;
- int i2c_error;
struct regulator_bulk_data supplies[4];
+ struct extcon_dev *extcon;
+ struct notifier_block extcon_nb;
+ struct work_struct extcon_wq;
+ int cable_state;
+ int i2c_error;
struct mutex lock; /* Protects fields below and device registers */
enum sii9234_state state;
@@ -864,6 +870,70 @@ static int sii9234_init_resources(struct sii9234 *ctx,
return 0;
}
+static void sii9234_extcon_work(struct work_struct *work)
+{
+ struct sii9234 *ctx =
+ container_of(work, struct sii9234, extcon_wq);
+ int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
+
+ if (state == ctx->cable_state)
+ return;
+
+ ctx->cable_state = state;
+
+ if (state > 0)
+ sii9234_cable_in(ctx);
+ else
+ sii9234_cable_out(ctx);
+}
+
+static int sii9234_extcon_notifier(struct notifier_block *self,
+ unsigned long event, void *ptr)
+{
+ struct sii9234 *ctx =
+ container_of(self, struct sii9234, extcon_nb);
+
+ schedule_work(&ctx->extcon_wq);
+
+ return NOTIFY_DONE;
+}
+
+static int sii9234_extcon_init(struct sii9234 *ctx)
+{
+ struct extcon_dev *edev;
+ struct device_node *musb, *muic;
+ int ret;
+
+ /* Get micro-USB connector node */
+ musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
+ /* Then get micro-USB Interface Controller node */
+ muic = of_get_next_parent(musb);
+
+ if (!muic) {
+ dev_info(ctx->dev,
+ "no extcon found, switching to 'always on' mode\n");
+ return 0;
+ }
+
+ edev = extcon_find_edev_by_node(muic);
+ of_node_put(muic);
+ if (IS_ERR(edev)) {
+ dev_err_probe(ctx->dev, PTR_ERR(edev),
+ "invalid or missing extcon\n");
+ }
+
+ ctx->extcon = edev;
+ ctx->extcon_nb.notifier_call = sii9234_extcon_notifier;
+ INIT_WORK(&ctx->extcon_wq, sii9234_extcon_work);
+ ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
+ if (ret) {
+ dev_err(ctx->dev, "failed to register notifier for MHL\n");
+ return ret;
+ }
+
+ return 0;
+}
+
static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
const struct drm_display_info *info,
const struct drm_display_mode *mode)
@@ -916,12 +986,17 @@ static int sii9234_probe(struct i2c_client *client)
if (ret < 0)
return ret;
+ ret = sii9234_extcon_init(ctx);
+ if (ret < 0)
+ return ret;
+
i2c_set_clientdata(client, ctx);
ctx->bridge.of_node = dev->of_node;
drm_bridge_add(&ctx->bridge);
- sii9234_cable_in(ctx);
+ if (!ctx->extcon)
+ sii9234_cable_in(ctx);
return 0;
}
@@ -930,7 +1005,15 @@ static void sii9234_remove(struct i2c_client *client)
{
struct sii9234 *ctx = i2c_get_clientdata(client);
- sii9234_cable_out(ctx);
+ if (ctx->extcon) {
+ extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
+ &ctx->extcon_nb);
+ flush_work(&ctx->extcon_wq);
+ if (ctx->cable_state > 0)
+ sii9234_cable_out(ctx);
+ } else {
+ sii9234_cable_out(ctx);
+ }
drm_bridge_remove(&ctx->bridge);
}
--
2.50.1On 21.07.2025 11:43, Henrik Grimler wrote:
> To use MHL we currently need the MHL chip to be permanently on, which
> consumes unnecessary power. Let's use extcon attached to MUIC to enable
> the MHL chip only if it detects an MHL cable.
>
> Signed-off-by: Henrik Grimler <henrik@grimler.se>
> ---
Thanks!
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
You should also add "select EXTCON" (like in case of Sii8620) to Kconfig
to avoid potential build break if extcon support is built as a module.
> drivers/gpu/drm/bridge/sii9234.c | 89 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 86 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
> index 0e0bb1bf71fdcef788715cfd6fa158a6992def33..4d84ba01ea76816bebdbc29d48a041c9c6cd508e 100644
> --- a/drivers/gpu/drm/bridge/sii9234.c
> +++ b/drivers/gpu/drm/bridge/sii9234.c
> @@ -19,6 +19,7 @@
>
> #include <linux/delay.h>
> #include <linux/err.h>
> +#include <linux/extcon.h>
> #include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/interrupt.h>
> @@ -26,6 +27,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/of_graph.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
>
> @@ -170,8 +172,12 @@ struct sii9234 {
> struct drm_bridge bridge;
> struct device *dev;
> struct gpio_desc *gpio_reset;
> - int i2c_error;
> struct regulator_bulk_data supplies[4];
> + struct extcon_dev *extcon;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_wq;
> + int cable_state;
> + int i2c_error;
>
> struct mutex lock; /* Protects fields below and device registers */
> enum sii9234_state state;
> @@ -864,6 +870,70 @@ static int sii9234_init_resources(struct sii9234 *ctx,
> return 0;
> }
>
> +static void sii9234_extcon_work(struct work_struct *work)
> +{
> + struct sii9234 *ctx =
> + container_of(work, struct sii9234, extcon_wq);
> + int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
> +
> + if (state == ctx->cable_state)
> + return;
> +
> + ctx->cable_state = state;
> +
> + if (state > 0)
> + sii9234_cable_in(ctx);
> + else
> + sii9234_cable_out(ctx);
> +}
> +
> +static int sii9234_extcon_notifier(struct notifier_block *self,
> + unsigned long event, void *ptr)
> +{
> + struct sii9234 *ctx =
> + container_of(self, struct sii9234, extcon_nb);
> +
> + schedule_work(&ctx->extcon_wq);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int sii9234_extcon_init(struct sii9234 *ctx)
> +{
> + struct extcon_dev *edev;
> + struct device_node *musb, *muic;
> + int ret;
> +
> + /* Get micro-USB connector node */
> + musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
> + /* Then get micro-USB Interface Controller node */
> + muic = of_get_next_parent(musb);
> +
> + if (!muic) {
> + dev_info(ctx->dev,
> + "no extcon found, switching to 'always on' mode\n");
> + return 0;
> + }
> +
> + edev = extcon_find_edev_by_node(muic);
> + of_node_put(muic);
> + if (IS_ERR(edev)) {
> + dev_err_probe(ctx->dev, PTR_ERR(edev),
> + "invalid or missing extcon\n");
> + }
> +
> + ctx->extcon = edev;
> + ctx->extcon_nb.notifier_call = sii9234_extcon_notifier;
> + INIT_WORK(&ctx->extcon_wq, sii9234_extcon_work);
> + ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
> + if (ret) {
> + dev_err(ctx->dev, "failed to register notifier for MHL\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
> const struct drm_display_info *info,
> const struct drm_display_mode *mode)
> @@ -916,12 +986,17 @@ static int sii9234_probe(struct i2c_client *client)
> if (ret < 0)
> return ret;
>
> + ret = sii9234_extcon_init(ctx);
> + if (ret < 0)
> + return ret;
> +
> i2c_set_clientdata(client, ctx);
>
> ctx->bridge.of_node = dev->of_node;
> drm_bridge_add(&ctx->bridge);
>
> - sii9234_cable_in(ctx);
> + if (!ctx->extcon)
> + sii9234_cable_in(ctx);
>
> return 0;
> }
> @@ -930,7 +1005,15 @@ static void sii9234_remove(struct i2c_client *client)
> {
> struct sii9234 *ctx = i2c_get_clientdata(client);
>
> - sii9234_cable_out(ctx);
> + if (ctx->extcon) {
> + extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
> + &ctx->extcon_nb);
> + flush_work(&ctx->extcon_wq);
> + if (ctx->cable_state > 0)
> + sii9234_cable_out(ctx);
> + } else {
> + sii9234_cable_out(ctx);
> + }
> drm_bridge_remove(&ctx->bridge);
> }
>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Marek,
On Fri, Aug 01, 2025 at 10:09:08AM +0200, Marek Szyprowski wrote:
> On 21.07.2025 11:43, Henrik Grimler wrote:
> > To use MHL we currently need the MHL chip to be permanently on, which
> > consumes unnecessary power. Let's use extcon attached to MUIC to enable
> > the MHL chip only if it detects an MHL cable.
> >
> > Signed-off-by: Henrik Grimler <henrik@grimler.se>
> > ---
>
> Thanks!
>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Thanks for testing! On trats2 I assume? After some more testing I
noticed that mhl hotplugging only works reliably if device screen is
on when cable is plugged in, otherwise I get errors like:
[ 281.203520] exynos-hdmi 12d00000.hdmi: [drm:hdmiphy_enable.part.0] *ERROR* PLL could not reach steady state
[ 281.313816] exynos-mixer 12c10000.mixer: timeout waiting for VSYNC
[ 281.420637] ------------[ cut here ]------------
[ 281.423861] WARNING: CPU: 3 PID: 1189 at drivers/gpu/drm/drm_atomic_helper.c:1720 drm_atomic_helper_wait_for_vblanks.part.0+0x264/0x26c
[ 281.436172] [CRTC:71:crtc-1] vblank wait timed out
[ 281.440870] Modules linked in: cpufreq_conservative rfcomm aes_arm aes_generic nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 cmac nft_reject algif_hash algif_skcipher nft_ct nf_conntrack af_alg nf_defrag_ipv6 nf_defrag_ipv4 bnep nf_tables ntc_thermistor hwmon brcmfmac_wcc s5k6a3 st_accel_spi spi_s3c64xx st_gyro_spi hci_uart st_sensors_spi btbcm btintel pwm_samsung bluetooth st_gyro_i2c st_accel_i2c st_gyro st_sensors_i2c st_accel st_sensors brcmfmac ecdh_generic ecc s5c73m3 libaes lima cfg80211 drm_shmem_helper gpu_sched exynos_adc brcmutil rfkill panel_samsung_s6e8aa0 s5p_sss exynos_rng s5p_g2d tm2_touchkey leds_an30259a cm36651 leds_aat1290 led_class_flash led_class ak8975 industrialio_triggered_buffer kfifo_buf gpio_keys uhid hid uinput ledtrig_pattern zram zsmalloc fuse loop nfnetlink ipv6 evdev [last unloaded: cpufreq_conservative]
[ 281.514974] CPU: 3 UID: 10000 PID: 1189 Comm: phoc Tainted: G W 6.16.0-postmarketos-exynos4 #38 PREEMPT
[ 281.515007] Tainted: [W]=WARN
[ 281.515013] Hardware name: Samsung Exynos (Flattened Device Tree)
[ 281.515027] Call trace:
[ 281.515062] unwind_backtrace from show_stack+0x18/0x1c
[ 281.515155] show_stack from dump_stack_lvl+0x50/0x64
[ 281.515219] dump_stack_lvl from __warn+0x88/0x15c
[ 281.515271] __warn from warn_slowpath_fmt+0x118/0x1ac
[ 281.515285] warn_slowpath_fmt from drm_atomic_helper_wait_for_vblanks.part.0+0x264/0x26c
[ 281.515340] drm_atomic_helper_wait_for_vblanks.part.0 from drm_atomic_helper_commit_tail_rpm+0x84/0x94
[ 281.515363] drm_atomic_helper_commit_tail_rpm from commit_tail+0xa8/0x1a0
[ 281.515385] commit_tail from drm_atomic_helper_commit+0x164/0x19c
[ 281.515402] drm_atomic_helper_commit from drm_atomic_commit+0xd0/0x104
[ 281.515489] drm_atomic_commit from drm_mode_atomic_ioctl+0x9f8/0xd14
[ 281.515512] drm_mode_atomic_ioctl from drm_ioctl+0x20c/0x4cc
[ 281.515562] drm_ioctl from sys_ioctl+0x5a8/0xc10
[ 281.515634] sys_ioctl from ret_fast_syscall+0x0/0x54
[ 281.515651] Exception stack(0xc5d05fa8 to 0xc5d05ff0)
[ 281.515665] 5fa0: beb4be88 beb4be88 0000000d c03864bc beb4be88 0000000d
[ 281.515675] 5fc0: beb4be88 beb4be88 c03864bc 00000036 acbe4450 ad8edc70 acbe4490 acd46cf0
[ 281.515682] 5fe0: beb4be58 beb4bd38 b6e5651f b6ebb040
[ 281.515693] ---[ end trace 0000000000000000 ]---
or device just hangs. I suppose this might not be an issue in the
sii9234 driver though, so will collect your tags (unless I do further
changes)!
> You should also add "select EXTCON" (like in case of Sii8620) to Kconfig
> to avoid potential build break if extcon support is built as a module.
Kernel robot also reported this issue, so I have added the select in
v2:
https://lore.kernel.org/linux-samsung-soc/20250724-exynos4-sii9234-driver-v2-0-faee244f1d40@grimler.se/
Best regards,
Henrik Grimler
>
> > drivers/gpu/drm/bridge/sii9234.c | 89 ++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 86 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
> > index 0e0bb1bf71fdcef788715cfd6fa158a6992def33..4d84ba01ea76816bebdbc29d48a041c9c6cd508e 100644
> > --- a/drivers/gpu/drm/bridge/sii9234.c
> > +++ b/drivers/gpu/drm/bridge/sii9234.c
> > @@ -19,6 +19,7 @@
> >
> > #include <linux/delay.h>
> > #include <linux/err.h>
> > +#include <linux/extcon.h>
> > #include <linux/gpio/consumer.h>
> > #include <linux/i2c.h>
> > #include <linux/interrupt.h>
> > @@ -26,6 +27,7 @@
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > +#include <linux/of_graph.h>
> > #include <linux/regulator/consumer.h>
> > #include <linux/slab.h>
> >
> > @@ -170,8 +172,12 @@ struct sii9234 {
> > struct drm_bridge bridge;
> > struct device *dev;
> > struct gpio_desc *gpio_reset;
> > - int i2c_error;
> > struct regulator_bulk_data supplies[4];
> > + struct extcon_dev *extcon;
> > + struct notifier_block extcon_nb;
> > + struct work_struct extcon_wq;
> > + int cable_state;
> > + int i2c_error;
> >
> > struct mutex lock; /* Protects fields below and device registers */
> > enum sii9234_state state;
> > @@ -864,6 +870,70 @@ static int sii9234_init_resources(struct sii9234 *ctx,
> > return 0;
> > }
> >
> > +static void sii9234_extcon_work(struct work_struct *work)
> > +{
> > + struct sii9234 *ctx =
> > + container_of(work, struct sii9234, extcon_wq);
> > + int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
> > +
> > + if (state == ctx->cable_state)
> > + return;
> > +
> > + ctx->cable_state = state;
> > +
> > + if (state > 0)
> > + sii9234_cable_in(ctx);
> > + else
> > + sii9234_cable_out(ctx);
> > +}
> > +
> > +static int sii9234_extcon_notifier(struct notifier_block *self,
> > + unsigned long event, void *ptr)
> > +{
> > + struct sii9234 *ctx =
> > + container_of(self, struct sii9234, extcon_nb);
> > +
> > + schedule_work(&ctx->extcon_wq);
> > +
> > + return NOTIFY_DONE;
> > +}
> > +
> > +static int sii9234_extcon_init(struct sii9234 *ctx)
> > +{
> > + struct extcon_dev *edev;
> > + struct device_node *musb, *muic;
> > + int ret;
> > +
> > + /* Get micro-USB connector node */
> > + musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
> > + /* Then get micro-USB Interface Controller node */
> > + muic = of_get_next_parent(musb);
> > +
> > + if (!muic) {
> > + dev_info(ctx->dev,
> > + "no extcon found, switching to 'always on' mode\n");
> > + return 0;
> > + }
> > +
> > + edev = extcon_find_edev_by_node(muic);
> > + of_node_put(muic);
> > + if (IS_ERR(edev)) {
> > + dev_err_probe(ctx->dev, PTR_ERR(edev),
> > + "invalid or missing extcon\n");
> > + }
> > +
> > + ctx->extcon = edev;
> > + ctx->extcon_nb.notifier_call = sii9234_extcon_notifier;
> > + INIT_WORK(&ctx->extcon_wq, sii9234_extcon_work);
> > + ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
> > + if (ret) {
> > + dev_err(ctx->dev, "failed to register notifier for MHL\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
> > const struct drm_display_info *info,
> > const struct drm_display_mode *mode)
> > @@ -916,12 +986,17 @@ static int sii9234_probe(struct i2c_client *client)
> > if (ret < 0)
> > return ret;
> >
> > + ret = sii9234_extcon_init(ctx);
> > + if (ret < 0)
> > + return ret;
> > +
> > i2c_set_clientdata(client, ctx);
> >
> > ctx->bridge.of_node = dev->of_node;
> > drm_bridge_add(&ctx->bridge);
> >
> > - sii9234_cable_in(ctx);
> > + if (!ctx->extcon)
> > + sii9234_cable_in(ctx);
> >
> > return 0;
> > }
> > @@ -930,7 +1005,15 @@ static void sii9234_remove(struct i2c_client *client)
> > {
> > struct sii9234 *ctx = i2c_get_clientdata(client);
> >
> > - sii9234_cable_out(ctx);
> > + if (ctx->extcon) {
> > + extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
> > + &ctx->extcon_nb);
> > + flush_work(&ctx->extcon_wq);
> > + if (ctx->cable_state > 0)
> > + sii9234_cable_out(ctx);
> > + } else {
> > + sii9234_cable_out(ctx);
> > + }
> > drm_bridge_remove(&ctx->bridge);
> > }
> >
> >
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
Hi Henrik, kernel test robot noticed the following build errors: [auto build test ERROR on ca2a6abdaee43808034cdb218428d2ed85fd3db8] url: https://github.com/intel-lab-lkp/linux/commits/Henrik-Grimler/drm-bridge-sii9234-fix-some-typos-in-comments-and-messages/20250721-174814 base: ca2a6abdaee43808034cdb218428d2ed85fd3db8 patch link: https://lore.kernel.org/r/20250721-exynos4-sii9234-driver-v1-3-2e47ed02f677%40grimler.se patch subject: [PATCH 3/3] drm/bridge: sii9234: use extcon cable detection logic to detect MHL config: arm64-randconfig-002-20250724 (https://download.01.org/0day-ci/archive/20250725/202507250036.RXWSmKW2-lkp@intel.com/config) compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 853c343b45b3e83cc5eeef5a52fc8cc9d8a09252) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250725/202507250036.RXWSmKW2-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202507250036.RXWSmKW2-lkp@intel.com/ All errors (new ones prefixed by >>): >> ld.lld: error: undefined symbol: extcon_register_notifier >>> referenced by sii9234.c:928 (drivers/gpu/drm/bridge/sii9234.c:928) >>> drivers/gpu/drm/bridge/sii9234.o:(sii9234_probe) in archive vmlinux.a -- >> ld.lld: error: undefined symbol: extcon_unregister_notifier >>> referenced by sii9234.c:1009 (drivers/gpu/drm/bridge/sii9234.c:1009) >>> drivers/gpu/drm/bridge/sii9234.o:(sii9234_remove) in archive vmlinux.a -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.