Re: [PATCH v7 8/9] drm: writeback: clean up writeback connectors in drm_connector_cleanup()

[Dmitry Baryshkov]

On Wed, Oct 21, 2026 at 11:22:48AM -0700, John Harrison wrote:
> On 10/19/26 06:12, Dmitry Baryshkov wrote:
> > Only drmm_writeback_connector_init() registers
> > drm_writeback_connector_cleanup(). Connectors set up with
> > drm_writeback_connector_init() (amdgpu, komeda, malidp, rcar-du, vc4)
> > never drop the formats blob or free the jobs still on the queue, leaking
> > them together with their framebuffer and fence references.
> > 
> > Call drm_writeback_connector_cleanup() from drm_connector_cleanup() for
> > writeback connectors, the same way the HDMI audio and SCDC state is torn
> > down there, so it runs for both kinds of initialization. The drmm action
> > now just wraps it. Clearing the blob pointer makes the second invocation
> > a no-op and also covers drm_connector_cleanup() being called after a
> > failed __drm_writeback_connector_init().
> > 
> > Fixes: 935774cd71fe ("drm: Add writeback connector type")
> > Assisted-by: LLM
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/drm_connector.c     |  3 +++
> >   drivers/gpu/drm/drm_crtc_internal.h |  3 +++
> >   drivers/gpu/drm/drm_writeback.c     | 30 +++++++++++++++++++++++-------
> >   3 files changed, 29 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 893716a9e489..1c2bef5e0636 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -929,6 +929,9 @@ void drm_connector_cleanup(struct drm_connector *connector)
> >   	if (connector->hdmi.scdc_work_initialized)
> >   		cancel_delayed_work_sync(&connector->hdmi.scdc_work);
> > +	if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
> > +		drm_writeback_connector_cleanup(connector);
> > +
> Could this be done via a new connector->funcs->cleanup() vfunc or something?
> Set that inside the writeback init path and it is guaranteed to then only be
> called for objects that explicitly requested it. That would be more robust
> and would keep sashiko happy. It would also keep a writeback connector
> specific declaration out of the crtc_internal header.

I'd rather not do it. These are the generic properties of every
writeback connector. The answer to Sahiko would be that yes, currently
all DRM_MODE_CONNECTOR_WRITEBACK use drm_writeback_connector. Once that
is changed for the Intel driver, the condition might/would change here.


-- 
With best wishes
Dmitry