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

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

On Thursday, 2020-02-06 17:46:36 +0000, Li, Juston wrote:
> On Wed, 2020-02-05 at 23:27 +0000, Eric Engestrom wrote:
> > 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
> 
> Yes, drm.h was copied from 'make headers_install' from drm-misc-next.
> It had been updated fairly recently so GETFB2 is the only delta.
> 
> Sorry, I didn't see the README so the commit message isn't exactly as
> requested.

I kind of expected that this was the case :)

All good then, with the commit message adjusted to make this clear,
patch 1 is:
Acked-by: Eric Engestrom <eric@engestrom.ch>

> 
> 
> Also, only drm.h was synced, is that preferred or would it be better to
> sync the entire header directory?

Usually people either update their driver's header and core, or just core,
so this is fine :)

I assume DanielS will merge this for you?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel