All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mario Kleiner <mario.kleiner.de@gmail.com>
To: "Kazlauskas, Nicholas" <Nicholas.Kazlauskas@amd.com>
Cc: "Sam Ravnborg" <sam@ravnborg.org>,
	"David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Intel Graphics Development" <intel-gfx@lists.freedesktop.org>,
	"Li, Sun peng (Leo)" <Sunpeng.Li@amd.com>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Koenig, Christian" <Christian.Koenig@amd.com>,
	"Francis, David" <David.Francis@amd.com>,
	"Maxime Ripard" <maxime.ripard@bootlin.com>,
	"Lakha, Bhawanpreet" <Bhawanpreet.Lakha@amd.com>,
	"Ben Skeggs" <bskeggs@redhat.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Sean Paul" <sean@poorly.run>,
	"Michel Dänzer" <michel@daenzer.net>
Subject: Re: [PATCH 3/3] drm/atomic: Rename crtc_state->pageflip_flags to async_flip
Date: Thu, 5 Sep 2019 14:33:37 +0200	[thread overview]
Message-ID: <CAEsyxyjM71CuDbYrniH70Eg_Xd_r_yd3eBDtiv9z1SKcpOKKCQ@mail.gmail.com> (raw)
In-Reply-To: <c6283ffa-151a-c0be-63f7-b3a4a9c5a0f5@amd.com>

On Wed, Sep 4, 2019 at 2:57 PM Kazlauskas, Nicholas
<Nicholas.Kazlauskas@amd.com> wrote:
>
> On 2019-09-03 3:06 p.m., Daniel Vetter wrote:
> > It's the only flag anyone actually cares about. Plus if we're unlucky,
> > the atomic ioctl might need a different flag for async flips. So
> > better to abstract this away from the uapi a bit.
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Michel Dänzer <michel@daenzer.net>
> > Cc: Alex Deucher <alexdeucher@gmail.com>
> > Cc: Adam Jackson <ajax@redhat.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: David Airlie <airlied@linux.ie>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > Cc: Leo Li <sunpeng.li@amd.com>
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: David Francis <David.Francis@amd.com>
> > Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> > Cc: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
>
> Series is:
>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>
> I would like to see a new flag eventually show up for atomic as well,
> but the existing one is effectively broken at this point and I would
> hope that no userspace is setting it expecting that it actually does
> something.

You mean it is generally broken? My software uses non-vsync'ed flips
for diagnostic purpose and iirc some gpu + driver combo didn't work as
expected anymore. But i thought that was one specific driver bug
(maybe on AMD + DC)?

>
> At this point we don't really gain anything from enabling atomic in DDX
> I think, most drivers already make use of DRM helpers to map these
> legacy IOCTLs to atomic anyway.
>

One thing i wanted to try, once i hopefully find some time in late
2019 / early 2020 (if nobody else starts working on such a thing
earlier), would be to add the ability to pass in a target flip time to
the pageflip ioctl for use with VRR. For that i thought adding a new
pageflip flag a la DRM_MODE_PAGE_FLIP_TARGETTIME) would be a good way
to reuse the existing page_flip_target ioctl and redefine the "uint32
sequence" field of struct drm_mode_crtc_page_flip_target to pass in
the target time - or at least the lower 32 bits of a target time.

So that would be one more page flip flag for the future. I'd like this
to be workable from X11, and the current DDX don't use the atomic
interface, apart from the modesetting DDX where it just got disabled
by default in xserver master due to various unresolved bugs afaik?

thanks,
-mario

