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

[Maxime Ripard]

On Fri, Mar 22, 2024 at 07:40:52AM +0100, Thomas Zimmermann wrote:
> 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?

I see your point, but I think it's not totally in the same situation.
Unless you have a fixed mode panel (and even then, some support EDID),
EDID is the de-facto standard to retrieve the modes from the attached
monitor.

You don't really have that standardization with monitor detection:
probing the DDC bus is one of the many valid methods to do so, along
with reading the HPD register, reading a GPIO state, etc.

So I would say it's much more critical for the detect helper than it is
for the get_modes one. But we could totally do it for get_modes too to
make it somewhat consistent.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 273 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240322/068f65cd/attachment.sig>