[igt-dev] [PATCH i-g-t v4 3/3] tests: add igt_is_dsc_supported_by_source()

[Kamil Konieczny]

Hi Swati,

one small question, see below.

On 2023-06-14 at 22:24:31 +0530, Swati Sharma wrote:
> Instead of assuming dsc is supported on gen11+ platforms,
> lets use has_dsc flag in i915_capability to check hardware
> support.
> 
> v2: -moved changes to lib (Ankit)
> v3: -keep includes in alphabetical order (Kamil)
>     -don't use igt_require() in lib (Kamil)
>     -improve return statement (Kamil)
> v4: -add debug print (Kamil)
> 
> Cc: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
> Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> ---
>  lib/igt_dsc.c               | 22 ++++++++++++++++++++++
>  lib/igt_dsc.h               |  1 +
>  tests/i915/kms_dsc.c        |  2 +-
>  tests/i915/kms_dsc_helper.c | 10 ++++++++++
>  tests/i915/kms_dsc_helper.h |  1 +
>  tests/kms_invalid_mode.c    |  3 ++-
>  6 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_dsc.c b/lib/igt_dsc.c
> index 6cbd8bae..76a420c1 100644
> --- a/lib/igt_dsc.c
> +++ b/lib/igt_dsc.c
> @@ -6,6 +6,7 @@
>  #include <unistd.h>
>  #include <fcntl.h>
>  #include <string.h>
> +#include "igt_core.h"
>  #include "igt_dsc.h"
>  #include "igt_sysfs.h"
>  
> @@ -41,6 +42,27 @@ static int write_dsc_debugfs(int drmfd, char *connector_name, const char *file_n
>  	return ret;
>  }
>  
> +/*
> + * igt_is_dsc_supported_by_source:
> + * @drmfd: A drm file descriptor
> + *
> + * Returns: True if DSC is supported by source, false otherwise.
> + */
> +bool igt_is_dsc_supported_by_source(int drmfd)
> +{
> +	char buf[4096];
> +	int dir, res;
> +
> +	dir = igt_debugfs_dir(drmfd);
> +	igt_assert(dir >= 0);
> +
> +	res = igt_debugfs_simple_read(dir, "i915_capabilities",
> +				      buf, sizeof(buf));
> +	close(dir);
> +
> +	return res > 0 ? strstr(buf, "has_dsc: yes") : 0;
> +}
> +
>  /*
>   * igt_is_dsc_supported_by_sink:
>   * @drmfd: A drm file descriptor
> diff --git a/lib/igt_dsc.h b/lib/igt_dsc.h
> index 241fc0ac..b58743b5 100644
> --- a/lib/igt_dsc.h
> +++ b/lib/igt_dsc.h
> @@ -9,6 +9,7 @@
>  #include "igt_fb.h"
>  #include "igt_kms.h"
>  
> +bool igt_is_dsc_supported_by_source(int drmfd);
>  bool igt_is_dsc_supported_by_sink(int drmfd, char *connector_name);
>  bool igt_is_fec_supported(int drmfd, char *connector_name);
>  bool igt_is_dsc_enabled(int drmfd, char *connector_name);
> diff --git a/tests/i915/kms_dsc.c b/tests/i915/kms_dsc.c
> index 5b4abd6a..c0707806 100644
> --- a/tests/i915/kms_dsc.c
> +++ b/tests/i915/kms_dsc.c
> @@ -287,7 +287,7 @@ igt_main_args("l", NULL, help_str, opt_handler, &data)
>  		igt_install_exit_handler(kms_dsc_exit_handler);
>  		igt_display_require(&data.display, data.drm_fd);
>  		igt_display_require_output(&data.display);
> -		igt_require(data.disp_ver >= 11);
> +		igt_require(is_dsc_supported_by_source(data.drm_fd));
>  	}
>  
>  	igt_describe("Tests basic display stream compression functionality if supported "
> diff --git a/tests/i915/kms_dsc_helper.c b/tests/i915/kms_dsc_helper.c
> index c90d833d..61f76dde 100644
> --- a/tests/i915/kms_dsc_helper.c
> +++ b/tests/i915/kms_dsc_helper.c
> @@ -53,6 +53,16 @@ void kms_dsc_exit_handler(int sig)
>  	restore_force_dsc_en();
>  }
>  
> +bool is_dsc_supported_by_source(int drmfd)
> +{
> +	if (!igt_is_dsc_supported_by_source(drmfd)) {
> +		igt_debug("DSC not supported by source\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  bool is_dsc_supported_by_sink(int drmfd, igt_output_t *output)
>  {
>  	if (!igt_is_dsc_supported_by_sink(drmfd, output->name)) {
> diff --git a/tests/i915/kms_dsc_helper.h b/tests/i915/kms_dsc_helper.h
> index f66191c7..2109bd76 100644
> --- a/tests/i915/kms_dsc_helper.h
> +++ b/tests/i915/kms_dsc_helper.h
> @@ -27,6 +27,7 @@ void save_force_dsc_en(int drmfd, igt_output_t *output);
>  void restore_force_dsc_en(void);
>  void kms_dsc_exit_handler(int sig);
>  bool is_dsc_supported_by_sink(int drmfd, igt_output_t *output);
> +bool is_dsc_supported_by_source(int drmfd);
>  bool check_gen11_dp_constraint(int drmfd, igt_output_t *output, enum pipe pipe);
>  bool check_gen11_bpc_constraint(int drmfd, igt_output_t *output, int input_bpc);
>  void force_dsc_output_format(int drmfd, igt_output_t *output,
> diff --git a/tests/kms_invalid_mode.c b/tests/kms_invalid_mode.c
> index 63da3a1c..2bb9912f 100644
> --- a/tests/kms_invalid_mode.c
> +++ b/tests/kms_invalid_mode.c
> @@ -60,7 +60,8 @@ can_bigjoiner(data_t *data)
>  	if (intel_display_ver(devid) > 12) {
>  		igt_debug("Platform supports uncompressed bigjoiner\n");

You do not check sink here.

>  		return true;
> -	} else if (intel_display_ver(devid) >= 11) {
> +	} else if (igt_is_dsc_supported_by_source(data->drm_fd)) {
> +		igt_debug("Platform supports compressed bigjoiner\n");
-------------------------- ^
>  		return igt_is_dsc_supported_by_sink(data->drm_fd, data->output->name);
---------------------- ^
Here you printed it supported bigjoiner, yet function may return
false depending on sink ?

Regards,
Kamil

>  	}
>  
> -- 
> 2.25.1
>