[2/2] drm/doc: Document expectation that userspace review looks at kernel uAPI.
diff mbox series

Message ID 20190424185617.16865-2-eric@anholt.net
State New
Headers show
Series
  • [1/2] drm/doc: Allow new UAPI to be used once it's in the driver's -next.
Related show

Commit Message

Eric Anholt April 24, 2019, 6:56 p.m. UTC
The point of this review process is that userspace using the new uAPI
can actually live with the uAPI being provided, and it's hard to know
that without having actually looked into a kernel patch yourself.

Signed-off-by: Eric Anholt <eric@anholt.net>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/gpu/drm-uapi.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Daniel Vetter April 24, 2019, 7:36 p.m. UTC | #1
On Wed, Apr 24, 2019 at 11:56:17AM -0700, Eric Anholt wrote:
> The point of this review process is that userspace using the new uAPI
> can actually live with the uAPI being provided, and it's hard to know
> that without having actually looked into a kernel patch yourself.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/gpu/drm-uapi.rst | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 8e5545dfbf82..298424b98d99 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -85,7 +85,9 @@ leads to a few additional requirements:
>  - The userspace side must be fully reviewed and tested to the standards of that
>    userspace project. For e.g. mesa this means piglit testcases and review on the
>    mailing list. This is again to ensure that the new interface actually gets the
> -  job done.
> +  job done.  The userspace-side reviewer should also provide at least an
> +  Acked-by on the kernel uAPI patch indicating that they've looked at how the
> +  kernel side is implementing the new feature being used.

Answers a question that just recently came up on merging new kms
properties.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  
>  - The userspace patches must be against the canonical upstream, not some vendor
>    fork. This is to make sure that no one cheats on the review and testing
> -- 
> 2.20.1
>
Pekka Paalanen May 21, 2019, 7:47 a.m. UTC | #2
On Wed, 24 Apr 2019 21:36:36 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Apr 24, 2019 at 11:56:17AM -0700, Eric Anholt wrote:
> > The point of this review process is that userspace using the new uAPI
> > can actually live with the uAPI being provided, and it's hard to know
> > that without having actually looked into a kernel patch yourself.
> > 
> > Signed-off-by: Eric Anholt <eric@anholt.net>
> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  Documentation/gpu/drm-uapi.rst | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> > index 8e5545dfbf82..298424b98d99 100644
> > --- a/Documentation/gpu/drm-uapi.rst
> > +++ b/Documentation/gpu/drm-uapi.rst
> > @@ -85,7 +85,9 @@ leads to a few additional requirements:
> >  - The userspace side must be fully reviewed and tested to the standards of that
> >    userspace project. For e.g. mesa this means piglit testcases and review on the
> >    mailing list. This is again to ensure that the new interface actually gets the
> > -  job done.
> > +  job done.  The userspace-side reviewer should also provide at least an
> > +  Acked-by on the kernel uAPI patch indicating that they've looked at how the
> > +  kernel side is implementing the new feature being used.  
> 
> Answers a question that just recently came up on merging new kms
> properties.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Hi,

for the record, I personally will not be able to provide such Acked-by
tag according to kernel review rules, because I am completely unfamiliar
with kernel DRM internals and cannot review kernel code at all. This
might make people expecting Weston to prove their uAPI disappointed,
since there are very few Weston reviewers available.

If you meant something else, please word it to that you actually meant.


Thanks,
pq
Daniel Vetter May 21, 2019, 8:26 a.m. UTC | #3
On Tue, May 21, 2019 at 10:47:34AM +0300, Pekka Paalanen wrote:
> On Wed, 24 Apr 2019 21:36:36 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Wed, Apr 24, 2019 at 11:56:17AM -0700, Eric Anholt wrote:
> > > The point of this review process is that userspace using the new uAPI
> > > can actually live with the uAPI being provided, and it's hard to know
> > > that without having actually looked into a kernel patch yourself.
> > > 
> > > Signed-off-by: Eric Anholt <eric@anholt.net>
> > > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > >  Documentation/gpu/drm-uapi.rst | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> > > index 8e5545dfbf82..298424b98d99 100644
> > > --- a/Documentation/gpu/drm-uapi.rst
> > > +++ b/Documentation/gpu/drm-uapi.rst
> > > @@ -85,7 +85,9 @@ leads to a few additional requirements:
> > >  - The userspace side must be fully reviewed and tested to the standards of that
> > >    userspace project. For e.g. mesa this means piglit testcases and review on the
> > >    mailing list. This is again to ensure that the new interface actually gets the
> > > -  job done.
> > > +  job done.  The userspace-side reviewer should also provide at least an
> > > +  Acked-by on the kernel uAPI patch indicating that they've looked at how the
> > > +  kernel side is implementing the new feature being used.  
> > 
> > Answers a question that just recently came up on merging new kms
> > properties.
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Hi,
> 
> for the record, I personally will not be able to provide such Acked-by
> tag according to kernel review rules, because I am completely unfamiliar
> with kernel DRM internals and cannot review kernel code at all. This
> might make people expecting Weston to prove their uAPI disappointed,
> since there are very few Weston reviewers available.
> 
> If you meant something else, please word it to that you actually meant.

Hm right, that wording is putting a bit too high a bar. We want the
userspace view point here, not force userspace people to review kernel
code. I'll try to clarify this a bit better.
-Daniel

Patch
diff mbox series

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 8e5545dfbf82..298424b98d99 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -85,7 +85,9 @@  leads to a few additional requirements:
 - The userspace side must be fully reviewed and tested to the standards of that
   userspace project. For e.g. mesa this means piglit testcases and review on the
   mailing list. This is again to ensure that the new interface actually gets the
-  job done.
+  job done.  The userspace-side reviewer should also provide at least an
+  Acked-by on the kernel uAPI patch indicating that they've looked at how the
+  kernel side is implementing the new feature being used.
 
 - The userspace patches must be against the canonical upstream, not some vendor
   fork. This is to make sure that no one cheats on the review and testing