On Tue, Aug 22, 2023 at 04:35:55PM +0200, Maxime Ripard wrote: > Hi, > > On Tue, Aug 22, 2023 at 04:16:08PM +0200, Daniel Vetter wrote: > > On Mon, Aug 14, 2023 at 03:56:12PM +0200, Maxime Ripard wrote: > > > Here's a series that creates a subclass of drm_connector specifically > > > targeted at HDMI controllers. > > > > > > The idea behind this series came from a recent discussion on IRC during > > > which we discussed infoframes generation of i915 vs everything else. > > > > > > Infoframes generation code still requires some decent boilerplate, with > > > each driver doing some variation of it. > > > > > > In parallel, while working on vc4, we ended up converting a lot of i915 > > > logic (mostly around format / bpc selection, and scrambler setup) to > > > apply on top of a driver that relies only on helpers. > > > > > > While currently sitting in the vc4 driver, none of that logic actually > > > relies on any driver or hardware-specific behaviour. > > > > > > The only missing piec to make it shareable are a bunch of extra > > > variables stored in a state (current bpc, format, RGB range selection, > > > etc.). > > > > > > Thus, I decided to create some generic subclass of drm_connector to > > > address HDMI connectors, with a bunch of helpers that will take care of > > > all the "HDMI Spec" related code. Scrambler setup is missing at the > > > moment but can easily be plugged in. > > > > > > Last week, Hans Verkuil also expressed interest in retrieving the > > > infoframes generated from userspace to create an infoframe-decode tool. > > > This series thus leverages the infoframe generation code to expose it > > > through debugfs. > > > > > > This entire series is only build-tested at the moment. Let me know what > > > you think, > > > > I think the idea overall makes sense, we we probably need it to roll out > > actual hdmi support to all the hdmi drivers we have. But there's the > > eternal issue of "C sucks at multiple inheritance". > > > > Which means if you have a driver that subclasses drm_connector already for > > it's driver needs it defacto cannot, or only under some serious pains, use > > this. > > That's what vc4 is doing, and it went fine I think? it was mostly a > matter of subclassing drm_hdmi_connector instead of drm_connector, and > adjusting the various pointers and accessors here and there. > > It does create a fairly big diffstat, but nothing too painful. Yeah it's the massive churn that's the pain for refactoring existing bigger drivers. Plus what do you do when you both need a hdmi connector and a dp connector (or a writeback connector). > > Which is kinda why in practice we tend to not subclass, but stuff > > subclass fields into a name sub-structure. So essentially struct > > drm_connector.hdmi and struct drm_connector_state.hdmi instead of > > drm_hdmi_connector and drm_hdmi_connector_state. The helper functions to > > set it all up would all still be the same roughly. It's less typesafe but > > I think the gain in practical use (like you could make i915 use the > > helpers probably, which with this approach here is practically > > impossible). > > Ack. > > > The only other nit is that we probably want to put some of the hdmi > > properties into struct drm_mode_config because there's no reason to have > > per-connector valid values. > > What property would you want to move? The rgb broadcast property looked very much like it's connector invariant. Just the one I noticed, I didn't check all the others. > > Also, it might be really good if you can find a co-conspirator who also > > wants to use this in their driver, then with some i915 extracting we'd > > have three, which should ensure the helper api is solid. > > I can convert sunxi (old) HDMI driver if needed. I'm not sure how > helpful it would be since it doesn't support bpc > 8, but it could be a > nice showcase still for "simple" HDMI controllers. Yeah that might be good. Or perhaps poke Rob Clark whether msm is interested and someone could do a conversion for dpu5 or so? Cheers, Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
