linux-kernel - Re: [PATCH 0/8] drm: writeback: clean up writeback connector initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aJDHAF69VOEHwcKO@e110455-lin.cambridge.arm.com>
Date: Mon, 4 Aug 2025 15:43:12 +0100
From: Liviu Dudau <liviu.dudau@....com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Jani Nikula <jani.nikula@...ux.intel.com>,
	"Kandpal, Suraj" <suraj.kandpal@...el.com>,
	Harry Wentland <harry.wentland@....com>,
	Leo Li <sunpeng.li@....com>, Rodrigo Siqueira <siqueira@...lia.com>,
	Alex Deucher <alexander.deucher@....com>,
	Christian König <christian.koenig@....com>,
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	Rob Clark <robin.clark@....qualcomm.com>,
	Dmitry Baryshkov <lumag@...nel.org>,
	Abhinav Kumar <abhinav.kumar@...ux.dev>,
	Jessica Zhang <jessica.zhang@....qualcomm.com>,
	Sean Paul <sean@...rly.run>,
	Marijn Suijten <marijn.suijten@...ainline.org>,
	Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
	Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>,
	Kieran Bingham <kieran.bingham+renesas@...asonboard.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Magnus Damm <magnus.damm@...il.com>,
	Dave Stevenson <dave.stevenson@...pberrypi.com>,
	Maíra Canal <mcanal@...lia.com>,
	Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>,
	amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	freedreno@...ts.freedesktop.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH 0/8] drm: writeback: clean up writeback connector
 initialization

Hi,

On Fri, Aug 01, 2025 at 04:51:08PM +0300, Dmitry Baryshkov wrote:
> Drivers using drm_writeback_connector_init() / _with_encoder() don't
> perform cleanup in a manner similar to drmm_writeback_connector_init()
> (see drm_writeback_connector_cleanup()). Migrate all existing drivers
> to use drmm_writeback_connector_init(), drop
> drm_writeback_connector_init() and drm_writeback_connector::encoder
> (it's unused afterwards).
> 
> This series leaves former drm_writeback_connector_init_with_encoder()
> (renamed to drm_writeback_connector_init as a non-managed counterpart
> for drmm_writeback_connector_init()). It is supposed to be used by
> drivers which can not use drmm functions (like Intel). However I think
> it would be better to drop it completely.

The intent of _init_with_encoder() was to be a special case for drivers
that use their own specific encoder and the rest use the generic function
that creates the virtual encoder inside the call. The API for
_init_with_encoder() was actually introduced 4 years after the original
patch, so that should give a hint.

drmm_writeback_connector_init() is more like _init_with_encoder() and
I don't remember reviewing it, so I'm not sure why that was considered
to be the better behaviour for the managed version. Now you're moving
all the drivers to the managed version and you have to duplicate code
in each driver to create the ENCODER_VIRTUAL encoder.

I'm not against the changes being made in the series, I just want to
see a better justification on why _init_with_encoder() behaviour is
better than the previous default that you're removing.

Best regards,
Liviu


> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
> ---
> Dmitry Baryshkov (8):
>       drm/amd/display: use drmm_writeback_connector_init()
>       drm/komeda: use drmm_writeback_connector_init()
>       drm/mali: use drmm_writeback_connector_init()
>       drm/msm/dpu: use drmm_writeback_connector_init()
>       drm/msm/dpu: use drmm_writeback_connector_init()
>       drm/vc4: use drmm_writeback_connector_init()
>       drm: writeback: drop excess connector initialization functions
>       drm: writeback: rename drm_writeback_connector_init_with_encoder()
> 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  2 +-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c   | 18 ++++--
>  .../drm/arm/display/komeda/komeda_wb_connector.c   | 30 ++++++----
>  drivers/gpu/drm/arm/malidp_mw.c                    | 25 ++++----
>  drivers/gpu/drm/drm_writeback.c                    | 69 +++-------------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c      | 10 +---
>  .../gpu/drm/renesas/rcar-du/rcar_du_writeback.c    | 23 +++++---
>  drivers/gpu/drm/vc4/vc4_txp.c                      |  9 ++-
>  include/drm/drm_writeback.h                        | 22 +------
>  9 files changed, 77 insertions(+), 131 deletions(-)
> ---
> base-commit: 94f208ff622b09309358abaf26d7acca0c318fae
> change-id: 20250801-wb-drop-encoder-97a0c75bd5d7
> 
> Best regards,
> -- 
> With best wishes
> Dmitry
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