Skip to content

Commit 91f9f4a

Browse files
aford173lumag
authored andcommitted
drm/bridge: adv7511: Fix Intermittent EDID failures
In the process of adding support for shared IRQ pins, a scenario was accidentally created where adv7511_irq_process returned prematurely causing the EDID to fail randomly. Since the interrupt handler is broken up into two main helper functions, update both of them to treat the helper functions as IRQ handlers. These IRQ routines process their respective tasks as before, but if they determine that actual work was done, mark the respective IRQ status accordingly, and delay the check until everything has been processed. This should guarantee the helper functions don't return prematurely while still returning proper values of either IRQ_HANDLED or IRQ_NONE. Reported-by: Liu Ying <[email protected]> Fixes: f3d9683 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins") Signed-off-by: Adam Ford <[email protected]> Tested-by: Liu Ying <[email protected]> # i.MX8MP EVK ADV7535 EDID retrieval w/o IRQ Reviewed-by: Dmitry Baryshkov <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] Signed-off-by: Dmitry Baryshkov <[email protected]> V3: Remove unnecessary declaration of ret by evaluating the return code of regmap_read directly. V2: Fix uninitialized cec_status Cut back a little on error handling to return either IRQ_NONE or IRQ_HANDLED.
1 parent 256abd8 commit 91f9f4a

File tree

3 files changed

+23
-14
lines changed

3 files changed

+23
-14
lines changed

drivers/gpu/drm/bridge/adv7511/adv7511.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ struct adv7511 {
401401

402402
#ifdef CONFIG_DRM_I2C_ADV7511_CEC
403403
int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511);
404-
void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
404+
int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
405405
#else
406406
static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
407407
{

drivers/gpu/drm/bridge/adv7511/adv7511_cec.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ static void adv7511_cec_rx(struct adv7511 *adv7511, int rx_buf)
119119
cec_received_msg(adv7511->cec_adap, &msg);
120120
}
121121

122-
void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
122+
int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
123123
{
124124
unsigned int offset = adv7511->info->reg_cec_offset;
125125
const u32 irq_tx_mask = ADV7511_INT1_CEC_TX_READY |
@@ -131,16 +131,19 @@ void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
131131
unsigned int rx_status;
132132
int rx_order[3] = { -1, -1, -1 };
133133
int i;
134+
int irq_status = IRQ_NONE;
134135

135-
if (irq1 & irq_tx_mask)
136+
if (irq1 & irq_tx_mask) {
136137
adv_cec_tx_raw_status(adv7511, irq1);
138+
irq_status = IRQ_HANDLED;
139+
}
137140

138141
if (!(irq1 & irq_rx_mask))
139-
return;
142+
return irq_status;
140143

141144
if (regmap_read(adv7511->regmap_cec,
142145
ADV7511_REG_CEC_RX_STATUS + offset, &rx_status))
143-
return;
146+
return irq_status;
144147

145148
/*
146149
* ADV7511_REG_CEC_RX_STATUS[5:0] contains the reception order of RX
@@ -172,6 +175,8 @@ void adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
172175

173176
adv7511_cec_rx(adv7511, rx_buf);
174177
}
178+
179+
return IRQ_HANDLED;
175180
}
176181

177182
static int adv7511_cec_adap_enable(struct cec_adapter *adap, bool enable)

drivers/gpu/drm/bridge/adv7511/adv7511_drv.c

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,8 @@ static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
469469
{
470470
unsigned int irq0, irq1;
471471
int ret;
472+
int cec_status = IRQ_NONE;
473+
int irq_status = IRQ_NONE;
472474

473475
ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0);
474476
if (ret < 0)
@@ -478,29 +480,31 @@ static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
478480
if (ret < 0)
479481
return ret;
480482

481-
/* If there is no IRQ to handle, exit indicating no IRQ data */
482-
if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
483-
!(irq1 & ADV7511_INT1_DDC_ERROR))
484-
return -ENODATA;
485-
486483
regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0);
487484
regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1);
488485

489-
if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder)
486+
if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder) {
490487
schedule_work(&adv7511->hpd_work);
488+
irq_status = IRQ_HANDLED;
489+
}
491490

492491
if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) {
493492
adv7511->edid_read = true;
494493

495494
if (adv7511->i2c_main->irq)
496495
wake_up_all(&adv7511->wq);
496+
irq_status = IRQ_HANDLED;
497497
}
498498

499499
#ifdef CONFIG_DRM_I2C_ADV7511_CEC
500-
adv7511_cec_irq_process(adv7511, irq1);
500+
cec_status = adv7511_cec_irq_process(adv7511, irq1);
501501
#endif
502502

503-
return 0;
503+
/* If there is no IRQ to handle, exit indicating no IRQ data */
504+
if (irq_status == IRQ_HANDLED || cec_status == IRQ_HANDLED)
505+
return IRQ_HANDLED;
506+
507+
return IRQ_NONE;
504508
}
505509

506510
static irqreturn_t adv7511_irq_handler(int irq, void *devid)
@@ -509,7 +513,7 @@ static irqreturn_t adv7511_irq_handler(int irq, void *devid)
509513
int ret;
510514

511515
ret = adv7511_irq_process(adv7511, true);
512-
return ret < 0 ? IRQ_NONE : IRQ_HANDLED;
516+
return ret < 0 ? IRQ_NONE : ret;
513517
}
514518

515519
/* -----------------------------------------------------------------------------

0 commit comments

Comments
 (0)