drm/bridge: it6505: DP audio support + shared-DAI hw_params fix
Subsystems (4)
v8 cover drm/bridge: it6505: DP audio support + shared-DAI hw_params fix
Revive HDMI/DP audio support for the it6505, originally posted by Jiaxin Yu [1], rebased onto current -next with the review feedback
addressed: &client->dev is gone (the driver gained it6505->dev) and "#sound-dai-cells" has since been added by commit 325af1bef5b9 ("dt-bindings: display: bridge: it6505: Add #sound-dai-cells").
Patches 1-12 fix pre-existing bugs found during review of earlier versions of this series; they carry Fixes: tags and are ordered first so stable trees can pick them.
Patch 13 wires up the it6505 audio helpers via hdmi_codec_ops, unblocking the mt8186-mt6366 sound card which references it6505 as the I2S3 codec. Patch 14 makes hw_params succeed with no display attached, needed when the I2S bus is shared with another codec as on MT8186 "steelix" Chromebooks, where the speaker path must keep working regardless of the it6505 link state.
Tested on a MT8186 (google,steelix) Chromebook: the sound card comes up, speakers, microphone and headset jack work. HDMI/DP audio output itself could not be tested for lack of a suitable adapter cable.
[1] https://lore.kernel.org/all/[email protected]/
---
v8:
* patch 4: keep the DP AUX unregister in remove() as well, so an i2c unbind waits for in-flight AUX device users before the regmap is freed; flagged by the Sashiko AI review of v7
* patch 5: terminate the new dev_err() messages with a newline
* patch 13: it6505_poweron() sets the powered flag and runs the logic reset and init sequence under audio_lock, closing a race with the delayed enable work flagged by the Sashiko AI review of v7; newline for the new dev_err() message
* rebased onto next-20260925
* patches 1-3, 6-12 and 14: no changes
v7:
* new patch 2: cancel link_works, hdcp_wait_ksv_list and hdcp_work in it6505_poweroff() so link training and HDCP authentication cannot run into a chip being powered off; pre-existing, found while assessing the Sashiko AI review of v6
* patch 13 (was 12): it6505_poweroff() clears the powered flag under audio_lock, closing the race between the codec ops and a concurrent power-off flagged by the Sashiko AI review of v6; the it6505_enable_audio() wrapper is folded into the delayed work
* the remaining v6 review findings are already fixed by later patches of this series (patches 3, 4, 6, 8, 11 and 12), concern pre-existing code outside its scope (whole-register restore in it6505_audio_input(), unchecked reads in it6505_aux_i2c_wait() and it6505_calc_video_info(), regulator disable order) or do not apply (AUX unregister on unbind, missing newline in dev_err())
* rebased onto next-20260923
* patches 1, 3-12 and 14: no changes
v6:
* patch 1: track notifier registration in a flag instead of clearing notifier_call, which a chain traversal racing the unregistration could have called as NULL (extcon traverses its raw notifier chain unlocked); flagged by Sashiko AI review of v5. The in-flight-callback race also flagged there is an extcon core limitation affecting all its consumers and cannot be closed from a consumer driver.
* patch 4: note that a failed regulator_disable() leaks the reference either way
* new patches 10 and 11: two more pre-existing fixes from v5 review, an uninitialised link-frequencies value and an error code written back to the reset register
* patches 2, 3, 5-9, 12 and 13: no changes
v5:
* patch 1: serialise notifier registration state with extcon_lock
* patch 2: also drop the runtime PM usage ref held for a connected display; retitled
* new patches 3-7 and 9: more pre-existing fixes from v4 review (AUX unregister symmetry, poweroff error path, IRQ status read errors, two divisions by zero, of_node_put before use)
* patch 10 (was 4): update plugged_cb/codec_dev under mode_lock
* patches 8 and 11: no changes
v4:
* patch 1: quiesce extcon notifier and IRQ before cancelling work, initialise extcon_wq in probe; retitled
* new patch 2: disable runtime PM on remove
* patch 3 (was 2): collected Chen-Yu Tsai's Reviewed-by
* patch 4 (was 3): start out muted; mute state and cached parameters updated under audio_lock; unregister codec after quiesce
* patch 5 (was 4): apply audio defaults at probe time
v3:
* new patches 1 and 2, fixing a pre-existing use-after-free on remove() and an out-of-bounds read with channel_count == 0
* patch 3 (was 1): serialise audio enable/disable with audio_lock; track mute state so the FIFO-error IRQ cannot undo a mute
* patch 4 (was 2): no changes
v2:
* keep and unregister the hdmi-codec platform_device on remove
* initialise delayed_audio before registering the codec device
* cancel delayed_audio synchronously on shutdown and remove
* disable audio immediately when muting
* patch 2: drop the encoder check instead of returning 0 early
v1: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v3: https://lore.kernel.org/all/[email protected]/
v4: https://lore.kernel.org/all/[email protected]/
v5: https://lore.kernel.org/all/[email protected]/
v6: https://lore.kernel.org/all/[email protected]/
v7: https://lore.kernel.org/all/[email protected]/
Daniel Golle (13):
drm/bridge: it6505: quiesce event sources and work on remove() drm/bridge: it6505: quiesce work items before powering off drm/bridge: it6505: balance and disable runtime PM on remove drm/bridge: it6505: unregister DP AUX adapter on bridge detach drm/bridge: it6505: complete poweroff even if disabling regulators
fails
drm/bridge: it6505: bail out of the IRQ handler when status reads fail drm/bridge: it6505: avoid division by zero in pixel clock calculation drm/bridge: it6505: avoid division by zero in audio FS debug print drm/bridge: it6505: guard against zero channel count in audio
infoframe
drm/bridge: it6505: hold endpoint OF node reference while parsing it drm/bridge: it6505: reject a too short link-frequencies property drm/bridge: it6505: don't write an error code back to the reset
register
drm/bridge: it6505: Don't reject audio hw_params without an encoder
Jiaxin Yu (1):
drm/bridge: it6505: Add audio support
drivers/gpu/drm/bridge/ite-it6505.c | 263 +++++++++++++++++++++------- 1 file changed, 202 insertions(+), 61 deletions(-)
base-commit: f5f84daefcd92d7a630066635ecea1433ed5eac7
Signature · 1 line
2.55.0
PATCH v8 1/14 drm/bridge: it6505: quiesce event sources and work on remove()
struct it6505 is freed by devres right after remove() returns, but remove() cancels none of the driver's work items, so link_works, hdcp_wait_ksv_list, hdcp_work and extcon_wq can still run afterwards and dereference freed memory. Cancelling alone would not be enough:
the threaded IRQ and the extcon notifier stay live until devres teardown and can requeue the works.
Unregister the extcon notifier and disable the IRQ first, then cancel all work. Make it6505_remove_notifier_module() idempotent, tracking the registration in a flag under extcon_lock, as both .detach() and remove() call it now. The notifier_call pointer is left intact:
extcon traverses its raw notifier chain without holding a lock, so a traversal racing with the unregistration may still invoke the callback. Also initialise extcon_wq in probe: cancel_work_sync() on a never-initialised work item trips WARN_ON(!work->func).
---
v8: no changes
v7: no changes
v6: track registration in a flag instead of clearing notifier_call,
which an unlocked chain traversal racing the unregistration could have called as NULL
v5: serialise notifier registration state with extcon_lock
v4:
* quiesce event sources before cancelling work; retitled
* initialise extcon_wq in probe to avoid WARN_ON(!work->func)
v3: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
drivers/gpu/drm/bridge/ite-it6505.c +21 -6
@@ -443,6 +443,7 @@ struct it6505 {struct drm_display_mode source_output_mode;struct drm_display_mode video_info;struct notifier_block event_nb; +bool extcon_registered;struct extcon_dev *extcon;struct work_struct extcon_wq;int extcon_state;
@@ -2934,11 +2935,15 @@ static int it6505_use_notifier_module(struct it6505 *it6505)int ret;struct device *dev = it6505->dev; +mutex_lock (&it6505->extcon_lock); it6505->event_nb.notifier_call = it6505_extcon_notifier; -INIT_WORK (&it6505->extcon_wq, it6505_extcon_work); ret =devm_extcon_register_notifier (it6505->dev, it6505->extcon,EXTCON_DISP_DP , &it6505->event_nb); +if (!ret) + it6505->extcon_registered = true; +mutex_unlock (&it6505->extcon_lock); +if (ret) {dev_err (dev,"failed to register notifier for DP" );return ret;
@@ -2951,13 +2956,16 @@ static int it6505_use_notifier_module(struct it6505 *it6505)static void it6505_remove_notifier_module (struct it6505 *it6505) { -if (it6505->extcon) { -devm_extcon_unregister_notifier (it6505->dev, - it6505->extcon,EXTCON_DISP_DP , +mutex_lock (&it6505->extcon_lock); +if (it6505->extcon && it6505->extcon_registered) { +devm_extcon_unregister_notifier (it6505->dev, it6505->extcon, +EXTCON_DISP_DP , &it6505->event_nb); - -flush_work (&it6505->extcon_wq); + it6505->extcon_registered = false; } +mutex_unlock (&it6505->extcon_lock); + +flush_work (&it6505->extcon_wq); }static void __maybe_unused it6505_delayed_audio (struct work_struct *work)
@@ -3613,6 +3621,7 @@ static int it6505_i2c_probe(struct i2c_client *client)INIT_WORK (&it6505->link_works, it6505_link_training_work);INIT_WORK (&it6505->hdcp_wait_ksv_list, it6505_hdcp_wait_ksv_list);INIT_DELAYED_WORK (&it6505->hdcp_work, it6505_hdcp_work); +INIT_WORK (&it6505->extcon_wq, it6505_extcon_work);init_completion (&it6505->extcon_completion);memset (it6505->dpcd,0 ,sizeof (it6505->dpcd)); it6505->powered = false;
@@ -3645,6 +3654,12 @@ static void it6505_i2c_remove(struct i2c_client *client)drm_bridge_remove (&it6505->bridge);drm_dp_aux_unregister (&it6505->aux);it6505_debugfs_remove (it6505); +it6505_remove_notifier_module (it6505); +disable_irq (it6505->irq); +cancel_work_sync (&it6505->link_works); +cancel_work_sync (&it6505->hdcp_wait_ksv_list); +cancel_delayed_work_sync (&it6505->hdcp_work); +cancel_work_sync (&it6505->extcon_wq);it6505_poweroff (it6505);it6505_remove_edid (it6505); }
Signature · 1 line
2.55.0
Re: [PATCH v8 01/14] drm/bridge: it6505: quiesce event sources and work on remove()
Quoted 16 lines
struct it6505 is freed by devres right after
remove()returns, butremove()cancels none of the driver's work items, solink_works,hdcp_wait_ksv_list,hdcp_workandextcon_wqcan still run afterwards and dereference freed memory. Cancelling alone would not be enough:
the threaded IRQ and the extcon notifier stay live until devres teardown and can requeue the works.Unregister the extcon notifier and disable the IRQ first, then cancel all work. Make
it6505_remove_notifier_module()idempotent, tracking the registration in a flag underextcon_lock, as both .detach() andremove()call it now. Thenotifier_callpointer is left intact:
extcon traverses its raw notifier chain without holding a lock, so a traversal racing with the unregistration may still invoke the callback. Also initialiseextcon_wqin probe:cancel_work_sync()on a never-initialised work item tripsWARN_ON(!work->func).
Quoted 2 lines
Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")Signed-off-by Daniel Golle <[email protected]>
Looks good:
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH v8 2/14 drm/bridge: it6505: quiesce work items before powering off
In reply to Daniel Golleit6505_poweroff() asserts the reset line and disables the regulators while link_works, hdcp_wait_ksv_list and hdcp_work may still be running. The work items and the register accessors only check it6505->powered, and the flag is cleared last, so a work item keeps driving link training or HDCP authentication into a chip that is already off until the flag catches up. Cancel the three work items before touching the power rails. None of them takes a runtime PM reference or extcon_lock, so the synchronous cancel is safe from every context that powers the chip off.
---
v8: no changes
v7: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/gpu/drm/bridge/ite-it6505.c +3 -0
@@ -2804,6 +2804,9 @@ static int it6505_poweroff(struct it6505 *it6505) }disable_irq_nosync (it6505->irq); +cancel_work_sync (&it6505->link_works); +cancel_work_sync (&it6505->hdcp_wait_ksv_list); +cancel_delayed_work_sync (&it6505->hdcp_work);if (pdata->gpiod_reset)gpiod_set_value_cansleep (pdata->gpiod_reset,1 );
Signature · 1 line
2.55.0
Re: [PATCH v8 02/14] drm/bridge: it6505: quiesce work items before powering off
Quoted 11 lines · Daniel Golle
it6505_poweroff()asserts the reset line and disables the regulators whilelink_works,hdcp_wait_ksv_listandhdcp_workmay still be running. The work items and the register accessors only checkit6505->powered, and the flag is cleared last, so a work item keeps driving link training or HDCP authentication into a chip that is already off until the flag catches up. Cancel the three work items before touching the power rails. None of them takes a runtime PM reference orextcon_lock, so the synchronous cancel is safe from every context that powers the chip off.[...]
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH v8 3/14 drm/bridge: it6505: balance and disable runtime PM on remove
In reply to Daniel GolleRuntime PM is enabled in probe but never disabled on remove, so rebinding the driver warns "Unbalanced pm_runtime_enable!". The extcon work also holds a usage reference while a display is connected, which unbinding leaks; the count survives in struct device, so after a rebind the device never runtime-suspends again.
Drop the usage reference held for a connected display and disable runtime PM in remove().
---
v8: no changes
v7: no changes
v6: no changes
v5: also drop the usage ref held for a connected display; retitled
v4: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/gpu/drm/bridge/ite-it6505.c +3 -0
@@ -3663,6 +3663,9 @@ static void it6505_i2c_remove(struct i2c_client *client)cancel_work_sync (&it6505->hdcp_wait_ksv_list);cancel_delayed_work_sync (&it6505->hdcp_work);cancel_work_sync (&it6505->extcon_wq); +if (it6505->extcon_state) +pm_runtime_put_sync (&client->dev); +pm_runtime_disable (&client->dev);it6505_poweroff (it6505);it6505_remove_edid (it6505); }
Signature · 1 line
2.55.0
Re: [PATCH v8 03/14] drm/bridge: it6505: balance and disable runtime PM on remove
Quoted 10 lines · Daniel Golle
Runtime PM is enabled in probe but never disabled on remove, so rebinding the driver warns "Unbalanced
pm_runtime_enable!". The extcon work also holds a usage reference while a display is connected, which unbinding leaks; the count survives in struct device, so after a rebind the device never runtime-suspends again.Drop the usage reference held for a connected display and disable runtime PM in
remove().[...]
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH v8 4/14 drm/bridge: it6505: unregister DP AUX adapter on bridge detach
In reply to Daniel GolleThe DP AUX adapter is registered in .attach() but only unregistered in i2c remove(). Unbinding and rebinding the DRM device while the i2c driver stays bound registers the adapter a second time, which fails. Also unregister it in .detach() and on the .attach() error path, which so far leaked the registration. remove() keeps its call, which waits for in-flight AUX device users before devres frees the regmap; drm_dp_aux_unregister() does nothing for an adapter that is already unregistered.
---
v8: keep the unregister in remove() so an i2c unbind waits for
in-flight AUX device users before the regmap is freed
v7: no changes
v6: no changes
v5: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/gpu/drm/bridge/ite-it6505.c +2 -0
@@ -3096,6 +3096,7 @@ static int it6505_bridge_attach(struct drm_bridge *bridge, ret =it6505_use_notifier_module (it6505);if (ret <0 ) {dev_err (dev,"use notifier module failed" ); +drm_dp_aux_unregister (&it6505->aux);return ret; } }
@@ -3109,6 +3110,7 @@ static void it6505_bridge_detach(struct drm_bridge *bridge)flush_work (&it6505->link_works);it6505_remove_notifier_module (it6505); +drm_dp_aux_unregister (&it6505->aux); } static enum drm_mode_status
Signature · 1 line
2.55.0
Luca Ceresoli
reviewerRe: [PATCH v8 04/14] drm/bridge: it6505: unregister DP AUX adapter on bridge detach
Hello Daniel,
Quoted 8 lines
The DP AUX adapter is registered in .attach() but only unregistered in i2c
remove(). Unbinding and rebinding the DRM device while the i2c driver stays bound registers the adapter a second time, which fails. Also unregister it in .detach() and on the .attach() error path, which so far leaked the registration.remove()keeps its call, which waits for in-flight AUX device users before devres frees the regmap;drm_dp_aux_unregister()does nothing for an adapter that is already unregistered.
The drm_dp_aux_unregister() docs don't say it is idempotent, so is it fair to assume it is and always will?
I think it would be much cleaner if user drivers took case of calling it at the appropriate times, not more.
A flag similar to it6505->extcon_state used in patch 3 could allow that?
Best regards,
Luca
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH v8 5/14 drm/bridge: it6505: complete poweroff even if disabling regulators fails
In reply to Daniel Golleit6505_poweroff() returns early when regulator_disable() fails, leaving it6505->powered set with the IRQ already disabled. The next it6505_poweron() then takes its early return and never re-enables the IRQ, leaving the bridge deaf to hotplug and link training interrupts. The regulator core keeps the consumer's enable count on a failed disable either way, so bailing out only adds a wedged bridge on top of the leaked reference. Log the error and complete the power-off state transition instead.
---
v8: terminate the messages with a newline
v7: no changes
v6: note that the regulator reference is leaked either way
v5: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/ite-it6505.c +4 -2
@@ -2814,13 +2814,15 @@ static int it6505_poweroff(struct it6505 *it6505)if (pdata->pwr18) { err =regulator_disable (pdata->pwr18);if (err) -return err; +dev_err (dev,"cannot disable pwr18 regulator: %d\n" , + err); }if (pdata->ovdd) { err =regulator_disable (pdata->ovdd);if (err) -return err; +dev_err (dev,"cannot disable ovdd regulator: %d\n" , + err); } it6505->powered = false;
Signature · 1 line
2.55.0
Re: [PATCH v8 05/14] drm/bridge: it6505: complete poweroff even if disabling regulators fails
Quoted 10 lines · Daniel Golle
it6505_poweroff()returns early whenregulator_disable()fails, leavingit6505->poweredset with the IRQ already disabled. The nextit6505_poweron()then takes its early return and never re-enables the IRQ, leaving the bridge deaf to hotplug and link training interrupts. The regulator core keeps the consumer's enable count on a failed disable either way, so bailing out only adds a wedged bridge on top of the leaked reference. Log the error and complete the power-off state transition instead.[...]
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH v8 6/14 drm/bridge: it6505: bail out of the IRQ handler when status reads fail
In reply to Daniel GolleWhen reading the interrupt status registers fails, the negative error codes end up in int_status[], where it6505_test_bit() sees almost all bits set: every interrupt sub-handler runs on garbage and the error values are even written back to the status registers. Return IRQ_NONE instead.
---
v8: no changes
v7: no changes
v6: no changes
v5: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/gpu/drm/bridge/ite-it6505.c +5 -0
@@ -2714,6 +2714,11 @@ static irqreturn_t it6505_int_threaded_handler(int unused, void *data) int_status[1 ] =it6505_read (it6505,INT_STATUS_02 ); int_status[2 ] =it6505_read (it6505,INT_STATUS_03 ); +if (int_status[0 ] <0 || int_status[1 ] <0 || int_status[2 ] <0 ) { +pm_runtime_put_sync (dev); +return IRQ_NONE ; + } +it6505_write (it6505,INT_STATUS_01 , int_status[0 ]);it6505_write (it6505,INT_STATUS_02 , int_status[1 ]);it6505_write (it6505,INT_STATUS_03 , int_status[2 ]);
Signature · 1 line
2.55.0
Luca Ceresoli
reviewerRe: [PATCH v8 06/14] drm/bridge: it6505: bail out of the IRQ handler when status reads fail
Hello Daniel,
Quoted 14 lines
When reading the interrupt status registers fails, the negative error codes end up in int_status[], where
it6505_test_bit()sees almost all bits set: every interrupt sub-handler runs on garbage and the error values are even written back to the status registers. ReturnIRQ_NONEinstead.Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")Signed-off-by Daniel Golle <[email protected]>drivers/gpu/drm/bridge/ite-it6505.c +0
-0
drivers/gpu/drm/bridge/ite-it6505.c static irqreturn_t it6505_int_threaded_handler(int unused, void *data) · line 2720
Quoted 7 lines
drivers/gpu/drm/bridge/ite-it6505.c +4
-0@@ -2714,6 +2714,11 @@ static irqreturn_t it6505_int_threaded_handler(int unused, void *data) int_status[1 ] =it6505_read (it6505,INT_STATUS_02 ); int_status[2 ] =it6505_read (it6505,INT_STATUS_03 ); +if (int_status[0 ] <0 || int_status[1 ] <0 || int_status[2 ] <0 ) { +pm_runtime_put_sync (dev); +return IRQ_NONE ; + }
One first question it whether a read failure is realistic or not, and whether you have experienced it on real hardware.
Theoretical or real, the issue is in the code, but I think a cleaner solution exists.
Considering that:
* it6505_read() does 2 things:
1. check whether it6505->powered is true, else return error
2. regmap_read()
* it6505_int_threaded_handler() already checks for it6505->powered earlier, so it6505_read() is basically a regmap_read() wrapper cramming an error or a register value in the same return value
I would consider replacing all the 3 it6505_read() calls with a single regmap_bulk_read(), making error management cleaner.
What do you think about this option?
Perhaps the three it6505_write() below could also become a bulk write.
Luca
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH v8 7/14 drm/bridge: it6505: avoid division by zero in pixel clock calculation
In reply to Daniel Golleit6505_calc_video_info() checks the sum of the three pixel clock counter samples for zero before dividing it by 3, so sums of 1 or 2 truncate to 0 and the following pixel clock calculation divides by zero. Divide first and check the result.
---
v8: no changes
v7: no changes
v6: no changes
v5: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/gpu/drm/bridge/ite-it6505.c +2 -1
@@ -751,12 +751,13 @@ static void it6505_calc_video_info(struct it6505 *it6505) sum += rddata; } + sum /=3 ; +if (sum ==0 ) {DRM_DEV_DEBUG_DRIVER (dev,"calc video timing error" );return ; } - sum /=3 ; pclk =13500 *2048 / sum; it6505->video_info.clock = pclk; it6505->video_info.hdisplay = hdew;
Signature · 1 line
2.55.0
Re: [PATCH v8 07/14] drm/bridge: it6505: avoid division by zero in pixel clock calculation
Quoted 4 lines · Daniel Golle
it6505_calc_video_info()checks the sum of the three pixel clock counter samples for zero before dividing it by 3, so sums of 1 or 2 truncate to 0 and the following pixel clock calculation divides by zero. Divide first and check the result.
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH v8 8/14 drm/bridge: it6505: avoid division by zero in audio FS debug print
In reply to Daniel GolleREG_AUDIO_INPUT_FREQ reads back 0 when no audio clock has been latched, and it6505_read() returns a negative errno on i2c failure. The debug print divides by the readback, and its arguments are evaluated even with debug output disabled. Only print when the value is usable as a divisor.
---
v8: no changes
v7: no changes
v6: no changes
v5: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/ite-it6505.c +5 -2
@@ -1623,8 +1623,11 @@ static void it6505_enable_audio(struct it6505 *it6505)it6505_set_bits (it6505,REG_AUDIO_SRC_CTRL ,AUDIO_FIFO_RESET ,0x00 );it6505_set_bits (it6505,REG_RESET_CTRL ,AUDIO_RESET ,0x00 ); regbe =it6505_read (it6505,REG_AUDIO_INPUT_FREQ ); -DRM_DEV_DEBUG_DRIVER (dev,"regbe:0x%02x audio input fs: %d.%d kHz" , - regbe,6750 / regbe, (6750 % regbe) *10 / regbe); +if (regbe >0 ) +DRM_DEV_DEBUG_DRIVER (dev, +"regbe:0x%02x audio input fs: %d.%d kHz" , + regbe,6750 / regbe, + (6750 % regbe) *10 / regbe);it6505_set_bits (it6505,REG_DATA_MUTE_CTRL ,EN_AUD_MUTE ,0x00 ); }
Signature · 1 line
2.55.0
Luca Ceresoli
reviewerRe: [PATCH v8 08/14] drm/bridge: it6505: avoid division by zero in audio FS debug print
Quoted 17 lines
REG_AUDIO_INPUT_FREQreads back 0 when no audio clock has been latched, andit6505_read()returns a negative errno on i2c failure. The debug print divides by the readback, and its arguments are evaluated even with debug output disabled. Only print when the value is usable as a divisor.Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")Signed-off-by Daniel Golle <[email protected]>drivers/gpu/drm/bridge/ite-it6505.c +0
-0@@ -1623,8 +1623,11 @@ static void it6505_enable_audio(struct it6505 *it6505)it6505_set_bits (it6505,REG_AUDIO_SRC_CTRL ,AUDIO_FIFO_RESET ,0x00 );it6505_set_bits (it6505,REG_RESET_CTRL ,AUDIO_RESET ,0x00 ); regbe =it6505_read (it6505,REG_AUDIO_INPUT_FREQ );
drivers/gpu/drm/bridge/ite-it6505.c static void it6505_enable_audio(struct it6505 *it6505) · line 1633
Quoted 10 lines
drivers/gpu/drm/bridge/ite-it6505.c +5
-2-DRM_DEV_DEBUG_DRIVER (dev,"regbe:0x%02x audio input fs: %d.%d kHz" , - regbe,6750 / regbe, (6750 % regbe) *10 / regbe); +if (regbe >0 ) +DRM_DEV_DEBUG_DRIVER (dev, +"regbe:0x%02x audio input fs: %d.%d kHz" , + regbe,6750 / regbe, + (6750 % regbe) *10 / regbe);it6505_set_bits (it6505,REG_DATA_MUTE_CTRL ,EN_AUD_MUTE ,0x00 ); }
By just looking at your patch and the current driver code I'm failing to understand what's going on with the REG_AUDIO_INPUT_FREQ register:
* it is read, but never written
* its value is used by:
- it6505_enable_audio() only for logging: is that logging really useful?
- it6505_audio_input(), just checking whether it is 0xFF or not; what's
that 0xFF? Is it documented? A macro hinting at what 0xFF means would
be better than just '0xFF'If you could share what that register is for, it would probably answer some of those questions.
Best regards,
Luca
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH v8 9/14 drm/bridge: it6505: guard against zero channel count in audio infoframe
In reply to Daniel Golleit6505->audio.channel_count stays 0 until link training or a valid hw_params call has run, but it6505_enable_audio() can be reached before that from the audio-FIFO-error IRQ, making it6505_enable_audio_infoframe() index the 8-entry audio_info_ca[] table with -1. Bail out while the channel count is not yet known.
Also fix the debug print on the invalid-channel-count path, which logged the previously cached count instead of the rejected one.
---
v8: no changes
v7: no changes
v6: no changes
v5: no changes
v4: no changes, collected Chen-Yu Tsai's Reviewed-by
v3: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/gpu/drm/bridge/ite-it6505.c +4 -1
@@ -1577,6 +1577,9 @@ static void it6505_enable_audio_infoframe(struct it6505 *it6505)struct device *dev = it6505->dev;u8 audio_info_ca[] = {0x00 ,0x00 ,0x01 ,0x03 ,0x07 ,0x0B ,0x0F ,0x1F }; +if (!it6505->audio.channel_count) +return ; +DRM_DEV_DEBUG_DRIVER (dev,"infoframe channel_allocation:0x%02x" , audio_info_ca[it6505->audio.channel_count -1 ]);
@@ -3012,7 +3015,7 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,if (params->cea.channels <=1 || params->cea.channels >8 ) {DRM_DEV_DEBUG_DRIVER (dev,"channel number: %d not support" , - it6505->audio.channel_count); + params->cea.channels);return -EINVAL ; }
Signature · 1 line
2.55.0
PATCH v8 10/14 drm/bridge: it6505: hold endpoint OF node reference while parsing it
In reply to Daniel Golleit6505_parse_dt() drops the reference to each endpoint node right after looking it up and then keeps reading its properties, racing with the node being freed under CONFIG_OF_DYNAMIC. Put the node only once parsing is done.
---
v8: no changes
v7: no changes
v6: no changes
v5: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/bridge/ite-it6505.c +2 -2
@@ -3369,7 +3369,6 @@ static void it6505_parse_dt(struct it6505 *it6505) } ep =of_graph_get_endpoint_by_regs (np,1 ,0 ); -of_node_put (ep);if (ep) { len =it6505_get_data_lanes_count (ep,1 ,4 );
@@ -3382,13 +3381,13 @@ static void it6505_parse_dt(struct it6505 *it6505) *max_lane_count =MAX_LANE_COUNT ;dev_err (dev,"error data-lanes, use default" ); } +of_node_put (ep); }else { *max_lane_count =MAX_LANE_COUNT ;dev_err (dev,"error endpoint, use default" ); } ep =of_graph_get_endpoint_by_regs (np,0 ,0 ); -of_node_put (ep);if (ep) { len = of_property_read_variable_u64_array(ep,
@@ -3408,6 +3407,7 @@ static void it6505_parse_dt(struct it6505 *it6505)dev_err (dev,"error link frequencies, use default" ); *max_dpi_pixel_clock =DPI_PIXEL_CLK_MAX ; } +of_node_put (ep); }else {dev_err (dev,"error endpoint, use default" ); *max_dpi_pixel_clock =DPI_PIXEL_CLK_MAX ;
Signature · 1 line
2.55.0
PATCH v8 11/14 drm/bridge: it6505: reject a too short link-frequencies property
In reply to Daniel Golleof_property_read_variable_u64_array() is asked for a minimum array size of zero, so a link-frequencies property shorter than one u64 -- a 32-bit value written by mistake, say -- returns 0 without storing anything. it6505_parse_dt() reads that as success and derives max_dpi_pixel_clock from an uninitialised stack variable. Ask for one element so a short property lands in the existing error path.
---
v8: no changes
v7: no changes
v6: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/bridge/ite-it6505.c +1 -1
@@ -3392,7 +3392,7 @@ static void it6505_parse_dt(struct it6505 *it6505)if (ep) { len =of_property_read_variable_u64_array (ep,"link-frequencies" , - &link_frequencies,0 , + &link_frequencies,1 ,1 );if (len >=0 ) {do_div (link_frequencies,1000 );
Signature · 1 line
2.55.0
PATCH v8 12/14 drm/bridge: it6505: don't write an error code back to the reset register
In reply to Daniel Golleit6505_audio_input() saves REG_RESET_CTRL, pulses AUDIO_RESET and writes the saved value back. it6505_read() returns a negative errno on i2c failure, which regmap truncates to eight bits on the way out:-EIO turns into 0xfb, holding video, audio, AUX and HDCP in reset. A failed REG_AUDIO_INPUT_FREQ read is just as wrong, as any negative value passes the != 0xFF test and reports an audio clock that was never sampled. Bail out before touching the register and only report audio input for a usable readback.
---
v8: no changes
v7: no changes
v6: new patch
--- drivers/gpu/drm/bridge/ite-it6505.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/gpu/drm/bridge/ite-it6505.c +4 -1
@@ -1516,12 +1516,15 @@ static bool it6505_audio_input(struct it6505 *it6505)int reg05, regbe; reg05 =it6505_read (it6505,REG_RESET_CTRL ); +if (reg05 <0 ) +return false; +it6505_set_bits (it6505,REG_RESET_CTRL ,AUDIO_RESET ,0x00 );usleep_range (3000 ,4000 ); regbe =it6505_read (it6505,REG_AUDIO_INPUT_FREQ );it6505_write (it6505,REG_RESET_CTRL , reg05); -return regbe !=0xFF ; +return regbe >0 && regbe !=0xFF ; }static void it6505_setup_audio_channel_status (struct it6505 *it6505)
Signature · 1 line
2.55.0
PATCH v8 13/14 drm/bridge: it6505: Add audio support
In reply to Daniel GolleFrom: Jiaxin Yu <[email protected]>
Add audio support for it6505 by bridging to the hdmi-codec: register an "hdmi-audio-codec" platform device from probe and wire up the previously unused audio helpers via hdmi_codec_ops. This unblocks the mt8186-mt6366 sound card which references it6505 as the I2S3 codec.
Audio starts out muted and is only enabled once the stream is unmuted, since some DP-to-HDMI dongles get into a bad state if the InfoFrame is sent without audio data. Enable/disable can race between the FIFO-error IRQ, the delayed enable work, the hdmi-codec ops and the HPD-low path, so the register sequences, the mute state and the cached stream parameters are serialised with a new audio_lock mutex. it6505_poweron() and it6505_poweroff() change it6505->powered under the same lock, poweron holding it across the logic reset and init
sequence: the codec ops and the delayed work check the flag before writing registers, and the mute and shutdown ops run from ALSA, where a work cancel cannot reach them. plugged_cb/codec_dev updates take mode_lock, the lock it6505_detect() holds when calling back into the codec.
---
v8: it6505_poweron() sets the powered flag and runs the logic reset
and init sequence under audio_lock, so the delayed enable work cannot interleave with it; terminate the new dev_err() message with a newline
v7: it6505_poweroff() clears the powered flag under audio_lock so the
codec ops and the delayed enable work cannot write to a chip being powered off; fold the it6505_enable_audio() wrapper into it6505_delayed_audio()
v6: no changes
v5: update plugged_cb/codec_dev under mode_lock
v4:
* start out muted
* update mute state and cached parameters under audio_lock; the FIFO-error IRQ probes and enables audio in one locked section
* unregister the codec device only after quiesce on remove
v3: serialise enable/disable with audio_lock; track mute state so
the FIFO-error IRQ cannot undo a mute
v2 (respin of Jiaxin Yu's v3 on current -next):
* drive enable/disable from .mute_stream (hdmi_codec_ops lost .trigger), use it6505->dev
* keep and unregister the codec platform_device on remove, cancel delayed_audio on shutdown and remove, disable audio when muting
--- drivers/gpu/drm/bridge/ite-it6505.c | 168 ++++++++++++++++++++++------ 1 file changed, 136 insertions(+), 32 deletions(-)
drivers/gpu/drm/bridge/ite-it6505.c +136 -32
@@ -407,6 +407,7 @@ struct it6505_audio_data {u8 i2s_data_delay;u8 i2s_ws_channel;u8 i2s_data_sequence; +bool mute; };struct it6505_audio_sample_rate_map {
@@ -439,6 +440,7 @@ struct it6505 {struct mutex extcon_lock;struct mutex mode_lock;/* used to bridge_detect */ struct mutex aux_lock;/* used to aux data transfers */ +struct mutex audio_lock;/* audio enable/disable vs. power on/off */ struct regmap *regmap;struct drm_display_mode source_output_mode;struct drm_display_mode video_info;
@@ -477,6 +479,7 @@ struct it6505 {bool enable_enhanced_frame;hdmi_codec_plugged_cb plugged_cb;struct device *codec_dev; +struct platform_device *audio_pdev;struct delayed_work delayed_audio;struct it6505_audio_data audio;struct dentry *debugfs;
@@ -1599,7 +1602,7 @@ static void it6505_enable_audio_infoframe(struct it6505 *it6505) EN_AUD_CTRL_PKT); } -static void it6505_disable_audio (struct it6505 *it6505) +static void __it6505_disable_audio (struct it6505 *it6505) {it6505_set_bits (it6505,REG_DATA_MUTE_CTRL ,EN_AUD_MUTE ,EN_AUD_MUTE );it6505_set_bits (it6505,REG_AUDIO_SRC_CTRL ,M_AUDIO_I2S_EN ,0x00 );
@@ -1607,13 +1610,21 @@ static void it6505_disable_audio(struct it6505 *it6505)it6505_set_bits (it6505,REG_RESET_CTRL ,AUDIO_RESET ,AUDIO_RESET ); } -static void it6505_enable_audio (struct it6505 *it6505) +static void it6505_disable_audio (struct it6505 *it6505) +{ +mutex_lock (&it6505->audio_lock); +__it6505_disable_audio (it6505); +mutex_unlock (&it6505->audio_lock); +} + +static void __it6505_enable_audio (struct it6505 *it6505) {struct device *dev = it6505->dev;int regbe;DRM_DEV_DEBUG_DRIVER (dev,"start" ); -it6505_disable_audio (it6505); + +__it6505_disable_audio (it6505);it6505_setup_audio_channel_status (it6505);it6505_setup_audio_format (it6505);
@@ -2331,19 +2342,12 @@ static void it6505_stop_link_train(struct it6505 *it6505)static void it6505_link_train_ok (struct it6505 *it6505) { -struct device *dev = it6505->dev; - it6505->link_state =LINK_OK ;/* disalbe mute enable avi info frame */ it6505_set_bits (it6505,REG_DATA_MUTE_CTRL ,EN_VID_MUTE ,0x00 );it6505_set_bits (it6505,REG_INFOFRAME_CTRL ,EN_VID_CTRL_PKT ,EN_VID_CTRL_PKT ); -if (it6505_audio_input (it6505)) { -DRM_DEV_DEBUG_DRIVER (dev,"Enable audio!" ); -it6505_enable_audio (it6505); - } -if (it6505->hdcp_desired)it6505_start_hdcp (it6505); }
@@ -2635,8 +2639,10 @@ static void it6505_irq_audio_fifo_error(struct it6505 *it6505)DRM_DEV_DEBUG_DRIVER (dev,"audio fifo error Interrupt" ); -if (it6505_audio_input (it6505)) -it6505_enable_audio (it6505); +mutex_lock (&it6505->audio_lock); +if (!it6505->audio.mute &&it6505_audio_input (it6505)) +__it6505_enable_audio (it6505); +mutex_unlock (&it6505->audio_lock); }static void it6505_irq_link_train_fail (struct it6505 *it6505)
@@ -2794,11 +2800,13 @@ static int it6505_poweron(struct it6505 *it6505)usleep_range (25000 ,35000 ); } +mutex_lock (&it6505->audio_lock); it6505->powered = true;it6505_reset_logic (it6505);it6505_int_mask_enable (it6505);it6505_init (it6505);it6505_lane_off (it6505); +mutex_unlock (&it6505->audio_lock);enable_irq (it6505->irq);
@@ -2823,6 +2831,8 @@ static int it6505_poweroff(struct it6505 *it6505)cancel_work_sync (&it6505->hdcp_wait_ksv_list);cancel_delayed_work_sync (&it6505->hdcp_work); +mutex_lock (&it6505->audio_lock); +if (pdata->gpiod_reset)gpiod_set_value_cansleep (pdata->gpiod_reset,1 );
@@ -2842,6 +2852,7 @@ static int it6505_poweroff(struct it6505 *it6505) it6505->powered = false; it6505->sink_count =0 ; +mutex_unlock (&it6505->audio_lock);return 0 ; }
@@ -2988,25 +2999,28 @@ static void it6505_remove_notifier_module(struct it6505 *it6505)flush_work (&it6505->extcon_wq); } -static void __maybe_unused it6505_delayed_audio (struct work_struct *work) +static void it6505_delayed_audio (struct work_struct *work) { struct it6505 *it6505 = container_of(work, struct it6505, delayed_audio.work);DRM_DEV_DEBUG_DRIVER (it6505->dev,"start" ); -if (!it6505->powered) +if (it6505->enable_drv_hold)return ; -if (!it6505->enable_drv_hold) -it6505_enable_audio (it6505); +mutex_lock (&it6505->audio_lock); +if (it6505->powered && !it6505->audio.mute) +__it6505_enable_audio (it6505); +mutex_unlock (&it6505->audio_lock); } -static int __maybe_unused it6505_audio_setup_hw_params (struct it6505 *it6505, -struct hdmi_codec_params - *params) +static int it6505_audio_setup_hw_params (struct it6505 *it6505, +struct hdmi_codec_params + *params) {struct device *dev = it6505->dev; +u8 word_length;int i =0 ; DRM_DEV_DEBUG_DRIVER(dev, "%s %d Hz, %d bit, %d channels\n", __func__,
@@ -3022,8 +3036,6 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,return -EINVAL ; } - it6505->audio.channel_count = params->cea.channels; -while (i <ARRAY_SIZE (audio_sample_rate_map) && params->sample_rate != audio_sample_rate_map[i].sample_rate_value) {
@@ -3034,21 +3046,20 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505, params->sample_rate);return -EINVAL ; } - it6505->audio.sample_rate = audio_sample_rate_map[i].rate;switch (params->sample_width) {case 16 : - it6505->audio.word_length =WORD_LENGTH_16BIT ; + word_length =WORD_LENGTH_16BIT ;break ;case 18 : - it6505->audio.word_length =WORD_LENGTH_18BIT ; + word_length =WORD_LENGTH_18BIT ;break ;case 20 : - it6505->audio.word_length =WORD_LENGTH_20BIT ; + word_length =WORD_LENGTH_20BIT ;break ;case 24 :case 32 : - it6505->audio.word_length =WORD_LENGTH_24BIT ; + word_length =WORD_LENGTH_24BIT ;break ;default : DRM_DEV_DEBUG_DRIVER(dev, "wordlength: %d bit not support",
@@ -3056,27 +3067,111 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,return -EINVAL ; } +mutex_lock (&it6505->audio_lock); + it6505->audio.channel_count = params->cea.channels; + it6505->audio.sample_rate = audio_sample_rate_map[i].rate; + it6505->audio.word_length = word_length; +mutex_unlock (&it6505->audio_lock); +return 0 ; } -static void __maybe_unused it6505_audio_shutdown (struct device *dev,void *data) +static void it6505_audio_shutdown (struct device *dev,void *data) {struct it6505 *it6505 =dev_get_drvdata (dev); +mutex_lock (&it6505->audio_lock); + it6505->audio.mute = true;if (it6505->powered) -it6505_disable_audio (it6505); +__it6505_disable_audio (it6505); +mutex_unlock (&it6505->audio_lock); +cancel_delayed_work_sync (&it6505->delayed_audio); +} + +static int it6505_audio_hw_params (struct device *dev,void *data, +struct hdmi_codec_daifmt *daifmt, +struct hdmi_codec_params *params) +{ +struct it6505 *it6505 =dev_get_drvdata (dev); + +return it6505_audio_setup_hw_params (it6505, params); +} + +static int it6505_audio_mute (struct device *dev,void *data, +bool enable,int direction) +{ +struct it6505 *it6505 =dev_get_drvdata (dev); + +DRM_DEV_DEBUG_DRIVER (dev,"mute: %d" , enable); + +/* +* Delay enabling audio until the stream is unmuted; InfoFrames +* without audio data upset some DP-to-HDMI dongles. +*/ +if (enable) { +mutex_lock (&it6505->audio_lock); + it6505->audio.mute = true; +if (it6505->powered) +__it6505_disable_audio (it6505); +mutex_unlock (&it6505->audio_lock); +cancel_delayed_work_sync (&it6505->delayed_audio); + }else { +mutex_lock (&it6505->audio_lock); + it6505->audio.mute = false; +mutex_unlock (&it6505->audio_lock); +queue_delayed_work (system_wq, &it6505->delayed_audio, +msecs_to_jiffies (180 )); + } + +return 0 ; } -static int __maybe_unused it6505_audio_hook_plugged_cb (struct device *dev, -void *data, -hdmi_codec_plugged_cb fn, -struct device *codec_dev) +static int it6505_audio_hook_plugged_cb (struct device *dev, +void *data, +hdmi_codec_plugged_cb fn, +struct device *codec_dev) {struct it6505 *it6505 = data; +mutex_lock (&it6505->mode_lock); it6505->plugged_cb = fn; it6505->codec_dev = codec_dev;it6505_plugged_status_to_codec (it6505); +mutex_unlock (&it6505->mode_lock); + +return 0 ; +} + +static const struct hdmi_codec_ops it6505_audio_codec_ops = { + .hw_params = it6505_audio_hw_params, + .mute_stream = it6505_audio_mute, + .audio_shutdown = it6505_audio_shutdown, + .hook_plugged_cb = it6505_audio_hook_plugged_cb, +}; + +static int it6505_register_audio_driver (struct device *dev) +{ +struct it6505 *it6505 =dev_get_drvdata (dev); +struct hdmi_codec_pdata codec_data = { + .ops = &it6505_audio_codec_ops, + .max_i2s_channels =8 , + .i2s =1 , + .no_capture_mute =1 , + .data = it6505, + }; +struct platform_device *pdev; + + it6505->audio.mute = true; +INIT_DELAYED_WORK (&it6505->delayed_audio, it6505_delayed_audio); + + pdev =platform_device_register_data (dev,HDMI_CODEC_DRV_NAME , +PLATFORM_DEVID_AUTO , &codec_data, +sizeof (codec_data)); +if (IS_ERR (pdev)) +return PTR_ERR (pdev); + + it6505->audio_pdev = pdev; +DRM_DEV_DEBUG_DRIVER (dev,"bound to %s" ,HDMI_CODEC_DRV_NAME );return 0 ; }
@@ -3592,6 +3687,7 @@ static int it6505_i2c_probe(struct i2c_client *client)mutex_init (&it6505->extcon_lock);mutex_init (&it6505->mode_lock);mutex_init (&it6505->aux_lock); +mutex_init (&it6505->audio_lock); it6505->bridge.of_node = client->dev.of_node; it6505->connector_status = connector_status_disconnected;
@@ -3640,6 +3736,12 @@ static int it6505_i2c_probe(struct i2c_client *client)if (err)return err; + err =it6505_register_audio_driver (dev); +if (err <0 ) { +dev_err (dev,"Failed to register audio driver: %d\n" , err); +return err; + } +INIT_WORK (&it6505->link_works, it6505_link_training_work);INIT_WORK (&it6505->hdcp_wait_ksv_list, it6505_hdcp_wait_ksv_list);INIT_DELAYED_WORK (&it6505->hdcp_work, it6505_hdcp_work);
@@ -3682,6 +3784,8 @@ static void it6505_i2c_remove(struct i2c_client *client)cancel_work_sync (&it6505->hdcp_wait_ksv_list);cancel_delayed_work_sync (&it6505->hdcp_work);cancel_work_sync (&it6505->extcon_wq); +platform_device_unregister (it6505->audio_pdev); +cancel_delayed_work_sync (&it6505->delayed_audio);if (it6505->extcon_state)pm_runtime_put_sync (&client->dev);pm_runtime_disable (&client->dev);
Signature · 1 line
2.55.0
PATCH v8 14/14 drm/bridge: it6505: Don't reject audio hw_params without an encoder
In reply to Daniel Golleit6505_audio_setup_hw_params() returns -ENODEV when no encoder is attached. With it6505 registering an hdmi-audio-codec this runs for every stream on the I2S DAI, and on mt8186-corsola, where the rt1019 speaker amplifier shares I2S3, the error tears down the whole DPCM backend and breaks speaker playback whenever no display is attached.
The function only caches stream parameters, which needs no encoder. Drop the check, and apply the audio defaults once at probe instead of in it6505_variable_config(), which would clobber the cached parameters again on every hotplug. Actual audio output remains gated by it6505->powered.
---
v8: no changes
v7: no changes
v6: no changes
v5: no changes
v4: apply audio defaults at probe time instead of in
it6505_variable_config()
v3: no changes
v2: drop the encoder check entirely instead of returning 0 early
--- drivers/gpu/drm/bridge/ite-it6505.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)
drivers/gpu/drm/bridge/ite-it6505.c +10 -13
@@ -1371,16 +1371,6 @@ static void it6505_variable_config(struct it6505 *it6505) it6505->link_state =LINK_IDLE ; it6505->hdcp_desired =HDCP_DESIRED ; it6505->auto_train_retry =AUTO_TRAIN_RETRY ; - it6505->audio.select =AUDIO_SELECT ; - it6505->audio.sample_rate =AUDIO_SAMPLE_RATE ; - it6505->audio.channel_count =AUDIO_CHANNEL_COUNT ; - it6505->audio.type =AUDIO_TYPE ; - it6505->audio.i2s_input_format =I2S_INPUT_FORMAT ; - it6505->audio.i2s_justified =I2S_JUSTIFIED ; - it6505->audio.i2s_data_delay =I2S_DATA_DELAY ; - it6505->audio.i2s_ws_channel =I2S_WS_CHANNEL ; - it6505->audio.i2s_data_sequence =I2S_DATA_SEQUENCE ; - it6505->audio.word_length =AUDIO_WORD_LENGTH ;memset (it6505->sha1_input,0 ,sizeof (it6505->sha1_input));memset (it6505->bksvs,0 ,sizeof (it6505->bksvs)); }
@@ -3027,9 +3017,6 @@ static int it6505_audio_setup_hw_params(struct it6505 *it6505, params->sample_rate, params->sample_width, params->cea.channels); -if (!it6505->bridge.encoder) -return -ENODEV ; -if (params->cea.channels <=1 || params->cea.channels >8 ) {DRM_DEV_DEBUG_DRIVER (dev,"channel number: %d not support" , params->cea.channels);
@@ -3161,6 +3148,16 @@ static int it6505_register_audio_driver(struct device *dev) };struct platform_device *pdev; + it6505->audio.select =AUDIO_SELECT ; + it6505->audio.sample_rate =AUDIO_SAMPLE_RATE ; + it6505->audio.channel_count =AUDIO_CHANNEL_COUNT ; + it6505->audio.type =AUDIO_TYPE ; + it6505->audio.i2s_input_format =I2S_INPUT_FORMAT ; + it6505->audio.i2s_justified =I2S_JUSTIFIED ; + it6505->audio.i2s_data_delay =I2S_DATA_DELAY ; + it6505->audio.i2s_ws_channel =I2S_WS_CHANNEL ; + it6505->audio.i2s_data_sequence =I2S_DATA_SEQUENCE ; + it6505->audio.word_length =AUDIO_WORD_LENGTH ; it6505->audio.mute = true;INIT_DELAYED_WORK (&it6505->delayed_audio, it6505_delayed_audio);
Signature · 1 line
2.55.0