On 2021-05-20 14:28, Stephen Boyd wrote:
Quoting khsieh@xxxxxxxxxxxxxx (2021-05-20 13:05:48)
On 2021-05-20 12:28, Stephen Boyd wrote:
>> Put dongle to D3 state so that it will not issue the unnecessary
>> second
>> irq_hpd with sink_count = 0. this will prevent the annoy but unharmful
>> DP_LINK_STATUS_UPDATED warning message.
>> Again, we can not disable hpd interrupt since dongle still attached
>> and
>> hdmi cable can be plugged in at any instant.
>>
>
> Right I'm not suggesting to disable hpd interrupt, just the hpd_irq
> interrupt once an unplug irq comes in, and do that in hardirq context.
> Also, I'm suggesting that we consider unplug as a higher priority if
> the
> hard irq handler is delayed for some reason and both an unplug irq and
> an hpd irq are pending in the hardware when the hard irq handler is
> running. Putting the dongle into D3 state won't fix these problems.
The unplug interrupt is not happen in this case since dongle still
attached.
The unplug interrupt only happen when dongle unplugged.
Agreed.
I think you mistakenly think DP_LINK_STATUS_UPDATED is caused by
unplug
interrupt.
Ok, got it.
DP_LINK_STATUS_UPDATED happen is due to dongle issue two consecutive
irq_hpd with sink_count = 0 when hdmi cable unplugged from dongle.
The first irq_hpd with sink_count = 0 is handled as expected to turn
off
display.
After that the second irq_hpd with sink_count = 0 is handled.
Since display had turned off, then there is nothing to do but spill
DP_LINK_STATUS_UPDATED warning message.
There is no unplug (hpd become low) happen in this case since dongle
still attached.
Agreed.
All interrupt (plug/irq_hpd and unplug) are required to be handled in
the order of happening.
We can not ignore any one.
For example, you plug/unplug two different resolution monitor
alternative to/from dongle and unplug dongle once for while.
I think the race condition you describe here all had been taken care
with
1) convert irq into event and store at event q in order.
2) irq handled base on transaction. Next irq can be handled when
previous irq transaction is done.
I'm mostly trying to point out that the irq handling and masking needs
to be done in the hard irq context and not in the kthread. It may or
may
not be related to this message that's printed.
What happens if the hardirq is blocked by some other irq that takes a
long time to process? Imagine this scenario:
CPU0 CPU1
---- ----
really_long_other_hardirq() {
hpd_irq
hpd_irq
hpd low
}
dp_display_irq_handler() {
<fork things to kthread>
}
Shouldn't we ignore any hpd_irq events in this scenario? And shouldn't
we be disabling the hpd_irq by masking it with DP_DP_IRQ_HPD_INT_MASK
when hpd goes low (i.e. DP_DP_HPD_UNPLUG_INT_MASK)?
1) irq_hpd interrupt always happen before unplug interrupt
2)if hdp_isr_status = (DP_DP_IRQ_HPD_INT_MASK |
DP_DP_HPD_UNPLUG_INT_MASK) at the time when read at
dp_display_irq_handler(),
then DP_DP_IRQ_HPD_INT_MASK will be add into evetn q first followed by
DP_DP_HPD_UNPLUG_INT_MASK be add into event q.
So that DP_DP_IRQ_HPD_INT_MASK will be executed by the event thread
before DP_DP_HPD_UNPLUG_INT_MASK.
On the other word, IRQ_HPD has higher priority over UNPLUG in the timing
matter.
By doing that we can shut down display gracefully.
If you insist, at hdp_isr_status = (DP_DP_IRQ_HPD_INT_MASK |
DP_DP_HPD_UNPLUG_INT_MASK) case,
we can have only add DP_DP_HPD_UNPLUG_INT_MASK to event q only by
dropping DP_DP_IRQ_HPD_INT_MASK.
Is this will work for you?
