[igt-dev] [PATCH i-g-t v3 3/3] tests: add igt_is_dsc_supported_by_source()
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Wed Jun 14 12:43:21 UTC 2023
On 6/2/2023 12:54 PM, Sharma, Swati2 wrote:
> Hi Kamil,
>
> Thanks for the reviews. Please find my reply inline.
>
> On 01-Jun-23 7:25 PM, Kamil Konieczny wrote:
>> Hi Swati,
>>
>> On 2023-05-31 at 12:24:07 +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)
>>>
>>> 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>
>>> ---
>>> 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, 36 insertions(+), 3 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 d142fae2..b46c4449 100644
>>> --- a/tests/i915/kms_dsc.c
>>> +++ b/tests/i915/kms_dsc.c
>>> @@ -265,7 +265,7 @@ igt_main
>>> 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..e4ab65f2 100644
>>> --- a/tests/kms_invalid_mode.c
>>> +++ b/tests/kms_invalid_mode.c
>>> @@ -60,9 +60,8 @@ can_bigjoiner(data_t *data)
>>> if (intel_display_ver(devid) > 12) {
>>> igt_debug("Platform supports uncompressed bigjoiner\n");
>>> return true;
>>> - } else if (intel_display_ver(devid) >= 11) {
>> ---------------------------------------------- ^
>> imho you should keep it here unless there are platforms <= 10 with
>> that capability ?
>
> i915 driver supports dsc from display_ver >= 11
> The whole point of removing this check is to get info from i915_cap to
> know if h/w supports dsc or not.
>
>> Also previous if had braces { } so you should also use them after else:
>
> okay.
LGTM.
As suggested by Kamil, should have braces if previous if block had braces.
With that fixed this is:
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>
>>
>>> + } else if (igt_is_dsc_supported_by_source(data->drm_fd))
>> ------------- ^
>>> return igt_is_dsc_supported_by_sink(data->drm_fd,
>>> data->output->name);
>>> - }
>>
>> imho better:
>> } else if (intel_display_ver(devid) >= 11) {
>> return igt_is_dsc_supported_by_source(data->drm_fd) &&
>> igt_is_dsc_supported_by_sink(data->drm_fd,
>> data->output->name);
>> }
>>
>> Regards,
>> Kamil
>>
>>> - }
>>> return false;
>>> }
>>> --
>>> 2.25.1
>>>
More information about the igt-dev
mailing list