> -----Original Message----- > From: John Harrison <[email protected]> > Sent: Monday, May 4, 2026 11:15 PM > To: Kandpal, Suraj <[email protected]>; > [email protected]; [email protected]; kernel- > [email protected]; [email protected]; linux- > [email protected]; [email protected]; intel- > [email protected] > Cc: Nautiyal, Ankit K <[email protected]>; Shankar, Uma > <[email protected]>; [email protected]; Murthy, > Arun R <[email protected]>; Nikula, Jani <[email protected]>; > [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; > [email protected]; > [email protected]; > [email protected]; [email protected] > Subject: Re: [v3,2/7] drm: writeback: Modify writeback init helpers > > On 3/16/26 01:30, Suraj Kandpal wrote: > > Now with drm_writeback_connector moved to drm_connector it makes > more > > sense use drm_connector as an argument rather than > > drm_writeback_connector. The writeback connector can easily be derived > > from drm_connector.
Hi John First of all thanks for helping to move this series forward. > So this patch and all five subsequent patches are basically the same search > and replace of base_conn->wb_conn to base_conn in the DRM level helper > functions, yes? I would add a little more explanation of why "it makes more > sense". Something like: "Some of the writeback helper functions require > access to the parent drm_connector object as well as the > drm_writeback_connector object itself. So, pass in the top level object and > traverse down rather than passing in the lower level object and traversing > back up. Even where such is not the case, update to use the top level object > for consistency across the interface." Sure will update the commit message. > > Also, there could be better consistency across these 'modify' patches. > First, the subject of patches 1-5 should be 'drm/writeback: ...' not > 'drm: writeback: ...'. Then you have 'modify XXX helpers', 'modify XXX params' > and 'modify params for XXX'. Sure will keep the subject consistent > It would be cleaner to pick a single variant and > use that for all the patches. Lastly, are the final two patches really > 'drm/connector:'? The header file with the function declarations being > updated is drm_modeset_helper_vtables.h. Which would make the prefix > 'drm/modeset'? Although, given that the declarations are specific to > writeback support, I would just stick with 'drm/writeback' > for all seven patches. Sure will update the prefix for last two patches as well. Although in regard to drm/writeback after grepping the git log it seems that drm: writeback: is the correct prefix actually the correct wording would be more prevalent prefix. So I would like to keep that the consistently across all my patches as well. Regards, Suraj Kandpal > > John. > > > > > Signed-off-by: Suraj Kandpal <[email protected]> > > --- > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c | 2 +- > > .../drm/arm/display/komeda/komeda_wb_connector.c | 5 +---- > > drivers/gpu/drm/arm/malidp_mw.c | 2 +- > > drivers/gpu/drm/drm_writeback.c | 14 ++++++-------- > > drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 2 +- > > .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c | 3 +-- > > drivers/gpu/drm/vc4/vc4_txp.c | 2 +- > > drivers/gpu/drm/vkms/vkms_writeback.c | 4 ++-- > > include/drm/drm_writeback.h | 4 ++-- > > 9 files changed, 16 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c > > index 8fea29720989..84a9c1d2bd8e 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c > > @@ -204,7 +204,7 @@ int amdgpu_dm_wb_connector_init(struct > > amdgpu_display_manager *dm, > > > > drm_connector_helper_add(&wbcon->base, > > &amdgpu_dm_wb_conn_helper_funcs); > > > > - res = drmm_writeback_connector_init(&dm->adev->ddev, &wbcon- > >base.writeback, > > + res = drmm_writeback_connector_init(&dm->adev->ddev, &wbcon- > >base, > > > &amdgpu_dm_wb_connector_funcs, > > encoder, > > amdgpu_dm_wb_formats, > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c > > b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c > > index fa2f63c142cd..85b34375d275 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c > > @@ -135,7 +135,6 @@ static int komeda_wb_connector_add(struct > komeda_kms_dev *kms, > > { > > struct komeda_dev *mdev = kms->base.dev_private; > > struct komeda_wb_connector *kwb_conn; > > - struct drm_writeback_connector *wb_conn; > > struct drm_display_info *info; > > struct drm_encoder *encoder; > > > > @@ -151,8 +150,6 @@ static int komeda_wb_connector_add(struct > > komeda_kms_dev *kms, > > > > kwb_conn->wb_layer = kcrtc->master->wb_layer; > > > > - wb_conn = &kwb_conn->base.writeback; > > - > > formats = komeda_get_layer_fourcc_list(&mdev->fmt_tbl, > > kwb_conn->wb_layer- > >layer_type, > > &n_formats); > > @@ -170,7 +167,7 @@ static int komeda_wb_connector_add(struct > > komeda_kms_dev *kms, > > > > encoder->possible_crtcs = drm_crtc_mask(&kcrtc->base); > > > > - err = drmm_writeback_connector_init(&kms->base, wb_conn, > > + err = drmm_writeback_connector_init(&kms->base, &kwb_conn- > >base, > > &komeda_wb_connector_funcs, > > encoder, > > formats, n_formats); > > diff --git a/drivers/gpu/drm/arm/malidp_mw.c > > b/drivers/gpu/drm/arm/malidp_mw.c index 472598b3e007..7d42b007ef19 > > 100644 > > --- a/drivers/gpu/drm/arm/malidp_mw.c > > +++ b/drivers/gpu/drm/arm/malidp_mw.c > > @@ -228,7 +228,7 @@ int malidp_mw_connector_init(struct drm_device > > *drm) > > > > encoder->possible_crtcs = drm_crtc_mask(&malidp->crtc); > > > > - ret = drmm_writeback_connector_init(drm, &malidp- > >mw_connector.writeback, > > + ret = drmm_writeback_connector_init(drm, &malidp->mw_connector, > > &malidp_mw_connector_funcs, > > encoder, > > formats, n_formats); > > diff --git a/drivers/gpu/drm/drm_writeback.c > > b/drivers/gpu/drm/drm_writeback.c index 7bf9f6374712..9a3037d11009 > > 100644 > > --- a/drivers/gpu/drm/drm_writeback.c > > +++ b/drivers/gpu/drm/drm_writeback.c > > @@ -242,7 +242,7 @@ static int __drm_writeback_connector_init(struct > drm_device *dev, > > * a custom encoder > > * > > * @dev: DRM device > > - * @wb_connector: Writeback connector to initialize > > + * @connector: Drm connector which contains the writeback connector > > + to initialize > > * @enc: handle to the already initialized drm encoder > > * @con_funcs: Connector funcs vtable > > * @formats: Array of supported pixel formats for the writeback > > engine @@ -267,13 +267,12 @@ static int > __drm_writeback_connector_init(struct drm_device *dev, > > * Returns: 0 on success, or a negative error code > > */ > > int drm_writeback_connector_init(struct drm_device *dev, > > - struct drm_writeback_connector > *wb_connector, > > + struct drm_connector *connector, > > const struct drm_connector_funcs > *con_funcs, > > struct drm_encoder *enc, > > const u32 *formats, int n_formats) > > { > > - struct drm_connector *connector = > > - drm_writeback_to_connector(wb_connector); > > + struct drm_writeback_connector *wb_connector = > > +&connector->writeback; > > int ret; > > > > ret = drm_connector_init(dev, connector, con_funcs, @@ -322,7 > > +321,7 @@ static void drm_writeback_connector_cleanup(struct > drm_device *dev, > > * a custom encoder > > * > > * @dev: DRM device > > - * @wb_connector: Writeback connector to initialize > > + * @connector: Drm connector containing the writeback connector to > > + initialize > > * @con_funcs: Connector funcs vtable > > * @enc: Encoder to connect this writeback connector > > * @formats: Array of supported pixel formats for the writeback > > engine @@ -338,13 +337,12 @@ static void > drm_writeback_connector_cleanup(struct drm_device *dev, > > * Returns: 0 on success, or a negative error code > > */ > > int drmm_writeback_connector_init(struct drm_device *dev, > > - struct drm_writeback_connector > *wb_connector, > > + struct drm_connector *connector, > > const struct drm_connector_funcs > *con_funcs, > > struct drm_encoder *enc, > > const u32 *formats, int n_formats) > > { > > - struct drm_connector *connector = > > - drm_writeback_to_connector(wb_connector); > > + struct drm_writeback_connector *wb_connector = > > +&connector->writeback; > > int ret; > > > > ret = drmm_connector_init(dev, connector, con_funcs, diff --git > > a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c > > index 930ba1ad777b..d4fc28951085 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c > > @@ -136,7 +136,7 @@ int dpu_writeback_init(struct drm_device *dev, > > struct drm_encoder *enc, > > > > drm_connector_helper_add(&dpu_wb_conn->base, > > &dpu_wb_conn_helper_funcs); > > > > - rc = drmm_writeback_connector_init(dev, &dpu_wb_conn- > >base.writeback, > > + rc = drmm_writeback_connector_init(dev, &dpu_wb_conn->base, > > &dpu_wb_conn_funcs, enc, > > format_list, num_formats); > > > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c > > b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c > > index cd09e0fbb030..1de8865fb751 100644 > > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c > > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_writeback.c > > @@ -203,7 +203,6 @@ static const u32 writeback_formats[] = { > > int rcar_du_writeback_init(struct rcar_du_device *rcdu, > > struct rcar_du_crtc *rcrtc) > > { > > - struct drm_writeback_connector *wb_conn = &rcrtc- > >writeback.writeback; > > struct drm_encoder *encoder; > > > > encoder = drmm_plain_encoder_alloc(&rcdu->ddev, NULL, @@ - > 218,7 > > +217,7 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu, > > drm_connector_helper_add(&rcrtc->writeback, > > &rcar_du_wb_conn_helper_funcs); > > > > - return drmm_writeback_connector_init(&rcdu->ddev, wb_conn, > > + return drmm_writeback_connector_init(&rcdu->ddev, &rcrtc- > >writeback, > > &rcar_du_wb_conn_funcs, > > encoder, > > writeback_formats, > > diff --git a/drivers/gpu/drm/vc4/vc4_txp.c > > b/drivers/gpu/drm/vc4/vc4_txp.c index de3db0834011..d08271142116 > > 100644 > > --- a/drivers/gpu/drm/vc4/vc4_txp.c > > +++ b/drivers/gpu/drm/vc4/vc4_txp.c > > @@ -601,7 +601,7 @@ static int vc4_txp_bind(struct device *dev, struct > > device *master, void *data) > > > > drm_connector_helper_add(&txp->connector, > > &vc4_txp_connector_helper_funcs); > > - ret = drmm_writeback_connector_init(drm, &txp- > >connector.writeback, > > + ret = drmm_writeback_connector_init(drm, &txp->connector, > > &vc4_txp_connector_funcs, > > encoder, > > drm_fmts, ARRAY_SIZE(drm_fmts)); > diff --git > > a/drivers/gpu/drm/vkms/vkms_writeback.c > > b/drivers/gpu/drm/vkms/vkms_writeback.c > > index cadb4cb372c5..b368c569cf0a 100644 > > --- a/drivers/gpu/drm/vkms/vkms_writeback.c > > +++ b/drivers/gpu/drm/vkms/vkms_writeback.c > > @@ -170,7 +170,6 @@ static const struct drm_connector_helper_funcs > vkms_wb_conn_helper_funcs = { > > int vkms_enable_writeback_connector(struct vkms_device *vkmsdev, > > struct vkms_output *vkms_output) > > { > > - struct drm_writeback_connector *wb = &vkms_output- > >wb_connector.writeback; > > int ret; > > > > ret = drmm_encoder_init(&vkmsdev->drm, &vkms_output- > >wb_encoder, @@ > > -183,7 +182,8 @@ int vkms_enable_writeback_connector(struct > > vkms_device *vkmsdev, > > > > drm_connector_helper_add(&vkms_output->wb_connector, > > &vkms_wb_conn_helper_funcs); > > > > - return drmm_writeback_connector_init(&vkmsdev->drm, wb, > > + return drmm_writeback_connector_init(&vkmsdev->drm, > > + &vkms_output->wb_connector, > > &vkms_wb_connector_funcs, > > &vkms_output->wb_encoder, > > vkms_wb_formats, > > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > > index 702141099520..c6960c7e634e 100644 > > --- a/include/drm/drm_writeback.h > > +++ b/include/drm/drm_writeback.h > > @@ -78,13 +78,13 @@ drm_writeback_to_connector(struct > drm_writeback_connector *wb_connector) > > } > > > > int drm_writeback_connector_init(struct drm_device *dev, > > - struct drm_writeback_connector > *wb_connector, > > + struct drm_connector *connector, > > const struct drm_connector_funcs > *con_funcs, > > struct drm_encoder *enc, > > const u32 *formats, int n_formats); > > > > int drmm_writeback_connector_init(struct drm_device *dev, > > - struct drm_writeback_connector > *wb_connector, > > + struct drm_connector *connector, > > const struct drm_connector_funcs > *con_funcs, > > struct drm_encoder *enc, > > const u32 *formats, int n_formats);
