On Sat, Jul 26, 2025 at 04:43:11PM +0000, Kandpal, Suraj wrote:
>
>
> > -----Original Message-----
> > From: Dmitry Baryshkov <dmitry.barysh...@oss.qualcomm.com>
> > Sent: Saturday, July 26, 2025 5:50 PM
> > To: Kandpal, Suraj <suraj.kand...@intel.com>
> > Cc: dri-devel@lists.freedesktop.org; intel...@lists.freedesktop.org; intel-
> > g...@lists.freedesktop.org; Nautiyal, Ankit K <ankit.k.nauti...@intel.com>;
> > Murthy, Arun R <arun.r.mur...@intel.com>; Shankar, Uma
> > <uma.shan...@intel.com>
> > Subject: Re: [PATCH 02/28] drm/writeback: Add a helper function to get
> > writeback connector
> >
> > On Fri, Jul 25, 2025 at 10:33:43AM +0530, Suraj Kandpal wrote:
> > > Now that we can initialize a drm_writeback_connector without having to
> > > initialize the drm_connector within it and leaving the responsibility
> > > of initialising the drm_connector and maintaining the association with
> > > drm_writeback_connector to it. This helper hooks lets drivers return
> > > the drm_writeback_connector associated with the give drm_connector.
> > >
> > > Signed-off-by: Suraj Kandpal <suraj.kand...@intel.com>
> > > ---
> > > drivers/gpu/drm/drm_writeback.c | 14 ++++++
> > > include/drm/drm_modeset_helper_vtables.h | 59
> > ++++++++++++++++++++++++
> > > include/drm/drm_writeback.h | 14 ++++--
> > > 3 files changed, 82 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_writeback.c
> > > b/drivers/gpu/drm/drm_writeback.c index fa58eb0dc7bf..e9f7123270d6
> > > 100644
> > > --- a/drivers/gpu/drm/drm_writeback.c
> > > +++ b/drivers/gpu/drm/drm_writeback.c
> > > @@ -107,6 +107,19 @@ static const struct dma_fence_ops
> > drm_writeback_fence_ops = {
> > > .get_timeline_name = drm_writeback_fence_get_timeline_name,
> > > };
> > >
> > > +struct drm_writeback_connector *
> > > +drm_connector_to_writeback(struct drm_connector *connector) {
> > > + const struct drm_connector_helper_funcs *funcs =
> > > + connector->helper_private;
> > > +
> > > + if (funcs->get_writeback_connector)
> > > + return funcs->get_writeback_connector(connector);
> > > +
> > > + return container_of(connector, struct drm_writeback_connector,
> > > +base); } EXPORT_SYMBOL(drm_connector_to_writeback);
> > > +
> > > static int create_writeback_properties(struct drm_device *dev) {
> > > struct drm_property *prop;
> > > @@ -443,6 +456,7 @@ drm_writeback_connector_init_with_conn(struct
> > drm_device *dev, struct drm_connec
> > > struct drm_writeback_connector
> > *wb_connector,
> > > struct drm_encoder *enc,
> > > const struct drm_connector_funcs
> > *con_funcs,
> > > + const struct
> > drm_writeback_connector_helper_funcs
> > > +*wb_funcs,
> > > const u32 *formats, int n_formats) {
> > > struct drm_property_blob *blob;
> > > diff --git a/include/drm/drm_modeset_helper_vtables.h
> > > b/include/drm/drm_modeset_helper_vtables.h
> > > index ce7c7aeac887..6b89b33d2304 100644
> > > --- a/include/drm/drm_modeset_helper_vtables.h
> > > +++ b/include/drm/drm_modeset_helper_vtables.h
> > > @@ -31,6 +31,7 @@
> > >
> > > #include <drm/drm_crtc.h>
> > > #include <drm/drm_encoder.h>
> > > +#include <drm/drm_writeback.h>
> > >
> > > /**
> > > * DOC: overview
> > > @@ -1179,6 +1180,25 @@ struct drm_connector_helper_funcs {
> > > *
> > > */
> > > void (*disable_hpd)(struct drm_connector *connector);
> > > +
> > > + /**
> > > + * @get_writeback_connector:
> > > + *
> > > + * This callback is used by drivers to get the writeback connector in
> > > + * case the init is done via drm_writeback_init_with_conn. Which
> > means
> > > + * the drivers don't have drm_connector embedded in
> > drm_writeback_connector
> > > + * so they need to send the associated writeback connector with this
> > > + * function.
> > > + *
> > > + * This operation is optional.
> > > + *
> > > + * This is mainly called from drm_writeback_set_gb.
> > > + *
> > > + * RETURNS:
> > > + *
> > > + * drm_writeback_connector assoiciated with the drm connector.
> > > + */
> > > + struct drm_writeback_connector *(*get_writeback_connector)(struct
> > > +drm_connector *connector);
> > > };
> > >
> > > /**
> > > @@ -1192,6 +1212,45 @@ static inline void
> > drm_connector_helper_add(struct drm_connector *connector,
> > > connector->helper_private = funcs;
> > > }
> > >
> > > +/**
> > > + * struct drm_writeback_connector_helper_funcs - helper operations
> > > +for writeback
> > > + * connectors.
> > > + *
> > > + * These functions are used by the atomic and legacy modeset helpers
> > > +and by the
> > > + * probe helpers.
> > > + */
> > > +struct drm_writeback_connector_helper_funcs {
> > > + /**
> > > + * @get_connector_from_writeback:
> > > + *
> > > + * This callback is used by drivers to get the drm_connector in
> > > + * case the init is done via drm_writeback_init_with_conn. Which
> > means
> > > + * the drivers don't have drm_connector embedded in
> > drm_writeback_connector
> > > + * so they need to send the associated drm_connector with this
> > > + * function.
> > > + *
> > > + * This operation is optional.
> > > + *
> > > + * RETURNS:
> > > + *
> > > + * drm_connector assoiciated with the drm_writeback_connector.
> > > + */
> > > + struct drm_connector
> > > + *(*get_connector_from_writeback)(struct drm_writeback_connector
> > > +*wbconnector); };
> > > +
> > > +/**
> > > + * drm_writeback_connector_helper_add - sets the helper vtable for a
> > > +connector
> > > + * @wb_connector: DRM writeback connector
> > > + * @funcs: helper vtable to set for @wb_connector */ static inline
> > > +void drm_writeback_connector_helper_add(struct
> > > +drm_writeback_connector *wb_connector,
> > > + const struct
> > drm_writeback_connector_helper_funcs *funcs) {
> > > + wb_connector->helper_private = funcs; }
> > > +
> > > /**
> > > * struct drm_plane_helper_funcs - helper operations for planes
> > > *
> > > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> > > index 149744dbeef0..77c3c64c132d 100644
> > > --- a/include/drm/drm_writeback.h
> > > +++ b/include/drm/drm_writeback.h
> > > @@ -84,6 +84,13 @@ struct drm_writeback_connector {
> > > * The name of the connector's fence timeline.
> > > */
> > > char timeline_name[32];
> > > +
> > > + /**
> > > + * @helper_private:
> > > + *
> > > + * helper private funcs for writeback_connector
> > > + */
> > > + const struct drm_writeback_connector_helper_funcs
> > *helper_private;
> > > };
> >
> > Unrelate to the commit? Also, where is this defined?
>
> This is very much related to this commit and defined on top right here in
> this commit.
Then please split this patch into two.
--
With best wishes
Dmitry
