Re: [igt-dev] [PATCH i-g-t] tests/kms_plane: require GAMMA_LUT property for legacy_lut tests

["Maíra Canal" <mcanal@igalia.com>]

On 4/24/23 15:18, Ville Syrjälä wrote:
> On Mon, Apr 24, 2023 at 02:24:07PM -0300, Maíra Canal wrote:
>> Currently, the tests pixel-format and pixel-format-source-clamping
>> fail for drivers that doesn't have the IGT_CRTC_GAMMA_LUT property,
>> as they are not able to complete drmModeCrtcSetGamma() successfully.
>> Therefore, require the IGT_CRTC_GAMMA_LUT property before running the
>> tests. This way the tests can skip gracefully for drivers without
>> this property.
>>
>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>> ---
>>   tests/kms_plane.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
>> index 62aee9ad..a0e4e80f 100644
>> --- a/tests/kms_plane.c
>> +++ b/tests/kms_plane.c
>> @@ -451,6 +451,8 @@ static void set_legacy_lut(data_t *data, enum pipe pipe,
>>   	uint16_t *lut;
>>   	int i, lut_size;
>>   
>> +	igt_require(igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_GAMMA_LUT));
> 
> Skipping all pixel format tests isn't a great plan.

I'm not sure if we can perform the tests without the legacy LUT, as
we need it to drop the precision down so that errors caused by the
RGB <-> YCbCr conversion end up being ignored.

Currently, I'm performing this test in VKMS and it is failing. As
VKMS don't provide the GAMMA_LUT property, the test simply fails
when hits the drmModeCrtcSetGamma() function. I believe that it
would be better to skip gracefully than fail, considering that
in both cases the pixel format tests are not being executed.

Best Regards,
- Maíra Canal

> 
> Also, you want to check crtc->gamma_size intead. That is what
> indicates support for the legacy gamma.
> 
>> +
>>   	crtc = drmModeGetCrtc(data->drm_fd, pipe_obj->crtc_id);
>>   	lut_size = crtc->gamma_size;
>>   	drmModeFreeCrtc(crtc);
>> @@ -474,6 +476,8 @@ static bool set_c8_legacy_lut(data_t *data, enum pipe pipe,
>>   	uint16_t *r, *g, *b;
>>   	int i, lut_size;
>>   
>> +	igt_require(igt_pipe_obj_has_prop(pipe_obj, IGT_CRTC_GAMMA_LUT));
>> +
>>   	crtc = drmModeGetCrtc(data->drm_fd, pipe_obj->crtc_id);
>>   	lut_size = crtc->gamma_size;
>>   	drmModeFreeCrtc(crtc);
>> -- 
>> 2.40.0
>