[PATCH i-g-t] tests/intel/kms_frontbuffer_tracking: Invalidate cached stuff

Vodapalli, Ravi Kumar ravi.kumar.vodapalli at intel.com
Tue Feb 20 18:10:22 UTC 2024


Hi Gustavo,

Below my inline comments

On 2/20/2024 11:31 PM, Gustavo Sousa wrote:
> +cc Ravi.
>
> Quoting Gustavo Sousa (2024-02-09 10:55:27-03:00)
>> The test pipe-fbc-rte updates the primary mode parameters for each valid
>> dynamic test case. Because of that, we might endup with invalid cached
>> data due to differences between the initial state of prim_mode_params
>> defined from the beginning of the test program and the possibly changed
>> state after pipe-fbc-rte.
>>
>> As an example, in a specific environment, the command
>>
>>   ./build/tests/kms_frontbuffer_tracking \
>>       --run-subtest pipe-fbc-rte,fbc-1p-primscrn-pri-indfb-draw-mmap-wc
>>
>> would result in fbc-1p-primscrn-pri-indfb-draw-mmap-wc failing because
>> it would try to read CRC from pipe B while the test was being actually
>> done in pipe A.
>>
>> Another potential issue worth noting is that even pipe-fbc-rte could
>> similarly fail if the set of dynamic subtests spanned across multiple
>> pipes.
>>
>> Let's fix that by making sure that cached stuff that would depend on the
>> primary mode parameters gets properly invalidated when prim_mode_params
>> is the target of init_mode_params(). This should fix the issues
>> mentioned above and also future-proof the code for any future test that
>> would need to modify the prim_mode_params.
>>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> ---
>> tests/intel/kms_frontbuffer_tracking.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/tests/intel/kms_frontbuffer_tracking.c b/tests/intel/kms_frontbuffer_tracking.c
>> index 912cca3f8d45..17f74990979e 100644
>> --- a/tests/intel/kms_frontbuffer_tracking.c
>> +++ b/tests/intel/kms_frontbuffer_tracking.c
>> @@ -1484,6 +1484,7 @@ static drmModeModeInfo *connector_get_mode(igt_output_t *output)
>> static void init_mode_params(struct modeset_params *params,
>>                               igt_output_t *output, enum pipe pipe)
>> {
>> +        int i;
can you rename variable i to cnt or any meaning full name to that the 
variable is used
>>          drmModeModeInfo *mode;
>>
>>          igt_output_override_mode(output, NULL);
>> @@ -1515,6 +1516,18 @@ static void init_mode_params(struct modeset_params *params,
>>          params->sprite.w = 64;
>>          params->sprite.h = 64;
>>
>> +        /* If we endup changing the primary mode parameters, we need to
>> +         * invalidate any existing cached stuff from a previous configuration. */
>> +        if (params == &prim_mode_params) {
>> +                if (pipe_crc) {
>> +                        igt_pipe_crc_free(pipe_crc);
>> +                        pipe_crc = NULL;
>> +                }
>> +
>> +                for (i = 0; i < FORMAT_COUNT; i++)
>> +                        blue_crcs[i].initialized = false;
>> +        }
>> +
>>          free(mode);
>> }
>> -- 
>> 2.43.0
>>
Thanks,
Ravi kumar V


More information about the igt-dev mailing list