All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-10-03 18:31 Juston Li
  2019-10-03 21:27 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: Juston Li @ 2019-10-03 18:31 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: Daniel Stone

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]);
+		}
+
+		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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* ✓ Fi.CI.BAT: success for drm: Add getfb2 ioctl
  2019-10-03 18:31 [RESEND PATCH v2] drm: Add getfb2 ioctl Juston Li
@ 2019-10-03 21:27 ` Patchwork
  2019-10-04 11:20 ` ✗ Fi.CI.IGT: failure " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-03 21:27 UTC (permalink / raw)
  To: Juston Li; +Cc: intel-gfx

== Series Details ==

Series: drm: Add getfb2 ioctl
URL   : https://patchwork.freedesktop.org/series/67553/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7000 -> Patchwork_14656
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/index.html

Known issues
------------

  Here are the changes found in Patchwork_14656 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [PASS][1] -> [INCOMPLETE][2] ([fdo#107718])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-u3/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/fi-icl-u3/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - {fi-tgl-u}:         [INCOMPLETE][5] ([fdo#111735]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-tgl-u/igt@gem_ctx_create@basic-files.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/fi-tgl-u/igt@gem_ctx_create@basic-files.html
    - {fi-icl-dsi}:       [INCOMPLETE][7] ([fdo#107713] / [fdo#109100]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-dsi/igt@gem_ctx_create@basic-files.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/fi-icl-dsi/igt@gem_ctx_create@basic-files.html

  * igt@i915_selftest@live_execlists:
    - {fi-icl-guc}:       [INCOMPLETE][9] ([fdo#107713]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-guc/igt@i915_selftest@live_execlists.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/fi-icl-guc/igt@i915_selftest@live_execlists.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [FAIL][11] ([fdo#109483]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
    - fi-kbl-7500u:       [FAIL][13] ([fdo#111045] / [fdo#111096]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@prime_self_import@basic-with_fd_dup:
    - fi-icl-u3:          [DMESG-WARN][15] ([fdo#107724]) -> [PASS][16] +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-u3/igt@prime_self_import@basic-with_fd_dup.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/fi-icl-u3/igt@prime_self_import@basic-with_fd_dup.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735


Participating hosts (52 -> 44)
------------------------------

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-skl-6260u fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7000 -> Patchwork_14656

  CI-20190529: 20190529
  CI_DRM_7000: a6af6b11a94cbffff1c70dbab04ef1e13d79e4ae @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5211: 1601e1571eb0f29a06b64494040b3ea7859a650f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14656: 83b146de852e77e9c1090a4d9cc2c88177872b24 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

83b146de852e drm: Add getfb2 ioctl

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* ✗ Fi.CI.IGT: failure for drm: Add getfb2 ioctl
  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 ` Patchwork
  2019-10-09 15:50 ` [RESEND PATCH v2] " Daniel Vetter
  2019-12-13 21:36   ` [Intel-gfx] " Ville Syrjälä
  3 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-04 11:20 UTC (permalink / raw)
  To: Juston Li; +Cc: intel-gfx

== Series Details ==

Series: drm: Add getfb2 ioctl
URL   : https://patchwork.freedesktop.org/series/67553/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7000_full -> Patchwork_14656_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14656_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14656_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14656_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_reloc@basic-cpu-active:
    - shard-skl:          [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl6/igt@gem_exec_reloc@basic-cpu-active.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-skl4/igt@gem_exec_reloc@basic-cpu-active.html

  * igt@gem_mmap_gtt@hang:
    - shard-kbl:          [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-kbl3/igt@gem_mmap_gtt@hang.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-kbl2/igt@gem_mmap_gtt@hang.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_mmap_gtt@hang:
    - {shard-tglb}:       NOTRUN -> [DMESG-WARN][5]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-tglb6/igt@gem_mmap_gtt@hang.html

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - {shard-tglb}:       [PASS][6] -> [INCOMPLETE][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb8/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-tglb5/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html

  
Known issues
------------

  Here are the changes found in Patchwork_14656_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][8] -> [SKIP][9] ([fdo#110841])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][10] -> [SKIP][11] ([fdo#110854])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb1/igt@gem_exec_balancer@smoke.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb5/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         [PASS][12] -> [SKIP][13] ([fdo#109276]) +16 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb1/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb6/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_exec_schedule@reorder-wide-bsd:
    - shard-iclb:         [PASS][14] -> [SKIP][15] ([fdo#111325]) +6 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb8/igt@gem_exec_schedule@reorder-wide-bsd.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb1/igt@gem_exec_schedule@reorder-wide-bsd.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-kbl:          [PASS][16] -> [DMESG-WARN][17] ([fdo#108686])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-kbl4/igt@gem_tiled_swapping@non-threaded.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-kbl7/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-hsw:          [PASS][18] -> [DMESG-WARN][19] ([fdo#111870])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-hsw8/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
    - shard-iclb:         [PASS][20] -> [DMESG-WARN][21] ([fdo#111870]) +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb6/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-snb:          [PASS][22] -> [DMESG-WARN][23] ([fdo#111870]) +1 similar issue
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-snb2/igt@gem_userptr_blits@sync-unmap.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-snb1/igt@gem_userptr_blits@sync-unmap.html

  * igt@gem_userptr_blits@sync-unmap-after-close:
    - shard-apl:          [PASS][24] -> [DMESG-WARN][25] ([fdo#109385] / [fdo#111870])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl1/igt@gem_userptr_blits@sync-unmap-after-close.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-apl3/igt@gem_userptr_blits@sync-unmap-after-close.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [PASS][26] -> [DMESG-WARN][27] ([fdo#108566]) +1 similar issue
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl4/igt@i915_suspend@sysfs-reader.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-apl7/igt@i915_suspend@sysfs-reader.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          [PASS][28] -> [FAIL][29] ([fdo#105363])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-glk6/igt@kms_flip@flip-vs-expired-vblank.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-glk4/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@plain-flip-fb-recreate:
    - shard-skl:          [PASS][30] -> [FAIL][31] ([fdo#100368])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl4/igt@kms_flip@plain-flip-fb-recreate.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-skl9/igt@kms_flip@plain-flip-fb-recreate.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][32] -> [FAIL][33] ([fdo#103167]) +3 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][34] -> [FAIL][35] ([fdo#108145] / [fdo#110403])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-skl4/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][36] -> [SKIP][37] ([fdo#109642] / [fdo#111068])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb4/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [PASS][38] -> [SKIP][39] ([fdo#109441]) +1 similar issue
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb6/igt@kms_psr@psr2_cursor_blt.html

  * igt@kms_sequence@get-forked:
    - shard-hsw:          [PASS][40] -> [DMESG-WARN][41] ([fdo#102614])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-hsw2/igt@kms_sequence@get-forked.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-hsw5/igt@kms_sequence@get-forked.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][42] -> [FAIL][43] ([fdo#99912])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl1/igt@kms_setmode@basic.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-apl3/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd1:
    - shard-iclb:         [SKIP][44] ([fdo#109276]) -> [PASS][45] +16 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb7/igt@gem_ctx_shared@exec-single-timeline-bsd1.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd1.html

  * igt@gem_exec_schedule@preempt-bsd:
    - shard-iclb:         [SKIP][46] ([fdo#111325]) -> [PASS][47] +6 similar issues
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@gem_exec_schedule@preempt-bsd.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb3/igt@gem_exec_schedule@preempt-bsd.html

  * igt@gem_mmap_gtt@hang:
    - shard-apl:          [DMESG-WARN][48] ([fdo#109385]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl3/igt@gem_mmap_gtt@hang.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-apl4/igt@gem_mmap_gtt@hang.html

  * igt@gem_userptr_blits@coherency-sync:
    - shard-hsw:          [DMESG-WARN][50] ([fdo#111870]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-hsw1/igt@gem_userptr_blits@coherency-sync.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-hsw7/igt@gem_userptr_blits@coherency-sync.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-snb:          [DMESG-WARN][52] ([fdo#111870]) -> [PASS][53] +1 similar issue
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-snb7/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-snb6/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-skl:          [DMESG-WARN][54] ([fdo#111870]) -> [PASS][55]
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-skl10/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
    - shard-glk:          [DMESG-WARN][56] ([fdo#111870]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-glk1/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-glk7/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_userptr_blits@sync-unmap-after-close:
    - shard-hsw:          [DMESG-WARN][58] ([fdo#110789] / [fdo#111870]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-hsw8/igt@gem_userptr_blits@sync-unmap-after-close.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-hsw6/igt@gem_userptr_blits@sync-unmap-after-close.html

  * {igt@i915_pm_dc@dc6-psr}:
    - shard-iclb:         [FAIL][60] ([fdo#110548]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@i915_pm_dc@dc6-psr.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb6/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_selftest@mock_fence:
    - {shard-tglb}:       [INCOMPLETE][62] ([fdo#111747]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb4/igt@i915_selftest@mock_fence.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-tglb4/igt@i915_selftest@mock_fence.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][64] ([fdo#108566]) -> [PASS][65] +5 similar issues
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_crc@pipe-b-cursor-256x256-random:
    - shard-skl:          [FAIL][66] ([fdo#103232]) -> [PASS][67]
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl8/igt@kms_cursor_crc@pipe-b-cursor-256x256-random.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-skl3/igt@kms_cursor_crc@pipe-b-cursor-256x256-random.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding:
    - shard-snb:          [INCOMPLETE][68] ([fdo#105411]) -> [PASS][69]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-snb1/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-snb4/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html

  * igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque:
    - {shard-tglb}:       [DMESG-WARN][70] ([fdo#111600]) -> [PASS][71]
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb2/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-tglb1/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html

  * igt@kms_flip@flip-vs-modeset-vs-hang:
    - shard-apl:          [INCOMPLETE][72] ([fdo#103927]) -> [PASS][73] +4 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl2/igt@kms_flip@flip-vs-modeset-vs-hang.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-apl8/igt@kms_flip@flip-vs-modeset-vs-hang.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          [INCOMPLETE][74] ([fdo#109507]) -> [PASS][75]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl6/igt@kms_flip@flip-vs-suspend.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-skl6/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-kbl:          [DMESG-WARN][76] ([fdo#103313]) -> [PASS][77]
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-kbl4/igt@kms_flip@flip-vs-suspend-interruptible.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt:
    - shard-iclb:         [FAIL][78] ([fdo#103167]) -> [PASS][79] +2 similar issues
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - {shard-tglb}:       [INCOMPLETE][80] ([fdo#111832]) -> [PASS][81] +2 similar issues
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb7/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-tglb6/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff:
    - {shard-tglb}:       [FAIL][82] ([fdo#103167]) -> [PASS][83] +4 similar issues
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [FAIL][84] ([fdo#108145]) -> [PASS][85]
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][86] ([fdo#103166]) -> [PASS][87]
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb2/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         [SKIP][88] ([fdo#109441]) -> [PASS][89]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb8/igt@kms_psr@psr2_dpms.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb2/igt@kms_psr@psr2_dpms.html

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-skl:          [INCOMPLETE][90] ([fdo#104108]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl5/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-skl7/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html

  * igt@perf@polling:
    - shard-skl:          [FAIL][92] ([fdo#110728]) -> [PASS][93]
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl10/igt@perf@polling.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-skl4/igt@perf@polling.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][94] ([fdo#111330]) -> [SKIP][95] ([fdo#109276])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb1/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb6/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@gem_mocs_settings@mocs-reset-dirty-render:
    - shard-iclb:         [INCOMPLETE][96] ([fdo#107713]) -> [SKIP][97] ([fdo#110206])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb1/igt@gem_mocs_settings@mocs-reset-dirty-render.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb7/igt@gem_mocs_settings@mocs-reset-dirty-render.html

  * igt@gem_mocs_settings@mocs-settings-bsd2:
    - shard-iclb:         [SKIP][98] ([fdo#109276]) -> [FAIL][99] ([fdo#111330]) +1 similar issue
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb3/igt@gem_mocs_settings@mocs-settings-bsd2.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/shard-iclb1/igt@gem_mocs_settings@mocs-settings-bsd2.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo# 111852 ]: https://bugs.freedesktop.org/show_bug.cgi?id= 111852 
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/sho

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14656/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
  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 ` Daniel Vetter
  2019-10-14 16:21   ` Li, Juston
  2019-11-21  8:26     ` Timo Aaltonen
  2019-12-13 21:36   ` [Intel-gfx] " Ville Syrjälä
  3 siblings, 2 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-09 15:50 UTC (permalink / raw)
  To: Juston Li; +Cc: intel-gfx, Daniel Stone, dri-devel

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

> ---
>  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
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
  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-11-21  8:26     ` Timo Aaltonen
  1 sibling, 1 reply; 22+ messages in thread
From: Li, Juston @ 2019-10-14 16:21 UTC (permalink / raw)
  To: daniel; +Cc: intel-gfx, daniels, dri-devel

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

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
  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
  0 siblings, 2 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-14 17:51 UTC (permalink / raw)
  To: Li, Juston, Sean Paul, Daniele Castagna; +Cc: intel-gfx, daniels, dri-devel

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

>
> 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
  2019-10-14 17:51     ` Daniel Vetter
@ 2019-10-14 18:30       ` Li, Juston
  2019-10-15 15:12       ` Daniele Castagna
  1 sibling, 0 replies; 22+ messages in thread
From: Li, Juston @ 2019-10-14 18:30 UTC (permalink / raw)
  To: daniel, dcastagna, sean, markyacoub; +Cc: intel-gfx, daniels, dri-devel

On Mon, 2019-10-14 at 19:51 +0200, Daniel Vetter 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

+Mark, who I've been talking about enabling render buffer compression.

This patch allows me to fix a regression w/ screenshot utility when
enabling RBC:
https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/1759155

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
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
  2019-10-14 17:51     ` Daniel Vetter
  2019-10-14 18:30       ` Li, Juston
@ 2019-10-15 15:12       ` Daniele Castagna
  1 sibling, 0 replies; 22+ messages in thread
From: Daniele Castagna @ 2019-10-15 15:12 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, Sean Paul, dri-devel, Li, Juston, daniels

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-11-21  8:26     ` Timo Aaltonen
  0 siblings, 0 replies; 22+ messages in thread
From: Timo Aaltonen @ 2019-11-21  8:26 UTC (permalink / raw)
  To: Daniel Vetter, Juston Li; +Cc: intel-gfx, Daniel Stone, dri-devel

On 9.10.2019 18.50, 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

xserver too

https://lists.x.org/archives/xorg-devel/2018-March/056363.html

to fix

https://gitlab.freedesktop.org/xorg/xserver/issues/33

which forces Ubuntu to disable CCS compression, and I'd like to get rid
of that patch.

Thanks Juston for pushing this!


-- 
t
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-11-21  8:26     ` Timo Aaltonen
  0 siblings, 0 replies; 22+ messages in thread
From: Timo Aaltonen @ 2019-11-21  8:26 UTC (permalink / raw)
  To: Daniel Vetter, Juston Li; +Cc: intel-gfx, Daniel Stone, dri-devel

On 9.10.2019 18.50, 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

xserver too

https://lists.x.org/archives/xorg-devel/2018-March/056363.html

to fix

https://gitlab.freedesktop.org/xorg/xserver/issues/33

which forces Ubuntu to disable CCS compression, and I'd like to get rid
of that patch.

Thanks Juston for pushing this!


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-11-21  8:26     ` Timo Aaltonen
  0 siblings, 0 replies; 22+ messages in thread
From: Timo Aaltonen @ 2019-11-21  8:26 UTC (permalink / raw)
  To: Daniel Vetter, Juston Li; +Cc: intel-gfx, Daniel Stone, dri-devel

On 9.10.2019 18.50, 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

xserver too

https://lists.x.org/archives/xorg-devel/2018-March/056363.html

to fix

https://gitlab.freedesktop.org/xorg/xserver/issues/33

which forces Ubuntu to disable CCS compression, and I'd like to get rid
of that patch.

Thanks Juston for pushing this!


-- 
t
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-11-21  9:28       ` Daniel Vetter
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-11-21  9:28 UTC (permalink / raw)
  To: Timo Aaltonen; +Cc: dri-devel, intel-gfx, Daniel Stone, Juston Li

On Thu, Nov 21, 2019 at 9:26 AM Timo Aaltonen <tjaalton@ubuntu.com> wrote:
>
> On 9.10.2019 18.50, 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
>
> xserver too
>
> https://lists.x.org/archives/xorg-devel/2018-March/056363.html
>
> to fix
>
> https://gitlab.freedesktop.org/xorg/xserver/issues/33
>
> which forces Ubuntu to disable CCS compression, and I'd like to get rid
> of that patch.
>
> Thanks Juston for pushing this!

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

... but someone needs to review all the patches and make sure kernel
patch + igt work and pass intel CI and then push it all, and given the
pile of committers we have that's not me I think. Just in case people
expect me to push this forward, I only jumped in here to make sure new
uapi is done by the book and checks all the boxes.
-Daniel
-- 
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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-11-21  9:28       ` Daniel Vetter
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-11-21  9:28 UTC (permalink / raw)
  To: Timo Aaltonen; +Cc: dri-devel, intel-gfx, Daniel Stone

On Thu, Nov 21, 2019 at 9:26 AM Timo Aaltonen <tjaalton@ubuntu.com> wrote:
>
> On 9.10.2019 18.50, 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
>
> xserver too
>
> https://lists.x.org/archives/xorg-devel/2018-March/056363.html
>
> to fix
>
> https://gitlab.freedesktop.org/xorg/xserver/issues/33
>
> which forces Ubuntu to disable CCS compression, and I'd like to get rid
> of that patch.
>
> Thanks Juston for pushing this!

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

... but someone needs to review all the patches and make sure kernel
patch + igt work and pass intel CI and then push it all, and given the
pile of committers we have that's not me I think. Just in case people
expect me to push this forward, I only jumped in here to make sure new
uapi is done by the book and checks all the boxes.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-11-22  1:07         ` Li, Juston
  0 siblings, 0 replies; 22+ messages in thread
From: Li, Juston @ 2019-11-22  1:07 UTC (permalink / raw)
  To: daniel, tjaalton; +Cc: intel-gfx, daniels, dri-devel

On Thu, 2019-11-21 at 10:28 +0100, Daniel Vetter wrote:
> On Thu, Nov 21, 2019 at 9:26 AM Timo Aaltonen <tjaalton@ubuntu.com>
> wrote:
> > On 9.10.2019 18.50, 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
> > 
> > xserver too
> > 
> > https://lists.x.org/archives/xorg-devel/2018-March/056363.html
> > 
> > to fix
> > 
> > https://gitlab.freedesktop.org/xorg/xserver/issues/33
> > 
> > which forces Ubuntu to disable CCS compression, and I'd like to get
> > rid
> > of that patch.
> > 
> > Thanks Juston for pushing this!
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> ... but someone needs to review all the patches and make sure kernel
> patch + igt work and pass intel CI and then push it all, and given
> the
> pile of committers we have that's not me I think. Just in case people
> expect me to push this forward, I only jumped in here to make sure
> new
> uapi is done by the book and checks all the boxes.
> -Daniel

Thanks for clarifying Daniel, I'll try to find someone to review.

Juston
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-11-22  1:07         ` Li, Juston
  0 siblings, 0 replies; 22+ messages in thread
From: Li, Juston @ 2019-11-22  1:07 UTC (permalink / raw)
  To: daniel, tjaalton; +Cc: intel-gfx, daniels, dri-devel

On Thu, 2019-11-21 at 10:28 +0100, Daniel Vetter wrote:
> On Thu, Nov 21, 2019 at 9:26 AM Timo Aaltonen <tjaalton@ubuntu.com>
> wrote:
> > On 9.10.2019 18.50, 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
> > 
> > xserver too
> > 
> > https://lists.x.org/archives/xorg-devel/2018-March/056363.html
> > 
> > to fix
> > 
> > https://gitlab.freedesktop.org/xorg/xserver/issues/33
> > 
> > which forces Ubuntu to disable CCS compression, and I'd like to get
> > rid
> > of that patch.
> > 
> > Thanks Juston for pushing this!
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> ... but someone needs to review all the patches and make sure kernel
> patch + igt work and pass intel CI and then push it all, and given
> the
> pile of committers we have that's not me I think. Just in case people
> expect me to push this forward, I only jumped in here to make sure
> new
> uapi is done by the book and checks all the boxes.
> -Daniel

Thanks for clarifying Daniel, I'll try to find someone to review.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-11-22  1:07         ` Li, Juston
  0 siblings, 0 replies; 22+ messages in thread
From: Li, Juston @ 2019-11-22  1:07 UTC (permalink / raw)
  To: daniel, tjaalton; +Cc: intel-gfx, daniels, dri-devel

On Thu, 2019-11-21 at 10:28 +0100, Daniel Vetter wrote:
> On Thu, Nov 21, 2019 at 9:26 AM Timo Aaltonen <tjaalton@ubuntu.com>
> wrote:
> > On 9.10.2019 18.50, 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
> > 
> > xserver too
> > 
> > https://lists.x.org/archives/xorg-devel/2018-March/056363.html
> > 
> > to fix
> > 
> > https://gitlab.freedesktop.org/xorg/xserver/issues/33
> > 
> > which forces Ubuntu to disable CCS compression, and I'd like to get
> > rid
> > of that patch.
> > 
> > Thanks Juston for pushing this!
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> ... but someone needs to review all the patches and make sure kernel
> patch + igt work and pass intel CI and then push it all, and given
> the
> pile of committers we have that's not me I think. Just in case people
> expect me to push this forward, I only jumped in here to make sure
> new
> uapi is done by the book and checks all the boxes.
> -Daniel

Thanks for clarifying Daniel, I'll try to find someone to review.

Juston
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
  2019-10-03 18:31 [RESEND PATCH v2] drm: Add getfb2 ioctl Juston Li
@ 2019-12-13 21:36   ` Ville Syrjälä
  2019-10-04 11:20 ` ✗ Fi.CI.IGT: failure " Patchwork
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2019-12-13 21:36 UTC (permalink / raw)
  To: Juston Li; +Cc: intel-gfx, Daniel Stone, dri-devel

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-12-13 21:36   ` Ville Syrjälä
  0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2019-12-13 21:36 UTC (permalink / raw)
  To: Juston Li; +Cc: intel-gfx, Daniel Stone, dri-devel

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RESEND PATCH v2] drm: Add getfb2 ioctl
  2019-12-13 21:36   ` [Intel-gfx] " Ville Syrjälä
@ 2019-12-13 22:52     ` Li, Juston
  -1 siblings, 0 replies; 22+ messages in thread
From: Li, Juston @ 2019-12-13 22:52 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, daniels, dri-devel

On Fri, 2019-12-13 at 23:36 +0200, Ville Syrjälä 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>
> > ---
> >  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?

Hmm yeah sorry I missed the protections handling.
I think we can just set the getfb2 ioctl flags as
DRM_MASTER|DRM_ROOT_ONLY

> > +
> > +		if (ret != 0)
> > +			goto out;
> 
> Could be just 'break;' and then we wouldn't even need the label.

Will do.

> 
> Rest lgtm.
> 

Much appreciated
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-12-13 22:52     ` Li, Juston
  0 siblings, 0 replies; 22+ messages in thread
From: Li, Juston @ 2019-12-13 22:52 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, daniels, dri-devel

On Fri, 2019-12-13 at 23:36 +0200, Ville Syrjälä 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>
> > ---
> >  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?

Hmm yeah sorry I missed the protections handling.
I think we can just set the getfb2 ioctl flags as
DRM_MASTER|DRM_ROOT_ONLY

> > +
> > +		if (ret != 0)
> > +			goto out;
> 
> Could be just 'break;' and then we wouldn't even need the label.

Will do.

> 
> Rest lgtm.
> 

Much appreciated
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [RESEND PATCH v2] drm: Add getfb2 ioctl
  2019-12-13 22:52     ` [Intel-gfx] " Li, Juston
@ 2019-12-14  0:01       ` Daniel Vetter
  -1 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-12-14  0:01 UTC (permalink / raw)
  To: Li, Juston; +Cc: dri-devel, intel-gfx, daniels

On Fri, Dec 13, 2019 at 10:52:03PM +0000, Li, Juston wrote:
> On Fri, 2019-12-13 at 23:36 +0200, Ville Syrjälä 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>
> > > ---
> > >  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?
> 
> Hmm yeah sorry I missed the protections handling.
> I think we can just set the getfb2 ioctl flags as
> DRM_MASTER|DRM_ROOT_ONLY

Since this was missed, can you pls extend the igt to test for this? Iirc
the getfb igt makes sure non-root doesn't get this ...

Also for consistency I think doing the same as getfb would be good. Fewer
surprises.
-Daniel

> 
> > > +
> > > +		if (ret != 0)
> > > +			goto out;
> > 
> > Could be just 'break;' and then we wouldn't even need the label.
> 
> Will do.
> 
> > 
> > Rest lgtm.
> > 
> 
> Much appreciated
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [RESEND PATCH v2] drm: Add getfb2 ioctl
@ 2019-12-14  0:01       ` Daniel Vetter
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-12-14  0:01 UTC (permalink / raw)
  To: Li, Juston; +Cc: dri-devel, intel-gfx, daniels

On Fri, Dec 13, 2019 at 10:52:03PM +0000, Li, Juston wrote:
> On Fri, 2019-12-13 at 23:36 +0200, Ville Syrjälä 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>
> > > ---
> > >  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?
> 
> Hmm yeah sorry I missed the protections handling.
> I think we can just set the getfb2 ioctl flags as
> DRM_MASTER|DRM_ROOT_ONLY

Since this was missed, can you pls extend the igt to test for this? Iirc
the getfb igt makes sure non-root doesn't get this ...

Also for consistency I think doing the same as getfb would be good. Fewer
surprises.
-Daniel

> 
> > > +
> > > +		if (ret != 0)
> > > +			goto out;
> > 
> > Could be just 'break;' and then we wouldn't even need the label.
> 
> Will do.
> 
> > 
> > Rest lgtm.
> > 
> 
> Much appreciated
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-12-14  0:01 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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ä
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

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.