[v2] drm/vc4: Return -EBUSY if there's already a pending flip event.
diff mbox

Message ID 1462297700-17491-1-git-send-email-robert.foss@collabora.com
State New
Headers show

Commit Message

Robert Foss May 3, 2016, 5:48 p.m. UTC
From: Robert Foss <robert.foss@collabora.com>

As per the documentation in drm_crtc.h, atomic_commit should return
-EBUSY if an asycnhronous update is requested and there is an earlier
update pending.

Note: docs cited here are drm_crtc.h, and the whole quote is:

    *  - -EBUSY, if an asynchronous updated is requested and there is
    *    an earlier updated pending. Drivers are allowed to support a queue
    *    of outstanding updates, but currently no driver supports that.
    *    Note that drivers must wait for preceding updates to complete if a
    *    synchronous update is requested, they are not allowed to fail the
    *    commit in that case.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---

Changes since v1:
- Corrected and simplified patch to piggyback on a previously existing check.

 drivers/gpu/drm/vc4/vc4_kms.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Eric Anholt May 3, 2016, 7:22 p.m. UTC | #1
robert.foss@collabora.com writes:

> From: Robert Foss <robert.foss@collabora.com>
>
> As per the documentation in drm_crtc.h, atomic_commit should return
> -EBUSY if an asycnhronous update is requested and there is an earlier
> update pending.
>
> Note: docs cited here are drm_crtc.h, and the whole quote is:
>
>     *  - -EBUSY, if an asynchronous updated is requested and there is
>     *    an earlier updated pending. Drivers are allowed to support a queue
>     *    of outstanding updates, but currently no driver supports that.
>     *    Note that drivers must wait for preceding updates to complete if a
>     *    synchronous update is requested, they are not allowed to fail the
>     *    commit in that case.
>
> Signed-off-by: Robert Foss <robert.foss@collabora.com>

This looks good to me.  Let's give it a few days on the list for any
other KMS folks to catch anything.
Robert Foss May 10, 2016, 3:35 p.m. UTC | #2
On 2016-05-03 03:22 PM, Eric Anholt wrote:
> robert.foss@collabora.com writes:
>
>> From: Robert Foss <robert.foss@collabora.com>
>>
>> As per the documentation in drm_crtc.h, atomic_commit should return
>> -EBUSY if an asycnhronous update is requested and there is an earlier
>> update pending.
>>
>> Note: docs cited here are drm_crtc.h, and the whole quote is:
>>
>>      *  - -EBUSY, if an asynchronous updated is requested and there is
>>      *    an earlier updated pending. Drivers are allowed to support a queue
>>      *    of outstanding updates, but currently no driver supports that.
>>      *    Note that drivers must wait for preceding updates to complete if a
>>      *    synchronous update is requested, they are not allowed to fail the
>>      *    commit in that case.
>>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>
> This looks good to me.  Let's give it a few days on the list for any
> other KMS folks to catch anything.
>

I haven't seen any further feedback regarding this patch.
Does anyone have objections to it being merged?
Eric Anholt May 10, 2016, 5:06 p.m. UTC | #3
Robert Foss <robert.foss@collabora.com> writes:

> On 2016-05-03 03:22 PM, Eric Anholt wrote:
>> robert.foss@collabora.com writes:
>>
>>> From: Robert Foss <robert.foss@collabora.com>
>>>
>>> As per the documentation in drm_crtc.h, atomic_commit should return
>>> -EBUSY if an asycnhronous update is requested and there is an earlier
>>> update pending.
>>>
>>> Note: docs cited here are drm_crtc.h, and the whole quote is:
>>>
>>>      *  - -EBUSY, if an asynchronous updated is requested and there is
>>>      *    an earlier updated pending. Drivers are allowed to support a queue
>>>      *    of outstanding updates, but currently no driver supports that.
>>>      *    Note that drivers must wait for preceding updates to complete if a
>>>      *    synchronous update is requested, they are not allowed to fail the
>>>      *    commit in that case.
>>>
>>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>>
>> This looks good to me.  Let's give it a few days on the list for any
>> other KMS folks to catch anything.
>>
>
> I haven't seen any further feedback regarding this patch.
> Does anyone have objections to it being merged?

I merged it to drm-vc4-next last night, actually :)
Robert Foss May 10, 2016, 5:19 p.m. UTC | #4
Thanks Eric!

On 2016-05-10 01:06 PM, Eric Anholt wrote:
> Robert Foss <robert.foss@collabora.com> writes:
>
>> On 2016-05-03 03:22 PM, Eric Anholt wrote:
>>> robert.foss@collabora.com writes:
>>>
>>>> From: Robert Foss <robert.foss@collabora.com>
>>>>
>>>> As per the documentation in drm_crtc.h, atomic_commit should return
>>>> -EBUSY if an asycnhronous update is requested and there is an earlier
>>>> update pending.
>>>>
>>>> Note: docs cited here are drm_crtc.h, and the whole quote is:
>>>>
>>>>       *  - -EBUSY, if an asynchronous updated is requested and there is
>>>>       *    an earlier updated pending. Drivers are allowed to support a queue
>>>>       *    of outstanding updates, but currently no driver supports that.
>>>>       *    Note that drivers must wait for preceding updates to complete if a
>>>>       *    synchronous update is requested, they are not allowed to fail the
>>>>       *    commit in that case.
>>>>
>>>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>>>
>>> This looks good to me.  Let's give it a few days on the list for any
>>> other KMS folks to catch anything.
>>>
>>
>> I haven't seen any further feedback regarding this patch.
>> Does anyone have objections to it being merged?
>
> I merged it to drm-vc4-next last night, actually :)
>

Patch
diff mbox

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 4718ae5..7c7188d 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -117,10 +117,18 @@  static int vc4_atomic_commit(struct drm_device *dev,
 		return -ENOMEM;
 
 	/* Make sure that any outstanding modesets have finished. */
-	ret = down_interruptible(&vc4->async_modeset);
-	if (ret) {
-		kfree(c);
-		return ret;
+	if (async) {
+		ret = down_trylock(&vc4->async_modeset);
+		if (ret) {
+			kfree(c);
+			return -EBUSY;
+		}
+	} else {
+		ret = down_interruptible(&vc4->async_modeset);
+		if (ret) {
+			kfree(c);
+			return ret;
+		}
 	}
 
 	ret = drm_atomic_helper_prepare_planes(dev, state);