Re: [PATCH i-g-t 08/10] lib/kms: Introduce igt_output_set_crtc()

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 10 Dec 2025, Jani Nikula <jani.nikula@intel.com> wrote:
> On Wed, 10 Dec 2025, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Wed, 10 Dec 2025, Jani Nikula <jani.nikula@intel.com> wrote:
>>> On Wed, 10 Dec 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>>>> +/**
>>>> + * igt_output_set_pipe:
>>>> + * @output: Target output for which the pipe is being set to
>>>> + * @pipe_obj: Display pipe to set to
>>>> + *
>>>> + * This function sets a @pipe to a specific @output connector by
>>>> + * setting the CRTC_ID property of the @pipe. The pipe
>>>> + * is only activated for all pipes except PIPE_NONE.
>>>> + */
>>>> +void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
>>>> +{
>>>> +	igt_output_set_crtc(output, igt_crtc_for_pipe(output->display, pipe));
>>>
>>> How does that work for the igt_output_set_pipe(output, PIPE_NONE) calls?
>>> Seems like the idea was to pass NULL here? Or should igt_crtc_for_pipe()
>>> handle PIPE_NONE and return NULL?
>>
>> Okay, so it's just this one commit that's busted, as the whole function
>> is removed in the next patch.
>
> I think igt_crtc_for_pipe() should handle PIPE_NONE, as there are a
> bunch of places in the next patch that pass, say, data->pipe_id to it,
> and it's a nightmare to check where all those come from.

Finishing my monologue:

On the condition that igt_crtc_for_pipe() is modified to return NULL for
PIPE_NONE,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


-- 
Jani Nikula, Intel