Re: [RESEND PATCH v2] drm: Add getfb2 ioctl

[Daniele Castagna <dcastagna@google.com>]

On Mon, Oct 14, 2019 at 1:51 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Oct 14, 2019 at 6:21 PM Li, Juston <juston.li@intel.com> wrote:
> >
> > On Wed, 2019-10-09 at 17:50 +0200, Daniel Vetter wrote:
> > > 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>
> > >
> > > Looks all good from a very quick glance (kernel, libdrm, igt), but
> > > where's
> > > the userspace? Link to weston/drm_hwc/whatever MR good enough.
> > > -Daniel
> >
> > My use case is a screenshot utility in chromiuomos that breaks with y-
> > tiled ccs enabled.
> > Review linked is just WIP hack for now since it depends on this
> > merging:
> > https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1815146
>
> Adding Sean & Daniele to confirm this is real cros.
> -Daniel

Yes, this is useful for cros and not only for the screenshot utility.
It is also useful for the transition from the splash screen to Chrome
login without flashing the screen black.

Thank you!

>
> >
> > Thanks
> > Juston
> >
> > > > ---
> > > >  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]);
> > > > +           }
> > > > +
> > > > +           if (ret != 0)
> > > > +                   goto out;
> > > > +   }
> > > > +
> > > > +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
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel