All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Mark Yacoub <markyacoub@chromium.org>
Cc: chunkuang.hu@kernel.org, David Airlie <airlied@linux.ie>,
	jason-jh.lin@mediatek.com, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, tzungbi@google.com,
	seanpaul@chromium.org, Thomas Zimmermann <tzimmermann@suse.de>,
	matthias.bgg@gmail.com, Mark Yacoub <markyacoub@google.com>
Subject: Re: [PATCH] drm: send vblank event with the attached sequence rather than current
Date: Fri, 3 Dec 2021 09:25:20 +0200	[thread overview]
Message-ID: <YanGYFVo6etc/j0T@intel.com> (raw)
In-Reply-To: <20211202151200.3125685-1-markyacoub@chromium.org>

On Thu, Dec 02, 2021 at 10:11:55AM -0500, Mark Yacoub wrote:
> From: Mark Yacoub <markyacoub@google.com>
> 
> [Why]
> drm_handle_vblank_events loops over vblank_event_list to send any event
> that is current or has passed.
> More than 1 event could be pending with past sequence time that need to
> be send. This can be a side effect of drivers without hardware vblank
> counter and they depend on the difference in the timestamps and the
> frame/field duration calculated in drm_update_vblank_count. This can
> lead to 1 vblirq being ignored due to very small diff, resulting in a
> subsequent vblank with 2 pending vblank events to be sent, each with a
> unique sequence expected by user space.
> 
> [How]
> Send each pending vblank event with the sequence it's waiting on instead
> of assigning the current sequence to all of them.
> 
> Fixes igt@kms_flip "Unexpected frame sequence"
> Tested on Jacuzzi (MT8183)
> 
> Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> ---
>  drivers/gpu/drm/drm_vblank.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 3417e1ac79185..47da8056abc14 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1902,7 +1902,7 @@ static void drm_handle_vblank_events(struct drm_device *dev, unsigned int pipe)
>  
>  		list_del(&e->base.link);
>  		drm_vblank_put(dev, pipe);
> -		send_vblank_event(dev, e, seq, now);
> +		send_vblank_event(dev, e, e->sequence, now);

This doesn't look right. The timestamp corresponds to 'seq' not
e->sequence (ie. whatever sequqnece number the user asked to wait
for).

>  	}
>  
>  	if (crtc && crtc->funcs->get_vblank_timestamp)
> -- 
> 2.34.0.rc2.393.gf8c9666880-goog

-- 
Ville Syrjälä
Intel

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Mark Yacoub <markyacoub@chromium.org>
Cc: dri-devel@lists.freedesktop.org, chunkuang.hu@kernel.org,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@linux.ie>,
	jason-jh.lin@mediatek.com, linux-kernel@vger.kernel.org,
	tzungbi@google.com, seanpaul@chromium.org,
	matthias.bgg@gmail.com, Mark Yacoub <markyacoub@google.com>
Subject: Re: [PATCH] drm: send vblank event with the attached sequence rather than current
Date: Fri, 3 Dec 2021 09:25:20 +0200	[thread overview]
Message-ID: <YanGYFVo6etc/j0T@intel.com> (raw)
In-Reply-To: <20211202151200.3125685-1-markyacoub@chromium.org>

On Thu, Dec 02, 2021 at 10:11:55AM -0500, Mark Yacoub wrote:
> From: Mark Yacoub <markyacoub@google.com>
> 
> [Why]
> drm_handle_vblank_events loops over vblank_event_list to send any event
> that is current or has passed.
> More than 1 event could be pending with past sequence time that need to
> be send. This can be a side effect of drivers without hardware vblank
> counter and they depend on the difference in the timestamps and the
> frame/field duration calculated in drm_update_vblank_count. This can
> lead to 1 vblirq being ignored due to very small diff, resulting in a
> subsequent vblank with 2 pending vblank events to be sent, each with a
> unique sequence expected by user space.
> 
> [How]
> Send each pending vblank event with the sequence it's waiting on instead
> of assigning the current sequence to all of them.
> 
> Fixes igt@kms_flip "Unexpected frame sequence"
> Tested on Jacuzzi (MT8183)
> 
> Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> ---
>  drivers/gpu/drm/drm_vblank.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 3417e1ac79185..47da8056abc14 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1902,7 +1902,7 @@ static void drm_handle_vblank_events(struct drm_device *dev, unsigned int pipe)
>  
>  		list_del(&e->base.link);
>  		drm_vblank_put(dev, pipe);
> -		send_vblank_event(dev, e, seq, now);
> +		send_vblank_event(dev, e, e->sequence, now);

This doesn't look right. The timestamp corresponds to 'seq' not
e->sequence (ie. whatever sequqnece number the user asked to wait
for).

>  	}
>  
>  	if (crtc && crtc->funcs->get_vblank_timestamp)
> -- 
> 2.34.0.rc2.393.gf8c9666880-goog

-- 
Ville Syrjälä
Intel

  reply	other threads:[~2021-12-03  7:25 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-02 15:11 [PATCH] drm: send vblank event with the attached sequence rather than current Mark Yacoub
2021-12-02 15:11 ` Mark Yacoub
2021-12-03  7:25 ` Ville Syrjälä [this message]
2021-12-03  7:25   ` Ville Syrjälä
2021-12-03 15:58 ` Matthias Brugger
2021-12-03 15:58   ` Matthias Brugger
2021-12-03 18:03   ` Michel Dänzer
2021-12-03 18:03     ` Michel Dänzer
2021-12-03 18:05     ` Mark Yacoub
2021-12-03 18:05       ` Mark Yacoub

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=YanGYFVo6etc/j0T@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=airlied@linux.ie \
    --cc=chunkuang.hu@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jason-jh.lin@mediatek.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markyacoub@chromium.org \
    --cc=markyacoub@google.com \
    --cc=matthias.bgg@gmail.com \
    --cc=seanpaul@chromium.org \
    --cc=tzimmermann@suse.de \
    --cc=tzungbi@google.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.