> Nicholas Kazlauskas
>
> >   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
> >   drivers/gpu/drm/drm_atomic_helper.c               | 2 +-
> >   drivers/gpu/drm/drm_atomic_state_helper.c         | 2 +-
> >   drivers/gpu/drm/nouveau/dispnv50/wndw.c           | 4 ++--
> >   include/drm/drm_crtc.h                            | 8 ++++----
> >   5 files changed, 10 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 0a71ed1e7762..2f0ef0820f00 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -5756,8 +5756,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> >                * change FB pitch, DCC state, rotation or mirroing.
> >                */
> >               bundle->flip_addrs[planes_count].flip_immediate =
> > -                     (crtc->state->pageflip_flags &
> > -                      DRM_MODE_PAGE_FLIP_ASYNC) != 0 &&
> > +                     crtc->state->async_flip &&
> >                       acrtc_state->update_type == UPDATE_TYPE_FAST;
> >
> >               timestamp_ns = ktime_get_ns();
> > @@ -6334,7 +6333,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
> >       amdgpu_dm_enable_crtc_interrupts(dev, state, true);
> >
> >       for_each_new_crtc_in_state(state, crtc, new_crtc_state, j)
> > -             if (new_crtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC)
> > +             if (new_crtc_state->async_flip)
> >                       wait_for_vblank = false;
> >
> >       /* update planes when needed per crtc*/
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index e9c6112e7f73..1e5293eb66e3 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3263,7 +3263,7 @@ static int page_flip_common(struct drm_atomic_state *state,
> >               return PTR_ERR(crtc_state);
> >
> >       crtc_state->event = event;
> > -     crtc_state->pageflip_flags = flags;
> > +     crtc_state->async_flip = flags & DRM_MODE_PAGE_FLIP_ASYNC;
> >
> >       plane_state = drm_atomic_get_plane_state(state, plane);
> >       if (IS_ERR(plane_state))
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index 46dc264a248b..d0a937fb0c56 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -128,7 +128,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
> >       state->zpos_changed = false;
> >       state->commit = NULL;
> >       state->event = NULL;
> > -     state->pageflip_flags = 0;
> > +     state->async_flip = false;
> >
> >       /* Self refresh should be canceled when a new update is available */
> >       state->active = drm_atomic_crtc_effectively_active(state);
> > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> > index 2db029371c91..5193b6257061 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> > @@ -267,7 +267,7 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw *wndw, bool modeset,
> >                       asyw->image.pitch[0] = fb->base.pitches[0];
> >               }
> >
> > -             if (!(asyh->state.pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC))
> > +             if (!asyh->state.async_flip)
> >                       asyw->image.interval = 1;
> >               else
> >                       asyw->image.interval = 0;
> > @@ -383,7 +383,7 @@ nv50_wndw_atomic_check_lut(struct nv50_wndw *wndw,
> >       }
> >
> >       /* Can't do an immediate flip while changing the LUT. */
> > -     asyh->state.pageflip_flags &= ~DRM_MODE_PAGE_FLIP_ASYNC;
> > +     asyh->state.async_flip = false;
> >   }
> >
> >   static int
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 7e2963cad543..900ae8d452b8 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -284,12 +284,12 @@ struct drm_crtc_state {
> >       u32 target_vblank;
> >
> >       /**
> > -      * @pageflip_flags:
> > +      * @async_flip:
> >        *
> > -      * DRM_MODE_PAGE_FLIP_* flags, as passed to the page flip ioctl.
> > -      * Zero in any other case.
> > +      * This is set when DRM_MODE_PAGE_FLIP_ASYNC is set in the legacy
> > +      * PAGE_FLIP IOCTL. It's not wired up for the atomic IOCTL itself yet.
> >        */
> > -     u32 pageflip_flags;
> > +     bool async_flip;
> >
> >       /**
> >        * @vrr_enabled:
> >
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2019-09-05 12:33 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-03 19:06 [PATCH 1/3] drm/atomic: Take the atomic toys away from X Daniel Vetter
2019-09-03 19:06 ` [PATCH 2/3] drm/atomic: Reject FLIP_ASYNC unconditionally Daniel Vetter
2019-09-03 19:06 ` [PATCH 3/3] drm/atomic: Rename crtc_state->pageflip_flags to async_flip Daniel Vetter
2019-09-04 12:57   ` Kazlauskas, Nicholas
2019-09-05 12:33     ` Mario Kleiner [this message]
2019-09-05 13:58       ` Daniel Vetter
2019-09-03 19:58 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/atomic: Take the atomic toys away from X Patchwork
2019-09-03 20:51 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-04  0:10 ` ✓ Fi.CI.IGT: " Patchwork
2019-09-05 14:19 ` [PATCH 1/3] " Maarten Lankhorst
2019-09-05 14:25   ` Daniel Vetter
2019-09-05 17:20 ` [Intel-gfx] " Rob Clark
2019-09-05 18:18 ` [PATCH] " Daniel Vetter
2019-09-05 20:43   ` Sasha Levin
2019-09-05 20:43   ` Sasha Levin
2019-09-05 18:53 ` Daniel Vetter
2020-05-08  9:06   ` Yves-Alexis Perez
2020-05-08  9:06     ` [Intel-gfx] " Yves-Alexis Perez
2020-05-08  9:06     ` Yves-Alexis Perez
2020-05-08  9:54     ` Greg KH
2020-05-08  9:54       ` [Intel-gfx] " Greg KH
2020-05-08  9:54       ` Greg KH
2020-05-08 11:59       ` Yves-Alexis Perez
2020-05-08 11:59         ` [Intel-gfx] " Yves-Alexis Perez
2020-05-08 11:59         ` Yves-Alexis Perez
2020-05-08 12:24         ` Greg KH
2020-05-08 12:24           ` [Intel-gfx] " Greg KH
2020-05-08 12:24           ` Greg KH
2019-09-05 18:58 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with drm/atomic: Take the atomic toys away from X (rev2) Patchwork
2019-09-05 19:21 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-05 19:53 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with drm/atomic: Take the atomic toys away from X (rev3) Patchwork
2019-09-05 20:17 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-06  0:52 ` ✓ Fi.CI.IGT: " Patchwork

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=CAEsyxyjM71CuDbYrniH70Eg_Xd_r_yd3eBDtiv9z1SKcpOKKCQ@mail.gmail.com \
    --to=mario.kleiner.de@gmail.com \
    --cc=Bhawanpreet.Lakha@amd.com \
    --cc=Christian.Koenig@amd.com \
    --cc=David.Francis@amd.com \
    --cc=Nicholas.Kazlauskas@amd.com \
    --cc=Sunpeng.Li@amd.com \
    --cc=airlied@linux.ie \
    --cc=bskeggs@redhat.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maxime.ripard@bootlin.com \
    --cc=michel@daenzer.net \
    --cc=sam@ravnborg.org \
    --cc=sean@poorly.run \
    /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.