Re: [PATCH i-g-t v9 29/49] lib/unigraf: Add helpers to read and write edid

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

On 3/17/26 16:32, Kory Maincent wrote:
> On Mon, 16 Mar 2026 17:17:50 +0100
> Louis Chauvet <louis.chauvet@bootlin.com> wrote:
> 
>> Read and writing EDID on unigraf can be useful to emulate different
>> screens, add helpers to read and write them.
>>
>> There is a limitation on the libTSI.so, it is only possible to read EDID
>> using buffers of size multiple of 128 bytes, and there is no way to query
>> the actual EDID size. The current implementation can only read EDID up to
>> 2048 bytes.
>>
>> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
>> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>> ---
>>   lib/unigraf/unigraf.c | 59
>> +++++++++++++++++++++++++++++++++++++++++++++++++++ lib/unigraf/unigraf.h |
>> 4 ++++ 2 files changed, 63 insertions(+)
>>
>> diff --git a/lib/unigraf/unigraf.c b/lib/unigraf/unigraf.c
>> index b752ff573ace..d06da6742e95 100644
>> --- a/lib/unigraf/unigraf.c
>> +++ b/lib/unigraf/unigraf.c
>> @@ -56,6 +56,11 @@ static char *unigraf_connector_name;
>>    */
>>   #define UNIGRAF_DEFAULT_INPUT_NAME "DP RX"
>>   
>> +/**
>> + * EDID_MAX_SIZE - Max EDID size that can be read from the unigraf. The
>> + */
> 
> The ? Missing a sentence?
> 
>> +#define EDID_MAX_SIZE 2048
>> +
>>   static void unigraf_close_device(void)
>>   {
>>   	if (!unigraf_device)
>> @@ -127,6 +132,20 @@ static void unigraf_init(void)
>>   		value;
>> 						\ })
>>   
> 
> ...
> 
>> +
>> +/**
>> + * unigraf_write_edid() - Write EDID data to the specified stream
>> + * @stream: The stream ID to write the EDID to
>> + * @edid: Pointer to the EDID structure to write
>> + * @edid_size: Size of the EDID data in bytes
>> + *
>> + * This function writes the provided EDID data to the specified stream.
>> + */
>> +void unigraf_write_edid(uint32_t stream, const struct edid *edid, uint32_t
>> edid_size) +{
> 
> edid_size not used in the function.

True, I added a min(edid_size, EDID_MAX_SIZE) to avoid out of bound 
accesses.

>> +	unigraf_debug("Write EDID for stream %d...\n", stream);
>> +
>> +	unigraf_write_u32(TSI_EDID_SELECT_STREAM, stream);
>> +	unigraf_write(TSI_EDID_TE_INPUT, edid, 2048);
> 
> Use EDID_MAX_SIZE here or the edid_size parameter?

Thanks!

> Regards,