Re: [PATCH v3 0/6] Add support for atomic async page-flips

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 30, 2022 at 05:19:07PM +0300, Ville Syrjälä wrote:
> On Fri, Sep 30, 2022 at 04:52:56PM +0300, Ville Syrjälä wrote:
> > On Thu, Sep 29, 2022 at 06:43:15PM +0000, Simon Ser wrote:
> > > This series adds support for DRM_MODE_PAGE_FLIP_ASYNC for atomic
> > > commits, aka. "immediate flip" (which might result in tearing).
> > > The feature was only available via the legacy uAPI, however for
> > > gaming use-cases it may be desirable to enable it via the atomic
> > > uAPI too.
> > > 
> > > - Patchwork: https://patchwork.freedesktop.org/series/107683/
> > > - User-space patch: https://github.com/Plagman/gamescope/pull/595
> > > - IGT patch: https://patchwork.freedesktop.org/series/107681/
> > 
> > So no tests that actually verify that the kernel properly rejects
> > stuff stuff like modesets, gamma LUT updates, plane movement,
> > etc.?
> 
> Pondering this a bit more, it just occurred to me the current driver
> level checks might easily lead to confusing behaviour. Eg. is
> the ioctl going to succeed if you ask for an async change of some
> random property while the crtc disabled, but fails if you ask for
> the same async property change when the crtc is active?
> 
> So another reason why rejecting most properties already at
> the uapi level might be a good idea.

And just to be clear this is pretty much what I suggest:

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 79730fa1dd8e..471a2c703847 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1392,6 +1392,13 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 				goto out;
 			}
 
+			if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC &&
+			    prop != dev->mode_config.prop_fb_id) {
+				drm_mode_object_put(obj);
+				ret = -EINVAL;
+				goto out;
+			}
+
 			if (copy_from_user(&prop_value,
 					   prop_values_ptr + copied_props,
 					   sizeof(prop_value))) {


That would actively discourage people from even attempting the
"just dump all the state into the ioctl" approach with async flips
since even the props whose value isn't even changing would be rejected.

-- 
Ville Syrjälä
Intel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux