Re: [Intel-gfx] [PATCH i-g-t 2/3] igt_kms: Allow kmstest_get_connector_config to take provided drmModeGetResources

2016-04-28 Thread Tvrtko Ursulin 


On 28/04/16 09:28, Daniel Vetter wrote:

On Wed, Apr 27, 2016 at 04:12:35PM +0100, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

This will enable the following patch to generate less dmesg spam.

Signed-off-by: Tvrtko Ursulin 
---
  lib/igt_kms.c   | 33 +++--
  lib/igt_kms.h   |  3 ++-
  tests/kms_3d.c  |  2 +-
  tests/kms_flip.c|  7 ---
  tests/kms_render.c  |  5 +++--
  tests/testdisplay.c |  2 +-
  6 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 7557bdc20fa4..07f523e2c39b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -749,6 +749,7 @@ bool kmstest_get_connector_default_mode(int drm_fd, 
drmModeConnector *connector,
  /**
   * _kmstest_connector_config:
   * @drm_fd: DRM fd
+ * @resources: pointer to drmModeRes or NULL
   * @connector_id: DRM connector id
   * @crtc_idx_mask: mask of allowed DRM CRTC indices
   * @config: structure filled with the possible configuration
@@ -757,17 +758,24 @@ bool kmstest_get_connector_default_mode(int drm_fd, 
drmModeConnector *connector,
   * This tries to find a suitable configuration for the given connector and 
CRTC
   * constraint and fills it into @config.
   */
-static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
+static bool _kmstest_connector_config(int drm_fd, drmModeRes *resources,
+ uint32_t connector_id,
  unsigned long crtc_idx_mask,
  struct kmstest_connector_config *config,
  bool probe)
  {
-   drmModeRes *resources;
drmModeConnector *connector;
drmModeEncoder *encoder;
+   bool free_resources;
int i, j;

-   resources = drmModeGetResources(drm_fd);
+   if (resources == NULL) {
+   resources = drmModeGetResources(drm_fd);
+   free_resources = true;
+   } else {
+   free_resources = false;
+   }


Imo you should just cache this internally in the lib instead of forcing
everyone to become more clever. We can just request resources once when
initializing the kmstest library, and then maybe provide a function to
force reprobing (for something like testdisplay, but that doesn't use
kmstest yet).


If you merge the two drm core patches patches 2/3 and 3/3 are best 
abandoned. :)


Regards,

Tvrtko


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

Re: [Intel-gfx] [PATCH i-g-t 2/3] igt_kms: Allow kmstest_get_connector_config to take provided drmModeGetResources

2016-04-28 Thread Daniel Vetter 
On Wed, Apr 27, 2016 at 04:12:35PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> This will enable the following patch to generate less dmesg spam.
> 
> Signed-off-by: Tvrtko Ursulin 
> ---
>  lib/igt_kms.c   | 33 +++--
>  lib/igt_kms.h   |  3 ++-
>  tests/kms_3d.c  |  2 +-
>  tests/kms_flip.c|  7 ---
>  tests/kms_render.c  |  5 +++--
>  tests/testdisplay.c |  2 +-
>  6 files changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 7557bdc20fa4..07f523e2c39b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -749,6 +749,7 @@ bool kmstest_get_connector_default_mode(int drm_fd, 
> drmModeConnector *connector,
>  /**
>   * _kmstest_connector_config:
>   * @drm_fd: DRM fd
> + * @resources: pointer to drmModeRes or NULL
>   * @connector_id: DRM connector id
>   * @crtc_idx_mask: mask of allowed DRM CRTC indices
>   * @config: structure filled with the possible configuration
> @@ -757,17 +758,24 @@ bool kmstest_get_connector_default_mode(int drm_fd, 
> drmModeConnector *connector,
>   * This tries to find a suitable configuration for the given connector and 
> CRTC
>   * constraint and fills it into @config.
>   */
> -static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
> +static bool _kmstest_connector_config(int drm_fd, drmModeRes *resources,
> +   uint32_t connector_id,
> unsigned long crtc_idx_mask,
> struct kmstest_connector_config *config,
> bool probe)
>  {
> - drmModeRes *resources;
>   drmModeConnector *connector;
>   drmModeEncoder *encoder;
> + bool free_resources;
>   int i, j;
>  
> - resources = drmModeGetResources(drm_fd);
> + if (resources == NULL) {
> + resources = drmModeGetResources(drm_fd);
> + free_resources = true;
> + } else {
> + free_resources = false;
> + }

Imo you should just cache this internally in the lib instead of forcing
everyone to become more clever. We can just request resources once when
initializing the kmstest library, and then maybe provide a function to
force reprobing (for something like testdisplay, but that doesn't use
kmstest yet).
-Daniel

> +
>   if (!resources) {
>   igt_warn("drmModeGetResources failed");
>   goto err1;
> @@ -840,7 +848,8 @@ found:
>   config->pipe = kmstest_get_pipe_from_crtc_id(drm_fd,
>config->crtc->crtc_id);
>  
> - drmModeFreeResources(resources);
> + if (free_resources)
> + drmModeFreeResources(resources);
>  
>   return true;
>  err4:
> @@ -848,7 +857,8 @@ err4:
>  err3:
>   drmModeFreeConnector(connector);
>  err2:
> - drmModeFreeResources(resources);
> + if (free_resources)
> + drmModeFreeResources(resources);
>  err1:
>   return false;
>  }
> @@ -856,6 +866,7 @@ err1:
>  /**
>   * kmstest_get_connector_config:
>   * @drm_fd: DRM fd
> + * @resources: pointer to drmModeRes or NULL
>   * @connector_id: DRM connector id
>   * @crtc_idx_mask: mask of allowed DRM CRTC indices
>   * @config: structure filled with the possible configuration
> @@ -863,12 +874,13 @@ err1:
>   * This tries to find a suitable configuration for the given connector and 
> CRTC
>   * constraint and fills it into @config.
>   */
> -bool kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
> +bool kmstest_get_connector_config(int drm_fd, drmModeRes *resources,
> +   uint32_t connector_id,
> unsigned long crtc_idx_mask,
> struct kmstest_connector_config *config)
>  {
> - return _kmstest_connector_config(drm_fd, connector_id, crtc_idx_mask,
> -  config, 0);
> + return _kmstest_connector_config(drm_fd, resources, connector_id,
> +  crtc_idx_mask, config, 0);
>  }
>  
>  /**
> @@ -886,8 +898,8 @@ bool kmstest_probe_connector_config(int drm_fd, uint32_t 
> connector_id,
>   unsigned long crtc_idx_mask,
>   struct kmstest_connector_config *config)
>  {
> - return _kmstest_connector_config(drm_fd, connector_id, crtc_idx_mask,
> -  config, 1);
> + return _kmstest_connector_config(drm_fd, NULL, connector_id,
> +  crtc_idx_mask, config, 1);
>  }
>  
>  /**
> @@ -1160,6 +1172,7 @@ static void igt_output_refresh(igt_output_t *output)
>   kmstest_free_connector_config(>config);
>  
>   ret = kmstest_get_connector_config(display->drm_fd,
> +NULL,
>

[Intel-gfx] [PATCH i-g-t 2/3] igt_kms: Allow kmstest_get_connector_config to take provided drmModeGetResources

2016-04-27 Thread Tvrtko Ursulin 
From: Tvrtko Ursulin 

This will enable the following patch to generate less dmesg spam.

Signed-off-by: Tvrtko Ursulin 
---
 lib/igt_kms.c   | 33 +++--
 lib/igt_kms.h   |  3 ++-
 tests/kms_3d.c  |  2 +-
 tests/kms_flip.c|  7 ---
 tests/kms_render.c  |  5 +++--
 tests/testdisplay.c |  2 +-
 6 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 7557bdc20fa4..07f523e2c39b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -749,6 +749,7 @@ bool kmstest_get_connector_default_mode(int drm_fd, 
drmModeConnector *connector,
 /**
  * _kmstest_connector_config:
  * @drm_fd: DRM fd
+ * @resources: pointer to drmModeRes or NULL
  * @connector_id: DRM connector id
  * @crtc_idx_mask: mask of allowed DRM CRTC indices
  * @config: structure filled with the possible configuration
@@ -757,17 +758,24 @@ bool kmstest_get_connector_default_mode(int drm_fd, 
drmModeConnector *connector,
  * This tries to find a suitable configuration for the given connector and CRTC
  * constraint and fills it into @config.
  */
-static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
+static bool _kmstest_connector_config(int drm_fd, drmModeRes *resources,
+ uint32_t connector_id,
  unsigned long crtc_idx_mask,
  struct kmstest_connector_config *config,
  bool probe)
 {
-   drmModeRes *resources;
drmModeConnector *connector;
drmModeEncoder *encoder;
+   bool free_resources;
int i, j;
 
-   resources = drmModeGetResources(drm_fd);
+   if (resources == NULL) {
+   resources = drmModeGetResources(drm_fd);
+   free_resources = true;
+   } else {
+   free_resources = false;
+   }
+
if (!resources) {
igt_warn("drmModeGetResources failed");
goto err1;
@@ -840,7 +848,8 @@ found:
config->pipe = kmstest_get_pipe_from_crtc_id(drm_fd,
 config->crtc->crtc_id);
 
-   drmModeFreeResources(resources);
+   if (free_resources)
+   drmModeFreeResources(resources);
 
return true;
 err4:
@@ -848,7 +857,8 @@ err4:
 err3:
drmModeFreeConnector(connector);
 err2:
-   drmModeFreeResources(resources);
+   if (free_resources)
+   drmModeFreeResources(resources);
 err1:
return false;
 }
@@ -856,6 +866,7 @@ err1:
 /**
  * kmstest_get_connector_config:
  * @drm_fd: DRM fd
+ * @resources: pointer to drmModeRes or NULL
  * @connector_id: DRM connector id
  * @crtc_idx_mask: mask of allowed DRM CRTC indices
  * @config: structure filled with the possible configuration
@@ -863,12 +874,13 @@ err1:
  * This tries to find a suitable configuration for the given connector and CRTC
  * constraint and fills it into @config.
  */
-bool kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
+bool kmstest_get_connector_config(int drm_fd, drmModeRes *resources,
+ uint32_t connector_id,
  unsigned long crtc_idx_mask,
  struct kmstest_connector_config *config)
 {
-   return _kmstest_connector_config(drm_fd, connector_id, crtc_idx_mask,
-config, 0);
+   return _kmstest_connector_config(drm_fd, resources, connector_id,
+crtc_idx_mask, config, 0);
 }
 
 /**
@@ -886,8 +898,8 @@ bool kmstest_probe_connector_config(int drm_fd, uint32_t 
connector_id,
unsigned long crtc_idx_mask,
struct kmstest_connector_config *config)
 {
-   return _kmstest_connector_config(drm_fd, connector_id, crtc_idx_mask,
-config, 1);
+   return _kmstest_connector_config(drm_fd, NULL, connector_id,
+crtc_idx_mask, config, 1);
 }
 
 /**
@@ -1160,6 +1172,7 @@ static void igt_output_refresh(igt_output_t *output)
kmstest_free_connector_config(>config);
 
ret = kmstest_get_connector_config(display->drm_fd,
+  NULL,
   output->id,
   crtc_idx_mask,
   >config);
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 5c8340171ab6..8cbba1673d28 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -158,7 +158,8 @@ void kmstest_force_edid(int drm_fd, drmModeConnector 
*connector,
 
 bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector 
*connector,
drmModeModeInfo *mode);
-bool kmstest_get_connector_config(int