| Message ID | 20200131214109.6323-2-juston.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,libdrm,1/2] include/drm: sync up drm.h to 3ff4c24bdb1f | expand |
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> > + > + err = DRM_IOCTL(fd, DRM_IOCTL_MODE_GETFB2, &get); > + if (err != 0) > + return NULL; > + > + ret = drmMalloc(sizeof(drmModeFB2)); > + if (!ret) > + return NULL; > + > + ret->fb_id = fb_id; > + ret->width = get.width; > + ret->height = get.height; > + ret->pixel_format = get.pixel_format; > + ret->flags = get.flags; > + ret->modifier = get.modifier[0]; > + memcpy(ret->handles, get.handles, sizeof(uint32_t) * 4); > + memcpy(ret->pitches, get.pitches, sizeof(uint32_t) * 4); > + memcpy(ret->offsets, get.offsets, sizeof(uint32_t) * 4); > + > + return ret; > +} > + > +drm_public void drmModeFreeFB2(drmModeFB2Ptr ptr) > +{ > + drmFree(ptr); > +} > diff --git a/xf86drmMode.h b/xf86drmMode.h > index 159a39937240..fc0bbd8dcb67 100644 > --- a/xf86drmMode.h > +++ b/xf86drmMode.h > @@ -225,6 +225,19 @@ typedef struct _drmModeFB { > uint32_t handle; > } drmModeFB, *drmModeFBPtr; > > +typedef struct _drmModeFB2 { > + uint32_t fb_id; > + uint32_t width, height; > + uint32_t pixel_format; /* fourcc code from drm_fourcc.h */ > + uint64_t modifier; /* applies to all buffers */ > + uint32_t flags; > + > + /* per-plane GEM handle; may be duplicate entries for multiple planes */ > + uint32_t handles[4]; > + uint32_t pitches[4]; /* bytes */ > + uint32_t offsets[4]; /* bytes */ > +} drmModeFB2, *drmModeFB2Ptr; > + > typedef struct drm_clip_rect drmModeClip, *drmModeClipPtr; > > typedef struct _drmModePropertyBlob { > @@ -343,6 +356,7 @@ typedef struct _drmModePlaneRes { > extern void drmModeFreeModeInfo( drmModeModeInfoPtr ptr ); > extern void drmModeFreeResources( drmModeResPtr ptr ); > extern void drmModeFreeFB( drmModeFBPtr ptr ); > +extern void drmModeFreeFB2( drmModeFB2Ptr ptr ); > extern void drmModeFreeCrtc( drmModeCrtcPtr ptr ); > extern void drmModeFreeConnector( drmModeConnectorPtr ptr ); > extern void drmModeFreeEncoder( drmModeEncoderPtr ptr ); > @@ -362,6 +376,7 @@ extern drmModeResPtr drmModeGetResources(int fd); > * Retrieve information about framebuffer bufferId > */ > extern drmModeFBPtr drmModeGetFB(int fd, uint32_t bufferId); > +extern drmModeFB2Ptr drmModeGetFB2(int fd, uint32_t bufferId); > > /** > * Creates a new framebuffer with an buffer object as its scanout buffer. > -- > 2.21.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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!
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
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. Also, only drm.h was synced, is that preferred or would it be better to sync the entire header directory? Thanks Juston
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?
Hi, On Tue, 11 Feb 2020 at 23:21, Eric Engestrom <eric.engestrom@intel.com> wrote: > On Thursday, 2020-02-06 17:46:36 +0000, Li, Juston wrote: > > 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? I've merged this now, thankyou very much for pushing this forward Juston! Cheers, Daniel
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; + + err = DRM_IOCTL(fd, DRM_IOCTL_MODE_GETFB2, &get); + if (err != 0) + return NULL; + + ret = drmMalloc(sizeof(drmModeFB2)); + if (!ret) + return NULL; + + ret->fb_id = fb_id; + ret->width = get.width; + ret->height = get.height; + ret->pixel_format = get.pixel_format; + ret->flags = get.flags; + ret->modifier = get.modifier[0]; + memcpy(ret->handles, get.handles, sizeof(uint32_t) * 4); + memcpy(ret->pitches, get.pitches, sizeof(uint32_t) * 4); + memcpy(ret->offsets, get.offsets, sizeof(uint32_t) * 4); + + return ret; +} + +drm_public void drmModeFreeFB2(drmModeFB2Ptr ptr) +{ + drmFree(ptr); +} diff --git a/xf86drmMode.h b/xf86drmMode.h index 159a39937240..fc0bbd8dcb67 100644 --- a/xf86drmMode.h +++ b/xf86drmMode.h @@ -225,6 +225,19 @@ typedef struct _drmModeFB { uint32_t handle; } drmModeFB, *drmModeFBPtr; +typedef struct _drmModeFB2 { + uint32_t fb_id; + uint32_t width, height; + uint32_t pixel_format; /* fourcc code from drm_fourcc.h */ + uint64_t modifier; /* applies to all buffers */ + uint32_t flags; + + /* per-plane GEM handle; may be duplicate entries for multiple planes */ + uint32_t handles[4]; + uint32_t pitches[4]; /* bytes */ + uint32_t offsets[4]; /* bytes */ +} drmModeFB2, *drmModeFB2Ptr; + typedef struct drm_clip_rect drmModeClip, *drmModeClipPtr; typedef struct _drmModePropertyBlob { @@ -343,6 +356,7 @@ typedef struct _drmModePlaneRes { extern void drmModeFreeModeInfo( drmModeModeInfoPtr ptr ); extern void drmModeFreeResources( drmModeResPtr ptr ); extern void drmModeFreeFB( drmModeFBPtr ptr ); +extern void drmModeFreeFB2( drmModeFB2Ptr ptr ); extern void drmModeFreeCrtc( drmModeCrtcPtr ptr ); extern void drmModeFreeConnector( drmModeConnectorPtr ptr ); extern void drmModeFreeEncoder( drmModeEncoderPtr ptr ); @@ -362,6 +376,7 @@ extern drmModeResPtr drmModeGetResources(int fd); * Retrieve information about framebuffer bufferId */ extern drmModeFBPtr drmModeGetFB(int fd, uint32_t bufferId); +extern drmModeFB2Ptr drmModeGetFB2(int fd, uint32_t bufferId); /** * Creates a new framebuffer with an buffer object as its scanout buffer.