Re: [PATCH i-g-t v4 5/5] lib/igt_kms: Add function to get valid pipe for specific output

[Louis Chauvet <louis.chauvet@bootlin.com>]

Le 24/01/2025 à 05:26, Karthik B S a écrit :

>> +/**
>> + * igt_get_pipe_for_output:
>> + * @display: display to fetch the pipes
>> + * @output: output to use
>> + *
>> + * Get a valid pipe for a specific output. The return value is the pipe first valid pipe for a
>> + * specific output.
>> + */
>> +enum pipe igt_get_pipe_for_output(igt_display_t *display,
>> +				  igt_output_t *output)
>> +{
>> +	enum pipe pipe;
>> +
>> +	for_each_pipe(display, pipe) {
>> +		if (igt_output_is_connected(output) &&
>> +		     (output->config.valid_crtc_idx_mask & (1 << (pipe))))
> 
> Please use 'igt_pipe_connector_valid'.

Sorry, I probably extended it to debug something and forgot to unexpand. 
It will be fixed for the next iteration.

> Also, this function is mostly a duplicate of the existing
> 'chamelium_get_pipe_for_output'. Could we reuse the existing function
> here instead of adding a new one?

I know, but:
1 - chamelium_get_pipe_for_output is only enabled with the chamelium 
build option;
2 - chamelium_get_pipe_for_output also enables the pipe during the process.

For 1, I can simply replace all calls to use the new helper.

For 2, I need to understand why they choose to call igt_output_set_pipe 
to get a pipe to see if I can simply replace the chamelium helper by 
mine or if I need to change my helper to do the same thing.

Thanks,
Louis Chauvet

> Thanks,
> Karthik.B.S
>> +			return pipe;
>> +	}
>> +
>> +	return PIPE_NONE;
>> +}


-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com