On Tue, 14 Nov 2023, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
> Hi
>
> Am 14.11.23 um 11:58 schrieb Jani Nikula:
>> Going forward, the struct drm_edid based functions drm_edid_read(),
>> drm_edid_connector_update() and drm_edid_connector_add_modes() are the
>> preferred ways of retrieving the EDID and updating the connector.
>>
>> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>
> If I'm not mistaken, the correct pattern is to read the EDID block in
> the detect callback and only parse it for modes in get_modes. If so, you
> might also inline this helper into its only caller in mgag200. I'll
> later split it up into detect and get_modes.
That's what we do in i915 anyway.
drm_edid_read() and drm_edid_connector_update() in ->detect and ->force,
and drm_edid_connector_add_modes() in ->get_modes.
BR,
Jani.
>
> Best regards
> Thomas
>
>> ---
>> drivers/gpu/drm/drm_probe_helper.c | 17 ++++++++---------
>> 1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
>> index 3f479483d7d8..309d88f13648 100644
>> --- a/drivers/gpu/drm/drm_probe_helper.c
>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>> @@ -1116,21 +1116,20 @@ EXPORT_SYMBOL(drm_crtc_helper_mode_valid_fixed);
>> */
>> int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector)
>> {
>> - struct edid *edid;
>> - int count = 0;
>> + const struct drm_edid *drm_edid;
>> + int count;
>>
>> if (!connector->ddc)
>> return 0;
>>
>> - edid = drm_get_edid(connector, connector->ddc);
>> + drm_edid = drm_edid_read(connector);
>> +
>> + /* clears property if EDID is NULL */
>> + drm_edid_connector_update(connector, drm_edid);
>>
>> - // clears property if EDID is NULL
>> - drm_connector_update_edid_property(connector, edid);
>> + count = drm_edid_connector_add_modes(connector);
>>
>> - if (edid) {
>> - count = drm_add_edid_modes(connector, edid);
>> - kfree(edid);
>> - }
>> + drm_edid_free(drm_edid);
>>
>> return count;
>> }
--
Jani Nikula, Intel
