diff mbox

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

Message ID 1461769956-33188-2-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin April 27, 2016, 3:12 p.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

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

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 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(-)

Comments

Daniel Vetter April 28, 2016, 8:28 a.m. UTC | #1 
On Wed, Apr 27, 2016 at 04:12:35PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This will enable the following patch to generate less dmesg spam.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  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(&output->config);
>  
>  	ret = kmstest_get_connector_config(display->drm_fd,
> +					   NULL,
>  					   output->id,
>  					   crtc_idx_mask,
>  					   &output->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 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);
>  bool kmstest_probe_connector_config(int drm_fd, uint32_t connector_id,
> diff --git a/tests/kms_3d.c b/tests/kms_3d.c
> index bfc981ee279d..e8bc4a42d8aa 100644
> --- a/tests/kms_3d.c
> +++ b/tests/kms_3d.c
> @@ -86,7 +86,7 @@ igt_simple_main
>  			continue;
>  
>  		/* create a configuration */
> -		ret = kmstest_get_connector_config(drm_fd, connector_id,
> +		ret = kmstest_get_connector_config(drm_fd, NULL, connector_id,
>  						   crtc_mask, &config);
>  		if (ret != true) {
>  			igt_info("Error creating configuration for:\n  ");
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index eda2fcc9212d..fec69254e4da 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -1042,7 +1042,8 @@ static void connector_find_preferred_mode(uint32_t connector_id, int crtc_idx,
>  {
>  	struct kmstest_connector_config config;
>  
> -	if (!kmstest_get_connector_config(drm_fd, connector_id, 1 << crtc_idx,
> +	if (!kmstest_get_connector_config(drm_fd, NULL,
> +					  connector_id, 1 << crtc_idx,
>  					  &config)) {
>  		o->mode_valid = 0;
>  		return;
> @@ -1086,11 +1087,11 @@ static void connector_find_compatible_mode(int crtc_idx0, int crtc_idx1,
>  	drmModeModeInfo *mode[2];
>  	int n, m;
>  
> -	if (!kmstest_get_connector_config(drm_fd, o->_connector[0],
> +	if (!kmstest_get_connector_config(drm_fd, NULL, o->_connector[0],
>  					  1 << crtc_idx0, &config[0]))
>  		return;
>  
> -	if (!kmstest_get_connector_config(drm_fd, o->_connector[1],
> +	if (!kmstest_get_connector_config(drm_fd, NULL, o->_connector[1],
>  					  1 << crtc_idx1, &config[1])) {
>  		kmstest_free_connector_config(&config[0]);
>  		return;
> diff --git a/tests/kms_render.c b/tests/kms_render.c
> index 72da87f19af3..735a1e117349 100644
> --- a/tests/kms_render.c
> +++ b/tests/kms_render.c
> @@ -210,8 +210,9 @@ static int run_test(const char *test_name, enum test_flags flags)
>  		for (j = 0; j < resources->count_crtcs; j++) {
>  			struct kmstest_connector_config cconf;
>  
> -			if (!kmstest_get_connector_config(drm_fd, connector_id,
> -							   1 << j, &cconf))
> +			if (!kmstest_get_connector_config(drm_fd, NULL,
> +							  connector_id,
> +							  1 << j, &cconf))
>  				continue;
>  
>  			test_connector(test_name, &cconf, flags);
> diff --git a/tests/testdisplay.c b/tests/testdisplay.c
> index 00b47bd06280..9d84d8cd4000 100644
> --- a/tests/testdisplay.c
> +++ b/tests/testdisplay.c
> @@ -201,7 +201,7 @@ static void connector_find_preferred_mode(uint32_t connector_id,
>  		ret = kmstest_probe_connector_config(drm_fd, connector_id,
>  						     crtc_idx_mask, &config);
>  	else
> -		ret = kmstest_get_connector_config(drm_fd, connector_id,
> +		ret = kmstest_get_connector_config(drm_fd, NULL, connector_id,
>  						   crtc_idx_mask, &config);
>  	if (!ret) {
>  		c->mode_valid = 0;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tvrtko Ursulin April 28, 2016, 9:05 a.m. UTC | #2 
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 <tvrtko.ursulin@intel.com>
>>
>> This will enable the following patch to generate less dmesg spam.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   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
diff mbox

Patch

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(&output->config);
 
 	ret = kmstest_get_connector_config(display->drm_fd,
+					   NULL,
 					   output->id,
 					   crtc_idx_mask,
 					   &output->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 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);
 bool kmstest_probe_connector_config(int drm_fd, uint32_t connector_id,
diff --git a/tests/kms_3d.c b/tests/kms_3d.c
index bfc981ee279d..e8bc4a42d8aa 100644
--- a/tests/kms_3d.c
+++ b/tests/kms_3d.c
@@ -86,7 +86,7 @@  igt_simple_main
 			continue;
 
 		/* create a configuration */
-		ret = kmstest_get_connector_config(drm_fd, connector_id,
+		ret = kmstest_get_connector_config(drm_fd, NULL, connector_id,
 						   crtc_mask, &config);
 		if (ret != true) {
 			igt_info("Error creating configuration for:\n  ");
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index eda2fcc9212d..fec69254e4da 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1042,7 +1042,8 @@  static void connector_find_preferred_mode(uint32_t connector_id, int crtc_idx,
 {
 	struct kmstest_connector_config config;
 
-	if (!kmstest_get_connector_config(drm_fd, connector_id, 1 << crtc_idx,
+	if (!kmstest_get_connector_config(drm_fd, NULL,
+					  connector_id, 1 << crtc_idx,
 					  &config)) {
 		o->mode_valid = 0;
 		return;
@@ -1086,11 +1087,11 @@  static void connector_find_compatible_mode(int crtc_idx0, int crtc_idx1,
 	drmModeModeInfo *mode[2];
 	int n, m;
 
-	if (!kmstest_get_connector_config(drm_fd, o->_connector[0],
+	if (!kmstest_get_connector_config(drm_fd, NULL, o->_connector[0],
 					  1 << crtc_idx0, &config[0]))
 		return;
 
-	if (!kmstest_get_connector_config(drm_fd, o->_connector[1],
+	if (!kmstest_get_connector_config(drm_fd, NULL, o->_connector[1],
 					  1 << crtc_idx1, &config[1])) {
 		kmstest_free_connector_config(&config[0]);
 		return;
diff --git a/tests/kms_render.c b/tests/kms_render.c
index 72da87f19af3..735a1e117349 100644
--- a/tests/kms_render.c
+++ b/tests/kms_render.c
@@ -210,8 +210,9 @@  static int run_test(const char *test_name, enum test_flags flags)
 		for (j = 0; j < resources->count_crtcs; j++) {
 			struct kmstest_connector_config cconf;
 
-			if (!kmstest_get_connector_config(drm_fd, connector_id,
-							   1 << j, &cconf))
+			if (!kmstest_get_connector_config(drm_fd, NULL,
+							  connector_id,
+							  1 << j, &cconf))
 				continue;
 
 			test_connector(test_name, &cconf, flags);
diff --git a/tests/testdisplay.c b/tests/testdisplay.c
index 00b47bd06280..9d84d8cd4000 100644
--- a/tests/testdisplay.c
+++ b/tests/testdisplay.c
@@ -201,7 +201,7 @@  static void connector_find_preferred_mode(uint32_t connector_id,
 		ret = kmstest_probe_connector_config(drm_fd, connector_id,
 						     crtc_idx_mask, &config);
 	else
-		ret = kmstest_get_connector_config(drm_fd, connector_id,
+		ret = kmstest_get_connector_config(drm_fd, NULL, connector_id,
 						   crtc_idx_mask, &config);
 	if (!ret) {
 		c->mode_valid = 0;