[PATCH v5 01/23] drm/encoder: Add of_graph port to struct drm_encoder

[Sascha Hauer]

On Wed, Feb 09, 2022 at 01:12:45PM +0200, Jani Nikula wrote:
> On Wed, 09 Feb 2022, Sascha Hauer <s.hauer at pengutronix.de> wrote:
> > David, Daniel,
> >
> > I'll need a word from you regarding this patch. It's needed in patch
> > 22/23 in this series.
> > vop2_crtc_atomic_enable() needs to control the mux which routes the
> > display output to the different encoders. Which encoder is used is
> > described in the of_graph port, so I need a way to identify the encoder
> > in the device tree.
> 
> I think the question is how useful is this going to be in general. IMO
> we should not be adding members that are useful in a single driver only.
> 
> For example i915 wraps encoders with:
> 
> 	struct intel_encoder {
> 		struct drm_encoder base;
> 
> 		/* i915 specific stuff here*/
> 	};
> 
> So that we can add stuff of our own there. Of course, it does mean a
> bunch of overhead for the first time you need to do it. But adding
> driver specific stuff to struct drm_encoder adds overhead for everyone.
> 
> All that said, *I* don't know how useful the port member would be in
> drivers that use device tree. Maybe it's worth it.

I don't know either.

Right now the drm_encoder is directly embedded into the encoder drivers
private data structures, like this:

struct rockchip_hdmi {
        struct drm_encoder encoder;
	...
};

I could change this to:

struct rockchip_encoder {
	struct device_node *port;
	struct drm_encoder encoder;
}

and then

struct rockchip_hdmi {
	struct rockchip_encoder encoder;
	...
};

That would solve the issue without touching generic DRM code if that's
preferred.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |