[igt-dev] [PATCH i-g-t v3 3/3] tests: add igt_is_dsc_supported_by_source()
Sharma, Swati2
swati2.sharma at intel.com
Wed Jun 14 09:33:55 UTC 2023
On 14-Jun-23 2:52 PM, Kamil Konieczny wrote:
> Hi Swati,
>
> On 2023-06-14 at 10:30:25 +0530, Sharma, Swati2 wrote:
>>
>> On 13-Jun-23 4:31 PM, Kamil Konieczny wrote:
>>> Hi Swati,
>>>
>>> On 2023-06-02 at 12:54:01 +0530, 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) {
>>> -------------------------------------------- ^^^^
>>>
>>> If you insist on removing this check above " >= 11"
>>> maybe also remove check of " > 12 " ? Then you could just write:
>>>
>>> if (igt_is_dsc_supported_by_source(data->drm_fd) &&
>>> igt_is_dsc_supported_by_sink(data->drm_fd, data->output->name)) {
>>>
>>> igt_debug("Platform supports uncompressed bigjoiner\n");
>>> return true;
>>> }
>>
>> There are 2 things here:
>> 1. if disp_ver > 12 then platform supports "uncompressed" big joiner, here
>> we don't have to check dsc support since its uncompressed.
>> 2. if disp_ver >= 11 && disp_ver <= 12 then platform supports "compressed"
>> big joiner and we need to check dsc support which instead of using via gen
>> check we are checking with debugfs now.
>>
>> So, big joiner check we still need only dsc check is not required.
>
> Than you for explanation, maybe it will be worth to add
> here some comment or igt_debug print line in > 12 case,
> it is not a blocker,
>
> Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
Thanks Kamil for the review. Will surely add it as igt_debug print
line.
>
>>
>>>
>>> This way you could also support any future change when new
>>> display will not support it.
>>>
>>> Regards,
>>> Kamil
>>>
>>>>> ---------------------------------------------- ^
>>>>> 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.
>>>>
>>>>>
>>>>>> + } 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