Re: [PATCH i-g-t v4 24/46] lib/unigraf: Add reset function

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

On Mon Jan 26, 2026 at 11:45 AM CET, Louis Chauvet wrote:
>
>
> On 1/23/26 18:39, Luca Ceresoli wrote:
>> On Mon Nov 10, 2025 at 2:39 PM CET, Louis Chauvet wrote:
>>> To have a consistent state at the beginning of tests, add a unigraf_reset
>>> function that will restore a known state.
>>>
>>> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
>>> ---
>>>   lib/unigraf/unigraf.c | 5 +++++
>>>   lib/unigraf/unigraf.h | 9 +++++++++
>>>   2 files changed, 14 insertions(+)
>>>
>>> diff --git a/lib/unigraf/unigraf.c b/lib/unigraf/unigraf.c
>>> index 59a85e1b84c9..e96c7fc3a91e 100644
>>> --- a/lib/unigraf/unigraf.c
>>> +++ b/lib/unigraf/unigraf.c
>>> @@ -278,6 +278,8 @@ bool unigraf_open_device(int drm_fd)
>>>   	unigraf_assert(TSIX_VIN_Select(unigraf_device, chosen_input));
>>>   	unigraf_assert(TSIX_VIN_Enable(unigraf_device, chosen_input));
>>>
>>> +	unigraf_reset();
>>> +
>>>   	return true;
>>>   }
>>>
>>> @@ -285,3 +287,6 @@ void unigraf_require_device(int drm_fd)
>>>   {
>>>   	igt_require(unigraf_open_device(drm_fd));
>>>   }
>>> +
>>> +void unigraf_reset(void)
>>> +{}
>>
>> Weird implementation :) But OK, code is added in patch 26.
>
> Patch 26 also introduce a different set of features. I wanted to avoid
> mixing the reset creation with it to simplify review and rebases.

Yes, sure, make sense! I was just looking funny :)

>>> diff --git a/lib/unigraf/unigraf.h b/lib/unigraf/unigraf.h
>>> index a93bd464bac3..76f4f8878c2c 100644
>>> --- a/lib/unigraf/unigraf.h
>>> +++ b/lib/unigraf/unigraf.h
>>> @@ -48,4 +48,13 @@ bool unigraf_open_device(int drm_fd);
>>>    */
>>>   void unigraf_require_device(int drm_fd);
>>>
>>> +/**
>>> + * unigraf_reset() - Reset the Unigraf device
>>> + *
>>> + * This function performs a hardware reset of the Unigraf device, restoring it to a
>>> + * default state. This includes resetting all configuration parameters, stream settings,
>>> + * and link parameters to default values.
>>> + */
>>> +void unigraf_reset(void);
>>
>> Why declaring it in a header file when it's implemented and used in the
>> same .c file? It should rather be static.
>
> Because I use it in the last patch of the series in
> tests/unigraf/unigraf_lt and I wanted to avoid changing the function later.

Ah, OK, it's fine then. I still haven't reached patch 46.

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com