Hi, On Sat, Aug 16, 2025 at 01:20:53AM +0300, Dmitry Baryshkov wrote: > On Thu, Aug 14, 2025 at 05:13:54PM +0100, liviu.du...@arm.com wrote: > > Hi, > > > > On Wed, Aug 13, 2025 at 10:04:22AM +0000, Kandpal, Suraj wrote: > > > > > > }; > > > > > > > > > > I still don't like that. This really doesn't belong here. If anything, > > > > > the drm_connector for writeback belongs to drm_crtc. > > > > > > > > Why? We already have generic HDMI field inside drm_connector. I am > > > > really > > > > hoping to be able to land DP parts next to it. In theory we can have a > > > > DVI- > > > > specific entry there (e.g. with the subconnector type). > > > > The idea is not to limit how the drivers subclass those structures. > > > > > > > > I don't see a good case why WB should deviate from that design. > > > > > > > > > If the issue is that some drivers need a custom drm_connector > > > > > subclass, then I'd rather turn the connector field of > > > > > drm_writeback_connector into a pointer. > > > > > > > > Having a pointer requires additional ops in order to get drm_connector > > > > from > > > > WB code and vice versa. Having drm_connector_wb inside drm_connector > > > > saves us from those ops (which don't manifest for any other kind of > > > > structure). > > > > Nor will it take any more space since union will reuse space already > > > > taken up by > > > > HDMI part. > > > > > > > > > > > > > > > Seems like this thread has died. We need to get a conclusion on the > > > design. > > > Laurent do you have any issue with the design given Dmitry's explanation > > > as to why this > > > Design is good for drm_writeback_connector. > > > > I'm with Laurent here. The idea for drm_connector (and a lot of drm > > structures) are to > > be used as base "classes" for extended structures. I don't know why HDMI > > connector ended > > up inside drm_connector as not all connectors have HDMI functionality, but > > that's a cleanup > > for another day. > > Maybe Maxime can better comment on it, but I think it was made exactly > for the purpose of not limiting the driver's design. For example, a lot > of drivers subclass drm_connector via drm_bridge_connector. If > struct drm_connector_hdmi was a wrapper around struct drm_connector, > then it would have been impossible to use HDMI helpers for bridge > drivers, while current design freely allows any driver to utilize > corresponding library code.
That's exactly why we ended up like this. With that design, we wouldn't have been able to "inherit" two connector "classes": bridge_connector is one, intel_connector another one. See here for the rationale: https://lore.kernel.org/dri-devel/ZOTDKHxn2bOg+Xmg@phenom.ffwll.local/ I don't think the "but we'll bloat drm_connector" makes sense either. There's already a *lot* of things that aren't useful to every connector (fwnode, display_info, edid in general, scaling, vrr, etc.) And it's not like we allocate more than a handful of them during a system's life. Maxime
signature.asc
Description: PGP signature
