Re: [PATCH v4 libdrm 2/2] Add drmModeGetFB2

[Eric Engestrom <eric.engestrom@intel.com>]

On Wednesday, 2020-02-05 23:10:21 +0000, Li, Juston wrote:
> On Wed, 2020-02-05 at 22:25 +0000, Eric Engestrom wrote:
> > On Friday, 2020-01-31 13:41:09 -0800, Juston Li wrote:
> > > From: Daniel Stone <daniels@collabora.com>
> > >
> > > Add a wrapper around the getfb2 ioctl, which returns extended
> > > framebuffer information mirroring addfb2, including multiple planes
> > > and
> > > modifiers.
> > >
> > > Changes since v3:
> > >  - remove unnecessary null check in drmModeFreeFB2 (Daniel Stone)
> > >
> > > Changes since v2:
> > >  - getfb2 ioctl has been merged upstream
> > >  - sync include/drm/drm.h in a seperate patch
> > >
> > > Changes since v1:
> > >  - functions should be drm_public
> > >  - modifier should be 64 bits
> > >  - update ioctl number
> > >
> > > Signed-off-by: Juston Li <juston.li@intel.com>
> > > Signed-off-by: Daniel Stone <daniels@collabora.com>
> > > ---
> > >  xf86drmMode.c | 36 ++++++++++++++++++++++++++++++++++++
> > >  xf86drmMode.h | 15 +++++++++++++++
> > >  2 files changed, 51 insertions(+)
> > >
> > > diff --git a/xf86drmMode.c b/xf86drmMode.c
> > > index 0cf7992c6e9a..94dc8ce38a5e 100644
> > > --- a/xf86drmMode.c
> > > +++ b/xf86drmMode.c
> > > @@ -1594,3 +1594,39 @@ drmModeRevokeLease(int fd, uint32_t
> > > lessee_id)
> > >  return 0;
> > >  return -errno;
> > >  }
> > > +
> > > +drm_public drmModeFB2Ptr
> > > +drmModeGetFB2(int fd, uint32_t fb_id)
> > > +{
> > > +struct drm_mode_fb_cmd2 get;
> > > +drmModeFB2Ptr ret;
> > > +int err;
> > > +
> > > +memclear(get);
> > > +get.fb_id = fb_id;
> >
> > As mentioned on IRC, could you write it like this instead?
> >
> > struct drm_mode_fb_cmd2 get = {
> > .fb_id = fb_id,
> > };
> >
> > With that, consider this patch
> > Reviewed-by: Eric Engestrom <eric@engestrom.ch>
> 
> Opps I sent v5 before seeing this but my code style differs and is
> probably incorrect :) I'll send v6 with the style corrected.
> 
> Thanks for reviewing!

Ah, sorry about that, our emails crossed paths.

As for the other patch (I mean 1/2), did you follow the instructions in
include/drm/README, specifically the section titled "When and how to
update these files" ?
Your commit message makes it look like you just applied that one change
instead of syncing with `make headers_install`.

Cheers,
  Eric
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel