[PATCH v5 12/13] drm/ast: Implement polling for VGA and SIL164 connectors

[Thomas Zimmermann]

Hi

Am 21.03.24 um 15:09 schrieb Maxime Ripard:
> Hi,
>
> On Wed, Mar 20, 2024 at 10:34:17AM +0100, Thomas Zimmermann wrote:
>> +/**
>> + * drm_connector_helper_detect_ctx - Read EDID and detect connector status.
>> + * @connector: The connector
>> + * @ctx: Acquire context
>> + * @force: Perform screen-destructive operations, if necessary
>> + *
>> + * Detects the connector status by reading the EDID using drm_probe_ddc(),
>> + * which requires connector->ddc to be set. Returns connector_status_connected
>> + * on success or connector_status_disconnected on failure.
>> + *
>> + * Returns:
>> + * The connector status as defined by enum drm_connector_status.
>> + */
>> +int drm_connector_helper_detect_ctx(struct drm_connector *connector,
>> +				    struct drm_modeset_acquire_ctx *ctx,
>> +				    bool force)
>> +{
>> +	struct i2c_adapter *ddc = connector->ddc;
>> +
>> +	if (!ddc)
>> +		return connector_status_unknown;
>> +
>> +	if (drm_probe_ddc(ddc))
>> +		return connector_status_connected;
>> +
>> +	return connector_status_disconnected;
>> +}
>> +EXPORT_SYMBOL(drm_connector_helper_detect_ctx);
> I think it would be better to make it more obvious that we rely on DDC
> to detect and we shouldn't consider it a generic helper that would work
> in all cases.
>
> drm_connector_helper_detect_probe_ddc maybe?

No objection from me about mentioning DDC. What what about 
drm_connector_helper_get_modes()? It relies on DDC as well, so I thought 
that that's the default. Should we consider renaming it?

Best regards
Thomas

>
> Maxime

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)