On Thu, 26 Oct 2023 at 14:53, Maxime Ripard <mripard@xxxxxxxxxx> wrote: > > On Thu, Oct 26, 2023 at 11:57:22AM +0300, Dmitry Baryshkov wrote: > > On Thu, 26 Oct 2023 at 11:07, Maxime Ripard <mripard@xxxxxxxxxx> wrote: > > > > > > On Thu, Oct 26, 2023 at 01:23:53AM +0300, Dmitry Baryshkov wrote: > > > > > +static int starfive_hdmi_register(struct drm_device *drm, struct starfive_hdmi *hdmi) > > > > > +{ > > > > > + struct drm_encoder *encoder = &hdmi->encoder; > > > > > + struct device *dev = hdmi->dev; > > > > > + > > > > > + encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node); > > > > > + > > > > > + /* > > > > > + * If we failed to find the CRTC(s) which this encoder is > > > > > + * supposed to be connected to, it's because the CRTC has > > > > > + * not been registered yet. Defer probing, and hope that > > > > > + * the required CRTC is added later. > > > > > + */ > > > > > + if (encoder->possible_crtcs == 0) > > > > > + return -EPROBE_DEFER; > > > > > + > > > > > + drm_encoder_helper_add(encoder, &starfive_hdmi_encoder_helper_funcs); > > > > > + > > > > > + hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD; > > > > > + > > > > > + drm_connector_helper_add(&hdmi->connector, > > > > > + &starfive_hdmi_connector_helper_funcs); > > > > > + drmm_connector_init(drm, &hdmi->connector, > > > > > + &starfive_hdmi_connector_funcs, > > > > > + DRM_MODE_CONNECTOR_HDMIA, > > > > > > > > On an embedded device one can not be so sure. There can be MHL or HDMI > > > > Alternative Mode. Usually we use drm_bridge here and drm_bridge_connector. > > > > > > On an HDMI driver, it's far from being a requirement, especially given > > > the limitations bridges have. > > > > It's a blessing that things like MHL / HDMI-in-USB-C / HDMI-to-MyDP > > are not widely used in the wild and are mostly non-existing except > > several phones that preate wide DP usage. > > And those can be supported without relying on bridges. Yes, they likely can, in the way that nouveau handles I2C TV encoders. But I don't think this can scale. We can have different devices attached to the DSI, LVDS, HDMI and even DP image sources. I don't see a scalable solution for either of them. E.g. by switching drm/msm to use panel bridges for DSI panels we were able to significantly unify and simplify code paths. > > Using drm_connector directly prevents one from handling possible > > modifications on the board level. For example, with the DRM connector > > in place, handling a separate HPD GPIO will result in code duplication > > from the hdmi-connector driver. Handling any other variations in the > > board design (which are pretty common in the embedded world) will also > > require changing the driver itself. drm_bridge / drm_bridge_connector > > save us from those issues. > > And we have other solutions there too. Like, EDIDs are pretty much in > the same spot with a lot of device variations, but it also works without > a common driver. I'd really wish we were having less bridges and more > helpers, but here we are. > > > BTW: what are the limitations of the drm_bridge wrt. HDMI output? I'm > > asking because we heavily depend on the bridge infrastructure for HDMI > > output. Maybe we are missing something there, which went unnoticed to > > me and my colleagues. > > A bridge cannot extend the connector state or use properties, for > example. It works for basic stuff but falls apart as soon as you're > trying to do something slightly advanced. Ack. I agree, we didn't have a necessity to implement properties up to now. But that sounds like an interesting topic for DSI-to-HDMI bridges and HDCP support. I'll need to check if any of the RB3/RB5/Dragonboard bridges are programmed with the HDCP keys. -- With best wishes Dmitry