[PATCH] drivers: drm: remove drm_simple_encoder_init then inline with drm_encoder_init

[PATCH] drivers: drm: remove drm_simple_encoder_init then inline with drm_encoder_init

[junsungpark774]

From: Park Junsung <junsungpark774@gmail.com>

remove drm_simple_encoder_init which is noted as a unnecessary
intermediate layer in the TODO list
and inline it into drm_encoder_init.

to achieve this,
declare and define drm_encoder_funcs with the static keyword.

has been verified by module compilation with no errors on modified area.

Signed-off-by: Park Junsung <junsungpark774@gmail.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index a026bd35ef48..52d9bb46ea95 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -44,6 +44,10 @@
 #include "qxl_drv.h"
 #include "qxl_object.h"
 
+static const struct drm_encoder_funcs drm_encoder_funcs_cleanup = {
+	.destroy = drm_encoder_cleanup,
+};
+
 static bool qxl_head_enabled(struct qxl_head *head)
 {
 	return head->width && head->height;
@@ -1169,10 +1173,11 @@ static int qdev_output_init(struct drm_device *dev, int num_output)
 	drm_connector_init(dev, &qxl_output->base,
 			   &qxl_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);
 
-	ret = drm_simple_encoder_init(dev, &qxl_output->enc,
-				      DRM_MODE_ENCODER_VIRTUAL);
+	ret = drm_encoder_init(dev, &qxl_output->enc,
+			       &drm_encoder_funcs_cleanup,
+			       DRM_MODE_ENCODER_VIRTUAL, NULL);
 	if (ret) {
-		drm_err(dev, "drm_simple_encoder_init() failed, error %d\n",
+		drm_err(dev, "drm_encoder_init() failed, error %d\n",
 			ret);
 		goto err_drm_connector_cleanup;
 	}
-- 
2.53.0

Re: [PATCH] drivers: drm: remove drm_simple_encoder_init then inline with drm_encoder_init

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of drm_connector_init() is ignored, leading to potential use of a partially initialized connector object on failure.
--

--- Patch [1]: [PATCH] drivers: drm: remove drm_simple_encoder_init then inline with drm_encoder_init ---
commit fb41dbd1014b2770b1e9b17f944e26382c1aa83c
Author: Park Junsung <junsungpark774@gmail.com>

drivers: drm: remove drm_simple_encoder_init then inline with drm_encoder_init

This commit removes the unnecessary intermediate layer drm_simple_encoder_init
and inlines it directly into drm_encoder_init. It also introduces a static
drm_encoder_funcs struct to handle encoder cleanup.

> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index a026bd35ef485..52d9bb46ea959 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
[ ... ]
> @@ -1169,10 +1173,11 @@ static int qdev_output_init(struct drm_device *dev, int num_output)
>  	drm_connector_init(dev, &qxl_output->base,
>  			   &qxl_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);
>  

[Severity: High]
This is a pre-existing issue, but is it safe to ignore the return value of
drm_connector_init() here in qdev_output_init()? 

If drm_connector_init() fails, for example due to an IDA allocation failure
under memory pressure, and the subsequent call to drm_encoder_init() succeeds,
could this result in using a partially initialized connector object? It seems
this might lead to undefined behavior or a kernel panic when the connector is
later accessed by the DRM core.

> -	ret = drm_simple_encoder_init(dev, &qxl_output->enc,
> -				      DRM_MODE_ENCODER_VIRTUAL);
> +	ret = drm_encoder_init(dev, &qxl_output->enc,
> +			       &drm_encoder_funcs_cleanup,
> +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>  	if (ret) {
> -		drm_err(dev, "drm_simple_encoder_init() failed, error %d\n",
> +		drm_err(dev, "drm_encoder_init() failed, error %d\n",
>  			ret);
>  		goto err_drm_connector_cleanup;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730084908.109270-1-junsungpark774@gmail.com?part=1