* [PATCH 0/4] Allow ASYNC flip with atomic helpers. @ 2017-01-16 15:44 Andrey Grodzovsky 2017-01-16 15:44 ` [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state Andrey Grodzovsky ` (3 more replies) 0 siblings, 4 replies; 27+ messages in thread From: Andrey Grodzovsky @ 2017-01-16 15:44 UTC (permalink / raw) To: dri-devel; +Cc: nouveau, amd-gfx, Alexander.Deucher, daniel.vetter This series is a folow-up on https://patchwork.kernel.org/patch/9501787/ The first patch makes changes to atomic helpers to allow for drives with ASYNC flip support to use them. Patches 2 and 3 are to use this in AMDGPU/DC and patch 4 is possible cleanup in nouveau/kms who seems to have the duplicate the helper as we did to support ASYNC flips. Andrey Grodzovsky (4): drm/atomic: Save flip flags in drm_plane_state drm/amdgpu: Remove flip_flag from amdgpu_crtc drm/amd/display: Switch to using atomic_helper for flip. drm/nouveau/kms/nv50: Switch to using atomic helper for flip. drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 ++-------------------- drivers/gpu/drm/drm_atomic_helper.c | 10 +-- drivers/gpu/drm/nouveau/nv50_display.c | 77 ++---------------- include/drm/drm_plane.h | 8 ++ 5 files changed, 22 insertions(+), 166 deletions(-) -- 1.9.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state 2017-01-16 15:44 [PATCH 0/4] Allow ASYNC flip with atomic helpers Andrey Grodzovsky @ 2017-01-16 15:44 ` Andrey Grodzovsky [not found] ` <1484581498-32309-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> ` (2 more replies) [not found] ` <1484581498-32309-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> ` (2 subsequent siblings) 3 siblings, 3 replies; 27+ messages in thread From: Andrey Grodzovsky @ 2017-01-16 15:44 UTC (permalink / raw) To: dri-devel; +Cc: nouveau, amd-gfx, Alexander.Deucher, daniel.vetter Allows using atomic flip helpers for drivers using ASYNC flip. Remove ASYNC_FLIP restriction in helpers and caches the page flip flags in drm_plane_state to be used in the low level drivers. Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> --- drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- include/drm/drm_plane.h | 8 ++++++++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..f83dc43 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2737,7 +2737,8 @@ static int page_flip_common( struct drm_atomic_state *state, struct drm_crtc *crtc, struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event) + struct drm_pending_vblank_event *event, + uint32_t flags) { struct drm_plane *plane = crtc->primary; struct drm_plane_state *plane_state; @@ -2754,6 +2755,7 @@ static int page_flip_common( if (IS_ERR(plane_state)) return PTR_ERR(plane_state); + plane_state->pflip_flags = flags; ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); if (ret != 0) @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, struct drm_atomic_state *state; int ret = 0; - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) - return -EINVAL; - state = drm_atomic_state_alloc(plane->dev); if (!state) return -ENOMEM; @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target( struct drm_crtc_state *crtc_state; int ret = 0; - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) - return -EINVAL; - state = drm_atomic_state_alloc(plane->dev); if (!state) return -ENOMEM; diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index db3bbde..86d8ffc 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -122,6 +122,14 @@ struct drm_plane_state { */ bool visible; + + /** + * @pflip_flags: + * + * Flip related config options + */ + u32 pflip_flags; + struct drm_atomic_state *state; }; -- 1.9.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 27+ messages in thread
[parent not found: <1484581498-32309-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state [not found] ` <1484581498-32309-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> @ 2017-01-16 20:21 ` Gustavo Padovan 2017-01-16 20:53 ` Grodzovsky, Andrey 2017-01-23 8:54 ` Daniel Vetter 1 sibling, 1 reply; 27+ messages in thread From: Gustavo Padovan @ 2017-01-16 20:21 UTC (permalink / raw) To: Andrey Grodzovsky Cc: Alexander.Deucher-5C7GfCeVMHo, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w Hi Andrey, 2017-01-16 Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>: > Allows using atomic flip helpers for drivers > using ASYNC flip. > Remove ASYNC_FLIP restriction in helpers and > caches the page flip flags in drm_plane_state > to be used in the low level drivers. > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- > include/drm/drm_plane.h | 8 ++++++++ > 2 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index a4e5477..f83dc43 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2737,7 +2737,8 @@ static int page_flip_common( > struct drm_atomic_state *state, > struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - struct drm_pending_vblank_event *event) > + struct drm_pending_vblank_event *event, > + uint32_t flags) Did you build this patch? It is changing the signature of page_flip_common() but no changes to the callers. Gustavo _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state 2017-01-16 20:21 ` Gustavo Padovan @ 2017-01-16 20:53 ` Grodzovsky, Andrey 0 siblings, 0 replies; 27+ messages in thread From: Grodzovsky, Andrey @ 2017-01-16 20:53 UTC (permalink / raw) To: Gustavo Padovan Cc: Deucher, Alexander, nouveau, amd-gfx, dri-devel, daniel.vetter > -----Original Message----- > From: Gustavo Padovan [mailto:gustavo@padovan.org] > Sent: Monday, January 16, 2017 3:22 PM > To: Grodzovsky, Andrey > Cc: dri-devel@lists.freedesktop.org; nouveau@lists.freedesktop.org; amd- > gfx@lists.freedesktop.org; Deucher, Alexander; daniel.vetter@intel.com > Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state > > Hi Andrey, > > 2017-01-16 Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>: > > > Allows using atomic flip helpers for drivers using ASYNC flip. > > Remove ASYNC_FLIP restriction in helpers and caches the page flip > > flags in drm_plane_state to be used in the low level drivers. > > > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- > > include/drm/drm_plane.h | 8 ++++++++ > > 2 files changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index a4e5477..f83dc43 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -2737,7 +2737,8 @@ static int page_flip_common( > > struct drm_atomic_state *state, > > struct drm_crtc *crtc, > > struct drm_framebuffer *fb, > > - struct drm_pending_vblank_event *event) > > + struct drm_pending_vblank_event *event, > > + uint32_t flags) > > Did you build this patch? It is changing the signature of > page_flip_common() but no changes to the callers. > > Gustavo Thanks for spotting this, I am afraid I've sent not the final version of the patch. I will resend the latest version later today. Thanks Andrey _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state [not found] ` <1484581498-32309-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> 2017-01-16 20:21 ` Gustavo Padovan @ 2017-01-23 8:54 ` Daniel Vetter 2017-01-23 19:48 ` Cheng, Tony 1 sibling, 1 reply; 27+ messages in thread From: Daniel Vetter @ 2017-01-23 8:54 UTC (permalink / raw) To: Andrey Grodzovsky Cc: Alexander.Deucher-5C7GfCeVMHo, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w On Mon, Jan 16, 2017 at 10:44:55AM -0500, Andrey Grodzovsky wrote: > Allows using atomic flip helpers for drivers > using ASYNC flip. > Remove ASYNC_FLIP restriction in helpers and > caches the page flip flags in drm_plane_state > to be used in the low level drivers. > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> It's mostly guesswork, but I think we should have the flip flags in the crtc, not in each plane. Similar to how we move the event from planes to crtc. -Daniel > --- > drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- > include/drm/drm_plane.h | 8 ++++++++ > 2 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index a4e5477..f83dc43 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2737,7 +2737,8 @@ static int page_flip_common( > struct drm_atomic_state *state, > struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - struct drm_pending_vblank_event *event) > + struct drm_pending_vblank_event *event, > + uint32_t flags) > { > struct drm_plane *plane = crtc->primary; > struct drm_plane_state *plane_state; > @@ -2754,6 +2755,7 @@ static int page_flip_common( > if (IS_ERR(plane_state)) > return PTR_ERR(plane_state); > > + plane_state->pflip_flags = flags; > > ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > if (ret != 0) > @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, > struct drm_atomic_state *state; > int ret = 0; > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > - return -EINVAL; > - > state = drm_atomic_state_alloc(plane->dev); > if (!state) > return -ENOMEM; > @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target( > struct drm_crtc_state *crtc_state; > int ret = 0; > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > - return -EINVAL; > - > state = drm_atomic_state_alloc(plane->dev); > if (!state) > return -ENOMEM; > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > index db3bbde..86d8ffc 100644 > --- a/include/drm/drm_plane.h > +++ b/include/drm/drm_plane.h > @@ -122,6 +122,14 @@ struct drm_plane_state { > */ > bool visible; > > + > + /** > + * @pflip_flags: > + * > + * Flip related config options > + */ > + u32 pflip_flags; > + > struct drm_atomic_state *state; > }; > > -- > 1.9.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state 2017-01-23 8:54 ` Daniel Vetter @ 2017-01-23 19:48 ` Cheng, Tony [not found] ` <MWHPR12MB14066404FBCEF3C03EF24BF998720-Gy0DoCVfaSXS1+HfpYZxvgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 2017-01-26 9:39 ` Daniel Vetter 0 siblings, 2 replies; 27+ messages in thread From: Cheng, Tony @ 2017-01-23 19:48 UTC (permalink / raw) To: Daniel Vetter, Grodzovsky, Andrey Cc: dc_upstream, nouveau, dri-devel, amd-gfx, Deucher, Alexander, daniel.vetter > -----Original Message----- > From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf > Of Daniel Vetter > Sent: Monday, January 23, 2017 3:55 AM > To: Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; > nouveau@lists.freedesktop.org; amd-gfx@lists.freedesktop.org; dri- > devel@lists.freedesktop.org; daniel.vetter@intel.com > Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state > > On Mon, Jan 16, 2017 at 10:44:55AM -0500, Andrey Grodzovsky wrote: > > Allows using atomic flip helpers for drivers using ASYNC flip. > > Remove ASYNC_FLIP restriction in helpers and caches the page flip > > flags in drm_plane_state to be used in the low level drivers. > > > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > > It's mostly guesswork, but I think we should have the flip flags in the crtc, not > in each plane. Similar to how we move the event from planes to crtc. > -Daniel What does ASYNC flip mean? HW flip as soon as possible and result in tearing on screen? If so I could imaging some use case where you have some UI control/menu overlay on top, and some game running on a underlay plane, and the game want to be able to flip as soon as possible. Or Daniel do you think ASYNC property will apply to all planes in CRTC? > > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- > > include/drm/drm_plane.h | 8 ++++++++ > > 2 files changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index a4e5477..f83dc43 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -2737,7 +2737,8 @@ static int page_flip_common( > > struct drm_atomic_state *state, > > struct drm_crtc *crtc, > > struct drm_framebuffer *fb, > > - struct drm_pending_vblank_event *event) > > + struct drm_pending_vblank_event *event, > > + uint32_t flags) > > { > > struct drm_plane *plane = crtc->primary; > > struct drm_plane_state *plane_state; @@ -2754,6 +2755,7 @@ static > > int page_flip_common( > > if (IS_ERR(plane_state)) > > return PTR_ERR(plane_state); > > > > + plane_state->pflip_flags = flags; > > > > ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > > if (ret != 0) > > @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc > *crtc, > > struct drm_atomic_state *state; > > int ret = 0; > > > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > > - return -EINVAL; > > - > > state = drm_atomic_state_alloc(plane->dev); > > if (!state) > > return -ENOMEM; > > @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target( > > struct drm_crtc_state *crtc_state; > > int ret = 0; > > > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > > - return -EINVAL; > > - > > state = drm_atomic_state_alloc(plane->dev); > > if (!state) > > return -ENOMEM; > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index > > db3bbde..86d8ffc 100644 > > --- a/include/drm/drm_plane.h > > +++ b/include/drm/drm_plane.h > > @@ -122,6 +122,14 @@ struct drm_plane_state { > > */ > > bool visible; > > > > + > > + /** > > + * @pflip_flags: > > + * > > + * Flip related config options > > + */ > > + u32 pflip_flags; > > + > > struct drm_atomic_state *state; > > }; > > > > -- > > 1.9.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <MWHPR12MB14066404FBCEF3C03EF24BF998720-Gy0DoCVfaSXS1+HfpYZxvgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>]
* RE: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state [not found] ` <MWHPR12MB14066404FBCEF3C03EF24BF998720-Gy0DoCVfaSXS1+HfpYZxvgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> @ 2017-01-26 5:01 ` Grodzovsky, Andrey [not found] ` <DM5PR12MB1147AEF3F8CFFAD4B69E114EEA770-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 0 siblings, 1 reply; 27+ messages in thread From: Grodzovsky, Andrey @ 2017-01-26 5:01 UTC (permalink / raw) To: Cheng, Tony, Daniel Vetter Cc: dc_upstream, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Deucher, Alexander, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w > -----Original Message----- > From: Cheng, Tony > Sent: Monday, January 23, 2017 2:49 PM > To: Daniel Vetter; Grodzovsky, Andrey > Cc: Deucher, Alexander; nouveau@lists.freedesktop.org; amd- > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; > daniel.vetter@intel.com; dc_upstream > Subject: RE: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state > > > > > -----Original Message----- > > From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On > > Behalf Of Daniel Vetter > > Sent: Monday, January 23, 2017 3:55 AM > > To: Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com> > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; > > nouveau@lists.freedesktop.org; amd-gfx@lists.freedesktop.org; dri- > > devel@lists.freedesktop.org; daniel.vetter@intel.com > > Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in > > drm_plane_state > > > > On Mon, Jan 16, 2017 at 10:44:55AM -0500, Andrey Grodzovsky wrote: > > > Allows using atomic flip helpers for drivers using ASYNC flip. > > > Remove ASYNC_FLIP restriction in helpers and caches the page flip > > > flags in drm_plane_state to be used in the low level drivers. > > > > > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > > > > It's mostly guesswork, but I think we should have the flip flags in > > the crtc, not in each plane. Similar to how we move the event from planes > to crtc. > > -Daniel > Do you mean here crtc or crtc_state ? > What does ASYNC flip mean? HW flip as soon as possible and result in tearing > on screen? If so I could imaging some use case where you have some UI > control/menu overlay on top, and some game running on a underlay plane, > and the game want to be able to flip as soon as possible. Or Daniel do you > think ASYNC property will apply to all planes in CRTC? > Also the client code both in nouveau and AMD/DAL iterates over new planes and their new states so it seems to me easier to retrieve the parameter from the plane_state and not adding loop to find matching crtc or its state, especially if it's before drm_atomic_helper_swap_state is called, when crtc->state still points to the old state ... [Grodzovsky, Andrey] > > > > > --- > > > drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- > > > include/drm/drm_plane.h | 8 ++++++++ > > > 2 files changed, 11 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > > b/drivers/gpu/drm/drm_atomic_helper.c > > > index a4e5477..f83dc43 100644 > > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > @@ -2737,7 +2737,8 @@ static int page_flip_common( > > > struct drm_atomic_state *state, > > > struct drm_crtc *crtc, > > > struct drm_framebuffer *fb, > > > - struct drm_pending_vblank_event *event) > > > + struct drm_pending_vblank_event *event, > > > + uint32_t flags) > > > { > > > struct drm_plane *plane = crtc->primary; > > > struct drm_plane_state *plane_state; @@ -2754,6 +2755,7 @@ static > > > int page_flip_common( > > > if (IS_ERR(plane_state)) > > > return PTR_ERR(plane_state); > > > > > > + plane_state->pflip_flags = flags; > > > > > > ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > > > if (ret != 0) > > > @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct > > > drm_crtc > > *crtc, > > > struct drm_atomic_state *state; > > > int ret = 0; > > > > > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > > > - return -EINVAL; > > > - > > > state = drm_atomic_state_alloc(plane->dev); > > > if (!state) > > > return -ENOMEM; > > > @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target( > > > struct drm_crtc_state *crtc_state; > > > int ret = 0; > > > > > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > > > - return -EINVAL; > > > - > > > state = drm_atomic_state_alloc(plane->dev); > > > if (!state) > > > return -ENOMEM; > > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index > > > db3bbde..86d8ffc 100644 > > > --- a/include/drm/drm_plane.h > > > +++ b/include/drm/drm_plane.h > > > @@ -122,6 +122,14 @@ struct drm_plane_state { > > > */ > > > bool visible; > > > > > > + > > > + /** > > > + * @pflip_flags: > > > + * > > > + * Flip related config options > > > + */ > > > + u32 pflip_flags; > > > + > > > struct drm_atomic_state *state; > > > }; > > > > > > -- > > > 1.9.1 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <DM5PR12MB1147AEF3F8CFFAD4B69E114EEA770-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>]
* Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state [not found] ` <DM5PR12MB1147AEF3F8CFFAD4B69E114EEA770-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> @ 2017-01-26 9:40 ` Daniel Vetter 0 siblings, 0 replies; 27+ messages in thread From: Daniel Vetter @ 2017-01-26 9:40 UTC (permalink / raw) To: Grodzovsky, Andrey Cc: dc_upstream, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Daniel Vetter, Deucher, Alexander, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w, Cheng, Tony On Thu, Jan 26, 2017 at 05:01:01AM +0000, Grodzovsky, Andrey wrote: > > > > -----Original Message----- > > From: Cheng, Tony > > Sent: Monday, January 23, 2017 2:49 PM > > To: Daniel Vetter; Grodzovsky, Andrey > > Cc: Deucher, Alexander; nouveau@lists.freedesktop.org; amd- > > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; > > daniel.vetter@intel.com; dc_upstream > > Subject: RE: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state > > > > > > > > > -----Original Message----- > > > From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On > > > Behalf Of Daniel Vetter > > > Sent: Monday, January 23, 2017 3:55 AM > > > To: Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com> > > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; > > > nouveau@lists.freedesktop.org; amd-gfx@lists.freedesktop.org; dri- > > > devel@lists.freedesktop.org; daniel.vetter@intel.com > > > Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in > > > drm_plane_state > > > > > > On Mon, Jan 16, 2017 at 10:44:55AM -0500, Andrey Grodzovsky wrote: > > > > Allows using atomic flip helpers for drivers using ASYNC flip. > > > > Remove ASYNC_FLIP restriction in helpers and caches the page flip > > > > flags in drm_plane_state to be used in the low level drivers. > > > > > > > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > > > > > > It's mostly guesswork, but I think we should have the flip flags in > > > the crtc, not in each plane. Similar to how we move the event from planes > > to crtc. > > > -Daniel > > > Do you mean here crtc or crtc_state ? Yes. > > What does ASYNC flip mean? HW flip as soon as possible and result in tearing > > on screen? If so I could imaging some use case where you have some UI > > control/menu overlay on top, and some game running on a underlay plane, > > and the game want to be able to flip as soon as possible. Or Daniel do you > > think ASYNC property will apply to all planes in CRTC? > > > Also the client code both in nouveau and AMD/DAL iterates over new planes and their new states > so it seems to me easier to retrieve the parameter from the plane_state and not adding loop > to find matching crtc or its state, especially if it's before drm_atomic_helper_swap_state is called, > when crtc->state still points to the old state ... It's pretty easy to go from plane_state to crtc_state. No need to loop at all. And the implementation shouldn't justify where we put it, the important part is the semantics. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state 2017-01-23 19:48 ` Cheng, Tony [not found] ` <MWHPR12MB14066404FBCEF3C03EF24BF998720-Gy0DoCVfaSXS1+HfpYZxvgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> @ 2017-01-26 9:39 ` Daniel Vetter 1 sibling, 0 replies; 27+ messages in thread From: Daniel Vetter @ 2017-01-26 9:39 UTC (permalink / raw) To: Cheng, Tony Cc: dc_upstream, nouveau, dri-devel, amd-gfx, Deucher, Alexander, daniel.vetter On Mon, Jan 23, 2017 at 07:48:54PM +0000, Cheng, Tony wrote: > > > > -----Original Message----- > > From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf > > Of Daniel Vetter > > Sent: Monday, January 23, 2017 3:55 AM > > To: Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com> > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; > > nouveau@lists.freedesktop.org; amd-gfx@lists.freedesktop.org; dri- > > devel@lists.freedesktop.org; daniel.vetter@intel.com > > Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state > > > > On Mon, Jan 16, 2017 at 10:44:55AM -0500, Andrey Grodzovsky wrote: > > > Allows using atomic flip helpers for drivers using ASYNC flip. > > > Remove ASYNC_FLIP restriction in helpers and caches the page flip > > > flags in drm_plane_state to be used in the low level drivers. > > > > > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > > > > It's mostly guesswork, but I think we should have the flip flags in the crtc, not > > in each plane. Similar to how we move the event from planes to crtc. > > -Daniel > > What does ASYNC flip mean? HW flip as soon as possible and result in > tearing on screen? If so I could imaging some use case where you have > some UI control/menu overlay on top, and some game running on a underlay > plane, and the game want to be able to flip as soon as possible. Or > Daniel do you think ASYNC property will apply to all planes in CRTC? Those kind of questions are exactly why I think we should wait with exposing async through the atomic ioctl until someone needs it. And yes async means "as fast as possible, with tearing". -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state 2017-01-16 15:44 ` [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state Andrey Grodzovsky [not found] ` <1484581498-32309-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> @ 2017-01-16 22:18 ` Laurent Pinchart 2017-01-17 4:03 ` Grodzovsky, Andrey 2017-01-17 8:14 ` Ville Syrjälä 2 siblings, 1 reply; 27+ messages in thread From: Laurent Pinchart @ 2017-01-16 22:18 UTC (permalink / raw) To: dri-devel; +Cc: Alexander.Deucher, daniel.vetter, amd-gfx, nouveau Hi Andrey, Thank you for the patch. On Monday 16 Jan 2017 10:44:55 Andrey Grodzovsky wrote: > Allows using atomic flip helpers for drivers > using ASYNC flip. > Remove ASYNC_FLIP restriction in helpers and > caches the page flip flags in drm_plane_state > to be used in the low level drivers. > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- > include/drm/drm_plane.h | 8 ++++++++ > 2 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..f83dc43 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2737,7 +2737,8 @@ static int page_flip_common( > struct drm_atomic_state *state, > struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - struct drm_pending_vblank_event *event) > + struct drm_pending_vblank_event *event, > + uint32_t flags) > { > struct drm_plane *plane = crtc->primary; > struct drm_plane_state *plane_state; > @@ -2754,6 +2755,7 @@ static int page_flip_common( > if (IS_ERR(plane_state)) > return PTR_ERR(plane_state); > > + plane_state->pflip_flags = flags; > > ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > if (ret != 0) > @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, > struct drm_atomic_state *state; > int ret = 0; > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > - return -EINVAL; > - With this change all drivers using the helper will not reject that async flag, even if they don't implement support for async page flip. You need to either patch them all to reject the flag, or implement async page flip support for all of them (preferable in the helpers, and then remove the * Note that for now so called async page flips (i.e. updates which are not * synchronized to vblank) are not supported, since the atomic interfaces have * no provisions for this yet. comment). > state = drm_atomic_state_alloc(plane->dev); > if (!state) > return -ENOMEM; > @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target( > struct drm_crtc_state *crtc_state; > int ret = 0; > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > - return -EINVAL; > - > state = drm_atomic_state_alloc(plane->dev); > if (!state) > return -ENOMEM; > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > index db3bbde..86d8ffc 100644 > --- a/include/drm/drm_plane.h > +++ b/include/drm/drm_plane.h > @@ -122,6 +122,14 @@ struct drm_plane_state { > */ > bool visible; > > + > + /** > + * @pflip_flags: > + * > + * Flip related config options > + */ > + u32 pflip_flags; > + > struct drm_atomic_state *state; > }; -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state 2017-01-16 22:18 ` Laurent Pinchart @ 2017-01-17 4:03 ` Grodzovsky, Andrey [not found] ` <DM5PR12MB1147FF9E484370F1572CB9C3EA7C0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 0 siblings, 1 reply; 27+ messages in thread From: Grodzovsky, Andrey @ 2017-01-17 4:03 UTC (permalink / raw) To: Laurent Pinchart, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: Deucher, Alexander, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w > -----Original Message----- > From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com] > Sent: Monday, January 16, 2017 5:18 PM > To: dri-devel@lists.freedesktop.org > Cc: Grodzovsky, Andrey; nouveau@lists.freedesktop.org; amd- > gfx@lists.freedesktop.org; Deucher, Alexander; daniel.vetter@intel.com > Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state > > Hi Andrey, > > Thank you for the patch. > > On Monday 16 Jan 2017 10:44:55 Andrey Grodzovsky wrote: > > Allows using atomic flip helpers for drivers using ASYNC flip. > > Remove ASYNC_FLIP restriction in helpers and caches the page flip > > flags in drm_plane_state to be used in the low level drivers. > > > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- > > include/drm/drm_plane.h | 8 ++++++++ > > 2 files changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..f83dc43 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -2737,7 +2737,8 @@ static int page_flip_common( > > struct drm_atomic_state *state, > > struct drm_crtc *crtc, > > struct drm_framebuffer *fb, > > - struct drm_pending_vblank_event *event) > > + struct drm_pending_vblank_event *event, > > + uint32_t flags) > > { > > struct drm_plane *plane = crtc->primary; > > struct drm_plane_state *plane_state; @@ -2754,6 +2755,7 @@ static > > int page_flip_common( > > if (IS_ERR(plane_state)) > > return PTR_ERR(plane_state); > > > > + plane_state->pflip_flags = flags; > > > > ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > > if (ret != 0) > > @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc > > *crtc, struct drm_atomic_state *state; > > int ret = 0; > > > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > > - return -EINVAL; > > - > > With this change all drivers using the helper will not reject that async flag, > even if they don't implement support for async page flip. You need to either > patch them all to reject the flag, or implement async page flip support for all > of them (preferable in the helpers, and then remove the Please check drm_mode_page_flip_ioctl, one of the checks in the beginning is if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) && !dev->mode_config.async_page_flip) return -EINVAL; We in DC explicitly set dev->mode_config.async_page_flip = true, any driver which is Not supporting ASYNC flip will fail the IOCTL at this point. Same in drm_mode_atomic_ioctl > > * Note that for now so called async page flips (i.e. updates which are not > * synchronized to vblank) are not supported, since the atomic interfaces > have > * no provisions for this yet. > > comment). Thanks, that a good catch, will remove. Andrey > > > state = drm_atomic_state_alloc(plane->dev); > > if (!state) > > return -ENOMEM; > > @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target( > > struct drm_crtc_state *crtc_state; > > int ret = 0; > > > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > > - return -EINVAL; > > - > > state = drm_atomic_state_alloc(plane->dev); > > if (!state) > > return -ENOMEM; > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index > > db3bbde..86d8ffc 100644 > > --- a/include/drm/drm_plane.h > > +++ b/include/drm/drm_plane.h > > @@ -122,6 +122,14 @@ struct drm_plane_state { > > */ > > bool visible; > > > > + > > + /** > > + * @pflip_flags: > > + * > > + * Flip related config options > > + */ > > + u32 pflip_flags; > > + > > struct drm_atomic_state *state; > > }; > > -- > Regards, > > Laurent Pinchart _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <DM5PR12MB1147FF9E484370F1572CB9C3EA7C0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>]
* Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state [not found] ` <DM5PR12MB1147FF9E484370F1572CB9C3EA7C0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> @ 2017-01-17 9:03 ` Laurent Pinchart 0 siblings, 0 replies; 27+ messages in thread From: Laurent Pinchart @ 2017-01-17 9:03 UTC (permalink / raw) To: Grodzovsky, Andrey Cc: Deucher, Alexander, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w Hi Andrey, On Tuesday 17 Jan 2017 04:03:11 Grodzovsky, Andrey wrote: > On Monday, January 16, 2017 5:18 PM Laurent Pinchart wrote: > > On Monday 16 Jan 2017 10:44:55 Andrey Grodzovsky wrote: > > > Allows using atomic flip helpers for drivers using ASYNC flip. > > > Remove ASYNC_FLIP restriction in helpers and caches the page flip > > > flags in drm_plane_state to be used in the low level drivers. > > > > >> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > >> --- > >> > >> drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- > >> include/drm/drm_plane.h | 8 ++++++++ > >> 2 files changed, 11 insertions(+), 7 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c > >> b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..f83dc43 100644 > >> --- a/drivers/gpu/drm/drm_atomic_helper.c > >> +++ b/drivers/gpu/drm/drm_atomic_helper.c > >> @@ -2737,7 +2737,8 @@ static int page_flip_common( > >> struct drm_atomic_state *state, > >> struct drm_crtc *crtc, > >> struct drm_framebuffer *fb, > >> - struct drm_pending_vblank_event *event) > >> + struct drm_pending_vblank_event *event, > >> + uint32_t flags) > >> { > >> struct drm_plane *plane = crtc->primary; > >> struct drm_plane_state *plane_state; > >> @@ -2754,6 +2755,7 @@ static int page_flip_common( > >> if (IS_ERR(plane_state)) > >> return PTR_ERR(plane_state); > >> > >> + plane_state->pflip_flags = flags; > >> > >> ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > >> if (ret != 0) > >> @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc > >> *crtc, > >> struct drm_atomic_state *state; > >> int ret = 0; > >> > >> - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > >> - return -EINVAL; > >> - > > > > With this change all drivers using the helper will not reject that async > > flag, even if they don't implement support for async page flip. You need > > to either patch them all to reject the flag, or implement async page flip > > support for all of them (preferable in the helpers, and then remove the > > Please check drm_mode_page_flip_ioctl, one of the checks in the beginning is > > if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) && > !dev->mode_config.async_page_flip) return -EINVAL; I think you're right. Sorry for the noise. > We in DC explicitly set dev->mode_config.async_page_flip = true, any driver > which is Not supporting ASYNC flip will fail the IOCTL at this point. > Same in drm_mode_atomic_ioctl > > > * Note that for now so called async page flips (i.e. updates which are > > not > > * synchronized to vblank) are not supported, since the atomic interfaces > > have > > * no provisions for this yet. > > > > comment). > > Thanks, that a good catch, will remove. -- Regards, Laurent Pinchart _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state 2017-01-16 15:44 ` [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state Andrey Grodzovsky [not found] ` <1484581498-32309-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> 2017-01-16 22:18 ` Laurent Pinchart @ 2017-01-17 8:14 ` Ville Syrjälä 2 siblings, 0 replies; 27+ messages in thread From: Ville Syrjälä @ 2017-01-17 8:14 UTC (permalink / raw) To: Andrey Grodzovsky Cc: Alexander.Deucher, nouveau, amd-gfx, dri-devel, daniel.vetter On Mon, Jan 16, 2017 at 10:44:55AM -0500, Andrey Grodzovsky wrote: > Allows using atomic flip helpers for drivers > using ASYNC flip. > Remove ASYNC_FLIP restriction in helpers and > caches the page flip flags in drm_plane_state > to be used in the low level drivers. > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > --- > drivers/gpu/drm/drm_atomic_helper.c | 10 +++------- > include/drm/drm_plane.h | 8 ++++++++ > 2 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index a4e5477..f83dc43 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2737,7 +2737,8 @@ static int page_flip_common( > struct drm_atomic_state *state, > struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - struct drm_pending_vblank_event *event) > + struct drm_pending_vblank_event *event, > + uint32_t flags) > { > struct drm_plane *plane = crtc->primary; > struct drm_plane_state *plane_state; > @@ -2754,6 +2755,7 @@ static int page_flip_common( > if (IS_ERR(plane_state)) > return PTR_ERR(plane_state); > > + plane_state->pflip_flags = flags; "pflip" looks off. Better just spell it out. These flags need to be reset in duplicate_state otherwise they leak into subsequent operations. This is why I don't really like the concept of "state" containing flags and stuff that are not really part of the state but rather part of the operation of moving from the old state to the new state. But maybe we don't have a better place for this sort of stuff? I have suggested at some point that we might rename drm_atomic_state to drm_atomic_transaction or something. Stuffing the flags (or just a bool perhaps?) in there might be less confusing. > > ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > if (ret != 0) > @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, > struct drm_atomic_state *state; > int ret = 0; > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > - return -EINVAL; > - > state = drm_atomic_state_alloc(plane->dev); > if (!state) > return -ENOMEM; > @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target( > struct drm_crtc_state *crtc_state; > int ret = 0; > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > - return -EINVAL; > - > state = drm_atomic_state_alloc(plane->dev); > if (!state) > return -ENOMEM; > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > index db3bbde..86d8ffc 100644 > --- a/include/drm/drm_plane.h > +++ b/include/drm/drm_plane.h > @@ -122,6 +122,14 @@ struct drm_plane_state { > */ > bool visible; > > + > + /** > + * @pflip_flags: > + * > + * Flip related config options > + */ > + u32 pflip_flags; > + > struct drm_atomic_state *state; > }; > > -- > 1.9.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <1484581498-32309-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>]
* [PATCH 2/4] drm/amdgpu: Remove flip_flag from amdgpu_crtc [not found] ` <1484581498-32309-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> @ 2017-01-16 15:44 ` Andrey Grodzovsky 2017-01-16 22:15 ` Laurent Pinchart 2017-01-16 15:44 ` [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip Andrey Grodzovsky 1 sibling, 1 reply; 27+ messages in thread From: Andrey Grodzovsky @ 2017-01-16 15:44 UTC (permalink / raw) To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: Andrey Grodzovsky, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Alexander.Deucher-5C7GfCeVMHo, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w, Harry.Wentland-5C7GfCeVMHo Follwing introduction of pflip_flags in drm_plane_state this is not needed anymore. Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index 4c0a86e..3ff3c14 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -443,7 +443,6 @@ struct amdgpu_crtc { enum amdgpu_interrupt_state vsync_timer_enabled; int otg_inst; - uint32_t flip_flags; /* After Set Mode target will be non-NULL */ struct dc_target *target; }; -- 1.9.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] drm/amdgpu: Remove flip_flag from amdgpu_crtc 2017-01-16 15:44 ` [PATCH 2/4] drm/amdgpu: Remove flip_flag from amdgpu_crtc Andrey Grodzovsky @ 2017-01-16 22:15 ` Laurent Pinchart 0 siblings, 0 replies; 27+ messages in thread From: Laurent Pinchart @ 2017-01-16 22:15 UTC (permalink / raw) To: dri-devel; +Cc: Alexander.Deucher, daniel.vetter, amd-gfx, nouveau Hi Andrey, Thank you for the patch. On Monday 16 Jan 2017 10:44:56 Andrey Grodzovsky wrote: > Follwing introduction of pflip_flags in drm_plane_state > this is not needed anymore. > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index 4c0a86e..3ff3c14 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > @@ -443,7 +443,6 @@ struct amdgpu_crtc { > enum amdgpu_interrupt_state vsync_timer_enabled; > > int otg_inst; > - uint32_t flip_flags; This breaks compilation of the amdgpu driver. You should squash this patch with 3/4. > /* After Set Mode target will be non-NULL */ > struct dc_target *target; > }; -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip. [not found] ` <1484581498-32309-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> 2017-01-16 15:44 ` [PATCH 2/4] drm/amdgpu: Remove flip_flag from amdgpu_crtc Andrey Grodzovsky @ 2017-01-16 15:44 ` Andrey Grodzovsky 2017-01-16 22:16 ` Laurent Pinchart 1 sibling, 1 reply; 27+ messages in thread From: Andrey Grodzovsky @ 2017-01-16 15:44 UTC (permalink / raw) To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: Andrey Grodzovsky, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Alexander.Deucher-5C7GfCeVMHo, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w, Harry.Wentland-5C7GfCeVMHo Change-Id: Iad3e0b9b3546e4e4dc79be9233daf4fe4dba83e0 Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> --- .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 ++-------------------- 1 file changed, 6 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c index a443b70..d4664bf 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property( return 0; } - -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event, - uint32_t flags) -{ - struct drm_plane *plane = crtc->primary; - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); - struct drm_atomic_state *state; - struct drm_plane_state *plane_state; - struct drm_crtc_state *crtc_state; - int ret = 0; - - state = drm_atomic_state_alloc(plane->dev); - if (!state) - return -ENOMEM; - - ret = drm_crtc_vblank_get(crtc); - if (ret) - return ret; - - state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); -retry: - crtc_state = drm_atomic_get_crtc_state(state, crtc); - if (IS_ERR(crtc_state)) { - ret = PTR_ERR(crtc_state); - goto fail; - } - crtc_state->event = event; - - plane_state = drm_atomic_get_plane_state(state, plane); - if (IS_ERR(plane_state)) { - ret = PTR_ERR(plane_state); - goto fail; - } - - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); - if (ret != 0) - goto fail; - drm_atomic_set_fb_for_plane(plane_state, fb); - - /* Make sure we don't accidentally do a full modeset. */ - state->allow_modeset = false; - if (!crtc_state->active) { - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", - crtc->base.id); - ret = -EINVAL; - goto fail; - } - acrtc->flip_flags = flags; - - ret = drm_atomic_nonblocking_commit(state); - -fail: - if (ret == -EDEADLK) - goto backoff; - - if (ret) - drm_crtc_vblank_put(crtc); - - drm_atomic_state_put(state); - - return ret; -backoff: - drm_atomic_state_clear(state); - drm_atomic_legacy_backoff(state); - - /* - * Someone might have exchanged the framebuffer while we dropped locks - * in the backoff code. We need to fix up the fb refcount tracking the - * core does for us. - */ - plane->old_fb = plane->fb; - - goto retry; -} - /* Implemented only the options currently availible for the driver */ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { .reset = drm_atomic_helper_crtc_reset, @@ -1145,7 +1068,7 @@ static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, .destroy = amdgpu_dm_crtc_destroy, .gamma_set = amdgpu_dm_atomic_crtc_gamma_set, .set_config = drm_atomic_helper_set_config, - .page_flip = amdgpu_atomic_helper_page_flip, + .page_flip_target = drm_atomic_helper_page_flip_target, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .atomic_set_property = dm_crtc_funcs_atomic_set_property @@ -1679,7 +1602,7 @@ static bool page_flip_needed( sizeof(old_state_tmp)) == 0 ? true:false; if (new_state->crtc && page_flip_required == false) { acrtc_new = to_amdgpu_crtc(new_state->crtc); - if (acrtc_new->flip_flags & DRM_MODE_PAGE_FLIP_ASYNC) + if (new_state->pflip_flags & DRM_MODE_PAGE_FLIP_ASYNC) page_flip_required = true; } return page_flip_required; @@ -2760,7 +2683,6 @@ int amdgpu_dm_atomic_commit( for_each_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_framebuffer *fb = plane_state->fb; if (!fb || !crtc || !crtc->state->planes_changed || @@ -2771,10 +2693,9 @@ int amdgpu_dm_atomic_commit( ret = amdgpu_crtc_page_flip_target(crtc, fb, crtc->state->event, - acrtc->flip_flags, - drm_crtc_vblank_count(crtc)); - /*clean up the flags for next usage*/ - acrtc->flip_flags = 0; + plane_state->pflip_flags, + crtc->state->target_vblank); + if (ret) break; } @@ -3143,8 +3064,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, * 1. This commit is not a page flip. * 2. This commit is a page flip, and targets are created. */ - if (!page_flip_needed(plane_state, old_plane_state, - true) || + if (!page_flip_needed(plane_state, old_plane_state, true) || action == DM_COMMIT_ACTION_DPMS_ON || action == DM_COMMIT_ACTION_SET) { struct dc_surface *surface; -- 1.9.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip. 2017-01-16 15:44 ` [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip Andrey Grodzovsky @ 2017-01-16 22:16 ` Laurent Pinchart 2017-01-18 1:50 ` Michel Dänzer 0 siblings, 1 reply; 27+ messages in thread From: Laurent Pinchart @ 2017-01-16 22:16 UTC (permalink / raw) To: dri-devel; +Cc: Alexander.Deucher, daniel.vetter, amd-gfx, nouveau Hi Andrey, Thank you for the patch. On Monday 16 Jan 2017 10:44:57 Andrey Grodzovsky wrote: > Change-Id: Iad3e0b9b3546e4e4dc79be9233daf4fe4dba83e0 > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > --- > .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 ++---------------- > 1 file changed, 6 insertions(+), 86 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c index > a443b70..d4664bf 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property( > return 0; > } > > - > -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, > - struct drm_framebuffer *fb, > - struct drm_pending_vblank_event *event, > - uint32_t flags) > -{ > - struct drm_plane *plane = crtc->primary; > - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); > - struct drm_atomic_state *state; > - struct drm_plane_state *plane_state; > - struct drm_crtc_state *crtc_state; > - int ret = 0; > - > - state = drm_atomic_state_alloc(plane->dev); > - if (!state) > - return -ENOMEM; > - > - ret = drm_crtc_vblank_get(crtc); The DRM core's atomic page flip helper doesn't get/put vblank. Have you double-checked that removing them isn't a problem ? > - if (ret) > - return ret; > - > - state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); > -retry: > - crtc_state = drm_atomic_get_crtc_state(state, crtc); > - if (IS_ERR(crtc_state)) { > - ret = PTR_ERR(crtc_state); > - goto fail; > - } > - crtc_state->event = event; > - > - plane_state = drm_atomic_get_plane_state(state, plane); > - if (IS_ERR(plane_state)) { > - ret = PTR_ERR(plane_state); > - goto fail; > - } > - > - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > - if (ret != 0) > - goto fail; > - drm_atomic_set_fb_for_plane(plane_state, fb); > - > - /* Make sure we don't accidentally do a full modeset. */ > - state->allow_modeset = false; > - if (!crtc_state->active) { > - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", > - crtc->base.id); > - ret = -EINVAL; > - goto fail; > - } > - acrtc->flip_flags = flags; > - > - ret = drm_atomic_nonblocking_commit(state); > - > -fail: > - if (ret == -EDEADLK) > - goto backoff; > - > - if (ret) > - drm_crtc_vblank_put(crtc); > - > - drm_atomic_state_put(state); > - > - return ret; > -backoff: > - drm_atomic_state_clear(state); > - drm_atomic_legacy_backoff(state); > - > - /* > - * Someone might have exchanged the framebuffer while we dropped locks > - * in the backoff code. We need to fix up the fb refcount tracking the > - * core does for us. > - */ > - plane->old_fb = plane->fb; > - > - goto retry; > -} > - > /* Implemented only the options currently availible for the driver */ > static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { > .reset = drm_atomic_helper_crtc_reset, > @@ -1145,7 +1068,7 @@ static int amdgpu_atomic_helper_page_flip(struct > drm_crtc *crtc, .destroy = amdgpu_dm_crtc_destroy, > .gamma_set = amdgpu_dm_atomic_crtc_gamma_set, > .set_config = drm_atomic_helper_set_config, > - .page_flip = amdgpu_atomic_helper_page_flip, > + .page_flip_target = drm_atomic_helper_page_flip_target, > .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > .atomic_set_property = dm_crtc_funcs_atomic_set_property > @@ -1679,7 +1602,7 @@ static bool page_flip_needed( > sizeof(old_state_tmp)) == 0 ? true:false; > if (new_state->crtc && page_flip_required == false) { > acrtc_new = to_amdgpu_crtc(new_state->crtc); > - if (acrtc_new->flip_flags & DRM_MODE_PAGE_FLIP_ASYNC) > + if (new_state->pflip_flags & DRM_MODE_PAGE_FLIP_ASYNC) > page_flip_required = true; > } > return page_flip_required; > @@ -2760,7 +2683,6 @@ int amdgpu_dm_atomic_commit( > for_each_plane_in_state(state, plane, old_plane_state, i) { > struct drm_plane_state *plane_state = plane->state; > struct drm_crtc *crtc = plane_state->crtc; > - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); > struct drm_framebuffer *fb = plane_state->fb; > > if (!fb || !crtc || !crtc->state->planes_changed || > @@ -2771,10 +2693,9 @@ int amdgpu_dm_atomic_commit( > ret = amdgpu_crtc_page_flip_target(crtc, > fb, > crtc->state->event, > - acrtc->flip_flags, > - drm_crtc_vblank_count(crtc)); > - /*clean up the flags for next usage*/ > - acrtc->flip_flags = 0; > + plane_state- >pflip_flags, > + crtc->state- >target_vblank); > + > if (ret) > break; > } > @@ -3143,8 +3064,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, > * 1. This commit is not a page flip. > * 2. This commit is a page flip, and targets are created. > */ > - if (!page_flip_needed(plane_state, old_plane_state, > - true) || > + if (!page_flip_needed(plane_state, old_plane_state, true) || This seems to be an unrelated change. > action == DM_COMMIT_ACTION_DPMS_ON || > action == DM_COMMIT_ACTION_SET) { > struct dc_surface *surface; -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip. 2017-01-16 22:16 ` Laurent Pinchart @ 2017-01-18 1:50 ` Michel Dänzer 2017-01-18 12:02 ` Laurent Pinchart ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Michel Dänzer @ 2017-01-18 1:50 UTC (permalink / raw) To: Laurent Pinchart Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Andrey Grodzovsky, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w On 17/01/17 07:16 AM, Laurent Pinchart wrote: > On Monday 16 Jan 2017 10:44:57 Andrey Grodzovsky wrote: >> Change-Id: Iad3e0b9b3546e4e4dc79be9233daf4fe4dba83e0 >> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> >> --- >> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 ++---------------- >> 1 file changed, 6 insertions(+), 86 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c index >> a443b70..d4664bf 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c >> @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property( >> return 0; >> } >> >> - >> -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, >> - struct drm_framebuffer *fb, >> - struct drm_pending_vblank_event *event, >> - uint32_t flags) >> -{ >> - struct drm_plane *plane = crtc->primary; >> - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); >> - struct drm_atomic_state *state; >> - struct drm_plane_state *plane_state; >> - struct drm_crtc_state *crtc_state; >> - int ret = 0; >> - >> - state = drm_atomic_state_alloc(plane->dev); >> - if (!state) >> - return -ENOMEM; >> - >> - ret = drm_crtc_vblank_get(crtc); > > The DRM core's atomic page flip helper doesn't get/put vblank. Have you > double-checked that removing them isn't a problem ? This patch makes the amdgpu DM code use the page_flip_target hook. drm_mode_page_flip_ioctl calls drm_crtc_vblank_get before the page_flip_target hook. You're right though that the fact that drm_atomic_helper_page_flip doesn't call drm_crtc_vblank_get is a bit alarming. From the DRM_IOCTL_MODE_PAGE_FLIP POV, drm_crtc_vblank_get must be called when userspace calls the ioctl (either by drm_mode_page_flip_ioctl or the page_flip hook implementation), and drm_crtc_vblank_put must be called when the flip completes and the event is sent to userspace. How is this supposed to be handled with atomic? Andrey, did you check via code audit and/or testing that the vblank reference count is still balanced after this change? >> @@ -3143,8 +3064,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, >> * 1. This commit is not a page flip. >> * 2. This commit is a page flip, and targets are > created. >> */ >> - if (!page_flip_needed(plane_state, old_plane_state, >> - true) || >> + if (!page_flip_needed(plane_state, old_plane_state, > true) || > > This seems to be an unrelated change. Yeah, such whitespace-only changes should be dropped. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip. 2017-01-18 1:50 ` Michel Dänzer @ 2017-01-18 12:02 ` Laurent Pinchart 2017-01-18 22:18 ` Grodzovsky, Andrey 2017-01-19 4:57 ` [PATCH v3 2/3] " Andrey Grodzovsky 2 siblings, 0 replies; 27+ messages in thread From: Laurent Pinchart @ 2017-01-18 12:02 UTC (permalink / raw) To: Michel Dänzer; +Cc: nouveau, amd-gfx, dri-devel, daniel.vetter Hi Michel, On Wednesday 18 Jan 2017 10:50:01 Michel Dänzer wrote: > On 17/01/17 07:16 AM, Laurent Pinchart wrote: > > On Monday 16 Jan 2017 10:44:57 Andrey Grodzovsky wrote: > >> Change-Id: Iad3e0b9b3546e4e4dc79be9233daf4fe4dba83e0 > >> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > >> --- > >> > >> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 > >> ++---------------- > >> 1 file changed, 6 insertions(+), 86 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c index > >> a443b70..d4664bf 100644 > >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > >> @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property( > >> > >> return 0; > >> > >> } > >> > >> - > >> -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, > >> - struct drm_framebuffer *fb, > >> - struct drm_pending_vblank_event *event, > >> - uint32_t flags) > >> -{ > >> - struct drm_plane *plane = crtc->primary; > >> - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); > >> - struct drm_atomic_state *state; > >> - struct drm_plane_state *plane_state; > >> - struct drm_crtc_state *crtc_state; > >> - int ret = 0; > >> - > >> - state = drm_atomic_state_alloc(plane->dev); > >> - if (!state) > >> - return -ENOMEM; > >> - > >> - ret = drm_crtc_vblank_get(crtc); > > > > The DRM core's atomic page flip helper doesn't get/put vblank. Have you > > double-checked that removing them isn't a problem ? > > This patch makes the amdgpu DM code use the page_flip_target hook. > drm_mode_page_flip_ioctl calls drm_crtc_vblank_get before the > page_flip_target hook. > > You're right though that the fact that drm_atomic_helper_page_flip > doesn't call drm_crtc_vblank_get is a bit alarming. From the > DRM_IOCTL_MODE_PAGE_FLIP POV, drm_crtc_vblank_get must be called when > userspace calls the ioctl (either by drm_mode_page_flip_ioctl or the > page_flip hook implementation), and drm_crtc_vblank_put must be called > when the flip completes and the event is sent to userspace. How is this > supposed to be handled with atomic? I'll let Daniel comment on that. > Andrey, did you check via code audit and/or testing that the vblank > reference count is still balanced after this change? > > >> @@ -3143,8 +3064,7 @@ int amdgpu_dm_atomic_check(struct drm_device *dev, > >> > >> * 1. This commit is not a page flip. > >> * 2. This commit is a page flip, and targets are > > > > created. > > > >> */ > >> > >> - if (!page_flip_needed(plane_state, old_plane_state, > >> - true) || > >> + if (!page_flip_needed(plane_state, old_plane_state, > > > > true) || > > > > This seems to be an unrelated change. > > Yeah, such whitespace-only changes should be dropped. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip. 2017-01-18 1:50 ` Michel Dänzer 2017-01-18 12:02 ` Laurent Pinchart @ 2017-01-18 22:18 ` Grodzovsky, Andrey [not found] ` <DM5PR12MB1147CFCD90203061E0F2EB95EA7F0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 2017-01-19 4:57 ` [PATCH v3 2/3] " Andrey Grodzovsky 2 siblings, 1 reply; 27+ messages in thread From: Grodzovsky, Andrey @ 2017-01-18 22:18 UTC (permalink / raw) To: Michel Dänzer, Laurent Pinchart Cc: dc_upstream, daniel.vetter, amd-gfx, dri-devel, nouveau > -----Original Message----- > From: Michel Dänzer [mailto:michel@daenzer.net] > Sent: Tuesday, January 17, 2017 8:50 PM > To: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org; Grodzovsky, Andrey; > daniel.vetter@intel.com; amd-gfx@lists.freedesktop.org; > nouveau@lists.freedesktop.org > Subject: Re: [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for > flip. > > On 17/01/17 07:16 AM, Laurent Pinchart wrote: > > On Monday 16 Jan 2017 10:44:57 Andrey Grodzovsky wrote: > >> Change-Id: Iad3e0b9b3546e4e4dc79be9233daf4fe4dba83e0 > >> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> > >> --- > >> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 ++--------- > ------- > >> 1 file changed, 6 insertions(+), 86 deletions(-) > >> > >> diff --git > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > index > >> a443b70..d4664bf 100644 > >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > >> @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property( > >> return 0; > >> } > >> > >> - > >> -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, > >> - struct drm_framebuffer *fb, > >> - struct drm_pending_vblank_event *event, > >> - uint32_t flags) > >> -{ > >> - struct drm_plane *plane = crtc->primary; > >> - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); > >> - struct drm_atomic_state *state; > >> - struct drm_plane_state *plane_state; > >> - struct drm_crtc_state *crtc_state; > >> - int ret = 0; > >> - > >> - state = drm_atomic_state_alloc(plane->dev); > >> - if (!state) > >> - return -ENOMEM; > >> - > >> - ret = drm_crtc_vblank_get(crtc); > > > > The DRM core's atomic page flip helper doesn't get/put vblank. Have > > you double-checked that removing them isn't a problem ? > > This patch makes the amdgpu DM code use the page_flip_target hook. > drm_mode_page_flip_ioctl calls drm_crtc_vblank_get before the > page_flip_target hook. > > You're right though that the fact that drm_atomic_helper_page_flip doesn't > call drm_crtc_vblank_get is a bit alarming. From the > DRM_IOCTL_MODE_PAGE_FLIP POV, drm_crtc_vblank_get must be called > when userspace calls the ioctl (either by drm_mode_page_flip_ioctl or the > page_flip hook implementation), and drm_crtc_vblank_put must be called > when the flip completes and the event is sent to userspace. How is this > supposed to be handled with atomic? First let me ask you - why we have to enable vlbank as part of pflip, I understand why it has to be enbbled in WAIT_FOR_VBLANK IOCTL but not sure about PFLIP IOCTL. IMHO in atomic as before in legacy page_flip, it's up to the driver implementation in atomic_commit hook to manage vblank ISR on,off. > > Andrey, did you check via code audit and/or testing that the vblank > reference count is still balanced after this change? > With the page_flip_target yes, if I switch back to page_flip hook then vblank ISR never gets enabled on FLIP IPCTL and then I see warning in DAL's pflip done IRQ handler from vblank_put which is obvious, which BTW didn't impact the flips, I still was able to run glxgears in vsync mode. > > >> @@ -3143,8 +3064,7 @@ int amdgpu_dm_atomic_check(struct > drm_device *dev, > >> * 1. This commit is not a page flip. > >> * 2. This commit is a page flip, and targets are > > created. > >> */ > >> - if (!page_flip_needed(plane_state, old_plane_state, > >> - true) || > >> + if (!page_flip_needed(plane_state, old_plane_state, > > true) || > > > > This seems to be an unrelated change. > > Yeah, such whitespace-only changes should be dropped. Will remove this. > > > -- > Earthling Michel Dänzer | http://www.amd.com > Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <DM5PR12MB1147CFCD90203061E0F2EB95EA7F0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>]
* Re: [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip. [not found] ` <DM5PR12MB1147CFCD90203061E0F2EB95EA7F0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> @ 2017-01-19 2:14 ` Michel Dänzer 0 siblings, 0 replies; 27+ messages in thread From: Michel Dänzer @ 2017-01-19 2:14 UTC (permalink / raw) To: Grodzovsky, Andrey, Laurent Pinchart Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dc_upstream, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w On 19/01/17 07:18 AM, Grodzovsky, Andrey wrote: >> From: Michel Dänzer [mailto:michel@daenzer.net] >> On 17/01/17 07:16 AM, Laurent Pinchart wrote: >>> On Monday 16 Jan 2017 10:44:57 Andrey Grodzovsky wrote: >>>> Change-Id: Iad3e0b9b3546e4e4dc79be9233daf4fe4dba83e0 >>>> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> >>>> --- >>>> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 ++--------- >> ------- >>>> 1 file changed, 6 insertions(+), 86 deletions(-) >>>> >>>> diff --git >> a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c >>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c >> index >>>> a443b70..d4664bf 100644 >>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c >>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c >>>> @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property( >>>> return 0; >>>> } >>>> >>>> - >>>> -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, >>>> - struct drm_framebuffer *fb, >>>> - struct drm_pending_vblank_event *event, >>>> - uint32_t flags) >>>> -{ >>>> - struct drm_plane *plane = crtc->primary; >>>> - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); >>>> - struct drm_atomic_state *state; >>>> - struct drm_plane_state *plane_state; >>>> - struct drm_crtc_state *crtc_state; >>>> - int ret = 0; >>>> - >>>> - state = drm_atomic_state_alloc(plane->dev); >>>> - if (!state) >>>> - return -ENOMEM; >>>> - >>>> - ret = drm_crtc_vblank_get(crtc); >>> >>> The DRM core's atomic page flip helper doesn't get/put vblank. Have >>> you double-checked that removing them isn't a problem ? >> >> This patch makes the amdgpu DM code use the page_flip_target hook. >> drm_mode_page_flip_ioctl calls drm_crtc_vblank_get before the >> page_flip_target hook. >> >> You're right though that the fact that drm_atomic_helper_page_flip doesn't >> call drm_crtc_vblank_get is a bit alarming. From the >> DRM_IOCTL_MODE_PAGE_FLIP POV, drm_crtc_vblank_get must be called >> when userspace calls the ioctl (either by drm_mode_page_flip_ioctl or the >> page_flip hook implementation), and drm_crtc_vblank_put must be called >> when the flip completes and the event is sent to userspace. How is this >> supposed to be handled with atomic? > > First let me ask you - why we have to enable vlbank as part of pflip, > I understand why it has to be enbbled in WAIT_FOR_VBLANK IOCTL > but not sure about PFLIP IOCTL. It's required for the vblank sequence counter to be accurate in the page flip completion event sent to userspace. > IMHO in atomic as before in legacy page_flip, it's up to the driver > implementation in atomic_commit hook to manage vblank ISR on,off. When using the page_flip hook, it's all up to the implementation of that hook. When using the page_flip_target hook, drm_mode_page_flip_ioctl calls vblank_get, so the hook implementation must make sure that a matching vblank_put call is made when the flip completes. >> Andrey, did you check via code audit and/or testing that the vblank >> reference count is still balanced after this change? >> > With the page_flip_target yes, if I switch back to page_flip hook then > vblank ISR never gets enabled on FLIP IPCTL and then I see warning in DAL's > pflip done IRQ handler from vblank_put which is obvious, For drivers using the atomic_helpers for flip, maybe there should be (or might be already) a corresponding helper which deals with things like this when the flip completes, so drivers don't have to call vblank_put and such. Anyway, it sounds like the vblank_get/puts are balanced with this patch for now. > which BTW didn't impact the flips, I still was able to run glxgears > in vsync mode. I don't think glxgears relies on the vblank sequence numbers being accurate. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 2/3] drm/amd/display: Switch to using atomic_helper for flip. 2017-01-18 1:50 ` Michel Dänzer 2017-01-18 12:02 ` Laurent Pinchart 2017-01-18 22:18 ` Grodzovsky, Andrey @ 2017-01-19 4:57 ` Andrey Grodzovsky 2 siblings, 0 replies; 27+ messages in thread From: Andrey Grodzovsky @ 2017-01-19 4:57 UTC (permalink / raw) To: dri-devel Cc: dc_upstream, nouveau, michel, amd-gfx, laurent.pinchart, Alexander.Deucher, daniel.vetter v3: Remove white space change. Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 89 ++-------------------- 2 files changed, 5 insertions(+), 85 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index 4c0a86e..3ff3c14 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -443,7 +443,6 @@ struct amdgpu_crtc { enum amdgpu_interrupt_state vsync_timer_enabled; int otg_inst; - uint32_t flip_flags; /* After Set Mode target will be non-NULL */ struct dc_target *target; }; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c index a443b70..70e01ad 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property( return 0; } - -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event, - uint32_t flags) -{ - struct drm_plane *plane = crtc->primary; - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); - struct drm_atomic_state *state; - struct drm_plane_state *plane_state; - struct drm_crtc_state *crtc_state; - int ret = 0; - - state = drm_atomic_state_alloc(plane->dev); - if (!state) - return -ENOMEM; - - ret = drm_crtc_vblank_get(crtc); - if (ret) - return ret; - - state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); -retry: - crtc_state = drm_atomic_get_crtc_state(state, crtc); - if (IS_ERR(crtc_state)) { - ret = PTR_ERR(crtc_state); - goto fail; - } - crtc_state->event = event; - - plane_state = drm_atomic_get_plane_state(state, plane); - if (IS_ERR(plane_state)) { - ret = PTR_ERR(plane_state); - goto fail; - } - - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); - if (ret != 0) - goto fail; - drm_atomic_set_fb_for_plane(plane_state, fb); - - /* Make sure we don't accidentally do a full modeset. */ - state->allow_modeset = false; - if (!crtc_state->active) { - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", - crtc->base.id); - ret = -EINVAL; - goto fail; - } - acrtc->flip_flags = flags; - - ret = drm_atomic_nonblocking_commit(state); - -fail: - if (ret == -EDEADLK) - goto backoff; - - if (ret) - drm_crtc_vblank_put(crtc); - - drm_atomic_state_put(state); - - return ret; -backoff: - drm_atomic_state_clear(state); - drm_atomic_legacy_backoff(state); - - /* - * Someone might have exchanged the framebuffer while we dropped locks - * in the backoff code. We need to fix up the fb refcount tracking the - * core does for us. - */ - plane->old_fb = plane->fb; - - goto retry; -} - /* Implemented only the options currently availible for the driver */ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { .reset = drm_atomic_helper_crtc_reset, @@ -1145,7 +1068,7 @@ static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, .destroy = amdgpu_dm_crtc_destroy, .gamma_set = amdgpu_dm_atomic_crtc_gamma_set, .set_config = drm_atomic_helper_set_config, - .page_flip = amdgpu_atomic_helper_page_flip, + .page_flip_target = drm_atomic_helper_page_flip_target, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .atomic_set_property = dm_crtc_funcs_atomic_set_property @@ -1679,7 +1602,7 @@ static bool page_flip_needed( sizeof(old_state_tmp)) == 0 ? true:false; if (new_state->crtc && page_flip_required == false) { acrtc_new = to_amdgpu_crtc(new_state->crtc); - if (acrtc_new->flip_flags & DRM_MODE_PAGE_FLIP_ASYNC) + if (new_state->pflip_flags & DRM_MODE_PAGE_FLIP_ASYNC) page_flip_required = true; } return page_flip_required; @@ -2760,7 +2683,6 @@ int amdgpu_dm_atomic_commit( for_each_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_framebuffer *fb = plane_state->fb; if (!fb || !crtc || !crtc->state->planes_changed || @@ -2771,10 +2693,9 @@ int amdgpu_dm_atomic_commit( ret = amdgpu_crtc_page_flip_target(crtc, fb, crtc->state->event, - acrtc->flip_flags, - drm_crtc_vblank_count(crtc)); - /*clean up the flags for next usage*/ - acrtc->flip_flags = 0; + plane_state->pflip_flags, + crtc->state->target_vblank); + if (ret) break; } -- 1.9.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 4/4] drm/nouveau/kms/nv50: Switch to using atomic helper for flip. 2017-01-16 15:44 [PATCH 0/4] Allow ASYNC flip with atomic helpers Andrey Grodzovsky 2017-01-16 15:44 ` [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state Andrey Grodzovsky [not found] ` <1484581498-32309-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> @ 2017-01-16 15:44 ` Andrey Grodzovsky 2017-01-16 20:39 ` [PATCH 0/4] Allow ASYNC flip with atomic helpers Laurent Pinchart 3 siblings, 0 replies; 27+ messages in thread From: Andrey Grodzovsky @ 2017-01-16 15:44 UTC (permalink / raw) To: dri-devel; +Cc: nouveau, amd-gfx, Alexander.Deucher, daniel.vetter Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> --- drivers/gpu/drm/nouveau/nv50_display.c | 77 +++------------------------------- 1 file changed, 5 insertions(+), 72 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index 2c2c645..419e00c 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -846,6 +846,10 @@ struct nv50_wndw_func { asyw->image.w = fb->base.width; asyw->image.h = fb->base.height; asyw->image.kind = (fb->nvbo->tile_flags & 0x0000ff00) >> 8; + + asyw->interval = + asyw->state.pflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ? 0 : 1; + if (asyw->image.kind) { asyw->image.layout = 0; if (drm->device.info.chipset >= 0xc0) @@ -2221,77 +2225,6 @@ struct nv50_base { .atomic_check = nv50_head_atomic_check, }; -/* This is identical to the version in the atomic helpers, except that - * it supports non-vblanked ("async") page flips. - */ -static int -nv50_head_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event, u32 flags) -{ - struct drm_plane *plane = crtc->primary; - struct drm_atomic_state *state; - struct drm_plane_state *plane_state; - struct drm_crtc_state *crtc_state; - int ret = 0; - - state = drm_atomic_state_alloc(plane->dev); - if (!state) - return -ENOMEM; - - state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); -retry: - crtc_state = drm_atomic_get_crtc_state(state, crtc); - if (IS_ERR(crtc_state)) { - ret = PTR_ERR(crtc_state); - goto fail; - } - crtc_state->event = event; - - plane_state = drm_atomic_get_plane_state(state, plane); - if (IS_ERR(plane_state)) { - ret = PTR_ERR(plane_state); - goto fail; - } - - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); - if (ret != 0) - goto fail; - drm_atomic_set_fb_for_plane(plane_state, fb); - - /* Make sure we don't accidentally do a full modeset. */ - state->allow_modeset = false; - if (!crtc_state->active) { - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", - crtc->base.id); - ret = -EINVAL; - goto fail; - } - - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) - nv50_wndw_atom(plane_state)->interval = 0; - - ret = drm_atomic_nonblocking_commit(state); -fail: - if (ret == -EDEADLK) - goto backoff; - - drm_atomic_state_put(state); - return ret; - -backoff: - drm_atomic_state_clear(state); - drm_atomic_legacy_backoff(state); - - /* - * Someone might have exchanged the framebuffer while we dropped locks - * in the backoff code. We need to fix up the fb refcount tracking the - * core does for us. - */ - plane->old_fb = plane->fb; - - goto retry; -} - static int nv50_head_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, uint32_t size) @@ -2386,7 +2319,7 @@ struct nv50_base { .gamma_set = nv50_head_gamma_set, .destroy = nv50_head_destroy, .set_config = drm_atomic_helper_set_config, - .page_flip = nv50_head_page_flip, + .page_flip = drm_atomic_helper_page_flip, .set_property = drm_atomic_helper_crtc_set_property, .atomic_duplicate_state = nv50_head_atomic_duplicate_state, .atomic_destroy_state = nv50_head_atomic_destroy_state, -- 1.9.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] Allow ASYNC flip with atomic helpers. 2017-01-16 15:44 [PATCH 0/4] Allow ASYNC flip with atomic helpers Andrey Grodzovsky ` (2 preceding siblings ...) 2017-01-16 15:44 ` [PATCH 4/4] drm/nouveau/kms/nv50: Switch to using atomic helper " Andrey Grodzovsky @ 2017-01-16 20:39 ` Laurent Pinchart 2017-01-16 21:13 ` Harry Wentland 3 siblings, 1 reply; 27+ messages in thread From: Laurent Pinchart @ 2017-01-16 20:39 UTC (permalink / raw) To: dri-devel; +Cc: Alexander.Deucher, daniel.vetter, amd-gfx, nouveau Hi Andrey, Thank you for the patches. On Monday 16 Jan 2017 10:44:54 Andrey Grodzovsky wrote: > This series is a folow-up on > https://patchwork.kernel.org/patch/9501787/ > > The first patch makes changes to atomic helpers > to allow for drives with ASYNC flip support to use them. > Patches 2 and 3 are to use this in AMDGPU/DC and > patch 4 is possible cleanup in nouveau/kms who seems > to have the duplicate the helper as we did to support > ASYNC flips. I have my doubts regarding this. I'd much rather see userspace moving to the atomic API instead of extending support for legacy APIs. > Andrey Grodzovsky (4): > drm/atomic: Save flip flags in drm_plane_state > drm/amdgpu: Remove flip_flag from amdgpu_crtc > drm/amd/display: Switch to using atomic_helper for flip. > drm/nouveau/kms/nv50: Switch to using atomic helper for flip. > > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - > .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 ------------------- > drivers/gpu/drm/drm_atomic_helper.c | 10 +-- > drivers/gpu/drm/nouveau/nv50_display.c | 77 ++---------------- > include/drm/drm_plane.h | 8 ++ > 5 files changed, 22 insertions(+), 166 deletions(-) -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] Allow ASYNC flip with atomic helpers. 2017-01-16 20:39 ` [PATCH 0/4] Allow ASYNC flip with atomic helpers Laurent Pinchart @ 2017-01-16 21:13 ` Harry Wentland 2017-01-16 22:14 ` Laurent Pinchart 0 siblings, 1 reply; 27+ messages in thread From: Harry Wentland @ 2017-01-16 21:13 UTC (permalink / raw) To: Laurent Pinchart, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: Alexander.Deucher-5C7GfCeVMHo, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w, Michel Dänzer, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2017-01-16 03:39 PM, Laurent Pinchart wrote: > Hi Andrey, > > Thank you for the patches. > > On Monday 16 Jan 2017 10:44:54 Andrey Grodzovsky wrote: >> This series is a folow-up on >> https://patchwork.kernel.org/patch/9501787/ >> >> The first patch makes changes to atomic helpers >> to allow for drives with ASYNC flip support to use them. >> Patches 2 and 3 are to use this in AMDGPU/DC and >> patch 4 is possible cleanup in nouveau/kms who seems >> to have the duplicate the helper as we did to support >> ASYNC flips. > > I have my doubts regarding this. I'd much rather see userspace moving to the > atomic API instead of extending support for legacy APIs. > This change is not about introducing the async flag but cleaning up the legacy helpers to make sure drivers that currently use it through the legacy IOCTLs can benefit from the helpers and not have to roll their own. If the problem is with the pflip_flags, wouldn't drivers still need that after moving userspace to the atomic IOCTL? I don't disagree with you on having userspace move to atomic but I don't expect to see all userspace drivers move to atomic in the next couple months. Why not clean this up in the meantime? Harry >> Andrey Grodzovsky (4): >> drm/atomic: Save flip flags in drm_plane_state >> drm/amdgpu: Remove flip_flag from amdgpu_crtc >> drm/amd/display: Switch to using atomic_helper for flip. >> drm/nouveau/kms/nv50: Switch to using atomic helper for flip. >> >> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - >> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 ------------------- >> drivers/gpu/drm/drm_atomic_helper.c | 10 +-- >> drivers/gpu/drm/nouveau/nv50_display.c | 77 ++---------------- >> include/drm/drm_plane.h | 8 ++ >> 5 files changed, 22 insertions(+), 166 deletions(-) > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] Allow ASYNC flip with atomic helpers. 2017-01-16 21:13 ` Harry Wentland @ 2017-01-16 22:14 ` Laurent Pinchart 2017-01-23 8:53 ` Daniel Vetter 0 siblings, 1 reply; 27+ messages in thread From: Laurent Pinchart @ 2017-01-16 22:14 UTC (permalink / raw) To: dri-devel Cc: nouveau, Michel Dänzer, amd-gfx, Alexander.Deucher, daniel.vetter Hi Harry, On Monday 16 Jan 2017 16:13:39 Harry Wentland wrote: > On 2017-01-16 03:39 PM, Laurent Pinchart wrote: > > On Monday 16 Jan 2017 10:44:54 Andrey Grodzovsky wrote: > >> This series is a folow-up on > >> https://patchwork.kernel.org/patch/9501787/ > >> > >> The first patch makes changes to atomic helpers > >> to allow for drives with ASYNC flip support to use them. > >> Patches 2 and 3 are to use this in AMDGPU/DC and > >> patch 4 is possible cleanup in nouveau/kms who seems > >> to have the duplicate the helper as we did to support > >> ASYNC flips. > > > > I have my doubts regarding this. I'd much rather see userspace moving to > > the atomic API instead of extending support for legacy APIs. > > This change is not about introducing the async flag but cleaning up the > legacy helpers to make sure drivers that currently use it through the > legacy IOCTLs can benefit from the helpers and not have to roll their own. > > If the problem is with the pflip_flags, wouldn't drivers still need that > after moving userspace to the atomic IOCTL? > > I don't disagree with you on having userspace move to atomic but I don't > expect to see all userspace drivers move to atomic in the next couple > months. Why not clean this up in the meantime? If this patch series was just about moving common driver code into the core, sure, but it goes beyond that. Or, actually, it needs to go beyond that, but doesn't yet. Removing the DRM_MODE_PAGE_FLIP_ASYNC test in patch 1/4 means that the DRM core will not reject async page flips anymore, for any driver that uses the helper. You thus need to either patch all drivers that use the helper to reject the flag, or implement the feature in the drivers (and preferably in the helpers then). The current version of this patch series will make all existing users of the helpers accept async page flips without actually implementing them. > >> Andrey Grodzovsky (4): > >> drm/atomic: Save flip flags in drm_plane_state > >> drm/amdgpu: Remove flip_flag from amdgpu_crtc > >> drm/amd/display: Switch to using atomic_helper for flip. > >> drm/nouveau/kms/nv50: Switch to using atomic helper for flip. > >> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - > >> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c | 92 --------------- > >> drivers/gpu/drm/drm_atomic_helper.c | 10 +-- > >> drivers/gpu/drm/nouveau/nv50_display.c | 77 ++------------- > >> include/drm/drm_plane.h | 8 ++ > >> 5 files changed, 22 insertions(+), 166 deletions(-) -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] Allow ASYNC flip with atomic helpers. 2017-01-16 22:14 ` Laurent Pinchart @ 2017-01-23 8:53 ` Daniel Vetter 0 siblings, 0 replies; 27+ messages in thread From: Daniel Vetter @ 2017-01-23 8:53 UTC (permalink / raw) To: Laurent Pinchart Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Michel Dänzer, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Alexander.Deucher-5C7GfCeVMHo, daniel.vetter-ral2JQCrhuEAvxtiuMwx3w On Tue, Jan 17, 2017 at 12:14:24AM +0200, Laurent Pinchart wrote: > Hi Harry, > > On Monday 16 Jan 2017 16:13:39 Harry Wentland wrote: > > On 2017-01-16 03:39 PM, Laurent Pinchart wrote: > > > On Monday 16 Jan 2017 10:44:54 Andrey Grodzovsky wrote: > > >> This series is a folow-up on > > >> https://patchwork.kernel.org/patch/9501787/ > > >> > > >> The first patch makes changes to atomic helpers > > >> to allow for drives with ASYNC flip support to use them. > > >> Patches 2 and 3 are to use this in AMDGPU/DC and > > >> patch 4 is possible cleanup in nouveau/kms who seems > > >> to have the duplicate the helper as we did to support > > >> ASYNC flips. > > > > > > I have my doubts regarding this. I'd much rather see userspace moving to > > > the atomic API instead of extending support for legacy APIs. > > > > This change is not about introducing the async flag but cleaning up the > > legacy helpers to make sure drivers that currently use it through the > > legacy IOCTLs can benefit from the helpers and not have to roll their own. > > > > If the problem is with the pflip_flags, wouldn't drivers still need that > > after moving userspace to the atomic IOCTL? > > > > I don't disagree with you on having userspace move to atomic but I don't > > expect to see all userspace drivers move to atomic in the next couple > > months. Why not clean this up in the meantime? > > If this patch series was just about moving common driver code into the core, > sure, but it goes beyond that. Or, actually, it needs to go beyond that, but > doesn't yet. Removing the DRM_MODE_PAGE_FLIP_ASYNC test in patch 1/4 means > that the DRM core will not reject async page flips anymore, for any driver > that uses the helper. You thus need to either patch all drivers that use the > helper to reject the flag, or implement the feature in the drivers (and > preferably in the helpers then). The current version of this patch series will > make all existing users of the helpers accept async page flips without > actually implementing them. "How do we check for this driver feature" was one of the opens I raised here on irc, even before Andrey started typing :-) Definitely needs to be addressed. What's imo optional is exposing async flips through the atomic ioctl, since atm no atomic userspace seems to be in need of async flips yet. And the semantics of multi-plane async flips are tricky to define properly, so definitely want that userspace validation. That's why I think just doing the compat stuff is ok. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2017-01-26 9:40 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-16 15:44 [PATCH 0/4] Allow ASYNC flip with atomic helpers Andrey Grodzovsky 2017-01-16 15:44 ` [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state Andrey Grodzovsky [not found] ` <1484581498-32309-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> 2017-01-16 20:21 ` Gustavo Padovan 2017-01-16 20:53 ` Grodzovsky, Andrey 2017-01-23 8:54 ` Daniel Vetter 2017-01-23 19:48 ` Cheng, Tony [not found] ` <MWHPR12MB14066404FBCEF3C03EF24BF998720-Gy0DoCVfaSXS1+HfpYZxvgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 2017-01-26 5:01 ` Grodzovsky, Andrey [not found] ` <DM5PR12MB1147AEF3F8CFFAD4B69E114EEA770-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 2017-01-26 9:40 ` Daniel Vetter 2017-01-26 9:39 ` Daniel Vetter 2017-01-16 22:18 ` Laurent Pinchart 2017-01-17 4:03 ` Grodzovsky, Andrey [not found] ` <DM5PR12MB1147FF9E484370F1572CB9C3EA7C0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 2017-01-17 9:03 ` Laurent Pinchart 2017-01-17 8:14 ` Ville Syrjälä [not found] ` <1484581498-32309-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org> 2017-01-16 15:44 ` [PATCH 2/4] drm/amdgpu: Remove flip_flag from amdgpu_crtc Andrey Grodzovsky 2017-01-16 22:15 ` Laurent Pinchart 2017-01-16 15:44 ` [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip Andrey Grodzovsky 2017-01-16 22:16 ` Laurent Pinchart 2017-01-18 1:50 ` Michel Dänzer 2017-01-18 12:02 ` Laurent Pinchart 2017-01-18 22:18 ` Grodzovsky, Andrey [not found] ` <DM5PR12MB1147CFCD90203061E0F2EB95EA7F0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org> 2017-01-19 2:14 ` Michel Dänzer 2017-01-19 4:57 ` [PATCH v3 2/3] " Andrey Grodzovsky 2017-01-16 15:44 ` [PATCH 4/4] drm/nouveau/kms/nv50: Switch to using atomic helper " Andrey Grodzovsky 2017-01-16 20:39 ` [PATCH 0/4] Allow ASYNC flip with atomic helpers Laurent Pinchart 2017-01-16 21:13 ` Harry Wentland 2017-01-16 22:14 ` Laurent Pinchart 2017-01-23 8:53 ` Daniel Vetter
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.