2023-11-24 15:24:57

by Alvin Šipraga

[permalink] [raw]
Subject: [PATCH v2 0/2] drm/bridge: adv7511: get edid in hpd_work to update CEC phys address

This series fixes a small bug where the CEC adapter could have an
invalid CEC address even though we got a hotplug connect and could have
read it.

Signed-off-by: Alvin Šipraga <[email protected]>
---
Changes in v2:
- Rearrange driver code to avoid the previous prototype of
adv7511_get_edid(), per Laurent's feedback
- Free the returned EDID to prevent a memory leak, per Jani's comment
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Alvin Šipraga (2):
drm/bridge: adv7511: rearrange hotplug work code
drm/bridge: adv7511: get edid in hpd_work to update CEC phys address

drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 154 ++++++++++++++++-----------
1 file changed, 90 insertions(+), 64 deletions(-)
---
base-commit: ab93edb2f94c3c0d5965be3815782472adbe3f52
change-id: 20231014-adv7511-cec-edid-ff75bd3ac929


2023-11-24 15:25:02

by Alvin Šipraga

[permalink] [raw]
Subject: [PATCH v2 1/2] drm/bridge: adv7511: rearrange hotplug work code

From: Alvin Šipraga <[email protected]>

In preparation for calling EDID helpers from the hotplug work, move the
hotplug work below the EDID helper section. No functional change.

Signed-off-by: Alvin Šipraga <[email protected]>
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 120 ++++++++++++++-------------
1 file changed, 62 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 8be235144f6d..5ffc5904bd59 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -406,64 +406,6 @@ static void adv7511_power_off(struct adv7511 *adv7511)
* Interrupt and hotplug detection
*/

-static bool adv7511_hpd(struct adv7511 *adv7511)
-{
- unsigned int irq0;
- int ret;
-
- ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0);
- if (ret < 0)
- return false;
-
- if (irq0 & ADV7511_INT0_HPD) {
- regmap_write(adv7511->regmap, ADV7511_REG_INT(0),
- ADV7511_INT0_HPD);
- return true;
- }
-
- return false;
-}
-
-static void adv7511_hpd_work(struct work_struct *work)
-{
- struct adv7511 *adv7511 = container_of(work, struct adv7511, hpd_work);
- enum drm_connector_status status;
- unsigned int val;
- int ret;
-
- ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, &val);
- if (ret < 0)
- status = connector_status_disconnected;
- else if (val & ADV7511_STATUS_HPD)
- status = connector_status_connected;
- else
- status = connector_status_disconnected;
-
- /*
- * The bridge resets its registers on unplug. So when we get a plug
- * event and we're already supposed to be powered, cycle the bridge to
- * restore its state.
- */
- if (status == connector_status_connected &&
- adv7511->connector.status == connector_status_disconnected &&
- adv7511->powered) {
- regcache_mark_dirty(adv7511->regmap);
- adv7511_power_on(adv7511);
- }
-
- if (adv7511->connector.status != status) {
- adv7511->connector.status = status;
-
- if (adv7511->connector.dev) {
- if (status == connector_status_disconnected)
- cec_phys_addr_invalidate(adv7511->cec_adap);
- drm_kms_helper_hotplug_event(adv7511->connector.dev);
- } else {
- drm_bridge_hpd_notify(&adv7511->bridge, status);
- }
- }
-}
-
static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
{
unsigned int irq0, irq1;
@@ -600,6 +542,68 @@ static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block,
return 0;
}

+/* -----------------------------------------------------------------------------
+ * Hotplug handling
+ */
+
+static bool adv7511_hpd(struct adv7511 *adv7511)
+{
+ unsigned int irq0;
+ int ret;
+
+ ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0);
+ if (ret < 0)
+ return false;
+
+ if (irq0 & ADV7511_INT0_HPD) {
+ regmap_write(adv7511->regmap, ADV7511_REG_INT(0),
+ ADV7511_INT0_HPD);
+ return true;
+ }
+
+ return false;
+}
+
+static void adv7511_hpd_work(struct work_struct *work)
+{
+ struct adv7511 *adv7511 = container_of(work, struct adv7511, hpd_work);
+ enum drm_connector_status status;
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, &val);
+ if (ret < 0)
+ status = connector_status_disconnected;
+ else if (val & ADV7511_STATUS_HPD)
+ status = connector_status_connected;
+ else
+ status = connector_status_disconnected;
+
+ /*
+ * The bridge resets its registers on unplug. So when we get a plug
+ * event and we're already supposed to be powered, cycle the bridge to
+ * restore its state.
+ */
+ if (status == connector_status_connected &&
+ adv7511->connector.status == connector_status_disconnected &&
+ adv7511->powered) {
+ regcache_mark_dirty(adv7511->regmap);
+ adv7511_power_on(adv7511);
+ }
+
+ if (adv7511->connector.status != status) {
+ adv7511->connector.status = status;
+
+ if (adv7511->connector.dev) {
+ if (status == connector_status_disconnected)
+ cec_phys_addr_invalidate(adv7511->cec_adap);
+ drm_kms_helper_hotplug_event(adv7511->connector.dev);
+ } else {
+ drm_bridge_hpd_notify(&adv7511->bridge, status);
+ }
+ }
+}
+
/* -----------------------------------------------------------------------------
* ADV75xx helpers
*/

--
2.42.1