See the comments in the code. Basically if the seqno is already
signalled then we get a NULL fence. If we then put the NULL fence
in a binary syncobj it counts as unsignalled, making that syncobj
pretty much useless for all expected uses.
Not 100% sure about the transfer to a timeline syncobj but I
believe it is needed there too, as AFAICT the add_point function
assumes the fence isn't NULL.
Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between binary and timeline v2")
Cc: stable(a)vger.kernel.org
Signed-off-by: Bas Nieuwenhuizen <bas(a)basnieuwenhuizen.nl>
---
drivers/gpu/drm/drm_syncobj.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index fdd2ec87cdd1..eb28a40400d2 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -861,6 +861,19 @@ static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private,
&fence);
if (ret)
goto err;
+
+ /* If the requested seqno is already signaled drm_syncobj_find_fence may
+ * return a NULL fence. To make sure the recipient gets signalled, use
+ * a new fence instead.
+ */
+ if (!fence) {
+ fence = dma_fence_allocate_private_stub();
+ if (!fence) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ }
+
chain = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL);
if (!chain) {
ret = -ENOMEM;
@@ -890,6 +903,19 @@ drm_syncobj_transfer_to_binary(struct drm_file *file_private,
args->src_point, args->flags, &fence);
if (ret)
goto err;
+
+ /* If the requested seqno is already signaled drm_syncobj_find_fence may
+ * return a NULL fence. To make sure the recipient gets signalled, use
+ * a new fence instead.
+ */
+ if (!fence) {
+ fence = dma_fence_allocate_private_stub();
+ if (!fence) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ }
+
drm_syncobj_replace_fence(binary_syncobj, fence);
dma_fence_put(fence);
err:
--
2.34.1
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: cec: copy sequence field for the reply
Author: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Date: Tue Nov 2 12:24:26 2021 +0000
When the reply for a non-blocking transmit arrives, the sequence
field for that reply was never filled in, so userspace would have no
way of associating the reply to the original transmit.
Copy the sequence field to ensure that this is now possible.
Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Fixes: 0dbacebede1e ([media] cec: move the CEC framework out of staging and to media)
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei(a)kernel.org>
drivers/media/cec/core/cec-adap.c | 1 +
1 file changed, 1 insertion(+)
---
diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 79fa36de8a04..cd9cb354dc2c 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1199,6 +1199,7 @@ void cec_received_msg_ts(struct cec_adapter *adap,
if (abort)
dst->rx_status |= CEC_RX_STATUS_FEATURE_ABORT;
msg->flags = dst->flags;
+ msg->sequence = dst->sequence;
/* Remove it from the wait_queue */
list_del_init(&data->list);
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: cec: copy sequence field for the reply
Author: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Date: Tue Nov 2 12:24:26 2021 +0000
When the reply for a non-blocking transmit arrives, the sequence
field for that reply was never filled in, so userspace would have no
way of associating the reply to the original transmit.
Copy the sequence field to ensure that this is now possible.
Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Fixes: 0dbacebede1e ([media] cec: move the CEC framework out of staging and to media)
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei(a)kernel.org>
drivers/media/cec/core/cec-adap.c | 1 +
1 file changed, 1 insertion(+)
---
diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 79fa36de8a04..cd9cb354dc2c 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1199,6 +1199,7 @@ void cec_received_msg_ts(struct cec_adapter *adap,
if (abort)
dst->rx_status |= CEC_RX_STATUS_FEATURE_ABORT;
msg->flags = dst->flags;
+ msg->sequence = dst->sequence;
/* Remove it from the wait_queue */
list_del_init(&data->list);
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: cec-pin: fix interrupt en/disable handling
Author: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Date: Wed Dec 1 13:41:25 2021 +0100
The en/disable_irq() functions keep track of the 'depth': i.e. if
interrupts are disabled twice, then it needs to enable_irq() calls to
enable them again. The cec-pin framework didn't take this into accound
and could disable irqs multiple times, and it expected that a single
enable_irq() would enable them again.
Move all calls to en/disable_irq() to the kthread where it is easy
to keep track of the current irq state and ensure that multiple
en/disable_irq calls never happen.
If interrupts where disabled twice, then they would never turn on
again, leaving the CEC adapter in a dead state.
Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
Fixes: 865463fc03ed (media: cec-pin: add error injection support)
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei(a)kernel.org>
drivers/media/cec/core/cec-pin.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
---
diff --git a/drivers/media/cec/core/cec-pin.c b/drivers/media/cec/core/cec-pin.c
index 2b6459df1e94..21f0f749713e 100644
--- a/drivers/media/cec/core/cec-pin.c
+++ b/drivers/media/cec/core/cec-pin.c
@@ -1033,6 +1033,7 @@ static int cec_pin_thread_func(void *_adap)
{
struct cec_adapter *adap = _adap;
struct cec_pin *pin = adap->pin;
+ bool irq_enabled = false;
for (;;) {
wait_event_interruptible(pin->kthread_waitq,
@@ -1060,6 +1061,7 @@ static int cec_pin_thread_func(void *_adap)
ns_to_ktime(pin->work_rx_msg.rx_ts));
msg->len = 0;
}
+
if (pin->work_tx_status) {
unsigned int tx_status = pin->work_tx_status;
@@ -1083,27 +1085,39 @@ static int cec_pin_thread_func(void *_adap)
switch (atomic_xchg(&pin->work_irq_change,
CEC_PIN_IRQ_UNCHANGED)) {
case CEC_PIN_IRQ_DISABLE:
- pin->ops->disable_irq(adap);
+ if (irq_enabled) {
+ pin->ops->disable_irq(adap);
+ irq_enabled = false;
+ }
cec_pin_high(pin);
cec_pin_to_idle(pin);
hrtimer_start(&pin->timer, ns_to_ktime(0),
HRTIMER_MODE_REL);
break;
case CEC_PIN_IRQ_ENABLE:
+ if (irq_enabled)
+ break;
pin->enable_irq_failed = !pin->ops->enable_irq(adap);
if (pin->enable_irq_failed) {
cec_pin_to_idle(pin);
hrtimer_start(&pin->timer, ns_to_ktime(0),
HRTIMER_MODE_REL);
+ } else {
+ irq_enabled = true;
}
break;
default:
break;
}
-
if (kthread_should_stop())
break;
}
+ if (pin->ops->disable_irq && irq_enabled)
+ pin->ops->disable_irq(adap);
+ hrtimer_cancel(&pin->timer);
+ cec_pin_read(pin);
+ cec_pin_to_idle(pin);
+ pin->state = CEC_ST_OFF;
return 0;
}
@@ -1129,13 +1143,7 @@ static int cec_pin_adap_enable(struct cec_adapter *adap, bool enable)
hrtimer_start(&pin->timer, ns_to_ktime(0),
HRTIMER_MODE_REL);
} else {
- if (pin->ops->disable_irq)
- pin->ops->disable_irq(adap);
- hrtimer_cancel(&pin->timer);
kthread_stop(pin->kthread);
- cec_pin_read(pin);
- cec_pin_to_idle(pin);
- pin->state = CEC_ST_OFF;
}
return 0;
}
@@ -1156,11 +1164,8 @@ void cec_pin_start_timer(struct cec_pin *pin)
if (pin->state != CEC_ST_RX_IRQ)
return;
- atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_UNCHANGED);
- pin->ops->disable_irq(pin->adap);
- cec_pin_high(pin);
- cec_pin_to_idle(pin);
- hrtimer_start(&pin->timer, ns_to_ktime(0), HRTIMER_MODE_REL);
+ atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_DISABLE);
+ wake_up_interruptible(&pin->kthread_waitq);
}
static int cec_pin_adap_transmit(struct cec_adapter *adap, u8 attempts,