Re: [PATCH v6 1/7] drm: writeback: Refactor drm_writeback_connector structure
From: John Harrison
Date: Tue Nov 03 2026 - 17:12:49 EST
On 11/3/26 00:30, Kandpal, Suraj wrote:
Yeah, the drmm route was proving too complex. Too many tangles too unravel all at once. It still seems like a better way to go, but that can wait until later. Getting something functional landed now and polishing it later is better than never getting anything merged at all.-----Original Message-----I think from the last revision of Dmitry's patch series we are currently not use drmm versions
From: John Harrison <John.Harrison@xxxxxxxxxx>
Sent: 18 July 2026 01:10
To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; freedreno@xxxxxxxxxxxxxxxxxxxxx;
dri-devel@xxxxxxxxxxxxxxxxxxxxx; kernel-list@xxxxxxxxxxxxxxx; amd-
gfx@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; intel-
xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Shankar, Uma <uma.shankar@xxxxxxxxx>; Murthy, Arun R
<arun.r.murthy@xxxxxxxxx>; abhinav.kumar@xxxxxxxxx;
tzimmermann@xxxxxxx; sean@xxxxxxxxx; marijn.suijten@xxxxxxxxxxxxxx;
laurent.pinchart+renesas@xxxxxxxxxxxxxxxx;
dave.stevenson@xxxxxxxxxxxxxxx; tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx;
kieran.bingham+renesas@xxxxxxxxxxxxxxxx; louis.chauvet@xxxxxxxxxxx;
kernel-dev@xxxxxxxxxx; Liviu Dudau <liviu.dudau@xxxxxxx>
Subject: Re: [PATCH v6 1/7] drm: writeback: Refactor drm_writeback_connector
structure
On 7/13/26 21:27, Suraj Kandpal wrote:
Some drivers cannot work with the current design where the connectoramdgpu_display_manager *dm,
is embedded within the drm_writeback_connector such as Intel and some
drivers that can get it working end up adding a lot of checks all
around the code to check if it's a writeback conenctor or not, this is
due to the limitation of inheritance in C.
To solve this move the drm_writeback_connector within the
drm_connector and remove the drm_connector base which was in
drm_writeback_connector. Make this drm_writeback_connector a union
with hdmi connector to save memory and since a connector can never be
both writeback and hdmi it should serve us well.
Do all other required modifications that come with these changes along
with addition of new function which returns the drm_connector when
drm_writeback_connector is present.
Modify drivers using the drm_writeback_connector to allow them to use
this connector without breaking them.
The drivers modified here are amd, komeda, mali, vc4, vkms, rcar_du,
msm
Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
Reviewed-by: Liviu Dudau <liviu.dudau@xxxxxxx>
Reviewed-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
Reviewed-by: John Harrison <John.Harrison@xxxxxxxxxx>
---
v5 -> v6:
- Rebase on latest kernel
v4 -> v5:
- Remove more unnecessary wrapping of drm_writeback_to_connector()
(John)
v3 -> v4:
- Use drm_writeback_to_connector() wherever possible (John)
- Do not wrap line where not needed (John)
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +-
.../display/amdgpu_dm/amdgpu_dm_connector.c | 4 +-
.../drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c | 8 +--
.../gpu/drm/arm/display/komeda/komeda_crtc.c | 6 +-
.../gpu/drm/arm/display/komeda/komeda_kms.h | 6 +-
.../arm/display/komeda/komeda_wb_connector.c | 8 +--
drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
drivers/gpu/drm/arm/malidp_drv.h | 2 +-
drivers/gpu/drm/arm/malidp_hw.c | 6 +-
drivers/gpu/drm/arm/malidp_mw.c | 8 +--
drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
drivers/gpu/drm/drm_writeback.c | 28 ++++----
.../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 3 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 14 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.h | 4 +-
.../gpu/drm/renesas/rcar-du/rcar_du_crtc.h | 6 +-
.../drm/renesas/rcar-du/rcar_du_writeback.c | 16 +++--
drivers/gpu/drm/vc4/vc4_txp.c | 12 ++--
drivers/gpu/drm/vkms/vkms_composer.c | 2 +-
drivers/gpu/drm/vkms/vkms_drv.h | 2 +-
drivers/gpu/drm/vkms/vkms_writeback.c | 13 ++--
include/drm/drm_connector.h | 69 +++++++++++++++++--
include/drm/drm_writeback.h | 66 +-----------------
24 files changed, 150 insertions(+), 141 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1726eb6a3c63..427a8a52d256 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4885,7 +4885,7 @@ static void dm_set_writeback(struct
struct drm_connector *connector,drm_connector_to_writeback(connector);
struct drm_connector_state *new_con_state)
{
- struct drm_writeback_connector *wb_conn =
+ struct drm_writeback_connector *wb_conn = &connector->writeback;*status,
struct amdgpu_device *adev = dm->adev;
struct amdgpu_crtc *acrtc;
struct dc_writeback_info *wb_info;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 91affbdb2d6c..a1faf9f3670e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -894,7 +894,7 @@ static inline void amdgpu_dm_set_mst_status(uint8_t
#define to_amdgpu_dm_connector(x) container_of(x, struct*connector,
amdgpu_dm_connector, base)
struct amdgpu_dm_wb_connector {
- struct drm_writeback_connector base;
+ struct drm_connector base;
struct dc_link *link;
};
diff --git
a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
index 88f938a8f24f..d48d764eae63 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
@@ -1368,11 +1368,9 @@ create_stream_for_sink(struct drm_connector
aconnector = to_amdgpu_dm_connector(connector);&amdgpu_dm_wb_conn_helper_funcs);
link = aconnector->dc_link;
} else {
- struct drm_writeback_connector *wbcon = NULL;
struct amdgpu_dm_wb_connector *dm_wbcon = NULL;
- wbcon = drm_connector_to_writeback(connector);
- dm_wbcon = to_amdgpu_dm_wb_connector(wbcon);
+ dm_wbcon = to_amdgpu_dm_wb_connector(connector);
link = dm_wbcon->link;
}
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 3ee31634549e..18ef4ce73b46 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
@@ -205,9 +205,9 @@ int amdgpu_dm_wb_connector_init(struct
amdgpu_display_manager *dm,
wbcon->link = link;
- drm_connector_helper_add(&wbcon->base.base,
+ drm_connector_helper_add(&wbcon->base,&amdgpu_dm_wb_connector_funcs,
+&amdgpu_dm_wb_conn_helper_funcs);
- res = drmm_writeback_connector_init(&dm->adev->ddev, &wbcon-
base,
+ res = drmm_writeback_connector_init(&dm->adev->ddev,
+&wbcon->base.writeback,
encoder,amdgpu_display_manager *dm,
amdgpu_dm_wb_formats,
@@ -219,8 +219,8 @@ int amdgpu_dm_wb_connector_init(struct
* Some of the properties below require access to state, like bpc.*kcrtc,
* Allocate some default initial connector state with our reset helper.
*/
- if (wbcon->base.base.funcs->reset)
- wbcon->base.base.funcs->reset(&wbcon->base.base);
+ if (wbcon->base.funcs->reset)
+ wbcon->base.funcs->reset(&wbcon->base);
return 0;
}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index e8cb782a6f8e..6611920c45fb 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -213,7 +213,7 @@ void komeda_crtc_handle_event(struct komeda_crtc
struct komeda_wb_connector *wb_conn = kcrtc->wb_conn;0);
if (wb_conn)
- drm_writeback_signal_completion(&wb_conn->base,
+ drm_writeback_signal_completion(&wb_conn-wb_connector.\n",
base.writeback, 0);
else
drm_warn(drm, "CRTC[%d]: EOW happen but no
drm_crtc_index(&kcrtc->base));conn_st);
@@ -269,9 +269,9 @@ komeda_crtc_do_flush(struct drm_crtc *crtc,
if (slave && has_bit(slave->id, kcrtc_st->affected_pipes))
komeda_pipeline_update(slave, old->state);
- conn_st = wb_conn ? wb_conn->base.base.state : NULL;
+ conn_st = wb_conn ? wb_conn->base.state : NULL;
if (conn_st && conn_st->writeback_job)
- drm_writeback_queue_job(&wb_conn->base, conn_st);
+ drm_writeback_queue_job(&wb_conn->base.writeback,
/* step 2: notify the HW to kickoff the update */NULL;
mdev->funcs->flush(mdev, master->id, kcrtc_st->active_pipes); diff
--git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index 83e61c4080c2..9c34302782c0 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -53,8 +53,8 @@ struct komeda_plane_state {
* struct komeda_wb_connector
*/
struct komeda_wb_connector {
- /** @base: &drm_writeback_connector */
- struct drm_writeback_connector base;
+ /** @base: &drm_connector */
+ struct drm_connector base;
/** @wb_layer: represents associated writeback pipeline of komeda */
struct komeda_layer *wb_layer;
@@ -139,7 +139,7 @@ struct komeda_kms_dev {
static inline bool is_writeback_only(struct drm_crtc_state *st)
{
struct komeda_wb_connector *wb_conn = to_kcrtc(st->crtc)-
wb_conn;
- struct drm_connector *conn = wb_conn ? &wb_conn->base.base :
+ struct drm_connector *conn = wb_conn ? &wb_conn->base : NULL;BIT(drm_connector_index(conn)));
return conn && (st->connector_mask ==
}drm_encoder *encoder,
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 c1bedbaa26f9..848ac37a76e4 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -53,7 +53,7 @@ komeda_wb_encoder_atomic_check(struct
return -EINVAL;@@ -183,9 +183,9 @@
}
- wb_layer = to_kconn(to_wb_conn(conn_st->connector))->wb_layer;
+ wb_layer = to_kconn(conn_st->connector)->wb_layer;
/*
* No need for a full modested when the only connector changed is
the @@ -151,7 +151,7 @@ static int komeda_wb_connector_add(struct
komeda_kms_dev *kms,
kwb_conn->wb_layer = kcrtc->master->wb_layer;
- wb_conn = &kwb_conn->base;
+ wb_conn = &kwb_conn->base.writeback;
formats = komeda_get_layer_fourcc_list(&mdev->fmt_tbl,
kwb_conn->wb_layer->layer_type,
static int komeda_wb_connector_add(struct komeda_kms_dev *kms,&komeda_wb_conn_helper_funcs);
return err;
}
- drm_connector_helper_add(&wb_conn->base,
+ drm_connector_helper_add(&kwb_conn->base,*crtc,
+&komeda_wb_conn_helper_funcs);
- info = &kwb_conn->base.base.display_info;
+ info = &kwb_conn->base.display_info;
info->bpc = __fls(kcrtc->master->improc->supported_color_depths);
info->color_formats =
kcrtc->master->improc->supported_color_formats;
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
b/drivers/gpu/drm/arm/malidp_crtc.c
index ebe8e1078777..4402c1de8c69 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -421,7 +421,7 @@ static int malidp_crtc_atomic_check(struct drm_crtc
u32 new_mask = crtc_state->connector_mask;*arg)
if ((old_mask ^ new_mask) ==
- (1 << drm_connector_index(&malidp->mw_connector.base)))
+ (1 << drm_connector_index(&malidp->mw_connector)))
crtc_state->connectors_changed = false;
}
diff --git a/drivers/gpu/drm/arm/malidp_drv.h
b/drivers/gpu/drm/arm/malidp_drv.h
index bc0387876dea..aa5599467d27 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -32,7 +32,7 @@ struct malidp_drm {
struct drm_device base;
struct malidp_hw_device *dev;
struct drm_crtc crtc;
- struct drm_writeback_connector mw_connector;
+ struct drm_connector mw_connector;
wait_queue_head_t wq;
struct drm_pending_vblank_event *event;
atomic_t config_valid;
diff --git a/drivers/gpu/drm/arm/malidp_hw.c
b/drivers/gpu/drm/arm/malidp_hw.c index 9b845d3f34e1..5a7bd27d3718
100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -1315,15 +1315,15 @@ static irqreturn_t malidp_se_irq(int irq, void
if (status & se->vsync_irq) {*/ diff --git
switch (hwdev->mw_state) {
case MW_ONESHOT:
- drm_writeback_signal_completion(&malidp-
mw_connector, 0);
+ drm_writeback_signal_completion(&malidp-
mw_connector.writeback,
+0);
break;
case MW_STOP:
- drm_writeback_signal_completion(&malidp-
mw_connector, 0);
+ drm_writeback_signal_completion(&malidp-
mw_connector.writeback,
+0);
/* disable writeback after stop */
hwdev->mw_state = MW_NOT_ENABLED;
break;
case MW_RESTART:
- drm_writeback_signal_completion(&malidp-
mw_connector, 0);
+ drm_writeback_signal_completion(&malidp-
mw_connector.writeback,
+0);
fallthrough; /* to a new start */
case MW_START:
/* writeback started, need to emulate one-shot mode
a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c*drm)
index a4c6a00c0941..4c77a459c241 100644
--- a/drivers/gpu/drm/arm/malidp_mw.c
+++ b/drivers/gpu/drm/arm/malidp_mw.c
@@ -212,7 +212,7 @@ int malidp_mw_connector_init(struct drm_device
if (!malidp->dev->hw->enable_memwrite)*drm,
return 0;
- drm_connector_helper_add(&malidp->mw_connector.base,
+ drm_connector_helper_add(&malidp->mw_connector,
&malidp_mw_connector_helper_funcs);
formats = get_writeback_formats(malidp, &n_formats); @@ -229,7
+229,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,
+ ret = drmm_writeback_connector_init(drm,
+&malidp->mw_connector.writeback,
&malidp_mw_connector_funcs,
encoder,
formats, n_formats);
@@ -244,8 +244,8 @@ void malidp_mw_atomic_commit(struct drm_device
struct drm_atomic_commit *old_state)*dev,
{
struct malidp_drm *malidp = drm_to_malidp(drm);
- struct drm_writeback_connector *mw_conn = &malidp-
mw_connector;
- struct drm_connector_state *conn_state = mw_conn->base.state;
+ struct drm_writeback_connector *mw_conn = &malidp-
mw_connector.writeback;
+ struct drm_connector_state *conn_state = malidp-
mw_connector.state;
struct malidp_hw_device *hwdev = malidp->dev;
struct malidp_mw_connector_state *mw_state;
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 1050dddadb17..a5c11832ff64 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1514,7 +1514,7 @@ static int prepare_signaling(struct drm_device
f[*num_fences].out_fence_ptr = fence_ptr;*drm_writeback_fence_get_driver_name(struct dma_fence *fence)
*fence_state = f;
- wb_conn = drm_connector_to_writeback(conn);
+ wb_conn = &conn->writeback;
fence = drm_writeback_get_out_fence(wb_conn);
if (!fence)
return -ENOMEM;
diff --git a/drivers/gpu/drm/drm_writeback.c
b/drivers/gpu/drm/drm_writeback.c index 6ba3833a93ca..f93c936643bb
100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -95,8 +95,9 @@ static const char
{drm_device *dev,
struct drm_writeback_connector *wb_connector =
fence_to_wb_connector(fence);
+ struct drm_connector *connector =
+drm_writeback_to_connector(wb_connector);
- return wb_connector->base.dev->driver->name;
+ return connector->dev->driver->name;
}
static const char *
@@ -193,7 +194,7 @@ static int __drm_writeback_connector_init(struct
struct drm_encoder *enc, const u32*formats,
int n_formats)FYI: I missed this before but drm_writeback_connector_init needs to be
{
- struct drm_connector *connector = &wb_connector->base;
+ struct drm_connector *connector =
+drm_writeback_to_connector(wb_connector);
struct drm_mode_config *config = &dev->mode_config;
struct drm_property_blob *blob;
int ret = create_writeback_properties(dev); @@ -275,7 +276,7 @@ int
drm_writeback_connector_init(struct drm_device *dev,
struct drm_encoder *enc,
const u32 *formats, int n_formats)
{
- struct drm_connector *connector = &wb_connector->base;
+ struct drm_connector *connector =
+drm_writeback_to_connector(wb_connector);
removed. The whole point of Dmitry's patch set is to switch to drmm instead of
drm init/alloc functions. I don't think it really affects this patch set much. But I
believe the last version of the i915/Xe writeback support was still using the
manual memory management interface via this function. That will need
updating to use the drmm version.
At least for now
Regards,
Suraj Kandpal
John.