All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
To: Simon Ser <simon.ser@intel.com>,
	Ville Syrjala <ville.syrjala@linux.intel.com>,
	Shayenne Moura <shayenneluzmoura@gmail.com>,
	Haneen Mohammed <hamohammed.sa@gmail.com>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	18oliveira.charles@gmail.com
Subject: [PATCH V2] drm/vkms: Avoid extra discount in the timestamp value
Date: Tue, 4 Jun 2019 23:45:43 -0300	[thread overview]
Message-ID: <20190605024543.pcsnkf74mmgfhtuh@smtp.gmail.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 3305 bytes --]

After the commit def35e7c5926 ("drm/vkms: Bugfix extra vblank frame")
some of the crc tests started to fail in the vkms with the following
error:

 [drm:drm_crtc_add_crc_entry [drm]] *ERROR* Overflow of CRC buffer,
    userspace reads too slow.
 [drm] failed to queue vkms_crc_work_handle
 ...

The aforementioned commit fixed the extra vblank added by
`drm_crtc_arm_vblank_event()` which is invoked inside
`vkms_crtc_atomic_flush()` if the vblank event count was zero, otherwise
`drm_crtc_send_vblank_event()` is invoked. The fix was implemented in
`vkms_get_vblank_timestamp()` by subtracting one period from the current
timestamp, as the code snippet below illustrates:

 if (!in_vblank_irq)
  *vblank_time -= output->period_ns;

The above fix works well when `drm_crtc_arm_vblank_event()` is invoked.
However, it does not properly work when `drm_crtc_send_vblank_event()`
executes since it subtracts the correct timestamp, which it shouldn't.
In this case, the `drm_crtc_accurate_vblank_count()` function will
returns the wrong frame number, which generates the aforementioned
error. Such decrease in `get_vblank_timestamp()` produce a negative
number in the following calculation within `drm_update_vblank_count()`:

 u64 diff_ns = ktime_to_ns(ktime_sub(t_vblank, vblank->time));

After this operation, the DIV_ROUND_CLOSEST_ULL macro is invoked using
diff_ns with a negative number, which generates an undefined result;
therefore, the returned frame is a huge and incorrect number. Finally,
the code below is part of the `vkms_crc_work_handle()`, note that the
while loop depends on the returned value from
`drm_crtc_accurate_vblank_count()` which may cause the loop to take a
long time to finish in case of huge value.

 frame_end = drm_crtc_accurate_vblank_count(crtc);
 while (frame_start <= frame_end)
   drm_crtc_add_crc_entry(crtc, true, frame_start++, &crc32);

This commit fixes this issue by checking if the vblank timestamp
corresponding to the current software vblank counter is equal to the
current vblank; if they are equal, it means that
`drm_crtc_send_vblank_event()` was invoked and vkms does not need to
discount the extra vblank, otherwise, `drm_crtc_arm_vblank_event()` was
executed and vkms have to discount the extra vblank. This fix made the
CRC tests work again whereas keep all tests from kms_flip working as
well.

V2: Update commit message

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Signed-off-by: Shayenne Moura <shayenneluzmoura@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 7508815fac11..3ce60e66673e 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -74,9 +74,13 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
 {
 	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
 	struct vkms_output *output = &vkmsdev->output;
+	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 
 	*vblank_time = output->vblank_hrtimer.node.expires;
 
+	if (*vblank_time == vblank->time)
+		return true;
+
 	if (!in_vblank_irq)
 		*vblank_time -= output->period_ns;
 
-- 
2.21.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

             reply	other threads:[~2019-06-05  2:45 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-05  2:45 Rodrigo Siqueira [this message]
2019-06-05 12:22 ` [PATCH V2] drm/vkms: Avoid extra discount in the timestamp value Daniel Vetter
2019-06-05 12:22   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190605024543.pcsnkf74mmgfhtuh@smtp.gmail.com \
    --to=rodrigosiqueiramelo@gmail.com \
    --cc=18oliveira.charles@gmail.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hamohammed.sa@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shayenneluzmoura@gmail.com \
    --cc=simon.ser@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.