Re: [PATCH v2] drm/gma500: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi

[Thomas Zimmermann]

Hi

Am 12.09.24 um 13:25 schrieb Jani Nikula:
> On Thu, 12 Sep 2024, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
> > Hi
> >
> > Am 12.09.24 um 11:38 schrieb Jani Nikula:
> > > On Thu, 12 Sep 2024, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
> > > > Am 12.09.24 um 10:56 schrieb Jani Nikula:
> > > > > Moreover, in this case .detect() only detects digital displays as
> > > > > reported by EDID. If you postpone that to .get_modes(), the probe helper
> > > > > will still report connected, and invent non-EDID fallback modes. The
> > > > > behaviour changes.
> > > > The change in behavior is intentional, because the current test seems
> > > > arbitrary. Does the driver not work with analog outputs?
> > > Not on a DVI/HDMI port. Same with i915.
> > >
> > > That's possibly the only way to distinguish a DVI-A display connected to
> > > DVI-D source.
> > That's a detect failure, but IMHO our probe helpers should really handle
> > this case.
> How? Allow returning detect failures from .get_modes()?

Something like that, I guess.

For the specific problem it would be enough to read the first 20 bytes 
of EDID data on DVI connectors and test the digital-input flag bit 
against the exact connector requirements. drm_probe_ddc() could do this. 
Non-DVI connectors would continue to read a single bytes to detect the DDC.

For more sophisticated problems, it would be good to introduce an 
intermediate callback that updates the connector state. So the probe 
logic would look like:

 1) call ->detect to read physical connector status
 2) return if physical status did not change
 3) increment epoch counter
 4) call ->update to update connector state and properties (EDID, etc) 
get new connector status
 5) call ->get_modes if connected

The initial ->detect would be minimal. The ->update, if implemented, 
could do more processing and error checking. It's result would be the 
connector's new status.

On a side note, I've recently spend quite a few patches on getting the 
BMC output for ast and mgag200 usable. Something like the above logic 
would have helped, I think. Because with the current probe logic, I had 
to implement steps 1 to 4 in ->detect itself. The result has to maintain 
physical status and epoch counter by itself. [1]

Best regards
Thomas

[1] 
https://gitlab.freedesktop.org/drm/kernel/-/commit/2a2391f857cdc5cf16f8df030944cef8d3d2bc30

> BR,
> Jani.

--
--
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)