On Tue, 14 May 2024, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
> Hi
>
> Am 14.05.24 um 14:55 schrieb Jani Nikula:
>> Prefer the struct drm_edid based functions for reading the EDID and
>> updating the connector.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>>
>> ---
>>
>> Cc: Xinliang Liu <xinliang.liu@xxxxxxxxxx>
>> Cc: Tian Tao <tiantao6@xxxxxxxxxxxxx>
>> Cc: Xinwei Kong <kong.kongxinwei@xxxxxxxxxxxxx>
>> Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
>> Cc: Yongqin Liu <yongqin.liu@xxxxxxxxxx>
>> Cc: John Stultz <jstultz@xxxxxxxxxx>
>> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> Cc: Maxime Ripard <mripard@xxxxxxxxxx>
>> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> ---
>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 17 ++++++++++-------
>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> index 94e2c573a7af..409c551c92af 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> @@ -24,14 +24,16 @@
>>
>> static int hibmc_connector_get_modes(struct drm_connector *connector)
>
> Could this function be replaced with a call to
> drm_connector_helper_get_modes()? With the patch, the only difference is
> the call to the _noedid() function, which the DRM core does
> automatically. There will still be a difference in the maximum
> resolution, but standardizing on 1024x768 seems preferable to me.
Looks like it might work, but personally, I'm trying to shy away from
any further cleanups than just switching to struct drm_edid. I've got my
plate pretty full already.
BR,
Jani.
>
> Best regards
> Thomas
>
>> {
>> - int count;
>> - void *edid;
>> struct hibmc_connector *hibmc_connector = to_hibmc_connector(connector);
>> + const struct drm_edid *drm_edid;
>> + int count;
>> +
>> + drm_edid = drm_edid_read_ddc(connector, &hibmc_connector->adapter);
>>
>> - edid = drm_get_edid(connector, &hibmc_connector->adapter);
>> - if (edid) {
>> - drm_connector_update_edid_property(connector, edid);
>> - count = drm_add_edid_modes(connector, edid);
>> + drm_edid_connector_update(connector, drm_edid);
>> +
>> + if (drm_edid) {
>> + count = drm_edid_connector_add_modes(connector);
>> if (count)
>> goto out;
>> }
>> @@ -42,7 +44,8 @@ static int hibmc_connector_get_modes(struct drm_connector *connector)
>> drm_set_preferred_mode(connector, 1024, 768);
>>
>> out:
>> - kfree(edid);
>> + drm_edid_free(drm_edid);
>> +
>> return count;
>> }
>>
--
Jani Nikula, Intel
