From: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Juston Li <juston.li@intel.com> Cc: intel-gfx@lists.freedesktop.org, Daniel Stone <daniels@collabora.com>, dri-devel@lists.freedesktop.org Subject: Re: [RESEND PATCH v2] drm: Add getfb2 ioctl Date: Fri, 13 Dec 2019 23:36:03 +0200 [thread overview] Message-ID: <20191213213603.GK1208@intel.com> (raw) In-Reply-To: <20191003183125.4520-1-juston.li@intel.com> On Thu, Oct 03, 2019 at 11:31:25AM -0700, Juston Li wrote: > From: Daniel Stone <daniels@collabora.com> > > getfb2 allows us to pass multiple planes and modifiers, just like addfb2 > over addfb. > > Changes since v1: > - unused modifiers set to 0 instead of DRM_FORMAT_MOD_INVALID > - update ioctl number > > Signed-off-by: Daniel Stone <daniels@collabora.com> > Signed-off-by: Juston Li <juston.li@intel.com> > --- > drivers/gpu/drm/drm_crtc_internal.h | 2 + > drivers/gpu/drm/drm_framebuffer.c | 110 ++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_ioctl.c | 1 + > include/uapi/drm/drm.h | 2 + > 4 files changed, 115 insertions(+) > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h > index c7d5e4c21423..16f2413403aa 100644 > --- a/drivers/gpu/drm/drm_crtc_internal.h > +++ b/drivers/gpu/drm/drm_crtc_internal.h > @@ -216,6 +216,8 @@ int drm_mode_rmfb_ioctl(struct drm_device *dev, > void *data, struct drm_file *file_priv); > int drm_mode_getfb(struct drm_device *dev, > void *data, struct drm_file *file_priv); > +int drm_mode_getfb2_ioctl(struct drm_device *dev, > + void *data, struct drm_file *file_priv); > int drm_mode_dirtyfb_ioctl(struct drm_device *dev, > void *data, struct drm_file *file_priv); > > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > index 57564318ceea..6db54f177443 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -31,6 +31,7 @@ > #include <drm/drm_file.h> > #include <drm/drm_fourcc.h> > #include <drm/drm_framebuffer.h> > +#include <drm/drm_gem.h> > #include <drm/drm_print.h> > #include <drm/drm_util.h> > > @@ -548,7 +549,116 @@ int drm_mode_getfb(struct drm_device *dev, > > out: > drm_framebuffer_put(fb); > + return ret; > +} > + > +/** > + * drm_mode_getfb2 - get extended FB info > + * @dev: drm device for the ioctl > + * @data: data pointer for the ioctl > + * @file_priv: drm file for the ioctl call > + * > + * Lookup the FB given its ID and return info about it. > + * > + * Called by the user via ioctl. > + * > + * Returns: > + * Zero on success, negative errno on failure. > + */ > +int drm_mode_getfb2_ioctl(struct drm_device *dev, > + void *data, struct drm_file *file_priv) > +{ > + struct drm_mode_fb_cmd2 *r = data; > + struct drm_framebuffer *fb; > + unsigned int i; > + int ret; > + > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return -EINVAL; > + > + fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id); > + if (!fb) > + return -ENOENT; > + > + /* For multi-plane framebuffers, we require the driver to place the > + * GEM objects directly in the drm_framebuffer. For single-plane > + * framebuffers, we can fall back to create_handle. > + */ > + if (!fb->obj[0] && > + (fb->format->num_planes > 1 || !fb->funcs->create_handle)) { > + ret = -ENODEV; > + goto out; > + } > + > + r->height = fb->height; > + r->width = fb->width; > + r->pixel_format = fb->format->format; > + > + r->flags = 0; > + if (dev->mode_config.allow_fb_modifiers) > + r->flags |= DRM_MODE_FB_MODIFIERS; > + > + for (i = 0; i < ARRAY_SIZE(r->handles); i++) { > + r->handles[i] = 0; > + r->pitches[i] = 0; > + r->offsets[i] = 0; > + r->modifier[i] = 0; > + } > > + for (i = 0; i < fb->format->num_planes; i++) { > + int j; > + > + r->pitches[i] = fb->pitches[i]; > + r->offsets[i] = fb->offsets[i]; > + if (dev->mode_config.allow_fb_modifiers) > + r->modifier[i] = fb->modifier; > + > + /* If we reuse the same object for multiple planes, also > + * return the same handle. > + */ > + for (j = 0; j < i; j++) { > + if (fb->obj[i] == fb->obj[j]) { > + r->handles[i] = r->handles[j]; > + break; > + } > + } > + > + if (r->handles[i]) > + continue; > + > + if (fb->obj[i]) { > + ret = drm_gem_handle_create(file_priv, fb->obj[i], > + &r->handles[i]); > + } else { > + WARN_ON(i > 0); > + ret = fb->funcs->create_handle(fb, file_priv, > + &r->handles[i]); > + } getfb1 doesn't allow non-master/root to see the handles. Here we don't seem to have that same protection? > + > + if (ret != 0) > + goto out; Could be just 'break;' and then we wouldn't even need the label. Rest lgtm. > + } > + > +out: > + if (ret != 0) { > + /* Delete any previously-created handles on failure. */ > + for (i = 0; i < ARRAY_SIZE(r->handles); i++) { > + int j; > + > + if (r->handles[i]) > + drm_gem_handle_delete(file_priv, r->handles[i]); > + > + /* Zero out any handles identical to the one we just > + * deleted. > + */ > + for (j = i + 1; j < ARRAY_SIZE(r->handles); j++) { > + if (r->handles[j] == r->handles[i]) > + r->handles[j] = 0; > + } > + } > + } > + > + drm_framebuffer_put(fb); > return ret; > } > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index fcd728d7cf72..b1fafce3ad8c 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -671,6 +671,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPROPERTY, drm_connector_property_set_ioctl, DRM_MASTER), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, 0), > + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB2, drm_mode_getfb2_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB, drm_mode_addfb_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb_ioctl, 0), > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > index 8a5b2f8f8eb9..021f33675ba2 100644 > --- a/include/uapi/drm/drm.h > +++ b/include/uapi/drm/drm.h > @@ -947,6 +947,8 @@ extern "C" { > #define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct drm_syncobj_transfer) > #define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, struct drm_syncobj_timeline_array) > > +#define DRM_IOCTL_MODE_GETFB2 DRM_IOWR(0xCE, struct drm_mode_fb_cmd2) > + > /** > * Device specific ioctls should only be in their respective headers > * The device specific ioctl range is from 0x40 to 0x9f. > -- > 2.21.0 -- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Juston Li <juston.li@intel.com> Cc: intel-gfx@lists.freedesktop.org, Daniel Stone <daniels@collabora.com>, dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [RESEND PATCH v2] drm: Add getfb2 ioctl Date: Fri, 13 Dec 2019 23:36:03 +0200 [thread overview] Message-ID: <20191213213603.GK1208@intel.com> (raw) In-Reply-To: <20191003183125.4520-1-juston.li@intel.com> On Thu, Oct 03, 2019 at 11:31:25AM -0700, Juston Li wrote: > From: Daniel Stone <daniels@collabora.com> > > getfb2 allows us to pass multiple planes and modifiers, just like addfb2 > over addfb. > > Changes since v1: > - unused modifiers set to 0 instead of DRM_FORMAT_MOD_INVALID > - update ioctl number > > Signed-off-by: Daniel Stone <daniels@collabora.com> > Signed-off-by: Juston Li <juston.li@intel.com> > --- > drivers/gpu/drm/drm_crtc_internal.h | 2 + > drivers/gpu/drm/drm_framebuffer.c | 110 ++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_ioctl.c | 1 + > include/uapi/drm/drm.h | 2 + > 4 files changed, 115 insertions(+) > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h > index c7d5e4c21423..16f2413403aa 100644 > --- a/drivers/gpu/drm/drm_crtc_internal.h > +++ b/drivers/gpu/drm/drm_crtc_internal.h > @@ -216,6 +216,8 @@ int drm_mode_rmfb_ioctl(struct drm_device *dev, > void *data, struct drm_file *file_priv); > int drm_mode_getfb(struct drm_device *dev, > void *data, struct drm_file *file_priv); > +int drm_mode_getfb2_ioctl(struct drm_device *dev, > + void *data, struct drm_file *file_priv); > int drm_mode_dirtyfb_ioctl(struct drm_device *dev, > void *data, struct drm_file *file_priv); > > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > index 57564318ceea..6db54f177443 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -31,6 +31,7 @@ > #include <drm/drm_file.h> > #include <drm/drm_fourcc.h> > #include <drm/drm_framebuffer.h> > +#include <drm/drm_gem.h> > #include <drm/drm_print.h> > #include <drm/drm_util.h> > > @@ -548,7 +549,116 @@ int drm_mode_getfb(struct drm_device *dev, > > out: > drm_framebuffer_put(fb); > + return ret; > +} > + > +/** > + * drm_mode_getfb2 - get extended FB info > + * @dev: drm device for the ioctl > + * @data: data pointer for the ioctl > + * @file_priv: drm file for the ioctl call > + * > + * Lookup the FB given its ID and return info about it. > + * > + * Called by the user via ioctl. > + * > + * Returns: > + * Zero on success, negative errno on failure. > + */ > +int drm_mode_getfb2_ioctl(struct drm_device *dev, > + void *data, struct drm_file *file_priv) > +{ > + struct drm_mode_fb_cmd2 *r = data; > + struct drm_framebuffer *fb; > + unsigned int i; > + int ret; > + > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return -EINVAL; > + > + fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id); > + if (!fb) > + return -ENOENT; > + > + /* For multi-plane framebuffers, we require the driver to place the > + * GEM objects directly in the drm_framebuffer. For single-plane > + * framebuffers, we can fall back to create_handle. > + */ > + if (!fb->obj[0] && > + (fb->format->num_planes > 1 || !fb->funcs->create_handle)) { > + ret = -ENODEV; > + goto out; > + } > + > + r->height = fb->height; > + r->width = fb->width; > + r->pixel_format = fb->format->format; > + > + r->flags = 0; > + if (dev->mode_config.allow_fb_modifiers) > + r->flags |= DRM_MODE_FB_MODIFIERS; > + > + for (i = 0; i < ARRAY_SIZE(r->handles); i++) { > + r->handles[i] = 0; > + r->pitches[i] = 0; > + r->offsets[i] = 0; > + r->modifier[i] = 0; > + } > > + for (i = 0; i < fb->format->num_planes; i++) { > + int j; > + > + r->pitches[i] = fb->pitches[i]; > + r->offsets[i] = fb->offsets[i]; > + if (dev->mode_config.allow_fb_modifiers) > + r->modifier[i] = fb->modifier; > + > + /* If we reuse the same object for multiple planes, also > + * return the same handle. > + */ > + for (j = 0; j < i; j++) { > + if (fb->obj[i] == fb->obj[j]) { > + r->handles[i] = r->handles[j]; > + break; > + } > + } > + > + if (r->handles[i]) > + continue; > + > + if (fb->obj[i]) { > + ret = drm_gem_handle_create(file_priv, fb->obj[i], > + &r->handles[i]); > + } else { > + WARN_ON(i > 0); > + ret = fb->funcs->create_handle(fb, file_priv, > + &r->handles[i]); > + } getfb1 doesn't allow non-master/root to see the handles. Here we don't seem to have that same protection? > + > + if (ret != 0) > + goto out; Could be just 'break;' and then we wouldn't even need the label. Rest lgtm. > + } > + > +out: > + if (ret != 0) { > + /* Delete any previously-created handles on failure. */ > + for (i = 0; i < ARRAY_SIZE(r->handles); i++) { > + int j; > + > + if (r->handles[i]) > + drm_gem_handle_delete(file_priv, r->handles[i]); > + > + /* Zero out any handles identical to the one we just > + * deleted. > + */ > + for (j = i + 1; j < ARRAY_SIZE(r->handles); j++) { > + if (r->handles[j] == r->handles[i]) > + r->handles[j] = 0; > + } > + } > + } > + > + drm_framebuffer_put(fb); > return ret; > } > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index fcd728d7cf72..b1fafce3ad8c 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -671,6 +671,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPROPERTY, drm_connector_property_set_ioctl, DRM_MASTER), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, 0), > + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB2, drm_mode_getfb2_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB, drm_mode_addfb_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb_ioctl, 0), > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > index 8a5b2f8f8eb9..021f33675ba2 100644 > --- a/include/uapi/drm/drm.h > +++ b/include/uapi/drm/drm.h > @@ -947,6 +947,8 @@ extern "C" { > #define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct drm_syncobj_transfer) > #define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, struct drm_syncobj_timeline_array) > > +#define DRM_IOCTL_MODE_GETFB2 DRM_IOWR(0xCE, struct drm_mode_fb_cmd2) > + > /** > * Device specific ioctls should only be in their respective headers > * The device specific ioctl range is from 0x40 to 0x9f. > -- > 2.21.0 -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-12-13 21:36 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-03 18:31 [RESEND PATCH v2] drm: Add getfb2 ioctl Juston Li 2019-10-03 21:27 ` ✓ Fi.CI.BAT: success for " Patchwork 2019-10-04 11:20 ` ✗ Fi.CI.IGT: failure " Patchwork 2019-10-09 15:50 ` [RESEND PATCH v2] " Daniel Vetter 2019-10-14 16:21 ` Li, Juston 2019-10-14 17:51 ` Daniel Vetter 2019-10-14 18:30 ` Li, Juston 2019-10-15 15:12 ` Daniele Castagna 2019-11-21 8:26 ` Timo Aaltonen 2019-11-21 8:26 ` [Intel-gfx] " Timo Aaltonen 2019-11-21 8:26 ` Timo Aaltonen 2019-11-21 9:28 ` Daniel Vetter 2019-11-21 9:28 ` [Intel-gfx] " Daniel Vetter 2019-11-22 1:07 ` Li, Juston 2019-11-22 1:07 ` [Intel-gfx] " Li, Juston 2019-11-22 1:07 ` Li, Juston 2019-12-13 21:36 ` Ville Syrjälä [this message] 2019-12-13 21:36 ` [Intel-gfx] " Ville Syrjälä 2019-12-13 22:52 ` Li, Juston 2019-12-13 22:52 ` [Intel-gfx] " Li, Juston 2019-12-14 0:01 ` Daniel Vetter 2019-12-14 0:01 ` Daniel Vetter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191213213603.GK1208@intel.com \ --to=ville.syrjala@linux.intel.com \ --cc=daniels@collabora.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=juston.li@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.