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
Attachment:
signature.asc
Description: PGP signature
