[PATCH 03/28] drm/writeback: Define function to get drm_connector from writeback

[Dmitry Baryshkov]

On Fri, Aug 01, 2025 at 02:57:48PM +0300, Jani Nikula wrote:
> On Fri, 01 Aug 2025, Dmitry Baryshkov <dmitry.baryshkov at oss.qualcomm.com> wrote:
> > Thinking in OOP terms, the encoder is just a field in the struct.
> > drm_connector is a base class for drm_writeback_connector. By making it
> > optional, you are definitely semi-breaking the abstraction.
> 
> The trouble is, in OOP terms, drm_connector is the "base class" for both
> drm_writeback_connector and intel_connector. We're already stretching
> what we can do with C.
> 
> Currently, it's always guaranteed all drm_connectors i915 ever sees are
> embedded within intel_connector. Changing from one pointer to another is
> trivial, guaranteed to work, and is never NULL if the source pointer is
> non-NULL. This is a design that's been around for the longest time.
> 
> The current writeback implementation forces a different design by always
> embedding drm_connector itself. We can't use a drm_connector that's
> embedded within an intel_connector with it. If we want to have our own
> stuff, we'd need an intel_writeback_connector wrapping
> drm_writeback_connector, and it gets even more complicated with all the
> interfaces that use intel_connector. It really shouldn't have to be this
> way.
> 
> Using the current drm_writeback_connector in i915 requires careful
> auditing of all drm_connector <-> intel_connector conversions, NULL
> checks, and graceful error handling, also in places that have no
> convenient way to return errors at all.
> 
> The OOP abstractions just break hard with C, we can't have multiple
> inheritance, and IMO the pragmatic approach is to let *drivers* do what
> they want, instead of having a midlayer helper design force something on
> them.

Yes. Basically, that's why I suggest refacoring drm_writeback_connector
to mvoe drm_connector_writeback into drm_connector itself (like it's done
for drm_connector_hdmi).

-- 
With best wishes
Dmitry