[i-g-t,3/5] lib/kms: Drop igt_display_init

Submitted by Daniel Vetter on Nov. 22, 2018, 9:36 a.m.

Details

Message ID 20181122093700.31211-3-daniel.vetter@ffwll.ch
State New
Series "Series without cover letter"
Headers show

Commit Message

Daniel Vetter Nov. 22, 2018, 9:36 a.m.
If you need the high-level functions, then you probably need a
full display. Unexport the non-requiring version, and adjust the
documentation. This also gives us proper docs for the recently
added igt_display_require.

Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 lib/igt_kms.c | 14 +++++---------
 lib/igt_kms.h |  1 -
 2 files changed, 5 insertions(+), 10 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d806ccc1e0b9..7214101e2696 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1861,16 +1861,17 @@  static void igt_fill_plane_format_mod(igt_display_t *display, igt_plane_t *plane
 static void igt_fill_display_format_mod(igt_display_t *display);
 
 /**
- * igt_display_init:
+ * igt_display_require:
  * @display: a pointer to an #igt_display_t structure
  * @drm_fd: a drm file descriptor
  *
  * Initialize @display and allocate the various resources required. Use
  * #igt_display_fini to release the resources when they are no longer required.
  *
- * Returns: true if the display has outputs and pipes available, false otherwise
+ * This function automatically skips if the kernel driver doesn't support any
+ * CRTC or outputs.
  */
-bool igt_display_init(igt_display_t *display, int drm_fd)
+void igt_display_require(igt_display_t *display, int drm_fd)
 {
 	drmModeRes *resources;
 	drmModePlaneRes *plane_resources;
@@ -2034,12 +2035,7 @@  bool igt_display_init(igt_display_t *display, int drm_fd)
 out:
 	LOG_UNINDENT(display);
 
-	return display->n_pipes && display->n_outputs;
-}
-
-void igt_display_require(igt_display_t *display, int drm_fd)
-{
-	igt_require(igt_display_init(display, drm_fd));
+	igt_require(display->n_pipes && display->n_outputs);
 }
 
 /**
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index e6aff339af60..09395360007f 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -382,7 +382,6 @@  struct igt_display {
 	int format_mod_count;
 };
 
-bool igt_display_init(igt_display_t *display, int drm_fd);
 void igt_display_require(igt_display_t *display, int drm_fd);
 void igt_display_fini(igt_display_t *display);
 void igt_display_reset(igt_display_t *display);

Comments

Antonio Argenziano Nov. 26, 2018, 9:37 p.m.
On 22/11/18 01:36, Daniel Vetter wrote:
> If you need the high-level functions, then you probably need a
> full display. Unexport the non-requiring version, and adjust the
> documentation. This also gives us proper docs for the recently
> added igt_display_require.

I actually liked having init and require to be separate. Are there no 
negative test cases that might play-around with a failing init?

other than that the series LGTM but you might want someone to look at 
[2/5] in details,
Acked-By: Antonio Argenziano
Daniel Vetter Nov. 27, 2018, 8:46 a.m.
On Mon, Nov 26, 2018 at 01:37:03PM -0800, Antonio Argenziano wrote:
> 
> 
> On 22/11/18 01:36, Daniel Vetter wrote:
> > If you need the high-level functions, then you probably need a
> > full display. Unexport the non-requiring version, and adjust the
> > documentation. This also gives us proper docs for the recently
> > added igt_display_require.
> 
> I actually liked having init and require to be separate. Are there no
> negative test cases that might play-around with a failing init?

We dont (yet?) have testcases for the igt_display library itself. For any
kernel tests the summar above explains it: If you want to write
low-level/negative tests of atomic/kms functionality, igt_display is to
high-level to be useful. All those tests use libdrm functions directly
(plus maybe some of the low-level helpers we have, the functions with
kmstest_ prefixes).

> other than that the series LGTM but you might want someone to look at [2/5]
> in details,
> Acked-By: Antonio Argenziano

Thanks for taking a look, I'll pull in the entire series now.
-Daniel