[PATCH i-g-t] lib/igt_kms: move setting DRM_CLIENT_CAP_WRITEBACK_CONNECTORS to kms_writeback

[Abhinav Kumar]

Hi Kamil

Thanks for the feedback.

On 5/10/2024 7:29 AM, Kamil Konieczny wrote:
> Hi Abhinav,
> On 2024-05-09 at 13:39:11 -0700, Abhinav Kumar wrote:
>> Currently DRM_CLIENT_CAP_WRITEBACK_CONNECTORS is set in the igt_display_require()
>> which is invoked for all IGT tests irrespective of whether the rest of the
>> writeback connector properties such as the FB_ID are set.
>>
>> For the writeback connectors to function properly, additional setup steps are required
>> (like setting up the output buffers and submitting the job). These steps are not a part
>> of the default IGT setup, so there is no guarantee that the pipeline will
>> be executed at all.
>>
>> It is unclear whether this is intentional to be able to run all IGT tests even
>> with writeback connector or it is only kms_writeback which is supposed to
>> be the one.
>>
>> Lets try with the latter approach by setting DRM_CLIENT_CAP_WRITEBACK_CONNECTORS
>> only within kms_writeback.
>>
>> changes since RFC:
>> 	- minor fixes to commit message
>>
>> Reviewed-by: Rob Clark <robdclark at gmail.com>
>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
>> ---
>>   lib/igt_kms.c         | 2 --
>>   tests/kms_writeback.c | 5 +++++
>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 98d3fb79cd22..f82102144ea1 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -2878,8 +2878,6 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>>   	if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
>>   		display->is_atomic = 1;
>>   
>> -	drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
>> -
>>   	resources = drmModeGetResources(display->drm_fd);
>>   	if (!resources)
>>   		goto out;
>> diff --git a/tests/kms_writeback.c b/tests/kms_writeback.c
>> index 6cd685b37af2..3448dc06f891 100644
>> --- a/tests/kms_writeback.c
>> +++ b/tests/kms_writeback.c
>> @@ -558,6 +558,7 @@ igt_main_args("b:c:f:dl", long_options, help_str, opt_handler, NULL)
>>   	igt_fb_t input_fb, input_fb_10bit;
>>   	drmModeModeInfo mode;
>>   	unsigned int fb_id;
>> +	int ret;
>>   
>>   	memset(&display, 0, sizeof(display));
>>   
>> @@ -570,6 +571,10 @@ igt_main_args("b:c:f:dl", long_options, help_str, opt_handler, NULL)
>>   
>>   		igt_require(display.is_atomic);
>>   
>> +		ret = drmSetClientCap(display.drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
>> +
>> +		igt_require(!ret);
> ---------------------^
> Use igt_require_f() or change 'ret' into has_writeback_connectors
> 

Yes, I will use igt_require_f() and add the relevant message.

> With that it is
> 
> Acked-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> 
> (plus ack from Petri on irc)
> 

Thanks, I have picked up both your acks.

> Regards,
> Kamil
> 
>> +
>>   		output = kms_writeback_get_output(&display);
>>   		igt_require(output);
>>   
>>
>> ---
>> base-commit: 4a5fd4e7cb2798636f6464e2bd61399f3242b322
>> change-id: 20240509-wb_igt_fix-87169651e0da
>>
>> Best regards,
>> -- 
>> Abhinav Kumar <quic_abhinavk at quicinc.com>
>>