On Thu, 02 Jun 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Tue, May 24, 2022 at 01:39:28PM +0300, Jani Nikula wrote:
>> Add default action when .get_modes() not set. This also defines what a
>> .get_modes() hook should do.
>>
>> Cc: David Airlie <airlied@xxxxxxxx>
>> Cc: Daniel Vetter <daniel@xxxxxxxx>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/drm_probe_helper.c | 14 +++++++++++++-
>> include/drm/drm_modeset_helper_vtables.h | 4 ++++
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
>> index 836bcd5b4cb6..9df17f0ae225 100644
>> --- a/drivers/gpu/drm/drm_probe_helper.c
>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>> @@ -360,7 +360,19 @@ static int drm_helper_probe_get_modes(struct drm_connector *connector)
>> connector->helper_private;
>> int count;
>>
>> - count = connector_funcs->get_modes(connector);
>> + if (connector_funcs->get_modes) {
>> + count = connector_funcs->get_modes(connector);
>> + } else {
>> + const struct drm_edid *drm_edid;
>> +
>> + /* Note: This requires connector->ddc is set */
>> + drm_edid = drm_edid_read(connector);
>> +
>> + /* Update modes etc. from the EDID */
>> + count = drm_edid_connector_update(connector, drm_edid);
>> +
>> + drm_edid_free(drm_edid);
>> + }
>
> Not really a huge fan of this automagic fallback. I think I'd prefer
> to keep it mandatory and just provide this as a helper for drivers to
> plug into the right spot. Otherwise I'm sure I'll forget how this works
> and then I'll be confused when I see a connector withput any apparent
> .get_modes() implementation.
Fair enough.
I'm not sure how useful that is going to be, though, at least not for
i915. If we add a .get_edid hook, where would you bolt that? It doesn't
feel right to set a .get_modes hook to a default function that goes on
to call a .get_edid hook. Or what do you think?
BR,
Jani.
>
>>
>> /*
>> * Fallback for when DDC probe failed in drm_get_edid() and thus skipped
>> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
>> index fafa70ac1337..b4402bc64e57 100644
>> --- a/include/drm/drm_modeset_helper_vtables.h
>> +++ b/include/drm/drm_modeset_helper_vtables.h
>> @@ -894,6 +894,10 @@ struct drm_connector_helper_funcs {
>> * libraries always call this with the &drm_mode_config.connection_mutex
>> * held. Because of this it's safe to inspect &drm_connector->state.
>> *
>> + * This callback is optional. By default, it reads the EDID using
>> + * drm_edid_read() and updates the connector display info, modes, and
>> + * properties using drm_edid_connector_update().
>> + *
>> * RETURNS:
>> *
>> * The number of modes added by calling drm_mode_probed_add().
>> --
>> 2.30.2
--
Jani Nikula, Intel Open Source Graphics Center
