Re: [PATCH 6/6] drm/tiny/gm12u320: convert to struct drm_edid

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

On Wed, 28 Aug 2024, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 22.08.24 um 19:42 schrieb Jani Nikula:
>> Prefer the struct drm_edid based functions for allocating the EDID and
>> updating the connector.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> ---
>>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>>   drivers/gpu/drm/tiny/gm12u320.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
>> index e0defb1d134f..0bd7707c053e 100644
>> --- a/drivers/gpu/drm/tiny/gm12u320.c
>> +++ b/drivers/gpu/drm/tiny/gm12u320.c
>> @@ -464,7 +464,7 @@ static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320)
>>    * Note this assumes this driver is only ever used with the Acer C120, if we
>>    * add support for other devices the vendor and model should be parameterized.
>>    */
>> -static struct edid gm12u320_edid = {
>> +static const struct edid gm12u320_edid = {
>>   	.header		= { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 },
>>   	.mfg_id		= { 0x04, 0x72 },	/* "ACR" */
>>   	.prod_code	= { 0x20, 0xc1 },	/* C120h */
>> @@ -523,8 +523,15 @@ static struct edid gm12u320_edid = {
>>   
>>   static int gm12u320_conn_get_modes(struct drm_connector *connector)
>>   {
>> -	drm_connector_update_edid_property(connector, &gm12u320_edid);
>> -	return drm_add_edid_modes(connector, &gm12u320_edid);
>> +	const struct drm_edid *drm_edid;
>> +	int count;
>> +
>> +	drm_edid = drm_edid_alloc(&gm12u320_edid, sizeof(gm12u320_edid));
>
> Wouldn't it be better to use drm_edid_read_custom() to get the 
> connector's edid override, if any?

It would, no doubt. But it's more work, and the conversion is more error
prone. I don't have the devices to test any of this, and I don't see
many people jumping in to help with that either.

So I'm opting for the minimum but positive incremental improvements with
the drm_edid conversions.

BR,
Jani.


>
> Best regards
> Thomas
>
>> +	drm_edid_connector_update(connector, drm_edid);
>> +	count = drm_edid_connector_add_modes(connector);
>> +	drm_edid_free(drm_edid);
>> +
>> +	return count;
>>   }
>>   
>>   static const struct drm_connector_helper_funcs gm12u320_conn_helper_funcs = {

-- 
Jani Nikula, Intel