On Wed, 09 Feb 2022, Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> 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. BR, Jani. > > Sascha > > On Wed, Feb 09, 2022 at 10:53:28AM +0100, Sascha Hauer wrote: >> Add a device node to drm_encoder which corresponds with the port node >> in the DT description of the encoder. This allows drivers to find the >> of_graph link between a crtc and an encoder. >> >> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> >> --- >> include/drm/drm_encoder.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h >> index 6e91a0280f31..3acd054b1eb3 100644 >> --- a/include/drm/drm_encoder.h >> +++ b/include/drm/drm_encoder.h >> @@ -99,6 +99,8 @@ struct drm_encoder { >> struct drm_device *dev; >> struct list_head head; >> >> + struct device_node *port; >> + >> struct drm_mode_object base; >> char *name; >> /** >> -- >> 2.30.2 >> >> -- Jani Nikula, Intel Open Source Graphics Center