| Message ID | 20241101-tonyk-async_flip-v9-1-681814efbfbe@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/atomic: Ease async flip restrictions | expand |
On Fri, Nov 01, 2024 at 03:23:47PM -0300, André Almeida wrote: > Currently, DRM atomic uAPI allows only primary planes to be flipped > asynchronously. However, each driver might be able to perform async > flips in other different plane types. To enable drivers to set their own > restrictions on which type of plane they can or cannot flip, use the > existing atomic_async_check() from struct drm_plane_helper_funcs to > enhance this flexibility, thus allowing different plane types to be able > to do async flips as well. > > In order to prevent regressions and such, we keep the current policy: we > skip the driver check for the primary plane, because it is always > allowed to do async flips on it. > > Signed-off-by: André Almeida <andrealmeid@igalia.com> > --- > Changes from v8: > - Rebased on top of 6.12-rc1 > --- > drivers/gpu/drm/drm_atomic_uapi.c | 39 +++++++++++++++++++++++++++++---------- > 1 file changed, 29 insertions(+), 10 deletions(-) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On Fri Nov 1, 2024 at 11:23 AM PDT, André Almeida wrote: > Currently, DRM atomic uAPI allows only primary planes to be flipped > asynchronously. However, each driver might be able to perform async > flips in other different plane types. To enable drivers to set their own > restrictions on which type of plane they can or cannot flip, use the > existing atomic_async_check() from struct drm_plane_helper_funcs to > enhance this flexibility, thus allowing different plane types to be able > to do async flips as well. > > In order to prevent regressions and such, we keep the current policy: we > skip the driver check for the primary plane, because it is always > allowed to do async flips on it. > > Signed-off-by: André Almeida <andrealmeid@igalia.com> Should I do a R-b too? The changes looked sound enough for me to feel like testing it as well. Tested Borderlands Game of the Year Enhanced on my RX 7700 XT at maximum settings at 1080p165, and the tearing support in labwc allowed it to reach over 700fps. No problems from the hardware cursor. Tested-by: Christopher Snowhill <chris@kode54.net> > --- > Changes from v8: > - Rebased on top of 6.12-rc1 > --- > drivers/gpu/drm/drm_atomic_uapi.c | 39 +++++++++++++++++++++++++++++---------- > 1 file changed, 29 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index 370dc676e3aa543c9827b50df20df78f02b738c9..a0120df4b63e6b3419b53eb3d3673882559501c6 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -27,8 +27,9 @@ > * Daniel Vetter <daniel.vetter@ffwll.ch> > */ > > -#include <drm/drm_atomic_uapi.h> > #include <drm/drm_atomic.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_atomic_uapi.h> > #include <drm/drm_framebuffer.h> > #include <drm/drm_print.h> > #include <drm/drm_drv.h> > @@ -1063,6 +1064,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state, > struct drm_plane *plane = obj_to_plane(obj); > struct drm_plane_state *plane_state; > struct drm_mode_config *config = &plane->dev->mode_config; > + const struct drm_plane_helper_funcs *plane_funcs = plane->helper_private; > > plane_state = drm_atomic_get_plane_state(state, plane); > if (IS_ERR(plane_state)) { > @@ -1070,15 +1072,32 @@ int drm_atomic_set_property(struct drm_atomic_state *state, > break; > } > > - if (async_flip && > - (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY || > - (prop != config->prop_fb_id && > - prop != config->prop_in_fence_fd && > - prop != config->prop_fb_damage_clips))) { > - ret = drm_atomic_plane_get_property(plane, plane_state, > - prop, &old_val); > - ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop); > - break; > + if (async_flip) { > + /* check if the prop does a nop change */ > + if ((plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) || > + (prop != config->prop_fb_id && > + prop != config->prop_in_fence_fd && > + prop != config->prop_fb_damage_clips)) { > + ret = drm_atomic_plane_get_property(plane, plane_state, > + prop, &old_val); > + ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop); > + break; > + } > + > + /* ask the driver if this non-primary plane is supported */ > + if (plane->type != DRM_PLANE_TYPE_PRIMARY) { > + ret = -EINVAL; > + > + if (plane_funcs && plane_funcs->atomic_async_check) > + ret = plane_funcs->atomic_async_check(plane, state); > + > + if (ret) { > + drm_dbg_atomic(prop->dev, > + "[PLANE:%d] does not support async flips\n", > + obj->id); > + break; > + } > + } > } > > ret = drm_atomic_plane_set_property(plane,
Hi Christopher, Em 03/11/2024 03:36, Christopher Snowhill escreveu: > On Fri Nov 1, 2024 at 11:23 AM PDT, André Almeida wrote: >> Currently, DRM atomic uAPI allows only primary planes to be flipped >> asynchronously. However, each driver might be able to perform async >> flips in other different plane types. To enable drivers to set their own >> restrictions on which type of plane they can or cannot flip, use the >> existing atomic_async_check() from struct drm_plane_helper_funcs to >> enhance this flexibility, thus allowing different plane types to be able >> to do async flips as well. >> >> In order to prevent regressions and such, we keep the current policy: we >> skip the driver check for the primary plane, because it is always >> allowed to do async flips on it. >> >> Signed-off-by: André Almeida <andrealmeid@igalia.com> > > Should I do a R-b too? If you can review the code, it's always really appreciated. > The changes looked sound enough for me to feel > like testing it as well. Tested Borderlands Game of the Year Enhanced on > my RX 7700 XT at maximum settings at 1080p165, and the tearing support in > labwc allowed it to reach over 700fps. No problems from the hardware > cursor. Thanks for testing and reporting! > > Tested-by: Christopher Snowhill <chris@kode54.net> >
On Mon Nov 4, 2024 at 12:52 PM PST, André Almeida wrote: > Hi Christopher, > > Em 03/11/2024 03:36, Christopher Snowhill escreveu: > > On Fri Nov 1, 2024 at 11:23 AM PDT, André Almeida wrote: > >> Currently, DRM atomic uAPI allows only primary planes to be flipped > >> asynchronously. However, each driver might be able to perform async > >> flips in other different plane types. To enable drivers to set their own > >> restrictions on which type of plane they can or cannot flip, use the > >> existing atomic_async_check() from struct drm_plane_helper_funcs to > >> enhance this flexibility, thus allowing different plane types to be able > >> to do async flips as well. > >> > >> In order to prevent regressions and such, we keep the current policy: we > >> skip the driver check for the primary plane, because it is always > >> allowed to do async flips on it. > >> > >> Signed-off-by: André Almeida <andrealmeid@igalia.com> > > > > Should I do a R-b too? > > If you can review the code, it's always really appreciated. I mean, I did review your changes, they looked good to me. I just didn't know the protocol for reporting review in addition to testing. > > > The changes looked sound enough for me to feel > > like testing it as well. Tested Borderlands Game of the Year Enhanced on > > my RX 7700 XT at maximum settings at 1080p165, and the tearing support in > > labwc allowed it to reach over 700fps. No problems from the hardware > > cursor. > > Thanks for testing and reporting! > > > > > Tested-by: Christopher Snowhill <chris@kode54.net> > >
On Tue, 5 Nov 2024 at 10:15, Christopher Snowhill <chris@kode54.net> wrote: > > On Mon Nov 4, 2024 at 12:52 PM PST, André Almeida wrote: > > Hi Christopher, > > > > Em 03/11/2024 03:36, Christopher Snowhill escreveu: > > > On Fri Nov 1, 2024 at 11:23 AM PDT, André Almeida wrote: > > >> Currently, DRM atomic uAPI allows only primary planes to be flipped > > >> asynchronously. However, each driver might be able to perform async > > >> flips in other different plane types. To enable drivers to set their own > > >> restrictions on which type of plane they can or cannot flip, use the > > >> existing atomic_async_check() from struct drm_plane_helper_funcs to > > >> enhance this flexibility, thus allowing different plane types to be able > > >> to do async flips as well. > > >> > > >> In order to prevent regressions and such, we keep the current policy: we > > >> skip the driver check for the primary plane, because it is always > > >> allowed to do async flips on it. > > >> > > >> Signed-off-by: André Almeida <andrealmeid@igalia.com> > > > > > > Should I do a R-b too? > > > > If you can review the code, it's always really appreciated. > > I mean, I did review your changes, they looked good to me. I just didn't > know the protocol for reporting review in addition to testing. Please respond with the R-B tag. Also ideally the Tested-by should contain the reference to a platform which was used to test it. > > > > > > The changes looked sound enough for me to feel > > > like testing it as well. Tested Borderlands Game of the Year Enhanced on > > > my RX 7700 XT at maximum settings at 1080p165, and the tearing support in > > > labwc allowed it to reach over 700fps. No problems from the hardware > > > cursor. > > > > Thanks for testing and reporting! > > > > > > > > Tested-by: Christopher Snowhill <chris@kode54.net> > > > >
On Tue Nov 5, 2024 at 2:51 AM PST, Dmitry Baryshkov wrote: > On Tue, 5 Nov 2024 at 10:15, Christopher Snowhill <chris@kode54.net> wrote: > > > > On Mon Nov 4, 2024 at 12:52 PM PST, André Almeida wrote: > > > Hi Christopher, > > > > > > Em 03/11/2024 03:36, Christopher Snowhill escreveu: > > > > On Fri Nov 1, 2024 at 11:23 AM PDT, André Almeida wrote: > > > >> Currently, DRM atomic uAPI allows only primary planes to be flipped > > > >> asynchronously. However, each driver might be able to perform async > > > >> flips in other different plane types. To enable drivers to set their own > > > >> restrictions on which type of plane they can or cannot flip, use the > > > >> existing atomic_async_check() from struct drm_plane_helper_funcs to > > > >> enhance this flexibility, thus allowing different plane types to be able > > > >> to do async flips as well. > > > >> > > > >> In order to prevent regressions and such, we keep the current policy: we > > > >> skip the driver check for the primary plane, because it is always > > > >> allowed to do async flips on it. > > > >> > > > >> Signed-off-by: André Almeida <andrealmeid@igalia.com> > > > > > > > > Should I do a R-b too? > > > > > > If you can review the code, it's always really appreciated. > > > > I mean, I did review your changes, they looked good to me. I just didn't > > know the protocol for reporting review in addition to testing. > > Please respond with the R-B tag. Also ideally the Tested-by should > contain the reference to a platform which was used to test it. > > > > > > > > > > The changes looked sound enough for me to feel > > > > like testing it as well. Tested Borderlands Game of the Year Enhanced on > > > > my RX 7700 XT at maximum settings at 1080p165, and the tearing support in > > > > labwc allowed it to reach over 700fps. No problems from the hardware > > > > cursor. > > > > > > Thanks for testing and reporting! > > > > > > > > > > > Tested-by: Christopher Snowhill <chris@kode54.net> > > > > > > Reviewed-by: Christopher Snowhill <chris@kode54.net> Tested-by: Christopher Snowhill <chris@kode54.net> on Radeon RX 7700 XT using labwc-git (I must admit, the documents do not make it clear what format the Testing-by tag should take, or how to append which hardware it was tested on, etc.)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 370dc676e3aa543c9827b50df20df78f02b738c9..a0120df4b63e6b3419b53eb3d3673882559501c6 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -27,8 +27,9 @@ * Daniel Vetter <daniel.vetter@ffwll.ch> */ -#include <drm/drm_atomic_uapi.h> #include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_atomic_uapi.h> #include <drm/drm_framebuffer.h> #include <drm/drm_print.h> #include <drm/drm_drv.h> @@ -1063,6 +1064,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state, struct drm_plane *plane = obj_to_plane(obj); struct drm_plane_state *plane_state; struct drm_mode_config *config = &plane->dev->mode_config; + const struct drm_plane_helper_funcs *plane_funcs = plane->helper_private; plane_state = drm_atomic_get_plane_state(state, plane); if (IS_ERR(plane_state)) { @@ -1070,15 +1072,32 @@ int drm_atomic_set_property(struct drm_atomic_state *state, break; } - if (async_flip && - (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY || - (prop != config->prop_fb_id && - prop != config->prop_in_fence_fd && - prop != config->prop_fb_damage_clips))) { - ret = drm_atomic_plane_get_property(plane, plane_state, - prop, &old_val); - ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop); - break; + if (async_flip) { + /* check if the prop does a nop change */ + if ((plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) || + (prop != config->prop_fb_id && + prop != config->prop_in_fence_fd && + prop != config->prop_fb_damage_clips)) { + ret = drm_atomic_plane_get_property(plane, plane_state, + prop, &old_val); + ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop); + break; + } + + /* ask the driver if this non-primary plane is supported */ + if (plane->type != DRM_PLANE_TYPE_PRIMARY) { + ret = -EINVAL; + + if (plane_funcs && plane_funcs->atomic_async_check) + ret = plane_funcs->atomic_async_check(plane, state); + + if (ret) { + drm_dbg_atomic(prop->dev, + "[PLANE:%d] does not support async flips\n", + obj->id); + break; + } + } } ret = drm_atomic_plane_set_property(plane,
Currently, DRM atomic uAPI allows only primary planes to be flipped asynchronously. However, each driver might be able to perform async flips in other different plane types. To enable drivers to set their own restrictions on which type of plane they can or cannot flip, use the existing atomic_async_check() from struct drm_plane_helper_funcs to enhance this flexibility, thus allowing different plane types to be able to do async flips as well. In order to prevent regressions and such, we keep the current policy: we skip the driver check for the primary plane, because it is always allowed to do async flips on it. Signed-off-by: André Almeida <andrealmeid@igalia.com> --- Changes from v8: - Rebased on top of 6.12-rc1 --- drivers/gpu/drm/drm_atomic_uapi.c | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-)